Merge pull request #7815 from 2403905/issue-7812

fix search response
owncloud · Feb 9, 2024 · 7470550 · 7470550
2 parents 39b0192 + 17a799c
commit 7470550
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 18 deletions.
diff --git a/changelog/unreleased/fix-search-responce.md 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
@@ -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 → services/search/pkg/query/error.go b/services/search/pkg/query/kql/error.go → 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
@@ -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
@@ -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
@@ -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
@@ -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}
 			}
 		}
 	}