Repository: owncloud/ocis
Pull request #7815 (2023-11-27 21:13:01)

fix search response
<!--
Thanks for submitting a change to ownCloud!

This is the bug tracker for the oCIS component. Find other components at https://github.com/owncloud/core/blob/master/.github/CONTRIBUTING.md#guidelines

For fixing potential security issues please see https://owncloud.org/security/

To make it possible for us to get your change reviewed and merged please carefully fill out the requested information below.

Please note that any kind of change needs first be submitted to the master branch which holds the next version of oCIS.

Please set the following labels:

- Set label "Status:Needs-Review" for review or "Status:In-Progress" if the PR still has open tasks.
- Assignment: assign to self
- Reviewers: pick at least one
-->

## Description
<!--- Describe your changes in detail -->
We fixed the search response code from 500 to 400 when the request is invalid

## Related Issue
<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->
- Fixes [https://github.com/owncloud/ocis/issues/7812](https://github.com/owncloud/ocis/issues/7812)

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
- test environment:
- test case 1:
- test case 2:
- ...

## Screenshots (if appropriate):

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Technical debt
- [ ] Tests only (no source changes)

## Checklist:
<!-- Tick the checkboxes when done. -->
<!-- Raise documentation ticket in owncloud.github.io/ -->
- [x] Code changes
- [ ] Unit tests added
- [ ] Acceptance tests added
- [ ] Documentation ticket raised: <link> 


sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=7815)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=7815&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=7815&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=7815&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=7815&resolved=false&types=CODE_SMELL)

[![23.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '23.1%')](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=7815&metric=new_coverage&view=list) [23.1% Coverage](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=7815&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=7815&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=7815&metric=new_duplicated_lines_density&view=list)


micbar: @2403905 @fschade Did this get stuck?
fschade: I missed the review request, sorry for that, I’ll check it in the morning, thanks for the reminder!

Patch:
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/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}
 			}
 		}
 	}


Test patch:
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},
 			},
 		},