Merge pull request #2401 from carrierwaveuploader/rework-multiple-fil…

…e-upload

Rework multiple file upload

README.md

@@ -190,6 +190,17 @@ u.avatars[0].current_path # => 'path/to/file.png'
 u.avatars[0].identifier # => 'file.png'
 ```
 
+If you want to preserve existing files on uploading new one, you can go like:
+
+```erb
+<% user.avatars.each do |avatar| %>
+  <%= hidden_field :user, :avatars, multiple: true, value: avatar.identifier %>
+<% end %>
+<%= form.file_field :avatars, multiple: true %>
+```
+
+Sorting avatars is supported as well by reordering `hidden_field`, an example using jQuery UI Sortable is available [here](https://github.com/carrierwaveuploader/carrierwave/wiki/How-to%3A-Add%2C-remove-and-reorder-images-using-multiple-file-upload).
+
 ## Changing the storage directory
 
 In order to change where uploaded files are put, just override the `store_dir`

lib/carrierwave/mount.rb

@@ -185,6 +185,18 @@ def #{column}_identifier
           _mounter(:#{column}).read_identifiers[0]
         end
 
+        def #{column}_integrity_error
+          #{column}_integrity_errors.last
+        end
+
+        def #{column}_processing_error
+          #{column}_processing_errors.last
+        end
+
+        def #{column}_download_error
+          #{column}_download_errors.last
+        end
+
         def store_previous_changes_for_#{column}
           attribute_changes = ::ActiveRecord.version.to_s.to_f >= 5.1 ? saved_changes : changes
           @_previous_changes_for_#{column} = attribute_changes[_mounter(:#{column}).serialization_column]
@@ -240,9 +252,9 @@ def remove_previously_stored_#{column}
     # [store_images!]           Stores all files that have been assigned with +images=+
     # [remove_images!]          Removes the uploaded file from the filesystem.
     #
-    # [images_integrity_error]  Returns an error object if the last files to be assigned caused an integrity error
-    # [images_processing_error] Returns an error object if the last files to be assigned caused a processing error
-    # [images_download_error]   Returns an error object if the last files to be remotely assigned caused a download error
+    # [image_integrity_errors]   Returns error objects of files which failed to pass integrity check
+    # [image_processing_errors]  Returns error objects of files which failed to be processed
+    # [image_download_errors]    Returns error objects of files which failed to be downloaded
     #
     # [image_identifiers]       Reads out the identifiers of the files
     #
@@ -395,16 +407,16 @@ def store_#{column}!
           _mounter(:#{column}).store!
         end
 
-        def #{column}_integrity_error
-          _mounter(:#{column}).integrity_error
+        def #{column}_integrity_errors
+          _mounter(:#{column}).integrity_errors
         end
 
-        def #{column}_processing_error
-          _mounter(:#{column}).processing_error
+        def #{column}_processing_errors
+          _mounter(:#{column}).processing_errors
         end
 
-        def #{column}_download_error
-          _mounter(:#{column}).download_error
+        def #{column}_download_errors
+          _mounter(:#{column}).download_errors
         end
 
         def mark_remove_#{column}_false

lib/carrierwave/mounter.rb

@@ -3,14 +3,17 @@ 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_error,
-      :processing_error, :download_error
+    attr_reader :column, :record, :remote_urls, :integrity_errors,
+      :processing_errors, :download_errors
     attr_accessor :remove, :remote_request_headers
 
     def initialize(record, column, options={})
       @record = record
       @column = column
       @options = record.class.uploader_options[column]
+      @download_errors = []
+      @processing_errors = []
+      @integrity_errors = []
     end
 
     def uploader_class
@@ -38,59 +41,63 @@ def uploaders
     end
 
     def cache(new_files)
-      return if new_files.blank?
+      return if !new_files.is_a?(Array) && new_files.blank?
+      old_uploaders = uploaders
       @uploaders = new_files.map do |new_file|
-        uploader = blank_uploader
-        uploader.cache!(new_file)
-        uploader
-      end
-
-      @integrity_error = nil
-      @processing_error = nil
-    rescue CarrierWave::IntegrityError => e
-      @integrity_error = e
-      raise e unless option(:ignore_integrity_errors)
-    rescue CarrierWave::ProcessingError => e
-      @processing_error = e
-      raise e unless option(:ignore_processing_errors)
+        handle_error do
+          if new_file.is_a?(String)
+            if (uploader = old_uploaders.detect { |uploader| uploader.identifier == new_file })
+              uploader.staged = true
+              uploader
+            else
+              begin
+                uploader = blank_uploader
+                uploader.retrieve_from_cache!(new_file)
+                uploader
+              rescue CarrierWave::InvalidParameter
+                nil
+              end
+            end
+          else
+            uploader = blank_uploader
+            uploader.cache!(new_file)
+            uploader
+          end
+        end
+      end.compact
     end
 
     def cache_names
       uploaders.map(&:cache_name).compact
     end
 
     def cache_names=(cache_names)
-      return if cache_names.blank? || uploaders.any?(&:cached?)
-      @uploaders = cache_names.map do |cache_name|
-        uploader = blank_uploader
-        uploader.retrieve_from_cache!(cache_name)
-        uploader
+      return if cache_names.blank?
+      clear_unstaged
+      cache_names.map do |cache_name|
+        begin
+          uploader = blank_uploader
+          uploader.retrieve_from_cache!(cache_name)
+          @uploaders << uploader
+        rescue CarrierWave::InvalidParameter
+          # ignore
+        end
       end
-    rescue CarrierWave::InvalidParameter
     end
 
     def remote_urls=(urls)
       return if urls.blank? || urls.all?(&:blank?)
 
       @remote_urls = urls
-      @download_error = nil
-      @integrity_error = nil
 
-      @uploaders = urls.zip(remote_request_headers || []).map do |url, header|
-        uploader = blank_uploader
-        uploader.download!(url, header || {})
-        uploader
+      clear_unstaged
+      urls.zip(remote_request_headers || []).map do |url, header|
+        handle_error do
+          uploader = blank_uploader
+          uploader.download!(url, header || {})
+          @uploaders << uploader
+        end
       end
-
-    rescue CarrierWave::DownloadError => e
-      @download_error = e
-      raise e unless option(:ignore_download_errors)
-    rescue CarrierWave::ProcessingError => e
-      @processing_error = e
-      raise e unless option(:ignore_processing_errors)
-    rescue CarrierWave::IntegrityError => e
-      @integrity_error = e
-      raise e unless option(:ignore_integrity_errors)
     end
 
     def store!
@@ -159,5 +166,22 @@ def option(name)
       self.uploader_options[name] ||= record.class.uploader_option(column, name)
     end
 
+    def clear_unstaged
+      @uploaders ||= []
+      @uploaders.keep_if(&:staged)
+    end
+
+    def handle_error
+      yield
+    rescue CarrierWave::DownloadError => e
+      @download_errors << e
+      raise e unless option(:ignore_download_errors)
+    rescue CarrierWave::ProcessingError => e
+      @processing_errors << e
+      raise e unless option(:ignore_processing_errors)
+    rescue CarrierWave::IntegrityError => e
+      @integrity_errors << e
+      raise e unless option(:ignore_integrity_errors)
+    end
   end # Mounter
 end # CarrierWave

lib/carrierwave/uploader/cache.rb

@@ -38,6 +38,16 @@ module Cache
       include CarrierWave::Uploader::Callbacks
       include CarrierWave::Uploader::Configuration
 
+      included do
+        prepend Module.new {
+          def initialize(*)
+            super
+            @staged = false
+          end
+        }
+        attr_accessor :staged
+      end
+
       module ClassMethods
 
         ##
@@ -125,6 +135,7 @@ def cache!(new_file = sanitized_file)
 
         self.cache_id = CarrierWave.generate_cache_id unless cache_id
 
+        @staged = true
         @filename = new_file.filename
         self.original_filename = new_file.filename
 
@@ -158,6 +169,7 @@ def cache!(new_file = sanitized_file)
       def retrieve_from_cache!(cache_name)
         with_callbacks(:retrieve_from_cache, cache_name) do
           self.cache_id, self.original_filename = cache_name.to_s.split('/', 2)
+          @staged = true
           @filename = original_filename
           @file = cache_storage.retrieve_from_cache!(full_filename(original_filename))
         end

lib/carrierwave/uploader/mountable.rb

@@ -33,6 +33,12 @@ def initialize(model=nil, mounted_as=nil)
         @mounted_as = mounted_as
       end
 
+      ##
+      # Returns array index of given uploader within currently mounted uploaders
+      #
+      def index
+        model.__send__(:_mounter, mounted_as).uploaders.index(self)
+      end
     end # Mountable
   end # Uploader
 end # CarrierWave

lib/carrierwave/uploader/store.rb

@@ -70,6 +70,7 @@ def store!(new_file=nil)
             end
             @file = new_file
             @cache_id = @identifier = nil
+            @staged = false
           end
         end
       end

lib/carrierwave/validations/active_model.rb

@@ -11,7 +11,7 @@ module ActiveModel
       class ProcessingValidator < ::ActiveModel::EachValidator
 
         def validate_each(record, attribute, value)
-          if e = record.__send__("#{attribute}_processing_error")
+          record.__send__("#{attribute}_processing_errors").each do |e|
             message = (e.message == e.class.to_s) ? :carrierwave_processing_error : e.message
             record.errors.add(attribute, message)
           end
@@ -21,7 +21,7 @@ def validate_each(record, attribute, value)
       class IntegrityValidator < ::ActiveModel::EachValidator
 
         def validate_each(record, attribute, value)
-          if e = record.__send__("#{attribute}_integrity_error")
+          record.__send__("#{attribute}_integrity_errors").each do |e|
             message = (e.message == e.class.to_s) ? :carrierwave_integrity_error : e.message
             record.errors.add(attribute, message)
           end
@@ -31,7 +31,7 @@ def validate_each(record, attribute, value)
       class DownloadValidator < ::ActiveModel::EachValidator
 
         def validate_each(record, attribute, value)
-          if e = record.__send__("#{attribute}_download_error")
+          record.__send__("#{attribute}_download_errors").each do |e|
             message = (e.message == e.class.to_s) ? :carrierwave_download_error : e.message
             record.errors.add(attribute, message)
           end