Skip to content

Commit f896518

Browse files
authored
Merge pull request #15 from gmac/library-updates
Refacoring
2 parents 6a06782 + 60af348 commit f896518

File tree

7 files changed

+112
-108
lines changed

7 files changed

+112
-108
lines changed

lib/graphql/cardinal/executor.rb

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
require_relative "./executor/execution_field"
55
require_relative "./executor/authorization"
66
require_relative "./executor/hot_paths"
7-
require_relative "./executor/response_hash"
7+
require_relative "./executor/result_hash"
88
require_relative "./executor/error_formatter"
99

1010
module GraphQL
@@ -42,8 +42,8 @@ def perform
4242
ExecutionScope.new(
4343
parent_type: @query.root_type_for_operation(operation.operation_type),
4444
selections: operation.selections,
45-
sources: [@root_object],
46-
responses: [@data],
45+
objects: [@root_object],
46+
results: [@data],
4747
)
4848
]
4949
when "mutation"
@@ -53,8 +53,8 @@ def perform
5353
ExecutionScope.new(
5454
parent_type: mutation_type,
5555
selections: exec_field.nodes,
56-
sources: [@root_object],
57-
responses: [@data],
56+
objects: [@root_object],
57+
results: [@data],
5858
)
5959
end
6060
else
@@ -67,9 +67,9 @@ def perform
6767
execute_scope(@exec_queue.shift) until @exec_queue.empty?
6868
end
6969

70-
response = { "data" => @errors.empty? ? @data : ErrorFormatter.new(@query, @data, @errors).perform }
71-
response["errors"] = @errors.map(&:to_h) unless @errors.empty?
72-
response
70+
result = { "data" => @errors.empty? ? @data : ErrorFormatter.new(@query, @data, @errors).perform }
71+
result["errors"] = @errors.map(&:to_h) unless @errors.empty?
72+
result
7373
end
7474

7575
private
@@ -79,7 +79,7 @@ def execute_scope(exec_scope)
7979
exec_scope.fields = execution_fields_by_key(exec_scope.parent_type, exec_scope.selections)
8080
exec_scope.fields.each_value do |exec_field|
8181
parent_type = exec_scope.parent_type
82-
parent_sources = exec_scope.sources
82+
parent_objects = exec_scope.objects
8383
field_name = exec_field.name
8484

8585
exec_field.scope = exec_scope
@@ -97,20 +97,20 @@ def execute_scope(exec_scope)
9797

9898
exec_field.result = if !field_resolver.authorized?(@context)
9999
@errors << AuthorizationError.new(type_name: parent_type.graphql_name, field_name: field_name, path: exec_field.path, base: true)
100-
Array.new(parent_sources.length, @errors.last)
100+
Array.new(parent_objects.length, @errors.last)
101101
elsif !Authorization.can_access_type?(value_type, @context)
102102
@errors << AuthorizationError.new(type_name: value_type.graphql_name, path: exec_field.path, base: true)
103-
Array.new(parent_sources.length, @errors.last)
103+
Array.new(parent_objects.length, @errors.last)
104104
else
105105
begin
106-
@tracers.each { _1.before_resolve_field(parent_type, field_name, parent_sources.length, @context) }
107-
field_resolver.resolve(parent_sources, exec_field.arguments(@variables), @context, exec_scope)
106+
@tracers.each { _1.before_resolve_field(parent_type, field_name, parent_objects.length, @context) }
107+
field_resolver.resolve(parent_objects, exec_field.arguments(@variables), @context, exec_scope)
108108
rescue StandardError => e
109109
report_exception(error: e, field: exec_field)
110110
@errors << InternalError.new(path: exec_field.path, base: true)
111-
Array.new(parent_sources.length, @errors.last)
111+
Array.new(parent_objects.length, @errors.last)
112112
ensure
113-
@tracers.each { _1.after_resolve_field(parent_type, field_name, parent_sources.length, @context) }
113+
@tracers.each { _1.after_resolve_field(parent_type, field_name, parent_objects.length, @context) }
114114
@exec_count += 1
115115
end
116116
end
@@ -121,41 +121,44 @@ def execute_scope(exec_scope)
121121
if exec_scope.lazy_fields_ready?
122122
exec_scope.send(:lazy_exec!) # << noop for loaders that have already run
123123
exec_scope.fields.each_value do |exec_field|
124-
sources = exec_field.result.is_a?(Promise) ? exec_field.result.value : exec_field.result
125-
resolve_execution_field(exec_field, sources)
124+
objects = exec_field.result.is_a?(Promise) ? exec_field.result.value : exec_field.result
125+
build_execution_field(exec_field, objects)
126126
end
127127
else
128128
# requeue the scope to wait on others that haven't built fields yet
129129
@exec_queue << exec_scope
130130
end
131131
else
132132
exec_scope.fields.each_value do |exec_field|
133-
resolve_execution_field(exec_field, exec_field.result)
133+
build_execution_field(exec_field, exec_field.result)
134134
end
135135
end
136136

137137
nil
138138
end
139139

140-
def resolve_execution_field(exec_field, resolved_sources)
141-
parent_sources = exec_field.scope.sources
142-
parent_responses = exec_field.scope.responses
140+
def build_execution_field(exec_field, resolved_objects)
141+
parent_objects = exec_field.scope.objects
142+
parent_results = exec_field.scope.results
143143
field_key = exec_field.key
144144
field_type = exec_field.type
145145
return_type = field_type.unwrap
146146

147-
if resolved_sources.length != parent_sources.length
148-
report_exception("Incorrect number of results resolved. Expected #{parent_sources.length}, got #{resolved_sources.length}", field: exec_field)
149-
resolved_sources = Array.new(parent_sources.length, nil)
147+
if resolved_objects.length != parent_objects.length
148+
report_exception("Incorrect number of results resolved. Expected #{parent_objects.length}, got #{resolved_objects.length}", field: exec_field)
149+
resolved_objects = Array.new(parent_objects.length, nil)
150150
end
151151

152152
if return_type.kind.composite?
153153
# build results with child selections
154-
next_sources = []
155-
next_responses = []
156-
resolved_sources.each_with_index do |source, i|
154+
next_objects = []
155+
next_results = []
156+
i = 0
157+
while i < resolved_objects.length
157158
# DANGER: HOT PATH!
158-
parent_responses[i][field_key] = build_composite_response(exec_field, field_type, source, next_sources, next_responses)
159+
object = resolved_objects[i]
160+
parent_results[i][field_key] = build_composite_result(exec_field, field_type, object, next_objects, next_results)
161+
i += 1
159162
end
160163

161164
if return_type.kind.abstract?
@@ -164,29 +167,33 @@ def resolve_execution_field(exec_field, resolved_sources)
164167
raise NotImplementedError, "No type resolver for `#{return_type.graphql_name}`"
165168
end
166169

167-
next_sources_by_type = Hash.new { |h, k| h[k] = [] }
168-
next_responses_by_type = Hash.new { |h, k| h[k] = [] }
169-
next_sources.each_with_index do |source, i|
170+
next_objects_by_type = Hash.new { |h, k| h[k] = [] }
171+
next_results_by_type = Hash.new { |h, k| h[k] = [] }
172+
173+
i = 0
174+
while i < next_objects.length
170175
# DANGER: HOT PATH!
171-
impl_type = type_resolver.call(source, @context)
172-
next_sources_by_type[impl_type] << (exec_field.name == TYPENAME_FIELD ? impl_type.graphql_name : source)
173-
next_responses_by_type[impl_type] << next_responses[i].tap { |r| r.typename = impl_type.graphql_name }
176+
object = next_objects[i]
177+
impl_type = type_resolver.call(object, @context)
178+
next_objects_by_type[impl_type] << (exec_field.name == TYPENAME_FIELD ? impl_type.graphql_name : object)
179+
next_results_by_type[impl_type] << next_results[i].tap { |r| r.typename = impl_type.graphql_name }
180+
i += 1
174181
end
175182

176183
loader_cache = {} # << all scopes in the abstract generation share a loader cache
177184
loader_group = []
178-
next_sources_by_type.each do |impl_type, impl_type_sources|
185+
next_objects_by_type.each do |impl_type, impl_type_objects|
179186
# check concrete type access only once per resolved type...
180187
unless Authorization.can_access_type?(impl_type, @context)
181188
@errors << AuthorizationError.new(type_name: impl_type.graphql_name, path: exec_field.path, base: true)
182-
impl_type_sources = Array.new(impl_type_sources.length, @errors.last)
189+
impl_type_objects = Array.new(impl_type_objects.length, @errors.last)
183190
end
184191

185192
loader_group << ExecutionScope.new(
186193
parent_type: impl_type,
187194
selections: exec_field.selections,
188-
sources: impl_type_sources,
189-
responses: next_responses_by_type[impl_type],
195+
objects: impl_type_objects,
196+
results: next_results_by_type[impl_type],
190197
loader_cache: loader_cache,
191198
loader_group: loader_group,
192199
path: exec_field.path,
@@ -199,17 +206,20 @@ def resolve_execution_field(exec_field, resolved_sources)
199206
@exec_queue << ExecutionScope.new(
200207
parent_type: return_type,
201208
selections: exec_field.selections,
202-
sources: next_sources,
203-
responses: next_responses,
209+
objects: next_objects,
210+
results: next_results,
204211
path: exec_field.path,
205212
parent: exec_field.scope,
206213
)
207214
end
208215
else
209216
# build leaf results
210-
resolved_sources.each_with_index do |val, i|
217+
i = 0
218+
while i < resolved_objects.length
211219
# DANGER: HOT PATH!
212-
parent_responses[i][field_key] = coerce_leaf_value(exec_field, field_type, val)
220+
val = resolved_objects[i]
221+
parent_results[i][field_key] = build_leaf_result(exec_field, field_type, val)
222+
i += 1
213223
end
214224
end
215225
end

lib/graphql/cardinal/executor/execution_scope.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,23 @@
33
module GraphQL::Cardinal
44
class Executor
55
class ExecutionScope
6-
attr_reader :parent_type, :selections, :sources, :responses, :path, :parent
6+
attr_reader :parent_type, :selections, :objects, :results, :path, :parent
77
attr_accessor :fields
88

99
def initialize(
1010
parent_type:,
1111
selections:,
12-
sources:,
13-
responses:,
12+
objects:,
13+
results:,
1414
loader_cache: nil,
1515
loader_group: nil,
1616
path: [],
1717
parent: nil
1818
)
1919
@parent_type = parent_type
2020
@selections = selections
21-
@sources = sources
22-
@responses = responses
21+
@objects = objects
22+
@results = results
2323
@loader_cache = loader_cache
2424
@loader_group = loader_group
2525
@path = path.freeze

lib/graphql/cardinal/executor/hot_paths.rb

Lines changed: 29 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,53 +3,33 @@
33
module GraphQL::Cardinal
44
class Executor
55
module HotPaths
6-
INCORRECT_LIST_VALUE = "Incorrect result for list field. Expected Array, got ".freeze
7-
8-
# DANGER: HOT PATH!
6+
# DANGER: HOT PATHS!
97
# Overhead added here scales dramatically...
10-
def build_composite_response(exec_field, current_type, source, next_sources, next_responses)
11-
# if object authorization check implemented, then...
12-
# unless Authorization.can_access_object?(return_type, source, @context)
138

14-
if source.nil? || source.is_a?(ExecutionError)
15-
build_missing_value(exec_field, current_type, source)
9+
INCORRECT_LIST_VALUE = "Incorrect result for list field. Expected Array, got ".freeze
10+
11+
def build_composite_result(exec_field, current_type, object, next_objects, next_results)
12+
if object.nil? || object.is_a?(ExecutionError)
13+
build_missing_value(exec_field, current_type, object)
1614
elsif current_type.list?
17-
unless source.is_a?(Array)
18-
report_exception("#{INCORRECT_LIST_VALUE}#{source.class}", field: exec_field)
15+
unless object.is_a?(Array)
16+
report_exception("#{INCORRECT_LIST_VALUE}#{object.class}", field: exec_field)
1917
return build_missing_value(exec_field, current_type, nil)
2018
end
2119

2220
current_type = current_type.of_type while current_type.non_null?
2321

24-
source.map do |src|
25-
build_composite_response(exec_field, current_type.of_type, src, next_sources, next_responses)
22+
object.map do |src|
23+
build_composite_result(exec_field, current_type.of_type, src, next_objects, next_results)
2624
end
2725
else
28-
next_sources << source
29-
next_responses << ResponseHash.new
30-
next_responses.last
31-
end
32-
end
33-
34-
# DANGER: HOT PATH!
35-
# Overhead added here scales dramatically...
36-
def build_missing_value(exec_field, current_type, val)
37-
# the provided value should always be nil or an error object
38-
if current_type.non_null?
39-
val ||= InvalidNullError.new(path: exec_field.path)
40-
end
41-
42-
if val
43-
val.replace_path(exec_field.path) unless val.path
44-
@errors << val unless val.base_error?
26+
next_objects << object
27+
next_results << ResultHash.new
28+
next_results.last
4529
end
46-
47-
val
4830
end
4931

50-
# DANGER: HOT PATH!
51-
# Overhead added here scales dramatically...
52-
def coerce_leaf_value(exec_field, current_type, val)
32+
def build_leaf_result(exec_field, current_type, val)
5333
if val.nil? || val.is_a?(StandardError)
5434
build_missing_value(exec_field, current_type, val)
5535
elsif current_type.list?
@@ -60,7 +40,7 @@ def coerce_leaf_value(exec_field, current_type, val)
6040

6141
current_type = current_type.of_type while current_type.non_null?
6242

63-
val.map { coerce_leaf_value(exec_field, current_type.of_type, _1) }
43+
val.map { build_leaf_result(exec_field, current_type.of_type, _1) }
6444
else
6545
begin
6646
current_type.unwrap.coerce_result(val, @context)
@@ -71,6 +51,20 @@ def coerce_leaf_value(exec_field, current_type, val)
7151
end
7252
end
7353
end
54+
55+
def build_missing_value(exec_field, current_type, val)
56+
# the provided value should always be nil or an error object
57+
if current_type.non_null?
58+
val ||= InvalidNullError.new(path: exec_field.path)
59+
end
60+
61+
if val
62+
val.replace_path(exec_field.path) unless val.path
63+
@errors << val unless val.base_error?
64+
end
65+
66+
val
67+
end
7468
end
7569
end
7670
end

lib/graphql/cardinal/executor/response_hash.rb renamed to lib/graphql/cardinal/executor/result_hash.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
module GraphQL::Cardinal
44
class Executor
5-
class ResponseHash < Hash
5+
class ResultHash < Hash
66
attr_accessor :typename
77
end
88
end

lib/graphql/cardinal/field_resolvers.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ def resolve(objects, _args, _ctx, _scope)
1111
raise NotImplementedError, "Resolver#resolve must be implemented."
1212
end
1313

14-
def map_sources(objects)
14+
def map_objects(objects)
1515
objects.map do |obj|
1616
yield(obj)
1717
rescue StandardError => e
@@ -30,7 +30,7 @@ def initialize(key)
3030
end
3131

3232
def resolve(objects, _args, _ctx, _scope)
33-
map_sources(objects) do |hash|
33+
map_objects(objects) do |hash|
3434
hash[@key]
3535
end
3636
end
@@ -42,7 +42,7 @@ def initialize(name)
4242
end
4343

4444
def resolve(objects, _args, _ctx, _scope)
45-
map_sources(objects) do |obj|
45+
map_objects(objects) do |obj|
4646
obj.public_send(@name)
4747
end
4848
end
@@ -51,7 +51,7 @@ def resolve(objects, _args, _ctx, _scope)
5151
class TypenameResolver < FieldResolver
5252
def resolve(objects, _args, _ctx, scope)
5353
typename = scope.parent_type.graphql_name.freeze
54-
map_sources(objects) { typename }
54+
map_objects(objects) { typename }
5555
end
5656
end
5757
end

0 commit comments

Comments
 (0)