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): fix bug in custom resolver, now body need not have all the fields. #6054

Merged
merged 14 commits into from
Jul 29, 2020
43 changes: 43 additions & 0 deletions graphql/e2e/custom_logic/custom_logic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,49 @@ func TestCustomFieldsShouldForwardHeaders(t *testing.T) {
require.Nilf(t, result.Errors, "%+v", result.Errors)
}

func TestCustomFieldsShouldSkipNonEmptyVariable(t *testing.T) {
schema := `
type User {
id: ID!
address:String
name: String
@custom(
http: {
url: "http://mock:8888/userName"
method: "GET"
body: "{uid: $id,address:$address}"
mode: SINGLE,
secretHeaders: ["GITHUB-API-TOKEN"]
}
)
age: Int! @search
}

# Dgraph.Secret GITHUB-API-TOKEN "some-api-token"
`

updateSchemaRequireNoGQLErrors(t, schema)
time.Sleep(2 * time.Second)

users := addUsers(t)

queryUser := `
query ($id: [ID!]){
queryUser(filter: {id: $id}, order: {asc: age}) {
name
age
}
}`
params := &common.GraphQLParams{
Query: queryUser,
Variables: map[string]interface{}{"id": []interface{}{
users[0].ID, users[1].ID, users[2].ID}},
}

result := params.ExecuteAsPost(t, alphaURL)
require.Nilf(t, result.Errors, "%+v", result.Errors)
}

func TestCustomFieldsShouldBeResolved(t *testing.T) {
// This test adds data, modifies the schema multiple times and fetches the data.
// It has the following modes.
Expand Down
1 change: 1 addition & 0 deletions graphql/resolve/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1793,6 +1793,7 @@ func (hr *httpResolver) rewriteAndExecute(ctx context.Context, field schema.Fiel
}
body = string(b)
}

b, err := makeRequest(hr.Client, hrc.Method, hrc.URL, body, hrc.ForwardHeaders)
if err != nil {
return emptyResult(externalRequestError(err, field))
Expand Down
27 changes: 16 additions & 11 deletions graphql/schema/wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1841,22 +1841,18 @@ func parseBodyTemplate(body string) (*interface{}, map[string]bool, error) {
return &m, requiredFields, nil
}

func getVar(key string, variables map[string]interface{}) (interface{}, error) {
func getVar(key string, variables map[string]interface{}) (interface{}, error, bool) {
if !strings.HasPrefix(key, "$") {
return nil, errors.Errorf("expected a variable to start with $. Found: %s", key)
return nil, errors.Errorf("expected a variable to start with $. Found: %s", key), true
}
val, ok := variables[key[1:]]
if !ok {
return nil, errors.Errorf("couldn't find variable: %s in variables map", key)
}

return val, nil
return val, nil, ok
}

func substituteSingleVarInBody(key string, valPtr *interface{},
variables map[string]interface{}) error {
// Look it up in the map and replace.
val, err := getVar(key, variables)
val, err, _ := getVar(key, variables)
if err != nil {
return err
}
Expand All @@ -1868,11 +1864,15 @@ func substituteVarInMapInBody(object, variables map[string]interface{}) error {
for k, v := range object {
switch val := v.(type) {
case string:
vval, err := getVar(val, variables)
vval, err, ok := getVar(val, variables)
if err != nil {
return err
}
object[k] = vval
if ok {
object[k] = vval
} else {
delete(object, k)
}
case map[string]interface{}:
if err := substituteVarInMapInBody(val, variables); err != nil {
return err
Expand All @@ -1892,10 +1892,15 @@ func substituteVarInSliceInBody(slice []interface{}, variables map[string]interf
for k, v := range slice {
switch val := v.(type) {
case string:
vval, err := getVar(val, variables)
vval, err, _ := getVar(val, variables)
if err != nil {
return err
}
//if ok {
// slice[k] = vval
//} else {
// delete(object, k)
//}
slice[k] = vval
case map[string]interface{}:
if err := substituteVarInMapInBody(val, variables); err != nil {
Expand Down
11 changes: 9 additions & 2 deletions graphql/schema/wrappers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,11 +424,18 @@ func TestSubstituteVarsInBody(t *testing.T) {
nil,
},
{
"variable not found error",
"Skip one missing variable in the HTTP body",
map[string]interface{}{"postID": "0x9"},
map[string]interface{}{"author": "$id", "post": map[string]interface{}{"id": "$postID"}},
map[string]interface{}{"post": map[string]interface{}{"id": "0x9"}},
nil,
},
{
"Skip all missing variables in the HTTP body",
map[string]interface{}{},
map[string]interface{}{"author": "$id", "post": map[string]interface{}{"id": "$postID"}},
map[string]interface{}{"post": map[string]interface{}{}},
nil,
errors.New("couldn't find variable: $id in variables map"),
},
}

Expand Down