From 5f0c274091d3cda38fdfd5c8d4af87f97ad5d9e9 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Mon, 28 Mar 2016 21:19:34 -0400 Subject: [PATCH] feature: Support inline fragments without a type condition --- lib/graphql/language/parser.rb | 3 ++- lib/graphql/language/transform.rb | 2 +- lib/graphql/query/serial_execution/selection_resolution.rb | 1 + .../rules/fragment_spreads_are_possible.rb | 5 +++-- lib/graphql/static_validation/rules/fragment_types_exist.rb | 1 + .../rules/fragments_are_on_composite_types.rb | 1 + lib/graphql/static_validation/type_stack.rb | 6 +++++- spec/graphql/directive_spec.rb | 3 +++ spec/graphql/language/parser_spec.rb | 1 + spec/graphql/language/transform_spec.rb | 5 +++++ 10 files changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/graphql/language/parser.rb b/lib/graphql/language/parser.rb index 0bac5bf6fa..1862791120 100644 --- a/lib/graphql/language/parser.rb +++ b/lib/graphql/language/parser.rb @@ -26,10 +26,11 @@ class Parser < Parslet::Parser directives.maybe.as(:optional_directives).as(:directives) } rule(:spread) { str("...") } + rule(:type_condition) { str("on ") >> name.as(:optional_string_content) } # TODO: `on` bug, see spec rule(:inline_fragment) { spread.as(:fragment_spread_keyword) >> space? >> - str("on ") >> name.as(:inline_fragment_type) >> space? >> + type_condition.maybe.as(:inline_fragment_type) >> space? >> directives.maybe.as(:optional_directives).as(:directives) >> space? >> selections.as(:selections) } diff --git a/lib/graphql/language/transform.rb b/lib/graphql/language/transform.rb index 8c6c239604..58077c1e91 100644 --- a/lib/graphql/language/transform.rb +++ b/lib/graphql/language/transform.rb @@ -40,7 +40,7 @@ def self.optional_sequence(name) inline_fragment_type: simple(:n), directives: sequence(:d), selections: sequence(:s), - ) { CREATE_NODE[:InlineFragment, type: n.to_s, directives: d, selections: s, position_source: kw]} + ) { CREATE_NODE[:InlineFragment, type: n, directives: d, selections: s, position_source: kw]} # Operation Definition rule( diff --git a/lib/graphql/query/serial_execution/selection_resolution.rb b/lib/graphql/query/serial_execution/selection_resolution.rb index f5da7e88bb..5aec7c9cb5 100644 --- a/lib/graphql/query/serial_execution/selection_resolution.rb +++ b/lib/graphql/query/serial_execution/selection_resolution.rb @@ -64,6 +64,7 @@ def flatten_fragment(ast_fragment) end def fragment_type_can_apply?(ast_fragment) + return true unless ast_fragment.type child_type = execution_context.get_type(ast_fragment.type) resolved_type = GraphQL::Query::TypeResolver.new(target, child_type, type).type !resolved_type.nil? diff --git a/lib/graphql/static_validation/rules/fragment_spreads_are_possible.rb b/lib/graphql/static_validation/rules/fragment_spreads_are_possible.rb index e19891c766..2b0145e163 100644 --- a/lib/graphql/static_validation/rules/fragment_spreads_are_possible.rb +++ b/lib/graphql/static_validation/rules/fragment_spreads_are_possible.rb @@ -5,8 +5,9 @@ def validate(context) context.visitor[GraphQL::Language::Nodes::InlineFragment] << -> (node, parent) { fragment_parent = context.object_types[-2] - fragment_child = context.object_types.last - validate_fragment_in_scope(fragment_parent, fragment_child, node, context) + if fragment_child = context.object_types.last + validate_fragment_in_scope(fragment_parent, fragment_child, node, context) + end } spreads_to_validate = [] diff --git a/lib/graphql/static_validation/rules/fragment_types_exist.rb b/lib/graphql/static_validation/rules/fragment_types_exist.rb index 51378d640c..dc2858be24 100644 --- a/lib/graphql/static_validation/rules/fragment_types_exist.rb +++ b/lib/graphql/static_validation/rules/fragment_types_exist.rb @@ -15,6 +15,7 @@ def validate(context) private def validate_type_exists(node, context) + return unless node.type type = context.schema.types.fetch(node.type, nil) if type.nil? context.errors << message("No such type #{node.type}, so it can't be a fragment condition", node) diff --git a/lib/graphql/static_validation/rules/fragments_are_on_composite_types.rb b/lib/graphql/static_validation/rules/fragments_are_on_composite_types.rb index 798762c614..15fecf801b 100644 --- a/lib/graphql/static_validation/rules/fragments_are_on_composite_types.rb +++ b/lib/graphql/static_validation/rules/fragments_are_on_composite_types.rb @@ -18,6 +18,7 @@ def validate(context) def validate_type_is_composite(node, context) type_name = node.type + return unless type_name type_def = context.schema.types[type_name] if type_def.nil? || !type_def.kind.composite? context.errors << message("Invalid fragment on type #{type_name} (must be Union, Interface or Object)", node) diff --git a/lib/graphql/static_validation/type_stack.rb b/lib/graphql/static_validation/type_stack.rb index 286f37f4f3..9fd75e1504 100644 --- a/lib/graphql/static_validation/type_stack.rb +++ b/lib/graphql/static_validation/type_stack.rb @@ -52,7 +52,11 @@ def self.get_strategy_for_node_class(node_class) class FragmentWithTypeStrategy def push(stack, node) - object_type = stack.schema.types.fetch(node.type, nil) + object_type = if node.type + stack.schema.types.fetch(node.type, nil) + else + stack.object_types.last + end if !object_type.nil? object_type = object_type.unwrap end diff --git a/spec/graphql/directive_spec.rb b/spec/graphql/directive_spec.rb index 7ed44246a2..17ed19486d 100644 --- a/spec/graphql/directive_spec.rb +++ b/spec/graphql/directive_spec.rb @@ -50,6 +50,8 @@ ... on Cheese @skip(if: false) { dontSkipInlineId: id } ... on Cheese @include(if: true) { includeInlineId: id } ... on Cheese @include(if: false) { dontIncludeInlineId: id } + ... @skip(if: true) { skipNoType: id } + ... @skip(if: false) { dontSkipNoType: id } } } fragment includeFlavorField on Cheese { includeFlavor: flavor } @@ -66,6 +68,7 @@ "includeFlavor" => "Brie", "dontSkipInlineId" => 1, "includeInlineId" => 1, + "dontSkipNoType" => 1, }, }} assert_equal(expected, result) diff --git a/spec/graphql/language/parser_spec.rb b/spec/graphql/language/parser_spec.rb index 82abb388ff..72c99e1bff 100644 --- a/spec/graphql/language/parser_spec.rb +++ b/spec/graphql/language/parser_spec.rb @@ -22,6 +22,7 @@ name, ... on Species { color }, ... family # background info, of course + ... @skip(if: true) { inlineFragSkip }, } } subscription watchStuff { diff --git a/spec/graphql/language/transform_spec.rb b/spec/graphql/language/transform_spec.rb index c73801cdc0..8b1ac259b7 100644 --- a/spec/graphql/language/transform_spec.rb +++ b/spec/graphql/language/transform_spec.rb @@ -153,4 +153,9 @@ def get_result(query_string, parse: nil, debug: false) res = get_result("{quoted: \"\\\" \\\\ \\/ \\b \\f \\n \\r \\t\"}", parse: :value_input_object) assert_equal("\" \\ / \b \f \n \r \t", res.arguments[0].value) end + + it 'transforms inline fragment optional type condition' do + res = get_result("{ ... @skip(if: true) { skippedField }, ... on Pet { isHousebroken } }") + assert_equal([nil, "Pet"], res.definitions.first.selections.map(&:type)) + end end