From: Jean-Philippe Lang Date: Thu, 13 Dec 2012 12:07:19 +0000 (+0000) Subject: Store attachments in subdirectories (#5298). X-Git-Tag: 2.3.0~446 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c99b638d61cc5dd6b9ffcf4212dfaca1973f7500;p=redmine.git Store attachments in subdirectories (#5298). Existing files can be moved to their target subdirectories using rake redmine:attachments:move_to_subdirectories. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10990 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 6fa079c91..c288d588b 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -16,6 +16,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. require "digest/md5" +require "fileutils" class Attachment < ActiveRecord::Base belongs_to :container, :polymorphic => true @@ -92,9 +93,6 @@ class Attachment < ActiveRecord::Base def filename=(arg) write_attribute :filename, sanitize_filename(arg.to_s) - if new_record? && disk_filename.blank? - self.disk_filename = Attachment.disk_filename(filename) - end filename end @@ -102,7 +100,13 @@ class Attachment < ActiveRecord::Base # and computes its MD5 hash def files_to_final_location if @temp_file && (@temp_file.size > 0) + self.disk_directory = target_directory + self.disk_filename = Attachment.disk_filename(filename, disk_directory) logger.info("Saving attachment '#{self.diskfile}' (#{@temp_file.size} bytes)") + path = File.dirname(diskfile) + unless File.directory?(path) + FileUtils.mkdir_p(path) + end md5 = Digest::MD5.new File.open(diskfile, "wb") do |f| if @temp_file.respond_to?(:read) @@ -134,7 +138,7 @@ class Attachment < ActiveRecord::Base # Returns file's location on disk def diskfile - File.join(self.class.storage_path, disk_filename.to_s) + File.join(self.class.storage_path, disk_directory.to_s, disk_filename.to_s) end def title @@ -259,6 +263,26 @@ class Attachment < ActiveRecord::Base Attachment.where("created_on < ? AND (container_type IS NULL OR container_type = '')", Time.now - age).destroy_all end + # Moves an existing attachment to its target directory + def move_to_target_directory! + if !new_record? & readable? + src = diskfile + self.disk_directory = target_directory + dest = diskfile + if src != dest && FileUtils.mkdir_p(File.dirname(dest)) && FileUtils.mv(src, dest) + update_column :disk_directory, disk_directory + end + end + end + + # Moves existing attachments that are stored at the root of the files + # directory (ie. created before Redmine 2.3) to their target subdirectories + def self.move_from_root_to_target_directory + Attachment.where("disk_directory IS NULL OR disk_directory = ''").find_each do |attachment| + attachment.move_to_target_directory! + end + end + private # Physically deletes the file from the file system @@ -276,8 +300,15 @@ class Attachment < ActiveRecord::Base @filename = just_filename.gsub(/[\/\?\%\*\:\|\"\'<>]+/, '_') end - # Returns an ASCII or hashed filename - def self.disk_filename(filename) + # Returns the subdirectory in which the attachment will be saved + def target_directory + time = created_on || DateTime.now + time.strftime("%Y/%m") + end + + # Returns an ASCII or hashed filename that do not + # exists yet in the given subdirectory + def self.disk_filename(filename, directory=nil) timestamp = DateTime.now.strftime("%y%m%d%H%M%S") ascii = '' if filename =~ %r{^[a-zA-Z0-9_\.\-]*$} @@ -287,7 +318,7 @@ class Attachment < ActiveRecord::Base # keep the extension if any ascii << $1 if filename =~ %r{(\.[a-zA-Z0-9]+)$} end - while File.exist?(File.join(@@storage_path, "#{timestamp}_#{ascii}")) + while File.exist?(File.join(storage_path, directory.to_s, "#{timestamp}_#{ascii}")) timestamp.succ! end "#{timestamp}_#{ascii}" diff --git a/db/migrate/20121213084931_add_attachments_disk_directory.rb b/db/migrate/20121213084931_add_attachments_disk_directory.rb new file mode 100644 index 000000000..d11fcad9b --- /dev/null +++ b/db/migrate/20121213084931_add_attachments_disk_directory.rb @@ -0,0 +1,9 @@ +class AddAttachmentsDiskDirectory < ActiveRecord::Migration + def up + add_column :attachments, :disk_directory, :string + end + + def down + remove_column :attachments, :disk_directory + end +end diff --git a/lib/tasks/redmine.rake b/lib/tasks/redmine.rake index 01a4e6227..1ecf507ba 100644 --- a/lib/tasks/redmine.rake +++ b/lib/tasks/redmine.rake @@ -21,6 +21,11 @@ namespace :redmine do task :prune => :environment do Attachment.prune end + + desc 'Moves attachments stored at the root of the file directory (ie. created before Redmine 2.3) to their subdirectories' + task :move_to_subdirectories => :environment do + Attachment.move_from_root_to_target_directory + end end namespace :tokens do diff --git a/test/fixtures/attachments.yml b/test/fixtures/attachments.yml index 604ac1ac4..d50d0357a 100644 --- a/test/fixtures/attachments.yml +++ b/test/fixtures/attachments.yml @@ -4,6 +4,7 @@ attachments_001: downloads: 0 content_type: text/plain disk_filename: 060719210727_error281.txt + disk_directory: "2006/07" container_id: 3 digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 1 @@ -16,6 +17,7 @@ attachments_002: downloads: 0 content_type: text/plain disk_filename: 060719210727_document.txt + disk_directory: "2006/07" container_id: 1 digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 2 @@ -28,6 +30,7 @@ attachments_003: downloads: 0 content_type: image/gif disk_filename: 060719210727_logo.gif + disk_directory: "2006/07" container_id: 4 digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 3 @@ -42,6 +45,7 @@ attachments_004: container_id: 3 downloads: 0 disk_filename: 060719210727_source.rb + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 4 filesize: 153 @@ -55,6 +59,7 @@ attachments_005: container_id: 3 downloads: 0 disk_filename: 060719210727_changeset_iso8859-1.diff + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 5 filesize: 687 @@ -67,6 +72,7 @@ attachments_006: container_id: 3 downloads: 0 disk_filename: 060719210727_archive.zip + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 6 filesize: 157 @@ -79,6 +85,7 @@ attachments_007: container_id: 4 downloads: 0 disk_filename: 060719210727_archive.zip + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 7 filesize: 157 @@ -91,6 +98,7 @@ attachments_008: container_id: 1 downloads: 0 disk_filename: 060719210727_project_file.zip + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 8 filesize: 320 @@ -103,6 +111,7 @@ attachments_009: container_id: 1 downloads: 0 disk_filename: 060719210727_archive.zip + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 9 filesize: 452 @@ -115,6 +124,7 @@ attachments_010: container_id: 2 downloads: 0 disk_filename: 060719210727_picture.jpg + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 10 filesize: 452 @@ -127,6 +137,7 @@ attachments_011: container_id: 1 downloads: 0 disk_filename: 060719210727_picture.jpg + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 11 filesize: 452 @@ -139,6 +150,7 @@ attachments_012: container_id: 1 downloads: 0 disk_filename: 060719210727_version_file.zip + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 12 filesize: 452 @@ -151,6 +163,7 @@ attachments_013: container_id: 1 downloads: 0 disk_filename: 060719210727_foo.zip + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 13 filesize: 452 @@ -163,6 +176,7 @@ attachments_014: container_id: 3 downloads: 0 disk_filename: 060719210727_changeset_utf8.diff + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 id: 14 filesize: 687 @@ -176,6 +190,7 @@ attachments_015: container_id: 14 downloads: 0 disk_filename: 060719210727_changeset_utf8.diff + disk_directory: "2006/07" digest: b91e08d0cf966d5c6ff411bd8c4cc3a2 filesize: 687 filename: private.diff @@ -187,6 +202,7 @@ attachments_016: downloads: 0 created_on: 2010-11-23 16:14:50 +09:00 disk_filename: 101123161450_testfile_1.png + disk_directory: "2010/11" container_id: 14 digest: 8e0294de2441577c529f170b6fb8f638 id: 16 @@ -200,6 +216,7 @@ attachments_017: downloads: 0 created_on: 2010-12-23 16:14:50 +09:00 disk_filename: 101223161450_testfile_2.png + disk_directory: "2010/12" container_id: 14 digest: 6bc2963e8d7ea0d3e68d12d1fba3d6ca id: 17 @@ -213,6 +230,7 @@ attachments_018: downloads: 0 created_on: 2011-01-23 16:14:50 +09:00 disk_filename: 101123161450_testfile_1.png + disk_directory: "2010/11" container_id: 14 digest: 8e0294de2441577c529f170b6fb8f638 id: 18 @@ -226,6 +244,7 @@ attachments_019: downloads: 0 created_on: 2011-02-23 16:14:50 +09:00 disk_filename: 101223161450_testfile_2.png + disk_directory: "2010/12" container_id: 14 digest: 6bc2963e8d7ea0d3e68d12d1fba3d6ca id: 19 @@ -234,3 +253,17 @@ attachments_019: filename: Testテスト.PNG filesize: 3582 author_id: 2 +attachments_020: + content_type: text/plain + downloads: 0 + created_on: 2012-05-12 16:14:50 +09:00 + disk_filename: 120512161450_root_attachment.txt + disk_directory: + container_id: 14 + digest: b0fe2abdb2599743d554a61d7da7ff74 + id: 20 + container_type: Issue + description: "" + filename: root_attachment.txt + filesize: 54 + author_id: 2 diff --git a/test/fixtures/files/060719210727_archive.zip b/test/fixtures/files/060719210727_archive.zip deleted file mode 100644 index 5467885d4..000000000 Binary files a/test/fixtures/files/060719210727_archive.zip and /dev/null differ diff --git a/test/fixtures/files/060719210727_changeset_iso8859-1.diff b/test/fixtures/files/060719210727_changeset_iso8859-1.diff deleted file mode 100644 index 9bade6ab9..000000000 --- a/test/fixtures/files/060719210727_changeset_iso8859-1.diff +++ /dev/null @@ -1,13 +0,0 @@ -Index: trunk/app/controllers/issues_controller.rb -=================================================================== ---- trunk/app/controllers/issues_controller.rb (révision 1483) -+++ trunk/app/controllers/issues_controller.rb (révision 1484) -@@ -149,7 +149,7 @@ - attach_files(@issue, params[:attachments]) - flash[:notice] = 'Demande créée avec succès' - Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added') -- redirect_to :controller => 'issues', :action => 'show', :id => @issue, :project_id => @project -+ redirect_to :controller => 'issues', :action => 'show', :id => @issue - return - end - end diff --git a/test/fixtures/files/060719210727_changeset_utf8.diff b/test/fixtures/files/060719210727_changeset_utf8.diff deleted file mode 100644 index e594f203a..000000000 --- a/test/fixtures/files/060719210727_changeset_utf8.diff +++ /dev/null @@ -1,13 +0,0 @@ -Index: trunk/app/controllers/issues_controller.rb -=================================================================== ---- trunk/app/controllers/issues_controller.rb (révision 1483) -+++ trunk/app/controllers/issues_controller.rb (révision 1484) -@@ -149,7 +149,7 @@ - attach_files(@issue, params[:attachments]) - flash[:notice] = 'Demande créée avec succès' - Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added') -- redirect_to :controller => 'issues', :action => 'show', :id => @issue, :project_id => @project -+ redirect_to :controller => 'issues', :action => 'show', :id => @issue - return - end - end diff --git a/test/fixtures/files/060719210727_source.rb b/test/fixtures/files/060719210727_source.rb deleted file mode 100644 index dccb59165..000000000 --- a/test/fixtures/files/060719210727_source.rb +++ /dev/null @@ -1,10 +0,0 @@ -# The Greeter class -class Greeter - def initialize(name) - @name = name.capitalize - end - - def salute - puts "Hello #{@name}!" - end -end diff --git a/test/fixtures/files/101123161450_testfile_1.png b/test/fixtures/files/101123161450_testfile_1.png deleted file mode 100644 index 6dad7f1d8..000000000 Binary files a/test/fixtures/files/101123161450_testfile_1.png and /dev/null differ diff --git a/test/fixtures/files/101223161450_testfile_2.png b/test/fixtures/files/101223161450_testfile_2.png deleted file mode 100644 index aec3fcfdf..000000000 Binary files a/test/fixtures/files/101223161450_testfile_2.png and /dev/null differ diff --git a/test/fixtures/files/2006/07/060719210727_archive.zip b/test/fixtures/files/2006/07/060719210727_archive.zip new file mode 100644 index 000000000..5467885d4 Binary files /dev/null and b/test/fixtures/files/2006/07/060719210727_archive.zip differ diff --git a/test/fixtures/files/2006/07/060719210727_changeset_iso8859-1.diff b/test/fixtures/files/2006/07/060719210727_changeset_iso8859-1.diff new file mode 100644 index 000000000..9bade6ab9 --- /dev/null +++ b/test/fixtures/files/2006/07/060719210727_changeset_iso8859-1.diff @@ -0,0 +1,13 @@ +Index: trunk/app/controllers/issues_controller.rb +=================================================================== +--- trunk/app/controllers/issues_controller.rb (révision 1483) ++++ trunk/app/controllers/issues_controller.rb (révision 1484) +@@ -149,7 +149,7 @@ + attach_files(@issue, params[:attachments]) + flash[:notice] = 'Demande créée avec succès' + Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added') +- redirect_to :controller => 'issues', :action => 'show', :id => @issue, :project_id => @project ++ redirect_to :controller => 'issues', :action => 'show', :id => @issue + return + end + end diff --git a/test/fixtures/files/2006/07/060719210727_changeset_utf8.diff b/test/fixtures/files/2006/07/060719210727_changeset_utf8.diff new file mode 100644 index 000000000..e594f203a --- /dev/null +++ b/test/fixtures/files/2006/07/060719210727_changeset_utf8.diff @@ -0,0 +1,13 @@ +Index: trunk/app/controllers/issues_controller.rb +=================================================================== +--- trunk/app/controllers/issues_controller.rb (révision 1483) ++++ trunk/app/controllers/issues_controller.rb (révision 1484) +@@ -149,7 +149,7 @@ + attach_files(@issue, params[:attachments]) + flash[:notice] = 'Demande créée avec succès' + Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added') +- redirect_to :controller => 'issues', :action => 'show', :id => @issue, :project_id => @project ++ redirect_to :controller => 'issues', :action => 'show', :id => @issue + return + end + end diff --git a/test/fixtures/files/2006/07/060719210727_source.rb b/test/fixtures/files/2006/07/060719210727_source.rb new file mode 100644 index 000000000..dccb59165 --- /dev/null +++ b/test/fixtures/files/2006/07/060719210727_source.rb @@ -0,0 +1,10 @@ +# The Greeter class +class Greeter + def initialize(name) + @name = name.capitalize + end + + def salute + puts "Hello #{@name}!" + end +end diff --git a/test/fixtures/files/2010/11/101123161450_testfile_1.png b/test/fixtures/files/2010/11/101123161450_testfile_1.png new file mode 100644 index 000000000..6dad7f1d8 Binary files /dev/null and b/test/fixtures/files/2010/11/101123161450_testfile_1.png differ diff --git a/test/fixtures/files/2010/12/101223161450_testfile_2.png b/test/fixtures/files/2010/12/101223161450_testfile_2.png new file mode 100644 index 000000000..aec3fcfdf Binary files /dev/null and b/test/fixtures/files/2010/12/101223161450_testfile_2.png differ diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb index 61ff048a7..ad759152f 100644 --- a/test/unit/attachment_test.rb +++ b/test/unit/attachment_test.rb @@ -52,10 +52,25 @@ class AttachmentTest < ActiveSupport::TestCase assert_equal 'text/plain', a.content_type assert_equal 0, a.downloads assert_equal '1478adae0d4eb06d35897518540e25d6', a.digest + + assert a.disk_directory + assert_match %r{\A\d{4}/\d{2}\z}, a.disk_directory + assert File.exist?(a.diskfile) assert_equal 59, File.size(a.diskfile) end + def test_copy_should_preserve_attributes + a = Attachment.find(1) + copy = a.copy + + assert_save copy + copy = Attachment.order('id DESC').first + %w(filename filesize content_type author_id created_on description digest disk_filename disk_directory diskfile).each do |attribute| + assert_equal a.send(attribute), copy.send(attribute), "#{attribute} was different" + end + end + def test_size_should_be_validated_for_new_file with_settings :attachment_max_size => 0 do a = Attachment.new(:container => Issue.find(1), @@ -123,6 +138,9 @@ class AttachmentTest < ActiveSupport::TestCase end def test_identical_attachments_at_the_same_time_should_not_overwrite + time = DateTime.now + DateTime.stubs(:now).returns(time) + a1 = Attachment.create!(:container => Issue.find(1), :file => uploaded_test_file("testfile.txt", ""), :author => User.find(1)) @@ -168,6 +186,21 @@ class AttachmentTest < ActiveSupport::TestCase end end + def test_move_from_root_to_target_directory_should_move_root_files + a = Attachment.find(20) + assert a.disk_directory.blank? + # Create a real file for this fixture + File.open(a.diskfile, "w") do |f| + f.write "test file at the root of files directory" + end + assert a.readable? + Attachment.move_from_root_to_target_directory + + a.reload + assert_equal '2012/05', a.disk_directory + assert a.readable? + end + context "Attachmnet.attach_files" do should "attach the file" do issue = Issue.first