diff --git a/services/search/pkg/parity/README.md b/services/search/pkg/parity/README.md index e829530f25..8b3691c710 100644 --- a/services/search/pkg/parity/README.md +++ b/services/search/pkg/parity/README.md @@ -422,10 +422,10 @@ Fixtures: Fixtures: -- `parent`, folder -- `child.pdf`, Path = ./parent/child.pdf -- `outside.txt` -- `elsewhere.txt` +- `parent`, ID = 1$1!parent, folder +- `child.pdf`, ID = 1$1!child.pdf, Path = ./parent/child.pdf +- `outside.txt`, ID = 1$1!outside.txt +- `elsewhere.txt`, ID = 1$1!elsewhere.txt | Case | Query | expected | bleve | OpenSearch | same? | |---|---|---|---|---|---| @@ -434,6 +434,10 @@ Fixtures: | SCOPE-03 | `*` in 1$1!1 under ./parent | child.pdf, parent | child.pdf, parent | child.pdf, parent | ✅ | | SCOPE-04 | `*` in 1$1!1 under ./parent/child.pdf | child.pdf | child.pdf | child.pdf | ✅ | | SCOPE-05 | `name:"*elsewhere*"` in 1$1!1 | no match | no match | no match | ✅ | +| SCOPE-06 | `driveid:"2$2"` | elsewhere.txt | elsewhere.txt | elsewhere.txt | ✅ | +| SCOPE-07 | `driveid:"1$1"` | child.pdf, outside.txt, parent | child.pdf, outside.txt, parent | child.pdf, outside.txt, parent | ✅ | +| SCOPE-08 | `NOT driveid:"2$2"` | child.pdf, outside.txt, parent | child.pdf, outside.txt, parent | child.pdf, outside.txt, parent | ✅ | +| SCOPE-09 | `name:"*elsewhere*" AND driveid:"2$2"` | elsewhere.txt | elsewhere.txt | elsewhere.txt | ✅ | ### invalid diff --git a/services/search/pkg/parity/query_scope_test.go b/services/search/pkg/parity/query_scope_test.go index 9bb01341f2..209de381de 100644 --- a/services/search/pkg/parity/query_scope_test.go +++ b/services/search/pkg/parity/query_scope_test.go @@ -19,7 +19,7 @@ func scopeGroup() queryGroup { fixtureFolder("parent"), fixtureDoc("child.pdf", withPath("./parent/child.pdf")), fixtureDoc("outside.txt"), - fixtureDoc("elsewhere.txt", withRoot("2$2!1")), + fixtureDoc("elsewhere.txt", withRoot("2$2!2")), }, cases: []queryCase{ {id: 1, query: `*`, want: []string{"parent", "child.pdf", "outside.txt", "elsewhere.txt"}}, @@ -27,6 +27,10 @@ func scopeGroup() queryGroup { {id: 3, query: `*`, ref: space("./parent"), want: []string{"parent", "child.pdf"}}, {id: 4, query: `*`, ref: space("./parent/child.pdf"), want: []string{"child.pdf"}}, {id: 5, query: `name:"*elsewhere*"`, ref: space(""), want: nil}, + {id: 6, query: `driveid:"2$2"`, want: []string{"elsewhere.txt"}}, + {id: 7, query: `driveid:"1$1"`, want: []string{"parent", "child.pdf", "outside.txt"}}, + {id: 8, query: `NOT driveid:"2$2"`, want: []string{"parent", "child.pdf", "outside.txt"}}, + {id: 9, query: `name:"*elsewhere*" AND driveid:"2$2"`, want: []string{"elsewhere.txt"}}, }, } } diff --git a/services/search/pkg/query/normalize.go b/services/search/pkg/query/normalize.go index c2aef1e820..bce1fd1be6 100644 --- a/services/search/pkg/query/normalize.go +++ b/services/search/pkg/query/normalize.go @@ -2,13 +2,14 @@ package query import ( "strconv" + "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "reflect" - "strings" "github.com/opencloud-eu/opencloud/pkg/ast" "github.com/opencloud-eu/opencloud/services/search/pkg/query/mimetype" + "github.com/opencloud-eu/opencloud/services/search/pkg/search" ) // Normalize is the shared KQL lowering pass between parse and compile: it @@ -41,6 +42,9 @@ func normalizeNodes(nodes []ast.Node, resolve func(string) string, defaultKey st if node.Key == "Type" { node.Value = resourceType(node.Value) } + if node.Key == "RootID" { + node.Value = search.CompleteRootID(node.Value) + } if exp := mimetype.Expand(node.Key, node.Value); exp != nil { out = append(out, normalizeNodes(exp, resolve, defaultKey)...) continue diff --git a/services/search/pkg/query/normalize_test.go b/services/search/pkg/query/normalize_test.go index 519e979a83..94eecfb300 100644 --- a/services/search/pkg/query/normalize_test.go +++ b/services/search/pkg/query/normalize_test.go @@ -82,6 +82,23 @@ var _ = Describe("Normalize", func() { })) }) + It("resolves driveId to RootID and completes the root id", func() { + got := norm( + &ast.StringNode{Key: "driveId", Value: "1$2"}, + &ast.OperatorNode{Value: "AND"}, + &ast.StringNode{Key: "driveid", Value: "1$2!4"}, + &ast.OperatorNode{Value: "AND"}, + &ast.StringNode{Key: "RootID", Value: "1$2"}, + ) + Expect(got).To(Equal([]ast.Node{ + &ast.StringNode{Key: "RootID", Value: "1$2!2"}, + &ast.OperatorNode{Value: "AND"}, + &ast.StringNode{Key: "RootID", Value: "1$2!4"}, + &ast.OperatorNode{Value: "AND"}, + &ast.StringNode{Key: "RootID", Value: "1$2!2"}, + })) + }) + // A bare restriction inside a named group inherits the group key; a keyed // child keeps its own key; a bare restriction in an unnamed group falls // back to Name. diff --git a/services/search/pkg/query/resolver.go b/services/search/pkg/query/resolver.go index 6f46d65952..5628dd886f 100644 --- a/services/search/pkg/query/resolver.go +++ b/services/search/pkg/query/resolver.go @@ -13,6 +13,7 @@ import ( var aliases = map[string]string{ "tag": "Tags", "favorite": "Favorites", + "driveid": "RootID", } // fieldIndex maps a lowercased KQL key to its canonical field name ("" is the diff --git a/services/search/pkg/search/pinnedroot_test.go b/services/search/pkg/search/pinnedroot_test.go new file mode 100644 index 0000000000..7346da06f9 --- /dev/null +++ b/services/search/pkg/search/pinnedroot_test.go @@ -0,0 +1,41 @@ +package search_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/opencloud-eu/opencloud/services/search/pkg/search" +) + +var _ = DescribeTable("PinnedRootID", + func(query, want string) { + Expect(search.PinnedRootID(query)).To(Equal(want)) + }, + Entry("bare driveId is completed to the root id", `driveId:"1$2"`, "1$2!2"), + Entry("full root id passes through", `driveId:"1$2!4" name:x`, "1$2!4"), + Entry("RootID works as well", `RootID:"1$2" AND name:x`, "1$2!2"), + Entry("top-level AND conjuncts pin", `driveId:"1$2" AND name:x AND tag:y`, "1$2!2"), + Entry("repeated identical AND restrictions pin", `driveId:"1$2" AND driveId:"1$2"`, "1$2!2"), + // KQL groups adjacent same-key restrictions into an implicit OR group + Entry("implicitly repeated restrictions do not pin", `driveId:"1$2" driveId:"1$2"`, ""), + Entry("no restriction, no pin", `name:x`, ""), + Entry("a top-level OR disables pruning", `name:x OR driveId:"1$2"`, ""), + Entry("OR between other terms disables pruning", `driveId:"1$2" AND (a OR b)`, "1$2!2"), + Entry("a negated restriction disables pruning", `NOT driveId:"1$2" AND name:x`, ""), + Entry("restrictions inside groups do not pin", `(driveId:"1$2") AND name:x`, ""), + Entry("two different roots disable pruning", `driveId:"1$2" AND driveId:"1$3"`, ""), + Entry("invalid query, no pin", `((`, ""), + Entry("a wildcard drive id does not pin", `driveId:"1$*" AND name:x`, ""), + Entry("a wildcard space id does not pin", `driveId:"1$2*"`, ""), +) + +var _ = DescribeTable("CompleteRootID", + func(in, want string) { + Expect(search.CompleteRootID(in)).To(Equal(want)) + }, + Entry("bare drive id gains the space id", "1$2", "1$2!2"), + Entry("complete id passes through", "1$2!4", "1$2!4"), + Entry("wildcards pass through", "1$*", "1$*"), + Entry("question marks pass through", "1$2?", "1$2?"), + Entry("no separator passes through", "abc", "abc"), +) diff --git a/services/search/pkg/search/search.go b/services/search/pkg/search/search.go index fd7cb6c0d6..7ab61ea88d 100644 --- a/services/search/pkg/search/search.go +++ b/services/search/pkg/search/search.go @@ -11,6 +11,8 @@ import ( gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/opencloud-eu/opencloud/pkg/ast" + "github.com/opencloud-eu/opencloud/pkg/kql" "github.com/opencloud-eu/reva/v2/pkg/conversions" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/grants" @@ -223,6 +225,69 @@ func convertToWebDAVPermissions(isShared, isMountpoint, isDir bool, p *provider. return b.String() } +// CompleteRootID completes a bare driveId ("storage$space") to the root +// resource id stored in the index. Assumes a space root's opaque id equals +// its space id (true for decomposedfs); elsewhere a bare driveId matches +// nothing, the pinning stays safe either way. +func CompleteRootID(v string) string { + // leave wildcards alone, "1$*!*" would match nothing + if strings.Contains(v, "!") || strings.ContainsAny(v, "*?") { + return v + } + if i := strings.LastIndex(v, "$"); i >= 0 && i+1 < len(v) { + return v + "!" + v[i+1:] + } + return v +} + +// PinnedRootID returns the single space root the query is pinned to via +// top-level AND driveId/RootID conjuncts, or "". OR, NOT and group-nested +// restrictions never pin: skipping a space would be wrong. +func PinnedRootID(query string) string { + a, err := kql.Builder{}.Build(query) + if err != nil { + return "" + } + pinned := "" + negated := false + for _, n := range a.Nodes { + switch node := n.(type) { + case *ast.OperatorNode: + if strings.EqualFold(node.Value, "OR") { + return "" + } + negated = strings.EqualFold(node.Value, "NOT") + continue + case ast.OperatorNode: + if strings.EqualFold(node.Value, "OR") { + return "" + } + negated = strings.EqualFold(node.Value, "NOT") + continue + } + var key, value string + switch node := n.(type) { + case *ast.StringNode: + key, value = node.Key, node.Value + case ast.StringNode: + key, value = node.Key, node.Value + } + if strings.EqualFold(key, "driveid") || strings.EqualFold(key, "rootid") { + // a negated or wildcard restriction cannot pin a single space + if negated || strings.ContainsAny(value, "*?") { + return "" + } + v := CompleteRootID(value) + if pinned != "" && pinned != v { + return "" + } + pinned = v + } + negated = false + } + return pinned +} + // ParseScope extract a scope value from the query string and returns search, scope strings func ParseScope(query string) (string, string) { match := scopeRegex.FindStringSubmatch(query) diff --git a/services/search/pkg/search/service.go b/services/search/pkg/search/service.go index 0c6611be10..7c8ae567dc 100644 --- a/services/search/pkg/search/service.go +++ b/services/search/pkg/search/service.go @@ -167,6 +167,17 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se Path: gpRes.Path, } } + // drive-pinned queries only need that space's index; the restriction + // stays in the query, mountpoints are kept for path mapping + var pinnedRoot *provider.ResourceId + if req.Ref == nil { + if pinned := PinnedRootID(req.Query); pinned != "" { + if rid, err := storagespace.ParseID(pinned); err == nil { + pinnedRoot = &rid + } + } + } + filters := []*provider.ListStorageSpacesRequest_Filter{ { Type: provider.ListStorageSpacesRequest_Filter_TYPE_USER, @@ -195,6 +206,9 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se // We still need the mountpoint in order to map the result paths to the according share continue } + if pinnedRoot != nil && space.SpaceType != _spaceTypeMountpoint && pinnedRoot.GetSpaceId() != space.Root.GetSpaceId() { + continue + } spaces = append(spaces, space) } diff --git a/services/search/pkg/search/service_test.go b/services/search/pkg/search/service_test.go index 3a75bd0c6e..a5bce09181 100644 --- a/services/search/pkg/search/service_test.go +++ b/services/search/pkg/search/service_test.go @@ -498,6 +498,27 @@ var _ = Describe("Searchprovider", func() { }, nil) }) + It("prunes the fan-out when the query pins a drive", func() { + res, err := s.Search(ctx, &searchsvc.SearchRequest{ + Query: `driveId:"storageid$personalspace" AND foo`, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(len(res.Matches)).To(Equal(1)) + Expect(res.Matches[0].Entity.Id.OpaqueId).To(Equal("foo-id")) + indexClient.AssertNumberOfCalls(GinkgoT(), "Search", 1) + }) + + It("keeps the full fan-out for an OR query", func() { + res, err := s.Search(ctx, &searchsvc.SearchRequest{ + Query: `driveId:"storageid$personalspace" OR foo`, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(len(res.Matches)).To(Equal(3)) + indexClient.AssertNumberOfCalls(GinkgoT(), "Search", 2) + }) + It("considers the search Ref parameter", func() { res, err := s.Search(ctx, &searchsvc.SearchRequest{ Query: "foo",