Skip to content

Commit

Permalink
Refactor: move where args are converted to their schema form.
Browse files Browse the repository at this point in the history
Previously, we did it in `GraphQLResolver#call`, which gets invoked as
part of resolving _every_ GraphQL field at every level of the response
structure. We don't want to convert args to their schema form if we're
not going to do anything with the args so it's better to defer it until
it's needed.
  • Loading branch information
myronmarston committed Feb 8, 2025
1 parent faa4890 commit 68bdca9
Show file tree
Hide file tree
Showing 17 changed files with 17 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def resolve(field:, object:, args:, context:, lookahead:)
query = @datastore_query_builder.new_query(
search_index_definitions: [@product_index_def],
monotonic_clock_deadline: context[:monotonic_clock_deadline],
filter: {"id" => {"equalToAnyOf" => [args.fetch("id")]}},
filter: {"id" => {"equalToAnyOf" => [args.fetch(:id)]}},
individual_docs_needed: true,
requested_fields: %w[
id sku package notes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def can_resolve?(field:, object:)
def resolve(field:, object:, args:, context:, lookahead:)
schema = context.fetch(:elastic_graph_schema)

representations = args.fetch("representations").map.with_index do |rep, index|
representations = args.fetch(:representations).map.with_index do |rep, index|
try_parse_representation(rep, schema) do |error_description|
context.add_error(::GraphQL::ExecutionError.new("Representation at index #{index} #{error_description}."))
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def resolve(field:, object:, args:, context:, lookahead:)
new_field_path = field_path + [path_segment]
return with(field_path: new_field_path) unless field.type.elasticgraph_category == :nested_sub_aggregation_connection

args = field.args_to_schema_form(args.except(:lookahead))
sub_agg_key = FieldPathEncoder.encode(new_field_path.map(&:name_in_graphql_query))
sub_agg_query = Support::HashUtil.verbose_fetch(sub_aggregations, sub_agg_key).query

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def resolve(field:, object:, args:, context:, lookahead:)
end

if field.type.relay_connection?
args = field.args_to_schema_form(args.except(:lookahead))
RelayConnection::ArrayAdapter.build(value, args, @schema_element_names, context)
else
value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ def call(parent_type, field, object, args, context)
# It is not a "real" arg in the schema and breaks `args_to_schema_form` when we call that
# so we need to peel it off here.
lookahead = args[:lookahead]
# Convert args to the form they were defined in the schema, undoing the normalization
# the GraphQL gem does to convert them to Ruby keyword args form.
args = schema_field.args_to_schema_form(args.except(:lookahead))

resolver = resolver_for(schema_field, object) do
raise <<~ERROR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def can_resolve?(field:, object:)
end

def resolve(field:, context:, lookahead:, args:, object:)
args = field.args_to_schema_form(args.except(:lookahead))
query = @resolver_query_adapter.build_query_from(field: field, args: args, lookahead: lookahead, context: context)
response = QuerySource.execute_one(query, for_context: context)
RelayConnection.maybe_wrap(response, field: field, context: context, lookahead: lookahead, query: query)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ def resolve(object:, field:, context:, lookahead:, args:)
build_filter(join.filter_id_field_name, nil, join.foreign_key_nested_paths, Array(id_or_ids)),
join.additional_filter
].reject(&:empty?)

args = field.args_to_schema_form(args.except(:lookahead))
query = @resolver_query_adapter
.build_query_from(field: field, args: args, lookahead: lookahead, context: context)
.merge_with(filters: filters)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def self.new(*fields, &block)
end

def resolve(field:, object:, context:, args:, lookahead:)
args = field.args_to_schema_form(args.except(:lookahead))
method_name = canonical_name_for(field.name, "Field")
public_send(method_name, **args_to_canonical_form(args))
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ module Arguments
def self.to_schema_form(args_value, args_owner)
# For custom scalar types (such as `_Any` for apollo federation), `args_owner` won't
# response to `arguments`.
return args_value unless args_owner.respond_to?(:arguments)
return (_ = args_value) unless args_owner.respond_to?(:arguments)

__skip__ = case args_value
when Hash, ::GraphQL::Schema::InputObject
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module ElasticGraph
class GraphQL
module Resolvers
type fieldArgs = ::Hash[::String, untyped]
type fieldArgs = ::Hash[::Symbol, untyped]

interface _Resolver
def can_resolve?: (field: Schema::Field, object: untyped) -> bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module ElasticGraph

private

def args_to_canonical_form: (fieldArgs) -> ::Hash[::Symbol, untyped]
def args_to_canonical_form: (::Hash[::String, untyped]) -> ::Hash[::Symbol, untyped]
def canonical_name_for: (::String | ::Symbol, ::String) -> ::Symbol
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module ElasticGraph
class Schema
module Arguments
def self.to_schema_form: (
::Hash[::String, untyped],
::Hash[::Symbol, untyped],
untyped
) -> ::Hash[::String, untyped]
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module ElasticGraph

def aggregated?: () -> bool
def sort_clauses_for: (::Array[::String]) -> ::Array[::Hash[::String, ::Hash[::String, ::String]]]
def args_to_schema_form: (::Hash[::String, untyped]) -> ::Hash[::String, untyped]
def args_to_schema_form: (::Hash[::Symbol, untyped]) -> ::Hash[::String, untyped]
def index_field_names_for_resolution: () -> ::Array[::String]
end
end
Expand Down
2 changes: 1 addition & 1 deletion elasticgraph-graphql/sig/graphql_gem.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module GraphQL
attr_reader name: ::String
attr_reader field: Schema::Field
attr_reader owner_type: Schema::_Type
attr_reader arguments: ::Hash[::String, untyped]
attr_reader arguments: ::Hash[::Symbol, untyped]

def initialize: (
query: Query,
Expand Down
3 changes: 1 addition & 2 deletions elasticgraph-graphql/spec/support/resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@
require "graphql"

module ResolverHelperMethods
def resolve(type_name, field_name, document = nil, lookahead: nil, query_overrides: {}, **options)
def resolve(type_name, field_name, document = nil, lookahead: nil, query_overrides: {}, **args)
query_override_adapter.query_overrides = query_overrides
field = graphql.schema.field_named(type_name, field_name)
args = field.args_to_schema_form(options)
lookahead ||= GraphQL::Execution::Lookahead::NULL_LOOKAHEAD
query_details_tracker = ElasticGraph::GraphQL::QueryDetailsTracker.empty

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,8 @@ def can_resolve?(field:, object:)

def resolve(field:, object:, args:, context:, lookahead:)
[
args.dig("operands", "x"),
args.dig("operands", "y"),
args.dig(:operands, "x"),
args.dig(:operands, "y"),
context[:additional_operand]
].compact.reduce(:*)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ module Resolvers

def resolve(args: {}, **options)
person, field = person_object_and_schema_field(**options)
args = field.args_to_schema_form(args)
lookahead = instance_double("GraphQL::Execution::Lookahead")
person.resolve(field: field, object: person, context: {}, args: args, lookahead: lookahead)
end
Expand Down

0 comments on commit 68bdca9

Please sign in to comment.