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.

(cherry picked from commit de2f4ab)
  • Loading branch information
JatinDev543 committed Feb 2, 2021
1 parent c86ca46 commit cc35e41
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 All @@ -46,6 +51,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 cc35e41

Please sign in to comment.