From f0e5437d27f008379b8523ce2b54d697e5e9be94 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Mon, 3 Apr 2017 11:54:29 +0000
Subject: [PATCH] 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     | 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 1e4f731e7..8e8cc0ac1 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'
 
@@ -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)
diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb
index 16f02e51a..0b188f8a1 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