From c73ca6a4e5169f042f8b6d0c0357d7d65a3ac548 Mon Sep 17 00:00:00 2001 From: Keepers Date: Wed, 7 Sep 2022 14:14:51 -0600 Subject: [PATCH] correct paths in selectors (#789) The path package changed the standard format of fullPath and repoRef design. This should have failed tests before being pushed to main, but was able to slip in falsely while github actions were configured to pass all tests until failed. --- src/internal/connector/graph_connector.go | 4 +- .../connector/graph_connector_test.go | 2 +- src/pkg/logger/logger.go | 7 +++ src/pkg/selectors/exchange.go | 16 ++--- src/pkg/selectors/exchange_test.go | 58 ++++++++++--------- src/pkg/selectors/helpers_test.go | 16 ++++- src/pkg/selectors/onedrive.go | 6 +- src/pkg/selectors/scopes.go | 4 +- src/pkg/selectors/scopes_test.go | 44 ++++++++++++-- 9 files changed, 106 insertions(+), 51 deletions(-) diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index ab578b79c..8b207b8d4 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -259,10 +259,10 @@ func (gc *GraphConnector) RestoreExchangeDataCollection( for _, dc := range dcs { var ( directory = strings.Join(dc.FullPath(), "") - user = dc.FullPath()[1] + user = dc.FullPath()[2] items = dc.Items() // TODO(ashmrtn): Update this when we have path struct support in collections. - category = path.ToCategoryType(dc.FullPath()[2]) + category = path.ToCategoryType(dc.FullPath()[3]) exit bool ) diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 557b79b46..f74006287 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -252,7 +252,7 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreMessages() { for i := 0; i < 3; i++ { mdc := mockconnector.NewMockExchangeCollection( - []string{"tenant", suite.user, category.String(), "Inbox"}, + []string{"tenant", path.ExchangeService.String(), suite.user, category.String(), "Inbox"}, 1) collection = append(collection, mdc) } diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index 0dd339d9f..1a72a574a 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -43,6 +43,13 @@ func singleton(level logLevel) *zap.SugaredLogger { return loggerton } + // when testing, ensure debug logging matches the test.v setting + for _, arg := range os.Args { + if arg == `--test.v=true` { + level = Development + } + } + // set up a logger core to use as a fallback levelFilter := zap.LevelEnablerFunc(func(lvl zapcore.Level) bool { switch level { diff --git a/src/pkg/selectors/exchange.go b/src/pkg/selectors/exchange.go index 91ec2a1d2..10ad0549f 100644 --- a/src/pkg/selectors/exchange.go +++ b/src/pkg/selectors/exchange.go @@ -555,24 +555,24 @@ func (ec exchangeCategory) unknownCat() categorizer { // => {exchUser: userPN, exchMailFolder: mailFolder, exchMail: mailID} func (ec exchangeCategory) pathValues(path []string) map[categorizer]string { m := map[categorizer]string{} - if len(path) < 5 { + if len(path) < 6 { return m } - m[ExchangeUser] = path[1] + m[ExchangeUser] = path[2] switch ec { case ExchangeContact: - m[ExchangeContactFolder] = path[3] - m[ExchangeContact] = path[4] + m[ExchangeContactFolder] = path[4] + m[ExchangeContact] = path[5] case ExchangeEvent: - m[ExchangeEventCalendar] = path[3] - m[ExchangeEvent] = path[4] + m[ExchangeEventCalendar] = path[4] + m[ExchangeEvent] = path[5] case ExchangeMail: - m[ExchangeMailFolder] = path[3] - m[ExchangeMail] = path[4] + m[ExchangeMailFolder] = path[4] + m[ExchangeMail] = path[5] } return m diff --git a/src/pkg/selectors/exchange_test.go b/src/pkg/selectors/exchange_test.go index 6624894b9..1aa725004 100644 --- a/src/pkg/selectors/exchange_test.go +++ b/src/pkg/selectors/exchange_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/internal/common" + "github.com/alcionai/corso/internal/path" "github.com/alcionai/corso/pkg/backup/details" "github.com/alcionai/corso/pkg/filters" ) @@ -777,8 +778,8 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_MatchesPath() { ) var ( - path = []string{"tid", usr, "email", fld, mail} - es = NewExchangeRestore() + pth = stubPath(path.ExchangeService, path.EmailCategory, usr, fld, mail) + es = NewExchangeRestore() ) table := []struct { @@ -807,7 +808,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_MatchesPath() { scopes := setScopesToDefault(test.scope) var aMatch bool for _, scope := range scopes { - pv := ExchangeMail.pathValues(path) + pv := ExchangeMail.pathValues(pth) if matchesPathValues(scope, ExchangeMail, pv) { aMatch = true break @@ -821,12 +822,12 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_MatchesPath() { func (suite *ExchangeSelectorSuite) TestIdPath() { table := []struct { cat exchangeCategory - path []string + pth []string expect map[exchangeCategory]string }{ { ExchangeContact, - []string{"tid", "uid", "contact", "cFld", "cid"}, + stubPath(path.ExchangeService, path.ContactsCategory, "uid", "cFld", "cid"), map[exchangeCategory]string{ ExchangeUser: "uid", ExchangeContactFolder: "cFld", @@ -835,15 +836,16 @@ func (suite *ExchangeSelectorSuite) TestIdPath() { }, { ExchangeEvent, - []string{"tid", "uid", "event", "eid"}, + stubPath(path.ExchangeService, path.EventsCategory, "uid", "eCld", "eid"), map[exchangeCategory]string{ - ExchangeUser: "uid", - ExchangeEvent: "eid", + ExchangeUser: "uid", + ExchangeEventCalendar: "eCld", + ExchangeEvent: "eid", }, }, { ExchangeMail, - []string{"tid", "uid", "email", "mFld", "mid"}, + stubPath(path.ExchangeService, path.EmailCategory, "uid", "mFld", "mid"), map[exchangeCategory]string{ ExchangeUser: "uid", ExchangeMailFolder: "mFld", @@ -852,7 +854,7 @@ func (suite *ExchangeSelectorSuite) TestIdPath() { }, { ExchangeCategoryUnknown, - []string{"tid", "uid", "contact", "cFld", "cid"}, + stubPath(path.ExchangeService, path.UnknownCategory, "u", "f", "i"), map[exchangeCategory]string{ ExchangeUser: "uid", }, @@ -880,10 +882,10 @@ func (suite *ExchangeSelectorSuite) TestExchangeRestore_Reduce() { return deets } - const ( - contact = "tid/uid/contacts/cfld/cid" - event = "tid/uid/events/ecld/eid" - mail = "tid/uid/email/mfld/mid" + var ( + 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") ) arr := func(s ...string) []string { @@ -1090,7 +1092,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())) - path = []string{"tid", "user", "email", "folder", mid} + pth = stubPath(path.ExchangeService, path.EmailCategory, "user", "folder", mid) ) table := []struct { @@ -1121,7 +1123,7 @@ func (suite *ExchangeSelectorSuite) TestPasses() { suite.T().Run(test.name, func(t *testing.T) { result := passes( cat, - cat.pathValues(path), + cat.pathValues(pth), deets, test.excludes, test.filters, @@ -1226,23 +1228,23 @@ func (suite *ExchangeSelectorSuite) TestExchangeCategory_leafCat() { } func (suite *ExchangeSelectorSuite) TestExchangeCategory_PathValues() { - contactPath := []string{"ten", "user", "contact", "cfolder", "contactitem"} + contactPath := stubPath(path.ExchangeService, path.ContactsCategory, "user", "cfolder", "contactitem") contactMap := map[categorizer]string{ - ExchangeUser: contactPath[1], - ExchangeContactFolder: contactPath[3], - ExchangeContact: contactPath[4], + ExchangeUser: contactPath[2], + ExchangeContactFolder: contactPath[4], + ExchangeContact: contactPath[5], } - eventPath := []string{"ten", "user", "event", "ecalendar", "eventitem"} + eventPath := stubPath(path.ExchangeService, path.EventsCategory, "user", "ecalendar", "eventitem") eventMap := map[categorizer]string{ - ExchangeUser: eventPath[1], - ExchangeEventCalendar: eventPath[3], - ExchangeEvent: eventPath[4], + ExchangeUser: eventPath[2], + ExchangeEventCalendar: eventPath[4], + ExchangeEvent: eventPath[5], } - mailPath := []string{"ten", "user", "mail", "mfolder", "mailitem"} + mailPath := stubPath(path.ExchangeService, path.EmailCategory, "user", "mfolder", "mailitem") mailMap := map[categorizer]string{ - ExchangeUser: contactPath[1], - ExchangeMailFolder: mailPath[3], - ExchangeMail: mailPath[4], + ExchangeUser: contactPath[2], + ExchangeMailFolder: mailPath[4], + ExchangeMail: mailPath[5], } table := []struct { diff --git a/src/pkg/selectors/helpers_test.go b/src/pkg/selectors/helpers_test.go index b29102ba9..5477b1d75 100644 --- a/src/pkg/selectors/helpers_test.go +++ b/src/pkg/selectors/helpers_test.go @@ -1,10 +1,12 @@ package selectors import ( + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/alcionai/corso/internal/path" "github.com/alcionai/corso/pkg/backup/details" "github.com/alcionai/corso/pkg/filters" ) @@ -40,7 +42,7 @@ func (mc mockCategorizer) unknownCat() categorizer { return unknownCatStub } -func (mc mockCategorizer) pathValues(path []string) map[categorizer]string { +func (mc mockCategorizer) pathValues(pth []string) map[categorizer]string { return map[categorizer]string{rootCatStub: "stub"} } @@ -140,3 +142,15 @@ 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} +} + +// stubRepoRef ensures test path production matches that of repoRef design, +// stubbing out static values where necessary. +func stubRepoRef(service path.ServiceType, data path.CategoryType, resourceOwner, folders, item string) string { + return strings.Join([]string{"tid", service.String(), resourceOwner, data.String(), folders, item}, "/") +} diff --git a/src/pkg/selectors/onedrive.go b/src/pkg/selectors/onedrive.go index a4391c09a..46561c3a0 100644 --- a/src/pkg/selectors/onedrive.go +++ b/src/pkg/selectors/onedrive.go @@ -188,15 +188,15 @@ func (c oneDriveCategory) unknownCat() categorizer { // => {odUser: userPN, odFolder: folder, odFileID: fileID} func (c oneDriveCategory) pathValues(path []string) map[categorizer]string { m := map[categorizer]string{} - if len(path) < 2 { + if len(path) < 3 { return m } - m[OneDriveUser] = path[1] + m[OneDriveUser] = path[2] /* TODO/Notice: Files contain folder structures, identified - in this code as being at index 3. This assumes a single + 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. diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index 68f42aac8..405bea830 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -287,11 +287,11 @@ const ( func pathTypeIn(p []string) pathType { // not all paths will be len=3. Most should be longer. // This just protects us from panicing below. - if len(p) < 3 { + if len(p) < 4 { return unknownPathType } - switch p[2] { + switch p[3] { case path.EmailCategory.String(): return exchangeMailPath case path.ContactsCategory.String(): diff --git a/src/pkg/selectors/scopes_test.go b/src/pkg/selectors/scopes_test.go index c268be621..4863e8206 100644 --- a/src/pkg/selectors/scopes_test.go +++ b/src/pkg/selectors/scopes_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/alcionai/corso/internal/path" "github.com/alcionai/corso/pkg/backup/details" "github.com/alcionai/corso/pkg/filters" ) @@ -241,12 +242,43 @@ func (suite *SelectorScopesSuite) TestReduce() { } func (suite *SelectorScopesSuite) TestPathTypeIn() { - t := suite.T() - assert.Equal(t, unknownPathType, pathTypeIn([]string{}), "empty") - assert.Equal(t, exchangeMailPath, pathTypeIn([]string{"", "", "email"}), "email") - assert.Equal(t, exchangeContactPath, pathTypeIn([]string{"", "", "contacts"}), "contact") - assert.Equal(t, exchangeEventPath, pathTypeIn([]string{"", "", "events"}), "event") - assert.Equal(t, unknownPathType, pathTypeIn([]string{"", "", "fnords"}), "bogus") + table := []struct { + name string + pathType pathType + pth []string + }{ + { + name: "empty", + pathType: unknownPathType, + pth: []string{}, + }, + { + name: "email", + pathType: exchangeMailPath, + pth: stubPath(path.ExchangeService, path.EmailCategory, "", "", ""), + }, + { + name: "contact", + pathType: exchangeContactPath, + pth: stubPath(path.ExchangeService, path.ContactsCategory, "", "", ""), + }, + { + name: "event", + pathType: exchangeEventPath, + pth: stubPath(path.ExchangeService, path.EventsCategory, "", "", ""), + }, + { + name: "bogus", + pathType: unknownPathType, + 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() {