summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--Gemfile1
-rw-r--r--Gemfile.lock1
-rw-r--r--app/graphql/gitlab_schema.rb2
-rw-r--r--app/graphql/loaders/base_loader.rb21
-rw-r--r--app/graphql/loaders/full_path_loader.rb24
-rw-r--r--app/graphql/loaders/iid_loader.rb40
-rw-r--r--config/dependency_decisions.yml6
-rw-r--r--lib/gitlab/graphql/authorize.rb2
-rw-r--r--spec/graphql/gitlab_schema_spec.rb2
-rw-r--r--spec/graphql/loaders/full_path_loader_spec.rb6
-rw-r--r--spec/graphql/loaders/iid_loader_spec.rb6
-rw-r--r--spec/support/helpers/graphql_helpers.rb17
12 files changed, 42 insertions, 86 deletions
diff --git a/Gemfile b/Gemfile
index 134b726c9eb..8f1d8f66f10 100644
--- a/Gemfile
+++ b/Gemfile
@@ -95,7 +95,6 @@ gem 'rack-cors', '~> 1.0.0', require: 'rack/cors'
# GraphQL API
gem 'graphql', '~> 1.7.14'
-gem 'graphql-batch', '~> 0.3.9'
gem 'graphql-preload', '~> 2.0.0'
gem 'graphiql-rails', '~> 1.4.10'
diff --git a/Gemfile.lock b/Gemfile.lock
index 68dd0fba256..8fe2f5f06f8 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1068,7 +1068,6 @@ DEPENDENCIES
grape_logging (~> 1.7)
graphiql-rails (~> 1.4.10)
graphql (~> 1.7.14)
- graphql-batch (~> 0.3.9)
graphql-preload (~> 2.0.0)
grpc (~> 1.11.0)
haml_lint (~> 0.26.0)
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 7392bf6f503..143ccf92c85 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/app/graphql/gitlab_schema.rb
@@ -1,7 +1,7 @@
Gitlab::Graphql::Authorize.register!
GitlabSchema = GraphQL::Schema.define do
- use GraphQL::Batch
+ use BatchLoader::GraphQL
enable_preloading
enable_authorization
diff --git a/app/graphql/loaders/base_loader.rb b/app/graphql/loaders/base_loader.rb
index c32c4daa91a..aad435ea09b 100644
--- a/app/graphql/loaders/base_loader.rb
+++ b/app/graphql/loaders/base_loader.rb
@@ -1,8 +1,10 @@
# Helper methods for all loaders
-class Loaders::BaseLoader < GraphQL::Batch::Loader
- # Convert a class method into a resolver proc. The method should follow the
- # (obj, args, ctx) calling convention
- class << self
+module Loaders::BaseLoader
+ extend ActiveSupport::Concern
+
+ class_methods do
+ # Convert a class method into a resolver proc. The method should follow the
+ # (obj, args, ctx) calling convention
def [](sym)
resolver = method(sym)
raise ArgumentError.new("#{self}.#{sym} is not a resolver") unless resolver.arity == 3
@@ -10,15 +12,4 @@ class Loaders::BaseLoader < GraphQL::Batch::Loader
resolver
end
end
-
- # Fulfill all keys. Pass a block that converts each result into a key.
- # Any keys not in results will be fulfilled with nil.
- def fulfill_all(results, keys, &key_blk)
- results.each do |result|
- key = yield result
- fulfill(key, result)
- end
-
- keys.each { |key| fulfill(key, nil) unless fulfilled?(key) }
- end
end
diff --git a/app/graphql/loaders/full_path_loader.rb b/app/graphql/loaders/full_path_loader.rb
index f99b487ce5d..27ada90b82a 100644
--- a/app/graphql/loaders/full_path_loader.rb
+++ b/app/graphql/loaders/full_path_loader.rb
@@ -1,23 +1,21 @@
-class Loaders::FullPathLoader < Loaders::BaseLoader
+module Loaders::FullPathLoader
+ include Loaders::BaseLoader
+
class << self
def project(obj, args, ctx)
project_by_full_path(args[:full_path])
end
def project_by_full_path(full_path)
- self.for(Project).load(full_path)
+ model_by_full_path(Project, full_path)
end
- end
-
- attr_reader :model
- def initialize(model)
- @model = model
- end
-
- def perform(keys)
- # `with_route` prevents relation.all.map(&:full_path)` from being N+1
- relation = model.where_full_path_in(keys).with_route
- fulfill_all(relation, keys, &:full_path)
+ def model_by_full_path(model, full_path)
+ BatchLoader.for(full_path).batch(key: "#{model.model_name.param_key}:full_path") do |full_paths, loader|
+ # `with_route` avoids an N+1 calculating full_path
+ results = model.where_full_path_in(full_paths).with_route
+ results.each { |project| loader.call(project.full_path, project) }
+ end
+ end
end
end
diff --git a/app/graphql/loaders/iid_loader.rb b/app/graphql/loaders/iid_loader.rb
index e89031da0c2..f9827765a92 100644
--- a/app/graphql/loaders/iid_loader.rb
+++ b/app/graphql/loaders/iid_loader.rb
@@ -1,35 +1,23 @@
-class Loaders::IidLoader < Loaders::BaseLoader
+class Loaders::IidLoader
+ include Loaders::BaseLoader
+
class << self
def merge_request(obj, args, ctx)
iid = args[:iid]
- promise = Loaders::FullPathLoader.project_by_full_path(args[:project])
+ project = Loaders::FullPathLoader.project_by_full_path(args[:project])
+ merge_request_by_project_and_iid(project, iid)
+ end
+
+ def merge_request_by_project_and_iid(project_loader, iid)
+ project_id = project_loader.__sync&.id
- promise.then do |project|
- if project
- merge_request_by_project_and_iid(project.id, iid)
- else
- nil
+ # IIDs are represented as the GraphQL `id` type, which is a string
+ BatchLoader.for(iid.to_s).batch(key: "merge_request:target_project:#{project_id}:iid") do |iids, loader|
+ if project_id
+ results = MergeRequest.where(target_project_id: project_id, iid: iids)
+ results.each { |mr| loader.call(mr.iid.to_s, mr) }
end
end
end
-
- def merge_request_by_project_and_iid(project_id, iid)
- self.for(MergeRequest, target_project_id: project_id.to_s).load(iid.to_s)
- end
- end
-
- attr_reader :model, :restrictions
-
- def initialize(model, restrictions = {})
- @model = model
- @restrictions = restrictions
- end
-
- def perform(keys)
- relation = model.where(iid: keys)
- relation = relation.where(restrictions) if restrictions.present?
-
- # IIDs are represented as the GraphQL `id` type, which is a string
- fulfill_all(relation, keys) { |instance| instance.iid.to_s }
end
end
diff --git a/config/dependency_decisions.yml b/config/dependency_decisions.yml
index 55a8c9dac9b..6616b85129e 100644
--- a/config/dependency_decisions.yml
+++ b/config/dependency_decisions.yml
@@ -534,9 +534,3 @@
:why: https://github.com/squaremo/bitsyntax-js/blob/master/LICENSE-MIT
:versions: []
:when: 2018-02-20 22:20:25.958123000 Z
- - promise.rb
- - Unlicense
- - :who:
- :why:
- :versions: []
- :when: 2017-09-05 13:10:22.752422892 Z
diff --git a/lib/gitlab/graphql/authorize.rb b/lib/gitlab/graphql/authorize.rb
index 1df130e519a..6f9de8c38b7 100644
--- a/lib/gitlab/graphql/authorize.rb
+++ b/lib/gitlab/graphql/authorize.rb
@@ -47,6 +47,8 @@ module Gitlab
def build_checker(current_user, abilities)
proc do |obj|
+ # Load the elements if they weren't loaded by BatchLoader yet
+ obj = obj.sync if obj.respond_to?(:sync)
obj if abilities.all? { |ability| Ability.allowed?(current_user, ability, obj) }
end
end
diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb
index 3582f297866..070b851b109 100644
--- a/spec/graphql/gitlab_schema_spec.rb
+++ b/spec/graphql/gitlab_schema_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
describe GitlabSchema do
it 'uses batch loading' do
- expect(described_class.instrumenters[:multiplex]).to include(GraphQL::Batch::SetupMultiplex)
+ expect(field_instrumenters).to include(BatchLoader::GraphQL)
end
it 'enables the preload instrumenter' do
diff --git a/spec/graphql/loaders/full_path_loader_spec.rb b/spec/graphql/loaders/full_path_loader_spec.rb
index 2a473239550..2732dd8c9da 100644
--- a/spec/graphql/loaders/full_path_loader_spec.rb
+++ b/spec/graphql/loaders/full_path_loader_spec.rb
@@ -24,12 +24,6 @@ describe Loaders::FullPathLoader do
expect(result).to be_nil
end
-
- it 'returns a promise' do
- batch do
- expect(resolve_project(project1.full_path)).to be_a(Promise)
- end
- end
end
def resolve_project(full_path)
diff --git a/spec/graphql/loaders/iid_loader_spec.rb b/spec/graphql/loaders/iid_loader_spec.rb
index 8a0c1f0791a..0a57d7c4ed4 100644
--- a/spec/graphql/loaders/iid_loader_spec.rb
+++ b/spec/graphql/loaders/iid_loader_spec.rb
@@ -50,12 +50,6 @@ describe Loaders::IidLoader do
expect(result).to be_nil
end
-
- it 'returns a promise' do
- batch do
- expect(resolve_mr(full_path, iid_1)).to be_a(Promise)
- end
- end
end
def resolve_mr(full_path, iid)
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index 5bb2cf9dd9e..1eaa7603af0 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -4,20 +4,17 @@ module GraphqlHelpers
kls[name].call(obj, args, ctx)
end
- # Runs a block inside a GraphQL::Batch wrapper
+ # Runs a block inside a BatchLoader::Executor wrapper
def batch(max_queries: nil, &blk)
wrapper = proc do
- GraphQL::Batch.batch do
- result = yield
-
- if result.is_a?(Array)
- Promise.all(result)
- else
- result
- end
+ begin
+ BatchLoader::Executor.ensure_current
+ blk.call
+ ensure
+ BatchLoader::Executor.clear_current
end
end
-
+
if max_queries
result = nil
expect { result = wrapper.call }.not_to exceed_query_limit(max_queries)