Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GraphQL): This PR extend int64 range to 64-bit numeric values and adds input coercing and validation for integers. #6275

Merged
merged 24 commits into from
Sep 3, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d9892f8
added validation and test for out of range
JatinDev543 Aug 26, 2020
31d12ac
modified test
JatinDev543 Aug 26, 2020
9489906
add int64 validation and added tests
JatinDev543 Aug 26, 2020
96c7b9f
fixe formatting
JatinDev543 Aug 26, 2020
a33e567
fixed test by adding schema
JatinDev543 Aug 27, 2020
cbf0c56
Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/graph…
JatinDev543 Aug 27, 2020
7085ca8
clean up code
JatinDev543 Aug 27, 2020
e23e1ae
fixed test
JatinDev543 Aug 27, 2020
820309a
fixed antipattern error
JatinDev543 Aug 27, 2020
b8826dd
Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/graph…
JatinDev543 Aug 27, 2020
23bffbd
moved predicates in json output file a bit
JatinDev543 Aug 27, 2020
acc700d
corrected tests and added check
JatinDev543 Aug 28, 2020
6ef3ef8
full 64-bit range support for Int64
abhimanyusinghgaur Aug 28, 2020
c324d28
added tests,modified validation
JatinDev543 Aug 28, 2020
bf88eb3
fixed tests and modified code
JatinDev543 Sep 1, 2020
d32a70a
fixed output coercing for resolver tests
JatinDev543 Sep 2, 2020
9962657
resolved abhimanyu's comments.
JatinDev543 Sep 2, 2020
3d2de0d
changed formatting a bit
JatinDev543 Sep 2, 2020
e8fd9ab
Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/graph…
JatinDev543 Sep 2, 2020
d1bd6ff
added missed test
JatinDev543 Sep 3, 2020
d8ab759
fuxed schemagen error
JatinDev543 Sep 3, 2020
5de04f1
unexported valueKindToString functions
JatinDev543 Sep 3, 2020
93742d3
Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/graph…
JatinDev543 Sep 3, 2020
13c3275
changed documentation message for int64
JatinDev543 Sep 3, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ func RunAll(t *testing.T) {
t.Run("alias works for mutations", mutationsWithAlias)
t.Run("three level deep", threeLevelDeepMutation)
t.Run("update mutation without set & remove", updateMutationWithoutSetRemove)
t.Run("Input coercing for int64 type", mutationInt64InputCoercing)
t.Run("Check cascade with mutation without ID field", checkCascadeWithMutationWithoutIDField)

// error tests
Expand Down
60 changes: 59 additions & 1 deletion graphql/e2e/common/error_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,25 @@
errors:
[ { "message": "Field `title` is not present in type `Author`. You can only use fields which are in type `Author`",
} ]
-
name: "Out of range error for int32 type"
gqlrequest: |
mutation {
addPost(input:[{title:"Dgraph",author:{name:"Bob"},numLikes:2147483648}]){
post{
title
numLikes
author{
name
}
}
}
}
gqlvariables:
{ }
errors:
[ { "message": "Out of range value '2147483648', for type `Int`",
"locations": [ { "line": 2, "column": 63 } ] } ]

-
name: "@cascade only accepts numUids or given type name as arguments for add or update payload "
Expand All @@ -129,4 +148,43 @@
{ }
errors:
[ { "message": "Field `name` is not present in type `AddAuthorPayload`. You can only use fields which are in type `AddAuthorPayload`",
} ]
} ]
-
name: "value (2^54) out of range for for int64 type"
gqlrequest: |
mutation {
addPost(input:[{title:"Dgraph",author:{name:"Bob"},numViews:18014399000000000}]){
post{
title
numLikes
author{
name
}
}
}
}
gqlvariables:
{ }
errors:
[ { "message": "Out of range value '18014399000000000', for type `Int64`",
"locations": [ { "line": 2, "column": 63 } ] } ]

-
name: "value (-(2^53)) out of range , boundary case for int64"
gqlrequest: |
mutation {
addPost(input:[{title:"Dgraph",author:{name:"Bob"},numViews:9007199300000000}]){
post{
title
numViews
author{
name
}
}
}
}
gqlvariables:
{ }
errors:
[ { "message": "Out of range value '9007199300000000', for type `Int64`",
"locations": [ { "line": 2, "column": 63 } ] } ]
40 changes: 35 additions & 5 deletions graphql/e2e/common/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -2784,10 +2784,8 @@ func deleteGqlType(
require.Equal(t, "Deleted", deleteType["msg"], "while deleting %s (filter: %v)",
typeName, filter)
}
} else {
if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" {
t.Errorf("errors mismatch (-want +got):\n%s", diff)
}
} else if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" {
t.Errorf("errors mismatch (-want +got):\n%s", diff)
}
}

Expand Down Expand Up @@ -3648,4 +3646,36 @@ func checkCascadeWithMutationWithoutIDField(t *testing.T) {

filter := map[string]interface{}{"xcode": map[string]interface{}{"eq": "S2"}}
deleteState(t, filter, 1, nil)
}
}

func mutationInt64InputCoercing(t *testing.T) {
addPost1Params := &GraphQLParams{
Query: `mutation {
addpost1(input: [{title: "Dgraph", numLikes:9.223372036854775e+18 ,numViews:"9223372036854775000",numComments:9223372036854775000.0}]) {
post1 {
title
numViews
numLikes
numComments
}
}
}`,
}

gqlResponse := addPost1Params.ExecuteAsPost(t, graphqlURL)
RequireNoGQLErrors(t, gqlResponse)

addPost1Expected := `{
"addpost1": {
"post1": [{
"title": "Dgraph",
"numViews": 9223372036854775000,
"numLikes": 9223372036854775000,
"numComments": 9223372036854775000
}]
}
}`
testutil.CompareJSON(t, addPost1Expected, string(gqlResponse.Data))
filter := map[string]interface{}{"title": map[string]interface{}{"eq": "Dgraph"}}
deleteGqlType(t, "post1", filter, 1, nil)
}
7 changes: 7 additions & 0 deletions graphql/e2e/directives/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,11 @@ type Post1 {
type Comment1 {
id: String! @id
replies: [Comment1]
}
type post1{
id:ID
title:String! @id @search(by: [regexp])
numLikes:Int64
numViews:Int64
numComments:Int64
}
36 changes: 36 additions & 0 deletions graphql/e2e/directives/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,25 @@
],
"upsert": true
},
{
"predicate": "post1.numComments",
"type": "int"
},
{
"predicate": "post1.title",
"type": "string",
"index": true,
"tokenizer": ["trigram", "hash"],
"upsert": true
},
{
"predicate": "post1.numViews",
"type": "int"
},
{
"predicate": "post1.numLikes",
"type": "int"
},
{
"predicate": "State.capital",
"type": "string"
Expand Down Expand Up @@ -449,6 +468,23 @@
],
"name": "Post1"
},
{
"fields": [
{
"name": "post1.numComments"
},
{
"name": "post1.title"
},
{
"name": "post1.numViews"
},
{
"name": "post1.numLikes"
}
],
"name": "post1"
},
{
"fields": [
{
Expand Down
8 changes: 8 additions & 0 deletions graphql/e2e/normal/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,12 @@ type Post1 {
type Comment1 {
id: String! @id
replies: [Comment1]
}

type post1{
id:ID
title:String! @id @search(by: [regexp])
numLikes:Int64
numViews:Int64
numComments:Int64
}
36 changes: 36 additions & 0 deletions graphql/e2e/normal/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,25 @@
],
"upsert": true
},
{
"predicate": "post1.numComments",
"type": "int"
},
{
"predicate": "post1.title",
"type": "string",
"index": true,
"tokenizer": ["trigram", "hash"],
"upsert": true
},
{
"predicate": "post1.numViews",
"type": "int"
},
{
"predicate": "post1.numLikes",
"type": "int"
},
{
"predicate": "Starship.length",
"type": "float"
Expand Down Expand Up @@ -537,6 +556,23 @@
],
"name": "Post1"
},
{
"fields": [
{
"name": "post1.numComments"
},
{
"name": "post1.title"
},
{
"name": "post1.numViews"
},
{
"name": "post1.numLikes"
}
],
"name": "post1"
},
{
"fields": [
{
Expand Down
1 change: 1 addition & 0 deletions graphql/schema/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func init() {
validator.AddRule("Check variable type is correct", variableTypeCheck)
validator.AddRule("Check for list type value", listTypeCheck)
validator.AddRule("Check arguments of cascade directive", directiveArgumentsCheck)
validator.AddRule("Check range for Int type", intRangeCheck)

}

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

import (
"errors"
"math"
"strconv"

"github.com/vektah/gqlparser/v2/ast"
"github.com/vektah/gqlparser/v2/validator"
)
Expand Down Expand Up @@ -89,3 +93,34 @@ func directiveArgumentsCheck(observers *validator.Events, addError validator.Add

})
}

func intRangeCheck(observers *validator.Events, addError validator.AddErrFunc) {
observers.OnValue(func(walker *validator.Walker, value *ast.Value) {
if value.Definition == nil || value.ExpectedType == nil || value.Kind != ast.IntValue{
return
}

var err error
var val int64
switch value.Definition.Name {
case "Int":
_, err = strconv.ParseInt(value.Raw, 10, 32)
case "Int64":
val, err = strconv.ParseInt(value.Raw, 10, 54)
default:
return
}

//Range of Json numbers is [-(2**53)+1, (2**53)-1] while of 54 bit integers is [-(2**53), (2**53)-1]
if err != nil {
if float64(val) == (-1)*math.Pow(2, 53) || errors.Is(err, strconv.ErrRange) {
addError(validator.Message("Out of range value '%s', for type `%s`",
value.Raw, value.Definition.Name), validator.At(value.Position))
return
}
addError(validator.Message("%s", err), validator.At(value.Position))
return
}

})
}