diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ff1ca07a0..ddf9ecde0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Handle OneDrive folders being deleted and recreated midway through a backup +- Automatically re-run a full delta query on incrmental if the prior backup is found to have malformed prior-state information. ## [v0.15.0] (beta) - 2023-10-31 diff --git a/src/internal/m365/collection/drive/collections.go b/src/internal/m365/collection/drive/collections.go index 75ae09731c..962a672d42 100644 --- a/src/internal/m365/collection/drive/collections.go +++ b/src/internal/m365/collection/drive/collections.go @@ -72,7 +72,88 @@ func NewCollections( } } -func deserializeMetadata( +func deserializeAndValidateMetadata( + ctx context.Context, + cols []data.RestoreCollection, + fb *fault.Bus, +) (map[string]string, map[string]map[string]string, bool, error) { + deltas, prevs, canUse, err := DeserializeMetadata(ctx, cols) + if err != nil || !canUse { + return deltas, prevs, false, clues.Stack(err).OrNil() + } + + // Go through and remove delta tokens if we didn't have any paths for them + // or one or more paths are empty (incorrect somehow). This will ensure we + // don't accidentally try to pull in delta results when we should have + // enumerated everything instead. + // + // Loop over the set of previous deltas because it's alright to have paths + // without a delta but not to have a delta without paths. This way ensures + // we check at least all the path sets for the deltas we have. + for drive := range deltas { + ictx := clues.Add(ctx, "drive_id", drive) + + paths := prevs[drive] + if len(paths) == 0 { + logger.Ctx(ictx).Info("dropping drive delta due to 0 prev paths") + delete(deltas, drive) + } + + // Drives have only a single delta token. If we find any folder that + // seems like the path is bad we need to drop the entire token and start + // fresh. Since we know the token will be gone we can also stop checking + // for other possibly incorrect folder paths. + for _, prevPath := range paths { + if len(prevPath) == 0 { + logger.Ctx(ictx).Info("dropping drive delta due to 0 len path") + delete(deltas, drive) + + break + } + } + } + + alertIfPrevPathsHaveCollisions(ctx, prevs, fb) + + return deltas, prevs, canUse, nil +} + +func alertIfPrevPathsHaveCollisions( + ctx context.Context, + prevs map[string]map[string]string, + fb *fault.Bus, +) { + for driveID, folders := range prevs { + prevPathCollisions := map[string]string{} + + for fid, prev := range folders { + if otherID, collision := prevPathCollisions[prev]; collision { + ctx = clues.Add( + ctx, + "collision_folder_id_1", fid, + "collision_folder_id_2", otherID, + "collision_drive_id", driveID, + "collision_prev_path", path.LoggableDir(prev)) + + fb.AddAlert(ctx, fault.NewAlert( + fault.AlertPreviousPathCollision, + "", // no namespace + "", // no item id + "previousPaths", + map[string]any{ + "collision_folder_id_1": fid, + "collision_folder_id_2": otherID, + "collision_drive_id": driveID, + "collision_prev_path": prev, + })) + } + + prevPathCollisions[prev] = fid + } + } +} + +func DeserializeMetadata( ctx context.Context, cols []data.RestoreCollection, ) (map[string]string, map[string]map[string]string, bool, error) { @@ -137,32 +218,6 @@ func deserializeMetadata( } } } - - // Go through and remove delta tokens if we didn't have any paths for them - // or one or more paths are empty (incorrect somehow). This will ensure we - // don't accidentally try to pull in delta results when we should have - // enumerated everything instead. - // - // Loop over the set of previous deltas because it's alright to have paths - // without a delta but not to have a delta without paths. This way ensures - // we check at least all the path sets for the deltas we have. - for drive := range prevDeltas { - paths := prevFolders[drive] - if len(paths) == 0 { - delete(prevDeltas, drive) - } - - // Drives have only a single delta token. If we find any folder that - // seems like the path is bad we need to drop the entire token and start - // fresh. Since we know the token will be gone we can also stop checking - // for other possibly incorrect folder paths. - for _, prevPath := range paths { - if len(prevPath) == 0 { - delete(prevDeltas, drive) - break - } - } - } } // if reads from items failed, return empty but no error @@ -215,7 +270,7 @@ func (c *Collections) Get( ssmb *prefixmatcher.StringSetMatchBuilder, errs *fault.Bus, ) ([]data.BackupCollection, bool, error) { - prevDriveIDToDelta, oldPrevPathsByDriveID, canUsePrevBackup, err := deserializeMetadata(ctx, prevMetadata) + deltasByDriveID, prevPathsByDriveID, canUsePrevBackup, err := deserializeAndValidateMetadata(ctx, prevMetadata, errs) if err != nil { return nil, false, err } @@ -224,7 +279,7 @@ func (c *Collections) Get( driveTombstones := map[string]struct{}{} - for driveID := range oldPrevPathsByDriveID { + for driveID := range prevPathsByDriveID { driveTombstones[driveID] = struct{}{} } @@ -257,8 +312,8 @@ func (c *Collections) Get( "drive_name", clues.Hide(driveName)) excludedItemIDs = map[string]struct{}{} - oldPrevPaths = oldPrevPathsByDriveID[driveID] - prevDeltaLink = prevDriveIDToDelta[driveID] + oldPrevPaths = prevPathsByDriveID[driveID] + prevDeltaLink = deltasByDriveID[driveID] // packagePaths is keyed by folder paths to a parent directory // which is marked as a package by its driveItem GetPackage @@ -426,6 +481,8 @@ func (c *Collections) Get( collections = append(collections, coll) } + alertIfPrevPathsHaveCollisions(ctx, driveIDToPrevPaths, errs) + // add metadata collections pathPrefix, err := c.handler.MetadataPathPrefix(c.tenantID) if err != nil { @@ -1012,13 +1069,13 @@ func includePath(ctx context.Context, dsc dirScopeChecker, folderPath path.Path) } func updatePath(paths map[string]string, id, newPath string) { - oldPath := paths[id] - if len(oldPath) == 0 { + currPath := paths[id] + if len(currPath) == 0 { paths[id] = newPath return } - if oldPath == newPath { + if currPath == newPath { return } @@ -1027,10 +1084,10 @@ func updatePath(paths map[string]string, id, newPath string) { // other components should take care of that. We do need to ensure that the // resulting map contains all folders though so we know the next time around. for folderID, p := range paths { - if !strings.HasPrefix(p, oldPath) { + if !strings.HasPrefix(p, currPath) { continue } - paths[folderID] = strings.Replace(p, oldPath, newPath, 1) + paths[folderID] = strings.Replace(p, currPath, newPath, 1) } } diff --git a/src/internal/m365/collection/drive/collections_test.go b/src/internal/m365/collection/drive/collections_test.go index 6dc378727d..92ac1cbd93 100644 --- a/src/internal/m365/collection/drive/collections_test.go +++ b/src/internal/m365/collection/drive/collections_test.go @@ -1084,6 +1084,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata() { cols []func() []graph.MetadataCollectionEntry expectedDeltas map[string]string expectedPaths map[string]map[string]string + expectedAlerts []string canUsePreviousBackup bool errCheck assert.ErrorAssertionFunc }{ @@ -1375,6 +1376,90 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata() { canUsePreviousBackup: false, errCheck: assert.NoError, }, + { + name: "DuplicatePreviousPaths", + cols: []func() []graph.MetadataCollectionEntry{ + func() []graph.MetadataCollectionEntry { + return []graph.MetadataCollectionEntry{ + graph.NewMetadataEntry( + bupMD.DeltaURLsFileName, + map[string]string{driveID1: deltaURL1}), + graph.NewMetadataEntry( + bupMD.PreviousPathFileName, + map[string]map[string]string{ + driveID1: { + folderID1: path1, + folderID2: path1, + }, + }), + } + }, + }, + expectedDeltas: map[string]string{ + driveID1: deltaURL1, + }, + expectedPaths: map[string]map[string]string{ + driveID1: { + folderID1: path1, + folderID2: path1, + }, + }, + expectedAlerts: []string{fault.AlertPreviousPathCollision}, + canUsePreviousBackup: true, + errCheck: assert.NoError, + }, + { + name: "DuplicatePreviousPaths_separateDrives", + cols: []func() []graph.MetadataCollectionEntry{ + func() []graph.MetadataCollectionEntry { + return []graph.MetadataCollectionEntry{ + graph.NewMetadataEntry( + bupMD.DeltaURLsFileName, + map[string]string{ + driveID1: deltaURL1, + }), + graph.NewMetadataEntry( + bupMD.PreviousPathFileName, + map[string]map[string]string{ + driveID1: { + folderID1: path1, + folderID2: path1, + }, + }), + } + }, + func() []graph.MetadataCollectionEntry { + return []graph.MetadataCollectionEntry{ + graph.NewMetadataEntry( + bupMD.DeltaURLsFileName, + map[string]string{driveID2: deltaURL2}), + graph.NewMetadataEntry( + bupMD.PreviousPathFileName, + map[string]map[string]string{ + driveID2: { + folderID1: path1, + }, + }), + } + }, + }, + expectedDeltas: map[string]string{ + driveID1: deltaURL1, + driveID2: deltaURL2, + }, + expectedPaths: map[string]map[string]string{ + driveID1: { + folderID1: path1, + folderID2: path1, + }, + driveID2: { + folderID1: path1, + }, + }, + expectedAlerts: []string{fault.AlertPreviousPathCollision}, + canUsePreviousBackup: true, + errCheck: assert.NoError, + }, } for _, test := range table { @@ -1406,12 +1491,22 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata() { data.NoFetchRestoreCollection{Collection: mc})) } - deltas, paths, canUsePreviousBackup, err := deserializeMetadata(ctx, cols) + fb := fault.New(true) + + deltas, paths, canUsePreviousBackup, err := deserializeAndValidateMetadata(ctx, cols, fb) test.errCheck(t, err) assert.Equal(t, test.canUsePreviousBackup, canUsePreviousBackup, "can use previous backup") assert.Equal(t, test.expectedDeltas, deltas, "deltas") assert.Equal(t, test.expectedPaths, paths, "paths") + + alertMsgs := []string{} + + for _, alert := range fb.Alerts() { + alertMsgs = append(alertMsgs, alert.Message) + } + + assert.ElementsMatch(t, test.expectedAlerts, alertMsgs, "alert messages") }) } } @@ -1439,7 +1534,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata_ReadFailure() fc := failingColl{} - _, _, canUsePreviousBackup, err := deserializeMetadata(ctx, []data.RestoreCollection{fc}) + _, _, canUsePreviousBackup, err := deserializeAndValidateMetadata(ctx, []data.RestoreCollection{fc}, fault.New(true)) require.NoError(t, err) require.False(t, canUsePreviousBackup) } @@ -1493,12 +1588,12 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { enumerator mock.EnumerateItemsDeltaByDrive canUsePreviousBackup bool errCheck assert.ErrorAssertionFunc - prevFolderPaths map[string]map[string]string + previousPaths map[string]map[string]string // Collection name -> set of item IDs. We can't check item data because // that's not mocked out. Metadata is checked separately. - expectedCollections map[string]map[data.CollectionState][]string - expectedDeltaURLs map[string]string - expectedFolderPaths map[string]map[string]string + expectedCollections map[string]map[data.CollectionState][]string + expectedDeltaURLs map[string]string + expectedPreviousPaths map[string]map[string]string // Items that should be excluded from the base. Only populated if the delta // was valid and there was at least 1 previous folder path. expectedDelList *pmMock.PrefixMap @@ -1524,7 +1619,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {"root": rootFolderPath1}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -1533,7 +1628,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: {"root": rootFolderPath1}, }, expectedDelList: pmMock.NewPrefixMap(map[string]map[string]struct{}{ @@ -1558,7 +1653,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {"root": rootFolderPath1}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -1567,7 +1662,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: {"root": rootFolderPath1}, }, expectedDelList: pmMock.NewPrefixMap(map[string]map[string]struct{}{ @@ -1593,7 +1688,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{}, + previousPaths: map[string]map[string]string{}, expectedCollections: map[string]map[data.CollectionState][]string{ rootFolderPath1: {data.NewState: {}}, folderPath1: {data.NewState: {"folder", "file"}}, @@ -1601,7 +1696,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1633,7 +1728,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{}, + previousPaths: map[string]map[string]string{}, expectedCollections: map[string]map[data.CollectionState][]string{ rootFolderPath1: {data.NewState: {}}, folderPath1: {data.NewState: {"folder", "file"}}, @@ -1641,7 +1736,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1671,7 +1766,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -1683,7 +1778,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1710,7 +1805,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -1718,7 +1813,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { folderPath1: {data.NewState: {"folder", "file"}}, }, expectedDeltaURLs: map[string]string{}, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1758,7 +1853,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -1768,7 +1863,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1820,7 +1915,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -1830,7 +1925,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1878,7 +1973,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -1888,7 +1983,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1928,7 +2023,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, driveID2: {}, }, @@ -1942,7 +2037,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { driveID1: delta, driveID2: delta2, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -1988,7 +2083,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, driveID2: {}, }, @@ -2002,7 +2097,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { driveID1: delta, driveID2: delta2, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2034,13 +2129,13 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: false, errCheck: assert.Error, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, - expectedCollections: nil, - expectedDeltaURLs: nil, - expectedFolderPaths: nil, - expectedDelList: nil, + expectedCollections: nil, + expectedDeltaURLs: nil, + expectedPreviousPaths: nil, + expectedDelList: nil, }, { name: "OneDrive_OneItemPage_InvalidPrevDelta_DeleteNonExistentFolder", @@ -2067,7 +2162,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2081,7 +2176,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder2": expectedPath1("/folder2"), @@ -2119,7 +2214,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2133,7 +2228,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder2": expectedPath1("/folder2"), @@ -2182,7 +2277,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2200,7 +2295,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder2": expectedPath1("/folder"), @@ -2237,7 +2332,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2255,7 +2350,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder2": expectedPath1("/folder"), @@ -2297,7 +2392,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -2307,7 +2402,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2355,7 +2450,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2370,7 +2465,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta2, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2402,7 +2497,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2415,7 +2510,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -2447,7 +2542,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -2458,7 +2553,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -2496,7 +2591,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -2505,7 +2600,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta2, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -2550,7 +2645,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -2560,7 +2655,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta2, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder1": folderPath1, @@ -2607,7 +2702,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2620,7 +2715,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta2, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -2663,7 +2758,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2676,7 +2771,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta2, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder1": folderPath1, @@ -2715,7 +2810,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -2725,7 +2820,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, "folder": folderPath1, @@ -2753,7 +2848,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -2762,7 +2857,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -2788,7 +2883,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ @@ -2797,7 +2892,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { expectedDeltaURLs: map[string]string{ driveID1: delta, }, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: { "root": rootFolderPath1, }, @@ -2822,7 +2917,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }, canUsePreviousBackup: true, errCheck: assert.NoError, - prevFolderPaths: map[string]map[string]string{ + previousPaths: map[string]map[string]string{ driveID1: {"root": rootFolderPath1}, driveID2: {"root": rootFolderPath2}, }, @@ -2831,7 +2926,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { rootFolderPath2: {data.DeletedState: {}}, }, expectedDeltaURLs: map[string]string{driveID1: delta}, - expectedFolderPaths: map[string]map[string]string{ + expectedPreviousPaths: map[string]map[string]string{ driveID1: {"root": rootFolderPath1}, }, expectedDelList: pmMock.NewPrefixMap(map[string]map[string]struct{}{}), @@ -2839,6 +2934,268 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { rootFolderPath2: true, }, }, + { + name: "duplicate previous paths in metadata", + drives: []models.Driveable{drive1, drive2}, + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + // contains duplicates in previousPath + driveID1: { + Pages: []mock.NextPage{{ + Items: []models.DriveItemable{ + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false), + driveItem("folder2", "folder2", driveBasePath1, "root", false, true, false), + driveItem("file2", "file2", driveBasePath1+"/folder2", "folder2", true, false, false), + }, + }}, + DeltaUpdate: pagers.DeltaUpdate{URL: delta}, + }, + // does not contain duplicates + driveID2: { + Pages: []mock.NextPage{{ + Items: []models.DriveItemable{ + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath2, "root", false, true, false), + driveItem("file", "file", driveBasePath2+"/folder", "folder", true, false, false), + driveItem("folder2", "folder2", driveBasePath2, "root", false, true, false), + driveItem("file2", "file2", driveBasePath2+"/folder2", "folder2", true, false, false), + }, + }}, + DeltaUpdate: pagers.DeltaUpdate{URL: delta2}, + }, + }, + }, + canUsePreviousBackup: true, + errCheck: assert.NoError, + previousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "folder": rootFolderPath1 + "/folder", + "folder2": rootFolderPath1 + "/folder", + "folder3": rootFolderPath1 + "/folder", + }, + driveID2: { + "root": rootFolderPath2, + "folder": rootFolderPath2 + "/folder", + "folder2": rootFolderPath2 + "/folder2", + }, + }, + expectedCollections: map[string]map[data.CollectionState][]string{ + rootFolderPath1: { + data.NewState: {"folder", "folder2"}, + }, + rootFolderPath1 + "/folder": { + data.NotMovedState: {"folder", "file"}, + }, + rootFolderPath1 + "/folder2": { + data.MovedState: {"folder2", "file2"}, + }, + rootFolderPath2: { + data.NewState: {"folder", "folder2"}, + }, + rootFolderPath2 + "/folder": { + data.NotMovedState: {"folder", "file"}, + }, + rootFolderPath2 + "/folder2": { + data.NotMovedState: {"folder2", "file2"}, + }, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + driveID2: delta2, + }, + expectedPreviousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "folder": rootFolderPath1 + "/folder2", // note: this is a bug, but is currently expected + "folder2": rootFolderPath1 + "/folder2", + "folder3": rootFolderPath1 + "/folder2", + }, + driveID2: { + "root": rootFolderPath2, + "folder": rootFolderPath2 + "/folder", + "folder2": rootFolderPath2 + "/folder2", + }, + }, + expectedDelList: pmMock.NewPrefixMap(map[string]map[string]struct{}{ + rootFolderPath1: makeExcludeMap("file", "file2"), + rootFolderPath2: makeExcludeMap("file", "file2"), + }), + doNotMergeItems: map[string]bool{}, + }, + { + name: "out of order item enumeration causes prev path collisions", + drives: []models.Driveable{drive1}, + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + driveID1: { + Pages: []mock.NextPage{{ + Items: []models.DriveItemable{ + driveRootItem("root"), + driveItem("fanny2", "fanny", driveBasePath1, "root", false, true, false), + driveItem("file2", "file2", driveBasePath1+"/fanny", "fanny2", true, false, false), + driveItem("nav", "nav", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/nav", "nav", true, false, false), + }, + }}, + DeltaUpdate: pagers.DeltaUpdate{URL: delta}, + }, + }, + }, + canUsePreviousBackup: true, + errCheck: assert.NoError, + previousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "nav": rootFolderPath1 + "/fanny", + }, + }, + expectedCollections: map[string]map[data.CollectionState][]string{ + rootFolderPath1: { + data.NewState: {"fanny2"}, + }, + rootFolderPath1 + "/nav": { + data.MovedState: {"nav", "file"}, + }, + rootFolderPath1 + "/fanny": { + data.NewState: {"fanny2", "file2"}, + }, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + }, + expectedPreviousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "nav": rootFolderPath1 + "/nav", + "fanny2": rootFolderPath1 + "/nav", // note: this is a bug, but currently expected + }, + }, + expectedDelList: pmMock.NewPrefixMap(map[string]map[string]struct{}{ + rootFolderPath1: makeExcludeMap("file", "file2"), + }), + doNotMergeItems: map[string]bool{}, + }, + { + name: "out of order item enumeration causes prev path collisions", + drives: []models.Driveable{drive1}, + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + driveID1: { + Pages: []mock.NextPage{{ + Items: []models.DriveItemable{ + driveRootItem("root"), + driveItem("fanny2", "fanny", driveBasePath1, "root", false, true, false), + driveItem("file2", "file2", driveBasePath1+"/fanny", "fanny2", true, false, false), + driveItem("nav", "nav", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/nav", "nav", true, false, false), + }, + }}, + DeltaUpdate: pagers.DeltaUpdate{URL: delta}, + }, + }, + }, + canUsePreviousBackup: true, + errCheck: assert.NoError, + previousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "nav": rootFolderPath1 + "/fanny", + }, + }, + expectedCollections: map[string]map[data.CollectionState][]string{ + rootFolderPath1: { + data.NewState: {"fanny2"}, + }, + rootFolderPath1 + "/nav": { + data.MovedState: {"nav", "file"}, + }, + rootFolderPath1 + "/fanny": { + data.NewState: {"fanny2", "file2"}, + }, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + }, + expectedPreviousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "nav": rootFolderPath1 + "/nav", + "fanny2": rootFolderPath1 + "/nav", // note: this is a bug, but currently expected + }, + }, + expectedDelList: pmMock.NewPrefixMap(map[string]map[string]struct{}{ + rootFolderPath1: makeExcludeMap("file", "file2"), + }), + doNotMergeItems: map[string]bool{}, + }, + { + name: "out of order item enumeration causes opposite prev path collisions", + drives: []models.Driveable{drive1}, + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + driveID1: { + Pages: []mock.NextPage{{ + Items: []models.DriveItemable{ + driveRootItem("root"), + driveItem("file1", "file1", driveBasePath1, "root", true, false, false), + driveItem("fanny", "fanny", driveBasePath1, "root", false, true, false), + driveItem("nav", "nav", driveBasePath1, "root", false, true, false), + driveItem("id1", "foo", driveBasePath1+"/fanny", "fanny", false, true, false), + driveItem("id2", "foo", driveBasePath1+"/nav", "nav", false, true, false), + }, + }}, + DeltaUpdate: pagers.DeltaUpdate{URL: delta}, + }, + }, + }, + canUsePreviousBackup: true, + errCheck: assert.NoError, + previousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "nav": rootFolderPath1 + "/nav", + "fanny": rootFolderPath1 + "/fanny", + "id1": rootFolderPath1 + "/nav/foo", + "id2": rootFolderPath1 + "/fanny/foo", + }, + }, + expectedCollections: map[string]map[data.CollectionState][]string{ + rootFolderPath1: { + data.NotMovedState: {"file1"}, + }, + rootFolderPath1 + "/nav": { + data.NotMovedState: {"nav"}, + }, + rootFolderPath1 + "/nav/foo": { + data.MovedState: {"id2"}, + }, + rootFolderPath1 + "/fanny": { + data.NotMovedState: {"fanny"}, + }, + rootFolderPath1 + "/fanny/foo": { + data.MovedState: {"id1"}, + }, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + }, + expectedPreviousPaths: map[string]map[string]string{ + driveID1: { + "root": rootFolderPath1, + "nav": rootFolderPath1 + "/nav", + "fanny": rootFolderPath1 + "/fanny", + "id1": rootFolderPath1 + "/nav/foo", // note: this is a bug, but currently expected + "id2": rootFolderPath1 + "/nav/foo", + }, + }, + expectedDelList: pmMock.NewPrefixMap(map[string]map[string]struct{}{ + rootFolderPath1: makeExcludeMap("file1"), + }), + doNotMergeItems: map[string]bool{}, + }, } for _, test := range table { suite.Run(test.name, func() { @@ -2880,7 +3237,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { }), graph.NewMetadataEntry( bupMD.PreviousPathFileName, - test.prevFolderPaths), + test.previousPaths), }, func(*support.ControllerOperationStatus) {}) assert.NoError(t, err, "creating metadata collection", clues.ToCore(err)) @@ -2893,7 +3250,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { delList := prefixmatcher.NewStringSetBuilder() cols, canUsePreviousBackup, err := c.Get(ctx, prevMetadata, delList, errs) - test.errCheck(t, err) + test.errCheck(t, err, clues.ToCore(err)) assert.Equal(t, test.canUsePreviousBackup, canUsePreviousBackup, "can use previous backup") assert.Equal(t, test.expectedSkippedCount, len(errs.Skipped())) @@ -2912,19 +3269,20 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() { } if folderPath == metadataPath.String() { - deltas, paths, _, err := deserializeMetadata( + deltas, prevs, _, err := deserializeAndValidateMetadata( ctx, []data.RestoreCollection{ dataMock.NewUnversionedRestoreCollection( t, data.NoFetchRestoreCollection{Collection: baseCol}), - }) + }, + errs) if !assert.NoError(t, err, "deserializing metadata", clues.ToCore(err)) { continue } assert.Equal(t, test.expectedDeltaURLs, deltas, "delta urls") - assert.Equal(t, test.expectedFolderPaths, paths, "folder paths") + assert.Equal(t, test.expectedPreviousPaths, prevs, "previous paths") continue } diff --git a/src/internal/m365/collection/drive/debug.go b/src/internal/m365/collection/drive/debug.go index 6895a8c521..7a4f5c46c9 100644 --- a/src/internal/m365/collection/drive/debug.go +++ b/src/internal/m365/collection/drive/debug.go @@ -7,6 +7,7 @@ import ( "github.com/alcionai/corso/src/internal/data" bupMD "github.com/alcionai/corso/src/pkg/backup/metadata" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/store" ) @@ -14,7 +15,7 @@ func DeserializeMetadataFiles( ctx context.Context, colls []data.RestoreCollection, ) ([]store.MetadataFile, error) { - deltas, prevs, _, err := deserializeMetadata(ctx, colls) + deltas, prevs, _, err := deserializeAndValidateMetadata(ctx, colls, fault.New(true)) files := []store.MetadataFile{ { diff --git a/src/internal/operations/inject/inject.go b/src/internal/operations/inject/inject.go index 9590b00f44..28713e15a9 100644 --- a/src/internal/operations/inject/inject.go +++ b/src/internal/operations/inject/inject.go @@ -3,6 +3,8 @@ package inject import ( "context" + "github.com/kopia/kopia/repo/manifest" + "github.com/alcionai/corso/src/internal/common/idname" "github.com/alcionai/corso/src/internal/common/prefixmatcher" "github.com/alcionai/corso/src/internal/data" @@ -15,7 +17,6 @@ import ( "github.com/alcionai/corso/src/pkg/export" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" - "github.com/kopia/kopia/repo/manifest" ) type ( diff --git a/src/internal/operations/manifests.go b/src/internal/operations/manifests.go index 37bc70d5c1..ecabe6d979 100644 --- a/src/internal/operations/manifests.go +++ b/src/internal/operations/manifests.go @@ -53,7 +53,7 @@ func produceManifestsAndMetadata( } // getManifestsAndMetadata calls kopia to retrieve prior backup manifests, -// metadata collections to supply backup heuristics. +// metadata collections to supply backup information. func getManifestsAndMetadata( ctx context.Context, bf inject.BaseFinder, diff --git a/src/pkg/fault/alert.go b/src/pkg/fault/alert.go index 5d4c97cea0..d7b207f8ff 100644 --- a/src/pkg/fault/alert.go +++ b/src/pkg/fault/alert.go @@ -4,6 +4,10 @@ import ( "github.com/alcionai/corso/src/cli/print" ) +const ( + AlertPreviousPathCollision = "previous_path_collision" +) + var _ print.Printable = &Alert{} // Alerts are informational-only notifications. The purpose of alerts is to diff --git a/src/pkg/fault/fault.go b/src/pkg/fault/fault.go index e6ea1bcd9b..97816405b0 100644 --- a/src/pkg/fault/fault.go +++ b/src/pkg/fault/fault.go @@ -208,7 +208,6 @@ func (e *Bus) AddAlert(ctx context.Context, a *Alert) { e.logAndAddAlert(ctx, a, 1) } -// logs the error and adds an alert. func (e *Bus) logAndAddAlert(ctx context.Context, a *Alert, trace int) { logger.CtxStack(ctx, trace+1). With("alert", a).