From 571c2fd5acf988046112e98bef08ddad55dbafca Mon Sep 17 00:00:00 2001
From: Myron Marston <myron@squareup.com>
Date: Sun, 10 Nov 2024 23:31:29 -0800
Subject: [PATCH] Refactor: simplify and optimize
 `ElasticGraph::GraphQL::Schema`.

- Avoid calling `::GraphQL::Schema#types` repeatedly. As reported in rmosolgo/graphql-ruby#5154,
  accessing `#types` repeatedly can be quite slow (at least on 2.4.0 - 2.4.2). While it's been
  optimized in 2.4.3, there's no need to access it more than once. Previously, we accessed it
  in `#lookup_type_by_name`; instead we can just use the type objects as we iterate over
  `::GraphQL::Schema#types` a single time.
- Remove unused `#defined_types` method.
- Inline the old `#lookup_type_by_name` logic into `#type_named`.
---
 .../lib/elastic_graph/graphql/schema.rb       | 31 +++++++------------
 .../unit/elastic_graph/graphql/schema_spec.rb | 15 ---------
 2 files changed, 12 insertions(+), 34 deletions(-)

diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/schema.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/schema.rb
index 19956248..354f2990 100644
--- a/elasticgraph-graphql/lib/elastic_graph/graphql/schema.rb
+++ b/elasticgraph-graphql/lib/elastic_graph/graphql/schema.rb
@@ -29,7 +29,7 @@ class Schema
         scalar_types.to_set.union(introspection_types)
       )
 
-      attr_reader :element_names, :defined_types, :config, :graphql_schema, :runtime_metadata
+      attr_reader :element_names, :config, :graphql_schema, :runtime_metadata
 
       def initialize(
         graphql_schema_string:,
@@ -53,7 +53,6 @@ def initialize(
           )
         end
 
-        @types_by_name = Hash.new { |hash, key| hash[key] = lookup_type_by_name(key) }
         @build_resolver = build_resolver
 
         # Note: as part of loading the schema, the GraphQL gem may use the resolver (such
@@ -68,7 +67,7 @@ def initialize(
 
         # Pre-load all defined types so that all field extras can get configured as part
         # of loading the schema, before we execute the first query.
-        @defined_types = build_defined_types_array(@graphql_schema)
+        @types_by_name = build_types_by_name
       end
 
       def type_from(graphql_type)
@@ -80,7 +79,11 @@ def type_from(graphql_type)
       # get type objects for wrapped types, but you need to get it from a field object of that
       # type.
       def type_named(type_name)
-        @types_by_name[type_name.to_s]
+        @types_by_name.fetch(type_name.to_s)
+      rescue KeyError => e
+        msg = "No type named #{type_name} could be found"
+        msg += "; Possible alternatives: [#{e.corrections.join(", ").delete('"')}]." if e.corrections.any?
+        raise Errors::NotFoundError, msg
       end
 
       def document_type_stored_in(index_definition_name)
@@ -103,7 +106,7 @@ def enum_value_named(type_name, enum_value_name)
 
       # The list of user-defined types that are indexed document types. (Indexed aggregation types will not be included in this.)
       def indexed_document_types
-        @indexed_document_types ||= defined_types.select(&:indexed_document?)
+        @indexed_document_types ||= @types_by_name.values.select(&:indexed_document?)
       end
 
       def to_s
@@ -128,24 +131,14 @@ def resolver
         def_delegators :resolver, :call, :resolve_type, :coerce_input, :coerce_result
       end
 
-      def lookup_type_by_name(type_name)
-        type_from(@graphql_schema.types.fetch(type_name))
-      rescue KeyError => e
-        msg = "No type named #{type_name} could be found"
-        msg += "; Possible alternatives: [#{e.corrections.join(", ").delete('"')}]." if e.corrections.any?
-        raise Errors::NotFoundError, msg
-      end
-
       def resolver
         @resolver ||= @build_resolver.call(self)
       end
 
-      def build_defined_types_array(graphql_schema)
-        graphql_schema
-          .types
-          .values
-          .reject { |t| BUILT_IN_TYPE_NAMES.include?(t.graphql_name) }
-          .map { |t| type_named(t.graphql_name) }
+      def build_types_by_name
+        graphql_schema.types.transform_values do |graphql_type|
+          @types_by_graphql_type[graphql_type]
+        end
       end
 
       def indexed_document_types_by_index_definition_name
diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema_spec.rb
index c15fe71d..43ce5a61 100644
--- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema_spec.rb
+++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema_spec.rb
@@ -70,21 +70,6 @@ class GraphQL
         end
       end
 
-      describe "#defined_types" do
-        it "returns a list containing all explicitly defined types (excluding built-ins)" do
-          schema = define_schema do |s|
-            s.enum_type "Options" do |t|
-              t.value "firstOption"
-            end
-            s.object_type "Color"
-          end
-
-          expect(schema.defined_types).to all be_a Schema::Type
-          expect(schema.defined_types.map(&:name)).to include(:Options, :Color, :Query)
-            .and exclude(:Int, :Float, :Boolean, :String, :ID)
-        end
-      end
-
       describe "#indexed_document_types" do
         it "returns a list containing all types defined as indexed types" do
           schema = define_schema do |s|