From 07b5acb92d9fbdf8518036b87a4d3ec3f35f803e Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Wed, 3 May 2023 18:51:53 -0700 Subject: [PATCH] Temporary solution to duplicate Exchange folders (#3300) For Exchange email or contacts backup, pick on the folder with the highest ID in the hopes that it will be the most recently used one A better solution for this situation should be coming soon --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [x] :clock1: Yes, but in a later PR - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * #3197 #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../connector/exchange/service_iterators.go | 51 ++ .../exchange/service_iterators_test.go | 600 +++++++++++++++++- 2 files changed, 639 insertions(+), 12 deletions(-) diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index dc2fa42ca..bab7bd5a8 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -51,6 +51,10 @@ func filterContainersAndFillCollections( // deleted from this map, leaving only the deleted folders behind tombstones = makeTombstones(dps) category = qp.Category + + // Stop-gap: Track folders by LocationPath and if there's duplicates pick + // the one with the lexicographically larger ID. + dupPaths = map[string]string{} ) logger.Ctx(ctx).Infow("filling collections", "len_deltapaths", len(dps)) @@ -99,6 +103,53 @@ func filterContainersAndFillCollections( continue } + // This is a duplicate collection. Either the collection we're examining now + // should be skipped or the collection we previously added should be + // skipped. + // + // Calendars is already using folder IDs so we don't need to pick the + // "newest" folder for that. + if oldCID := dupPaths[locPath.String()]; category != path.EventsCategory && len(oldCID) > 0 { + if cID < oldCID { + logger.Ctx(ictx).Infow( + "skipping duplicate folder with lesser ID", + "previous_folder_id", clues.Hide(oldCID), + "current_folder_id", clues.Hide(cID), + "duplicate_path", locPath) + + // Readd this entry to the tombstone map because we remove it first off. + if oldDP, ok := dps[cID]; ok { + tombstones[cID] = oldDP.path + } + + // Continuing here ensures we don't add anything to the paths map or the + // delta map which is the behavior we want. + continue + } + + logger.Ctx(ictx).Infow( + "switching duplicate folders as newer folder found", + "previous_folder_id", clues.Hide(oldCID), + "current_folder_id", clues.Hide(cID), + "duplicate_path", locPath) + + // Remove the previous collection from the maps. This will make us think + // it's a new item and properly populate it if it ever: + // * moves + // * replaces the current entry (current entry moves/is deleted) + delete(collections, oldCID) + delete(deltaURLs, oldCID) + delete(currPaths, oldCID) + + // Re-add the tombstone entry for the old folder so that it can be marked + // as deleted if need. + if oldDP, ok := dps[oldCID]; ok { + tombstones[oldCID] = oldDP.path + } + } + + dupPaths[locPath.String()] = cID + if len(prevPathStr) > 0 { if prevPath, err = pathFromPrevString(prevPathStr); err != nil { logger.CtxErr(ictx, err).Error("parsing prev path") diff --git a/src/internal/connector/exchange/service_iterators_test.go b/src/internal/connector/exchange/service_iterators_test.go index 7c5b3593c..5c1e14d90 100644 --- a/src/internal/connector/exchange/service_iterators_test.go +++ b/src/internal/connector/exchange/service_iterators_test.go @@ -192,18 +192,6 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { expectNewColls: 2, expectMetadataColls: 1, }, - { - name: "happy path, many containers, same display name", - getter: map[string]mockGetterResults{ - "1": commonResult, - "2": commonResult, - }, - resolver: newMockResolver(container1, container2), - scope: allScope, - expectErr: assert.NoError, - expectNewColls: 2, - expectMetadataColls: 1, - }, { name: "no containers pass scope", getter: map[string]mockGetterResults{ @@ -370,6 +358,594 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { } } +func checkMetadata( + t *testing.T, + ctx context.Context, //revive:disable-line:context-as-argument + cat path.CategoryType, + expect DeltaPaths, + c data.BackupCollection, +) { + catPaths, err := parseMetadataCollections( + ctx, + []data.RestoreCollection{data.NotFoundRestoreCollection{Collection: c}}, + fault.New(true)) + if !assert.NoError(t, err, "getting metadata", clues.ToCore(err)) { + return + } + + assert.Equal(t, expect, catPaths[cat]) +} + +func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_DuplicateFolders() { + type scopeCat struct { + scope selectors.ExchangeScope + cat path.CategoryType + } + + var ( + qp = graph.QueryParams{ + ResourceOwner: inMock.NewProvider("user_id", "user_name"), + Credentials: suite.creds, + } + statusUpdater = func(*support.ConnectorOperationStatus) {} + + dataTypes = []scopeCat{ + { + scope: selectors.NewExchangeBackup(nil).MailFolders(selectors.Any())[0], + cat: path.EmailCategory, + }, + { + scope: selectors.NewExchangeBackup(nil).ContactFolders(selectors.Any())[0], + cat: path.ContactsCategory, + }, + } + + location = path.Builder{}.Append("foo", "bar") + + result1 = mockGetterResults{ + added: []string{"a1", "a2", "a3"}, + removed: []string{"r1", "r2", "r3"}, + newDelta: api.DeltaUpdate{URL: "delta_url"}, + } + result2 = mockGetterResults{ + added: []string{"a4", "a5", "a6"}, + removed: []string{"r4", "r5", "r6"}, + newDelta: api.DeltaUpdate{URL: "delta_url2"}, + } + + container1 = mockContainer{ + id: strPtr("1"), + displayName: strPtr("bar"), + p: path.Builder{}.Append("1"), + l: location, + } + container2 = mockContainer{ + id: strPtr("2"), + displayName: strPtr("bar"), + p: path.Builder{}.Append("2"), + l: location, + } + ) + + oldPath1 := func(t *testing.T, cat path.CategoryType) path.Path { + res, err := location.Append("1").ToDataLayerPath( + suite.creds.AzureTenantID, + qp.ResourceOwner.ID(), + path.ExchangeService, + cat, + false) + require.NoError(t, err, clues.ToCore(err)) + + return res + } + + oldPath2 := func(t *testing.T, cat path.CategoryType) path.Path { + res, err := location.Append("2").ToDataLayerPath( + suite.creds.AzureTenantID, + qp.ResourceOwner.ID(), + path.ExchangeService, + cat, + false) + require.NoError(t, err, clues.ToCore(err)) + + return res + } + + locPath := func(t *testing.T, cat path.CategoryType) path.Path { + res, err := location.ToDataLayerPath( + suite.creds.AzureTenantID, + qp.ResourceOwner.ID(), + path.ExchangeService, + cat, + false) + require.NoError(t, err, clues.ToCore(err)) + + return res + } + + table := []struct { + name string + getter mockGetter + resolver graph.ContainerResolver + inputMetadata func(t *testing.T, cat path.CategoryType) DeltaPaths + expectNewColls int + expectDeleted int + expectAdded []string + expectRemoved []string + expectMetadata func(t *testing.T, cat path.CategoryType) DeltaPaths + }{ + { + name: "1 moved to duplicate", + getter: map[string]mockGetterResults{ + "1": result1, + "2": result2, + }, + resolver: newMockResolver(container1, container2), + inputMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "1": DeltaPath{ + delta: "old_delta", + path: oldPath1(t, cat).String(), + }, + "2": DeltaPath{ + delta: "old_delta", + path: locPath(t, cat).String(), + }, + } + }, + expectDeleted: 1, + expectAdded: result2.added, + expectRemoved: result2.removed, + expectMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "2": DeltaPath{ + delta: "delta_url2", + path: locPath(t, cat).String(), + }, + } + }, + }, + { + name: "1 moved to duplicate, other order", + getter: map[string]mockGetterResults{ + "1": result1, + "2": result2, + }, + resolver: newMockResolver(container2, container1), + inputMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "1": DeltaPath{ + delta: "old_delta", + path: oldPath1(t, cat).String(), + }, + "2": DeltaPath{ + delta: "old_delta", + path: locPath(t, cat).String(), + }, + } + }, + expectDeleted: 1, + expectAdded: result2.added, + expectRemoved: result2.removed, + expectMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "2": DeltaPath{ + delta: "delta_url2", + path: locPath(t, cat).String(), + }, + } + }, + }, + { + name: "both move to duplicate", + getter: map[string]mockGetterResults{ + "1": result1, + "2": result2, + }, + resolver: newMockResolver(container1, container2), + inputMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "1": DeltaPath{ + delta: "old_delta", + path: oldPath1(t, cat).String(), + }, + "2": DeltaPath{ + delta: "old_delta", + path: oldPath2(t, cat).String(), + }, + } + }, + expectDeleted: 1, + expectAdded: result2.added, + expectRemoved: result2.removed, + expectMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "2": DeltaPath{ + delta: "delta_url2", + path: locPath(t, cat).String(), + }, + } + }, + }, + { + name: "both new", + getter: map[string]mockGetterResults{ + "1": result1, + "2": result2, + }, + resolver: newMockResolver(container1, container2), + inputMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{} + }, + expectNewColls: 1, + expectAdded: result2.added, + expectRemoved: result2.removed, + expectMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "2": DeltaPath{ + delta: "delta_url2", + path: locPath(t, cat).String(), + }, + } + }, + }, + { + name: "add 1 remove 2", + getter: map[string]mockGetterResults{ + "1": result1, + }, + resolver: newMockResolver(container1), + inputMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "2": DeltaPath{ + delta: "old_delta", + path: locPath(t, cat).String(), + }, + } + }, + expectNewColls: 1, + expectDeleted: 1, + expectAdded: result1.added, + expectRemoved: result1.removed, + expectMetadata: func(t *testing.T, cat path.CategoryType) DeltaPaths { + return DeltaPaths{ + "1": DeltaPath{ + delta: "delta_url", + path: locPath(t, cat).String(), + }, + } + }, + }, + } + + for _, sc := range dataTypes { + suite.Run(sc.cat.String(), func() { + qp.Category = sc.cat + + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext() + defer flush() + + collections := map[string]data.BackupCollection{} + + err := filterContainersAndFillCollections( + ctx, + qp, + test.getter, + collections, + statusUpdater, + test.resolver, + sc.scope, + test.inputMetadata(t, sc.cat), + control.Options{FailureHandling: control.FailFast}, + fault.New(true)) + require.NoError(t, err, "getting collections", clues.ToCore(err)) + + // collection assertions + + deleteds, news, metadatas := 0, 0, 0 + for _, c := range collections { + if c.State() == data.DeletedState { + deleteds++ + continue + } + + if c.FullPath().Service() == path.ExchangeMetadataService { + metadatas++ + checkMetadata(t, ctx, sc.cat, test.expectMetadata(t, sc.cat), c) + continue + } + + if c.State() == data.NewState { + news++ + } + + exColl, ok := c.(*Collection) + require.True(t, ok, "collection is an *exchange.Collection") + + if exColl.LocationPath() != nil { + assert.Equal(t, location.String(), exColl.LocationPath().String()) + } + + ids := [][]string{ + make([]string, 0, len(exColl.added)), + make([]string, 0, len(exColl.removed)), + } + + for i, cIDs := range []map[string]struct{}{exColl.added, exColl.removed} { + for id := range cIDs { + ids[i] = append(ids[i], id) + } + } + + assert.ElementsMatch(t, test.expectAdded, ids[0], "added items") + assert.ElementsMatch(t, test.expectRemoved, ids[1], "removed items") + } + + assert.Equal(t, test.expectDeleted, deleteds, "deleted collections") + assert.Equal(t, test.expectNewColls, news, "new collections") + assert.Equal(t, 1, metadatas, "metadata collections") + }) + } + }) + } +} + +func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_DuplicateFolders_Events() { + var ( + qp = graph.QueryParams{ + ResourceOwner: inMock.NewProvider("user_id", "user_name"), + Category: path.EventsCategory, + Credentials: suite.creds, + } + statusUpdater = func(*support.ConnectorOperationStatus) {} + + scope = selectors.NewExchangeBackup(nil).EventCalendars(selectors.Any())[0] + + location = path.Builder{}.Append("foo", "bar") + + result1 = mockGetterResults{ + added: []string{"a1", "a2", "a3"}, + removed: []string{"r1", "r2", "r3"}, + newDelta: api.DeltaUpdate{URL: "delta_url"}, + } + result2 = mockGetterResults{ + added: []string{"a4", "a5", "a6"}, + removed: []string{"r4", "r5", "r6"}, + newDelta: api.DeltaUpdate{URL: "delta_url2"}, + } + + container1 = mockContainer{ + id: strPtr("1"), + displayName: strPtr("bar"), + p: path.Builder{}.Append("1"), + l: location, + } + container2 = mockContainer{ + id: strPtr("2"), + displayName: strPtr("bar"), + p: path.Builder{}.Append("2"), + l: location, + } + ) + + oldPath1, err := location.Append("1").ToDataLayerPath( + suite.creds.AzureTenantID, + qp.ResourceOwner.ID(), + path.ExchangeService, + qp.Category, + false) + require.NoError(suite.T(), err, clues.ToCore(err)) + + oldPath2, err := location.Append("2").ToDataLayerPath( + suite.creds.AzureTenantID, + qp.ResourceOwner.ID(), + path.ExchangeService, + qp.Category, + false) + require.NoError(suite.T(), err, clues.ToCore(err)) + + idPath1, err := path.Builder{}.Append("1").ToDataLayerPath( + suite.creds.AzureTenantID, + qp.ResourceOwner.ID(), + path.ExchangeService, + qp.Category, + false) + require.NoError(suite.T(), err, clues.ToCore(err)) + + idPath2, err := path.Builder{}.Append("2").ToDataLayerPath( + suite.creds.AzureTenantID, + qp.ResourceOwner.ID(), + path.ExchangeService, + qp.Category, + false) + require.NoError(suite.T(), err, clues.ToCore(err)) + + table := []struct { + name string + getter mockGetter + resolver graph.ContainerResolver + inputMetadata DeltaPaths + expectNewColls int + expectDeleted int + expectMetadata DeltaPaths + }{ + { + name: "1 moved to duplicate", + getter: map[string]mockGetterResults{ + "1": result1, + "2": result2, + }, + resolver: newMockResolver(container1, container2), + inputMetadata: DeltaPaths{ + "1": DeltaPath{ + delta: "old_delta", + path: oldPath1.String(), + }, + "2": DeltaPath{ + delta: "old_delta", + path: idPath2.String(), + }, + }, + expectMetadata: DeltaPaths{ + "1": DeltaPath{ + delta: "delta_url", + path: idPath1.String(), + }, + "2": DeltaPath{ + delta: "delta_url2", + path: idPath2.String(), + }, + }, + }, + { + name: "both move to duplicate", + getter: map[string]mockGetterResults{ + "1": result1, + "2": result2, + }, + resolver: newMockResolver(container1, container2), + inputMetadata: DeltaPaths{ + "1": DeltaPath{ + delta: "old_delta", + path: oldPath1.String(), + }, + "2": DeltaPath{ + delta: "old_delta", + path: oldPath2.String(), + }, + }, + expectMetadata: DeltaPaths{ + "1": DeltaPath{ + delta: "delta_url", + path: idPath1.String(), + }, + "2": DeltaPath{ + delta: "delta_url2", + path: idPath2.String(), + }, + }, + }, + { + name: "both new", + getter: map[string]mockGetterResults{ + "1": result1, + "2": result2, + }, + resolver: newMockResolver(container1, container2), + inputMetadata: DeltaPaths{}, + expectNewColls: 2, + expectMetadata: DeltaPaths{ + "1": DeltaPath{ + delta: "delta_url", + path: idPath1.String(), + }, + "2": DeltaPath{ + delta: "delta_url2", + path: idPath2.String(), + }, + }, + }, + { + name: "add 1 remove 2", + getter: map[string]mockGetterResults{ + "1": result1, + }, + resolver: newMockResolver(container1), + inputMetadata: DeltaPaths{ + "2": DeltaPath{ + delta: "old_delta", + path: idPath2.String(), + }, + }, + expectNewColls: 1, + expectDeleted: 1, + expectMetadata: DeltaPaths{ + "1": DeltaPath{ + delta: "delta_url", + path: idPath1.String(), + }, + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext() + defer flush() + + collections := map[string]data.BackupCollection{} + + err := filterContainersAndFillCollections( + ctx, + qp, + test.getter, + collections, + statusUpdater, + test.resolver, + scope, + test.inputMetadata, + control.Options{FailureHandling: control.FailFast}, + fault.New(true)) + require.NoError(t, err, "getting collections", clues.ToCore(err)) + + // collection assertions + + deleteds, news, metadatas := 0, 0, 0 + for _, c := range collections { + if c.State() == data.DeletedState { + deleteds++ + continue + } + + if c.FullPath().Service() == path.ExchangeMetadataService { + metadatas++ + checkMetadata(t, ctx, qp.Category, test.expectMetadata, c) + continue + } + + if c.State() == data.NewState { + news++ + } + } + + assert.Equal(t, test.expectDeleted, deleteds, "deleted collections") + assert.Equal(t, test.expectNewColls, news, "new collections") + assert.Equal(t, 1, metadatas, "metadata collections") + + // items in collections assertions + for k, expect := range test.getter { + coll := collections[k] + + if coll == nil { + continue + } + + exColl, ok := coll.(*Collection) + require.True(t, ok, "collection is an *exchange.Collection") + + ids := [][]string{ + make([]string, 0, len(exColl.added)), + make([]string, 0, len(exColl.removed)), + } + + for i, cIDs := range []map[string]struct{}{exColl.added, exColl.removed} { + for id := range cIDs { + ids[i] = append(ids[i], id) + } + } + + assert.ElementsMatch(t, expect.added, ids[0], "added items") + assert.ElementsMatch(t, expect.removed, ids[1], "removed items") + } + }) + } +} + func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_repeatedItems() { newDelta := api.DeltaUpdate{URL: "delta_url"}