Skip to content

Commit

Permalink
Manage transaction rollback
Browse files Browse the repository at this point in the history
  When we try to create new active record object into transaction and meet rollback:
    database row will not be created but uploader has created file in folder and doen't remove it.

  I changed behavior: now we store file only after commit action. This true and for update action in transaction.

  Changes in spec/orm/activerecord_spec.rb:
    - add tests which covers this flows
    - replace cleaning path from 'file path' to 'public_path' folder. Cause folder 'spec/fixtures/uploads ' is not used.
  • Loading branch information
Alexander Koshelapov committed Jul 26, 2017
1 parent f43c068 commit 665f225
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 12 deletions.
5 changes: 2 additions & 3 deletions lib/carrierwave/orm/activerecord.rb
Expand Up @@ -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)
Expand Down
56 changes: 47 additions & 9 deletions spec/orm/activerecord_spec.rb
Expand Up @@ -648,7 +648,7 @@ def filename
end

after do
FileUtils.rm_rf(file_path("uploads"))
FileUtils.rm_rf(public_path("uploads"))
end

describe 'normally' do
Expand Down Expand Up @@ -735,7 +735,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
Expand All @@ -758,14 +758,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)
Expand All @@ -783,7 +821,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
Expand Down Expand Up @@ -826,7 +864,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
Expand Down Expand Up @@ -1367,7 +1405,7 @@ def filename
end

after do
FileUtils.rm_rf(file_path("uploads"))
FileUtils.rm_rf(public_path("uploads"))
end

describe 'normally' do
Expand Down Expand Up @@ -1444,7 +1482,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
Expand Down Expand Up @@ -1481,7 +1519,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
Expand Down Expand Up @@ -1523,7 +1561,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
Expand Down

0 comments on commit 665f225

Please sign in to comment.