From ca5f1970676a8c047f8f388f884aae32e24380e4 Mon Sep 17 00:00:00 2001 From: Keepers Date: Thu, 8 Dec 2022 18:34:20 -0700 Subject: [PATCH] export ownerCats and fetchPrevSnap (#1741) ## Type of change - [x] :hamster: Trivial/Minor ## Issue(s) * #1725 ## Test Plan - [x] :zap: Unit test --- .../connector/exchange/service_iterators.go | 23 +++++++++++++++++- src/internal/kopia/snapshot_manager.go | 24 +++++++++---------- src/internal/kopia/snapshot_manager_test.go | 18 +++++++------- src/internal/kopia/wrapper.go | 16 ++++++------- src/internal/kopia/wrapper_test.go | 8 +++---- 5 files changed, 55 insertions(+), 34 deletions(-) diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index 1ee7654c5..144d768c6 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -157,7 +157,28 @@ func FilterContainersAndFillCollections( collections[metadataKey] = col } - return errs + // TODO(ashmrtn): getFetchIDFunc functions should probably just return a + // multierror and all of the error handling should just use those so that it + // all ends up more consistent. + merrs := multierror.Append(nil, errs) + + col, err = makeMetadataCollection( + qp.Credentials.AzureTenantID, + qp.ResourceOwner, + qp.Category, + deltaTokens, + statusUpdater, + ) + if err != nil { + merrs = multierror.Append( + merrs, + errors.Wrap(err, "making metadata collection"), + ) + } else if col != nil { + collections[metadataKey] = col + } + + return merrs.ErrorOrNil() } func IterativeCollectContactContainers( diff --git a/src/internal/kopia/snapshot_manager.go b/src/internal/kopia/snapshot_manager.go index 23148c3b5..57e72b9fc 100644 --- a/src/internal/kopia/snapshot_manager.go +++ b/src/internal/kopia/snapshot_manager.go @@ -33,9 +33,9 @@ type snapshotManager interface { LoadSnapshots(ctx context.Context, ids []manifest.ID) ([]*snapshot.Manifest, error) } -type ownersCats struct { - resourceOwners map[string]struct{} - serviceCats map[string]struct{} +type OwnersCats struct { + ResourceOwners map[string]struct{} + ServiceCats map[string]struct{} } func serviceCatTag(p path.Path) string { @@ -49,15 +49,15 @@ func makeTagKV(k string) (string, string) { // tagsFromStrings returns a map[string]string with tags for all ownersCats // passed in. Currently uses placeholder values for each tag because there can // be multiple instances of resource owners and categories in a single snapshot. -func tagsFromStrings(oc *ownersCats) map[string]string { - res := make(map[string]string, len(oc.serviceCats)+len(oc.resourceOwners)) +func tagsFromStrings(oc *OwnersCats) map[string]string { + res := make(map[string]string, len(oc.ServiceCats)+len(oc.ResourceOwners)) - for k := range oc.serviceCats { + for k := range oc.ServiceCats { tk, tv := makeTagKV(k) res[tk] = tv } - for k := range oc.resourceOwners { + for k := range oc.ResourceOwners { tk, tv := makeTagKV(k) res[tk] = tv } @@ -181,16 +181,16 @@ func fetchPrevManifests( return manifestsSinceLastComplete(mans), nil } -// fetchPrevSnapshotManifests returns a set of manifests for complete and maybe +// FetchPrevSnapshotManifests returns a set of manifests for complete and maybe // incomplete snapshots for the given (resource owner, service, category) // tuples. Up to two manifests can be returned per tuple: one complete and one // incomplete. An incomplete manifest may be returned if it is newer than the // newest complete manifest for the tuple. Manifests are deduped such that if // multiple tuples match the same manifest it will only be returned once. -func fetchPrevSnapshotManifests( +func FetchPrevSnapshotManifests( ctx context.Context, sm snapshotManager, - oc *ownersCats, + oc *OwnersCats, ) []*snapshot.Manifest { mans := map[manifest.ID]*snapshot.Manifest{} @@ -198,10 +198,10 @@ func fetchPrevSnapshotManifests( // there's a previous incomplete snapshot and/or a previous complete snapshot // we can pass in. Can be expanded to return more than the most recent // snapshots, but may require more memory at runtime. - for serviceCat := range oc.serviceCats { + for serviceCat := range oc.ServiceCats { serviceTagKey, serviceTagValue := makeTagKV(serviceCat) - for resourceOwner := range oc.resourceOwners { + for resourceOwner := range oc.ResourceOwners { resourceOwnerTagKey, resourceOwnerTagValue := makeTagKV(resourceOwner) tags := map[string]string{ diff --git a/src/internal/kopia/snapshot_manager_test.go b/src/internal/kopia/snapshot_manager_test.go index 015c000e6..c0da2990c 100644 --- a/src/internal/kopia/snapshot_manager_test.go +++ b/src/internal/kopia/snapshot_manager_test.go @@ -35,24 +35,24 @@ var ( testUser2 = "user2" testUser3 = "user3" - testAllUsersAllCats = &ownersCats{ - resourceOwners: map[string]struct{}{ + testAllUsersAllCats = &OwnersCats{ + ResourceOwners: map[string]struct{}{ testUser1: {}, testUser2: {}, testUser3: {}, }, - serviceCats: map[string]struct{}{ + ServiceCats: map[string]struct{}{ testMail: {}, testEvents: {}, }, } - testAllUsersMail = &ownersCats{ - resourceOwners: map[string]struct{}{ + testAllUsersMail = &OwnersCats{ + ResourceOwners: map[string]struct{}{ testUser1: {}, testUser2: {}, testUser3: {}, }, - serviceCats: map[string]struct{}{ + ServiceCats: map[string]struct{}{ testMail: {}, }, } @@ -168,7 +168,7 @@ func TestSnapshotFetchUnitSuite(t *testing.T) { func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshots() { table := []struct { name string - input *ownersCats + input *OwnersCats data []manifestInfo // Use this to denote which manifests in data should be expected. Allows // defining data in a table while not repeating things between data and @@ -442,7 +442,7 @@ func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshots() { } } - snaps := fetchPrevSnapshotManifests(ctx, msm, test.input) + snaps := FetchPrevSnapshotManifests(ctx, msm, test.input) expected := make([]*snapshot.Manifest, 0, len(test.expectedIdxs)) for _, i := range test.expectedIdxs { @@ -532,7 +532,7 @@ func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshotsWorksWithErrors() { }, } - snaps := fetchPrevSnapshotManifests(ctx, msm, input) + snaps := FetchPrevSnapshotManifests(ctx, msm, input) // Only 1 snapshot should be chosen because the other two attempts fail. // However, which one is returned is non-deterministic because maps are used. diff --git a/src/internal/kopia/wrapper.go b/src/internal/kopia/wrapper.go index 23f3a4903..8c5db2008 100644 --- a/src/internal/kopia/wrapper.go +++ b/src/internal/kopia/wrapper.go @@ -410,11 +410,11 @@ func inflateDirTree( ctx context.Context, collections []data.Collection, progress *corsoProgress, -) (fs.Directory, *ownersCats, error) { +) (fs.Directory, *OwnersCats, error) { roots := make(map[string]*treeMap) - ownerCats := &ownersCats{ - resourceOwners: make(map[string]struct{}), - serviceCats: make(map[string]struct{}), + ownerCats := &OwnersCats{ + ResourceOwners: make(map[string]struct{}), + ServiceCats: make(map[string]struct{}), } for _, s := range collections { @@ -423,8 +423,8 @@ func inflateDirTree( } serviceCat := serviceCatTag(s.FullPath()) - ownerCats.serviceCats[serviceCat] = struct{}{} - ownerCats.resourceOwners[s.FullPath().ResourceOwner()] = struct{}{} + ownerCats.ServiceCats[serviceCat] = struct{}{} + ownerCats.ResourceOwners[s.FullPath().ResourceOwner()] = struct{}{} itemPath := s.FullPath().Elements() @@ -543,12 +543,12 @@ func (w Wrapper) BackupCollections( func (w Wrapper) makeSnapshotWithRoot( ctx context.Context, root fs.Directory, - oc *ownersCats, + oc *OwnersCats, progress *corsoProgress, ) (*BackupStats, error) { var man *snapshot.Manifest - prevSnaps := fetchPrevSnapshotManifests(ctx, w.c, oc) + prevSnaps := FetchPrevSnapshotManifests(ctx, w.c, oc) bc := &stats.ByteCounter{} diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index 4e2e31f93..d50cc991d 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/src/internal/kopia/wrapper_test.go @@ -569,8 +569,8 @@ func (suite *KopiaUnitSuite) TestBuildDirectoryTree() { dirTree, oc, err := inflateDirTree(ctx, collections, progress) require.NoError(t, err) - assert.Equal(t, expectedServiceCats, oc.serviceCats) - assert.Equal(t, expectedResourceOwners, oc.resourceOwners) + assert.Equal(t, expectedServiceCats, oc.ServiceCats) + assert.Equal(t, expectedResourceOwners, oc.ResourceOwners) assert.Equal(t, encodeAsPath(testTenant), dirTree.Name()) @@ -670,8 +670,8 @@ func (suite *KopiaUnitSuite) TestBuildDirectoryTree_MixedDirectory() { dirTree, oc, err := inflateDirTree(ctx, test.layout, progress) require.NoError(t, err) - assert.Equal(t, expectedServiceCats, oc.serviceCats) - assert.Equal(t, expectedResourceOwners, oc.resourceOwners) + assert.Equal(t, expectedServiceCats, oc.ServiceCats) + assert.Equal(t, expectedResourceOwners, oc.ResourceOwners) assert.Equal(t, encodeAsPath(testTenant), dirTree.Name())