diff --git a/graphql/e2e/custom_logic/cmd/graphqlresponse.yaml b/graphql/e2e/custom_logic/cmd/graphqlresponse.yaml index f40b21ad452..10d45e768e9 100644 --- a/graphql/e2e/custom_logic/cmd/graphqlresponse.yaml +++ b/graphql/e2e/custom_logic/cmd/graphqlresponse.yaml @@ -89,6 +89,34 @@ country(code: ID!): Country! } +- name: "argsonfields" + description: "Test case to check args on fields can be passed by Dgraph" + schema: | + type Country { + code(size: Int!): String + name: String + } + + type Query { + country(code: ID!): Country! + } + request: | + query($id: ID!) { country(code: $id) { + code(size: 100) + name + }} + variables: | + {"id":"BI"} + response: | + { + "data":{ + "country":{ + "name":"Burundi", + "code":"BI" + } + } + } + - name: "validcountrywitherror" description: "Test case to validate dgraph can handle both valid data and error" schema: | diff --git a/graphql/e2e/custom_logic/cmd/main.go b/graphql/e2e/custom_logic/cmd/main.go index e4d935d8afb..36ac72590c1 100644 --- a/graphql/e2e/custom_logic/cmd/main.go +++ b/graphql/e2e/custom_logic/cmd/main.go @@ -1154,6 +1154,7 @@ func main() { // for queries vsch := graphql.MustParseSchema(graphqlResponses["validcountry"].Schema, &query{}) http.Handle("/validcountry", &relay.Handler{Schema: vsch}) + http.HandleFunc("/argsonfields", commonGraphqlHandler("argsonfields")) http.HandleFunc("/validcountrywitherror", commonGraphqlHandler("validcountrywitherror")) http.HandleFunc("/graphqlerr", commonGraphqlHandler("graphqlerr")) http.Handle("/validcountries", &relay.Handler{ diff --git a/graphql/e2e/custom_logic/custom_logic_test.go b/graphql/e2e/custom_logic/custom_logic_test.go index b76ac03dfef..ab94bd2d429 100644 --- a/graphql/e2e/custom_logic/custom_logic_test.go +++ b/graphql/e2e/custom_logic/custom_logic_test.go @@ -49,7 +49,7 @@ const ( director: [MovieDirector] } type Country @remote { - code: String + code(size: Int): String name: String states: [State] std: Int @@ -1209,6 +1209,37 @@ func TestCustomLogicGraphql(t *testing.T) { require.JSONEq(t, string(result.Data), `{"getCountry1":{"code":"BI","name":"Burundi"}}`) } +func TestCustomLogicGraphqlWithArgumentsOnFields(t *testing.T) { + schema := customTypes + ` + type Query { + getCountry2(id: ID!): Country! + @custom( + http: { + url: "http://mock:8888/argsonfields" + method: "POST" + forwardHeaders: ["Content-Type"] + graphql: "query($id: ID!) { country(code: $id) }" + } + ) + }` + updateSchemaRequireNoGQLErrors(t, schema) + time.Sleep(2 * time.Second) + query := ` + query { + getCountry2(id: "BI"){ + code(size: 100) + name + } + }` + params := &common.GraphQLParams{ + Query: query, + } + + result := params.ExecuteAsPost(t, alphaURL) + common.RequireNoGQLErrors(t, result) + require.JSONEq(t, string(result.Data), `{"getCountry2":{"code":"BI","name":"Burundi"}}`) +} + func TestCustomLogicGraphqlWithError(t *testing.T) { schema := customTypes + ` type Query { diff --git a/graphql/schema/custom_http_config_test.yaml b/graphql/schema/custom_http_config_test.yaml index b70975d923a..67b6a1fc085 100644 --- a/graphql/schema/custom_http_config_test.yaml +++ b/graphql/schema/custom_http_config_test.yaml @@ -46,11 +46,85 @@ { "id": "0x1" } - - name: "custom mutation" + name: "custom query with arguments on fields" + type: "query" + gqlschema: | + input ExactFilter { + eq: String + } + + input MyFilter { + ids: ID + name: ExactFilter + } + + type Country @remote { + code(first: Int!, filter: MyFilter): String + name: String + } + + type Query { + getCountry1(id: ID!): Country! @custom(http: { + url: "http://google.com/validcountry", + method: "POST", + forwardHeaders: ["Content-Type"], + graphql: "query($id: ID!) { country(code: $id) }", + skipIntrospection: true + }) + } + gqlquery: | + query { + getCountry1(id: "0x1") { + name + code(first: 10, filter: {ids: "0x123", name: { eq: "github" }}) + } + } + remoteschema: | + input ExactFilter { + eq: String + } + + input MyFilter { + ids: ID + name: ExactFilter + } + + type Country @remote { + code(first: Int!, filter: MyFilter): String + name: String + } + + type Query { + country(code: ID!): Country! @custom(http: { + url: "http://google.com/validcountry", + method: "POST", + graphql: "query($code: ID!) { country(code: $code) }", + skipIntrospection: true + }) + } + remotequery: |- + query($id: ID!) { country(code: $id) { + name + code(first: 10, filter: {ids:"0x123",name:{eq:"github"}}) + }} + remotevariables: |- + { "id": "0x1" } + +- + name: "custom mutation with arguments on field" type: "mutation" gqlschema: | + input ExactFilter { + eq: String + } + + input MyFilter { + ids: ID + name: ExactFilter + } + type Country @remote { - code: String + code(get: Int!, choose: MyFilter): String name: String states: [State] std: Int @@ -84,7 +158,7 @@ gqlquery: | mutation addCountry1($input: CountryInput!) { addCountry1(input: $input) { - code + code(get: 10, choose: {ids: "0x123", name: { eq: "github" }}) name states { code @@ -110,8 +184,17 @@ } } remoteschema: | + input ExactFilter { + eq: String + } + + input MyFilter { + ids: ID + name: ExactFilter + } + type Country { - code: String + code(get: Int!, choose: MyFilter): String name: String states: [State] std: Int @@ -144,7 +227,7 @@ } remotequery: |- mutation($input: CountryInput!) { setCountry(country: $input) { - code + code(get: 10, choose: {ids:"0x123",name:{eq:"github"}}) name states{ code diff --git a/graphql/schema/gqlschema.go b/graphql/schema/gqlschema.go index 4d13d5c9c34..185f5a2421d 100644 --- a/graphql/schema/gqlschema.go +++ b/graphql/schema/gqlschema.go @@ -326,13 +326,13 @@ func copyAstFieldDef(src *ast.FieldDefinition) *ast.FieldDefinition { var dirs ast.DirectiveList dirs = append(dirs, src.Directives...) - // Lets leave out copying the arguments as types in input schemas are not supposed to contain - // them. We add arguments for filters and order statements later. + // We add arguments for filters and order statements later. dst := &ast.FieldDefinition{ Name: src.Name, DefaultValue: src.DefaultValue, Type: src.Type, Directives: dirs, + Arguments: src.Arguments, Position: src.Position, } return dst @@ -1240,6 +1240,7 @@ func createField(schema *ast.Schema, fld *ast.FieldDefinition) *ast.FieldDefinit newFldType := *fld.Type newFld.Type = &newFldType newFld.Directives = nil + newFld.Arguments = nil return &newFld } diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml index 492ffb75a77..e980514eaa2 100644 --- a/graphql/schema/gqlschema_test.yml +++ b/graphql/schema/gqlschema_test.yml @@ -62,16 +62,6 @@ invalid_schemas: {"message": "Type A; Field f: Nested lists are invalid.", "locations": [{"line":2, "column": 3}]} ] - - - name: "There shoudnt be arguments on any field" - input: | - type T { - f(a: Int): String - } - errlist: [ - {"message": "Type T; Field f: You can't give arguments to fields.", "locations": [{"line": 2, "column": 3}]} - ] - - name: "Enum indexes clash trigram and regexp" input: | @@ -1903,6 +1893,15 @@ invalid_schemas: "locations":[{"line":1, "column":11}]}, ] + + - name: "There shoudnt be any reserved arguments on any field" + input: | + type T { + f(first: Int): String + } + errlist: [ + {"message": "Type T; Field f: can't have first as an argument because it is a reserved argument.", "locations": [{"line": 2, "column": 3}]}] + - name: "remote type with @custom directives on fields shouldn't be allowed." description: "Remote types are not resolved further currently, hence they shouldn't have fields with @custom directive on them." diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index fab311ed575..6bf1e62a61d 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -694,12 +694,22 @@ func fieldArgumentCheck(typ *ast.Definition, field *ast.FieldDefinition) *gqlerr if isQueryOrMutationType(typ) { return nil } - if field.Arguments != nil { - return gqlerror.ErrorPosf( - field.Position, - "Type %s; Field %s: You can't give arguments to fields.", - typ.Name, field.Name, - ) + // We don't need to verify the argument names for fields which are part of a remote type as + // we don't add any of our own arguments to them. + remote := typ.Directives.ForName(remoteDirective) + if remote != nil { + return nil + } + for _, arg := range field.Arguments { + if isReservedArgument(arg.Name) { + return gqlerror.ErrorPosf( + field.Position, + "Type %s; Field %s: can't have %s as an argument because it is a reserved "+ + "argument.", + typ.Name, field.Name, arg.Name, + ) + + } } return nil } @@ -1649,6 +1659,14 @@ func isScalar(s string) bool { return ok } +func isReservedArgument(name string) bool { + switch name { + case "first", "offset", "filter", "order": + return true + } + return false +} + func isReservedKeyWord(name string) bool { if isScalar(name) || isQueryOrMutation(name) || name == "uid" { return true diff --git a/graphql/schema/testdata/schemagen/input/type-with-arguments-on-field.graphql b/graphql/schema/testdata/schemagen/input/type-with-arguments-on-field.graphql new file mode 100644 index 00000000000..d544aa76608 --- /dev/null +++ b/graphql/schema/testdata/schemagen/input/type-with-arguments-on-field.graphql @@ -0,0 +1,10 @@ +interface Abstract { + id: ID! + name(random: Int!, size: String): String! +} + +type Message implements Abstract { + content(pick: Int!, name: String): String! + author: String + datePosted: DateTime +} diff --git a/graphql/schema/testdata/schemagen/output/type-with-arguments-on-field.graphql b/graphql/schema/testdata/schemagen/output/type-with-arguments-on-field.graphql new file mode 100644 index 00000000000..cc84536dd58 --- /dev/null +++ b/graphql/schema/testdata/schemagen/output/type-with-arguments-on-field.graphql @@ -0,0 +1,279 @@ +####################### +# Input Schema +####################### + +interface Abstract { + id: ID! + name(random: Int!, size: String): String! +} + +type Message implements Abstract { + id: ID! + name(random: Int!, size: String): String! + content(pick: Int!, name: String): String! + author: String + datePosted: DateTime +} + +####################### +# Extended Definitions +####################### + +scalar DateTime + +enum DgraphIndex { + int + float + bool + hash + exact + term + fulltext + trigram + regexp + year + month + day + hour +} + +input AuthRule { + and: [AuthRule] + or: [AuthRule] + not: AuthRule + rule: String +} + +enum HTTPMethod { + GET + POST + PUT + PATCH + DELETE +} + +enum Mode { + BATCH + SINGLE +} + +input CustomHTTP { + url: String! + method: HTTPMethod! + body: String + graphql: String + mode: Mode + forwardHeaders: [String!] + secretHeaders: [String!] + skipIntrospection: Boolean +} + +directive @hasInverse(field: String!) on FIELD_DEFINITION +directive @search(by: [DgraphIndex!]) on FIELD_DEFINITION +directive @dgraph(type: String, pred: String) on OBJECT | INTERFACE | FIELD_DEFINITION +directive @id on FIELD_DEFINITION +directive @secret(field: String!, pred: String) on OBJECT | INTERFACE +directive @auth( + query: AuthRule, + add: AuthRule, + update: AuthRule, + delete:AuthRule) on OBJECT +directive @custom(http: CustomHTTP) on FIELD_DEFINITION +directive @remote on OBJECT | INTERFACE +directive @cascade on FIELD + +input IntFilter { + eq: Int + le: Int + lt: Int + ge: Int + gt: Int +} + +input FloatFilter { + eq: Float + le: Float + lt: Float + ge: Float + gt: Float +} + +input DateTimeFilter { + eq: DateTime + le: DateTime + lt: DateTime + ge: DateTime + gt: DateTime +} + +input StringTermFilter { + allofterms: String + anyofterms: String +} + +input StringRegExpFilter { + regexp: String +} + +input StringFullTextFilter { + alloftext: String + anyoftext: String +} + +input StringExactFilter { + eq: String + le: String + lt: String + ge: String + gt: String +} + +input StringHashFilter { + eq: String +} + +####################### +# Generated Types +####################### + +type AddMessagePayload { + message(filter: MessageFilter, order: MessageOrder, first: Int, offset: Int): [Message] + numUids: Int +} + +type DeleteAbstractPayload { + msg: String + numUids: Int +} + +type DeleteMessagePayload { + msg: String + numUids: Int +} + +type UpdateAbstractPayload { + abstract(filter: AbstractFilter, order: AbstractOrder, first: Int, offset: Int): [Abstract] + numUids: Int +} + +type UpdateMessagePayload { + message(filter: MessageFilter, order: MessageOrder, first: Int, offset: Int): [Message] + numUids: Int +} + +####################### +# Generated Enums +####################### + +enum AbstractOrderable { + name +} + +enum MessageOrderable { + name + content + author + datePosted +} + +####################### +# Generated Inputs +####################### + +input AbstractFilter { + id: [ID!] + not: AbstractFilter +} + +input AbstractOrder { + asc: AbstractOrderable + desc: AbstractOrderable + then: AbstractOrder +} + +input AbstractPatch { + name: String +} + +input AbstractRef { + id: ID! +} + +input AddMessageInput { + name: String! + content: String! + author: String + datePosted: DateTime +} + +input MessageFilter { + id: [ID!] + not: MessageFilter +} + +input MessageOrder { + asc: MessageOrderable + desc: MessageOrderable + then: MessageOrder +} + +input MessagePatch { + name: String + content: String + author: String + datePosted: DateTime +} + +input MessageRef { + id: ID + name: String + content: String + author: String + datePosted: DateTime +} + +input UpdateAbstractInput { + filter: AbstractFilter! + set: AbstractPatch + remove: AbstractPatch +} + +input UpdateMessageInput { + filter: MessageFilter! + set: MessagePatch + remove: MessagePatch +} + +####################### +# Generated Query +####################### + +type Query { + getAbstract(id: ID!): Abstract + queryAbstract(filter: AbstractFilter, order: AbstractOrder, first: Int, offset: Int): [Abstract] + getMessage(id: ID!): Message + queryMessage(filter: MessageFilter, order: MessageOrder, first: Int, offset: Int): [Message] +} + +####################### +# Generated Mutations +####################### + +type Mutation { + updateAbstract(input: UpdateAbstractInput!): UpdateAbstractPayload + deleteAbstract(filter: AbstractFilter!): DeleteAbstractPayload + addMessage(input: [AddMessageInput!]!): AddMessagePayload + updateMessage(input: UpdateMessageInput!): UpdateMessagePayload + deleteMessage(filter: MessageFilter!): DeleteMessagePayload +} + +####################### +# Generated Subscriptions +####################### + +type Subscription { + getAbstract(id: ID!): Abstract + queryAbstract(filter: AbstractFilter, order: AbstractOrder, first: Int, offset: Int): [Abstract] + getMessage(id: ID!): Message + queryMessage(filter: MessageFilter, order: MessageOrder, first: Int, offset: Int): [Message] +} diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go index 96366c4f542..8e86543dac3 100644 --- a/graphql/schema/wrappers.go +++ b/graphql/schema/wrappers.go @@ -1959,6 +1959,19 @@ func buildGraphqlRequestFields(writer *bytes.Buffer, field *ast.Field) { castedField := field.SelectionSet[i].(*ast.Field) writer.WriteString(castedField.Name) + if len(castedField.Arguments) > 0 { + writer.WriteString("(") + for idx, arg := range castedField.Arguments { + if idx != 0 { + writer.WriteString(", ") + } + writer.WriteString(arg.Name) + writer.WriteString(": ") + writer.WriteString(arg.Value.String()) + } + writer.WriteString(")") + } + if len(castedField.SelectionSet) > 0 { // recursively add fields. buildGraphqlRequestFields(writer, castedField)