From bf398f6c8db3495a34e062b42a822097b92acbeb Mon Sep 17 00:00:00 2001 From: ryanfkeepers Date: Fri, 11 Aug 2023 11:50:11 -0600 Subject: [PATCH] fixing up selectors adds a set of fixes targeted at the selectors package. Largely updates testing code. But also updates the resource check at the beginning of the Reduce flow to check for a match on any of the resources in the ServiceResources slice. --- src/pkg/path/path.go | 2 +- src/pkg/path/resource_path.go | 2 +- src/pkg/path/service_resource.go | 22 ++++- src/pkg/path/service_resource_test.go | 4 +- src/pkg/selectors/exchange_test.go | 40 +++++++-- src/pkg/selectors/helpers_test.go | 21 ++++- src/pkg/selectors/onedrive_test.go | 21 +++-- src/pkg/selectors/scopes.go | 7 +- src/pkg/selectors/scopes_test.go | 115 +++++++++++++++++++++----- src/pkg/selectors/sharepoint_test.go | 27 +++++- 10 files changed, 212 insertions(+), 49 deletions(-) diff --git a/src/pkg/path/path.go b/src/pkg/path/path.go index 3454d1e9e..1cfa6b31d 100644 --- a/src/pkg/path/path.go +++ b/src/pkg/path/path.go @@ -186,7 +186,7 @@ func FromDataLayerPath(p string, isItem bool) (Path, error) { return nil, clues.New("path has too few segments").With("path_string", p) } - srs, catIdx, err := ElementsToServiceResources(pb.elements[1:]) + srs, catIdx, err := elementsToServiceResources(pb.elements[1:]) if err != nil { return nil, clues.Stack(err) } diff --git a/src/pkg/path/resource_path.go b/src/pkg/path/resource_path.go index 1bd380286..a8cb6222a 100644 --- a/src/pkg/path/resource_path.go +++ b/src/pkg/path/resource_path.go @@ -31,7 +31,7 @@ func newDataLayerResourcePath( cat CategoryType, isItem bool, ) dataLayerResourcePath { - pfx := append([]string{tenant}, ServiceResourcesToElements(srs)...) + pfx := append([]string{tenant}, serviceResourcesToElements(srs)...) pfx = append(pfx, cat.String()) return dataLayerResourcePath{ diff --git a/src/pkg/path/service_resource.go b/src/pkg/path/service_resource.go index c2db97f7d..77317d029 100644 --- a/src/pkg/path/service_resource.go +++ b/src/pkg/path/service_resource.go @@ -37,7 +37,7 @@ func (sr ServiceResource) validate() error { } // --------------------------------------------------------------------------- -// Collection +// Exported Helpers // --------------------------------------------------------------------------- // NewServiceResources is a lenient constructor for building a @@ -74,7 +74,21 @@ func NewServiceResources(elems ...any) ([]ServiceResource, error) { return srs, nil } -func ServiceResourcesToElements(srs []ServiceResource) Elements { +func ServiceResourcesToResources(srs []ServiceResource) []string { + prs := make([]string, len(srs)) + + for i := range srs { + prs[i] = srs[i].ProtectedResource + } + + return prs +} + +// --------------------------------------------------------------------------- +// Unexported Helpers +// --------------------------------------------------------------------------- + +func serviceResourcesToElements(srs []ServiceResource) Elements { es := make(Elements, 0, len(srs)*2) for _, tuple := range srs { @@ -85,7 +99,7 @@ func ServiceResourcesToElements(srs []ServiceResource) Elements { return es } -// ElementsToServiceResources turns as many pairs of elems as possible +// elementsToServiceResources turns as many pairs of elems as possible // into ServiceResource tuples. Elems must begin with a service, but // may contain more entries than there are service/resource pairs. // This transformer will continue consuming elements until it finds an @@ -93,7 +107,7 @@ func ServiceResourcesToElements(srs []ServiceResource) Elements { // Returns the serviceResource pairs, the first index containing element // that is not part of a service/resource pair, and an error if elems is // len==0 or contains no services. -func ElementsToServiceResources(elems Elements) ([]ServiceResource, int, error) { +func elementsToServiceResources(elems Elements) ([]ServiceResource, int, error) { if len(elems) == 0 { return nil, -1, clues.Wrap(errMissingSegment, "service") } diff --git a/src/pkg/path/service_resource_test.go b/src/pkg/path/service_resource_test.go index 90c320b59..97139c49a 100644 --- a/src/pkg/path/service_resource_test.go +++ b/src/pkg/path/service_resource_test.go @@ -157,7 +157,7 @@ func (suite *ServiceResourceUnitSuite) TestServiceResourceToElements() { suite.Run(test.name, func() { t := suite.T() - result := ServiceResourcesToElements(test.srs) + result := serviceResourcesToElements(test.srs) // not ElementsMatch, order matters assert.Equal(t, test.expect, result) @@ -234,7 +234,7 @@ func (suite *ServiceResourceUnitSuite) TestElementsToServiceResource() { suite.Run(test.name, func() { t := suite.T() - srs, idx, err := ElementsToServiceResources(test.elems) + srs, idx, err := elementsToServiceResources(test.elems) test.expectErr(t, err, clues.ToCore(err)) assert.Equal(t, test.expectIdx, idx) assert.Equal(t, test.expectSRS, srs) diff --git a/src/pkg/selectors/exchange_test.go b/src/pkg/selectors/exchange_test.go index c3dd0beb4..c02565f18 100644 --- a/src/pkg/selectors/exchange_test.go +++ b/src/pkg/selectors/exchange_test.go @@ -815,10 +815,12 @@ func (suite *ExchangeSelectorSuite) TestExchangeRestore_Reduce() { } joinedFldrs := strings.Join(newElems, "/") - - sr0 := p.ServiceResources()[0] - - return stubRepoRef(sr0.Service, p.Category(), sr0.ProtectedResource, joinedFldrs, p.Item()) + return stubRepoRef( + suite.T(), + p.ServiceResources(), + p.Category(), + joinedFldrs, + p.Item()) } makeDeets := func(refs ...path.Path) *details.Details { @@ -1060,12 +1062,36 @@ func (suite *ExchangeSelectorSuite) TestExchangeRestore_Reduce() { func (suite *ExchangeSelectorSuite) TestExchangeRestore_Reduce_locationRef() { var ( - contact = stubRepoRef(path.ExchangeService, path.ContactsCategory, "uid", "id5/id6", "cid") contactLocation = "conts/my_cont" - event = stubRepoRef(path.ExchangeService, path.EventsCategory, "uid", "id1/id2", "eid") eventLocation = "cal/my_cal" - mail = stubRepoRef(path.ExchangeService, path.EmailCategory, "uid", "id3/id4", "mid") mailLocation = "inbx/my_mail" + contact = stubRepoRef( + suite.T(), + []path.ServiceResource{{ + Service: path.ExchangeService, + ProtectedResource: "uid", + }}, + path.ContactsCategory, + "id5/id6", + "cid") + event = stubRepoRef( + suite.T(), + []path.ServiceResource{{ + Service: path.ExchangeService, + ProtectedResource: "uid", + }}, + path.EventsCategory, + "id1/id2", + "eid") + mail = stubRepoRef( + suite.T(), + []path.ServiceResource{{ + Service: path.ExchangeService, + ProtectedResource: "uid", + }}, + path.EmailCategory, + "id3/id4", + "mid") ) makeDeets := func(refs ...string) *details.Details { diff --git a/src/pkg/selectors/helpers_test.go b/src/pkg/selectors/helpers_test.go index 613556ab7..f3fa72a0e 100644 --- a/src/pkg/selectors/helpers_test.go +++ b/src/pkg/selectors/helpers_test.go @@ -2,7 +2,6 @@ package selectors import ( "fmt" - "strings" "testing" "github.com/alcionai/clues" @@ -229,6 +228,22 @@ func stubPath(t *testing.T, user string, s []string, cat path.CategoryType) path // 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}, "/") +func stubRepoRef( + t *testing.T, + srs []path.ServiceResource, + cat path.CategoryType, + folders, item string, +) string { + fs := path.Split(folders) + fs = append(fs, item) + + pb, err := path.Build( + "tid", + srs, + cat, + true, + fs...) + require.NoError(t, err, clues.ToCore(err)) + + return pb.String() } diff --git a/src/pkg/selectors/onedrive_test.go b/src/pkg/selectors/onedrive_test.go index 693a6e29d..83a954532 100644 --- a/src/pkg/selectors/onedrive_test.go +++ b/src/pkg/selectors/onedrive_test.go @@ -161,23 +161,32 @@ func (suite *OneDriveSelectorSuite) TestToOneDriveRestore() { func (suite *OneDriveSelectorSuite) TestOneDriveRestore_Reduce() { var ( file = stubRepoRef( - path.OneDriveService, + suite.T(), + []path.ServiceResource{{ + Service: path.OneDriveService, + ProtectedResource: "uid", + }}, path.FilesCategory, - "uid", "drive/driveID/root:/folderA.d/folderB.d", "file") fileParent = "folderA/folderB" file2 = stubRepoRef( - path.OneDriveService, + suite.T(), + []path.ServiceResource{{ + Service: path.OneDriveService, + ProtectedResource: "uid", + }}, path.FilesCategory, - "uid", "drive/driveID/root:/folderA.d/folderC.d", "file2") fileParent2 = "folderA/folderC" file3 = stubRepoRef( - path.OneDriveService, + suite.T(), + []path.ServiceResource{{ + Service: path.OneDriveService, + ProtectedResource: "uid", + }}, path.FilesCategory, - "uid", "drive/driveID/root:/folderD.d/folderE.d", "file3") fileParent3 = "folderD/folderE" diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index 5a453bf4f..e1e3eb881 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -544,8 +544,11 @@ func reduce[T scopeT, C categoryT]( continue } - // first check, every entry needs to match the selector's resource owners. - if !matchesResourceOwner.Compare(repoPath.ResourceOwner()) { + // first check, every entry needs to have at least one protected resource + // that matches the selector's protected resources. + if !matchesResourceOwner.CompareAny( + path.ServiceResourcesToResources( + repoPath.ServiceResources())...) { continue } diff --git a/src/pkg/selectors/scopes_test.go b/src/pkg/selectors/scopes_test.go index aeae23ef3..02a128590 100644 --- a/src/pkg/selectors/scopes_test.go +++ b/src/pkg/selectors/scopes_test.go @@ -22,15 +22,15 @@ import ( // tests // --------------------------------------------------------------------------- -type SelectorScopesSuite struct { +type ScopesUnitSuite struct { tester.Suite } -func TestSelectorScopesSuite(t *testing.T) { - suite.Run(t, &SelectorScopesSuite{Suite: tester.NewUnitSuite(t)}) +func TestScopesUnitSuite(t *testing.T) { + suite.Run(t, &ScopesUnitSuite{Suite: tester.NewUnitSuite(t)}) } -func (suite *SelectorScopesSuite) TestContains() { +func (suite *ScopesUnitSuite) TestContains() { table := []struct { name string scope func() mockScope @@ -108,7 +108,7 @@ func (suite *SelectorScopesSuite) TestContains() { } } -func (suite *SelectorScopesSuite) TestGetCatValue() { +func (suite *ScopesUnitSuite) TestGetCatValue() { t := suite.T() stub := stubScope("") @@ -122,7 +122,7 @@ func (suite *SelectorScopesSuite) TestGetCatValue() { getCatValue(stub, mockCategorizer("foo"))) } -func (suite *SelectorScopesSuite) TestIsAnyTarget() { +func (suite *ScopesUnitSuite) TestIsAnyTarget() { t := suite.T() stub := stubScope("") assert.True(t, isAnyTarget(stub, rootCatStub)) @@ -253,16 +253,19 @@ var reduceTestTable = []struct { }, } -func (suite *SelectorScopesSuite) TestReduce() { +func (suite *ScopesUnitSuite) TestReduce() { deets := func() details.Details { return details.Details{ DetailsModel: details.DetailsModel{ Entries: []details.Entry{ { RepoRef: stubRepoRef( - pathServiceStub, + suite.T(), + []path.ServiceResource{{ + Service: pathServiceStub, + ProtectedResource: rootCatStub.String(), + }}, pathCatStub, - rootCatStub.String(), "stub", leafCatStub.String(), ), @@ -298,16 +301,90 @@ func (suite *SelectorScopesSuite) TestReduce() { } } -func (suite *SelectorScopesSuite) TestReduce_locationRef() { +func (suite *ScopesUnitSuite) TestReduce_locationRef() { + deets := func() details.Details { + return details.Details{ + DetailsModel: details.DetailsModel{ + Entries: []details.Entry{{ + RepoRef: stubRepoRef( + suite.T(), + []path.ServiceResource{{ + Service: pathServiceStub, + ProtectedResource: rootCatStub.String(), + }}, + pathCatStub, + "stub", + leafCatStub.String(), + ), + LocationRef: "a/b/c//defg", + }}, + }, + } + } + dataCats := map[path.CategoryType]mockCategorizer{ + pathCatStub: rootCatStub, + } + + for _, test := range reduceTestTable { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + ds := deets() + result := reduce[mockScope]( + ctx, + &ds, + test.sel().Selector, + dataCats, + fault.New(true)) + require.NotNil(t, result) + assert.Len(t, result.Entries, test.expectLen) + }) + } +} + +func (suite *ScopesUnitSuite) TestReduce_multipleServiceResources() { deets := func() details.Details { return details.Details{ DetailsModel: details.DetailsModel{ Entries: []details.Entry{ + // tid/serv/"matching-id"/subserv/"non-matching-id"/... { RepoRef: stubRepoRef( - pathServiceStub, + suite.T(), + []path.ServiceResource{ + { + Service: pathServiceStub, + ProtectedResource: rootCatStub.String(), + }, + { + Service: pathServiceStub, + ProtectedResource: "foo", + }, + }, + pathCatStub, + "stub", + leafCatStub.String(), + ), + LocationRef: "a/b/c//defg", + }, + // tid/serv/"non-matching-id"/subserv/"matching-id"/... + { + RepoRef: stubRepoRef( + suite.T(), + []path.ServiceResource{ + { + Service: pathServiceStub, + ProtectedResource: "foo", + }, + { + Service: pathServiceStub, + ProtectedResource: rootCatStub.String(), + }, + }, pathCatStub, - rootCatStub.String(), "stub", leafCatStub.String(), ), @@ -341,7 +418,7 @@ func (suite *SelectorScopesSuite) TestReduce_locationRef() { } } -func (suite *SelectorScopesSuite) TestScopesByCategory() { +func (suite *ScopesUnitSuite) TestScopesByCategory() { t := suite.T() s1 := stubScope("") s2 := stubScope("") @@ -357,7 +434,7 @@ func (suite *SelectorScopesSuite) TestScopesByCategory() { assert.Empty(t, result[leafCatStub]) } -func (suite *SelectorScopesSuite) TestPasses() { +func (suite *ScopesUnitSuite) TestPasses() { var ( cat = rootCatStub pth = stubPath(suite.T(), "uid", []string{"fld"}, path.EventsCategory) @@ -401,7 +478,7 @@ func toMockScope(sc []scope) []mockScope { return ms } -func (suite *SelectorScopesSuite) TestMatchesPathValues() { +func (suite *ScopesUnitSuite) TestMatchesPathValues() { cat := rootCatStub short := "brunheelda" @@ -460,7 +537,7 @@ func (suite *SelectorScopesSuite) TestMatchesPathValues() { } } -func (suite *SelectorScopesSuite) TestDefaultItemOptions() { +func (suite *ScopesUnitSuite) TestDefaultItemOptions() { table := []struct { name string cfg Config @@ -521,7 +598,7 @@ func (suite *SelectorScopesSuite) TestDefaultItemOptions() { } } -func (suite *SelectorScopesSuite) TestClean() { +func (suite *ScopesUnitSuite) TestClean() { table := []struct { name string input []string @@ -568,7 +645,7 @@ func (suite *SelectorScopesSuite) TestClean() { } } -func (suite *SelectorScopesSuite) TestScopeConfig() { +func (suite *ScopesUnitSuite) TestScopeConfig() { input := "input" table := []struct { @@ -608,7 +685,7 @@ func (ms mockFMTState) Width() (int, bool) { return 0, false } func (ms mockFMTState) Precision() (int, bool) { return 0, false } func (ms mockFMTState) Flag(int) bool { return false } -func (suite *SelectorScopesSuite) TestScopesPII() { +func (suite *ScopesUnitSuite) TestScopesPII() { table := []struct { name string s mockScope diff --git a/src/pkg/selectors/sharepoint_test.go b/src/pkg/selectors/sharepoint_test.go index a41bb950b..19593c12e 100644 --- a/src/pkg/selectors/sharepoint_test.go +++ b/src/pkg/selectors/sharepoint_test.go @@ -155,9 +155,12 @@ func (suite *SharePointSelectorSuite) TestSharePointRestore_Reduce() { } return stubRepoRef( - path.SharePointService, + suite.T(), + []path.ServiceResource{{ + Service: path.SharePointService, + ProtectedResource: siteID, + }}, cat, - siteID, strings.Join(folderElems, "/"), item) } @@ -188,8 +191,24 @@ func (suite *SharePointSelectorSuite) TestSharePointRestore_Reduce() { "sid", append(slices.Clone(prefixElems), itemElems3...), "item3") - item4 = stubRepoRef(path.SharePointService, path.PagesCategory, "sid", pairGH, "item4") - item5 = stubRepoRef(path.SharePointService, path.PagesCategory, "sid", pairGH, "item5") + item4 = stubRepoRef( + suite.T(), + []path.ServiceResource{{ + Service: path.SharePointService, + ProtectedResource: "sid", + }}, + path.PagesCategory, + pairGH, + "item4") + item5 = stubRepoRef( + suite.T(), + []path.ServiceResource{{ + Service: path.SharePointService, + ProtectedResource: "sid", + }}, + path.PagesCategory, + pairGH, + "item5") ) deets := &details.Details{