claude cleanups. (#181)

Author: gmac

lib/graphql/stitching/client.rb

@@ -25,7 +25,7 @@ def initialize(locations: nil, supergraph: nil, composer_options: {})
           raise ArgumentError, "Cannot provide both locations and a supergraph."
         elsif supergraph && !supergraph.is_a?(Supergraph)
           raise ArgumentError, "Provided supergraph must be a GraphQL::Stitching::Supergraph instance."
-        elsif supergraph && composer_options.any?
+        elsif supergraph && !composer_options.empty?
           raise ArgumentError, "Cannot provide composer options with a pre-built supergraph."
         elsif supergraph
           supergraph
@@ -50,7 +50,7 @@ def execute(raw_query = nil, query: nil, variables: nil, operation_name: nil, co
 
         if validate
           validation_errors = request.validate
-          return error_result(request, validation_errors) if validation_errors.any?
+          return error_result(request, validation_errors) unless validation_errors.empty?
         end
 
         load_plan(request)

lib/graphql/stitching/composer.rb

@@ -423,7 +423,7 @@ def build_merged_arguments(type_name, members_by_location, owner, field_name: ni
             memo[location] = argument.default_value
           end
 
-          if default_values_by_location.any?
+          unless default_values_by_location.empty?
             kwargs[:default_value] = @default_value_merger.call(default_values_by_location, {
               type_name: type_name,
               field_name: field_name,

lib/graphql/stitching/composer/type_resolver_config.rb

@@ -8,7 +8,7 @@ class TypeResolverConfig
 
       class << self
         def extract_directive_assignments(schema, location, assignments)
-          return EMPTY_OBJECT unless assignments && assignments.any?
+          return EMPTY_OBJECT unless assignments && !assignments.empty?
 
           assignments.each_with_object({}) do |kwargs, memo|
             type = kwargs[:parent_type_name] ? schema.get_type(kwargs[:parent_type_name]) : schema.query

lib/graphql/stitching/executor.rb

@@ -79,7 +79,7 @@ def exec!(next_steps)
 
       def exec_task(task)
         next_steps = task.load.tap(&:compact!)
-        exec!(next_steps) if next_steps.any?
+        exec!(next_steps) unless next_steps.empty?
       end
     end
   end

lib/graphql/stitching/executor/root_source.rb

@@ -21,7 +21,7 @@ def fetch(ops)
         @executor.query_count += 1
 
         if result["data"]
-          if op.path.any?
+          unless op.path.empty?
             # Nested root scopes must expand their pathed origin set
             origin_set = op.path.reduce([@executor.data]) do |set, ns|
               set.flat_map { |obj| obj && obj[ns] }.tap(&:compact!)
@@ -44,32 +44,36 @@ def fetch(ops)
       # Builds root source documents
       # "query MyOperation_1($var:VarType) { rootSelections ... }"
       def build_document(op, operation_name = nil, operation_directives = nil)
-        doc = String.new
-        doc << op.operation_type
+        doc_buffer = String.new
+        doc_buffer << op.operation_type
 
         if operation_name
-          doc << " #{operation_name}_#{op.step}"
+          doc_buffer << " " << operation_name << "_" << op.step.to_s
         end
 
-        if op.variables.any?
-          variable_defs = op.variables.map { |k, v| "$#{k}:#{v}" }.join(",")
-          doc << "(#{variable_defs})"
+        unless op.variables.empty?
+          doc_buffer << "("
+          op.variables.each_with_index do |(k, v), i|
+            doc_buffer << "," unless i.zero?
+            doc_buffer << "$" << k << ":" << v
+          end
+          doc_buffer << ")"
         end
 
         if operation_directives
-          doc << " #{operation_directives} "
+          doc_buffer << " " << operation_directives << " "
         end
 
-        doc << op.selections
-        doc
+        doc_buffer << op.selections
+        doc_buffer
       end
 
       # Format response errors without a document location (because it won't match the request doc),
       # and prepend any insertion path for the scope into error paths.
       def format_errors!(errors, path)
         errors.each do |err|
           err.delete("locations")
-          err["path"].unshift(*path) if err["path"] && path.any?
+          err["path"].unshift(*path) if err["path"] && !path.empty?
         end
         errors
       end

lib/graphql/stitching/executor/type_resolver_source.rb

@@ -20,10 +20,10 @@ def fetch(ops)
             origin_set.select! { _1[TypeResolver::TYPENAME_EXPORT_NODE.alias] == op.if_type }
           end
 
-          memo[op] = origin_set if origin_set.any?
+          memo[op] = origin_set unless origin_set.empty?
         end
 
-        if origin_sets_by_operation.any?
+        unless origin_sets_by_operation.empty?
           query_document, variable_names = build_document(
             origin_sets_by_operation,
             @executor.request.operation_name,
@@ -51,61 +51,76 @@ def fetch(ops)
       # }"
       def build_document(origin_sets_by_operation, operation_name = nil, operation_directives = nil)
         variable_defs = {}
-        query_fields = origin_sets_by_operation.map.with_index do |(op, origin_set), batch_index|
+        fields_buffer = String.new
+
+        origin_sets_by_operation.each_with_index do |(op, origin_set), batch_index|
           variable_defs.merge!(op.variables)
           resolver = @executor.request.supergraph.resolvers_by_version[op.resolver]
+          fields_buffer << " " unless batch_index.zero?
 
           if resolver.list?
-            arguments = resolver.arguments.map.with_index do |arg, i|
+            fields_buffer << "_" << batch_index.to_s << "_result: " << resolver.field << "("
+
+            resolver.arguments.each_with_index do |arg, i|
+              fields_buffer << "," unless i.zero?
               if arg.key?
                 variable_name = "_#{batch_index}_key_#{i}".freeze
                 @variables[variable_name] = origin_set.map { arg.build(_1) }
                 variable_defs[variable_name] = arg.to_type_signature
-                "#{arg.name}:$#{variable_name}"
+                fields_buffer << arg.name << ":$" << variable_name
               else
-                "#{arg.name}:#{arg.value.print}"
+                fields_buffer << arg.name << ":" << arg.value.print
               end
             end
 
-            "_#{batch_index}_result: #{resolver.field}(#{arguments.join(",")}) #{op.selections}"
+            fields_buffer << ") " << op.selections
           else
-            origin_set.map.with_index do |origin_obj, index|
-              arguments = resolver.arguments.map.with_index do |arg, i|
+            origin_set.each_with_index do |origin_obj, index|
+              fields_buffer << " " unless index.zero?
+              fields_buffer << "_" << batch_index.to_s << "_" << index.to_s << "_result: " << resolver.field << "("
+
+              resolver.arguments.each_with_index do |arg, i|
+                fields_buffer << "," unless i.zero?
                 if arg.key?
                   variable_name = "_#{batch_index}_#{index}_key_#{i}".freeze
                   @variables[variable_name] = arg.build(origin_obj)
                   variable_defs[variable_name] = arg.to_type_signature
-                  "#{arg.name}:$#{variable_name}"
+                  fields_buffer << arg.name << ":$" << variable_name
                 else
-                  "#{arg.name}:#{arg.value.print}"
+                  fields_buffer << arg.name << ":" << arg.value.print
                 end
               end
 
-              "_#{batch_index}_#{index}_result: #{resolver.field}(#{arguments.join(",")}) #{op.selections}"
+              fields_buffer << ") " << op.selections
             end
           end
         end
 
-        doc = String.new(QUERY_OP) # << resolver fulfillment always uses query
+        doc_buffer = String.new(QUERY_OP) # << resolver fulfillment always uses query
 
         if operation_name
-          doc << " #{operation_name}"
+          doc_buffer << " " << operation_name
           origin_sets_by_operation.each_key do |op|
-            doc << "_#{op.step}"
+            doc_buffer << "_" << op.step.to_s
           end
         end
 
-        if variable_defs.any?
-          doc << "(#{variable_defs.map { |k, v| "$#{k}:#{v}" }.join(",")})"
+        unless variable_defs.empty?
+          doc_buffer << "("
+          variable_defs.each_with_index do |(k, v), i|
+            doc_buffer << "," unless i.zero?
+            doc_buffer << "$" << k << ":" << v
+          end
+          doc_buffer << ")"
         end
 
         if operation_directives
-          doc << " #{operation_directives} "
+          doc_buffer << " " << operation_directives << " "
         end
 
-        doc << "{ #{query_fields.join(" ")} }"
+        doc_buffer << "{ " << fields_buffer << " }"
 
-        return doc, variable_defs.keys.tap do |names|
+        return doc_buffer, variable_defs.keys.tap do |names|
           names.reject! { @variables.key?(_1) }
         end
       end
@@ -120,10 +135,11 @@ def merge_results!(origin_sets_by_operation, raw_result)
             origin_set.map.with_index { |_, index| raw_result["_#{batch_index}_#{index}_result"] }
           end
 
-          next unless results&.any?
+          next if results.nil? || results.empty?
 
           origin_set.each_with_index do |origin_obj, index|
-            origin_obj.merge!(results[index]) if results[index]
+            result = results[index]
+            origin_obj.merge!(result) if result
           end
         end
       end
@@ -132,7 +148,7 @@ def merge_results!(origin_sets_by_operation, raw_result)
       def extract_errors!(origin_sets_by_operation, errors)
         ops = origin_sets_by_operation.keys
         origin_sets = origin_sets_by_operation.values
-        pathed_errors_by_op_index_and_object_id = {}
+        pathed_errors_by_op_index_and_object_id = Hash.new { |h, k| h[k] = {} }
 
         errors_result = errors.each_with_object([]) do |err, memo|
           err.delete("locations")
@@ -151,8 +167,8 @@ def extract_errors!(origin_sets_by_operation, errors)
               end
 
               if origin_obj
-                by_op_index = pathed_errors_by_op_index_and_object_id[result_alias[1].to_i] ||= {}
-                by_object_id = by_op_index[origin_obj.object_id] ||= []
+                pathed_errors_by_op_index = pathed_errors_by_op_index_and_object_id[result_alias[1].to_i]
+                by_object_id = pathed_errors_by_op_index[origin_obj.object_id] ||= []
                 by_object_id << err
                 next
               end
@@ -162,9 +178,9 @@ def extract_errors!(origin_sets_by_operation, errors)
           memo << err
         end
 
-        if pathed_errors_by_op_index_and_object_id.any?
+        unless pathed_errors_by_op_index_and_object_id.empty?
           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"))
+            repath_errors!(pathed_errors_by_object_id, ops[op_index].path)
             errors_result.push(*pathed_errors_by_object_id.each_value)
           end
         end
@@ -180,7 +196,7 @@ def repath_errors!(pathed_errors_by_object_id, forward_path, current_path=[], ro
         current_path.push(forward_path.shift)
         scope = root[current_path.last]
 
-        if forward_path.any? && scope.is_a?(Array)
+        if !forward_path.empty? && scope.is_a?(Array)
           scope.each_with_index do |element, index|
             inner_elements = element.is_a?(Array) ? element.flatten : [element]
             inner_elements.each do |inner_element|
@@ -190,7 +206,7 @@ def repath_errors!(pathed_errors_by_object_id, forward_path, current_path=[], ro
             end
           end
 
-        elsif forward_path.any?
+        elsif !forward_path.empty?
           repath_errors!(pathed_errors_by_object_id, forward_path, current_path, scope)
 
         elsif scope.is_a?(Array)

lib/graphql/stitching/http_executable.rb

@@ -80,7 +80,7 @@ def extract_multipart_form(request, document, variables)
 
         return if files_by_path.none?
 
-        map = {}
+        map = Hash.new { |h, k| h[k] = [] }
         files = files_by_path.values.tap(&:uniq!)
         variables_copy = variables.dup
 
@@ -90,7 +90,6 @@ def extract_multipart_form(request, document, variables)
           path.each_with_index do |key, i|
             if i == path.length - 1
               file_index = files.index(copy[key]).to_s
-              map[file_index] ||= []
               map[file_index] << "variables.#{path.join(".")}"
               copy[key] = nil
             elsif orig[key].object_id == copy[key].object_id

lib/graphql/stitching/plan.rb

@@ -2,21 +2,42 @@
 
 module GraphQL
   module Stitching
-    # Immutable-ish structures representing a query plan.
+    # Immutable structures representing a query plan.
     # May serialize to/from JSON.
     class Plan
-      Op = Struct.new(
-        :step,
-        :after,
-        :location,
-        :operation_type,
-        :selections,
-        :variables,
-        :path,
-        :if_type,
-        :resolver,
-        keyword_init: true
-      ) do
+      class Op
+        attr_reader :step
+        attr_reader :after
+        attr_reader :location
+        attr_reader :operation_type
+        attr_reader :selections
+        attr_reader :variables
+        attr_reader :path
+        attr_reader :if_type
+        attr_reader :resolver
+        
+        def initialize(
+          step:,
+          after:,
+          location:,
+          operation_type:,
+          selections:,
+          variables: nil,
+          path: nil,
+          if_type: nil,
+          resolver: nil
+        )
+          @step = step
+          @after = after
+          @location = location
+          @operation_type = operation_type
+          @selections = selections
+          @variables = variables
+          @path = path
+          @if_type = if_type
+          @resolver = resolver
+        end
+
         def as_json
           {
             step: step,
@@ -30,6 +51,18 @@ def as_json
             resolver: resolver
           }.tap(&:compact!)
         end
+
+        def ==(other)
+          step == other.step &&
+            after == other.after &&
+            location == other.location &&
+            operation_type == other.operation_type &&
+            selections == other.selections &&
+            variables == other.variables &&
+            path == other.path &&
+            if_type == other.if_type &&
+            resolver == other.resolver
+        end
       end
 
       class << self