From 85fd72fc9a3cbfc787ec6f57b2cf9480674803b5 Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Tue, 28 Feb 2017 10:48:17 +0800 Subject: [PATCH] reuse existing identical disk files for new attachments --- app/models/attachment.rb | 19 +++++++++++++++++++ test/unit/attachment_test.rb | 8 ++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/app/models/attachment.rb b/app/models/attachment.rb index d038437..7265471 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -56,6 +56,7 @@ class Attachment < ActiveRecord::Base before_create :files_to_final_location after_rollback :delete_from_disk, :on => :create after_commit :delete_from_disk, :on => :destroy + after_commit :reuse_existing_file_if_possible, :on => :create safe_attributes 'filename', 'content_type', 'description' @@ -387,6 +388,24 @@ class Attachment < ActiveRecord::Base private + def reuse_existing_file_if_possible + with_lock do + if existing = Attachment + .lock + .where(digest: self.digest, filesize: self.filesize) + .where('id <> ? and disk_filename <> ?', + self.id, self.disk_filename) + .first + + original_diskfile = self.diskfile + self.update_columns disk_directory: existing.disk_directory, + disk_filename: existing.disk_filename + File.delete(original_diskfile) if File.exist?(original_diskfile) + end + end + end + + # Physically deletes the file from the file system def delete_from_disk! if disk_filename.present? && File.exist?(diskfile) diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb index 81d7e3c..f9d882e 100644 --- a/test/unit/attachment_test.rb +++ b/test/unit/attachment_test.rb @@ -94,6 +94,10 @@ class AttachmentTest < ActiveSupport::TestCase end def test_copy_should_preserve_attributes + + # prevent re-use of data from other attachments with equal contents + Attachment.where('id <> 1').destroy_all + a = Attachment.find(1) copy = a.copy @@ -220,14 +224,14 @@ class AttachmentTest < ActiveSupport::TestCase assert_equal 'text/plain', a.content_type end - def test_identical_attachments_at_the_same_time_should_not_overwrite + def test_identical_attachments_should_reuse_same_file a1 = Attachment.create!(:container => Issue.find(1), :file => uploaded_test_file("testfile.txt", ""), :author => User.find(1)) a2 = Attachment.create!(:container => Issue.find(1), :file => uploaded_test_file("testfile.txt", ""), :author => User.find(1)) - assert a1.disk_filename != a2.disk_filename + assert_equal a1.diskfile, a2.diskfile end def test_filename_should_be_basenamed -- 2.1.4