From e83dac7d247ebc8b5fcf1296033d28e3c64d8a4c Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Tue, 6 Oct 2026 10:45:57 +0200 Subject: [PATCH] feat(search): offset paging with a stable order The search service takes a `from` offset next to the page size and applies it after the cross-space merge; every space answers the full prefix up to from+size, since an offset cannot be distributed. Pages are stable: both engines and the merge break score ties by id, and OpenSearch counts every match (track_total_hits). No engine pages beyond the first 10000 matches, the OpenSearch result window, so a from and size reaching beyond it are rejected on both engines. `page_size` becomes optional on the wire: absent is the default of 200, 0 asks for no matches, -1 for all; WebDAV and the tags listing set it accordingly, and the WebDAV report's ``, parsed since the report exists and never read, now pages the merged list. The gRPC wrapper passes the request through and keys its cache by the whole request. --- .../opencloud/services/search/v0/search.pb.go | 78 ++++++++------ .../services/search/v0/search.swagger.json | 5 + .../opencloud/services/search/v0/search.proto | 8 +- services/graph/pkg/service/v0/tags.go | 5 +- services/search/pkg/bleve/backend.go | 17 ++- services/search/pkg/opensearch/backend.go | 20 ++-- services/search/pkg/parity/parity_test.go | 6 +- services/search/pkg/search/search.go | 27 ++++- services/search/pkg/search/service.go | 54 ++++++++-- services/search/pkg/search/service_test.go | 101 +++++++++++++++++- .../search/pkg/service/grpc/v0/service.go | 20 ++-- services/webdav/pkg/service/v0/search.go | 24 ++++- services/webdav/pkg/service/v0/search_test.go | 31 ++++++ 13 files changed, 321 insertions(+), 75 deletions(-) diff --git a/protogen/gen/opencloud/services/search/v0/search.pb.go b/protogen/gen/opencloud/services/search/v0/search.pb.go index 82489194fa..656aa320d2 100644 --- a/protogen/gen/opencloud/services/search/v0/search.pb.go +++ b/protogen/gen/opencloud/services/search/v0/search.pb.go @@ -87,12 +87,15 @@ type SearchRequest struct { unknownFields protoimpl.UnknownFields // Optional. The maximum number of entries to return in the response - PageSize int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + PageSize *int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3,oneof" json:"page_size,omitempty"` // Optional. A pagination token returned from a previous call to `Get` // that indicates from where search should continue PageToken string `protobuf:"bytes,2,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` Query string `protobuf:"bytes,3,opt,name=query,proto3" json:"query,omitempty"` Ref *v0.Reference `protobuf:"bytes,4,opt,name=ref,proto3" json:"ref,omitempty"` + // Optional. Number of leading matches to skip in the globally sorted, + // cross-space merged result list. + From int32 `protobuf:"varint,7,opt,name=from,proto3" json:"from,omitempty"` } func (x *SearchRequest) Reset() { @@ -128,8 +131,8 @@ func (*SearchRequest) Descriptor() ([]byte, []int) { } func (x *SearchRequest) GetPageSize() int32 { - if x != nil { - return x.PageSize + if x != nil && x.PageSize != nil { + return *x.PageSize } return 0 } @@ -155,6 +158,13 @@ func (x *SearchRequest) GetRef() *v0.Reference { return nil } +func (x *SearchRequest) GetFrom() int32 { + if x != nil { + return x.From + } + return 0 +} + type SearchResponse struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -226,7 +236,7 @@ type SearchIndexRequest struct { unknownFields protoimpl.UnknownFields // Optional. The maximum number of entries to return in the response - PageSize int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + PageSize *int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3,oneof" json:"page_size,omitempty"` // Optional. A pagination token returned from a previous call to `Get` // that indicates from where search should continue PageToken string `protobuf:"bytes,2,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` @@ -267,8 +277,8 @@ func (*SearchIndexRequest) Descriptor() ([]byte, []int) { } func (x *SearchIndexRequest) GetPageSize() int32 { - if x != nil { - return x.PageSize + if x != nil && x.PageSize != nil { + return *x.PageSize } return 0 } @@ -544,31 +554,10 @@ var file_opencloud_services_search_v0_search_proto_rawDesc = []byte{ 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f, 0x6d, 0x61, 0x73, 0x6b, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x1e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x64, 0x75, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2e, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xae, 0x01, 0x0a, 0x0d, 0x53, 0x65, 0x61, 0x72, 0x63, 0x68, - 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x21, 0x0a, 0x09, 0x70, 0x61, 0x67, 0x65, 0x5f, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xdb, 0x01, 0x0a, 0x0d, 0x53, 0x65, 0x61, 0x72, 0x63, 0x68, + 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x26, 0x0a, 0x09, 0x70, 0x61, 0x67, 0x65, 0x5f, 0x73, 0x69, 0x7a, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x42, 0x04, 0xe2, 0x41, 0x01, 0x01, - 0x52, 0x08, 0x70, 0x61, 0x67, 0x65, 0x53, 0x69, 0x7a, 0x65, 0x12, 0x23, 0x0a, 0x0a, 0x70, 0x61, - 0x67, 0x65, 0x5f, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x42, 0x04, - 0xe2, 0x41, 0x01, 0x01, 0x52, 0x09, 0x70, 0x61, 0x67, 0x65, 0x54, 0x6f, 0x6b, 0x65, 0x6e, 0x12, - 0x14, 0x0a, 0x05, 0x71, 0x75, 0x65, 0x72, 0x79, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, - 0x71, 0x75, 0x65, 0x72, 0x79, 0x12, 0x3f, 0x0a, 0x03, 0x72, 0x65, 0x66, 0x18, 0x04, 0x20, 0x01, - 0x28, 0x0b, 0x32, 0x27, 0x2e, 0x6f, 0x70, 0x65, 0x6e, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x2e, 0x6d, - 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x73, 0x2e, 0x73, 0x65, 0x61, 0x72, 0x63, 0x68, 0x2e, 0x76, - 0x30, 0x2e, 0x52, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, 0x63, 0x65, 0x42, 0x04, 0xe2, 0x41, 0x01, - 0x01, 0x52, 0x03, 0x72, 0x65, 0x66, 0x22, 0x9c, 0x01, 0x0a, 0x0e, 0x53, 0x65, 0x61, 0x72, 0x63, - 0x68, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x3d, 0x0a, 0x07, 0x6d, 0x61, 0x74, - 0x63, 0x68, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x23, 0x2e, 0x6f, 0x70, 0x65, - 0x6e, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x2e, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x73, 0x2e, - 0x73, 0x65, 0x61, 0x72, 0x63, 0x68, 0x2e, 0x76, 0x30, 0x2e, 0x4d, 0x61, 0x74, 0x63, 0x68, 0x52, - 0x07, 0x6d, 0x61, 0x74, 0x63, 0x68, 0x65, 0x73, 0x12, 0x26, 0x0a, 0x0f, 0x6e, 0x65, 0x78, 0x74, - 0x5f, 0x70, 0x61, 0x67, 0x65, 0x5f, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, - 0x09, 0x52, 0x0d, 0x6e, 0x65, 0x78, 0x74, 0x50, 0x61, 0x67, 0x65, 0x54, 0x6f, 0x6b, 0x65, 0x6e, - 0x12, 0x23, 0x0a, 0x0d, 0x74, 0x6f, 0x74, 0x61, 0x6c, 0x5f, 0x6d, 0x61, 0x74, 0x63, 0x68, 0x65, - 0x73, 0x18, 0x03, 0x20, 0x01, 0x28, 0x05, 0x52, 0x0c, 0x74, 0x6f, 0x74, 0x61, 0x6c, 0x4d, 0x61, - 0x74, 0x63, 0x68, 0x65, 0x73, 0x22, 0xb3, 0x01, 0x0a, 0x12, 0x53, 0x65, 0x61, 0x72, 0x63, 0x68, - 0x49, 0x6e, 0x64, 0x65, 0x78, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x21, 0x0a, 0x09, - 0x70, 0x61, 0x67, 0x65, 0x5f, 0x73, 0x69, 0x7a, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x42, - 0x04, 0xe2, 0x41, 0x01, 0x01, 0x52, 0x08, 0x70, 0x61, 0x67, 0x65, 0x53, 0x69, 0x7a, 0x65, 0x12, + 0x48, 0x00, 0x52, 0x08, 0x70, 0x61, 0x67, 0x65, 0x53, 0x69, 0x7a, 0x65, 0x88, 0x01, 0x01, 0x12, 0x23, 0x0a, 0x0a, 0x70, 0x61, 0x67, 0x65, 0x5f, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x42, 0x04, 0xe2, 0x41, 0x01, 0x01, 0x52, 0x09, 0x70, 0x61, 0x67, 0x65, 0x54, 0x6f, 0x6b, 0x65, 0x6e, 0x12, 0x14, 0x0a, 0x05, 0x71, 0x75, 0x65, 0x72, 0x79, 0x18, 0x03, 0x20, @@ -576,7 +565,32 @@ var file_opencloud_services_search_v0_search_proto_rawDesc = []byte{ 0x66, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x27, 0x2e, 0x6f, 0x70, 0x65, 0x6e, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x2e, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x73, 0x2e, 0x73, 0x65, 0x61, 0x72, 0x63, 0x68, 0x2e, 0x76, 0x30, 0x2e, 0x52, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, 0x63, 0x65, - 0x42, 0x04, 0xe2, 0x41, 0x01, 0x01, 0x52, 0x03, 0x72, 0x65, 0x66, 0x22, 0xa1, 0x01, 0x0a, 0x13, + 0x42, 0x04, 0xe2, 0x41, 0x01, 0x01, 0x52, 0x03, 0x72, 0x65, 0x66, 0x12, 0x18, 0x0a, 0x04, 0x66, + 0x72, 0x6f, 0x6d, 0x18, 0x07, 0x20, 0x01, 0x28, 0x05, 0x42, 0x04, 0xe2, 0x41, 0x01, 0x01, 0x52, + 0x04, 0x66, 0x72, 0x6f, 0x6d, 0x42, 0x0c, 0x0a, 0x0a, 0x5f, 0x70, 0x61, 0x67, 0x65, 0x5f, 0x73, + 0x69, 0x7a, 0x65, 0x22, 0x9c, 0x01, 0x0a, 0x0e, 0x53, 0x65, 0x61, 0x72, 0x63, 0x68, 0x52, 0x65, + 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x3d, 0x0a, 0x07, 0x6d, 0x61, 0x74, 0x63, 0x68, 0x65, + 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x23, 0x2e, 0x6f, 0x70, 0x65, 0x6e, 0x63, 0x6c, + 0x6f, 0x75, 0x64, 0x2e, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x73, 0x2e, 0x73, 0x65, 0x61, + 0x72, 0x63, 0x68, 0x2e, 0x76, 0x30, 0x2e, 0x4d, 0x61, 0x74, 0x63, 0x68, 0x52, 0x07, 0x6d, 0x61, + 0x74, 0x63, 0x68, 0x65, 0x73, 0x12, 0x26, 0x0a, 0x0f, 0x6e, 0x65, 0x78, 0x74, 0x5f, 0x70, 0x61, + 0x67, 0x65, 0x5f, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0d, + 0x6e, 0x65, 0x78, 0x74, 0x50, 0x61, 0x67, 0x65, 0x54, 0x6f, 0x6b, 0x65, 0x6e, 0x12, 0x23, 0x0a, + 0x0d, 0x74, 0x6f, 0x74, 0x61, 0x6c, 0x5f, 0x6d, 0x61, 0x74, 0x63, 0x68, 0x65, 0x73, 0x18, 0x03, + 0x20, 0x01, 0x28, 0x05, 0x52, 0x0c, 0x74, 0x6f, 0x74, 0x61, 0x6c, 0x4d, 0x61, 0x74, 0x63, 0x68, + 0x65, 0x73, 0x22, 0xc6, 0x01, 0x0a, 0x12, 0x53, 0x65, 0x61, 0x72, 0x63, 0x68, 0x49, 0x6e, 0x64, + 0x65, 0x78, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x26, 0x0a, 0x09, 0x70, 0x61, 0x67, + 0x65, 0x5f, 0x73, 0x69, 0x7a, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x42, 0x04, 0xe2, 0x41, + 0x01, 0x01, 0x48, 0x00, 0x52, 0x08, 0x70, 0x61, 0x67, 0x65, 0x53, 0x69, 0x7a, 0x65, 0x88, 0x01, + 0x01, 0x12, 0x23, 0x0a, 0x0a, 0x70, 0x61, 0x67, 0x65, 0x5f, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x18, + 0x02, 0x20, 0x01, 0x28, 0x09, 0x42, 0x04, 0xe2, 0x41, 0x01, 0x01, 0x52, 0x09, 0x70, 0x61, 0x67, + 0x65, 0x54, 0x6f, 0x6b, 0x65, 0x6e, 0x12, 0x14, 0x0a, 0x05, 0x71, 0x75, 0x65, 0x72, 0x79, 0x18, + 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x71, 0x75, 0x65, 0x72, 0x79, 0x12, 0x3f, 0x0a, 0x03, + 0x72, 0x65, 0x66, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x27, 0x2e, 0x6f, 0x70, 0x65, 0x6e, + 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x2e, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x73, 0x2e, 0x73, + 0x65, 0x61, 0x72, 0x63, 0x68, 0x2e, 0x76, 0x30, 0x2e, 0x52, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, + 0x63, 0x65, 0x42, 0x04, 0xe2, 0x41, 0x01, 0x01, 0x52, 0x03, 0x72, 0x65, 0x66, 0x42, 0x0c, 0x0a, + 0x0a, 0x5f, 0x70, 0x61, 0x67, 0x65, 0x5f, 0x73, 0x69, 0x7a, 0x65, 0x22, 0xa1, 0x01, 0x0a, 0x13, 0x53, 0x65, 0x61, 0x72, 0x63, 0x68, 0x49, 0x6e, 0x64, 0x65, 0x78, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x3d, 0x0a, 0x07, 0x6d, 0x61, 0x74, 0x63, 0x68, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x23, 0x2e, 0x6f, 0x70, 0x65, 0x6e, 0x63, 0x6c, 0x6f, 0x75, 0x64, @@ -801,6 +815,8 @@ func file_opencloud_services_search_v0_search_proto_init() { } } } + file_opencloud_services_search_v0_search_proto_msgTypes[0].OneofWrappers = []interface{}{} + file_opencloud_services_search_v0_search_proto_msgTypes[2].OneofWrappers = []interface{}{} type x struct{} out := protoimpl.TypeBuilder{ File: protoimpl.DescBuilder{ diff --git a/protogen/gen/opencloud/services/search/v0/search.swagger.json b/protogen/gen/opencloud/services/search/v0/search.swagger.json index db24bd32b5..b726552014 100644 --- a/protogen/gen/opencloud/services/search/v0/search.swagger.json +++ b/protogen/gen/opencloud/services/search/v0/search.swagger.json @@ -574,6 +574,11 @@ }, "ref": { "$ref": "#/definitions/v0Reference" + }, + "from": { + "type": "integer", + "format": "int32", + "description": "Optional. Number of leading matches to skip in the globally sorted,\ncross-space merged result list." } } }, diff --git a/protogen/proto/opencloud/services/search/v0/search.proto b/protogen/proto/opencloud/services/search/v0/search.proto index 806245d95e..dd9300a6c0 100644 --- a/protogen/proto/opencloud/services/search/v0/search.proto +++ b/protogen/proto/opencloud/services/search/v0/search.proto @@ -64,7 +64,7 @@ service IndexProvider { message SearchRequest { // Optional. The maximum number of entries to return in the response - int32 page_size = 1 [(google.api.field_behavior) = OPTIONAL]; + optional int32 page_size = 1 [(google.api.field_behavior) = OPTIONAL]; // Optional. A pagination token returned from a previous call to `Get` // that indicates from where search should continue @@ -72,6 +72,10 @@ message SearchRequest { string query = 3; opencloud.messages.search.v0.Reference ref = 4 [(google.api.field_behavior) = OPTIONAL]; + + // Optional. Number of leading matches to skip in the globally sorted, + // cross-space merged result list. + int32 from = 7 [(google.api.field_behavior) = OPTIONAL]; } message SearchResponse { @@ -85,7 +89,7 @@ message SearchResponse { message SearchIndexRequest { // Optional. The maximum number of entries to return in the response - int32 page_size = 1 [(google.api.field_behavior) = OPTIONAL]; + optional int32 page_size = 1 [(google.api.field_behavior) = OPTIONAL]; // Optional. A pagination token returned from a previous call to `Get` // that indicates from where search should continue diff --git a/services/graph/pkg/service/v0/tags.go b/services/graph/pkg/service/v0/tags.go index 1a459e8962..d79bc02936 100644 --- a/services/graph/pkg/service/v0/tags.go +++ b/services/graph/pkg/service/v0/tags.go @@ -7,13 +7,14 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/go-chi/render" + libregraph "github.com/opencloud-eu/libre-graph-api-go" + "github.com/opencloud-eu/opencloud/pkg/conversions" searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" revaCtx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/storagespace" "github.com/opencloud-eu/reva/v2/pkg/tags" - libregraph "github.com/opencloud-eu/libre-graph-api-go" "go-micro.dev/v4/metadata" ) @@ -24,7 +25,7 @@ func (g Graph) GetTags(w http.ResponseWriter, r *http.Request) { ctx = metadata.Set(ctx, revaCtx.TokenHeader, th) sr, err := g.searchService.Search(ctx, &searchsvc.SearchRequest{ Query: "Tags:*", - PageSize: -1, + PageSize: conversions.ToPointer(int32(-1)), }) if err != nil { g.logger.Error().Err(err).Msg("Could not search for tags") diff --git a/services/search/pkg/bleve/backend.go b/services/search/pkg/bleve/backend.go index d8426e5435..5d0b364708 100644 --- a/services/search/pkg/bleve/backend.go +++ b/services/search/pkg/bleve/backend.go @@ -82,17 +82,16 @@ func (b *Backend) Search(_ context.Context, sir *searchService.SearchIndexReques } } + size, err := search.EnginePageSize(sir.PageSize, math.MaxInt) + if err != nil { + return nil, err + } + bleveReq := bleve.NewSearchRequest(q) bleveReq.Highlight = bleve.NewHighlight() - - switch { - case sir.PageSize == -1: - bleveReq.Size = math.MaxInt - case sir.PageSize == 0: - bleveReq.Size = 200 - default: - bleveReq.Size = int(sir.PageSize) - } + // ties by id, like the cross-space merge + bleveReq.SortBy([]string{"-_score", "_id"}) + bleveReq.Size = size bleveReq.Fields = []string{"*"} res, err := b.index.Search(bleveReq) diff --git a/services/search/pkg/opensearch/backend.go b/services/search/pkg/opensearch/backend.go index 330dcd577d..b3ff91fb2b 100644 --- a/services/search/pkg/opensearch/backend.go +++ b/services/search/pkg/opensearch/backend.go @@ -109,17 +109,19 @@ func (b *Backend) Search(ctx context.Context, sir *searchService.SearchIndexRequ } } - searchParams := opensearchgoAPI.SearchParams{ - SourceExcludes: []string{"Content"}, // Do not send back the full content in the search response, as it is only needed for highlighting and can be large. The highlighted snippets will be sent back in the response instead. + size, err := search.EnginePageSize(sir.PageSize, 1000) + if err != nil { + return nil, err } - switch { - case sir.PageSize == -1: - searchParams.Size = conversions.ToPointer(1000) - case sir.PageSize == 0: - searchParams.Size = conversions.ToPointer(200) - default: - searchParams.Size = conversions.ToPointer(int(sir.PageSize)) + searchParams := opensearchgoAPI.SearchParams{ + SourceExcludes: []string{"Content"}, // Do not send back the full content in the search response, as it is only needed for highlighting and can be large. The highlighted snippets will be sent back in the response instead. + // ties by id, like the cross-space merge + Sort: []string{"_score:desc", "ID:asc"}, + TrackScores: conversions.ToPointer(true), + // count every match, the default stops at 10000 + TrackTotalHits: true, + Size: conversions.ToPointer(size), } req, err := osu.BuildSearchReq(&opensearchgoAPI.SearchReq{ diff --git a/services/search/pkg/parity/parity_test.go b/services/search/pkg/parity/parity_test.go index 41695d18b0..f65eff1161 100644 --- a/services/search/pkg/parity/parity_test.go +++ b/services/search/pkg/parity/parity_test.go @@ -11,6 +11,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/errtypes" + "github.com/opencloud-eu/opencloud/pkg/conversions" searchMessage "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/search/v0" searchService "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" "github.com/opencloud-eu/opencloud/services/search/pkg/search" @@ -284,7 +285,10 @@ var _ = Describe("Queries", func() { Skip(e.unavailable) } - request := &searchService.SearchIndexRequest{Query: c.query, PageSize: c.limit, Ref: c.ref} + request := &searchService.SearchIndexRequest{Query: c.query, Ref: c.ref} + if c.limit != 0 { + request.PageSize = conversions.ToPointer(c.limit) + } expected := override{want: c.want, wantCount: c.wantCount, wantBadRequest: c.wantBadRequest} _, overridden := c.engineOverrides[name] diff --git a/services/search/pkg/search/search.go b/services/search/pkg/search/search.go index fb81135f2f..ae2db60310 100644 --- a/services/search/pkg/search/search.go +++ b/services/search/pkg/search/search.go @@ -1,6 +1,7 @@ package search import ( + "cmp" "context" "errors" "fmt" @@ -126,6 +127,18 @@ func ResolveReference(ctx context.Context, ref *provider.Reference, ri *provider }, nil } +// MaxResultWindow is how deep a search pages: OpenSearch returns no matches +// beyond its index.max_result_window, so no engine does. +const MaxResultWindow = 10000 + +// CheckResultWindow rejects a page that reaches beyond MaxResultWindow. +func CheckResultWindow(from, size int32) error { + if int64(from)+int64(size) > MaxResultWindow { + return fmt.Errorf("from and size reach beyond the first %d matches", MaxResultWindow) + } + return nil +} + type matchArray []*searchmsg.Match func (ma matchArray) Len() int { @@ -134,8 +147,20 @@ func (ma matchArray) Len() int { func (ma matchArray) Swap(i, j int) { ma[i], ma[j] = ma[j], ma[i] } + +// Less orders by score and breaks ties by id: without that, matches with the +// same score change places between requests and pages overlap. The engines +// order their pages the same way. func (ma matchArray) Less(i, j int) bool { - return ma[i].GetScore() > ma[j].GetScore() + if ma[i].GetScore() != ma[j].GetScore() { + return ma[i].GetScore() > ma[j].GetScore() + } + a, b := ma[i].GetEntity().GetId(), ma[j].GetEntity().GetId() + return cmp.Or( + cmp.Compare(a.GetStorageId(), b.GetStorageId()), + cmp.Compare(a.GetSpaceId(), b.GetSpaceId()), + cmp.Compare(a.GetOpaqueId(), b.GetOpaqueId()), + ) < 0 } func logDocCount(engine Engine, logger log.Logger) { diff --git a/services/search/pkg/search/service.go b/services/search/pkg/search/service.go index a45968197e..7bb3a9ce33 100644 --- a/services/search/pkg/search/service.go +++ b/services/search/pkg/search/service.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math" "path/filepath" "reflect" "sort" @@ -30,6 +31,7 @@ import ( "google.golang.org/grpc/metadata" "google.golang.org/protobuf/types/known/fieldmaskpb" + "github.com/opencloud-eu/opencloud/pkg/conversions" "github.com/opencloud-eu/opencloud/pkg/log" searchmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/search/v0" searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" @@ -137,6 +139,7 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se return nil, errtypes.BadRequest("empty query provided") } req.Query = query + if len(scope) > 0 { scopedID, err := storagespace.ParseID(scope) if err != nil { @@ -299,13 +302,17 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se } } - // compile one sorted list of matches from all spaces and apply the limit if needed + // compile one sorted list of matches from all spaces and apply from/limit if needed sort.Sort(matches) - limit := req.PageSize - if limit == 0 { - limit = 200 + limit := PageSizeOrDefault(req.PageSize) + if from := int(req.GetFrom()); from > 0 { + if from < len(matches) { + matches = matches[from:] + } else { + matches = nil + } } - if int32(len(matches)) > limit && limit != -1 { + if limit != -1 && int32(len(matches)) > limit { matches = matches[0:limit] } @@ -316,6 +323,40 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se }, nil } +// PageSizeOrDefault reads an absent page size as 200; -1 is no limit. +func PageSizeOrDefault(pageSize *int32) int32 { + if pageSize != nil { + return *pageSize + } + return 200 +} + +// EnginePageSize is the page an engine fetches: the default for an absent +// size, unlimited for -1, and never beyond the result window. +func EnginePageSize(pageSize *int32, unlimited int) (int, error) { + size := PageSizeOrDefault(pageSize) + if err := CheckResultWindow(0, size); err != nil { + return 0, errtypes.BadRequest(err.Error()) + } + if size == -1 { + return unlimited, nil + } + return int(size), nil +} + +// engineFetchSize: the global offset cannot be distributed across spaces, so +// every space must return the full prefix up to from+limit for the merge. +func engineFetchSize(from int32, pageSize *int32) int32 { + limit := PageSizeOrDefault(pageSize) + if limit <= 0 || from <= 0 { + return limit + } + if total := int64(from) + int64(limit); total <= math.MaxInt32 { + return int32(total) + } + return math.MaxInt32 +} + func (s *Service) searchIndex(ctx context.Context, req *searchsvc.SearchRequest, space *provider.StorageSpace, mountpointID string) (*searchsvc.SearchIndexResponse, error) { if req.Ref != nil && (req.Ref.ResourceId.StorageId != space.Root.StorageId || @@ -418,7 +459,8 @@ func (s *Service) searchIndex(ctx context.Context, req *searchsvc.SearchRequest, ResourceId: searchRootID, Path: searchPathPrefix, }, - PageSize: req.PageSize, + // not GetPageSize(): the getter collapses nil (default) and explicit 0 + PageSize: conversions.ToPointer(engineFetchSize(req.GetFrom(), req.PageSize)), } start := time.Now() res, err := s.engine.Search(ctx, searchRequest) diff --git a/services/search/pkg/search/service_test.go b/services/search/pkg/search/service_test.go index 5db825bc9e..8448620925 100644 --- a/services/search/pkg/search/service_test.go +++ b/services/search/pkg/search/service_test.go @@ -2,6 +2,7 @@ package search_test import ( "context" + "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -17,6 +18,7 @@ import ( "github.com/stretchr/testify/mock" "google.golang.org/grpc" + "github.com/opencloud-eu/opencloud/pkg/conversions" "github.com/opencloud-eu/opencloud/pkg/log" searchmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/search/v0" searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" @@ -260,6 +262,72 @@ var _ = Describe("Searchprovider", func() { }) }) + // two personal spaces, the engine answers each by its space id + var ( + spaceA = &sprovider.StorageSpace{ + Id: &sprovider.StorageSpaceId{OpaqueId: "storageid$a!a"}, + Root: &sprovider.ResourceId{StorageId: "storageid", SpaceId: "a", OpaqueId: "a"}, + SpaceType: "personal", + } + spaceB = &sprovider.StorageSpace{ + Id: &sprovider.StorageSpaceId{OpaqueId: "storageid$b!b"}, + Root: &sprovider.ResourceId{StorageId: "storageid", SpaceId: "b", OpaqueId: "b"}, + SpaceType: "personal", + } + ) + listsSpacesAB := func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&sprovider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*sprovider.StorageSpace{spaceA, spaceB}, + }, nil) + } + searchOfSpace := func(id string) *mock.Call { + return indexClient.On("Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { + return req.GetRef().GetResourceId().GetSpaceId() == id + })) + } + + Context("with two personal spaces returning matches of the same score", func() { + match := func(space, name string) *searchmsg.Match { + return &searchmsg.Match{Score: 1, Entity: &searchmsg.Entity{ + Id: &searchmsg.ResourceID{StorageId: "storageid", SpaceId: space, OpaqueId: name}, + Ref: &searchmsg.Reference{ResourceId: &searchmsg.ResourceID{StorageId: "storageid", SpaceId: space, OpaqueId: space}, Path: "./" + name}, + Name: name, + }} + } + + // answers of the spaces arrive in the order of their delays + searchWith := func(delayA, delayB time.Duration) { + listsSpacesAB() + searchOfSpace("a").After(delayA).Return(func(context.Context, *searchsvc.SearchIndexRequest) (*searchsvc.SearchIndexResponse, error) { + return &searchsvc.SearchIndexResponse{TotalMatches: 2, Matches: []*searchmsg.Match{match("a", "a1"), match("a", "a2")}}, nil + }) + searchOfSpace("b").After(delayB).Return(func(context.Context, *searchsvc.SearchIndexRequest) (*searchsvc.SearchIndexResponse, error) { + return &searchsvc.SearchIndexResponse{TotalMatches: 2, Matches: []*searchmsg.Match{match("b", "b1"), match("b", "b2")}}, nil + }) + } + + page := func(from int32) []string { + res, err := s.Search(ctx, &searchsvc.SearchRequest{Query: "foo", From: from, PageSize: conversions.ToPointer(int32(2))}) + Expect(err).ToNot(HaveOccurred()) + names := []string{} + for _, m := range res.Matches { + names = append(names, m.GetEntity().GetName()) + } + return names + } + + DescribeTable("cuts pages that neither overlap nor skip, whichever space answers first", + func(delayA, delayB time.Duration) { + searchWith(delayA, delayB) + Expect(page(0)).To(Equal([]string{"a1", "a2"})) + Expect(page(2)).To(Equal([]string{"b1", "b2"})) + }, + Entry("space a answers first", time.Duration(0), 20*time.Millisecond), + Entry("space b answers first", 20*time.Millisecond, time.Duration(0)), + ) + }) + Context("with a personal space with a filter", func() { BeforeEach(func() { gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&sprovider.ListStorageSpacesResponse{ @@ -550,7 +618,7 @@ var _ = Describe("Searchprovider", func() { It("sorts and limits the combined results from all spaces", func() { res, err := s.Search(ctx, &searchsvc.SearchRequest{ Query: "foo", - PageSize: 2, + PageSize: conversions.ToPointer(int32(2)), }) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) @@ -558,6 +626,37 @@ var _ = Describe("Searchprovider", func() { ids := []string{res.Matches[0].Entity.Id.OpaqueId, res.Matches[1].Entity.Id.OpaqueId} Expect(ids).To(Equal([]string{"grant-shared-id", "foo-id"})) }) + + It("applies the from offset after the cross-space merge", func() { + res, err := s.Search(ctx, &searchsvc.SearchRequest{ + Query: "foo", + PageSize: conversions.ToPointer(int32(2)), + From: 1, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(len(res.Matches)).To(Equal(2)) + ids := []string{res.Matches[0].Entity.Id.OpaqueId, res.Matches[1].Entity.Id.OpaqueId} + Expect(ids).To(Equal([]string{"foo-id", "grant-irrelevant-id"})) + + for _, call := range indexClient.Calls { + if call.Method != "Search" { + continue + } + req := call.Arguments.Get(1).(*searchsvc.SearchIndexRequest) + Expect(req.GetPageSize()).To(Equal(int32(3)), "every space must return the full prefix up to from+size") + } + }) + + It("returns no matches when from points past the merged list", func() { + res, err := s.Search(ctx, &searchsvc.SearchRequest{ + Query: "foo", + PageSize: conversions.ToPointer(int32(2)), + From: 10, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.Matches).To(BeEmpty()) + Expect(res.TotalMatches).To(Equal(int32(3))) + }) }) }) }) diff --git a/services/search/pkg/service/grpc/v0/service.go b/services/search/pkg/service/grpc/v0/service.go index 48f14ec75c..d5f8637d7e 100644 --- a/services/search/pkg/service/grpc/v0/service.go +++ b/services/search/pkg/service/grpc/v0/service.go @@ -3,7 +3,6 @@ package service import ( "context" "errors" - "fmt" "sync" "time" @@ -22,10 +21,10 @@ import ( "go-micro.dev/v4/metadata" "golang.org/x/sync/errgroup" grpcmetadata "google.golang.org/grpc/metadata" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/durationpb" "github.com/opencloud-eu/opencloud/pkg/log" - v0 "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/search/v0" searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" "github.com/opencloud-eu/opencloud/services/search/pkg/config" "github.com/opencloud-eu/opencloud/services/search/pkg/search" @@ -100,15 +99,11 @@ func (s Service) Search(ctx context.Context, in *searchsvc.SearchRequest, out *s } ctx = revactx.ContextSetUser(ctx, u) - key := cacheKey(in.Query, in.PageSize, in.Ref, u) + key := cacheKey(in, u) res, ok := s.FromCache(key) if !ok { var err error - res, err = s.searcher.Search(ctx, &searchsvc.SearchRequest{ - Query: in.Query, - PageSize: in.PageSize, - Ref: in.Ref, - }) + res, err = s.searcher.Search(ctx, in) if err != nil { switch err.(type) { case errtypes.BadRequest: @@ -265,6 +260,11 @@ func (s Service) Cache(key string, res *searchsvc.SearchResponse) { _ = s.cache.Set(key, res) } -func cacheKey(query string, pagesize int32, ref *v0.Reference, user *user.User) string { - return fmt.Sprintf("%s|%d|%s$%s!%s/%s|%s", query, pagesize, ref.GetResourceId().GetStorageId(), ref.GetResourceId().GetSpaceId(), ref.GetResourceId().GetOpaqueId(), ref.GetPath(), user.GetId().GetOpaqueId()) +// cacheKey is the request as the searcher sees it, per user. An absent page +// size is the default, both share an entry. +func cacheKey(req *searchsvc.SearchRequest, user *user.User) string { + keyed := proto.Clone(req).(*searchsvc.SearchRequest) + keyed.PageSize = proto.Int32(search.PageSizeOrDefault(req.PageSize)) + b, _ := proto.MarshalOptions{Deterministic: true}.Marshal(keyed) + return user.GetId().GetOpaqueId() + "|" + string(b) } diff --git a/services/webdav/pkg/service/v0/search.go b/services/webdav/pkg/service/v0/search.go index e2f6a3f4a4..ed558b05c4 100644 --- a/services/webdav/pkg/service/v0/search.go +++ b/services/webdav/pkg/service/v0/search.go @@ -23,6 +23,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/tags" "github.com/opencloud-eu/reva/v2/pkg/utils" + "github.com/opencloud-eu/opencloud/pkg/conversions" searchmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/search/v0" searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/thumbnail" @@ -74,9 +75,10 @@ func (g Webdav) Search(w http.ResponseWriter, r *http.Request) { ctx := revactx.ContextSetToken(r.Context(), t) ctx = metadata.Set(ctx, revactx.TokenHeader, t) - req := &searchsvc.SearchRequest{ - Query: rep.SearchFiles.Search.Pattern, - PageSize: int32(rep.SearchFiles.Search.Limit), + req, err := searchRequestOf(rep.SearchFiles.Search) + if err != nil { + renderError(w, r, errBadRequest(err.Error())) + return } // Limit search to the according space when searching /dav/spaces/ @@ -115,6 +117,22 @@ func (g Webdav) Search(w http.ResponseWriter, r *http.Request) { g.sendSearchResponse(davPrefix, rsp, w, r, user) } +// searchRequestOf maps the search element of the report: limit -1 is no +// limit, 0 the default, offset skips leading matches of the merged list. +func searchRequestOf(search reportSearchFilesSearch) (*searchsvc.SearchRequest, error) { + if search.Limit < -1 { + return nil, fmt.Errorf("limit must be -1, 0 or positive") + } + if search.Offset < 0 { + return nil, fmt.Errorf("offset must not be negative") + } + req := &searchsvc.SearchRequest{Query: search.Pattern, From: int32(search.Offset)} + if search.Limit != 0 { + req.PageSize = conversions.ToPointer(int32(search.Limit)) + } + return req, nil +} + func (g Webdav) sendSearchResponse(davPrefix string, rsp *searchsvc.SearchResponse, w http.ResponseWriter, r *http.Request, user *userv1beta1.User) { logger := g.log.SubloggerWithRequestID(r.Context()) responsesXML, err := multistatusResponse(r.Context(), davPrefix, g.config.OpenCloudPublicURL, rsp.Matches, user) diff --git a/services/webdav/pkg/service/v0/search_test.go b/services/webdav/pkg/service/v0/search_test.go index 5bfa2782d8..ff0bf5412a 100644 --- a/services/webdav/pkg/service/v0/search_test.go +++ b/services/webdav/pkg/service/v0/search_test.go @@ -5,6 +5,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "google.golang.org/protobuf/testing/protocmp" + + "github.com/opencloud-eu/opencloud/pkg/conversions" + searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" ) func TestSearch(t *testing.T) { @@ -33,3 +37,30 @@ var _ = Describe("SpacesSearchRegex", func() { Entry("unrelated path", "/dav/files/123", "", false), ) }) + +var _ = Describe("searchRequestOf", func() { + DescribeTable("maps pattern, limit and offset of the report", + func(search reportSearchFilesSearch, want *searchsvc.SearchRequest) { + got, err := searchRequestOf(search) + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(BeComparableTo(want, protocmp.Transform())) + }, + Entry("the pattern alone, the service applies its default page size", + reportSearchFilesSearch{Pattern: "notes"}, &searchsvc.SearchRequest{Query: "notes"}), + Entry("a limit", reportSearchFilesSearch{Pattern: "notes", Limit: 25}, + &searchsvc.SearchRequest{Query: "notes", PageSize: conversions.ToPointer(int32(25))}), + Entry("no limit", reportSearchFilesSearch{Pattern: "notes", Limit: -1}, + &searchsvc.SearchRequest{Query: "notes", PageSize: conversions.ToPointer(int32(-1))}), + Entry("an offset pages the merged list", reportSearchFilesSearch{Pattern: "notes", Limit: 25, Offset: 50}, + &searchsvc.SearchRequest{Query: "notes", PageSize: conversions.ToPointer(int32(25)), From: 50}), + ) + + DescribeTable("rejects", + func(search reportSearchFilesSearch) { + _, err := searchRequestOf(search) + Expect(err).To(HaveOccurred()) + }, + Entry("a negative offset", reportSearchFilesSearch{Pattern: "notes", Offset: -1}), + Entry("a limit below -1", reportSearchFilesSearch{Pattern: "notes", Limit: -2}), + ) +})