diff --git a/app/graphql/resolvers/base_resolver.rb b/app/graphql/resolvers/base_resolver.rb
index cf0642930ad5a3055f0694824e3288044d6c6fb3..7daff68c069a107f88b1a94f82730e22f63b28cd 100644
--- a/app/graphql/resolvers/base_resolver.rb
+++ b/app/graphql/resolvers/base_resolver.rb
@@ -3,27 +3,33 @@
module Resolvers
class BaseResolver < GraphQL::Schema::Resolver
extend ::Gitlab::Utils::Override
+ include ::Gitlab::Utils::StrongMemoize
def self.single
@single ||= Class.new(self) do
+ def ready?(**args)
+ ready, early_return = super
+ [ready, select_result(early_return)]
+ end
+
def resolve(**args)
- super.first
+ select_result(super)
end
def single?
true
end
+
+ def select_result(results)
+ results&.first
+ end
end
end
def self.last
- @last ||= Class.new(self) do
- def resolve(**args)
- super.last
- end
-
- def single?
- true
+ @last ||= Class.new(self.single) do
+ def select_result(results)
+ results&.last
end
end
end
@@ -59,6 +65,17 @@ def object
end
end
+ def synchronized_object
+ strong_memoize(:synchronized_object) do
+ case object
+ when BatchLoader::GraphQL
+ object.sync
+ else
+ object
+ end
+ end
+ end
+
def single?
false
end
diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md
index c8fa90e94d4f1f3b86b82506e9f1ebdb7adac7dc..9ebcd70d1f9be1c1d338a8f2e1b31b54a9de55d8 100644
--- a/doc/development/api_graphql_styleguide.md
+++ b/doc/development/api_graphql_styleguide.md
@@ -619,6 +619,31 @@ lot of dependent objects.
To limit the amount of queries performed, we can use `BatchLoader`.
+### Correct use of `Resolver#ready?`
+
+Resolvers have two public API methods as part of the framework: `#ready?(**args)` and `#resolve(**args)`.
+We can use `#ready?` to perform set-up, validation or early-return without invoking `#resolve`.
+
+Good reasons to use `#ready?` include:
+
+- validating mutually exclusive arguments (see [validating arguments](#validating-arguments))
+- Returning `Relation.none` if we know before-hand that no results are possible
+- Performing setup such as initializing instance variables (although consider lazily initialized methods for this)
+
+Implementations of [`Resolver#ready?(**args)`](https://graphql-ruby.org/api-doc/1.10.9/GraphQL/Schema/Resolver#ready%3F-instance_method) should
+return `(Boolean, early_return_data)` as follows:
+
+```ruby
+def ready?(**args)
+ [false, 'have this instead']
+end
+```
+
+For this reason, whenever you call a resolver (mainly in tests - as framework
+abstractions Resolvers should not be considered re-usable, finders are to be
+preferred), remember to call the `ready?` method and check the boolean flag
+before calling `resolve`! An example can be seen in our [`GraphQLHelpers`](https://gitlab.com/gitlab-org/gitlab/-/blob/2d395f32d2efbb713f7bc861f96147a2a67e92f2/spec/support/helpers/graphql_helpers.rb#L20-27).
+
## Mutations
Mutations are used to change any stored values, or to trigger
@@ -785,7 +810,7 @@ def ready?(**args)
end
# Always remember to call `#super`
- super(args)
+ super
end
```
diff --git a/spec/graphql/resolvers/base_resolver_spec.rb b/spec/graphql/resolvers/base_resolver_spec.rb
index 0a21b2797ee7cac5126c8438dee3286c487967cb..6c384349577ff0b864cadce74dc5c670c3a7d429 100644
--- a/spec/graphql/resolvers/base_resolver_spec.rb
+++ b/spec/graphql/resolvers/base_resolver_spec.rb
@@ -41,9 +41,35 @@ def resolve(**args)
end
end
+ context 'when the resolver returns early' do
+ let(:resolver) do
+ Class.new(described_class) do
+ def ready?(**args)
+ [false, %w(early return)]
+ end
+
+ def resolve(**args)
+ raise 'Should not get here'
+ end
+ end
+ end
+
+ it 'runs correctly in our test framework' do
+ expect(resolve(resolver)).to contain_exactly('early', 'return')
+ end
+
+ it 'single selects the first early return value' do
+ expect(resolve(resolver.single)).to eq('early')
+ end
+
+ it 'last selects the last early return value' do
+ expect(resolve(resolver.last)).to eq('return')
+ end
+ end
+
describe '.last' do
it 'returns a subclass from the resolver' do
- expect(last_resolver.last.superclass).to eq(last_resolver)
+ expect(last_resolver.last.ancestors).to include(last_resolver)
end
it 'returns the same subclass every time' do
@@ -95,4 +121,28 @@ def resolve(**args)
end
end
end
+
+ describe '#synchronized_object' do
+ let(:object) { double(foo: :the_foo) }
+
+ let(:resolver) do
+ Class.new(described_class) do
+ def resolve(**args)
+ [synchronized_object.foo]
+ end
+ end
+ end
+
+ it 'handles raw objects' do
+ expect(resolve(resolver, obj: object)).to contain_exactly(:the_foo)
+ end
+
+ it 'handles lazy objects' do
+ delayed = BatchLoader::GraphQL.for(1).batch do |_, loader|
+ loader.call(1, object)
+ end
+
+ expect(resolve(resolver, obj: delayed)).to contain_exactly(:the_foo)
+ end
+ end
end
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index b3d7f7bcece7a94bcc4c3c23bdba8b5fb6bb25ef..7082424b89977993f041292e8ecaff7290d9808d 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -11,9 +11,19 @@ def self.fieldnamerize(underscored_field_name)
underscored_field_name.to_s.camelize(:lower)
end
- # Run a loader's named resolver
+ # Run a loader's named resolver in a way that closely mimics the framework.
+ #
+ # First the `ready?` method is called. If it turns out that the resolver is not
+ # ready, then the early return is returned instead.
+ #
+ # Then the resolve method is called.
def resolve(resolver_class, obj: nil, args: {}, ctx: {}, field: nil)
- resolver_class.new(object: obj, context: ctx, field: field).resolve(args)
+ resolver = resolver_class.new(object: obj, context: ctx, field: field)
+ ready, early_return = sync_all { resolver.ready?(**args) }
+
+ return early_return unless ready
+
+ resolver.resolve(args)
end
# Eagerly run a loader's named resolver
@@ -51,12 +61,12 @@ def batch(max_queries: nil, &blk)
# BatchLoader::GraphQL returns a wrapper, so we need to :sync in order
# to get the actual values
def batch_sync(max_queries: nil, &blk)
- wrapper = proc do
- lazy_vals = yield
- lazy_vals.is_a?(Array) ? lazy_vals.map { |val| sync(val) } : sync(lazy_vals)
- end
+ batch(max_queries: max_queries) { sync_all(&blk) }
+ end
- batch(max_queries: max_queries, &wrapper)
+ def sync_all(&blk)
+ lazy_vals = yield
+ lazy_vals.is_a?(Array) ? lazy_vals.map { |val| sync(val) } : sync(lazy_vals)
end
def graphql_query_for(name, attributes = {}, fields = nil)
diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb
index 3e2193a9069cec04979d57a9b2f9ea9b67b39711..e204b9ccd68527e0e5e9c0098eefc0dbcae845ba 100644
--- a/spec/support/matchers/graphql_matchers.rb
+++ b/spec/support/matchers/graphql_matchers.rb
@@ -85,9 +85,16 @@ def expected_field_names
RSpec::Matchers.define :have_graphql_arguments do |*expected|
include GraphqlHelpers
+ def expected_names
+ @names ||= Array.wrap(expected).map { |name| GraphqlHelpers.fieldnamerize(name) }
+ end
+
match do |field|
- argument_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) }
- expect(field.arguments.keys).to contain_exactly(*argument_names)
+ expect(field.arguments.keys).to contain_exactly(*expected_names)
+ end
+
+ failure_message do |field|
+ "expected that #{field.name} would have the following fields: #{expected_names.inspect}, but it has #{field.arguments.keys.inspect}."
end
end