lib cleanups. (#177)

Author: gmac

lib/graphql/stitching/executor/root_source.rb

@@ -16,7 +16,7 @@ def fetch(ops)
           @executor.request.operation_name,
           @executor.request.operation_directives,
         )
-        query_variables = @executor.request.variables.slice(*op.variables.keys)
+        query_variables = @executor.request.variables.slice(*op.variables.each_key)
         result = @executor.request.supergraph.execute_at_location(op.location, query_document, query_variables, @executor.request)
         @executor.query_count += 1
 

lib/graphql/stitching/executor/type_resolver_source.rb

@@ -165,7 +165,7 @@ def extract_errors!(origin_sets_by_operation, errors)
         if pathed_errors_by_op_index_and_object_id.any?
           pathed_errors_by_op_index_and_object_id.each do |op_index, pathed_errors_by_object_id|
             repath_errors!(pathed_errors_by_object_id, ops.dig(op_index, "path"))
-            errors_result.concat(pathed_errors_by_object_id.values)
+            errors_result.push(*pathed_errors_by_object_id.each_value)
           end
         end
 

lib/graphql/stitching/planner.rb

@@ -24,7 +24,7 @@ def perform
       end
 
       def steps
-        @steps_by_entrypoint.values.sort_by!(&:index)
+        @steps_by_entrypoint.each_value.select { _1.selections.any? }.sort_by!(&:index)
       end
 
       private
@@ -238,6 +238,7 @@ def extract_locale_selections(
               selection_set = extract_locale_selections(current_location, field_type, parent_index, node.selections, path, locale_variables)
               path.pop
 
+              next if selection_set.empty?
               locale_selections << node.merge(selections: selection_set)
             end
 
@@ -250,6 +251,7 @@ def extract_locale_selections(
             extract_locale_selections(current_location, fragment_type, parent_index, node.selections, path, locale_variables, selection_set)
 
             unless is_same_scope
+              next if selection_set.empty?
               locale_selections << node.merge(selections: selection_set)
               requires_typename = true
             end
@@ -265,6 +267,7 @@ def extract_locale_selections(
             extract_locale_selections(current_location, fragment_type, parent_index, fragment.selections, path, locale_variables, selection_set)
 
             unless is_same_scope
+              next if selection_set.empty?
               locale_selections << GraphQL::Language::Nodes::InlineFragment.new(type: fragment.type, selections: selection_set)
             end
 
@@ -284,7 +287,7 @@ def extract_locale_selections(
           remote_selections_by_location = delegate_remote_selections(parent_type, remote_selections)
 
           # D) Create paths routing to new entrypoint locations via resolver queries.
-          routes = @supergraph.route_type_to_locations(parent_type.graphql_name, current_location, remote_selections_by_location.keys)
+          routes = @supergraph.route_type_to_locations(parent_type.graphql_name, current_location, remote_selections_by_location.each_key)
 
           # E) Translate resolver pathways into new entrypoints.
           routes.each_value do |route|

lib/graphql/stitching/request.rb

@@ -147,6 +147,11 @@ def validate
         @query.static_errors
       end
 
+      # @return [Boolean] is the request valid?
+      def valid?
+        validate.empty?
+      end
+
       # Gets and sets the query plan for the request. Assigned query plans may pull from a cache,
       # which is useful for redundant GraphQL documents (commonly sent by frontend clients).
       # ```ruby

lib/graphql/stitching/supergraph.rb

@@ -38,7 +38,7 @@ def initialize(schema:, fields: {}, resolvers: {}, visibility_profiles: [], exec
         @locations_by_type_and_field = @memoized_introspection_types.each_with_object(fields) do |(type_name, type), memo|
           next unless type.kind.fields?
 
-          memo[type_name] = type.fields.keys.each_with_object({}) do |field_name, m|
+          memo[type_name] = type.fields.each_key.each_with_object({}) do |field_name, m|
             m[field_name] = [SUPERGRAPH_LOCATION]
           end
         end.freeze
@@ -75,7 +75,7 @@ def fields
       end
 
       def locations
-        @executables.keys.reject { _1 == SUPERGRAPH_LOCATION }
+        @executables.each_key.reject { _1 == SUPERGRAPH_LOCATION }
       end
 
       def memoized_schema_fields(type_name)
@@ -130,7 +130,7 @@ def fields_by_type_and_location
       # "Type" => ["location1", "location2", ...]
       def locations_by_type
         @locations_by_type ||= @locations_by_type_and_field.each_with_object({}) do |(type_name, fields), memo|
-          memo[type_name] = fields.values.flatten.uniq
+          memo[type_name] = fields.values.tap(&:flatten!).tap(&:uniq!)
         end
       end
 

lib/graphql/stitching/supergraph/from_definition.rb

@@ -21,11 +21,8 @@ def from_definition(schema, executables:)
         field_map = {}
         resolver_map = {}
         possible_locations = {}
-        visibility_profiles = if (visibility_def = schema.directives[GraphQL::Stitching.visibility_directive])
-          visibility_def.get_argument("profiles").default_value
-        else
-          []
-        end
+        visibility_definition = schema.directives[GraphQL::Stitching.visibility_directive]
+        visibility_profiles = visibility_definition&.get_argument("profiles")&.default_value || EMPTY_ARRAY
 
         schema.types.each do |type_name, type|
           next if type.introspection?
@@ -75,7 +72,7 @@ def from_definition(schema, executables:)
           end
         end
 
-        executables = possible_locations.keys.each_with_object({}) do |location, memo|
+        executables = possible_locations.each_key.each_with_object({}) do |location, memo|
           executable = executables[location] || executables[location.to_sym]
           if validate_executable!(location, executable)
             memo[location] = executable

test/graphql/stitching/integration/introspection_test.rb

@@ -85,60 +85,4 @@ def test_performs_type_introspection_with_other_stitching
 
     assert_equal expected, result
   end
-
-  def test_handles_introspection_type_inline_fragments
-    result = plan_and_execute(@supergraph, %|
-      {
-        __typename
-        ...on __Directive { __typename }
-        ...on __EnumValue { __typename }
-        ...on __InputValue { __typename }
-        ...on __Field { __typename }
-        ...on __Schema { __typename }
-        ...on __Type { __typename }
-        product(upc: "1") {
-          ...on __Directive { __typename }
-          ...on __EnumValue { __typename }
-          ...on __InputValue { __typename }
-          ...on __Field { __typename }
-          ...on __Schema { __typename }
-          ...on __Type { __typename }
-        }
-      }
-    |)
-
-    expected = { "data" => { "__typename" => "Query", "product" => {} } }
-    assert_equal expected, result
-  end
-
-  def test_handles_introspection_type_fragment_spreads
-    result = plan_and_execute(@supergraph, %|
-      fragment DirectiveAttrs on __Directive { __typename }
-      fragment EnumValueAttrs on __EnumValue { __typename }
-      fragment InputValueAttrs on __InputValue { __typename }
-      fragment FieldAttrs on __Field { __typename }
-      fragment SchemaAttrs on __Schema { __typename }
-      fragment TypeAttrs on __Type { __typename }
-      {
-        __typename
-        ...DirectiveAttrs
-        ...EnumValueAttrs
-        ...InputValueAttrs
-        ...FieldAttrs
-        ...SchemaAttrs
-        ...TypeAttrs
-        product(upc: "1") {
-          ...DirectiveAttrs
-          ...EnumValueAttrs
-          ...InputValueAttrs
-          ...FieldAttrs
-          ...SchemaAttrs
-          ...TypeAttrs
-        }
-      }
-    |)
-
-    expected = { "data" => { "__typename" => "Query", "product" => {} } }
-    assert_equal expected, result
-  end
 end

test/graphql/stitching_test.rb

@@ -19,6 +19,22 @@ def test_digest_gets_and_sets_hashing_implementation
     end
   end
 
+  def test_gets_and_sets_library_directive_names
+    stitch_name = GraphQL::Stitching.stitch_directive
+    visibility_name = GraphQL::Stitching.visibility_directive
+
+    begin
+      GraphQL::Stitching.stitch_directive = "test"
+      assert_equal "test", GraphQL::Stitching.stitch_directive
+
+      GraphQL::Stitching.visibility_directive = "test"
+      assert_equal "test", GraphQL::Stitching.visibility_directive
+    ensure
+      GraphQL::Stitching.stitch_directive = stitch_name
+      GraphQL::Stitching.visibility_directive = visibility_name
+    end
+  end
+
   private
 
   def new_type_resolver

test/test_helper.rb

@@ -7,6 +7,7 @@
   # ignore warnings from auto-generated GraphQL lib code.
   Warning.ignore(/.*mismatched indentations.*/)
   Warning.ignore(/.*lib\/graphql\/language\/nodes.rb:.*/)
+  Warning.ignore(/Composer option `[^`]+` is deprecated.*/)
 end
 
 require 'bundler/setup'
@@ -121,6 +122,7 @@ def plan_and_execute(supergraph, query, variables={}, raw: false)
     variables: variables,
   )
 
+  assert request.valid?, "Expected request to be valid: #{request.validate.map(&:message)}"
   plan = request.plan
   executor = GraphQL::Stitching::Executor.new(request)
   result = executor.perform(raw: raw)