From 57c4f056ef8af00e1e1914ceb98ef8ed3a613959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 4 Jul 2022 16:35:44 +0200 Subject: [PATCH 1/4] Return proper error codes, e.g. when providing an empty query --- services/search/pkg/search/provider/searchprovider.go | 2 +- services/search/pkg/service/v0/service.go | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/services/search/pkg/search/provider/searchprovider.go b/services/search/pkg/search/provider/searchprovider.go index 310909021..52954b3e1 100644 --- a/services/search/pkg/search/provider/searchprovider.go +++ b/services/search/pkg/search/provider/searchprovider.go @@ -67,7 +67,7 @@ func New(gwClient gateway.GatewayAPIClient, indexClient search.IndexClient, mach func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*searchsvc.SearchResponse, error) { if req.Query == "" { - return nil, errtypes.PreconditionFailed("empty query provided") + return nil, errtypes.BadRequest("empty query provided") } p.logger.Debug().Str("query", req.Query).Msg("performing a search") diff --git a/services/search/pkg/service/v0/service.go b/services/search/pkg/service/v0/service.go index d3fd24724..1b003e88d 100644 --- a/services/search/pkg/service/v0/service.go +++ b/services/search/pkg/service/v0/service.go @@ -7,10 +7,12 @@ import ( "github.com/blevesearch/bleve/v2" revactx "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/events" "github.com/cs3org/reva/v2/pkg/events/server" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/go-micro/plugins/v4/events/natsjs" + merrors "go-micro.dev/v4/errors" "go-micro.dev/v4/metadata" grpcmetadata "google.golang.org/grpc/metadata" @@ -95,7 +97,12 @@ func (s Service) Search(ctx context.Context, in *searchsvc.SearchRequest, out *s Query: in.Query, }) if err != nil { - return err + switch err.(type) { + case errtypes.BadRequest: + return merrors.BadRequest(s.id, err.Error()) + default: + return merrors.InternalServerError(s.id, err.Error()) + } } out.Matches = res.Matches From b6d374f683cbe2b5d7c583a7d3c00e186e1c804d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 4 Jul 2022 16:35:44 +0200 Subject: [PATCH 2/4] Support specifying a search limit --- services/search/pkg/search/index/index.go | 3 +++ services/search/pkg/search/provider/searchprovider.go | 1 + services/search/pkg/service/v0/service.go | 3 ++- services/webdav/pkg/service/v0/search.go | 3 ++- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/services/search/pkg/search/index/index.go b/services/search/pkg/search/index/index.go index 8ce98b18b..7ad010740 100644 --- a/services/search/pkg/search/index/index.go +++ b/services/search/pkg/search/index/index.go @@ -223,6 +223,9 @@ func (i *Index) Search(ctx context.Context, req *searchsvc.SearchIndexRequest) ( ) bleveReq := bleve.NewSearchRequest(query) bleveReq.Size = 200 + if req.PageSize > 0 { + bleveReq.Size = int(req.PageSize) + } bleveReq.Fields = []string{"*"} res, err := i.bleveIndex.Search(bleveReq) if err != nil { diff --git a/services/search/pkg/search/provider/searchprovider.go b/services/search/pkg/search/provider/searchprovider.go index 52954b3e1..d82c72463 100644 --- a/services/search/pkg/search/provider/searchprovider.go +++ b/services/search/pkg/search/provider/searchprovider.go @@ -144,6 +144,7 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s }, Path: mountpointPrefix, }, + PageSize: req.PageSize, }) if err != nil { p.logger.Error().Err(err).Str("space", space.Id.OpaqueId).Msg("failed to search the index") diff --git a/services/search/pkg/service/v0/service.go b/services/search/pkg/service/v0/service.go index 1b003e88d..9fd0da6d4 100644 --- a/services/search/pkg/service/v0/service.go +++ b/services/search/pkg/service/v0/service.go @@ -94,7 +94,8 @@ func (s Service) Search(ctx context.Context, in *searchsvc.SearchRequest, out *s ctx = grpcmetadata.AppendToOutgoingContext(ctx, revactx.TokenHeader, t) res, err := s.provider.Search(ctx, &searchsvc.SearchRequest{ - Query: in.Query, + Query: in.Query, + PageSize: in.PageSize, }) if err != nil { switch err.(type) { diff --git a/services/webdav/pkg/service/v0/search.go b/services/webdav/pkg/service/v0/search.go index 7ba77db16..084475ab9 100644 --- a/services/webdav/pkg/service/v0/search.go +++ b/services/webdav/pkg/service/v0/search.go @@ -44,7 +44,8 @@ func (g Webdav) Search(w http.ResponseWriter, r *http.Request) { ctx := revactx.ContextSetToken(r.Context(), t) ctx = metadata.Set(ctx, revactx.TokenHeader, t) rsp, err := g.searchClient.Search(ctx, &searchsvc.SearchRequest{ - Query: rep.SearchFiles.Search.Pattern, + Query: rep.SearchFiles.Search.Pattern, + PageSize: int32(rep.SearchFiles.Search.Limit), }) if err != nil { e := merrors.Parse(err.Error()) From 731ecb00eec70cce8b5dbb85be2d5907a2a738c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 5 Jul 2022 09:23:11 +0200 Subject: [PATCH 3/4] Add changelog --- changelog/unreleased/polish-search.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/polish-search.md diff --git a/changelog/unreleased/polish-search.md b/changelog/unreleased/polish-search.md new file mode 100644 index 000000000..5993a1779 --- /dev/null +++ b/changelog/unreleased/polish-search.md @@ -0,0 +1,5 @@ +Bugfix: Polish search + +We improved the feedback when providing invalid search queries and added support for limiting the number of results returned. + +https://github.com/owncloud/ocis/pull/4094 From c5ec47696e8575089a44676228590da1f8d58d1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 5 Jul 2022 10:06:13 +0200 Subject: [PATCH 4/4] Adapt expected failures --- tests/acceptance/expected-failures-API-on-OCIS-storage.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/acceptance/expected-failures-API-on-OCIS-storage.md b/tests/acceptance/expected-failures-API-on-OCIS-storage.md index b2bc3a85a..a229ffbef 100644 --- a/tests/acceptance/expected-failures-API-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-API-on-OCIS-storage.md @@ -965,10 +965,6 @@ _ocdav: api compatibility, return correct status code_ - [apiWebdavOperations/search.feature:264](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L264) - [apiWebdavOperations/search.feature:270](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L270) -### [Different response status code while searching with empty pattern with new webdav](https://github.com/owncloud/ocis/issues/4016) - -- [apiWebdavOperations/search.feature:103](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L103) - ### [No permisions propertry in response while searching for files and folders on ocis with new webdav](https://github.com/owncloud/ocis/issues/4009) - [apiWebdavOperations/search.feature:208](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L208)