Merge pull request #2209 from skosh/manage_transaction_rollback

Manage transaction rollback

lib/carrierwave/orm/activerecord.rb

@@ -52,13 +52,12 @@ def mount_base(column, uploader=nil, options={}, &block)
       validates_processing_of column if uploader_option(column.to_sym, :validate_processing)
       validates_download_of column if uploader_option(column.to_sym, :validate_download)
 
-      after_save :"store_#{column}!"
       before_save :"write_#{column}_identifier"
+      after_save :"store_previous_changes_for_#{column}"
       after_commit :"remove_#{column}!", :on => :destroy
       after_commit :"mark_remove_#{column}_false", :on => :update
-
-      after_save :"store_previous_changes_for_#{column}"
       after_commit :"remove_previously_stored_#{column}", :on => :update
+      after_commit :"store_#{column}!", :on => [:create, :update]
 
       class_eval <<-RUBY, __FILE__, __LINE__+1
         def #{column}=(new_file)

spec/orm/activerecord_spec.rb

@@ -672,7 +672,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     describe 'normally' do
@@ -757,7 +757,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     it "should remove old file if old file had a different path" do
@@ -780,14 +780,52 @@ def filename
       Event.transaction do
         @event.image = stub_file('new.jpeg')
         @event.save
-        expect(File.exist?(public_path('uploads/new.jpeg'))).to be_truthy
         expect(File.exist?(public_path('uploads/old.jpeg'))).to be_truthy
         raise ActiveRecord::Rollback
       end
       expect(File.exist?(public_path('uploads/old.jpeg'))).to be_truthy
     end
   end
 
+  describe "#mount_uploader into transaction" do
+    before do
+      @uploader.version :thumb
+      reset_class("Event")
+      Event.mount_uploader(:image, @uploader)
+      @event = Event.new
+    end
+
+    after do
+      FileUtils.rm_rf(public_path("uploads"))
+    end
+
+    it "should not store file during rollback" do
+      Event.transaction do
+        @event.image = stub_file('new.jpeg')
+        @event.save
+
+        raise ActiveRecord::Rollback
+      end
+
+      expect(File.exist?(public_path('uploads/new.jpeg'))).to be_falsey
+    end
+
+    it "should not change file during rollback" do
+      @event.image = stub_file('old.jpeg')
+      @event.save
+
+      Event.transaction do
+        @event.image = stub_file('new.jpeg')
+        @event.save
+
+        raise ActiveRecord::Rollback
+      end
+
+      expect(File.exist?(public_path('uploads/new.jpeg'))).to be_falsey
+      expect(File.exist?(public_path('uploads/old.jpeg'))).to be_truthy
+    end
+  end
+
   describe '#mount_uploader removing old files with multiple uploaders' do
     before do
       @uploader = Class.new(CarrierWave::Uploader::Base)
@@ -805,7 +843,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     it "should remove old file1 and file2 if old file1 and file2 had a different paths" do
@@ -848,7 +886,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     it "should remove old file if old file had a different path" do
@@ -1389,7 +1427,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     describe 'normally' do
@@ -1466,7 +1504,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     it "should remove old file if old file had a different path" do
@@ -1503,7 +1541,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     it "should remove old file1 and file2 if old file1 and file2 had a different paths" do
@@ -1545,7 +1583,7 @@ def filename
     end
 
     after do
-      FileUtils.rm_rf(file_path("uploads"))
+      FileUtils.rm_rf(public_path("uploads"))
     end
 
     it "should remove old file if old file had a different path" do