Merge pull request #3424 from opencloud-eu/feat/search-scope

feat(search): scope searches to a drive via the driveId field

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
 

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"}},
 		},
 	}
 }

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

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.

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

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"),
+)

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)

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)
 	}
 

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",