From ca2fbda2601ffa72c95d11ecb0c7f623ded2e315 Mon Sep 17 00:00:00 2001 From: ryanfkeepers Date: Wed, 29 Mar 2023 16:33:32 -0600 Subject: [PATCH] match sites with a url path suffix Adds suffix matching to the site owner id/name lookup process. This allows consumers to query for a site by only its path (ex: "/sites/foo") and still end up with a well formed id, name tuple. One gotcha is that this requires the lookup maps to be populated. If a lookup map is not passed in with a suffix matcher, the fallback lookup will fail. --- src/internal/connector/graph_connector.go | 33 ++++++++---- .../connector/graph_connector_test.go | 13 ++++- src/pkg/filters/filters.go | 54 +++++++++++++++++-- src/pkg/filters/filters_test.go | 45 +++++++++++++--- src/pkg/selectors/scopes.go | 4 +- 5 files changed, 123 insertions(+), 26 deletions(-) diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 482661917..40c1a7559 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -10,6 +10,7 @@ import ( "github.com/alcionai/clues" "github.com/pkg/errors" + "golang.org/x/exp/maps" "github.com/alcionai/corso/src/internal/connector/discovery/api" "github.com/alcionai/corso/src/internal/connector/graph" @@ -17,6 +18,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/filters" ) // --------------------------------------------------------------------------- @@ -267,12 +269,17 @@ var ErrResourceOwnerNotFound = clues.New("resource owner not found in tenant") // getOwnerIDAndNameFrom looks up the owner's canonical id and display name. // if idToName and nameToID are populated, and the owner is a key of one of -// those maps, then those values are returned. As a fallback, the resource -// calls the discovery api to fetch the user or site using the owner value. -// This fallback assumes that the owner is a well formed ID or display name -// of appropriate design (PrincipalName for users, WebURL for sites). -// If the fallback lookup is used, the maps are populated to contain the -// id and name references. +// those maps, then those values are returned. +// +// As a fallback, the resource calls the discovery api to fetch the user or +// site using the owner value. This fallback assumes that the owner is a well +// formed ID or display name of appropriate design (PrincipalName for users, +// WebURL for sites). If the fallback lookup is used, the maps are populated +// to contain the id and name references. +// +// Consumers are allowed to pass in a path suffix (eg: /sites/foo) as a site +// owner, but only if they also pass in a nameToID map. A nil map will cascade +// to the fallback, which will fail for having a malformed id value. func (r resourceClient) getOwnerIDAndNameFrom( ctx context.Context, discovery api.Client, @@ -281,8 +288,8 @@ func (r resourceClient) getOwnerIDAndNameFrom( ) (string, string, error) { if n, ok := idToName[owner]; ok { return owner, n, nil - } else if i, ok := nameToID[owner]; ok { - return i, owner, nil + } else if id, ok := nameToID[owner]; ok { + return id, owner, nil } ctx = clues.Add(ctx, "owner_identifier", owner) @@ -292,9 +299,13 @@ func (r resourceClient) getOwnerIDAndNameFrom( err error ) - // if r.enum == Sites { - // TODO: check all suffixes in nameToID - // } + // check if the provided owner is a suffix of a weburl in the lookup map + if r.enum == Sites { + url, _, ok := filters.PathSuffix([]string{owner}).CompareAny(maps.Keys(nameToID)...) + if ok { + return nameToID[url], url, nil + } + } id, name, err = r.getter.GetIDAndName(ctx, owner) if err != nil { diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index be298d370..ea55b5b64 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -64,7 +64,8 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { enum: Users, getter: &mockNameIDGetter{id: ownerID, name: ownerName}, } - noLookup = &resourceClient{enum: Users, getter: &mockNameIDGetter{}} + noLookup = &resourceClient{enum: Users, getter: &mockNameIDGetter{}} + siteLookup = &resourceClient{enum: Sites, getter: &mockNameIDGetter{}} ) table := []struct { @@ -167,6 +168,16 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { expectName: "", expectErr: assert.Error, }, + { + name: "site suffix lookup", + owner: "/url/path", + rc: siteLookup, + idToName: nil, + nameToID: map[string]string{"http://some/site/url/path": ownerID}, + expectID: ownerID, + expectName: "http://some/site/url/path", + expectErr: assert.NoError, + }, } for _, test := range table { suite.Run(test.name, func() { diff --git a/src/pkg/filters/filters.go b/src/pkg/filters/filters.go index 8a2504f27..1e9d7a7cd 100644 --- a/src/pkg/filters/filters.go +++ b/src/pkg/filters/filters.go @@ -359,21 +359,23 @@ func newSliceFilter(c comparator, targets, normTargets []string, negate bool) Fi // ---------------------------------------------------------------------------------------------------- // CompareAny checks whether any one of all the provided -// inputs passes the filter. +// inputs passes the filter. If one passes, that value is +// returned, as well as its index in the input range. +// If nothing matches, returns ("", -1, false) // // Note that, as a gotcha, CompareAny can resolve truthily // for both the standard and negated versions of a filter. // Ex: consider the input CompareAny(true, false), which // will return true for both Equals(true) and NotEquals(true), // because at least one element matches for both filters. -func (f Filter) CompareAny(inputs ...string) bool { - for _, in := range inputs { +func (f Filter) CompareAny(inputs ...string) (string, int, bool) { + for i, in := range inputs { if f.Compare(in) { - return true + return in, i, true } } - return false + return "", -1, false } // Compare checks whether the input passes the filter. @@ -442,6 +444,48 @@ func (f Filter) Compare(input string) bool { return res } +// Matches extends Compare by not only checking if +// the input passes the filter, but if it passes, the +// target which matched and its index are returned as well. +// If more than one value matches the input, only the +// first is returned. +// returns ("", -1, false) if no match is found. +// TODO: only partially implemented. +// func (f Filter) Matches(input string) (string, int, bool) { +// var ( +// cmp func(string, string) bool +// res bool +// targets = f.NormalizedTargets +// ) + +// switch f.Comparator { +// case TargetPathPrefix: +// cmp = pathPrefix +// case TargetPathContains: +// cmp = pathContains +// case TargetPathSuffix: +// cmp = pathSuffix +// case TargetPathEquals: +// cmp = pathEquals +// default: +// return "", -1, false +// } + +// for i, tgt := range targets { +// res = cmp(norm(tgt), norm(input)) + +// if !f.Negate && res { +// return f.Targets[i], i, true +// } + +// if f.Negate && !res { +// return f.Targets[i], i, true +// } +// } + +// return "", -1, false +// } + // true if t == i func equals(target, input string) bool { return target == input diff --git a/src/pkg/filters/filters_test.go b/src/pkg/filters/filters_test.go index 7ee32e3e0..fdb691016 100644 --- a/src/pkg/filters/filters_test.go +++ b/src/pkg/filters/filters_test.go @@ -45,20 +45,49 @@ func (suite *FiltersSuite) TestEquals_any() { nf := filters.NotEqual("foo") table := []struct { - name string - input []string - expectF assert.BoolAssertionFunc - expectNF assert.BoolAssertionFunc + name string + input []string + expectF assert.BoolAssertionFunc + expectFVal string + expectFIdx int + expectNF assert.BoolAssertionFunc + expectNFVal string + expectNFIdx int }{ - {"includes target", []string{"foo", "bar"}, assert.True, assert.True}, - {"not includes target", []string{"baz", "qux"}, assert.False, assert.True}, + { + name: "includes target", + input: []string{"foo", "bar"}, + expectF: assert.True, + expectFVal: "foo", + expectFIdx: 0, + expectNF: assert.True, + expectNFVal: "bar", + expectNFIdx: 1, + }, + { + name: "not includes target", + input: []string{"baz", "qux"}, + expectF: assert.False, + expectFVal: "", + expectFIdx: -1, + expectNF: assert.True, + expectNFVal: "baz", + expectNFIdx: 0, + }, } for _, test := range table { suite.Run(test.name, func() { t := suite.T() - test.expectF(t, f.CompareAny(test.input...), "filter") - test.expectNF(t, nf.CompareAny(test.input...), "negated filter") + v, i, b := f.CompareAny(test.input...) + test.expectF(t, b, "filter") + assert.Equal(t, test.expectFIdx, i, "index") + assert.Equal(t, test.expectFVal, v, "value") + + v, i, b = nf.CompareAny(test.input...) + test.expectNF(t, b, "neg-filter") + assert.Equal(t, test.expectNFIdx, i, "neg-index") + assert.Equal(t, test.expectNFVal, v, "neg-value") }) } } diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index dca906705..3446c4463 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -223,7 +223,9 @@ func matchesAny[T scopeT, C categoryT](s T, cat C, inpts []string) bool { return false } - return s[cat.String()].CompareAny(inpts...) + _, _, pass := s[cat.String()].CompareAny(inpts...) + + return pass } // getCategory returns the scope's category value.