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

Implement keep_old_files option #11

Merged
merged 4 commits into from
Dec 25, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ MiniPaperclip.config.tap do |config|
config.s3_acl # s3 object acl
config.s3_cache_control # Set this value to Cache-Control header when put-object
config.interpolates # minimum templates using by `String#gsub!`
config.keep_old_files # Delete old attached file if it's false (default false)
config.read_timeout # timeout when attachment set url
config.logger # You can set logger object.
end
Expand Down
1 change: 1 addition & 0 deletions lib/mini_paperclip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def config
},
hash_data: ":class/:attachment/:id/:style/:updated_at",
url_missing_path: ":attachment/:style/missing.png",
keep_old_files: false,
read_timeout: 60,
logger: Logger.new($stdout),
)
Expand Down
10 changes: 10 additions & 0 deletions lib/mini_paperclip/attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ def assign(file)
@waiting_write_file = nil
@meta_content_type = nil

if present?
push_delete_files
end

if file.nil?
assign_nil
elsif file.instance_of?(Attachment)
Expand Down Expand Up @@ -116,6 +120,12 @@ def process_and_store
@storage.write(style, temp)
end
end

# should delete after write for copy
if !@config.keep_old_files
do_delete_files
end

ensure
if @waiting_write_file.respond_to?(:close!)
@waiting_write_file.close!
Expand Down
2 changes: 1 addition & 1 deletion lib/mini_paperclip/class_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def has_attached_file(attachment_name, option_config = {})
instance_variable_set("@#{attachment_name}", Attachment.new(self, attachment_name, option_config))
end
define_method("#{attachment_name}=") do |file|
a = Attachment.new(self, attachment_name, option_config)
a = public_send(attachment_name)
a.assign(file)
instance_variable_set("@#{attachment_name}", a)
end
Expand Down
1 change: 1 addition & 0 deletions lib/mini_paperclip/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class Config < Struct.new(
:s3_acl,
:s3_cache_control,
:interpolates,
:keep_old_files,
:read_timeout,
:logger,
keyword_init: true,
Expand Down
1 change: 1 addition & 0 deletions lib/mini_paperclip/storage/filesystem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def push_delete_file(style)
end

def do_delete_files
return if @deletes.empty?
debug("deleting by filesystem #{@deletes}")
FileUtils.rm_f(@deletes)
end
Expand Down
1 change: 1 addition & 0 deletions lib/mini_paperclip/storage/s3.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def push_delete_file(style)
end

def do_delete_files
return if @deletes.empty?
debug("deleting by S3 to bucket:#{@config.s3_bucket_name},objects:#{@deletes}")
Aws::S3::Client.new.delete_objects(
bucket: @config.s3_bucket_name,
Expand Down
2 changes: 1 addition & 1 deletion spec/mini_paperclip/class_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
end

it "should success to save and delete files on s3" do
original_config = MiniPaperclip.config
original_config = MiniPaperclip.config.dup
MiniPaperclip.config.storage = :s3
MiniPaperclip.config.s3_bucket_name = 'test'

Expand Down
6 changes: 3 additions & 3 deletions spec/mini_paperclip/storage/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@

it "#path_for with missing file" do
attachment = MiniPaperclip::Attachment.new(record, :image)
config = MiniPaperclip.config
config = MiniPaperclip.config.dup
base = MiniPaperclip::Storage::Base.new(attachment, config)
expect(base.path_for(:original)).to eq('images/original/missing.png')
end

it "#path_for with empty file" do
attachment = MiniPaperclip::Attachment.new(record, :image)
config = MiniPaperclip.config
config = MiniPaperclip.config.dup
base = MiniPaperclip::Storage::Base.new(attachment, config)
record.image_file_name = ""
expect(base.path_for(:original)).to eq('records/images/25a80ba6aa8c48f17ea32fc7935fab633e807238.')
end

it "#path_for with file" do
attachment = MiniPaperclip::Attachment.new(record, :image)
config = MiniPaperclip.config
config = MiniPaperclip.config.dup
base = MiniPaperclip::Storage::Base.new(attachment, config)
record.image_file_name = "test.png"
expect(base.path_for(:original)).to eq('records/images/25a80ba6aa8c48f17ea32fc7935fab633e807238.png')
Expand Down
28 changes: 28 additions & 0 deletions spec/mini_paperclip_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,34 @@
let(:record) { Record.new }

describe "#config" do
it "#keep_old_files = false" do
old_file = Rack::Test::UploadedFile.new "spec/paperclip.jpg", 'image/jpeg'
record.image.config.keep_old_files = false
record.image = old_file
record.save!
old_file_path = record.image.storage.file_path(:original)

expect(File.exists?(old_file_path)).to eq(true)
new_file = Rack::Test::UploadedFile.new "spec/opaopa.gif", 'image/gif'
record.image = new_file
record.save!
expect(File.exists?(old_file_path)).to eq(false)
end

it "#keep_old_files = true" do
old_file = Rack::Test::UploadedFile.new "spec/paperclip.jpg", 'image/jpeg'
record.image.config.keep_old_files = true
record.image = old_file
record.save!
old_file_path = record.image.storage.file_path(:original)

expect(File.exists?(old_file_path)).to eq(true)
new_file = Rack::Test::UploadedFile.new "spec/opaopa.gif", 'image/gif'
record.image = new_file
record.save!
expect(File.exists?(old_file_path)).to eq(true)
end

it "#hash_data" do
default_hash_data = MiniPaperclip.config.hash_data
interpolated_hash_data = record.image.storage.interpolator.interpolate(default_hash_data, :original)
Expand Down