From 7a8c6eaf78811bc43998ed005e0fc5d787cedebb Mon Sep 17 00:00:00 2001 From: Keepers Date: Wed, 31 Aug 2022 10:43:54 -0600 Subject: [PATCH] clean up/simplify filter packagee (#695) --- src/pkg/filters/filters.go | 185 ++++++++++++++--------------- src/pkg/filters/filters_test.go | 86 +++++--------- src/pkg/selectors/exchange.go | 8 +- src/pkg/selectors/exchange_test.go | 16 +-- src/pkg/selectors/helpers_test.go | 10 +- src/pkg/selectors/scopes.go | 23 ++-- src/pkg/selectors/scopes_test.go | 14 +-- src/pkg/selectors/selectors.go | 14 +-- 8 files changed, 159 insertions(+), 197 deletions(-) diff --git a/src/pkg/filters/filters.go b/src/pkg/filters/filters.go index 64ca33c9d..c95582ad1 100644 --- a/src/pkg/filters/filters.go +++ b/src/pkg/filters/filters.go @@ -9,35 +9,23 @@ type comparator int const ( UnknownComparator comparator = iota // a == b - Equal + EqualTo // a > b - Greater + GreaterThan // a < b - Less - // a < b < c - Between + LessThan // "foo,bar,baz" contains "foo" - Contains + TargetContains // "foo" is found in "foo,bar,baz" - In + TargetIn // always passes - Pass + Passes // always fails - Fail + Fails // passthrough for the target - Identity + IdentityValue ) -const delimiter = "," - -func join(s ...string) string { - return strings.Join(s, delimiter) -} - -func split(s string) []string { - return strings.Split(s, delimiter) -} - func norm(s string) string { return strings.ToLower(s) } @@ -47,91 +35,117 @@ func norm(s string) string { // true if Filter.Comparer(filter.target, v) is true. type Filter struct { Comparator comparator `json:"comparator"` - Category any `json:"category"` // a caller-provided identifier. Probably an iota or string const. - Target string `json:"target"` // the value to compare against - Negate bool `json:"negate"` // when true, negate the comparator result + Target string `json:"target"` // the value to compare against + Negate bool `json:"negate"` // when true, negate the comparator result } // ---------------------------------------------------------------------------------------------------- // Constructors // ---------------------------------------------------------------------------------------------------- -// NewEquals creates a filter which Matches(v) is true if +// Equal creates a filter where Compare(v) is true if // target == v -func NewEquals(negate bool, category any, target string) Filter { - return Filter{Equal, category, target, negate} +func Equal(target string) Filter { + return newFilter(EqualTo, target, false) } -// NewGreater creates a filter which Matches(v) is true if +// NotEqual creates a filter where Compare(v) is true if +// target != v +func NotEqual(target string) Filter { + return newFilter(EqualTo, target, true) +} + +// Greater creates a filter where Compare(v) is true if // target > v -func NewGreater(negate bool, category any, target string) Filter { - return Filter{Greater, category, target, negate} +func Greater(target string) Filter { + return newFilter(GreaterThan, target, false) } -// NewLess creates a filter which Matches(v) is true if +// NotGreater creates a filter where Compare(v) is true if +// target <= v +func NotGreater(target string) Filter { + return newFilter(GreaterThan, target, true) +} + +// Less creates a filter where Compare(v) is true if // target < v -func NewLess(negate bool, category any, target string) Filter { - return Filter{Less, category, target, negate} +func Less(target string) Filter { + return newFilter(LessThan, target, false) } -// NewBetween creates a filter which Matches(v) is true if -// lesser < v && v < greater -func NewBetween(negate bool, category any, lesser, greater string) Filter { - return Filter{Between, category, join(lesser, greater), negate} +// NotLess creates a filter where Compare(v) is true if +// target >= v +func NotLess(target string) Filter { + return newFilter(LessThan, target, true) } -// NewContains creates a filter which Matches(v) is true if -// super.Contains(v) -func NewContains(negate bool, category any, super string) Filter { - return Filter{Contains, category, super, negate} +// Contains creates a filter where Compare(v) is true if +// target.Contains(v) +func Contains(target string) Filter { + return newFilter(TargetContains, target, false) } -// NewIn creates a filter which Matches(v) is true if -// v.Contains(substr) -func NewIn(negate bool, category any, substr string) Filter { - return Filter{In, category, substr, negate} +// NotContains creates a filter where Compare(v) is true if +// !target.Contains(v) +func NotContains(target string) Filter { + return newFilter(TargetContains, target, true) } -// NewPass creates a filter where Matches(v) always returns true -func NewPass() Filter { - return Filter{Pass, nil, "*", false} +// In creates a filter where Compare(v) is true if +// v.Contains(target) +func In(target string) Filter { + return newFilter(TargetIn, target, false) } -// NewFail creates a filter where Matches(v) always returns false -func NewFail() Filter { - return Filter{Fail, nil, "", false} +// NotIn creates a filter where Compare(v) is true if +// !v.Contains(target) +func NotIn(target string) Filter { + return newFilter(TargetIn, target, true) } -// NewIdentity creates a filter intended to hold values, rather than -// compare them. Functionally, it'll behave the same as Equals. -func NewIdentity(id string) Filter { - return Filter{Identity, nil, id, false} +// Pass creates a filter where Compare(v) always returns true +func Pass() Filter { + return newFilter(Passes, "*", false) +} + +// Fail creates a filter where Compare(v) always returns false +func Fail() Filter { + return newFilter(Fails, "", false) +} + +// Identity creates a filter intended to hold values, rather than +// compare them. Comparatively, it'll behave the same as Equals. +func Identity(id string) Filter { + return newFilter(IdentityValue, id, false) +} + +// newFilter is the standard filter constructor. +func newFilter(c comparator, target string, negate bool) Filter { + return Filter{c, target, negate} } // ---------------------------------------------------------------------------------------------------- // Comparisons // ---------------------------------------------------------------------------------------------------- -// Checks whether the filter matches the input -func (f Filter) Matches(input string) bool { +// Compare checks whether the input passes the filter. +func (f Filter) Compare(input string) bool { var cmp func(string, string) bool switch f.Comparator { - case Equal, Identity: + case EqualTo, IdentityValue: cmp = equals - case Greater: + case GreaterThan: cmp = greater - case Less: + case LessThan: cmp = less - case Between: - cmp = between - case Contains: + case TargetContains: cmp = contains - case In: + case TargetIn: cmp = in - case Pass: + case Passes: return true - case Fail: + case Fails: return false } @@ -158,15 +172,6 @@ func less(target, input string) bool { return target < input } -// assumes target is a delimited string. -// true if both: -// - less(target[0], input) -// - greater(target[1], input) -func between(target, input string) bool { - parts := split(target) - return less(parts[0], input) && greater(parts[1], input) -} - // true if target contains input as a substring. func contains(target, input string) bool { return strings.Contains(target, input) @@ -181,34 +186,22 @@ func in(target, input string) bool { // Helpers // ---------------------------------------------------------------------------------------------------- -// Targets returns the Target value split into a slice. -func (f Filter) Targets() []string { - return split(f.Target) +// prefixString maps the comparators to string prefixes for printing. +var prefixString = map[comparator]string{ + EqualTo: "eq:", + GreaterThan: "gt:", + LessThan: "lt:", + TargetContains: "cont:", + TargetIn: "in:", } func (f Filter) String() string { - var prefix string - switch f.Comparator { - case Equal: - prefix = "eq:" - case Greater: - prefix = "gt:" - case Less: - prefix = "lt:" - case Between: - prefix = "btwn:" - case Contains: - prefix = "cont:" - case In: - prefix = "in:" - case Pass: + case Passes: return "pass" - case Fail: + case Fails: return "fail" - case Identity: - default: // no prefix } - return prefix + f.Target + return prefixString[f.Comparator] + f.Target } diff --git a/src/pkg/filters/filters_test.go b/src/pkg/filters/filters_test.go index 1499bb59c..f25f9232f 100644 --- a/src/pkg/filters/filters_test.go +++ b/src/pkg/filters/filters_test.go @@ -18,9 +18,8 @@ func TestFiltersSuite(t *testing.T) { } func (suite *FiltersSuite) TestEquals() { - makeFilt := filters.NewEquals - f := makeFilt(false, "", "foo") - nf := makeFilt(true, "", "foo") + f := filters.Equal("foo") + nf := filters.NotEqual("foo") table := []struct { input string @@ -32,16 +31,15 @@ func (suite *FiltersSuite) TestEquals() { } for _, test := range table { suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") + test.expectF(t, f.Compare(test.input), "filter") + test.expectNF(t, nf.Compare(test.input), "negated filter") }) } } func (suite *FiltersSuite) TestGreater() { - makeFilt := filters.NewGreater - f := makeFilt(false, "", "5") - nf := makeFilt(true, "", "5") + f := filters.Greater("5") + nf := filters.NotGreater("5") table := []struct { input string @@ -54,16 +52,15 @@ func (suite *FiltersSuite) TestGreater() { } for _, test := range table { suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") + test.expectF(t, f.Compare(test.input), "filter") + test.expectNF(t, nf.Compare(test.input), "negated filter") }) } } func (suite *FiltersSuite) TestLess() { - makeFilt := filters.NewLess - f := makeFilt(false, "", "5") - nf := makeFilt(true, "", "5") + f := filters.Less("5") + nf := filters.NotLess("5") table := []struct { input string @@ -76,39 +73,15 @@ func (suite *FiltersSuite) TestLess() { } for _, test := range table { suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") - }) - } -} - -func (suite *FiltersSuite) TestBetween() { - makeFilt := filters.NewBetween - f := makeFilt(false, "", "abc", "def") - nf := makeFilt(true, "", "abc", "def") - - table := []struct { - input string - expectF assert.BoolAssertionFunc - expectNF assert.BoolAssertionFunc - }{ - {"cd", assert.True, assert.False}, - {"a", assert.False, assert.True}, - {"1", assert.False, assert.True}, - {"f", assert.False, assert.True}, - } - for _, test := range table { - suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") + test.expectF(t, f.Compare(test.input), "filter") + test.expectNF(t, nf.Compare(test.input), "negated filter") }) } } func (suite *FiltersSuite) TestContains() { - makeFilt := filters.NewContains - f := makeFilt(false, "", "smurfs") - nf := makeFilt(true, "", "smurfs") + f := filters.Contains("smurfs") + nf := filters.NotContains("smurfs") table := []struct { input string @@ -120,16 +93,15 @@ func (suite *FiltersSuite) TestContains() { } for _, test := range table { suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") + test.expectF(t, f.Compare(test.input), "filter") + test.expectNF(t, nf.Compare(test.input), "negated filter") }) } } func (suite *FiltersSuite) TestContains_Joined() { - makeFilt := filters.NewContains - f := makeFilt(false, "", "smarf,userid") - nf := makeFilt(true, "", "smarf,userid") + f := filters.Contains("smarf,userid") + nf := filters.NotContains("smarf,userid") table := []struct { input string @@ -142,16 +114,15 @@ func (suite *FiltersSuite) TestContains_Joined() { } for _, test := range table { suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") + test.expectF(t, f.Compare(test.input), "filter") + test.expectNF(t, nf.Compare(test.input), "negated filter") }) } } func (suite *FiltersSuite) TestIn() { - makeFilt := filters.NewIn - f := makeFilt(false, "", "murf") - nf := makeFilt(true, "", "murf") + f := filters.In("murf") + nf := filters.NotIn("murf") table := []struct { input string @@ -163,16 +134,15 @@ func (suite *FiltersSuite) TestIn() { } for _, test := range table { suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") + test.expectF(t, f.Compare(test.input), "filter") + test.expectNF(t, nf.Compare(test.input), "negated filter") }) } } func (suite *FiltersSuite) TestIn_Joined() { - makeFilt := filters.NewIn - f := makeFilt(false, "", "userid") - nf := makeFilt(true, "", "userid") + f := filters.In("userid") + nf := filters.NotIn("userid") table := []struct { input string @@ -184,8 +154,8 @@ func (suite *FiltersSuite) TestIn_Joined() { } for _, test := range table { suite.T().Run(test.input, func(t *testing.T) { - test.expectF(t, f.Matches(test.input), "filter") - test.expectNF(t, nf.Matches(test.input), "negated filter") + test.expectF(t, f.Compare(test.input), "filter") + test.expectNF(t, nf.Compare(test.input), "negated filter") }) } } diff --git a/src/pkg/selectors/exchange.go b/src/pkg/selectors/exchange.go index 9b6d19592..a6a52af78 100644 --- a/src/pkg/selectors/exchange.go +++ b/src/pkg/selectors/exchange.go @@ -270,7 +270,7 @@ func (sr *ExchangeRestore) MailReceivedAfter(timeStrings string) []ExchangeScope ExchangeMail, ExchangeFilterMailReceivedAfter, []string{timeStrings}, - wrapFilter(filters.NewLess)), + wrapFilter(filters.Less)), } } @@ -284,7 +284,7 @@ func (sr *ExchangeRestore) MailReceivedBefore(timeStrings string) []ExchangeScop ExchangeMail, ExchangeFilterMailReceivedBefore, []string{timeStrings}, - wrapFilter(filters.NewGreater)), + wrapFilter(filters.Greater)), } } @@ -299,7 +299,7 @@ func (sr *ExchangeRestore) MailSender(senderIDs []string) []ExchangeScope { ExchangeMail, ExchangeFilterMailSender, senderIDs, - wrapFilter(filters.NewIn)), + wrapFilter(filters.In)), } } @@ -314,7 +314,7 @@ func (sr *ExchangeRestore) MailSubject(subjectSubstrings []string) []ExchangeSco ExchangeMail, ExchangeFilterMailSubject, subjectSubstrings, - wrapFilter(filters.NewIn)), + wrapFilter(filters.In)), } } diff --git a/src/pkg/selectors/exchange_test.go b/src/pkg/selectors/exchange_test.go index 6fc03be2b..af3f19e07 100644 --- a/src/pkg/selectors/exchange_test.go +++ b/src/pkg/selectors/exchange_test.go @@ -337,7 +337,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeSelector_Exclude_Users() { map[categorizer]string{ExchangeUser: join(u1, u2)}, ) - if sc[scopeKeyCategory].Matches(ExchangeContactFolder.String()) { + if sc[scopeKeyCategory].Compare(ExchangeContactFolder.String()) { scopeMustHave( t, ExchangeScope(sc), @@ -348,7 +348,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeSelector_Exclude_Users() { ) } - if sc[scopeKeyCategory].Matches(ExchangeEvent.String()) { + if sc[scopeKeyCategory].Compare(ExchangeEvent.String()) { scopeMustHave( t, ExchangeScope(sc), @@ -358,7 +358,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeSelector_Exclude_Users() { ) } - if sc[scopeKeyCategory].Matches(ExchangeMailFolder.String()) { + if sc[scopeKeyCategory].Compare(ExchangeMailFolder.String()) { scopeMustHave( t, ExchangeScope(sc), @@ -391,7 +391,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeSelector_Include_Users() { map[categorizer]string{ExchangeUser: join(u1, u2)}, ) - if sc[scopeKeyCategory].Matches(ExchangeContactFolder.String()) { + if sc[scopeKeyCategory].Compare(ExchangeContactFolder.String()) { scopeMustHave( t, ExchangeScope(sc), @@ -402,7 +402,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeSelector_Include_Users() { ) } - if sc[scopeKeyCategory].Matches(ExchangeEvent.String()) { + if sc[scopeKeyCategory].Compare(ExchangeEvent.String()) { scopeMustHave( t, ExchangeScope(sc), @@ -412,7 +412,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeSelector_Include_Users() { ) } - if sc[scopeKeyCategory].Matches(ExchangeMailFolder.String()) { + if sc[scopeKeyCategory].Compare(ExchangeMailFolder.String()) { scopeMustHave( t, ExchangeScope(sc), @@ -566,7 +566,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_Category() { suite.T().Run(test.is.String()+test.expect.String(), func(t *testing.T) { eb := NewExchangeBackup() eb.Includes = []scope{ - {scopeKeyCategory: filters.NewIdentity(test.is.String())}, + {scopeKeyCategory: filters.Identity(test.is.String())}, } scope := eb.Scopes()[0] test.check(t, test.expect, scope.Category()) @@ -600,7 +600,7 @@ func (suite *ExchangeSelectorSuite) TestExchangeScope_IncludesCategory() { suite.T().Run(test.is.String()+test.expect.String(), func(t *testing.T) { eb := NewExchangeBackup() eb.Includes = []scope{ - {scopeKeyCategory: filters.NewIdentity(test.is.String())}, + {scopeKeyCategory: filters.Identity(test.is.String())}, } scope := eb.Scopes()[0] test.check(t, scope.IncludesCategory(test.expect)) diff --git a/src/pkg/selectors/helpers_test.go b/src/pkg/selectors/helpers_test.go index 05fac065f..a4883ce06 100644 --- a/src/pkg/selectors/helpers_test.go +++ b/src/pkg/selectors/helpers_test.go @@ -99,11 +99,11 @@ func stubScope(match string) mockScope { return mockScope{ rootCatStub.String(): passAny, - scopeKeyCategory: filters.NewIdentity(rootCatStub.String()), - scopeKeyGranularity: filters.NewIdentity(Item), - scopeKeyResource: filters.NewIdentity(stubResource), - scopeKeyDataType: filters.NewIdentity(rootCatStub.String()), - shouldMatch: filters.NewIdentity(sm), + scopeKeyCategory: filters.Identity(rootCatStub.String()), + scopeKeyGranularity: filters.Identity(Item), + scopeKeyResource: filters.Identity(stubResource), + scopeKeyDataType: filters.Identity(rootCatStub.String()), + shouldMatch: filters.Identity(sm), } } diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index c69f36a96..3d4eef46a 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -121,10 +121,10 @@ func makeScope[T scopeT]( resources, vs []string, ) T { s := T{ - scopeKeyCategory: filters.NewIdentity(cat.String()), - scopeKeyDataType: filters.NewIdentity(cat.leafCat().String()), - scopeKeyGranularity: filters.NewIdentity(granularity), - scopeKeyResource: filters.NewIdentity(join(resources...)), + scopeKeyCategory: filters.Identity(cat.String()), + scopeKeyDataType: filters.Identity(cat.leafCat().String()), + scopeKeyGranularity: filters.Identity(granularity), + scopeKeyResource: filters.Identity(join(resources...)), cat.String(): filterize(vs...), cat.rootCat().String(): filterize(resources...), } @@ -140,11 +140,11 @@ func makeFilterScope[T scopeT]( f func([]string) filters.Filter, ) T { return T{ - scopeKeyCategory: filters.NewIdentity(cat.String()), - scopeKeyDataType: filters.NewIdentity(cat.leafCat().String()), - scopeKeyGranularity: filters.NewIdentity(Filter), - scopeKeyInfoFilter: filters.NewIdentity(filterCat.String()), - scopeKeyResource: filters.NewIdentity(Filter), + scopeKeyCategory: filters.Identity(cat.String()), + scopeKeyDataType: filters.Identity(cat.leafCat().String()), + scopeKeyGranularity: filters.Identity(Filter), + scopeKeyInfoFilter: filters.Identity(filterCat.String()), + scopeKeyResource: filters.Identity(Filter), filterCat.String(): f(clean(vs)), } } @@ -164,7 +164,7 @@ func matches[T scopeT, C categoryT](s T, cat C, target string) bool { return false } - return s[cat.String()].Matches(target) + return s[cat.String()].Compare(target) } // getCategory returns the scope's category value. @@ -409,8 +409,7 @@ func matchesPathValues[T scopeT, C categoryT]( // all parts of the scope must match cc := c.(C) if !isAnyTarget(sc, cc) { - f := filters.NewContains(false, cc, join(scopeVals...)) - if !f.Matches(pathVal) { + if filters.NotContains(join(scopeVals...)).Compare(pathVal) { return false } } diff --git a/src/pkg/selectors/scopes_test.go b/src/pkg/selectors/scopes_test.go index 39bad9bc2..3828f9391 100644 --- a/src/pkg/selectors/scopes_test.go +++ b/src/pkg/selectors/scopes_test.go @@ -53,7 +53,7 @@ func (suite *SelectorScopesSuite) TestContains() { name: "blank value", scope: func() mockScope { stub := stubScope("") - stub[rootCatStub.String()] = filters.NewEquals(false, nil, "") + stub[rootCatStub.String()] = filters.Equal("") return stub }, check: rootCatStub.String(), @@ -427,23 +427,23 @@ func (suite *SelectorScopesSuite) TestWrapFilter() { }{ { name: "any", - filter: filters.NewContains, + filter: filters.Contains, input: Any(), - comparator: int(filters.Pass), + comparator: int(filters.Passes), target: AnyTgt, }, { name: "none", - filter: filters.NewIn, + filter: filters.In, input: None(), - comparator: int(filters.Fail), + comparator: int(filters.Fails), target: NoneTgt, }, { name: "something", - filter: filters.NewEquals, + filter: filters.Equal, input: []string{"userid"}, - comparator: int(filters.Equal), + comparator: int(filters.EqualTo), target: "userid", }, } diff --git a/src/pkg/selectors/selectors.go b/src/pkg/selectors/selectors.go index d8660d211..c02ae213a 100644 --- a/src/pkg/selectors/selectors.go +++ b/src/pkg/selectors/selectors.go @@ -56,8 +56,8 @@ const ( ) var ( - passAny = filters.NewPass() - failAny = filters.NewFail() + passAny = filters.Pass() + failAny = filters.Fail() ) // All is the resource name that gets output when the resource is AnyTgt. @@ -258,7 +258,7 @@ func toResourceTypeMap(ms []scope) map[string][]string { k = All } - r[k] = addToSet(r[k], m[scopeKeyDataType].Targets()) + r[k] = addToSet(r[k], split(m[scopeKeyDataType].Target)) } return r @@ -357,13 +357,13 @@ func filterize(s ...string) filters.Filter { return failAny } - return filters.NewEquals(false, "", s[0]) + return filters.Equal(s[0]) } - return filters.NewContains(false, "", join(s...)) + return filters.Contains(join(s...)) } -type filterFunc func(bool, any, string) filters.Filter +type filterFunc func(string) filters.Filter // wrapFilter produces a func that filterizes the input by: // - cleans the input string @@ -386,6 +386,6 @@ func wrapFilter(ff filterFunc) func([]string) filters.Filter { ss := join(s...) - return ff(false, nil, ss) + return ff(ss) } }