Convert from GraphQL::Batch to BatchLoader
This commit is contained in:
parent
9c6c17cbcd
commit
287c34ca1f
1
Gemfile
1
Gemfile
|
@ -95,7 +95,6 @@ gem 'rack-cors', '~> 1.0.0', require: 'rack/cors'
|
||||||
|
|
||||||
# GraphQL API
|
# GraphQL API
|
||||||
gem 'graphql', '~> 1.7.14'
|
gem 'graphql', '~> 1.7.14'
|
||||||
gem 'graphql-batch', '~> 0.3.9'
|
|
||||||
gem 'graphql-preload', '~> 2.0.0'
|
gem 'graphql-preload', '~> 2.0.0'
|
||||||
gem 'graphiql-rails', '~> 1.4.10'
|
gem 'graphiql-rails', '~> 1.4.10'
|
||||||
|
|
||||||
|
|
|
@ -1068,7 +1068,6 @@ DEPENDENCIES
|
||||||
grape_logging (~> 1.7)
|
grape_logging (~> 1.7)
|
||||||
graphiql-rails (~> 1.4.10)
|
graphiql-rails (~> 1.4.10)
|
||||||
graphql (~> 1.7.14)
|
graphql (~> 1.7.14)
|
||||||
graphql-batch (~> 0.3.9)
|
|
||||||
graphql-preload (~> 2.0.0)
|
graphql-preload (~> 2.0.0)
|
||||||
grpc (~> 1.11.0)
|
grpc (~> 1.11.0)
|
||||||
haml_lint (~> 0.26.0)
|
haml_lint (~> 0.26.0)
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
Gitlab::Graphql::Authorize.register!
|
Gitlab::Graphql::Authorize.register!
|
||||||
|
|
||||||
GitlabSchema = GraphQL::Schema.define do
|
GitlabSchema = GraphQL::Schema.define do
|
||||||
use GraphQL::Batch
|
use BatchLoader::GraphQL
|
||||||
|
|
||||||
enable_preloading
|
enable_preloading
|
||||||
enable_authorization
|
enable_authorization
|
||||||
|
|
|
@ -1,8 +1,10 @@
|
||||||
# Helper methods for all loaders
|
# Helper methods for all loaders
|
||||||
class Loaders::BaseLoader < GraphQL::Batch::Loader
|
module Loaders::BaseLoader
|
||||||
|
extend ActiveSupport::Concern
|
||||||
|
|
||||||
|
class_methods do
|
||||||
# Convert a class method into a resolver proc. The method should follow the
|
# Convert a class method into a resolver proc. The method should follow the
|
||||||
# (obj, args, ctx) calling convention
|
# (obj, args, ctx) calling convention
|
||||||
class << self
|
|
||||||
def [](sym)
|
def [](sym)
|
||||||
resolver = method(sym)
|
resolver = method(sym)
|
||||||
raise ArgumentError.new("#{self}.#{sym} is not a resolver") unless resolver.arity == 3
|
raise ArgumentError.new("#{self}.#{sym} is not a resolver") unless resolver.arity == 3
|
||||||
|
@ -10,15 +12,4 @@ class Loaders::BaseLoader < GraphQL::Batch::Loader
|
||||||
resolver
|
resolver
|
||||||
end
|
end
|
||||||
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
|
end
|
||||||
|
|
|
@ -1,23 +1,21 @@
|
||||||
class Loaders::FullPathLoader < Loaders::BaseLoader
|
module Loaders::FullPathLoader
|
||||||
|
include Loaders::BaseLoader
|
||||||
|
|
||||||
class << self
|
class << self
|
||||||
def project(obj, args, ctx)
|
def project(obj, args, ctx)
|
||||||
project_by_full_path(args[:full_path])
|
project_by_full_path(args[:full_path])
|
||||||
end
|
end
|
||||||
|
|
||||||
def project_by_full_path(full_path)
|
def project_by_full_path(full_path)
|
||||||
self.for(Project).load(full_path)
|
model_by_full_path(Project, full_path)
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
attr_reader :model
|
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|
|
||||||
def initialize(model)
|
# `with_route` avoids an N+1 calculating full_path
|
||||||
@model = model
|
results = model.where_full_path_in(full_paths).with_route
|
||||||
end
|
results.each { |project| loader.call(project.full_path, project) }
|
||||||
|
end
|
||||||
def perform(keys)
|
end
|
||||||
# `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)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,35 +1,23 @@
|
||||||
class Loaders::IidLoader < Loaders::BaseLoader
|
class Loaders::IidLoader
|
||||||
|
include Loaders::BaseLoader
|
||||||
|
|
||||||
class << self
|
class << self
|
||||||
def merge_request(obj, args, ctx)
|
def merge_request(obj, args, ctx)
|
||||||
iid = args[:iid]
|
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)
|
||||||
promise.then do |project|
|
|
||||||
if project
|
|
||||||
merge_request_by_project_and_iid(project.id, iid)
|
|
||||||
else
|
|
||||||
nil
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def merge_request_by_project_and_iid(project_id, iid)
|
def merge_request_by_project_and_iid(project_loader, iid)
|
||||||
self.for(MergeRequest, target_project_id: project_id.to_s).load(iid.to_s)
|
project_id = project_loader.__sync&.id
|
||||||
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
|
# IIDs are represented as the GraphQL `id` type, which is a string
|
||||||
fulfill_all(relation, keys) { |instance| instance.iid.to_s }
|
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
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -534,9 +534,3 @@
|
||||||
:why: https://github.com/squaremo/bitsyntax-js/blob/master/LICENSE-MIT
|
:why: https://github.com/squaremo/bitsyntax-js/blob/master/LICENSE-MIT
|
||||||
:versions: []
|
:versions: []
|
||||||
:when: 2018-02-20 22:20:25.958123000 Z
|
:when: 2018-02-20 22:20:25.958123000 Z
|
||||||
- promise.rb
|
|
||||||
- Unlicense
|
|
||||||
- :who:
|
|
||||||
:why:
|
|
||||||
:versions: []
|
|
||||||
:when: 2017-09-05 13:10:22.752422892 Z
|
|
||||||
|
|
|
@ -47,6 +47,8 @@ module Gitlab
|
||||||
|
|
||||||
def build_checker(current_user, abilities)
|
def build_checker(current_user, abilities)
|
||||||
proc do |obj|
|
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) }
|
obj if abilities.all? { |ability| Ability.allowed?(current_user, ability, obj) }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -2,7 +2,7 @@ require 'spec_helper'
|
||||||
|
|
||||||
describe GitlabSchema do
|
describe GitlabSchema do
|
||||||
it 'uses batch loading' do
|
it 'uses batch loading' do
|
||||||
expect(described_class.instrumenters[:multiplex]).to include(GraphQL::Batch::SetupMultiplex)
|
expect(field_instrumenters).to include(BatchLoader::GraphQL)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'enables the preload instrumenter' do
|
it 'enables the preload instrumenter' do
|
||||||
|
|
|
@ -24,12 +24,6 @@ describe Loaders::FullPathLoader do
|
||||||
|
|
||||||
expect(result).to be_nil
|
expect(result).to be_nil
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns a promise' do
|
|
||||||
batch do
|
|
||||||
expect(resolve_project(project1.full_path)).to be_a(Promise)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def resolve_project(full_path)
|
def resolve_project(full_path)
|
||||||
|
|
|
@ -50,12 +50,6 @@ describe Loaders::IidLoader do
|
||||||
|
|
||||||
expect(result).to be_nil
|
expect(result).to be_nil
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns a promise' do
|
|
||||||
batch do
|
|
||||||
expect(resolve_mr(full_path, iid_1)).to be_a(Promise)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def resolve_mr(full_path, iid)
|
def resolve_mr(full_path, iid)
|
||||||
|
|
|
@ -4,17 +4,14 @@ module GraphqlHelpers
|
||||||
kls[name].call(obj, args, ctx)
|
kls[name].call(obj, args, ctx)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Runs a block inside a GraphQL::Batch wrapper
|
# Runs a block inside a BatchLoader::Executor wrapper
|
||||||
def batch(max_queries: nil, &blk)
|
def batch(max_queries: nil, &blk)
|
||||||
wrapper = proc do
|
wrapper = proc do
|
||||||
GraphQL::Batch.batch do
|
begin
|
||||||
result = yield
|
BatchLoader::Executor.ensure_current
|
||||||
|
blk.call
|
||||||
if result.is_a?(Array)
|
ensure
|
||||||
Promise.all(result)
|
BatchLoader::Executor.clear_current
|
||||||
else
|
|
||||||
result
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue