Handle assignments in an ActiveModel::Dirty-friendly way

lib/carrierwave/mount.rb

@@ -132,7 +132,7 @@ def uploader_option(column, option)
     #     end
     #
     def mount_uploader(column, uploader=nil, options={}, &block)
-      mount_base(column, uploader, options, &block)
+      mount_base(column, uploader, options.merge(multiple: false), &block)
 
       mod = Module.new
       include mod
@@ -170,14 +170,6 @@ def remote_#{column}_request_header=(header)
           _mounter(:#{column}).remote_request_headers = [header]
         end
 
-        def write_#{column}_identifier
-          return if frozen?
-          mounter = _mounter(:#{column})
-
-          mounter.clear! if mounter.remove?
-          write_uploader(mounter.serialization_column, mounter.identifiers.first)
-        end
-
         def #{column}_identifier
           _mounter(:#{column}).read_identifiers[0]
         end
@@ -306,7 +298,7 @@ def remove_rolled_back_#{column}
     #     end
     #
     def mount_uploaders(column, uploader=nil, options={}, &block)
-      mount_base(column, uploader, options, &block)
+      mount_base(column, uploader, options.merge(multiple: true), &block)
 
       mod = Module.new
       include mod
@@ -345,14 +337,6 @@ def remote_#{column}_request_headers=(headers)
           _mounter(:#{column}).remote_request_headers = headers
         end
 
-        def write_#{column}_identifier
-          return if frozen?
-          mounter = _mounter(:#{column})
-
-          mounter.clear! if mounter.remove?
-          write_uploader(mounter.serialization_column, mounter.identifiers.presence)
-        end
-
         def #{column}_identifiers
           _mounter(:#{column}).read_identifiers
         end
@@ -439,6 +423,10 @@ def #{column}_download_errors
           _mounter(:#{column}).download_errors
         end
 
+        def write_#{column}_identifier
+          _mounter(:#{column}).write_identifier
+        end
+
         def mark_remove_#{column}_false
           _mounter(:#{column}).remove = false
         end
@@ -474,9 +462,9 @@ def write_uploader(column, identifier); end
 
       def _mounter(column)
         # We cannot memoize in frozen objects :(
-        return Mounter.new(self, column) if frozen?
+        return Mounter.build(self, column) if frozen?
         @_mounters ||= {}
-        @_mounters[column] ||= Mounter.new(self, column)
+        @_mounters[column] ||= Mounter.build(self, column)
       end
 
     end # Extension

lib/carrierwave/mounter.rb

@@ -3,11 +3,39 @@ module CarrierWave
   # this is an internal class, used by CarrierWave::Mount so that
   # we don't pollute the model with a lot of methods.
   class Mounter # :nodoc:
-    attr_reader :column, :record, :remote_urls, :integrity_errors,
-                :processing_errors, :download_errors
-    attr_accessor :remove, :remote_request_headers, :uploader_options
+    class Single < Mounter # :nodoc
+      def identifier
+        uploaders.first&.identifier
+      end
+
+      def temporary_identifier
+        temporary_identifiers.first
+      end
+    end
+
+    class Multiple < Mounter # :nodoc
+      def identifier
+        uploaders.map(&:identifier).presence
+      end
+
+      def temporary_identifier
+        temporary_identifiers.presence
+      end
+    end
+
+    def self.build(record, column)
+      if record.class.uploader_options[column][:multiple]
+        Multiple.new(record, column)
+      else
+        Single.new(record, column)
+      end
+    end
 
-    def initialize(record, column, options={})
+    attr_reader :column, :record, :remote_urls, :remove,
+                :integrity_errors, :processing_errors, :download_errors
+    attr_accessor :remote_request_headers, :uploader_options
+
+    def initialize(record, column)
       @record = record
       @column = column
       @options = record.class.uploader_options[column]
@@ -65,6 +93,7 @@ def cache(new_files)
           end
         end
       end.compact
+      write_temporary_identifier
     end
 
     def cache_names
@@ -82,6 +111,7 @@ def cache_names=(cache_names)
       rescue CarrierWave::InvalidParameter
         # ignore
       end
+      write_temporary_identifier
     end
 
     def remote_urls=(urls)
@@ -101,12 +131,20 @@ def remote_urls=(urls)
           @uploaders << uploader
         end
       end
+      write_temporary_identifier
     end
 
     def store!
       uploaders.reject(&:blank?).each(&:store!)
     end
 
+    def write_identifier
+      return if record.frozen?
+
+      clear! if remove?
+      record.write_uploader(serialization_column, identifier)
+    end
+
     def urls(*args)
       uploaders.map { |u| u.url(*args) }
     end
@@ -115,6 +153,11 @@ def blank?
       uploaders.none?(&:present?)
     end
 
+    def remove=(value)
+      @remove = value
+      write_temporary_identifier
+    end
+
     def remove?
       remove.present? && (remove == true || remove !~ /\A0|false$\z/)
     end
@@ -185,5 +228,19 @@ def handle_error
       @integrity_errors << e
       raise e unless option(:ignore_integrity_errors)
     end
+
+    def write_temporary_identifier
+      return if record.frozen?
+
+      record.write_uploader(serialization_column, temporary_identifier)
+    end
+
+    def temporary_identifiers
+      if remove?
+        []
+      else
+        uploaders.map { |uploader| uploader.temporary_identifier }
+      end
+    end
   end # Mounter
 end # CarrierWave

lib/carrierwave/orm/activerecord.rb

@@ -6,40 +6,6 @@ module ActiveRecord
 
     include CarrierWave::Mount
 
-    ##
-    # See +CarrierWave::Mount#mount_uploader+ for documentation
-    #
-    def mount_uploader(column, uploader=nil, options={}, &block)
-      super
-
-      mod = Module.new
-      prepend mod
-      mod.class_eval <<-RUBY, __FILE__, __LINE__+1
-        def remote_#{column}_url=(url)
-          column = _mounter(:#{column}).serialization_column
-          __send__(:"\#{column}_will_change!")
-          super
-        end
-      RUBY
-    end
-
-    ##
-    # See +CarrierWave::Mount#mount_uploaders+ for documentation
-    #
-    def mount_uploaders(column, uploader=nil, options={}, &block)
-      super
-
-      mod = Module.new
-      prepend mod
-      mod.class_eval <<-RUBY, __FILE__, __LINE__+1
-        def remote_#{column}_urls=(url)
-          column = _mounter(:#{column}).serialization_column
-          __send__(:"\#{column}_will_change!")
-          super
-        end
-      RUBY
-    end
-
   private
 
     def mount_base(column, uploader=nil, options={}, &block)
@@ -69,33 +35,6 @@ def mount_base(column, uploader=nil, options={}, &block)
       mod = Module.new
       prepend mod
       mod.class_eval <<-RUBY, __FILE__, __LINE__+1
-        def #{column}=(new_file)
-          column = _mounter(:#{column}).serialization_column
-          if !(new_file.blank? && __send__(:#{column}).blank?)
-            __send__(:"\#{column}_will_change!")
-          end
-
-          super
-        end
-
-        def #{column}_cache=(cache_name)
-          column = _mounter(:#{column}).serialization_column
-          __send__(:"\#{column}_will_change!") if cache_name.present?
-          super
-        end
-
-        def remove_#{column}=(value)
-          column = _mounter(:#{column}).serialization_column
-          result = super
-          __send__(:"\#{column}_will_change!") if _mounter(:#{column}).remove?
-          result
-        end
-
-        def write_#{column}_identifier
-          return unless has_attribute?(_mounter(:#{column}).serialization_column)
-          super
-        end
-
         # Reset cached mounter on record reload
         def reload(*)
           @_mounters = nil
@@ -109,6 +48,11 @@ def initialize_dup(other)
           super
           _mounter(:"#{column}").cache(old_uploaders)
         end
+
+        def write_#{column}_identifier
+          return unless has_attribute?(_mounter(:#{column}).serialization_column)
+          super
+        end
       RUBY
     end
 

lib/carrierwave/uploader/proxy.rb

@@ -33,6 +33,19 @@ def identifier
         @identifier || (file && storage.try(:identifier))
       end
 
+      ##
+      # Returns a String which is to be used as a temporary value which gets assigned to the column.
+      # The purpose is to mark the column that it will be updated. Finally before the save it will be
+      # overwritten by the #identifier value, which is usually #filename.
+      #
+      # === Returns
+      #
+      # [String] a temporary_identifier, by default @original_filename is used
+      #
+      def temporary_identifier
+        @original_filename || @identifier
+      end
+
       ##
       # Read the contents of the file
       #

spec/mount_multiple_spec.rb

@@ -195,8 +195,7 @@ def rotate
 
       it "does nothing when an empty string is assigned" do
         expect(instance).not_to receive(:write_uploader)
-
-        instance.images = [test_file_stub]
+        instance.images = ''
       end
 
       it "accepts another uploader instances" do
@@ -1086,8 +1085,8 @@ def download!(uri, headers = {})
 
     describe '#write_images_identifier' do
       it "writes to the given column" do
-        expect(instance).to receive(:write_uploader).with(:monkey, [test_file_name])
         instance.images = [test_file_stub]
+        expect(instance).to receive(:write_uploader).with(:monkey, [test_file_name])
         instance.write_images_identifier
       end
 

spec/mount_single_spec.rb

@@ -135,14 +135,14 @@ def rotate
         expect(@instance.image.current_path).to match(/^#{public_path('uploads/tmp')}/)
       end
 
-      it "should do nothing when nil is assigned" do
-        expect(@instance).not_to receive(:write_uploader)
+      it "should not change the attribute when nil is assigned" do
+        expect(@instance).to receive(:write_uploader).with(:image, nil)
         @instance.image = nil
       end
 
-      it "should do nothing when an empty string is assigned" do
-        expect(@instance).not_to receive(:write_uploader)
-        @instance.image = stub_file('test.jpg')
+      it "should not change the attribute when an empty string is assigned" do
+        expect(@instance).to receive(:write_uploader).with(:image, nil)
+        @instance.image = ''
       end
 
       it "should fail silently if the image fails an allowlist integrity check" do
@@ -589,8 +589,8 @@ def monkey
 
     describe '#write_image_identifier' do
       it "should write to the column" do
-        expect(@instance).to receive(:write_uploader).with(:image, "test.jpg")
         @instance.image = stub_file('test.jpg')
+        expect(@instance).to receive(:write_uploader).with(:image, "test.jpg")
         @instance.write_image_identifier
       end
 
@@ -819,8 +819,8 @@ def download!(uri, headers = {})
 
     describe '#write_image_identifier' do
       it "should write to the given column" do
-        expect(@instance).to receive(:write_uploader).with(:monkey, "test.jpg")
         @instance.image = stub_file('test.jpg')
+        expect(@instance).to receive(:write_uploader).with(:monkey, "test.jpg")
         @instance.write_image_identifier
       end