From 83c913fe7e8e41099e8fc44918f888abe6ba8343 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 18 Apr 2016 16:42:42 +0800 Subject: [PATCH 01/32] Add test for parseLiteral on ComplexScalar Commit: b3a512787618cfbed20c9717daf2d69444bf7f33 [b3a5127] Parents: 5ea2ff1e17 Author: Tim Griesser Date: 9 March 2016 at 10:02:07 A --- variables_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/variables_test.go b/variables_test.go index adfe823c..99c46738 100644 --- a/variables_test.go +++ b/variables_test.go @@ -251,6 +251,33 @@ func TestVariables_ObjectsAndNullability_UsingInlineStructs_DoesNotUseIncorrectV t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } } +func TestVariables_ObjectsAndNullability_UsingInlineStructs_ProperlyRunsParseLiteralOnComplexScalarTypes(t *testing.T) { + doc := ` + { + fieldWithObjectInput(input: {a: "foo", d: "SerializedValue"}) + } + ` + expected := &graphql.Result{ + Data: map[string]interface{}{ + "fieldWithObjectInput": `{"a":"foo","d":"DeserializedValue"}`, + }, + } + // parse query + ast := testutil.TestParse(t, doc) + + // execute + ep := graphql.ExecuteParams{ + Schema: variablesTestSchema, + AST: ast, + } + result := testutil.TestExecute(t, ep) + if len(result.Errors) > 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + } +} func testVariables_ObjectsAndNullability_UsingVariables_GetAST(t *testing.T) *ast.Document { doc := ` From 0e4b18cab93504383dcb5b2475cb950ddd3174c6 Mon Sep 17 00:00:00 2001 From: Kevin Mulvey Date: Fri, 13 May 2016 09:10:22 -0400 Subject: [PATCH 02/32] correct misspellings, gofmt --- abstract_test.go | 4 +- definition_test.go | 46 +++++------ directives.go | 4 +- enum_type_test.go | 12 +-- executor.go | 8 +- executor_test.go | 10 +-- graphql_test.go | 4 +- introspection.go | 2 +- introspection_test.go | 4 +- language/lexer/lexer_test.go | 122 +++++++++++++-------------- language/visitor/visitor_test.go | 4 +- lists_test.go | 32 ++++---- mutations_test.go | 8 +- nonnull_test.go | 136 +++++++++++++++---------------- rules.go | 86 +++++++++---------- schema.go | 2 +- validator.go | 2 +- values.go | 4 +- variables_test.go | 48 +++++------ 19 files changed, 269 insertions(+), 269 deletions(-) diff --git a/abstract_test.go b/abstract_test.go index 37f2eb3d..bd983db3 100644 --- a/abstract_test.go +++ b/abstract_test.go @@ -405,7 +405,7 @@ func TestResolveTypeOnInterfaceYieldsUsefulError(t *testing.T) { }, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Runtime Object type "Human" is not a possible type for "Pet".`, Locations: []location.SourceLocation{}, }, @@ -523,7 +523,7 @@ func TestResolveTypeOnUnionYieldsUsefulError(t *testing.T) { }, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Runtime Object type "Human" is not a possible type for "Pet".`, Locations: []location.SourceLocation{}, }, diff --git a/definition_test.go b/definition_test.go index 6664feab..419492e3 100644 --- a/definition_test.go +++ b/definition_test.go @@ -366,17 +366,17 @@ func TestTypeSystem_DefinitionExample_StringifiesSimpleTypes(t *testing.T) { expected string } tests := []Test{ - Test{graphql.Int, "Int"}, - Test{blogArticle, "Article"}, - Test{interfaceType, "Interface"}, - Test{unionType, "Union"}, - Test{enumType, "Enum"}, - Test{inputObjectType, "InputObject"}, - Test{graphql.NewNonNull(graphql.Int), "Int!"}, - Test{graphql.NewList(graphql.Int), "[Int]"}, - Test{graphql.NewNonNull(graphql.NewList(graphql.Int)), "[Int]!"}, - Test{graphql.NewList(graphql.NewNonNull(graphql.Int)), "[Int!]"}, - Test{graphql.NewList(graphql.NewList(graphql.Int)), "[[Int]]"}, + {graphql.Int, "Int"}, + {blogArticle, "Article"}, + {interfaceType, "Interface"}, + {unionType, "Union"}, + {enumType, "Enum"}, + {inputObjectType, "InputObject"}, + {graphql.NewNonNull(graphql.Int), "Int!"}, + {graphql.NewList(graphql.Int), "[Int]"}, + {graphql.NewNonNull(graphql.NewList(graphql.Int)), "[Int]!"}, + {graphql.NewList(graphql.NewNonNull(graphql.Int)), "[Int!]"}, + {graphql.NewList(graphql.NewList(graphql.Int)), "[[Int]]"}, } for _, test := range tests { ttypeStr := fmt.Sprintf("%v", test.ttype) @@ -392,12 +392,12 @@ func TestTypeSystem_DefinitionExample_IdentifiesInputTypes(t *testing.T) { expected bool } tests := []Test{ - Test{graphql.Int, true}, - Test{objectType, false}, - Test{interfaceType, false}, - Test{unionType, false}, - Test{enumType, true}, - Test{inputObjectType, true}, + {graphql.Int, true}, + {objectType, false}, + {interfaceType, false}, + {unionType, false}, + {enumType, true}, + {inputObjectType, true}, } for _, test := range tests { ttypeStr := fmt.Sprintf("%v", test.ttype) @@ -419,12 +419,12 @@ func TestTypeSystem_DefinitionExample_IdentifiesOutputTypes(t *testing.T) { expected bool } tests := []Test{ - Test{graphql.Int, true}, - Test{objectType, true}, - Test{interfaceType, true}, - Test{unionType, true}, - Test{enumType, true}, - Test{inputObjectType, false}, + {graphql.Int, true}, + {objectType, true}, + {interfaceType, true}, + {unionType, true}, + {enumType, true}, + {inputObjectType, false}, } for _, test := range tests { ttypeStr := fmt.Sprintf("%v", test.ttype) diff --git a/directives.go b/directives.go index 67a1aa0d..ac37e877 100644 --- a/directives.go +++ b/directives.go @@ -35,7 +35,7 @@ var IncludeDirective *Directive = NewDirective(&Directive{ Description: "Directs the executor to include this field or fragment only when " + "the `if` argument is true.", Args: []*Argument{ - &Argument{ + { PrivateName: "if", Type: NewNonNull(Boolean), PrivateDescription: "Included when true.", @@ -54,7 +54,7 @@ var SkipDirective *Directive = NewDirective(&Directive{ Description: "Directs the executor to skip this field or fragment when the `if` " + "argument is true.", Args: []*Argument{ - &Argument{ + { PrivateName: "if", Type: NewNonNull(Boolean), PrivateDescription: "Skipped when true.", diff --git a/enum_type_test.go b/enum_type_test.go index 7187a686..e784d70f 100644 --- a/enum_type_test.go +++ b/enum_type_test.go @@ -155,7 +155,7 @@ func TestTypeSystem_EnumValues_DoesNotAcceptStringLiterals(t *testing.T) { expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Argument "fromEnum" expected type "Color" but got: "GREEN".`, }, }, @@ -182,7 +182,7 @@ func TestTypeSystem_EnumValues_DoesNotAcceptInternalValueInPlaceOfEnumLiteral(t expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Argument "fromEnum" expected type "Color" but got: 1.`, }, }, @@ -198,7 +198,7 @@ func TestTypeSystem_EnumValues_DoesNotAcceptEnumLiteralInPlaceOfInt(t *testing.T expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Argument "fromInt" expected type "Int" but got: GREEN.`, }, }, @@ -248,7 +248,7 @@ func TestTypeSystem_EnumValues_DoesNotAcceptInternalValueAsEnumVariable(t *testi expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$color" expected value of type "Color!" but got: 2.`, }, }, @@ -266,7 +266,7 @@ func TestTypeSystem_EnumValues_DoesNotAcceptStringVariablesAsEnumInput(t *testin expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$color" of type "String!" used in position expecting type "Color".`, }, }, @@ -284,7 +284,7 @@ func TestTypeSystem_EnumValues_DoesNotAcceptInternalValueVariableAsEnumInput(t * expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$color" of type "Int!" used in position expecting type "Color".`, }, }, diff --git a/executor.go b/executor.go index fb4e14c1..7230be2b 100644 --- a/executor.go +++ b/executor.go @@ -110,7 +110,7 @@ func buildExecutionContext(p BuildExecutionCtxParams) (*ExecutionContext, error) opName := p.OperationName if opName == "" { // get first opName - for k, _ := range operations { + for k := range operations { opName = k break } @@ -397,10 +397,10 @@ func doesFragmentConditionMatch(eCtx *ExecutionContext, fragment ast.Node, ttype if conditionalType == ttype { return true } - if conditionalType.Name() == ttype.Name() { + if conditionalType.Name() == ttype.Name() { return true } - + if conditionalType, ok := conditionalType.(Abstract); ok { return conditionalType.IsPossibleType(ttype) } @@ -739,7 +739,7 @@ func defaultResolveFn(p ResolveParams) (interface{}, error) { } /** - * This method looks up the field on the given type defintion. + * This method looks up the field on the given type definition. * It has special casing for the two introspection fields, __schema * and __typename. __typename is special because it can always be * queried as a field, even in situations where no other fields diff --git a/executor_test.go b/executor_test.go index 7922d8c8..cbcf7709 100644 --- a/executor_test.go +++ b/executor_test.go @@ -465,10 +465,10 @@ func TestNullsOutErrorSubtrees(t *testing.T) { "syncError": nil, } expectedErrors := []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Error getting syncError", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 3, Column: 7, }, }, @@ -619,7 +619,7 @@ func TestThrowsIfNoOperationIsProvidedWithMultipleOperations(t *testing.T) { } expectedErrors := []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Must provide operation name if query contains multiple operations.", Locations: []location.SourceLocation{}, }, @@ -1050,7 +1050,7 @@ func TestFailsWhenAnIsTypeOfCheckIsNotMet(t *testing.T) { }, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Expected value of type "SpecialType" but got: graphql_test.testNotSpecialType.`, Locations: []location.SourceLocation{}, }, @@ -1119,7 +1119,7 @@ func TestFailsToExecuteQueryContainingATypeDefinition(t *testing.T) { expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "GraphQL cannot execute a request containing a ObjectDefinition", Locations: []location.SourceLocation{}, }, diff --git a/graphql_test.go b/graphql_test.go index d7d59105..365d11bd 100644 --- a/graphql_test.go +++ b/graphql_test.go @@ -19,7 +19,7 @@ var Tests = []T{} func init() { Tests = []T{ - T{ + { Query: ` query HeroNameQuery { hero { @@ -36,7 +36,7 @@ func init() { }, }, }, - T{ + { Query: ` query HeroNameAndFriendsQuery { hero { diff --git a/introspection.go b/introspection.go index 81bc61f4..266a145c 100644 --- a/introspection.go +++ b/introspection.go @@ -440,7 +440,7 @@ mutation operations.`, Type: __Type, Description: "Request the type information of a single type.", Args: []*Argument{ - &Argument{ + { PrivateName: "name", Type: NewNonNull(String), }, diff --git a/introspection_test.go b/introspection_test.go index eabcfc62..4304fa86 100644 --- a/introspection_test.go +++ b/introspection_test.go @@ -1211,11 +1211,11 @@ func TestIntrospection_FailsAsExpectedOnThe__TypeRootFieldWithoutAnArg(t *testin ` expected := &graphql.Result{ Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Field "__type" argument "name" of type "String!" ` + `is required but not provided.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 3, Column: 9}, + {Line: 3, Column: 9}, }, }, }, diff --git a/language/lexer/lexer_test.go b/language/lexer/lexer_test.go index 1db38bcf..dc2b3dc6 100644 --- a/language/lexer/lexer_test.go +++ b/language/lexer/lexer_test.go @@ -18,7 +18,7 @@ func createSource(body string) *source.Source { func TestSkipsWhiteSpace(t *testing.T) { tests := []Test{ - Test{ + { Body: ` foo @@ -31,7 +31,7 @@ func TestSkipsWhiteSpace(t *testing.T) { Value: "foo", }, }, - Test{ + { Body: ` #comment foo#comment @@ -43,7 +43,7 @@ func TestSkipsWhiteSpace(t *testing.T) { Value: "foo", }, }, - Test{ + { Body: `,,,foo,,,`, Expected: Token{ Kind: TokenKind[NAME], @@ -82,7 +82,7 @@ func TestErrorsRespectWhitespace(t *testing.T) { func TestLexesStrings(t *testing.T) { tests := []Test{ - Test{ + { Body: "\"simple\"", Expected: Token{ Kind: TokenKind[STRING], @@ -91,7 +91,7 @@ func TestLexesStrings(t *testing.T) { Value: "simple", }, }, - Test{ + { Body: "\" white space \"", Expected: Token{ Kind: TokenKind[STRING], @@ -100,7 +100,7 @@ func TestLexesStrings(t *testing.T) { Value: " white space ", }, }, - Test{ + { Body: "\"quote \\\"\"", Expected: Token{ Kind: TokenKind[STRING], @@ -109,7 +109,7 @@ func TestLexesStrings(t *testing.T) { Value: `quote "`, }, }, - Test{ + { Body: "\"escaped \\n\\r\\b\\t\\f\"", Expected: Token{ Kind: TokenKind[STRING], @@ -118,7 +118,7 @@ func TestLexesStrings(t *testing.T) { Value: "escaped \n\r\b\t\f", }, }, - Test{ + { Body: "\"slashes \\\\ \\/\"", Expected: Token{ Kind: TokenKind[STRING], @@ -127,7 +127,7 @@ func TestLexesStrings(t *testing.T) { Value: "slashes \\ \\/", }, }, - Test{ + { Body: "\"unicode \\u1234\\u5678\\u90AB\\uCDEF\"", Expected: Token{ Kind: TokenKind[STRING], @@ -150,7 +150,7 @@ func TestLexesStrings(t *testing.T) { func TestLexReportsUsefulStringErrors(t *testing.T) { tests := []Test{ - Test{ + { Body: "\"no end quote", Expected: `Syntax Error GraphQL (1:14) Unterminated string. @@ -158,7 +158,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "\"multi\nline\"", Expected: `Syntax Error GraphQL (1:7) Unterminated string. @@ -167,7 +167,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { 2: line" `, }, - Test{ + { Body: "\"multi\rline\"", Expected: `Syntax Error GraphQL (1:7) Unterminated string. @@ -176,7 +176,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { 2: line" `, }, - Test{ + { Body: "\"multi\u2028line\"", Expected: `Syntax Error GraphQL (1:7) Unterminated string. @@ -185,7 +185,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { 2: line" `, }, - Test{ + { Body: "\"multi\u2029line\"", Expected: `Syntax Error GraphQL (1:7) Unterminated string. @@ -194,7 +194,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { 2: line" `, }, - Test{ + { Body: "\"bad \\z esc\"", Expected: `Syntax Error GraphQL (1:7) Bad character escape sequence. @@ -202,7 +202,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "\"bad \\x esc\"", Expected: `Syntax Error GraphQL (1:7) Bad character escape sequence. @@ -210,7 +210,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "\"bad \\u1 esc\"", Expected: `Syntax Error GraphQL (1:7) Bad character escape sequence. @@ -218,7 +218,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "\"bad \\u0XX1 esc\"", Expected: `Syntax Error GraphQL (1:7) Bad character escape sequence. @@ -226,7 +226,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "\"bad \\uXXXX esc\"", Expected: `Syntax Error GraphQL (1:7) Bad character escape sequence. @@ -234,7 +234,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "\"bad \\uFXXX esc\"", Expected: `Syntax Error GraphQL (1:7) Bad character escape sequence. @@ -242,7 +242,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "\"bad \\uXXXF esc\"", Expected: `Syntax Error GraphQL (1:7) Bad character escape sequence. @@ -264,7 +264,7 @@ func TestLexReportsUsefulStringErrors(t *testing.T) { func TestLexesNumbers(t *testing.T) { tests := []Test{ - Test{ + { Body: "4", Expected: Token{ Kind: TokenKind[INT], @@ -273,7 +273,7 @@ func TestLexesNumbers(t *testing.T) { Value: "4", }, }, - Test{ + { Body: "4.123", Expected: Token{ Kind: TokenKind[FLOAT], @@ -282,7 +282,7 @@ func TestLexesNumbers(t *testing.T) { Value: "4.123", }, }, - Test{ + { Body: "-4", Expected: Token{ Kind: TokenKind[INT], @@ -291,7 +291,7 @@ func TestLexesNumbers(t *testing.T) { Value: "-4", }, }, - Test{ + { Body: "9", Expected: Token{ Kind: TokenKind[INT], @@ -300,7 +300,7 @@ func TestLexesNumbers(t *testing.T) { Value: "9", }, }, - Test{ + { Body: "0", Expected: Token{ Kind: TokenKind[INT], @@ -309,7 +309,7 @@ func TestLexesNumbers(t *testing.T) { Value: "0", }, }, - Test{ + { Body: "-4.123", Expected: Token{ Kind: TokenKind[FLOAT], @@ -318,7 +318,7 @@ func TestLexesNumbers(t *testing.T) { Value: "-4.123", }, }, - Test{ + { Body: "0.123", Expected: Token{ Kind: TokenKind[FLOAT], @@ -327,7 +327,7 @@ func TestLexesNumbers(t *testing.T) { Value: "0.123", }, }, - Test{ + { Body: "123e4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -336,7 +336,7 @@ func TestLexesNumbers(t *testing.T) { Value: "123e4", }, }, - Test{ + { Body: "123E4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -345,7 +345,7 @@ func TestLexesNumbers(t *testing.T) { Value: "123E4", }, }, - Test{ + { Body: "123e-4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -354,7 +354,7 @@ func TestLexesNumbers(t *testing.T) { Value: "123e-4", }, }, - Test{ + { Body: "123e+4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -363,7 +363,7 @@ func TestLexesNumbers(t *testing.T) { Value: "123e+4", }, }, - Test{ + { Body: "-1.123e4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -372,7 +372,7 @@ func TestLexesNumbers(t *testing.T) { Value: "-1.123e4", }, }, - Test{ + { Body: "-1.123E4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -381,7 +381,7 @@ func TestLexesNumbers(t *testing.T) { Value: "-1.123E4", }, }, - Test{ + { Body: "-1.123e-4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -390,7 +390,7 @@ func TestLexesNumbers(t *testing.T) { Value: "-1.123e-4", }, }, - Test{ + { Body: "-1.123e+4", Expected: Token{ Kind: TokenKind[FLOAT], @@ -399,7 +399,7 @@ func TestLexesNumbers(t *testing.T) { Value: "-1.123e+4", }, }, - Test{ + { Body: "-1.123e4567", Expected: Token{ Kind: TokenKind[FLOAT], @@ -422,7 +422,7 @@ func TestLexesNumbers(t *testing.T) { func TestLexReportsUsefulNumbeErrors(t *testing.T) { tests := []Test{ - Test{ + { Body: "00", Expected: `Syntax Error GraphQL (1:2) Invalid number, unexpected digit after 0: "0". @@ -430,7 +430,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "+1", Expected: `Syntax Error GraphQL (1:1) Unexpected character "+". @@ -438,7 +438,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "1.", Expected: `Syntax Error GraphQL (1:3) Invalid number, expected digit but got: EOF. @@ -446,7 +446,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { ^ `, }, - Test{ + { Body: ".123", Expected: `Syntax Error GraphQL (1:1) Unexpected character ".". @@ -454,7 +454,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "1.A", Expected: `Syntax Error GraphQL (1:3) Invalid number, expected digit but got: "A". @@ -462,7 +462,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "-A", Expected: `Syntax Error GraphQL (1:2) Invalid number, expected digit but got: "A". @@ -470,7 +470,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "1.0e", Expected: `Syntax Error GraphQL (1:5) Invalid number, expected digit but got: EOF. @@ -478,7 +478,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { ^ `, }, - Test{ + { Body: "1.0eA", Expected: `Syntax Error GraphQL (1:5) Invalid number, expected digit but got: "A". @@ -500,7 +500,7 @@ func TestLexReportsUsefulNumbeErrors(t *testing.T) { func TestLexesPunctuation(t *testing.T) { tests := []Test{ - Test{ + { Body: "!", Expected: Token{ Kind: TokenKind[BANG], @@ -509,7 +509,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "$", Expected: Token{ Kind: TokenKind[DOLLAR], @@ -518,7 +518,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "(", Expected: Token{ Kind: TokenKind[PAREN_L], @@ -527,7 +527,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: ")", Expected: Token{ Kind: TokenKind[PAREN_R], @@ -536,7 +536,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "...", Expected: Token{ Kind: TokenKind[SPREAD], @@ -545,7 +545,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: ":", Expected: Token{ Kind: TokenKind[COLON], @@ -554,7 +554,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "=", Expected: Token{ Kind: TokenKind[EQUALS], @@ -563,7 +563,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "@", Expected: Token{ Kind: TokenKind[AT], @@ -572,7 +572,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "[", Expected: Token{ Kind: TokenKind[BRACKET_L], @@ -581,7 +581,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "]", Expected: Token{ Kind: TokenKind[BRACKET_R], @@ -590,7 +590,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "{", Expected: Token{ Kind: TokenKind[BRACE_L], @@ -599,7 +599,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "|", Expected: Token{ Kind: TokenKind[PIPE], @@ -608,7 +608,7 @@ func TestLexesPunctuation(t *testing.T) { Value: "", }, }, - Test{ + { Body: "}", Expected: Token{ Kind: TokenKind[BRACE_R], @@ -631,7 +631,7 @@ func TestLexesPunctuation(t *testing.T) { func TestLexReportsUsefulUnknownCharacterError(t *testing.T) { tests := []Test{ - Test{ + { Body: "..", Expected: `Syntax Error GraphQL (1:1) Unexpected character ".". @@ -639,7 +639,7 @@ func TestLexReportsUsefulUnknownCharacterError(t *testing.T) { ^ `, }, - Test{ + { Body: "?", Expected: `Syntax Error GraphQL (1:1) Unexpected character "?". @@ -647,7 +647,7 @@ func TestLexReportsUsefulUnknownCharacterError(t *testing.T) { ^ `, }, - Test{ + { Body: "\u203B", Expected: `Syntax Error GraphQL (1:1) Unexpected character "※". diff --git a/language/visitor/visitor_test.go b/language/visitor/visitor_test.go index 412f96c0..b792ce8d 100644 --- a/language/visitor/visitor_test.go +++ b/language/visitor/visitor_test.go @@ -253,7 +253,7 @@ func TestVisitor_AllowsANamedFunctionsVisitorAPI(t *testing.T) { v := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - "Name": visitor.NamedVisitFuncs{ + "Name": { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.Name: @@ -262,7 +262,7 @@ func TestVisitor_AllowsANamedFunctionsVisitorAPI(t *testing.T) { return visitor.ActionNoChange, nil }, }, - "SelectionSet": visitor.NamedVisitFuncs{ + "SelectionSet": { Enter: func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.SelectionSet: diff --git a/lists_test.go b/lists_test.go index 4bc47cd7..9639512b 100644 --- a/lists_test.go +++ b/lists_test.go @@ -255,10 +255,10 @@ func TestLists_NonNullListOfNullableObjectsReturnsNull(t *testing.T) { "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, @@ -325,10 +325,10 @@ func TestLists_NonNullListOfNullableFunc_ReturnsNull(t *testing.T) { "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, @@ -421,10 +421,10 @@ func TestLists_NullableListOfNonNullObjects_ContainsNull(t *testing.T) { }, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, @@ -486,10 +486,10 @@ func TestLists_NullableListOfNonNullFunc_ContainsNull(t *testing.T) { }, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, @@ -597,10 +597,10 @@ func TestLists_NonNullListOfNonNullObjects_ContainsNull(t *testing.T) { "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, @@ -618,10 +618,10 @@ func TestLists_NonNullListOfNonNullObjects_ReturnsNull(t *testing.T) { "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, @@ -669,10 +669,10 @@ func TestLists_NonNullListOfNonNullFunc_ContainsNull(t *testing.T) { "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, @@ -695,10 +695,10 @@ func TestLists_NonNullListOfNonNullFunc_ReturnsNull(t *testing.T) { "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: "Cannot return null for non-nullable field DataType.test.", Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 1, Column: 10, }, diff --git a/mutations_test.go b/mutations_test.go index 0a52d136..a97dda52 100644 --- a/mutations_test.go +++ b/mutations_test.go @@ -218,16 +218,16 @@ func TestMutations_EvaluatesMutationsCorrectlyInThePresenceOfAFailedMutation(t * "sixth": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot change the number`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 8, Column: 7}, + {Line: 8, Column: 7}, }, }, - gqlerrors.FormattedError{ + { Message: `Cannot change the number`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 17, Column: 7}, + {Line: 17, Column: 7}, }, }, }, diff --git a/nonnull_test.go b/nonnull_test.go index b52c6829..75b3f6cb 100644 --- a/nonnull_test.go +++ b/nonnull_test.go @@ -121,10 +121,10 @@ func TestNonNull_NullsANullableFieldThatThrowsSynchronously(t *testing.T) { "sync": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: syncError, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 3, Column: 9, }, }, @@ -159,10 +159,10 @@ func TestNonNull_NullsANullableFieldThatThrowsInAPromise(t *testing.T) { "promise": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: promiseError, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 3, Column: 9, }, }, @@ -199,10 +199,10 @@ func TestNonNull_NullsASynchronouslyReturnedObjectThatContainsANullableFieldThat "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: nonNullSyncError, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 4, Column: 11, }, }, @@ -239,10 +239,10 @@ func TestNonNull_NullsASynchronouslyReturnedObjectThatContainsANonNullableFieldT "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: nonNullPromiseError, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 4, Column: 11, }, }, @@ -279,10 +279,10 @@ func TestNonNull_NullsAnObjectReturnedInAPromiseThatContainsANonNullableFieldTha "promiseNest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: nonNullSyncError, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 4, Column: 11, }, }, @@ -319,10 +319,10 @@ func TestNonNull_NullsAnObjectReturnedInAPromiseThatContainsANonNullableFieldTha "promiseNest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: nonNullPromiseError, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 4, Column: 11, }, }, @@ -404,76 +404,76 @@ func TestNonNull_NullsAComplexTreeOfNullableFieldsThatThrow(t *testing.T) { }, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: syncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 4, Column: 11}, + {Line: 4, Column: 11}, }, }, - gqlerrors.FormattedError{ + { Message: syncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 7, Column: 13}, + {Line: 7, Column: 13}, }, }, - gqlerrors.FormattedError{ + { Message: syncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 11, Column: 13}, + {Line: 11, Column: 13}, }, }, - gqlerrors.FormattedError{ + { Message: syncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 16, Column: 11}, + {Line: 16, Column: 11}, }, }, - gqlerrors.FormattedError{ + { Message: syncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 19, Column: 13}, + {Line: 19, Column: 13}, }, }, - gqlerrors.FormattedError{ + { Message: syncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 23, Column: 13}, + {Line: 23, Column: 13}, }, }, - gqlerrors.FormattedError{ + { Message: promiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 5, Column: 11}, + {Line: 5, Column: 11}, }, }, - gqlerrors.FormattedError{ + { Message: promiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 8, Column: 13}, + {Line: 8, Column: 13}, }, }, - gqlerrors.FormattedError{ + { Message: promiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 12, Column: 13}, + {Line: 12, Column: 13}, }, }, - gqlerrors.FormattedError{ + { Message: promiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 17, Column: 11}, + {Line: 17, Column: 11}, }, }, - gqlerrors.FormattedError{ + { Message: promiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 20, Column: 13}, + {Line: 20, Column: 13}, }, }, - gqlerrors.FormattedError{ + { Message: promiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 24, Column: 13}, + {Line: 24, Column: 13}, }, }, }, @@ -557,28 +557,28 @@ func TestNonNull_NullsTheFirstNullableObjectAfterAFieldThrowsInALongChainOfField "anotherPromiseNest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: nonNullSyncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 8, Column: 19}, + {Line: 8, Column: 19}, }, }, - gqlerrors.FormattedError{ + { Message: nonNullSyncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 19, Column: 19}, + {Line: 19, Column: 19}, }, }, - gqlerrors.FormattedError{ + { Message: nonNullPromiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 30, Column: 19}, + {Line: 30, Column: 19}, }, }, - gqlerrors.FormattedError{ + { Message: nonNullPromiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 41, Column: 19}, + {Line: 41, Column: 19}, }, }, }, @@ -681,10 +681,10 @@ func TestNonNull_NullsASynchronouslyReturnedObjectThatContainsANonNullableFieldT "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullSync.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 4, Column: 11}, + {Line: 4, Column: 11}, }, }, }, @@ -719,10 +719,10 @@ func TestNonNull_NullsASynchronouslyReturnedObjectThatContainsANonNullableFieldT "nest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullPromise.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 4, Column: 11}, + {Line: 4, Column: 11}, }, }, }, @@ -758,10 +758,10 @@ func TestNonNull_NullsAnObjectReturnedInAPromiseThatContainsANonNullableFieldTha "promiseNest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullSync.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 4, Column: 11}, + {Line: 4, Column: 11}, }, }, }, @@ -796,10 +796,10 @@ func TestNonNull_NullsAnObjectReturnedInAPromiseThatContainsANonNullableFieldTha "promiseNest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullPromise.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 4, Column: 11}, + {Line: 4, Column: 11}, }, }, }, @@ -955,28 +955,28 @@ func TestNonNull_NullsTheFirstNullableObjectAfterAFieldReturnsNullInALongChainOf "anotherPromiseNest": nil, }, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullSync.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 8, Column: 19}, + {Line: 8, Column: 19}, }, }, - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullSync.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 19, Column: 19}, + {Line: 19, Column: 19}, }, }, - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullPromise.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 30, Column: 19}, + {Line: 30, Column: 19}, }, }, - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullPromise.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 41, Column: 19}, + {Line: 41, Column: 19}, }, }, }, @@ -1011,10 +1011,10 @@ func TestNonNull_NullsTheTopLevelIfSyncNonNullableFieldThrows(t *testing.T) { expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: nonNullSyncError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 2, Column: 17}, + {Line: 2, Column: 17}, }, }, }, @@ -1043,10 +1043,10 @@ func TestNonNull_NullsTheTopLevelIfSyncNonNullableFieldErrors(t *testing.T) { expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: nonNullPromiseError, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 2, Column: 17}, + {Line: 2, Column: 17}, }, }, }, @@ -1075,10 +1075,10 @@ func TestNonNull_NullsTheTopLevelIfSyncNonNullableFieldReturnsNull(t *testing.T) expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullSync.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 2, Column: 17}, + {Line: 2, Column: 17}, }, }, }, @@ -1107,10 +1107,10 @@ func TestNonNull_NullsTheTopLevelIfSyncNonNullableFieldResolvesNull(t *testing.T expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Cannot return null for non-nullable field DataType.nonNullPromise.`, Locations: []location.SourceLocation{ - location.SourceLocation{Line: 2, Column: 17}, + {Line: 2, Column: 17}, }, }, }, diff --git a/rules.go b/rules.go index 80f10754..534ad216 100644 --- a/rules.go +++ b/rules.go @@ -65,7 +65,7 @@ func newValidationRuleError(message string, nodes []ast.Node) (string, error) { func ArgumentsOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Argument: visitor.NamedVisitFuncs{ + kinds.Argument: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { var action = visitor.ActionNoChange var result interface{} @@ -104,7 +104,7 @@ func ArgumentsOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInsta func DefaultValuesOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.VariableDefinition: visitor.NamedVisitFuncs{ + kinds.VariableDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { var action = visitor.ActionNoChange var result interface{} @@ -151,7 +151,7 @@ func DefaultValuesOfCorrectTypeRule(context *ValidationContext) *ValidationRuleI func FieldsOnCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Field: visitor.NamedVisitFuncs{ + kinds.Field: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { var action = visitor.ActionNoChange var result interface{} @@ -194,7 +194,7 @@ func FieldsOnCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance func FragmentsOnCompositeTypesRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.InlineFragment: visitor.NamedVisitFuncs{ + kinds.InlineFragment: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.InlineFragment); ok { ttype := context.Type() @@ -208,7 +208,7 @@ func FragmentsOnCompositeTypesRule(context *ValidationContext) *ValidationRuleIn return visitor.ActionNoChange, nil }, }, - kinds.FragmentDefinition: visitor.NamedVisitFuncs{ + kinds.FragmentDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.FragmentDefinition); ok { ttype := context.Type() @@ -243,7 +243,7 @@ func FragmentsOnCompositeTypesRule(context *ValidationContext) *ValidationRuleIn func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Argument: visitor.NamedVisitFuncs{ + kinds.Argument: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { var action = visitor.ActionNoChange var result interface{} @@ -324,7 +324,7 @@ func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance func KnownDirectivesRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Directive: visitor.NamedVisitFuncs{ + kinds.Directive: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { var action = visitor.ActionNoChange var result interface{} @@ -398,7 +398,7 @@ func KnownDirectivesRule(context *ValidationContext) *ValidationRuleInstance { func KnownFragmentNamesRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.FragmentSpread: visitor.NamedVisitFuncs{ + kinds.FragmentSpread: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { var action = visitor.ActionNoChange var result interface{} @@ -437,7 +437,7 @@ func KnownFragmentNamesRule(context *ValidationContext) *ValidationRuleInstance func KnownTypeNamesRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Named: visitor.NamedVisitFuncs{ + kinds.Named: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.Named); ok { typeNameValue := "" @@ -474,7 +474,7 @@ func LoneAnonymousOperationRule(context *ValidationContext) *ValidationRuleInsta var operationCount = 0 visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Document: visitor.NamedVisitFuncs{ + kinds.Document: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.Document); ok { operationCount = 0 @@ -487,7 +487,7 @@ func LoneAnonymousOperationRule(context *ValidationContext) *ValidationRuleInsta return visitor.ActionNoChange, nil }, }, - kinds.OperationDefinition: visitor.NamedVisitFuncs{ + kinds.OperationDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.OperationDefinition); ok { if node.Name == nil && operationCount > 1 { @@ -553,7 +553,7 @@ func NoFragmentCyclesRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.FragmentDefinition: visitor.NamedVisitFuncs{ + kinds.FragmentDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.FragmentDefinition); ok && node != nil { errors := []error{} @@ -641,7 +641,7 @@ func NoUndefinedVariablesRule(context *ValidationContext) *ValidationRuleInstanc var definedVariableNames = map[string]bool{} visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.OperationDefinition: visitor.NamedVisitFuncs{ + kinds.OperationDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.OperationDefinition); ok && node != nil { operation = node @@ -651,7 +651,7 @@ func NoUndefinedVariablesRule(context *ValidationContext) *ValidationRuleInstanc return visitor.ActionNoChange, nil }, }, - kinds.VariableDefinition: visitor.NamedVisitFuncs{ + kinds.VariableDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.VariableDefinition); ok && node != nil { variableName := "" @@ -663,7 +663,7 @@ func NoUndefinedVariablesRule(context *ValidationContext) *ValidationRuleInstanc return visitor.ActionNoChange, nil }, }, - kinds.Variable: visitor.NamedVisitFuncs{ + kinds.Variable: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if variable, ok := p.Node.(*ast.Variable); ok && variable != nil { variableName := "" @@ -693,7 +693,7 @@ func NoUndefinedVariablesRule(context *ValidationContext) *ValidationRuleInstanc return visitor.ActionNoChange, nil }, }, - kinds.FragmentSpread: visitor.NamedVisitFuncs{ + kinds.FragmentSpread: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.FragmentSpread); ok && node != nil { // Only visit fragments of a particular name once per operation @@ -733,7 +733,7 @@ func NoUnusedFragmentsRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.OperationDefinition: visitor.NamedVisitFuncs{ + kinds.OperationDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.OperationDefinition); ok && node != nil { spreadNames = map[string]bool{} @@ -742,7 +742,7 @@ func NoUnusedFragmentsRule(context *ValidationContext) *ValidationRuleInstance { return visitor.ActionNoChange, nil }, }, - kinds.FragmentDefinition: visitor.NamedVisitFuncs{ + kinds.FragmentDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if def, ok := p.Node.(*ast.FragmentDefinition); ok && def != nil { defName := "" @@ -757,7 +757,7 @@ func NoUnusedFragmentsRule(context *ValidationContext) *ValidationRuleInstance { return visitor.ActionNoChange, nil }, }, - kinds.FragmentSpread: visitor.NamedVisitFuncs{ + kinds.FragmentSpread: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if spread, ok := p.Node.(*ast.FragmentSpread); ok && spread != nil { spreadName := "" @@ -769,14 +769,14 @@ func NoUnusedFragmentsRule(context *ValidationContext) *ValidationRuleInstance { return visitor.ActionNoChange, nil }, }, - kinds.Document: visitor.NamedVisitFuncs{ + kinds.Document: { Leave: func(p visitor.VisitFuncParams) (string, interface{}) { fragmentNameUsed := map[string]interface{}{} var reduceSpreadFragments func(spreads map[string]bool) reduceSpreadFragments = func(spreads map[string]bool) { - for fragName, _ := range spreads { + for fragName := range spreads { if isFragNameUsed, _ := fragmentNameUsed[fragName]; isFragNameUsed != true { fragmentNameUsed[fragName] = true @@ -834,7 +834,7 @@ func NoUnusedVariablesRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.OperationDefinition: visitor.NamedVisitFuncs{ + kinds.OperationDefinition: { Enter: func(p visitor.VisitFuncParams) (string, interface{}) { visitedFragmentNames = map[string]bool{} variableDefs = []*ast.VariableDefinition{} @@ -862,7 +862,7 @@ func NoUnusedVariablesRule(context *ValidationContext) *ValidationRuleInstance { return visitor.ActionNoChange, nil }, }, - kinds.VariableDefinition: visitor.NamedVisitFuncs{ + kinds.VariableDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if def, ok := p.Node.(*ast.VariableDefinition); ok && def != nil { variableDefs = append(variableDefs, def) @@ -871,7 +871,7 @@ func NoUnusedVariablesRule(context *ValidationContext) *ValidationRuleInstance { return visitor.ActionSkip, nil }, }, - kinds.Variable: visitor.NamedVisitFuncs{ + kinds.Variable: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if variable, ok := p.Node.(*ast.Variable); ok && variable != nil { if variable.Name != nil { @@ -881,7 +881,7 @@ func NoUnusedVariablesRule(context *ValidationContext) *ValidationRuleInstance { return visitor.ActionNoChange, nil }, }, - kinds.FragmentSpread: visitor.NamedVisitFuncs{ + kinds.FragmentSpread: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if spreadAST, ok := p.Node.(*ast.FragmentSpread); ok && spreadAST != nil { // Only visit fragments of a particular name once per operation @@ -1232,7 +1232,7 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul // ensure field traversal orderedName := sort.StringSlice{} - for responseName, _ := range fieldMap { + for responseName := range fieldMap { orderedName = append(orderedName, responseName) } orderedName.Sort() @@ -1274,7 +1274,7 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.SelectionSet: visitor.NamedVisitFuncs{ + kinds.SelectionSet: { Leave: func(p visitor.VisitFuncParams) (string, interface{}) { if selectionSet, ok := p.Node.(*ast.SelectionSet); ok && selectionSet != nil { parentType, _ := context.ParentType().(Named) @@ -1378,7 +1378,7 @@ func PossibleFragmentSpreadsRule(context *ValidationContext) *ValidationRuleInst visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.InlineFragment: visitor.NamedVisitFuncs{ + kinds.InlineFragment: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.InlineFragment); ok && node != nil { fragType := context.Type() @@ -1395,7 +1395,7 @@ func PossibleFragmentSpreadsRule(context *ValidationContext) *ValidationRuleInst return visitor.ActionNoChange, nil }, }, - kinds.FragmentSpread: visitor.NamedVisitFuncs{ + kinds.FragmentSpread: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.FragmentSpread); ok && node != nil { fragName := "" @@ -1433,7 +1433,7 @@ func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleIns visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Field: visitor.NamedVisitFuncs{ + kinds.Field: { Leave: func(p visitor.VisitFuncParams) (string, interface{}) { // Validate on leave to allow for deeper errors to appear first. if fieldAST, ok := p.Node.(*ast.Field); ok && fieldAST != nil { @@ -1477,7 +1477,7 @@ func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleIns return visitor.ActionNoChange, nil }, }, - kinds.Directive: visitor.NamedVisitFuncs{ + kinds.Directive: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { // Validate on leave to allow for deeper errors to appear first. @@ -1540,7 +1540,7 @@ func ScalarLeafsRule(context *ValidationContext) *ValidationRuleInstance { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Field: visitor.NamedVisitFuncs{ + kinds.Field: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.Field); ok && node != nil { nodeName := "" @@ -1586,19 +1586,19 @@ func UniqueArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.Field: visitor.NamedVisitFuncs{ + kinds.Field: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { knownArgNames = map[string]*ast.Name{} return visitor.ActionNoChange, nil }, }, - kinds.Directive: visitor.NamedVisitFuncs{ + kinds.Directive: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { knownArgNames = map[string]*ast.Name{} return visitor.ActionNoChange, nil }, }, - kinds.Argument: visitor.NamedVisitFuncs{ + kinds.Argument: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.Argument); ok { argName := "" @@ -1634,7 +1634,7 @@ func UniqueFragmentNamesRule(context *ValidationContext) *ValidationRuleInstance visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.FragmentDefinition: visitor.NamedVisitFuncs{ + kinds.FragmentDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.FragmentDefinition); ok && node != nil { fragmentName := "" @@ -1670,7 +1670,7 @@ func UniqueOperationNamesRule(context *ValidationContext) *ValidationRuleInstanc visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.OperationDefinition: visitor.NamedVisitFuncs{ + kinds.OperationDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.OperationDefinition); ok && node != nil { operationName := "" @@ -1706,7 +1706,7 @@ func VariablesAreInputTypesRule(context *ValidationContext) *ValidationRuleInsta visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.VariableDefinition: visitor.NamedVisitFuncs{ + kinds.VariableDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.VariableDefinition); ok && node != nil { ttype, _ := typeFromAST(*context.Schema(), node.Type) @@ -1778,14 +1778,14 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.OperationDefinition: visitor.NamedVisitFuncs{ + kinds.OperationDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { varDefMap = map[string]*ast.VariableDefinition{} visitedFragmentNames = map[string]bool{} return visitor.ActionNoChange, nil }, }, - kinds.VariableDefinition: visitor.NamedVisitFuncs{ + kinds.VariableDefinition: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if varDefAST, ok := p.Node.(*ast.VariableDefinition); ok { defName := "" @@ -1797,7 +1797,7 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI return visitor.ActionNoChange, nil }, }, - kinds.FragmentSpread: visitor.NamedVisitFuncs{ + kinds.FragmentSpread: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { // Only visit fragments of a particular name once per operation if spreadAST, ok := p.Node.(*ast.FragmentSpread); ok { @@ -1813,7 +1813,7 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI return visitor.ActionNoChange, nil }, }, - kinds.Variable: visitor.NamedVisitFuncs{ + kinds.Variable: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if variableAST, ok := p.Node.(*ast.Variable); ok && variableAST != nil { varName := "" @@ -1946,7 +1946,7 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) bool { func gatherSpreads(node ast.Node) (spreadNodes []*ast.FragmentSpread) { visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ - kinds.FragmentSpread: visitor.NamedVisitFuncs{ + kinds.FragmentSpread: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.FragmentSpread); ok && node != nil { spreadNodes = append(spreadNodes, node) diff --git a/schema.go b/schema.go index b2be7bba..4e1c37a6 100644 --- a/schema.go +++ b/schema.go @@ -258,7 +258,7 @@ func assertObjectImplementsInterface(object *Object, iface *Interface) error { ifaceFieldMap := iface.Fields() // Assert each interface field is implemented. - for fieldName, _ := range ifaceFieldMap { + for fieldName := range ifaceFieldMap { objectField := objectFieldMap[fieldName] ifaceField := ifaceFieldMap[fieldName] diff --git a/validator.go b/validator.go index 2873fd64..785aeb92 100644 --- a/validator.go +++ b/validator.go @@ -94,7 +94,7 @@ func visitUsingRules(schema *Schema, astDoc *ast.Document, rules []ValidationRul } } - // If the result is "false" (ie action === Action.Skip), we're not visiting any descendent nodes, + // If the result is "false" (ie action === Action.Skip), we're not visiting any descendant nodes, // but need to update typeInfo. if action == visitor.ActionSkip { typeInfo.Leave(node) diff --git a/values.go b/values.go index 6b3ff169..e228db14 100644 --- a/values.go +++ b/values.go @@ -246,13 +246,13 @@ func isValidInputValue(value interface{}, ttype Input) bool { fields := ttype.Fields() // Ensure every provided field is defined. - for fieldName, _ := range valueMap { + for fieldName := range valueMap { if _, ok := fields[fieldName]; !ok { return false } } // Ensure every defined field is valid. - for fieldName, _ := range fields { + for fieldName := range fields { isValid := isValidInputValue(valueMap[fieldName], fields[fieldName].Type) if !isValid { return false diff --git a/variables_test.go b/variables_test.go index b8e118f2..a5bf594c 100644 --- a/variables_test.go +++ b/variables_test.go @@ -368,11 +368,11 @@ func TestVariables_ObjectsAndNullability_UsingVariables_ErrorsOnNullForNestedNon expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "TestInputObject" but ` + `got: {"a":"foo","b":"bar","c":null}.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -403,11 +403,11 @@ func TestVariables_ObjectsAndNullability_UsingVariables_ErrorsOnIncorrectType(t expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "TestInputObject" but ` + `got: "foo bar".`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -441,11 +441,11 @@ func TestVariables_ObjectsAndNullability_UsingVariables_ErrorsOnOmissionOfNested expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "TestInputObject" but ` + `got: {"a":"foo","b":"bar"}.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -481,11 +481,11 @@ func TestVariables_ObjectsAndNullability_UsingVariables_ErrorsOnAdditionOfUnknow expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "TestInputObject" but ` + `got: {"a":"foo","b":"bar","c":"baz","d":"dog"}.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -692,10 +692,10 @@ func TestVariables_NonNullableScalars_DoesNotAllowNonNullableInputsToBeOmittedIn expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$value" of required type "String!" was not provided.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 31, }, }, @@ -731,10 +731,10 @@ func TestVariables_NonNullableScalars_DoesNotAllowNonNullableInputsToBeSetToNull expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$value" of required type "String!" was not provided.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 31, }, }, @@ -959,10 +959,10 @@ func TestVariables_ListsAndNullability_DoesNotAllowNonNullListsToBeNull(t *testi expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" of required type "[String]!" was not provided.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -1116,11 +1116,11 @@ func TestVariables_ListsAndNullability_DoesNotAllowListOfNonNullsToContainNull(t expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "[String!]" but got: ` + `["A",null,"B"].`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -1155,10 +1155,10 @@ func TestVariables_ListsAndNullability_DoesNotAllowNonNullListOfNonNullsToBeNull expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" of required type "[String!]!" was not provided.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -1223,11 +1223,11 @@ func TestVariables_ListsAndNullability_DoesNotAllowNonNullListOfNonNullsToContai expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "[String!]!" but got: ` + `["A",null,"B"].`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -1264,10 +1264,10 @@ func TestVariables_ListsAndNullability_DoesNotAllowInvalidTypesToBeUsedAsValues( expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "TestType!" which cannot be used as an input type.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, @@ -1302,10 +1302,10 @@ func TestVariables_ListsAndNullability_DoesNotAllowUnknownTypesToBeUsedAsValues( expected := &graphql.Result{ Data: nil, Errors: []gqlerrors.FormattedError{ - gqlerrors.FormattedError{ + { Message: `Variable "$input" expected value of type "UnknownType!" which cannot be used as an input type.`, Locations: []location.SourceLocation{ - location.SourceLocation{ + { Line: 2, Column: 17, }, }, From c223119739094336673192a2f8170bab059baf47 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 11:20:07 +0800 Subject: [PATCH 03/32] Extract completeListValue function from CompleteValue --- executor.go | 50 +++++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/executor.go b/executor.go index b810f358..f739700b 100644 --- a/executor.go +++ b/executor.go @@ -614,29 +614,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie // If field type is List, complete each item in the list with the inner type if returnType, ok := returnType.(*List); ok { - - resultVal := reflect.ValueOf(result) - parentTypeName := "" - if info.ParentType != nil { - parentTypeName = info.ParentType.Name() - } - err := invariant( - resultVal.IsValid() && resultVal.Type().Kind() == reflect.Slice, - fmt.Sprintf("User Error: expected iterable, but did not find one "+ - "for field %v.%v.", parentTypeName, info.FieldName), - ) - if err != nil { - panic(gqlerrors.FormatError(err)) - } - - itemType := returnType.OfType - completedResults := []interface{}{} - for i := 0; i < resultVal.Len(); i++ { - val := resultVal.Index(i).Interface() - completedItem := completeValueCatchingError(eCtx, itemType, fieldASTs, info, val) - completedResults = append(completedResults, completedItem) - } - return completedResults + return completeListValue(eCtx, returnType, fieldASTs, info, result) } // If field type is Scalar or Enum, serialize to a valid value, returning @@ -714,6 +692,32 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie } +// completeListValue complete a list value by completeing each item in the list with the inner type +func completeListValue(eCtx *ExecutionContext, returnType *List, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { + resultVal := reflect.ValueOf(result) + parentTypeName := "" + if info.ParentType != nil { + parentTypeName = info.ParentType.Name() + } + err := invariant( + resultVal.IsValid() && resultVal.Type().Kind() == reflect.Slice, + fmt.Sprintf("User Error: expected iterable, but did not find one "+ + "for field %v.%v.", parentTypeName, info.FieldName), + ) + if err != nil { + panic(gqlerrors.FormatError(err)) + } + + itemType := returnType.OfType + completedResults := []interface{}{} + for i := 0; i < resultVal.Len(); i++ { + val := resultVal.Index(i).Interface() + completedItem := completeValueCatchingError(eCtx, itemType, fieldASTs, info, val) + completedResults = append(completedResults, completedItem) + } + return completedResults +} + func defaultResolveFn(p ResolveParams) (interface{}, error) { // try to resolve p.Source as a struct first sourceVal := reflect.ValueOf(p.Source) From da4f176d55fbaba50ced1ef635c2357edcbe6b72 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 11:35:16 +0800 Subject: [PATCH 04/32] Extract completeLeafValue from completeValue --- definition.go | 12 ++++++++++++ executor.go | 19 +++++++++---------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/definition.go b/definition.go index bf42b530..8e682bfa 100644 --- a/definition.go +++ b/definition.go @@ -77,6 +77,18 @@ func IsOutputType(ttype Type) bool { return false } +// Leaf interface for types that may be leaf values +type Leaf interface { + Name() string + Description() string + String() string + Error() error + Serialize(value interface{}) interface{} +} + +var _ Leaf = (*Scalar)(nil) +var _ Leaf = (*Enum)(nil) + // IsLeafType determines if given type is a leaf value func IsLeafType(ttype Type) bool { named := GetNamed(ttype) diff --git a/executor.go b/executor.go index f739700b..0b237fa1 100644 --- a/executor.go +++ b/executor.go @@ -620,18 +620,10 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie // If field type is Scalar or Enum, serialize to a valid value, returning // null if serialization is not possible. if returnType, ok := returnType.(*Scalar); ok { - serializedResult := returnType.Serialize(result) - if isNullish(serializedResult) { - return nil - } - return serializedResult + return completeLeafValue(eCtx, returnType, fieldASTs, info, result) } if returnType, ok := returnType.(*Enum); ok { - serializedResult := returnType.Serialize(result) - if isNullish(serializedResult) { - return nil - } - return serializedResult + return completeLeafValue(eCtx, returnType, fieldASTs, info, result) } // ast.Field type must be Object, Interface or Union and expect sub-selections. @@ -691,6 +683,13 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return results.Data } +func completeLeafValue(eCtx *ExecutionContext, returnType Leaf, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { + serializedResult := returnType.Serialize(result) + if isNullish(serializedResult) { + return nil + } + return serializedResult +} // completeListValue complete a list value by completeing each item in the list with the inner type func completeListValue(eCtx *ExecutionContext, returnType *List, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { From 20e82a8ca660bebab6f8979a408ee3a48bf9d805 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 11:42:00 +0800 Subject: [PATCH 05/32] Update documentation for completeLeafValue and completeListValue --- executor.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 0b237fa1..3f72b718 100644 --- a/executor.go +++ b/executor.go @@ -683,6 +683,8 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return results.Data } + +// completeLeafValue complete a leaf value (Scalar / Enum) by serializing to a valid value, returning nil if serialization is not possible. func completeLeafValue(eCtx *ExecutionContext, returnType Leaf, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { serializedResult := returnType.Serialize(result) if isNullish(serializedResult) { @@ -691,7 +693,7 @@ func completeLeafValue(eCtx *ExecutionContext, returnType Leaf, fieldASTs []*ast return serializedResult } -// completeListValue complete a list value by completeing each item in the list with the inner type +// completeListValue complete a list value by completing each item in the list with the inner type func completeListValue(eCtx *ExecutionContext, returnType *List, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { resultVal := reflect.ValueOf(result) parentTypeName := "" From 89b14fe485db19d25c754ffe08b05a8ac9b1c5b0 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 11:56:34 +0800 Subject: [PATCH 06/32] Extract completeObjectValue from completeValue --- executor.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/executor.go b/executor.go index 3f72b718..2142fb67 100644 --- a/executor.go +++ b/executor.go @@ -585,8 +585,6 @@ func completeValueCatchingError(eCtx *ExecutionContext, returnType Type, fieldAS func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { - // TODO: explore resolving go-routines in completeValue - resultVal := reflect.ValueOf(result) if resultVal.IsValid() && resultVal.Type().Kind() == reflect.Func { if propertyFn, ok := result.(func() interface{}); ok { @@ -626,12 +624,14 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return completeLeafValue(eCtx, returnType, fieldASTs, info, result) } + if returnType, ok := returnType.(*Object); ok { + return completeObjectValue(eCtx, returnType, fieldASTs, info, result) + } + // ast.Field type must be Object, Interface or Union and expect sub-selections. var runtimeType *Object - switch returnType := returnType.(type) { - case *Object: - runtimeType = returnType - case Abstract: + + if returnType, ok := returnType.(Abstract); ok { runtimeType = returnType.ObjectType(result, info) if runtimeType != nil && !returnType.IsPossibleType(runtimeType) { panic(gqlerrors.NewFormattedError( @@ -640,16 +640,24 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie )) } } + if runtimeType == nil { return nil } + return completeObjectValue(eCtx, runtimeType, fieldASTs, info, result) + +} + +// completeObjectValue completes an Object value by evaluating all sub-selections +func completeObjectValue(eCtx *ExecutionContext, returnType *Object, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { + // If there is an isTypeOf predicate function, call it with the // current result. If isTypeOf returns false, then raise an error rather // than continuing execution. - if runtimeType.IsTypeOf != nil && !runtimeType.IsTypeOf(result, info) { + if returnType.IsTypeOf != nil && !returnType.IsTypeOf(result, info) { panic(gqlerrors.NewFormattedError( - fmt.Sprintf(`Expected value of type "%v" but got: %T.`, runtimeType, result), + fmt.Sprintf(`Expected value of type "%v" but got: %T.`, returnType, result), )) } @@ -664,7 +672,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie if selectionSet != nil { innerParams := CollectFieldsParams{ ExeContext: eCtx, - RuntimeType: runtimeType, + RuntimeType: returnType, SelectionSet: selectionSet, Fields: subFieldASTs, VisitedFragmentNames: visitedFragmentNames, @@ -674,7 +682,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie } executeFieldsParams := ExecuteFieldsParams{ ExecutionContext: eCtx, - ParentType: runtimeType, + ParentType: returnType, Source: result, Fields: subFieldASTs, } From 3ca6410148a7d939feba0ab4c3b7dcc2db405090 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 12:02:30 +0800 Subject: [PATCH 07/32] Extract completeAbstractValue from completeValue --- executor.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/executor.go b/executor.go index 2142fb67..2c854f91 100644 --- a/executor.go +++ b/executor.go @@ -628,7 +628,19 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return completeObjectValue(eCtx, returnType, fieldASTs, info, result) } - // ast.Field type must be Object, Interface or Union and expect sub-selections. + if returnType, ok := returnType.(Abstract); ok { + return completeAbstractValue(eCtx, returnType, fieldASTs, info, result) + } + + // Not reachable + return nil + +} + +// completeObjectValue completes value of an Abstract type (Union / Interface) by determining the runtime type +// of that value, then completing based on that type. +func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { + var runtimeType *Object if returnType, ok := returnType.(Abstract); ok { @@ -646,10 +658,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie } return completeObjectValue(eCtx, runtimeType, fieldASTs, info, result) - } - -// completeObjectValue completes an Object value by evaluating all sub-selections func completeObjectValue(eCtx *ExecutionContext, returnType *Object, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { // If there is an isTypeOf predicate function, call it with the From 2f153863445706f08b1ec4a6e49202a7f0002cf7 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 12:06:06 +0800 Subject: [PATCH 08/32] Add invariant for unreachable condition --- executor.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 2c854f91..48b68b91 100644 --- a/executor.go +++ b/executor.go @@ -633,8 +633,13 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie } // Not reachable + err := invariant(false, + fmt.Sprintf(`Cannot complete value of unexpected type "%v."`, returnType), + ) + if err != nil { + panic(gqlerrors.FormatError(err)) + } return nil - } // completeObjectValue completes value of an Abstract type (Union / Interface) by determining the runtime type From 4fb792e07053c3699d3c97dd9e9e112db0f17c9e Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 12:07:33 +0800 Subject: [PATCH 09/32] Fix error message for missing operation --- executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 48b68b91..941b40b3 100644 --- a/executor.go +++ b/executor.go @@ -106,7 +106,7 @@ func buildExecutionContext(p BuildExecutionCtxParams) (*ExecutionContext, error) } if operation == nil { - if p.OperationName == "" { + if p.OperationName != "" { return nil, fmt.Errorf(`Unknown operation named "%v".`, p.OperationName) } return nil, fmt.Errorf(`Must provide an operation`) From 58f2928b94007b502f882eee4a9a1f82dd118603 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 16:59:34 +0800 Subject: [PATCH 10/32] [RFC] Proposed change to directive location introspection This proposes a change to how we represent the ability to validate the locations of directives via introspection. Specifically, this deprecates `onField`, `onFragment`, and `onOperation` in favor of `locations` which is a list of `__DirectiveLocation`. **Rationale:** This allows for a more fine-grained validation of directive placement, now you can assert that a directive is allowed on queries but not mutations, or allowed on fragment definitions but not on fragment spreads. Also, this makes expanding the locations a directive is allowed to be placed easier to do, as future expansions will not affect the introspection API. This should be considered a prereq to #265. Finally, this is a prereq to a forthcoming RFC to add directives to the type schema language, one of the last missing pieces to represent a full schema using this language. Currently considering something like: ``` directive @skip(if: Boolean) on FIELD, FRAGMENT_SPREAD, INLINE_FRAGMENT ``` **Drawbacks:** Any change to the introspection API is a challenge. Especially so for graphql-js which is used as both client tools via Graph*i*QL and as a node.js server. To account for this, I've left the existing fields as deprecated, and continued to support these deprecated fields in `buildClientSchema`, which is used by Graph*i*QL. While graphql-js will likely continue to expose these deprecated fields for some time to come, the spec itself should not include these fields if this change is reflected. Commit: e89c19d2ce584f924c2e0e472a61bb805ce260d4 [e89c19d] Parents: 57d71e108a Author: Lee Byron Date: 18 March 2016 at 7:33:28 AM SGT Commit Date: 18 March 2016 at 7:57:11 AM SGT Labels: HEAD --- directives.go | 66 +++++++++++++++----- introspection.go | 96 ++++++++++++++++++++++++++++- introspection_test.go | 95 ++++++++++++++++++++++++---- rules.go | 59 ++++++++++++++---- rules_known_directives_rule_test.go | 6 +- schema.go | 6 ++ testutil/introspection_query.go | 4 +- testutil/rules_test_harness.go | 4 +- 8 files changed, 284 insertions(+), 52 deletions(-) diff --git a/directives.go b/directives.go index 63411104..3dc98b78 100644 --- a/directives.go +++ b/directives.go @@ -1,28 +1,58 @@ package graphql +const ( + DirectiveLocationQuery = "QUERY" + DirectiveLocationMutation = "MUTATION" + DirectiveLocationSubscription = "SUBSCRIPTION" + DirectiveLocationField = "FIELD" + DirectiveLocationFragmentDefinition = "FRAGMENT_DEFINITION" + DirectiveLocationFragmentSpread = "FRAGMENT_SPREAD" + DirectiveLocationInlineFragment = "INLINE_FRAGMENT" +) + // Directive structs are used by the GraphQL runtime as a way of modifying execution // behavior. Type system creators will usually not create these directly. type Directive struct { Name string `json:"name"` Description string `json:"description"` + Locations []string `json:"locations"` Args []*Argument `json:"args"` - OnOperation bool `json:"onOperation"` - OnFragment bool `json:"onFragment"` - OnField bool `json:"onField"` + + err error } func NewDirective(config *Directive) *Directive { if config == nil { config = &Directive{} } - return &Directive{ - Name: config.Name, - Description: config.Description, - Args: config.Args, - OnOperation: config.OnOperation, - OnFragment: config.OnFragment, - OnField: config.OnField, + dir := &Directive{} + + // Ensure directive is named + err := invariant(config.Name != "", "Directive must be named.") + if err != nil { + dir.err = err + return dir + } + + // Ensure directive name is valid + err = assertValidName(config.Name) + if err != nil { + dir.err = err + return dir + } + + // Ensure locations are provided for directive + err = invariant(len(config.Locations) > 0, "Must provide locations for directive.") + if err != nil { + dir.err = err + return dir } + + dir.Name = config.Name + dir.Description = config.Description + dir.Locations = config.Locations + dir.Args = config.Args + return dir } // IncludeDirective is used to conditionally include fields or fragments @@ -30,6 +60,11 @@ var IncludeDirective = NewDirective(&Directive{ Name: "include", Description: "Directs the executor to include this field or fragment only when " + "the `if` argument is true.", + Locations: []string{ + DirectiveLocationField, + DirectiveLocationFragmentSpread, + DirectiveLocationInlineFragment, + }, Args: []*Argument{ &Argument{ PrivateName: "if", @@ -37,9 +72,6 @@ var IncludeDirective = NewDirective(&Directive{ PrivateDescription: "Included when true.", }, }, - OnOperation: false, - OnFragment: true, - OnField: true, }) // SkipDirective Used to conditionally skip (exclude) fields or fragments @@ -54,7 +86,9 @@ var SkipDirective = NewDirective(&Directive{ PrivateDescription: "Skipped when true.", }, }, - OnOperation: false, - OnFragment: true, - OnField: true, + Locations: []string{ + DirectiveLocationField, + DirectiveLocationFragmentSpread, + DirectiveLocationInlineFragment, + }, }) diff --git a/introspection.go b/introspection.go index 99e7c04e..1896b5af 100644 --- a/introspection.go +++ b/introspection.go @@ -27,6 +27,7 @@ var inputValueType *Object var enumValueType *Object var typeKindEnum *Enum +var directiveLocationEnum *Enum var SchemaMetaFieldDef *FieldDefinition var TypeMetaFieldDef *FieldDefinition @@ -80,6 +81,42 @@ func init() { }, }) + directiveLocationEnum = NewEnum(EnumConfig{ + Name: "__DirectiveLocation", + Description: "A Directive can be adjacent to many parts of the GraphQL language, a " + + "__DirectiveLocation describes one such possible adjacencies.", + Values: EnumValueConfigMap{ + "QUERY": &EnumValueConfig{ + Value: DirectiveLocationQuery, + Description: "Location adjacent to a query operation.", + }, + "MUTATION": &EnumValueConfig{ + Value: DirectiveLocationMutation, + Description: "Location adjacent to a mutation operation.", + }, + "SUBSCRIPTION": &EnumValueConfig{ + Value: DirectiveLocationSubscription, + Description: "Location adjacent to a subscription operation.", + }, + "FIELD": &EnumValueConfig{ + Value: DirectiveLocationField, + Description: "Location adjacent to a field.", + }, + "FRAGMENT_DEFINITION": &EnumValueConfig{ + Value: DirectiveLocationFragmentDefinition, + Description: "Location adjacent to a fragment definition.", + }, + "FRAGMENT_SPREAD": &EnumValueConfig{ + Value: DirectiveLocationFragmentSpread, + Description: "Location adjacent to a fragment spread.", + }, + "INLINE_FRAGMENT": &EnumValueConfig{ + Value: DirectiveLocationInlineFragment, + Description: "Location adjacent to an inline fragment.", + }, + }, + }) + // Note: some fields (for e.g "fields", "interfaces") are defined later due to cyclic reference typeType = NewObject(ObjectConfig{ Name: "__Type", @@ -228,19 +265,72 @@ func init() { "description": &Field{ Type: String, }, + "locations": &Field{ + Type: NewNonNull(NewList( + NewNonNull(directiveLocationEnum), + )), + }, "args": &Field{ Type: NewNonNull(NewList( NewNonNull(inputValueType), )), }, + // NOTE: the following three fields are deprecated and are no longer part + // of the GraphQL specification. "onOperation": &Field{ - Type: NewNonNull(Boolean), + DeprecationReason: "Use `locations`.", + Type: NewNonNull(Boolean), + Resolve: func(p ResolveParams) (interface{}, error) { + if dir, ok := p.Source.(*Directive); ok { + res := false + for _, loc := range dir.Locations { + if loc == DirectiveLocationQuery || + loc == DirectiveLocationMutation || + loc == DirectiveLocationSubscription { + res = true + break + } + } + return res, nil + } + return false, nil + }, }, "onFragment": &Field{ - Type: NewNonNull(Boolean), + DeprecationReason: "Use `locations`.", + Type: NewNonNull(Boolean), + Resolve: func(p ResolveParams) (interface{}, error) { + if dir, ok := p.Source.(*Directive); ok { + res := false + for _, loc := range dir.Locations { + if loc == DirectiveLocationFragmentSpread || + loc == DirectiveLocationInlineFragment || + loc == DirectiveLocationFragmentDefinition { + res = true + break + } + } + return res, nil + } + return false, nil + }, }, "onField": &Field{ - Type: NewNonNull(Boolean), + DeprecationReason: "Use `locations`.", + Type: NewNonNull(Boolean), + Resolve: func(p ResolveParams) (interface{}, error) { + if dir, ok := p.Source.(*Directive); ok { + res := false + for _, loc := range dir.Locations { + if loc == DirectiveLocationField { + res = true + break + } + } + return res, nil + } + return false, nil + }, }, }, }) diff --git a/introspection_test.go b/introspection_test.go index 6425b9db..3b70ed88 100644 --- a/introspection_test.go +++ b/introspection_test.go @@ -626,6 +626,28 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { "isDeprecated": false, "deprecationReason": nil, }, + map[string]interface{}{ + "name": "locations", + "args": []interface{}{}, + "type": map[string]interface{}{ + "kind": "NON_NULL", + "name": nil, + "ofType": map[string]interface{}{ + "kind": "LIST", + "name": nil, + "ofType": map[string]interface{}{ + "kind": "NON_NULL", + "name": nil, + "ofType": map[string]interface{}{ + "kind": "ENUM", + "name": "__DirectiveLocation", + }, + }, + }, + }, + "isDeprecated": false, + "deprecationReason": nil, + }, map[string]interface{}{ "name": "args", "args": []interface{}{}, @@ -660,8 +682,8 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { "ofType": nil, }, }, - "isDeprecated": false, - "deprecationReason": nil, + "isDeprecated": true, + "deprecationReason": "Use `locations`.", }, map[string]interface{}{ "name": "onFragment", @@ -675,8 +697,8 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { "ofType": nil, }, }, - "isDeprecated": false, - "deprecationReason": nil, + "isDeprecated": true, + "deprecationReason": "Use `locations`.", }, map[string]interface{}{ "name": "onField", @@ -690,8 +712,8 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { "ofType": nil, }, }, - "isDeprecated": false, - "deprecationReason": nil, + "isDeprecated": true, + "deprecationReason": "Use `locations`.", }, }, "inputFields": nil, @@ -699,10 +721,60 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { "enumValues": nil, "possibleTypes": nil, }, + map[string]interface{}{ + "kind": "ENUM", + "name": "__DirectiveLocation", + "fields": nil, + "inputFields": nil, + "interfaces": nil, + "enumValues": []interface{}{ + map[string]interface{}{ + "name": "QUERY", + "isDeprecated": false, + "deprecationReason": nil, + }, + map[string]interface{}{ + "name": "MUTATION", + "isDeprecated": false, + "deprecationReason": nil, + }, + map[string]interface{}{ + "name": "SUBSCRIPTION", + "isDeprecated": false, + "deprecationReason": nil, + }, + map[string]interface{}{ + "name": "FIELD", + "isDeprecated": false, + "deprecationReason": nil, + }, + map[string]interface{}{ + "name": "FRAGMENT_DEFINITION", + "isDeprecated": false, + "deprecationReason": nil, + }, + map[string]interface{}{ + "name": "FRAGMENT_SPREAD", + "isDeprecated": false, + "deprecationReason": nil, + }, + map[string]interface{}{ + "name": "INLINE_FRAGMENT", + "isDeprecated": false, + "deprecationReason": nil, + }, + }, + "possibleTypes": nil, + }, }, "directives": []interface{}{ map[string]interface{}{ "name": "include", + "locations": []interface{}{ + "FIELD", + "FRAGMENT_SPREAD", + "INLINE_FRAGMENT", + }, "args": []interface{}{ map[string]interface{}{ "defaultValue": nil, @@ -718,12 +790,14 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { }, }, }, - "onOperation": false, - "onFragment": true, - "onField": true, }, map[string]interface{}{ "name": "skip", + "locations": []interface{}{ + "FIELD", + "FRAGMENT_SPREAD", + "INLINE_FRAGMENT", + }, "args": []interface{}{ map[string]interface{}{ "defaultValue": nil, @@ -739,9 +813,6 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { }, }, }, - "onOperation": false, - "onFragment": true, - "onField": true, }, }, }, diff --git a/rules.go b/rules.go index 1b518846..f20a3164 100644 --- a/rules.go +++ b/rules.go @@ -461,6 +461,10 @@ func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance } } +func MisplaceDirectiveMessage(directiveName string, location string) string { + return fmt.Sprintf(`Directive "%v" may not be used on %v.`, directiveName, location) +} + // KnownDirectivesRule Known directives // // A GraphQL document is only valid if all `@directives` are known by the @@ -501,26 +505,26 @@ func KnownDirectivesRule(context *ValidationContext) *ValidationRuleInstance { return action, result } - if appliedTo.GetKind() == kinds.OperationDefinition && directiveDef.OnOperation == false { - reportError( - context, - fmt.Sprintf(`Directive "%v" may not be used on "%v".`, nodeName, "operation"), - []ast.Node{node}, - ) + candidateLocation := getLocationForAppliedNode(appliedTo) + + directiveHasLocation := false + for _, loc := range directiveDef.Locations { + if loc == candidateLocation { + directiveHasLocation = true + break + } } - if appliedTo.GetKind() == kinds.Field && directiveDef.OnField == false { + + if candidateLocation == "" { reportError( context, - fmt.Sprintf(`Directive "%v" may not be used on "%v".`, nodeName, "field"), + MisplaceDirectiveMessage(nodeName, node.GetKind()), []ast.Node{node}, ) - } - if (appliedTo.GetKind() == kinds.FragmentSpread || - appliedTo.GetKind() == kinds.InlineFragment || - appliedTo.GetKind() == kinds.FragmentDefinition) && directiveDef.OnFragment == false { + } else if !directiveHasLocation { reportError( context, - fmt.Sprintf(`Directive "%v" may not be used on "%v".`, nodeName, "fragment"), + MisplaceDirectiveMessage(nodeName, candidateLocation), []ast.Node{node}, ) } @@ -536,6 +540,35 @@ func KnownDirectivesRule(context *ValidationContext) *ValidationRuleInstance { } } +func getLocationForAppliedNode(appliedTo ast.Node) string { + kind := appliedTo.GetKind() + if kind == kinds.OperationDefinition { + appliedTo, _ := appliedTo.(*ast.OperationDefinition) + if appliedTo.Operation == "query" { + return DirectiveLocationQuery + } + if appliedTo.Operation == "mutation" { + return DirectiveLocationMutation + } + if appliedTo.Operation == "subscription" { + return DirectiveLocationSubscription + } + } + if kind == kinds.Field { + return DirectiveLocationField + } + if kind == kinds.FragmentSpread { + return DirectiveLocationFragmentSpread + } + if kind == kinds.InlineFragment { + return DirectiveLocationInlineFragment + } + if kind == kinds.FragmentDefinition { + return DirectiveLocationFragmentDefinition + } + return "" +} + // KnownFragmentNamesRule Known fragment names // // A GraphQL document is only valid if all `...Fragment` fragment spreads refer diff --git a/rules_known_directives_rule_test.go b/rules_known_directives_rule_test.go index 1a5e7d5e..ea6c07e9 100644 --- a/rules_known_directives_rule_test.go +++ b/rules_known_directives_rule_test.go @@ -79,8 +79,8 @@ func TestValidate_KnownDirectives_WithMisplacedDirectives(t *testing.T) { ...Frag @operationOnly } `, []gqlerrors.FormattedError{ - testutil.RuleError(`Directive "include" may not be used on "operation".`, 2, 17), - testutil.RuleError(`Directive "operationOnly" may not be used on "field".`, 3, 14), - testutil.RuleError(`Directive "operationOnly" may not be used on "fragment".`, 4, 17), + testutil.RuleError(`Directive "include" may not be used on QUERY.`, 2, 17), + testutil.RuleError(`Directive "operationOnly" may not be used on FIELD.`, 3, 14), + testutil.RuleError(`Directive "operationOnly" may not be used on FRAGMENT_SPREAD.`, 4, 17), }) } diff --git a/schema.go b/schema.go index 108cdbac..758ea1dd 100644 --- a/schema.go +++ b/schema.go @@ -62,6 +62,12 @@ func NewSchema(config SchemaConfig) (Schema, error) { SkipDirective, } } + // Ensure directive definitions are error-free + for _, dir := range schema.directives { + if dir.err != nil { + return schema, dir.err + } + } // Build type map now to detect any errors within this schema. typeMap := TypeMap{} diff --git a/testutil/introspection_query.go b/testutil/introspection_query.go index 555ad9df..a46153c7 100644 --- a/testutil/introspection_query.go +++ b/testutil/introspection_query.go @@ -12,12 +12,10 @@ var IntrospectionQuery = ` directives { name description + locations args { ...InputValue } - onOperation - onFragment - onField } } } diff --git a/testutil/rules_test_harness.go b/testutil/rules_test_harness.go index 3691f6c4..e053ffa5 100644 --- a/testutil/rules_test_harness.go +++ b/testutil/rules_test_harness.go @@ -460,8 +460,8 @@ func init() { Query: queryRoot, Directives: []*graphql.Directive{ graphql.NewDirective(&graphql.Directive{ - Name: "operationOnly", - OnOperation: true, + Name: "operationOnly", + Locations: []string{graphql.DirectiveLocationQuery}, }), graphql.IncludeDirective, graphql.SkipDirective, From 2f4057244ca7a9784830124253680989a7bcfdb0 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 17:21:35 +0800 Subject: [PATCH 11/32] Fix error message for missing operation with tests to confirm error Add tests confirming #319 Commit: 3278e861837cd3f7d17eaec54f3f04b175300826 [3278e86] Parents: 3ce90faf26 Author: Lee Byron Date: 23 March 2 --- executor.go | 2 +- executor_test.go | 145 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 143 insertions(+), 4 deletions(-) diff --git a/executor.go b/executor.go index 941b40b3..f54951c9 100644 --- a/executor.go +++ b/executor.go @@ -109,7 +109,7 @@ func buildExecutionContext(p BuildExecutionCtxParams) (*ExecutionContext, error) if p.OperationName != "" { return nil, fmt.Errorf(`Unknown operation named "%v".`, p.OperationName) } - return nil, fmt.Errorf(`Must provide an operation`) + return nil, fmt.Errorf(`Must provide an operation.`) } variableValues, err := getVariableValues(p.Schema, operation.GetVariableDefinitions(), p.Args) diff --git a/executor_test.go b/executor_test.go index f7915ec6..e9fc2592 100644 --- a/executor_test.go +++ b/executor_test.go @@ -521,7 +521,7 @@ func TestNullsOutErrorSubtrees(t *testing.T) { } } -func TestUsesTheInlineOperationIfNoOperationIsProvided(t *testing.T) { +func TestUsesTheInlineOperationIfNoOperationNameIsProvided(t *testing.T) { doc := `{ a }` data := map[string]interface{}{ @@ -566,7 +566,7 @@ func TestUsesTheInlineOperationIfNoOperationIsProvided(t *testing.T) { } } -func TestUsesTheOnlyOperationIfNoOperationIsProvided(t *testing.T) { +func TestUsesTheOnlyOperationIfNoOperationNameIsProvided(t *testing.T) { doc := `query Example { a }` data := map[string]interface{}{ @@ -611,7 +611,100 @@ func TestUsesTheOnlyOperationIfNoOperationIsProvided(t *testing.T) { } } -func TestThrowsIfNoOperationIsProvidedWithMultipleOperations(t *testing.T) { +func TestUsesTheNamedOperationIfOperationNameIsProvided(t *testing.T) { + + doc := `query Example { first: a } query OtherExample { second: a }` + data := map[string]interface{}{ + "a": "b", + } + + expected := &graphql.Result{ + Data: map[string]interface{}{ + "second": "b", + }, + } + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Type", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + }) + if err != nil { + t.Fatalf("Error in schema %v", err.Error()) + } + + // parse query + ast := testutil.TestParse(t, doc) + + // execute + ep := graphql.ExecuteParams{ + Schema: schema, + AST: ast, + Root: data, + OperationName: "OtherExample", + } + result := testutil.TestExecute(t, ep) + if len(result.Errors) > 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + } +} + +func TestThrowsIfNoOperationIsProvided(t *testing.T) { + + doc := `fragment Example on Type { a }` + data := map[string]interface{}{ + "a": "b", + } + + expectedErrors := []gqlerrors.FormattedError{ + gqlerrors.FormattedError{ + Message: "Must provide an operation.", + Locations: []location.SourceLocation{}, + }, + } + + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Type", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + }) + if err != nil { + t.Fatalf("Error in schema %v", err.Error()) + } + + // parse query + ast := testutil.TestParse(t, doc) + + // execute + ep := graphql.ExecuteParams{ + Schema: schema, + AST: ast, + Root: data, + } + result := testutil.TestExecute(t, ep) + if len(result.Errors) != 1 { + t.Fatalf("wrong result, expected len(1) unexpected len: %v", len(result.Errors)) + } + if result.Data != nil { + t.Fatalf("wrong result, expected nil result.Data, got %v", result.Data) + } + if !reflect.DeepEqual(expectedErrors, result.Errors) { + t.Fatalf("unexpected result, Diff: %v", testutil.Diff(expectedErrors, result.Errors)) + } +} +func TestThrowsIfNoOperationNameIsProvidedWithMultipleOperations(t *testing.T) { doc := `query Example { a } query OtherExample { a }` data := map[string]interface{}{ @@ -660,6 +753,52 @@ func TestThrowsIfNoOperationIsProvidedWithMultipleOperations(t *testing.T) { } } +func TestThrowsIfUnknownOperationNameIsProvided(t *testing.T) { + + doc := `query Example { a } query OtherExample { a }` + data := map[string]interface{}{ + "a": "b", + } + + expectedErrors := []gqlerrors.FormattedError{ + gqlerrors.FormattedError{ + Message: `Unknown operation named "UnknownExample".`, + Locations: []location.SourceLocation{}, + }, + } + + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Type", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + }) + if err != nil { + t.Fatalf("Error in schema %v", err.Error()) + } + + // parse query + ast := testutil.TestParse(t, doc) + + // execute + ep := graphql.ExecuteParams{ + Schema: schema, + AST: ast, + Root: data, + OperationName: "UnknownExample", + } + result := testutil.TestExecute(t, ep) + if result.Data != nil { + t.Fatalf("wrong result, expected nil result.Data, got %v", result.Data) + } + if !reflect.DeepEqual(expectedErrors, result.Errors) { + t.Fatalf("unexpected result, Diff: %v", testutil.Diff(expectedErrors, result.Errors)) + } +} func TestUsesTheQuerySchemaForQueries(t *testing.T) { doc := `query Q { a } mutation M { c } subscription S { a }` From 2322d258313c9307a838ae7fe1d19e70f57937a1 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 19:58:02 +0800 Subject: [PATCH 12/32] [RFC] Directives in schema language This adds directives to schema language and to the utilities that use it (schema parser, and buildASTSchema). Directives are one of the few missing pieces from representing a full schema in the schema language. Note: the schema language is still experimental, so there is no corresponding change to the spec yet. DirectiveDefinition : - directive @ Name ArgumentsDefinition? on DirectiveLocations DirectiveLocations : - Name - DirectiveLocations | Name Example: ``` directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT ``` Commit: fdafe32724f2d6dac1ba360f1a440c0e633e75d9 [fdafe32] Parents: bf763b6f53 Author: Lee Byron Date: 18 March 2016 at 10:47:06 AM SGT Commit Date: 23 March 2016 at 6:19:46 AM SGT --- directives.go | 53 +++++++++++----- language/ast/definitions.go | 44 ++++++++++++- language/ast/node.go | 1 + language/kinds/kinds.go | 83 +++++++++++++++---------- language/parser/parser.go | 71 +++++++++++++++++++++ language/printer/printer.go | 16 +++++ language/printer/schema_printer_test.go | 4 ++ language/visitor/visitor.go | 1 + schema-kitchen-sink.graphql | 7 +++ testutil/rules_test_harness.go | 2 +- 10 files changed, 230 insertions(+), 52 deletions(-) diff --git a/directives.go b/directives.go index 3dc98b78..676eab75 100644 --- a/directives.go +++ b/directives.go @@ -21,10 +21,15 @@ type Directive struct { err error } -func NewDirective(config *Directive) *Directive { - if config == nil { - config = &Directive{} - } +// DirectiveConfig options for creating a new GraphQLDirective +type DirectiveConfig struct { + Name string `json:"name"` + Description string `json:"description"` + Locations []string `json:"locations"` + Args FieldConfigArgument `json:"args"` +} + +func NewDirective(config DirectiveConfig) *Directive { dir := &Directive{} // Ensure directive is named @@ -48,15 +53,31 @@ func NewDirective(config *Directive) *Directive { return dir } + args := []*Argument{} + + for argName, argConfig := range config.Args { + err := assertValidName(argName) + if err != nil { + dir.err = err + return dir + } + args = append(args, &Argument{ + PrivateName: argName, + PrivateDescription: argConfig.Description, + Type: argConfig.Type, + DefaultValue: argConfig.DefaultValue, + }) + } + dir.Name = config.Name dir.Description = config.Description dir.Locations = config.Locations - dir.Args = config.Args + dir.Args = args return dir } // IncludeDirective is used to conditionally include fields or fragments -var IncludeDirective = NewDirective(&Directive{ +var IncludeDirective = NewDirective(DirectiveConfig{ Name: "include", Description: "Directs the executor to include this field or fragment only when " + "the `if` argument is true.", @@ -65,25 +86,23 @@ var IncludeDirective = NewDirective(&Directive{ DirectiveLocationFragmentSpread, DirectiveLocationInlineFragment, }, - Args: []*Argument{ - &Argument{ - PrivateName: "if", - Type: NewNonNull(Boolean), - PrivateDescription: "Included when true.", + Args: FieldConfigArgument{ + "if": &ArgumentConfig{ + Type: NewNonNull(Boolean), + Description: "Included when true.", }, }, }) // SkipDirective Used to conditionally skip (exclude) fields or fragments -var SkipDirective = NewDirective(&Directive{ +var SkipDirective = NewDirective(DirectiveConfig{ Name: "skip", Description: "Directs the executor to skip this field or fragment when the `if` " + "argument is true.", - Args: []*Argument{ - &Argument{ - PrivateName: "if", - Type: NewNonNull(Boolean), - PrivateDescription: "Skipped when true.", + Args: FieldConfigArgument{ + "if": &ArgumentConfig{ + Type: NewNonNull(Boolean), + Description: "Skipped when true.", }, }, Locations: []string{ diff --git a/language/ast/definitions.go b/language/ast/definitions.go index a619e78d..52539e7b 100644 --- a/language/ast/definitions.go +++ b/language/ast/definitions.go @@ -5,7 +5,6 @@ import ( ) type Definition interface { - // TODO: determine the minimal set of interface for `Definition` GetOperation() string GetVariableDefinitions() []*VariableDefinition GetSelectionSet() *SelectionSet @@ -17,6 +16,7 @@ type Definition interface { var _ Definition = (*OperationDefinition)(nil) var _ Definition = (*FragmentDefinition)(nil) var _ Definition = (*TypeExtensionDefinition)(nil) +var _ Definition = (*DirectiveDefinition)(nil) var _ Definition = (Definition)(nil) // OperationDefinition implements Node, Definition @@ -192,3 +192,45 @@ func (def *TypeExtensionDefinition) GetSelectionSet() *SelectionSet { func (def *TypeExtensionDefinition) GetOperation() string { return "" } + +// DirectiveDefinition implements Node, Definition +type DirectiveDefinition struct { + Kind string + Loc *Location + Name *Name + Arguments []*InputValueDefinition + Locations []*Name +} + +func NewDirectiveDefinition(def *DirectiveDefinition) *DirectiveDefinition { + if def == nil { + def = &DirectiveDefinition{} + } + return &DirectiveDefinition{ + Kind: kinds.DirectiveDefinition, + Loc: def.Loc, + Name: def.Name, + Arguments: def.Arguments, + Locations: def.Locations, + } +} + +func (def *DirectiveDefinition) GetKind() string { + return def.Kind +} + +func (def *DirectiveDefinition) GetLoc() *Location { + return def.Loc +} + +func (def *DirectiveDefinition) GetVariableDefinitions() []*VariableDefinition { + return []*VariableDefinition{} +} + +func (def *DirectiveDefinition) GetSelectionSet() *SelectionSet { + return &SelectionSet{} +} + +func (def *DirectiveDefinition) GetOperation() string { + return "" +} diff --git a/language/ast/node.go b/language/ast/node.go index 22879877..43b9d63b 100644 --- a/language/ast/node.go +++ b/language/ast/node.go @@ -40,3 +40,4 @@ var _ Node = (*EnumDefinition)(nil) var _ Node = (*EnumValueDefinition)(nil) var _ Node = (*InputObjectDefinition)(nil) var _ Node = (*TypeExtensionDefinition)(nil) +var _ Node = (*DirectiveDefinition)(nil) diff --git a/language/kinds/kinds.go b/language/kinds/kinds.go index d1161763..36e6d3d8 100644 --- a/language/kinds/kinds.go +++ b/language/kinds/kinds.go @@ -1,38 +1,55 @@ package kinds const ( - OperationDefinition = "OperationDefinition" - FragmentDefinition = "FragmentDefinition" - Document = "Document" - SelectionSet = "SelectionSet" - Name = "Name" - Directive = "Directive" - VariableDefinition = "VariableDefinition" - Variable = "Variable" - Named = "Named" // previously NamedType - List = "List" // previously ListType - NonNull = "NonNull" - InlineFragment = "InlineFragment" - FragmentSpread = "FragmentSpread" - Field = "Field" - Array = "Array" - Argument = "Argument" - IntValue = "IntValue" - FloatValue = "FloatValue" - StringValue = "StringValue" - BooleanValue = "BooleanValue" - EnumValue = "EnumValue" - ListValue = "ListValue" - ObjectValue = "ObjectValue" - ObjectField = "ObjectField" - ObjectDefinition = "ObjectDefinition" - FieldDefinition = "FieldDefinition" - InputValueDefinition = "InputValueDefinition" - InterfaceDefinition = "InterfaceDefinition" - UnionDefinition = "UnionDefinition" - ScalarDefinition = "ScalarDefinition" - EnumDefinition = "EnumDefinition" - EnumValueDefinition = "EnumValueDefinition" - InputObjectDefinition = "InputObjectDefinition" + // Name + Name = "Name" + + // Document + Document = "Document" + OperationDefinition = "OperationDefinition" + VariableDefinition = "VariableDefinition" + Variable = "Variable" + SelectionSet = "SelectionSet" + Field = "Field" + Argument = "Argument" + + // Fragments + FragmentSpread = "FragmentSpread" + InlineFragment = "InlineFragment" + FragmentDefinition = "FragmentDefinition" + + // Values + IntValue = "IntValue" + FloatValue = "FloatValue" + StringValue = "StringValue" + BooleanValue = "BooleanValue" + EnumValue = "EnumValue" + ListValue = "ListValue" + ObjectValue = "ObjectValue" + ObjectField = "ObjectField" + + // Directives + Directive = "Directive" + + // Types + Named = "Named" // previously NamedType + List = "List" // previously ListType + NonNull = "NonNull" // previously NonNull + + // Types Definitions + ObjectDefinition = "ObjectDefinition" // previously ObjectTypeDefinition + FieldDefinition = "FieldDefinition" + InputValueDefinition = "InputValueDefinition" + InterfaceDefinition = "InterfaceDefinition" // previously InterfaceTypeDefinition + UnionDefinition = "UnionDefinition" // previously UnionTypeDefinition + ScalarDefinition = "ScalarDefinition" // previously ScalarTypeDefinition + EnumDefinition = "EnumDefinition" // previously EnumTypeDefinition + EnumValueDefinition = "EnumValueDefinition" + InputObjectDefinition = "InputObjectDefinition" // previously InputObjectTypeDefinition + + // Types Extensions TypeExtensionDefinition = "TypeExtensionDefinition" + + // Directive Definitions + DirectiveDefinition = "DirectiveDefinition" ) diff --git a/language/parser/parser.go b/language/parser/parser.go index 7b480c3d..8a7bb07a 100644 --- a/language/parser/parser.go +++ b/language/parser/parser.go @@ -175,6 +175,12 @@ func parseDocument(parser *Parser) (*ast.Document, error) { return nil, err } nodes = append(nodes, node) + case "directive": + node, err := parseDirectiveDefinition(parser) + if err != nil { + return nil, err + } + nodes = append(nodes, node) default: if err := unexpected(parser, lexer.Token{}); err != nil { return nil, err @@ -1196,6 +1202,71 @@ func parseTypeExtensionDefinition(parser *Parser) (*ast.TypeExtensionDefinition, }), nil } +/** + * DirectiveDefinition : + * - directive @ Name ArgumentsDefinition? on DirectiveLocations + */ +func parseDirectiveDefinition(parser *Parser) (*ast.DirectiveDefinition, error) { + start := parser.Token.Start + _, err := expectKeyWord(parser, "directive") + if err != nil { + return nil, err + } + _, err = expect(parser, lexer.TokenKind[lexer.AT]) + if err != nil { + return nil, err + } + name, err := parseName(parser) + if err != nil { + return nil, err + } + args, err := parseArgumentDefs(parser) + if err != nil { + return nil, err + } + _, err = expectKeyWord(parser, "on") + if err != nil { + return nil, err + } + locations, err := parseDirectiveLocations(parser) + if err != nil { + return nil, err + } + + return ast.NewDirectiveDefinition(&ast.DirectiveDefinition{ + Loc: loc(parser, start), + Name: name, + Arguments: args, + Locations: locations, + }), nil +} + +/** + * DirectiveLocations : + * - Name + * - DirectiveLocations | Name + */ + +func parseDirectiveLocations(parser *Parser) ([]*ast.Name, error) { + locations := []*ast.Name{} + for { + name, err := parseName(parser) + if err != nil { + return locations, err + } + locations = append(locations, name) + + hasPipe, err := skip(parser, lexer.TokenKind[lexer.PIPE]) + if err != nil { + return locations, err + } + if !hasPipe { + break + } + } + return locations, nil +} + /* Core parsing utility functions */ // Returns a location object, used to identify the place in diff --git a/language/printer/printer.go b/language/printer/printer.go index 4cca90e4..ae7ca784 100644 --- a/language/printer/printer.go +++ b/language/printer/printer.go @@ -571,6 +571,22 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ } return visitor.ActionNoChange, nil }, + "DirectiveDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { + switch node := p.Node.(type) { + case *ast.DirectiveDefinition: + args := wrap("(", join(toSliceString(node.Arguments), ", "), ")") + str := fmt.Sprintf("directive @%v%v on %v", node.Name, args, join(toSliceString(node.Locations), " | ")) + return visitor.ActionUpdate, str + case map[string]interface{}: + name := getMapValueString(node, "Name") + locations := toSliceString(getMapValue(node, "Locations")) + args := toSliceString(getMapValue(node, "Arguments")) + argsStr := wrap("(", join(args, ", "), ")") + str := fmt.Sprintf("directive @%v%v on %v", name, argsStr, join(locations, " | ")) + return visitor.ActionUpdate, str + } + return visitor.ActionNoChange, nil + }, } func Print(astNode ast.Node) (printed interface{}) { diff --git a/language/printer/schema_printer_test.go b/language/printer/schema_printer_test.go index 344c8ac4..cb5b258d 100644 --- a/language/printer/schema_printer_test.go +++ b/language/printer/schema_printer_test.go @@ -84,6 +84,10 @@ input InputType { extend type Foo { seven(argument: [String]): Type } + +directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT ` results := printer.Print(astDoc) if !reflect.DeepEqual(expected, results) { diff --git a/language/visitor/visitor.go b/language/visitor/visitor.go index 62c50149..759e3c86 100644 --- a/language/visitor/visitor.go +++ b/language/visitor/visitor.go @@ -117,6 +117,7 @@ var QueryDocumentKeys = KeyMap{ "Fields", }, "TypeExtensionDefinition": []string{"Definition"}, + "DirectiveDefinition": []string{"Name", "Arguments", "Locations"}, } type stack struct { diff --git a/schema-kitchen-sink.graphql b/schema-kitchen-sink.graphql index 052dfbe4..b67572fe 100644 --- a/schema-kitchen-sink.graphql +++ b/schema-kitchen-sink.graphql @@ -31,3 +31,10 @@ input InputType { extend type Foo { seven(argument: [String]): Type } + +directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +directive @include(if: Boolean!) + on FIELD + | FRAGMENT_SPREAD + | INLINE_FRAGMENT diff --git a/testutil/rules_test_harness.go b/testutil/rules_test_harness.go index e053ffa5..688c47a3 100644 --- a/testutil/rules_test_harness.go +++ b/testutil/rules_test_harness.go @@ -459,7 +459,7 @@ func init() { schema, err := graphql.NewSchema(graphql.SchemaConfig{ Query: queryRoot, Directives: []*graphql.Directive{ - graphql.NewDirective(&graphql.Directive{ + graphql.NewDirective(graphql.DirectiveConfig{ Name: "operationOnly", Locations: []string{graphql.DirectiveLocationQuery}, }), From d66b3f3c6b72fd8b6c386ad63217cd994e2d1d30 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 20:25:02 +0800 Subject: [PATCH 13/32] Updating schema parser to more closely match current state of RFC https://github.com/facebook/graphql/pull/90 Commit: b0885a038ec0e654962d69fb910ac86659279579 [b0885a0] Parents: fdafe32724 Author: Lee Byron Date: 23 March 2016 at 6:35:37 AM SGT Commit Date: 23 March 2016 at 6:35:39 AM SGT --- language/ast/definitions.go | 4 +- language/ast/type_definitions.go | 117 ++++++++++++++++++------------- language/kinds/kinds.go | 4 +- language/parser/parser.go | 53 +++++++------- language/printer/printer.go | 34 +++++---- 5 files changed, 119 insertions(+), 93 deletions(-) diff --git a/language/ast/definitions.go b/language/ast/definitions.go index 52539e7b..7796df67 100644 --- a/language/ast/definitions.go +++ b/language/ast/definitions.go @@ -15,9 +15,7 @@ type Definition interface { // Ensure that all definition types implements Definition interface var _ Definition = (*OperationDefinition)(nil) var _ Definition = (*FragmentDefinition)(nil) -var _ Definition = (*TypeExtensionDefinition)(nil) -var _ Definition = (*DirectiveDefinition)(nil) -var _ Definition = (Definition)(nil) +var _ Definition = (TypeSystemDefinition)(nil) // OperationDefinition implements Node, Definition type OperationDefinition struct { diff --git a/language/ast/type_definitions.go b/language/ast/type_definitions.go index 95810070..f06970fa 100644 --- a/language/ast/type_definitions.go +++ b/language/ast/type_definitions.go @@ -4,13 +4,74 @@ import ( "github.com/graphql-go/graphql/language/kinds" ) -// Ensure that all typeDefinition types implements Definition interface -var _ Definition = (*ObjectDefinition)(nil) -var _ Definition = (*InterfaceDefinition)(nil) -var _ Definition = (*UnionDefinition)(nil) -var _ Definition = (*ScalarDefinition)(nil) -var _ Definition = (*EnumDefinition)(nil) -var _ Definition = (*InputObjectDefinition)(nil) +type TypeDefinition interface { + GetOperation() string + GetVariableDefinitions() []*VariableDefinition + GetSelectionSet() *SelectionSet + GetKind() string + GetLoc() *Location +} + +var _ TypeDefinition = (*ScalarDefinition)(nil) +var _ TypeDefinition = (*ObjectDefinition)(nil) +var _ TypeDefinition = (*InterfaceDefinition)(nil) +var _ TypeDefinition = (*UnionDefinition)(nil) +var _ TypeDefinition = (*EnumDefinition)(nil) +var _ TypeDefinition = (*InputObjectDefinition)(nil) + +type TypeSystemDefinition interface { + GetOperation() string + GetVariableDefinitions() []*VariableDefinition + GetSelectionSet() *SelectionSet + GetKind() string + GetLoc() *Location +} + +var _ TypeSystemDefinition = (TypeDefinition)(nil) +var _ TypeSystemDefinition = (*TypeExtensionDefinition)(nil) +var _ TypeSystemDefinition = (*DirectiveDefinition)(nil) + +// ScalarDefinition implements Node, Definition +type ScalarDefinition struct { + Kind string + Loc *Location + Name *Name +} + +func NewScalarDefinition(def *ScalarDefinition) *ScalarDefinition { + if def == nil { + def = &ScalarDefinition{} + } + return &ScalarDefinition{ + Kind: kinds.ScalarDefinition, + Loc: def.Loc, + Name: def.Name, + } +} + +func (def *ScalarDefinition) GetKind() string { + return def.Kind +} + +func (def *ScalarDefinition) GetLoc() *Location { + return def.Loc +} + +func (def *ScalarDefinition) GetName() *Name { + return def.Name +} + +func (def *ScalarDefinition) GetVariableDefinitions() []*VariableDefinition { + return []*VariableDefinition{} +} + +func (def *ScalarDefinition) GetSelectionSet() *SelectionSet { + return &SelectionSet{} +} + +func (def *ScalarDefinition) GetOperation() string { + return "" +} // ObjectDefinition implements Node, Definition type ObjectDefinition struct { @@ -206,48 +267,6 @@ func (def *UnionDefinition) GetOperation() string { return "" } -// ScalarDefinition implements Node, Definition -type ScalarDefinition struct { - Kind string - Loc *Location - Name *Name -} - -func NewScalarDefinition(def *ScalarDefinition) *ScalarDefinition { - if def == nil { - def = &ScalarDefinition{} - } - return &ScalarDefinition{ - Kind: kinds.ScalarDefinition, - Loc: def.Loc, - Name: def.Name, - } -} - -func (def *ScalarDefinition) GetKind() string { - return def.Kind -} - -func (def *ScalarDefinition) GetLoc() *Location { - return def.Loc -} - -func (def *ScalarDefinition) GetName() *Name { - return def.Name -} - -func (def *ScalarDefinition) GetVariableDefinitions() []*VariableDefinition { - return []*VariableDefinition{} -} - -func (def *ScalarDefinition) GetSelectionSet() *SelectionSet { - return &SelectionSet{} -} - -func (def *ScalarDefinition) GetOperation() string { - return "" -} - // EnumDefinition implements Node, Definition type EnumDefinition struct { Kind string diff --git a/language/kinds/kinds.go b/language/kinds/kinds.go index 36e6d3d8..c4ca662c 100644 --- a/language/kinds/kinds.go +++ b/language/kinds/kinds.go @@ -36,13 +36,15 @@ const ( List = "List" // previously ListType NonNull = "NonNull" // previously NonNull + // Type System Definitions + // Types Definitions + ScalarDefinition = "ScalarDefinition" // previously ScalarTypeDefinition ObjectDefinition = "ObjectDefinition" // previously ObjectTypeDefinition FieldDefinition = "FieldDefinition" InputValueDefinition = "InputValueDefinition" InterfaceDefinition = "InterfaceDefinition" // previously InterfaceTypeDefinition UnionDefinition = "UnionDefinition" // previously UnionTypeDefinition - ScalarDefinition = "ScalarDefinition" // previously ScalarTypeDefinition EnumDefinition = "EnumDefinition" // previously EnumTypeDefinition EnumValueDefinition = "EnumValueDefinition" InputObjectDefinition = "InputObjectDefinition" // previously InputObjectTypeDefinition diff --git a/language/parser/parser.go b/language/parser/parser.go index 8a7bb07a..db482ec6 100644 --- a/language/parser/parser.go +++ b/language/parser/parser.go @@ -133,6 +133,12 @@ func parseDocument(parser *Parser) (*ast.Document, error) { return nil, err } nodes = append(nodes, node) + case "scalar": + node, err := parseScalarTypeDefinition(parser) + if err != nil { + return nil, err + } + nodes = append(nodes, node) case "type": node, err := parseObjectTypeDefinition(parser) if err != nil { @@ -151,12 +157,6 @@ func parseDocument(parser *Parser) (*ast.Document, error) { return nil, err } nodes = append(nodes, node) - case "scalar": - node, err := parseScalarTypeDefinition(parser) - if err != nil { - return nil, err - } - nodes = append(nodes, node) case "enum": node, err := parseEnumTypeDefinition(parser) if err != nil { @@ -860,6 +860,26 @@ func parseNamed(parser *Parser) (*ast.Named, error) { /* Implements the parsing rules in the Type Definition section. */ +/** + * ScalarTypeDefinition : scalar Name + */ +func parseScalarTypeDefinition(parser *Parser) (*ast.ScalarDefinition, error) { + start := parser.Token.Start + _, err := expectKeyWord(parser, "scalar") + if err != nil { + return nil, err + } + name, err := parseName(parser) + if err != nil { + return nil, err + } + def := ast.NewScalarDefinition(&ast.ScalarDefinition{ + Name: name, + Loc: loc(parser, start), + }) + return def, nil +} + /** * ObjectTypeDefinition : type Name ImplementsInterfaces? { FieldDefinition+ } */ @@ -1085,26 +1105,6 @@ func parseUnionMembers(parser *Parser) ([]*ast.Named, error) { return members, nil } -/** - * ScalarTypeDefinition : scalar Name - */ -func parseScalarTypeDefinition(parser *Parser) (*ast.ScalarDefinition, error) { - start := parser.Token.Start - _, err := expectKeyWord(parser, "scalar") - if err != nil { - return nil, err - } - name, err := parseName(parser) - if err != nil { - return nil, err - } - def := ast.NewScalarDefinition(&ast.ScalarDefinition{ - Name: name, - Loc: loc(parser, start), - }) - return def, nil -} - /** * EnumTypeDefinition : enum Name { EnumValueDefinition+ } */ @@ -1246,7 +1246,6 @@ func parseDirectiveDefinition(parser *Parser) (*ast.DirectiveDefinition, error) * - Name * - DirectiveLocations | Name */ - func parseDirectiveLocations(parser *Parser) ([]*ast.Name, error) { locations := []*ast.Name{} for { diff --git a/language/printer/printer.go b/language/printer/printer.go index ae7ca784..9bfa451a 100644 --- a/language/printer/printer.go +++ b/language/printer/printer.go @@ -130,6 +130,8 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ } return visitor.ActionNoChange, nil }, + + // Document "Document": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.Document: @@ -258,6 +260,8 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ } return visitor.ActionNoChange, nil }, + + // Fragments "FragmentSpread": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.InlineFragment: @@ -306,6 +310,7 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ return visitor.ActionNoChange, nil }, + // Value "IntValue": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.IntValue: @@ -383,6 +388,7 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ return visitor.ActionNoChange, nil }, + // Directive "Directive": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.Directive: @@ -397,6 +403,7 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ return visitor.ActionNoChange, nil }, + // Type "Named": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.Named: @@ -425,6 +432,20 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ return visitor.ActionNoChange, nil }, + // Type System Definitions + "ScalarDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { + switch node := p.Node.(type) { + case *ast.ScalarDefinition: + name := fmt.Sprintf("%v", node.Name) + str := "scalar " + name + return visitor.ActionUpdate, str + case map[string]interface{}: + name := getMapValueString(node, "Name") + str := "scalar " + name + return visitor.ActionUpdate, str + } + return visitor.ActionNoChange, nil + }, "ObjectDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.ObjectDefinition: @@ -506,19 +527,6 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ } return visitor.ActionNoChange, nil }, - "ScalarDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.ScalarDefinition: - name := fmt.Sprintf("%v", node.Name) - str := "scalar " + name - return visitor.ActionUpdate, str - case map[string]interface{}: - name := getMapValueString(node, "Name") - str := "scalar " + name - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, "EnumDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.EnumDefinition: From e23ac77f2b1e34dfeff02469edca600cb54a3b7e Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Mon, 30 May 2016 23:50:59 +0800 Subject: [PATCH 14/32] [RFC] Add Schema Definition to IDL. This implements the schema definition in the spec proposal in https://github.com/facebook/graphql/pull/90 Adjusts AST, parser, printer, visitor, but also changes the API of buildASTSchema to require a schema definition instead of passing the type names into the function. Commit: 8379e71f7011fe044574f4bdef2a761d18d6cf2c [8379e71] Parents: 176076c8a6 Author: Lee Byron Date: 23 March 2016 at 7:38:15 AM SGT Labels: HEAD --- executor.go | 8 +- language/ast/definitions.go | 9 +- language/ast/node.go | 4 +- language/ast/type_definitions.go | 67 +++++ language/kinds/kinds.go | 2 + language/parser/parser.go | 100 ++++++- language/printer/printer.go | 36 ++- language/printer/printer_old.go | 359 ------------------------ language/printer/schema_printer_test.go | 7 +- language/visitor/visitor.go | 9 +- rules.go | 6 +- schema-kitchen-sink.graphql | 5 + type_info.go | 6 +- 13 files changed, 226 insertions(+), 392 deletions(-) delete mode 100644 language/printer/printer_old.go diff --git a/executor.go b/executor.go index f54951c9..44999213 100644 --- a/executor.go +++ b/executor.go @@ -152,7 +152,7 @@ func executeOperation(p ExecuteOperationParams) *Result { Fields: fields, } - if p.Operation.GetOperation() == "mutation" { + if p.Operation.GetOperation() == ast.OperationTypeMutation { return executeFieldsSerially(executeFieldsParams) } return executeFields(executeFieldsParams) @@ -166,9 +166,9 @@ func getOperationRootType(schema Schema, operation ast.Definition) (*Object, err } switch operation.GetOperation() { - case "query": + case ast.OperationTypeQuery: return schema.QueryType(), nil - case "mutation": + case ast.OperationTypeMutation: mutationType := schema.MutationType() if mutationType.PrivateName == "" { return nil, gqlerrors.NewError( @@ -181,7 +181,7 @@ func getOperationRootType(schema Schema, operation ast.Definition) (*Object, err ) } return mutationType, nil - case "subscription": + case ast.OperationTypeSubscription: subscriptionType := schema.SubscriptionType() if subscriptionType.PrivateName == "" { return nil, gqlerrors.NewError( diff --git a/language/ast/definitions.go b/language/ast/definitions.go index 7796df67..c224a9fd 100644 --- a/language/ast/definitions.go +++ b/language/ast/definitions.go @@ -15,7 +15,14 @@ type Definition interface { // Ensure that all definition types implements Definition interface var _ Definition = (*OperationDefinition)(nil) var _ Definition = (*FragmentDefinition)(nil) -var _ Definition = (TypeSystemDefinition)(nil) +var _ Definition = (TypeSystemDefinition)(nil) // experimental non-spec addition. + +// Note: subscription is an experimental non-spec addition. +const ( + OperationTypeQuery = "query" + OperationTypeMutation = "mutation" + OperationTypeSubscription = "subscription" +) // OperationDefinition implements Node, Definition type OperationDefinition struct { diff --git a/language/ast/node.go b/language/ast/node.go index 43b9d63b..cd63a0fc 100644 --- a/language/ast/node.go +++ b/language/ast/node.go @@ -30,12 +30,14 @@ var _ Node = (*Directive)(nil) var _ Node = (*Named)(nil) var _ Node = (*List)(nil) var _ Node = (*NonNull)(nil) +var _ Node = (*SchemaDefinition)(nil) +var _ Node = (*OperationTypeDefinition)(nil) +var _ Node = (*ScalarDefinition)(nil) var _ Node = (*ObjectDefinition)(nil) var _ Node = (*FieldDefinition)(nil) var _ Node = (*InputValueDefinition)(nil) var _ Node = (*InterfaceDefinition)(nil) var _ Node = (*UnionDefinition)(nil) -var _ Node = (*ScalarDefinition)(nil) var _ Node = (*EnumDefinition)(nil) var _ Node = (*EnumValueDefinition)(nil) var _ Node = (*InputObjectDefinition)(nil) diff --git a/language/ast/type_definitions.go b/language/ast/type_definitions.go index f06970fa..dd7940c9 100644 --- a/language/ast/type_definitions.go +++ b/language/ast/type_definitions.go @@ -27,10 +27,77 @@ type TypeSystemDefinition interface { GetLoc() *Location } +var _ TypeSystemDefinition = (*SchemaDefinition)(nil) var _ TypeSystemDefinition = (TypeDefinition)(nil) var _ TypeSystemDefinition = (*TypeExtensionDefinition)(nil) var _ TypeSystemDefinition = (*DirectiveDefinition)(nil) +// SchemaDefinition implements Node, Definition +type SchemaDefinition struct { + Kind string + Loc *Location + OperationTypes []*OperationTypeDefinition +} + +func NewSchemaDefinition(def *SchemaDefinition) *SchemaDefinition { + if def == nil { + def = &SchemaDefinition{} + } + return &SchemaDefinition{ + Kind: kinds.SchemaDefinition, + Loc: def.Loc, + OperationTypes: def.OperationTypes, + } +} + +func (def *SchemaDefinition) GetKind() string { + return def.Kind +} + +func (def *SchemaDefinition) GetLoc() *Location { + return def.Loc +} + +func (def *SchemaDefinition) GetVariableDefinitions() []*VariableDefinition { + return []*VariableDefinition{} +} + +func (def *SchemaDefinition) GetSelectionSet() *SelectionSet { + return &SelectionSet{} +} + +func (def *SchemaDefinition) GetOperation() string { + return "" +} + +// ScalarDefinition implements Node, Definition +type OperationTypeDefinition struct { + Kind string + Loc *Location + Operation string + Type *Named +} + +func NewOperationTypeDefinition(def *OperationTypeDefinition) *OperationTypeDefinition { + if def == nil { + def = &OperationTypeDefinition{} + } + return &OperationTypeDefinition{ + Kind: kinds.OperationTypeDefinition, + Loc: def.Loc, + Operation: def.Operation, + Type: def.Type, + } +} + +func (def *OperationTypeDefinition) GetKind() string { + return def.Kind +} + +func (def *OperationTypeDefinition) GetLoc() *Location { + return def.Loc +} + // ScalarDefinition implements Node, Definition type ScalarDefinition struct { Kind string diff --git a/language/kinds/kinds.go b/language/kinds/kinds.go index c4ca662c..40bc994e 100644 --- a/language/kinds/kinds.go +++ b/language/kinds/kinds.go @@ -37,6 +37,8 @@ const ( NonNull = "NonNull" // previously NonNull // Type System Definitions + SchemaDefinition = "SchemaDefinition" + OperationTypeDefinition = "OperationTypeDefinition" // Types Definitions ScalarDefinition = "ScalarDefinition" // previously ScalarTypeDefinition diff --git a/language/parser/parser.go b/language/parser/parser.go index db482ec6..d1e01c50 100644 --- a/language/parser/parser.go +++ b/language/parser/parser.go @@ -133,6 +133,14 @@ func parseDocument(parser *Parser) (*ast.Document, error) { return nil, err } nodes = append(nodes, node) + + // Note: the Type System IDL is an experimental non-spec addition. + case "schema": + node, err := parseSchemaDefinition(parser) + if err != nil { + return nil, err + } + nodes = append(nodes, node) case "scalar": node, err := parseScalarTypeDefinition(parser) if err != nil { @@ -204,8 +212,6 @@ func parseDocument(parser *Parser) (*ast.Document, error) { * OperationDefinition : * - SelectionSet * - OperationType Name? VariableDefinitions? Directives? SelectionSet - * - * OperationType : one of query mutation */ func parseOperationDefinition(parser *Parser) (*ast.OperationDefinition, error) { start := parser.Token.Start @@ -215,27 +221,17 @@ func parseOperationDefinition(parser *Parser) (*ast.OperationDefinition, error) return nil, err } return ast.NewOperationDefinition(&ast.OperationDefinition{ - Operation: "query", + Operation: ast.OperationTypeQuery, Directives: []*ast.Directive{}, SelectionSet: selectionSet, Loc: loc(parser, start), }), nil } - operationToken, err := expect(parser, lexer.TokenKind[lexer.NAME]) + operation, err := parseOperationType(parser) if err != nil { return nil, err } - operation := "" - switch operationToken.Value { - case "mutation": - fallthrough - case "subscription": - fallthrough - case "query": - operation = operationToken.Value - default: - return nil, unexpected(parser, operationToken) - } + var name *ast.Name if peek(parser, lexer.TokenKind[lexer.NAME]) { name, err = parseName(parser) @@ -262,6 +258,26 @@ func parseOperationDefinition(parser *Parser) (*ast.OperationDefinition, error) }), nil } +/** + * OperationType : one of query mutation subscription + */ +func parseOperationType(parser *Parser) (string, error) { + operationToken, err := expect(parser, lexer.TokenKind[lexer.NAME]) + if err != nil { + return "", err + } + switch operationToken.Value { + case ast.OperationTypeQuery: + return operationToken.Value, nil + case ast.OperationTypeMutation: + return operationToken.Value, nil + case ast.OperationTypeSubscription: + return operationToken.Value, nil + default: + return "", unexpected(parser, operationToken) + } +} + /** * VariableDefinitions : ( VariableDefinition+ ) */ @@ -860,6 +876,60 @@ func parseNamed(parser *Parser) (*ast.Named, error) { /* Implements the parsing rules in the Type Definition section. */ +/** + * SchemaDefinition : schema { OperationTypeDefinition+ } + * + * OperationTypeDefinition : OperationType : NamedType + */ +func parseSchemaDefinition(parser *Parser) (*ast.SchemaDefinition, error) { + start := parser.Token.Start + _, err := expectKeyWord(parser, "schema") + if err != nil { + return nil, err + } + operationTypesI, err := many( + parser, + lexer.TokenKind[lexer.BRACE_L], + parseOperationTypeDefinition, + lexer.TokenKind[lexer.BRACE_R], + ) + if err != nil { + return nil, err + } + operationTypes := []*ast.OperationTypeDefinition{} + for _, op := range operationTypesI { + if op, ok := op.(*ast.OperationTypeDefinition); ok { + operationTypes = append(operationTypes, op) + } + } + def := ast.NewSchemaDefinition(&ast.SchemaDefinition{ + OperationTypes: operationTypes, + Loc: loc(parser, start), + }) + return def, nil +} + +func parseOperationTypeDefinition(parser *Parser) (interface{}, error) { + start := parser.Token.Start + operation, err := parseOperationType(parser) + if err != nil { + return nil, err + } + _, err = expect(parser, lexer.TokenKind[lexer.COLON]) + if err != nil { + return nil, err + } + ttype, err := parseNamed(parser) + if err != nil { + return nil, err + } + return ast.NewOperationTypeDefinition(&ast.OperationTypeDefinition{ + Operation: operation, + Type: ttype, + Loc: loc(parser, start), + }), nil +} + /** * ScalarTypeDefinition : scalar Name */ diff --git a/language/printer/printer.go b/language/printer/printer.go index 9bfa451a..3b10f05a 100644 --- a/language/printer/printer.go +++ b/language/printer/printer.go @@ -6,6 +6,7 @@ import ( "github.com/graphql-go/graphql/language/ast" "github.com/graphql-go/graphql/language/visitor" + "github.com/kr/pretty" "reflect" ) @@ -146,7 +147,7 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ "OperationDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.OperationDefinition: - op := node.Operation + op := string(node.Operation) name := fmt.Sprintf("%v", node.Name) varDefs := wrap("(", join(toSliceString(node.VariableDefinitions), ", "), ")") @@ -155,7 +156,7 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ // Anonymous queries with no directives or variable definitions can use // the query short form. str := "" - if name == "" && directives == "" && varDefs == "" && op == "query" { + if name == "" && directives == "" && varDefs == "" && op == ast.OperationTypeQuery { str = selectionSet } else { str = join([]string{ @@ -175,7 +176,7 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ directives := join(toSliceString(getMapValue(node, "Directives")), " ") selectionSet := getMapValueString(node, "SelectionSet") str := "" - if name == "" && directives == "" && varDefs == "" && op == "query" { + if name == "" && directives == "" && varDefs == "" && op == ast.OperationTypeQuery { str = selectionSet } else { str = join([]string{ @@ -433,6 +434,35 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ }, // Type System Definitions + "SchemaDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { + pretty.Println("===SchemaDefinitions", p.Node) + switch node := p.Node.(type) { + case *ast.SchemaDefinition: + operationTypesBlock := block(node.OperationTypes) + str := fmt.Sprintf("schema %v", operationTypesBlock) + return visitor.ActionUpdate, str + case map[string]interface{}: + operationTypes := toSliceString(getMapValue(node, "OperationTypes")) + operationTypesBlock := block(operationTypes) + str := fmt.Sprintf("schema %v", operationTypesBlock) + return visitor.ActionUpdate, str + } + return visitor.ActionNoChange, nil + }, + "OperationTypeDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { + pretty.Println("===OperationTypeDefinition", p.Node) + switch node := p.Node.(type) { + case *ast.OperationTypeDefinition: + str := fmt.Sprintf("%v: %v", node.Operation, node.Type) + return visitor.ActionUpdate, str + case map[string]interface{}: + operation := getMapValueString(node, "Operation") + ttype := getMapValueString(node, "Type") + str := fmt.Sprintf("%v: %v", operation, ttype) + return visitor.ActionUpdate, str + } + return visitor.ActionNoChange, nil + }, "ScalarDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.ScalarDefinition: diff --git a/language/printer/printer_old.go b/language/printer/printer_old.go deleted file mode 100644 index 71d9157e..00000000 --- a/language/printer/printer_old.go +++ /dev/null @@ -1,359 +0,0 @@ -package printer - -import ( - "fmt" - - "github.com/graphql-go/graphql/language/ast" - "github.com/graphql-go/graphql/language/visitor" - // "log" -) - -var printDocASTReducer11 = map[string]visitor.VisitFunc{ - "Name": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.Name: - return visitor.ActionUpdate, node.Value - } - return visitor.ActionNoChange, nil - - }, - "Variable": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.Variable: - return visitor.ActionUpdate, fmt.Sprintf("$%v", node.Name) - } - return visitor.ActionNoChange, nil - }, - "Document": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.Document: - definitions := toSliceString(node.Definitions) - return visitor.ActionUpdate, join(definitions, "\n\n") + "\n" - } - return visitor.ActionNoChange, nil - }, - "OperationDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.OperationDefinition: - op := node.Operation - name := fmt.Sprintf("%v", node.Name) - - defs := wrap("(", join(toSliceString(node.VariableDefinitions), ", "), ")") - directives := join(toSliceString(node.Directives), " ") - selectionSet := fmt.Sprintf("%v", node.SelectionSet) - str := "" - if name == "" { - str = selectionSet - } else { - str = join([]string{ - op, - join([]string{name, defs}, ""), - directives, - selectionSet, - }, " ") - } - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "VariableDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.VariableDefinition: - variable := fmt.Sprintf("%v", node.Variable) - ttype := fmt.Sprintf("%v", node.Type) - defaultValue := fmt.Sprintf("%v", node.DefaultValue) - - return visitor.ActionUpdate, variable + ": " + ttype + wrap(" = ", defaultValue, "") - - } - return visitor.ActionNoChange, nil - }, - "SelectionSet": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.SelectionSet: - str := block(node.Selections) - return visitor.ActionUpdate, str - - } - return visitor.ActionNoChange, nil - }, - "Field": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.Field: - - alias := fmt.Sprintf("%v", node.Alias) - name := fmt.Sprintf("%v", node.Name) - args := toSliceString(node.Arguments) - directives := toSliceString(node.Directives) - selectionSet := fmt.Sprintf("%v", node.SelectionSet) - - str := join( - []string{ - wrap("", alias, ": ") + name + wrap("(", join(args, ", "), ")"), - join(directives, " "), - selectionSet, - }, - " ", - ) - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "Argument": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.Argument: - name := fmt.Sprintf("%v", node.Name) - value := fmt.Sprintf("%v", node.Value) - return visitor.ActionUpdate, name + ": " + value - } - return visitor.ActionNoChange, nil - }, - "FragmentSpread": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.FragmentSpread: - name := fmt.Sprintf("%v", node.Name) - directives := toSliceString(node.Directives) - return visitor.ActionUpdate, "..." + name + wrap(" ", join(directives, " "), "") - } - return visitor.ActionNoChange, nil - }, - "InlineFragment": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.InlineFragment: - typeCondition := fmt.Sprintf("%v", node.TypeCondition) - directives := toSliceString(node.Directives) - selectionSet := fmt.Sprintf("%v", node.SelectionSet) - return visitor.ActionUpdate, "... on " + typeCondition + " " + wrap("", join(directives, " "), " ") + selectionSet - } - return visitor.ActionNoChange, nil - }, - "FragmentDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.FragmentDefinition: - name := fmt.Sprintf("%v", node.Name) - typeCondition := fmt.Sprintf("%v", node.TypeCondition) - directives := toSliceString(node.Directives) - selectionSet := fmt.Sprintf("%v", node.SelectionSet) - return visitor.ActionUpdate, "fragment " + name + " on " + typeCondition + " " + wrap("", join(directives, " "), " ") + selectionSet - } - return visitor.ActionNoChange, nil - }, - - "IntValue": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.IntValue: - return visitor.ActionUpdate, fmt.Sprintf("%v", node.Value) - } - return visitor.ActionNoChange, nil - }, - "FloatValue": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.FloatValue: - return visitor.ActionUpdate, fmt.Sprintf("%v", node.Value) - } - return visitor.ActionNoChange, nil - }, - "StringValue": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.StringValue: - return visitor.ActionUpdate, `"` + fmt.Sprintf("%v", node.Value) + `"` - } - return visitor.ActionNoChange, nil - }, - "BooleanValue": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.BooleanValue: - return visitor.ActionUpdate, fmt.Sprintf("%v", node.Value) - } - return visitor.ActionNoChange, nil - }, - "EnumValue": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.EnumValue: - return visitor.ActionUpdate, fmt.Sprintf("%v", node.Value) - } - return visitor.ActionNoChange, nil - }, - "ListValue": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.ListValue: - return visitor.ActionUpdate, "[" + join(toSliceString(node.Values), ", ") + "]" - } - return visitor.ActionNoChange, nil - }, - "ObjectValue": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.ObjectValue: - return visitor.ActionUpdate, "{" + join(toSliceString(node.Fields), ", ") + "}" - } - return visitor.ActionNoChange, nil - }, - "ObjectField": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.ObjectField: - name := fmt.Sprintf("%v", node.Name) - value := fmt.Sprintf("%v", node.Value) - return visitor.ActionUpdate, name + ": " + value - } - return visitor.ActionNoChange, nil - }, - - "Directive": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.Directive: - name := fmt.Sprintf("%v", node.Name) - args := toSliceString(node.Arguments) - return visitor.ActionUpdate, "@" + name + wrap("(", join(args, ", "), ")") - } - return visitor.ActionNoChange, nil - }, - - "Named": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.Named: - return visitor.ActionUpdate, fmt.Sprintf("%v", node.Name) - } - return visitor.ActionNoChange, nil - }, - "List": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.List: - return visitor.ActionUpdate, "[" + fmt.Sprintf("%v", node.Type) + "]" - } - return visitor.ActionNoChange, nil - }, - "NonNull": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.NonNull: - return visitor.ActionUpdate, fmt.Sprintf("%v", node.Type) + "!" - } - return visitor.ActionNoChange, nil - }, - - "ObjectDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.ObjectDefinition: - name := fmt.Sprintf("%v", node.Name) - interfaces := toSliceString(node.Interfaces) - fields := node.Fields - str := "type " + name + " " + wrap("implements ", join(interfaces, ", "), " ") + block(fields) - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "FieldDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.FieldDefinition: - name := fmt.Sprintf("%v", node.Name) - ttype := fmt.Sprintf("%v", node.Type) - args := toSliceString(node.Arguments) - str := name + wrap("(", join(args, ", "), ")") + ": " + ttype - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "InputValueDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.InputValueDefinition: - name := fmt.Sprintf("%v", node.Name) - ttype := fmt.Sprintf("%v", node.Type) - defaultValue := fmt.Sprintf("%v", node.DefaultValue) - str := name + ": " + ttype + wrap(" = ", defaultValue, "") - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "InterfaceDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.InterfaceDefinition: - name := fmt.Sprintf("%v", node.Name) - fields := node.Fields - str := "interface " + name + " " + block(fields) - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "UnionDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.UnionDefinition: - name := fmt.Sprintf("%v", node.Name) - types := toSliceString(node.Types) - str := "union " + name + " = " + join(types, " | ") - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "ScalarDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.ScalarDefinition: - name := fmt.Sprintf("%v", node.Name) - str := "scalar " + name - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "EnumDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.EnumDefinition: - name := fmt.Sprintf("%v", node.Name) - values := node.Values - str := "enum " + name + " " + block(values) - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, - "EnumValueDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.EnumValueDefinition: - name := fmt.Sprintf("%v", node.Name) - return visitor.ActionUpdate, name - } - return visitor.ActionNoChange, nil - }, - "InputObjectDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.InputObjectDefinition: - name := fmt.Sprintf("%v", node.Name) - fields := node.Fields - return visitor.ActionUpdate, "input " + name + " " + block(fields) - } - return visitor.ActionNoChange, nil - }, - "TypeExtensionDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - switch node := p.Node.(type) { - case *ast.TypeExtensionDefinition: - definition := fmt.Sprintf("%v", node.Definition) - str := "extend " + definition - return visitor.ActionUpdate, str - } - return visitor.ActionNoChange, nil - }, -} - -func Print11(astNode ast.Node) (printed interface{}) { - // defer func() interface{} { - // if r := recover(); r != nil { - // log.Println("Error: %v", r) - // return printed - // } - // return printed - // }() - printed = visitor.Visit(astNode, &visitor.VisitorOptions{ - LeaveKindMap: printDocASTReducer, - }, nil) - return printed -} - -// -//func PrintMap(astNodeMap map[string]interface{}) (printed interface{}) { -// defer func() interface{} { -// if r := recover(); r != nil { -// return fmt.Sprintf("%v", astNodeMap) -// } -// return printed -// }() -// printed = visitor.Visit(astNodeMap, &visitor.VisitorOptions{ -// LeaveKindMap: printDocASTReducer, -// }, nil) -// return printed -//} diff --git a/language/printer/schema_printer_test.go b/language/printer/schema_printer_test.go index cb5b258d..3e0f6f69 100644 --- a/language/printer/schema_printer_test.go +++ b/language/printer/schema_printer_test.go @@ -53,7 +53,12 @@ func TestSchemaPrinter_PrintsKitchenSink(t *testing.T) { query := string(b) astDoc := parse(t, query) - expected := `type Foo implements Bar { + expected := `schema { + query: QueryType + mutation: MutationType +} + +type Foo implements Bar { one: Type two(argument: InputType!): Type three(argument: InputType, other: String): Int diff --git a/language/visitor/visitor.go b/language/visitor/visitor.go index 759e3c86..b59df235 100644 --- a/language/visitor/visitor.go +++ b/language/visitor/visitor.go @@ -83,6 +83,10 @@ var QueryDocumentKeys = KeyMap{ "List": []string{"Type"}, "NonNull": []string{"Type"}, + "SchemaDefinition": []string{"OperationTypes"}, + "OperationTypeDefinition": []string{"Type"}, + + "ScalarDefinition": []string{"Name"}, "ObjectDefinition": []string{ "Name", "Interfaces", @@ -106,7 +110,6 @@ var QueryDocumentKeys = KeyMap{ "Name", "Types", }, - "ScalarDefinition": []string{"Name"}, "EnumDefinition": []string{ "Name", "Values", @@ -116,8 +119,10 @@ var QueryDocumentKeys = KeyMap{ "Name", "Fields", }, + "TypeExtensionDefinition": []string{"Definition"}, - "DirectiveDefinition": []string{"Name", "Arguments", "Locations"}, + + "DirectiveDefinition": []string{"Name", "Arguments", "Locations"}, } type stack struct { diff --git a/rules.go b/rules.go index f20a3164..392b90ba 100644 --- a/rules.go +++ b/rules.go @@ -544,13 +544,13 @@ func getLocationForAppliedNode(appliedTo ast.Node) string { kind := appliedTo.GetKind() if kind == kinds.OperationDefinition { appliedTo, _ := appliedTo.(*ast.OperationDefinition) - if appliedTo.Operation == "query" { + if appliedTo.Operation == ast.OperationTypeQuery { return DirectiveLocationQuery } - if appliedTo.Operation == "mutation" { + if appliedTo.Operation == ast.OperationTypeMutation { return DirectiveLocationMutation } - if appliedTo.Operation == "subscription" { + if appliedTo.Operation == ast.OperationTypeSubscription { return DirectiveLocationSubscription } } diff --git a/schema-kitchen-sink.graphql b/schema-kitchen-sink.graphql index b67572fe..efc1b469 100644 --- a/schema-kitchen-sink.graphql +++ b/schema-kitchen-sink.graphql @@ -1,5 +1,10 @@ # Filename: schema-kitchen-sink.graphql +schema { + query: QueryType + mutation: MutationType +} + type Foo implements Bar { one: Type two(argument: InputType!): Type diff --git a/type_info.go b/type_info.go index 3c06c29b..39838295 100644 --- a/type_info.go +++ b/type_info.go @@ -110,11 +110,11 @@ func (ti *TypeInfo) Enter(node ast.Node) { } ti.directive = schema.Directive(nameVal) case *ast.OperationDefinition: - if node.Operation == "query" { + if node.Operation == ast.OperationTypeQuery { ttype = schema.QueryType() - } else if node.Operation == "mutation" { + } else if node.Operation == ast.OperationTypeMutation { ttype = schema.MutationType() - } else if node.Operation == "subscription" { + } else if node.Operation == ast.OperationTypeSubscription { ttype = schema.SubscriptionType() } ti.typeStack = append(ti.typeStack, ttype) From d7a15e095b45bf6952b44e1a77b72c619eb21ec7 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 00:02:29 +0800 Subject: [PATCH 15/32] Minor follow-up to #311 Commit: 533fc43f7d5836fc9b6a3eafde9801db9a3ee011 [533fc43] Parents: 371a5a0908 Author: Lee Byron Date: 23 March 2016 at 8:21:04 AM SGT Commit Date: 23 March 2016 at 8:22:42 AM SGT --- executor.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/executor.go b/executor.go index 44999213..2893214a 100644 --- a/executor.go +++ b/executor.go @@ -594,6 +594,8 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie panic(gqlerrors.FormatError(err)) } + // If field type is NonNull, complete for inner type, and throw field error + // if result is null. if returnType, ok := returnType.(*NonNull); ok { completed := completeValue(eCtx, returnType.OfType, fieldASTs, info, result) if completed == nil { @@ -606,6 +608,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return completed } + // If result value is null-ish (null, undefined, or NaN) then return null. if isNullish(result) { return nil } @@ -615,8 +618,8 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return completeListValue(eCtx, returnType, fieldASTs, info, result) } - // If field type is Scalar or Enum, serialize to a valid value, returning - // null if serialization is not possible. + // If field type is a leaf type, Scalar or Enum, serialize to a valid value, + // returning null if serialization is not possible. if returnType, ok := returnType.(*Scalar); ok { return completeLeafValue(eCtx, returnType, fieldASTs, info, result) } @@ -624,15 +627,18 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return completeLeafValue(eCtx, returnType, fieldASTs, info, result) } - if returnType, ok := returnType.(*Object); ok { - return completeObjectValue(eCtx, returnType, fieldASTs, info, result) - } - + // If field type is an abstract type, Interface or Union, determine the + // runtime Object type and complete for that type. if returnType, ok := returnType.(Abstract); ok { return completeAbstractValue(eCtx, returnType, fieldASTs, info, result) } - // Not reachable + // If field type is Object, execute and complete all sub-selections. + if returnType, ok := returnType.(*Object); ok { + return completeObjectValue(eCtx, returnType, fieldASTs, info, result) + } + + // Not reachable. All possible output types have been considered. err := invariant(false, fmt.Sprintf(`Cannot complete value of unexpected type "%v."`, returnType), ) @@ -642,7 +648,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return nil } -// completeObjectValue completes value of an Abstract type (Union / Interface) by determining the runtime type +// completeAbstractValue completes value of an Abstract type (Union / Interface) by determining the runtime type // of that value, then completing based on that type. func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { @@ -664,6 +670,8 @@ func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldAST return completeObjectValue(eCtx, runtimeType, fieldASTs, info, result) } + +// completeObjectValue complete an Object value by executing all sub-selections. func completeObjectValue(eCtx *ExecutionContext, returnType *Object, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { // If there is an isTypeOf predicate function, call it with the From 574088c033e3ce7cbddfb004439e02221b1af1a5 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 00:04:05 +0800 Subject: [PATCH 16/32] Remove unused function parameters Commit: 43992e3a60d7bf8f5e7d2f29dfae302b5ba72eec [43992e3] Parents: 533fc43f7d Author: Lee Byron Date: 23 March 2016 at 8:58:05 AM SGT --- executor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor.go b/executor.go index 2893214a..e70631d3 100644 --- a/executor.go +++ b/executor.go @@ -624,7 +624,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie return completeLeafValue(eCtx, returnType, fieldASTs, info, result) } if returnType, ok := returnType.(*Enum); ok { - return completeLeafValue(eCtx, returnType, fieldASTs, info, result) + return completeLeafValue(returnType, result) } // If field type is an abstract type, Interface or Union, determine the @@ -715,7 +715,7 @@ func completeObjectValue(eCtx *ExecutionContext, returnType *Object, fieldASTs [ } // completeLeafValue complete a leaf value (Scalar / Enum) by serializing to a valid value, returning nil if serialization is not possible. -func completeLeafValue(eCtx *ExecutionContext, returnType Leaf, fieldASTs []*ast.Field, info ResolveInfo, result interface{}) interface{} { +func completeLeafValue(returnType Leaf, result interface{}) interface{} { serializedResult := returnType.Serialize(result) if isNullish(serializedResult) { return nil From d46d5458a9485a0b71c781b18ce1627c3a09be84 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 09:41:14 +0800 Subject: [PATCH 17/32] Remove unused function parameters Commit: 43992e3a60d7bf8f5e7d2f29dfae302b5ba72eec [43992e3] Parents: 533fc43f7d Author: Lee Byron Date: 23 March 2016 at 8:58:05 AM SGT --- executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor.go b/executor.go index e70631d3..f925325a 100644 --- a/executor.go +++ b/executor.go @@ -621,7 +621,7 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie // If field type is a leaf type, Scalar or Enum, serialize to a valid value, // returning null if serialization is not possible. if returnType, ok := returnType.(*Scalar); ok { - return completeLeafValue(eCtx, returnType, fieldASTs, info, result) + return completeLeafValue(returnType, result) } if returnType, ok := returnType.(*Enum); ok { return completeLeafValue(returnType, result) From 03b92b0230f165ff27078db28aceea8c1cbc2432 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 09:44:26 +0800 Subject: [PATCH 18/32] Move getTypeOf to execute.js and rename to defaultResolveTypeFn to mirror defaultResolveFn Commit: edc405a11508a110759ce53c9efb2eb6dd2d181c [edc405a] Parents: 3f6a7f4ca9 Author: jeff@procata.com Date: 24 March 2016 at 3:15:33 PM SGT --- definition.go | 26 -------------------------- executor.go | 41 +++++++++++++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/definition.go b/definition.go index 8e682bfa..4e3b41a8 100644 --- a/definition.go +++ b/definition.go @@ -143,7 +143,6 @@ func IsCompositeType(ttype interface{}) bool { // Abstract interface for types that may describe the parent context of a selection set. type Abstract interface { Name() string - ObjectType(value interface{}, info ResolveInfo) *Object PossibleTypes() []*Object IsPossibleType(ttype *Object) bool } @@ -760,12 +759,6 @@ func (it *Interface) IsPossibleType(ttype *Object) bool { } return false } -func (it *Interface) ObjectType(value interface{}, info ResolveInfo) *Object { - if it.ResolveType != nil { - return it.ResolveType(value, info) - } - return getTypeOf(value, info, it) -} func (it *Interface) String() string { return it.PrivateName } @@ -773,19 +766,6 @@ func (it *Interface) Error() error { return it.err } -func getTypeOf(value interface{}, info ResolveInfo, abstractType Abstract) *Object { - possibleTypes := abstractType.PossibleTypes() - for _, possibleType := range possibleTypes { - if possibleType.IsTypeOf == nil { - continue - } - if res := possibleType.IsTypeOf(value, info); res { - return possibleType - } - } - return nil -} - // Union Type Definition // // When a field can return one of a heterogeneous set of types, a Union type @@ -901,12 +881,6 @@ func (ut *Union) IsPossibleType(ttype *Object) bool { } return false } -func (ut *Union) ObjectType(value interface{}, info ResolveInfo) *Object { - if ut.ResolveType != nil { - return ut.ResolveType(value, info) - } - return getTypeOf(value, info, ut) -} func (ut *Union) String() string { return ut.PrivateName } diff --git a/executor.go b/executor.go index f925325a..2eb538f9 100644 --- a/executor.go +++ b/executor.go @@ -654,14 +654,19 @@ func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldAST var runtimeType *Object - if returnType, ok := returnType.(Abstract); ok { - runtimeType = returnType.ObjectType(result, info) - if runtimeType != nil && !returnType.IsPossibleType(runtimeType) { - panic(gqlerrors.NewFormattedError( - fmt.Sprintf(`Runtime Object type "%v" is not a possible type `+ - `for "%v".`, runtimeType, returnType), - )) - } + if unionReturnType, ok := returnType.(*Union); ok && unionReturnType.ResolveType != nil { + runtimeType = unionReturnType.ResolveType(result, info) + } else if interfaceReturnType, ok := returnType.(*Interface); ok && interfaceReturnType.ResolveType != nil { + runtimeType = interfaceReturnType.ResolveType(result, info) + } else { + runtimeType = defaultResolveTypeFn(result, info, returnType) + } + + if runtimeType != nil && !returnType.IsPossibleType(runtimeType) { + panic(gqlerrors.NewFormattedError( + fmt.Sprintf(`Runtime Object type "%v" is not a possible type `+ + `for "%v".`, runtimeType, returnType), + )) } if runtimeType == nil { @@ -749,6 +754,26 @@ func completeListValue(eCtx *ExecutionContext, returnType *List, fieldASTs []*as return completedResults } +// defaultResolveTypeFn If a resolveType function is not given, then a default resolve behavior is +// used which tests each possible type for the abstract type by calling +// isTypeOf for the object being coerced, returning the first type that matches. +func defaultResolveTypeFn(value interface{}, info ResolveInfo, abstractType Abstract) *Object { + possibleTypes := abstractType.PossibleTypes() + for _, possibleType := range possibleTypes { + if possibleType.IsTypeOf == nil { + continue + } + if res := possibleType.IsTypeOf(value, info); res { + return possibleType + } + } + return nil +} + +// defaultResolveFn If a resolve function is not given, then a default resolve behavior is used +// which takes the property of the source object of the same name as the field +// and returns it as the result, or if it's a function, returns the result +// of calling that function. func defaultResolveFn(p ResolveParams) (interface{}, error) { // try to resolve p.Source as a struct first sourceVal := reflect.ValueOf(p.Source) From 7b0297818fb2e76fbb8602acb633e002d589fc31 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 10:02:25 +0800 Subject: [PATCH 19/32] Restored deprecated fields in `directives` in introspective query for coverage. Should be removed once `onOperation`, `onFragment`, `onField` are dropped from spec. --- introspection_test.go | 8 ++++++++ testutil/introspection_query.go | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/introspection_test.go b/introspection_test.go index 3b70ed88..45aa9a40 100644 --- a/introspection_test.go +++ b/introspection_test.go @@ -790,6 +790,10 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { }, }, }, + // deprecated, but included for coverage till removed + "onOperation": false, + "onFragment": true, + "onField": true, }, map[string]interface{}{ "name": "skip", @@ -813,6 +817,10 @@ func TestIntrospection_ExecutesAnIntrospectionQuery(t *testing.T) { }, }, }, + // deprecated, but included for coverage till removed + "onOperation": false, + "onFragment": true, + "onField": true, }, }, }, diff --git a/testutil/introspection_query.go b/testutil/introspection_query.go index a46153c7..d92b81e2 100644 --- a/testutil/introspection_query.go +++ b/testutil/introspection_query.go @@ -16,6 +16,10 @@ var IntrospectionQuery = ` args { ...InputValue } + # deprecated, but included for coverage till removed + onOperation + onFragment + onField } } } From cb9b512461b46f456fb9393076786f5f424e415b Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 12:13:13 +0800 Subject: [PATCH 20/32] Add tests for default resolve function. --- executor_resolve_test.go | 266 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 266 insertions(+) create mode 100644 executor_resolve_test.go diff --git a/executor_resolve_test.go b/executor_resolve_test.go new file mode 100644 index 00000000..1617e0d0 --- /dev/null +++ b/executor_resolve_test.go @@ -0,0 +1,266 @@ +package graphql_test + +import ( + "encoding/json" + "github.com/graphql-go/graphql" + "github.com/graphql-go/graphql/testutil" + "reflect" + "testing" +) + +func testSchema(t *testing.T, testField *graphql.Field) graphql.Schema { + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "test": testField, + }, + }), + }) + if err != nil { + t.Fatalf("Invalid schema: %v", err) + } + return schema +} + +func TestExecutesResolveFunction_DefaultFunctionAccessesProperties(t *testing.T) { + schema := testSchema(t, &graphql.Field{Type: graphql.String}) + + source := map[string]interface{}{ + "test": "testValue", + } + + expected := map[string]interface{}{ + "test": "testValue", + } + + result := graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test }`, + RootObject: source, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } +} + +func TestExecutesResolveFunction_DefaultFunctionCallsMethods(t *testing.T) { + schema := testSchema(t, &graphql.Field{Type: graphql.String}) + + source := map[string]interface{}{ + "test": func() interface{} { + return "testValue" + }, + } + + expected := map[string]interface{}{ + "test": "testValue", + } + + result := graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test }`, + RootObject: source, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } +} + +func TestExecutesResolveFunction_UsesProvidedResolveFunction(t *testing.T) { + schema := testSchema(t, &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "aStr": &graphql.ArgumentConfig{Type: graphql.String}, + "aInt": &graphql.ArgumentConfig{Type: graphql.Int}, + }, + Resolve: func(p graphql.ResolveParams) (interface{}, error) { + b, err := json.Marshal(p.Args) + return string(b), err + }, + }) + + expected := map[string]interface{}{ + "test": "{}", + } + result := graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test }`, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } + + expected = map[string]interface{}{ + "test": `{"aStr":"String!"}`, + } + result = graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test(aStr: "String!") }`, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } + + expected = map[string]interface{}{ + "test": `{"aInt":-123,"aStr":"String!"}`, + } + result = graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test(aInt: -123, aStr: "String!") }`, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } +} + +func TestExecutesResolveFunction_UsesProvidedResolveFunction_SourceIsStruct_WithoutJSONTags(t *testing.T) { + + // For structs without JSON tags, it will map to upper-cased exported field names + type SubObjectWithoutJSONTags struct { + Str string + Int int + } + + schema := testSchema(t, &graphql.Field{ + Type: graphql.NewObject(graphql.ObjectConfig{ + Name: "SubObject", + Description: "Maps GraphQL Object `SubObject` to Go struct `SubObjectWithoutJSONTags`", + Fields: graphql.Fields{ + "Str": &graphql.Field{Type: graphql.String}, + "Int": &graphql.Field{Type: graphql.Int}, + }, + }), + Args: graphql.FieldConfigArgument{ + "aStr": &graphql.ArgumentConfig{Type: graphql.String}, + "aInt": &graphql.ArgumentConfig{Type: graphql.Int}, + }, + Resolve: func(p graphql.ResolveParams) (interface{}, error) { + aStr, _ := p.Args["aStr"].(string) + aInt, _ := p.Args["aInt"].(int) + return &SubObjectWithoutJSONTags{ + Str: aStr, + Int: aInt, + }, nil + }, + }) + + expected := map[string]interface{}{ + "test": map[string]interface{}{ + "Str": nil, + "Int": 0, + }, + } + result := graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test { Str, Int } }`, + }) + + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } + + expected = map[string]interface{}{ + "test": map[string]interface{}{ + "Str": "String!", + "Int": 0, + }, + } + result = graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test(aStr: "String!") { Str, Int } }`, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } + + expected = map[string]interface{}{ + "test": map[string]interface{}{ + "Str": "String!", + "Int": -123, + }, + } + result = graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test(aInt: -123, aStr: "String!") { Str, Int } }`, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } +} + +func TestExecutesResolveFunction_UsesProvidedResolveFunction_SourceIsStruct_WithJSONTags(t *testing.T) { + + // For structs without JSON tags, it will map to upper-cased exported field names + type SubObjectWithJSONTags struct { + OtherField string `json:""` + Str string `json:"str"` + Int int `json:"int"` + } + + schema := testSchema(t, &graphql.Field{ + Type: graphql.NewObject(graphql.ObjectConfig{ + Name: "SubObject", + Description: "Maps GraphQL Object `SubObject` to Go struct `SubObjectWithJSONTags`", + Fields: graphql.Fields{ + "str": &graphql.Field{Type: graphql.String}, + "int": &graphql.Field{Type: graphql.Int}, + }, + }), + Args: graphql.FieldConfigArgument{ + "aStr": &graphql.ArgumentConfig{Type: graphql.String}, + "aInt": &graphql.ArgumentConfig{Type: graphql.Int}, + }, + Resolve: func(p graphql.ResolveParams) (interface{}, error) { + aStr, _ := p.Args["aStr"].(string) + aInt, _ := p.Args["aInt"].(int) + return &SubObjectWithJSONTags{ + Str: aStr, + Int: aInt, + }, nil + }, + }) + + expected := map[string]interface{}{ + "test": map[string]interface{}{ + "str": nil, + "int": 0, + }, + } + result := graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test { str, int } }`, + }) + + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } + + expected = map[string]interface{}{ + "test": map[string]interface{}{ + "str": "String!", + "int": 0, + }, + } + result = graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test(aStr: "String!") { str, int } }`, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } + + expected = map[string]interface{}{ + "test": map[string]interface{}{ + "str": "String!", + "int": -123, + }, + } + result = graphql.Do(graphql.Params{ + Schema: schema, + RequestString: `{ test(aInt: -123, aStr: "String!") { str, int } }`, + }) + if !reflect.DeepEqual(expected, result.Data) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result.Data)) + } +} From 3571f4fc8b7d002da6d8269bdee5569d8fa1f18b Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 12:16:48 +0800 Subject: [PATCH 21/32] Follow up to: Move getTypeOf to executor and rename to defaultResolveTypeFn to mirror defaultResolveFn --- executor.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/executor.go b/executor.go index 2eb538f9..6ce6fb7c 100644 --- a/executor.go +++ b/executor.go @@ -662,6 +662,10 @@ func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldAST runtimeType = defaultResolveTypeFn(result, info, returnType) } + if runtimeType == nil { + return nil + } + if runtimeType != nil && !returnType.IsPossibleType(runtimeType) { panic(gqlerrors.NewFormattedError( fmt.Sprintf(`Runtime Object type "%v" is not a possible type `+ @@ -669,10 +673,6 @@ func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldAST )) } - if runtimeType == nil { - return nil - } - return completeObjectValue(eCtx, runtimeType, fieldASTs, info, result) } From 4cafaafff6e132b34411e9d8cfa3034df4774e3e Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 17:46:08 +0800 Subject: [PATCH 22/32] Removed logs --- language/printer/printer.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/language/printer/printer.go b/language/printer/printer.go index 3b10f05a..47f9b2a4 100644 --- a/language/printer/printer.go +++ b/language/printer/printer.go @@ -6,7 +6,6 @@ import ( "github.com/graphql-go/graphql/language/ast" "github.com/graphql-go/graphql/language/visitor" - "github.com/kr/pretty" "reflect" ) @@ -435,7 +434,6 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ // Type System Definitions "SchemaDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - pretty.Println("===SchemaDefinitions", p.Node) switch node := p.Node.(type) { case *ast.SchemaDefinition: operationTypesBlock := block(node.OperationTypes) @@ -450,7 +448,6 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ return visitor.ActionNoChange, nil }, "OperationTypeDefinition": func(p visitor.VisitFuncParams) (string, interface{}) { - pretty.Println("===OperationTypeDefinition", p.Node) switch node := p.Node.(type) { case *ast.OperationTypeDefinition: str := fmt.Sprintf("%v: %v", node.Operation, node.Type) From 79f48da704036914912ccfbf60e1aba6d5714ad1 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 19:00:29 +0800 Subject: [PATCH 23/32] Add GraphQLSchema types field This is a rebased and updated version of @tgriesser's #199. I further extended it to completely remove the side-effectful mutation of Interface types rather than just deferring that mutation to schema creation time. This introduces a *breaking* change to the Type System API. Now, any individual Interface type does not have the required information to answer `getPossibleTypes` or `isPossibleType` without knowing the other types in the Schema. These methods were moved to the Schema API, accepting the abstract type as the first parameter. This also introduces a *breaking* change to the type comparator functions: `isTypeSubTypeOf` and `doTypesOverlap` which now require a Schema as a first argument. Commit: 6a1f23e1f9c1e6bf4cea837bc9bb6eae0fb5c382 [6a1f23e] Parents: a781b556ee Author: Lee Byron Date: 23 March 2016 at 1:17:43 PM SGT Commit Date: 25 March 2016 at 6:35:39 AM SGT --- abstract_test.go | 44 +----- definition.go | 80 ++-------- definition_test.go | 2 + executor.go | 24 ++- introspection.go | 4 +- rules.go | 30 ++-- ...s_overlapping_fields_can_be_merged_test.go | 11 +- schema.go | 138 +++++++++++++----- testutil/rules_test_harness.go | 6 + union_interface_test.go | 5 +- validation_test.go | 7 +- 11 files changed, 175 insertions(+), 176 deletions(-) diff --git a/abstract_test.go b/abstract_test.go index 37f2eb3d..8ebbb10e 100644 --- a/abstract_test.go +++ b/abstract_test.go @@ -36,7 +36,7 @@ func TestIsTypeOfUsedToResolveRuntimeTypeForInterface(t *testing.T) { }) // ie declare that Dog belongs to Pet interface - _ = graphql.NewObject(graphql.ObjectConfig{ + dogType := graphql.NewObject(graphql.ObjectConfig{ Name: "Dog", Interfaces: []*graphql.Interface{ petType, @@ -67,7 +67,7 @@ func TestIsTypeOfUsedToResolveRuntimeTypeForInterface(t *testing.T) { }, }) // ie declare that Cat belongs to Pet interface - _ = graphql.NewObject(graphql.ObjectConfig{ + catType := graphql.NewObject(graphql.ObjectConfig{ Name: "Cat", Interfaces: []*graphql.Interface{ petType, @@ -112,6 +112,7 @@ func TestIsTypeOfUsedToResolveRuntimeTypeForInterface(t *testing.T) { }, }, }), + Types: []graphql.Type{catType, dogType}, }) if err != nil { t.Fatalf("Error in schema %v", err.Error()) @@ -288,12 +289,6 @@ func TestResolveTypeOnInterfaceYieldsUsefulError(t *testing.T) { Fields: graphql.Fields{ "name": &graphql.Field{ Type: graphql.String, - Resolve: func(p graphql.ResolveParams) (interface{}, error) { - if human, ok := p.Source.(*testHuman); ok { - return human.Name, nil - } - return nil, nil - }, }, }, }) @@ -302,28 +297,12 @@ func TestResolveTypeOnInterfaceYieldsUsefulError(t *testing.T) { Interfaces: []*graphql.Interface{ petType, }, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testDog) - return ok - }, Fields: graphql.Fields{ "name": &graphql.Field{ Type: graphql.String, - Resolve: func(p graphql.ResolveParams) (interface{}, error) { - if dog, ok := p.Source.(*testDog); ok { - return dog.Name, nil - } - return nil, nil - }, }, "woofs": &graphql.Field{ Type: graphql.Boolean, - Resolve: func(p graphql.ResolveParams) (interface{}, error) { - if dog, ok := p.Source.(*testDog); ok { - return dog.Woofs, nil - } - return nil, nil - }, }, }, }) @@ -332,28 +311,12 @@ func TestResolveTypeOnInterfaceYieldsUsefulError(t *testing.T) { Interfaces: []*graphql.Interface{ petType, }, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testCat) - return ok - }, Fields: graphql.Fields{ "name": &graphql.Field{ Type: graphql.String, - Resolve: func(p graphql.ResolveParams) (interface{}, error) { - if cat, ok := p.Source.(*testCat); ok { - return cat.Name, nil - } - return nil, nil - }, }, "meows": &graphql.Field{ Type: graphql.Boolean, - Resolve: func(p graphql.ResolveParams) (interface{}, error) { - if cat, ok := p.Source.(*testCat); ok { - return cat.Meows, nil - } - return nil, nil - }, }, }, }) @@ -373,6 +336,7 @@ func TestResolveTypeOnInterfaceYieldsUsefulError(t *testing.T) { }, }, }), + Types: []graphql.Type{catType, dogType}, }) if err != nil { t.Fatalf("Error in schema %v", err.Error()) diff --git a/definition.go b/definition.go index 4e3b41a8..141770bd 100644 --- a/definition.go +++ b/definition.go @@ -143,13 +143,21 @@ func IsCompositeType(ttype interface{}) bool { // Abstract interface for types that may describe the parent context of a selection set. type Abstract interface { Name() string - PossibleTypes() []*Object - IsPossibleType(ttype *Object) bool } var _ Abstract = (*Interface)(nil) var _ Abstract = (*Union)(nil) +func IsAbstractType(ttype interface{}) bool { + if _, ok := ttype.(*Interface); ok { + return true + } + if _, ok := ttype.(*Union); ok { + return true + } + return false +} + // Nullable interface for types that can accept null as a value. type Nullable interface { } @@ -393,21 +401,6 @@ func NewObject(config ObjectConfig) *Object { objectType.IsTypeOf = config.IsTypeOf objectType.typeConfig = config - /* - addImplementationToInterfaces() - Update the interfaces to know about this implementation. - This is an rare and unfortunate use of mutation in the type definition - implementations, but avoids an expensive "getPossibleTypes" - implementation for Interface - */ - interfaces := objectType.Interfaces() - if interfaces == nil { - return objectType - } - for _, iface := range interfaces { - iface.implementations = append(iface.implementations, objectType) - } - return objectType } func (gt *Object) AddFieldConfig(fieldName string, fieldConfig *Field) { @@ -671,11 +664,8 @@ type Interface struct { PrivateDescription string `json:"description"` ResolveType ResolveTypeFn - typeConfig InterfaceConfig - fields FieldDefinitionMap - implementations []*Object - possibleTypes map[string]bool - + typeConfig InterfaceConfig + fields FieldDefinitionMap err error } type InterfaceConfig struct { @@ -704,7 +694,6 @@ func NewInterface(config InterfaceConfig) *Interface { it.PrivateDescription = config.Description it.ResolveType = config.ResolveType it.typeConfig = config - it.implementations = []*Object{} return it } @@ -737,28 +726,6 @@ func (it *Interface) Fields() (fields FieldDefinitionMap) { it.fields = fields return it.fields } -func (it *Interface) PossibleTypes() []*Object { - return it.implementations -} -func (it *Interface) IsPossibleType(ttype *Object) bool { - if ttype == nil { - return false - } - if len(it.possibleTypes) == 0 { - possibleTypes := map[string]bool{} - for _, possibleType := range it.PossibleTypes() { - if possibleType == nil { - continue - } - possibleTypes[possibleType.PrivateName] = true - } - it.possibleTypes = possibleTypes - } - if val, ok := it.possibleTypes[ttype.PrivateName]; ok { - return val - } - return false -} func (it *Interface) String() string { return it.PrivateName } @@ -857,30 +824,9 @@ func NewUnion(config UnionConfig) *Union { return objectType } -func (ut *Union) PossibleTypes() []*Object { +func (ut *Union) Types() []*Object { return ut.types } -func (ut *Union) IsPossibleType(ttype *Object) bool { - - if ttype == nil { - return false - } - if len(ut.possibleTypes) == 0 { - possibleTypes := map[string]bool{} - for _, possibleType := range ut.PossibleTypes() { - if possibleType == nil { - continue - } - possibleTypes[possibleType.PrivateName] = true - } - ut.possibleTypes = possibleTypes - } - - if val, ok := ut.possibleTypes[ttype.PrivateName]; ok { - return val - } - return false -} func (ut *Union) String() string { return ut.PrivateName } diff --git a/definition_test.go b/definition_test.go index 363c8024..27413295 100644 --- a/definition_test.go +++ b/definition_test.go @@ -365,6 +365,7 @@ func TestTypeSystem_DefinitionExample_IncludesInterfacesSubTypesInTheTypeMap(t * }, }, }), + Types: []graphql.Type{someSubType}, }) if err != nil { t.Fatalf("unexpected error, got: %v", err) @@ -408,6 +409,7 @@ func TestTypeSystem_DefinitionExample_IncludesInterfacesThunkSubtypesInTheTypeMa }, }, }), + Types: []graphql.Type{someSubType}, }) if err != nil { t.Fatalf("unexpected error, got: %v", err) diff --git a/executor.go b/executor.go index 6ce6fb7c..99966bd0 100644 --- a/executor.go +++ b/executor.go @@ -425,8 +425,11 @@ func doesFragmentConditionMatch(eCtx *ExecutionContext, fragment ast.Node, ttype if conditionalType.Name() == ttype.Name() { return true } - if conditionalType, ok := conditionalType.(Abstract); ok { - return conditionalType.IsPossibleType(ttype) + if conditionalType, ok := conditionalType.(*Interface); ok { + return eCtx.Schema.IsPossibleType(conditionalType, ttype) + } + if conditionalType, ok := conditionalType.(*Union); ok { + return eCtx.Schema.IsPossibleType(conditionalType, ttype) } case *ast.InlineFragment: typeConditionAST := fragment.TypeCondition @@ -443,8 +446,11 @@ func doesFragmentConditionMatch(eCtx *ExecutionContext, fragment ast.Node, ttype if conditionalType.Name() == ttype.Name() { return true } - if conditionalType, ok := conditionalType.(Abstract); ok { - return conditionalType.IsPossibleType(ttype) + if conditionalType, ok := conditionalType.(*Interface); ok { + return eCtx.Schema.IsPossibleType(conditionalType, ttype) + } + if conditionalType, ok := conditionalType.(*Union); ok { + return eCtx.Schema.IsPossibleType(conditionalType, ttype) } } @@ -539,6 +545,7 @@ func resolveField(eCtx *ExecutionContext, parentType *Object, source interface{} result, resolveFnError = resolveFn(ResolveParams{ Source: source, Args: args, + Schema: eCtx.Schema, Info: info, Context: eCtx.Context, }) @@ -629,7 +636,10 @@ func completeValue(eCtx *ExecutionContext, returnType Type, fieldASTs []*ast.Fie // If field type is an abstract type, Interface or Union, determine the // runtime Object type and complete for that type. - if returnType, ok := returnType.(Abstract); ok { + if returnType, ok := returnType.(*Union); ok { + return completeAbstractValue(eCtx, returnType, fieldASTs, info, result) + } + if returnType, ok := returnType.(*Interface); ok { return completeAbstractValue(eCtx, returnType, fieldASTs, info, result) } @@ -666,7 +676,7 @@ func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldAST return nil } - if runtimeType != nil && !returnType.IsPossibleType(runtimeType) { + if runtimeType != nil && !eCtx.Schema.IsPossibleType(returnType, runtimeType) { panic(gqlerrors.NewFormattedError( fmt.Sprintf(`Runtime Object type "%v" is not a possible type `+ `for "%v".`, runtimeType, returnType), @@ -758,7 +768,7 @@ func completeListValue(eCtx *ExecutionContext, returnType *List, fieldASTs []*as // used which tests each possible type for the abstract type by calling // isTypeOf for the object being coerced, returning the first type that matches. func defaultResolveTypeFn(value interface{}, info ResolveInfo, abstractType Abstract) *Object { - possibleTypes := abstractType.PossibleTypes() + possibleTypes := info.Schema.PossibleTypes(abstractType) for _, possibleType := range possibleTypes { if possibleType.IsTypeOf == nil { continue diff --git a/introspection.go b/introspection.go index 1896b5af..096b10cd 100644 --- a/introspection.go +++ b/introspection.go @@ -491,9 +491,9 @@ func init() { Resolve: func(p ResolveParams) (interface{}, error) { switch ttype := p.Source.(type) { case *Interface: - return ttype.PossibleTypes(), nil + return p.Schema.PossibleTypes(ttype), nil case *Union: - return ttype.PossibleTypes(), nil + return p.Schema.PossibleTypes(ttype), nil } return nil, nil }, diff --git a/rules.go b/rules.go index 392b90ba..5dd26c7a 100644 --- a/rules.go +++ b/rules.go @@ -214,9 +214,9 @@ func FieldsOnCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance nodeName = node.Name.Value } - if ttype, ok := ttype.(Abstract); ok { - siblingInterfaces := getSiblingInterfacesIncludingField(ttype, nodeName) - implementations := getImplementationsIncludingField(ttype, nodeName) + if ttype, ok := ttype.(Abstract); ok && IsAbstractType(ttype) { + siblingInterfaces := getSiblingInterfacesIncludingField(context.Schema(), ttype, nodeName) + implementations := getImplementationsIncludingField(context.Schema(), ttype, nodeName) suggestedMaps := map[string]bool{} for _, s := range siblingInterfaces { if _, ok := suggestedMaps[s]; !ok { @@ -253,10 +253,10 @@ func FieldsOnCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance } // Return implementations of `type` that include `fieldName` as a valid field. -func getImplementationsIncludingField(ttype Abstract, fieldName string) []string { +func getImplementationsIncludingField(schema *Schema, ttype Abstract, fieldName string) []string { result := []string{} - for _, t := range ttype.PossibleTypes() { + for _, t := range schema.PossibleTypes(ttype) { fields := t.Fields() if _, ok := fields[fieldName]; ok { result = append(result, fmt.Sprintf(`%v`, t.Name())) @@ -271,8 +271,8 @@ func getImplementationsIncludingField(ttype Abstract, fieldName string) []string // that they implement. If those interfaces include `field` as a valid field, // return them, sorted by how often the implementations include the other // interface. -func getSiblingInterfacesIncludingField(ttype Abstract, fieldName string) []string { - implementingObjects := ttype.PossibleTypes() +func getSiblingInterfacesIncludingField(schema *Schema, ttype Abstract, fieldName string) []string { + implementingObjects := schema.PossibleTypes(ttype) result := []string{} suggestedInterfaceSlice := []*suggestedInterface{} @@ -1476,7 +1476,7 @@ func getFragmentType(context *ValidationContext, name string) Type { return ttype } -func doTypesOverlap(t1 Type, t2 Type) bool { +func doTypesOverlap(schema *Schema, t1 Type, t2 Type) bool { if t1 == t2 { return true } @@ -1485,7 +1485,7 @@ func doTypesOverlap(t1 Type, t2 Type) bool { return false } if t2, ok := t2.(Abstract); ok { - for _, ttype := range t2.PossibleTypes() { + for _, ttype := range schema.PossibleTypes(t2) { if ttype == t1 { return true } @@ -1495,7 +1495,7 @@ func doTypesOverlap(t1 Type, t2 Type) bool { } if t1, ok := t1.(Abstract); ok { if _, ok := t2.(*Object); ok { - for _, ttype := range t1.PossibleTypes() { + for _, ttype := range schema.PossibleTypes(t1) { if ttype == t2 { return true } @@ -1503,11 +1503,11 @@ func doTypesOverlap(t1 Type, t2 Type) bool { return false } t1TypeNames := map[string]bool{} - for _, ttype := range t1.PossibleTypes() { + for _, ttype := range schema.PossibleTypes(t1) { t1TypeNames[ttype.Name()] = true } if t2, ok := t2.(Abstract); ok { - for _, ttype := range t2.PossibleTypes() { + for _, ttype := range schema.PossibleTypes(t2) { if hasT1TypeName, _ := t1TypeNames[ttype.Name()]; hasT1TypeName { return true } @@ -1533,7 +1533,7 @@ func PossibleFragmentSpreadsRule(context *ValidationContext) *ValidationRuleInst fragType := context.Type() parentType, _ := context.ParentType().(Type) - if fragType != nil && parentType != nil && !doTypesOverlap(fragType, parentType) { + if fragType != nil && parentType != nil && !doTypesOverlap(context.Schema(), fragType, parentType) { reportError( context, fmt.Sprintf(`Fragment cannot be spread here as objects of `+ @@ -1554,7 +1554,7 @@ func PossibleFragmentSpreadsRule(context *ValidationContext) *ValidationRuleInst } fragType := getFragmentType(context, fragName) parentType, _ := context.ParentType().(Type) - if fragType != nil && parentType != nil && !doTypesOverlap(fragType, parentType) { + if fragType != nil && parentType != nil && !doTypesOverlap(context.Schema(), fragType, parentType) { reportError( context, fmt.Sprintf(`Fragment "%v" cannot be spread here as objects of `+ @@ -2011,7 +2011,7 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI if err != nil { varType = nil } - if varType != nil && !isTypeSubTypeOf(effectiveType(varType, varDef), usage.Type) { + if varType != nil && !isTypeSubTypeOf(context.Schema(), effectiveType(varType, varDef), usage.Type) { reportError( context, fmt.Sprintf(`Variable "$%v" of type "%v" used in position `+ diff --git a/rules_overlapping_fields_can_be_merged_test.go b/rules_overlapping_fields_can_be_merged_test.go index 903367ea..dd153a78 100644 --- a/rules_overlapping_fields_can_be_merged_test.go +++ b/rules_overlapping_fields_can_be_merged_test.go @@ -314,7 +314,7 @@ func init() { }, }, }) - _ = graphql.NewObject(graphql.ObjectConfig{ + IntBox := graphql.NewObject(graphql.ObjectConfig{ Name: "IntBox", Interfaces: (graphql.InterfacesThunk)(func() []*graphql.Interface { return []*graphql.Interface{someBoxInterface} @@ -339,7 +339,7 @@ func init() { }, }, }) - _ = graphql.NewObject(graphql.ObjectConfig{ + NonNullStringBox1Impl := graphql.NewObject(graphql.ObjectConfig{ Name: "NonNullStringBox1Impl", Interfaces: (graphql.InterfacesThunk)(func() []*graphql.Interface { return []*graphql.Interface{someBoxInterface, nonNullStringBox1Interface} @@ -364,7 +364,7 @@ func init() { }, }, }) - _ = graphql.NewObject(graphql.ObjectConfig{ + NonNullStringBox2Impl := graphql.NewObject(graphql.ObjectConfig{ Name: "NonNullStringBox2Impl", Interfaces: (graphql.InterfacesThunk)(func() []*graphql.Interface { return []*graphql.Interface{someBoxInterface, nonNullStringBox2Interface} @@ -417,6 +417,11 @@ func init() { }, }, }), + Types: []graphql.Type{ + IntBox, + NonNullStringBox1Impl, + NonNullStringBox2Impl, + }, }) if err != nil { panic(err) diff --git a/schema.go b/schema.go index 758ea1dd..f3a4eaff 100644 --- a/schema.go +++ b/schema.go @@ -8,6 +8,7 @@ type SchemaConfig struct { Query *Object Mutation *Object Subscription *Object + Types []Type Directives []*Directive } @@ -30,6 +31,8 @@ type Schema struct { queryType *Object mutationType *Object subscriptionType *Object + implementations map[string][]*Object + possibleTypeMap map[string]map[string]bool } func NewSchema(config SchemaConfig) (Schema, error) { @@ -71,32 +74,59 @@ func NewSchema(config SchemaConfig) (Schema, error) { // Build type map now to detect any errors within this schema. typeMap := TypeMap{} - objectTypes := []*Object{ - schema.QueryType(), - schema.MutationType(), - schema.SubscriptionType(), - typeType, - schemaType, + initialTypes := []Type{} + if schema.QueryType() != nil { + initialTypes = append(initialTypes, schema.QueryType()) } - for _, objectType := range objectTypes { - if objectType == nil { - continue - } - if objectType.err != nil { - return schema, objectType.err + if schema.MutationType() != nil { + initialTypes = append(initialTypes, schema.MutationType()) + } + if schema.SubscriptionType() != nil { + initialTypes = append(initialTypes, schema.SubscriptionType()) + } + if schemaType != nil { + initialTypes = append(initialTypes, schemaType) + } + + for _, ttype := range config.Types { + // assume that user will never add a nil object to config + initialTypes = append(initialTypes, ttype) + } + + for _, ttype := range initialTypes { + if ttype.Error() != nil { + return schema, ttype.Error() } - typeMap, err = typeMapReducer(typeMap, objectType) + typeMap, err = typeMapReducer(&schema, typeMap, ttype) if err != nil { return schema, err } } + schema.typeMap = typeMap + + // Keep track of all implementations by interface name. + if schema.implementations == nil { + schema.implementations = map[string][]*Object{} + } + for _, ttype := range schema.typeMap { + if ttype, ok := ttype.(*Object); ok { + for _, iface := range ttype.Interfaces() { + impls, ok := schema.implementations[iface.Name()] + if impls == nil || !ok { + impls = []*Object{} + } + impls = append(impls, ttype) + schema.implementations[iface.Name()] = impls + } + } + } + // Enforce correct interface implementations - for _, ttype := range typeMap { - switch ttype := ttype.(type) { - case *Object: + for _, ttype := range schema.typeMap { + if ttype, ok := ttype.(*Object); ok { for _, iface := range ttype.Interfaces() { - err := assertObjectImplementsInterface(ttype, iface) + err := assertObjectImplementsInterface(&schema, ttype, iface) if err != nil { return schema, err } @@ -140,7 +170,39 @@ func (gq *Schema) Type(name string) Type { return gq.TypeMap()[name] } -func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { +func (gq *Schema) PossibleTypes(abstractType Abstract) []*Object { + if abstractType, ok := abstractType.(*Union); ok { + return abstractType.Types() + } + if abstractType, ok := abstractType.(*Interface); ok { + if impls, ok := gq.implementations[abstractType.Name()]; ok { + return impls + } + } + return []*Object{} +} +func (gq *Schema) IsPossibleType(abstractType Abstract, possibleType *Object) bool { + possibleTypeMap := gq.possibleTypeMap + if possibleTypeMap == nil { + possibleTypeMap = map[string]map[string]bool{} + } + + if typeMap, ok := possibleTypeMap[abstractType.Name()]; !ok { + typeMap = map[string]bool{} + for _, possibleType := range gq.PossibleTypes(abstractType) { + typeMap[possibleType.Name()] = true + } + possibleTypeMap[abstractType.Name()] = typeMap + } + + gq.possibleTypeMap = possibleTypeMap + if typeMap, ok := possibleTypeMap[abstractType.Name()]; ok { + isPossible, _ := typeMap[possibleType.Name()] + return isPossible + } + return false +} +func typeMapReducer(schema *Schema, typeMap TypeMap, objectType Type) (TypeMap, error) { var err error if objectType == nil || objectType.Name() == "" { return typeMap, nil @@ -149,11 +211,11 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { switch objectType := objectType.(type) { case *List: if objectType.OfType != nil { - return typeMapReducer(typeMap, objectType.OfType) + return typeMapReducer(schema, typeMap, objectType.OfType) } case *NonNull: if objectType.OfType != nil { - return typeMapReducer(typeMap, objectType.OfType) + return typeMapReducer(schema, typeMap, objectType.OfType) } case *Object: if objectType.err != nil { @@ -179,7 +241,7 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { switch objectType := objectType.(type) { case *Union: - types := objectType.PossibleTypes() + types := schema.PossibleTypes(objectType) if objectType.err != nil { return typeMap, objectType.err } @@ -187,13 +249,13 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { if innerObjectType.err != nil { return typeMap, innerObjectType.err } - typeMap, err = typeMapReducer(typeMap, innerObjectType) + typeMap, err = typeMapReducer(schema, typeMap, innerObjectType) if err != nil { return typeMap, err } } case *Interface: - types := objectType.PossibleTypes() + types := schema.PossibleTypes(objectType) if objectType.err != nil { return typeMap, objectType.err } @@ -201,7 +263,7 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { if innerObjectType.err != nil { return typeMap, innerObjectType.err } - typeMap, err = typeMapReducer(typeMap, innerObjectType) + typeMap, err = typeMapReducer(schema, typeMap, innerObjectType) if err != nil { return typeMap, err } @@ -215,7 +277,7 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { if innerObjectType.err != nil { return typeMap, innerObjectType.err } - typeMap, err = typeMapReducer(typeMap, innerObjectType) + typeMap, err = typeMapReducer(schema, typeMap, innerObjectType) if err != nil { return typeMap, err } @@ -230,12 +292,12 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { } for _, field := range fieldMap { for _, arg := range field.Args { - typeMap, err = typeMapReducer(typeMap, arg.Type) + typeMap, err = typeMapReducer(schema, typeMap, arg.Type) if err != nil { return typeMap, err } } - typeMap, err = typeMapReducer(typeMap, field.Type) + typeMap, err = typeMapReducer(schema, typeMap, field.Type) if err != nil { return typeMap, err } @@ -247,12 +309,12 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { } for _, field := range fieldMap { for _, arg := range field.Args { - typeMap, err = typeMapReducer(typeMap, arg.Type) + typeMap, err = typeMapReducer(schema, typeMap, arg.Type) if err != nil { return typeMap, err } } - typeMap, err = typeMapReducer(typeMap, field.Type) + typeMap, err = typeMapReducer(schema, typeMap, field.Type) if err != nil { return typeMap, err } @@ -263,7 +325,7 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { return typeMap, objectType.err } for _, field := range fieldMap { - typeMap, err = typeMapReducer(typeMap, field.Type) + typeMap, err = typeMapReducer(schema, typeMap, field.Type) if err != nil { return typeMap, err } @@ -272,7 +334,7 @@ func typeMapReducer(typeMap TypeMap, objectType Type) (TypeMap, error) { return typeMap, nil } -func assertObjectImplementsInterface(object *Object, iface *Interface) error { +func assertObjectImplementsInterface(schema *Schema, object *Object, iface *Interface) error { objectFieldMap := object.Fields() ifaceFieldMap := iface.Fields() @@ -294,7 +356,7 @@ func assertObjectImplementsInterface(object *Object, iface *Interface) error { // Assert interface field type is satisfied by object field type, by being // a valid subtype. (covariant) err = invariant( - isTypeSubTypeOf(objectField.Type, ifaceField.Type), + isTypeSubTypeOf(schema, objectField.Type, ifaceField.Type), fmt.Sprintf(`%v.%v expects type "%v" but `+ `%v.%v provides type "%v".`, iface, fieldName, ifaceField.Type, @@ -395,7 +457,7 @@ func isEqualType(typeA Type, typeB Type) bool { * Provided a type and a super type, return true if the first type is either * equal or a subset of the second super type (covariant). */ -func isTypeSubTypeOf(maybeSubType Type, superType Type) bool { +func isTypeSubTypeOf(schema *Schema, maybeSubType Type, superType Type) bool { // Equivalent type is a valid subtype if maybeSubType == superType { return true @@ -404,19 +466,19 @@ func isTypeSubTypeOf(maybeSubType Type, superType Type) bool { // If superType is non-null, maybeSubType must also be nullable. if superType, ok := superType.(*NonNull); ok { if maybeSubType, ok := maybeSubType.(*NonNull); ok { - return isTypeSubTypeOf(maybeSubType.OfType, superType.OfType) + return isTypeSubTypeOf(schema, maybeSubType.OfType, superType.OfType) } return false } if maybeSubType, ok := maybeSubType.(*NonNull); ok { // If superType is nullable, maybeSubType may be non-null. - return isTypeSubTypeOf(maybeSubType.OfType, superType) + return isTypeSubTypeOf(schema, maybeSubType.OfType, superType) } // If superType type is a list, maybeSubType type must also be a list. if superType, ok := superType.(*List); ok { if maybeSubType, ok := maybeSubType.(*List); ok { - return isTypeSubTypeOf(maybeSubType.OfType, superType.OfType) + return isTypeSubTypeOf(schema, maybeSubType.OfType, superType.OfType) } return false } else if _, ok := maybeSubType.(*List); ok { @@ -427,12 +489,12 @@ func isTypeSubTypeOf(maybeSubType Type, superType Type) bool { // If superType type is an abstract type, maybeSubType type may be a currently // possible object type. if superType, ok := superType.(*Interface); ok { - if maybeSubType, ok := maybeSubType.(*Object); ok && superType.IsPossibleType(maybeSubType) { + if maybeSubType, ok := maybeSubType.(*Object); ok && schema.IsPossibleType(superType, maybeSubType) { return true } } if superType, ok := superType.(*Union); ok { - if maybeSubType, ok := maybeSubType.(*Object); ok && superType.IsPossibleType(maybeSubType) { + if maybeSubType, ok := maybeSubType.(*Object); ok && schema.IsPossibleType(superType, maybeSubType) { return true } } diff --git a/testutil/rules_test_harness.go b/testutil/rules_test_harness.go index 688c47a3..6a8429f5 100644 --- a/testutil/rules_test_harness.go +++ b/testutil/rules_test_harness.go @@ -466,6 +466,12 @@ func init() { graphql.IncludeDirective, graphql.SkipDirective, }, + Types: []graphql.Type{ + catType, + dogType, + humanType, + alienType, + }, }) if err != nil { panic(err) diff --git a/union_interface_test.go b/union_interface_test.go index 2da90f40..80ecbd97 100644 --- a/union_interface_test.go +++ b/union_interface_test.go @@ -111,6 +111,7 @@ var personType = graphql.NewObject(graphql.ObjectConfig{ var unionInterfaceTestSchema, _ = graphql.NewSchema(graphql.SchemaConfig{ Query: personType, + Types: []graphql.Type{petType}, }) var garfield = &testCat2{"Garfield", false} @@ -206,8 +207,8 @@ func TestUnionIntersectionTypes_CanIntrospectOnUnionAndIntersectionTypes(t *test if len(result.Errors) != len(expected.Errors) { t.Fatalf("Unexpected errors, Diff: %v", testutil.Diff(expected.Errors, result.Errors)) } - if !reflect.DeepEqual(expected, result) { - t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + if !testutil.ContainSubset(expected.Data.(map[string]interface{}), result.Data.(map[string]interface{})) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected.Data, result.Data)) } } func TestUnionIntersectionTypes_ExecutesUsingUnionTypes(t *testing.T) { diff --git a/validation_test.go b/validation_test.go index 85d2c98d..e43c2e96 100644 --- a/validation_test.go +++ b/validation_test.go @@ -113,6 +113,7 @@ func schemaWithFieldType(ttype graphql.Output) (graphql.Schema, error) { }, }, }), + Types: []graphql.Type{ttype}, }) } func schemaWithInputObject(ttype graphql.Input) (graphql.Schema, error) { @@ -173,6 +174,7 @@ func schemaWithObjectImplementingType(implementedType *graphql.Interface) (graph }, }, }), + Types: []graphql.Type{badObjectType}, }) } func schemaWithUnionOfType(ttype *graphql.Object) (graphql.Schema, error) { @@ -396,7 +398,7 @@ func TestTypeSystem_SchemaMustContainUniquelyNamedTypes_RejectsASchemaWhichHaveS }, }, }) - _ = graphql.NewObject(graphql.ObjectConfig{ + FirstBadObject := graphql.NewObject(graphql.ObjectConfig{ Name: "BadObject", Interfaces: []*graphql.Interface{ anotherInterface, @@ -407,7 +409,7 @@ func TestTypeSystem_SchemaMustContainUniquelyNamedTypes_RejectsASchemaWhichHaveS }, }, }) - _ = graphql.NewObject(graphql.ObjectConfig{ + SecondBadObject := graphql.NewObject(graphql.ObjectConfig{ Name: "BadObject", Interfaces: []*graphql.Interface{ anotherInterface, @@ -428,6 +430,7 @@ func TestTypeSystem_SchemaMustContainUniquelyNamedTypes_RejectsASchemaWhichHaveS }) _, err := graphql.NewSchema(graphql.SchemaConfig{ Query: queryType, + Types: []graphql.Type{FirstBadObject, SecondBadObject}, }) expectedError := `Schema must contain unique named types but contains multiple types named "BadObject".` if err == nil || err.Error() != expectedError { From 1e33c35ba9684a3ca02bf0de8b24688636cd3295 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 22:17:08 +0800 Subject: [PATCH 24/32] [RFC] Add explicit context arg to graphql execution This **BREAKING** change introduces a new argument to the GraphQL execution API which is presented to resolution functions: `context`. Removed `Schema` from `ResolveParams`, already available in `ResolveParams.Info`. `IsTypeOf` and `ResolveType` params are now struct, similar to `FieldResolveFn`. `context` is now available for resolving types in `IsTypeOf` and `ResolveType`, similar to `FieldResolveFn`. Commit: d7cc6f9aed462588291bc821238650c98ad53580 [d7cc6f9] Parents: 576b6a15d1 Author: Lee Byron Date: 23 March 2016 at 10:30:13 AM SGT Commit Date: 25 March 2016 at 8:20:10 AM SGT --- abstract_test.go | 32 ++++----- definition.go | 53 ++++++++++++--- definition_test.go | 6 +- executor.go | 40 +++++++---- executor_test.go | 4 +- introspection.go | 4 +- ...s_overlapping_fields_can_be_merged_test.go | 6 +- testutil/rules_test_harness.go | 14 ++-- testutil/testutil.go | 4 +- union_interface_test.go | 51 +++++++++----- validation_test.go | 68 +++++++++---------- 11 files changed, 175 insertions(+), 107 deletions(-) diff --git a/abstract_test.go b/abstract_test.go index 8ebbb10e..fd16b859 100644 --- a/abstract_test.go +++ b/abstract_test.go @@ -41,8 +41,8 @@ func TestIsTypeOfUsedToResolveRuntimeTypeForInterface(t *testing.T) { Interfaces: []*graphql.Interface{ petType, }, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testDog) + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + _, ok := p.Value.(*testDog) return ok }, Fields: graphql.Fields{ @@ -72,8 +72,8 @@ func TestIsTypeOfUsedToResolveRuntimeTypeForInterface(t *testing.T) { Interfaces: []*graphql.Interface{ petType, }, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testCat) + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + _, ok := p.Value.(*testCat) return ok }, Fields: graphql.Fields{ @@ -162,8 +162,8 @@ func TestIsTypeOfUsedToResolveRuntimeTypeForUnion(t *testing.T) { dogType := graphql.NewObject(graphql.ObjectConfig{ Name: "Dog", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testDog) + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + _, ok := p.Value.(*testDog) return ok }, Fields: graphql.Fields{ @@ -177,8 +177,8 @@ func TestIsTypeOfUsedToResolveRuntimeTypeForUnion(t *testing.T) { }) catType := graphql.NewObject(graphql.ObjectConfig{ Name: "Cat", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testCat) + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + _, ok := p.Value.(*testCat) return ok }, Fields: graphql.Fields{ @@ -270,14 +270,14 @@ func TestResolveTypeOnInterfaceYieldsUsefulError(t *testing.T) { Type: graphql.String, }, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { - if _, ok := value.(*testCat); ok { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { + if _, ok := p.Value.(*testCat); ok { return catType } - if _, ok := value.(*testDog); ok { + if _, ok := p.Value.(*testDog); ok { return dogType } - if _, ok := value.(*testHuman); ok { + if _, ok := p.Value.(*testHuman); ok { return humanType } return nil @@ -425,14 +425,14 @@ func TestResolveTypeOnUnionYieldsUsefulError(t *testing.T) { Types: []*graphql.Object{ dogType, catType, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { - if _, ok := value.(*testCat); ok { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { + if _, ok := p.Value.(*testCat); ok { return catType } - if _, ok := value.(*testDog); ok { + if _, ok := p.Value.(*testDog); ok { return dogType } - if _, ok := value.(*testHuman); ok { + if _, ok := p.Value.(*testHuman); ok { return humanType } return nil diff --git a/definition.go b/definition.go index 141770bd..a3b03d22 100644 --- a/definition.go +++ b/definition.go @@ -369,7 +369,22 @@ type Object struct { err error } -type IsTypeOfFn func(value interface{}, info ResolveInfo) bool +// IsTypeOfParams Params for IsTypeOfFn() +type IsTypeOfParams struct { + // Value that needs to be resolve. + // Use this to decide which GraphQLObject this value maps to. + Value interface{} + + // Info is a collection of information about the current execution state. + Info ResolveInfo + + // Context argument is a context value that is provided to every resolve function within an execution. + // It is commonly + // used to represent an authenticated user, or request-specific caches. + Context context.Context +} + +type IsTypeOfFn func(p IsTypeOfParams) bool type InterfacesThunk func() []*Interface @@ -560,14 +575,19 @@ func defineFieldMap(ttype Named, fields Fields) (FieldDefinitionMap, error) { } // ResolveParams Params for FieldResolveFn() -// TODO: clean up GQLFRParams fields type ResolveParams struct { + // Source is the source value Source interface{} - Args map[string]interface{} - Info ResolveInfo - Schema Schema - //This can be used to provide per-request state - //from the application. + + // Args is a map of arguments for current GraphQL request + Args map[string]interface{} + + // Info is a collection of information about the current execution state. + Info ResolveInfo + + // Context argument is a context value that is provided to every resolve function within an execution. + // It is commonly + // used to represent an authenticated user, or request-specific caches. Context context.Context } @@ -666,7 +686,7 @@ type Interface struct { typeConfig InterfaceConfig fields FieldDefinitionMap - err error + err error } type InterfaceConfig struct { Name string `json:"name"` @@ -675,7 +695,22 @@ type InterfaceConfig struct { Description string `json:"description"` } -type ResolveTypeFn func(value interface{}, info ResolveInfo) *Object +// ResolveTypeParams Params for ResolveTypeFn() +type ResolveTypeParams struct { + // Value that needs to be resolve. + // Use this to decide which GraphQLObject this value maps to. + Value interface{} + + // Info is a collection of information about the current execution state. + Info ResolveInfo + + // Context argument is a context value that is provided to every resolve function within an execution. + // It is commonly + // used to represent an authenticated user, or request-specific caches. + Context context.Context +} + +type ResolveTypeFn func(p ResolveTypeParams) *Object func NewInterface(config InterfaceConfig) *Interface { it := &Interface{} diff --git a/definition_test.go b/definition_test.go index 27413295..715250e0 100644 --- a/definition_test.go +++ b/definition_test.go @@ -114,7 +114,7 @@ var blogSubscription = graphql.NewObject(graphql.ObjectConfig{ var objectType = graphql.NewObject(graphql.ObjectConfig{ Name: "Object", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, }) @@ -352,7 +352,7 @@ func TestTypeSystem_DefinitionExample_IncludesInterfacesSubTypesInTheTypeMap(t * }, }, Interfaces: []*graphql.Interface{someInterface}, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, }) @@ -396,7 +396,7 @@ func TestTypeSystem_DefinitionExample_IncludesInterfacesThunkSubtypesInTheTypeMa Interfaces: (graphql.InterfacesThunk)(func() []*graphql.Interface { return []*graphql.Interface{someInterface} }), - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, }) diff --git a/executor.go b/executor.go index 99966bd0..9abbd9a3 100644 --- a/executor.go +++ b/executor.go @@ -526,8 +526,6 @@ func resolveField(eCtx *ExecutionContext, parentType *Object, source interface{} // TODO: find a way to memoize, in case this field is within a List type. args, _ := getArgumentValues(fieldDef.Args, fieldAST.Arguments, eCtx.VariableValues) - // The resolve function's optional third argument is a collection of - // information about the current execution state. info := ResolveInfo{ FieldName: fieldName, FieldASTs: fieldASTs, @@ -545,7 +543,6 @@ func resolveField(eCtx *ExecutionContext, parentType *Object, source interface{} result, resolveFnError = resolveFn(ResolveParams{ Source: source, Args: args, - Schema: eCtx.Schema, Info: info, Context: eCtx.Context, }) @@ -664,12 +661,17 @@ func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldAST var runtimeType *Object + resolveTypeParams := ResolveTypeParams{ + Value: result, + Info: info, + Context: eCtx.Context, + } if unionReturnType, ok := returnType.(*Union); ok && unionReturnType.ResolveType != nil { - runtimeType = unionReturnType.ResolveType(result, info) + runtimeType = unionReturnType.ResolveType(resolveTypeParams) } else if interfaceReturnType, ok := returnType.(*Interface); ok && interfaceReturnType.ResolveType != nil { - runtimeType = interfaceReturnType.ResolveType(result, info) + runtimeType = interfaceReturnType.ResolveType(resolveTypeParams) } else { - runtimeType = defaultResolveTypeFn(result, info, returnType) + runtimeType = defaultResolveTypeFn(resolveTypeParams, returnType) } if runtimeType == nil { @@ -692,10 +694,17 @@ func completeObjectValue(eCtx *ExecutionContext, returnType *Object, fieldASTs [ // If there is an isTypeOf predicate function, call it with the // current result. If isTypeOf returns false, then raise an error rather // than continuing execution. - if returnType.IsTypeOf != nil && !returnType.IsTypeOf(result, info) { - panic(gqlerrors.NewFormattedError( - fmt.Sprintf(`Expected value of type "%v" but got: %T.`, returnType, result), - )) + if returnType.IsTypeOf != nil { + p := IsTypeOfParams{ + Value: result, + Info: info, + Context: eCtx.Context, + } + if !returnType.IsTypeOf(p) { + panic(gqlerrors.NewFormattedError( + fmt.Sprintf(`Expected value of type "%v" but got: %T.`, returnType, result), + )) + } } // Collect sub-fields to execute to complete this value. @@ -767,13 +776,18 @@ func completeListValue(eCtx *ExecutionContext, returnType *List, fieldASTs []*as // defaultResolveTypeFn If a resolveType function is not given, then a default resolve behavior is // used which tests each possible type for the abstract type by calling // isTypeOf for the object being coerced, returning the first type that matches. -func defaultResolveTypeFn(value interface{}, info ResolveInfo, abstractType Abstract) *Object { - possibleTypes := info.Schema.PossibleTypes(abstractType) +func defaultResolveTypeFn(p ResolveTypeParams, abstractType Abstract) *Object { + possibleTypes := p.Info.Schema.PossibleTypes(abstractType) for _, possibleType := range possibleTypes { if possibleType.IsTypeOf == nil { continue } - if res := possibleType.IsTypeOf(value, info); res { + isTypeOfParams := IsTypeOfParams{ + Value: p.Value, + Info: p.Info, + Context: p.Context, + } + if res := possibleType.IsTypeOf(isTypeOfParams); res { return possibleType } } diff --git a/executor_test.go b/executor_test.go index e9fc2592..5a1ad7c5 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1261,8 +1261,8 @@ func TestFailsWhenAnIsTypeOfCheckIsNotMet(t *testing.T) { specialType := graphql.NewObject(graphql.ObjectConfig{ Name: "SpecialType", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - if _, ok := value.(testSpecialType); ok { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + if _, ok := p.Value.(testSpecialType); ok { return true } return false diff --git a/introspection.go b/introspection.go index 096b10cd..8b67ed0a 100644 --- a/introspection.go +++ b/introspection.go @@ -491,9 +491,9 @@ func init() { Resolve: func(p ResolveParams) (interface{}, error) { switch ttype := p.Source.(type) { case *Interface: - return p.Schema.PossibleTypes(ttype), nil + return p.Info.Schema.PossibleTypes(ttype), nil case *Union: - return p.Schema.PossibleTypes(ttype), nil + return p.Info.Schema.PossibleTypes(ttype), nil } return nil, nil }, diff --git a/rules_overlapping_fields_can_be_merged_test.go b/rules_overlapping_fields_can_be_merged_test.go index dd153a78..a7827f30 100644 --- a/rules_overlapping_fields_can_be_merged_test.go +++ b/rules_overlapping_fields_can_be_merged_test.go @@ -291,7 +291,7 @@ var schema graphql.Schema func init() { someBoxInterface = graphql.NewInterface(graphql.InterfaceConfig{ Name: "SomeBox", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return stringBoxObject }, Fields: graphql.Fields{ @@ -330,7 +330,7 @@ func init() { }) var nonNullStringBox1Interface = graphql.NewInterface(graphql.InterfaceConfig{ Name: "NonNullStringBox1", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return stringBoxObject }, Fields: graphql.Fields{ @@ -355,7 +355,7 @@ func init() { }) var nonNullStringBox2Interface = graphql.NewInterface(graphql.InterfaceConfig{ Name: "NonNullStringBox2", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return stringBoxObject }, Fields: graphql.Fields{ diff --git a/testutil/rules_test_harness.go b/testutil/rules_test_harness.go index 6a8429f5..035d8a61 100644 --- a/testutil/rules_test_harness.go +++ b/testutil/rules_test_harness.go @@ -70,7 +70,7 @@ func init() { }) var dogType = graphql.NewObject(graphql.ObjectConfig{ Name: "Dog", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Fields: graphql.Fields{ @@ -146,7 +146,7 @@ func init() { var catType = graphql.NewObject(graphql.ObjectConfig{ Name: "Cat", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Fields: graphql.Fields{ @@ -182,7 +182,7 @@ func init() { dogType, catType, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { // not used for validation return nil }, @@ -198,7 +198,7 @@ func init() { var humanType = graphql.NewObject(graphql.ObjectConfig{ Name: "Human", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Interfaces: []*graphql.Interface{ @@ -229,7 +229,7 @@ func init() { var alienType = graphql.NewObject(graphql.ObjectConfig{ Name: "Alien", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Interfaces: []*graphql.Interface{ @@ -259,7 +259,7 @@ func init() { dogType, humanType, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { // not used for validation return nil }, @@ -270,7 +270,7 @@ func init() { alienType, humanType, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { // not used for validation return nil }, diff --git a/testutil/testutil.go b/testutil/testutil.go index 9951d8f3..0a5ccc87 100644 --- a/testutil/testutil.go +++ b/testutil/testutil.go @@ -133,8 +133,8 @@ func init() { Description: "Which movies they appear in.", }, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { - if character, ok := value.(StarWarsChar); ok { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { + if character, ok := p.Value.(StarWarsChar); ok { id, _ := strconv.Atoi(character.ID) human := GetHuman(id) if human.ID != "" { diff --git a/union_interface_test.go b/union_interface_test.go index 80ecbd97..d4cc151b 100644 --- a/union_interface_test.go +++ b/union_interface_test.go @@ -6,6 +6,7 @@ import ( "github.com/graphql-go/graphql" "github.com/graphql-go/graphql/testutil" + "golang.org/x/net/context" ) type testNamedType interface { @@ -49,8 +50,8 @@ var dogType = graphql.NewObject(graphql.ObjectConfig{ Type: graphql.Boolean, }, }, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testDog2) + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + _, ok := p.Value.(*testDog2) return ok }, }) @@ -67,8 +68,8 @@ var catType = graphql.NewObject(graphql.ObjectConfig{ Type: graphql.Boolean, }, }, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testCat2) + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + _, ok := p.Value.(*testCat2) return ok }, }) @@ -77,11 +78,11 @@ var petType = graphql.NewUnion(graphql.UnionConfig{ Types: []*graphql.Object{ dogType, catType, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { - if _, ok := value.(*testCat2); ok { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { + if _, ok := p.Value.(*testCat2); ok { return catType } - if _, ok := value.(*testDog2); ok { + if _, ok := p.Value.(*testDog2); ok { return dogType } return nil @@ -103,8 +104,8 @@ var personType = graphql.NewObject(graphql.ObjectConfig{ Type: graphql.NewList(namedType), }, }, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { - _, ok := value.(*testPerson) + IsTypeOf: func(p graphql.IsTypeOfParams) bool { + _, ok := p.Value.(*testPerson) return ok }, }) @@ -498,6 +499,9 @@ func TestUnionIntersectionTypes_AllowsFragmentConditionsToBeAbstractTypes(t *tes } func TestUnionIntersectionTypes_GetsExecutionInfoInResolver(t *testing.T) { + var encounteredContextValue string + var encounteredSchema graphql.Schema + var encounteredRootValue string var personType2 *graphql.Object namedType2 := graphql.NewInterface(graphql.InterfaceConfig{ @@ -507,7 +511,10 @@ func TestUnionIntersectionTypes_GetsExecutionInfoInResolver(t *testing.T) { Type: graphql.String, }, }, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { + encounteredSchema = p.Info.Schema + encounteredContextValue, _ = p.Context.Value("authToken").(string) + encounteredRootValue = p.Info.RootValue.(*testPerson).Name return personType2 }, }) @@ -552,17 +559,29 @@ func TestUnionIntersectionTypes_GetsExecutionInfoInResolver(t *testing.T) { // parse query ast := testutil.TestParse(t, doc) + // create context + ctx := context.Background() + ctx = context.WithValue(ctx, "authToken", "contextStringValue123") + // execute ep := graphql.ExecuteParams{ - Schema: schema2, - AST: ast, - Root: john2, + Schema: schema2, + AST: ast, + Root: john2, + Context: ctx, } result := testutil.TestExecute(t, ep) - if len(result.Errors) != len(expected.Errors) { - t.Fatalf("Unexpected errors, Diff: %v", testutil.Diff(expected.Errors, result.Errors)) - } + if !reflect.DeepEqual(expected, result) { t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } + if !reflect.DeepEqual("contextStringValue123", encounteredContextValue) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff("contextStringValue123", encounteredContextValue)) + } + if !reflect.DeepEqual("John", encounteredRootValue) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff("John", encounteredRootValue)) + } + if !reflect.DeepEqual(schema2, encounteredSchema) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(schema2, encounteredSchema)) + } } diff --git a/validation_test.go b/validation_test.go index e43c2e96..6c8fc521 100644 --- a/validation_test.go +++ b/validation_test.go @@ -29,7 +29,7 @@ var someObjectType = graphql.NewObject(graphql.ObjectConfig{ }) var objectWithIsTypeOf = graphql.NewObject(graphql.ObjectConfig{ Name: "ObjectWithIsTypeOf", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Fields: graphql.Fields{ @@ -40,7 +40,7 @@ var objectWithIsTypeOf = graphql.NewObject(graphql.ObjectConfig{ }) var someUnionType = graphql.NewUnion(graphql.UnionConfig{ Name: "SomeUnion", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Types: []*graphql.Object{ @@ -49,7 +49,7 @@ var someUnionType = graphql.NewUnion(graphql.UnionConfig{ }) var someInterfaceType = graphql.NewInterface(graphql.InterfaceConfig{ Name: "SomeInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -181,7 +181,7 @@ func schemaWithUnionOfType(ttype *graphql.Object) (graphql.Schema, error) { badObjectType := graphql.NewUnion(graphql.UnionConfig{ Name: "BadUnion", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Types: []*graphql.Object{ttype}, @@ -389,7 +389,7 @@ func TestTypeSystem_SchemaMustContainUniquelyNamedTypes_RejectsASchemaWhichHaveS anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -548,7 +548,7 @@ func TestTypeSystem_FieldsArgsMustBeObjects_AcceptsAnObjectTypeWithFieldArgs(t * func TestTypeSystem_ObjectInterfacesMustBeArray_AcceptsAnObjectTypeWithArrayInterfaces(t *testing.T) { anotherInterfaceType := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -576,7 +576,7 @@ func TestTypeSystem_ObjectInterfacesMustBeArray_AcceptsAnObjectTypeWithArrayInte func TestTypeSystem_ObjectInterfacesMustBeArray_AcceptsAnObjectTypeWithInterfacesAsFunctionReturningAnArray(t *testing.T) { anotherInterfaceType := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -602,7 +602,7 @@ func TestTypeSystem_ObjectInterfacesMustBeArray_AcceptsAnObjectTypeWithInterface func TestTypeSystem_UnionTypesMustBeArray_AcceptsAUnionTypeWithArrayTypes(t *testing.T) { _, err := schemaWithFieldType(graphql.NewUnion(graphql.UnionConfig{ Name: "SomeUnion", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Types: []*graphql.Object{ @@ -616,7 +616,7 @@ func TestTypeSystem_UnionTypesMustBeArray_AcceptsAUnionTypeWithArrayTypes(t *tes func TestTypeSystem_UnionTypesMustBeArray_RejectsAUnionTypeWithoutTypes(t *testing.T) { _, err := schemaWithFieldType(graphql.NewUnion(graphql.UnionConfig{ Name: "SomeUnion", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, })) @@ -628,7 +628,7 @@ func TestTypeSystem_UnionTypesMustBeArray_RejectsAUnionTypeWithoutTypes(t *testi func TestTypeSystem_UnionTypesMustBeArray_RejectsAUnionTypeWithEmptyTypes(t *testing.T) { _, err := schemaWithFieldType(graphql.NewUnion(graphql.UnionConfig{ Name: "SomeUnion", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Types: []*graphql.Object{}, @@ -692,7 +692,7 @@ func TestTypeSystem_InputObjectsMustHaveFields_RejectsAnInputObjectTypeWithEmpty func TestTypeSystem_ObjectTypesMustBeAssertable_AcceptsAnObjectTypeWithAnIsTypeOfFunction(t *testing.T) { _, err := schemaWithFieldType(graphql.NewObject(graphql.ObjectConfig{ Name: "AnotherObject", - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Fields: graphql.Fields{ @@ -710,7 +710,7 @@ func TestTypeSystem_InterfaceTypesMustBeResolvable_AcceptsAnInterfaceTypeDefinin anotherInterfaceType := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -745,7 +745,7 @@ func TestTypeSystem_InterfaceTypesMustBeResolvable_AcceptsAnInterfaceWithImpleme _, err := schemaWithFieldType(graphql.NewObject(graphql.ObjectConfig{ Name: "SomeObject", Interfaces: []*graphql.Interface{anotherInterfaceType}, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Fields: graphql.Fields{ @@ -763,7 +763,7 @@ func TestTypeSystem_InterfaceTypesMustBeResolvable_AcceptsAnInterfaceTypeDefinin anotherInterfaceType := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -775,7 +775,7 @@ func TestTypeSystem_InterfaceTypesMustBeResolvable_AcceptsAnInterfaceTypeDefinin _, err := schemaWithFieldType(graphql.NewObject(graphql.ObjectConfig{ Name: "SomeObject", Interfaces: []*graphql.Interface{anotherInterfaceType}, - IsTypeOf: func(value interface{}, info graphql.ResolveInfo) bool { + IsTypeOf: func(p graphql.IsTypeOfParams) bool { return true }, Fields: graphql.Fields{ @@ -794,7 +794,7 @@ func TestTypeSystem_UnionTypesMustBeResolvable_AcceptsAUnionTypeDefiningResolveT _, err := schemaWithFieldType(graphql.NewUnion(graphql.UnionConfig{ Name: "SomeUnion", Types: []*graphql.Object{someObjectType}, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, })) @@ -817,7 +817,7 @@ func TestTypeSystem_UnionTypesMustBeResolvable_AcceptsAUnionTypeDefiningResolveT _, err := schemaWithFieldType(graphql.NewUnion(graphql.UnionConfig{ Name: "SomeUnion", Types: []*graphql.Object{objectWithIsTypeOf}, - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, })) @@ -985,7 +985,7 @@ func TestTypeSystem_ObjectFieldsMustHaveOutputTypes_RejectsAnEmptyObjectFieldTyp func TestTypeSystem_ObjectsCanOnlyImplementInterfaces_AcceptsAnObjectImplementingAnInterface(t *testing.T) { anotherInterfaceType := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1123,7 +1123,7 @@ func TestTypeSystem_NonNullMustAcceptGraphQLTypes_RejectsNilAsNonNullableType(t func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWhichImplementsAnInterface(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1159,7 +1159,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWhi func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWhichImplementsAnInterfaceAlongWithMoreFields(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1198,7 +1198,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWhi func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWhichImpementsAnInterfaceFieldAlongWithAdditionalOptionalArguments(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1237,7 +1237,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWhi func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWhichImplementsAnInterfaceFieldAlongWithAdditionalRequiredArguments(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1277,7 +1277,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWhi func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectMissingAnInterfaceField(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1310,7 +1310,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectMis func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWithAnIncorrectlyTypedInterfaceField(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1356,7 +1356,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWit anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1385,7 +1385,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWit var anotherInterface *graphql.Interface anotherInterface = graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: (graphql.FieldsThunk)(func() graphql.Fields { @@ -1416,7 +1416,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWit func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWithASubtypedInterfaceField_Union(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1442,7 +1442,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWit func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectMissingAnInterfaceArgument(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1474,7 +1474,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectMis func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWithAnIncorrectlyTypedInterfaceArgument(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1511,7 +1511,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWit func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWithAnEquivalentlyModifiedInterfaceField(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1537,7 +1537,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWit func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWithANonListInterfaceFieldListType(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1565,7 +1565,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWit func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWithAListInterfaceFieldNonListType(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1593,7 +1593,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWit func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWithSubsetNonNullInterfaceFieldType(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ @@ -1620,7 +1620,7 @@ func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_AcceptsAnObjectWit func TestTypeSystem_ObjectsMustAdhereToInterfaceTheyImplement_RejectsAnObjectWithASupersetNullableInterfaceFieldType(t *testing.T) { anotherInterface := graphql.NewInterface(graphql.InterfaceConfig{ Name: "AnotherInterface", - ResolveType: func(value interface{}, info graphql.ResolveInfo) *graphql.Object { + ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return nil }, Fields: graphql.Fields{ From 8c471432bf6063600f9e30aaae40f489ac2bc9ce Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 22:34:07 +0800 Subject: [PATCH 25/32] Improve coercion error messages * Replace silent conversion to null with an invariant and a useful error message Commit: dea5aacca01f4429026bea3d97ea7cbc88b33bcf [dea5aac] Parents: 39744381d5 Author: Jeff Moore Date: 5 April 2016 at 12:48:14 PM SGT Committer: Lee Byron --- executor.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/executor.go b/executor.go index 9abbd9a3..67ecd2b4 100644 --- a/executor.go +++ b/executor.go @@ -674,11 +674,14 @@ func completeAbstractValue(eCtx *ExecutionContext, returnType Abstract, fieldAST runtimeType = defaultResolveTypeFn(resolveTypeParams, returnType) } - if runtimeType == nil { - return nil + err := invariant(runtimeType != nil, + fmt.Sprintf(`Could not determine runtime type of value "%v" for field %v.%v.`, result, info.ParentType, info.FieldName), + ) + if err != nil { + panic(err) } - if runtimeType != nil && !eCtx.Schema.IsPossibleType(returnType, runtimeType) { + if !eCtx.Schema.IsPossibleType(returnType, runtimeType) { panic(gqlerrors.NewFormattedError( fmt.Sprintf(`Runtime Object type "%v" is not a possible type `+ `for "%v".`, runtimeType, returnType), From 6e675a844d0c9b638905000b2238375f702ddfa0 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 22:39:15 +0800 Subject: [PATCH 26/32] Test that `executor` threads `RootValue` correctly - Similar to `context` test --- executor_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/executor_test.go b/executor_test.go index 5a1ad7c5..2c5867f8 100644 --- a/executor_test.go +++ b/executor_test.go @@ -405,6 +405,56 @@ func TestCorrectlyThreadsArguments(t *testing.T) { } } +func TestThreadsRootValueContextCorrectly(t *testing.T) { + + query := ` + query Example { a } + ` + + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Type", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + Resolve: func(p graphql.ResolveParams) (interface{}, error) { + val, _ := p.Info.RootValue.(map[string]interface{})["stringKey"].(string) + return val, nil + }, + }, + }, + }), + }) + if err != nil { + t.Fatalf("Error in schema %v", err.Error()) + } + + // parse query + ast := testutil.TestParse(t, query) + + // execute + ep := graphql.ExecuteParams{ + Schema: schema, + AST: ast, + Root: map[string]interface{}{ + "stringKey": "stringValue", + }, + } + result := testutil.TestExecute(t, ep) + if len(result.Errors) > 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } + + expected := &graphql.Result{ + Data: map[string]interface{}{ + "a": "stringValue", + }, + } + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + } +} + func TestThreadsContextCorrectly(t *testing.T) { query := ` From 95bc03226d01fcc996fbc118c7e6c7f9010d6f7a Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 22:50:16 +0800 Subject: [PATCH 27/32] Fix bug where @include directive is ignored if @skip is present. Commit: d6da0bff7f877e6a4fb66119796809f9c207f841 [d6da0bf] Parents: 136630f8fd Author: Dylan Thacker-Smith Date: 5 April 2016 at 12:56:27 PM SGT Committer: Lee Byron Labels: HEAD --- directives_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++ executor.go | 7 +++--- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/directives_test.go b/directives_test.go index f376015c..13945c46 100644 --- a/directives_test.go +++ b/directives_test.go @@ -515,3 +515,66 @@ func TestDirectivesWorksOnFragmentUnlessTrueOmitsFragment(t *testing.T) { t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } } + +func TestDirectivesWorksWithSkipAndIncludeDirectives_IncludeAndNoSkip(t *testing.T) { + query := `{ a, b @include(if: true) @skip(if: false) }` + expected := &graphql.Result{ + Data: map[string]interface{}{ + "a": "a", + "b": "b", + }, + } + result := executeDirectivesTestQuery(t, query) + if len(result.Errors) != 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + } +} + +func TestDirectivesWorksWithSkipAndIncludeDirectives_IncludeAndSkip(t *testing.T) { + query := `{ a, b @include(if: true) @skip(if: true) }` + expected := &graphql.Result{ + Data: map[string]interface{}{ + "a": "a", + }, + } + result := executeDirectivesTestQuery(t, query) + if len(result.Errors) != 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + } +} +func TestDirectivesWorksWithSkipAndIncludeDirectives_NoIncludeAndSkip(t *testing.T) { + query := `{ a, b @include(if: false) @skip(if: true) }` + expected := &graphql.Result{ + Data: map[string]interface{}{ + "a": "a", + }, + } + result := executeDirectivesTestQuery(t, query) + if len(result.Errors) != 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + } +} +func TestDirectivesWorksWithSkipAndIncludeDirectives_NoIncludeOrSkip(t *testing.T) { + query := `{ a, b @include(if: false) @skip(if: false) }` + expected := &graphql.Result{ + Data: map[string]interface{}{ + "a": "a", + }, + } + result := executeDirectivesTestQuery(t, query) + if len(result.Errors) != 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) + } +} diff --git a/executor.go b/executor.go index 67ecd2b4..edfcc169 100644 --- a/executor.go +++ b/executor.go @@ -371,12 +371,11 @@ func shouldIncludeNode(eCtx *ExecutionContext, directives []*ast.Directive) bool if err != nil { return defaultReturnValue } - if skipIf, ok := argValues["if"]; ok { - if boolSkipIf, ok := skipIf.(bool); ok { - return !boolSkipIf + if skipIf, ok := argValues["if"].(bool); ok { + if skipIf { + return false } } - return defaultReturnValue } for _, directive := range directives { if directive == nil || directive.Name == nil { From 15268b788c482969a25a84ed639bca350ec232eb Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 22:55:06 +0800 Subject: [PATCH 28/32] Spec compliant @skip/@include. Commit: 47f87fa701cb33fc1fb0ca65b4668c7a14a5ad11 [47f87fa] Parents: 9e5d959353 Author: Lee Byron Date: 5 April 2016 at 1:00:00 PM SGT remove non spec compliant test Commit: 07e627adda2b236e720ec352b21eb3043170c60f [07e627a] Parents: cf5b2340f9 Author: Lee Byron Date: 5 April 2016 at 1:09:42 PM SGT --- directives_test.go | 100 +-------------------------------------------- executor.go | 12 +++--- 2 files changed, 7 insertions(+), 105 deletions(-) diff --git a/directives_test.go b/directives_test.go index 13945c46..41d751d3 100644 --- a/directives_test.go +++ b/directives_test.go @@ -418,104 +418,6 @@ func TestDirectivesWorksOnAnonymousInlineFragmentUnlessTrueIncludesAnonymousInli } } -func TestDirectivesWorksOnFragmentIfFalseOmitsFragment(t *testing.T) { - query := ` - query Q { - a - ...Frag - } - fragment Frag on TestType @include(if: false) { - b - } - ` - expected := &graphql.Result{ - Data: map[string]interface{}{ - "a": "a", - }, - } - result := executeDirectivesTestQuery(t, query) - if len(result.Errors) != 0 { - t.Fatalf("wrong result, unexpected errors: %v", result.Errors) - } - if !reflect.DeepEqual(expected, result) { - t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) - } -} - -func TestDirectivesWorksOnFragmentIfTrueIncludesFragment(t *testing.T) { - query := ` - query Q { - a - ...Frag - } - fragment Frag on TestType @include(if: true) { - b - } - ` - expected := &graphql.Result{ - Data: map[string]interface{}{ - "a": "a", - "b": "b", - }, - } - result := executeDirectivesTestQuery(t, query) - if len(result.Errors) != 0 { - t.Fatalf("wrong result, unexpected errors: %v", result.Errors) - } - if !reflect.DeepEqual(expected, result) { - t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) - } -} - -func TestDirectivesWorksOnFragmentUnlessFalseIncludesFragment(t *testing.T) { - query := ` - query Q { - a - ...Frag - } - fragment Frag on TestType @skip(if: false) { - b - } - ` - expected := &graphql.Result{ - Data: map[string]interface{}{ - "a": "a", - "b": "b", - }, - } - result := executeDirectivesTestQuery(t, query) - if len(result.Errors) != 0 { - t.Fatalf("wrong result, unexpected errors: %v", result.Errors) - } - if !reflect.DeepEqual(expected, result) { - t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) - } -} - -func TestDirectivesWorksOnFragmentUnlessTrueOmitsFragment(t *testing.T) { - query := ` - query Q { - a - ...Frag - } - fragment Frag on TestType @skip(if: true) { - b - } - ` - expected := &graphql.Result{ - Data: map[string]interface{}{ - "a": "a", - }, - } - result := executeDirectivesTestQuery(t, query) - if len(result.Errors) != 0 { - t.Fatalf("wrong result, unexpected errors: %v", result.Errors) - } - if !reflect.DeepEqual(expected, result) { - t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) - } -} - func TestDirectivesWorksWithSkipAndIncludeDirectives_IncludeAndNoSkip(t *testing.T) { query := `{ a, b @include(if: true) @skip(if: false) }` expected := &graphql.Result{ @@ -548,6 +450,7 @@ func TestDirectivesWorksWithSkipAndIncludeDirectives_IncludeAndSkip(t *testing.T t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } } + func TestDirectivesWorksWithSkipAndIncludeDirectives_NoIncludeAndSkip(t *testing.T) { query := `{ a, b @include(if: false) @skip(if: true) }` expected := &graphql.Result{ @@ -563,6 +466,7 @@ func TestDirectivesWorksWithSkipAndIncludeDirectives_NoIncludeAndSkip(t *testing t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } } + func TestDirectivesWorksWithSkipAndIncludeDirectives_NoIncludeOrSkip(t *testing.T) { query := `{ a, b @include(if: false) @skip(if: false) }` expected := &graphql.Result{ diff --git a/executor.go b/executor.go index edfcc169..63ce75a9 100644 --- a/executor.go +++ b/executor.go @@ -327,8 +327,7 @@ func collectFields(p CollectFieldsParams) map[string][]*ast.Field { } if fragment, ok := fragment.(*ast.FragmentDefinition); ok { - if !shouldIncludeNode(p.ExeContext, fragment.Directives) || - !doesFragmentConditionMatch(p.ExeContext, fragment, p.RuntimeType) { + if !doesFragmentConditionMatch(p.ExeContext, fragment, p.RuntimeType) { continue } innerParams := CollectFieldsParams{ @@ -372,7 +371,7 @@ func shouldIncludeNode(eCtx *ExecutionContext, directives []*ast.Directive) bool return defaultReturnValue } if skipIf, ok := argValues["if"].(bool); ok { - if skipIf { + if skipIf == true { return false } } @@ -395,12 +394,11 @@ func shouldIncludeNode(eCtx *ExecutionContext, directives []*ast.Directive) bool if err != nil { return defaultReturnValue } - if includeIf, ok := argValues["if"]; ok { - if boolIncludeIf, ok := includeIf.(bool); ok { - return boolIncludeIf + if includeIf, ok := argValues["if"].(bool); ok { + if includeIf == false { + return false } } - return defaultReturnValue } return defaultReturnValue } From f5eed8169124bb204cebaf0f278d832244391cbf Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 23:22:20 +0800 Subject: [PATCH 29/32] Add tests for type comparators Commit: 3201ebb825f7bab4ca4856238ade4603e15a5abc [3201ebb] Parents: c7e6a75277 Author: Lee Byron Date: 6 April 2016 at 11:08:06 AM SGT Labels: HEAD --- type_comparators_test.go | 149 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 type_comparators_test.go diff --git a/type_comparators_test.go b/type_comparators_test.go new file mode 100644 index 00000000..995166ef --- /dev/null +++ b/type_comparators_test.go @@ -0,0 +1,149 @@ +package graphql + +import ( + "testing" +) + +func TestIsEqualType_SameReferenceAreEqual(t *testing.T) { + if !isEqualType(String, String) { + t.Fatalf("Expected same reference to be equal") + } +} + +func TestIsEqualType_IntAndFloatAreNotEqual(t *testing.T) { + if isEqualType(Int, Float) { + t.Fatalf("Expected GraphQLInt and GraphQLFloat to not equal") + } +} + +func TestIsEqualType_ListsOfSameTypeAreEqual(t *testing.T) { + if !isEqualType(NewList(Int), NewList(Int)) { + t.Fatalf("Expected lists of same type are equal") + } +} + +func TestIsEqualType_ListsAreNotEqualToItem(t *testing.T) { + if isEqualType(NewList(Int), Int) { + t.Fatalf("Expected lists are not equal to item") + } +} + +func TestIsEqualType_NonNullOfSameTypeAreEqual(t *testing.T) { + if !isEqualType(NewNonNull(Int), NewNonNull(Int)) { + t.Fatalf("Expected non-null of same type are equal") + } +} +func TestIsEqualType_NonNullIsNotEqualToNullable(t *testing.T) { + if isEqualType(NewNonNull(Int), Int) { + t.Fatalf("Expected non-null is not equal to nullable") + } +} + +func testSchemaForIsTypeSubTypeOfTest(t *testing.T, fields Fields) *Schema { + schema, err := NewSchema(SchemaConfig{ + Query: NewObject(ObjectConfig{ + Name: "Query", + Fields: fields, + }), + }) + if err != nil { + t.Fatalf("Invalid schema: %v", err) + } + return &schema +} + +func TestIsTypeSubTypeOf_SameReferenceIsSubtype(t *testing.T) { + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: String}, + }) + if !isTypeSubTypeOf(schema, String, String) { + t.Fatalf("Expected same reference is subtype") + } +} +func TestIsTypeSubTypeOf_IntIsNotSubtypeOfFloat(t *testing.T) { + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: String}, + }) + if isTypeSubTypeOf(schema, Int, Float) { + t.Fatalf("Expected int is not subtype of float") + } +} +func TestIsTypeSubTypeOf_NonNullIsSubtypeOfNullable(t *testing.T) { + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: String}, + }) + if !isTypeSubTypeOf(schema, NewNonNull(Int), Int) { + t.Fatalf("Expected non-null is subtype of nullable") + } +} +func TestIsTypeSubTypeOf_NullableIsNotSubtypeOfNonNull(t *testing.T) { + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: String}, + }) + if isTypeSubTypeOf(schema, Int, NewNonNull(Int)) { + t.Fatalf("Expected nullable is not subtype of non-null") + } +} +func TestIsTypeSubTypeOf_ItemIsNotSubTypeOfList(t *testing.T) { + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: String}, + }) + if isTypeSubTypeOf(schema, Int, NewList(Int)) { + t.Fatalf("Expected item is not subtype of list") + } +} +func TestIsTypeSubTypeOf_ListIsNotSubtypeOfItem(t *testing.T) { + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: String}, + }) + if isTypeSubTypeOf(schema, NewList(Int), Int) { + t.Fatalf("Expected list is not subtype of item") + } +} + +func TestIsTypeSubTypeOf_MemberIsSubtypeOfUnion(t *testing.T) { + memberType := NewObject(ObjectConfig{ + Name: "Object", + IsTypeOf: func(p IsTypeOfParams) bool { + return true + }, + Fields: Fields{ + "field": &Field{Type: String}, + }, + }) + unionType := NewUnion(UnionConfig{ + Name: "Union", + Types: []*Object{memberType}, + }) + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: unionType}, + }) + if !isTypeSubTypeOf(schema, memberType, unionType) { + t.Fatalf("Expected member is subtype of union") + } +} + +func TestIsTypeSubTypeOf_ImplementationIsSubtypeOfInterface(t *testing.T) { + ifaceType := NewInterface(InterfaceConfig{ + Name: "Interface", + Fields: Fields{ + "field": &Field{Type: String}, + }, + }) + implType := NewObject(ObjectConfig{ + Name: "Object", + IsTypeOf: func(p IsTypeOfParams) bool { + return true + }, + Interfaces: []*Interface{ifaceType}, + Fields: Fields{ + "field": &Field{Type: String}, + }, + }) + schema := testSchemaForIsTypeSubTypeOfTest(t, Fields{ + "field": &Field{Type: implType}, + }) + if !isTypeSubTypeOf(schema, implType, ifaceType) { + t.Fatalf("Expected implementation is subtype of interface") + } +} From c1dcff7bba4f47b7952c3f3ec9781500ed954b22 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 23:35:45 +0800 Subject: [PATCH 30/32] Clean up redundant code in `completeValueCatchingError` - `completeValue` would already have resolved the value for `func() interface{}` --- executor.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/executor.go b/executor.go index 63ce75a9..1df24a0c 100644 --- a/executor.go +++ b/executor.go @@ -573,14 +573,6 @@ func completeValueCatchingError(eCtx *ExecutionContext, returnType Type, fieldAS return completed } completed = completeValue(eCtx, returnType, fieldASTs, info, result) - resultVal := reflect.ValueOf(completed) - if resultVal.IsValid() && resultVal.Type().Kind() == reflect.Func { - if propertyFn, ok := completed.(func() interface{}); ok { - return propertyFn() - } - err := gqlerrors.NewFormattedError("Error resolving func. Expected `func() interface{}` signature") - panic(gqlerrors.FormatError(err)) - } return completed } From cec8092f786493f3794308c6ac97954745a730ee Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Tue, 31 May 2016 23:47:22 +0800 Subject: [PATCH 31/32] Tests for `NewDirectives()` for better coverage --- directives_test.go | 116 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/directives_test.go b/directives_test.go index 41d751d3..164ecd7b 100644 --- a/directives_test.go +++ b/directives_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/graphql-go/graphql" + "github.com/graphql-go/graphql/gqlerrors" + "github.com/graphql-go/graphql/language/location" "github.com/graphql-go/graphql/testutil" ) @@ -37,6 +39,120 @@ func executeDirectivesTestQuery(t *testing.T, doc string) *graphql.Result { return testutil.TestExecute(t, ep) } +func TestDirectives_DirectivesMustBeNamed(t *testing.T) { + invalidDirective := graphql.NewDirective(graphql.DirectiveConfig{ + Locations: []string{ + graphql.DirectiveLocationField, + }, + }) + _, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "TestType", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + Directives: []*graphql.Directive{invalidDirective}, + }) + expectedErr := gqlerrors.FormattedError{ + Message: "Directive must be named.", + Locations: []location.SourceLocation{}, + } + if !reflect.DeepEqual(expectedErr, err) { + t.Fatalf("Expected error to be equal, got: %v", testutil.Diff(expectedErr, err)) + } +} + +func TestDirectives_DirectiveNameMustBeValid(t *testing.T) { + invalidDirective := graphql.NewDirective(graphql.DirectiveConfig{ + Name: "123invalid name", + Locations: []string{ + graphql.DirectiveLocationField, + }, + }) + _, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "TestType", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + Directives: []*graphql.Directive{invalidDirective}, + }) + expectedErr := gqlerrors.FormattedError{ + Message: `Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "123invalid name" does not.`, + Locations: []location.SourceLocation{}, + } + if !reflect.DeepEqual(expectedErr, err) { + t.Fatalf("Expected error to be equal, got: %v", testutil.Diff(expectedErr, err)) + } +} + +func TestDirectives_DirectiveNameMustProvideLocations(t *testing.T) { + invalidDirective := graphql.NewDirective(graphql.DirectiveConfig{ + Name: "skip", + }) + _, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "TestType", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + Directives: []*graphql.Directive{invalidDirective}, + }) + expectedErr := gqlerrors.FormattedError{ + Message: `Must provide locations for directive.`, + Locations: []location.SourceLocation{}, + } + if !reflect.DeepEqual(expectedErr, err) { + t.Fatalf("Expected error to be equal, got: %v", testutil.Diff(expectedErr, err)) + } +} + +func TestDirectives_DirectiveArgNamesMustBeValid(t *testing.T) { + invalidDirective := graphql.NewDirective(graphql.DirectiveConfig{ + Name: "skip", + Description: "Directs the executor to skip this field or fragment when the `if` " + + "argument is true.", + Args: graphql.FieldConfigArgument{ + "123if": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.Boolean), + Description: "Skipped when true.", + }, + }, + Locations: []string{ + graphql.DirectiveLocationField, + graphql.DirectiveLocationFragmentSpread, + graphql.DirectiveLocationInlineFragment, + }, + }) + _, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "TestType", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + Directives: []*graphql.Directive{invalidDirective}, + }) + expectedErr := gqlerrors.FormattedError{ + Message: `Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "123if" does not.`, + Locations: []location.SourceLocation{}, + } + if !reflect.DeepEqual(expectedErr, err) { + t.Fatalf("Expected error to be equal, got: %v", testutil.Diff(expectedErr, err)) + } +} + func TestDirectivesWorksWithoutDirectives(t *testing.T) { query := `{ a, b }` expected := &graphql.Result{ From a241e1cac73928a834a7691328c283e413b41661 Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Wed, 1 Jun 2016 01:09:02 +0800 Subject: [PATCH 32/32] RFC: Return type overlap validation Implements https://github.com/facebook/graphql/pull/162 This alters the "Overlapping Fields Can Be Merged" validation rule to better express return type validation issues. The existing rule has presented some false-positives in some schema where interfaces with co-variant types are commonly used. The "same return type" check doesn't remove any ambiguity. Instead what that "same return type" check is attempting to prevent is spreading two fragments (or inline fragments) which have fields with return types where ambiguity would be introduced in the response. In order to curb false-positives, we later changed this rule such that if two fields were known to never apply simultaneously, then we would skip the remainder of the rule. ``` { ... on Person { foo: fullName } ... on Pet { foo: petName } } ``` However this can introduce false-negatives! ``` { ... on Person { foo: birthday { bar: year } } ... on Business { foo: location { bar: street } } } ``` In the above example, `data.foo.bar` could be of type `Int` or type `String`, it's ambiguous! This differing return type breaks some client model implementations (Fragment models) in addition to just being confusing. Commit: c034de91acce10d5c06d03bd332c6ebd45e2213c [c034de9] Parents: ffe76c51c4 Author: Lee Byron Date: 7 April 2016 at 2:00:31 PM SGT Labels: HEAD --- rules.go | 202 +++++++++----- ...s_overlapping_fields_can_be_merged_test.go | 249 ++++++++++++++++-- 2 files changed, 354 insertions(+), 97 deletions(-) diff --git a/rules.go b/rules.go index 5dd26c7a..f0c24fc4 100644 --- a/rules.go +++ b/rules.go @@ -1245,6 +1245,40 @@ func sameType(typeA, typeB Type) bool { return false } +// Two types conflict if both types could not apply to a value simultaneously. +// Composite types are ignored as their individual field types will be compared +// later recursively. However List and Non-Null types must match. +func doTypesConflict(type1 Output, type2 Output) bool { + if type1, ok := type1.(*List); ok { + if type2, ok := type2.(*List); ok { + return doTypesConflict(type1.OfType, type2.OfType) + } + return true + } + if type2, ok := type2.(*List); ok { + if type1, ok := type1.(*List); ok { + return doTypesConflict(type1.OfType, type2.OfType) + } + return true + } + if type1, ok := type1.(*NonNull); ok { + if type2, ok := type2.(*NonNull); ok { + return doTypesConflict(type1.OfType, type2.OfType) + } + return true + } + if type2, ok := type2.(*NonNull); ok { + if type1, ok := type1.(*NonNull); ok { + return doTypesConflict(type1.OfType, type2.OfType) + } + return true + } + if IsLeafType(type1) || IsLeafType(type2) { + return type1 != type2 + } + return false +} + // OverlappingFieldsCanBeMergedRule Overlapping fields can be merged // // A selection set is only valid if all fields (including spreading any @@ -1252,9 +1286,12 @@ func sameType(typeA, typeB Type) bool { // without ambiguity. func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRuleInstance { + var getSubfieldMap func(ast1 *ast.Field, type1 Output, ast2 *ast.Field, type2 Output) map[string][]*fieldDefPair + var subfieldConflicts func(conflicts []*conflict, responseName string, ast1 *ast.Field, ast2 *ast.Field) *conflict + var findConflicts func(parentFieldsAreMutuallyExclusive bool, fieldMap map[string][]*fieldDefPair) (conflicts []*conflict) + comparedSet := newPairSet() - var findConflicts func(fieldMap map[string][]*fieldDefPair) (conflicts []*conflict) - findConflict := func(responseName string, field *fieldDefPair, field2 *fieldDefPair) *conflict { + findConflict := func(parentFieldsAreMutuallyExclusive bool, responseName string, field *fieldDefPair, field2 *fieldDefPair) *conflict { parentType1 := field.ParentType ast1 := field.Field @@ -1269,46 +1306,21 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul return nil } - // If the statically known parent types could not possibly apply at the same - // time, then it is safe to permit them to diverge as they will not present - // any ambiguity by differing. - // It is known that two parent types could never overlap if they are - // different Object types. Interface or Union types might overlap - if not - // in the current state of the schema, then perhaps in some future version, - // thus may not safely diverge. - if parentType1 != parentType2 { - _, ok1 := parentType1.(*Object) - _, ok2 := parentType2.(*Object) - if ok1 && ok2 { - return nil - } - } - // Memoize, do not report the same issue twice. + // Note: Two overlapping ASTs could be encountered both when + // `parentFieldsAreMutuallyExclusive` is true and is false, which could + // produce different results (when `true` being a subset of `false`). + // However we do not need to include this piece of information when + // memoizing since this rule visits leaf fields before their parent fields, + // ensuring that `parentFieldsAreMutuallyExclusive` is `false` the first + // time two overlapping fields are encountered, ensuring that the full + // set of validation rules are always checked when necessary. if comparedSet.Has(ast1, ast2) { return nil } comparedSet.Add(ast1, ast2) - name1 := "" - if ast1.Name != nil { - name1 = ast1.Name.Value - } - name2 := "" - if ast2.Name != nil { - name2 = ast2.Name.Value - } - if name1 != name2 { - return &conflict{ - Reason: conflictReason{ - Name: responseName, - Message: fmt.Sprintf(`%v and %v are different fields`, name1, name2), - }, - FieldsLeft: []ast.Node{ast1}, - FieldsRight: []ast.Node{ast2}, - } - } - + // The return type for each field. var type1 Type var type2 Type if def1 != nil { @@ -1318,27 +1330,74 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul type2 = def2.Type } - if type1 != nil && type2 != nil && !sameType(type1, type2) { - return &conflict{ - Reason: conflictReason{ - Name: responseName, - Message: fmt.Sprintf(`they return differing types %v and %v`, type1, type2), - }, - FieldsLeft: []ast.Node{ast1}, - FieldsRight: []ast.Node{ast2}, + // If it is known that two fields could not possibly apply at the same + // time, due to the parent types, then it is safe to permit them to diverge + // in aliased field or arguments used as they will not present any ambiguity + // by differing. + // It is known that two parent types could never overlap if they are + // different Object types. Interface or Union types might overlap - if not + // in the current state of the schema, then perhaps in some future version, + // thus may not safely diverge. + _, isParentType1Object := parentType1.(*Object) + _, isParentType2Object := parentType2.(*Object) + fieldsAreMutuallyExclusive := parentFieldsAreMutuallyExclusive || parentType1 != parentType2 && isParentType1Object && isParentType2Object + + if !fieldsAreMutuallyExclusive { + // Two aliases must refer to the same field. + name1 := "" + name2 := "" + + if ast1.Name != nil { + name1 = ast1.Name.Value + } + if ast2.Name != nil { + name2 = ast2.Name.Value + } + if name1 != name2 { + return &conflict{ + Reason: conflictReason{ + Name: responseName, + Message: fmt.Sprintf(`%v and %v are different fields`, name1, name2), + }, + FieldsLeft: []ast.Node{ast1}, + FieldsRight: []ast.Node{ast2}, + } + } + + // Two field calls must have the same arguments. + if !sameArguments(ast1.Arguments, ast2.Arguments) { + return &conflict{ + Reason: conflictReason{ + Name: responseName, + Message: `they have differing arguments`, + }, + FieldsLeft: []ast.Node{ast1}, + FieldsRight: []ast.Node{ast2}, + } } } - if !sameArguments(ast1.Arguments, ast2.Arguments) { + + if type1 != nil && type2 != nil && doTypesConflict(type1, type2) { return &conflict{ Reason: conflictReason{ Name: responseName, - Message: `they have differing arguments`, + Message: fmt.Sprintf(`they return conflicting types %v and %v`, type1, type2), }, FieldsLeft: []ast.Node{ast1}, FieldsRight: []ast.Node{ast2}, } } + subFieldMap := getSubfieldMap(ast1, type1, ast2, type2) + if subFieldMap != nil { + conflicts := findConflicts(fieldsAreMutuallyExclusive, subFieldMap) + return subfieldConflicts(conflicts, responseName, ast1, ast2) + } + + return nil + } + + getSubfieldMap = func(ast1 *ast.Field, type1 Output, ast2 *ast.Field, type2 Output) map[string][]*fieldDefPair { selectionSet1 := ast1.SelectionSet selectionSet2 := ast2.SelectionSet if selectionSet1 != nil && selectionSet2 != nil { @@ -1357,32 +1416,34 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul visitedFragmentNames, subfieldMap, ) - conflicts := findConflicts(subfieldMap) - if len(conflicts) > 0 { - - conflictReasons := []conflictReason{} - conflictFieldsLeft := []ast.Node{ast1} - conflictFieldsRight := []ast.Node{ast2} - for _, c := range conflicts { - conflictReasons = append(conflictReasons, c.Reason) - conflictFieldsLeft = append(conflictFieldsLeft, c.FieldsLeft...) - conflictFieldsRight = append(conflictFieldsRight, c.FieldsRight...) - } + return subfieldMap + } + return nil + } - return &conflict{ - Reason: conflictReason{ - Name: responseName, - Message: conflictReasons, - }, - FieldsLeft: conflictFieldsLeft, - FieldsRight: conflictFieldsRight, - } + subfieldConflicts = func(conflicts []*conflict, responseName string, ast1 *ast.Field, ast2 *ast.Field) *conflict { + if len(conflicts) > 0 { + conflictReasons := []conflictReason{} + conflictFieldsLeft := []ast.Node{ast1} + conflictFieldsRight := []ast.Node{ast2} + for _, c := range conflicts { + conflictReasons = append(conflictReasons, c.Reason) + conflictFieldsLeft = append(conflictFieldsLeft, c.FieldsLeft...) + conflictFieldsRight = append(conflictFieldsRight, c.FieldsRight...) + } + + return &conflict{ + Reason: conflictReason{ + Name: responseName, + Message: conflictReasons, + }, + FieldsLeft: conflictFieldsLeft, + FieldsRight: conflictFieldsRight, } } return nil } - - findConflicts = func(fieldMap map[string][]*fieldDefPair) (conflicts []*conflict) { + findConflicts = func(parentFieldsAreMutuallyExclusive bool, fieldMap map[string][]*fieldDefPair) (conflicts []*conflict) { // ensure field traversal orderedName := sort.StringSlice{} @@ -1395,7 +1456,7 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul fields, _ := fieldMap[responseName] for _, fieldA := range fields { for _, fieldB := range fields { - c := findConflict(responseName, fieldA, fieldB) + c := findConflict(parentFieldsAreMutuallyExclusive, responseName, fieldA, fieldB) if c != nil { conflicts = append(conflicts, c) } @@ -1429,6 +1490,9 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ kinds.SelectionSet: visitor.NamedVisitFuncs{ + // Note: we validate on the reverse traversal so deeper conflicts will be + // caught first, for correct calculation of mutual exclusivity and for + // clearer error messages. Leave: func(p visitor.VisitFuncParams) (string, interface{}) { if selectionSet, ok := p.Node.(*ast.SelectionSet); ok && selectionSet != nil { parentType, _ := context.ParentType().(Named) @@ -1439,7 +1503,7 @@ func OverlappingFieldsCanBeMergedRule(context *ValidationContext) *ValidationRul nil, nil, ) - conflicts := findConflicts(fieldMap) + conflicts := findConflicts(false, fieldMap) if len(conflicts) > 0 { for _, c := range conflicts { responseName := c.Reason.Name diff --git a/rules_overlapping_fields_can_be_merged_test.go b/rules_overlapping_fields_can_be_merged_test.go index a7827f30..b38b13a1 100644 --- a/rules_overlapping_fields_can_be_merged_test.go +++ b/rules_overlapping_fields_can_be_merged_test.go @@ -286,6 +286,7 @@ func TestValidate_OverlappingFieldsCanBeMerged_ReportsDeepConflictToNearestCommo var someBoxInterface *graphql.Interface var stringBoxObject *graphql.Object +var intBoxObject *graphql.Object var schema graphql.Schema func init() { @@ -294,39 +295,72 @@ func init() { ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { return stringBoxObject }, - Fields: graphql.Fields{ - "unrelatedField": &graphql.Field{ - Type: graphql.String, - }, - }, + Fields: graphql.FieldsThunk(func() graphql.Fields { + return graphql.Fields{ + "deepBox": &graphql.Field{ + Type: someBoxInterface, + }, + "unrelatedField": &graphql.Field{ + Type: graphql.String, + }, + } + }), }) stringBoxObject = graphql.NewObject(graphql.ObjectConfig{ Name: "StringBox", Interfaces: (graphql.InterfacesThunk)(func() []*graphql.Interface { return []*graphql.Interface{someBoxInterface} }), - Fields: graphql.Fields{ - "scalar": &graphql.Field{ - Type: graphql.String, - }, - "unrelatedField": &graphql.Field{ - Type: graphql.String, - }, - }, + Fields: graphql.FieldsThunk(func() graphql.Fields { + return graphql.Fields{ + "scalar": &graphql.Field{ + Type: graphql.String, + }, + "deepBox": &graphql.Field{ + Type: stringBoxObject, + }, + "unrelatedField": &graphql.Field{ + Type: graphql.String, + }, + "listStringBox": &graphql.Field{ + Type: graphql.NewList(stringBoxObject), + }, + "stringBox": &graphql.Field{ + Type: stringBoxObject, + }, + "intBox": &graphql.Field{ + Type: intBoxObject, + }, + } + }), }) - IntBox := graphql.NewObject(graphql.ObjectConfig{ + intBoxObject = graphql.NewObject(graphql.ObjectConfig{ Name: "IntBox", Interfaces: (graphql.InterfacesThunk)(func() []*graphql.Interface { return []*graphql.Interface{someBoxInterface} }), - Fields: graphql.Fields{ - "scalar": &graphql.Field{ - Type: graphql.Int, - }, - "unrelatedField": &graphql.Field{ - Type: graphql.String, - }, - }, + Fields: graphql.FieldsThunk(func() graphql.Fields { + return graphql.Fields{ + "scalar": &graphql.Field{ + Type: graphql.Int, + }, + "deepBox": &graphql.Field{ + Type: someBoxInterface, + }, + "unrelatedField": &graphql.Field{ + Type: graphql.String, + }, + "listStringBox": &graphql.Field{ + Type: graphql.NewList(stringBoxObject), + }, + "stringBox": &graphql.Field{ + Type: stringBoxObject, + }, + "intBox": &graphql.Field{ + Type: intBoxObject, + }, + } + }), }) var nonNullStringBox1Interface = graphql.NewInterface(graphql.InterfaceConfig{ Name: "NonNullStringBox1", @@ -351,6 +385,9 @@ func init() { "unrelatedField": &graphql.Field{ Type: graphql.String, }, + "deepBox": &graphql.Field{ + Type: someBoxInterface, + }, }, }) var nonNullStringBox2Interface = graphql.NewInterface(graphql.InterfaceConfig{ @@ -376,6 +413,9 @@ func init() { "unrelatedField": &graphql.Field{ Type: graphql.String, }, + "deepBox": &graphql.Field{ + Type: someBoxInterface, + }, }, }) @@ -418,7 +458,8 @@ func init() { }, }), Types: []graphql.Type{ - IntBox, + intBoxObject, + stringBoxObject, NonNullStringBox1Impl, NonNullStringBox2Impl, }, @@ -446,20 +487,172 @@ func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_Conf } `, []gqlerrors.FormattedError{ testutil.RuleError( - `Fields "scalar" conflict because they return differing types Int and String!.`, + `Fields "scalar" conflict because they return conflicting types Int and String!.`, 5, 15, 8, 15), }) } -func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_AllowsDiffereingReturnTypesWhichCannotOverlap(t *testing.T) { - // This is valid since an object cannot be both an IntBox and a StringBox. +func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_CompatibleReturnShapesOnDifferentReturnTypes(t *testing.T) { + // In this case `deepBox` returns `SomeBox` in the first usage, and + // `StringBox` in the second usage. These return types are not the same! + // however this is valid because the return *shapes* are compatible. testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` + { + someBox { + ... on SomeBox { + deepBox { + unrelatedField + } + } + ... on StringBox { + deepBox { + unrelatedField + } + } + } + } + `) +} +func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_DisallowsDifferingReturnTypesDespiteNoOverlap(t *testing.T) { + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` { someBox { - ...on IntBox { + ... on IntBox { + scalar + } + ... on StringBox { + scalar + } + } + } + `, []gqlerrors.FormattedError{ + testutil.RuleError( + `Fields "scalar" conflict because they return conflicting types Int and String.`, + 5, 15, + 8, 15), + }) +} +func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_DisallowsDifferingReturnTypeNullabilityDespiteNoOverlap(t *testing.T) { + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` + { + someBox { + ... on NonNullStringBox1 { + scalar + } + ... on StringBox { scalar } - ...on StringBox { + } + } + `, []gqlerrors.FormattedError{ + testutil.RuleError( + `Fields "scalar" conflict because they return conflicting types String! and String.`, + 5, 15, + 8, 15), + }) +} +func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_DisallowsDifferingReturnTypeListDespiteNoOverlap(t *testing.T) { + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` + { + someBox { + ... on IntBox { + box: listStringBox { + scalar + } + } + ... on StringBox { + box: stringBox { + scalar + } + } + } + } + `, []gqlerrors.FormattedError{ + testutil.RuleError( + `Fields "box" conflict because they return conflicting types [StringBox] and StringBox.`, + 5, 15, + 10, 15), + }) + + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` + { + someBox { + ... on IntBox { + box: stringBox { + scalar + } + } + ... on StringBox { + box: listStringBox { + scalar + } + } + } + } + `, []gqlerrors.FormattedError{ + testutil.RuleError( + `Fields "box" conflict because they return conflicting types StringBox and [StringBox].`, + 5, 15, + 10, 15), + }) +} +func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_DisallowsDifferingSubfields(t *testing.T) { + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` + { + someBox { + ... on IntBox { + box: stringBox { + val: scalar + val: unrelatedField + } + } + ... on StringBox { + box: stringBox { + val: scalar + } + } + } + } + `, []gqlerrors.FormattedError{ + testutil.RuleError( + `Fields "val" conflict because scalar and unrelatedField are different fields.`, + 6, 17, + 7, 17), + }) +} +func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_DisallowsDifferingDeepReturnTypesDespiteNoOverlap(t *testing.T) { + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` + { + someBox { + ... on IntBox { + box: stringBox { + scalar + } + } + ... on StringBox { + box: intBox { + scalar + } + } + } + } + `, []gqlerrors.FormattedError{ + testutil.RuleError( + `Fields "box" conflict because subfields "scalar" conflict because they return conflicting types String and Int.`, + 5, 15, + 6, 17, + 10, 15, + 11, 17), + }) +} +func TestValidate_OverlappingFieldsCanBeMerged_ReturnTypesMustBeUnambiguous_AllowsNonConflictingOverlappingTypes(t *testing.T) { + testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.OverlappingFieldsCanBeMergedRule, ` + { + someBox { + ... on IntBox { + scalar: unrelatedField + } + ... on StringBox { scalar } }