diff --git a/src/internal/m365/backup.go b/src/internal/m365/backup.go index f916c7257..7b54c36cb 100644 --- a/src/internal/m365/backup.go +++ b/src/internal/m365/backup.go @@ -100,7 +100,7 @@ func (ctrl *Controller) ProduceBackupCollections( } case path.GroupsService: - colls, ssmb, canUsePreviousBackup, err = groups.ProduceBackupCollections( + colls, ssmb, err = groups.ProduceBackupCollections( ctx, bpc, ctrl.AC, @@ -111,6 +111,10 @@ func (ctrl *Controller) ProduceBackupCollections( return nil, nil, false, err } + // canUsePreviousBacukp can be always returned true for groups as we + // return a tombstone collection in case the metadata read fails + canUsePreviousBackup = true + default: return nil, nil, false, clues.Wrap(clues.New(service.String()), "service not supported").WithClues(ctx) } diff --git a/src/internal/m365/backup_test.go b/src/internal/m365/backup_test.go index 88708aa14..f7e51f89d 100644 --- a/src/internal/m365/backup_test.go +++ b/src/internal/m365/backup_test.go @@ -11,6 +11,9 @@ import ( "github.com/stretchr/testify/suite" inMock "github.com/alcionai/corso/src/internal/common/idname/mock" + "github.com/alcionai/corso/src/internal/common/ptr" + "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/internal/data/mock" "github.com/alcionai/corso/src/internal/m365/service/exchange" odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts" "github.com/alcionai/corso/src/internal/m365/service/sharepoint" @@ -574,3 +577,123 @@ func (suite *GroupsCollectionIntgSuite) TestCreateGroupsCollection_SharePoint() assert.NotZero(t, status.Successes) t.Log(status.String()) } + +func (suite *GroupsCollectionIntgSuite) TestCreateGroupsCollection_SharePoint_InvalidMetadata() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + var ( + groupID = tconfig.M365GroupID(t) + ctrl = newController(ctx, t, path.GroupsService) + groupIDs = []string{groupID} + ) + + id, name, err := ctrl.PopulateProtectedResourceIDAndName(ctx, groupID, nil) + require.NoError(t, err, clues.ToCore(err)) + + sel := selectors.NewGroupsBackup(groupIDs) + sel.Include(sel.LibraryFolders([]string{"test"}, selectors.PrefixMatch())) + + sel.SetDiscreteOwnerIDName(id, name) + + site, err := suite.connector.AC.Groups().GetRootSite(ctx, groupID) + require.NoError(t, err, clues.ToCore(err)) + + pth, err := path.Build( + suite.tenantID, + groupID, + path.GroupsService, + path.LibrariesCategory, + true, + odConsts.SitesPathDir, + ptr.Val(site.GetId())) + require.NoError(t, err, clues.ToCore(err)) + + mmc := []data.RestoreCollection{ + mock.Collection{ + Path: pth, + ItemData: []data.Item{ + &mock.Item{ + ItemID: "previouspath", + Reader: io.NopCloser(bytes.NewReader([]byte("invalid"))), + }, + }, + }, + } + + bpc := inject.BackupProducerConfig{ + LastBackupVersion: version.NoBackup, + Options: control.DefaultOptions(), + ProtectedResource: inMock.NewProvider(id, name), + Selector: sel.Selector, + MetadataCollections: mmc, + } + + collections, excludes, canUsePreviousBackup, err := ctrl.ProduceBackupCollections( + ctx, + bpc, + fault.New(true)) + require.NoError(t, err, clues.ToCore(err)) + assert.True(t, canUsePreviousBackup, "can use previous backup") + // No excludes yet as this isn't an incremental backup. + assert.True(t, excludes.Empty()) + + // we don't know an exact count of drives this will produce, + // but it should be more than one. + assert.Greater(t, len(collections), 1) + + p, err := path.BuildMetadata( + suite.tenantID, + groupID, + path.GroupsService, + path.LibrariesCategory, + false) + require.NoError(t, err, clues.ToCore(err)) + + p, err = p.Append(false, odConsts.SitesPathDir) + require.NoError(t, err, clues.ToCore(err)) + + foundSitesMetadata := false + foundRootTombstone := false + + sp, err := path.BuildPrefix( + suite.tenantID, + groupID, + path.GroupsService, + path.LibrariesCategory) + require.NoError(t, err, clues.ToCore(err)) + + sp, err = sp.Append(false, odConsts.SitesPathDir, ptr.Val(site.GetId())) + require.NoError(t, err, clues.ToCore(err)) + + for _, coll := range collections { + if coll.State() == data.DeletedState { + if coll.PreviousPath() != nil && coll.PreviousPath().String() == sp.String() { + foundRootTombstone = true + } + + continue + } + + sitesMetadataCollection := coll.FullPath().String() == p.String() + + for object := range coll.Items(ctx, fault.New(true)) { + if object.ID() == "previouspath" && sitesMetadataCollection { + foundSitesMetadata = true + } + + buf := &bytes.Buffer{} + _, err := buf.ReadFrom(object.ToReader()) + assert.NoError(t, err, "reading item", clues.ToCore(err)) + } + } + + assert.True(t, foundSitesMetadata, "missing sites metadata") + assert.True(t, foundRootTombstone, "missing root tombstone") + + status := ctrl.Wait() + assert.NotZero(t, status.Successes) + t.Log(status.String()) +} diff --git a/src/internal/m365/collection/drive/collections.go b/src/internal/m365/collection/drive/collections.go index 2f54b0429..35c11d1be 100644 --- a/src/internal/m365/collection/drive/collections.go +++ b/src/internal/m365/collection/drive/collections.go @@ -135,11 +135,6 @@ func deserializeMetadata( continue } - if err == nil { - // Successful decode. - continue - } - // This is conservative, but report an error if either any of the items // for any of the deserialized maps have duplicate drive IDs or there's // some other problem deserializing things. This will cause the entire @@ -147,7 +142,9 @@ func deserializeMetadata( // these cases. We can make the logic for deciding when to continue vs. // when to fail less strict in the future if needed. if err != nil { - return nil, nil, false, clues.Stack(err).WithClues(ictx) + errs.Fail(clues.Stack(err).WithClues(ictx)) + + return map[string]string{}, map[string]map[string]string{}, false, nil } } } diff --git a/src/internal/m365/collection/drive/collections_test.go b/src/internal/m365/collection/drive/collections_test.go index d0e33477f..622f5029c 100644 --- a/src/internal/m365/collection/drive/collections_test.go +++ b/src/internal/m365/collection/drive/collections_test.go @@ -978,7 +978,9 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata() { { // Bad formats are logged but skip adding entries to the maps and don't // return an error. - name: "BadFormat", + name: "BadFormat", + expectedDeltas: map[string]string{}, + expectedPaths: map[string]map[string]string{}, cols: []func() []graph.MetadataCollectionEntry{ func() []graph.MetadataCollectionEntry { return []graph.MetadataCollectionEntry{ @@ -989,7 +991,7 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata() { }, }, canUsePreviousBackup: false, - errCheck: assert.Error, + errCheck: assert.NoError, }, { // Unexpected files are logged and skipped. They don't cause an error to @@ -1054,10 +1056,10 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata() { } }, }, - expectedDeltas: nil, - expectedPaths: nil, + expectedDeltas: map[string]string{}, + expectedPaths: map[string]map[string]string{}, canUsePreviousBackup: false, - errCheck: assert.Error, + errCheck: assert.NoError, }, { name: "DriveAlreadyFound_Deltas", @@ -1084,10 +1086,10 @@ func (suite *OneDriveCollectionsUnitSuite) TestDeserializeMetadata() { } }, }, - expectedDeltas: nil, - expectedPaths: nil, + expectedDeltas: map[string]string{}, + expectedPaths: map[string]map[string]string{}, canUsePreviousBackup: false, - errCheck: assert.Error, + errCheck: assert.NoError, }, } diff --git a/src/internal/m365/collection/groups/channel_handler.go b/src/internal/m365/collection/groups/channel_handler.go index 80c36cbef..db50446ca 100644 --- a/src/internal/m365/collection/groups/channel_handler.go +++ b/src/internal/m365/collection/groups/channel_handler.go @@ -67,6 +67,15 @@ func (bh channelsBackupHandler) canonicalPath( false) } +func (bh channelsBackupHandler) PathPrefix(tenantID string) (path.Path, error) { + return path.Build( + tenantID, + bh.protectedResource, + path.GroupsService, + path.ChannelMessagesCategory, + false) +} + func (bh channelsBackupHandler) GetChannelMessage( ctx context.Context, teamID, channelID, itemID string, diff --git a/src/internal/m365/service/groups/backup.go b/src/internal/m365/service/groups/backup.go index 1943b8fb4..25210ade3 100644 --- a/src/internal/m365/service/groups/backup.go +++ b/src/internal/m365/service/groups/backup.go @@ -22,6 +22,7 @@ import ( "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/backup/identity" "github.com/alcionai/corso/src/pkg/backup/metadata" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -35,19 +36,18 @@ func ProduceBackupCollections( creds account.M365Config, su support.StatusUpdater, errs *fault.Bus, -) ([]data.BackupCollection, *prefixmatcher.StringSetMatcher, bool, error) { +) ([]data.BackupCollection, *prefixmatcher.StringSetMatcher, error) { b, err := bpc.Selector.ToGroupsBackup() if err != nil { - return nil, nil, false, clues.Wrap(err, "groupsDataCollection: parsing selector") + return nil, nil, clues.Wrap(err, "groupsDataCollection: parsing selector") } var ( - el = errs.Local() - collections = []data.BackupCollection{} - categories = map[path.CategoryType]struct{}{} - ssmb = prefixmatcher.NewStringSetBuilder() - canUsePreviousBackup bool - sitesPreviousPaths = map[string]string{} + el = errs.Local() + collections = []data.BackupCollection{} + categories = map[path.CategoryType]struct{}{} + ssmb = prefixmatcher.NewStringSetBuilder() + sitesPreviousPaths = map[string]string{} ) ctx = clues.Add( @@ -60,7 +60,7 @@ func ProduceBackupCollections( bpc.ProtectedResource.ID(), api.CallConfig{}) if err != nil { - return nil, nil, false, clues.Wrap(err, "getting group").WithClues(ctx) + return nil, nil, clues.Wrap(err, "getting group").WithClues(ctx) } isTeam := api.IsTeam(ctx, group) @@ -81,7 +81,7 @@ func ProduceBackupCollections( case path.LibrariesCategory: sites, err := ac.Groups().GetAllSites(ctx, bpc.ProtectedResource.ID(), errs) if err != nil { - return nil, nil, false, err + return nil, nil, err } siteMetadataCollection := map[string][]data.RestoreCollection{} @@ -108,14 +108,14 @@ func ProduceBackupCollections( ac.Drives(), scope) - cp, err := bh.SitePathPrefix(creds.AzureTenantID) + sp, err := bh.SitePathPrefix(creds.AzureTenantID) if err != nil { - return nil, nil, false, clues.Wrap(err, "getting canonical path") + return nil, nil, clues.Wrap(err, "getting site path") } - sitesPreviousPaths[ptr.Val(s.GetId())] = cp.String() + sitesPreviousPaths[ptr.Val(s.GetId())] = sp.String() - cs, cupb, err := site.CollectLibraries( + cs, canUsePreviousBackup, err := site.CollectLibraries( ctx, sbpc, bh, @@ -128,11 +128,11 @@ func ProduceBackupCollections( continue } - dbcs = append(dbcs, cs...) + if !canUsePreviousBackup { + dbcs = append(dbcs, data.NewTombstoneCollection(sp, control.Options{})) + } - // FIXME(meain): This can cause incorrect backup - // https://github.com/alcionai/corso/issues/4371 - canUsePreviousBackup = canUsePreviousBackup || cupb + dbcs = append(dbcs, cs...) } case path.ChannelMessagesCategory: @@ -140,10 +140,12 @@ func ProduceBackupCollections( continue } - dbcs, canUsePreviousBackup, err = groups.CreateCollections( + bh := groups.NewChannelBackupHandler(bpc.ProtectedResource.ID(), ac.Channels()) + + cs, canUsePreviousBackup, err := groups.CreateCollections( ctx, bpc, - groups.NewChannelBackupHandler(bpc.ProtectedResource.ID(), ac.Channels()), + bh, creds.AzureTenantID, scope, su, @@ -152,6 +154,17 @@ func ProduceBackupCollections( el.AddRecoverable(ctx, err) continue } + + if !canUsePreviousBackup { + tp, err := bh.PathPrefix(creds.AzureTenantID) + if err != nil { + return nil, nil, clues.Wrap(err, "getting message path") + } + + dbcs = append(dbcs, data.NewTombstoneCollection(tp, control.Options{})) + } + + dbcs = append(dbcs, cs...) } collections = append(collections, dbcs...) @@ -170,7 +183,7 @@ func ProduceBackupCollections( su, errs) if err != nil { - return nil, nil, false, err + return nil, nil, err } collections = append(collections, baseCols...) @@ -183,12 +196,12 @@ func ProduceBackupCollections( sitesPreviousPaths, su) if err != nil { - return nil, nil, false, err + return nil, nil, err } collections = append(collections, md) - return collections, ssmb.ToReader(), canUsePreviousBackup, el.Failure() + return collections, ssmb.ToReader(), el.Failure() } func getSitesMetadataCollection(