Merge branch 'ajk-graphql-field-extensions-over-instrumentation' into 'master'

Replace calls-gitaly instrumentation with  a field extension [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55306

app/graphql/gitlab_schema.rb

@@ -13,8 +13,6 @@ class GitlabSchema < GraphQL::Schema
   use GraphQL::Pagination::Connections
   use BatchLoader::GraphQL
   use Gitlab::Graphql::Authorize
-  use Gitlab::Graphql::Present
-  use Gitlab::Graphql::CallsGitaly
   use Gitlab::Graphql::Pagination::Connections
   use Gitlab::Graphql::GenericTracing
   use Gitlab::Graphql::Timeout, max_seconds: Gitlab.config.gitlab.graphql_timeout

app/graphql/resolvers/base_resolver.rb

@@ -12,8 +12,17 @@ module Resolvers
       @requires_argument = true
     end
 
+    def self.calls_gitaly!
+      @calls_gitaly = true
+    end
+
     def self.field_options
-      super.merge(requires_argument: @requires_argument)
+      extra_options = {
+        requires_argument: @requires_argument,
+        calls_gitaly: @calls_gitaly
+      }.compact
+
+      super.merge(extra_options)
     end
 
     def self.singular_type

app/graphql/resolvers/last_commit_resolver.rb

@@ -4,6 +4,8 @@ module Resolvers
   class LastCommitResolver < BaseResolver
     type Types::CommitType, null: true
 
+    calls_gitaly!
+
     alias_method :tree, :object
 
     def resolve(**args)

app/graphql/resolvers/metrics/dashboard_resolver.rb

@@ -3,19 +3,30 @@
 module Resolvers
   module Metrics
     class DashboardResolver < Resolvers::BaseResolver
+      type Types::Metrics::DashboardType, null: true
+      calls_gitaly!
+
       argument :path, GraphQL::STRING_TYPE,
                required: true,
-               description: "Path to a file which defines metrics dashboard eg: 'config/prometheus/common_metrics.yml'."
-
-      type Types::Metrics::DashboardType, null: true
+               description: "Path to a file which defines metrics dashboard " \
+                            "eg: 'config/prometheus/common_metrics.yml'."
 
       alias_method :environment, :object
 
       def resolve(**args)
         return unless environment
 
-        ::PerformanceMonitoring::PrometheusDashboard
-          .find_for(project: environment.project, user: context[:current_user], path: args[:path], options: { environment: environment })
+        ::PerformanceMonitoring::PrometheusDashboard.find_for(**args, **service_params)
+      end
+
+      private
+
+      def service_params
+        {
+          project: environment.project,
+          user: current_user,
+          options: { environment: environment }
+        }
       end
     end
   end

app/graphql/resolvers/snippets/blobs_resolver.rb

@@ -8,6 +8,7 @@ module Resolvers
 
       type Types::Snippets::BlobType.connection_type, null: true
       authorize :read_snippet
+      calls_gitaly!
 
       alias_method :snippet, :object
 

app/graphql/resolvers/tree_resolver.rb

@@ -4,6 +4,8 @@ module Resolvers
   class TreeResolver < BaseResolver
     type Types::Tree::TreeType, null: true
 
+    calls_gitaly!
+
     argument :path, GraphQL::STRING_TYPE,
               required: false,
               default_value: '',

app/graphql/types/base_field.rb

@@ -9,16 +9,25 @@ module Types
 
     DEFAULT_COMPLEXITY = 1
 
-    def initialize(*args, **kwargs, &block)
+    def initialize(**kwargs, &block)
       @calls_gitaly = !!kwargs.delete(:calls_gitaly)
-      @constant_complexity = !!kwargs[:complexity]
+      @constant_complexity = kwargs[:complexity].is_a?(Integer) && kwargs[:complexity] > 0
       @requires_argument = !!kwargs.delete(:requires_argument)
       kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity])
       @feature_flag = kwargs[:feature_flag]
       kwargs = check_feature_flag(kwargs)
       kwargs = gitlab_deprecation(kwargs)
 
-      super(*args, **kwargs, &block)
+      super(**kwargs, &block)
+
+      # We want to avoid the overhead of this in prod
+      extension ::Gitlab::Graphql::CallsGitaly::FieldExtension if Gitlab.dev_or_test_env?
+
+      extension ::Gitlab::Graphql::Present::FieldExtension
+    end
+
+    def may_call_gitaly?
+      @constant_complexity || @calls_gitaly
     end
 
     def requires_argument?
@@ -54,8 +63,10 @@ module Types
     end
 
     def check_feature_flag(args)
-      args[:description] = feature_documentation_message(args[:feature_flag], args[:description]) if args[:feature_flag].present?
-      args.delete(:feature_flag)
+      ff = args.delete(:feature_flag)
+      return args unless ff.present?
+
+      args[:description] = feature_documentation_message(ff, args[:description])
 
       args
     end
@@ -78,7 +89,9 @@ module Types
       # items which can be loaded.
       proc do |ctx, args, child_complexity|
         # Resolvers may add extra complexity depending on used arguments
-        complexity = child_complexity + self.resolver&.try(:resolver_complexity, args, child_complexity: child_complexity).to_i
+        complexity = child_complexity + resolver&.try(
+          :resolver_complexity, args, child_complexity: child_complexity
+        ).to_i
         complexity += 1 if calls_gitaly?
         complexity += complexity * connection_complexity_multiplier(ctx, args)
 
@@ -93,7 +106,7 @@ module Types
 
       page_size   = field_defn.connection_max_page_size || ctx.schema.default_max_page_size
       limit_value = [args[:first], args[:last], page_size].compact.min
-      multiplier  = self.resolver&.try(:complexity_multiplier, args).to_f
+      multiplier  = resolver&.try(:complexity_multiplier, args).to_f
       limit_value * multiplier
     end
   end

app/graphql/types/current_user_todos.rb

@@ -16,9 +16,10 @@ module Types
           end
 
     def current_user_todos(state: nil)
-      state ||= %i(done pending) # TodosFinder treats a `nil` state param as `pending`
+      state ||= %i[done pending] # TodosFinder treats a `nil` state param as `pending`
+      klass = unpresented.class
 
-      TodosFinder.new(current_user, state: state, type: object.class.name, target_id: object.id).execute
+      TodosFinder.new(current_user, state: state, type: klass.name, target_id: object.id).execute
     end
   end
 end

app/graphql/types/snippets/blob_type.rb

@@ -14,7 +14,6 @@ module Types
 
       field :plain_data, GraphQL::STRING_TYPE,
             description: 'Blob plain highlighted data.',
-            calls_gitaly: true,
             null: true
 
       field :raw_path, GraphQL::STRING_TYPE,

app/graphql/types/tree/blob_type.rb

@@ -15,6 +15,7 @@ module Types
       field :web_path, GraphQL::STRING_TYPE, null: true,
             description: 'Web path of the blob.'
       field :lfs_oid, GraphQL::STRING_TYPE, null: true,
+            calls_gitaly: true,
             description: 'LFS ID of the blob.'
       field :mode, GraphQL::STRING_TYPE, null: true,
             description: 'Blob mode in numeric format.'

lib/gitlab/graphql/calls_gitaly.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Graphql
-    # Wraps the field resolution to count Gitaly calls before and after.
-    # Raises an error if the field calls Gitaly but hadn't declared such.
-    module CallsGitaly
-      extend ActiveSupport::Concern
-
-      def self.use(schema_definition)
-        schema_definition.instrument(:field, Gitlab::Graphql::CallsGitaly::Instrumentation.new, after_built_ins: true)
-      end
-    end
-  end
-end

lib/gitlab/graphql/calls_gitaly/field_extension.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module CallsGitaly
+      # Check if any `calls_gitaly: true` declarations need to be added
+      #
+      # See BaseField: this extension is not applied if the field does not
+      # need it (i.e. it has a constant complexity or knows that it calls
+      # gitaly)
+      class FieldExtension < ::GraphQL::Schema::FieldExtension
+        include Laziness
+
+        def resolve(object:, arguments:, **rest)
+          yield(object, arguments, [current_gitaly_call_count, accounted_for])
+        end
+
+        def after_resolve(value:, memo:, **rest)
+          (value, count) = value_with_count(value, memo)
+          calls_gitaly_check(count)
+          accounted_for(count)
+
+          value
+        end
+
+        private
+
+        # Resolutions are not nested nicely (due to laziness), so we have to
+        # know not just how many calls were made before resolution started, but
+        # also how many were accounted for by fields with the correct settings
+        # in between.
+        #
+        # e.g. the following is not just plausible, but common:
+        #
+        #   enter A.user (lazy)
+        #   enter A.x
+        #   leave A.x
+        #   enter A.calls_gitaly
+        #   leave A.calls_gitaly (accounts for 1 call)
+        #   leave A.user
+        #
+        # In this circumstance we need to mark the calls made by A.calls_gitaly
+        # as accounted for, even though they were made after we yielded
+        # in A.user
+        def value_with_count(value, (previous_count, previous_accounted_for))
+          newly_accounted_for = accounted_for - previous_accounted_for
+          value = force(value)
+          count = [current_gitaly_call_count - (previous_count + newly_accounted_for), 0].max
+
+          [value, count]
+        end
+
+        def current_gitaly_call_count
+          Gitlab::GitalyClient.get_request_count || 0
+        end
+
+        def calls_gitaly_check(calls)
+          return if calls < 1 || field.may_call_gitaly?
+
+          error = RuntimeError.new(<<~ERROR)
+            #{field_name} unexpectedly calls Gitaly!
+
+            Please either specify a constant complexity or add `calls_gitaly: true`
+            to the field declaration
+          ERROR
+          Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
+        end
+
+        def accounted_for(count = nil)
+          return 0 unless Gitlab::SafeRequestStore.active?
+
+          Gitlab::SafeRequestStore["graphql_gitaly_accounted_for"] ||= 0
+
+          if count.nil?
+            Gitlab::SafeRequestStore["graphql_gitaly_accounted_for"]
+          else
+            Gitlab::SafeRequestStore["graphql_gitaly_accounted_for"] += count
+          end
+        end
+
+        def field_name
+          "#{field.owner.graphql_name}.#{field.graphql_name}"
+        end
+      end
+    end
+  end
+end

lib/gitlab/graphql/calls_gitaly/instrumentation.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Graphql
-    module CallsGitaly
-      class Instrumentation
-        # Check if any `calls_gitaly: true` declarations need to be added
-        # Do nothing if a constant complexity was provided
-        def instrument(_type, field)
-          type_object = field.metadata[:type_class]
-          return field unless type_object.respond_to?(:calls_gitaly?)
-          return field if type_object.constant_complexity? || type_object.calls_gitaly?
-
-          old_resolver_proc = field.resolve_proc
-
-          gitaly_wrapped_resolve = -> (typed_object, args, ctx) do
-            previous_gitaly_call_count = Gitlab::GitalyClient.get_request_count
-            result = old_resolver_proc.call(typed_object, args, ctx)
-            current_gitaly_call_count = Gitlab::GitalyClient.get_request_count
-            calls_gitaly_check(type_object, current_gitaly_call_count - previous_gitaly_call_count)
-            result
-          end
-
-          field.redefine do
-            resolve(gitaly_wrapped_resolve)
-          end
-        end
-
-        def calls_gitaly_check(type_object, calls)
-          return if calls < 1
-
-          # Will inform you if there needs to be `calls_gitaly: true` as a kwarg in the field declaration
-          # if there is at least 1 Gitaly call involved with the field resolution.
-          error = RuntimeError.new("Gitaly is called for field '#{type_object.name}' on #{type_object.owner.try(:name)} - please either specify a constant complexity or add `calls_gitaly: true` to the field declaration")
-          Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
-        end
-      end
-    end
-  end
-end

lib/gitlab/graphql/present.rb

@@ -12,11 +12,30 @@ module Gitlab
         def self.presenter_class
           @presenter_class
         end
+
+        def self.present(object, attrs)
+          klass = @presenter_class
+          return object if !klass || object.is_a?(klass)
+
+          @presenter_class.new(object, **attrs)
+        end
+      end
+
+      def unpresented
+        unwrapped || object
       end
 
-      def self.use(schema_definition)
-        schema_definition.instrument(:field, ::Gitlab::Graphql::Present::Instrumentation.new)
+      def present(object_type, attrs)
+        return unless object_type.respond_to?(:present)
+
+        self.unwrapped ||= object
+        # @object belongs to Schema::Object, which does not expose a writer.
+        @object = object_type.present(unwrapped, attrs) # rubocop: disable Gitlab/ModuleWithInstanceVariables
       end
+
+      private
+
+      attr_accessor :unwrapped
     end
   end
 end

lib/gitlab/graphql/present/field_extension.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module Present
+      class FieldExtension < ::GraphQL::Schema::FieldExtension
+        SAFE_CONTEXT_KEYS = %i[current_user].freeze
+
+        def resolve(object:, arguments:, context:)
+          attrs = safe_context_values(context)
+
+          # We need to handle the object being either a Schema::Object or an
+          # inner Schema::Object#object. This depends on whether the field
+          # has a @resolver_proc or not.
+          if object.is_a?(::Types::BaseObject)
+            object.present(field.owner, attrs)
+            yield(object, arguments)
+          else
+            # This is the legacy code-path, hit if the field has a @resolver_proc
+            # TODO: remove this when resolve procs are removed from the
+            # graphql-ruby library, and all field instrumentation is removed.
+            # See: https://github.com/rmosolgo/graphql-ruby/issues/3385
+            presented = field.owner.try(:present, object, attrs) || object
+            yield(presented, arguments)
+          end
+        end
+
+        private
+
+        def safe_context_values(context)
+          context.to_h.slice(*SAFE_CONTEXT_KEYS)
+        end
+      end
+    end
+  end
+end

lib/gitlab/graphql/present/instrumentation.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Graphql
-    module Present
-      class Instrumentation
-        SAFE_CONTEXT_KEYS = %i[current_user].freeze
-
-        def instrument(type, field)
-          return field unless field.metadata[:type_class]
-
-          presented_in = field.metadata[:type_class].owner
-          return field unless presented_in.respond_to?(:presenter_class)
-          return field unless presented_in.presenter_class
-
-          old_resolver = field.resolve_proc
-
-          resolve_with_presenter = -> (presented_type, args, context) do
-            # We need to wrap the original presentation type into a type that
-            # uses the presenter as an object.
-            object = presented_type.object
-
-            if object.is_a?(presented_in.presenter_class)
-              next old_resolver.call(presented_type, args, context)
-            end
-
-            attrs = safe_context_values(context)
-            presenter = presented_in.presenter_class.new(object, **attrs)
-
-            # we have to use the new `authorized_new` method, as `new` is protected
-            wrapped = presented_type.class.authorized_new(presenter, context)
-
-            old_resolver.call(wrapped, args, context)
-          end
-
-          field.redefine do
-            resolve(resolve_with_presenter)
-          end
-        end
-
-        private
-
-        def safe_context_values(context)
-          context.to_h.slice(*SAFE_CONTEXT_KEYS)
-        end
-      end
-    end
-  end
-end

spec/graphql/gitlab_schema_spec.rb

@@ -18,14 +18,6 @@ RSpec.describe GitlabSchema do
     expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize::Instrumentation))
   end
 
-  it 'enables using presenters' do
-    expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present::Instrumentation))
-  end
-
-  it 'enables using gitaly call checker' do
-    expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::CallsGitaly::Instrumentation))
-  end
-
   it 'has the base mutation' do
     expect(described_class.mutation).to eq(::Types::MutationType)
   end
@@ -47,7 +39,7 @@ RSpec.describe GitlabSchema do
   end
 
   describe '.execute' do
-    context 'for different types of users' do
+    context 'with different types of users' do
       context 'when no context' do
         it 'returns DEFAULT_MAX_COMPLEXITY' do
           expect(GraphQL::Schema)
@@ -78,13 +70,15 @@ RSpec.describe GitlabSchema do
 
       context 'when a logged in user' do
         it 'returns AUTHENTICATED_COMPLEXITY' do
-          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY))
+          expect(GraphQL::Schema).to receive(:execute)
+            .with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY))
 
           described_class.execute('query', context: { current_user: user })
         end
 
         it 'returns AUTHENTICATED_MAX_DEPTH' do
-          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_depth: GitlabSchema::AUTHENTICATED_MAX_DEPTH))
+          expect(GraphQL::Schema).to receive(:execute)
+            .with('query', hash_including(max_depth: GitlabSchema::AUTHENTICATED_MAX_DEPTH))
 
           described_class.execute('query', context: { current_user: user })
         end
@@ -94,7 +88,8 @@ RSpec.describe GitlabSchema do
         it 'returns ADMIN_COMPLEXITY' do
           user = build :user, :admin
 
-          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY))
+          expect(GraphQL::Schema).to receive(:execute)
+            .with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY))
 
           described_class.execute('query', context: { current_user: user })
         end
@@ -130,7 +125,7 @@ RSpec.describe GitlabSchema do
   end
 
   describe '.object_from_id' do
-    context 'for subclasses of `ApplicationRecord`' do
+    context 'with subclasses of `ApplicationRecord`' do
       let_it_be(:user) { create(:user) }
 
       it 'returns the correct record' do
@@ -162,7 +157,7 @@ RSpec.describe GitlabSchema do
       end
     end
 
-    context 'for classes that are not ActiveRecord subclasses and have implemented .lazy_find' do
+    context 'with classes that are not ActiveRecord subclasses and have implemented .lazy_find' do
       it 'returns the correct record' do
         note = create(:discussion_note_on_merge_request)
 
@@ -182,7 +177,7 @@ RSpec.describe GitlabSchema do
       end
     end
 
-    context 'for other classes' do
+    context 'with other classes' do
       # We cannot use an anonymous class here as `GlobalID` expects `.name` not
       # to return `nil`
       before do

spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Gitlab::Graphql::CallsGitaly::FieldExtension, :request_store do
+  include GraphqlHelpers
+
+  let(:field_args) { {} }
+  let(:owner) { fresh_object_type }
+  let(:field) do
+    ::Types::BaseField.new(name: 'value', type: GraphQL::STRING_TYPE, null: true, owner: owner, **field_args)
+  end
+
+  def resolve_value
+    resolve_field(field, { value: 'foo' }, object_type: owner)
+  end
+
+  context 'when the field calls gitaly' do
+    before do
+      owner.define_method :value do
+        Gitlab::SafeRequestStore['gitaly_call_actual'] = 1
+        'fresh-from-the-gitaly-mines!'
+      end
+    end
+
+    context 'when the field has a constant complexity' do
+      let(:field_args) { { complexity: 100 } }
+
+      it 'allows the call' do
+        expect { resolve_value }.not_to raise_error
+      end
+    end
+
+    context 'when the field declares that it calls gitaly' do
+      let(:field_args) { { calls_gitaly: true } }
+
+      it 'allows the call' do
+        expect { resolve_value }.not_to raise_error
+      end
+    end
+
+    context 'when the field does not have these arguments' do
+      let(:field_args) { {} }
+
+      it 'notices, and raises, mentioning the field' do
+        expect { resolve_value }.to raise_error(include('Object.value'))
+      end
+    end
+  end
+
+  context 'when it does not call gitaly' do
+    let(:field_args) { {} }
+
+    it 'does not raise' do
+      value = resolve_value
+
+      expect(value).to eq 'foo'
+    end
+  end
+
+  context 'when some field calls gitaly while we were waiting' do
+    let(:extension) { described_class.new(field: field, options: {}) }
+
+    it 'is acceptable if all are accounted for' do
+      object = :anything
+      arguments = :any_args
+
+      ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 3
+      ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 0
+
+      expect do |b|
+        extension.resolve(object: object, arguments: arguments, &b)
+      end.to yield_with_args(object, arguments, [3, 0])
+
+      ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 13
+      ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 10
+
+      expect { extension.after_resolve(value: 'foo', memo: [3, 0]) }.not_to raise_error
+    end
+
+    it 'is unacceptable if some of the calls are unaccounted for' do
+      ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 10
+      ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 9
+
+      expect { extension.after_resolve(value: 'foo', memo: [0, 0]) }.to raise_error(include('Object.value'))
+    end
+  end
+end

spec/lib/gitlab/graphql/calls_gitaly/instrumentation_spec.rb

-# frozen_string_literal: true
-require 'spec_helper'
-
-RSpec.describe Gitlab::Graphql::CallsGitaly::Instrumentation do
-  subject { described_class.new }
-
-  describe '#calls_gitaly_check' do
-    let(:gitaly_field) { Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, null: true, calls_gitaly: true) }
-    let(:no_gitaly_field) { Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, null: true, calls_gitaly: false) }
-
-    context 'if there are no Gitaly calls' do
-      it 'does not raise an error if calls_gitaly is false' do
-        expect { subject.send(:calls_gitaly_check, no_gitaly_field, 0) }.not_to raise_error
-      end
-    end
-
-    context 'if there is at least 1 Gitaly call' do
-      it 'raises an error if calls_gitaly: is false or not defined' do
-        expect { subject.send(:calls_gitaly_check, no_gitaly_field, 1) }.to raise_error(/specify a constant complexity or add `calls_gitaly: true`/)
-      end
-    end
-  end
-end

spec/lib/gitlab/graphql/present/field_extension_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Gitlab::Graphql::Present::FieldExtension do
+  include GraphqlHelpers
+
+  let_it_be(:user) { create(:user) }
+
+  let(:object) { double(value: 'foo') }
+  let(:owner) { fresh_object_type }
+  let(:field_name) { 'value' }
+  let(:field) do
+    ::Types::BaseField.new(name: field_name, type: GraphQL::STRING_TYPE, null: true, owner: owner)
+  end
+
+  let(:base_presenter) do
+    Class.new(SimpleDelegator) do
+      def initialize(object, **options)
+        super(object)
+        @object = object
+        @options = options
+      end
+    end
+  end
+
+  def resolve_value
+    resolve_field(field, object, current_user: user, object_type: owner)
+  end
+
+  context 'when the object does not declare a presenter' do
+    it 'does not affect normal resolution' do
+      expect(resolve_value).to eq 'foo'
+    end
+  end
+
+  describe 'interactions with inheritance' do
+    def parent
+      type = fresh_object_type('Parent')
+      type.present_using(provide_foo)
+      type.field :foo, ::GraphQL::INT_TYPE, null: true
+      type.field :value, ::GraphQL::STRING_TYPE, null: true
+      type
+    end
+
+    def child
+      type = Class.new(parent)
+      type.graphql_name 'Child'
+      type.present_using(provide_bar)
+      type.field :bar, ::GraphQL::INT_TYPE, null: true
+      type
+    end
+
+    def provide_foo
+      Class.new(base_presenter) do
+        def foo
+          100
+        end
+      end
+    end
+
+    def provide_bar
+      Class.new(base_presenter) do
+        def bar
+          101
+        end
+      end
+    end
+
+    it 'can resolve value, foo and bar' do
+      type = child
+      value = resolve_field(:value, object, object_type: type)
+      foo = resolve_field(:foo, object, object_type: type)
+      bar = resolve_field(:bar, object, object_type: type)
+
+      expect([value, foo, bar]).to eq ['foo', 100, 101]
+    end
+  end
+
+  shared_examples 'calling the presenter method' do
+    it 'calls the presenter method' do
+      expect(resolve_value).to eq presenter.new(object, current_user: user).send(field_name)
+    end
+  end
+
+  context 'when the object declares a presenter' do
+    before do
+      owner.present_using(presenter)
+    end
+
+    context 'when the presenter overrides the original method' do
+      def twice
+        Class.new(base_presenter) do
+          def value
+            @object.value * 2
+          end
+        end
+      end
+
+      let(:presenter) { twice }
+
+      it_behaves_like 'calling the presenter method'
+    end
+
+    # This is exercised here using an explicit `resolve:` proc, but
+    # @resolver_proc values are used in field instrumentation as well.
+    context 'when the field uses a resolve proc' do
+      let(:presenter) { base_presenter }
+      let(:field) do
+        ::Types::BaseField.new(
+          name: field_name,
+          type: GraphQL::STRING_TYPE,
+          null: true,
+          owner: owner,
+          resolve: ->(obj, args, ctx) { 'Hello from a proc' }
+        )
+      end
+
+      specify { expect(resolve_value).to eq 'Hello from a proc' }
+    end
+
+    context 'when the presenter provides a new method' do
+      def presenter
+        Class.new(base_presenter) do
+          def current_username
+            "Hello #{@options[:current_user]&.username} from the presenter!"
+          end
+        end
+      end
+
+      context 'when we select the original field' do
+        it 'is unaffected' do
+          expect(resolve_value).to eq 'foo'
+        end
+      end
+
+      context 'when we select the new field' do
+        let(:field_name) { 'current_username' }
+
+        it_behaves_like 'calling the presenter method'
+      end
+    end
+  end
+end

spec/requests/api/graphql/issue/issue_spec.rb

@@ -8,10 +8,9 @@ RSpec.describe 'Query.issue(id)' do
   let_it_be(:project) { create(:project) }
   let_it_be(:issue) { create(:issue, project: project) }
   let_it_be(:current_user) { create(:user) }
+  let_it_be(:issue_params) { { 'id' => issue.to_global_id.to_s } }
 
   let(:issue_data) { graphql_data['issue'] }
-
-  let_it_be(:issue_params) { { 'id' => issue.to_global_id.to_s } }
   let(:issue_fields) { all_graphql_fields_for('Issue'.classify) }
 
   let(:query) do
@@ -62,7 +61,7 @@ RSpec.describe 'Query.issue(id)' do
       )
     end
 
-    context 'selecting any single field' do
+    context 'when selecting any single field' do
       where(:field) do
         scalar_fields_of('Issue').map { |name| [name] }
       end
@@ -84,13 +83,13 @@ RSpec.describe 'Query.issue(id)' do
       end
     end
 
-    context 'selecting multiple fields' do
+    context 'when selecting multiple fields' do
       let(:issue_fields) { ['title', 'description', 'updatedBy { username }'] }
 
       it 'returns the Issue with the specified fields' do
         post_graphql(query, current_user: current_user)
 
-        expect(issue_data.keys).to eq( %w(title description updatedBy) )
+        expect(issue_data.keys).to eq %w[title description updatedBy]
         expect(issue_data['title']).to eq(issue.title)
         expect(issue_data['description']).to eq(issue.description)
         expect(issue_data['updatedBy']['username']).to eq(issue.author.username)
@@ -110,14 +109,14 @@ RSpec.describe 'Query.issue(id)' do
       it 'returns correct attributes' do
         post_graphql(query, current_user: current_user)
 
-        expect(issue_data.keys).to eq( %w(moved movedTo) )
+        expect(issue_data.keys).to eq %w[moved movedTo]
         expect(issue_data['moved']).to eq(true)
         expect(issue_data['movedTo']['title']).to eq(new_issue.title)
       end
     end
 
     context 'when passed a non-Issue gid' do
-      let(:mr) {create(:merge_request)}
+      let(:mr) { create(:merge_request) }
 
       it 'returns an error' do
         gid = mr.to_global_id.to_s

spec/support/helpers/graphql_helpers.rb

@@ -38,7 +38,10 @@ module GraphqlHelpers
     # All resolution goes through fields, so we need to create one here that
     # uses our resolver. Thankfully, apart from the field name, resolvers
     # contain all the configuration needed to define one.
-    field_options = resolver_class.field_options.merge(name: 'field_value')
+    field_options = resolver_class.field_options.merge(
+      owner: resolver_parent,
+      name: 'field_value'
+    )
     field = ::Types::BaseField.new(**field_options)
 
     # All mutations accept a single `:input` argument. Wrap arguments here.