Skip to content

Commit

Permalink
Delete files only once when marked for removal
Browse files Browse the repository at this point in the history
On update clearing uploaders in write_#{column}_identifier is enough, remove_previously_stored_#{column} will do the rest
  • Loading branch information
mshibuya committed Jun 22, 2019
1 parent 54766c0 commit 67800fd
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 41 deletions.
14 changes: 4 additions & 10 deletions lib/carrierwave/mount.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,8 @@ def write_#{column}_identifier
return if frozen?
mounter = _mounter(:#{column})
if mounter.remove?
write_uploader(mounter.serialization_column, nil)
elsif mounter.identifiers.first
write_uploader(mounter.serialization_column, mounter.identifiers.first)
end
mounter.clear! if mounter.remove?
write_uploader(mounter.serialization_column, mounter.identifiers.first)
end
def #{column}_identifier
Expand Down Expand Up @@ -341,11 +338,8 @@ def write_#{column}_identifier
return if frozen?
mounter = _mounter(:#{column})
if mounter.remove?
write_uploader(mounter.serialization_column, nil)
elsif mounter.identifiers.any?
write_uploader(mounter.serialization_column, mounter.identifiers)
end
mounter.clear! if mounter.remove?
write_uploader(mounter.serialization_column, mounter.identifiers.presence)
end
def #{column}_identifiers
Expand Down
10 changes: 5 additions & 5 deletions lib/carrierwave/mounter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,7 @@ def remote_urls=(urls)
end

def store!
if remove?
remove!
else
uploaders.reject(&:blank?).each(&:store!)
end
uploaders.reject(&:blank?).each(&:store!)
end

def urls(*args)
Expand All @@ -125,6 +121,10 @@ def remove!
@uploaders = []
end

def clear!
@uploaders = []
end

def serialization_column
option(:mount_on) || column
end
Expand Down
21 changes: 5 additions & 16 deletions spec/mount_multiple_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -600,21 +600,6 @@ def monkey
expect(instance.images[0].staged).to be false
end
end

context "removes an uploaded file when remove_images is true" do
let(:images) { [test_file_stub] }

before do
instance.images = images
@image_path = instance.images[0].current_path.dup
instance.remove_images = true
instance.store_images!
end

it { expect(instance.images).to be_empty }

it { expect(File.exist?(@image_path)).to be_falsey }
end
end

describe '#remove_images!' do
Expand Down Expand Up @@ -827,7 +812,11 @@ def monkey
instance.remove_images = true
end

it "removes from the column when remove_images is true" do
it "clears existing uploaders" do
expect(instance.images).to be_empty
end

it "removes from the column" do
expect(instance).to receive(:write_uploader).with(:images, nil)
end
end
Expand Down
10 changes: 1 addition & 9 deletions spec/mount_single_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -373,15 +373,6 @@ def default_url
@instance.store_image!
expect(@instance.image.current_path).to eq(public_path('uploads/test.jpg'))
end

it "should remove an uploaded file when remove_image? returns true" do
@instance.image = stub_file('test.jpg')
path = @instance.image.current_path
@instance.remove_image = true
@instance.store_image!
expect(@instance.image).to be_blank
expect(File.exist?(path)).to be_falsey
end
end

describe '#remove_image!' do
Expand Down Expand Up @@ -579,6 +570,7 @@ def monkey
@instance.remove_image = true
expect(@instance).to receive(:write_uploader).with(:image, nil)
@instance.write_image_identifier
expect(@instance.image).to be_blank
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/orm/activerecord_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ def filename
expect(File.exist?(public_path('uploads/old.jpeg'))).to be_truthy
end

pending("should only delete the file once when the file is removed") do
it "should only delete the file once when the file is removed" do
@event.remove_image = true
expect_any_instance_of(CarrierWave::SanitizedFile).to receive(:delete).exactly(1).times
expect(@event.save).to be_truthy
Expand Down

0 comments on commit 67800fd

Please sign in to comment.