Skip to content

Commit

Permalink
Refactor: simplify and optimiize ElasticGraph::GraphQL::Schema.
Browse files Browse the repository at this point in the history
- 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`.
  • Loading branch information
myronmarston committed Nov 12, 2024
1 parent 91dbffd commit c748940
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 34 deletions.
31 changes: 12 additions & 19 deletions elasticgraph-graphql/lib/elastic_graph/graphql/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:,
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down

0 comments on commit c748940

Please sign in to comment.