Reuse existing identical disk files for new attachments (#25215).

Patch by Jens Kraemer.

git-svn-id: http://svn.redmine.org/redmine/trunk@16458 e93f8b46-1217-0410-a6f0-8f06a7374b81

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'
 
@@ -411,6 +412,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)

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