Fix planning key deduplication (#150)

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"

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

lib/graphql/stitching/shaper.rb → 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

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(

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

lib/graphql/stitching/planner_step.rb → 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

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)

lib/graphql/stitching/version.rb

@@ -2,6 +2,6 @@
 
 module GraphQL
   module Stitching
-    VERSION = "1.4.2"
+    VERSION = "1.4.3"
   end
 end

test/graphql/stitching/shaper/grooming_test.rb → 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

test/graphql/stitching/shaper/null_bubbling_test.rb → 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

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
         }
       }
     |