From ad691148fef8a9ae54b56b768a02d264d81db1d7 Mon Sep 17 00:00:00 2001 From: Vaibhav Kamra Date: Thu, 29 Dec 2022 15:41:04 -0800 Subject: [PATCH] Mark folder updated when an updated item is added (#1987) ## Description If an item is updated, update the folder entries it belongs to. Also contains a minor refactor - unexport `FolderEntry` to `folderEntry` ## Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No ## Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :hamster: Trivial/Minor ## Issue(s) * #1812 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- CHANGELOG.md | 1 + src/internal/kopia/upload.go | 6 +- src/internal/operations/backup.go | 10 +- src/pkg/backup/details/details.go | 24 +- src/pkg/backup/details/details_test.go | 300 ++++++++++++++++--------- 5 files changed, 219 insertions(+), 122 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b6d0236a..683638e57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Folder entries in backup details now indicate whether an item in the hierarchy was updated - Incremental backup support for Exchange ([#1777](https://github.com/alcionai/corso/issues/1777)). This is currently enabled by specifying the `--enable-incrementals` with the `backup create` command. This functionality will be enabled by default in an upcoming release. - Folder entries in backup details now include size and modified time for the hierarchy ([#1896](https://github.com/alcionai/corso/issues/1896)) diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index f44a4c69d..7a3bf8617 100644 --- a/src/internal/kopia/upload.go +++ b/src/internal/kopia/upload.go @@ -184,7 +184,11 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { ) folders := details.FolderEntriesForPath(parent) - cp.deets.AddFoldersForItem(folders, *d.info) + cp.deets.AddFoldersForItem( + folders, + *d.info, + true, // itemUpdated = true + ) } // Kopia interface function used as a callback when kopia finishes hashing a file. diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index cd7a18be9..9f6571213 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -515,18 +515,20 @@ func mergeDetails( ) } + // TODO(ashmrtn): This may need updated if we start using this merge + // strategry for items that were cached in kopia. + itemUpdated := newPath.String() != rr.String() + deets.Add( newPath.String(), newPath.ShortRef(), newPath.ToBuilder().Dir().ShortRef(), - // TODO(ashmrtn): This may need updated if we start using this merge - // strategry for items that were cached in kopia. - newPath.String() != rr.String(), + itemUpdated, item, ) folders := details.FolderEntriesForPath(newPath.ToBuilder().Dir()) - deets.AddFoldersForItem(folders, item) + deets.AddFoldersForItem(folders, item, itemUpdated) // Track how many entries we added so that we know if we got them all when // we're done. diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index 21f392c98..27075ec72 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -14,10 +14,11 @@ import ( "github.com/alcionai/corso/src/pkg/path" ) -type FolderEntry struct { +type folderEntry struct { RepoRef string ShortRef string ParentRef string + Updated bool Info ItemInfo } @@ -106,7 +107,7 @@ func (dm DetailsModel) Items() []*DetailsEntry { type Builder struct { d Details mu sync.Mutex `json:"-"` - knownFolders map[string]FolderEntry `json:"-"` + knownFolders map[string]folderEntry `json:"-"` } func (b *Builder) Add(repoRef, shortRef, parentRef string, updated bool, info ItemInfo) { @@ -130,13 +131,13 @@ func (b *Builder) Details() *Details { // 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 *path.Builder) []FolderEntry { - folders := []FolderEntry{} +func FolderEntriesForPath(parent *path.Builder) []folderEntry { + folders := []folderEntry{} for len(parent.Elements()) > 0 { nextParent := parent.Dir() - folders = append(folders, FolderEntry{ + folders = append(folders, folderEntry{ RepoRef: parent.String(), ShortRef: parent.ShortRef(), ParentRef: nextParent.ShortRef(), @@ -156,12 +157,12 @@ func FolderEntriesForPath(parent *path.Builder) []FolderEntry { // 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) { +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{} + b.knownFolders = map[string]folderEntry{} } for _, folder := range folders { @@ -180,6 +181,12 @@ func (b *Builder) AddFoldersForItem(folders []FolderEntry, itemInfo ItemInfo) { 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 } } @@ -206,12 +213,13 @@ func (d *Details) add(repoRef, shortRef, parentRef string, updated bool, info It } // addFolder adds an entry for the given folder. -func (d *Details) addFolder(folder FolderEntry) { +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, }) } diff --git a/src/pkg/backup/details/details_test.go b/src/pkg/backup/details/details_test.go index 90dad366c..328576e99 100644 --- a/src/pkg/backup/details/details_test.go +++ b/src/pkg/backup/details/details_test.go @@ -1,4 +1,4 @@ -package details_test +package details import ( "testing" @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/common" - "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/path" ) @@ -33,13 +32,13 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { table := []struct { name string - entry details.DetailsEntry + entry DetailsEntry expectHs []string expectVs []string }{ { name: "no info", - entry: details.DetailsEntry{ + entry: DetailsEntry{ RepoRef: "reporef", ShortRef: "deadbeef", }, @@ -48,12 +47,12 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { }, { name: "exchange event info", - entry: details.DetailsEntry{ + entry: DetailsEntry{ RepoRef: "reporef", ShortRef: "deadbeef", - ItemInfo: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeEvent, + ItemInfo: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeEvent, EventStart: now, EventEnd: now, Organizer: "organizer", @@ -67,12 +66,12 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { }, { name: "exchange contact info", - entry: details.DetailsEntry{ + entry: DetailsEntry{ RepoRef: "reporef", ShortRef: "deadbeef", - ItemInfo: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeContact, + ItemInfo: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeContact, ContactName: "contactName", }, }, @@ -82,12 +81,12 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { }, { name: "exchange mail info", - entry: details.DetailsEntry{ + entry: DetailsEntry{ RepoRef: "reporef", ShortRef: "deadbeef", - ItemInfo: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeMail, + ItemInfo: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeMail, Sender: "sender", Subject: "subject", Received: now, @@ -99,11 +98,11 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { }, { name: "sharepoint info", - entry: details.DetailsEntry{ + entry: DetailsEntry{ RepoRef: "reporef", ShortRef: "deadbeef", - ItemInfo: details.ItemInfo{ - SharePoint: &details.SharePointInfo{ + ItemInfo: ItemInfo{ + SharePoint: &SharePointInfo{ ItemName: "itemName", ParentPath: "parentPath", Size: 1000, @@ -118,11 +117,11 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { }, { name: "oneDrive info", - entry: details.DetailsEntry{ + entry: DetailsEntry{ RepoRef: "reporef", ShortRef: "deadbeef", - ItemInfo: details.ItemInfo{ - OneDrive: &details.OneDriveInfo{ + ItemInfo: ItemInfo{ + OneDrive: &OneDriveInfo{ ItemName: "itemName", ParentPath: "parentPath", Size: 1000, @@ -149,7 +148,7 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { var pathItemsTable = []struct { name string - ents []details.DetailsEntry + ents []DetailsEntry expectRefs []string }{ { @@ -159,14 +158,14 @@ var pathItemsTable = []struct { }, { name: "single entry", - ents: []details.DetailsEntry{ + ents: []DetailsEntry{ {RepoRef: "abcde"}, }, expectRefs: []string{"abcde"}, }, { name: "multiple entries", - ents: []details.DetailsEntry{ + ents: []DetailsEntry{ {RepoRef: "abcde"}, {RepoRef: "12345"}, }, @@ -174,13 +173,13 @@ var pathItemsTable = []struct { }, { name: "multiple entries with folder", - ents: []details.DetailsEntry{ + ents: []DetailsEntry{ {RepoRef: "abcde"}, {RepoRef: "12345"}, { RepoRef: "deadbeef", - ItemInfo: details.ItemInfo{ - Folder: &details.FolderInfo{ + ItemInfo: ItemInfo{ + Folder: &FolderInfo{ DisplayName: "test folder", }, }, @@ -193,8 +192,8 @@ var pathItemsTable = []struct { func (suite *DetailsUnitSuite) TestDetailsModel_Path() { for _, test := range pathItemsTable { suite.T().Run(test.name, func(t *testing.T) { - d := details.Details{ - DetailsModel: details.DetailsModel{ + d := Details{ + DetailsModel: DetailsModel{ Entries: test.ents, }, } @@ -206,8 +205,8 @@ func (suite *DetailsUnitSuite) TestDetailsModel_Path() { func (suite *DetailsUnitSuite) TestDetailsModel_Items() { for _, test := range pathItemsTable { suite.T().Run(test.name, func(t *testing.T) { - d := details.Details{ - DetailsModel: details.DetailsModel{ + d := Details{ + DetailsModel: DetailsModel{ Entries: test.ents, }, } @@ -227,8 +226,8 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { 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 := details.ItemInfo{ - Exchange: &details.ExchangeInfo{ + itemInfo := ItemInfo{ + Exchange: &ExchangeInfo{ Size: 20, Modified: itemTime, }, @@ -236,19 +235,19 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { table := []struct { name string - folders []details.FolderEntry + folders []folderEntry expectedShortRefs []string - expectedFolderInfo map[string]details.FolderInfo + expectedFolderInfo map[string]FolderInfo }{ { name: "MultipleFolders", - folders: []details.FolderEntry{ + folders: []folderEntry{ { RepoRef: "rr1", ShortRef: "sr1", ParentRef: "pr1", - Info: details.ItemInfo{ - Folder: &details.FolderInfo{ + Info: ItemInfo{ + Folder: &FolderInfo{ Modified: folderTimeOlderThanItem, }, }, @@ -257,28 +256,28 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { RepoRef: "rr2", ShortRef: "sr2", ParentRef: "pr2", - Info: details.ItemInfo{ - Folder: &details.FolderInfo{ + Info: ItemInfo{ + Folder: &FolderInfo{ Modified: folderTimeNewerThanItem, }, }, }, }, expectedShortRefs: []string{"sr1", "sr2"}, - expectedFolderInfo: map[string]details.FolderInfo{ + expectedFolderInfo: map[string]FolderInfo{ "sr1": {Size: 20, Modified: itemTime}, "sr2": {Size: 20, Modified: folderTimeNewerThanItem}, }, }, { name: "MultipleFoldersWithRepeats", - folders: []details.FolderEntry{ + folders: []folderEntry{ { RepoRef: "rr1", ShortRef: "sr1", ParentRef: "pr1", - Info: details.ItemInfo{ - Folder: &details.FolderInfo{ + Info: ItemInfo{ + Folder: &FolderInfo{ Modified: folderTimeOlderThanItem, }, }, @@ -287,8 +286,8 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { RepoRef: "rr2", ShortRef: "sr2", ParentRef: "pr2", - Info: details.ItemInfo{ - Folder: &details.FolderInfo{ + Info: ItemInfo{ + Folder: &FolderInfo{ Modified: folderTimeOlderThanItem, }, }, @@ -297,8 +296,8 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { RepoRef: "rr1", ShortRef: "sr1", ParentRef: "pr1", - Info: details.ItemInfo{ - Folder: &details.FolderInfo{ + Info: ItemInfo{ + Folder: &FolderInfo{ Modified: folderTimeOlderThanItem, }, }, @@ -307,15 +306,15 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { RepoRef: "rr3", ShortRef: "sr3", ParentRef: "pr3", - Info: details.ItemInfo{ - Folder: &details.FolderInfo{ + Info: ItemInfo{ + Folder: &FolderInfo{ Modified: folderTimeNewerThanItem, }, }, }, }, expectedShortRefs: []string{"sr1", "sr2", "sr3"}, - expectedFolderInfo: map[string]details.FolderInfo{ + expectedFolderInfo: map[string]FolderInfo{ // Two items were added "sr1": {Size: 40, Modified: itemTime}, "sr2": {Size: 20, Modified: itemTime}, @@ -325,8 +324,8 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { - builder := details.Builder{} - builder.AddFoldersForItem(test.folders, itemInfo) + builder := Builder{} + builder.AddFoldersForItem(test.folders, itemInfo, true) deets := builder.Details() assert.Len(t, deets.Entries, len(test.expectedShortRefs)) @@ -339,49 +338,132 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { } } +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", + Info: ItemInfo{ + Folder: &FolderInfo{}, + }, + Updated: true, + }, + { + RepoRef: "rr2", + ShortRef: "sr2", + ParentRef: "pr2", + Info: ItemInfo{ + Folder: &FolderInfo{}, + }, + }, + }, + itemUpdated: false, + expectedFolderUpdatedValue: map[string]bool{ + "sr1": true, + "sr2": false, + }, + }, + { + name: "ItemUpdated", + folders: []folderEntry{ + { + RepoRef: "rr1", + ShortRef: "sr1", + ParentRef: "pr1", + Info: ItemInfo{ + Folder: &FolderInfo{}, + }, + }, + { + RepoRef: "rr2", + ShortRef: "sr2", + ParentRef: "pr2", + Info: ItemInfo{ + Folder: &FolderInfo{}, + }, + }, + }, + itemUpdated: true, + expectedFolderUpdatedValue: map[string]bool{ + "sr1": true, + "sr2": true, + }, + }, + } + for _, test := range table { + suite.T().Run(test.name, func(t *testing.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 details.ItemInfo - expectedFolderInfo details.FolderInfo + item ItemInfo + expectedFolderInfo FolderInfo }{ { name: "Exchange", - item: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ + item: ItemInfo{ + Exchange: &ExchangeInfo{ Size: 20, Modified: itemTime, }, }, - expectedFolderInfo: details.FolderInfo{ + expectedFolderInfo: FolderInfo{ Size: 20, Modified: itemTime, }, }, { name: "OneDrive", - item: details.ItemInfo{ - OneDrive: &details.OneDriveInfo{ + item: ItemInfo{ + OneDrive: &OneDriveInfo{ Size: 20, Modified: itemTime, }, }, - expectedFolderInfo: details.FolderInfo{ + expectedFolderInfo: FolderInfo{ Size: 20, Modified: itemTime, }, }, { name: "SharePoint", - item: details.ItemInfo{ - SharePoint: &details.SharePointInfo{ + item: ItemInfo{ + SharePoint: &SharePointInfo{ Size: 20, Modified: itemTime, }, }, - expectedFolderInfo: details.FolderInfo{ + expectedFolderInfo: FolderInfo{ Size: 20, Modified: itemTime, }, @@ -389,17 +471,17 @@ func (suite *DetailsUnitSuite) TestDetails_AddFoldersDifferentServices() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { - folderEntry := details.FolderEntry{ + folder := folderEntry{ RepoRef: "rr1", ShortRef: "sr1", ParentRef: "pr1", - Info: details.ItemInfo{ - Folder: &details.FolderInfo{}, + Info: ItemInfo{ + Folder: &FolderInfo{}, }, } - builder := details.Builder{} - builder.AddFoldersForItem([]details.FolderEntry{folderEntry}, test.item) + builder := Builder{} + builder.AddFoldersForItem([]folderEntry{folder}, test.item, true) deets := builder.Details() require.Len(t, deets.Entries, 1) @@ -469,92 +551,92 @@ func (suite *DetailsUnitSuite) TestUpdateItem() { table := []struct { name string - input details.ItemInfo + input ItemInfo newPath path.Path errCheck assert.ErrorAssertionFunc - expectedItem details.ItemInfo + expectedItem ItemInfo }{ { name: "ExchangeEventNoChange", - input: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeEvent, + input: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeEvent, }, }, errCheck: assert.NoError, - expectedItem: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeEvent, + expectedItem: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeEvent, }, }, }, { name: "ExchangeContactNoChange", - input: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeContact, + input: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeContact, }, }, errCheck: assert.NoError, - expectedItem: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeContact, + expectedItem: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeContact, }, }, }, { name: "ExchangeMailNoChange", - input: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeMail, + input: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeMail, }, }, errCheck: assert.NoError, - expectedItem: details.ItemInfo{ - Exchange: &details.ExchangeInfo{ - ItemType: details.ExchangeMail, + expectedItem: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeMail, }, }, }, { name: "OneDrive", - input: details.ItemInfo{ - OneDrive: &details.OneDriveInfo{ - ItemType: details.OneDriveItem, + input: ItemInfo{ + OneDrive: &OneDriveInfo{ + ItemType: OneDriveItem, ParentPath: folder1, }, }, newPath: newOneDrivePath, errCheck: assert.NoError, - expectedItem: details.ItemInfo{ - OneDrive: &details.OneDriveInfo{ - ItemType: details.OneDriveItem, + expectedItem: ItemInfo{ + OneDrive: &OneDriveInfo{ + ItemType: OneDriveItem, ParentPath: folder2, }, }, }, { name: "SharePoint", - input: details.ItemInfo{ - SharePoint: &details.SharePointInfo{ - ItemType: details.SharePointItem, + input: ItemInfo{ + SharePoint: &SharePointInfo{ + ItemType: SharePointItem, ParentPath: folder1, }, }, newPath: newOneDrivePath, errCheck: assert.NoError, - expectedItem: details.ItemInfo{ - SharePoint: &details.SharePointInfo{ - ItemType: details.SharePointItem, + expectedItem: ItemInfo{ + SharePoint: &SharePointInfo{ + ItemType: SharePointItem, ParentPath: folder2, }, }, }, { name: "OneDriveBadPath", - input: details.ItemInfo{ - OneDrive: &details.OneDriveInfo{ - ItemType: details.OneDriveItem, + input: ItemInfo{ + OneDrive: &OneDriveInfo{ + ItemType: OneDriveItem, ParentPath: folder1, }, }, @@ -563,9 +645,9 @@ func (suite *DetailsUnitSuite) TestUpdateItem() { }, { name: "SharePointBadPath", - input: details.ItemInfo{ - SharePoint: &details.SharePointInfo{ - ItemType: details.SharePointItem, + input: ItemInfo{ + SharePoint: &SharePointInfo{ + ItemType: SharePointItem, ParentPath: folder1, }, }, @@ -577,7 +659,7 @@ func (suite *DetailsUnitSuite) TestUpdateItem() { for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { item := test.input - err := details.UpdateItem(&item, test.newPath) + err := UpdateItem(&item, test.newPath) test.errCheck(t, err) if err != nil {