Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: extract lookahead from args in individual resolvers. #164

Draft
wants to merge 1 commit into
base: myron/improve-perf/move-arg-conversion
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def can_resolve?(field:, object:)
field.parent_type.name == :Query && field.name == :product
end

def resolve(field:, object:, args:, context:, lookahead:)
def resolve(field:, object:, args:, context:)
query = @datastore_query_builder.new_query(
search_index_definitions: [@product_index_def],
monotonic_clock_deadline: context[:monotonic_clock_deadline],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def can_resolve?(field:, object:)
field.parent_type.name == :Query && field.name == :_entities
end

def resolve(field:, object:, args:, context:, lookahead:)
def resolve(field:, object:, args:, context:)
schema = context.fetch(:elastic_graph_schema)

representations = args.fetch(:representations).map.with_index do |rep, index|
Expand All @@ -43,7 +43,7 @@ def resolve(field:, object:, args:, context:, lookahead:)
# so we build the hash of those attributes once here.
query_attributes = ElasticGraph::GraphQL::QueryAdapter::RequestedFields
.new(schema)
.query_attributes_for(field: field, lookahead: lookahead)
.query_attributes_for(field: field, lookahead: args.fetch(:lookahead))
.merge(monotonic_clock_deadline: context[:monotonic_clock_deadline])

# Build a separate query per adapter instance since each adapter instance is capable of building
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def can_resolve?(field:, object:)
field.parent_type.name == :Query && field.name == :_service
end

def resolve(field:, object:, args:, context:, lookahead:)
def resolve(field:, object:, args:, context:)
{"sdl" => service_sdl(context.fetch(:elastic_graph_schema).graphql_schema)}
end

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

def resolve(field:, object:, args:, context:, lookahead:)
return with(field_path: field_path + [PathSegment.for(field: field, lookahead: lookahead)]) if field.type.object?
def resolve(field:, object:, args:, context:)
return with(field_path: field_path + [PathSegment.for(field: field, lookahead: args.fetch(:lookahead))]) if field.type.object?

key = Key::AggregatedValue.new(
aggregation_name: aggregation_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ def can_resolve?(field:, object:)
true
end

def resolve(field:, object:, args:, context:, lookahead:)
new_field_path = field_path + [PathSegment.for(field: field, lookahead: lookahead)]
def resolve(field:, object:, args:, context:)
new_field_path = field_path + [PathSegment.for(field: field, lookahead: args.fetch(:lookahead))]
return with(field_path: new_field_path) if field.type.object?

bucket_entry = Support::HashUtil.verbose_fetch(bucket, "key")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ def can_resolve?(field:, object:)
true
end

def resolve(field:, object:, args:, context:, lookahead:)
path_segment = PathSegment.for(field: field, lookahead: lookahead)
def resolve(field:, object:, args:, context:)
path_segment = PathSegment.for(field: field, lookahead: args.fetch(:lookahead))
new_field_path = field_path + [path_segment]
return with(field_path: new_field_path) unless field.type.elasticgraph_category == :nested_sub_aggregation_connection

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def can_resolve?(field:, object:)
object.is_a?(DatastoreResponse::Document) || object.is_a?(::Hash)
end

def resolve(field:, object:, args:, context:, lookahead:)
def resolve(field:, object:, args:, context:)
field_name = field.name_in_index.to_s
data =
case object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ def initialize(schema:, runtime_metadata:, resolvers:)
def call(parent_type, field, object, args, context)
schema_field = @schema.field_named(parent_type.graphql_name, field.name)

# Extract the `:lookahead` extra that we have configured all fields to provide.
# See https://graphql-ruby.org/api-doc/1.10.8/GraphQL/Execution/Lookahead.html for more info.
# 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]

resolver = resolver_for(schema_field, object) do
raise <<~ERROR
No resolver yet implemented for this case.
Expand All @@ -52,7 +46,7 @@ def call(parent_type, field, object, args, context)
ERROR
end

result = resolver.resolve(field: schema_field, object: object, args: args, context: context, lookahead: lookahead)
result = resolver.resolve(field: schema_field, object: object, args: args, context: context)

# Give the field a chance to coerce the result before returning it. Initially, this is only used to deal with
# enum value overrides (e.g. so that if `DayOfWeek.MONDAY` has been overridden to `DayOfWeek.MON`, we can coerce
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ def can_resolve?(field:, object:)
field.parent_type.name == :Query && field.type.collection?
end

def resolve(field:, context:, lookahead:, args:, object:)
def resolve(field:, context:, args:, object:)
lookahead = args.fetch(:lookahead)
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def can_resolve?(field:, object:)
!!field.relation_join
end

def resolve(object:, field:, context:, lookahead:, args:)
def resolve(object:, field:, context:, args:)
log_warning = ->(**options) { log_field_problem_warning(field: field, **options) }
join = field.relation_join
id_or_ids = join.extract_id_or_ids_from(object, log_warning)
Expand All @@ -36,6 +36,7 @@ def resolve(object:, field:, context:, lookahead:, args:)
join.additional_filter
].reject(&:empty?)

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

def resolve(field:, object:, context:, args:, lookahead:)
def resolve(field:, object:, context:, args:)
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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module ElasticGraph
field: Schema::Field,
object: untyped,
context: ::GraphQL::Query::Context,
args: fieldArgs,
lookahead: ::GraphQL::Execution::Lookahead) -> untyped
args: fieldArgs
) -> untyped
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions elasticgraph-graphql/spec/support/resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
require "graphql"

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

::GraphQL::Dataloader.with_dataloading do |dataloader|
Expand All @@ -38,7 +38,7 @@ def resolve(type_name, field_name, document = nil, lookahead: nil, query_overrid
# [^1]: https://github.com/rmosolgo/graphql-ruby/blob/v2.1.0/lib/graphql/pagination/connection.rb#L94-L96
# [^2]: https://github.com/rmosolgo/graphql-ruby/blob/v2.1.0/lib/graphql/execution/interpreter/runtime.rb#L935-L941
::Thread.current[:__graphql_runtime_info] = ::Hash.new { |h, k| h[k] = ::GraphQL::Execution::Interpreter::Runtime::CurrentState.new }
resolver.resolve(field: field, object: document, context: context, args: args, lookahead: lookahead)
resolver.resolve(field: field, object: document, context: context, args: args)
ensure
::Thread.current[:__graphql_runtime_info] = nil
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ def can_resolve?(field:, object:)
field.name == :multiply
end

def resolve(field:, object:, args:, context:, lookahead:)
def resolve(field:, object:, args:, context:)
[
args.dig(:operands, "x"),
args.dig(:operands, "y"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ module Resolvers
def resolve(args: {}, **options)
person, field = person_object_and_schema_field(**options)
lookahead = instance_double("GraphQL::Execution::Lookahead")
person.resolve(field: field, object: person, context: {}, args: args, lookahead: lookahead)
person.resolve(field: field, object: person, context: {}, args: args.merge(lookahead: lookahead))
end
end

Expand Down