From 3d15a0d649c4e77f46a79800dd790fa66cde5bf8 Mon Sep 17 00:00:00 2001 From: ryanfkeepers Date: Tue, 15 Aug 2023 13:29:14 -0600 Subject: [PATCH] first pass on compliance with the reason establishes behavior around using the reasoner interface in a world where paths can contain multiple services. Primarily focused on ensuring the reasoner clearly guides maintainers towards proper usage. --- src/internal/kopia/backup_bases.go | 6 +++ src/internal/kopia/backup_bases_test.go | 45 ++++++++++++++++---- src/internal/kopia/base_finder.go | 11 +++-- src/internal/kopia/upload.go | 17 ++++---- src/internal/operations/manifests.go | 7 +-- src/internal/operations/test/helper_test.go | 7 ++- src/pkg/backup/identity/identity.go | 11 +++++ src/pkg/backup/identity/mock/reasoner.go | 47 +++++++++++++++++++++ src/pkg/path/service_category_test.go | 7 +-- src/pkg/path/service_resource.go | 12 +++++- src/pkg/path/service_type.go | 2 +- src/pkg/selectors/reasons.go | 21 ++++++--- 12 files changed, 155 insertions(+), 38 deletions(-) create mode 100644 src/pkg/backup/identity/mock/reasoner.go diff --git a/src/internal/kopia/backup_bases.go b/src/internal/kopia/backup_bases.go index 4c2c95fb7..28a68c244 100644 --- a/src/internal/kopia/backup_bases.go +++ b/src/internal/kopia/backup_bases.go @@ -111,6 +111,12 @@ func (bb *backupBases) ClearAssistBases() { bb.assistBases = nil } +// BaseKeyServiceCategory makes a backup base key using +// the reasoner's Service and Category. +func BaseKeyServiceCategory(br identity.Reasoner) string { + return br.Service().String() + br.Category().String() +} + // MergeBackupBases reduces the two BackupBases into a single BackupBase. // Assumes the passed in BackupBases represents a prior backup version (across // some migration that disrupts lookup), and that the BackupBases used to call diff --git a/src/internal/kopia/backup_bases_test.go b/src/internal/kopia/backup_bases_test.go index faa402162..c871c7ac8 100644 --- a/src/internal/kopia/backup_bases_test.go +++ b/src/internal/kopia/backup_bases_test.go @@ -14,6 +14,7 @@ import ( "github.com/alcionai/corso/src/internal/version" "github.com/alcionai/corso/src/pkg/backup" "github.com/alcionai/corso/src/pkg/backup/identity" + idMock "github.com/alcionai/corso/src/pkg/backup/identity/mock" "github.com/alcionai/corso/src/pkg/path" ) @@ -460,18 +461,12 @@ func (suite *BackupBasesUnitSuite) TestMergeBackupBases() { bb := makeBackupBases(test.merge, test.assist) other := makeBackupBases(test.otherMerge, test.otherAssist) - expected := test.expect() ctx, flush := tester.NewContext(t) defer flush() - got := bb.MergeBackupBases( - ctx, - other, - func(r identity.Reasoner) string { - return r.Service().String() + r.Category().String() - }) - AssertBackupBasesEqual(t, expected, got) + got := bb.MergeBackupBases(ctx, other, BaseKeyServiceCategory) + AssertBackupBasesEqual(t, test.expect(), got) }) } } @@ -843,3 +838,37 @@ func (suite *BackupBasesUnitSuite) TestFixupAndVerify() { }) } } + +func (suite *BackupBasesUnitSuite) TestBaseKeyServiceCategory() { + table := []struct { + name string + service path.ServiceType + category path.CategoryType + expect string + }{ + { + name: "unknown", + service: path.UnknownService, + category: path.UnknownCategory, + expect: "unknown", + }, + { + name: "known service", + service: path.ExchangeService, + category: path.EmailCategory, + expect: "exchangeEmail", + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + result := BaseKeyServiceCategory(idMock.Reason{ + TenantID: "tid", + Cat: test.category, + Svc: test.service, + }) + assert.Equal(t, test.expect, result) + }) + } +} diff --git a/src/internal/kopia/base_finder.go b/src/internal/kopia/base_finder.go index a53cb52cf..8e1ff79a5 100644 --- a/src/internal/kopia/base_finder.go +++ b/src/internal/kopia/base_finder.go @@ -70,11 +70,14 @@ func (r reason) Category() path.CategoryType { } func (r reason) SubtreePath() (path.Path, error) { - p, err := path.BuildPrefix( - r.Tenant(), - r.ProtectedResource(), + srs, err := path.NewServiceResources( r.Service(), - r.Category()) + r.ProtectedResource()) + if err != nil { + return nil, clues.Wrap(err, "building path service prefix") + } + + p, err := path.BuildPrefix(r.Tenant(), srs, r.Category()) return p, clues.Wrap(err, "building path").OrNil() } diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index 2d815594f..b10af64ba 100644 --- a/src/internal/kopia/upload.go +++ b/src/internal/kopia/upload.go @@ -196,14 +196,17 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { return } + ctx := clues.Add( + cp.ctx, + "services", path.ServiceResourcesToServices(d.repoPath.ServiceResources()), + "category", d.repoPath.Category().String()) + // These items were sourced from a base snapshot or were cached in kopia so we // never had to materialize their details in-memory. if d.info == nil || d.cached { if d.prevPath == nil { cp.errs.AddRecoverable(cp.ctx, clues.New("item sourced from previous backup with no previous path"). - With( - "service", d.repoPath.Service().String(), - "category", d.repoPath.Category().String()). + WithClues(ctx). Label(fault.LabelForceNoBackupCreation)) return @@ -219,9 +222,7 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { d.locationPath) if err != nil { cp.errs.AddRecoverable(cp.ctx, clues.Wrap(err, "adding item to merge list"). - With( - "service", d.repoPath.Service().String(), - "category", d.repoPath.Category().String()). + WithClues(ctx). Label(fault.LabelForceNoBackupCreation)) } @@ -235,9 +236,7 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { *d.info) if err != nil { cp.errs.AddRecoverable(cp.ctx, clues.New("adding item to details"). - With( - "service", d.repoPath.Service().String(), - "category", d.repoPath.Category().String()). + WithClues(ctx). Label(fault.LabelForceNoBackupCreation)) return diff --git a/src/internal/operations/manifests.go b/src/internal/operations/manifests.go index 95b313adc..298f4ab41 100644 --- a/src/internal/operations/manifests.go +++ b/src/internal/operations/manifests.go @@ -80,12 +80,7 @@ func getManifestsAndMetadata( // 3. the current reasons contain all the necessary manifests. // Note: This is not relevant for assist backups, since they are newly introduced // and they don't exist with fallback reasons. - bb = bb.MergeBackupBases( - ctx, - fbb, - func(r identity.Reasoner) string { - return r.Service().String() + r.Category().String() - }) + bb = bb.MergeBackupBases(ctx, fbb, kopia.BaseKeyServiceCategory) if !getMetadata { return bb, nil, false, nil diff --git a/src/internal/operations/test/helper_test.go b/src/internal/operations/test/helper_test.go index 0fdd63e96..1e154a1bb 100644 --- a/src/internal/operations/test/helper_test.go +++ b/src/internal/operations/test/helper_test.go @@ -33,6 +33,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/backup/identity" + idMock "github.com/alcionai/corso/src/pkg/backup/identity/mock" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/repository" "github.com/alcionai/corso/src/pkg/count" @@ -244,7 +245,11 @@ func checkBackupIsInManifests( for _, category := range categories { t.Run(category.String(), func(t *testing.T) { var ( - r = kopia.NewReason("", resourceOwner, sel.PathService(), category) + r = idMock.Reason{ + Cat: category, + Svc: sel.PathService(), + Resource: resourceOwner, + } tags = map[string]string{kopia.TagBackupCategory: ""} found bool ) diff --git a/src/pkg/backup/identity/identity.go b/src/pkg/backup/identity/identity.go index 0f0d77416..1eac0b0f9 100644 --- a/src/pkg/backup/identity/identity.go +++ b/src/pkg/backup/identity/identity.go @@ -5,9 +5,20 @@ import "github.com/alcionai/corso/src/pkg/path" // Reasoner describes the parts of the backup that make up its // data identity: the tenant, protected resources, services, and // categories which are held within the backup. +// +// Reasoner only recognizes the "primary" protected resource and +// service. IE: subservice resources and services are not recognized +// as part of the backup Reason. type Reasoner interface { Tenant() string + // ProtectedResource represents the Primary protected resource. + // IE: if a path or backup supports subservices, this value + // should only provide the first service's resource, and not the + // resource for any subservice. ProtectedResource() string + // Service represents the Primary service. + // IE: if a path or backup supports subservices, this value + // should only provide the first service; not a subservice. Service() path.ServiceType Category() path.CategoryType // SubtreePath returns the path prefix for data in existing backups that have diff --git a/src/pkg/backup/identity/mock/reasoner.go b/src/pkg/backup/identity/mock/reasoner.go new file mode 100644 index 000000000..14de9baa3 --- /dev/null +++ b/src/pkg/backup/identity/mock/reasoner.go @@ -0,0 +1,47 @@ +package mock + +import ( + "github.com/alcionai/clues" + + "github.com/alcionai/corso/src/pkg/path" +) + +type Reason struct { + TenantID string + Cat path.CategoryType + Svc path.ServiceType + Resource string + SubtreeErr error +} + +func (r Reason) Tenant() string { + return r.TenantID +} + +func (r Reason) Category() path.CategoryType { + return r.Cat +} + +func (r Reason) Service() path.ServiceType { + return r.Svc +} + +func (r Reason) ProtectedResource() string { + return r.Resource +} + +func (r Reason) SubtreePath() (path.Path, error) { + if r.SubtreeErr != nil { + return nil, r.SubtreeErr + } + + p, err := path.BuildPrefix( + r.Tenant(), + []path.ServiceResource{{ + ProtectedResource: r.Resource, + Service: r.Svc, + }}, + r.Category()) + + return p, clues.Wrap(err, "building path").OrNil() +} diff --git a/src/pkg/path/service_category_test.go b/src/pkg/path/service_category_test.go index 37aa5774e..46cc7f4b7 100644 --- a/src/pkg/path/service_category_test.go +++ b/src/pkg/path/service_category_test.go @@ -113,9 +113,10 @@ func (suite *ServiceCategoryUnitSuite) TestToServiceType() { } for _, test := range table { suite.Run(test.name, func() { - t := suite.T() - - assert.Equal(t, test.expected, toServiceType(test.service)) + assert.Equal( + suite.T(), + test.expected, + ToServiceType(test.service)) }) } } diff --git a/src/pkg/path/service_resource.go b/src/pkg/path/service_resource.go index 037011a24..b5e034f97 100644 --- a/src/pkg/path/service_resource.go +++ b/src/pkg/path/service_resource.go @@ -85,6 +85,16 @@ func ServiceResourcesToResources(srs []ServiceResource) []string { return prs } +func ServiceResourcesToServices(srs []ServiceResource) []ServiceType { + sts := make([]ServiceType, len(srs)) + + for i := range srs { + sts[i] = srs[i].Service + } + + return sts +} + func ServiceResourcesMatchServices(srs []ServiceResource, sts []ServiceType) bool { return slices.EqualFunc(srs, sts, func(sr ServiceResource, st ServiceType) bool { return sr.Service == st @@ -125,7 +135,7 @@ func elementsToServiceResources(elems Elements) ([]ServiceResource, int, error) ) for j := 1; i < len(elems); i, j = i+2, j+2 { - service := toServiceType(elems[i]) + service := ToServiceType(elems[i]) if service == UnknownService { if i == 0 { return nil, -1, clues.Wrap(errMissingSegment, "service") diff --git a/src/pkg/path/service_type.go b/src/pkg/path/service_type.go index 009ed75f4..3256875d5 100644 --- a/src/pkg/path/service_type.go +++ b/src/pkg/path/service_type.go @@ -33,7 +33,7 @@ const ( GroupsMetadataService // groupsMetadata ) -func toServiceType(service string) ServiceType { +func ToServiceType(service string) ServiceType { s := strings.ToLower(service) switch s { diff --git a/src/pkg/selectors/reasons.go b/src/pkg/selectors/reasons.go index a27d87f52..6a97bb8b0 100644 --- a/src/pkg/selectors/reasons.go +++ b/src/pkg/selectors/reasons.go @@ -3,6 +3,8 @@ package selectors import ( "golang.org/x/exp/maps" + "github.com/alcionai/clues" + "github.com/alcionai/corso/src/internal/common/idname" "github.com/alcionai/corso/src/pkg/backup/identity" "github.com/alcionai/corso/src/pkg/path" @@ -38,11 +40,12 @@ func (br backupReason) Category() path.CategoryType { } func (br backupReason) SubtreePath() (path.Path, error) { - return path.BuildPrefix( - br.tenant, - br.resource, - br.service, - br.category) + srs, err := path.NewServiceResources(br.service, br.resource) + if err != nil { + return nil, clues.Wrap(err, "building path prefix services") + } + + return path.BuildPrefix(br.tenant, srs, br.category) } func (br backupReason) key() string { @@ -59,6 +62,14 @@ type servicerCategorizerProvider interface { idname.Provider } +// produces the Reasoner basis described by the selector. +// In cases of reasons with subservices (ie, multiple +// services described by a backup or path), the selector +// will only ever generate a ServiceResource for the first +// service+resource pair in the set. +// +// TODO: it may be possible, if necessary, to add subservice +// recognition to the service via additional scopes. func reasonsFor( sel servicerCategorizerProvider, tenantID string,