From 573f55686f232261d75ca5ed81eeeaaa81aacaf9 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Tue, 13 Sep 2022 13:58:20 -0700 Subject: [PATCH] Make ExchangeDataCollection use path.Path internally (#820) * Have exchange data collection store path.Path Still complies with the old FullPath() string interface until we update that. * Pass path.Path to NewCollection for exchange Basically fixes up errors introduced by previous commit. * Fixup exchange recovery path indices All exchange paths now use the path struct, meaning the service, category, and user elements are in the standard positions. * use path package in selector reduction (#822) Currently, during a reduction process, scopes compare their values to the raw split on repoRef. This causes some brittle indexing to retrieve values from the rr, carrying assumptions that are difficult to track across changes. This PR trades the string split for the paths package to better integrate identification of the path values. Adds some mocks and amends some error behaviors in order to fit paths into the current testing schema. Co-authored-by: Keepers --- src/cli/backup/exchange.go | 2 +- .../exchange/exchange_data_collection.go | 12 ++- .../exchange/exchange_data_collection_test.go | 51 ++++++++++-- .../connector/exchange/service_iterators.go | 51 ++++++++---- src/internal/connector/graph_connector.go | 14 +--- src/internal/operations/restore.go | 2 +- src/internal/path/resource_path.go | 8 +- src/pkg/selectors/exchange.go | 40 +++++---- src/pkg/selectors/exchange_test.go | 81 +++++++------------ src/pkg/selectors/helpers_test.go | 20 +++-- src/pkg/selectors/onedrive.go | 26 ++---- src/pkg/selectors/scopes.go | 52 ++++-------- src/pkg/selectors/scopes_test.go | 59 ++++---------- 13 files changed, 195 insertions(+), 223 deletions(-) diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index 3d7ab4638..12c9d63a8 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -385,7 +385,7 @@ func detailsExchangeCmd(cmd *cobra.Command, args []string) error { sel.Include(sel.Users(selectors.Any())) } - ds := sel.Reduce(d) + ds := sel.Reduce(ctx, d) if len(ds.Entries) == 0 { Info(ctx, "nothing to display: no items in the backup match the provided selectors") return nil diff --git a/src/internal/connector/exchange/exchange_data_collection.go b/src/internal/connector/exchange/exchange_data_collection.go index bd29e555c..ddccf7242 100644 --- a/src/internal/connector/exchange/exchange_data_collection.go +++ b/src/internal/connector/exchange/exchange_data_collection.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "io" + "strings" absser "github.com/microsoft/kiota-abstractions-go/serialization" kw "github.com/microsoft/kiota-serialization-json-go" @@ -18,6 +19,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/internal/path" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/logger" ) @@ -53,13 +55,13 @@ type Collection struct { statusUpdater support.StatusUpdater // FullPath is the slice representation of the action context passed down through the hierarchy. // The original request can be gleaned from the slice. (e.g. {, , "emails"}) - fullPath []string + fullPath path.Path } // NewExchangeDataCollection creates an ExchangeDataCollection with fullPath is annotated func NewCollection( user string, - fullPath []string, + fullPath path.Path, collectionType optionIdentifier, service graph.Service, statusUpdater support.StatusUpdater, @@ -107,7 +109,11 @@ func GetQueryAndSerializeFunc(optID optionIdentifier) (GraphRetrievalFunc, Graph // FullPath returns the Collection's fullPath []string func (col *Collection) FullPath() []string { - return append([]string{}, col.fullPath...) + // TODO(ashmrtn): Remove this when data.Collection.FullPath returns a + // path.Path. This assumes we don't have adversarial users that use '/' in + // their folder names. + r := col.fullPath.String() + return strings.Split(r, "/") } // populateByOptionIdentifier is a utility function that uses col.collectionType to be able to serialize diff --git a/src/internal/connector/exchange/exchange_data_collection_test.go b/src/internal/connector/exchange/exchange_data_collection_test.go index 3305dba42..a6c61eb37 100644 --- a/src/internal/connector/exchange/exchange_data_collection_test.go +++ b/src/internal/connector/exchange/exchange_data_collection_test.go @@ -5,7 +5,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/path" ) type ExchangeDataCollectionSuite struct { @@ -44,31 +47,65 @@ func (suite *ExchangeDataCollectionSuite) TestExchangeDataReader_Empty() { } func (suite *ExchangeDataCollectionSuite) TestExchangeData_FullPath() { + t := suite.T() + tenant := "a-tenant" user := "a-user" - fullPath := []string{"a-tenant", user, "emails"} + folder := "a-folder" + expected := []string{ + tenant, + path.ExchangeService.String(), + user, + path.EmailCategory.String(), + folder, + } + + fullPath, err := path.Builder{}.Append(folder).ToDataLayerExchangePathForCategory( + tenant, + user, + path.EmailCategory, + false, + ) + require.NoError(t, err) + edc := Collection{ user: user, fullPath: fullPath, } - assert.Equal(suite.T(), edc.FullPath(), fullPath) + + // TODO(ashmrtn): Update when data.Collection.FullPath returns path.Path. + assert.Equal(t, expected, edc.FullPath()) } func (suite *ExchangeDataCollectionSuite) TestExchangeDataCollection_NewExchangeDataCollection() { + tenant := "a-tenant" + user := "a-user" + folder := "a-folder" name := "User" + + fullPath, err := path.Builder{}.Append(folder).ToDataLayerExchangePathForCategory( + tenant, + user, + path.EmailCategory, + false, + ) + require.NoError(suite.T(), err) + edc := Collection{ user: name, - fullPath: []string{"Directory", "File", "task"}, + fullPath: fullPath, } suite.Equal(name, edc.user) - suite.Contains(edc.FullPath(), "Directory") - suite.Contains(edc.FullPath(), "File") - suite.Contains(edc.FullPath(), "task") + suite.Contains(edc.FullPath(), fullPath.Tenant()) + suite.Contains(edc.FullPath(), fullPath.Service().String()) + suite.Contains(edc.FullPath(), fullPath.Category().String()) + suite.Contains(edc.FullPath(), fullPath.ResourceOwner()) + suite.Contains(edc.FullPath(), fullPath.Folder()) } func (suite *ExchangeDataCollectionSuite) TestExchangeCollection_AddJob() { eoc := Collection{ user: "Dexter", - fullPath: []string{"Today", "is", "currently", "different"}, + fullPath: nil, } suite.Zero(len(eoc.jobs)) diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index 6709669d9..12f1ce1d5 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -2,7 +2,6 @@ package exchange import ( "context" - "strings" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/pkg/errors" @@ -78,7 +77,7 @@ func resolveCollectionPath( resolver graph.ContainerResolver, tenantID, user, folderID string, category path.CategoryType, -) ([]string, error) { +) (path.Path, error) { if resolver == nil { // Allows caller to default to old-style path. return nil, errors.WithStack(errNilResolver) @@ -89,19 +88,12 @@ func resolveCollectionPath( return nil, errors.Wrap(err, "resolving folder ID") } - fullPath, err := p.ToDataLayerExchangePathForCategory( + return p.ToDataLayerExchangePathForCategory( tenantID, user, category, false, ) - if err != nil { - return nil, errors.Wrap(err, "converting to canonical path") - } - - // TODO(ashmrtn): This can return the path directly when Collections take - // path.Path. - return strings.Split(fullPath.String(), "/"), nil } // IterateSelectAllDescendablesForCollection utility function for @@ -156,7 +148,18 @@ func IterateSelectAllDescendablesForCollections( // Saving to messages to list. Indexed by folder directory := *entry.GetParentFolderId() - dirPath := []string{qp.Credentials.TenantID, qp.User, category.String(), directory} + + dirPath, err := path.Builder{}.Append(directory).ToDataLayerExchangePathForCategory( + qp.Credentials.TenantID, + qp.User, + category, + false, + ) + if err != nil { + errs = support.WrapAndAppend("converting to resource path", err, errs) + // This really shouldn't be happening unless we have a bad category. + return true + } if _, ok = collections[directory]; !ok { newPath, err := resolveCollectionPath( @@ -271,9 +274,21 @@ func IterateSelectAllEventsFromCalendars( return true } + dirPath, err := path.Builder{}.Append(*directory).ToDataLayerExchangePathForCategory( + qp.Credentials.TenantID, + qp.User, + path.EventsCategory, + false, + ) + if err != nil { + // This really shouldn't be happening. + errs = support.WrapAndAppend("converting to resource path", err, errs) + return true + } + edc := NewCollection( qp.User, - []string{qp.Credentials.TenantID, qp.User, path.EventsCategory.String(), *directory}, + dirPath, events, service, statusUpdater, @@ -376,11 +391,17 @@ func IterateFilterFolderDirectoriesForCollections( } directory := *folder.GetId() - dirPath := []string{ + + dirPath, err := path.Builder{}.Append(directory).ToDataLayerExchangePathForCategory( qp.Credentials.TenantID, qp.User, - path.EmailCategory.String(), - directory, + path.EmailCategory, + false, + ) + if err != nil { + // This really shouldn't be happening. + errs = support.WrapAndAppend("converting to resource path", err, errs) + return true } p, err := resolveCollectionPath( diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 09fdf9dc6..13be3a088 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -268,20 +268,8 @@ func (gc *GraphConnector) RestoreExchangeDataCollection( exit bool ) - // email uses the new path format category = path.ToCategoryType(dc.FullPath()[3]) - if category == path.UnknownCategory { - // events and calendar use the old path format - category = path.ToCategoryType(dc.FullPath()[2]) - } - - // get the user from the path index based on the path pattern. - switch category { - case path.EmailCategory: - user = dc.FullPath()[2] - case path.ContactsCategory, path.EventsCategory: - user = dc.FullPath()[1] - } + user = dc.FullPath()[2] if _, ok := pathCounter[directory]; !ok { pathCounter[directory] = true diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go index c484220a5..3501acd01 100644 --- a/src/internal/operations/restore.go +++ b/src/internal/operations/restore.go @@ -107,7 +107,7 @@ func (op *RestoreOperation) Run(ctx context.Context) (err error) { } // format the details and retrieve the items from kopia - fds := er.Reduce(d) + fds := er.Reduce(ctx, d) if len(fds.Entries) == 0 { return errors.New("nothing to restore: no items in the backup match the provided selectors") } diff --git a/src/internal/path/resource_path.go b/src/internal/path/resource_path.go index 4af5371f4..d7d9addc1 100644 --- a/src/internal/path/resource_path.go +++ b/src/internal/path/resource_path.go @@ -4,6 +4,8 @@ import ( "github.com/pkg/errors" ) +var ErrorUnknownService = errors.New("unknown service string") + type ServiceType int //go:generate stringer -type=ServiceType -linecomment @@ -21,6 +23,8 @@ func toServiceType(service string) ServiceType { } } +var ErrorUnknownCategory = errors.New("unknown category string") + type CategoryType int //go:generate stringer -type=CategoryType -linecomment @@ -56,12 +60,12 @@ var serviceCategories = map[ServiceType]map[CategoryType]struct{}{ func validateServiceAndCategoryStrings(s, c string) (ServiceType, CategoryType, error) { service := toServiceType(s) if service == UnknownService { - return UnknownService, UnknownCategory, errors.Errorf("unknown service string %q", s) + return UnknownService, UnknownCategory, errors.Wrapf(ErrorUnknownService, "%q", s) } category := ToCategoryType(c) if category == UnknownCategory { - return UnknownService, UnknownCategory, errors.Errorf("unknown category string %q", c) + return UnknownService, UnknownCategory, errors.Wrapf(ErrorUnknownCategory, "%q", c) } if err := validateServiceAndCategory(service, category); err != nil { diff --git a/src/pkg/selectors/exchange.go b/src/pkg/selectors/exchange.go index 45914cb94..67bf2badf 100644 --- a/src/pkg/selectors/exchange.go +++ b/src/pkg/selectors/exchange.go @@ -1,6 +1,7 @@ package selectors import ( + "context" "strconv" "github.com/alcionai/corso/src/internal/common" @@ -550,39 +551,33 @@ func (ec exchangeCategory) unknownCat() categorizer { return ExchangeCategoryUnknown } -// transforms a path to a map of identified properties. -// TODO: this should use service-specific funcs in the Paths pkg. Instead of -// peeking at the path directly, the caller should compare against values like -// path.UserPN() and path.Folders(). +// pathValues transforms a path to a map of identified properties. // -// Malformed (ie, short len) paths will return incomplete results. // Example: -// [tenantID, userPN, "mail", mailFolder, mailID] +// [tenantID, service, userPN, category, mailFolder, mailID] // => {exchUser: userPN, exchMailFolder: mailFolder, exchMail: mailID} -func (ec exchangeCategory) pathValues(p []string) map[categorizer]string { - m := map[categorizer]string{} - if len(p) < 5 { - return m - } +func (ec exchangeCategory) pathValues(p path.Path) map[categorizer]string { + var folderCat, itemCat categorizer switch ec { case ExchangeContact: - m[ExchangeUser] = p[1] - m[ExchangeContactFolder] = p[3] - m[ExchangeContact] = p[4] + folderCat, itemCat = ExchangeContactFolder, ExchangeContact case ExchangeEvent: - m[ExchangeUser] = p[1] - m[ExchangeEventCalendar] = p[3] - m[ExchangeEvent] = p[4] + folderCat, itemCat = ExchangeEventCalendar, ExchangeEvent case ExchangeMail: - m[ExchangeUser] = p[2] - m[ExchangeMailFolder] = p[4] - m[ExchangeMail] = p[5] + folderCat, itemCat = ExchangeMailFolder, ExchangeMail + + default: + return map[categorizer]string{} } - return m + return map[categorizer]string{ + ExchangeUser: p.ResourceOwner(), + folderCat: p.Folder(), + itemCat: p.Item(), + } } // pathKeys returns the path keys recognized by the receiver's leaf type. @@ -677,8 +672,9 @@ func (s ExchangeScope) setDefaults() { // Reduce filters the entries in a details struct to only those that match the // inclusions, filters, and exclusions in the selector. -func (s exchange) Reduce(deets *details.Details) *details.Details { +func (s exchange) Reduce(ctx context.Context, deets *details.Details) *details.Details { return reduce[ExchangeScope]( + ctx, deets, s.Selector, map[path.CategoryType]exchangeCategory{ diff --git a/src/pkg/selectors/exchange_test.go b/src/pkg/selectors/exchange_test.go index 5fecc8cd4..43c4f5c9f 100644 --- a/src/pkg/selectors/exchange_test.go +++ b/src/pkg/selectors/exchange_test.go @@ -1,7 +1,7 @@ package selectors import ( - "strings" + "context" "testing" "time" @@ -779,7 +779,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_MatchesPath() { ) var ( - pth = stubPath(path.ExchangeService, path.EmailCategory, usr, fld, mail) + pth = stubPath(suite.T(), usr, []string{fld, mail}, path.EmailCategory) es = NewExchangeRestore() ) @@ -823,12 +823,12 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_MatchesPath() { func (suite *ExchangeSelectorSuite) TestIdPath() { table := []struct { cat exchangeCategory - pth []string + pth path.Path expect map[exchangeCategory]string }{ { ExchangeContact, - stubPath(path.ExchangeService, path.ContactsCategory, "uid", "cFld", "cid"), + stubPath(suite.T(), "uid", []string{"cFld", "cid"}, path.ContactsCategory), map[exchangeCategory]string{ ExchangeUser: "uid", ExchangeContactFolder: "cFld", @@ -837,7 +837,7 @@ func (suite *ExchangeSelectorSuite) TestIdPath() { }, { ExchangeEvent, - stubPath(path.ExchangeService, path.EventsCategory, "uid", "eCld", "eid"), + stubPath(suite.T(), "uid", []string{"eCld", "eid"}, path.EventsCategory), map[exchangeCategory]string{ ExchangeUser: "uid", ExchangeEventCalendar: "eCld", @@ -846,20 +846,13 @@ func (suite *ExchangeSelectorSuite) TestIdPath() { }, { ExchangeMail, - stubPath(path.ExchangeService, path.EmailCategory, "uid", "mFld", "mid"), + stubPath(suite.T(), "uid", []string{"mFld", "mid"}, path.EmailCategory), map[exchangeCategory]string{ ExchangeUser: "uid", ExchangeMailFolder: "mFld", ExchangeMail: "mid", }, }, - { - ExchangeCategoryUnknown, - stubPath(path.ExchangeService, path.UnknownCategory, "u", "f", "i"), - map[exchangeCategory]string{ - ExchangeUser: "uid", - }, - }, } for _, test := range table { suite.T().Run(test.cat.String(), func(t *testing.T) {}) @@ -884,11 +877,8 @@ func (suite *ExchangeSelectorSuite) TestExchangeRestore_Reduce() { } var ( - // TODO: contacts and events currently do not comply with the mail path pattern - // contact = stubRepoRef(path.ExchangeService, path.ContactsCategory, "uid", "cfld", "cid") - // event = stubRepoRef(path.ExchangeService, path.EventsCategory, "uid", "ecld", "eid") - contact = strings.Join([]string{"tid", "uid", path.ContactsCategory.String(), "cfld", "cid"}, "/") - event = strings.Join([]string{"tid", "uid", path.EventsCategory.String(), "ecld", "eid"}, "/") + contact = stubRepoRef(path.ExchangeService, path.ContactsCategory, "uid", "cfld", "cid") + event = stubRepoRef(path.ExchangeService, path.EventsCategory, "uid", "ecld", "eid") mail = stubRepoRef(path.ExchangeService, path.EmailCategory, "uid", "mfld", "mid") ) @@ -1019,7 +1009,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeRestore_Reduce() { for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { sel := test.makeSelector() - results := sel.Reduce(test.deets) + results := sel.Reduce(context.Background(), test.deets) paths := results.Paths() assert.Equal(t, test.expect, paths) }) @@ -1096,7 +1086,7 @@ func (suite *ExchangeSelectorSuite) TestPasses() { mail = setScopesToDefault(es.Mails(Any(), Any(), []string{mid})) otherMail = setScopesToDefault(es.Mails(Any(), Any(), []string{"smarf"})) noMail = setScopesToDefault(es.Mails(Any(), Any(), None())) - pth = stubPath(path.ExchangeService, path.EmailCategory, "user", "folder", mid) + pth = stubPath(suite.T(), "user", []string{"folder", mid}, path.EmailCategory) ) table := []struct { @@ -1232,51 +1222,38 @@ func (suite *ExchangeSelectorSuite) TestExchangeCategory_leafCat() { } func (suite *ExchangeSelectorSuite) TestExchangeCategory_PathValues() { - // TODO: currently events and contacts are non-compliant with email path patterns - // contactPath := stubPath(path.ExchangeService, path.ContactsCategory, "user", "cfolder", "contactitem") - // contactMap := map[categorizer]string{ - // ExchangeUser: contactPath[2], - // ExchangeContactFolder: contactPath[4], - // ExchangeContact: contactPath[5], - // } - // eventPath := stubPath(path.ExchangeService, path.EventsCategory, "user", "ecalendar", "eventitem") - // eventMap := map[categorizer]string{ - // ExchangeUser: eventPath[2], - // ExchangeEventCalendar: eventPath[4], - // ExchangeEvent: eventPath[5], - // } - mailPath := stubPath(path.ExchangeService, path.EmailCategory, "user", "mfolder", "mailitem") - mailMap := map[categorizer]string{ - ExchangeUser: mailPath[2], - ExchangeMailFolder: mailPath[4], - ExchangeMail: mailPath[5], - } - contactPath := []string{"tid", "user", path.ContactsCategory.String(), "cfolder", "contactitem"} + t := suite.T() + + contactPath := stubPath(t, "user", []string{"cfolder", "contactitem"}, path.ContactsCategory) contactMap := map[categorizer]string{ - ExchangeUser: contactPath[1], - ExchangeContactFolder: contactPath[3], - ExchangeContact: contactPath[4], + ExchangeUser: contactPath.ResourceOwner(), + ExchangeContactFolder: contactPath.Folder(), + ExchangeContact: contactPath.Item(), } - eventPath := []string{"tid", "user", path.EventsCategory.String(), "ecalendar", "eventitem"} + eventPath := stubPath(t, "user", []string{"ecalendar", "eventitem"}, path.EventsCategory) eventMap := map[categorizer]string{ - ExchangeUser: eventPath[1], - ExchangeEventCalendar: eventPath[3], - ExchangeEvent: eventPath[4], + ExchangeUser: eventPath.ResourceOwner(), + ExchangeEventCalendar: eventPath.Folder(), + ExchangeEvent: eventPath.Item(), + } + mailPath := stubPath(t, "user", []string{"mfolder", "mailitem"}, path.EmailCategory) + mailMap := map[categorizer]string{ + ExchangeUser: mailPath.ResourceOwner(), + ExchangeMailFolder: mailPath.Folder(), + ExchangeMail: mailPath.Item(), } table := []struct { cat exchangeCategory - path []string + path path.Path expect map[categorizer]string }{ - {ExchangeCategoryUnknown, nil, map[categorizer]string{}}, - {ExchangeCategoryUnknown, []string{"a"}, map[categorizer]string{}}, {ExchangeContact, contactPath, contactMap}, {ExchangeEvent, eventPath, eventMap}, {ExchangeMail, mailPath, mailMap}, } for _, test := range table { - suite.T().Run(test.cat.String(), func(t *testing.T) { + suite.T().Run(string(test.cat), func(t *testing.T) { assert.Equal(t, test.cat.pathValues(test.path), test.expect) }) } @@ -1301,7 +1278,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeCategory_PathKeys() { {ExchangeUser, user}, } for _, test := range table { - suite.T().Run(test.cat.String(), func(t *testing.T) { + suite.T().Run(string(test.cat), func(t *testing.T) { assert.Equal(t, test.cat.pathKeys(), test.expect) }) } diff --git a/src/pkg/selectors/helpers_test.go b/src/pkg/selectors/helpers_test.go index bafffe791..ceb3923ee 100644 --- a/src/pkg/selectors/helpers_test.go +++ b/src/pkg/selectors/helpers_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/alcionai/corso/src/internal/path" "github.com/alcionai/corso/src/pkg/backup/details" @@ -20,8 +21,12 @@ type mockCategorizer string const ( unknownCatStub mockCategorizer = "" - rootCatStub mockCategorizer = "rootCatStub" - leafCatStub mockCategorizer = "leafCatStub" + // wrap Exchange data here to get around path pkg assertions about path content. + rootCatStub mockCategorizer = mockCategorizer(ExchangeUser) + leafCatStub mockCategorizer = mockCategorizer(ExchangeEvent) + + pathServiceStub = path.ExchangeService + pathCatStub = path.EmailCategory ) var _ categorizer = unknownCatStub @@ -42,7 +47,7 @@ func (mc mockCategorizer) unknownCat() categorizer { return unknownCatStub } -func (mc mockCategorizer) pathValues(pth []string) map[categorizer]string { +func (mc mockCategorizer) pathValues(pth path.Path) map[categorizer]string { return map[categorizer]string{rootCatStub: "stub"} } @@ -152,8 +157,13 @@ func scopeMustHave[T scopeT](t *testing.T, sc T, m map[categorizer]string) { // stubPath ensures test path production matches that of fullPath design, // stubbing out static values where necessary. -func stubPath(service path.ServiceType, data path.CategoryType, resourceOwner, folders, item string) []string { - return []string{"tid", service.String(), resourceOwner, data.String(), folders, item} +func stubPath(t *testing.T, user string, s []string, cat path.CategoryType) path.Path { + pth, err := path.Builder{}. + Append(s...). + ToDataLayerExchangePathForCategory("tid", user, cat, true) + require.NoError(t, err) + + return pth } // stubRepoRef ensures test path production matches that of repoRef design, diff --git a/src/pkg/selectors/onedrive.go b/src/pkg/selectors/onedrive.go index 3cec025a6..6f2942f71 100644 --- a/src/pkg/selectors/onedrive.go +++ b/src/pkg/selectors/onedrive.go @@ -1,6 +1,7 @@ package selectors import ( + "github.com/alcionai/corso/src/internal/path" "github.com/alcionai/corso/src/pkg/backup/details" ) @@ -183,31 +184,14 @@ func (c oneDriveCategory) unknownCat() categorizer { } // pathValues transforms a path to a map of identified properties. -// TODO: this should use service-specific funcs in the Paths pkg. Instead of -// peeking at the path directly, the caller should compare against values like -// path.UserPN() and path.Folders(). // -// Malformed (ie, short len) paths will return incomplete results. // Example: -// [tenantID, userPN, "files", folder, fileID] +// [tenantID, service, userPN, category, folder, fileID] // => {odUser: userPN, odFolder: folder, odFileID: fileID} -func (c oneDriveCategory) pathValues(path []string) map[categorizer]string { - m := map[categorizer]string{} - if len(path) < 3 { - return m +func (c oneDriveCategory) pathValues(p path.Path) map[categorizer]string { + return map[categorizer]string{ + OneDriveUser: p.ResourceOwner(), } - - m[OneDriveUser] = path[2] - /* - TODO/Notice: - Files contain folder structures, identified - in this code as being at index 4. This assumes a single - folder, while in reality users can express subfolder - hierarchies of arbirary depth. Subfolder handling is coming - at a later time. - */ - // TODO: populate path values when known. - return m } // pathKeys returns the path keys recognized by the receiver's leaf type. diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index 87355636c..a2c9f1060 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -1,11 +1,12 @@ package selectors import ( - "strings" + "context" "github.com/alcionai/corso/src/internal/path" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/filters" + "github.com/alcionai/corso/src/pkg/logger" ) // --------------------------------------------------------------------------- @@ -32,17 +33,16 @@ type ( unknownCat() categorizer // pathValues should produce a map of category:string pairs populated by extracting - // values out of the path that match the given categorizer. + // values out of the path.Path struct. // - // Ex: given a path like "tenant/service/root/dataType/folder/itemID", the func should - // autodetect the data type using 'service' and 'dataType', and use the remaining - // details to construct a map similar to this: + // Ex: given a path builder like ["tenant", "service", "resource", "dataType", "folder", "itemID"], + // the func should use the path to construct a map similar to this: // { - // rootCat: root, + // rootCat: resource, // folderCat: folder, // itemCat: itemID, // } - pathValues([]string) map[categorizer]string + pathValues(path.Path) map[categorizer]string // pathKeys produces a list of categorizers that can be used as keys in the pathValues // map. The combination of the two funcs generically interprets the context of the @@ -207,6 +207,7 @@ func isAnyTarget[T scopeT, C categoryT](s T, cat C) bool { // inclusions, filters, and exclusions in the selector. // func reduce[T scopeT, C categoryT]( + ctx context.Context, deets *details.Details, s Selector, dataCategories map[path.CategoryType]C, @@ -224,10 +225,13 @@ func reduce[T scopeT, C categoryT]( // for each entry, compare that entry against the scopes of the same data type for _, ent := range deets.Entries { - // todo: use Path pkg for this - repoPath := strings.Split(ent.RepoRef, "/") + repoPath, err := path.FromDataLayerPath(ent.RepoRef, true) + if err != nil { + logger.Ctx(ctx).Debugw("transforming repoRef to path", "err", err) + continue + } - dc, ok := dataCategories[pathTypeIn(repoPath)] + dc, ok := dataCategories[repoPath.Category()] if !ok { continue } @@ -251,34 +255,6 @@ func reduce[T scopeT, C categoryT]( return reduced } -// return the service data type of the path. -// TODO: this is a hack. We don't want this identification to occur in this -// package. It should get handled in paths, since that's where service- and -// data-type-specific assertions are owned. -// Ideally, we'd use something like path.DataType() instead of this func. -func pathTypeIn(p []string) path.CategoryType { - // not all paths will be len=3. Most should be longer. - // This just protects us from panicing below. - if len(p) < 4 { - return path.UnknownCategory - } - - if c := path.ToCategoryType(p[3]); c != path.UnknownCategory { - return c - } - - switch p[2] { - case path.EmailCategory.String(): - return path.EmailCategory - case path.ContactsCategory.String(): - return path.ContactsCategory - case path.EventsCategory.String(): - return path.EventsCategory - } - - return path.UnknownCategory -} - // groups each scope by its category of data (specified by the service-selector). // ex: a slice containing the scopes [mail1, mail2, event1] // would produce a map like { mail: [1, 2], event: [1] } diff --git a/src/pkg/selectors/scopes_test.go b/src/pkg/selectors/scopes_test.go index 3af959d0c..f4aca08ec 100644 --- a/src/pkg/selectors/scopes_test.go +++ b/src/pkg/selectors/scopes_test.go @@ -1,6 +1,7 @@ package selectors import ( + "context" "testing" "github.com/stretchr/testify/assert" @@ -216,65 +217,37 @@ func (suite *SelectorScopesSuite) TestReduce() { return details.Details{ DetailsModel: details.DetailsModel{ Entries: []details.DetailsEntry{ - {RepoRef: rootCatStub.String() + "/stub/" + leafCatStub.String()}, + { + RepoRef: stubRepoRef( + pathServiceStub, + pathCatStub, + rootCatStub.String(), + "stub", + leafCatStub.String(), + ), + }, }, }, } } dataCats := map[path.CategoryType]mockCategorizer{ - path.UnknownCategory: rootCatStub, + pathCatStub: rootCatStub, } for _, test := range reduceTestTable { suite.T().Run(test.name, func(t *testing.T) { ds := deets() - result := reduce[mockScope](&ds, test.sel().Selector, dataCats) + result := reduce[mockScope]( + context.Background(), + &ds, + test.sel().Selector, + dataCats) require.NotNil(t, result) assert.Len(t, result.Entries, test.expectLen) }) } } -func (suite *SelectorScopesSuite) TestPathTypeIn() { - table := []struct { - name string - pathType path.CategoryType - pth []string - }{ - { - name: "empty", - pathType: path.UnknownCategory, - pth: []string{}, - }, - { - name: "email", - pathType: path.EmailCategory, - pth: stubPath(path.ExchangeService, path.EmailCategory, "", "", ""), - }, - { - name: "contact", - pathType: path.ContactsCategory, - pth: stubPath(path.ExchangeService, path.ContactsCategory, "", "", ""), - }, - { - name: "event", - pathType: path.EventsCategory, - pth: stubPath(path.ExchangeService, path.EventsCategory, "", "", ""), - }, - { - name: "bogus", - pathType: path.UnknownCategory, - pth: []string{"", "", "", "fnords", "", ""}, - }, - } - for _, test := range table { - suite.T().Run(test.name, func(t *testing.T) { - result := pathTypeIn(test.pth) - assert.Equal(t, test.pathType, result) - }) - } -} - func (suite *SelectorScopesSuite) TestScopesByCategory() { t := suite.T() s1 := stubScope("")