Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove uploaded files if transaction is rolled back #2546

Merged
merged 3 commits into from May 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions lib/carrierwave/mount.rb
Expand Up @@ -199,10 +199,22 @@ def store_previous_changes_for_#{column}
@_previous_changes_for_#{column} = attribute_changes[_mounter(:#{column}).serialization_column]
end

def reset_previous_changes_for_#{column}
# We use this variable to pass information from save time to commit time.
# Make sure this doesn't persist across multiple transactions
@_previous_changes_for_#{column} = nil
end

def remove_previously_stored_#{column}
before, after = @_previous_changes_for_#{column}
_mounter(:#{column}).remove_previous([before], [after])
end

def remove_rolled_back_#{column}
before, after = @_previous_changes_for_#{column}
_mounter(:#{column}).remove_previous([after], [before])
@_previous_changes_for_#{column} = nil
end
RUBY
end

Expand Down Expand Up @@ -351,9 +363,23 @@ def store_previous_changes_for_#{column}
@_previous_changes_for_#{column} = attribute_changes[_mounter(:#{column}).serialization_column]
end

def reset_previous_changes_for_#{column}
# We use this variable to pass information from save time to commit time.
# Make sure this doesn't persist across multiple transactions
@_previous_changes_for_#{column} = nil
end

def remove_previously_stored_#{column}
return unless @_previous_changes_for_#{column}
_mounter(:#{column}).remove_previous(*@_previous_changes_for_#{column})
end

def remove_rolled_back_#{column}
return unless @_previous_changes_for_#{column}
before, after = @_previous_changes_for_#{column}
_mounter(:#{column}).remove_previous(after, before)
@_previous_changes_for_#{column} = nil
end
RUBY
end

Expand Down
7 changes: 5 additions & 2 deletions lib/carrierwave/orm/activerecord.rb
Expand Up @@ -56,12 +56,15 @@ 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 :"reset_previous_changes_for_#{column}"
fsateler marked this conversation as resolved.
Show resolved Hide resolved
after_commit :"remove_previously_stored_#{column}", :on => :update
after_commit :"store_#{column}!", :on => [:create, :update]
after_rollback :"remove_rolled_back_#{column}"

mod = Module.new
prepend mod
Expand Down
39 changes: 39 additions & 0 deletions spec/orm/activerecord_spec.rb
Expand Up @@ -785,6 +785,45 @@ def filename
end
expect(File.exist?(public_path('uploads/old.jpeg'))).to be_truthy
end

it 'should remove new file if transaction is rollback' do
Event.transaction do
@event.image = stub_file('new.jpeg')
@event.save
expect(File.exist?(public_path('uploads/new.jpeg'))).to be_truthy
raise ActiveRecord::Rollback
end
expect(File.exist?(public_path('uploads/new.jpeg'))).to be_falsey
end

it 'should give correct url during transaction' do
Event.transaction do
@event.image = stub_file('new.jpeg')
@event.save
expect(@event.image_url).to eq '/uploads/new.jpeg'
raise ActiveRecord::Rollback
end
end

it 'should raise error at save if storage cannot be done, preserving old' do
allow(@event).to receive(:store_image!).and_raise(CarrierWave::UploadError)
Event.transaction do
@event.image = stub_file('new.jpeg')
expect{ @event.save }.to raise_error(CarrierWave::UploadError)
raise ActiveRecord::Rollback
end
expect(File.exist?(public_path('uploads/old.jpeg'))).to be_truthy
end

it 'should remove new file if transaction is rollback' do
Event.transaction do
@event.image = stub_file('new.jpeg')
@event.save
expect(File.exist?(public_path('uploads/new.jpeg'))).to be_truthy
raise ActiveRecord::Rollback
end
expect(File.exist?(public_path('uploads/new.jpeg'))).to be_falsey
end
end

describe "#mount_uploader into transaction" do
Expand Down