From 819329a447fdb548321611823b692fa27b6a9085 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 9 Aug 2018 08:55:21 -0400 Subject: [PATCH 01/16] Add basic field filter flow --- lib/graphql/schema.rb | 2 +- lib/graphql/schema/field.rb | 96 +++++++++++++++++++++++- lib/graphql/schema/field_filter.rb | 27 +++++++ spec/graphql/schema/field_filter_spec.rb | 92 +++++++++++++++++++++++ 4 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 lib/graphql/schema/field_filter.rb create mode 100644 spec/graphql/schema/field_filter_spec.rb diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index de9473046e..8b8f0fd63c 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -19,7 +19,6 @@ require "graphql/schema/warden" require "graphql/schema/build_from_definition" - require "graphql/schema/member" require "graphql/schema/wrapper" require "graphql/schema/list" @@ -28,6 +27,7 @@ require "graphql/schema/enum_value" require "graphql/schema/enum" require "graphql/schema/field" +require "graphql/schema/field_filter" require "graphql/schema/input_object" require "graphql/schema/interface" require "graphql/schema/resolver" diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index e49f2b8f6f..0f57c7aa54 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -125,7 +125,8 @@ def scoped? # @param complexity [Numeric] When provided, set the complexity for this field # @param scope [Boolean] If true, the return type's `.scope_items` method will be called on the return value # @param subscription_scope [Symbol, String] A key in `context` which will be used to scope subscription payloads - def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function: nil, description: nil, deprecation_reason: nil, method: nil, connection: nil, max_page_size: nil, scope: nil, resolve: nil, introspection: false, hash_key: nil, camelize: true, complexity: 1, extras: [], resolver_class: nil, subscription_scope: nil, arguments: {}, &definition_block) + # @param filters [Array] Named filters to apply to this field (registered with {.filter}) + def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function: nil, description: nil, deprecation_reason: nil, method: nil, connection: nil, max_page_size: nil, scope: nil, resolve: nil, introspection: false, hash_key: nil, camelize: true, complexity: 1, extras: [], filters: [], resolver_class: nil, subscription_scope: nil, arguments: {}, &definition_block) if name.nil? raise ArgumentError, "missing first `name` argument or keyword `name:`" @@ -169,6 +170,8 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function @extras = extras @resolver_class = resolver_class @scope = scope + @filters = [] + self.filters(*Array(filters)) # Override the default from HasArguments @own_arguments = {} @@ -202,6 +205,57 @@ def description(text = nil) end end + # @param filters [Array, Hash Object>] Add filters to this field by name + # @return [Array err + raise ArgumentError, "Failed to find filter `#{try_f.inspect}` among #{cls_filters.keys}" + end + + # TODO what API to support? + def filter(name, options = nil) + filters({name => options}) + end + + def self.filter(name, filter) + own_filters[name] = filter + end + + def self.filters + own_filters.merge(superclass.respond_to?(:filters) ? superclass.filters : {}) + end + + def self.own_filters + @own_filters ||= {} + end + def complexity(new_complexity) case new_complexity when Proc @@ -430,11 +484,45 @@ def public_send_field(obj, graphql_args, field_ctx) ruby_kwargs = NO_ARGS end + with_filters(obj, ruby_kwargs, field_ctx.query.context) do |filtered_obj, filtered_args| + if filtered_args.any? + filtered_obj.public_send(@method_sym, **filtered_args) + else + filtered_obj.public_send(@method_sym) + end + end + end - if ruby_kwargs.any? - obj.public_send(@method_sym, **ruby_kwargs) + # TODO this needs the same kind of work as instrumenters to avoid long, boring stack traces + def with_filters(obj, args, ctx) + memos = [] + value = run_before_filter(0, memos, obj, args, ctx) { |filtered_obj, filtered_args| yield(filtered_obj, filtered_args) } + ctx.schema.after_lazy(value) do |resolved_value| + run_after_filter(0, memos, obj, args, ctx, resolved_value) + end + end + + def run_before_filter(idx, memos, obj, args, ctx) + filter = @filters[idx] + if filter + filter.before_resolve(object: obj, arguments: args, context: ctx) do |filtered_obj, filtered_args, memo| + memos << memo + run_before_filter(idx + 1, memos, filtered_obj, filtered_args, ctx) { |o, a| yield(o, a) } + end else - obj.public_send(@method_sym) + yield(obj, args) + end + end + + def run_after_filter(idx, memos, obj, args, ctx, value) + filter = @filters[idx] + if filter + memo = memos[idx] + filtered_value = filter.after_resolve(object: obj, arguments: args, context: ctx, value: value, memo: memo) + # TODO after_lazy? + run_after_filter(idx + 1, memos, obj, args, ctx, filtered_value) + else + value end end end diff --git a/lib/graphql/schema/field_filter.rb b/lib/graphql/schema/field_filter.rb new file mode 100644 index 0000000000..fa42c0fe39 --- /dev/null +++ b/lib/graphql/schema/field_filter.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +module GraphQL + class Schema + + # TODO rename? since it's more than just filter + class FieldFilter + # @return [GraphQL::Schema::Field] + attr_reader :field + + # @return [Object] + attr_reader :options + + def initialize(field:, options:) + @field = field + @options = options + end + + def before_resolve(object:, arguments:, context:) + yield(object, arguments) + end + + def after_resolve(object:, arguments:, context:, value:) + value + end + end + end +end diff --git a/spec/graphql/schema/field_filter_spec.rb b/spec/graphql/schema/field_filter_spec.rb new file mode 100644 index 0000000000..6b88a5392b --- /dev/null +++ b/spec/graphql/schema/field_filter_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true +require "spec_helper" + +describe GraphQL::Schema::FieldFilter do + module FilterTestSchema + class BaseField < GraphQL::Schema::Field + class DoubleFilter < GraphQL::Schema::FieldFilter + def after_resolve(object:, value:, arguments:, context:, memo:) + value * 2 + end + end + + class MultiplyByOption < GraphQL::Schema::FieldFilter + def after_resolve(object:, value:, arguments:, context:, memo:) + value * options[:factor] + end + end + + class MultiplyByArgument < GraphQL::Schema::FieldFilter + def initialize(field:, options:) + field.argument(:factor, Integer, required: true) + super + end + + def before_resolve(object:, arguments:, context:) + factor = arguments.delete(:factor) + yield(object, arguments, factor) + end + + def after_resolve(object:, value:, arguments:, context:, memo:) + value * memo + end + end + + # TODO support `filter(cls, as: ...) with default to underscored name` + filter(:double, DoubleFilter) + filter(:multiply_by_option, MultiplyByOption) + filter(:multiply_by_argument, MultiplyByArgument) + end + + class BaseObject < GraphQL::Schema::Object + field_class(BaseField) + end + + class Query < BaseObject + field :doubled, Integer, null: false, method: :pass_thru do + filters(:double) + argument :input, Integer, required: true + end + + def pass_thru(input:) + input # return it as-is, it will be modified by filters + end + + field :trippled_by_option, Integer, null: false, method: :pass_thru do + filter(:multiply_by_option, factor: 3) + argument :input, Integer, required: true + end + + field :multiply_input, Integer, null: false, method: :pass_thru do + filter(:multiply_by_argument) + argument :input, Integer, required: true + end + end + + class Schema < GraphQL::Schema + query(Query) + end + end + + def exec_query(query_str, **kwargs) + FilterTestSchema::Schema.execute(query_str, **kwargs) + end + + describe "modifying return values" do + it "returns the modified value" do + res = exec_query("{ doubled(input: 5) }") + assert_equal 10, res["data"]["doubled"] + end + + it "has access to config options" do + # The factor of three came from an option + res = exec_query("{ trippledByOption(input: 4) }") + assert_equal 12, res["data"]["trippledByOption"] + end + + it "can hide arguments from resolve methods" do + res = exec_query("{ multiplyInput(input: 3, factor: 5) }") + assert_equal 15, res["data"]["multiplyInput"] + end + end +end From d6f1df230deb3897aa30203f813c4fc1533aee39 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 9 Aug 2018 09:41:50 -0400 Subject: [PATCH 02/16] Add API docs --- lib/graphql/schema/field.rb | 3 --- lib/graphql/schema/field_filter.rb | 35 +++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 0f57c7aa54..5c1e09ebf9 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -229,9 +229,6 @@ def filters(*filters) filter_cls = cls_filters.fetch(name) # Make a new filter ... filter = filter_cls.new(field: self, options: options) - # But `.freeze` it, so that ivars aren't updated during execution, - # which would be subject to all kinds of race conditions. - filter.freeze @filters << filter end end diff --git a/lib/graphql/schema/field_filter.rb b/lib/graphql/schema/field_filter.rb index fa42c0fe39..a5be279772 100644 --- a/lib/graphql/schema/field_filter.rb +++ b/lib/graphql/schema/field_filter.rb @@ -1,7 +1,14 @@ # frozen_string_literal: true module GraphQL class Schema - + # Extend this class to make field-level customizations to resolve behavior. + # + # When a filter is added to a field with `filter(:my_filter)`, a `MyFilter` instance + # is created, and its hooks are applied whenever that field is called. + # + # The instance is frozen so that instance variables aren't modified during query execution, + # which could cause all kinds of issues due to race conditions. + # # TODO rename? since it's more than just filter class FieldFilter # @return [GraphQL::Schema::Field] @@ -10,16 +17,38 @@ class FieldFilter # @return [Object] attr_reader :options + # Called when the filter is mounted with `filter(name, options)`. + # The instance is frozen to avoid improper use of state during execution. + # @param field [GraphQL::Schema::Field] The field where this filter was mounted + # @param options [Object] The second argument to `filter`, or `nil` if nothing was passed. def initialize(field:, options:) @field = field @options = options + freeze end + # Called before resolving {#field}. It should either: + # - `yield` values to continue execution; OR + # - return something else to shortcut field execution. + # @param object [Object] The object the field is being resolved on + # @param arguments [Hash] Ruby keyword arguments for resolving this field + # @param context [Query::Context] the context for this query + # @yieldparam object [Object] The object to continue resolving the field on + # @yieldparam arguments [Hash] The keyword arguments to continue resolving with + # @yieldparam memo [Object] Any filter-specific value which will be passed to {#after_resolve} later def before_resolve(object:, arguments:, context:) - yield(object, arguments) + yield(object, arguments, nil) end - def after_resolve(object:, arguments:, context:, value:) + # Called after {#field} was resolved, but before the value was added to the GraphQL response. + # Whatever this hook returns will be used as the return value. + # @param object [Object] The object the field is being resolved on + # @param arguments [Hash] Ruby keyword arguments for resolving this field + # @param context [Query::Context] the context for this query + # @param value [Object] Whatever the field previously returned + # @param memo [Object] The third value yielded by {#before_resolve}, or `nil` if there wasn't one + # @return [Object] The return value for this field. + def after_resolve(object:, arguments:, context:, value:, memo:) value end end From 4471a09e3235e03edba2a1e825f5195380592913 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 9 Aug 2018 10:29:26 -0400 Subject: [PATCH 03/16] Add Field#to_options for duping fields --- lib/graphql/schema/field.rb | 38 +++++++++++++++++++++++-------- lib/graphql/schema/mutation.rb | 2 +- spec/graphql/schema/field_spec.rb | 30 ++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 5c1e09ebf9..9d4984be20 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -104,9 +104,35 @@ def scoped? end end + def to_options + { + name: name, + type: @return_type_expr, + null: @return_type_null, + owner: owner, + complexity: @complexity, + extras: @extras, + arguments: arguments, + description: description, + deprecation_reason: deprecation_reason, + method: method_sym, + hash_key: nil, # This is currently equivalent to `method:`, so just pass one + connection: @connection, + max_page_size: max_page_size, + introspection: @introspection, + resolver_class: @resolver_class, + resolve: @resolve, + field: @field, + function: @function, + camelize: nil, # It was already applied, so maybe nil will work + scope: @scope, + subscription_scope: @subscription_scope, + filters: @filters, + } + end + # @param name [Symbol] The underscore-cased version of this field name (will be camelized for the GraphQL API) - # @param return_type_expr [Class, GraphQL::BaseType, Array] The return type of this field - # @param desc [String] Field description + # @param type [Class, GraphQL::BaseType, Array] The return type of this field # @param owner [Class] The type that this field belongs to # @param null [Boolean] `true` if this field may return `null`, `false` if it is never `null` # @param description [String] Field description @@ -145,7 +171,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function @name = camelize ? Member::BuildType.camelize(name.to_s) : name.to_s @description = description if field.is_a?(GraphQL::Schema::Field) - @field_instance = field + raise ArgumentError, "Instead of passing a field as `field:`, use `Field.from_options(field.to_options.merge(overrides))`" else @field = field end @@ -274,12 +300,6 @@ def complexity(new_complexity) # @return [GraphQL::Field] def to_graphql - # this field was previously defined and passed here, so delegate to it - if @field_instance - return @field_instance.to_graphql - end - - field_defn = if @field @field.dup elsif @function diff --git a/lib/graphql/schema/mutation.rb b/lib/graphql/schema/mutation.rb index fffe1d3f79..24eb19599c 100644 --- a/lib/graphql/schema/mutation.rb +++ b/lib/graphql/schema/mutation.rb @@ -122,7 +122,7 @@ def generate_payload_type description("Autogenerated return type of #{mutation_name}") mutation(mutation_class) mutation_fields.each do |name, f| - field(name, field: f) + field(f.to_options.merge(name: name)) end end end diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index 7ab52501db..3aec207ab9 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -258,4 +258,34 @@ assert_equal "Broken!!", field.deprecation_reason end end + + describe "#to_options" do + it "has a key for each of `#initialize`'s kwargs" do + # Sadly, GraphQL::Schema::Field.instance_method(:initialize).parameters + # doesn't work because of how `AcceptsDefinition` prepends over it. + field_file = File.read("lib/graphql/schema/field.rb") + def_line = field_file.lines.find { |l| l.include?("def initialize(") } + keywords = def_line.scan(/[a-z_]+: /).map { |match| match[0..-3].to_sym } + + field = GraphQL::Schema::Field.from_options(:thing, Integer, null: true) + options = field.to_options + keywords.each do |keyword| + assert options.key?(keyword), "#{keyword.inspect} is present in #{options.keys}" + end + end + + it "duplicates fields" do + fields_checked = 0 + Jazz::Query.fields.each do |name, field_defn| + fields_checked += 1 + field_as_options = field_defn.to_options + rebuilt_field = GraphQL::Schema::Field.from_options(field_as_options) + assert_equal field_as_options, rebuilt_field.to_options, "It duplicates #{name}" + end + + # Just make sure it actually ran a few times, + # this is the total number of fields on Jazz::Query + assert_equal 14, fields_checked + end + end end From f06ab794fe96450210de14577cc9e5930bcfe718 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 9 Aug 2018 11:25:42 -0400 Subject: [PATCH 04/16] Migrate connections to field filters --- .../relay/connection_instrumentation.rb | 4 +- lib/graphql/schema.rb | 2 +- lib/graphql/schema/field.rb | 77 ++++++++-------- lib/graphql/schema/field/connection_filter.rb | 52 +++++++++++ spec/graphql/introspection/type_type_spec.rb | 2 +- .../relay/connection_instrumentation_spec.rb | 10 +-- spec/graphql/schema/field_spec.rb | 12 ++- spec/support/star_trek/schema.rb | 86 +++++++++++------- spec/support/star_wars/schema.rb | 88 +++++++++++-------- 9 files changed, 210 insertions(+), 123 deletions(-) create mode 100644 lib/graphql/schema/field/connection_filter.rb diff --git a/lib/graphql/relay/connection_instrumentation.rb b/lib/graphql/relay/connection_instrumentation.rb index e193a7e293..1ddec45a9d 100644 --- a/lib/graphql/relay/connection_instrumentation.rb +++ b/lib/graphql/relay/connection_instrumentation.rb @@ -32,7 +32,9 @@ def self.default_arguments # - Merging in the default arguments # - Transforming its resolve function to return a connection object def self.instrument(type, field) - if field.connection? + # Don't apply the wrapper to class-based fields, since they + # use Schema::Field::ConnectionFilter + if field.connection? && !field.metadata[:type_class] connection_arguments = default_arguments.merge(field.arguments) original_resolve = field.resolve_proc original_lazy_resolve = field.lazy_resolve_proc diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 8b8f0fd63c..b7752d2b92 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -26,8 +26,8 @@ require "graphql/schema/argument" require "graphql/schema/enum_value" require "graphql/schema/enum" -require "graphql/schema/field" require "graphql/schema/field_filter" +require "graphql/schema/field" require "graphql/schema/input_object" require "graphql/schema/interface" require "graphql/schema/resolver" diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 9d4984be20..05bb69f2c7 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true # test_via: ../object.rb +require "graphql/schema/field/connection_filter" + module GraphQL class Schema class Field @@ -86,7 +88,8 @@ def connection? elsif @return_type_expr Member::BuildType.to_type_name(@return_type_expr) else - raise "No connection info possible" + # As a last ditch, try to force loading the return type: + type.unwrap.name end @connection = return_type_name.end_with?("Connection") else @@ -196,8 +199,6 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function @extras = extras @resolver_class = resolver_class @scope = scope - @filters = [] - self.filters(*Array(filters)) # Override the default from HasArguments @own_arguments = {} @@ -212,6 +213,13 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function @owner = owner @subscription_scope = subscription_scope + # Do this last so we have as much context as possible when initializing them: + @filters = [] + self.filters(*Array(filters)) + if connection? + self.filter(ConnectionFilter) + end + if definition_block if definition_block.arity == 1 instance_exec(self, &definition_block) @@ -238,7 +246,12 @@ def filters(*filters) # Read the value @filters else + if @resolve || @function + raise ArgumentError, "Filters are not supported with resolve procs or functions, but #{owner.name}.#{name} has: #{@resolve || @function}\nUse a method or a Schema::Resolver instead." + end + cls_filters = self.class.filters + # Normalize to a Hash of {name => options} filters_with_options = if filters.last.is_a?(Hash) filters.pop else @@ -249,17 +262,20 @@ def filters(*filters) filters_with_options[f] = nil end - try_f = nil + # Find the matching class and initialize it filters_with_options.each do |name, options| - try_f = name - filter_cls = cls_filters.fetch(name) - # Make a new filter ... - filter = filter_cls.new(field: self, options: options) - @filters << filter + filter_cls = if name.is_a?(Class) + name + else + begin + cls_filters.fetch(name) + rescue KeyError + raise ArgumentError, "Failed to find filter `#{name}` among #{cls_filters.keys}" + end + end + @filters << filter_cls.new(field: self, options: options) end end - rescue KeyError => err - raise ArgumentError, "Failed to find filter `#{try_f.inspect}` among #{cls_filters.keys}" end # TODO what API to support? @@ -298,6 +314,9 @@ def complexity(new_complexity) end + # @return [Integer, nil] Applied to connections if present + attr_reader :max_page_size + # @return [GraphQL::Field] def to_graphql field_defn = if @field @@ -330,22 +349,11 @@ def to_graphql field_defn.resolve = self.method(:resolve_field) field_defn.connection = connection? - field_defn.connection_max_page_size = @max_page_size + field_defn.connection_max_page_size = max_page_size field_defn.introspection = @introspection field_defn.complexity = @complexity field_defn.subscription_scope = @subscription_scope - # apply this first, so it can be overriden below - if connection? - # TODO: this could be a bit weird, because these fields won't be present - # after initialization, only in the `to_graphql` response. - # This calculation _could_ be moved up if need be. - argument :after, "String", "Returns the elements in the list that come after the specified cursor.", required: false - argument :before, "String", "Returns the elements in the list that come before the specified cursor.", required: false - argument :first, "Int", "Returns the first _n_ elements from the list.", required: false - argument :last, "Int", "Returns the last _n_ elements from the list.", required: false - end - arguments.each do |name, defn| arg_graphql = defn.to_graphql field_defn.arguments[arg_graphql.name] = arg_graphql @@ -406,16 +414,19 @@ def resolve_field(obj, args, ctx) inner_obj = after_obj && after_obj.object if authorized?(inner_obj, query_ctx) && arguments.each_value.all? { |a| a.authorized?(inner_obj, query_ctx) } # Then if it passed, resolve the field - v = if @resolve_proc + if @resolve_proc # Might be nil, still want to call the func in that case - @resolve_proc.call(inner_obj, args, ctx) + v = @resolve_proc.call(inner_obj, args, ctx) + # This is usually applied in `public_send_field`, + # so apply it here instead: + # TODO consider not applying it? + apply_scope(v, ctx) elsif @resolver_class singleton_inst = @resolver_class.new(object: inner_obj, context: query_ctx) public_send_field(singleton_inst, args, ctx) else public_send_field(after_obj, args, ctx) end - apply_scope(v, ctx) else nil end @@ -485,14 +496,6 @@ def public_send_field(obj, graphql_args, field_ctx) end end - if connection? - # Remove pagination args before passing it to a user method - ruby_kwargs.delete(:first) - ruby_kwargs.delete(:last) - ruby_kwargs.delete(:before) - ruby_kwargs.delete(:after) - end - @extras.each do |extra_arg| # TODO: provide proper tests for `:ast_node`, `:irep_node`, `:parent`, others? ruby_kwargs[extra_arg] = field_ctx.public_send(extra_arg) @@ -501,12 +504,14 @@ def public_send_field(obj, graphql_args, field_ctx) ruby_kwargs = NO_ARGS end - with_filters(obj, ruby_kwargs, field_ctx.query.context) do |filtered_obj, filtered_args| - if filtered_args.any? + query_ctx = field_ctx.query.context + with_filters(obj, ruby_kwargs, query_ctx) do |filtered_obj, filtered_args| + value = if filtered_args.any? filtered_obj.public_send(@method_sym, **filtered_args) else filtered_obj.public_send(@method_sym) end + apply_scope(value, query_ctx) end end diff --git a/lib/graphql/schema/field/connection_filter.rb b/lib/graphql/schema/field/connection_filter.rb new file mode 100644 index 0000000000..b805954c77 --- /dev/null +++ b/lib/graphql/schema/field/connection_filter.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module GraphQL + class Schema + class Field + class ConnectionFilter < GraphQL::Schema::FieldFilter + def initialize(field:, options:) + field.argument :after, "String", "Returns the elements in the list that come after the specified cursor.", required: false + field.argument :before, "String", "Returns the elements in the list that come before the specified cursor.", required: false + field.argument :first, "Int", "Returns the first _n_ elements from the list.", required: false + field.argument :last, "Int", "Returns the last _n_ elements from the list.", required: false + super + end + + # Remove pagination args before passing it to a user method + def before_resolve(object:, arguments:, context:) + next_args = arguments.dup + next_args.delete(:first) + next_args.delete(:last) + next_args.delete(:before) + next_args.delete(:after) + yield(object, next_args) + end + + def after_resolve(value:, object:, arguments:, context:, memo:) + # TODO this should be extracted away + if value.is_a? GraphQL::ExecutionError + # This isn't even going to work because context doesn't have ast_node anymore + context.add_error(value) + nil + elsif value.nil? + nil + else + if object.is_a?(GraphQL::Schema::Object) + object = object.object + end + connection_class = GraphQL::Relay::BaseConnection.connection_for_nodes(value) + connection_class.new( + value, + arguments, + field: field, + max_page_size: field.max_page_size, + parent: object, + context: context, + ) + end + end + + end + end + end +end diff --git a/spec/graphql/introspection/type_type_spec.rb b/spec/graphql/introspection/type_type_spec.rb index 94a7a27823..d85dfc16aa 100644 --- a/spec/graphql/introspection/type_type_spec.rb +++ b/spec/graphql/introspection/type_type_spec.rb @@ -147,7 +147,7 @@ type_result = res["data"]["__schema"]["types"].find { |t| t["name"] == "Faction" } field_result = type_result["fields"].find { |f| f["name"] == "bases" } - all_arg_names = ["first", "after", "last", "before", "nameIncludes"] + all_arg_names = ["after", "before", "first", "last", "nameIncludes"] returned_arg_names = field_result["args"].map { |a| a["name"] } assert_equal all_arg_names, returned_arg_names end diff --git a/spec/graphql/relay/connection_instrumentation_spec.rb b/spec/graphql/relay/connection_instrumentation_spec.rb index aaf2ec3a75..caea27153e 100644 --- a/spec/graphql/relay/connection_instrumentation_spec.rb +++ b/spec/graphql/relay/connection_instrumentation_spec.rb @@ -11,11 +11,6 @@ assert_equal ["tests"], test_type.fields.keys end - it "keeps a reference to the function" do - conn_field = StarWars::Faction.graphql_definition.fields["shipsWithMaxPageSize"] - assert_instance_of StarWars::ShipsWithMaxPageSize, conn_field.function - end - let(:build_schema) { test_type = nil @@ -73,9 +68,8 @@ GRAPHQL ctx = { before_built_ins: [], after_built_ins: [] } star_wars_query(query_str, {}, context: ctx) - # The second item is different here: - # Before the object is wrapped in a connection, the instrumentation sees `Array` - assert_equal ["StarWars::FactionRecord", "Array", "GraphQL::Relay::ArrayConnection"], ctx[:before_built_ins] + # These are data classes, later they're wrapped with type proxies + assert_equal ["StarWars::FactionRecord", "GraphQL::Relay::ArrayConnection", "GraphQL::Relay::ArrayConnection"], ctx[:before_built_ins] # After the object is wrapped in a connection, it sees the connection object assert_equal ["StarWars::Faction", "StarWars::ShipConnectionWithParentType", "GraphQL::Types::Relay::PageInfo"], ctx[:after_built_ins] end diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index 3aec207ab9..ce8ee650cb 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -205,14 +205,12 @@ end it "makes a suggestion when the type is false" do - thing = Class.new(GraphQL::Schema::Object) do - graphql_name "Thing" - # False might come from an invalid `!` - field :stuff, false, null: false - end - err = assert_raises ArgumentError do - thing.fields["stuff"].type + Class.new(GraphQL::Schema::Object) do + graphql_name "Thing" + # False might come from an invalid `!` + field :stuff, false, null: false + end end assert_includes err.message, "Thing.stuff" diff --git a/spec/support/star_trek/schema.rb b/spec/support/star_trek/schema.rb index c355162c74..b12bbb3d9a 100644 --- a/spec/support/star_trek/schema.rb +++ b/spec/support/star_trek/schema.rb @@ -73,18 +73,18 @@ def field_name end end - # Example of GraphQL::Function used with the connection helper: - class ShipsWithMaxPageSize < GraphQL::Function - argument :nameIncludes, GraphQL::STRING_TYPE - def call(obj, args, ctx) - all_ships = obj.ships.map { |ship_id| StarTrek::DATA["Ship"][ship_id] } - if args[:nameIncludes] - all_ships = all_ships.select { |ship| ship.name.include?(args[:nameIncludes])} + + class ShipsWithMaxPageSize < GraphQL::Schema::Resolver + argument :name_includes, String, required: false + type Ship.connection_type, null: true + + def resolve(name_includes: nil) + all_ships = object.ships.map { |ship_id| StarTrek::DATA["Ship"][ship_id] } + if name_includes + all_ships = all_ships.select { |ship| ship.name.include?(name_includes)} end all_ships end - - type Ship.connection_type end class ShipConnectionWithParentType < GraphQL::Types::Relay::BaseConnection @@ -101,10 +101,14 @@ class Faction < GraphQL::Schema::Object field :id, ID, null: false, resolve: GraphQL::Relay::GlobalIdResolve.new(type: Faction) field :name, String, null: true - field :ships, ShipConnectionWithParentType, connection: true, max_page_size: 1000, null: true, resolve: ->(obj, args, ctx) { - all_ships = obj.ships.map {|ship_id| StarTrek::DATA["Ship"][ship_id] } - if args[:nameIncludes] - case args[:nameIncludes] + field :ships, ShipConnectionWithParentType, connection: true, max_page_size: 1000, null: true do + argument :name_includes, String, required: false + end + + def ships(name_includes: nil) + all_ships = object.ships.map {|ship_id| StarTrek::DATA["Ship"][ship_id] } + if name_includes + case name_includes when "error" all_ships = GraphQL::ExecutionError.new("error from within connection") when "raisedError" @@ -119,25 +123,24 @@ class Faction < GraphQL::Schema::Object prev_all_ships = all_ships all_ships = LazyWrapper.new { prev_all_ships } else - all_ships = all_ships.select { |ship| ship.name.include?(args[:nameIncludes])} + all_ships = all_ships.select { |ship| ship.name.include?(name_includes)} end end all_ships - } do - # You can define arguments here and use them in the connection - argument :nameIncludes, String, required: false end - field :shipsWithMaxPageSize, "Ships with max page size", max_page_size: 2, function: ShipsWithMaxPageSize.new + field :shipsWithMaxPageSize, "Ships with max page size", max_page_size: 2, resolver: ShipsWithMaxPageSize + + field :bases, BaseConnectionWithTotalCountType, null: true, connection: true do + argument :name_includes, String, required: false + end - field :bases, BaseConnectionWithTotalCountType, null: true, connection: true, resolve: ->(obj, args, ctx) { - all_bases = obj.bases - if args[:nameIncludes] - all_bases = all_bases.where(name: Regexp.new(args[:nameIncludes])) + def bases(name_includes: nil) + all_bases = object.bases + if name_includes + all_bases = all_bases.where(name: Regexp.new(name_includes)) end all_bases - } do - argument :nameIncludes, String, required: false end field :basesClone, BaseType.connection_type, null: true @@ -152,13 +155,24 @@ def bases_by_name(order: nil) end end - field :basesWithMaxLimitRelation, BaseType.connection_type, null: true, max_page_size: 2, resolve: Proc.new { Base.all} - field :basesWithMaxLimitArray, BaseType.connection_type, null: true, max_page_size: 2, resolve: Proc.new { Base.all.to_a } - field :basesWithDefaultMaxLimitRelation, BaseType.connection_type, null: true, resolve: Proc.new { Base.all } - field :basesWithDefaultMaxLimitArray, BaseType.connection_type, null: true, resolve: Proc.new { Base.all.to_a } - field :basesWithLargeMaxLimitRelation, BaseType.connection_type, null: true, max_page_size: 1000, resolve: Proc.new { Base.all } + def all_bases + Base.all + end - field :basesWithCustomEdge, CustomEdgeBaseConnectionType, null: true, connection: true, resolve: ->(o, a, c) { LazyNodesWrapper.new(o.bases) } + def all_bases_array + all_bases.to_a + end + + field :basesWithMaxLimitRelation, BaseType.connection_type, null: true, max_page_size: 2, method: :all_bases + field :basesWithMaxLimitArray, BaseType.connection_type, null: true, max_page_size: 2, method: :all_bases_array + field :basesWithDefaultMaxLimitRelation, BaseType.connection_type, null: true, method: :all_bases + field :basesWithDefaultMaxLimitArray, BaseType.connection_type, null: true, method: :all_bases_array + field :basesWithLargeMaxLimitRelation, BaseType.connection_type, null: true, max_page_size: 1000, method: :all_bases + + field :basesWithCustomEdge, CustomEdgeBaseConnectionType, null: true, connection: true + def bases_with_custom_edge + LazyNodesWrapper.new(object.bases) + end end class IntroduceShipMutation < GraphQL::Schema::RelayClassicMutation @@ -296,7 +310,9 @@ class QueryType < GraphQL::Schema::Object field :largestBase, BaseType, null: true, resolve: ->(obj, args, ctx) { Base.find(3) } - field :newestBasesGroupedByFaction, BaseType.connection_type, null: true, resolve: ->(obj, args, ctx) { + field :newestBasesGroupedByFaction, BaseType.connection_type, null: true + + def newest_bases_grouped_by_faction agg = Base.collection.aggregate([{ "$group" => { "_id" => "$faction_id", @@ -306,11 +322,13 @@ class QueryType < GraphQL::Schema::Object Base. in(id: agg.map { |doc| doc['baseId'] }). order_by(faction_id: -1) - } + end + + field :basesWithNullName, BaseType.connection_type, null: false - field :basesWithNullName, BaseType.connection_type, null: false, resolve: ->(obj, args, ctx) { + def bases_with_null_name [OpenStruct.new(id: nil)] - } + end field :node, field: GraphQL::Relay::Node.field diff --git a/spec/support/star_wars/schema.rb b/spec/support/star_wars/schema.rb index d86bbf92d2..98b9b16178 100644 --- a/spec/support/star_wars/schema.rb +++ b/spec/support/star_wars/schema.rb @@ -86,18 +86,17 @@ def field_name end end - # Example of GraphQL::Function used with the connection helper: - class ShipsWithMaxPageSize < GraphQL::Function - argument :nameIncludes, GraphQL::STRING_TYPE - def call(obj, args, ctx) - all_ships = obj.ships.map { |ship_id| StarWars::DATA["Ship"][ship_id] } - if args[:nameIncludes] - all_ships = all_ships.select { |ship| ship.name.include?(args[:nameIncludes])} + class ShipsWithMaxPageSize < GraphQL::Schema::Resolver + argument :name_includes, String, required: false + type Ship.connection_type, null: true + + def resolve(name_includes: nil) + all_ships = object.ships.map { |ship_id| StarWars::DATA["Ship"][ship_id] } + if name_includes + all_ships = all_ships.select { |ship| ship.name.include?(name_includes)} end all_ships end - - type Ship.connection_type end class ShipConnectionWithParentType < GraphQL::Types::Relay::BaseConnection @@ -114,10 +113,14 @@ class Faction < GraphQL::Schema::Object field :id, ID, null: false, resolve: GraphQL::Relay::GlobalIdResolve.new(type: Faction) field :name, String, null: true - field :ships, ShipConnectionWithParentType, connection: true, max_page_size: 1000, null: true, resolve: ->(obj, args, ctx) { - all_ships = obj.ships.map {|ship_id| StarWars::DATA["Ship"][ship_id] } - if args[:nameIncludes] - case args[:nameIncludes] + field :ships, ShipConnectionWithParentType, connection: true, max_page_size: 1000, null: true do + argument :name_includes, String, required: false + end + + def ships(name_includes: nil) + all_ships = object.ships.map {|ship_id| StarWars::DATA["Ship"][ship_id] } + if name_includes + case name_includes when "error" all_ships = GraphQL::ExecutionError.new("error from within connection") when "raisedError" @@ -132,25 +135,24 @@ class Faction < GraphQL::Schema::Object prev_all_ships = all_ships all_ships = LazyWrapper.new { prev_all_ships } else - all_ships = all_ships.select { |ship| ship.name.include?(args[:nameIncludes])} + all_ships = all_ships.select { |ship| ship.name.include?(name_includes)} end end all_ships - } do - # You can define arguments here and use them in the connection - argument :nameIncludes, String, required: false end - field :shipsWithMaxPageSize, "Ships with max page size", max_page_size: 2, function: ShipsWithMaxPageSize.new + field :shipsWithMaxPageSize, "Ships with max page size", max_page_size: 2, resolver: ShipsWithMaxPageSize + + field :bases, BasesConnectionWithTotalCountType, null: true, connection: true do + argument :name_includes, String, required: false + end - field :bases, BasesConnectionWithTotalCountType, null: true, connection: true, resolve: ->(obj, args, ctx) { - all_bases = Base.where(id: obj.bases) - if args[:nameIncludes] - all_bases = all_bases.where("name LIKE ?", "%#{args[:nameIncludes]}%") + def bases(name_includes: nil) + all_bases = Base.where(id: object.bases) + if name_includes + all_bases = all_bases.where("name LIKE ?", "%#{name_includes}%") end all_bases - } do - argument :nameIncludes, String, required: false end field :basesClone, BaseConnection, null: true @@ -165,12 +167,20 @@ def bases_by_name(order: nil) end end - field :basesWithMaxLimitRelation, BaseConnection, null: true, max_page_size: 2, resolve: Proc.new { Base.all} - field :basesWithMaxLimitArray, BaseConnection, null: true, max_page_size: 2, resolve: Proc.new { Base.all.to_a } - field :basesWithDefaultMaxLimitRelation, BaseConnection, null: true, resolve: Proc.new { Base.all } - field :basesWithDefaultMaxLimitArray, BaseConnection, null: true, resolve: Proc.new { Base.all.to_a } - field :basesWithLargeMaxLimitRelation, BaseConnection, null: true, max_page_size: 1000, resolve: Proc.new { Base.all } - field :basesWithoutNodes, BaseConnectionWithoutNodes, null: true, resolve: Proc.new { Base.all.to_a } + def all_bases + Base.all + end + + def all_bases_array + all_bases.to_a + end + + field :basesWithMaxLimitRelation, BaseConnection, null: true, max_page_size: 2, method: :all_bases + field :basesWithMaxLimitArray, BaseConnection, null: true, max_page_size: 2, method: :all_bases_array + field :basesWithDefaultMaxLimitRelation, BaseConnection, null: true, method: :all_bases + field :basesWithDefaultMaxLimitArray, BaseConnection, null: true, method: :all_bases_array + field :basesWithLargeMaxLimitRelation, BaseConnection, null: true, max_page_size: 1000, method: :all_bases + field :basesWithoutNodes, BaseConnectionWithoutNodes, null: true, method: :all_bases_array field :basesAsSequelDataset, BasesConnectionWithTotalCountType, null: true, connection: true, max_page_size: 1000 do argument :nameIncludes, String, required: false @@ -184,7 +194,11 @@ def bases_as_sequel_dataset(name_includes: nil) all_bases end - field :basesWithCustomEdge, CustomEdgeBaseConnectionType, null: true, connection: true, resolve: ->(o, a, c) { LazyNodesWrapper.new(o.bases) } + field :basesWithCustomEdge, CustomEdgeBaseConnectionType, null: true, connection: true, method: :lazy_bases + + def lazy_bases + LazyNodesWrapper.new(object.bases) + end end class IntroduceShipMutation < GraphQL::Schema::RelayClassicMutation @@ -320,16 +334,20 @@ class QueryType < GraphQL::Schema::Object field :largestBase, BaseType, null: true, resolve: ->(obj, args, ctx) { Base.find(3) } - field :newestBasesGroupedByFaction, BaseConnection, null: true, resolve: ->(obj, args, ctx) { + field :newestBasesGroupedByFaction, BaseConnection, null: true + + def newest_bases_grouped_by_faction Base .having('id in (select max(id) from bases group by faction_id)') .group(:id) .order('faction_id desc') - } + end - field :basesWithNullName, BaseConnection, null: false, resolve: ->(obj, args, ctx) { + field :basesWithNullName, BaseConnection, null: false + + def bases_with_null_name [OpenStruct.new(id: nil)] - } + end field :node, field: GraphQL::Relay::Node.field From 0a77f51182f39eb113668306140de13e8f792f71 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 9 Aug 2018 12:45:29 -0400 Subject: [PATCH 05/16] Refactor scoping to be a filter --- lib/graphql/schema.rb | 7 +-- lib/graphql/schema/field.rb | 53 +++++++++----------- lib/graphql/schema/field/scope_filter.rb | 18 +++++++ lib/graphql/schema/relay_classic_mutation.rb | 2 +- 4 files changed, 46 insertions(+), 34 deletions(-) create mode 100644 lib/graphql/schema/field/scope_filter.rb diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index b7752d2b92..902153ed54 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -30,12 +30,13 @@ require "graphql/schema/field" require "graphql/schema/input_object" require "graphql/schema/interface" +require "graphql/schema/scalar" +require "graphql/schema/object" +require "graphql/schema/union" + require "graphql/schema/resolver" require "graphql/schema/mutation" require "graphql/schema/relay_classic_mutation" -require "graphql/schema/object" -require "graphql/schema/scalar" -require "graphql/schema/union" module GraphQL # A GraphQL schema which may be queried with {GraphQL::Query}. diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 05bb69f2c7..8e304ee9ce 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true # test_via: ../object.rb require "graphql/schema/field/connection_filter" +require "graphql/schema/field/scope_filter" module GraphQL class Schema @@ -103,7 +104,7 @@ def scoped? # The default was overridden @scope else - @return_type_expr && type.unwrap.respond_to?(:scope_items) && (connection? || type.list?) + @return_type_expr.is_a?(Array) || (@return_type_expr.is_a?(String) && @return_type_expr.include?("[")) || connection? end end @@ -130,7 +131,7 @@ def to_options camelize: nil, # It was already applied, so maybe nil will work scope: @scope, subscription_scope: @subscription_scope, - filters: @filters, + filters: @filters.each_with_object({}).map { |f, obj| obj[f.class] = f.options }, } end @@ -216,6 +217,13 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function # Do this last so we have as much context as possible when initializing them: @filters = [] self.filters(*Array(filters)) + # This should run before connection filter, + # but should it run after the definition block? + if scoped? + self.filter(ScopeFilter) + end + # The problem with putting this after the definition_block + # is that it would override arguments if connection? self.filter(ConnectionFilter) end @@ -241,36 +249,36 @@ def description(text = nil) # @param filters [Array, Hash Object>] Add filters to this field by name # @return [Array options} - filters_with_options = if filters.last.is_a?(Hash) - filters.pop + filters_with_options = if new_filters.last.is_a?(Hash) + new_filters.pop else {} end - filters.each do |f| + new_filters.each do |f| filters_with_options[f] = nil end # Find the matching class and initialize it - filters_with_options.each do |name, options| - filter_cls = if name.is_a?(Class) - name + filters_with_options.each do |filter_name, options| + filter_cls = if filter_name.is_a?(Class) + filter_name else begin - cls_filters.fetch(name) + cls_filters.fetch(filter_name) rescue KeyError - raise ArgumentError, "Failed to find filter `#{name}` among #{cls_filters.keys}" + raise ArgumentError, "#{owner}.#{name}: failed to find filter `#{filter_name}` among #{cls_filters.keys}" end end @filters << filter_cls.new(field: self, options: options) @@ -416,11 +424,7 @@ def resolve_field(obj, args, ctx) # Then if it passed, resolve the field if @resolve_proc # Might be nil, still want to call the func in that case - v = @resolve_proc.call(inner_obj, args, ctx) - # This is usually applied in `public_send_field`, - # so apply it here instead: - # TODO consider not applying it? - apply_scope(v, ctx) + @resolve_proc.call(inner_obj, args, ctx) elsif @resolver_class singleton_inst = @resolver_class.new(object: inner_obj, context: query_ctx) public_send_field(singleton_inst, args, ctx) @@ -472,16 +476,6 @@ def resolve_field_method(obj, ruby_kwargs, ctx) private - def apply_scope(value, ctx) - if scoped? - ctx.schema.after_lazy(value) do |inner_value| - @type.unwrap.scope_items(inner_value, ctx) - end - else - value - end - end - NO_ARGS = {}.freeze def public_send_field(obj, graphql_args, field_ctx) @@ -506,12 +500,11 @@ def public_send_field(obj, graphql_args, field_ctx) query_ctx = field_ctx.query.context with_filters(obj, ruby_kwargs, query_ctx) do |filtered_obj, filtered_args| - value = if filtered_args.any? + if filtered_args.any? filtered_obj.public_send(@method_sym, **filtered_args) else filtered_obj.public_send(@method_sym) end - apply_scope(value, query_ctx) end end diff --git a/lib/graphql/schema/field/scope_filter.rb b/lib/graphql/schema/field/scope_filter.rb new file mode 100644 index 0000000000..677849eabd --- /dev/null +++ b/lib/graphql/schema/field/scope_filter.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module GraphQL + class Schema + class Field + class ScopeFilter < GraphQL::Schema::FieldFilter + def after_resolve(value:, context:, **rest) + ret_type = @field.type.unwrap + if ret_type.respond_to?(:scope_items) + ret_type.scope_items(value, context) + else + value + end + end + end + end + end +end diff --git a/lib/graphql/schema/relay_classic_mutation.rb b/lib/graphql/schema/relay_classic_mutation.rb index 8c33b7b502..8ee9675fff 100644 --- a/lib/graphql/schema/relay_classic_mutation.rb +++ b/lib/graphql/schema/relay_classic_mutation.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true - +require "graphql/types/string" module GraphQL class Schema # Mutations that extend this base class get some conventions added for free: From 7d092728d94f7781010185740869e57c3d946b10 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 9 Aug 2018 12:57:28 -0400 Subject: [PATCH 06/16] Remove field #to_options --- lib/graphql/schema/field.rb | 29 +---------------------------- lib/graphql/schema/mutation.rb | 4 +++- spec/graphql/schema/field_spec.rb | 30 ------------------------------ 3 files changed, 4 insertions(+), 59 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 8e304ee9ce..fdd180f398 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -108,33 +108,6 @@ def scoped? end end - def to_options - { - name: name, - type: @return_type_expr, - null: @return_type_null, - owner: owner, - complexity: @complexity, - extras: @extras, - arguments: arguments, - description: description, - deprecation_reason: deprecation_reason, - method: method_sym, - hash_key: nil, # This is currently equivalent to `method:`, so just pass one - connection: @connection, - max_page_size: max_page_size, - introspection: @introspection, - resolver_class: @resolver_class, - resolve: @resolve, - field: @field, - function: @function, - camelize: nil, # It was already applied, so maybe nil will work - scope: @scope, - subscription_scope: @subscription_scope, - filters: @filters.each_with_object({}).map { |f, obj| obj[f.class] = f.options }, - } - end - # @param name [Symbol] The underscore-cased version of this field name (will be camelized for the GraphQL API) # @param type [Class, GraphQL::BaseType, Array] The return type of this field # @param owner [Class] The type that this field belongs to @@ -175,7 +148,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function @name = camelize ? Member::BuildType.camelize(name.to_s) : name.to_s @description = description if field.is_a?(GraphQL::Schema::Field) - raise ArgumentError, "Instead of passing a field as `field:`, use `Field.from_options(field.to_options.merge(overrides))`" + raise ArgumentError, "Instead of passing a field as `field:`, use `add_field(field)` to add an already-defined field." else @field = field end diff --git a/lib/graphql/schema/mutation.rb b/lib/graphql/schema/mutation.rb index 24eb19599c..c737e18fe3 100644 --- a/lib/graphql/schema/mutation.rb +++ b/lib/graphql/schema/mutation.rb @@ -122,7 +122,9 @@ def generate_payload_type description("Autogenerated return type of #{mutation_name}") mutation(mutation_class) mutation_fields.each do |name, f| - field(f.to_options.merge(name: name)) + # Reattach the already-defined field here + # (The field's `.owner` will still point to the mutation, not the object type, I think) + add_field(f) end end end diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index ce8ee650cb..9d85e28c1e 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -256,34 +256,4 @@ assert_equal "Broken!!", field.deprecation_reason end end - - describe "#to_options" do - it "has a key for each of `#initialize`'s kwargs" do - # Sadly, GraphQL::Schema::Field.instance_method(:initialize).parameters - # doesn't work because of how `AcceptsDefinition` prepends over it. - field_file = File.read("lib/graphql/schema/field.rb") - def_line = field_file.lines.find { |l| l.include?("def initialize(") } - keywords = def_line.scan(/[a-z_]+: /).map { |match| match[0..-3].to_sym } - - field = GraphQL::Schema::Field.from_options(:thing, Integer, null: true) - options = field.to_options - keywords.each do |keyword| - assert options.key?(keyword), "#{keyword.inspect} is present in #{options.keys}" - end - end - - it "duplicates fields" do - fields_checked = 0 - Jazz::Query.fields.each do |name, field_defn| - fields_checked += 1 - field_as_options = field_defn.to_options - rebuilt_field = GraphQL::Schema::Field.from_options(field_as_options) - assert_equal field_as_options, rebuilt_field.to_options, "It duplicates #{name}" - end - - # Just make sure it actually ran a few times, - # this is the total number of fields on Jazz::Query - assert_equal 14, fields_checked - end - end end From 4fa7ebde26b5ed6b366c2dc709687caf924657da Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 10 Aug 2018 16:55:30 -0400 Subject: [PATCH 07/16] Simplify filter API --- lib/graphql/schema/field.rb | 50 +++++++--------------- spec/graphql/schema/field_filter_spec.rb | 53 ++++++++++-------------- 2 files changed, 38 insertions(+), 65 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index fdd180f398..2b120bff58 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -128,7 +128,7 @@ def scoped? # @param complexity [Numeric] When provided, set the complexity for this field # @param scope [Boolean] If true, the return type's `.scope_items` method will be called on the return value # @param subscription_scope [Symbol, String] A key in `context` which will be used to scope subscription payloads - # @param filters [Array] Named filters to apply to this field (registered with {.filter}) + # @param filters [Array] Named filters to apply to this field (see also {#filter}) def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function: nil, description: nil, deprecation_reason: nil, method: nil, connection: nil, max_page_size: nil, scope: nil, resolve: nil, introspection: false, hash_key: nil, camelize: true, complexity: 1, extras: [], filters: [], resolver_class: nil, subscription_scope: nil, arguments: {}, &definition_block) if name.nil? @@ -189,7 +189,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function # Do this last so we have as much context as possible when initializing them: @filters = [] - self.filters(*Array(filters)) + self.filters(filters) # This should run before connection filter, # but should it run after the definition block? if scoped? @@ -220,9 +220,12 @@ def description(text = nil) end end - # @param filters [Array, Hash Object>] Add filters to this field by name - # @return [Array, Hash Object>] Add filters to this field + # @return [Array] Filters to apply to this field + def filters(new_filters) if new_filters.none? # Read the value @filters @@ -231,49 +234,28 @@ def filters(*new_filters) raise ArgumentError, "Filters are not supported with resolve procs or functions,\nbut #{owner.name}.#{name} has: #{@resolve || @function}\nSo, it can have filters: #{filters}.\nUse a method or a Schema::Resolver instead." end - cls_filters = self.class.filters # Normalize to a Hash of {name => options} filters_with_options = if new_filters.last.is_a?(Hash) new_filters.pop else {} end - new_filters.each do |f| filters_with_options[f] = nil end - # Find the matching class and initialize it - filters_with_options.each do |filter_name, options| - filter_cls = if filter_name.is_a?(Class) - filter_name - else - begin - cls_filters.fetch(filter_name) - rescue KeyError - raise ArgumentError, "#{owner}.#{name}: failed to find filter `#{filter_name}` among #{cls_filters.keys}" - end - end - @filters << filter_cls.new(field: self, options: options) + # Initialize each class and stash the instance + filters_with_options.each do |filter_class, options| + @filters << filter_class.new(field: self, options: options) end end end - # TODO what API to support? - def filter(name, options = nil) - filters({name => options}) - end - - def self.filter(name, filter) - own_filters[name] = filter - end - - def self.filters - own_filters.merge(superclass.respond_to?(:filters) ? superclass.filters : {}) - end - - def self.own_filters - @own_filters ||= {} + # Add `filter` to this field, initialized with `options` if provided. + # @param filter [Class] subclass of {Schema::FieldFilter} + # @param options [Object] if provided, given as `options:` when initializing `filter`. + def filter(filter, options = nil) + filters([{filter => options}]) end def complexity(new_complexity) diff --git a/spec/graphql/schema/field_filter_spec.rb b/spec/graphql/schema/field_filter_spec.rb index 6b88a5392b..2ae7fa17d2 100644 --- a/spec/graphql/schema/field_filter_spec.rb +++ b/spec/graphql/schema/field_filter_spec.rb @@ -3,48 +3,40 @@ describe GraphQL::Schema::FieldFilter do module FilterTestSchema - class BaseField < GraphQL::Schema::Field - class DoubleFilter < GraphQL::Schema::FieldFilter - def after_resolve(object:, value:, arguments:, context:, memo:) - value * 2 - end + class DoubleFilter < GraphQL::Schema::FieldFilter + def after_resolve(object:, value:, arguments:, context:, memo:) + value * 2 end + end - class MultiplyByOption < GraphQL::Schema::FieldFilter - def after_resolve(object:, value:, arguments:, context:, memo:) - value * options[:factor] - end + class MultiplyByOption < GraphQL::Schema::FieldFilter + def after_resolve(object:, value:, arguments:, context:, memo:) + value * options[:factor] end + end - class MultiplyByArgument < GraphQL::Schema::FieldFilter - def initialize(field:, options:) - field.argument(:factor, Integer, required: true) - super - end - - def before_resolve(object:, arguments:, context:) - factor = arguments.delete(:factor) - yield(object, arguments, factor) - end + class MultiplyByArgument < GraphQL::Schema::FieldFilter + def initialize(field:, options:) + field.argument(:factor, Integer, required: true) + super + end - def after_resolve(object:, value:, arguments:, context:, memo:) - value * memo - end + def before_resolve(object:, arguments:, context:) + factor = arguments.delete(:factor) + yield(object, arguments, factor) end - # TODO support `filter(cls, as: ...) with default to underscored name` - filter(:double, DoubleFilter) - filter(:multiply_by_option, MultiplyByOption) - filter(:multiply_by_argument, MultiplyByArgument) + def after_resolve(object:, value:, arguments:, context:, memo:) + value * memo + end end class BaseObject < GraphQL::Schema::Object - field_class(BaseField) end class Query < BaseObject field :doubled, Integer, null: false, method: :pass_thru do - filters(:double) + filter(DoubleFilter) argument :input, Integer, required: true end @@ -53,12 +45,11 @@ def pass_thru(input:) end field :trippled_by_option, Integer, null: false, method: :pass_thru do - filter(:multiply_by_option, factor: 3) + filter(MultiplyByOption, factor: 3) argument :input, Integer, required: true end - field :multiply_input, Integer, null: false, method: :pass_thru do - filter(:multiply_by_argument) + field :multiply_input, Integer, null: false, method: :pass_thru, filters: [MultiplyByArgument] do argument :input, Integer, required: true end end From 7715a4ad36780991561e33e026bb8c5c722fe6b9 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 10 Aug 2018 17:09:20 -0400 Subject: [PATCH 08/16] rename FieldFilter -> FieldExtension --- lib/graphql/schema.rb | 2 +- lib/graphql/schema/field.rb | 95 ++++++++++--------- ...tion_filter.rb => connection_extension.rb} | 2 +- .../{scope_filter.rb => scope_extension.rb} | 2 +- .../{field_filter.rb => field_extension.rb} | 14 ++- ...filter_spec.rb => field_extension_spec.rb} | 24 +++-- 6 files changed, 76 insertions(+), 63 deletions(-) rename lib/graphql/schema/field/{connection_filter.rb => connection_extension.rb} (96%) rename lib/graphql/schema/field/{scope_filter.rb => scope_extension.rb} (85%) rename lib/graphql/schema/{field_filter.rb => field_extension.rb} (82%) rename spec/graphql/schema/{field_filter_spec.rb => field_extension_spec.rb} (74%) diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 902153ed54..81f29fb626 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -26,7 +26,7 @@ require "graphql/schema/argument" require "graphql/schema/enum_value" require "graphql/schema/enum" -require "graphql/schema/field_filter" +require "graphql/schema/field_extension" require "graphql/schema/field" require "graphql/schema/input_object" require "graphql/schema/interface" diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 2b120bff58..c72d186ad8 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # test_via: ../object.rb -require "graphql/schema/field/connection_filter" -require "graphql/schema/field/scope_filter" +require "graphql/schema/field/connection_extension" +require "graphql/schema/field/scope_extension" module GraphQL class Schema @@ -128,8 +128,8 @@ def scoped? # @param complexity [Numeric] When provided, set the complexity for this field # @param scope [Boolean] If true, the return type's `.scope_items` method will be called on the return value # @param subscription_scope [Symbol, String] A key in `context` which will be used to scope subscription payloads - # @param filters [Array] Named filters to apply to this field (see also {#filter}) - def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function: nil, description: nil, deprecation_reason: nil, method: nil, connection: nil, max_page_size: nil, scope: nil, resolve: nil, introspection: false, hash_key: nil, camelize: true, complexity: 1, extras: [], filters: [], resolver_class: nil, subscription_scope: nil, arguments: {}, &definition_block) + # @param extensions [Array] Named extensions to apply to this field (see also {#extension}) + def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function: nil, description: nil, deprecation_reason: nil, method: nil, connection: nil, max_page_size: nil, scope: nil, resolve: nil, introspection: false, hash_key: nil, camelize: true, complexity: 1, extras: [], extensions: [], resolver_class: nil, subscription_scope: nil, arguments: {}, &definition_block) if name.nil? raise ArgumentError, "missing first `name` argument or keyword `name:`" @@ -188,17 +188,19 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function @subscription_scope = subscription_scope # Do this last so we have as much context as possible when initializing them: - @filters = [] - self.filters(filters) - # This should run before connection filter, + @extensions = [] + if extensions.any? + self.extensions(extensions) + end + # This should run before connection extension, # but should it run after the definition block? if scoped? - self.filter(ScopeFilter) + self.extension(ScopeExtension) end # The problem with putting this after the definition_block # is that it would override arguments if connection? - self.filter(ConnectionFilter) + self.extension(ConnectionExtension) end if definition_block @@ -220,42 +222,47 @@ def description(text = nil) end end - # Read filter instances from this field, + # Read extension instances from this field, # or add new classes/options to be initialized on this field. # - # @param filters [Array, Hash Object>] Add filters to this field - # @return [Array] Filters to apply to this field - def filters(new_filters) - if new_filters.none? + # @param extensions [Array, Hash Object>] Add extensions to this field + # @return [Array] extensions to apply to this field + def extensions(new_extensions = nil) + if new_extensions.nil? # Read the value - @filters + @extensions else if @resolve || @function - raise ArgumentError, "Filters are not supported with resolve procs or functions,\nbut #{owner.name}.#{name} has: #{@resolve || @function}\nSo, it can have filters: #{filters}.\nUse a method or a Schema::Resolver instead." + raise ArgumentError, <<~MSG +Extensions are not supported with resolve procs or functions, +but #{owner.name}.#{name} has: #{@resolve || @function} +So, it can't have extensions: #{extensions}. +Use a method or a Schema::Resolver instead. +MSG end # Normalize to a Hash of {name => options} - filters_with_options = if new_filters.last.is_a?(Hash) - new_filters.pop + extensions_with_options = if new_extensions.last.is_a?(Hash) + new_extensions.pop else {} end - new_filters.each do |f| - filters_with_options[f] = nil + new_extensions.each do |f| + extensions_with_options[f] = nil end # Initialize each class and stash the instance - filters_with_options.each do |filter_class, options| - @filters << filter_class.new(field: self, options: options) + extensions_with_options.each do |extension_class, options| + @extensions << extension_class.new(field: self, options: options) end end end - # Add `filter` to this field, initialized with `options` if provided. - # @param filter [Class] subclass of {Schema::FieldFilter} - # @param options [Object] if provided, given as `options:` when initializing `filter`. - def filter(filter, options = nil) - filters([{filter => options}]) + # Add `extension` to this field, initialized with `options` if provided. + # @param extension [Class] subclass of {Schema::Fieldextension} + # @param options [Object] if provided, given as `options:` when initializing `extension`. + def extension(extension, options = nil) + extensions([{extension => options}]) end def complexity(new_complexity) @@ -454,43 +461,43 @@ def public_send_field(obj, graphql_args, field_ctx) end query_ctx = field_ctx.query.context - with_filters(obj, ruby_kwargs, query_ctx) do |filtered_obj, filtered_args| - if filtered_args.any? - filtered_obj.public_send(@method_sym, **filtered_args) + with_extensions(obj, ruby_kwargs, query_ctx) do |extended_obj, extended_args| + if extended_args.any? + extended_obj.public_send(@method_sym, **extended_args) else - filtered_obj.public_send(@method_sym) + extended_obj.public_send(@method_sym) end end end # TODO this needs the same kind of work as instrumenters to avoid long, boring stack traces - def with_filters(obj, args, ctx) + def with_extensions(obj, args, ctx) memos = [] - value = run_before_filter(0, memos, obj, args, ctx) { |filtered_obj, filtered_args| yield(filtered_obj, filtered_args) } + value = run_before_extension(0, memos, obj, args, ctx) { |extended_obj, extended_args| yield(extended_obj, extended_args) } ctx.schema.after_lazy(value) do |resolved_value| - run_after_filter(0, memos, obj, args, ctx, resolved_value) + run_after_extension(0, memos, obj, args, ctx, resolved_value) end end - def run_before_filter(idx, memos, obj, args, ctx) - filter = @filters[idx] - if filter - filter.before_resolve(object: obj, arguments: args, context: ctx) do |filtered_obj, filtered_args, memo| + def run_before_extension(idx, memos, obj, args, ctx) + extension = @extensions[idx] + if extension + extension.before_resolve(object: obj, arguments: args, context: ctx) do |extended_obj, extended_args, memo| memos << memo - run_before_filter(idx + 1, memos, filtered_obj, filtered_args, ctx) { |o, a| yield(o, a) } + run_before_extension(idx + 1, memos, extended_obj, extended_args, ctx) { |o, a| yield(o, a) } end else yield(obj, args) end end - def run_after_filter(idx, memos, obj, args, ctx, value) - filter = @filters[idx] - if filter + def run_after_extension(idx, memos, obj, args, ctx, value) + extension = @extensions[idx] + if extension memo = memos[idx] - filtered_value = filter.after_resolve(object: obj, arguments: args, context: ctx, value: value, memo: memo) + extended_value = extension.after_resolve(object: obj, arguments: args, context: ctx, value: value, memo: memo) # TODO after_lazy? - run_after_filter(idx + 1, memos, obj, args, ctx, filtered_value) + run_after_extension(idx + 1, memos, obj, args, ctx, extended_value) else value end diff --git a/lib/graphql/schema/field/connection_filter.rb b/lib/graphql/schema/field/connection_extension.rb similarity index 96% rename from lib/graphql/schema/field/connection_filter.rb rename to lib/graphql/schema/field/connection_extension.rb index b805954c77..21bbeffb10 100644 --- a/lib/graphql/schema/field/connection_filter.rb +++ b/lib/graphql/schema/field/connection_extension.rb @@ -3,7 +3,7 @@ module GraphQL class Schema class Field - class ConnectionFilter < GraphQL::Schema::FieldFilter + class ConnectionExtension < GraphQL::Schema::FieldExtension def initialize(field:, options:) field.argument :after, "String", "Returns the elements in the list that come after the specified cursor.", required: false field.argument :before, "String", "Returns the elements in the list that come before the specified cursor.", required: false diff --git a/lib/graphql/schema/field/scope_filter.rb b/lib/graphql/schema/field/scope_extension.rb similarity index 85% rename from lib/graphql/schema/field/scope_filter.rb rename to lib/graphql/schema/field/scope_extension.rb index 677849eabd..1c95f48979 100644 --- a/lib/graphql/schema/field/scope_filter.rb +++ b/lib/graphql/schema/field/scope_extension.rb @@ -3,7 +3,7 @@ module GraphQL class Schema class Field - class ScopeFilter < GraphQL::Schema::FieldFilter + class ScopeExtension < GraphQL::Schema::FieldExtension def after_resolve(value:, context:, **rest) ret_type = @field.type.unwrap if ret_type.respond_to?(:scope_items) diff --git a/lib/graphql/schema/field_filter.rb b/lib/graphql/schema/field_extension.rb similarity index 82% rename from lib/graphql/schema/field_filter.rb rename to lib/graphql/schema/field_extension.rb index a5be279772..957f84b61e 100644 --- a/lib/graphql/schema/field_filter.rb +++ b/lib/graphql/schema/field_extension.rb @@ -3,24 +3,22 @@ module GraphQL class Schema # Extend this class to make field-level customizations to resolve behavior. # - # When a filter is added to a field with `filter(:my_filter)`, a `MyFilter` instance + # When a extension is added to a field with `extension(MyExtension)`, a `MyExtension` instance # is created, and its hooks are applied whenever that field is called. # # The instance is frozen so that instance variables aren't modified during query execution, # which could cause all kinds of issues due to race conditions. - # - # TODO rename? since it's more than just filter - class FieldFilter + class FieldExtension # @return [GraphQL::Schema::Field] attr_reader :field # @return [Object] attr_reader :options - # Called when the filter is mounted with `filter(name, options)`. + # Called when the extension is mounted with `extension(name, options)`. # The instance is frozen to avoid improper use of state during execution. - # @param field [GraphQL::Schema::Field] The field where this filter was mounted - # @param options [Object] The second argument to `filter`, or `nil` if nothing was passed. + # @param field [GraphQL::Schema::Field] The field where this extension was mounted + # @param options [Object] The second argument to `extension`, or `nil` if nothing was passed. def initialize(field:, options:) @field = field @options = options @@ -35,7 +33,7 @@ def initialize(field:, options:) # @param context [Query::Context] the context for this query # @yieldparam object [Object] The object to continue resolving the field on # @yieldparam arguments [Hash] The keyword arguments to continue resolving with - # @yieldparam memo [Object] Any filter-specific value which will be passed to {#after_resolve} later + # @yieldparam memo [Object] Any extension-specific value which will be passed to {#after_resolve} later def before_resolve(object:, arguments:, context:) yield(object, arguments, nil) end diff --git a/spec/graphql/schema/field_filter_spec.rb b/spec/graphql/schema/field_extension_spec.rb similarity index 74% rename from spec/graphql/schema/field_filter_spec.rb rename to spec/graphql/schema/field_extension_spec.rb index 2ae7fa17d2..02d610be4c 100644 --- a/spec/graphql/schema/field_filter_spec.rb +++ b/spec/graphql/schema/field_extension_spec.rb @@ -1,21 +1,21 @@ # frozen_string_literal: true require "spec_helper" -describe GraphQL::Schema::FieldFilter do +describe GraphQL::Schema::FieldExtension do module FilterTestSchema - class DoubleFilter < GraphQL::Schema::FieldFilter + class DoubleFilter < GraphQL::Schema::FieldExtension def after_resolve(object:, value:, arguments:, context:, memo:) value * 2 end end - class MultiplyByOption < GraphQL::Schema::FieldFilter + class MultiplyByOption < GraphQL::Schema::FieldExtension def after_resolve(object:, value:, arguments:, context:, memo:) value * options[:factor] end end - class MultiplyByArgument < GraphQL::Schema::FieldFilter + class MultiplyByArgument < GraphQL::Schema::FieldExtension def initialize(field:, options:) field.argument(:factor, Integer, required: true) super @@ -36,20 +36,20 @@ class BaseObject < GraphQL::Schema::Object class Query < BaseObject field :doubled, Integer, null: false, method: :pass_thru do - filter(DoubleFilter) + extension(DoubleFilter) argument :input, Integer, required: true end def pass_thru(input:) - input # return it as-is, it will be modified by filters + input # return it as-is, it will be modified by extensions end field :trippled_by_option, Integer, null: false, method: :pass_thru do - filter(MultiplyByOption, factor: 3) + extension(MultiplyByOption, factor: 3) argument :input, Integer, required: true end - field :multiply_input, Integer, null: false, method: :pass_thru, filters: [MultiplyByArgument] do + field :multiply_input, Integer, null: false, method: :pass_thru, extensions: [MultiplyByArgument] do argument :input, Integer, required: true end end @@ -63,6 +63,14 @@ def exec_query(query_str, **kwargs) FilterTestSchema::Schema.execute(query_str, **kwargs) end + describe "reading" do + it "has a reader method" do + field = FilterTestSchema::Query.fields["multiplyInput"] + assert_equal 1, field.extensions.size + assert_instance_of FilterTestSchema::MultiplyByArgument, field.extensions.first + end + end + describe "modifying return values" do it "returns the modified value" do res = exec_query("{ doubled(input: 5) }") From e0598b7b474d3314e53aa3dd8a0b2a98dee58c11 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 10 Aug 2018 17:24:54 -0400 Subject: [PATCH 09/16] Add a guide --- guides/type_definitions/field_extensions.md | 106 ++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 guides/type_definitions/field_extensions.md diff --git a/guides/type_definitions/field_extensions.md b/guides/type_definitions/field_extensions.md new file mode 100644 index 0000000000..3eeabaaae6 --- /dev/null +++ b/guides/type_definitions/field_extensions.md @@ -0,0 +1,106 @@ +--- +layout: guide +doc_stub: false +search: true +section: Type Definitions +title: Field Extensions +desc: Programmatically modify field configuration and resolution +index: 10 +class_based_api: true +--- + +{{ "GraphQL::Schema::FieldExtension" | api_doc }} provides a way to modify user-defined fields in a programmatic way. For example, Relay connections may are implemented as a field extension. + +### Making a new extension + +Field extensions are subclasses of {{ "GraphQL::Schema::FieldExtension" | api_doc }}: + +```ruby +class MyExtension < GraphQL::Schema::FieldExtension +end +``` + +### Using an extension + +Defined extensions can be added to fields using the `extensions: [...]` option or the `extension(...)` method: + +```ruby +field :name, String, null: false, extensions: [UpcaseExtension] +# or: +field :description, String, null: false do + extension(UpcaseExtension) +end +``` + +See below for how extensions may modify fields. + +### Modifying field configuration + +When extensions are attached, they are initialized with a `field:` and `options:`. During `#initialize`, they may extend the field they're attached to. For example: + +```ruby +class SearchableExtension < GraphQL::Schema::FieldExtension + def initialize(field:, options:) + # add an argument to this field: + field.argument(:query, String, required: false, description: "A search query") + # and always call super: + super + end +end +``` + +This way, an extension can encapsulate a behavior requiring several configuration options. + +### Modifying field execution + +Extensions have two hooks that wrap field resolution. Since GraphQL-Ruby supports deferred execution, these hooks _might not_ be called back-to-back. + +First, {{ "GraphQL::Schema::FieldExtension#before_resolve" | api_doc }} is called. `before_resolve` should `yield(object, arguments)` to continue execution. If it doesn't `yield`, then the field won't resolve, and the methods return value will be returned to GraphQL instead. + +After resolution, {{ "GraphQL::Schema::FieldExtension#after_resolve" | api_doc }} is called. Whatever that method returns will be used as the field's return value. + +See the linked API docs for the parameters of those methods. + +#### Execution "memo" + +One parameter to `after_resolve` deserves special attention: `memo:`. `before_resolve` _may_ yield a third value. For example: + +```ruby +def before_resolve(object:, arguments:, **rest) + # yield the current time as `memo` + yield(object, arguments, Time.now.to_i) +end +``` + +If a third value is yielded, it will be passed to `after_resolve` as `memo:`, for example: + +```ruby +def after_resolve(value:, memo:, **rest) + puts "Elapsed: #{Time.now.to_i - memo}" + # Return the original value + value +end +``` + +This allows the `before_resolve` hook to pass data to `after_resolve`. + +Instance variables may not be used because, in a given GraphQL query, the same field may be resolved several times concurrently, and that would result in overriding the instance variable in an unpredictable way. (In fact, extensions are frozen to prevent instance variable writes.) + +### Extension options + +The `extension(...)` method takes an optional second argument, for example: + +```ruby +extension(LimitExtension, limit: 20) +``` + +In this case, `{limit: 20}` will be passed as `options:` to `#initialize` and `options[:limit]` will be `20`. + +For example, options can be used for modifying execution: + +```ruby +def after_resolve(value:, **rest) + # Apply the limit from the options + value.limit(options[:limit]) +end +``` From da7b3dd79a18585e61f5d7fa42475e2dd1459e01 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 10 Aug 2018 17:25:45 -0400 Subject: [PATCH 10/16] Use compatible heredoc --- lib/graphql/schema/field.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index c72d186ad8..46e5a2019f 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -233,7 +233,7 @@ def extensions(new_extensions = nil) @extensions else if @resolve || @function - raise ArgumentError, <<~MSG + raise ArgumentError, <<-MSG Extensions are not supported with resolve procs or functions, but #{owner.name}.#{name} has: #{@resolve || @function} So, it can't have extensions: #{extensions}. From 87065501f8d05f671208bab732c4ecbb249120d5 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 14 Aug 2018 11:16:15 -0400 Subject: [PATCH 11/16] Add #apply hook --- guides/type_definitions/field_extensions.md | 6 ++---- lib/graphql/schema/field_extension.rb | 7 +++++++ spec/graphql/schema/field_extension_spec.rb | 3 +-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/guides/type_definitions/field_extensions.md b/guides/type_definitions/field_extensions.md index 3eeabaaae6..db95dcd15a 100644 --- a/guides/type_definitions/field_extensions.md +++ b/guides/type_definitions/field_extensions.md @@ -36,15 +36,13 @@ See below for how extensions may modify fields. ### Modifying field configuration -When extensions are attached, they are initialized with a `field:` and `options:`. During `#initialize`, they may extend the field they're attached to. For example: +When extensions are attached, they are initialized with a `field:` and `options:`. Then, `#apply` is called, when they may extend the field they're attached to. For example: ```ruby class SearchableExtension < GraphQL::Schema::FieldExtension - def initialize(field:, options:) + def apply # add an argument to this field: field.argument(:query, String, required: false, description: "A search query") - # and always call super: - super end end ``` diff --git a/lib/graphql/schema/field_extension.rb b/lib/graphql/schema/field_extension.rb index 957f84b61e..fa9d923ad8 100644 --- a/lib/graphql/schema/field_extension.rb +++ b/lib/graphql/schema/field_extension.rb @@ -22,9 +22,16 @@ class FieldExtension def initialize(field:, options:) @field = field @options = options + apply freeze end + # Called when this extension is attached to a field. + # The field definition may be extended during this method. + # @return [void] + def apply + end + # Called before resolving {#field}. It should either: # - `yield` values to continue execution; OR # - return something else to shortcut field execution. diff --git a/spec/graphql/schema/field_extension_spec.rb b/spec/graphql/schema/field_extension_spec.rb index 02d610be4c..d96c571078 100644 --- a/spec/graphql/schema/field_extension_spec.rb +++ b/spec/graphql/schema/field_extension_spec.rb @@ -16,9 +16,8 @@ def after_resolve(object:, value:, arguments:, context:, memo:) end class MultiplyByArgument < GraphQL::Schema::FieldExtension - def initialize(field:, options:) + def apply field.argument(:factor, Integer, required: true) - super end def before_resolve(object:, arguments:, context:) From 28b694579114a0ebf0d6df9fceaae51bc47c5f5d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 14 Aug 2018 11:20:39 -0400 Subject: [PATCH 12/16] Update connection extension --- lib/graphql/schema/field/connection_extension.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/graphql/schema/field/connection_extension.rb b/lib/graphql/schema/field/connection_extension.rb index 21bbeffb10..3862c844c1 100644 --- a/lib/graphql/schema/field/connection_extension.rb +++ b/lib/graphql/schema/field/connection_extension.rb @@ -4,12 +4,11 @@ module GraphQL class Schema class Field class ConnectionExtension < GraphQL::Schema::FieldExtension - def initialize(field:, options:) + def apply field.argument :after, "String", "Returns the elements in the list that come after the specified cursor.", required: false field.argument :before, "String", "Returns the elements in the list that come before the specified cursor.", required: false field.argument :first, "Int", "Returns the first _n_ elements from the list.", required: false field.argument :last, "Int", "Returns the last _n_ elements from the list.", required: false - super end # Remove pagination args before passing it to a user method @@ -23,7 +22,6 @@ def before_resolve(object:, arguments:, context:) end def after_resolve(value:, object:, arguments:, context:, memo:) - # TODO this should be extracted away if value.is_a? GraphQL::ExecutionError # This isn't even going to work because context doesn't have ast_node anymore context.add_error(value) From f64e33149447081f176a15549580165d59e2fdc1 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 14 Aug 2018 11:27:05 -0400 Subject: [PATCH 13/16] rewrite with_extensions for an early default case iterative style --- lib/graphql/schema/field.rb | 50 ++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 8191ec8219..5ac83d4ccf 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -471,35 +471,33 @@ def public_send_field(obj, graphql_args, field_ctx) end end - # TODO this needs the same kind of work as instrumenters to avoid long, boring stack traces + # Wrap execution with hooks. + # Written iteratively to avoid big stack traces. + # @return [Object] Whatever the def with_extensions(obj, args, ctx) - memos = [] - value = run_before_extension(0, memos, obj, args, ctx) { |extended_obj, extended_args| yield(extended_obj, extended_args) } - ctx.schema.after_lazy(value) do |resolved_value| - run_after_extension(0, memos, obj, args, ctx, resolved_value) - end - end - - def run_before_extension(idx, memos, obj, args, ctx) - extension = @extensions[idx] - if extension - extension.before_resolve(object: obj, arguments: args, context: ctx) do |extended_obj, extended_args, memo| - memos << memo - run_before_extension(idx + 1, memos, extended_obj, extended_args, ctx) { |o, a| yield(o, a) } - end - else + if @extensions.none? yield(obj, args) - end - end - - def run_after_extension(idx, memos, obj, args, ctx, value) - extension = @extensions[idx] - if extension - memo = memos[idx] - extended_value = extension.after_resolve(object: obj, arguments: args, context: ctx, value: value, memo: memo) - # TODO after_lazy? - run_after_extension(idx + 1, memos, obj, args, ctx, extended_value) else + memos = [] + @extensions.each do |ext| + ext.before_resolve(object: obj, arguments: args, context: ctx) do |extended_obj, extended_args, memo| + # update this scope with the yielded value + obj = extended_obj + args = extended_args + # record the memo (or nil if none was yielded) + memos << memo + end + end + # Call the block which actually calls resolve + value = yield(obj, args) + + ctx.schema.after_lazy(value) do |resolved_value| + @extensions.each_with_index do |ext, idx| + memo = memos[idx] + # TODO after_lazy? + value = ext.after_resolve(object: obj, arguments: args, context: ctx, value: value, memo: memo) + end + end value end end From 333dd6d1ad3f62085896c5a47b4e901142a99e40 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 14 Aug 2018 11:31:30 -0400 Subject: [PATCH 14/16] fix guide typo --- guides/type_definitions/field_extensions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/type_definitions/field_extensions.md b/guides/type_definitions/field_extensions.md index db95dcd15a..de8cdc09de 100644 --- a/guides/type_definitions/field_extensions.md +++ b/guides/type_definitions/field_extensions.md @@ -9,7 +9,7 @@ index: 10 class_based_api: true --- -{{ "GraphQL::Schema::FieldExtension" | api_doc }} provides a way to modify user-defined fields in a programmatic way. For example, Relay connections may are implemented as a field extension. +{{ "GraphQL::Schema::FieldExtension" | api_doc }} provides a way to modify user-defined fields in a programmatic way. For example, Relay connections are implemented as a field extension ({{ "GraphQL::Schema::Field::ConnectionExtension" | api_doc }}). ### Making a new extension From ddee95105d1bb9b00ff64da0fb6436db1d5431bf Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 14 Aug 2018 14:58:50 -0400 Subject: [PATCH 15/16] pass the original obj & args to after_resolve --- lib/graphql/schema/field.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 5ac83d4ccf..4318ea71c6 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -478,6 +478,10 @@ def with_extensions(obj, args, ctx) if @extensions.none? yield(obj, args) else + # Save these so that the originals can be re-given to `after_resolve` handlers. + original_args = args + original_obj = obj + memos = [] @extensions.each do |ext| ext.before_resolve(object: obj, arguments: args, context: ctx) do |extended_obj, extended_args, memo| @@ -495,7 +499,7 @@ def with_extensions(obj, args, ctx) @extensions.each_with_index do |ext, idx| memo = memos[idx] # TODO after_lazy? - value = ext.after_resolve(object: obj, arguments: args, context: ctx, value: value, memo: memo) + value = ext.after_resolve(object: original_obj, arguments: original_args, context: ctx, value: value, memo: memo) end end value From 46db5a99dc6d0fcf1b1516a871a439f292fe4374 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 14 Aug 2018 15:01:55 -0400 Subject: [PATCH 16/16] Return the after-resolve'd value --- lib/graphql/schema/field.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 4318ea71c6..2d54c3846a 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -499,10 +499,10 @@ def with_extensions(obj, args, ctx) @extensions.each_with_index do |ext, idx| memo = memos[idx] # TODO after_lazy? - value = ext.after_resolve(object: original_obj, arguments: original_args, context: ctx, value: value, memo: memo) + resolved_value = ext.after_resolve(object: original_obj, arguments: original_args, context: ctx, value: resolved_value, memo: memo) end + resolved_value end - value end end end