summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2019-04-30 07:55:48 +0000
committerSean McGivern <sean@gitlab.com>2019-04-30 07:55:48 +0000
commitc10d009191b379078d28a7cba5fed4060aa346c8 (patch)
treef1d9fee213ea95e3b0921ec9eedc790ec764acb7
parentcc2c513bd5c270212035d0c5d8a00250d7aca05c (diff)
parent25818bd7ae765422c934d0a32efb4ba353d11183 (diff)
downloadgitlab-ce-c10d009191b379078d28a7cba5fed4060aa346c8.tar.gz
Merge branch 'sh-disable-batch-load-replace-methods' into 'master'
Disable method replacement in avatar loading Closes #60903 See merge request gitlab-org/gitlab-ce!27866
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock4
-rw-r--r--app/models/concerns/avatarable.rb3
-rw-r--r--changelogs/unreleased/sh-disable-batch-load-replace-methods.yml5
-rw-r--r--spec/uploaders/object_storage_spec.rb8
5 files changed, 18 insertions, 4 deletions
diff --git a/Gemfile b/Gemfile
index 95d65ec30c7..e075e2478a8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -285,7 +285,7 @@ gem 'gettext_i18n_rails', '~> 1.8.0'
gem 'gettext_i18n_rails_js', '~> 1.3'
gem 'gettext', '~> 3.2.2', require: false, group: :development
-gem 'batch-loader', '~> 1.2.2'
+gem 'batch-loader', '~> 1.4.0'
# Perf bar
gem 'peek', '~> 1.0.1'
diff --git a/Gemfile.lock b/Gemfile.lock
index c08f0c24ba6..c5ad2357434 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -76,7 +76,7 @@ GEM
thread_safe (~> 0.3, >= 0.3.1)
babosa (1.0.2)
base32 (0.3.2)
- batch-loader (1.2.2)
+ batch-loader (1.4.0)
bcrypt (3.1.12)
bcrypt_pbkdf (1.0.0)
benchmark-ips (2.3.0)
@@ -999,7 +999,7 @@ DEPENDENCIES
awesome_print
babosa (~> 1.0.2)
base32 (~> 0.3.0)
- batch-loader (~> 1.2.2)
+ batch-loader (~> 1.4.0)
bcrypt_pbkdf (~> 1.0)
benchmark-ips (~> 2.3.0)
better_errors (~> 2.5.0)
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index 4687ec7d166..80278e07e65 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -91,7 +91,8 @@ module Avatarable
private
def retrieve_upload_from_batch(identifier)
- BatchLoader.for(identifier: identifier, model: self).batch(key: self.class) do |upload_params, loader, args|
+ BatchLoader.for(identifier: identifier, model: self)
+ .batch(key: self.class, cache: true, replace_methods: false) do |upload_params, loader, args|
model_class = args[:key]
paths = upload_params.flat_map do |params|
params[:model].upload_paths(params[:identifier])
diff --git a/changelogs/unreleased/sh-disable-batch-load-replace-methods.yml b/changelogs/unreleased/sh-disable-batch-load-replace-methods.yml
new file mode 100644
index 00000000000..00f897ac4b1
--- /dev/null
+++ b/changelogs/unreleased/sh-disable-batch-load-replace-methods.yml
@@ -0,0 +1,5 @@
+---
+title: Disable method replacement in avatar loading
+merge_request: 27866
+author:
+type: performance
diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb
index 9ce9a353913..a62830c35f1 100644
--- a/spec/uploaders/object_storage_spec.rb
+++ b/spec/uploaders/object_storage_spec.rb
@@ -771,6 +771,14 @@ describe ObjectStorage do
expect { avatars }.not_to exceed_query_limit(1)
end
+ it 'does not attempt to replace methods' do
+ models.each do |model|
+ expect(model.avatar.upload).to receive(:method_missing).and_call_original
+
+ model.avatar.upload.path
+ end
+ end
+
it 'fetches a unique upload for each model' do
expect(avatars.map(&:url).uniq).to eq(avatars.map(&:url))
expect(avatars.map(&:upload).uniq).to eq(avatars.map(&:upload))