From 1f2633981367de7dc31ca763c4a0d1c77ac5108b Mon Sep 17 00:00:00 2001 From: Keepers Date: Wed, 4 Jan 2023 17:59:16 -0700 Subject: [PATCH] Match resource owners at top of reduce (#1891) ## Description Checks for resource owner matches in the top of the reduce func using the selector owners, instead of waiting until the path match check. ## Does this PR need a docs update or release note? - [x] :no_entry: No ## Type of change - [x] :sunflower: Feature ## Issue(s) * #1617 ## Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- CHANGELOG.md | 14 +++- src/cli/backup/exchange.go | 3 +- src/cli/backup/onedrive.go | 3 +- src/cli/backup/sharepoint.go | 3 +- src/cli/restore/exchange.go | 3 +- src/cli/restore/onedrive.go | 3 +- src/cli/restore/sharepoint.go | 3 +- src/cli/utils/exchange.go | 28 ++++--- src/cli/utils/exchange_test.go | 3 +- src/cli/utils/onedrive.go | 29 +++---- src/cli/utils/onedrive_test.go | 7 +- src/cli/utils/sharepoint.go | 22 ++--- src/cli/utils/sharepoint_test.go | 8 +- src/cli/utils/testdata/opts.go | 1 + .../connector/graph_connector_helper_test.go | 15 ++-- src/internal/operations/backup_test.go | 5 +- src/internal/operations/restore_test.go | 1 + src/pkg/selectors/example_selectors_test.go | 4 +- src/pkg/selectors/exchange_test.go | 12 +-- src/pkg/selectors/helpers_test.go | 11 +-- src/pkg/selectors/scopes.go | 16 ++++ src/pkg/selectors/scopes_test.go | 59 ++++++++----- src/pkg/selectors/selectors_test.go | 83 +++++++++++-------- 23 files changed, 197 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 683638e57..28c4d2de3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,13 +5,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased] (alpha) + +### Changed + +- The selectors Reduce() process will only include details that match the DiscreteOwner, if one is specified. +- New selector constructors will automatically set the DiscreteOwner if given a single-item slice. ### Fixed - Fixed issue where repository connect progress bar was clobbering backup/restore operation output. +- Fixed issue where a `backup create exchange` produced one backup record per data type. -## [v0.0.4] (alpha) +### Known Issues + +- `backup list` will not display a resource owner for backups created prior to this release. + +## [v0.0.4] (alpha) - 2022-12-23 ### Added diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index 527e13765..f7cfffbc2 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -505,8 +505,7 @@ func runDetailsExchangeCmd( return nil, errors.Wrap(err, "Failed to get backup details in the repository") } - sel := selectors.NewExchangeRestore(nil) // TODO: generate selector in IncludeExchangeRestoreDataSelectors - utils.IncludeExchangeRestoreDataSelectors(sel, opts) + sel := utils.IncludeExchangeRestoreDataSelectors(opts) utils.FilterExchangeRestoreInfoSelectors(sel, opts) // if no selector flags were specified, get all data in the service. diff --git a/src/cli/backup/onedrive.go b/src/cli/backup/onedrive.go index 5b602d8ee..691fd79f4 100644 --- a/src/cli/backup/onedrive.go +++ b/src/cli/backup/onedrive.go @@ -396,8 +396,7 @@ func runDetailsOneDriveCmd( return nil, errors.Wrap(err, "Failed to get backup details in the repository") } - sel := selectors.NewOneDriveRestore(nil) // TODO: generate selector in IncludeExchangeRestoreDataSelectors - utils.IncludeOneDriveRestoreDataSelectors(sel, opts) + sel := utils.IncludeOneDriveRestoreDataSelectors(opts) utils.FilterOneDriveRestoreInfoSelectors(sel, opts) // if no selector flags were specified, get all data in the service. diff --git a/src/cli/backup/sharepoint.go b/src/cli/backup/sharepoint.go index a2b7889ac..8b517905b 100644 --- a/src/cli/backup/sharepoint.go +++ b/src/cli/backup/sharepoint.go @@ -479,8 +479,7 @@ func runDetailsSharePointCmd( return nil, errors.Wrap(err, "Failed to get backup details in the repository") } - sel := selectors.NewSharePointRestore(nil) // TODO: generate selector in IncludeSharePointRestoreDataSelectors - utils.IncludeSharePointRestoreDataSelectors(sel, opts) + sel := utils.IncludeSharePointRestoreDataSelectors(opts) utils.FilterSharePointRestoreInfoSelectors(sel, opts) // if no selector flags were specified, get all data in the service. diff --git a/src/cli/restore/exchange.go b/src/cli/restore/exchange.go index d034cce98..eae8eb6c3 100644 --- a/src/cli/restore/exchange.go +++ b/src/cli/restore/exchange.go @@ -216,8 +216,7 @@ func restoreExchangeCmd(cmd *cobra.Command, args []string) error { defer utils.CloseRepo(ctx, r) - sel := selectors.NewExchangeRestore(nil) // TODO: generate selector in IncludeExchangeRestoreDataSelectors - utils.IncludeExchangeRestoreDataSelectors(sel, opts) + sel := utils.IncludeExchangeRestoreDataSelectors(opts) utils.FilterExchangeRestoreInfoSelectors(sel, opts) // if no selector flags were specified, get all data in the service. diff --git a/src/cli/restore/onedrive.go b/src/cli/restore/onedrive.go index a65dae5c6..336cc6496 100644 --- a/src/cli/restore/onedrive.go +++ b/src/cli/restore/onedrive.go @@ -153,8 +153,7 @@ func restoreOneDriveCmd(cmd *cobra.Command, args []string) error { defer utils.CloseRepo(ctx, r) - sel := selectors.NewOneDriveRestore(nil) // TODO: generate selector in IncludeOneDriveRestoreDataSelectors - utils.IncludeOneDriveRestoreDataSelectors(sel, opts) + sel := utils.IncludeOneDriveRestoreDataSelectors(opts) utils.FilterOneDriveRestoreInfoSelectors(sel, opts) // if no selector flags were specified, get all data in the service. diff --git a/src/cli/restore/sharepoint.go b/src/cli/restore/sharepoint.go index fadd36a48..498836258 100644 --- a/src/cli/restore/sharepoint.go +++ b/src/cli/restore/sharepoint.go @@ -154,8 +154,7 @@ func restoreSharePointCmd(cmd *cobra.Command, args []string) error { defer utils.CloseRepo(ctx, r) - sel := selectors.NewSharePointRestore(nil) // TODO: generate selector in IncludeSharePointRestoreDataSelectors - utils.IncludeSharePointRestoreDataSelectors(sel, opts) + sel := utils.IncludeSharePointRestoreDataSelectors(opts) utils.FilterSharePointRestoreInfoSelectors(sel, opts) // if no selector flags were specified, get all data in the service. diff --git a/src/cli/utils/exchange.go b/src/cli/utils/exchange.go index d10b98b61..4373dc2f8 100644 --- a/src/cli/utils/exchange.go +++ b/src/cli/utils/exchange.go @@ -128,30 +128,32 @@ func ValidateExchangeRestoreFlags(backupID string, opts ExchangeOpts) error { // IncludeExchangeRestoreDataSelectors builds the common data-selector // inclusions for exchange commands. -func IncludeExchangeRestoreDataSelectors( - sel *selectors.ExchangeRestore, - opts ExchangeOpts, -) { +func IncludeExchangeRestoreDataSelectors(opts ExchangeOpts) *selectors.ExchangeRestore { + users := opts.Users + if len(users) == 0 { + users = selectors.Any() + } + + sel := selectors.NewExchangeRestore(users) + lc, lcf := len(opts.Contact), len(opts.ContactFolder) le, lef := len(opts.Email), len(opts.EmailFolder) lev, lec := len(opts.Event), len(opts.EventCalendar) // either scope the request to a set of users if lc+lcf+le+lef+lev+lec == 0 { - if len(opts.Users) == 0 { - opts.Users = selectors.Any() - } + sel.Include(sel.Users(users)) - sel.Include(sel.Users(opts.Users)) - - return + return sel } opts.EmailFolder = trimFolderSlash(opts.EmailFolder) // or add selectors for each type of data - AddExchangeInclude(sel, opts.Users, opts.ContactFolder, opts.Contact, sel.Contacts) - AddExchangeInclude(sel, opts.Users, opts.EmailFolder, opts.Email, sel.Mails) - AddExchangeInclude(sel, opts.Users, opts.EventCalendar, opts.Event, sel.Events) + AddExchangeInclude(sel, users, opts.ContactFolder, opts.Contact, sel.Contacts) + AddExchangeInclude(sel, users, opts.EmailFolder, opts.Email, sel.Mails) + AddExchangeInclude(sel, users, opts.EventCalendar, opts.Event, sel.Events) + + return sel } // FilterExchangeRestoreInfoSelectors builds the common info-selector filters. diff --git a/src/cli/utils/exchange_test.go b/src/cli/utils/exchange_test.go index 58c22d17d..cd084f542 100644 --- a/src/cli/utils/exchange_test.go +++ b/src/cli/utils/exchange_test.go @@ -301,8 +301,7 @@ func (suite *ExchangeUtilsSuite) TestIncludeExchangeRestoreDataSelectors() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { - sel := selectors.NewExchangeRestore(nil) - utils.IncludeExchangeRestoreDataSelectors(sel, test.opts) + sel := utils.IncludeExchangeRestoreDataSelectors(test.opts) assert.Len(t, sel.Includes, test.expectIncludeLen) }) } diff --git a/src/cli/utils/onedrive.go b/src/cli/utils/onedrive.go index a668593f8..63564855d 100644 --- a/src/cli/utils/onedrive.go +++ b/src/cli/utils/onedrive.go @@ -70,27 +70,22 @@ func AddOneDriveFilter( // IncludeOneDriveRestoreDataSelectors builds the common data-selector // inclusions for OneDrive commands. -func IncludeOneDriveRestoreDataSelectors( - sel *selectors.OneDriveRestore, - opts OneDriveOpts, -) { +func IncludeOneDriveRestoreDataSelectors(opts OneDriveOpts) *selectors.OneDriveRestore { + users := opts.Users + if len(users) == 0 { + users = selectors.Any() + } + + sel := selectors.NewOneDriveRestore(users) + lp, ln := len(opts.Paths), len(opts.Names) // only use the inclusion if either a path or item name // is specified - if lp+ln == 0 { - return - } - - if len(opts.Users) == 0 { - opts.Users = selectors.Any() - } - - // either scope the request to a set of users if lp+ln == 0 { sel.Include(sel.Users(opts.Users)) - return + return sel } opts.Paths = trimFolderSlash(opts.Paths) @@ -102,12 +97,14 @@ func IncludeOneDriveRestoreDataSelectors( containsFolders, prefixFolders := splitFoldersIntoContainsAndPrefix(opts.Paths) if len(containsFolders) > 0 { - sel.Include(sel.Items(opts.Users, containsFolders, opts.Names)) + sel.Include(sel.Items(users, containsFolders, opts.Names)) } if len(prefixFolders) > 0 { - sel.Include(sel.Items(opts.Users, prefixFolders, opts.Names, selectors.PrefixMatch())) + sel.Include(sel.Items(users, prefixFolders, opts.Names, selectors.PrefixMatch())) } + + return sel } // FilterOneDriveRestoreInfoSelectors builds the common info-selector filters. diff --git a/src/cli/utils/onedrive_test.go b/src/cli/utils/onedrive_test.go index 15bc4dce1..ff7f92d62 100644 --- a/src/cli/utils/onedrive_test.go +++ b/src/cli/utils/onedrive_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/cli/utils" - "github.com/alcionai/corso/src/pkg/selectors" ) type OneDriveUtilsSuite struct { @@ -40,7 +39,7 @@ func (suite *OneDriveUtilsSuite) TestIncludeOneDriveRestoreDataSelectors() { Paths: empty, Names: empty, }, - expectIncludeLen: 0, + expectIncludeLen: 1, }, { name: "single inputs", @@ -90,9 +89,7 @@ func (suite *OneDriveUtilsSuite) TestIncludeOneDriveRestoreDataSelectors() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { - sel := selectors.NewOneDriveRestore(nil) - // no return, mutates sel as a side effect - utils.IncludeOneDriveRestoreDataSelectors(sel, test.opts) + sel := utils.IncludeOneDriveRestoreDataSelectors(test.opts) assert.Len(t, sel.Includes, test.expectIncludeLen) }) } diff --git a/src/cli/utils/sharepoint.go b/src/cli/utils/sharepoint.go index f5e3d602f..0ef21fd33 100644 --- a/src/cli/utils/sharepoint.go +++ b/src/cli/utils/sharepoint.go @@ -54,22 +54,22 @@ func AddSharePointFilter( // IncludeSharePointRestoreDataSelectors builds the common data-selector // inclusions for SharePoint commands. -func IncludeSharePointRestoreDataSelectors( - sel *selectors.SharePointRestore, - opts SharePointOpts, -) { +func IncludeSharePointRestoreDataSelectors(opts SharePointOpts) *selectors.SharePointRestore { + sites := opts.Sites + lp, li := len(opts.LibraryPaths), len(opts.LibraryItems) ls, lwu := len(opts.Sites), len(opts.WebURLs) slp, sli := len(opts.ListPaths), len(opts.ListItems) if ls == 0 { - opts.Sites = selectors.Any() + sites = selectors.Any() } - if lp+li+lwu+slp+sli == 0 { - sel.Include(sel.Sites(opts.Sites)) + sel := selectors.NewSharePointRestore(sites) - return + if lp+li+lwu+slp+sli == 0 { + sel.Include(sel.Sites(sites)) + return sel } if lp+li > 0 { @@ -81,11 +81,11 @@ func IncludeSharePointRestoreDataSelectors( containsFolders, prefixFolders := splitFoldersIntoContainsAndPrefix(opts.LibraryPaths) if len(containsFolders) > 0 { - sel.Include(sel.LibraryItems(opts.Sites, containsFolders, opts.LibraryItems)) + sel.Include(sel.LibraryItems(sites, containsFolders, opts.LibraryItems)) } if len(prefixFolders) > 0 { - sel.Include(sel.LibraryItems(opts.Sites, prefixFolders, opts.LibraryItems, selectors.PrefixMatch())) + sel.Include(sel.LibraryItems(sites, prefixFolders, opts.LibraryItems, selectors.PrefixMatch())) } } @@ -118,6 +118,8 @@ func IncludeSharePointRestoreDataSelectors( sel.Include(sel.WebURL(suffixURLs, selectors.SuffixMatch())) } } + + return sel } // FilterSharePointRestoreInfoSelectors builds the common info-selector filters. diff --git a/src/cli/utils/sharepoint_test.go b/src/cli/utils/sharepoint_test.go index 95ef19e89..de2048d93 100644 --- a/src/cli/utils/sharepoint_test.go +++ b/src/cli/utils/sharepoint_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/cli/utils" - "github.com/alcionai/corso/src/pkg/selectors" ) type SharePointUtilsSuite struct { @@ -164,11 +163,8 @@ func (suite *SharePointUtilsSuite) TestIncludeSharePointRestoreDataSelectors() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { - sel := selectors.NewSharePointRestore(nil) - // no return, mutates sel as a side effect - t.Logf("Options sent: %v\n", test.opts) - utils.IncludeSharePointRestoreDataSelectors(sel, test.opts) - assert.Len(t, sel.Includes, test.expectIncludeLen, sel) + sel := utils.IncludeSharePointRestoreDataSelectors(test.opts) + assert.Len(t, sel.Includes, test.expectIncludeLen) }) } } diff --git a/src/cli/utils/testdata/opts.go b/src/cli/utils/testdata/opts.go index 68e130555..68a33911a 100644 --- a/src/cli/utils/testdata/opts.go +++ b/src/cli/utils/testdata/opts.go @@ -268,6 +268,7 @@ var ( { Name: "BadFileCreatedAfter", Opts: utils.OneDriveOpts{ + Users: selectors.Any(), FileCreatedAfter: "foo", Populated: utils.PopulatedFlags{ utils.FileCreatedAfterFN: struct{}{}, diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index 688d78ad2..757506735 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -782,11 +782,13 @@ func makeExchangeBackupSel( for _, d := range dests { for c := range d.cats { + resourceOwners[d.resourceOwner] = struct{}{} + + // nil owners here, but we'll need to stitch this together + // below after the loops are complete. sel := selectors.NewExchangeBackup(nil) builder := sel.MailFolders - resourceOwners[d.resourceOwner] = struct{}{} - switch c { case path.ContactsCategory: builder = sel.ContactFolders @@ -814,12 +816,15 @@ func makeOneDriveBackupSel( dests []destAndCats, ) selectors.Selector { toInclude := [][]selectors.OneDriveScope{} - resourceOwners := []string{} + resourceOwners := map[string]struct{}{} for _, d := range dests { + resourceOwners[d.resourceOwner] = struct{}{} + + // nil owners here, we'll need to stitch this together + // below after the loops are complete. sel := selectors.NewOneDriveBackup(nil) - resourceOwners = append(resourceOwners, d.resourceOwner) toInclude = append(toInclude, sel.Folders( []string{d.resourceOwner}, []string{d.dest}, @@ -827,7 +832,7 @@ func makeOneDriveBackupSel( )) } - sel := selectors.NewOneDriveBackup(resourceOwners) + sel := selectors.NewOneDriveBackup(maps.Keys(resourceOwners)) sel.Include(toInclude...) return sel.Selector diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index e046051c5..ab0f5cd97 100644 --- a/src/internal/operations/backup_test.go +++ b/src/internal/operations/backup_test.go @@ -358,13 +358,16 @@ func (suite *BackupOpSuite) TestBackupOperation_PersistResults() { } for _, test := range table { suite.T().Run(test.expectStatus.String(), func(t *testing.T) { + sel := selectors.Selector{} + sel.DiscreteOwner = "bombadil" + op, err := NewBackupOperation( ctx, control.Options{}, kw, sw, acct, - selectors.Selector{DiscreteOwner: "test"}, + sel, evmock.NewBus()) require.NoError(t, err) test.expectErr(t, op.persistResults(now, &test.stats)) diff --git a/src/internal/operations/restore_test.go b/src/internal/operations/restore_test.go index f0b58d3cd..4c2a8ca34 100644 --- a/src/internal/operations/restore_test.go +++ b/src/internal/operations/restore_test.go @@ -178,6 +178,7 @@ func (suite *RestoreOpIntegrationSuite) SetupSuite() { users := []string{m365UserID} bsel := selectors.NewExchangeBackup(users) + bsel.DiscreteOwner = m365UserID bsel.Include( bsel.MailFolders(users, []string{exchange.DefaultMailFolder}, selectors.PrefixMatch()), bsel.ContactFolders(users, []string{exchange.DefaultContactFolder}, selectors.PrefixMatch()), diff --git a/src/pkg/selectors/example_selectors_test.go b/src/pkg/selectors/example_selectors_test.go index 5c70cd2db..9825de2b1 100644 --- a/src/pkg/selectors/example_selectors_test.go +++ b/src/pkg/selectors/example_selectors_test.go @@ -128,7 +128,7 @@ var ( DetailsModel: details.DetailsModel{ Entries: []details.DetailsEntry{ { - RepoRef: "tID/exchange/uID/email/example/itemID", + RepoRef: "tID/exchange/your-user-id/email/example/itemID", ShortRef: "xyz", ItemInfo: details.ItemInfo{ Exchange: &details.ExchangeInfo{ @@ -155,7 +155,7 @@ func Example_reduceDetails() { // We haven't added any scopes to our selector yet, so none of the data is retained. fmt.Println("Before adding scopes:", len(filteredDetails.Entries)) - ser.Include(ser.Mails([]string{"uID"}, []string{"example"}, []string{"xyz"})) + ser.Include(ser.Mails([]string{"your-user-id"}, []string{"example"}, []string{"xyz"})) ser.Filter(ser.MailSubject("the answer to life")) // Now that we've selected our data, we should find a result. diff --git a/src/pkg/selectors/exchange_test.go b/src/pkg/selectors/exchange_test.go index 0701dcdb2..780b4e36f 100644 --- a/src/pkg/selectors/exchange_test.go +++ b/src/pkg/selectors/exchange_test.go @@ -781,7 +781,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_MatchesPath() { var ( pth = stubPath(suite.T(), usr, []string{fld1, fld2, mail}, path.EmailCategory) short = "thisisahashofsomekind" - es = NewExchangeRestore(Any()) // TODO: move into test so that test user set is embedded in the selector + es = NewExchangeRestore(Any()) ) table := []struct { @@ -791,9 +791,9 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_MatchesPath() { expect assert.BoolAssertionFunc }{ {"all user's items", es.Users(Any()), "", assert.True}, - {"no user's items", es.Users(None()), "", assert.False}, + {"no user's items", es.Users(None()), "", assert.True}, {"matching user", es.Users([]string{usr}), "", assert.True}, - {"non-matching user", es.Users([]string{"smarf"}), "", assert.False}, + {"non-matching user", es.Users([]string{"smarf"}), "", assert.True}, {"one of multiple users", es.Users([]string{"smarf", usr}), "", assert.True}, {"all folders", es.MailFolders(Any(), Any()), "", assert.True}, {"no folders", es.MailFolders(Any(), None()), "", assert.False}, @@ -1194,14 +1194,14 @@ func (suite *ExchangeSelectorSuite) TestPasses() { }{ {"empty", nil, nil, nil, assert.False}, {"in Any", nil, nil, anyUser, assert.True}, - {"in None", nil, nil, noUser, assert.False}, + {"in None", nil, nil, noUser, assert.True}, {"in Mail", nil, nil, mail, assert.True}, {"in Other", nil, nil, otherMail, assert.False}, {"in no Mail", nil, nil, noMail, assert.False}, {"ex Any", anyUser, nil, anyUser, assert.False}, {"ex Any filter", anyUser, anyUser, nil, assert.False}, - {"ex None", noUser, nil, anyUser, assert.True}, - {"ex None filter mail", noUser, mail, nil, assert.True}, + {"ex None", noUser, nil, anyUser, assert.False}, + {"ex None filter mail", noUser, mail, nil, assert.False}, {"ex None filter any user", noUser, anyUser, nil, assert.False}, {"ex Mail", mail, nil, anyUser, assert.False}, {"ex Other", otherMail, nil, anyUser, assert.True}, diff --git a/src/pkg/selectors/helpers_test.go b/src/pkg/selectors/helpers_test.go index 0c3215c6b..32587a85e 100644 --- a/src/pkg/selectors/helpers_test.go +++ b/src/pkg/selectors/helpers_test.go @@ -148,13 +148,14 @@ type mockSel struct { Selector } -func stubSelector() mockSel { +func stubSelector(resourceOwners []string) mockSel { return mockSel{ Selector: Selector{ - Service: ServiceExchange, - Excludes: []scope{scope(stubScope(""))}, - Filters: []scope{scope(stubScope(""))}, - Includes: []scope{scope(stubScope(""))}, + ResourceOwners: filterize(scopeConfig{}, resourceOwners...), + Service: ServiceExchange, + Excludes: []scope{scope(stubScope(""))}, + Filters: []scope{scope(stubScope(""))}, + Includes: []scope{scope(stubScope(""))}, }, } } diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index 924db2ef8..5ec2f0829 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -295,6 +295,12 @@ func reduce[T scopeT, C categoryT]( return nil } + // if a DiscreteOwner is specified, only match details for that owner. + matchesResourceOwner := s.ResourceOwners + if len(s.DiscreteOwner) > 0 { + matchesResourceOwner = filterize(scopeConfig{}, s.DiscreteOwner) + } + // aggregate each scope type by category for easier isolation in future processing. excls := scopesByCategory[T](s.Excludes, dataCategories, false) filts := scopesByCategory[T](s.Filters, dataCategories, true) @@ -310,6 +316,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()) { + continue + } + dc, ok := dataCategories[repoPath.Category()] if !ok { continue @@ -439,6 +450,11 @@ func matchesPathValues[T scopeT, C categoryT]( shortRef string, ) bool { for _, c := range cat.pathKeys() { + // resourceOwners are now checked at the beginning of the reduction. + if c == c.rootCat() { + continue + } + // the pathValues must have an entry for the given categorizer pathVal, ok := pathValues[c] if !ok { diff --git a/src/pkg/selectors/scopes_test.go b/src/pkg/selectors/scopes_test.go index 6e21dcbcf..86139dcee 100644 --- a/src/pkg/selectors/scopes_test.go +++ b/src/pkg/selectors/scopes_test.go @@ -129,15 +129,16 @@ func (suite *SelectorScopesSuite) TestIsAnyTarget() { } var reduceTestTable = []struct { - name string - sel func() mockSel - expectLen int - expectPasses assert.BoolAssertionFunc + name string + sel func() mockSel + expectLen int + expectPassesReduce assert.BoolAssertionFunc + expectPasses assert.BoolAssertionFunc }{ { - name: "include all", + name: "include all resource owners", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) sel.Filters = nil sel.Excludes = nil return sel @@ -146,9 +147,32 @@ var reduceTestTable = []struct { expectPasses: assert.True, }, { - name: "include none", + name: "include all scopes", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) + sel.Filters = nil + sel.Excludes = nil + return sel + }, + expectLen: 1, + expectPasses: assert.True, + }, + { + name: "include none resource owners", + sel: func() mockSel { + sel := stubSelector(None()) + sel.Includes[0] = scope(stubScope(AnyTgt)) + sel.Filters = nil + sel.Excludes = nil + return sel + }, + expectLen: 0, + expectPasses: assert.True, // passes() does not check owners + }, + { + name: "include none scopes", + sel: func() mockSel { + sel := stubSelector(Any()) sel.Includes[0] = scope(stubScope("none")) sel.Filters = nil sel.Excludes = nil @@ -160,7 +184,7 @@ var reduceTestTable = []struct { { name: "filter and include all", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) sel.Excludes = nil return sel }, @@ -170,7 +194,7 @@ var reduceTestTable = []struct { { name: "include all filter none", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) sel.Filters[0] = scope(stubInfoScope("none")) sel.Excludes = nil return sel @@ -181,7 +205,7 @@ var reduceTestTable = []struct { { name: "include all exclude all", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) sel.Filters = nil return sel }, @@ -191,7 +215,7 @@ var reduceTestTable = []struct { { name: "include all exclude none", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) sel.Filters = nil sel.Excludes[0] = scope(stubScope("none")) return sel @@ -202,7 +226,7 @@ var reduceTestTable = []struct { { name: "filter all exclude all", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) sel.Includes = nil return sel }, @@ -212,7 +236,7 @@ var reduceTestTable = []struct { { name: "filter all exclude none", sel: func() mockSel { - sel := stubSelector() + sel := stubSelector(Any()) sel.Includes = nil sel.Excludes[0] = scope(stubScope("none")) return sel @@ -357,13 +381,6 @@ func (suite *SelectorScopesSuite) TestMatchesPathValues() { shortRef: short, expect: assert.True, }, - { - name: "root matches shortRef", - rootVal: short, - leafVal: leafCatStub.String(), - shortRef: short, - expect: assert.False, - }, } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { diff --git a/src/pkg/selectors/selectors_test.go b/src/pkg/selectors/selectors_test.go index 2570cc863..1515e9de9 100644 --- a/src/pkg/selectors/selectors_test.go +++ b/src/pkg/selectors/selectors_test.go @@ -35,7 +35,7 @@ func (suite *SelectorSuite) TestBadCastErr() { func (suite *SelectorSuite) TestPrintable() { t := suite.T() - sel := stubSelector() + sel := stubSelector(Any()) p := sel.Printable() assert.Equal(t, sel.Service.String(), p.Service) @@ -45,41 +45,58 @@ func (suite *SelectorSuite) TestPrintable() { } func (suite *SelectorSuite) TestPrintable_IncludedResources() { - t := suite.T() - - sel := stubSelector() - p := sel.Printable() - res := p.Resources() - - assert.Equal(t, "All", res, "stub starts out as an all-pass") - - stubWithResource := func(resource string) scope { - ss := stubScope("") - ss[rootCatStub.String()] = filterize(scopeConfig{}, resource) - - return scope(ss) + table := []struct { + name string + resourceOwners []string + expect func(string) bool + reason string + }{ + { + name: "distinct", + resourceOwners: []string{"foo", "smarf", "fnords"}, + expect: func(s string) bool { + return strings.HasSuffix(s, "(2 more)") + }, + reason: "should end with (2 more)", + }, + { + name: "distinct", + resourceOwners: nil, + expect: func(s string) bool { + return strings.HasSuffix(s, "None") + }, + reason: "no resource owners should produce None", + }, } + for _, test := range table { + suite.T().Run(test.name, func(t *testing.T) { + sel := stubSelector(test.resourceOwners) + p := sel.Printable() + res := p.Resources() - sel.Includes = []scope{ - stubWithResource("foo"), - stubWithResource("smarf"), - stubWithResource("fnords"), + assert.Equal(t, "All", res, "stub starts out as an all-pass") + + stubWithResource := func(resource string) scope { + ss := stubScope("") + ss[rootCatStub.String()] = filterize(scopeConfig{}, resource) + + return scope(ss) + } + + sel.Includes = []scope{} + sel.Filters = []scope{} + + for _, ro := range test.resourceOwners { + sel.Includes = append(sel.Includes, stubWithResource(ro)) + sel.Filters = append(sel.Filters, stubWithResource(ro)) + } + + p = sel.Printable() + res = p.Resources() + + assert.True(t, test.expect(res), test.reason) + }) } - - p = sel.Printable() - res = p.Resources() - - assert.True(t, strings.HasSuffix(res, "(2 more)"), "resource '"+res+"' should have (2 more) suffix") - - p.Includes = nil - res = p.Resources() - - assert.Equal(t, "All", res, "filters is also an all-pass") - - p.Filters = nil - res = p.Resources() - - assert.Equal(t, "None", res, "resource with no Includes or Filters should state None") } func (suite *SelectorSuite) TestToResourceTypeMap() {