From 148e91633cbe8a2c941d2bae1c7a038fd79533f8 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 7 Nov 2016 23:47:58 -0800 Subject: [PATCH 01/13] feat(Execution::Boxed) put off resultion until the last minute --- lib/graphql/execution.rb | 1 + lib/graphql/execution/boxed.rb | 81 +++++++++++ lib/graphql/query.rb | 4 + lib/graphql/query/context.rb | 18 ++- lib/graphql/query/serial_execution.rb | 5 +- .../serial_execution/field_resolution.rb | 52 ++++--- .../serial_execution/value_resolution.rb | 9 +- lib/graphql/schema.rb | 4 +- spec/graphql/execution/boxed_spec.rb | 134 ++++++++++++++++++ 9 files changed, 285 insertions(+), 23 deletions(-) create mode 100644 lib/graphql/execution/boxed.rb create mode 100644 spec/graphql/execution/boxed_spec.rb diff --git a/lib/graphql/execution.rb b/lib/graphql/execution.rb index c409a72235..66db49c535 100644 --- a/lib/graphql/execution.rb +++ b/lib/graphql/execution.rb @@ -1,2 +1,3 @@ +require "graphql/execution/boxed" require "graphql/execution/directive_checks" require "graphql/execution/typecast" diff --git a/lib/graphql/execution/boxed.rb b/lib/graphql/execution/boxed.rb new file mode 100644 index 0000000000..dde03758a2 --- /dev/null +++ b/lib/graphql/execution/boxed.rb @@ -0,0 +1,81 @@ +module GraphQL + module Execution + class Boxed + def self.unbox(val) + b = Unbox.unbox_in_place(val) + Unbox.deep_sync(b) + end + + def initialize(boxed_value = nil, &get_value_func) + if boxed_value + @get_value_func = ->{ boxed_value.value } + else + @get_value_func = get_value_func + end + @resolved = false + end + + def value + if !@resolved + @resolved = true + @value = @get_value_func.call + end + @value + end + + def then(&block) + @then = self.class.new { + next_val = block.call(value) + } + end + end + + module Unbox + def self.unbox_in_place(value) + boxes = [] + + each_box(value) do |obj, key, value| + inner_b = value.then do |inner_v| + obj[key] = inner_v + unbox_in_place(inner_v) + end + boxes.push(inner_b) + end + + Boxed.new { boxes.map(&:value) } + end + + def self.each_box(value, &block) + case value + when Hash + value.each do |k, v| + if v.is_a?(Boxed) + yield(value, k, v) + else + each_box(v, &block) + end + end + when Array + value.each_with_index do |v, i| + if v.is_a?(Boxed) + yield(value, i, v) + else + each_box(v, &block) + end + end + end + end + + def self.deep_sync(val) + case val + when Boxed + deep_sync(val.value) + when Array + val.each { |v| deep_sync(v) } + when Hash + val.each { |k, v| deep_sync(v) } + end + end + end + end +end diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index 99a39d2041..33152cc50d 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -192,6 +192,10 @@ def resolve_type(type) @schema.resolve_type(type, @context) end + def boxed?(value) + @schema.boxes[value.class] + end + private # Assert that the passed-in query string is internally consistent diff --git a/lib/graphql/query/context.rb b/lib/graphql/query/context.rb index d7ed9922f7..3666902bc5 100644 --- a/lib/graphql/query/context.rb +++ b/lib/graphql/query/context.rb @@ -59,19 +59,29 @@ def []=(key, value) @values[key] = value end - def spawn(path:, irep_node:) - FieldResolutionContext.new(context: self, path: path, irep_node: irep_node) + def spawn(path:, irep_node:, parent_type:, field:, irep_nodes:) + FieldResolutionContext.new( + context: self, + path: path, + irep_node: irep_node, + parent_type: parent_type, + field: field, + irep_nodes: irep_nodes, + ) end class FieldResolutionContext extend Forwardable - attr_reader :path, :irep_node + attr_reader :path, :irep_node, :field, :parent_type, :irep_nodes - def initialize(context:, path:, irep_node:) + def initialize(context:, path:, irep_node:, field:, parent_type:, irep_nodes:) @context = context @path = path @irep_node = irep_node + @field = field + @parent_type = parent_type + @irep_nodes = irep_nodes end def_delegators :@context, :[], :[]=, :spawn, :query, :schema, :warden, :errors, :execution_strategy, :strategy diff --git a/lib/graphql/query/serial_execution.rb b/lib/graphql/query/serial_execution.rb index 7db7980856..0aa3c95ee0 100644 --- a/lib/graphql/query/serial_execution.rb +++ b/lib/graphql/query/serial_execution.rb @@ -17,11 +17,14 @@ class SerialExecution def execute(ast_operation, root_type, query_object) irep_root = query_object.internal_representation[ast_operation.name] - operation_resolution.resolve( + result = operation_resolution.resolve( irep_root, root_type, query_object ) + + GraphQL::Execution::Boxed.unbox(result) + result end def field_resolution diff --git a/lib/graphql/query/serial_execution/field_resolution.rb b/lib/graphql/query/serial_execution/field_resolution.rb index e241816e27..54e4f40e69 100644 --- a/lib/graphql/query/serial_execution/field_resolution.rb +++ b/lib/graphql/query/serial_execution/field_resolution.rb @@ -9,9 +9,15 @@ def initialize(irep_nodes, parent_type, target, query_ctx) @irep_nodes = irep_nodes @parent_type = parent_type @target = target - @field_ctx = query_ctx.spawn(path: query_ctx.path + [irep_node.name], irep_node: irep_node) @query = query_ctx.query @field = @query.get_field(parent_type, irep_node.definition_name) + @field_ctx = query_ctx.spawn( + parent_type: parent_type, + field: field, + path: query_ctx.path + [irep_node.name], + irep_node: irep_node, + irep_nodes: irep_nodes, + ) @arguments = @query.arguments_for(irep_node, @field) end @@ -47,21 +53,35 @@ def get_finished_value(raw_value) end end - begin - GraphQL::Query::SerialExecution::ValueResolution.resolve( - parent_type, - field, - field.type, - raw_value, - @irep_nodes, - @field_ctx, - ) - rescue GraphQL::InvalidNullError => err - if field.type.kind.non_null? - raise(err) - else - err.parent_error? || @query.context.errors.push(err) - nil + box_method = @query.boxed?(raw_value) + if box_method + GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |val| + GraphQL::Query::SerialExecution::ValueResolution.resolve( + parent_type, + field, + field.type, + val, + @irep_nodes, + @field_ctx, + ) + } + else + begin + GraphQL::Query::SerialExecution::ValueResolution.resolve( + parent_type, + field, + field.type, + raw_value, + @irep_nodes, + @field_ctx, + ) + rescue GraphQL::InvalidNullError => err + if field.type.kind.non_null? + raise(err) + else + err.parent_error? || @query.context.errors.push(err) + nil + end end end end diff --git a/lib/graphql/query/serial_execution/value_resolution.rb b/lib/graphql/query/serial_execution/value_resolution.rb index 8925a48e38..1246174355 100644 --- a/lib/graphql/query/serial_execution/value_resolution.rb +++ b/lib/graphql/query/serial_execution/value_resolution.rb @@ -18,7 +18,14 @@ def self.resolve(parent_type, field_defn, field_type, value, irep_nodes, query_c when GraphQL::TypeKinds::LIST wrapped_type = field_type.of_type result = value.each_with_index.map do |inner_value, index| - inner_ctx = query_ctx.spawn(path: query_ctx.path + [index], irep_node: query_ctx.irep_node) + inner_ctx = query_ctx.spawn( + path: query_ctx.path + [index], + irep_node: query_ctx.irep_node, + irep_nodes: irep_nodes, + parent_type: parent_type, + field: field_defn, + ) + inner_result = resolve( parent_type, field_defn, diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index f819238c51..db9cd51a15 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -57,6 +57,7 @@ class Schema instrument: -> (schema, type, instrumenter) { schema.instrumenters[type] << instrumenter }, query_analyzer: ->(schema, analyzer) { schema.query_analyzers << analyzer }, middleware: ->(schema, middleware) { schema.middleware << middleware }, + boxed_value: ->(schema, box_class, boxed_method) { schema.boxes[box_class] = boxed_method }, rescue_from: ->(schema, err_class, &block) { schema.rescue_from(err_class, &block)} attr_accessor \ @@ -64,7 +65,7 @@ class Schema :query_execution_strategy, :mutation_execution_strategy, :subscription_execution_strategy, :max_depth, :max_complexity, :orphan_types, :directives, - :query_analyzers, :middleware, :instrumenters + :query_analyzers, :middleware, :instrumenters, :boxes BUILT_IN_TYPES = Hash[[INT_TYPE, STRING_TYPE, FLOAT_TYPE, BOOLEAN_TYPE, ID_TYPE].map{ |type| [type.name, type] }] DIRECTIVES = [GraphQL::Directive::IncludeDirective, GraphQL::Directive::SkipDirective, GraphQL::Directive::DeprecatedDirective] @@ -90,6 +91,7 @@ def initialize @object_from_id_proc = nil @id_from_object_proc = nil @instrumenters = Hash.new { |h, k| h[k] = [] } + @boxes = Hash.new { |h, k| h.find { |k2, v2| k < k2 ? h[k] = v2 : nil } } # Default to the built-in execution strategy: @query_execution_strategy = GraphQL::Query::SerialExecution @mutation_execution_strategy = GraphQL::Query::SerialExecution diff --git a/spec/graphql/execution/boxed_spec.rb b/spec/graphql/execution/boxed_spec.rb new file mode 100644 index 0000000000..b6068ec9ba --- /dev/null +++ b/spec/graphql/execution/boxed_spec.rb @@ -0,0 +1,134 @@ +require "spec_helper" + +describe GraphQL::Execution::Boxed do + class Box + def initialize(item) + @item = item + end + + def item + @item + end + end + + class SumAll + ALL = [] + attr_reader :own_value + attr_accessor :value + + def initialize(own_value) + @own_value = own_value + ALL << self + end + + def value + @value ||= begin + total_value = ALL.map(&:own_value).reduce(&:+) + ALL.each { |v| v.value = total_value} + ALL.clear + total_value + end + @value + end + end + + BoxedSum = GraphQL::ObjectType.define do + name "BoxedSum" + field :value, !types.Int do + resolve ->(o, a, c) { o } + end + field :nestedSum, BoxedSum do + argument :value, !types.Int + resolve ->(o, args, c) { SumAll.new(o + args[:value]) } + end + end + + BoxedQuery = GraphQL::ObjectType.define do + name "Query" + + field :int, !types.Int do + argument :value, !types.Int + argument :plus, types.Int, default_value: 0 + resolve ->(o, args, c) { Box.new(args[:value] + args[:plus]) } + end + + field :sum, !types.Int do + argument :value, !types.Int + resolve ->(o, args, c) { SumAll.new(args[:value]) } + end + + field :nestedSum, BoxedSum do + argument :value, !types.Int + resolve ->(o, args, c) { SumAll.new(args[:value]) } + end + + field :listSum, types[BoxedSum] do + argument :values, types[types.Int] + resolve ->(o, args, c) { args[:values] } + end + end + + BoxedSchema = GraphQL::Schema.define do + query(BoxedQuery) + boxed_value(Box, :item) + boxed_value(SumAll, :value) + end + + def run_query(query_str) + BoxedSchema.execute(query_str) + end + + describe "unboxing" do + it "calls value handlers" do + res = run_query('{ int(value: 2, plus: 1)}') + assert_equal 3, res["data"]["int"] + end + + it "can do out-of-bounds processing" do + res = run_query %| + { + a: sum(value: 2) + b: sum(value: 4) + c: sum(value: 6) + } + | + + assert_equal [12, 12, 12], res["data"].values + end + + it "can do nested boxed values" do + res = run_query %| + { + a: nestedSum(value: 3) { + value + nestedSum(value: 7) { + value + } + } + b: nestedSum(value: 2) { + value + nestedSum(value: 11) { + value + } + } + + c: listSum(values: [1,2]) { + nestedSum(value: 3) { + value + } + } + } + | + + expected_data = { + "a"=>{"value"=>14, "nestedSum"=>{"value"=>46}}, + "b"=>{"value"=>14, "nestedSum"=>{"value"=>46}}, + "c"=>[{"nestedSum"=>{"value"=>14}}, {"nestedSum"=>{"value"=>14}}], + } + + assert_equal expected_data, res["data"] + end + + it "propagates nulls" + end +end From e5ccc38d4acb0c7c00c602ffc4e9e7fce2b84a75 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 8 Nov 2016 13:34:36 -0800 Subject: [PATCH 02/13] feat(Boxed) resolve mutation fields immediately --- lib/graphql/execution/boxed.rb | 128 ++++++++++++------ lib/graphql/query.rb | 10 +- .../serial_execution/field_resolution.rb | 2 +- .../serial_execution/operation_resolution.rb | 5 +- .../serial_execution/selection_resolution.rb | 12 +- lib/graphql/schema.rb | 4 +- spec/graphql/execution/boxed_spec.rb | 30 ++++ 7 files changed, 139 insertions(+), 52 deletions(-) diff --git a/lib/graphql/execution/boxed.rb b/lib/graphql/execution/boxed.rb index dde03758a2..b2731c6baa 100644 --- a/lib/graphql/execution/boxed.rb +++ b/lib/graphql/execution/boxed.rb @@ -1,20 +1,29 @@ module GraphQL module Execution + # This wraps a value which is available, but not yet calculated, like a promise or future. + # + # Calling `#value` will trigger calculation & return the "boxed" value. + # + # This is an itty-bitty promise-like object, with key differences: + # - It has only two states, not-resolved and resolved + # - It has no error-catching functionality class Boxed + # Traverse `val`, lazily unboxing any values along the way + # @param val [Object] A data structure containing mixed unboxed values and `Boxed` instances + # @return void def self.unbox(val) b = Unbox.unbox_in_place(val) Unbox.deep_sync(b) end - def initialize(boxed_value = nil, &get_value_func) - if boxed_value - @get_value_func = ->{ boxed_value.value } - else - @get_value_func = get_value_func - end + # Create a `Boxed` which will get its inner value by calling the block + # @param get_value_func [Proc] a block to get the inner value (later) + def initialize(&get_value_func) + @get_value_func = get_value_func @resolved = false end + # @return [Object] The wrapped value, calling the lazy block if necessary def value if !@resolved @resolved = true @@ -23,57 +32,92 @@ def value @value end + # @return [Boxed] A Boxed whose value depends on another Boxed, plus any transformations in `block` def then(&block) - @then = self.class.new { + self.class.new { next_val = block.call(value) } end - end - module Unbox - def self.unbox_in_place(value) - boxes = [] + # Helpers for dealing with data structures containing {Boxed} instances + module Unbox + # Mutate `value`, replacing {Boxed} instances in place with their resolved values + # @return [void] + def self.unbox_in_place(value) + boxes = [] - each_box(value) do |obj, key, value| - inner_b = value.then do |inner_v| - obj[key] = inner_v - unbox_in_place(inner_v) + each_box(value) do |obj, key, value| + inner_b = value.then do |inner_v| + obj[key] = inner_v + unbox_in_place(inner_v) + end + boxes.push(inner_b) end - boxes.push(inner_b) - end - Boxed.new { boxes.map(&:value) } - end + Boxed.new { boxes.map(&:value) } + end - def self.each_box(value, &block) - case value - when Hash - value.each do |k, v| - if v.is_a?(Boxed) - yield(value, k, v) - else - each_box(v, &block) + # If `value` is a collection, call `block` + # with any {Boxed} instances in the collection + # @return [void] + def self.each_box(value, &block) + case value + when Hash + value.each do |k, v| + if v.is_a?(Boxed) + yield(value, k, v) + else + each_box(v, &block) + end end - end - when Array - value.each_with_index do |v, i| - if v.is_a?(Boxed) - yield(value, i, v) - else - each_box(v, &block) + when Array + value.each_with_index do |v, i| + if v.is_a?(Boxed) + yield(value, i, v) + else + each_box(v, &block) + end end end end + + # Traverse `val`, triggering resolution for each {Boxed}. + # These {Boxed}s are expected to mutate their owner data structures + # during resolution! (They're created with the `.then` calls in `unbox_in_place`). + # @return [void] + def self.deep_sync(val) + case val + when Boxed + deep_sync(val.value) + when Array + val.each { |v| deep_sync(v) } + when Hash + val.each { |k, v| deep_sync(v) } + end + end end - def self.deep_sync(val) - case val - when Boxed - deep_sync(val.value) - when Array - val.each { |v| deep_sync(v) } - when Hash - val.each { |k, v| deep_sync(v) } + class BoxMethodMap + def initialize + @storage = {} + end + + # @param boxed_class [Class] A class which represents a boxed value (subclasses may also be used) + # @param boxed_value_method [Symbol] The method to call on this class to get its value + def set(boxed_class, boxed_value_method) + @storage[boxed_class] = boxed_value_method + end + + # @param value [Object] an object which may have a `boxed_value_method` registered for its class or superclasses + # @return [Symbol, nil] The `boxed_value_method` for this object, or nil + def get(value) + if @storage.key?(value.class) + @storage[value.class] + else + value_class = value.class + registered_superclass = @storage.each_key.find { |boxed_class| value_class < boxed_class } + @storage[value_class] = @storage[registered_superclass] + end end end end diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index 33152cc50d..5292032838 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -82,12 +82,14 @@ def initialize(schema, query_string = nil, document: nil, context: nil, variable # Trying to execute a document # with no operations returns an empty hash @ast_variables = [] + @mutation = false if @operations.any? @selected_operation = find_operation(@operations, @operation_name) if @selected_operation.nil? @validation_errors << GraphQL::Query::OperationNameMissingError.new(@operations.keys) else @ast_variables = @selected_operation.variables + @mutation = @selected_operation.operation_type == "mutation" end end end @@ -192,8 +194,12 @@ def resolve_type(type) @schema.resolve_type(type, @context) end - def boxed?(value) - @schema.boxes[value.class] + def boxed_value_method(value) + @schema.boxes.get(value) + end + + def mutation? + @mutation end private diff --git a/lib/graphql/query/serial_execution/field_resolution.rb b/lib/graphql/query/serial_execution/field_resolution.rb index 54e4f40e69..f8d8b46181 100644 --- a/lib/graphql/query/serial_execution/field_resolution.rb +++ b/lib/graphql/query/serial_execution/field_resolution.rb @@ -53,7 +53,7 @@ def get_finished_value(raw_value) end end - box_method = @query.boxed?(raw_value) + box_method = @query.boxed_value_method(raw_value) if box_method GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |val| GraphQL::Query::SerialExecution::ValueResolution.resolve( diff --git a/lib/graphql/query/serial_execution/operation_resolution.rb b/lib/graphql/query/serial_execution/operation_resolution.rb index 1ec468ebb4..f3dcb2f1c0 100644 --- a/lib/graphql/query/serial_execution/operation_resolution.rb +++ b/lib/graphql/query/serial_execution/operation_resolution.rb @@ -2,12 +2,13 @@ module GraphQL class Query class SerialExecution module OperationResolution - def self.resolve(irep_node, target, query) + def self.resolve(irep_node, current_type, query) result = query.context.execution_strategy.selection_resolution.resolve( query.root_value, - target, + current_type, [irep_node], query.context, + mutation: query.mutation? ) result diff --git a/lib/graphql/query/serial_execution/selection_resolution.rb b/lib/graphql/query/serial_execution/selection_resolution.rb index 2e09f13601..6cf6b00b2f 100644 --- a/lib/graphql/query/serial_execution/selection_resolution.rb +++ b/lib/graphql/query/serial_execution/selection_resolution.rb @@ -2,18 +2,24 @@ module GraphQL class Query class SerialExecution module SelectionResolution - def self.resolve(target, current_type, irep_nodes, query_ctx) + def self.resolve(target, current_type, irep_nodes, query_ctx, mutation: false ) + own_selections = query_ctx.query.selections(irep_nodes, current_type) selection_result = {} own_selections.each do |name, child_irep_nodes| - selection_result.merge!(query_ctx.execution_strategy.field_resolution.new( + field_result = query_ctx.execution_strategy.field_resolution.new( child_irep_nodes, current_type, target, query_ctx - ).result) + ).result + + if mutation + GraphQL::Execution::Boxed.unbox(field_result) + end + selection_result.merge!(field_result) end selection_result diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index db9cd51a15..9a9173645c 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -57,7 +57,7 @@ class Schema instrument: -> (schema, type, instrumenter) { schema.instrumenters[type] << instrumenter }, query_analyzer: ->(schema, analyzer) { schema.query_analyzers << analyzer }, middleware: ->(schema, middleware) { schema.middleware << middleware }, - boxed_value: ->(schema, box_class, boxed_method) { schema.boxes[box_class] = boxed_method }, + boxed_value: ->(schema, box_class, boxed_value_method) { schema.boxes.set(box_class, boxed_value_method) }, rescue_from: ->(schema, err_class, &block) { schema.rescue_from(err_class, &block)} attr_accessor \ @@ -91,7 +91,7 @@ def initialize @object_from_id_proc = nil @id_from_object_proc = nil @instrumenters = Hash.new { |h, k| h[k] = [] } - @boxes = Hash.new { |h, k| h.find { |k2, v2| k < k2 ? h[k] = v2 : nil } } + @boxes = GraphQL::Execution::Boxed::BoxMethodMap.new # Default to the built-in execution strategy: @query_execution_strategy = GraphQL::Query::SerialExecution @mutation_execution_strategy = GraphQL::Query::SerialExecution diff --git a/spec/graphql/execution/boxed_spec.rb b/spec/graphql/execution/boxed_spec.rb index b6068ec9ba..b67e85255f 100644 --- a/spec/graphql/execution/boxed_spec.rb +++ b/spec/graphql/execution/boxed_spec.rb @@ -70,6 +70,7 @@ def value BoxedSchema = GraphQL::Schema.define do query(BoxedQuery) + mutation(BoxedQuery) boxed_value(Box, :item) boxed_value(SumAll, :value) end @@ -130,5 +131,34 @@ def run_query(query_str) end it "propagates nulls" + + it "resolves mutation fields right away" do + res = run_query %| + mutation { + a: sum(value: 2) + b: sum(value: 4) + c: sum(value: 6) + } + | + + assert_equal [2, 4, 6], res["data"].values + end + end + + describe "BoxMethodMap" do + class SubBox < Box; end + + let(:map) { GraphQL::Execution::Boxed::BoxMethodMap.new } + + it "finds methods for classes and subclasses" do + map.set(Box, :item) + map.set(SumAll, :value) + b = Box.new(1) + sub_b = SubBox.new(2) + s = SumAll.new(3) + assert_equal(:item, map.get(b)) + assert_equal(:item, map.get(sub_b)) + assert_equal(:value, map.get(s)) + end end end From b7af097296e5de17ce0e646d09237ccab70a005d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 8 Nov 2016 13:47:50 -0800 Subject: [PATCH 03/13] fix(Boxed) use query-local batching --- spec/graphql/execution/boxed_spec.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/graphql/execution/boxed_spec.rb b/spec/graphql/execution/boxed_spec.rb index b67e85255f..d6cbd5ceaf 100644 --- a/spec/graphql/execution/boxed_spec.rb +++ b/spec/graphql/execution/boxed_spec.rb @@ -12,20 +12,20 @@ def item end class SumAll - ALL = [] attr_reader :own_value attr_accessor :value - def initialize(own_value) + def initialize(ctx, own_value) @own_value = own_value - ALL << self + @all = ctx[:__sum_all__] ||= [] + @all << self end def value @value ||= begin - total_value = ALL.map(&:own_value).reduce(&:+) - ALL.each { |v| v.value = total_value} - ALL.clear + total_value = @all.map(&:own_value).reduce(&:+) + @all.each { |v| v.value = total_value} + @all.clear total_value end @value @@ -39,7 +39,7 @@ def value end field :nestedSum, BoxedSum do argument :value, !types.Int - resolve ->(o, args, c) { SumAll.new(o + args[:value]) } + resolve ->(o, args, c) { SumAll.new(c, o + args[:value]) } end end @@ -54,12 +54,12 @@ def value field :sum, !types.Int do argument :value, !types.Int - resolve ->(o, args, c) { SumAll.new(args[:value]) } + resolve ->(o, args, c) { SumAll.new(c, args[:value]) } end field :nestedSum, BoxedSum do argument :value, !types.Int - resolve ->(o, args, c) { SumAll.new(args[:value]) } + resolve ->(o, args, c) { SumAll.new(c, args[:value]) } end field :listSum, types[BoxedSum] do @@ -155,7 +155,7 @@ class SubBox < Box; end map.set(SumAll, :value) b = Box.new(1) sub_b = SubBox.new(2) - s = SumAll.new(3) + s = SumAll.new({}, 3) assert_equal(:item, map.get(b)) assert_equal(:item, map.get(sub_b)) assert_equal(:value, map.get(s)) From a6fcbf16a7f6fb4bf87e0eb3a0ba4c1fabf76c19 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 8 Nov 2016 22:58:04 -0800 Subject: [PATCH 04/13] feat(Boxed) start on null propagation --- lib/graphql/query/serial_execution.rb | 2 - .../serial_execution/field_resolution.rb | 56 +++++----- .../serial_execution/operation_resolution.rb | 2 + spec/graphql/execution/boxed_spec.rb | 102 +++++++++++++----- 4 files changed, 105 insertions(+), 57 deletions(-) diff --git a/lib/graphql/query/serial_execution.rb b/lib/graphql/query/serial_execution.rb index 0aa3c95ee0..7b42e6429e 100644 --- a/lib/graphql/query/serial_execution.rb +++ b/lib/graphql/query/serial_execution.rb @@ -22,8 +22,6 @@ def execute(ast_operation, root_type, query_object) root_type, query_object ) - - GraphQL::Execution::Boxed.unbox(result) result end diff --git a/lib/graphql/query/serial_execution/field_resolution.rb b/lib/graphql/query/serial_execution/field_resolution.rb index f8d8b46181..d5e1a99161 100644 --- a/lib/graphql/query/serial_execution/field_resolution.rb +++ b/lib/graphql/query/serial_execution/field_resolution.rb @@ -37,6 +37,17 @@ def execution_context # After getting the value from the field's resolve method, # continue by "finishing" the value, eg. executing sub-fields or coercing values def get_finished_value(raw_value) + box_method = @query.boxed_value_method(raw_value) + if box_method + GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |inner_value| + get_finished_value_without_box(inner_value) + } + else + get_finished_value_without_box(raw_value) + end + end + + def get_finished_value_without_box(raw_value) case raw_value when GraphQL::ExecutionError raw_value.ast_node = irep_node.ast_node @@ -53,35 +64,22 @@ def get_finished_value(raw_value) end end - box_method = @query.boxed_value_method(raw_value) - if box_method - GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |val| - GraphQL::Query::SerialExecution::ValueResolution.resolve( - parent_type, - field, - field.type, - val, - @irep_nodes, - @field_ctx, - ) - } - else - begin - GraphQL::Query::SerialExecution::ValueResolution.resolve( - parent_type, - field, - field.type, - raw_value, - @irep_nodes, - @field_ctx, - ) - rescue GraphQL::InvalidNullError => err - if field.type.kind.non_null? - raise(err) - else - err.parent_error? || @query.context.errors.push(err) - nil - end + + begin + GraphQL::Query::SerialExecution::ValueResolution.resolve( + parent_type, + field, + field.type, + raw_value, + @irep_nodes, + @field_ctx, + ) + rescue GraphQL::InvalidNullError => err + if field.type.kind.non_null? + raise(err) + else + err.parent_error? || @query.context.errors.push(err) + nil end end end diff --git a/lib/graphql/query/serial_execution/operation_resolution.rb b/lib/graphql/query/serial_execution/operation_resolution.rb index f3dcb2f1c0..f309126eba 100644 --- a/lib/graphql/query/serial_execution/operation_resolution.rb +++ b/lib/graphql/query/serial_execution/operation_resolution.rb @@ -11,6 +11,8 @@ def self.resolve(irep_node, current_type, query) mutation: query.mutation? ) + GraphQL::Execution::Boxed.unbox(result) + result rescue GraphQL::InvalidNullError => err err.parent_error? || query.context.errors.push(err) diff --git a/spec/graphql/execution/boxed_spec.rb b/spec/graphql/execution/boxed_spec.rb index d6cbd5ceaf..c8ee30114b 100644 --- a/spec/graphql/execution/boxed_spec.rb +++ b/spec/graphql/execution/boxed_spec.rb @@ -34,31 +34,47 @@ def value BoxedSum = GraphQL::ObjectType.define do name "BoxedSum" - field :value, !types.Int do - resolve ->(o, a, c) { o } + field :value, types.Int do + resolve ->(o, a, c) { o == 13 ? nil : o } end - field :nestedSum, BoxedSum do + field :nestedSum, !BoxedSum do argument :value, !types.Int - resolve ->(o, args, c) { SumAll.new(c, o + args[:value]) } + resolve ->(o, args, c) { + if args[:value] == 13 + Box.new(nil) + else + SumAll.new(c, o + args[:value]) + end + } + end + + field :nullableNestedSum, BoxedSum do + argument :value, types.Int + resolve ->(o, args, c) { + if args[:value] == 13 + Box.new(nil) + else + SumAll.new(c, o + args[:value]) + end + } end end BoxedQuery = GraphQL::ObjectType.define do name "Query" - field :int, !types.Int do argument :value, !types.Int argument :plus, types.Int, default_value: 0 - resolve ->(o, args, c) { Box.new(args[:value] + args[:plus]) } + resolve ->(o, a, c) { Box.new(a[:value] + a[:plus])} end - field :sum, !types.Int do + field :nestedSum, !BoxedSum do argument :value, !types.Int resolve ->(o, args, c) { SumAll.new(c, args[:value]) } end - field :nestedSum, BoxedSum do - argument :value, !types.Int + field :nullableNestedSum, BoxedSum do + argument :value, types.Int resolve ->(o, args, c) { SumAll.new(c, args[:value]) } end @@ -85,18 +101,6 @@ def run_query(query_str) assert_equal 3, res["data"]["int"] end - it "can do out-of-bounds processing" do - res = run_query %| - { - a: sum(value: 2) - b: sum(value: 4) - c: sum(value: 6) - } - | - - assert_equal [12, 12, 12], res["data"].values - end - it "can do nested boxed values" do res = run_query %| { @@ -130,18 +134,64 @@ def run_query(query_str) assert_equal expected_data, res["data"] end - it "propagates nulls" + it "propagates nulls" do + res = run_query %| + { + nestedSum(value: 1) { + value + nestedSum(value: 13) { + value + } + } + }| + + assert_equal(nil, res["data"]) + assert_equal 1, res["errors"].length + + + res = run_query %| + { + nullableNestedSum(value: 1) { + value + nullableNestedSum(value: 2) { + nestedSum(value: 13) { + value + } + } + } + }| + + pp res + expected_data = { + "nullableNestedSum" => { + "value" => 1, + "nullableNestedSum" => nil, + } + } + assert_equal(expected_data, res["data"]) + assert_equal 1, res["errors"].length + + end it "resolves mutation fields right away" do + res = run_query %| + { + a: nestedSum(value: 2) { value } + b: nestedSum(value: 4) { value } + c: nestedSum(value: 6) { value } + }| + + assert_equal [12, 12, 12], res["data"].values.map { |d| d["value"] } + res = run_query %| mutation { - a: sum(value: 2) - b: sum(value: 4) - c: sum(value: 6) + a: nestedSum(value: 2) { value } + b: nestedSum(value: 4) { value } + c: nestedSum(value: 6) { value } } | - assert_equal [2, 4, 6], res["data"].values + assert_equal [2, 4, 6], res["data"].values.map { |d| d["value"] } end end From a03d81a8d4ef427db927829a61b7b28f71216d99 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 8 Nov 2016 23:32:51 -0800 Subject: [PATCH 05/13] refactor(Execution) move Boxed support to Execution::Execute --- lib/graphql.rb | 2 +- lib/graphql/execution.rb | 1 + lib/graphql/execution/execute.rb | 226 ++++++++++++++++++ lib/graphql/query/serial_execution.rb | 3 +- .../serial_execution/field_resolution.rb | 20 +- .../serial_execution/operation_resolution.rb | 7 +- .../serial_execution/selection_resolution.rb | 12 +- .../serial_execution/value_resolution.rb | 9 +- 8 files changed, 236 insertions(+), 44 deletions(-) create mode 100644 lib/graphql/execution/execute.rb diff --git a/lib/graphql.rb b/lib/graphql.rb index ed42b2c6ac..7143f15016 100644 --- a/lib/graphql.rb +++ b/lib/graphql.rb @@ -67,6 +67,7 @@ def self.scan_with_ragel(query_string) require "graphql/introspection" require "graphql/language" require "graphql/analysis" +require "graphql/execution" require "graphql/schema" require "graphql/schema/loader" require "graphql/schema/printer" @@ -81,5 +82,4 @@ def self.scan_with_ragel(query_string) require "graphql/static_validation" require "graphql/version" require "graphql/relay" -require "graphql/execution" require "graphql/compatibility" diff --git a/lib/graphql/execution.rb b/lib/graphql/execution.rb index 66db49c535..c5aaaa77b3 100644 --- a/lib/graphql/execution.rb +++ b/lib/graphql/execution.rb @@ -1,3 +1,4 @@ require "graphql/execution/boxed" require "graphql/execution/directive_checks" +require "graphql/execution/execute" require "graphql/execution/typecast" diff --git a/lib/graphql/execution/execute.rb b/lib/graphql/execution/execute.rb new file mode 100644 index 0000000000..75f932d225 --- /dev/null +++ b/lib/graphql/execution/execute.rb @@ -0,0 +1,226 @@ +module GraphQL + module Execution + class Execute + def execute(ast_operation, root_type, query_object) + irep_root = query_object.internal_representation[ast_operation.name] + + result = resolve_operation( + irep_root, + root_type, + query_object + ) + result + end + + private + + def resolve_operation(irep_node, current_type, query) + result = resolve_selection( + query.root_value, + current_type, + [irep_node], + query.context, + mutation: query.mutation? + ) + + GraphQL::Execution::Boxed.unbox(result) + + result + rescue GraphQL::InvalidNullError => err + err.parent_error? || query.context.errors.push(err) + nil + end + + def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: false ) + + own_selections = query_ctx.query.selections(irep_nodes, current_type) + + selection_result = {} + + own_selections.each do |name, child_irep_nodes| + field_result = resolve_field( + child_irep_nodes, + current_type, + object, + query_ctx + ) + + if mutation + GraphQL::Execution::Boxed.unbox(field_result) + end + selection_result.merge!(field_result) + end + + selection_result + end + + def resolve_field(irep_nodes, parent_type, object, query_ctx) + irep_node = irep_nodes.first + query = query_ctx.query + field = query.get_field(parent_type, irep_node.definition_name) + field_ctx = query_ctx.spawn( + parent_type: parent_type, + field: field, + path: query_ctx.path + [irep_node.name], + irep_node: irep_node, + irep_nodes: irep_nodes, + ) + + arguments = query.arguments_for(irep_node, field) + middlewares = query.schema.middleware + resolve_arguments = [parent_type, object, field, arguments, field_ctx] + + raw_value = begin + # only run a middleware chain if there are any middleware + if middlewares.any? + chain = GraphQL::Schema::MiddlewareChain.new( + steps: middlewares + [FieldResolveStep], + arguments: resolve_arguments + ) + chain.call + else + FieldResolveStep.call(*resolve_arguments) + end + rescue GraphQL::ExecutionError => err + err + end + + result_name = irep_node.name + + box_method = query.boxed_value_method(raw_value) + result = if box_method + GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |inner_value| + continue_resolve_field(irep_nodes, parent_type, field, inner_value, field_ctx) + } + else + continue_resolve_field(irep_nodes, parent_type, field, raw_value, field_ctx) + end + + { irep_node.name => result } + end + + def continue_resolve_field(irep_nodes, parent_type, field, raw_value, field_ctx) + irep_node = irep_nodes.first + query = field_ctx.query + + case raw_value + when GraphQL::ExecutionError + raw_value.ast_node = irep_node.ast_node + raw_value.path = field_ctx.path + query.context.errors.push(raw_value) + when Array + list_errors = raw_value.each_with_index.select { |value, _| value.is_a?(GraphQL::ExecutionError) } + if list_errors.any? + list_errors.each do |error, index| + error.ast_node = irep_node.ast_node + error.path = field_ctx.path + [index] + query.context.errors.push(error) + end + end + end + + + begin + resolve_value( + parent_type, + field, + field.type, + raw_value, + irep_nodes, + field_ctx, + ) + rescue GraphQL::InvalidNullError => err + if field.type.kind.non_null? + raise(err) + else + err.parent_error? || query.context.errors.push(err) + nil + end + end + end + + def resolve_value(parent_type, field_defn, field_type, value, irep_nodes, query_ctx) + if value.nil? || value.is_a?(GraphQL::ExecutionError) + if field_type.kind.non_null? + raise GraphQL::InvalidNullError.new(parent_type.name, field_defn.name, value) + else + nil + end + else + case field_type.kind + when GraphQL::TypeKinds::SCALAR + field_type.coerce_result(value) + when GraphQL::TypeKinds::ENUM + field_type.coerce_result(value, query_ctx.query.warden) + when GraphQL::TypeKinds::LIST + wrapped_type = field_type.of_type + result = value.each_with_index.map do |inner_value, index| + inner_ctx = query_ctx.spawn( + path: query_ctx.path + [index], + irep_node: query_ctx.irep_node, + irep_nodes: irep_nodes, + parent_type: parent_type, + field: field_defn, + ) + + inner_result = resolve_value( + parent_type, + field_defn, + wrapped_type, + inner_value, + irep_nodes, + inner_ctx, + ) + inner_result + end + result + when GraphQL::TypeKinds::NON_NULL + wrapped_type = field_type.of_type + resolve_value( + parent_type, + field_defn, + wrapped_type, + value, + irep_nodes, + query_ctx, + ) + when GraphQL::TypeKinds::OBJECT + resolve_selection( + value, + field_type, + irep_nodes, + query_ctx + ) + when GraphQL::TypeKinds::UNION, GraphQL::TypeKinds::INTERFACE + query = query_ctx.query + resolved_type = query.resolve_type(value) + possible_types = query.possible_types(field_type) + + if !possible_types.include?(resolved_type) + raise GraphQL::UnresolvedTypeError.new(irep_nodes.first.definition_name, field_type, parent_type, resolved_type, possible_types) + else + resolve_value( + parent_type, + field_defn, + resolved_type, + value, + irep_nodes, + query_ctx, + ) + end + else + raise("Unknown type kind: #{field_type.kind}") + end + end + end + + # A `.call`-able suitable to be the last step in a middleware chain + module FieldResolveStep + # Execute the field's resolve method + def self.call(_parent_type, parent_object, field_definition, field_args, context, _next = nil) + field_definition.resolve(parent_object, field_args, context) + end + end + end + end +end diff --git a/lib/graphql/query/serial_execution.rb b/lib/graphql/query/serial_execution.rb index 7b42e6429e..7db7980856 100644 --- a/lib/graphql/query/serial_execution.rb +++ b/lib/graphql/query/serial_execution.rb @@ -17,12 +17,11 @@ class SerialExecution def execute(ast_operation, root_type, query_object) irep_root = query_object.internal_representation[ast_operation.name] - result = operation_resolution.resolve( + operation_resolution.resolve( irep_root, root_type, query_object ) - result end def field_resolution diff --git a/lib/graphql/query/serial_execution/field_resolution.rb b/lib/graphql/query/serial_execution/field_resolution.rb index d5e1a99161..e241816e27 100644 --- a/lib/graphql/query/serial_execution/field_resolution.rb +++ b/lib/graphql/query/serial_execution/field_resolution.rb @@ -9,15 +9,9 @@ def initialize(irep_nodes, parent_type, target, query_ctx) @irep_nodes = irep_nodes @parent_type = parent_type @target = target + @field_ctx = query_ctx.spawn(path: query_ctx.path + [irep_node.name], irep_node: irep_node) @query = query_ctx.query @field = @query.get_field(parent_type, irep_node.definition_name) - @field_ctx = query_ctx.spawn( - parent_type: parent_type, - field: field, - path: query_ctx.path + [irep_node.name], - irep_node: irep_node, - irep_nodes: irep_nodes, - ) @arguments = @query.arguments_for(irep_node, @field) end @@ -37,17 +31,6 @@ def execution_context # After getting the value from the field's resolve method, # continue by "finishing" the value, eg. executing sub-fields or coercing values def get_finished_value(raw_value) - box_method = @query.boxed_value_method(raw_value) - if box_method - GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |inner_value| - get_finished_value_without_box(inner_value) - } - else - get_finished_value_without_box(raw_value) - end - end - - def get_finished_value_without_box(raw_value) case raw_value when GraphQL::ExecutionError raw_value.ast_node = irep_node.ast_node @@ -64,7 +47,6 @@ def get_finished_value_without_box(raw_value) end end - begin GraphQL::Query::SerialExecution::ValueResolution.resolve( parent_type, diff --git a/lib/graphql/query/serial_execution/operation_resolution.rb b/lib/graphql/query/serial_execution/operation_resolution.rb index f309126eba..1ec468ebb4 100644 --- a/lib/graphql/query/serial_execution/operation_resolution.rb +++ b/lib/graphql/query/serial_execution/operation_resolution.rb @@ -2,17 +2,14 @@ module GraphQL class Query class SerialExecution module OperationResolution - def self.resolve(irep_node, current_type, query) + def self.resolve(irep_node, target, query) result = query.context.execution_strategy.selection_resolution.resolve( query.root_value, - current_type, + target, [irep_node], query.context, - mutation: query.mutation? ) - GraphQL::Execution::Boxed.unbox(result) - result rescue GraphQL::InvalidNullError => err err.parent_error? || query.context.errors.push(err) diff --git a/lib/graphql/query/serial_execution/selection_resolution.rb b/lib/graphql/query/serial_execution/selection_resolution.rb index 6cf6b00b2f..2e09f13601 100644 --- a/lib/graphql/query/serial_execution/selection_resolution.rb +++ b/lib/graphql/query/serial_execution/selection_resolution.rb @@ -2,24 +2,18 @@ module GraphQL class Query class SerialExecution module SelectionResolution - def self.resolve(target, current_type, irep_nodes, query_ctx, mutation: false ) - + def self.resolve(target, current_type, irep_nodes, query_ctx) own_selections = query_ctx.query.selections(irep_nodes, current_type) selection_result = {} own_selections.each do |name, child_irep_nodes| - field_result = query_ctx.execution_strategy.field_resolution.new( + selection_result.merge!(query_ctx.execution_strategy.field_resolution.new( child_irep_nodes, current_type, target, query_ctx - ).result - - if mutation - GraphQL::Execution::Boxed.unbox(field_result) - end - selection_result.merge!(field_result) + ).result) end selection_result diff --git a/lib/graphql/query/serial_execution/value_resolution.rb b/lib/graphql/query/serial_execution/value_resolution.rb index 1246174355..8925a48e38 100644 --- a/lib/graphql/query/serial_execution/value_resolution.rb +++ b/lib/graphql/query/serial_execution/value_resolution.rb @@ -18,14 +18,7 @@ def self.resolve(parent_type, field_defn, field_type, value, irep_nodes, query_c when GraphQL::TypeKinds::LIST wrapped_type = field_type.of_type result = value.each_with_index.map do |inner_value, index| - inner_ctx = query_ctx.spawn( - path: query_ctx.path + [index], - irep_node: query_ctx.irep_node, - irep_nodes: irep_nodes, - parent_type: parent_type, - field: field_defn, - ) - + inner_ctx = query_ctx.spawn(path: query_ctx.path + [index], irep_node: query_ctx.irep_node) inner_result = resolve( parent_type, field_defn, From 1eae2bb23358295327025b5e288a6f377f7f52fd Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 8 Nov 2016 23:33:06 -0800 Subject: [PATCH 06/13] feat(Schema) add .default_execution_strategy --- lib/graphql/schema.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 9a9173645c..d3ad02fb24 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -67,6 +67,12 @@ class Schema :orphan_types, :directives, :query_analyzers, :middleware, :instrumenters, :boxes + class << self + attr_accessor :default_execution_strategy + end + + self.default_execution_strategy = GraphQL::Execution::Execute + BUILT_IN_TYPES = Hash[[INT_TYPE, STRING_TYPE, FLOAT_TYPE, BOOLEAN_TYPE, ID_TYPE].map{ |type| [type.name, type] }] DIRECTIVES = [GraphQL::Directive::IncludeDirective, GraphQL::Directive::SkipDirective, GraphQL::Directive::DeprecatedDirective] DYNAMIC_FIELDS = ["__type", "__typename", "__schema"] @@ -93,9 +99,9 @@ def initialize @instrumenters = Hash.new { |h, k| h[k] = [] } @boxes = GraphQL::Execution::Boxed::BoxMethodMap.new # Default to the built-in execution strategy: - @query_execution_strategy = GraphQL::Query::SerialExecution - @mutation_execution_strategy = GraphQL::Query::SerialExecution - @subscription_execution_strategy = GraphQL::Query::SerialExecution + @query_execution_strategy = self.class.default_execution_strategy + @mutation_execution_strategy = self.class.default_execution_strategy + @subscription_execution_strategy = self.class.default_execution_strategy end def rescue_from(*args, &block) From fe04c502e1a96126c9d009f830c3c04a8f1665fb Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 8 Nov 2016 23:57:02 -0800 Subject: [PATCH 07/13] feat(Execution) add SelectionResult for storing type info --- lib/graphql/execution.rb | 1 + lib/graphql/execution/boxed.rb | 2 +- lib/graphql/execution/execute.rb | 39 ++++++------- lib/graphql/execution/selection_result.rb | 57 +++++++++++++++++++ .../serial_execution/field_resolution.rb | 8 ++- .../serial_execution/value_resolution.rb | 9 ++- 6 files changed, 91 insertions(+), 25 deletions(-) create mode 100644 lib/graphql/execution/selection_result.rb diff --git a/lib/graphql/execution.rb b/lib/graphql/execution.rb index c5aaaa77b3..e88cb5e351 100644 --- a/lib/graphql/execution.rb +++ b/lib/graphql/execution.rb @@ -1,4 +1,5 @@ require "graphql/execution/boxed" require "graphql/execution/directive_checks" require "graphql/execution/execute" +require "graphql/execution/selection_result" require "graphql/execution/typecast" diff --git a/lib/graphql/execution/boxed.rb b/lib/graphql/execution/boxed.rb index b2731c6baa..0653208b15 100644 --- a/lib/graphql/execution/boxed.rb +++ b/lib/graphql/execution/boxed.rb @@ -62,7 +62,7 @@ def self.unbox_in_place(value) # @return [void] def self.each_box(value, &block) case value - when Hash + when Hash, SelectionResult value.each do |k, v| if v.is_a?(Boxed) yield(value, k, v) diff --git a/lib/graphql/execution/execute.rb b/lib/graphql/execution/execute.rb index 75f932d225..4b05586f0e 100644 --- a/lib/graphql/execution/execute.rb +++ b/lib/graphql/execution/execute.rb @@ -1,46 +1,39 @@ module GraphQL module Execution class Execute - def execute(ast_operation, root_type, query_object) - irep_root = query_object.internal_representation[ast_operation.name] + def execute(ast_operation, root_type, query) + irep_root = query.internal_representation[ast_operation.name] - result = resolve_operation( - irep_root, - root_type, - query_object - ) - result - end - - private - - def resolve_operation(irep_node, current_type, query) result = resolve_selection( query.root_value, - current_type, - [irep_node], + root_type, + [irep_root], query.context, mutation: query.mutation? ) GraphQL::Execution::Boxed.unbox(result) - result + result.to_h rescue GraphQL::InvalidNullError => err err.parent_error? || query.context.errors.push(err) nil end - def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: false ) + private - own_selections = query_ctx.query.selections(irep_nodes, current_type) + def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: false ) + query = query_ctx.query + own_selections = query.selections(irep_nodes, current_type) - selection_result = {} + selection_result = SelectionResult.new own_selections.each do |name, child_irep_nodes| + field = query.get_field(current_type, child_irep_nodes.first.definition_name) field_result = resolve_field( child_irep_nodes, current_type, + field, object, query_ctx ) @@ -48,16 +41,18 @@ def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: fal if mutation GraphQL::Execution::Boxed.unbox(field_result) end - selection_result.merge!(field_result) + + field_result.each do |key, val| + selection_result.set(key, val, field.type) + end end selection_result end - def resolve_field(irep_nodes, parent_type, object, query_ctx) + def resolve_field(irep_nodes, parent_type, field, object, query_ctx) irep_node = irep_nodes.first query = query_ctx.query - field = query.get_field(parent_type, irep_node.definition_name) field_ctx = query_ctx.spawn( parent_type: parent_type, field: field, diff --git a/lib/graphql/execution/selection_result.rb b/lib/graphql/execution/selection_result.rb new file mode 100644 index 0000000000..f137bbc085 --- /dev/null +++ b/lib/graphql/execution/selection_result.rb @@ -0,0 +1,57 @@ +module GraphQL + module Execution + class SelectionResult + def initialize + @storage = {} + end + + def set(key, value, type) + @storage[key] = FieldResult.new(type: type, value: value) + end + + def [](key) + @storage.fetch(key).value + end + + def []=(key, value) + @storage.fetch(key).value = value + end + + def each + @storage.each do |key, field_res| + yield(key, field_res.value, field_res) + end + end + + def to_h + flatten(self) + end + + private + + def flatten(obj) + case obj + when SelectionResult + flattened = {} + obj.each do |key, val| + flattened[key] = flatten(val) + end + flattened + when Array + obj.map { |v| flatten(v) } + else + obj + end + end + + class FieldResult + attr_reader :type + attr_accessor :value + def initialize(type:, value:) + @type = type + @value = value + end + end + end + end +end diff --git a/lib/graphql/query/serial_execution/field_resolution.rb b/lib/graphql/query/serial_execution/field_resolution.rb index e241816e27..2c0b12d628 100644 --- a/lib/graphql/query/serial_execution/field_resolution.rb +++ b/lib/graphql/query/serial_execution/field_resolution.rb @@ -9,9 +9,15 @@ def initialize(irep_nodes, parent_type, target, query_ctx) @irep_nodes = irep_nodes @parent_type = parent_type @target = target - @field_ctx = query_ctx.spawn(path: query_ctx.path + [irep_node.name], irep_node: irep_node) @query = query_ctx.query @field = @query.get_field(parent_type, irep_node.definition_name) + @field_ctx = query_ctx.spawn( + path: query_ctx.path + [irep_node.name], + irep_node: irep_node, + parent_type: parent_type, + field: field, + irep_nodes: irep_nodes + ) @arguments = @query.arguments_for(irep_node, @field) end diff --git a/lib/graphql/query/serial_execution/value_resolution.rb b/lib/graphql/query/serial_execution/value_resolution.rb index 8925a48e38..7e547fd1a8 100644 --- a/lib/graphql/query/serial_execution/value_resolution.rb +++ b/lib/graphql/query/serial_execution/value_resolution.rb @@ -18,7 +18,14 @@ def self.resolve(parent_type, field_defn, field_type, value, irep_nodes, query_c when GraphQL::TypeKinds::LIST wrapped_type = field_type.of_type result = value.each_with_index.map do |inner_value, index| - inner_ctx = query_ctx.spawn(path: query_ctx.path + [index], irep_node: query_ctx.irep_node) + inner_ctx = query_ctx.spawn( + path: query_ctx.path + [index], + irep_node: query_ctx.irep_node, + parent_type: wrapped_type, + field: field_defn, + irep_nodes: irep_nodes, + ) + inner_result = resolve( parent_type, field_defn, From 4051d7b113cfc8609a721c9c1824029d10d184ae Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 10 Nov 2016 14:29:04 -0800 Subject: [PATCH 08/13] feat(GraphQL::Execution::Execute) propagate nulls; add path and location to invalid null errors --- .../compatibility/execution_specification.rb | 203 ++---------------- .../execution_specification/counter_schema.rb | 52 +++++ .../specification_schema.rb | 154 +++++++++++++ lib/graphql/execution.rb | 1 + lib/graphql/execution/boxed.rb | 84 +------- .../execution/boxed/boxed_method_map.rb | 29 +++ lib/graphql/execution/boxed/unbox.rb | 62 ++++++ lib/graphql/execution/execute.rb | 75 ++++--- lib/graphql/execution/field_result.rb | 38 ++++ lib/graphql/execution/selection_result.rb | 62 ++++-- lib/graphql/query/context.rb | 2 +- spec/graphql/execution/boxed_spec.rb | 1 - spec/graphql/execution/execute_spec.rb | 3 + spec/graphql/non_null_type_spec.rb | 12 +- spec/graphql/query/executor_spec.rb | 4 +- 15 files changed, 446 insertions(+), 336 deletions(-) create mode 100644 lib/graphql/compatibility/execution_specification/counter_schema.rb create mode 100644 lib/graphql/compatibility/execution_specification/specification_schema.rb create mode 100644 lib/graphql/execution/boxed/boxed_method_map.rb create mode 100644 lib/graphql/execution/boxed/unbox.rb create mode 100644 lib/graphql/execution/field_result.rb create mode 100644 spec/graphql/execution/execute_spec.rb diff --git a/lib/graphql/compatibility/execution_specification.rb b/lib/graphql/compatibility/execution_specification.rb index c8091551ef..61dd8fc321 100644 --- a/lib/graphql/compatibility/execution_specification.rb +++ b/lib/graphql/compatibility/execution_specification.rb @@ -1,3 +1,6 @@ +require "graphql/compatibility/execution_specification/counter_schema" +require "graphql/compatibility/execution_specification/specification_schema" + module GraphQL module Compatibility # Test an execution strategy. This spec is not meant as a development aid. @@ -22,164 +25,22 @@ module Compatibility # - Relay features # module ExecutionSpecification - DATA = { - "1001" => OpenStruct.new({ - name: "Fannie Lou Hamer", - birthdate: Time.new(1917, 10, 6), - organization_ids: [], - }), - "1002" => OpenStruct.new({ - name: "John Lewis", - birthdate: Time.new(1940, 2, 21), - organization_ids: ["2001"], - }), - "1003" => OpenStruct.new({ - name: "Diane Nash", - birthdate: Time.new(1938, 5, 15), - organization_ids: ["2001", "2002"], - }), - "1004" => OpenStruct.new({ - name: "Ralph Abernathy", - birthdate: Time.new(1926, 3, 11), - organization_ids: ["2002"], - }), - "2001" => OpenStruct.new({ - name: "SNCC", - leader_id: nil, # fail on purpose - }), - "2002" => OpenStruct.new({ - name: "SCLC", - leader_id: "1004", - }), - } - # Make a minitest suite for this execution strategy, making sure it # fulfills all the requirements of this library. # @param execution_strategy [<#new, #execute>] An execution strategy class # @return [Class] A test suite for this execution strategy def self.build_suite(execution_strategy) Class.new(Minitest::Test) do - def self.build_schema(execution_strategy) - organization_type = nil - - timestamp_type = GraphQL::ScalarType.define do - name "Timestamp" - coerce_input ->(value) { Time.at(value.to_i) } - coerce_result ->(value) { value.to_i } - end - - named_entity_interface_type = GraphQL::InterfaceType.define do - name "NamedEntity" - field :name, !types.String - end - - person_type = GraphQL::ObjectType.define do - name "Person" - interfaces [named_entity_interface_type] - field :name, !types.String - field :birthdate, timestamp_type - field :age, types.Int do - argument :on, !timestamp_type - resolve ->(obj, args, ctx) { - if obj.birthdate.nil? - nil - else - age_on = args[:on] - age_years = age_on.year - obj.birthdate.year - this_year_birthday = Time.new(age_on.year, obj.birthdate.month, obj.birthdate.day) - if this_year_birthday > age_on - age_years -= 1 - end - end - age_years - } - end - field :organizations, types[organization_type] do - resolve ->(obj, args, ctx) { - obj.organization_ids.map { |id| DATA[id] } - } - end - field :first_organization, !organization_type do - resolve ->(obj, args, ctx) { - DATA[obj.organization_ids.first] - } - end - end - - organization_type = GraphQL::ObjectType.define do - name "Organization" - interfaces [named_entity_interface_type] - field :name, !types.String - field :leader, !person_type do - resolve ->(obj, args, ctx) { - DATA[obj.leader_id] || (ctx[:return_error] ? ExecutionError.new("Error on Nullable") : nil) - } - end - field :returnedError, types.String do - resolve ->(o, a, c) { - GraphQL::ExecutionError.new("This error was returned") - } - end - field :raisedError, types.String do - resolve ->(o, a, c) { - raise GraphQL::ExecutionError.new("This error was raised") - } - end - - field :nodePresence, !types[!types.Boolean] do - resolve ->(o, a, ctx) { - [ - ctx.irep_node.is_a?(GraphQL::InternalRepresentation::Node), - ctx.ast_node.is_a?(GraphQL::Language::Nodes::AbstractNode), - false, # just testing - ] - } - end - end - - node_union_type = GraphQL::UnionType.define do - name "Node" - possible_types [person_type, organization_type] - end - - query_type = GraphQL::ObjectType.define do - name "Query" - field :node, node_union_type do - argument :id, !types.ID - resolve ->(obj, args, ctx) { - obj[args[:id]] - } - end - - field :organization, !organization_type do - argument :id, !types.ID - resolve ->(obj, args, ctx) { - args[:id].start_with?("2") && obj[args[:id]] - } - end - - field :organizations, types[organization_type] do - resolve ->(obj, args, ctx) { - [obj["2001"], obj["2002"]] - } - end - end - - GraphQL::Schema.define do - query_execution_strategy execution_strategy - query query_type - - resolve_type ->(obj, ctx) { - obj.respond_to?(:birthdate) ? person_type : organization_type - } - end + class << self + attr_accessor :counter_schema, :specification_schema end - @@schema = build_schema(execution_strategy) + self.specification_schema = SpecificationSchema.build(execution_strategy) + self.counter_schema = CounterSchema.build(execution_strategy) def execute_query(query_string, **kwargs) - kwargs[:root_value] = DATA - @@schema.execute(query_string, **kwargs) + kwargs[:root_value] = SpecificationSchema::DATA + self.class.specification_schema.execute(query_string, **kwargs) end def test_it_fetches_data @@ -409,47 +270,7 @@ def test_it_doesnt_add_errors_for_invalid_nulls_from_execution_errors end def test_it_only_resolves_fields_once_on_typed_fragments - count = 0 - counter_type = nil - - has_count_interface = GraphQL::InterfaceType.define do - name "HasCount" - field :count, types.Int - field :counter, ->{ counter_type } - end - - counter_type = GraphQL::ObjectType.define do - name "Counter" - interfaces [has_count_interface] - field :count, types.Int, resolve: ->(o,a,c) { count += 1 } - field :counter, has_count_interface, resolve: ->(o,a,c) { :counter } - end - - alt_counter_type = GraphQL::ObjectType.define do - name "AltCounter" - interfaces [has_count_interface] - field :count, types.Int, resolve: ->(o,a,c) { count += 1 } - field :counter, has_count_interface, resolve: ->(o,a,c) { :counter } - end - - has_counter_interface = GraphQL::InterfaceType.define do - name "HasCounter" - field :counter, counter_type - end - - query_type = GraphQL::ObjectType.define do - name "Query" - interfaces [has_counter_interface] - field :counter, has_count_interface, resolve: ->(o,a,c) { :counter } - end - - schema = GraphQL::Schema.define( - query: query_type, - resolve_type: ->(o, c) { o == :counter ? counter_type : nil }, - orphan_types: [alt_counter_type], - ) - - res = schema.execute(" + res = self.class.counter_schema.execute(" { counter { count } ... on HasCounter { @@ -462,10 +283,10 @@ def test_it_only_resolves_fields_once_on_typed_fragments "counter" => { "count" => 1 } } assert_equal expected_data, res["data"] - assert_equal 1, count + assert_equal 1, self.class.counter_schema.metadata[:count] # Deep typed children are correctly distinguished: - res = schema.execute(" + res = self.class.counter_schema.execute(" { counter { ... on Counter { diff --git a/lib/graphql/compatibility/execution_specification/counter_schema.rb b/lib/graphql/compatibility/execution_specification/counter_schema.rb new file mode 100644 index 0000000000..94f87fc79c --- /dev/null +++ b/lib/graphql/compatibility/execution_specification/counter_schema.rb @@ -0,0 +1,52 @@ +module GraphQL + module Compatibility + module ExecutionSpecification + module CounterSchema + def self.build(execution_strategy) + counter_type = nil + schema = nil + + has_count_interface = GraphQL::InterfaceType.define do + name "HasCount" + field :count, types.Int + field :counter, ->{ counter_type } + end + + counter_type = GraphQL::ObjectType.define do + name "Counter" + interfaces [has_count_interface] + field :count, types.Int, resolve: ->(o,a,c) { schema.metadata[:count] += 1 } + field :counter, has_count_interface, resolve: ->(o,a,c) { :counter } + end + + alt_counter_type = GraphQL::ObjectType.define do + name "AltCounter" + interfaces [has_count_interface] + field :count, types.Int, resolve: ->(o,a,c) { schema.metadata[:count] += 1 } + field :counter, has_count_interface, resolve: ->(o,a,c) { :counter } + end + + has_counter_interface = GraphQL::InterfaceType.define do + name "HasCounter" + field :counter, counter_type + end + + query_type = GraphQL::ObjectType.define do + name "Query" + interfaces [has_counter_interface] + field :counter, has_count_interface, resolve: ->(o,a,c) { :counter } + end + + schema = GraphQL::Schema.define( + query: query_type, + resolve_type: ->(o, c) { o == :counter ? counter_type : nil }, + orphan_types: [alt_counter_type], + query_execution_strategy: execution_strategy, + ) + schema.metadata[:count] = 0 + schema + end + end + end + end +end diff --git a/lib/graphql/compatibility/execution_specification/specification_schema.rb b/lib/graphql/compatibility/execution_specification/specification_schema.rb new file mode 100644 index 0000000000..aa01aa408d --- /dev/null +++ b/lib/graphql/compatibility/execution_specification/specification_schema.rb @@ -0,0 +1,154 @@ +module GraphQL + module Compatibility + module ExecutionSpecification + module SpecificationSchema + DATA = { + "1001" => OpenStruct.new({ + name: "Fannie Lou Hamer", + birthdate: Time.new(1917, 10, 6), + organization_ids: [], + }), + "1002" => OpenStruct.new({ + name: "John Lewis", + birthdate: Time.new(1940, 2, 21), + organization_ids: ["2001"], + }), + "1003" => OpenStruct.new({ + name: "Diane Nash", + birthdate: Time.new(1938, 5, 15), + organization_ids: ["2001", "2002"], + }), + "1004" => OpenStruct.new({ + name: "Ralph Abernathy", + birthdate: Time.new(1926, 3, 11), + organization_ids: ["2002"], + }), + "2001" => OpenStruct.new({ + name: "SNCC", + leader_id: nil, # fail on purpose + }), + "2002" => OpenStruct.new({ + name: "SCLC", + leader_id: "1004", + }), + } + + def self.build(execution_strategy) + organization_type = nil + + timestamp_type = GraphQL::ScalarType.define do + name "Timestamp" + coerce_input ->(value) { Time.at(value.to_i) } + coerce_result ->(value) { value.to_i } + end + + named_entity_interface_type = GraphQL::InterfaceType.define do + name "NamedEntity" + field :name, !types.String + end + + person_type = GraphQL::ObjectType.define do + name "Person" + interfaces [named_entity_interface_type] + field :name, !types.String + field :birthdate, timestamp_type + field :age, types.Int do + argument :on, !timestamp_type + resolve ->(obj, args, ctx) { + if obj.birthdate.nil? + nil + else + age_on = args[:on] + age_years = age_on.year - obj.birthdate.year + this_year_birthday = Time.new(age_on.year, obj.birthdate.month, obj.birthdate.day) + if this_year_birthday > age_on + age_years -= 1 + end + end + age_years + } + end + field :organizations, types[organization_type] do + resolve ->(obj, args, ctx) { + obj.organization_ids.map { |id| DATA[id] } + } + end + field :first_organization, !organization_type do + resolve ->(obj, args, ctx) { + DATA[obj.organization_ids.first] + } + end + end + + organization_type = GraphQL::ObjectType.define do + name "Organization" + interfaces [named_entity_interface_type] + field :name, !types.String + field :leader, !person_type do + resolve ->(obj, args, ctx) { + DATA[obj.leader_id] || (ctx[:return_error] ? ExecutionError.new("Error on Nullable") : nil) + } + end + field :returnedError, types.String do + resolve ->(o, a, c) { + GraphQL::ExecutionError.new("This error was returned") + } + end + field :raisedError, types.String do + resolve ->(o, a, c) { + raise GraphQL::ExecutionError.new("This error was raised") + } + end + + field :nodePresence, !types[!types.Boolean] do + resolve ->(o, a, ctx) { + [ + ctx.irep_node.is_a?(GraphQL::InternalRepresentation::Node), + ctx.ast_node.is_a?(GraphQL::Language::Nodes::AbstractNode), + false, # just testing + ] + } + end + end + + node_union_type = GraphQL::UnionType.define do + name "Node" + possible_types [person_type, organization_type] + end + + query_type = GraphQL::ObjectType.define do + name "Query" + field :node, node_union_type do + argument :id, !types.ID + resolve ->(obj, args, ctx) { + obj[args[:id]] + } + end + + field :organization, !organization_type do + argument :id, !types.ID + resolve ->(obj, args, ctx) { + args[:id].start_with?("2") && obj[args[:id]] + } + end + + field :organizations, types[organization_type] do + resolve ->(obj, args, ctx) { + [obj["2001"], obj["2002"]] + } + end + end + + GraphQL::Schema.define do + query_execution_strategy execution_strategy + query query_type + + resolve_type ->(obj, ctx) { + obj.respond_to?(:birthdate) ? person_type : organization_type + } + end + end + end + end + end +end diff --git a/lib/graphql/execution.rb b/lib/graphql/execution.rb index e88cb5e351..2b8d95af7d 100644 --- a/lib/graphql/execution.rb +++ b/lib/graphql/execution.rb @@ -1,5 +1,6 @@ require "graphql/execution/boxed" require "graphql/execution/directive_checks" require "graphql/execution/execute" +require "graphql/execution/field_result" require "graphql/execution/selection_result" require "graphql/execution/typecast" diff --git a/lib/graphql/execution/boxed.rb b/lib/graphql/execution/boxed.rb index 0653208b15..10e459487f 100644 --- a/lib/graphql/execution/boxed.rb +++ b/lib/graphql/execution/boxed.rb @@ -1,3 +1,5 @@ +require "graphql/execution/boxed/boxed_method_map" +require "graphql/execution/boxed/unbox" module GraphQL module Execution # This wraps a value which is available, but not yet calculated, like a promise or future. @@ -38,88 +40,6 @@ def then(&block) next_val = block.call(value) } end - - # Helpers for dealing with data structures containing {Boxed} instances - module Unbox - # Mutate `value`, replacing {Boxed} instances in place with their resolved values - # @return [void] - def self.unbox_in_place(value) - boxes = [] - - each_box(value) do |obj, key, value| - inner_b = value.then do |inner_v| - obj[key] = inner_v - unbox_in_place(inner_v) - end - boxes.push(inner_b) - end - - Boxed.new { boxes.map(&:value) } - end - - # If `value` is a collection, call `block` - # with any {Boxed} instances in the collection - # @return [void] - def self.each_box(value, &block) - case value - when Hash, SelectionResult - value.each do |k, v| - if v.is_a?(Boxed) - yield(value, k, v) - else - each_box(v, &block) - end - end - when Array - value.each_with_index do |v, i| - if v.is_a?(Boxed) - yield(value, i, v) - else - each_box(v, &block) - end - end - end - end - - # Traverse `val`, triggering resolution for each {Boxed}. - # These {Boxed}s are expected to mutate their owner data structures - # during resolution! (They're created with the `.then` calls in `unbox_in_place`). - # @return [void] - def self.deep_sync(val) - case val - when Boxed - deep_sync(val.value) - when Array - val.each { |v| deep_sync(v) } - when Hash - val.each { |k, v| deep_sync(v) } - end - end - end - - class BoxMethodMap - def initialize - @storage = {} - end - - # @param boxed_class [Class] A class which represents a boxed value (subclasses may also be used) - # @param boxed_value_method [Symbol] The method to call on this class to get its value - def set(boxed_class, boxed_value_method) - @storage[boxed_class] = boxed_value_method - end - - # @param value [Object] an object which may have a `boxed_value_method` registered for its class or superclasses - # @return [Symbol, nil] The `boxed_value_method` for this object, or nil - def get(value) - if @storage.key?(value.class) - @storage[value.class] - else - value_class = value.class - registered_superclass = @storage.each_key.find { |boxed_class| value_class < boxed_class } - @storage[value_class] = @storage[registered_superclass] - end - end - end end end end diff --git a/lib/graphql/execution/boxed/boxed_method_map.rb b/lib/graphql/execution/boxed/boxed_method_map.rb new file mode 100644 index 0000000000..8e8e38689a --- /dev/null +++ b/lib/graphql/execution/boxed/boxed_method_map.rb @@ -0,0 +1,29 @@ +module GraphQL + module Execution + class Boxed + class BoxMethodMap + def initialize + @storage = {} + end + + # @param boxed_class [Class] A class which represents a boxed value (subclasses may also be used) + # @param boxed_value_method [Symbol] The method to call on this class to get its value + def set(boxed_class, boxed_value_method) + @storage[boxed_class] = boxed_value_method + end + + # @param value [Object] an object which may have a `boxed_value_method` registered for its class or superclasses + # @return [Symbol, nil] The `boxed_value_method` for this object, or nil + def get(value) + if @storage.key?(value.class) + @storage[value.class] + else + value_class = value.class + registered_superclass = @storage.each_key.find { |boxed_class| value_class < boxed_class } + @storage[value_class] = @storage[registered_superclass] + end + end + end + end + end +end diff --git a/lib/graphql/execution/boxed/unbox.rb b/lib/graphql/execution/boxed/unbox.rb new file mode 100644 index 0000000000..fd328968ef --- /dev/null +++ b/lib/graphql/execution/boxed/unbox.rb @@ -0,0 +1,62 @@ +module GraphQL + module Execution + class Boxed + # Helpers for dealing with data structures containing {Boxed} instances + module Unbox + # Mutate `value`, replacing {Boxed} instances in place with their resolved values + # @return [void] + def self.unbox_in_place(value) + boxes = [] + + each_box(value) do |field_result| + inner_b = field_result.value.then do |inner_v| + field_result.value = inner_v + unbox_in_place(inner_v) + end + boxes.push(inner_b) + end + + Boxed.new { boxes.map(&:value) } + end + + # If `value` is a collection, call `block` + # with any {Boxed} instances in the collection + # @return [void] + def self.each_box(value, &block) + case value + when SelectionResult + value.each do |key, field_result| + each_box(field_result, &block) + end + when Array + value.each do |field_result| + each_box(field_result, &block) + end + when FieldResult + field_value = value.value + if field_value.is_a?(Boxed) + yield(value) + else + each_box(field_value, &block) + end + end + end + + # Traverse `val`, triggering resolution for each {Boxed}. + # These {Boxed}s are expected to mutate their owner data structures + # during resolution! (They're created with the `.then` calls in `unbox_in_place`). + # @return [void] + def self.deep_sync(val) + case val + when Boxed + deep_sync(val.value) + when Array + val.each { |v| deep_sync(v.value) } + when Hash + val.each { |k, v| deep_sync(v.value) } + end + end + end + end + end +end diff --git a/lib/graphql/execution/execute.rb b/lib/graphql/execution/execute.rb index 4b05586f0e..9f608bc343 100644 --- a/lib/graphql/execution/execute.rb +++ b/lib/graphql/execution/execute.rb @@ -1,6 +1,8 @@ module GraphQL module Execution class Execute + PROPAGATE_NULL = :__graphql_propagate_null__ + def execute(ast_operation, root_type, query) irep_root = query.internal_representation[ast_operation.name] @@ -15,9 +17,6 @@ def execute(ast_operation, root_type, query) GraphQL::Execution::Boxed.unbox(result) result.to_h - rescue GraphQL::InvalidNullError => err - err.parent_error? || query.context.errors.push(err) - nil end private @@ -26,11 +25,12 @@ def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: fal query = query_ctx.query own_selections = query.selections(irep_nodes, current_type) - selection_result = SelectionResult.new + selection_result = SelectionResult.new(type: current_type) own_selections.each do |name, child_irep_nodes| field = query.get_field(current_type, child_irep_nodes.first.definition_name) field_result = resolve_field( + selection_result, child_irep_nodes, current_type, field, @@ -42,15 +42,13 @@ def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: fal GraphQL::Execution::Boxed.unbox(field_result) end - field_result.each do |key, val| - selection_result.set(key, val, field.type) - end + selection_result.set(name, field_result) end selection_result end - def resolve_field(irep_nodes, parent_type, field, object, query_ctx) + def resolve_field(owner, irep_nodes, parent_type, field, object, query_ctx) irep_node = irep_nodes.first query = query_ctx.query field_ctx = query_ctx.spawn( @@ -80,8 +78,6 @@ def resolve_field(irep_nodes, parent_type, field, object, query_ctx) err end - result_name = irep_node.name - box_method = query.boxed_value_method(raw_value) result = if box_method GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |inner_value| @@ -91,7 +87,13 @@ def resolve_field(irep_nodes, parent_type, field, object, query_ctx) continue_resolve_field(irep_nodes, parent_type, field, raw_value, field_ctx) end - { irep_node.name => result } + FieldResult.new( + owner: owner, + parent_type: parent_type, + field: field, + name: irep_node.name, + value: result, + ) end def continue_resolve_field(irep_nodes, parent_type, field, raw_value, field_ctx) @@ -114,30 +116,27 @@ def continue_resolve_field(irep_nodes, parent_type, field, raw_value, field_ctx) end end + resolve_value( + parent_type, + field, + field.type, + raw_value, + irep_nodes, + field_ctx, + ) + end - begin - resolve_value( - parent_type, - field, - field.type, - raw_value, - irep_nodes, - field_ctx, - ) - rescue GraphQL::InvalidNullError => err - if field.type.kind.non_null? - raise(err) + def resolve_value(parent_type, field_defn, field_type, value, irep_nodes, field_ctx) + if value.nil? + if field_type.kind.non_null? + field_ctx.add_error(GraphQL::ExecutionError.new("Cannot return null for non-nullable field #{parent_type.name}.#{field_defn.name}")) + PROPAGATE_NULL else - err.parent_error? || query.context.errors.push(err) nil end - end - end - - def resolve_value(parent_type, field_defn, field_type, value, irep_nodes, query_ctx) - if value.nil? || value.is_a?(GraphQL::ExecutionError) + elsif value.is_a?(GraphQL::ExecutionError) if field_type.kind.non_null? - raise GraphQL::InvalidNullError.new(parent_type.name, field_defn.name, value) + PROPAGATE_NULL else nil end @@ -146,13 +145,13 @@ def resolve_value(parent_type, field_defn, field_type, value, irep_nodes, query_ when GraphQL::TypeKinds::SCALAR field_type.coerce_result(value) when GraphQL::TypeKinds::ENUM - field_type.coerce_result(value, query_ctx.query.warden) + field_type.coerce_result(value, field_ctx.query.warden) when GraphQL::TypeKinds::LIST wrapped_type = field_type.of_type result = value.each_with_index.map do |inner_value, index| - inner_ctx = query_ctx.spawn( - path: query_ctx.path + [index], - irep_node: query_ctx.irep_node, + inner_ctx = field_ctx.spawn( + path: field_ctx.path + [index], + irep_node: field_ctx.irep_node, irep_nodes: irep_nodes, parent_type: parent_type, field: field_defn, @@ -177,17 +176,17 @@ def resolve_value(parent_type, field_defn, field_type, value, irep_nodes, query_ wrapped_type, value, irep_nodes, - query_ctx, + field_ctx, ) when GraphQL::TypeKinds::OBJECT resolve_selection( value, field_type, irep_nodes, - query_ctx + field_ctx ) when GraphQL::TypeKinds::UNION, GraphQL::TypeKinds::INTERFACE - query = query_ctx.query + query = field_ctx.query resolved_type = query.resolve_type(value) possible_types = query.possible_types(field_type) @@ -200,7 +199,7 @@ def resolve_value(parent_type, field_defn, field_type, value, irep_nodes, query_ resolved_type, value, irep_nodes, - query_ctx, + field_ctx, ) end else diff --git a/lib/graphql/execution/field_result.rb b/lib/graphql/execution/field_result.rb new file mode 100644 index 0000000000..f0fd7327bc --- /dev/null +++ b/lib/graphql/execution/field_result.rb @@ -0,0 +1,38 @@ +module GraphQL + module Execution + class FieldResult + attr_reader :value, :parent_type, :field, :name, :owner + def initialize(parent_type:, field:, value:, name:, owner:) + @parent_type = parent_type + @field = field + @owner = owner + @name = name + self.value = value + end + + def value=(new_value) + if new_value.is_a?(SelectionResult) + if new_value.invalid_null? + new_value = new_value.invalid_null + else + new_value.owner = self + end + end + + if new_value == GraphQL::Execution::Execute::PROPAGATE_NULL + if field.type.kind.non_null? + @owner.propagate_null(@name, new_value) + else + @value = nil + end + else + @value = new_value + end + end + + def inspect + "# #{value.inspect} (#{field.type})>" + end + end + end +end diff --git a/lib/graphql/execution/selection_result.rb b/lib/graphql/execution/selection_result.rb index f137bbc085..864b25532f 100644 --- a/lib/graphql/execution/selection_result.rb +++ b/lib/graphql/execution/selection_result.rb @@ -1,30 +1,59 @@ module GraphQL module Execution class SelectionResult - def initialize + def initialize(type:) + @type = type @storage = {} + @owner = nil + @invalid_null = false end - def set(key, value, type) - @storage[key] = FieldResult.new(type: type, value: value) + def set(key, field_result) + @storage[key] = field_result end - def [](key) - @storage.fetch(key).value - end - - def []=(key, value) - @storage.fetch(key).value = value + def fetch(key) + @storage.fetch(key) end def each @storage.each do |key, field_res| - yield(key, field_res.value, field_res) + yield(key, field_res) end end def to_h - flatten(self) + if @invalid_null + nil + else + flatten(self) + end + end + + def propagate_null(key, value) + if @owner + @owner.value = value + end + @invalid_null = value + end + + def invalid_null? + @invalid_null + end + + def invalid_null + @invalid_null + end + + def owner=(field_result) + if @owner + raise("Can't change owners of SelectionResult") + else + @owner = field_result + if @invalid_null + @owner.value = @invalid_null + end + end end private @@ -39,19 +68,12 @@ def flatten(obj) flattened when Array obj.map { |v| flatten(v) } + when FieldResult + flatten(obj.value) else obj end end - - class FieldResult - attr_reader :type - attr_accessor :value - def initialize(type:, value:) - @type = type - @value = value - end - end end end end diff --git a/lib/graphql/query/context.rb b/lib/graphql/query/context.rb index 3666902bc5..78bd968956 100644 --- a/lib/graphql/query/context.rb +++ b/lib/graphql/query/context.rb @@ -95,7 +95,7 @@ def ast_node # @param error [GraphQL::ExecutionError] an execution error # @return [void] def add_error(error) - unless error.is_a?(ExecutionError) + if !error.is_a?(ExecutionError) raise TypeError, "expected error to be a ExecutionError, but was #{error.class}" end diff --git a/spec/graphql/execution/boxed_spec.rb b/spec/graphql/execution/boxed_spec.rb index c8ee30114b..3f00594721 100644 --- a/spec/graphql/execution/boxed_spec.rb +++ b/spec/graphql/execution/boxed_spec.rb @@ -161,7 +161,6 @@ def run_query(query_str) } }| - pp res expected_data = { "nullableNestedSum" => { "value" => 1, diff --git a/spec/graphql/execution/execute_spec.rb b/spec/graphql/execution/execute_spec.rb new file mode 100644 index 0000000000..43f13cc55b --- /dev/null +++ b/spec/graphql/execution/execute_spec.rb @@ -0,0 +1,3 @@ +require "spec_helper" + +ExecuteSuite = GraphQL::Compatibility::ExecutionSpecification.build_suite(GraphQL::Execution::Execute) diff --git a/spec/graphql/non_null_type_spec.rb b/spec/graphql/non_null_type_spec.rb index 6da5d1fa69..edd9c547aa 100644 --- a/spec/graphql/non_null_type_spec.rb +++ b/spec/graphql/non_null_type_spec.rb @@ -6,7 +6,11 @@ query_string = %|{ cow { name cantBeNullButIs } }| result = DummySchema.execute(query_string) assert_equal({"cow" => nil }, result["data"]) - assert_equal([{"message"=>"Cannot return null for non-nullable field Cow.cantBeNullButIs"}], result["errors"]) + assert_equal([{ + "message"=>"Cannot return null for non-nullable field Cow.cantBeNullButIs", + "locations"=>[{"line"=>1, "column"=>14}], + "path"=>["cow", "cantBeNullButIs"], + }], result["errors"]) end it "propagates the null up to the next nullable field" do @@ -25,7 +29,11 @@ | result = DummySchema.execute(query_string) assert_equal(nil, result["data"]) - assert_equal([{"message"=>"Cannot return null for non-nullable field DeepNonNull.nonNullInt"}], result["errors"]) + assert_equal([{ + "message"=>"Cannot return null for non-nullable field DeepNonNull.nonNullInt", + "locations"=>[{"line"=>8, "column"=>15}], + "path"=>["nn1", "nn2", "nn3", "nni3"], + }], result["errors"]) end end end diff --git a/spec/graphql/query/executor_spec.rb b/spec/graphql/query/executor_spec.rb index adf74ec452..a802ce705a 100644 --- a/spec/graphql/query/executor_spec.rb +++ b/spec/graphql/query/executor_spec.rb @@ -127,7 +127,9 @@ "data" => { "cow" => nil }, "errors" => [ { - "message" => "Cannot return null for non-nullable field Cow.cantBeNullButIs" + "message" => "Cannot return null for non-nullable field Cow.cantBeNullButIs", + "locations"=>[{"line"=>1, "column"=>28}], + "path"=>["cow", "cantBeNullButIs"], } ] } From 2f81c3084878f3b0ac43eccbe2c803bda8a7eac0 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 10 Nov 2016 14:46:46 -0800 Subject: [PATCH 09/13] refactor(Boxed) rename Boxed => Lazy --- lib/graphql/execution.rb | 2 +- .../execution/boxed/boxed_method_map.rb | 29 -------- lib/graphql/execution/boxed/unbox.rb | 62 ----------------- lib/graphql/execution/execute.rb | 10 +-- lib/graphql/execution/{boxed.rb => lazy.rb} | 21 +++--- lib/graphql/execution/lazy/lazy_method_map.rb | 29 ++++++++ lib/graphql/execution/lazy/resolve.rb | 67 +++++++++++++++++++ lib/graphql/query.rb | 4 +- lib/graphql/schema.rb | 6 +- .../execution/{boxed_spec.rb => lazy_spec.rb} | 54 +++++++-------- 10 files changed, 144 insertions(+), 140 deletions(-) delete mode 100644 lib/graphql/execution/boxed/boxed_method_map.rb delete mode 100644 lib/graphql/execution/boxed/unbox.rb rename lib/graphql/execution/{boxed.rb => lazy.rb} (58%) create mode 100644 lib/graphql/execution/lazy/lazy_method_map.rb create mode 100644 lib/graphql/execution/lazy/resolve.rb rename spec/graphql/execution/{boxed_spec.rb => lazy_spec.rb} (80%) diff --git a/lib/graphql/execution.rb b/lib/graphql/execution.rb index 2b8d95af7d..8a7157c2cf 100644 --- a/lib/graphql/execution.rb +++ b/lib/graphql/execution.rb @@ -1,6 +1,6 @@ -require "graphql/execution/boxed" require "graphql/execution/directive_checks" require "graphql/execution/execute" require "graphql/execution/field_result" +require "graphql/execution/lazy" require "graphql/execution/selection_result" require "graphql/execution/typecast" diff --git a/lib/graphql/execution/boxed/boxed_method_map.rb b/lib/graphql/execution/boxed/boxed_method_map.rb deleted file mode 100644 index 8e8e38689a..0000000000 --- a/lib/graphql/execution/boxed/boxed_method_map.rb +++ /dev/null @@ -1,29 +0,0 @@ -module GraphQL - module Execution - class Boxed - class BoxMethodMap - def initialize - @storage = {} - end - - # @param boxed_class [Class] A class which represents a boxed value (subclasses may also be used) - # @param boxed_value_method [Symbol] The method to call on this class to get its value - def set(boxed_class, boxed_value_method) - @storage[boxed_class] = boxed_value_method - end - - # @param value [Object] an object which may have a `boxed_value_method` registered for its class or superclasses - # @return [Symbol, nil] The `boxed_value_method` for this object, or nil - def get(value) - if @storage.key?(value.class) - @storage[value.class] - else - value_class = value.class - registered_superclass = @storage.each_key.find { |boxed_class| value_class < boxed_class } - @storage[value_class] = @storage[registered_superclass] - end - end - end - end - end -end diff --git a/lib/graphql/execution/boxed/unbox.rb b/lib/graphql/execution/boxed/unbox.rb deleted file mode 100644 index fd328968ef..0000000000 --- a/lib/graphql/execution/boxed/unbox.rb +++ /dev/null @@ -1,62 +0,0 @@ -module GraphQL - module Execution - class Boxed - # Helpers for dealing with data structures containing {Boxed} instances - module Unbox - # Mutate `value`, replacing {Boxed} instances in place with their resolved values - # @return [void] - def self.unbox_in_place(value) - boxes = [] - - each_box(value) do |field_result| - inner_b = field_result.value.then do |inner_v| - field_result.value = inner_v - unbox_in_place(inner_v) - end - boxes.push(inner_b) - end - - Boxed.new { boxes.map(&:value) } - end - - # If `value` is a collection, call `block` - # with any {Boxed} instances in the collection - # @return [void] - def self.each_box(value, &block) - case value - when SelectionResult - value.each do |key, field_result| - each_box(field_result, &block) - end - when Array - value.each do |field_result| - each_box(field_result, &block) - end - when FieldResult - field_value = value.value - if field_value.is_a?(Boxed) - yield(value) - else - each_box(field_value, &block) - end - end - end - - # Traverse `val`, triggering resolution for each {Boxed}. - # These {Boxed}s are expected to mutate their owner data structures - # during resolution! (They're created with the `.then` calls in `unbox_in_place`). - # @return [void] - def self.deep_sync(val) - case val - when Boxed - deep_sync(val.value) - when Array - val.each { |v| deep_sync(v.value) } - when Hash - val.each { |k, v| deep_sync(v.value) } - end - end - end - end - end -end diff --git a/lib/graphql/execution/execute.rb b/lib/graphql/execution/execute.rb index 9f608bc343..7396a197b5 100644 --- a/lib/graphql/execution/execute.rb +++ b/lib/graphql/execution/execute.rb @@ -14,7 +14,7 @@ def execute(ast_operation, root_type, query) mutation: query.mutation? ) - GraphQL::Execution::Boxed.unbox(result) + GraphQL::Execution::Lazy.resolve(result) result.to_h end @@ -39,7 +39,7 @@ def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: fal ) if mutation - GraphQL::Execution::Boxed.unbox(field_result) + GraphQL::Execution::Lazy.resolve(field_result) end selection_result.set(name, field_result) @@ -78,9 +78,9 @@ def resolve_field(owner, irep_nodes, parent_type, field, object, query_ctx) err end - box_method = query.boxed_value_method(raw_value) - result = if box_method - GraphQL::Execution::Boxed.new { raw_value.public_send(box_method) }.then { |inner_value| + lazy_method_name = query.lazy_method(raw_value) + result = if lazy_method_name + GraphQL::Execution::Lazy.new { raw_value.public_send(lazy_method_name) }.then { |inner_value| continue_resolve_field(irep_nodes, parent_type, field, inner_value, field_ctx) } else diff --git a/lib/graphql/execution/boxed.rb b/lib/graphql/execution/lazy.rb similarity index 58% rename from lib/graphql/execution/boxed.rb rename to lib/graphql/execution/lazy.rb index 10e459487f..0812c20eec 100644 --- a/lib/graphql/execution/boxed.rb +++ b/lib/graphql/execution/lazy.rb @@ -1,24 +1,23 @@ -require "graphql/execution/boxed/boxed_method_map" -require "graphql/execution/boxed/unbox" +require "graphql/execution/lazy/lazy_method_map" +require "graphql/execution/lazy/resolve" module GraphQL module Execution # This wraps a value which is available, but not yet calculated, like a promise or future. # - # Calling `#value` will trigger calculation & return the "boxed" value. + # Calling `#value` will trigger calculation & return the "lazy" value. # # This is an itty-bitty promise-like object, with key differences: # - It has only two states, not-resolved and resolved # - It has no error-catching functionality - class Boxed - # Traverse `val`, lazily unboxing any values along the way - # @param val [Object] A data structure containing mixed unboxed values and `Boxed` instances + class Lazy + # Traverse `val`, lazily resolving any values along the way + # @param val [Object] A data structure containing mixed plain values and `Lazy` instances # @return void - def self.unbox(val) - b = Unbox.unbox_in_place(val) - Unbox.deep_sync(b) + def self.resolve(val) + Resolve.resolve(val) end - # Create a `Boxed` which will get its inner value by calling the block + # Create a {Lazy} which will get its inner value by calling the block # @param get_value_func [Proc] a block to get the inner value (later) def initialize(&get_value_func) @get_value_func = get_value_func @@ -34,7 +33,7 @@ def value @value end - # @return [Boxed] A Boxed whose value depends on another Boxed, plus any transformations in `block` + # @return [Lazy] A {Lazy} whose value depends on another {Lazy}, plus any transformations in `block` def then(&block) self.class.new { next_val = block.call(value) diff --git a/lib/graphql/execution/lazy/lazy_method_map.rb b/lib/graphql/execution/lazy/lazy_method_map.rb new file mode 100644 index 0000000000..3b3ae6c604 --- /dev/null +++ b/lib/graphql/execution/lazy/lazy_method_map.rb @@ -0,0 +1,29 @@ +module GraphQL + module Execution + class Lazy + class LazyMethodMap + def initialize + @storage = {} + end + + # @param lazy_class [Class] A class which represents a lazy value (subclasses may also be used) + # @param lazy_value_method [Symbol] The method to call on this class to get its value + def set(lazy_class, lazy_value_method) + @storage[lazy_class] = lazy_value_method + end + + # @param value [Object] an object which may have a `lazy_value_method` registered for its class or superclasses + # @return [Symbol, nil] The `lazy_value_method` for this object, or nil + def get(value) + if @storage.key?(value.class) + @storage[value.class] + else + value_class = value.class + registered_superclass = @storage.each_key.find { |lazy_class| value_class < lazy_class } + @storage[value_class] = @storage[registered_superclass] + end + end + end + end + end +end diff --git a/lib/graphql/execution/lazy/resolve.rb b/lib/graphql/execution/lazy/resolve.rb new file mode 100644 index 0000000000..c1e114d61e --- /dev/null +++ b/lib/graphql/execution/lazy/resolve.rb @@ -0,0 +1,67 @@ +module GraphQL + module Execution + class Lazy + # Helpers for dealing with data structures containing {Lazy} instances + module Resolve + def self.resolve(value) + lazies = resolve_in_place(value) + deep_sync(lazies) + end + + # Mutate `value`, replacing {Lazy} instances in place with their resolved values + # @return [void] + def self.resolve_in_place(value) + lazies = [] + + each_lazy(value) do |field_result| + inner_lazy = field_result.value.then do |inner_v| + field_result.value = inner_v + resolve_in_place(inner_v) + end + lazies.push(inner_lazy) + end + + Lazy.new { lazies.map(&:value) } + end + + # If `value` is a collection, call `block` + # with any {Lazy} instances in the collection + # @return [void] + def self.each_lazy(value, &block) + case value + when SelectionResult + value.each do |key, field_result| + each_lazy(field_result, &block) + end + when Array + value.each do |field_result| + each_lazy(field_result, &block) + end + when FieldResult + field_value = value.value + if field_value.is_a?(Lazy) + yield(value) + else + each_lazy(field_value, &block) + end + end + end + + # Traverse `val`, triggering resolution for each {Lazy}. + # These {Lazy}s are expected to mutate their owner data structures + # during resolution! (They're created with the `.then` calls in `resolve_in_place`). + # @return [void] + def self.deep_sync(val) + case val + when Lazy + deep_sync(val.value) + when Array + val.each { |v| deep_sync(v.value) } + when Hash + val.each { |k, v| deep_sync(v.value) } + end + end + end + end + end +end diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index 5292032838..8c91badc0c 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -194,8 +194,8 @@ def resolve_type(type) @schema.resolve_type(type, @context) end - def boxed_value_method(value) - @schema.boxes.get(value) + def lazy_method(value) + @schema.lazy_methods.get(value) end def mutation? diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index d3ad02fb24..92998b075c 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -57,7 +57,7 @@ class Schema instrument: -> (schema, type, instrumenter) { schema.instrumenters[type] << instrumenter }, query_analyzer: ->(schema, analyzer) { schema.query_analyzers << analyzer }, middleware: ->(schema, middleware) { schema.middleware << middleware }, - boxed_value: ->(schema, box_class, boxed_value_method) { schema.boxes.set(box_class, boxed_value_method) }, + lazy_resolve: ->(schema, lazy_class, lazy_value_method) { schema.lazy_methods.set(lazy_class, lazy_value_method) }, rescue_from: ->(schema, err_class, &block) { schema.rescue_from(err_class, &block)} attr_accessor \ @@ -65,7 +65,7 @@ class Schema :query_execution_strategy, :mutation_execution_strategy, :subscription_execution_strategy, :max_depth, :max_complexity, :orphan_types, :directives, - :query_analyzers, :middleware, :instrumenters, :boxes + :query_analyzers, :middleware, :instrumenters, :lazy_methods class << self attr_accessor :default_execution_strategy @@ -97,7 +97,7 @@ def initialize @object_from_id_proc = nil @id_from_object_proc = nil @instrumenters = Hash.new { |h, k| h[k] = [] } - @boxes = GraphQL::Execution::Boxed::BoxMethodMap.new + @lazy_methods = GraphQL::Execution::Lazy::LazyMethodMap.new # Default to the built-in execution strategy: @query_execution_strategy = self.class.default_execution_strategy @mutation_execution_strategy = self.class.default_execution_strategy diff --git a/spec/graphql/execution/boxed_spec.rb b/spec/graphql/execution/lazy_spec.rb similarity index 80% rename from spec/graphql/execution/boxed_spec.rb rename to spec/graphql/execution/lazy_spec.rb index 3f00594721..b4ff0d22c4 100644 --- a/spec/graphql/execution/boxed_spec.rb +++ b/spec/graphql/execution/lazy_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" -describe GraphQL::Execution::Boxed do - class Box +describe GraphQL::Execution::Lazy do + class Wrapper def initialize(item) @item = item end @@ -32,27 +32,27 @@ def value end end - BoxedSum = GraphQL::ObjectType.define do - name "BoxedSum" + LazySum = GraphQL::ObjectType.define do + name "LazySum" field :value, types.Int do resolve ->(o, a, c) { o == 13 ? nil : o } end - field :nestedSum, !BoxedSum do + field :nestedSum, !LazySum do argument :value, !types.Int resolve ->(o, args, c) { if args[:value] == 13 - Box.new(nil) + Wrapper.new(nil) else SumAll.new(c, o + args[:value]) end } end - field :nullableNestedSum, BoxedSum do + field :nullableNestedSum, LazySum do argument :value, types.Int resolve ->(o, args, c) { if args[:value] == 13 - Box.new(nil) + Wrapper.new(nil) else SumAll.new(c, o + args[:value]) end @@ -60,48 +60,48 @@ def value end end - BoxedQuery = GraphQL::ObjectType.define do + LazyQuery = GraphQL::ObjectType.define do name "Query" field :int, !types.Int do argument :value, !types.Int argument :plus, types.Int, default_value: 0 - resolve ->(o, a, c) { Box.new(a[:value] + a[:plus])} + resolve ->(o, a, c) { Wrapper.new(a[:value] + a[:plus])} end - field :nestedSum, !BoxedSum do + field :nestedSum, !LazySum do argument :value, !types.Int resolve ->(o, args, c) { SumAll.new(c, args[:value]) } end - field :nullableNestedSum, BoxedSum do + field :nullableNestedSum, LazySum do argument :value, types.Int resolve ->(o, args, c) { SumAll.new(c, args[:value]) } end - field :listSum, types[BoxedSum] do + field :listSum, types[LazySum] do argument :values, types[types.Int] resolve ->(o, args, c) { args[:values] } end end - BoxedSchema = GraphQL::Schema.define do - query(BoxedQuery) - mutation(BoxedQuery) - boxed_value(Box, :item) - boxed_value(SumAll, :value) + LazySchema = GraphQL::Schema.define do + query(LazyQuery) + mutation(LazyQuery) + lazy_resolve(Wrapper, :item) + lazy_resolve(SumAll, :value) end def run_query(query_str) - BoxedSchema.execute(query_str) + LazySchema.execute(query_str) end - describe "unboxing" do + describe "resolving" do it "calls value handlers" do res = run_query('{ int(value: 2, plus: 1)}') assert_equal 3, res["data"]["int"] end - it "can do nested boxed values" do + it "can do nested lazy values" do res = run_query %| { a: nestedSum(value: 3) { @@ -194,16 +194,16 @@ def run_query(query_str) end end - describe "BoxMethodMap" do - class SubBox < Box; end + describe "LazyMethodMap" do + class SubWrapper < Wrapper; end - let(:map) { GraphQL::Execution::Boxed::BoxMethodMap.new } + let(:map) { GraphQL::Execution::Lazy::LazyMethodMap.new } it "finds methods for classes and subclasses" do - map.set(Box, :item) + map.set(Wrapper, :item) map.set(SumAll, :value) - b = Box.new(1) - sub_b = SubBox.new(2) + b = Wrapper.new(1) + sub_b = Wrapper.new(2) s = SumAll.new({}, 3) assert_equal(:item, map.get(b)) assert_equal(:item, map.get(sub_b)) From 36db37d43ab7dc6ab95ab86a228fb8ac86b13160 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 10 Nov 2016 14:58:50 -0800 Subject: [PATCH 10/13] feat(Lazy) if a GraphQL::ExecutionError is raised during lazy resolution, rescue it --- lib/graphql/execution/lazy.rb | 3 +++ spec/graphql/execution/lazy_spec.rb | 42 ++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/lib/graphql/execution/lazy.rb b/lib/graphql/execution/lazy.rb index 0812c20eec..6b7323aee0 100644 --- a/lib/graphql/execution/lazy.rb +++ b/lib/graphql/execution/lazy.rb @@ -31,6 +31,9 @@ def value @value = @get_value_func.call end @value + rescue GraphQL::ExecutionError => err + @resolved = true + @value = err end # @return [Lazy] A {Lazy} whose value depends on another {Lazy}, plus any transformations in `block` diff --git a/spec/graphql/execution/lazy_spec.rb b/spec/graphql/execution/lazy_spec.rb index b4ff0d22c4..337d1b400f 100644 --- a/spec/graphql/execution/lazy_spec.rb +++ b/spec/graphql/execution/lazy_spec.rb @@ -2,11 +2,19 @@ describe GraphQL::Execution::Lazy do class Wrapper - def initialize(item) - @item = item + def initialize(item = nil, &block) + if block + @block = block + else + @item = item + end end def item + if @block + @item = @block.call() + @block = nil + end @item end end @@ -75,7 +83,13 @@ def value field :nullableNestedSum, LazySum do argument :value, types.Int - resolve ->(o, args, c) { SumAll.new(c, args[:value]) } + resolve ->(o, args, c) { + if args[:value] == 13 + Wrapper.new { raise GraphQL::ExecutionError.new("13 is unlucky") } + else + SumAll.new(c, args[:value]) + end + } end field :listSum, types[LazySum] do @@ -169,7 +183,29 @@ def run_query(query_str) } assert_equal(expected_data, res["data"]) assert_equal 1, res["errors"].length + end + + it "handles raised errors" do + res = run_query %| + { + a: nullableNestedSum(value: 1) { value } + b: nullableNestedSum(value: 13) { value } + c: nullableNestedSum(value: 2) { value } + }| + + expected_data = { + "a" => { "value" => 3 }, + "b" => nil, + "c" => { "value" => 3 }, + } + assert_equal expected_data, res["data"] + expected_errors = [{ + "message"=>"13 is unlucky", + "locations"=>[{"line"=>4, "column"=>9}], + "path"=>["b"], + }] + assert_equal expected_errors, res["errors"] end it "resolves mutation fields right away" do From f050d95a3e300df175be933ad12421e7443d6b27 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 10 Nov 2016 15:13:20 -0800 Subject: [PATCH 11/13] doc(Lazy) add yardoc for Lazy code --- lib/graphql/execution/execute.rb | 5 ++-- lib/graphql/execution/field_result.rb | 27 +++++++++++++----- lib/graphql/execution/lazy/lazy_method_map.rb | 3 ++ lib/graphql/execution/lazy/resolve.rb | 4 +-- lib/graphql/execution/selection_result.rb | 28 +++++++++++-------- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/lib/graphql/execution/execute.rb b/lib/graphql/execution/execute.rb index 7396a197b5..be6db118f5 100644 --- a/lib/graphql/execution/execute.rb +++ b/lib/graphql/execution/execute.rb @@ -1,5 +1,6 @@ module GraphQL module Execution + # A valid execution strategy class Execute PROPAGATE_NULL = :__graphql_propagate_null__ @@ -25,7 +26,7 @@ def resolve_selection(object, current_type, irep_nodes, query_ctx, mutation: fal query = query_ctx.query own_selections = query.selections(irep_nodes, current_type) - selection_result = SelectionResult.new(type: current_type) + selection_result = SelectionResult.new own_selections.each do |name, child_irep_nodes| field = query.get_field(current_type, child_irep_nodes.first.definition_name) @@ -89,9 +90,7 @@ def resolve_field(owner, irep_nodes, parent_type, field, object, query_ctx) FieldResult.new( owner: owner, - parent_type: parent_type, field: field, - name: irep_node.name, value: result, ) end diff --git a/lib/graphql/execution/field_result.rb b/lib/graphql/execution/field_result.rb index f0fd7327bc..c49ba9f414 100644 --- a/lib/graphql/execution/field_result.rb +++ b/lib/graphql/execution/field_result.rb @@ -1,19 +1,32 @@ module GraphQL module Execution + # This is one key-value pair in a GraphQL response. class FieldResult - attr_reader :value, :parent_type, :field, :name, :owner - def initialize(parent_type:, field:, value:, name:, owner:) - @parent_type = parent_type + # @return [Any, Lazy] the GraphQL-ready response value, or a {Lazy} instance + attr_reader :value + + # @return [GraphQL::Field] The field which resolved this value + attr_reader :field + + # @return [SelectionResult] The result object that this field belongs to + attr_reader :owner + + def initialize(field:, value:, owner:) @field = field @owner = owner - @name = name self.value = value end + # Set a new value for this field in the response. + # It may be updated after resolving a {Lazy}. + # If it is {Execute::PROPAGATE_NULL}, tell the owner to propagate null. + # If the value is a {SelectionResult}, make a link with it, and if it's already null, + # propagate the null as needed. + # @param new_value [Any] The GraphQL-ready value def value=(new_value) if new_value.is_a?(SelectionResult) if new_value.invalid_null? - new_value = new_value.invalid_null + new_value = GraphQL::Execution::Execute::PROPAGATE_NULL else new_value.owner = self end @@ -21,7 +34,7 @@ def value=(new_value) if new_value == GraphQL::Execution::Execute::PROPAGATE_NULL if field.type.kind.non_null? - @owner.propagate_null(@name, new_value) + @owner.propagate_null else @value = nil end @@ -31,7 +44,7 @@ def value=(new_value) end def inspect - "# #{value.inspect} (#{field.type})>" + "#" end end end diff --git a/lib/graphql/execution/lazy/lazy_method_map.rb b/lib/graphql/execution/lazy/lazy_method_map.rb index 3b3ae6c604..2f51b68d09 100644 --- a/lib/graphql/execution/lazy/lazy_method_map.rb +++ b/lib/graphql/execution/lazy/lazy_method_map.rb @@ -1,6 +1,9 @@ module GraphQL module Execution class Lazy + # {GraphQL::Schema} uses this to match returned values to lazy resolution methods. + # Methods may be registered for classes, they apply to its subclasses also. + # The result of this lookup is cached for future resolutions. class LazyMethodMap def initialize @storage = {} diff --git a/lib/graphql/execution/lazy/resolve.rb b/lib/graphql/execution/lazy/resolve.rb index c1e114d61e..25a41a69a1 100644 --- a/lib/graphql/execution/lazy/resolve.rb +++ b/lib/graphql/execution/lazy/resolve.rb @@ -3,13 +3,13 @@ module Execution class Lazy # Helpers for dealing with data structures containing {Lazy} instances module Resolve + # Mutate `value`, replacing {Lazy} instances in place with their resolved values + # @return [void] def self.resolve(value) lazies = resolve_in_place(value) deep_sync(lazies) end - # Mutate `value`, replacing {Lazy} instances in place with their resolved values - # @return [void] def self.resolve_in_place(value) lazies = [] diff --git a/lib/graphql/execution/selection_result.rb b/lib/graphql/execution/selection_result.rb index 864b25532f..d876479284 100644 --- a/lib/graphql/execution/selection_result.rb +++ b/lib/graphql/execution/selection_result.rb @@ -1,27 +1,33 @@ module GraphQL module Execution + # A set of key-value pairs suitable for a GraphQL response. class SelectionResult - def initialize(type:) - @type = type + def initialize @storage = {} @owner = nil @invalid_null = false end + # @param key [String] The name for this value in the result + # @param field_result [FieldResult] The result for this field def set(key, field_result) @storage[key] = field_result end + # @param key [String] The name of an already-defined result + # @return [FieldResult] The result for this field def fetch(key) @storage.fetch(key) end + # Visit each key-result pair in this result def each @storage.each do |key, field_res| yield(key, field_res) end end + # @return [Hash] A plain Hash representation of this result def to_h if @invalid_null nil @@ -30,29 +36,27 @@ def to_h end end - def propagate_null(key, value) + # A field has been unexpectedly nullified. + # Tell the owner {FieldResult} if it is present. + # Record {#invalid_null} in case an owner is added later. + def propagate_null if @owner - @owner.value = value + @owner.value = GraphQL::Execution::Execute::PROPAGATE_NULL end - @invalid_null = value + @invalid_null = true end + # @return [Boolean] True if this selection has been nullified by a null child def invalid_null? @invalid_null end - def invalid_null - @invalid_null - end - + # @param field_result [FieldResult] The field that this selection belongs to (used for propagating nulls) def owner=(field_result) if @owner raise("Can't change owners of SelectionResult") else @owner = field_result - if @invalid_null - @owner.value = @invalid_null - end end end From 61a606418fc1a3497dfc6fac3c7bb7893328adb2 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 11 Nov 2016 13:09:21 -0600 Subject: [PATCH 12/13] doc(Lazy) add a guide --- guides/_layouts/default.html | 1 + guides/schema/lazy_execution.md | 84 +++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 guides/schema/lazy_execution.md diff --git a/guides/_layouts/default.html b/guides/_layouts/default.html index 8053cb4d82..2404663b23 100644 --- a/guides/_layouts/default.html +++ b/guides/_layouts/default.html @@ -44,6 +44,7 @@

Guides

  • Configuration Options
  • Code Reuse
  • Instrumentation
  • +
  • Lazy Execution
  • Limiting Visibility
  • Testing
  • diff --git a/guides/schema/lazy_execution.md b/guides/schema/lazy_execution.md new file mode 100644 index 0000000000..d9af8e01e2 --- /dev/null +++ b/guides/schema/lazy_execution.md @@ -0,0 +1,84 @@ +--- +title: Schema — Lazy Execution +--- + +With lazy execution, you can optimize access to external services (such as databases) by making batched calls. Building a lazy loader has three steps: + +- Define a lazy-loading class with _one_ method for loading & returning a value +- Connect it to your schema with {{ "GraphQL::Schema#lazy_resolve" | api_doc }} +- In `resolve` functions, return instances of the lazy-loading class + +[`graphql-batch`](https://github.com/shopify/graphql-batch) provides a powerful, flexible toolkit for lazy resolution with GraphQL. + +## Example: Batched Find + +Here's a way to find many objects by ID using one database call, preventing N+1 queries. + +1. Lazy-loading class which finds models by ID. + + ```ruby + class LazyFindPerson + def initialize(query_ctx, person_id) + @person_id = person_id + # Initialize the loading state for this query, + # or get the previously-initiated state + @lazy_state = query_ctx[:lazy_find_person] ||= { + pending_ids: [], + loaded_ids: {}, + } + # Register this ID to be loaded later: + @lazy_state[:pending_ids] << person_id + end + + # Return the loaded record, hitting the database if needed + def person + # Check if the record was already loaded: + loaded_record = @lazy_state[:loaded_ids][@person_id] + if loaded_record + # The pending IDs were already loaded, + # so return the result of that previous load + loaded_record + else + # The record hasn't been loaded yet, so + # hit the database with all pending IDs + pending_ids = @lazy_state[:pending_ids] + people = Person.where(id: pending_ids) + people.each { |person| @lazy_state[:loaded_ids][person.id] = person } + # Now, get the matching person from the loaded result: + @lazy_state[:loaded_ids][@person_id] + end + end + ``` + +2. Connect the lazy resolve method + + ```ruby + MySchema = GraphQL::Schema.define do + # ... + lazy_resolve(LazyFindPerson, :person) + end + ``` + +3. Return lazy objects from `resolve` + + ```ruby + field :author, PersonType do + resolve ->(obj, args, ctx) { + LazyFindPerson(ctx, obj.author_id) + } + end + ``` + +Now, calls to `author` will use batched database access. For example, this query: + +```graphql +{ + p1: post(id: 1) { author { name } } + p2: post(id: 2) { author { name } } + p3: post(id: 3) { author { name } } +} +``` + +Will only make one query to load the `author` values. + +The example above is simple and has some shortcomings. Consider the `graphql-batch` gem for a robust solution to batched resolution. From f9f21de5173cbf7ecafbb4c04fc1ef97fee96fcd Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 11 Nov 2016 18:02:28 -0600 Subject: [PATCH 13/13] feat(LazyExecutionSpecification) add compat suite --- lib/graphql/compatibility.rb | 1 + .../lazy_execution_specification.rb | 54 +++++++++++++++++++ .../lazy_schema.rb | 54 +++++++++++++++++++ .../lazy_execution_specification_spec.rb | 3 ++ spec/graphql/execution/execute_spec.rb | 1 + 5 files changed, 113 insertions(+) create mode 100644 lib/graphql/compatibility/lazy_execution_specification.rb create mode 100644 lib/graphql/compatibility/lazy_execution_specification/lazy_schema.rb create mode 100644 spec/graphql/compatibility/lazy_execution_specification_spec.rb diff --git a/lib/graphql/compatibility.rb b/lib/graphql/compatibility.rb index 4068dc71a7..443d68f4af 100644 --- a/lib/graphql/compatibility.rb +++ b/lib/graphql/compatibility.rb @@ -1,3 +1,4 @@ require "graphql/compatibility/execution_specification" +require "graphql/compatibility/lazy_execution_specification" require "graphql/compatibility/query_parser_specification" require "graphql/compatibility/schema_parser_specification" diff --git a/lib/graphql/compatibility/lazy_execution_specification.rb b/lib/graphql/compatibility/lazy_execution_specification.rb new file mode 100644 index 0000000000..1a2408fa28 --- /dev/null +++ b/lib/graphql/compatibility/lazy_execution_specification.rb @@ -0,0 +1,54 @@ +require "graphql/compatibility/lazy_execution_specification/lazy_schema" + +module GraphQL + module Compatibility + module LazyExecutionSpecification + # @param execution_strategy [<#new, #execute>] An execution strategy class + # @return [Class] A test suite for this execution strategy + def self.build_suite(execution_strategy) + Class.new(Minitest::Test) do + class << self + attr_accessor :lazy_schema + end + + self.lazy_schema = LazySchema.build(execution_strategy) + + def test_it_resolves_lazy_values + pushes = [] + query_str = %| + { + p1: push(value: 1) { + value + } + p2: push(value: 2) { + push(value: 3) { + value + } + } + p3: push(value: 4) { + push(value: 5) { + value + } + } + } + | + res = self.class.lazy_schema.execute(query_str, context: {pushes: pushes}) + + expected_data = { + "p1"=>{"value"=>1}, + "p2"=>{"push"=>{"value"=>3}}, + "p3"=>{"push"=>{"value"=>5}}, + } + assert_equal expected_data, res["data"] + + expected_pushes = [ + [1,2,4], # first level + [3,5], # second level + ] + assert_equal expected_pushes, pushes + end + end + end + end + end +end diff --git a/lib/graphql/compatibility/lazy_execution_specification/lazy_schema.rb b/lib/graphql/compatibility/lazy_execution_specification/lazy_schema.rb new file mode 100644 index 0000000000..ceef78c50a --- /dev/null +++ b/lib/graphql/compatibility/lazy_execution_specification/lazy_schema.rb @@ -0,0 +1,54 @@ +module GraphQL + module Compatibility + module LazyExecutionSpecification + module LazySchema + class LazyPush + attr_reader :value + def initialize(ctx, value) + @value = value + @context = ctx + pushes = @context[:lazy_pushes] ||= [] + pushes << @value + end + + def push + if @context[:lazy_pushes].include?(@value) + @context[:pushes] << @context[:lazy_pushes] + @context[:lazy_pushes] = [] + end + self + end + end + + def self.build(execution_strategy) + lazy_push_type = GraphQL::ObjectType.define do + name "LazyPush" + field :value, types.Int + field :push, lazy_push_type do + argument :value, types.Int + resolve ->(o, a, c) { + LazyPush.new(c, a[:value]) + } + end + end + + query_type = GraphQL::ObjectType.define do + name "Query" + field :push, lazy_push_type do + argument :value, types.Int + resolve ->(o, a, c) { + LazyPush.new(c, a[:value]) + } + end + end + + GraphQL::Schema.define do + query(query_type) + query_execution_strategy(execution_strategy) + lazy_resolve(LazyPush, :push) + end + end + end + end + end +end diff --git a/spec/graphql/compatibility/lazy_execution_specification_spec.rb b/spec/graphql/compatibility/lazy_execution_specification_spec.rb new file mode 100644 index 0000000000..8b2c09fefd --- /dev/null +++ b/spec/graphql/compatibility/lazy_execution_specification_spec.rb @@ -0,0 +1,3 @@ +require "spec_helper" + +LazySpecSuite = GraphQL::Compatibility::LazyExecutionSpecification.build_suite(GraphQL::Execution::Execute) diff --git a/spec/graphql/execution/execute_spec.rb b/spec/graphql/execution/execute_spec.rb index 43f13cc55b..4b33c9ccff 100644 --- a/spec/graphql/execution/execute_spec.rb +++ b/spec/graphql/execution/execute_spec.rb @@ -1,3 +1,4 @@ require "spec_helper" ExecuteSuite = GraphQL::Compatibility::ExecutionSpecification.build_suite(GraphQL::Execution::Execute) +LazyExecuteSuite = GraphQL::Compatibility::LazyExecutionSpecification.build_suite(GraphQL::Execution::Execute)