summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Kozono <mkozono@gmail.com>2018-06-19 12:32:01 -0700
committerMichael Kozono <mkozono@gmail.com>2018-06-20 10:38:17 -0700
commit7d91984ced810a3fb4e11416c057012fbae687ce (patch)
tree918205e7465291810baad173af1bb61dac3fc818
parent1a426b040f3cef46dd63a54afcb65404b18ebfc6 (diff)
downloadgitlab-ce-7d91984ced810a3fb4e11416c057012fbae687ce.tar.gz
Avoid N+1 queries while processing uploads
-rw-r--r--lib/gitlab/verify/uploads.rb2
-rw-r--r--spec/lib/gitlab/verify/uploads_spec.rb45
2 files changed, 38 insertions, 9 deletions
diff --git a/lib/gitlab/verify/uploads.rb b/lib/gitlab/verify/uploads.rb
index 01f09ab8df7..73fc43cb590 100644
--- a/lib/gitlab/verify/uploads.rb
+++ b/lib/gitlab/verify/uploads.rb
@@ -12,7 +12,7 @@ module Gitlab
private
def all_relation
- Upload.all
+ Upload.all.preload(:model)
end
def local?(upload)
diff --git a/spec/lib/gitlab/verify/uploads_spec.rb b/spec/lib/gitlab/verify/uploads_spec.rb
index 296866d3319..38c30fab1ba 100644
--- a/spec/lib/gitlab/verify/uploads_spec.rb
+++ b/spec/lib/gitlab/verify/uploads_spec.rb
@@ -47,20 +47,49 @@ describe Gitlab::Verify::Uploads do
before do
stub_uploads_object_storage(AvatarUploader)
upload.update!(store: ObjectStorage::Store::REMOTE)
- expect(CarrierWave::Storage::Fog::File).to receive(:new).and_return(file)
end
- it 'passes uploads in object storage that exist' do
- expect(file).to receive(:exists?).and_return(true)
+ describe 'returned hash object' do
+ before do
+ expect(CarrierWave::Storage::Fog::File).to receive(:new).and_return(file)
+ end
- expect(failures).to eq({})
+ it 'passes uploads in object storage that exist' do
+ expect(file).to receive(:exists?).and_return(true)
+
+ expect(failures).to eq({})
+ end
+
+ it 'fails uploads in object storage that do not exist' do
+ expect(file).to receive(:exists?).and_return(false)
+
+ expect(failures.keys).to contain_exactly(upload)
+ expect(failure).to include('Remote object does not exist')
+ end
end
- it 'fails uploads in object storage that do not exist' do
- expect(file).to receive(:exists?).and_return(false)
+ describe 'performance' do
+ before do
+ allow(file).to receive(:exists?)
+ allow(CarrierWave::Storage::Fog::File).to receive(:new).and_return(file)
+ end
+
+ it "avoids N+1 queries" do
+ control_count = ActiveRecord::QueryRecorder.new { perform_task }
+
+ # Create additional uploads in object storage
+ projects = create_list(:project, 3, :with_avatar)
+ uploads = projects.flat_map(&:uploads)
+ uploads.each do |upload|
+ upload.update!(store: ObjectStorage::Store::REMOTE)
+ end
+
+ expect { perform_task }.not_to exceed_query_limit(control_count)
+ end
- expect(failures.keys).to contain_exactly(upload)
- expect(failure).to include('Remote object does not exist')
+ def perform_task
+ described_class.new(batch_size: 100).run_batches { }
+ end
end
end
end