GraphQL::Schema::FieldExtension should not modify options

[gael-ian]

Describe the bug

We need to apply the same extension options to several fields. To reduce duplication, we'd like to use the .with_options method provided by ActiveSupport on any Object but it does not work as expected.

Steps to reproduce

Imagine we have something like that:

# May come from an external gem
class Extension < ::GraphQL::Schema::FieldExtension
  def apply = @name = options.delete :name
  def resolve(context:, object:, arguments:, **_rest) = yield object, arguments, name:
end

class Field < ::GraphQL::Schema::Field
  def initialize(...)
    super
    extension(Extension)
  end
end

class Mutation < Types::Objects::Base
  field_class Field

  with_options extension_options: { name: :value } do
    field :record_create, mutation: Records::Create
    field :record_update, mutation: Records::Update
  end
end

In GraphQL::Schema::FieldExtension, the constructor simply assign options to the extension. If the extension manipulate its options Hash (with #merge or #delete), it will also be altered for subsequent calls. In case of a #delete, later fields will not be extended.

Expected behavior

with_options should work as expected and actually forward options to every .field call.

Actual behavior

with_options works as expected but field extensions may consume options in a destructive way.

This can be fixed with a single line change as:

module GraphQL
  class Schema
    class FieldExtension
      # […]
      def initialize(field:, options:)
        @field = field
-       @options = options || {}
+       @options = options&.dup || {}
        @added_default_arguments = nil
        apply
      end
      # […]
    end
  end
end

[rmosolgo]

Hi!

I don't want to add a .dup to the default behavior because most of the time, it will be a needless allocation. I'd rather keep the default path as efficient as I can.

Instead, you might:

• Implement def initialize to copy the incoming options, as you've suggested above;
• Or, modify def apply not to modify options, eg @name = options[:name] instead
• Or, pass fresh options to each extensions ... call instead of reusing the same Hash.

I hope one of those options works for you!

[gael-ian]

Hi @rmosolgo

And thank you for your answer.

Sadly, the first two solutions can't be applied to field extensions provided by an external gem (without monky-patching them) but I understand your point.

Thank you