From d1e86264e97033e4cdc67e3c588bd269ac96f6fe Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Mon, 13 Mar 2023 10:41:35 -0700 Subject: [PATCH] Rename folder metadata files to just `.dirmeta` (#2758) Removing the folder name from the dirmeta file name plays better with upcoming incremental backup changes because we no longer have to discover what the old name of the metadata file was Bumps the Corso backup version number Contains minor changes to how tests detect root folder's metadata --- #### 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: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * closes #2754 #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../connector/graph_connector_helper_test.go | 12 +++++++----- .../connector/graph_connector_onedrive_test.go | 13 ++++++++++--- src/internal/connector/onedrive/collection.go | 5 ++++- src/internal/connector/onedrive/permission.go | 10 ++++++---- src/internal/version/backup.go | 7 ++++++- 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index 4e6ee0969..cae2d1ac1 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -682,12 +682,12 @@ func compareOneDriveItem( t *testing.T, expected map[string][]byte, item data.Stream, - dest control.RestoreDestination, restorePermissions bool, + rootDir bool, ) bool { // Skip OneDrive permissions in the folder that used to be the root. We don't // have a good way to materialize these in the test right now. - if item.UUID() == dest.ContainerName+onedrive.DirMetaFileSuffix { + if rootDir && item.UUID() == onedrive.DirMetaFileSuffix { return false } @@ -777,8 +777,8 @@ func compareItem( service path.ServiceType, category path.CategoryType, item data.Stream, - dest control.RestoreDestination, restorePermissions bool, + rootDir bool, ) bool { if mt, ok := item.(data.StreamModTime); ok { assert.NotZero(t, mt.ModTime()) @@ -798,7 +798,7 @@ func compareItem( } case path.OneDriveService: - return compareOneDriveItem(t, expected, item, dest, restorePermissions) + return compareOneDriveItem(t, expected, item, restorePermissions, rootDir) default: assert.FailNowf(t, "unexpected service: %s", service.String()) @@ -850,6 +850,8 @@ func checkCollections( service = returned.FullPath().Service() category = returned.FullPath().Category() expectedColData = expected[returned.FullPath().String()] + folders = returned.FullPath().Elements() + rootDir = folders[len(folders)-1] == dest.ContainerName ) // Need to iterate through all items even if we don't expect to find a match @@ -875,7 +877,7 @@ func checkCollections( continue } - if !compareItem(t, expectedColData, service, category, item, dest, restorePermissions) { + if !compareItem(t, expectedColData, service, category, item, restorePermissions, rootDir) { gotItems-- } } diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 1d002d716..4d3ec0024 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -192,7 +192,8 @@ func (c *onedriveCollection) withFile(name string, fileData []byte, perm permDat name+onedrive.DataFileSuffix, fileData)) - case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker, version.OneDrive4DirIncludesPermissions: + case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker, + version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName: c.items = append(c.items, onedriveItemWithData( c.t, name+onedrive.DataFileSuffix, @@ -217,7 +218,7 @@ func (c *onedriveCollection) withFile(name string, fileData []byte, perm permDat func (c *onedriveCollection) withFolder(name string, perm permData) *onedriveCollection { switch c.backupVersion { - case 0, version.OneDrive4DirIncludesPermissions: + case 0, version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName: return c case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker: @@ -247,6 +248,12 @@ func (c *onedriveCollection) withPermissions(perm permData) *onedriveCollection } name := c.pathElements[len(c.pathElements)-1] + metaName := name + + if c.backupVersion >= version.OneDrive5DirMetaNoName { + // We switched to just .dirmeta for metadata file names. + metaName = "" + } if name == "root:" { return c @@ -255,7 +262,7 @@ func (c *onedriveCollection) withPermissions(perm permData) *onedriveCollection metadata := onedriveMetadata( c.t, name, - name+onedrive.DirMetaFileSuffix, + metaName+onedrive.DirMetaFileSuffix, perm, c.backupVersion >= versionPermissionSwitchedToID) diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index d5470315c..bd9ab3e2d 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -339,6 +339,7 @@ func (oc *Collection) populateItems(ctx context.Context, errs *fault.Bus) { itemInfo details.ItemInfo itemMeta io.ReadCloser itemMetaSize int + metaFileName string metaSuffix string err error ) @@ -356,10 +357,12 @@ func (oc *Collection) populateItems(ctx context.Context, errs *fault.Bus) { if isFile { atomic.AddInt64(&itemsFound, 1) + metaFileName = itemName metaSuffix = MetaFileSuffix } else { atomic.AddInt64(&dirsFound, 1) + // metaFileName not set for directories so we get just ".dirmeta" metaSuffix = DirMetaFileSuffix } @@ -464,7 +467,7 @@ func (oc *Collection) populateItems(ctx context.Context, errs *fault.Bus) { }) oc.data <- &metadataItem{ - id: itemName + metaSuffix, + id: metaFileName + metaSuffix, data: metaReader, modTime: time.Now(), } diff --git a/src/internal/connector/onedrive/permission.go b/src/internal/connector/onedrive/permission.go index bb880defd..1ea9cf2c0 100644 --- a/src/internal/connector/onedrive/permission.go +++ b/src/internal/connector/onedrive/permission.go @@ -68,11 +68,13 @@ func getCollectionMetadata( // Root folder doesn't have a metadata file associated with it. folders := collectionPath.Folders() + metaName := folders[len(folders)-1] + DirMetaFileSuffix - meta, err := fetchAndReadMetadata( - ctx, - dc, - folders[len(folders)-1]+DirMetaFileSuffix) + if backupVersion >= version.OneDrive5DirMetaNoName { + metaName = DirMetaFileSuffix + } + + meta, err := fetchAndReadMetadata(ctx, dc, metaName) if err != nil { return Metadata{}, clues.Wrap(err, "collection metadata") } diff --git a/src/internal/version/backup.go b/src/internal/version/backup.go index a3afdc8c1..e99e7627b 100644 --- a/src/internal/version/backup.go +++ b/src/internal/version/backup.go @@ -2,7 +2,7 @@ package version import "math" -const Backup = 4 +const Backup = 5 // Various labels to refer to important version changes. // Labels don't need 1:1 service:version representation. Add a new @@ -25,6 +25,11 @@ const ( // collection as the folder itself. OneDrive4DirIncludesPermissions = 4 + // OneDrive5DirMetaNoName changed the directory metadata file name from + // .dirmeta to just .dirmeta to avoid issues with folder renames + // during incremental backups. + OneDrive5DirMetaNoName = 5 + // OneDriveXNameInMeta points to the backup format version where we begin // storing files in kopia with their item ID instead of their OneDrive file // name.