From b30479d6d5362b761d0981191035607586795e9c Mon Sep 17 00:00:00 2001 From: Greg MacWilliam Date: Sat, 6 Apr 2024 15:21:25 -0400 Subject: [PATCH] smarter boundary config. --- lib/graphql/stitching/composer.rb | 46 ++++++++------ .../{static_config.rb => boundary_config.rb} | 46 +++++++++----- lib/graphql/stitching/version.rb | 2 +- .../composer/merge_boundaries_test.rb | 63 ++++++++++++++++++- test/test_helper.rb | 2 +- 5 files changed, 118 insertions(+), 41 deletions(-) rename lib/graphql/stitching/composer/{static_config.rb => boundary_config.rb} (55%) diff --git a/lib/graphql/stitching/composer.rb b/lib/graphql/stitching/composer.rb index 890e3f0c..ae2db7ae 100644 --- a/lib/graphql/stitching/composer.rb +++ b/lib/graphql/stitching/composer.rb @@ -3,7 +3,7 @@ require_relative "./composer/base_validator" require_relative "./composer/validate_interfaces" require_relative "./composer/validate_boundaries" -require_relative "./composer/static_config" +require_relative "./composer/boundary_config" module GraphQL module Stitching @@ -62,7 +62,7 @@ def initialize( @default_value_merger = default_value_merger || BASIC_VALUE_MERGER @directive_kwarg_merger = directive_kwarg_merger || BASIC_VALUE_MERGER @root_field_location_selector = root_field_location_selector || BASIC_ROOT_FIELD_LOCATION_SELECTOR - @stitch_directives = {} + @boundary_configs = {} @field_map = nil @boundary_map = nil @@ -188,13 +188,8 @@ def prepare_locations_input(locations_input) raise ComposerError, "The schema for `#{location}` location must be a GraphQL::Schema class." end - if config = StaticConfig.extract_directive_assignments(schema, location, input[:stitch]) - @stitch_directives.merge!(config) - end - - if config = StaticConfig.extract_federation_entities(schema, location) - @stitch_directives.merge!(config) - end + @boundary_configs.merge!(BoundaryConfig.extract_directive_assignments(schema, location, input[:stitch])) + @boundary_configs.merge!(BoundaryConfig.extract_federation_entities(schema, location)) schemas[location.to_s] = schema executables[location.to_s] = input[:executable] || schema @@ -534,19 +529,17 @@ def merge_deprecations(type_name, members_by_location, field_name: nil, argument def extract_boundaries(type_name, types_by_location) types_by_location.each do |location, type_candidate| type_candidate.fields.each do |field_name, field_candidate| - boundary_type_name = field_candidate.type.unwrap.graphql_name + boundary_type = field_candidate.type.unwrap boundary_structure = Util.flatten_type_structure(field_candidate.type) - boundary_kwargs = @stitch_directives["#{location}.#{field_name}"] || [] + boundary_configs = @boundary_configs.fetch("#{location}.#{field_name}", []) field_candidate.directives.each do |directive| next unless directive.graphql_name == GraphQL::Stitching.stitch_directive - boundary_kwargs << directive.arguments.keyword_arguments + boundary_configs << BoundaryConfig.from_kwargs(directive.arguments.keyword_arguments) end - boundary_kwargs.each do |kwargs| - key = kwargs.fetch(:key) - impl_type_name = kwargs.fetch(:type_name, boundary_type_name) - key_selections = GraphQL.parse("{ #{key} }").definitions[0].selections + boundary_configs.each do |config| + key_selections = GraphQL.parse("{ #{config.key} }").definitions[0].selections if key_selections.length != 1 raise ComposerError, "Boundary key at #{type_name}.#{field_name} must specify exactly one key." @@ -555,6 +548,8 @@ def extract_boundaries(type_name, types_by_location) argument_name = key_selections[0].alias argument_name ||= if field_candidate.arguments.size == 1 field_candidate.arguments.keys.first + elsif field_candidate.arguments[config.key] + config.key end argument = field_candidate.arguments[argument_name] @@ -568,15 +563,26 @@ def extract_boundaries(type_name, types_by_location) raise ComposerError, "Mismatched input/output for #{type_name}.#{field_name}.#{argument_name} boundary. Arguments must map directly to results." end - @boundary_map[impl_type_name] ||= [] - @boundary_map[impl_type_name] << Boundary.new( + boundary_type_name = if config.type_name + if !boundary_type.kind.abstract? + raise ComposerError, "Resolver config may only specify a type name for abstract resolvers." + elsif !boundary_type.possible_types.find { _1.graphql_name == config.type_name } + raise ComposerError, "Type `#{config.type_name}` is not a possible return type for query `#{field_name}`." + end + config.type_name + else + boundary_type.graphql_name + end + + @boundary_map[boundary_type_name] ||= [] + @boundary_map[boundary_type_name] << Boundary.new( location: location, - type_name: impl_type_name, + type_name: boundary_type_name, key: key_selections[0].name, field: field_candidate.name, arg: argument_name, list: boundary_structure.first.list?, - federation: kwargs[:federation] || false, + federation: config.federation, ) end end diff --git a/lib/graphql/stitching/composer/static_config.rb b/lib/graphql/stitching/composer/boundary_config.rb similarity index 55% rename from lib/graphql/stitching/composer/static_config.rb rename to lib/graphql/stitching/composer/boundary_config.rb index d8249603..c024d71b 100644 --- a/lib/graphql/stitching/composer/static_config.rb +++ b/lib/graphql/stitching/composer/boundary_config.rb @@ -2,33 +2,31 @@ module GraphQL::Stitching class Composer - class StaticConfig - + class BoundaryConfig ENTITY_TYPENAME = "_Entity" ENTITIES_QUERY = "_entities" class << self def extract_directive_assignments(schema, location, assignments) - return nil unless assignments && assignments.any? + return EMPTY_OBJECT unless assignments && assignments.any? - assignments.each_with_object({}) do |cfg, memo| - type = cfg[:parent_type_name] ? schema.get_type(cfg[:parent_type_name]) : schema.query - raise ComposerError, "Invalid stitch directive type `#{cfg[:parent_type_name]}`" unless type + assignments.each_with_object({}) do |kwargs, memo| + type = kwargs[:parent_type_name] ? schema.get_type(kwargs[:parent_type_name]) : schema.query + raise ComposerError, "Invalid stitch directive type `#{kwargs[:parent_type_name]}`" unless type - field = type.get_field(cfg[:field_name]) - raise ComposerError, "Invalid stitch directive field `#{cfg[:field_name]}`" unless field + field = type.get_field(kwargs[:field_name]) + raise ComposerError, "Invalid stitch directive field `#{kwargs[:field_name]}`" unless field field_path = "#{location}.#{field.name}" memo[field_path] ||= [] - memo[field_path] << cfg.slice(:key, :type_name) + memo[field_path] << from_kwargs(kwargs) end end def extract_federation_entities(schema, location) - return nil unless has_federation_entities?(schema) + return EMPTY_OBJECT unless federation_entities_schema?(schema) - result = {} - schema.possible_types(schema.get_type(ENTITY_TYPENAME)).each do |entity_type| + schema.possible_types(schema.get_type(ENTITY_TYPENAME)).each_with_object({}) do |entity_type, memo| entity_type.directives.each do |directive| next unless directive.graphql_name == "key" @@ -36,26 +34,40 @@ def extract_federation_entities(schema, location) raise ComposerError, "Composite federation keys are not supported." unless /^\w+$/.match?(key) field_path = "#{location}._entities" - result[field_path] ||= [] - result[field_path] << { + memo[field_path] ||= [] + memo[field_path] << new( key: key, type_name: entity_type.graphql_name, federation: true, - } + ) end end + end - result + def from_kwargs(kwargs) + new( + key: kwargs[:key], + type_name: kwargs[:type_name] || kwargs[:typeName], + federation: kwargs[:federation] || false, + ) end private - def has_federation_entities?(schema) + def federation_entities_schema?(schema) entity_type = schema.get_type(ENTITY_TYPENAME) entities_query = schema.query.get_field(ENTITIES_QUERY) entity_type && entity_type.kind.union? && entities_query && entities_query.type.unwrap == entity_type end end + + attr_reader :key, :type_name, :federation + + def initialize(key:, type_name:, federation: false) + @key = key + @type_name = type_name + @federation = federation + end end end end diff --git a/lib/graphql/stitching/version.rb b/lib/graphql/stitching/version.rb index e411c199..a2a3c95f 100644 --- a/lib/graphql/stitching/version.rb +++ b/lib/graphql/stitching/version.rb @@ -2,6 +2,6 @@ module GraphQL module Stitching - VERSION = "1.2.3" + VERSION = "1.2.4" end end diff --git a/test/graphql/stitching/composer/merge_boundaries_test.rb b/test/graphql/stitching/composer/merge_boundaries_test.rb index 68a14ab9..8e092065 100644 --- a/test/graphql/stitching/composer/merge_boundaries_test.rb +++ b/test/graphql/stitching/composer/merge_boundaries_test.rb @@ -44,7 +44,7 @@ def test_merges_boundaries_with_multiple_keys | b = %| type T { id:ID! upc:ID! } - type Query { b(id: ID, upc:ID):T @stitch(key: "id:id") @stitch(key: "upc:upc") } + type Query { b(id: ID, code: ID):T @stitch(key: "id") @stitch(key: "code:upc") } | c = %| type T { id:ID! } @@ -54,7 +54,7 @@ def test_merges_boundaries_with_multiple_keys supergraph = compose_definitions({ "a" => a, "b" => b, "c" => c }) assert_boundary(supergraph, "T", location: "a", key: "upc", field: "a", arg: "upc") - assert_boundary(supergraph, "T", location: "b", key: "upc", field: "b", arg: "upc") + assert_boundary(supergraph, "T", location: "b", key: "upc", field: "b", arg: "code") assert_boundary(supergraph, "T", location: "b", key: "id", field: "b", arg: "id") assert_boundary(supergraph, "T", location: "c", key: "id", field: "c", arg: "id") end @@ -120,6 +120,65 @@ def test_expands_union_boundary_accessors_to_relevant_types assert_boundary(supergraph, "Apple", location: "b", key: "id", field: "a", arg: "id") end + + def test_builds_union_boundaries_for_select_typenames + a = %| + type Apple { id:ID! name:String } + type Banana { id:ID! name:String } + type Coconut { id:ID! name:String } + union Fruit = Apple \| Banana \| Coconut + type Query { + fruitA(id:ID!):Fruit + @stitch(key: "id", typeName: "Apple") + @stitch(key: "id", typeName: "Banana", federation: true) + coconut(id: ID!): Coconut + @stitch(key: "id") + } + | + b = %| + type Apple { id:ID! color:String } + type Banana { id:ID! color:String } + type Coconut { id:ID! color:String } + union Fruit = Apple \| Banana \| Coconut + type Query { + fruitB(id:ID!):Fruit @stitch(key: "id") + } + | + + supergraph = compose_definitions({ "a" => a, "b" => b }) + assert_equal ["fruitA", "fruitB"], supergraph.boundaries["Apple"].map(&:field).sort + assert_equal ["fruitA", "fruitB"], supergraph.boundaries["Banana"].map(&:field).sort + assert_equal ["coconut", "fruitB"], supergraph.boundaries["Coconut"].map(&:field).sort + assert_equal ["fruitB"], supergraph.boundaries["Fruit"].map(&:field).sort + + assert_equal false, supergraph.boundaries["Apple"].find { _1.location == "a" }.federation + assert_equal true, supergraph.boundaries["Banana"].find { _1.location == "a" }.federation + end + + def test_raises_when_given_typename_is_not_a_possible_type + a = %| + type Apple { id:ID! name:String } + type Banana { id:ID! name:String } + union Fruit = Apple + type Query { + apple(id: ID!): Apple @stitch(key: "id") + fruitA(id:ID!):Fruit @stitch(key: "id", typeName: "Banana") + } + | + b = %| + type Apple { id:ID! color:String } + type Banana { id:ID! color:String } + union Fruit = Apple \| Banana + type Query { + fruitB(id:ID!):Fruit @stitch(key: "id") + } + | + + assert_error "`Banana` is not a possible return type" do + compose_definitions({ "a" => a, "b" => b }) + end + end + private def assert_boundary(supergraph, type_name, location:, key: nil, field: nil, arg: nil) diff --git a/test/test_helper.rb b/test/test_helper.rb index e6173444..5aa9df67 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -18,7 +18,7 @@ ComposerError = GraphQL::Stitching::Composer::ComposerError ValidationError = GraphQL::Stitching::Composer::ValidationError -STITCH_DEFINITION = "directive @stitch(key: String!) repeatable on FIELD_DEFINITION\n" +STITCH_DEFINITION = "directive @stitch(key: String!, typeName: String, federation: Boolean=false) repeatable on FIELD_DEFINITION\n" def squish_string(str) str.gsub(/\s+/, " ").strip