From 72c9fe10a403f2a0f13c7ab38725efa55c2d27ab Mon Sep 17 00:00:00 2001 From: Greg MacWilliam Date: Sun, 7 Apr 2024 00:40:18 -0400 Subject: [PATCH] Smarter boundary argument, take 2 (#131) --- README.md | 47 ++++++++++---- 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/graphql/stitching/supergraph_test.rb | 15 ++--- test/test_helper.rb | 2 +- 7 files changed, 160 insertions(+), 61 deletions(-) rename lib/graphql/stitching/composer/{static_config.rb => boundary_config.rb} (55%) diff --git a/README.md b/README.md index dcd29d60..3a3c1374 100644 --- a/README.md +++ b/README.md @@ -10,12 +10,13 @@ GraphQL stitching composes a single schema from multiple underlying GraphQL reso - Shared objects, fields, enums, and inputs across locations. - Combining local and remote schemas. - File uploads via [multipart form spec](https://github.com/jaydenseric/graphql-multipart-request-spec). +- Tested with all minor versions of `graphql-ruby`. **NOT Supported:** - Computed fields (ie: federation-style `@requires`). - Subscriptions, defer/stream. -This Ruby implementation is a sibling to [GraphQL Tools](https://the-guild.dev/graphql/stitching) (JS) and [Bramble](https://movio.github.io/bramble/) (Go), and its capabilities fall somewhere in between them. GraphQL stitching is similar in concept to [Apollo Federation](https://www.apollographql.com/docs/federation/), though more generic. The opportunity here is for a Ruby application to stitch its local schemas together or onto remote sources without requiring an additional proxy service running in another language. If your goal is to build a purely high-throughput API gateway, consider not using Ruby. +This Ruby implementation is a sibling to [GraphQL Tools](https://the-guild.dev/graphql/stitching) (JS) and [Bramble](https://movio.github.io/bramble/) (Go), and its capabilities fall somewhere in between them. GraphQL stitching is similar in concept to [Apollo Federation](https://www.apollographql.com/docs/federation/), though more generic. The opportunity here is for a Ruby application to stitch its local schemas together or onto remote sources without requiring an additional proxy service running in another language. If your goal is to build a purely high-performance API gateway, consider not using Ruby. ## Getting started @@ -179,7 +180,7 @@ See [error handling](./docs/mechanics.md#stitched-errors) tips for list queries. #### Abstract queries -It's okay for stitching queries to be implemented through abstract types. An abstract query will provide access to all of its possible types. For interfaces, the key selection should match a field within the interface. For unions, all possible types must implement the key selection individually. +It's okay for stitching queries to be implemented through abstract types. An abstract query will provide access to all of its possible types by default, each of which must implement the key. ```graphql interface Node { @@ -194,13 +195,33 @@ type Query { } ``` +To customize which types an abstract query provides and their respective keys, you may extend the `@stitch` directive with a `typeName` constraint. This can be repeated to select multiple types. + +```graphql +directive @stitch(key: String!, typeName: String) repeatable on FIELD_DEFINITION + +type Product { sku: ID! } +type Order { id: ID! } +type Customer { id: ID! } # << not stitched +union Entity = Product | Order | Customer + +type Query { + entity(key: ID!): Entity + @stitch(key: "sku", typeName: "Product") + @stitch(key: "id", typeName: "Order") +} +``` + #### Multiple query arguments -Stitching infers which argument to use for queries with a single argument. For queries that accept multiple arguments, the key must provide an argument mapping specified as `":"`. Note the `"id:id"` key: +Stitching infers which argument to use for queries with a single argument, or when the key name matches its intended argument. For queries that accept multiple arguments with unmatched names, the key may provide an argument mapping specified as `":"`. ```graphql +type Product { + id: ID! +} type Query { - product(id: ID, upc: ID): Product @stitch(key: "id:id") + product(by_id: ID, by_sku: ID): Product @stitch(key: "by_id:id") } ``` @@ -210,8 +231,8 @@ A type may exist in multiple locations across the graph using different keys, fo ```graphql type Product { id:ID! } # storefronts location -type Product { id:ID! upc:ID! } # products location -type Product { upc:ID! } # catelog location +type Product { id:ID! sku:ID! } # products location +type Product { sku:ID! } # catelog location ``` In the above graph, the `storefronts` and `catelog` locations have different keys that join through an intermediary. This pattern is perfectly valid and resolvable as long as the intermediary provides stitching queries for each possible key: @@ -219,11 +240,11 @@ In the above graph, the `storefronts` and `catelog` locations have different key ```graphql type Product { id: ID! - upc: ID! + sku: ID! } type Query { productById(id: ID!): Product @stitch(key: "id") - productByUpc(upc: ID!): Product @stitch(key: "upc") + productByUpc(sku: ID!): Product @stitch(key: "sku") } ``` @@ -232,10 +253,10 @@ The `@stitch` directive is also repeatable, allowing a single query to associate ```graphql type Product { id: ID! - upc: ID! + sku: ID! } type Query { - product(id: ID, upc: ID): Product @stitch(key: "id:id") @stitch(key: "upc:upc") + product(id: ID, sku: ID): Product @stitch(key: "id") @stitch(key: "sku") } ``` @@ -269,11 +290,11 @@ A clean SDL string may also have stitching directives applied via static configu sdl_string = <<~GRAPHQL type Product { id: ID! - upc: ID! + sku: ID! } type Query { productById(id: ID!): Product - productByUpc(upc: ID!): Product + productByUpc(sku: ID!): Product } GRAPHQL @@ -283,7 +304,7 @@ supergraph = GraphQL::Stitching::Composer.new.perform({ executable: ->() { ... }, stitch: [ { field_name: "productById", key: "id" }, - { field_name: "productByUpc", key: "upc" }, + { field_name: "productByUpc", key: "sku" }, ] }, # ... 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/graphql/stitching/supergraph_test.rb b/test/graphql/stitching/supergraph_test.rb index f0bf0777..72a05ceb 100644 --- a/test/graphql/stitching/supergraph_test.rb +++ b/test/graphql/stitching/supergraph_test.rb @@ -314,9 +314,9 @@ def test_route_type_to_locations_returns_nil_for_unreachable_locations describe "#to_definition / #from_definition" do def setup alpha = %| - type T implements A { id:ID! a:String } - interface A { id:ID! } - type Query { a(id:ID!):A @stitch(key: "id") } + interface I { id:ID! } + type T implements I { id:ID! a:String } + type Query { a(id:ID!):I @stitch(key: "id") } | bravo = %| type T { id:ID! b:String } @@ -332,16 +332,16 @@ def test_to_definition_annotates_schema assert @schema_sdl.include?("directive @resolver") assert @schema_sdl.include?("directive @source") assert @schema_sdl.include?(squish_string(%| - interface A @resolver(location: "alpha", key: "id", field: "a", arg: "id") { + interface I @resolver(location: "alpha", key: "id", field: "a", arg: "id") { |)) assert @schema_sdl.include?(squish_string(%| - type T implements A @resolver(location: "bravo", key: "id", field: "b", arg: "id") - @resolver(typeName: "A", location: "alpha", key: "id", field: "a", arg: "id") { + type T implements I @resolver(location: "bravo", key: "id", field: "b", arg: "id") + @resolver(typeName: "I", location: "alpha", key: "id", field: "a", arg: "id") { |)) assert @schema_sdl.include?(%|id: ID! @source(location: "alpha") @source(location: "bravo")|) assert @schema_sdl.include?(%|a: String @source(location: "alpha")|) assert @schema_sdl.include?(%|b: String @source(location: "bravo")|) - assert @schema_sdl.include?(%|a(id: ID!): A @source(location: "alpha")|) + assert @schema_sdl.include?(%|a(id: ID!): I @source(location: "alpha")|) assert @schema_sdl.include?(%|b(id: ID!): T @source(location: "bravo")|) end @@ -365,6 +365,7 @@ def test_from_definition_restores_supergraph assert_equal @supergraph.boundaries, supergraph_import.boundaries assert_equal ["alpha", "bravo"], supergraph_import.locations.sort assert_equal @supergraph.schema.types.keys.sort, supergraph_import.schema.types.keys.sort + assert_equal @supergraph.boundaries, supergraph_import.boundaries end def test_normalizes_executable_location_names 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