From 17a799c1fa57058ad47225e6b3240ecf3a1ba128 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Mon, 27 Nov 2023 22:12:38 +0100 Subject: [PATCH] fix search response --- changelog/unreleased/fix-search-responce.md | 6 ++++++ services/search/pkg/engine/bleve.go | 4 ++++ services/search/pkg/query/{kql => }/error.go | 10 ++++++++- services/search/pkg/query/kql/cast.go | 5 +++-- .../search/pkg/query/kql/dictionary_test.go | 21 ++++++++++--------- services/search/pkg/query/kql/kql_test.go | 3 ++- services/search/pkg/query/kql/validate.go | 8 +++---- 7 files changed, 39 insertions(+), 18 deletions(-) create mode 100644 changelog/unreleased/fix-search-responce.md rename services/search/pkg/query/{kql => }/error.go (82%) diff --git a/changelog/unreleased/fix-search-responce.md b/changelog/unreleased/fix-search-responce.md new file mode 100644 index 00000000000..e7f308a153c --- /dev/null +++ b/changelog/unreleased/fix-search-responce.md @@ -0,0 +1,6 @@ +Bugfix: Fix search response + +We fixed the search response code from 500 to 400 when the request is invalid + +https://github.com/owncloud/ocis/pull/7815 +https://github.com/owncloud/ocis/issues/7812 diff --git a/services/search/pkg/engine/bleve.go b/services/search/pkg/engine/bleve.go index a0b3f26f30c..54375d99250 100644 --- a/services/search/pkg/engine/bleve.go +++ b/services/search/pkg/engine/bleve.go @@ -22,6 +22,7 @@ import ( storageProvider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "google.golang.org/protobuf/types/known/timestamppb" + "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" @@ -123,6 +124,9 @@ func BuildBleveMapping() (mapping.IndexMapping, error) { func (b *Bleve) Search(ctx context.Context, sir *searchService.SearchIndexRequest) (*searchService.SearchIndexResponse, error) { createdQuery, err := b.queryCreator.Create(sir.Query) if err != nil { + if searchQuery.IsValidationError(err) { + return nil, errtypes.BadRequest(err.Error()) + } return nil, err } diff --git a/services/search/pkg/query/kql/error.go b/services/search/pkg/query/error.go similarity index 82% rename from services/search/pkg/query/kql/error.go rename to services/search/pkg/query/error.go index 5f171d77fdb..89f504c8a45 100644 --- a/services/search/pkg/query/kql/error.go +++ b/services/search/pkg/query/error.go @@ -1,4 +1,4 @@ -package kql +package query import ( "fmt" @@ -37,3 +37,11 @@ type UnsupportedTimeRangeError struct { func (e UnsupportedTimeRangeError) Error() string { return fmt.Sprintf("unable to convert '%v' to a time range", e.Value) } + +func IsValidationError(err error) bool { + switch err.(type) { + case *StartsWithBinaryOperatorError, *NamedGroupInvalidNodesError, *UnsupportedTimeRangeError: + return true + } + return false +} diff --git a/services/search/pkg/query/kql/cast.go b/services/search/pkg/query/kql/cast.go index 06fb800ce5a..c6ca30541ac 100644 --- a/services/search/pkg/query/kql/cast.go +++ b/services/search/pkg/query/kql/cast.go @@ -5,6 +5,7 @@ import ( "time" "github.com/jinzhu/now" + "github.com/owncloud/ocis/v2/services/search/pkg/query" "github.com/owncloud/ocis/v2/services/search/pkg/query/ast" ) @@ -84,7 +85,7 @@ func toTimeRange(in interface{}) (*time.Time, *time.Time, error) { value, err := toString(in) if err != nil { - return &from, &to, UnsupportedTimeRangeError{} + return &from, &to, &query.UnsupportedTimeRangeError{} } c := &now.Config{ @@ -131,7 +132,7 @@ func toTimeRange(in interface{}) (*time.Time, *time.Time, error) { } if from.IsZero() || to.IsZero() { - return nil, nil, UnsupportedTimeRangeError{} + return nil, nil, &query.UnsupportedTimeRangeError{} } return &from, &to, nil diff --git a/services/search/pkg/query/kql/dictionary_test.go b/services/search/pkg/query/kql/dictionary_test.go index ba2780db895..adf4bd2d45e 100644 --- a/services/search/pkg/query/kql/dictionary_test.go +++ b/services/search/pkg/query/kql/dictionary_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/jinzhu/now" + "github.com/owncloud/ocis/v2/services/search/pkg/query" tAssert "github.com/stretchr/testify/assert" "github.com/owncloud/ocis/v2/services/search/pkg/query/ast" @@ -34,13 +35,13 @@ func TestParse_Spec(t *testing.T) { }, { name: `AND`, - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolAND}, }, }, { name: `AND cat AND dog`, - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolAND}, }, }, @@ -80,13 +81,13 @@ func TestParse_Spec(t *testing.T) { }, { name: `OR`, - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolOR}, }, }, { name: `OR cat AND dog`, - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolOR}, }, }, @@ -862,37 +863,37 @@ func TestParse_Errors(t *testing.T) { tests := []testCase{ { query: "animal:(mammal:cat mammal:dog reptile:turtle)", - error: kql.NamedGroupInvalidNodesError{ + error: query.NamedGroupInvalidNodesError{ Node: &ast.StringNode{Key: "mammal", Value: "cat"}, }, }, { query: "animal:(cat mammal:dog turtle)", - error: kql.NamedGroupInvalidNodesError{ + error: query.NamedGroupInvalidNodesError{ Node: &ast.StringNode{Key: "mammal", Value: "dog"}, }, }, { query: "animal:(AND cat)", - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolAND}, }, }, { query: "animal:(OR cat)", - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolOR}, }, }, { query: "(AND cat)", - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolAND}, }, }, { query: "(OR cat)", - error: kql.StartsWithBinaryOperatorError{ + error: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolOR}, }, }, diff --git a/services/search/pkg/query/kql/kql_test.go b/services/search/pkg/query/kql/kql_test.go index de050a47413..c130f755820 100644 --- a/services/search/pkg/query/kql/kql_test.go +++ b/services/search/pkg/query/kql/kql_test.go @@ -3,6 +3,7 @@ package kql_test import ( "testing" + "github.com/owncloud/ocis/v2/services/search/pkg/query" tAssert "github.com/stretchr/testify/assert" "github.com/owncloud/ocis/v2/services/search/pkg/query/ast" @@ -22,7 +23,7 @@ func TestNewAST(t *testing.T) { { name: "error", givenQuery: kql.BoolAND, - expectedError: kql.StartsWithBinaryOperatorError{ + expectedError: query.StartsWithBinaryOperatorError{ Node: &ast.OperatorNode{Value: kql.BoolAND}, }, }, diff --git a/services/search/pkg/query/kql/validate.go b/services/search/pkg/query/kql/validate.go index 9e9a2428b99..96c6761ab17 100644 --- a/services/search/pkg/query/kql/validate.go +++ b/services/search/pkg/query/kql/validate.go @@ -1,6 +1,7 @@ package kql import ( + "github.com/owncloud/ocis/v2/services/search/pkg/query" "github.com/owncloud/ocis/v2/services/search/pkg/query/ast" ) @@ -9,10 +10,9 @@ func validateAst(a *ast.Ast) error { case *ast.OperatorNode: switch node.Value { case BoolAND, BoolOR: - return StartsWithBinaryOperatorError{Node: node} + return &query.StartsWithBinaryOperatorError{Node: node} } } - return nil } @@ -21,14 +21,14 @@ func validateGroupNode(n *ast.GroupNode) error { case *ast.OperatorNode: switch node.Value { case BoolAND, BoolOR: - return StartsWithBinaryOperatorError{Node: node} + return &query.StartsWithBinaryOperatorError{Node: node} } } if n.Key != "" { for _, node := range n.Nodes { if ast.NodeKey(node) != "" { - return NamedGroupInvalidNodesError{Node: node} + return &query.NamedGroupInvalidNodesError{Node: node} } } }