From afa873426895d4df3f11e4addb3a1cb06a8f6711 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Wed, 19 Oct 2022 16:03:31 -0700 Subject: [PATCH] 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 - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [x] :robot: Test - [ ] :computer: CI/Deployment - [ ] :hamster: Trivial/Minor ## Issue(s) * closes #1231 * #913 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/cli/backup/onedrive.go | 4 + src/cli/backup/onedrive_test.go | 39 +++++++ src/cli/utils/onedrive.go | 2 +- src/cli/utils/testdata/opts.go | 142 ++++++++++++++++++++++++++ src/pkg/selectors/testdata/details.go | 68 +++++++----- 5 files changed, 227 insertions(+), 28 deletions(-) diff --git a/src/cli/backup/onedrive.go b/src/cli/backup/onedrive.go index 701c9d449..a050d47ce 100644 --- a/src/cli/backup/onedrive.go +++ b/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) { diff --git a/src/cli/backup/onedrive_test.go b/src/cli/backup/onedrive_test.go index f9b1d686d..2f7654969 100644 --- a/src/cli/backup/onedrive_test.go +++ b/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) + }) + } +} diff --git a/src/cli/utils/onedrive.go b/src/cli/utils/onedrive.go index 41c00b9fd..a5badfa02 100644 --- a/src/cli/utils/onedrive.go +++ b/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") } diff --git a/src/cli/utils/testdata/opts.go b/src/cli/utils/testdata/opts.go index 2af755c52..c145a829c 100644 --- a/src/cli/utils/testdata/opts.go +++ b/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 diff --git a/src/pkg/selectors/testdata/details.go b/src/pkg/selectors/testdata/details.go index d7a90260f..e9a148e29 100644 --- a/src/pkg/selectors/testdata/details.go +++ b/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, }, }, },