add prefix option to scopes (#1141)

## Description

adds extensible options to folder-level scopes that allows the caller to specify whether they want a
prefix-comparison matcher or a contains-comparison matcher.

Also corrects the behavior of the prefix filter so that it accurately follows the "target is prefix of input" specification, rather than the reverse.

## Type of change

- [x] 🌻 Feature

## Issue(s)

* #1133

## Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

src/pkg/filters/filters.go

@@ -24,8 +24,7 @@ const (
 	Fails
 	// passthrough for the target
 	IdentityValue
-	// target is a prefix of the value it is compared
+	// "foo" is a prefix of "foo/bar/baz"
-	// against
 	TargetPrefixes
 )
 
@@ -143,6 +142,18 @@ func newFilter(c comparator, target string, negate bool) Filter {
 // Comparisons
 // ----------------------------------------------------------------------------------------------------
 
+// CompareAny checks whether any one of all the provided
+// inputs passes the filter.
+func (f Filter) CompareAny(inputs ...string) bool {
+	for _, in := range inputs {
+		if f.Compare(in) {
+			return true
+		}
+	}
+
+	return false
+}
+
 // Compare checks whether the input passes the filter.
 func (f Filter) Compare(input string) bool {
 	var cmp func(string, string) bool
@@ -201,7 +212,7 @@ func in(target, input string) bool {
 
 // true if target has input as a prefix.
 func prefixed(target, input string) bool {
-	return strings.HasPrefix(target, input)
+	return strings.HasPrefix(input, target)
 }
 
 // ----------------------------------------------------------------------------------------------------

src/pkg/filters/filters_test.go

@@ -37,6 +37,27 @@ func (suite *FiltersSuite) TestEquals() {
 	}
 }
 
+func (suite *FiltersSuite) TestEquals_any() {
+	f := filters.Equal("foo")
+	nf := filters.NotEqual("foo")
+
+	table := []struct {
+		name     string
+		input    []string
+		expectF  assert.BoolAssertionFunc
+		expectNF assert.BoolAssertionFunc
+	}{
+		{"includes target", []string{"foo", "bar"}, assert.True, assert.True},
+		{"not includes target", []string{"baz", "qux"}, assert.False, assert.True},
+	}
+	for _, test := range table {
+		suite.T().Run(test.name, func(t *testing.T) {
+			test.expectF(t, f.CompareAny(test.input...), "filter")
+			test.expectNF(t, nf.CompareAny(test.input...), "negated filter")
+		})
+	}
+}
+
 func (suite *FiltersSuite) TestGreater() {
 	f := filters.Greater("5")
 	nf := filters.NotGreater("5")
@@ -161,11 +182,13 @@ func (suite *FiltersSuite) TestIn_Joined() {
 }
 
 func (suite *FiltersSuite) TestPrefixes() {
-	input := "folderA"
+	target := "folderA"
+	f := filters.Prefix(target)
+	nf := filters.NotPrefix(target)
 
 	table := []struct {
 		name     string
-		target   string
+		input    string
 		expectF  assert.BoolAssertionFunc
 		expectNF assert.BoolAssertionFunc
 	}{
@@ -177,10 +200,8 @@ func (suite *FiltersSuite) TestPrefixes() {
 	}
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
-			f := filters.Prefix(test.target)
+			test.expectF(t, f.Compare(test.input), "filter")
-			nf := filters.NotPrefix(test.target)
+			test.expectNF(t, nf.Compare(test.input), "negated filter")
-			test.expectF(t, f.Compare(input), "filter")
-			test.expectNF(t, nf.Compare(input), "negated filter")
 		})
 	}
 }

src/pkg/selectors/exchange.go

@@ -189,12 +189,12 @@ func (s *exchange) Contacts(users, folders, contacts []string) []ExchangeScope {
 // If any slice contains selectors.Any, that slice is reduced to [selectors.Any]
 // If any slice contains selectors.None, that slice is reduced to [selectors.None]
 // If any slice is empty, it defaults to [selectors.None]
-func (s *exchange) ContactFolders(users, folders []string) []ExchangeScope {
+func (s *exchange) ContactFolders(users, folders []string, opts ...option) []ExchangeScope {
 	scopes := []ExchangeScope{}
 
 	scopes = append(
 		scopes,
-		makeScope[ExchangeScope](ExchangeContactFolder, users, folders),
+		makeScope[ExchangeScope](ExchangeContactFolder, users, folders, opts...),
 	)
 
 	return scopes
@@ -221,12 +221,12 @@ func (s *exchange) Events(users, calendars, events []string) []ExchangeScope {
 // If any slice contains selectors.Any, that slice is reduced to [selectors.Any]
 // If any slice contains selectors.None, that slice is reduced to [selectors.None]
 // If any slice is empty, it defaults to [selectors.None]
-func (s *exchange) EventCalendars(users, events []string) []ExchangeScope {
+func (s *exchange) EventCalendars(users, events []string, opts ...option) []ExchangeScope {
 	scopes := []ExchangeScope{}
 
 	scopes = append(
 		scopes,
-		makeScope[ExchangeScope](ExchangeEventCalendar, users, events),
+		makeScope[ExchangeScope](ExchangeEventCalendar, users, events, opts...),
 	)
 
 	return scopes
@@ -252,12 +252,12 @@ func (s *exchange) Mails(users, folders, mails []string) []ExchangeScope {
 // If any slice contains selectors.Any, that slice is reduced to [selectors.Any]
 // If any slice contains selectors.None, that slice is reduced to [selectors.None]
 // If any slice is empty, it defaults to [selectors.None]
-func (s *exchange) MailFolders(users, folders []string) []ExchangeScope {
+func (s *exchange) MailFolders(users, folders []string, opts ...option) []ExchangeScope {
 	scopes := []ExchangeScope{}
 
 	scopes = append(
 		scopes,
-		makeScope[ExchangeScope](ExchangeMailFolder, users, folders),
+		makeScope[ExchangeScope](ExchangeMailFolder, users, folders, opts...),
 	)
 
 	return scopes
@@ -429,44 +429,6 @@ func (sr *ExchangeRestore) MailSubject(subject string) []ExchangeScope {
 	}
 }
 
-// ---------------------------------------------------------------------------
-// Destination
-// ---------------------------------------------------------------------------
-
-type ExchangeDestination Destination
-
-func NewExchangeDestination() ExchangeDestination {
-	return ExchangeDestination{}
-}
-
-// GetOrDefault gets the destination of the provided category.  If no
-// destination is set, returns the current value.
-func (d ExchangeDestination) GetOrDefault(cat exchangeCategory, current string) string {
-	dest, ok := d[cat.String()]
-	if !ok {
-		return current
-	}
-
-	return dest.Target
-}
-
-// Sets the destination value of the provided category.  Returns an error
-// if a destination is already declared for that category.
-func (d ExchangeDestination) Set(cat exchangeCategory, dest string) error {
-	if len(dest) == 0 {
-		return nil
-	}
-
-	cs := cat.String()
-	if curr, ok := d[cs]; ok {
-		return existingDestinationErr(cs, curr.Target)
-	}
-
-	d[cs] = filterize(dest)
-
-	return nil
-}
-
 // ---------------------------------------------------------------------------
 // Categories
 // ---------------------------------------------------------------------------

src/pkg/selectors/exchange_test.go

@@ -479,55 +479,6 @@ func (suite *ExchangeSelectorSuite) TestExchangeSelector_Include_Users() {
 	}
 }
 
-func (suite *ExchangeSelectorSuite) TestNewExchangeDestination() {
-	t := suite.T()
-	dest := NewExchangeDestination()
-	assert.Len(t, dest, 0)
-}
-
-func (suite *ExchangeSelectorSuite) TestExchangeDestination_Set() {
-	dest := NewExchangeDestination()
-
-	table := []exchangeCategory{
-		ExchangeCategoryUnknown,
-		ExchangeContact,
-		ExchangeContactFolder,
-		ExchangeEvent,
-		ExchangeMail,
-		ExchangeMailFolder,
-		ExchangeUser,
-	}
-	for _, test := range table {
-		suite.T().Run(test.String(), func(t *testing.T) {
-			assert.NoError(t, dest.Set(test, "foo"))
-			assert.Error(t, dest.Set(test, "foo"))
-		})
-	}
-
-	assert.NoError(suite.T(), dest.Set(ExchangeUser, ""))
-}
-
-func (suite *ExchangeSelectorSuite) TestExchangeDestination_GetOrDefault() {
-	dest := NewExchangeDestination()
-
-	table := []exchangeCategory{
-		ExchangeCategoryUnknown,
-		ExchangeContact,
-		ExchangeContactFolder,
-		ExchangeEvent,
-		ExchangeMail,
-		ExchangeMailFolder,
-		ExchangeUser,
-	}
-	for _, test := range table {
-		suite.T().Run(test.String(), func(t *testing.T) {
-			assert.Equal(t, "bar", dest.GetOrDefault(test, "bar"))
-			assert.NoError(t, dest.Set(test, "foo"))
-			assert.Equal(t, "foo", dest.GetOrDefault(test, "bar"))
-		})
-	}
-}
-
 func (suite *ExchangeSelectorSuite) TestExchangeBackup_Scopes() {
 	eb := NewExchangeBackup()
 	eb.Include(eb.Users(Any()))

src/pkg/selectors/onedrive.go

@@ -177,12 +177,12 @@ func (s *oneDrive) Users(users []string) []OneDriveScope {
 // If any slice contains selectors.Any, that slice is reduced to [selectors.Any]
 // If any slice contains selectors.None, that slice is reduced to [selectors.None]
 // If any slice is empty, it defaults to [selectors.None]
-func (s *oneDrive) Folders(users, folders []string) []OneDriveScope {
+func (s *oneDrive) Folders(users, folders []string, opts ...option) []OneDriveScope {
 	scopes := []OneDriveScope{}
 
 	scopes = append(
 		scopes,
-		makeScope[OneDriveScope](OneDriveFolder, users, folders),
+		makeScope[OneDriveScope](OneDriveFolder, users, folders, opts...),
 	)
 
 	return scopes

src/pkg/selectors/scopes.go

@@ -119,12 +119,19 @@ type (
 func makeScope[T scopeT](
 	cat categorizer,
 	resources, vs []string,
+	opts ...option,
 ) T {
+	sc := scopeConfig{}
+
+	for _, opt := range opts {
+		opt(&sc)
+	}
+
 	s := T{
 		scopeKeyCategory:       filters.Identity(cat.String()),
 		scopeKeyDataType:       filters.Identity(cat.leafCat().String()),
-		cat.String():           filterize(vs...),
+		cat.String():           filterize(sc, vs...),
-		cat.rootCat().String(): filterize(resources...),
+		cat.rootCat().String(): filterize(scopeConfig{}, resources...),
 	}
 
 	return s
@@ -189,10 +196,20 @@ func getCatValue[T scopeT](s T, cat categorizer) []string {
 // set sets a value by category to the scope.  Only intended for internal
 // use, not for exporting to callers.
 func set[T scopeT](s T, cat categorizer, v []string) T {
-	s[cat.String()] = filterize(v...)
+	s[cat.String()] = filterize(scopeConfig{}, v...)
 	return s
 }
 
+// returns true if the category is included in the scope's category type,
+// and the value is set to None().
+func isNoneTarget[T scopeT, C categoryT](s T, cat C) bool {
+	if !typeAndCategoryMatches(cat, s.categorizer()) {
+		return false
+	}
+
+	return s[cat.String()].Target == NoneTgt
+}
+
 // returns true if the category is included in the scope's category type,
 // and the value is set to Any().
 func isAnyTarget[T scopeT, C categoryT](s T, cat C) bool {
@@ -361,67 +378,54 @@ func matchesPathValues[T scopeT, C categoryT](
 	shortRef string,
 ) bool {
 	for _, c := range cat.pathKeys() {
-		scopeVals := getCatValue(sc, c)
-
-		// the scope must define the targets to match on
-		if len(scopeVals) == 0 {
-			return false
-		}
-
-		// None() fails all matches
-		if scopeVals[0] == NoneTgt {
-			return false
-		}
-
 		// the pathValues must have an entry for the given categorizer
 		pathVal, ok := pathValues[c]
 		if !ok {
 			return false
 		}
 
-		// all parts of the scope must match
 		cc := c.(C)
-		if !isAnyTarget(sc, cc) {
-			var (
-				match = false
-				// Used to check if the path contains the value specified in scopeVals
-				pathHas = filters.Contains(pathVal)
-				// Used to check if the path has the value specified in scopeVal as a prefix
-				pathPrefix = filters.Prefix(pathVal)
-				// Used to check if the shortRef equals the value specified in scopeVals
-				shortRefEq = filters.Equal(shortRef)
-			)
 
-			for _, scopeVal := range scopeVals {
+		if isNoneTarget(sc, cc) {
-				switch {
+			return false
-				case c.isLeaf() && len(shortRef) > 0:
+		}
-					// Leaf category - we do a "contains" match for path or equality match on
+
-					// the shortRef
+		if isAnyTarget(sc, cc) {
-					if pathHas.Compare(scopeVal) || shortRefEq.Compare(scopeVal) {
+			// continue, not return: all path keys must match the entry to succeed
-						match = true
+			continue
-					}
+		}
-				case !c.isLeaf() && c != c.rootCat():
+
-					// Folder category - we check if the scope is a prefix
+		var (
-					// TODO: If the scopeVal is not a "path" - then we'll want to check
+			match  bool
-					// if any of the path elements match the scopeVal exactly
+			isLeaf = c.isLeaf()
-					if pathPrefix.Compare(scopeVal) {
+			isRoot = c == c.rootCat()
-						match = true
+		)
-					}
+
-				default:
+		switch {
-					if pathHas.Compare(scopeVal) {
+		// Leaf category - the scope can match either the path value (the item ID itself),
-						match = true
+		// or the shortRef hash representing the item.
-					}
+		case isLeaf && len(shortRef) > 0:
-				}
+			match = matches(sc, cc, pathVal) || matches(sc, cc, shortRef)
-				// short circuit if we found a match
+
-				if match {
+		// Folder category - checks if any target folder is a prefix of the path folders.
+		// Assumes (correctly) that we need to split the targets and re-compose them into
+		// individual prefix matchers.
+		// TODO: assumes all folders require prefix matchers.  Users can now specify whether
+		// the folder filter is a prefix match or not.  We should respect that configuration.
+		case !isLeaf && !isRoot:
+			for _, tgt := range getCatValue(sc, c) {
+				if filters.Prefix(tgt).Compare(pathVal) {
+					match = true
 					break
 				}
 			}
 
-			if !match {
+		default:
-				// Didn't match any scope
+			match = matches(sc, cc, pathVal)
-				return false
+		}
-			}
+
+		if !match {
+			return false
 		}
 	}
 

src/pkg/selectors/scopes_test.go

@@ -65,7 +65,7 @@ func (suite *SelectorScopesSuite) TestContains() {
 			name: "blank target",
 			scope: func() mockScope {
 				stub := stubScope("")
-				stub[rootCatStub.String()] = filterize("fnords")
+				stub[rootCatStub.String()] = filterize(scopeConfig{}, "fnords")
 				return stub
 			},
 			check:  "",
@@ -75,7 +75,7 @@ func (suite *SelectorScopesSuite) TestContains() {
 			name: "matching target",
 			scope: func() mockScope {
 				stub := stubScope("")
-				stub[rootCatStub.String()] = filterize(rootCatStub.String())
+				stub[rootCatStub.String()] = filterize(scopeConfig{}, rootCatStub.String())
 				return stub
 			},
 			check:  rootCatStub.String(),
@@ -85,7 +85,7 @@ func (suite *SelectorScopesSuite) TestContains() {
 			name: "non-matching target",
 			scope: func() mockScope {
 				stub := stubScope("")
-				stub[rootCatStub.String()] = filterize(rootCatStub.String())
+				stub[rootCatStub.String()] = filterize(scopeConfig{}, rootCatStub.String())
 				return stub
 			},
 			check:  "smarf",
@@ -105,7 +105,7 @@ func (suite *SelectorScopesSuite) TestGetCatValue() {
 	t := suite.T()
 
 	stub := stubScope("")
-	stub[rootCatStub.String()] = filterize(rootCatStub.String())
+	stub[rootCatStub.String()] = filterize(scopeConfig{}, rootCatStub.String())
 
 	assert.Equal(t,
 		[]string{rootCatStub.String()},
@@ -265,7 +265,7 @@ func (suite *SelectorScopesSuite) TestScopesByCategory() {
 	t := suite.T()
 	s1 := stubScope("")
 	s2 := stubScope("")
-	s2[scopeKeyCategory] = filterize(unknownCatStub.String())
+	s2[scopeKeyCategory] = filterize(scopeConfig{}, unknownCatStub.String())
 	result := scopesByCategory[mockScope](
 		[]scope{scope(s1), scope(s2)},
 		map[path.CategoryType]mockCategorizer{
@@ -368,8 +368,8 @@ func (suite *SelectorScopesSuite) TestMatchesPathValues() {
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
 			sc := stubScope("")
-			sc[rootCatStub.String()] = filterize(test.rootVal)
+			sc[rootCatStub.String()] = filterize(scopeConfig{}, test.rootVal)
-			sc[leafCatStub.String()] = filterize(test.leafVal)
+			sc[leafCatStub.String()] = filterize(scopeConfig{}, test.leafVal)
 
 			test.expect(t, matchesPathValues(sc, cat, pvs, test.shortRef))
 		})
@@ -486,3 +486,30 @@ func (suite *SelectorScopesSuite) TestWrapFilter() {
 		})
 	}
 }
+
+func (suite *SelectorScopesSuite) TestScopeConfig() {
+	input := "input"
+
+	table := []struct {
+		name   string
+		config scopeConfig
+		expect int
+	}{
+		{
+			name:   "no configs set",
+			config: scopeConfig{},
+			expect: int(filters.EqualTo),
+		},
+		{
+			name:   "force prefix",
+			config: scopeConfig{usePrefixFilter: true},
+			expect: int(filters.TargetPrefixes),
+		},
+	}
+	for _, test := range table {
+		suite.T().Run(test.name, func(t *testing.T) {
+			result := filterize(test.config, input)
+			assert.Equal(t, test.expect, int(result.Comparator))
+		})
+	}
+}

src/pkg/selectors/selectors.go

@@ -309,12 +309,23 @@ func addToSet(set []string, v []string) []string {
 }
 
 // ---------------------------------------------------------------------------
-// Destination
+// helpers
 // ---------------------------------------------------------------------------
 
-type Destination scope
+type scopeConfig struct {
+	usePrefixFilter bool
+}
 
-var ErrorDestinationAlreadySet = errors.New("destination is already declared")
+type option func(*scopeConfig)
+
+// PrefixMatch ensures the selector uses a Prefix comparator, instead
+// of contains or equals.  Will not override a default Any() or None()
+// comparator.
+func PrefixMatch() option {
+	return func(sc *scopeConfig) {
+		sc.usePrefixFilter = true
+	}
+}
 
 // ---------------------------------------------------------------------------
 // helpers
@@ -324,10 +335,6 @@ func badCastErr(cast, is service) error {
 	return errors.Wrapf(ErrorBadSelectorCast, "%s service is not %s", cast, is)
 }
 
-func existingDestinationErr(category, is string) error {
-	return errors.Wrapf(ErrorDestinationAlreadySet, "%s destination already set to %s", category, is)
-}
-
 func join(s ...string) string {
 	return strings.Join(s, delimiter)
 }
@@ -362,20 +369,25 @@ func clean(s []string) []string {
 // filterize turns the slice into a filter.
 // if the input is Any(), returns a passAny filter.
 // if the input is None(), returns a failAny filter.
+// if the scopeConfig specifies a filter, use that filter.
 // if the input is len(1), returns an Equals filter.
 // otherwise returns a Contains filter.
-func filterize(s ...string) filters.Filter {
+func filterize(sc scopeConfig, s ...string) filters.Filter {
 	s = clean(s)
 
+	if len(s) == 0 || s[0] == NoneTgt {
+		return failAny
+	}
+
+	if s[0] == AnyTgt {
+		return passAny
+	}
+
+	if sc.usePrefixFilter {
+		return filters.Prefix(join(s...))
+	}
+
 	if len(s) == 1 {
-		if s[0] == AnyTgt {
-			return passAny
-		}
-
-		if s[0] == NoneTgt {
-			return failAny
-		}
-
 		return filters.Equal(s[0])
 	}
 

src/pkg/selectors/selectors_reduce_test.go

@@ -192,6 +192,23 @@ func (suite *SelectorReduceSuite) TestReduce() {
 			},
 			expected: []details.DetailsEntry{testdata.ExchangeEmailItems[0]},
 		},
+		// TODO (keepers): all folders are treated as prefix-matches at this time.
+		// so this test actually does nothing different.  In the future, we'll
+		// need to amend the non-prefix folder tests to expect non-prefix matches.
+		{
+			name: "ExchangeMailByFolderPrefix",
+			selFunc: func() selectors.Reducer {
+				sel := selectors.NewExchangeRestore()
+				sel.Include(sel.MailFolders(
+					selectors.Any(),
+					[]string{testdata.ExchangeEmailBasePath.Folder()},
+					selectors.PrefixMatch(), // force prefix matching
+				))
+
+				return sel
+			},
+			expected: []details.DetailsEntry{testdata.ExchangeEmailItems[0]},
+		},
 		{
 			name: "ExchangeMailByFolderRoot",
 			selFunc: func() selectors.Reducer {

src/pkg/selectors/selectors_test.go

@@ -29,11 +29,6 @@ func (suite *SelectorSuite) TestBadCastErr() {
 	assert.Error(suite.T(), err)
 }
 
-func (suite *SelectorSuite) TestExistingDestinationErr() {
-	err := existingDestinationErr("foo", "bar")
-	assert.Error(suite.T(), err)
-}
-
 func (suite *SelectorSuite) TestPrintable() {
 	t := suite.T()
 
@@ -57,7 +52,7 @@ func (suite *SelectorSuite) TestPrintable_IncludedResources() {
 
 	stubWithResource := func(resource string) scope {
 		ss := stubScope("")
-		ss[rootCatStub.String()] = filterize(resource)
+		ss[rootCatStub.String()] = filterize(scopeConfig{}, resource)
 
 		return scope(ss)
 	}
@@ -102,8 +97,8 @@ func (suite *SelectorSuite) TestToResourceTypeMap() {
 			input: []scope{
 				scope(stubScope("")),
 				{
-					rootCatStub.String(): filterize("smarf"),
+					rootCatStub.String(): filterize(scopeConfig{}, "smarf"),
-					scopeKeyDataType:     filterize(unknownCatStub.String()),
+					scopeKeyDataType:     filterize(scopeConfig{}, unknownCatStub.String()),
 				},
 			},
 			expect: map[string][]string{
@@ -116,8 +111,8 @@ func (suite *SelectorSuite) TestToResourceTypeMap() {
 			input: []scope{
 				scope(stubScope("")),
 				{
-					rootCatStub.String(): filterize(AnyTgt),
+					rootCatStub.String(): filterize(scopeConfig{}, AnyTgt),
-					scopeKeyDataType:     filterize("other"),
+					scopeKeyDataType:     filterize(scopeConfig{}, "other"),
 				},
 			},
 			expect: map[string][]string{
@@ -138,9 +133,9 @@ func (suite *SelectorSuite) TestContains() {
 	key := rootCatStub
 	target := "fnords"
 	does := stubScope("")
-	does[key.String()] = filterize(target)
+	does[key.String()] = filterize(scopeConfig{}, target)
 	doesNot := stubScope("")
-	doesNot[key.String()] = filterize("smarf")
+	doesNot[key.String()] = filterize(scopeConfig{}, "smarf")
 
 	assert.True(t, matches(does, key, target), "does contain")
 	assert.False(t, matches(doesNot, key, target), "does not contain")