Skip to content

Commit

Permalink
fix(GraphQL): Added error for case when multiple filter functions are…
Browse files Browse the repository at this point in the history
… used in filter. (#7368)

we were having indeterministic behavior in the filter when we have multiple filter functions as below .

query {
  queryFoo(filter:{value:{eq:2 le:2}})
     {
      value
    }
  }
  
It was because we were selecting the first filter function from the list of functions whose order is not guaranteed.
Now we are returning an error for this.
  • Loading branch information
JatinDev543 authored Jan 27, 2021
1 parent 0561801 commit de2f4ab
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
16 changes: 15 additions & 1 deletion graphql/e2e/common/error_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -348,4 +348,18 @@
{ }
errors:
[ { "message": "Out of range value '2147483648', for type `Int`",
"locations": [ { "line": 2, "column": 53 } ] } ]
"locations": [ { "line": 2, "column": 53 } ] } ]

- name: "Error when multiple filter functions are used"
gqlrequest: |
query {
queryBook(filter:{bookId: {eq:2 le:2}})
{
bookId
}
}
gqlvariables: |
{ }
errors:
[ { "message": "Int64Filter filter expects only one filter function, got: 2",
"locations": [ { "line": 2, "column": 29 } ] } ]
1 change: 1 addition & 0 deletions graphql/schema/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func init() {
validator.AddRule("Check arguments of cascade directive", directiveArgumentsCheck)
validator.AddRule("Check range for Int type", intRangeCheck)
validator.AddRule("Input Coercion to List", listInputCoercion)
validator.AddRule("Check filter functions", filterCheck)

}

Expand Down
17 changes: 17 additions & 0 deletions graphql/schema/validation_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@ package schema

import (
"errors"
"github.com/dgraph-io/dgraph/x"
"strconv"

"github.com/dgraph-io/gqlparser/v2/ast"
"github.com/dgraph-io/gqlparser/v2/validator"
)

var allowedFilters = []string{"StringHashFilter", "StringExactFilter", "StringFullTextFilter",
"StringRegExpFilter", "StringTermFilter", "DateTimeFilter", "FloatFilter", "Int64Filter", "IntFilter", "PointGeoFilter",
"ContainsFilter", "IntersectsFilter", "PolygonGeoFilter"}

func listInputCoercion(observers *validator.Events, addError validator.AddErrFunc) {
observers.OnValue(func(walker *validator.Walker, value *ast.Value) {
if value.Definition == nil || value.ExpectedType == nil {
Expand Down Expand Up @@ -51,6 +56,18 @@ func listInputCoercion(observers *validator.Events, addError validator.AddErrFun
})
}

func filterCheck(observers *validator.Events, addError validator.AddErrFunc) {
observers.OnValue(func(walker *validator.Walker, value *ast.Value) {
if value.Definition == nil {
return
}

if x.HasString(allowedFilters, value.Definition.Name) && len(value.Children) > 1 {
addError(validator.Message("%s filter expects only one filter function, got: %d", value.Definition.Name, len(value.Children)), validator.At(value.Position))
}
})
}

func variableTypeCheck(observers *validator.Events, addError validator.AddErrFunc) {
observers.OnValue(func(walker *validator.Walker, value *ast.Value) {
if value.Definition == nil || value.ExpectedType == nil ||
Expand Down

0 comments on commit de2f4ab

Please sign in to comment.