adapt to the new cops

app/models/lfs_object.rb

@@ -7,7 +7,7 @@ class LfsObject < ActiveRecord::Base
 
   validates :oid, presence: true, uniqueness: true
 
-  scope :with_files_stored_locally, ->() { where(file_store: [nil, LfsObjectUploader::Store::LOCAL]) }
+  scope :with_files_stored_locally, -> { where(file_store: [nil, LfsObjectUploader::Store::LOCAL]) }
 
   mount_uploader :file, LfsObjectUploader
 

app/models/upload.rb

@@ -17,16 +17,13 @@ class Upload < ActiveRecord::Base
   end
 
   def self.record(uploader)
-    upload = uploader.upload&.dup || new
-
-    upload.update_attributes(
+    create(
       size: uploader.file.size,
       path: uploader.upload_path,
       model: uploader.model,
       uploader: uploader.class.to_s,
       store: uploader.try(:object_store) || ObjectStorage::Store::LOCAL
     )
-    upload
   end
 
   def self.hexdigest(path)

app/uploaders/file_uploader.rb

@@ -16,13 +16,7 @@ class FileUploader < GitlabUploader
   DYNAMIC_PATH_PATTERN = %r{(?<secret>\h{32})/(?<identifier>.*)}
 
   attr_accessor :model
-  attr_reader :secret
 
-  # TODO: remove this, FileUploader should not have storage_options, this class
-  # should be abstract, or even a Concern that simply add the secret
-  #
-  # Then create a new AdhocUploader that implement the base_dir logic of this class,
-  # which is wrong anyways.
   storage_options Gitlab.config.uploads
 
   def self.root
@@ -32,7 +26,7 @@ class FileUploader < GitlabUploader
   def self.absolute_path(upload)
     File.join(
       absolute_base_dir(upload.model),
-      upload.path # this already contain the dynamic_segment, see #upload_path
+      upload.path # already contain the dynamic_segment, see #upload_path
     )
   end
 
@@ -40,7 +34,7 @@ class FileUploader < GitlabUploader
     model_path_segment(model)
   end
 
-  # this is used in migrations and import/exports
+  # used in migrations and import/exports
   def self.absolute_base_dir(model)
     File.join(root, base_dir(model))
   end

ee/app/uploaders/object_storage.rb

@@ -59,8 +59,6 @@ module ObjectStorage
       before :store, :verify_license!
     end
 
-    attr_reader :object_store
-
     class_methods do
       def object_store_options
         storage_options&.object_store
@@ -99,10 +97,12 @@ module ObjectStorage
       @object_store ||= model.try(store_serialization_column) || Store::LOCAL
     end
 
+    # rubocop:disable Gitlab/ModuleWithInstanceVariables
     def object_store=(value)
       @object_store = value || Store::LOCAL
-      @storage = storage_for(@object_store)
+      @storage = storage_for(object_store)
     end
+    # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
     # Return true if the current file is part or the model (i.e. is mounted in the model)
     #
@@ -114,7 +114,7 @@ module ObjectStorage
     def persist_object_store!
       return unless persist_object_store?
 
-      updated = model.update_column(store_serialization_column, @object_store)
+      updated = model.update_column(store_serialization_column, object_store)
       raise ActiveRecordError unless updated
     end
 
@@ -145,21 +145,17 @@ module ObjectStorage
       return unless file
 
       file_to_delete = file
-      self.object_store = new_store # this changes the storage and file
+      self.object_store = new_store # changes the storage and file
 
       cache_stored_file! if file_storage?
 
       with_callbacks(:store, file_to_delete) do # for #store_versions!
         storage.store!(file).tap do |new_file|
-          @file = new_file
           begin
-            # Triggering a model.save! will cause the new_file to be deleted.
-            # I still need to investigate exactly why, but this seems like a weird interaction
-            # between activerecord and carrierwave
+            retrieve_from_store!(identifier)
             persist_object_store!
             file_to_delete.delete if new_file.exists?
           rescue => e
-            # since we change storage store the new storage
             # in case of failure delete new file
             new_file.delete
             raise e
@@ -272,7 +268,7 @@ module ObjectStorage
       # To be safe, keep this directory outside of the the cache directory
       # because calling CarrierWave.clean_cache_files! will remove any files in
       # the cache directory.
-      File.join(work_dir, @cache_id, version_name.to_s, for_file)
+      File.join(work_dir, cache_id, version_name.to_s, for_file)
     end
   end
 end

spec/support/shared_examples/uploaders/object_storage_shared_examples.rb

@@ -4,44 +4,51 @@ shared_context 'with storage' do |store, **stub_params|
   end
 end
 
+shared_examples "matches the method pattern" do |method|
+  let(:target) { subject }
+  let(:args) { nil }
+  let(:pattern) { patterns[method] }
+
+  it do
+    return skip "No pattern provided, skipping." unless pattern
+
+    expect(target.method(method).call(*args)).to match(pattern)
+  end
+end
+
 shared_examples "builds correct paths" do |**patterns|
+  let(:patterns) { patterns }
+
   before do
     allow(subject).to receive(:filename).and_return('<filename>')
   end
 
   describe "#store_dir" do
-    it "matches the pattern" do
-      expect(subject.store_dir).to match(patterns[:store_dir])
-    end
-  end if patterns.has_key?(:store_dir)
+    it_behaves_like "matches the method pattern", :store_dir
+  end
 
   describe "#cache_dir" do
-    it "matches the pattern" do
-      expect(subject.cache_dir).to match(patterns[:cache_dir])
-    end
-  end if patterns.has_key?(:cache_dir)
+    it_behaves_like "matches the method pattern", :cache_dir
+  end
 
   describe "#work_dir" do
-    it "matches the pattern" do
-      expect(subject.work_dir).to match(patterns[:work_dir])
-    end
-  end if patterns.has_key?(:work_dir)
+    it_behaves_like "matches the method pattern", :work_dir
+  end
 
   describe "#upload_path" do
-    it "matches the pattern" do
-      expect(subject.upload_path).to match(patterns[:upload_path])
-    end
-  end if patterns.has_key?(:upload_path)
+    it_behaves_like "matches the method pattern", :upload_path
+  end
 
   describe ".absolute_path" do
-    it "matches the pattern" do
-      expect(subject.class.absolute_path(upload)).to match(patterns[:absolute_path])
+    it_behaves_like "matches the method pattern", :absolute_path do
+      let(:target) { subject.class }
+      let(:args) { [upload] }
     end
-  end if patterns.has_key?(:absolute_path)
+  end
 
   describe ".base_dir" do
-    it "matches the pattern" do
-      expect(subject.class.base_dir).to match(patterns[:base_dir])
+    it_behaves_like "matches the method pattern", :base_dir do
+      let(:target) { subject.class }
     end
-  end if patterns.has_key?(:base_dir)
+  end
 end