Fix OneDrive details list flag validation (#1232)

## Description

Call the OneDrive flag validation function to ensure flags that take time strings are formatted properly. Also add tests to ensure there's no future regressions for this

First commit has important code changes. Remainder of commits have test data/setup

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [x] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🐹 Trivial/Minor

## Issue(s)

* closes #1231 
* #913 

## Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

src/cli/backup/onedrive.go

@@ -350,6 +350,10 @@ func runDetailsOneDriveCmd(
 	backupID string,
 	opts utils.OneDriveOpts,
 ) (*details.Details, error) {
+	if err := utils.ValidateOneDriveRestoreFlags(backupID, opts); err != nil {
+		return nil, err
+	}
+
 	d, _, err := r.BackupDetails(ctx, backupID)
 	if err != nil {
 		if errors.Is(err, kopia.ErrNotFound) {

src/cli/backup/onedrive_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/alcionai/corso/src/cli/utils/testdata"
 	"github.com/alcionai/corso/src/internal/tester"
 )
 
@@ -86,3 +87,41 @@ func (suite *OneDriveSuite) TestValidateOneDriveBackupCreateFlags() {
 		})
 	}
 }
+
+func (suite *OneDriveSuite) TestOneDriveBackupDetailsSelectors() {
+	ctx, flush := tester.NewContext()
+	defer flush()
+
+	for _, test := range testdata.OneDriveOptionDetailLookups {
+		suite.T().Run(test.Name, func(t *testing.T) {
+			output, err := runDetailsOneDriveCmd(
+				ctx,
+				test.BackupGetter,
+				"backup-ID",
+				test.Opts,
+			)
+			assert.NoError(t, err)
+
+			assert.ElementsMatch(t, test.Expected, output.Entries)
+		})
+	}
+}
+
+func (suite *OneDriveSuite) TestOneDriveBackupDetailsSelectorsBadFormats() {
+	ctx, flush := tester.NewContext()
+	defer flush()
+
+	for _, test := range testdata.BadOneDriveOptionsFormats {
+		suite.T().Run(test.Name, func(t *testing.T) {
+			output, err := runDetailsOneDriveCmd(
+				ctx,
+				test.BackupGetter,
+				"backup-ID",
+				test.Opts,
+			)
+
+			assert.Error(t, err)
+			assert.Empty(t, output)
+		})
+	}
+}

src/cli/utils/onedrive.go

@@ -47,7 +47,7 @@ func ValidateOneDriveRestoreFlags(backupID string, opts OneDriveOpts) error {
 		return errors.New("invalid time format for modified-after")
 	}
 
-	if _, ok := opts.Populated[FileModifiedAfterFN]; ok && !IsValidTimeFormat(opts.FileModifiedBefore) {
+	if _, ok := opts.Populated[FileModifiedBeforeFN]; ok && !IsValidTimeFormat(opts.FileModifiedBefore) {
 		return errors.New("invalid time format for modified-before")
 	}
 

src/cli/utils/testdata/opts.go

@@ -252,6 +252,148 @@ var (
 	}
 )
 
+type OneDriveOptionsTest struct {
+	Name         string
+	Opts         utils.OneDriveOpts
+	BackupGetter *MockBackupGetter
+	Expected     []details.DetailsEntry
+}
+
+var (
+	// BadOneDriveOptionsFormats contains OneDriveOpts with flags that should
+	// cause errors about the format of the input flag. Mocks are configured to
+	// allow the system to run if it doesn't throw an error on formatting.
+	BadOneDriveOptionsFormats = []OneDriveOptionsTest{
+		{
+			Name: "BadFileCreatedAfter",
+			Opts: utils.OneDriveOpts{
+				FileCreatedAfter: "foo",
+				Populated: utils.PopulatedFlags{
+					utils.FileCreatedAfterFN: struct{}{},
+				},
+			},
+		},
+		{
+			Name: "EmptyFileCreatedAfter",
+			Opts: utils.OneDriveOpts{
+				FileCreatedAfter: "",
+				Populated: utils.PopulatedFlags{
+					utils.FileCreatedAfterFN: struct{}{},
+				},
+			},
+		},
+		{
+			Name: "BadFileCreatedBefore",
+			Opts: utils.OneDriveOpts{
+				FileCreatedBefore: "foo",
+				Populated: utils.PopulatedFlags{
+					utils.FileCreatedBeforeFN: struct{}{},
+				},
+			},
+		},
+		{
+			Name: "EmptyFileCreatedBefore",
+			Opts: utils.OneDriveOpts{
+				FileCreatedBefore: "",
+				Populated: utils.PopulatedFlags{
+					utils.FileCreatedBeforeFN: struct{}{},
+				},
+			},
+		},
+		{
+			Name: "BadFileModifiedAfter",
+			Opts: utils.OneDriveOpts{
+				FileModifiedAfter: "foo",
+				Populated: utils.PopulatedFlags{
+					utils.FileModifiedAfterFN: struct{}{},
+				},
+			},
+		},
+		{
+			Name: "EmptyFileModifiedAfter",
+			Opts: utils.OneDriveOpts{
+				FileModifiedAfter: "",
+				Populated: utils.PopulatedFlags{
+					utils.FileModifiedAfterFN: struct{}{},
+				},
+			},
+		},
+		{
+			Name: "BadFileModifiedBefore",
+			Opts: utils.OneDriveOpts{
+				FileModifiedBefore: "foo",
+				Populated: utils.PopulatedFlags{
+					utils.FileModifiedBeforeFN: struct{}{},
+				},
+			},
+		},
+		{
+			Name: "EmptyFileModifiedBefore",
+			Opts: utils.OneDriveOpts{
+				FileModifiedBefore: "",
+				Populated: utils.PopulatedFlags{
+					utils.FileModifiedBeforeFN: struct{}{},
+				},
+			},
+		},
+	}
+
+	// OneDriveOptionDetailLookups contains flag inputs and expected results for
+	// some choice input patterns. This set is not exhaustive. All inputs and
+	// outputs are according to the data laid out in selectors/testdata. Mocks are
+	// configured to return the full dataset listed in selectors/testdata.
+	OneDriveOptionDetailLookups = []OneDriveOptionsTest{
+		{
+			Name:     "AllFiles",
+			Expected: testdata.OneDriveItems,
+			Opts: utils.OneDriveOpts{
+				Paths: selectors.Any(),
+			},
+		},
+		{
+			Name:     "FolderPrefixMatch",
+			Expected: testdata.OneDriveItems,
+			Opts: utils.OneDriveOpts{
+				Paths: []string{testdata.OneDriveFolderFolder},
+			},
+		},
+		{
+			Name:     "FolderPrefixMatchTrailingSlash",
+			Expected: testdata.OneDriveItems,
+			Opts: utils.OneDriveOpts{
+				Paths: []string{testdata.OneDriveFolderFolder + "/"},
+			},
+		},
+		{
+			Name:     "FolderPrefixMatchTrailingSlash",
+			Expected: testdata.OneDriveItems,
+			Opts: utils.OneDriveOpts{
+				Paths: []string{testdata.OneDriveFolderFolder + "/"},
+			},
+		},
+		{
+			Name: "ShortRef",
+			Expected: []details.DetailsEntry{
+				testdata.OneDriveItems[0],
+				testdata.OneDriveItems[1],
+			},
+			Opts: utils.OneDriveOpts{
+				Names: []string{
+					testdata.OneDriveItems[0].ShortRef,
+					testdata.OneDriveItems[1].ShortRef,
+				},
+			},
+		},
+		{
+			Name:     "CreatedBefore",
+			Expected: []details.DetailsEntry{testdata.OneDriveItems[1]},
+			Opts: utils.OneDriveOpts{
+				FileCreatedBefore: common.FormatTime(testdata.Time1.Add(time.Second)),
+			},
+		},
+	}
+)
+
 // MockBackupGetter implements the repo.BackupGetter interface and returns
 // (selectors/testdata.GetDetailsSet(), nil, nil) when BackupDetails is called
 // on the nil instance. If an instance is given or Backups is called returns an

src/pkg/selectors/testdata/details.go

@@ -42,6 +42,8 @@ const (
 var (
 	Time1 = time.Date(2022, 9, 21, 10, 0, 0, 0, time.UTC)
 	Time2 = time.Date(2022, 10, 21, 10, 0, 0, 0, time.UTC)
+	Time3 = time.Date(2023, 9, 21, 10, 0, 0, 0, time.UTC)
+	Time4 = time.Date(2023, 10, 21, 10, 0, 0, 0, time.UTC)
 
 	ExchangeEmailInboxPath = mustParsePath("tenant-id/exchange/user-id/email/Inbox", false)
 	ExchangeEmailBasePath  = mustAppendPath(ExchangeEmailInboxPath, "subfolder", false)
@@ -161,9 +163,18 @@ var (
 		},
 	}
 
-	OneDriveBasePath  = mustParsePath("tenant-id/onedrive/user-id/files/folder/subfolder", false)
-	OneDriveItemPath1 = mustAppendPath(OneDriveBasePath, ItemName1, true)
-	OneDriveItemPath2 = mustAppendPath(OneDriveBasePath, ItemName2, true)
+	OneDriveRootPath   = mustParsePath("tenant-id/onedrive/user-id/files/drives/foo/root:", false)
+	OneDriveFolderPath = mustAppendPath(OneDriveRootPath, "folder", false)
+	OneDriveBasePath1  = mustAppendPath(OneDriveFolderPath, "a", false)
+	OneDriveBasePath2  = mustAppendPath(OneDriveFolderPath, "b", false)
+
+	OneDriveItemPath1 = mustAppendPath(OneDriveFolderPath, ItemName1, true)
+	OneDriveItemPath2 = mustAppendPath(OneDriveBasePath1, ItemName2, true)
+	OneDriveItemPath3 = mustAppendPath(OneDriveBasePath2, ItemName3, true)
+
+	OneDriveFolderFolder  = stdpath.Join(OneDriveFolderPath.Folders()[3:]...)
+	OneDriveParentFolder1 = stdpath.Join(OneDriveBasePath1.Folders()[3:]...)
+	OneDriveParentFolder2 = stdpath.Join(OneDriveBasePath2.Folders()[3:]...)
 
 	OneDriveItems = []details.DetailsEntry{
 		{
@@ -172,18 +183,12 @@ var (
 			ParentRef: OneDriveItemPath1.ToBuilder().Dir().ShortRef(),
 			ItemInfo: details.ItemInfo{
 				OneDrive: &details.OneDriveInfo{
-					ItemType: details.OneDriveItem,
-					ParentPath: stdpath.Join(
-						append(
-							[]string{
-								"drives",
-								"foo",
-								"root:",
-							},
-							OneDriveItemPath1.Folders()...,
-						)...,
-					),
-					ItemName: OneDriveItemPath1.Item() + "name",
+					ItemType:   details.OneDriveItem,
+					ParentPath: OneDriveFolderFolder,
+					ItemName:   OneDriveItemPath1.Item() + "name",
+					Size:       int64(23),
+					Created:    Time2,
+					Modified:   Time4,
 				},
 			},
 		},
@@ -193,18 +198,27 @@ var (
 			ParentRef: OneDriveItemPath2.ToBuilder().Dir().ShortRef(),
 			ItemInfo: details.ItemInfo{
 				OneDrive: &details.OneDriveInfo{
-					ItemType: details.OneDriveItem,
-					ParentPath: stdpath.Join(
-						append(
-							[]string{
-								"drives",
-								"foo",
-								"root:",
-							},
-							OneDriveItemPath2.Folders()...,
-						)...,
-					),
-					ItemName: OneDriveItemPath2.Item() + "name",
+					ItemType:   details.OneDriveItem,
+					ParentPath: OneDriveParentFolder1,
+					ItemName:   OneDriveItemPath2.Item() + "name",
+					Size:       int64(42),
+					Created:    Time1,
+					Modified:   Time3,
+				},
+			},
+		},
+		{
+			RepoRef:   OneDriveItemPath3.String(),
+			ShortRef:  OneDriveItemPath3.ShortRef(),
+			ParentRef: OneDriveItemPath3.ToBuilder().Dir().ShortRef(),
+			ItemInfo: details.ItemInfo{
+				OneDrive: &details.OneDriveInfo{
+					ItemType:   details.OneDriveItem,
+					ParentPath: OneDriveParentFolder2,
+					ItemName:   OneDriveItemPath3.Item() + "name",
+					Size:       int64(19),
+					Created:    Time2,
+					Modified:   Time4,
 				},
 			},
 		},