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

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'

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)

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])

changelogs/unreleased/sh-disable-batch-load-replace-methods.yml

+---
+title: Disable method replacement in avatar loading
+merge_request: 27866
+author:
+type: performance

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))