Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions services/search/pkg/parity/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -378,10 +378,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? |
|---|---|---|---|---|---|
Expand All @@ -390,6 +390,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

Expand Down
6 changes: 5 additions & 1 deletion services/search/pkg/parity/query_scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@ 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"}},
{id: 2, query: `*`, ref: space(""), want: []string{"parent", "child.pdf", "outside.txt"}},
{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"}},
},
}
}
6 changes: 5 additions & 1 deletion services/search/pkg/query/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions services/search/pkg/query/normalize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions services/search/pkg/query/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions services/search/pkg/search/pinnedroot_test.go
Original file line number Diff line number Diff line change
@@ -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"),
)
65 changes: 65 additions & 0 deletions services/search/pkg/search/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions services/search/pkg/search/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down
21 changes: 21 additions & 0 deletions services/search/pkg/search/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down