From 3d648888d49b07c1f23e15b28722c14083de37be Mon Sep 17 00:00:00 2001 From: Greg MacWilliam Date: Thu, 4 Jul 2024 22:36:10 -0400 Subject: [PATCH] Fix planning key deduplication (#150) --- lib/graphql/stitching.rb | 4 +-- lib/graphql/stitching/executor.rb | 3 +- .../stitching/{ => executor}/shaper.rb | 4 +-- lib/graphql/stitching/plan.rb | 2 +- lib/graphql/stitching/planner.rb | 12 +++++--- .../{planner_step.rb => planner/step.rb} | 6 ++-- lib/graphql/stitching/resolver.rb | 2 +- lib/graphql/stitching/version.rb | 2 +- .../shaper_grooming_test.rb} | 16 +++++----- .../shaper_null_bubbling_test.rb} | 30 +++++++++---------- .../stitching/planner/plan_abstracts_test.rb | 3 -- 11 files changed, 42 insertions(+), 42 deletions(-) rename lib/graphql/stitching/{ => executor}/shaper.rb (99%) rename lib/graphql/stitching/{planner_step.rb => planner/step.rb} (97%) rename test/graphql/stitching/{shaper/grooming_test.rb => executor/shaper_grooming_test.rb} (84%) rename test/graphql/stitching/{shaper/null_bubbling_test.rb => executor/shaper_null_bubbling_test.rb} (83%) diff --git a/lib/graphql/stitching.rb b/lib/graphql/stitching.rb index 1d4eab30..d6cc2f78 100644 --- a/lib/graphql/stitching.rb +++ b/lib/graphql/stitching.rb @@ -26,16 +26,14 @@ def stitching_directive_names end require_relative "stitching/supergraph" -require_relative "stitching/resolver" require_relative "stitching/client" require_relative "stitching/composer" require_relative "stitching/executor" require_relative "stitching/http_executable" require_relative "stitching/plan" -require_relative "stitching/planner_step" require_relative "stitching/planner" require_relative "stitching/request" -require_relative "stitching/shaper" +require_relative "stitching/resolver" require_relative "stitching/skip_include" require_relative "stitching/util" require_relative "stitching/version" diff --git a/lib/graphql/stitching/executor.rb b/lib/graphql/stitching/executor.rb index 0f05f35b..9fd20303 100644 --- a/lib/graphql/stitching/executor.rb +++ b/lib/graphql/stitching/executor.rb @@ -3,6 +3,7 @@ require "json" require_relative "./executor/resolver_source" require_relative "./executor/root_source" +require_relative "./executor/shaper" module GraphQL module Stitching @@ -33,7 +34,7 @@ def perform(raw: false) result = {} if @data && @data.length > 0 - result["data"] = raw ? @data : GraphQL::Stitching::Shaper.new(@request).perform!(@data) + result["data"] = raw ? @data : Shaper.new(@request).perform!(@data) end if @errors.length > 0 diff --git a/lib/graphql/stitching/shaper.rb b/lib/graphql/stitching/executor/shaper.rb similarity index 99% rename from lib/graphql/stitching/shaper.rb rename to lib/graphql/stitching/executor/shaper.rb index fa4a40d2..1a4767d5 100644 --- a/lib/graphql/stitching/shaper.rb +++ b/lib/graphql/stitching/executor/shaper.rb @@ -1,8 +1,8 @@ # typed: false # frozen_string_literal: true -module GraphQL - module Stitching +module GraphQL::Stitching + class Executor # Shapes the final results payload to the request selection and schema definition. # This eliminates unrequested export selections and applies null bubbling. # @api private diff --git a/lib/graphql/stitching/plan.rb b/lib/graphql/stitching/plan.rb index 18338481..903b0654 100644 --- a/lib/graphql/stitching/plan.rb +++ b/lib/graphql/stitching/plan.rb @@ -2,7 +2,7 @@ module GraphQL module Stitching - # Immutable structures representing a query plan. + # Immutable (in theory) structures representing a query plan. # May serialize to/from JSON. class Plan Op = Struct.new( diff --git a/lib/graphql/stitching/planner.rb b/lib/graphql/stitching/planner.rb index d95f9c74..b48edafa 100644 --- a/lib/graphql/stitching/planner.rb +++ b/lib/graphql/stitching/planner.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "./planner/step" + module GraphQL module Stitching class Planner @@ -85,7 +87,7 @@ def add_step( end if step.nil? - @steps_by_entrypoint[entrypoint] = PlannerStep.new( + @steps_by_entrypoint[entrypoint] = Step.new( index: next_index, after: parent_index, location: location, @@ -261,7 +263,7 @@ def extract_locale_selections( # B.4) Add a `__typename` export to abstracts and types that implement # fragments so that resolved type information is available during execution. - if requires_typename + if requires_typename && !locale_selections.include?(Resolver::TYPENAME_EXPORT_NODE) locale_selections << Resolver::TYPENAME_EXPORT_NODE end @@ -277,6 +279,10 @@ def extract_locale_selections( route.reduce(locale_selections) do |parent_selections, resolver| # E.1) Add the key of each resolver query into the prior location's selection set. parent_selections.push(*resolver.key.export_nodes) if resolver.key + parent_selections.uniq! do |node| + export_node = node.is_a?(GraphQL::Language::Nodes::Field) && Resolver.export_key?(node.alias) + export_node ? node.alias : node + end # E.2) Add a planner step for each new entrypoint location. add_step( @@ -289,8 +295,6 @@ def extract_locale_selections( ).selections end end - - locale_selections.uniq! { _1.alias || _1.name } end locale_selections diff --git a/lib/graphql/stitching/planner_step.rb b/lib/graphql/stitching/planner/step.rb similarity index 97% rename from lib/graphql/stitching/planner_step.rb rename to lib/graphql/stitching/planner/step.rb index 3f097614..1a7818a4 100644 --- a/lib/graphql/stitching/planner_step.rb +++ b/lib/graphql/stitching/planner/step.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -module GraphQL - module Stitching +module GraphQL::Stitching + class Planner # A planned step in the sequence of stitching entrypoints together. # This is a mutable object that may change throughout the planning process. # It ultimately builds an immutable Plan::Op at the end of planning. - class PlannerStep + class Step GRAPHQL_PRINTER = GraphQL::Language::Printer.new attr_reader :index, :location, :parent_type, :operation_type, :path diff --git a/lib/graphql/stitching/resolver.rb b/lib/graphql/stitching/resolver.rb index cfa5fb6b..d754fd06 100644 --- a/lib/graphql/stitching/resolver.rb +++ b/lib/graphql/stitching/resolver.rb @@ -47,7 +47,7 @@ def list? end def version - @version ||= Digest::SHA2.hexdigest(as_json.to_json) + @version ||= Digest::SHA2.hexdigest("#{Stitching::VERSION}/#{as_json.to_json}") end def ==(other) diff --git a/lib/graphql/stitching/version.rb b/lib/graphql/stitching/version.rb index e05c41fb..17306c6d 100644 --- a/lib/graphql/stitching/version.rb +++ b/lib/graphql/stitching/version.rb @@ -2,6 +2,6 @@ module GraphQL module Stitching - VERSION = "1.4.2" + VERSION = "1.4.3" end end diff --git a/test/graphql/stitching/shaper/grooming_test.rb b/test/graphql/stitching/executor/shaper_grooming_test.rb similarity index 84% rename from test/graphql/stitching/shaper/grooming_test.rb rename to test/graphql/stitching/executor/shaper_grooming_test.rb index 25cdf364..531bddf4 100644 --- a/test/graphql/stitching/shaper/grooming_test.rb +++ b/test/graphql/stitching/executor/shaper_grooming_test.rb @@ -3,7 +3,7 @@ require "test_helper" require_relative "../../../schemas/introspection" -describe "GraphQL::Stitching::Shaper, grooming" do +describe "GraphQL::Stitching::Executor::GraphQL::Stitching::Executor::Shaper, grooming" do def test_prunes_stitching_fields schema_sdl = "type Test { req: String! opt: String } type Query { test: Test }" request = GraphQL::Stitching::Request.new( @@ -27,7 +27,7 @@ def test_prunes_stitching_fields } } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_adds_missing_fields @@ -50,7 +50,7 @@ def test_adds_missing_fields } } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_grooms_through_inline_fragments @@ -82,7 +82,7 @@ def test_grooms_through_inline_fragments } } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_grooms_through_fragment_spreads @@ -110,7 +110,7 @@ def test_grooms_through_fragment_spreads } } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_renames_root_query_typenames @@ -130,7 +130,7 @@ def test_renames_root_query_typenames raw = { "__typename" => "QueryRoot", "typename1" => "QueryRoot", "typename2" => "QueryRoot" } expected = { "__typename" => "Query", "typename1" => "Query", "typename2" => "Query" } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_renames_root_mutation_typenames @@ -150,7 +150,7 @@ def test_renames_root_mutation_typenames raw = { "__typename" => "MutationRoot", "typename1" => "MutationRoot", "typename2" => "MutationRoot" } expected = { "__typename" => "Mutation", "typename1" => "Mutation", "typename2" => "Mutation" } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_handles_introspection_types @@ -162,6 +162,6 @@ def test_handles_introspection_types ) raw = schema.execute(query: INTROSPECTION_QUERY).to_h - assert GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end end diff --git a/test/graphql/stitching/shaper/null_bubbling_test.rb b/test/graphql/stitching/executor/shaper_null_bubbling_test.rb similarity index 83% rename from test/graphql/stitching/shaper/null_bubbling_test.rb rename to test/graphql/stitching/executor/shaper_null_bubbling_test.rb index 15c66241..0f9231d2 100644 --- a/test/graphql/stitching/shaper/null_bubbling_test.rb +++ b/test/graphql/stitching/executor/shaper_null_bubbling_test.rb @@ -2,7 +2,7 @@ require "test_helper" -describe "GraphQL::Stitching::Shaper, null bubbling" do +describe "GraphQL::Stitching::Executor::GraphQL::Stitching::Executor::Shaper, null bubbling" do def test_basic_object_structure schema_sdl = "type Test { req: String! opt: String } type Query { test: Test }" request = GraphQL::Stitching::Request.new( @@ -22,7 +22,7 @@ def test_basic_object_structure } } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_single_object_scopes @@ -39,7 +39,7 @@ def test_bubbles_null_for_single_object_scopes } expected = { "test" => nil } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_recursive_object_scopes @@ -55,7 +55,7 @@ def test_bubbles_null_for_recursive_object_scopes } } - assert_nil GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_nil GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_basic_list_structure @@ -77,7 +77,7 @@ def test_basic_list_structure ] } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_list_elements @@ -99,7 +99,7 @@ def test_bubbles_null_for_list_elements ] } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_required_list_elements @@ -118,7 +118,7 @@ def test_bubbles_null_for_required_list_elements "test" => nil, } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_required_lists @@ -134,7 +134,7 @@ def test_bubbles_null_for_required_lists ] } - assert_nil GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_nil GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_basic_nested_list_structure @@ -156,7 +156,7 @@ def test_basic_nested_list_structure ] } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_nested_list_elements @@ -178,7 +178,7 @@ def test_bubbles_null_for_nested_list_elements ] } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_nested_required_list_elements @@ -200,7 +200,7 @@ def test_bubbles_null_for_nested_required_list_elements ] } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_for_inner_required_lists @@ -219,7 +219,7 @@ def test_bubbles_null_for_inner_required_lists "test" => nil } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubbles_null_through_nested_required_list_scopes @@ -235,7 +235,7 @@ def test_bubbles_null_through_nested_required_list_scopes ] } - assert_nil GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_nil GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubble_through_inline_fragment @@ -264,7 +264,7 @@ def test_bubble_through_inline_fragment "test" => nil } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end def test_bubble_through_fragment_spreads @@ -289,6 +289,6 @@ def test_bubble_through_fragment_spreads "test" => nil } - assert_equal expected, GraphQL::Stitching::Shaper.new(request).perform!(raw) + assert_equal expected, GraphQL::Stitching::Executor::Shaper.new(request).perform!(raw) end end diff --git a/test/graphql/stitching/planner/plan_abstracts_test.rb b/test/graphql/stitching/planner/plan_abstracts_test.rb index ae175f2c..a069f4f0 100644 --- a/test/graphql/stitching/planner/plan_abstracts_test.rb +++ b/test/graphql/stitching/planner/plan_abstracts_test.rb @@ -124,7 +124,6 @@ def test_expands_interface_selection_fragments price } _export___typename: __typename - _export___typename: __typename } } | @@ -156,8 +155,6 @@ def test_expands_nested_interface_selection_fragments ... on Product { _export_id: id _export___typename: __typename } ... on Bundle { name price } _export___typename: __typename - _export___typename: __typename - _export___typename: __typename } } |