From f01e25ad834e9b899d67d10b526cb94d50319932 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Wed, 19 Apr 2023 13:27:55 -0700 Subject: [PATCH] Redo folder generation in backup details (#3140) Augment the folder backup details entries with LocationRef and some information about what data type generated the entry. Also add top-level container information like drive name/ID if applicable Refactor code to do folder generation to make it more contained and require less parameters to add entries to backup details Important changes are in details.go. All other changes are just to keep up with the slightly modified function API or update tests --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * closes #3120 * closes #2138 #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/cli/backup/exchange_e2e_test.go | 5 +- .../connector/exchange/mock/collections.go | 1 + .../connector/exchange/service_restore.go | 8 +- .../connector/graph_connector_test.go | 9 +- src/internal/connector/onedrive/restore.go | 6 +- src/internal/connector/sharepoint/restore.go | 12 +- src/internal/data/mock/collection.go | 54 + src/internal/kopia/upload.go | 21 +- src/internal/kopia/upload_test.go | 88 +- src/internal/kopia/wrapper_test.go | 101 +- src/internal/operations/backup.go | 9 +- src/internal/operations/backup_test.go | 26 +- src/internal/operations/restore_test.go | 2 +- src/internal/streamstore/collectables_test.go | 22 +- src/pkg/backup/details/details.go | 359 ++++-- src/pkg/backup/details/details_test.go | 1118 ++++++++--------- .../repository/repository_unexported_test.go | 24 +- 17 files changed, 1002 insertions(+), 863 deletions(-) create mode 100644 src/internal/data/mock/collection.go diff --git a/src/cli/backup/exchange_e2e_test.go b/src/cli/backup/exchange_e2e_test.go index 759258058..ef37d00fb 100644 --- a/src/cli/backup/exchange_e2e_test.go +++ b/src/cli/backup/exchange_e2e_test.go @@ -471,8 +471,9 @@ func runExchangeDetailsCmdTest(suite *PreparedBackupExchangeE2ESuite, category p i++ } - // At least the prefix of the path should be encoded as folders. - assert.Greater(t, foundFolders, 4) + // We only backup the default folder for each category so there should be at + // least that folder (we don't make details entries for prefix folders). + assert.GreaterOrEqual(t, foundFolders, 1) } // --------------------------------------------------------------------------- diff --git a/src/internal/connector/exchange/mock/collections.go b/src/internal/connector/exchange/mock/collections.go index 593af6b9c..4d78523b1 100644 --- a/src/internal/connector/exchange/mock/collections.go +++ b/src/internal/connector/exchange/mock/collections.go @@ -166,6 +166,7 @@ func (med *Data) ToReader() io.ReadCloser { func (med *Data) Info() details.ItemInfo { return details.ItemInfo{ Exchange: &details.ExchangeInfo{ + ItemType: details.ExchangeMail, Sender: "foo@bar.com", Subject: "Hello world!", Received: time.Now(), diff --git a/src/internal/connector/exchange/service_restore.go b/src/internal/connector/exchange/service_restore.go index 10808e7ed..c9178e947 100644 --- a/src/internal/connector/exchange/service_restore.go +++ b/src/internal/connector/exchange/service_restore.go @@ -442,15 +442,13 @@ func restoreCollection( continue } - var locationRef string + locationRef := &path.Builder{} if category == path.ContactsCategory { - locationRef = itemPath.Folder(false) + locationRef = locationRef.Append(itemPath.Folders()...) } err = deets.Add( - itemPath.String(), - itemPath.ShortRef(), - "", + itemPath, locationRef, true, details.ItemInfo{ diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 5d1e47e43..123f3f959 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -510,7 +510,8 @@ func runRestore( assert.Equal(t, numRestoreItems, status.Successes, "restored status.Successes") assert.Len( t, - deets.Entries, + // Don't check folders as those are now added to details. + deets.Items(), numRestoreItems, "details entries contains same item count as total successful items restored") @@ -1085,8 +1086,10 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames // Always just 1 because it's just 1 collection. assert.Equal(t, totalItems, status.Objects, "status.Objects") assert.Equal(t, totalItems, status.Successes, "status.Successes") - assert.Equal( - t, totalItems, len(deets.Entries), + assert.Len( + t, + deets.Items(), + totalItems, "details entries contains same item count as total successful items restored") t.Log("Restore complete") diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index d022d5d82..cf968bedd 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -249,10 +249,8 @@ func RestoreCollection( } err = deets.Add( - itemPath.String(), - itemPath.ShortRef(), - "", - "", // TODO: implement locationRef + itemPath, + &path.Builder{}, // TODO: implement locationRef true, itemInfo) if err != nil { diff --git a/src/internal/connector/sharepoint/restore.go b/src/internal/connector/sharepoint/restore.go index 351230d6a..aa71b67a6 100644 --- a/src/internal/connector/sharepoint/restore.go +++ b/src/internal/connector/sharepoint/restore.go @@ -264,10 +264,8 @@ func RestoreListCollection( } err = deets.Add( - itemPath.String(), - itemPath.ShortRef(), - "", - "", // TODO: implement locationRef + itemPath, + &path.Builder{}, // TODO: implement locationRef true, itemInfo) if err != nil { @@ -354,10 +352,8 @@ func RestorePageCollection( } err = deets.Add( - itemPath.String(), - itemPath.ShortRef(), - "", - "", // TODO: implement locationRef + itemPath, + &path.Builder{}, // TODO: implement locationRef true, itemInfo) if err != nil { diff --git a/src/internal/data/mock/collection.go b/src/internal/data/mock/collection.go new file mode 100644 index 000000000..63f2b2dd8 --- /dev/null +++ b/src/internal/data/mock/collection.go @@ -0,0 +1,54 @@ +package mock + +import ( + "io" + "time" + + "github.com/alcionai/corso/src/pkg/backup/details" +) + +type Stream struct { + ID string + Reader io.ReadCloser + ReadErr error + ItemSize int64 + ModifiedTime time.Time + DeletedFlag bool + ItemInfo details.ItemInfo +} + +func (s *Stream) UUID() string { + return s.ID +} + +func (s Stream) Deleted() bool { + return s.DeletedFlag +} + +func (s *Stream) ToReader() io.ReadCloser { + if s.ReadErr != nil { + return io.NopCloser(errReader{s.ReadErr}) + } + + return s.Reader +} + +func (s *Stream) Info() details.ItemInfo { + return s.ItemInfo +} + +func (s *Stream) Size() int64 { + return s.ItemSize +} + +func (s *Stream) ModTime() time.Time { + return s.ModifiedTime +} + +type errReader struct { + readErr error +} + +func (er errReader) Read([]byte) (int, error) { + return 0, er.readErr +} diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index 57029d1d4..b12f579f8 100644 --- a/src/internal/kopia/upload.go +++ b/src/internal/kopia/upload.go @@ -208,20 +208,9 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { return } - var ( - locationFolders string - parent = d.repoPath.ToBuilder().Dir() - ) - - if d.locationPath != nil { - locationFolders = d.locationPath.String() - } - err = cp.deets.Add( - d.repoPath.String(), - d.repoPath.ShortRef(), - parent.ShortRef(), - locationFolders, + d.repoPath, + d.locationPath, !d.cached, *d.info) if err != nil { @@ -234,12 +223,6 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { return } - - folders := details.FolderEntriesForPath(parent, d.locationPath) - cp.deets.AddFoldersForItem( - folders, - *d.info, - !d.cached) } // Kopia interface function used as a callback when kopia finishes hashing a file. diff --git a/src/internal/kopia/upload_test.go b/src/internal/kopia/upload_test.go index d2e37b0d0..64579e017 100644 --- a/src/internal/kopia/upload_test.go +++ b/src/internal/kopia/upload_test.go @@ -386,7 +386,15 @@ var finishedFileTable = []struct { cachedItems: func(fname string, fpath path.Path) map[string]testInfo { return map[string]testInfo{ fname: { - info: &itemDetails{info: &details.ItemInfo{}, repoPath: fpath}, + info: &itemDetails{ + info: &details.ItemInfo{ + Exchange: &details.ExchangeInfo{ + ItemType: details.ExchangeMail, + }, + }, + repoPath: fpath, + locationPath: path.Builder{}.Append(fpath.Folders()...), + }, err: nil, totalBytes: 100, }, @@ -394,7 +402,7 @@ var finishedFileTable = []struct { }, expectedBytes: 100, // 1 file and 5 folders. - expectedNumEntries: 6, + expectedNumEntries: 2, }, { name: "PendingNoDetails", @@ -413,8 +421,15 @@ var finishedFileTable = []struct { cachedItems: func(fname string, fpath path.Path) map[string]testInfo { return map[string]testInfo{ fname: { - info: &itemDetails{info: &details.ItemInfo{}, repoPath: fpath}, - err: assert.AnError, + info: &itemDetails{ + info: &details.ItemInfo{ + Exchange: &details.ExchangeInfo{ + ItemType: details.ExchangeMail, + }, + }, + repoPath: fpath, + }, + err: assert.AnError, }, } }, @@ -521,71 +536,6 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFileCachedNoPrevPathErrors() { assert.Error(t, cp.errs.Failure(), clues.ToCore(cp.errs.Failure())) } -func (suite *CorsoProgressUnitSuite) TestFinishedFileBuildsHierarchyNewItem() { - t := suite.T() - // Order of folders in hierarchy from root to leaf (excluding the item). - expectedFolderOrder := suite.targetFilePath.ToBuilder().Dir().Elements() - - // Setup stuff. - bd := &details.Builder{} - cp := corsoProgress{ - UploadProgress: &snapshotfs.NullUploadProgress{}, - deets: bd, - pending: map[string]*itemDetails{}, - toMerge: newMergeDetails(), - errs: fault.New(true), - } - - deets := &itemDetails{info: &details.ItemInfo{}, repoPath: suite.targetFilePath} - cp.put(suite.targetFileName, deets) - require.Len(t, cp.pending, 1) - - cp.FinishedFile(suite.targetFileName, nil) - - assert.Equal(t, 0, cp.toMerge.ItemsToMerge()) - - // Gather information about the current state. - var ( - curRef *details.DetailsEntry - refToEntry = map[string]*details.DetailsEntry{} - ) - - entries := bd.Details().Entries - - for i := 0; i < len(entries); i++ { - e := &entries[i] - if e.Folder == nil { - continue - } - - refToEntry[e.ShortRef] = e - - if e.Folder.DisplayName == expectedFolderOrder[len(expectedFolderOrder)-1] { - curRef = e - } - } - - // Actual tests start here. - var rootRef *details.DetailsEntry - - // Traverse the details entries from leaf to root, following the ParentRef - // fields. At the end rootRef should point to the root of the path. - for i := len(expectedFolderOrder) - 1; i >= 0; i-- { - name := expectedFolderOrder[i] - - require.NotNil(t, curRef) - assert.Equal(t, name, curRef.Folder.DisplayName) - - rootRef = curRef - curRef = refToEntry[curRef.ParentRef] - } - - // Hierarchy root's ParentRef = "" and map will return nil. - assert.Nil(t, curRef) - require.NotNil(t, rootRef) - assert.Empty(t, rootRef.ParentRef) -} - func (suite *CorsoProgressUnitSuite) TestFinishedFileBaseItemDoesntBuildHierarchy() { type expectedRef struct { oldRef *path.Builder diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index 7753bcd35..684d3fae2 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/src/internal/kopia/wrapper_test.go @@ -20,7 +20,9 @@ import ( exchMock "github.com/alcionai/corso/src/internal/connector/exchange/mock" "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/internal/data/mock" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -294,12 +296,12 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() { assert.Equal(t, 0, stats.ErrorCount) assert.False(t, stats.Incomplete) - // 47 file and 6 folder entries. + // 47 file and 2 folder entries. details := deets.Details().Entries assert.Len( t, details, - test.expectedUploadedFiles+test.expectedCachedFiles+6, + test.expectedUploadedFiles+test.expectedCachedFiles+2, ) for _, entry := range details { @@ -331,6 +333,8 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() { } } +// TODO(ashmrtn): This should really be moved to an e2e test that just checks +// details for certain things. func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { tmp, err := path.Build( testTenant, @@ -342,7 +346,14 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { require.NoError(suite.T(), err, clues.ToCore(err)) storePath := tmp - locPath := tmp + locPath := path.Builder{}.Append(tmp.Folders()...) + + baseOneDriveItemInfo := details.OneDriveInfo{ + ItemType: details.OneDriveItem, + DriveID: "drive-id", + DriveName: "drive-name", + ItemName: "item", + } // tags that are supplied by the caller. This includes basic tags to support // lookups and extra tags the caller may want to apply. @@ -385,13 +396,32 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { numDeetsEntries: 3, hasMetaDeets: true, cols: func() []data.BackupCollection { - mc := exchMock.NewCollection( - storePath, - locPath, - 3) - mc.Names[0] = testFileName - mc.Names[1] = testFileName + metadata.MetaFileSuffix - mc.Names[2] = storePath.Folders()[0] + metadata.DirMetaFileSuffix + streams := []data.Stream{} + fileNames := []string{ + testFileName, + testFileName + metadata.MetaFileSuffix, + metadata.DirMetaFileSuffix, + } + + for _, name := range fileNames { + info := baseOneDriveItemInfo + info.ItemName = name + + ms := &mock.Stream{ + ID: name, + Reader: io.NopCloser(&bytes.Buffer{}), + ItemSize: 0, + ItemInfo: details.ItemInfo{OneDrive: &info}, + } + + streams = append(streams, ms) + } + + mc := &mockBackupCollection{ + path: storePath, + loc: locPath, + streams: streams, + } return []data.BackupCollection{mc} }, @@ -404,12 +434,22 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { numDeetsEntries: 1, hasMetaDeets: false, cols: func() []data.BackupCollection { - mc := exchMock.NewCollection( - storePath, - locPath, - 1) - mc.Names[0] = testFileName - mc.ColState = data.NotMovedState + info := baseOneDriveItemInfo + info.ItemName = testFileName + + ms := &mock.Stream{ + ID: testFileName, + Reader: io.NopCloser(&bytes.Buffer{}), + ItemSize: 0, + ItemInfo: details.ItemInfo{OneDrive: &info}, + } + + mc := &mockBackupCollection{ + path: storePath, + loc: locPath, + streams: []data.Stream{ms}, + state: data.NotMovedState, + } return []data.BackupCollection{mc} }, @@ -441,12 +481,12 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { assert.Equal(t, 0, stats.ErrorCount) assert.False(t, stats.Incomplete) - // 47 file and 5 folder entries. + // 47 file and 1 folder entries. details := deets.Details().Entries assert.Len( t, details, - test.numDeetsEntries+5, + test.numDeetsEntries+1, ) for _, entry := range details { @@ -461,7 +501,7 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_NoDetailsForMeta() { // Shouldn't have any items to merge because the cached files are metadata // files. - assert.Equal(t, 0, prevShortRefs.ItemsToMerge()) + assert.Equal(t, 0, prevShortRefs.ItemsToMerge(), "merge items") checkSnapshotTags( t, @@ -556,7 +596,9 @@ func (suite *KopiaIntegrationSuite) TestRestoreAfterCompressionChange() { type mockBackupCollection struct { path path.Path + loc *path.Builder streams []data.Stream + state data.CollectionState } func (c *mockBackupCollection) Items(context.Context, *fault.Bus) <-chan data.Stream { @@ -581,8 +623,12 @@ func (c mockBackupCollection) PreviousPath() path.Path { return nil } +func (c mockBackupCollection) LocationPath() *path.Builder { + return c.loc +} + func (c mockBackupCollection) State() data.CollectionState { - return data.NewState + return c.state } func (c mockBackupCollection) DoNotMergeItems() bool { @@ -592,6 +638,8 @@ func (c mockBackupCollection) DoNotMergeItems() bool { func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { t := suite.T() + loc1 := path.Builder{}.Append(suite.storePath1.Folders()...) + loc2 := path.Builder{}.Append(suite.storePath2.Folders()...) tags := map[string]string{} reason := Reason{ ResourceOwner: testUser, @@ -606,6 +654,7 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { collections := []data.BackupCollection{ &mockBackupCollection{ path: suite.storePath1, + loc: loc1, streams: []data.Stream{ &exchMock.Data{ ID: testFileName, @@ -619,6 +668,7 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { }, &mockBackupCollection{ path: suite.storePath2, + loc: loc2, streams: []data.Stream{ &exchMock.Data{ ID: testFileName3, @@ -654,8 +704,8 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { assert.Equal(t, 6, stats.TotalDirectoryCount) assert.Equal(t, 1, stats.IgnoredErrorCount) assert.False(t, stats.Incomplete) - // 5 file and 6 folder entries. - assert.Len(t, deets.Details().Entries, 5+6) + // 5 file and 2 folder entries. + assert.Len(t, deets.Details().Entries, 5+2) failedPath, err := suite.storePath2.Append(testFileName4, true) require.NoError(t, err, clues.ToCore(err)) @@ -836,7 +886,8 @@ func (suite *KopiaSimpleRepoIntegrationSuite) SetupTest() { collections := []data.BackupCollection{} for _, parent := range []path.Path{suite.testPath1, suite.testPath2} { - collection := &mockBackupCollection{path: parent} + loc := path.Builder{}.Append(parent.Folders()...) + collection := &mockBackupCollection{path: parent, loc: loc} for _, item := range suite.files[parent.String()] { collection.streams = append( @@ -876,8 +927,8 @@ func (suite *KopiaSimpleRepoIntegrationSuite) SetupTest() { require.Equal(t, stats.TotalDirectoryCount, expectedDirs) require.Equal(t, stats.IgnoredErrorCount, 0) require.False(t, stats.Incomplete) - // 6 file and 6 folder entries. - assert.Len(t, deets.Details().Entries, expectedFiles+expectedDirs) + // 6 file and 2 folder entries. + assert.Len(t, deets.Details().Entries, expectedFiles+2) suite.snapshotID = manifest.ID(stats.SnapshotID) } diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 978c6e2a2..ac185d3d0 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -670,19 +670,14 @@ func mergeDetails( itemUpdated := newPath.String() != rr.String() || locUpdated err = deets.Add( - newPath.String(), - newPath.ShortRef(), - newPath.ToBuilder().Dir().ShortRef(), - newLoc.String(), + newPath, + newLoc, itemUpdated, item) if err != nil { return clues.Wrap(err, "adding item to details") } - folders := details.FolderEntriesForPath(newPath.ToBuilder().Dir(), newLoc) - deets.AddFoldersForItem(folders, item, itemUpdated) - // Track how many entries we added so that we know if we got them all when // we're done. addedEntries++ diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index bd95275c8..698e5f118 100644 --- a/src/internal/operations/backup_test.go +++ b/src/internal/operations/backup_test.go @@ -267,9 +267,10 @@ func makeMetadataPath( func makeFolderEntry( t *testing.T, - pb *path.Builder, + pb, loc *path.Builder, size int64, modTime time.Time, + dt details.ItemType, ) *details.DetailsEntry { t.Helper() @@ -277,13 +278,14 @@ func makeFolderEntry( RepoRef: pb.String(), ShortRef: pb.ShortRef(), ParentRef: pb.Dir().ShortRef(), - LocationRef: pb.PopFront().PopFront().PopFront().PopFront().Dir().String(), + LocationRef: loc.Dir().String(), ItemInfo: details.ItemInfo{ Folder: &details.FolderInfo{ ItemType: details.FolderItem, - DisplayName: pb.Elements()[len(pb.Elements())-1], + DisplayName: pb.LastElem(), Modified: modTime, Size: size, + DataType: dt, }, }, } @@ -347,6 +349,7 @@ func makeDetailsEntry( ParentPath: l.PopFront().String(), Size: int64(size), DriveID: "drive-id", + DriveName: "drive-name", } default: @@ -1210,6 +1213,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsFolde ro, path.EmailCategory.String(), "work", + "project8", "item1", } @@ -1218,7 +1222,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsFolde pathElems, true) - locPath1 = path.Builder{}.Append(pathElems[:len(pathElems)-1]...) + locPath1 = path.Builder{}.Append(itemPath1.Folders()...) backup1 = backup.Backup{ BaseModel: model.BaseModel{ @@ -1270,12 +1274,15 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsFolde // update the details itemDetails.Exchange.Modified = now - for i := 1; i < len(pathElems); i++ { + for i := 1; i <= len(locPath1.Elements()); i++ { expectedEntries = append(expectedEntries, *makeFolderEntry( t, - path.Builder{}.Append(pathElems[:i]...), + // Include prefix elements in the RepoRef calculations. + path.Builder{}.Append(pathElems[:4+i]...), + path.Builder{}.Append(locPath1.Elements()[:i]...), int64(itemSize), - itemDetails.Exchange.Modified)) + itemDetails.Exchange.Modified, + details.ExchangeMail)) } ctx, flush := tester.NewContext() @@ -1302,7 +1309,10 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsFolde // compares two details slices. Useful for tests where serializing the // entries can produce minor variations in the time struct, causing // assert.elementsMatch to fail. -func compareDeetEntries(t *testing.T, expect, result []details.DetailsEntry) { +func compareDeetEntries( + t *testing.T, + expect, result []details.DetailsEntry, +) { if !assert.Equal(t, len(expect), len(result), "entry slices should be equal len") { require.ElementsMatch(t, expect, result) } diff --git a/src/internal/operations/restore_test.go b/src/internal/operations/restore_test.go index db778c0b5..9b00e122e 100644 --- a/src/internal/operations/restore_test.go +++ b/src/internal/operations/restore_test.go @@ -463,7 +463,7 @@ func (suite *RestoreOpIntegrationSuite) TestRestore_Run() { require.NotEmpty(t, ro.Results, "restoreOp results") require.NotNil(t, ds, "restored details") assert.Equal(t, ro.Status, Completed, "restoreOp status") - assert.Equal(t, ro.Results.ItemsWritten, len(ds.Entries), "item write count matches len details") + assert.Equal(t, ro.Results.ItemsWritten, len(ds.Items()), "item write count matches len details") assert.Less(t, 0, ro.Results.ItemsRead, "restore items read") assert.Less(t, int64(0), ro.Results.BytesRead, "bytes read") assert.Equal(t, 1, ro.Results.ResourceOwners, "resource Owners") diff --git a/src/internal/streamstore/collectables_test.go b/src/internal/streamstore/collectables_test.go index 030293738..8eb3dc7fd 100644 --- a/src/internal/streamstore/collectables_test.go +++ b/src/internal/streamstore/collectables_test.go @@ -64,6 +64,11 @@ func (suite *StreamStoreIntgSuite) TearDownSubTest() { } func (suite *StreamStoreIntgSuite) TestStreamer() { + deetsPath, err := path.FromDataLayerPath("tenant-id/exchange/user-id/email/Inbox/folder1/foo", true) + require.NoError(suite.T(), err, clues.ToCore(err)) + + locPath := path.Builder{}.Append(deetsPath.Folders()...) + table := []struct { name string deets func(*testing.T) *details.Details @@ -81,12 +86,15 @@ func (suite *StreamStoreIntgSuite) TestStreamer() { deets: func(t *testing.T) *details.Details { deetsBuilder := &details.Builder{} require.NoError(t, deetsBuilder.Add( - "rr", "sr", "pr", "lr", + deetsPath, + locPath, true, details.ItemInfo{ - Exchange: &details.ExchangeInfo{Subject: "hello world"}, + Exchange: &details.ExchangeInfo{ + ItemType: details.ExchangeMail, + Subject: "hello world", + }, })) - return deetsBuilder.Details() }, errs: func() *fault.Errors { return nil }, @@ -112,10 +120,14 @@ func (suite *StreamStoreIntgSuite) TestStreamer() { deets: func(t *testing.T) *details.Details { deetsBuilder := &details.Builder{} require.NoError(t, deetsBuilder.Add( - "rr", "sr", "pr", "lr", + deetsPath, + locPath, true, details.ItemInfo{ - Exchange: &details.ExchangeInfo{Subject: "hello world"}, + Exchange: &details.ExchangeInfo{ + ItemType: details.ExchangeMail, + Subject: "hello world", + }, })) return deetsBuilder.Details() diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index b77891743..0e99583f2 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -10,6 +10,7 @@ import ( "github.com/alcionai/clues" "github.com/dustin/go-humanize" + "golang.org/x/exp/maps" "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/internal/common" @@ -40,6 +41,35 @@ func (ul uniqueLoc) InDetails() *path.Builder { return path.Builder{}.Append(ul.pb.Elements()[ul.prefixElems:]...) } +// elementCount returns the number of non-prefix elements in the LocationIDer +// (i.e. the number of elements in the InDetails path.Builder). +func (ul uniqueLoc) elementCount() int { + res := len(ul.pb.Elements()) - ul.prefixElems + if res < 0 { + res = 0 + } + + return res +} + +func (ul *uniqueLoc) dir() { + if ul.elementCount() == 0 { + return + } + + ul.pb = ul.pb.Dir() +} + +// lastElem returns the unescaped last element in the location. If the location +// is empty returns an empty string. +func (ul uniqueLoc) lastElem() string { + if ul.elementCount() == 0 { + return "" + } + + return ul.pb.LastElem() +} + // Having service-specific constructors can be kind of clunky, but in this case // I think they'd be useful to ensure the proper args are used since this // path.Builder is used as a key in some maps. @@ -95,15 +125,6 @@ func NewSharePointLocationIDer( } } -type folderEntry struct { - RepoRef string - ShortRef string - ParentRef string - LocationRef string - Updated bool - Info ItemInfo -} - // -------------------------------------------------------------------------------- // Model // -------------------------------------------------------------------------------- @@ -218,19 +239,117 @@ func (de DetailsEntry) isMetaFile() bool { // Builder should be used to create a details model. type Builder struct { d Details - mu sync.Mutex `json:"-"` - knownFolders map[string]folderEntry `json:"-"` + mu sync.Mutex `json:"-"` + knownFolders map[string]DetailsEntry `json:"-"` } func (b *Builder) Add( - repoRef, shortRef, parentRef, locationRef string, + repoRef path.Path, + locationRef *path.Builder, updated bool, info ItemInfo, ) error { b.mu.Lock() defer b.mu.Unlock() - return b.d.add(repoRef, shortRef, parentRef, locationRef, updated, info) + entry, err := b.d.add( + repoRef, + locationRef, + updated, + info) + if err != nil { + return clues.Wrap(err, "adding entry to details") + } + + if err := b.addFolderEntries( + repoRef.ToBuilder().Dir(), + locationRef, + entry, + ); err != nil { + return clues.Wrap(err, "adding folder entries") + } + + return nil +} + +func (b *Builder) addFolderEntries( + repoRef, locationRef *path.Builder, + entry DetailsEntry, +) error { + if len(repoRef.Elements()) < len(locationRef.Elements()) { + return clues.New("RepoRef shorter than LocationRef"). + With("repo_ref", repoRef, "location_ref", locationRef) + } + + if b.knownFolders == nil { + b.knownFolders = map[string]DetailsEntry{} + } + + // Need a unique location because we want to have separate folders for + // different drives and categories even if there's duplicate folder names in + // them. + uniqueLoc, err := entry.uniqueLocation(locationRef) + if err != nil { + return clues.Wrap(err, "getting LocationIDer") + } + + for uniqueLoc.elementCount() > 0 { + mapKey := uniqueLoc.ID().ShortRef() + + name := uniqueLoc.lastElem() + if len(name) == 0 { + return clues.New("folder with no display name"). + With("repo_ref", repoRef, "location_ref", uniqueLoc.InDetails()) + } + + shortRef := repoRef.ShortRef() + rr := repoRef.String() + + // Get the parent of this entry to add as the LocationRef for the folder. + uniqueLoc.dir() + + repoRef = repoRef.Dir() + parentRef := repoRef.ShortRef() + + folder, ok := b.knownFolders[mapKey] + if !ok { + loc := uniqueLoc.InDetails().String() + + folder = DetailsEntry{ + RepoRef: rr, + ShortRef: shortRef, + ParentRef: parentRef, + LocationRef: loc, + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + ItemType: FolderItem, + // TODO(ashmrtn): Use the item type returned by the entry once + // SharePoint properly sets it. + DisplayName: name, + }, + }, + } + + if err := entry.updateFolder(folder.Folder); err != nil { + return clues.Wrap(err, "adding folder"). + With("parent_repo_ref", repoRef, "location_ref", loc) + } + } + + folder.Folder.Size += entry.size() + folder.Updated = folder.Updated || entry.Updated + + itemModified := entry.Modified() + if folder.Folder.Modified.Before(itemModified) { + folder.Folder.Modified = itemModified + } + + // Always update the map because we're storing structs not pointers to + // structs. + b.knownFolders[mapKey] = folder + } + + return nil } func (b *Builder) Details() *Details { @@ -238,96 +357,11 @@ func (b *Builder) Details() *Details { defer b.mu.Unlock() // Write the cached folder entries to details - for _, folder := range b.knownFolders { - b.d.addFolder(folder) - } + b.d.Entries = append(b.d.Entries, maps.Values(b.knownFolders)...) return &b.d } -// TODO(ashmrtn): If we never need to pre-populate the modified time of a folder -// we should just merge this with AddFoldersForItem, have Add call -// AddFoldersForItem, and unexport AddFoldersForItem. -func FolderEntriesForPath(parent, location *path.Builder) []folderEntry { - folders := []folderEntry{} - lfs := location - - for len(parent.Elements()) > 0 { - var ( - nextParent = parent.Dir() - lr string - dn = parent.LastElem() - ) - - // TODO: We may have future cases where the storage hierarchy - // doesn't match the location hierarchy. - if lfs != nil { - lr = lfs.String() - - if len(lfs.Elements()) > 0 { - dn = lfs.LastElem() - } - } - - folders = append(folders, folderEntry{ - RepoRef: parent.String(), - ShortRef: parent.ShortRef(), - ParentRef: nextParent.ShortRef(), - LocationRef: lr, - Info: ItemInfo{ - Folder: &FolderInfo{ - ItemType: FolderItem, - DisplayName: dn, - }, - }, - }) - - parent = nextParent - - if lfs != nil { - lfs = lfs.Dir() - } - } - - return folders -} - -// AddFoldersForItem adds entries for the given folders. It skips adding entries that -// have been added by previous calls. -func (b *Builder) AddFoldersForItem(folders []folderEntry, itemInfo ItemInfo, updated bool) { - b.mu.Lock() - defer b.mu.Unlock() - - if b.knownFolders == nil { - b.knownFolders = map[string]folderEntry{} - } - - for _, folder := range folders { - if existing, ok := b.knownFolders[folder.ShortRef]; ok { - // We've seen this folder before for a different item. - // Update the "cached" folder entry - folder = existing - } - - // Update the folder's size and modified time - itemModified := itemInfo.Modified() - - folder.Info.Folder.Size += itemInfo.size() - - if folder.Info.Folder.Modified.Before(itemModified) { - folder.Info.Folder.Modified = itemModified - } - - // If the item being added was "updated" - propagate that to the - // folder entries - if updated { - folder.Updated = true - } - - b.knownFolders[folder.ShortRef] = folder - } -} - // -------------------------------------------------------------------------------- // Details // -------------------------------------------------------------------------------- @@ -340,15 +374,20 @@ type Details struct { } func (d *Details) add( - repoRef, shortRef, parentRef, locationRef string, + repoRef path.Path, + locationRef *path.Builder, updated bool, info ItemInfo, -) error { +) (DetailsEntry, error) { + if locationRef == nil { + return DetailsEntry{}, clues.New("nil LocationRef").With("repo_ref", repoRef) + } + entry := DetailsEntry{ - RepoRef: repoRef, - ShortRef: shortRef, - ParentRef: parentRef, - LocationRef: locationRef, + RepoRef: repoRef.String(), + ShortRef: repoRef.ShortRef(), + ParentRef: repoRef.ToBuilder().Dir().ShortRef(), + LocationRef: locationRef.String(), Updated: updated, ItemInfo: info, } @@ -356,13 +395,8 @@ func (d *Details) add( // Use the item name and the path for the ShortRef. This ensures that renames // within a directory generate unique ShortRefs. if info.infoType() == OneDriveItem || info.infoType() == SharePointLibrary { - p, err := path.FromDataLayerPath(repoRef, true) - if err != nil { - return clues.Wrap(err, "munging OneDrive or SharePoint ShortRef") - } - if info.OneDrive == nil && info.SharePoint == nil { - return clues.New("item is not SharePoint or OneDrive type") + return entry, clues.New("item is not SharePoint or OneDrive type") } filename := "" @@ -381,25 +415,14 @@ func (d *Details) add( // M365 ID of this file and also have a subfolder in the folder with a // display name that matches the file's display name. That would result in // duplicate ShortRefs, which we can't allow. - elements := p.Elements() - elements = append(elements[:len(elements)-1], filename, p.Item()) + elements := repoRef.Elements() + elements = append(elements[:len(elements)-1], filename, repoRef.Item()) entry.ShortRef = path.Builder{}.Append(elements...).ShortRef() } d.Entries = append(d.Entries, entry) - return nil -} - -// addFolder adds an entry for the given folder. -func (d *Details) addFolder(folder folderEntry) { - d.Entries = append(d.Entries, DetailsEntry{ - RepoRef: folder.RepoRef, - ShortRef: folder.ShortRef, - ParentRef: folder.ParentRef, - ItemInfo: folder.Info, - Updated: folder.Updated, - }) + return entry, nil } // Marshal complies with the marshaller interface in streamStore. @@ -666,7 +689,7 @@ func (i ItemInfo) Modified() time.Time { return time.Time{} } -func (i ItemInfo) uniqueLocation(baseLoc *path.Builder) (LocationIDer, error) { +func (i ItemInfo) uniqueLocation(baseLoc *path.Builder) (*uniqueLoc, error) { switch { case i.Exchange != nil: return i.Exchange.uniqueLocation(baseLoc) @@ -682,11 +705,30 @@ func (i ItemInfo) uniqueLocation(baseLoc *path.Builder) (LocationIDer, error) { } } +func (i ItemInfo) updateFolder(f *FolderInfo) error { + switch { + case i.Exchange != nil: + return i.Exchange.updateFolder(f) + + case i.OneDrive != nil: + return i.OneDrive.updateFolder(f) + + case i.SharePoint != nil: + return i.SharePoint.updateFolder(f) + + default: + return clues.New("unsupported type") + } +} + type FolderInfo struct { ItemType ItemType `json:"itemType,omitempty"` DisplayName string `json:"displayName"` Modified time.Time `json:"modified,omitempty"` Size int64 `json:"size,omitempty"` + DataType ItemType `json:"dataType,omitempty"` + DriveName string `json:"driveName,omitempty"` + DriveID string `json:"driveID,omitempty"` } func (i FolderInfo) Headers() []string { @@ -762,7 +804,7 @@ func (i *ExchangeInfo) UpdateParentPath(newLocPath *path.Builder) { i.ParentPath = newLocPath.String() } -func (i *ExchangeInfo) uniqueLocation(baseLoc *path.Builder) (LocationIDer, error) { +func (i *ExchangeInfo) uniqueLocation(baseLoc *path.Builder) (*uniqueLoc, error) { var category path.CategoryType switch i.ItemType { @@ -774,7 +816,24 @@ func (i *ExchangeInfo) uniqueLocation(baseLoc *path.Builder) (LocationIDer, erro category = path.EmailCategory } - return NewExchangeLocationIDer(category, baseLoc.Elements()...) + loc, err := NewExchangeLocationIDer(category, baseLoc.Elements()...) + + return &loc, err +} + +func (i *ExchangeInfo) updateFolder(f *FolderInfo) error { + // Use a switch instead of a rather large if-statement. Just make sure it's an + // Exchange type. If it's not return an error. + switch i.ItemType { + case ExchangeContact, ExchangeEvent, ExchangeMail: + default: + return clues.New("unsupported non-Exchange ItemType"). + With("item_type", i.ItemType) + } + + f.DataType = i.ItemType + + return nil } // SharePointInfo describes a sharepoint item @@ -815,12 +874,24 @@ func (i *SharePointInfo) UpdateParentPath(newLocPath *path.Builder) { i.ParentPath = newLocPath.PopFront().String() } -func (i *SharePointInfo) uniqueLocation(baseLoc *path.Builder) (LocationIDer, error) { +func (i *SharePointInfo) uniqueLocation(baseLoc *path.Builder) (*uniqueLoc, error) { if len(i.DriveID) == 0 { return nil, clues.New("empty drive ID") } - return NewSharePointLocationIDer(i.DriveID, baseLoc.Elements()...), nil + loc := NewSharePointLocationIDer(i.DriveID, baseLoc.Elements()...) + + return &loc, nil +} + +func (i *SharePointInfo) updateFolder(f *FolderInfo) error { + // TODO(ashmrtn): Change to just SharePointLibrary when the code that + // generates the item type is fixed. + if i.ItemType == OneDriveItem || i.ItemType == SharePointLibrary { + return updateFolderWithinDrive(SharePointLibrary, i.DriveName, i.DriveID, f) + } + + return clues.New("unsupported non-SharePoint ItemType").With("item_type", i.ItemType) } // OneDriveInfo describes a oneDrive item @@ -860,10 +931,34 @@ func (i *OneDriveInfo) UpdateParentPath(newLocPath *path.Builder) { i.ParentPath = newLocPath.PopFront().String() } -func (i *OneDriveInfo) uniqueLocation(baseLoc *path.Builder) (LocationIDer, error) { +func (i *OneDriveInfo) uniqueLocation(baseLoc *path.Builder) (*uniqueLoc, error) { if len(i.DriveID) == 0 { return nil, clues.New("empty drive ID") } - return NewOneDriveLocationIDer(i.DriveID, baseLoc.Elements()...), nil + loc := NewOneDriveLocationIDer(i.DriveID, baseLoc.Elements()...) + + return &loc, nil +} + +func (i *OneDriveInfo) updateFolder(f *FolderInfo) error { + return updateFolderWithinDrive(OneDriveItem, i.DriveName, i.DriveID, f) +} + +func updateFolderWithinDrive( + t ItemType, + driveName, driveID string, + f *FolderInfo, +) error { + if len(driveName) == 0 { + return clues.New("empty drive name") + } else if len(driveID) == 0 { + return clues.New("empty drive ID") + } + + f.DriveName = driveName + f.DriveID = driveID + f.DataType = t + + return nil } diff --git a/src/pkg/backup/details/details_test.go b/src/pkg/backup/details/details_test.go index 93620decb..b0c421954 100644 --- a/src/pkg/backup/details/details_test.go +++ b/src/pkg/backup/details/details_test.go @@ -173,6 +173,549 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { } } +func exchangeEntry(t *testing.T, id string, size int, it ItemType) DetailsEntry { + rr := makeItemPath( + t, + path.ExchangeService, + path.EmailCategory, + "tenant-id", + "user-id", + []string{"Inbox", "folder1", id}) + + return DetailsEntry{ + RepoRef: rr.String(), + ShortRef: rr.ShortRef(), + ParentRef: rr.ToBuilder().Dir().ShortRef(), + LocationRef: rr.Folder(true), + ItemInfo: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: it, + Modified: time.Now(), + Size: int64(size), + }, + }, + } +} + +func oneDriveishEntry(t *testing.T, id string, size int, it ItemType) DetailsEntry { + service := path.OneDriveService + category := path.FilesCategory + info := ItemInfo{ + OneDrive: &OneDriveInfo{ + ItemName: "bar", + DriveID: "drive-id", + DriveName: "drive-name", + Modified: time.Now(), + ItemType: it, + Size: int64(size), + }, + } + + if it == SharePointLibrary { + service = path.SharePointService + category = path.LibrariesCategory + + info.OneDrive = nil + info.SharePoint = &SharePointInfo{ + ItemName: "bar", + DriveID: "drive-id", + DriveName: "drive-name", + Modified: time.Now(), + ItemType: it, + Size: int64(size), + } + } + + rr := makeItemPath( + t, + service, + category, + "tenant-id", + "user-id", + []string{ + "drives", + "drive-id", + "root:", + "Inbox", + "folder1", + id, + }) + + loc := path.Builder{}.Append(rr.Folders()...).PopFront().PopFront() + + return DetailsEntry{ + RepoRef: rr.String(), + ShortRef: rr.ShortRef(), + ParentRef: rr.ToBuilder().Dir().ShortRef(), + LocationRef: loc.String(), + ItemInfo: info, + } +} + +func (suite *DetailsUnitSuite) TestDetailsAdd_NoLocationFolders() { + t := suite.T() + table := []struct { + name string + entry DetailsEntry + // shortRefEqual allows checking that OneDrive and SharePoint have their + // ShortRef updated in the returned entry. + // + // TODO(ashmrtn): Remove this when we don't need extra munging for + // OneDrive/SharePoint file name changes. + shortRefEqual assert.ComparisonAssertionFunc + }{ + { + name: "Exchange Email", + entry: exchangeEntry(t, "foo", 42, ExchangeMail), + shortRefEqual: assert.Equal, + }, + { + name: "OneDrive File", + entry: oneDriveishEntry(t, "foo", 42, OneDriveItem), + shortRefEqual: assert.NotEqual, + }, + { + name: "SharePoint File", + entry: oneDriveishEntry(t, "foo", 42, SharePointLibrary), + shortRefEqual: assert.NotEqual, + }, + { + name: "Legacy SharePoint File", + entry: func() DetailsEntry { + res := oneDriveishEntry(t, "foo", 42, SharePointLibrary) + res.SharePoint.ItemType = OneDriveItem + + return res + }(), + shortRefEqual: assert.NotEqual, + }, + } + + for _, test := range table { + for _, updated := range []bool{false, true} { + suite.Run(fmt.Sprintf("%s Updated %v", test.name, updated), func() { + t := suite.T() + + rr, err := path.FromDataLayerPath(test.entry.RepoRef, true) + require.NoError(t, err, clues.ToCore(err)) + + db := &Builder{} + + // Make a local copy so we can modify it. + localItem := test.entry + + err = db.Add(rr, &path.Builder{}, updated, localItem.ItemInfo) + require.NoError(t, err, clues.ToCore(err)) + + // Clear LocationRef that's automatically populated since we passed an + // empty builder above. + localItem.LocationRef = "" + localItem.Updated = updated + + expectedShortRef := localItem.ShortRef + localItem.ShortRef = "" + + deets := db.Details() + assert.Len(t, deets.Entries, 1) + + got := deets.Entries[0] + gotShortRef := got.ShortRef + got.ShortRef = "" + + assert.Equal(t, localItem, got, "DetailsEntry") + test.shortRefEqual(t, expectedShortRef, gotShortRef, "ShortRef") + }) + } + } +} + +func (suite *DetailsUnitSuite) TestDetailsAdd_LocationFolders() { + t := suite.T() + + exchangeMail1 := exchangeEntry(t, "foo1", 42, ExchangeMail) + oneDrive1 := oneDriveishEntry(t, "foo1", 42, OneDriveItem) + sharePoint1 := oneDriveishEntry(t, "foo1", 42, SharePointLibrary) + sharePointLegacy1 := oneDriveishEntry(t, "foo1", 42, SharePointLibrary) + sharePointLegacy1.SharePoint.ItemType = OneDriveItem + + // Sleep for a little so we get a larger difference in mod times between the + // earlier and later entries. + time.Sleep(100 * time.Millisecond) + + // Get fresh item IDs so we can check that folders populate with the latest + // mod time. Also the details API is built with the idea that duplicate items + // aren't added (it has no checking for that). + exchangeMail2 := exchangeEntry(t, "foo2", 43, ExchangeMail) + exchangeContact1 := exchangeEntry(t, "foo3", 44, ExchangeContact) + + exchangeFolders := []DetailsEntry{ + { + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + DisplayName: "Inbox", + ItemType: FolderItem, + DataType: ExchangeMail, + }, + }, + }, + { + LocationRef: "Inbox", + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + DisplayName: "folder1", + ItemType: FolderItem, + DataType: ExchangeMail, + }, + }, + }, + } + + exchangeContactFolders := []DetailsEntry{ + { + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + DisplayName: "Inbox", + ItemType: FolderItem, + DataType: ExchangeContact, + }, + }, + }, + { + LocationRef: "Inbox", + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + DisplayName: "folder1", + ItemType: FolderItem, + DataType: ExchangeContact, + }, + }, + }, + } + + oneDriveishFolders := []DetailsEntry{ + { + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + DisplayName: "root:", + ItemType: FolderItem, + DriveName: "drive-name", + DriveID: "drive-id", + }, + }, + }, + { + LocationRef: "root:", + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + DisplayName: "Inbox", + ItemType: FolderItem, + DriveName: "drive-name", + DriveID: "drive-id", + }, + }, + }, + { + LocationRef: "root:/Inbox", + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ + DisplayName: "folder1", + ItemType: FolderItem, + DriveName: "drive-name", + DriveID: "drive-id", + }, + }, + }, + } + + table := []struct { + name string + entries func() []DetailsEntry + expectedDirs func() []DetailsEntry + }{ + { + name: "One Exchange Email None Updated", + entries: func() []DetailsEntry { + e := exchangeMail1 + ei := *exchangeMail1.Exchange + e.Exchange = &ei + + return []DetailsEntry{e} + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range exchangeFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.Size = exchangeMail1.Exchange.Size + e.Folder.Modified = exchangeMail1.Exchange.Modified + + res = append(res, e) + } + + return res + }, + }, + { + name: "One Exchange Email Updated", + entries: func() []DetailsEntry { + e := exchangeMail1 + ei := *exchangeMail1.Exchange + e.Exchange = &ei + e.Updated = true + + return []DetailsEntry{e} + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range exchangeFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.Size = exchangeMail1.Exchange.Size + e.Folder.Modified = exchangeMail1.Exchange.Modified + e.Updated = true + + res = append(res, e) + } + + return res + }, + }, + { + name: "Two Exchange Emails None Updated", + entries: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range []DetailsEntry{exchangeMail1, exchangeMail2} { + e := entry + ei := *entry.Exchange + e.Exchange = &ei + + res = append(res, e) + } + + return res + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range exchangeFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.Size = exchangeMail1.Exchange.Size + exchangeMail2.Exchange.Size + e.Folder.Modified = exchangeMail2.Exchange.Modified + + res = append(res, e) + } + + return res + }, + }, + { + name: "Two Exchange Emails One Updated", + entries: func() []DetailsEntry { + res := []DetailsEntry{} + + for i, entry := range []DetailsEntry{exchangeMail1, exchangeMail2} { + e := entry + ei := *entry.Exchange + e.Exchange = &ei + e.Updated = i == 1 + + res = append(res, e) + } + + return res + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range exchangeFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.Size = exchangeMail1.Exchange.Size + exchangeMail2.Exchange.Size + e.Folder.Modified = exchangeMail2.Exchange.Modified + e.Updated = true + + res = append(res, e) + } + + return res + }, + }, + { + name: "One Email And One Contact None Updated", + entries: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range []DetailsEntry{exchangeMail1, exchangeContact1} { + e := entry + ei := *entry.Exchange + e.Exchange = &ei + + res = append(res, e) + } + + return res + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range exchangeFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.Size = exchangeMail1.Exchange.Size + e.Folder.Modified = exchangeMail1.Exchange.Modified + + res = append(res, e) + } + + for _, entry := range exchangeContactFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.Size = exchangeContact1.Exchange.Size + e.Folder.Modified = exchangeContact1.Exchange.Modified + + res = append(res, e) + } + + return res + }, + }, + { + name: "One OneDrive Item None Updated", + entries: func() []DetailsEntry { + e := oneDrive1 + ei := *oneDrive1.OneDrive + e.OneDrive = &ei + + return []DetailsEntry{e} + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range oneDriveishFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.DataType = OneDriveItem + e.Folder.Size = oneDrive1.OneDrive.Size + e.Folder.Modified = oneDrive1.OneDrive.Modified + + res = append(res, e) + } + + return res + }, + }, + { + name: "One SharePoint Item None Updated", + entries: func() []DetailsEntry { + e := sharePoint1 + ei := *sharePoint1.SharePoint + e.SharePoint = &ei + + return []DetailsEntry{e} + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range oneDriveishFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.DataType = SharePointLibrary + e.Folder.Size = sharePoint1.SharePoint.Size + e.Folder.Modified = sharePoint1.SharePoint.Modified + + res = append(res, e) + } + + return res + }, + }, + { + name: "One SharePoint Legacy Item None Updated", + entries: func() []DetailsEntry { + e := sharePoint1 + ei := *sharePoint1.SharePoint + e.SharePoint = &ei + + return []DetailsEntry{e} + }, + expectedDirs: func() []DetailsEntry { + res := []DetailsEntry{} + + for _, entry := range oneDriveishFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.DataType = SharePointLibrary + e.Folder.Size = sharePoint1.SharePoint.Size + e.Folder.Modified = sharePoint1.SharePoint.Modified + + res = append(res, e) + } + + return res + }, + }, + } + + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + db := &Builder{} + + for _, entry := range test.entries() { + rr, err := path.FromDataLayerPath(entry.RepoRef, true) + require.NoError(t, err, clues.ToCore(err)) + + loc, err := path.Builder{}.SplitUnescapeAppend(entry.LocationRef) + require.NoError(t, err, clues.ToCore(err)) + + err = db.Add(rr, loc, entry.Updated, entry.ItemInfo) + require.NoError(t, err, clues.ToCore(err)) + } + + deets := db.Details() + gotDirs := []DetailsEntry{} + + for _, entry := range deets.Entries { + // Other test checks items are populated properly. + if entry.infoType() != FolderItem { + continue + } + + // Not Comparing these right now. + entry.RepoRef = "" + entry.ShortRef = "" + entry.ParentRef = "" + + gotDirs = append(gotDirs, entry) + } + + assert.ElementsMatch(t, test.expectedDirs(), gotDirs) + }) + } +} + var pathItemsTable = []struct { name string ents []DetailsEntry @@ -369,125 +912,6 @@ func (suite *DetailsUnitSuite) TestDetailsModel_FilterMetaFiles() { assert.Len(t, d.Entries, 3) } -func (suite *DetailsUnitSuite) TestDetails_Add_ShortRefs() { - itemNames := []string{ - "item1", - "item2", - } - - table := []struct { - name string - service path.ServiceType - category path.CategoryType - itemInfoFunc func(name string) ItemInfo - expectedUniqueRefs int - }{ - { - name: "OneDrive", - service: path.OneDriveService, - category: path.FilesCategory, - itemInfoFunc: func(name string) ItemInfo { - return ItemInfo{ - OneDrive: &OneDriveInfo{ - ItemType: OneDriveItem, - ItemName: name, - }, - } - }, - expectedUniqueRefs: len(itemNames), - }, - { - name: "SharePoint", - service: path.SharePointService, - category: path.LibrariesCategory, - itemInfoFunc: func(name string) ItemInfo { - return ItemInfo{ - SharePoint: &SharePointInfo{ - ItemType: SharePointLibrary, - ItemName: name, - }, - } - }, - expectedUniqueRefs: len(itemNames), - }, - { - name: "SharePoint List", - service: path.SharePointService, - category: path.ListsCategory, - itemInfoFunc: func(name string) ItemInfo { - return ItemInfo{ - SharePoint: &SharePointInfo{ - ItemType: SharePointList, - ItemName: name, - }, - } - }, - // Should all end up as the starting shortref. - expectedUniqueRefs: 1, - }, - { - name: "Exchange no change", - service: path.ExchangeService, - category: path.EmailCategory, - itemInfoFunc: func(name string) ItemInfo { - return ItemInfo{ - Exchange: &ExchangeInfo{ - ItemType: ExchangeMail, - Sender: "a-person@foo.com", - Subject: name, - Recipient: []string{"another-person@bar.com"}, - }, - } - }, - // Should all end up as the starting shortref. - expectedUniqueRefs: 1, - }, - } - - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - b := Builder{} - - for _, name := range itemNames { - item := test.itemInfoFunc(name) - itemPath := makeItemPath( - suite.T(), - test.service, - test.category, - "a-tenant", - "a-user", - []string{ - "drive-id", - "root:", - "folder", - name + "-id", - }, - ) - - require.NoError(t, b.Add( - itemPath.String(), - "deadbeef", - itemPath.ToBuilder().Dir().String(), - itemPath.String(), - false, - item, - )) - } - - deets := b.Details() - shortRefs := map[string]struct{}{} - - for _, d := range deets.Items() { - shortRefs[d.ShortRef] = struct{}{} - } - - assert.Len(t, shortRefs, test.expectedUniqueRefs, "items don't have unique ShortRefs") - }) - } -} - func (suite *DetailsUnitSuite) TestDetails_Add_ShortRefs_Unique_From_Folder() { t := suite.T() @@ -495,8 +919,10 @@ func (suite *DetailsUnitSuite) TestDetails_Add_ShortRefs_Unique_From_Folder() { name := "itemName" info := ItemInfo{ OneDrive: &OneDriveInfo{ - ItemType: OneDriveItem, - ItemName: name, + ItemType: OneDriveItem, + ItemName: name, + DriveName: "drive-name", + DriveID: "drive-id", }, } @@ -530,10 +956,9 @@ func (suite *DetailsUnitSuite) TestDetails_Add_ShortRefs_Unique_From_Folder() { ) err := b.Add( - itemPath.String(), - "deadbeef", - itemPath.ToBuilder().Dir().String(), - itemPath.String(), + itemPath, + // Don't need to generate folders for this entry we just want the ShortRef. + &path.Builder{}, false, info) require.NoError(t, err) @@ -546,294 +971,6 @@ func (suite *DetailsUnitSuite) TestDetails_Add_ShortRefs_Unique_From_Folder() { assert.NotEqual(t, otherItemPath.ShortRef(), items[0].ShortRef, "same ShortRef as subfolder item") } -func (suite *DetailsUnitSuite) TestDetails_AddFolders() { - itemTime := time.Date(2022, 10, 21, 10, 0, 0, 0, time.UTC) - folderTimeOlderThanItem := time.Date(2022, 9, 21, 10, 0, 0, 0, time.UTC) - folderTimeNewerThanItem := time.Date(2022, 11, 21, 10, 0, 0, 0, time.UTC) - - itemInfo := ItemInfo{ - Exchange: &ExchangeInfo{ - Size: 20, - Modified: itemTime, - }, - } - - table := []struct { - name string - folders []folderEntry - expectedShortRefs []string - expectedFolderInfo map[string]FolderInfo - }{ - { - name: "MultipleFolders", - folders: []folderEntry{ - { - RepoRef: "rr1", - ShortRef: "sr1", - ParentRef: "pr1", - LocationRef: "lr1", - Info: ItemInfo{ - Folder: &FolderInfo{ - Modified: folderTimeOlderThanItem, - }, - }, - }, - { - RepoRef: "rr2", - ShortRef: "sr2", - ParentRef: "pr2", - LocationRef: "lr2", - Info: ItemInfo{ - Folder: &FolderInfo{ - Modified: folderTimeNewerThanItem, - }, - }, - }, - }, - expectedShortRefs: []string{"sr1", "sr2"}, - expectedFolderInfo: map[string]FolderInfo{ - "sr1": {Size: 20, Modified: itemTime}, - "sr2": {Size: 20, Modified: folderTimeNewerThanItem}, - }, - }, - { - name: "MultipleFoldersWithRepeats", - folders: []folderEntry{ - { - RepoRef: "rr1", - ShortRef: "sr1", - ParentRef: "pr1", - LocationRef: "lr1", - Info: ItemInfo{ - Folder: &FolderInfo{ - Modified: folderTimeOlderThanItem, - }, - }, - }, - { - RepoRef: "rr2", - ShortRef: "sr2", - ParentRef: "pr2", - LocationRef: "lr2", - Info: ItemInfo{ - Folder: &FolderInfo{ - Modified: folderTimeOlderThanItem, - }, - }, - }, - { - RepoRef: "rr1", - ShortRef: "sr1", - ParentRef: "pr1", - LocationRef: "lr1", - Info: ItemInfo{ - Folder: &FolderInfo{ - Modified: folderTimeOlderThanItem, - }, - }, - }, - { - RepoRef: "rr3", - ShortRef: "sr3", - ParentRef: "pr3", - LocationRef: "lr3", - Info: ItemInfo{ - Folder: &FolderInfo{ - Modified: folderTimeNewerThanItem, - }, - }, - }, - }, - expectedShortRefs: []string{"sr1", "sr2", "sr3"}, - expectedFolderInfo: map[string]FolderInfo{ - // Two items were added - "sr1": {Size: 40, Modified: itemTime}, - "sr2": {Size: 20, Modified: itemTime}, - "sr3": {Size: 20, Modified: folderTimeNewerThanItem}, - }, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - builder := Builder{} - builder.AddFoldersForItem(test.folders, itemInfo, true) - deets := builder.Details() - assert.Len(t, deets.Entries, len(test.expectedShortRefs)) - - for _, e := range deets.Entries { - assert.Contains(t, test.expectedShortRefs, e.ShortRef) - assert.Equal(t, test.expectedFolderInfo[e.ShortRef].Size, e.Folder.Size) - assert.Equal(t, test.expectedFolderInfo[e.ShortRef].Modified, e.Folder.Modified) - } - }) - } -} - -func (suite *DetailsUnitSuite) TestDetails_AddFoldersUpdate() { - itemInfo := ItemInfo{ - Exchange: &ExchangeInfo{}, - } - - table := []struct { - name string - folders []folderEntry - itemUpdated bool - expectedFolderUpdatedValue map[string]bool - }{ - { - name: "ItemNotUpdated_NoChange", - folders: []folderEntry{ - { - RepoRef: "rr1", - ShortRef: "sr1", - ParentRef: "pr1", - LocationRef: "lr1", - Info: ItemInfo{ - Folder: &FolderInfo{}, - }, - Updated: true, - }, - { - RepoRef: "rr2", - ShortRef: "sr2", - ParentRef: "pr2", - LocationRef: "lr2", - Info: ItemInfo{ - Folder: &FolderInfo{}, - }, - }, - }, - itemUpdated: false, - expectedFolderUpdatedValue: map[string]bool{ - "sr1": true, - "sr2": false, - }, - }, - { - name: "ItemUpdated", - folders: []folderEntry{ - { - RepoRef: "rr1", - ShortRef: "sr1", - ParentRef: "pr1", - LocationRef: "lr1", - Info: ItemInfo{ - Folder: &FolderInfo{}, - }, - }, - { - RepoRef: "rr2", - ShortRef: "sr2", - ParentRef: "pr2", - LocationRef: "lr2", - Info: ItemInfo{ - Folder: &FolderInfo{}, - }, - }, - }, - itemUpdated: true, - expectedFolderUpdatedValue: map[string]bool{ - "sr1": true, - "sr2": true, - }, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - builder := Builder{} - builder.AddFoldersForItem(test.folders, itemInfo, test.itemUpdated) - deets := builder.Details() - assert.Len(t, deets.Entries, len(test.expectedFolderUpdatedValue)) - - for _, e := range deets.Entries { - assert.Equalf( - t, - test.expectedFolderUpdatedValue[e.ShortRef], - e.Updated, "%s updated value incorrect", - e.ShortRef) - } - }) - } -} - -func (suite *DetailsUnitSuite) TestDetails_AddFoldersDifferentServices() { - itemTime := time.Date(2022, 10, 21, 10, 0, 0, 0, time.UTC) - - table := []struct { - name string - item ItemInfo - expectedFolderInfo FolderInfo - }{ - { - name: "Exchange", - item: ItemInfo{ - Exchange: &ExchangeInfo{ - Size: 20, - Modified: itemTime, - }, - }, - expectedFolderInfo: FolderInfo{ - Size: 20, - Modified: itemTime, - }, - }, - { - name: "OneDrive", - item: ItemInfo{ - OneDrive: &OneDriveInfo{ - Size: 20, - Modified: itemTime, - }, - }, - expectedFolderInfo: FolderInfo{ - Size: 20, - Modified: itemTime, - }, - }, - { - name: "SharePoint", - item: ItemInfo{ - SharePoint: &SharePointInfo{ - Size: 20, - Modified: itemTime, - }, - }, - expectedFolderInfo: FolderInfo{ - Size: 20, - Modified: itemTime, - }, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - folder := folderEntry{ - RepoRef: "rr1", - ShortRef: "sr1", - ParentRef: "pr1", - LocationRef: "lr1", - Info: ItemInfo{ - Folder: &FolderInfo{}, - }, - } - - builder := Builder{} - builder.AddFoldersForItem([]folderEntry{folder}, test.item, true) - deets := builder.Details() - require.Len(t, deets.Entries, 1) - - got := deets.Entries[0].Folder - - assert.Equal(t, test.expectedFolderInfo, *got) - }) - } -} - func makeItemPath( t *testing.T, service path.ServiceType, @@ -963,161 +1100,6 @@ func (suite *DetailsUnitSuite) TestUpdateItem() { } } -var ( - basePath = path.Builder{}.Append("ten", "serv", "user", "type") - baseFolderEnts = []folderEntry{ - { - RepoRef: basePath.String(), - ShortRef: basePath.ShortRef(), - ParentRef: basePath.Dir().ShortRef(), - LocationRef: "", - Info: ItemInfo{ - Folder: &FolderInfo{ - ItemType: FolderItem, - DisplayName: "type", - }, - }, - }, - { - RepoRef: basePath.Dir().String(), - ShortRef: basePath.Dir().ShortRef(), - ParentRef: basePath.Dir().Dir().ShortRef(), - LocationRef: "", - Info: ItemInfo{ - Folder: &FolderInfo{ - ItemType: FolderItem, - DisplayName: "user", - }, - }, - }, - { - RepoRef: basePath.Dir().Dir().String(), - ShortRef: basePath.Dir().Dir().ShortRef(), - ParentRef: basePath.Dir().Dir().Dir().ShortRef(), - LocationRef: "", - Info: ItemInfo{ - Folder: &FolderInfo{ - ItemType: FolderItem, - DisplayName: "serv", - }, - }, - }, - { - RepoRef: basePath.Dir().Dir().Dir().String(), - ShortRef: basePath.Dir().Dir().Dir().ShortRef(), - ParentRef: "", - LocationRef: "", - Info: ItemInfo{ - Folder: &FolderInfo{ - ItemType: FolderItem, - DisplayName: "ten", - }, - }, - }, - } -) - -func folderEntriesFor(pathElems []string, locElems []string) []folderEntry { - p := basePath.Append(pathElems...) - l := path.Builder{}.Append(locElems...) - - ents := make([]folderEntry, 0, len(pathElems)+4) - - for range pathElems { - dn := p.LastElem() - if l != nil && len(l.Elements()) > 0 { - dn = l.LastElem() - } - - fe := folderEntry{ - RepoRef: p.String(), - ShortRef: p.ShortRef(), - ParentRef: p.Dir().ShortRef(), - LocationRef: l.String(), - Info: ItemInfo{ - Folder: &FolderInfo{ - ItemType: FolderItem, - DisplayName: dn, - }, - }, - } - - l = l.Dir() - p = p.Dir() - - ents = append(ents, fe) - } - - return append(ents, baseFolderEnts...) -} - -func (suite *DetailsUnitSuite) TestFolderEntriesForPath() { - var ( - fnords = []string{"fnords"} - smarf = []string{"fnords", "smarf"} - beau = []string{"beau"} - regard = []string{"beau", "regard"} - ) - - table := []struct { - name string - parent *path.Builder - location *path.Builder - expect []folderEntry - }{ - { - name: "base path, parent only", - parent: basePath, - expect: baseFolderEnts, - }, - { - name: "single depth parent only", - parent: basePath.Append(fnords...), - expect: folderEntriesFor(fnords, nil), - }, - { - name: "single depth with location", - parent: basePath.Append(fnords...), - location: path.Builder{}.Append(beau...), - expect: folderEntriesFor(fnords, beau), - }, - { - name: "two depth parent only", - parent: basePath.Append(smarf...), - expect: folderEntriesFor(smarf, nil), - }, - { - name: "two depth with location", - parent: basePath.Append(smarf...), - location: path.Builder{}.Append(regard...), - expect: folderEntriesFor(smarf, regard), - }, - { - name: "mismatched depth, parent longer", - parent: basePath.Append(smarf...), - location: path.Builder{}.Append(beau...), - expect: folderEntriesFor(smarf, beau), - }, - // We can't handle this right now. But we don't have any cases - // which immediately require it, either. Keeping in the test - // as a reminder that this might be required at some point. - // { - // name: "mismatched depth, location longer", - // parent: basePath.Append(fnords...), - // location: basePath.Append(regard...), - // expect: folderEntriesFor(fnords, regard), - // }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - result := FolderEntriesForPath(test.parent, test.location) - assert.ElementsMatch(t, test.expect, result) - }) - } -} - func (suite *DetailsUnitSuite) TestDetails_Marshal() { for _, test := range pathItemsTable { suite.Run(test.name, func() { diff --git a/src/pkg/repository/repository_unexported_test.go b/src/pkg/repository/repository_unexported_test.go index 9f7416615..e29350f6e 100644 --- a/src/pkg/repository/repository_unexported_test.go +++ b/src/pkg/repository/repository_unexported_test.go @@ -20,6 +20,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/store" "github.com/alcionai/corso/src/pkg/store/mock" @@ -335,13 +336,18 @@ func (suite *RepositoryModelIntgSuite) TestGetBackupDetails() { ) info := details.ItemInfo{ - Folder: &details.FolderInfo{ - DisplayName: "test", + Exchange: &details.ExchangeInfo{ + ItemType: details.ExchangeMail, }, } + repoPath, err := path.FromDataLayerPath(tenantID+"/exchange/user-id/email/test/foo", true) + require.NoError(suite.T(), err, clues.ToCore(err)) + + loc := path.Builder{}.Append(repoPath.Folders()...) + builder := &details.Builder{} - require.NoError(suite.T(), builder.Add("ref", "short", "pref", "lref", false, info)) + require.NoError(suite.T(), builder.Add(repoPath, loc, false, info)) table := []struct { name string @@ -411,15 +417,19 @@ func (suite *RepositoryModelIntgSuite) TestGetBackupErrors() { item = fault.FileErr(err, "file-id", "file-name", map[string]any{"foo": "bar"}) skip = fault.FileSkip(fault.SkipMalware, "s-file-id", "s-file-name", map[string]any{"foo": "bar"}) info = details.ItemInfo{ - Folder: &details.FolderInfo{ - DisplayName: "test", + Exchange: &details.ExchangeInfo{ + ItemType: details.ExchangeMail, }, } ) - builder := &details.Builder{} + repoPath, err2 := path.FromDataLayerPath(tenantID+"/exchange/user-id/email/test/foo", true) + require.NoError(suite.T(), err2, clues.ToCore(err2)) - require.NoError(suite.T(), builder.Add("ref", "short", "pref", "lref", false, info)) + loc := path.Builder{}.Append(repoPath.Folders()...) + + builder := &details.Builder{} + require.NoError(suite.T(), builder.Add(repoPath, loc, false, info)) table := []struct { name string