Merge pull request #1879 from hedgesky/regard_from_version_on_recreat…

…e_with_args

regard :from_version option on calling #recreate_versions! with args

lib/carrierwave/uploader/versions.rb

@@ -220,23 +220,60 @@ def url(*args)
       # Recreate versions and reprocess them. This can be used to recreate
       # versions if their parameters somehow have changed.
       #
-      def recreate_versions!(*versions)
+      def recreate_versions!(*names)
         # Some files could possibly not be stored on the local disk. This
         # doesn't play nicely with processing. Make sure that we're only
         # processing a cached file
         #
         # The call to store! will trigger the necessary callbacks to both
         # process this version and all sub-versions
-        if versions.any?
-          file = sanitized_file if !cached?
-          store_versions!(file, versions)
+
+        if names.any?
+          file = sanitized_file
+          set_versions_to_cache_and_store(names)
+          cache!(file)
+          store!(file)
+          reset_versions_to_cache_and_store
         else
           cache! if !cached?
           store!
         end
       end
 
     private
+
+      def set_versions_to_cache_and_store(names)
+        @versions_to_cache = source_versions_of(names)
+        @versions_to_store = active_versions_with_names_in(@versions_to_cache + names)
+      end
+
+      def reset_versions_to_cache_and_store
+        @versions_to_cache, @versions_to_store = nil, nil
+      end
+
+      def versions_to_cache
+        @versions_to_cache || dependent_versions
+      end
+
+      def versions_to_store
+        @versions_to_store || active_versions
+      end
+
+      def source_versions_of(requested_names)
+        versions.inject([]) do |sources, (name, uploader)|
+          next sources unless requested_names.include?(name)
+          next sources unless source_name = uploader.class.version_options[:from_version]
+
+          sources << [source_name, versions[source_name]]
+        end.uniq
+      end
+
+      def active_versions_with_names_in(names)
+        active_versions.select do |pretendent_name, uploader|
+          names.include?(pretendent_name)
+        end
+      end
+
       def assign_parent_cache_id(file)
         active_versions.each do |name, uploader|
           uploader.parent_cache_id = @cache_id
@@ -270,19 +307,14 @@ def full_original_filename
       end
 
       def cache_versions!(new_file)
-        dependent_versions.each do |name, v|
+        versions_to_cache.each do |name, v|
           v.send(:cache_id=, @cache_id)
           v.cache!(new_file)
         end
       end
 
-      def store_versions!(new_file, versions=nil)
-        if versions
-          active = Hash[active_versions]
-          versions.each { |v| active[v].try(:store!, new_file) } unless active.empty?
-        else
-          active_versions.each { |name, v| v.store!(new_file) }
-        end
+      def store_versions!(new_file)
+        versions_to_store.each { |name, v| v.store!(new_file) }
       end
 
       def remove_versions!

spec/uploader/versions_spec.rb

@@ -638,5 +638,59 @@ def upcase
         expect(File.read(public_path(@uploader.thumb.to_s))).to eq(File.read(public_path(@uploader.small_thumb.to_s)))
       end
     end
+
+    describe "#recreate_versions!" do
+      let(:bork_file) { File.open(file_path('bork.txt')) }
+      let(:original_contents) { File.read(public_path(@uploader.to_s)) }
+      let(:thumb_contents) { File.read(public_path(@uploader.thumb.to_s)) }
+      let(:small_thumb_contents) { File.read(public_path(@uploader.small_thumb.to_s)) }
+
+      context "when no versions are given" do
+        it "should process file based on the version" do
+          @uploader.store!(bork_file)
+          @uploader.recreate_versions!
+          expect(thumb_contents).to eq(small_thumb_contents)
+        end
+      end
+
+      context "when version is given" do
+        it "should process file based on the version" do
+          @uploader.store!(bork_file)
+          FileUtils.rm([@uploader.small_thumb.path, @uploader.thumb.path])
+          @uploader.recreate_versions!(:small_thumb)
+          expect(File.exists?(public_path(@uploader.thumb.to_s))).to be true
+          expect(small_thumb_contents).to eq(thumb_contents)
+          expect(small_thumb_contents).not_to eq(original_contents)
+        end
+
+        it "reprocess parent version, too" do
+          @uploader.store!(bork_file)
+          FileUtils.rm(@uploader.thumb.path)
+          @uploader.recreate_versions!(:small_thumb)
+        end
+
+        it "works fine when recreating both dependent and parent versions" do
+          @uploader.store!(bork_file)
+          FileUtils.rm([@uploader.small_thumb.path, @uploader.thumb.path])
+          @uploader.recreate_versions!(:small_thumb, :thumb)
+          expect(File.exists?(public_path(@uploader.small_thumb.to_s))).to be true
+          expect(File.exists?(public_path(@uploader.thumb.to_s))).to be true
+
+          # doesn't depend on arguments order
+          FileUtils.rm([@uploader.small_thumb.path, @uploader.thumb.path])
+          @uploader.recreate_versions!(:thumb, :small_thumb)
+          expect(File.exists?(public_path(@uploader.small_thumb.to_s))).to be true
+          expect(File.exists?(public_path(@uploader.thumb.to_s))).to be true
+        end
+
+        it "doesn't touch other versions" do
+          @uploader_class.version(:another)
+          @uploader.store!(bork_file)
+          FileUtils.rm(@uploader.another.path)
+          @uploader.recreate_versions!(:small_thumb)
+          expect(File.exists?(public_path(@uploader.another.to_s))).to be false
+        end
+      end
+    end
   end
 end