Download all attachments at once (#7056).

Patch by Mizuki ISHIKAWA.


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

12 files changed, 147 insertions, 2 deletions

diff --git a/Gemfile b/Gemfile
index 0ec948420..b0e848dae 100644
--- a/Gemfile
+++ b/Gemfile
@@ -17,6 +17,7 @@ gem "nokogiri", "~> 1.10.0"
 gem 'i18n', '~> 1.8.2'
 gem "rbpdf", "~> 1.20.0"
 gem 'addressable'
+gem 'rubyzip', (RUBY_VERSION < '2.4' ? '~> 1.3.0' : '~> 2.2.0')
 
 # Windows does not include zoneinfo files, so bundle the tzinfo-data gem
 gem 'tzinfo-data', platforms: [:mingw, :x64_mingw, :mswin]
diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index db90b55da..7e198d0bb 100644
--- a/app/controllers/attachments_controller.rb
+++ b/app/controllers/attachments_controller.rb
@@ -19,6 +19,8 @@
 
 class AttachmentsController < ApplicationController
   before_action :find_attachment, :only => [:show, :download, :thumbnail, :update, :destroy]
+  before_action :find_container, :only => [:edit_all, :update_all, :download_all]
+  before_action :find_downloadable_attachments, :only => :download_all
   before_action :find_editable_attachments, :only => [:edit_all, :update_all]
   before_action :file_readable, :read_authorize, :only => [:show, :download, :thumbnail]
   before_action :update_authorize, :only => :update
@@ -132,6 +134,20 @@ class AttachmentsController < ApplicationController
     render :action => 'edit_all'
   end
 
+  def download_all
+    Tempfile.create('attachments_zip-', Rails.root.join('tmp')) do |tempfile|
+      zip_file = Attachment.archive_attachments(tempfile, @attachments)
+      if zip_file
+        send_data(
+          File.read(zip_file.path),
+          :type => 'application/zip',
+          :filename => "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip")
+      else
+        render_404
+      end
+    end
+  end
+
   def update
     @attachment.safe_attributes = params[:attachment]
     saved = @attachment.save
@@ -195,6 +211,11 @@ class AttachmentsController < ApplicationController
   end
 
   def find_editable_attachments
+    @attachments = @container.attachments.select(&:editable?)
+    render_404 if @attachments.empty?
+  end
+
+  def find_container
     klass = params[:object_type].to_s.singularize.classify.constantize rescue nil
     unless klass && klass.reflect_on_association(:attachments)
       render_404
@@ -206,15 +227,24 @@ class AttachmentsController < ApplicationController
       render_403
       return
     end
-    @attachments = @container.attachments.select(&:editable?)
     if @container.respond_to?(:project)
       @project = @container.project
     end
-    render_404 if @attachments.empty?
   rescue ActiveRecord::RecordNotFound
     render_404
   end
 
+  def find_downloadable_attachments
+    @attachments = @container.attachments.select{|a| File.readable?(a.diskfile) }
+    bulk_download_max_size = Setting.bulk_download_max_size.to_i.kilobytes
+    if @attachments.sum(&:filesize) > bulk_download_max_size
+      flash[:error] = l(:error_bulk_download_size_too_big,
+                        :max_size => bulk_download_max_size.to_i.kilobytes)
+      redirect_to back_url
+      return
+    end
+  end
+
   # Checks that the file exists and is readable
   def file_readable
     if @attachment.readable?
diff --git a/app/helpers/attachments_helper.rb b/app/helpers/attachments_helper.rb
index 47e5ea78a..e496958bf 100644
--- a/app/helpers/attachments_helper.rb
+++ b/app/helpers/attachments_helper.rb
@@ -27,6 +27,10 @@ module AttachmentsHelper
     object_attachments_path container.class.name.underscore.pluralize, container.id
   end
 
+  def container_attachments_download_path(container)
+    object_attachments_download_path container.class.name.underscore.pluralize, container.id
+  end
+
   # Displays view/delete links to the attachments of the given object
   # Options:
   #   :author -- author names are not displayed if set to false
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 6b8c75cc4..a89a0c37e 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -19,6 +19,7 @@
 
 require "digest"
 require "fileutils"
+require "zip"
 
 class Attachment < ActiveRecord::Base
   include Redmine::SafeAttributes
@@ -345,6 +346,30 @@ class Attachment < ActiveRecord::Base
     Attachment.where("created_on < ? AND (container_type IS NULL OR container_type = '')", Time.now - age).destroy_all
   end
 
+  def self.archive_attachments(out_file, attachments)
+    attachments = attachments.select{|attachment| File.readable?(attachment.diskfile) }
+    return nil if attachments.blank?
+
+    Zip.unicode_names = true
+    archived_file_names = []
+    Zip::File.open(out_file.path, Zip::File::CREATE) do |zip|
+      attachments.each do |attachment|
+        filename = attachment.filename
+        # rename the file if a file with the same name already exists
+        dup_count = 0
+        while archived_file_names.include?(filename)
+          dup_count += 1
+          basename = File.basename(attachment.filename, '.*')
+          extname = File.extname(attachment.filename)
+          filename = "#{basename}(#{dup_count})#{extname}"
+        end
+        zip.add(filename, attachment.diskfile)
+        archived_file_names << filename
+      end
+    end
+    out_file
+  end
+
   # Moves an existing attachment to its target directory
   def move_to_target_directory!
     return unless !new_record? & readable?
diff --git a/app/views/attachments/_links.html.erb b/app/views/attachments/_links.html.erb
index 0a9f5e3eb..25d022029 100644
--- a/app/views/attachments/_links.html.erb
+++ b/app/views/attachments/_links.html.erb
@@ -5,6 +5,11 @@
               :title => l(:label_edit_attachments),
               :class => 'icon-only icon-edit'
              ) if options[:editable] %>
+  <%= link_to(l(:label_download_all_attachments),
+              container_attachments_download_path(container),
+              :title => l(:label_download_all_attachments),
+              :class => 'icon-only icon-download'
+             ) if attachments.size > 1 %>
 </div>
 <table>
 <% for attachment in attachments %>
diff --git a/app/views/settings/_attachments.html.erb b/app/views/settings/_attachments.html.erb
index 818845e55..f0430e4a9 100644
--- a/app/views/settings/_attachments.html.erb
+++ b/app/views/settings/_attachments.html.erb
@@ -3,6 +3,8 @@
 <div class="box tabular settings">
 <p><%= setting_text_field :attachment_max_size, :size => 6 %> <%= l(:"number.human.storage_units.units.kb") %></p>
 
+<p><%= setting_text_field :bulk_download_max_size, :size => 6 %> <%= l(:"number.human.storage_units.units.kb") %></p>
+
 <p><%= setting_text_area :attachment_extensions_allowed %>
 <em class="info"><%= l(:text_comma_separated) %> <%= l(:label_example) %>: txt, png</em></p>
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7bb506c57..2b4850c05 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -214,6 +214,7 @@ en:
   error_unable_delete_issue_status: 'Unable to delete issue status (%{value})'
   error_unable_to_connect: "Unable to connect (%{value})"
   error_attachment_too_big: "This file cannot be uploaded because it exceeds the maximum allowed file size (%{max_size})"
+  error_bulk_download_size_too_big: "These attachments cannot be bulk downloaded because the total file size exceeds the maximum allowed size (%{max_size})"
   error_session_expired: "Your session has expired. Please login again."
   error_token_expired: "This password recovery link has expired, please try again."
   warning_attachments_not_saved: "%{count} file(s) could not be saved."
@@ -401,6 +402,7 @@ en:
   setting_self_registration: Self-registration
   setting_show_custom_fields_on_registration: Show custom fields on registration
   setting_attachment_max_size: Maximum attachment size
+  setting_bulk_download_max_size: Maximum total size for bulk download
   setting_issues_export_limit: Issues export limit
   setting_mail_from: Emission email address
   setting_bcc_recipients: Blind carbon copy recipients (bcc)
@@ -1018,6 +1020,7 @@ en:
   label_users_visibility_all: All active users
   label_users_visibility_members_of_visible_projects: Members of visible projects
   label_edit_attachments: Edit attached files
+  label_download_all_attachments: Download all files
   label_link_copied_issue: Link copied issue
   label_ask: Ask
   label_search_attachments_yes: Search attachment filenames and descriptions
diff --git a/config/routes.rb b/config/routes.rb
index 264532042..7e8cdeac9 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -289,6 +289,7 @@ Rails.application.routes.draw do
   resources :attachments, :only => [:show, :update, :destroy]
   get 'attachments/:object_type/:object_id/edit', :to => 'attachments#edit_all', :as => :object_attachments_edit
   patch 'attachments/:object_type/:object_id', :to => 'attachments#update_all', :as => :object_attachments
+  get 'attachments/:object_type/:object_id/download', :to => 'attachments#download_all', :as => :object_attachments_download
 
   resources :groups do
     resources :memberships, :controller => 'principal_memberships'
diff --git a/config/settings.yml b/config/settings.yml
index d33523aeb..5aaaaac28 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -66,6 +66,9 @@ session_timeout:
 attachment_max_size:
   format: int
   default: 5120
+bulk_download_max_size:
+  format: int
+  default: 102400
 attachment_extensions_allowed:
   default:
 attachment_extensions_denied:
diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb
index 33181cb8c..2576f5f7a 100644
--- a/test/functional/attachments_controller_test.rb
+++ b/test/functional/attachments_controller_test.rb
@@ -577,6 +577,50 @@ class AttachmentsControllerTest < Redmine::ControllerTest
     assert_equal 'This is a Ruby source file', attachment.description
   end
 
+  def test_download_all_with_valid_container
+    @request.session[:user_id] = 2
+    get :download_all, :params => {
+        :object_type => 'issues',
+        :object_id => '2'
+      }
+    assert_response 200
+    assert_equal response.headers['Content-Type'], 'application/zip'
+    assert_match /issue-2-attachments.zip/, response.headers['Content-Disposition']
+    assert_not_includes Dir.entries(Rails.root.join('tmp')), /attachments_zip/
+  end
+
+  def test_download_all_with_invalid_container
+    @request.session[:user_id] = 2
+    get :download_all, :params => {
+        :object_type => 'issues',
+        :object_id => '999'
+      }
+    assert_response 404
+  end
+
+  def test_download_all_without_readable_attachments
+    @request.session[:user_id] = 2
+    get :download_all, :params => {
+        :object_type => 'issues',
+        :object_id => '1'
+      }
+    assert_equal Issue.find(1).attachments, []
+    assert_response 404
+  end
+
+  def test_download_all_with_maximum_bulk_download_size_larger_than_attachments
+    with_settings :bulk_download_max_size => 0 do
+      @request.session[:user_id] = 2
+      get :download_all, :params => {
+          :object_type => 'issues',
+          :object_id => '2',
+          :back_url => '/issues/2'
+      }
+      assert_redirected_to '/issues/2'
+      assert_equal flash[:error], 'These attachments cannot be bulk downloaded because the total file size exceeds the maximum allowed size (0)'
+    end
+  end
+
   def test_destroy_issue_attachment
     set_tmp_attachments_directory
     issue = Issue.find(3)
diff --git a/test/integration/routing/attachments_test.rb b/test/integration/routing/attachments_test.rb
index 14a5c784a..1a323d6fb 100644
--- a/test/integration/routing/attachments_test.rb
+++ b/test/integration/routing/attachments_test.rb
@@ -35,5 +35,6 @@ class RoutingAttachmentsTest < Redmine::RoutingTest
 
     should_route 'GET /attachments/issues/1/edit' => 'attachments#edit_all', :object_type => 'issues', :object_id => '1'
     should_route 'PATCH /attachments/issues/1' => 'attachments#update_all', :object_type => 'issues', :object_id => '1'
+    should_route 'GET /attachments/issues/1/download' => 'attachments#download_all', :object_type => 'issues', :object_id => '1'
   end
 end
diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb
index 7e12483f5..5c32d1e75 100644
--- a/test/unit/attachment_test.rb
+++ b/test/unit/attachment_test.rb
@@ -278,6 +278,32 @@ class AttachmentTest < ActiveSupport::TestCase
     end
   end
 
+  def test_archive_attachments
+    attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1)
+    Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile|
+      zip_file = Attachment.archive_attachments(tempfile, [attachment])
+      assert_instance_of File, zip_file
+    end
+  end
+
+  def test_archive_attachments_without_attachments
+    Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile|
+      zip_file = Attachment.archive_attachments(tempfile, [])
+      assert_nil zip_file
+    end
+  end
+
+  def test_archive_attachments_should_rename_duplicate_file_names
+    attachment1 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1)
+    attachment2 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1)
+    Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile|
+      zip_file = Attachment.archive_attachments(tempfile, [attachment1, attachment2])
+      Zip::File.open(zip_file.path) do |z|
+        assert_equal ['testfile.txt', 'testfile(1).txt'], z.map(&:name)
+      end
+    end
+  end
+
   def test_move_from_root_to_target_directory_should_move_root_files
     a = Attachment.find(20)
     assert a.disk_directory.blank?