Commit 05958f8c authored by Kerri Miller's avatar Kerri Miller

Merge branch 'ajk-31213-grouped-batchloader' into 'master'

Increase graphql batchloading efficiency

See merge request gitlab-org/gitlab!46919
parents 175d0cd7 45af0d65
...@@ -12,14 +12,11 @@ module Gitlab ...@@ -12,14 +12,11 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find def find
BatchLoader::GraphQL.for({ model: model_class, id: model_id.to_i }).batch do |loader_info, loader| BatchLoader::GraphQL.for(model_id.to_i).batch(key: model_class) do |ids, loader, args|
per_model = loader_info.group_by { |info| info[:model] } model = args[:key]
per_model.each do |model, info|
ids = info.map { |i| i[:id] }
results = model.where(id: ids) results = model.where(id: ids)
results.each { |record| loader.call({ model: model, id: record.id }, record) } results.each { |record| loader.call(record.id, record) }
end
end end
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -4,8 +4,9 @@ require 'spec_helper' ...@@ -4,8 +4,9 @@ require 'spec_helper'
RSpec.describe Gitlab::Graphql::Loaders::BatchModelLoader do RSpec.describe Gitlab::Graphql::Loaders::BatchModelLoader do
describe '#find' do describe '#find' do
let(:issue) { create(:issue) } let_it_be(:issue) { create(:issue) }
let(:user) { create(:user) } let_it_be(:other_user) { create(:user) }
let_it_be(:user) { create(:user) }
it 'finds a model by id' do it 'finds a model by id' do
issue_result = described_class.new(Issue, issue.id).find issue_result = described_class.new(Issue, issue.id).find
...@@ -16,15 +17,25 @@ RSpec.describe Gitlab::Graphql::Loaders::BatchModelLoader do ...@@ -16,15 +17,25 @@ RSpec.describe Gitlab::Graphql::Loaders::BatchModelLoader do
end end
it 'only queries once per model' do it 'only queries once per model' do
other_user = create(:user)
user
issue
expect do expect do
[described_class.new(User, other_user.id).find, [described_class.new(User, other_user.id).find,
described_class.new(User, user.id).find, described_class.new(User, user.id).find,
described_class.new(Issue, issue.id).find].map(&:sync) described_class.new(Issue, issue.id).find].map(&:sync)
end.not_to exceed_query_limit(2) end.not_to exceed_query_limit(2)
end end
it 'does not force values unnecessarily' do
expect do
a = described_class.new(User, user.id).find
b = described_class.new(Issue, issue.id).find
b.sync
c = described_class.new(User, other_user.id).find
a.sync
c.sync
end.not_to exceed_query_limit(2)
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment