From 3b524365ac56f08f2545402c0cc1ce45470e9cea Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Wed, 20 Mar 2019 18:09:15 +0800 Subject: [PATCH 2/2] implements background issue PDF and CSV exports --- app/controllers/issues_controller.rb | 57 +++++++++++++-- app/jobs/export_job.rb | 75 +++++++++++++++++++ app/models/attachment.rb | 5 ++ app/models/mailer.rb | 18 +++++ app/views/mailer/export_notification.html.erb | 8 +++ app/views/mailer/export_notification.text.erb | 8 +++ config/locales/en.yml | 6 ++ lib/redmine/export/background_export.rb | 82 +++++++++++++++++++++ lib/redmine/export/issue_export.rb | 81 +++++++++++++++++++++ test/unit/export_job_test.rb | 38 ++++++++++ .../lib/redmine/export/background_export_test.rb | 45 ++++++++++++ test/unit/lib/redmine/export/issue_export_test.rb | 83 ++++++++++++++++++++++ 12 files changed, 502 insertions(+), 4 deletions(-) create mode 100644 app/jobs/export_job.rb create mode 100644 app/views/mailer/export_notification.html.erb create mode 100644 app/views/mailer/export_notification.text.erb create mode 100644 lib/redmine/export/background_export.rb create mode 100644 lib/redmine/export/issue_export.rb create mode 100644 test/unit/export_job_test.rb create mode 100644 test/unit/lib/redmine/export/background_export_test.rb create mode 100644 test/unit/lib/redmine/export/issue_export_test.rb diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 685d6444a..625705718 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -65,12 +65,12 @@ class IssuesController < ApplicationController render_feed(@issues, :title => "#{@project || Setting.app_title}: #{l(:label_issue_plural)}") } format.csv { - @issues = @query.issues(:limit => Setting.issues_export_limit.to_i) - send_data(query_to_csv(@issues, @query, params[:csv]), :type => 'text/csv; header=present', :filename => 'issues.csv') + handle_issues_export + return } format.pdf { - @issues = @query.issues(:limit => Setting.issues_export_limit.to_i) - send_file_headers! :type => 'application/pdf', :filename => 'issues.pdf' + handle_issues_export + return } end else @@ -603,4 +603,53 @@ class IssuesController < ApplicationController redirect_back_or_default issue_path(@issue) end end + + # exports with less than this number of issues are done inline (not handed + # over to the configured AJ backend at all) + PERFORM_NOW_LIMIT = 1000 + + # Issues CSV / PDF export via ActiveJob + # See Redmine::Exports::BackgroundExport for how this works. + def handle_issues_export + limit = Setting.issues_export_limit.to_i + issue_count = @query.issue_count + export = Redmine::Export::BackgroundExport.new( + @query, + worker_class: Redmine::Export::IssueExport, + perform_now: issue_count <= PERFORM_NOW_LIMIT, + params: { + format: params[:format], + options: { + project_id: @project.try(:identifier), + limit: limit, + csv: { encoding: params[:encoding] }.merge(params[:csv]||{}) + } + } + ) + + if export.wait + if attachment = export.result + send_file attachment.diskfile, + filename: Redmine::Export::IssueExport.filename(params[:format]), + type: attachment.content_type, + disposition: 'attachment' + return + else + flash[:error] = l(:error_issue_export_failed) + end + else + flash[:notice] = l(:notice_issue_export_scheduled) + end + + params[:format] = nil + parameters = params.permit!.to_h.except(:action, :controller, :csv, :project_id, :format, :encoding) + + if @project + redirect_to project_issues_url(@project, parameters) + else + redirect_to issues_url(parameters) + end + end + + end diff --git a/app/jobs/export_job.rb b/app/jobs/export_job.rb new file mode 100644 index 000000000..454863a32 --- /dev/null +++ b/app/jobs/export_job.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +# Background job for creating CSV/PDF exports. +# +class ExportJob < ActiveJob::Base + include Redmine::I18n + + def perform(filename, query_hash, user_id, created_at, + worker_class_name, params, notify_if_finished_after = nil) + + User.current = User.anonymous + if user = User.active.find_by_id(user_id) + User.current = user + set_language_if_valid user.language || Setting.default_language + end + + @notify_if_finished_after = notify_if_finished_after.to_i + + worker_class = worker_class_name.constantize + export = worker_class.new(query_hash, params) + + + if data = export.generate_data + file = DataFile.new data, filename, export.content_type + @attachment = Attachment.new(file: file, author: User.current) + @attachment.skip_filesize_validation! + unless @attachment.save + @error = @attachment.errors.full_messages.join("\n").presence + end + else + @error = I18n.t(:error_export_failed) + end + + if notify? + if @attachment && @error.blank? + # the controller has given up waiting now, so there is no need for the + # unique filename anymore. Let's change it to something nice. This + # allows us to have URLs like attachments/download/100/issues.pdf + # instead of attachments/download/100/.pdf + @attachment.update_column :filename, export.filename + end + Mailer.export_notification( + User.current, Time.at(created_at), @attachment, @error + ).deliver + end + + end + + + private + + def notify? + if @notify_if_finished_after > 0 + # 1 second margin to avoid lost exports. This may lead to + # a notification being sent out even if the file was delivered + # directly but this is the lesser evil. + Time.at(@notify_if_finished_after) - 1.second < Time.now + else + false + end + end + + # StringIO plus filename and content_type. + # The result of #generate_data should be of this kind + class DataFile < StringIO + attr_reader :original_filename, :content_type + def initialize(string, filename, content_type) + super string + @original_filename = filename + @content_type = content_type + end + end + + +end diff --git a/app/models/attachment.rb b/app/models/attachment.rb index a4870e380..9ef6cb5a5 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -70,11 +70,16 @@ class Attachment < ActiveRecord::Base end def validate_max_file_size + return if @skip_filesize_validation if @temp_file && self.filesize > Setting.attachment_max_size.to_i.kilobytes errors.add(:base, l(:error_attachment_too_big, :max_size => Setting.attachment_max_size.to_i.kilobytes)) end end + def skip_filesize_validation! + @skip_filesize_validation = true + end + def validate_file_extension if @temp_file extension = File.extname(filename) diff --git a/app/models/mailer.rb b/app/models/mailer.rb index 5ef10c1d7..71a9466ff 100644 --- a/app/models/mailer.rb +++ b/app/models/mailer.rb @@ -602,6 +602,24 @@ class Mailer < ActionMailer::Base end end + def export_notification(user, date, attachment, error) + set_language_if_valid(user.language) + @date = date + @error = error + if attachment + @filename = attachment.filename + @url = download_named_attachment_url(attachment, @filename) + else + @url = issues_url + end + redmine_headers 'Sender' => user.login, 'Url' => @url + + mail to: user, + subject: l(error.blank? ? + :mail_subject_export_finished : + :mail_subject_export_failed) + end + # Activates/desactivates email deliveries during +block+ def self.with_deliveries(enabled = true, &block) was_enabled = ActionMailer::Base.perform_deliveries diff --git a/app/views/mailer/export_notification.html.erb b/app/views/mailer/export_notification.html.erb new file mode 100644 index 000000000..415b692b3 --- /dev/null +++ b/app/views/mailer/export_notification.html.erb @@ -0,0 +1,8 @@ +<% if @error.present? %> +

<%= l :mail_body_export_failed, date: format_time(@date) %>

+

<%= @error %>

+<% else %> +

<%= l :mail_body_export_finished, date: format_time(@date) %>

+

<%= link_to @filename, @url %>

+<% end %> + diff --git a/app/views/mailer/export_notification.text.erb b/app/views/mailer/export_notification.text.erb new file mode 100644 index 000000000..0b4d78808 --- /dev/null +++ b/app/views/mailer/export_notification.text.erb @@ -0,0 +1,8 @@ +<% if @error.present? %> +<%= l :mail_body_export_failed, date: format_time(@date) %> +<%= @error %> +<% else %> +<%= l :mail_body_export_finished, date: format_time(@date) %> + +<%= @url %> +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 332d5e206..1a139082d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -187,6 +187,7 @@ en: notice_new_password_must_be_different: The new password must be different from the current password notice_import_finished: "%{count} items have been imported" notice_import_finished_with_errors: "%{count} out of %{total} items could not be imported" + notice_issue_export_scheduled: "Exporting your issues takes longer than usual andwill be continued in the background. You will be notified once the process is completed." error_can_t_load_default_data: "Default configuration could not be loaded: %{value}" error_scm_not_found: "The entry or revision was not found in the repository." @@ -225,6 +226,7 @@ en: error_can_not_delete_auth_source: "This authentication mode is in use and cannot be deleted." error_spent_on_future_date: "Cannot log time on a future date" error_not_allowed_to_log_time_for_other_users: "You are not allowed to log time for other users" + error_issue_export_failed: The issue export failed. mail_subject_lost_password: "Your %{value} password" mail_body_lost_password: 'To change your password, click on the following link:' @@ -250,6 +252,10 @@ en: mail_body_security_notification_notify_disabled: "Email address %{value} no longer receives notifications." mail_body_settings_updated: "The following settings were changed:" mail_body_password_updated: "Your password has been changed." + mail_subject_export_failed: Export failed + mail_body_export_failed: The export that was started by you on %{date} has failed. + mail_subject_export_finished: Export finished + mail_body_export_finished: "The export that was started by you on %{date} is available for download now:" field_name: Name field_description: Description diff --git a/lib/redmine/export/background_export.rb b/lib/redmine/export/background_export.rb new file mode 100644 index 000000000..40ca195f8 --- /dev/null +++ b/lib/redmine/export/background_export.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'timeout' +require 'securerandom' + +module Redmine + module Export + + # This class models an export that may or may not take too long to wait for + # in a web request. + # + # The initializer creates an export that will result in an attachment with + # a random filename. Use the wait instance methods to wait at most the + # specified number of seconds for completion of the job. If this returns + # true, the generated file is available as an Attachment object through + # #result. Otherwise, the export is taking longer and the user will be + # notified by email. + # + # This makes most sense with a background job runner like DelayedJob + # configured for ActiveJob. Without this, the job will be run immediately + # and everything will be like before - #initialize (which launches the + # background job) will block until the export is done, and wait will return + # immediately true since the attachments exists when calling code gets + # around to call it. + # + # Jobs can be forced to run inline via the perform_now: argument. This is + # used in IssuesController to always run small exports immediately instead + # of putting them in a queue where they potentially have to wait + # unnecessarily long for large exports to complete. + class BackgroundExport + + def initialize(query, wait_for: 30.seconds, + user: User.current, + perform_now: false, + worker_class: Redmine::Export::IssueExport, + params: {}) + + @wait_for = wait_for + @filename = SecureRandom.uuid + @user = user + ExportJob.send( + perform_now ? :perform_now : :perform_later, + @filename, + serialize_query(query), user.id, Time.now.to_i, + worker_class.name, params, + wait_for.from_now.to_i + ) + end + + def serialize_query(query) + query.as_params.tap do |params| + params[:c].map!(&:to_s) if params[:c] + end + end + + # returns true if the export finished in time + def wait + Timeout.timeout(@wait_for) { + sleep 1 while not finished? + true + } + rescue Timeout::Error + false + end + + def result + scope.first + end + + private + + def finished? + Attachment.uncached { scope.any? } + end + + def scope + Attachment.where(author: @user, filename: @filename) + end + + end + end +end diff --git a/lib/redmine/export/issue_export.rb b/lib/redmine/export/issue_export.rb new file mode 100644 index 000000000..3b97ff4c8 --- /dev/null +++ b/lib/redmine/export/issue_export.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +module Redmine + module Export + class IssueExport + include IssuesHelper + include QueriesHelper + include Redmine::Export::PDF::IssuesPdfHelper + + def initialize(query_hash, params) + @format = params[:format] + unless %w(csv pdf).include? @format + raise "Cannot handle export format: #{params[:format]}" + end + + if options = params[:options] + @project_id = options[:project_id] + @limit = options[:limit] + @csv_options = options[:csv] + end + + @project = Project.find @project_id if @project_id + + if id = query_hash[:query_id] + @query = IssueQuery.find id + else + @query = IssueQuery.new name: "_" + @query.build_from_params(query_hash) + end + @query.project = @project + end + + # expected by QueriesHelper#query_to_csv + def params + @csv_options || {} + end + + def generate_data + issues = @query.lazy_issues limit: @limit + + case @format + when "csv" + query_to_csv(issues, @query, @csv_options) + when "pdf" + issues_to_pdf(issues, @project, @query) + end + end + + def content_type + self.class.content_type @format + end + + def filename + self.class.filename @format + end + + + # format potentially is params[:format] which may be something + # unexpected, which is why we dont just use "issues#{format}". + def self.filename(format) + case format + when "csv" + "issues.csv" + when "pdf" + "issues.pdf" + end + end + + def self.content_type(format) + case format + when "csv" + "text/csv; header=present" + when "pdf" + "application/pdf" + end + end + + + end + end +end diff --git a/test/unit/export_job_test.rb b/test/unit/export_job_test.rb new file mode 100644 index 000000000..e7c049a3f --- /dev/null +++ b/test/unit/export_job_test.rb @@ -0,0 +1,38 @@ +require_relative '../test_helper' + +class DummyWorker + def initialize(hsh, params) + @data = hsh[:data] + params[:more_data] + end + + def generate_data + @data + end + def content_type + 'text/plain' + end + def filename + 'dummy.txt' + end +end + +class ExportJobTest < ActiveJob::TestCase + fixtures :users + + setup do + set_tmp_attachments_directory + end + + test 'should call worker class and generate attachment' do + assert_difference 'Attachment.count' do + ExportJob.perform_now( + 'random-filename', {data: 'some test'}, 1, Time.now.to_i, + 'DummyWorker', {more_data: ' data'} + ) + end + assert a = Attachment.find_by_filename('random-filename') + assert_equal 1, a.author_id + assert_equal 'text/plain', a.content_type + assert_equal "some test data", IO.read(a.diskfile) + end +end diff --git a/test/unit/lib/redmine/export/background_export_test.rb b/test/unit/lib/redmine/export/background_export_test.rb new file mode 100644 index 000000000..d2ef4695b --- /dev/null +++ b/test/unit/lib/redmine/export/background_export_test.rb @@ -0,0 +1,45 @@ +require_relative '../../../../test_helper' + +class BackgroundExportTest < ActiveJob::TestCase + fixtures :projects, :enabled_modules, :users, :members, + :member_roles, :roles, :trackers, :issue_statuses, + :issue_categories, :enumerations, :issues, + :watchers, :custom_fields, :custom_values, :versions, + :queries, + :projects_trackers, + :custom_fields_trackers, + :workflows + + setup do + @query = IssueQuery.new name: '_' + @project = Project.find 1 + User.current = @user = User.find 1 + end + + test 'should enqueue export job' do + assert_enqueued_with(job: ExportJob) do + Redmine::Export::BackgroundExport.new @query, + params: {format: 'csv', options:{limit: 10000}} + end + end + + test 'wait should wait and return false if not finished' do + e = Redmine::Export::BackgroundExport.new @query, + wait_for: 1.second, + params: {format: 'csv', options:{limit: 10000}} + t = Time.now + refute e.wait + assert Time.now - t > 1.second + end + + test 'wait should return true if finished' do + e = Redmine::Export::BackgroundExport.new @query, + params: {format: 'csv', options:{limit: 10000}} + class << e + def finished?; true end + end + + Timeout.timeout(1) { assert e.wait } + end + +end diff --git a/test/unit/lib/redmine/export/issue_export_test.rb b/test/unit/lib/redmine/export/issue_export_test.rb new file mode 100644 index 000000000..7877612df --- /dev/null +++ b/test/unit/lib/redmine/export/issue_export_test.rb @@ -0,0 +1,83 @@ +require_relative '../../../../test_helper' + +class IssueExportTest < ActiveSupport::TestCase + fixtures :projects, :enabled_modules, :users, :members, + :member_roles, :roles, :trackers, :issue_statuses, + :issue_categories, :enumerations, :issues, + :watchers, :custom_fields, :custom_values, :versions, + :queries, + :projects_trackers, + :custom_fields_trackers, + :workflows + + setup do + @query = IssueQuery.new name: '_' + @project = Project.find 1 + User.current = @user = User.find 1 + end + + test 'should generate issues pdf' do + export = Redmine::Export::IssueExport.new @query.as_params, + format: 'pdf', + options: { limit: 10000 } + assert data = export.generate_data + assert data.length > 0 + assert_equal 'issues.pdf', export.filename + end + + test 'should generate issues csv with new query' do + export = Redmine::Export::IssueExport.new @query.as_params, + format: 'csv', + options: { limit: 10000 } + assert data = export.generate_data + assert data.length > 0 + assert_equal 'issues.csv', export.filename + + assert s = data.lines + s.shift # headers + assert_equal Issue.visible.open.count, s.size + end + + test 'should honor filter and project' do + @query.add_filter("issue_id", '><', ['2','3']) + + export = Redmine::Export::IssueExport.new @query.as_params, + format: 'csv', + options: { limit: 10000, project_id: 1 } + assert data = export.generate_data + assert s = data.lines + s.shift # headers + assert_equal (@project.issues.visible.open.count - 2), s.size + end + + test 'should honor limit' do + export = Redmine::Export::IssueExport.new @query.as_params, + format: 'csv', + options: { limit: 2 } + assert data = export.generate_data + assert s = data.lines + assert_equal 3, s.size + end + + test 'should honor sorting' do + @query.sort_criteria = [['id', 'desc']] + export = Redmine::Export::IssueExport.new @query.as_params, format: 'csv' + assert data = export.generate_data + assert s = data.lines + s.shift # headers + ids = s.map{|l| l.split(',').first.to_i} + assert_equal ids, ids.sort{|a, b| b <=> a} + + + @query.sort_criteria = [['id', 'asc']] + export = Redmine::Export::IssueExport.new @query.as_params, format: 'csv' + assert data = export.generate_data + assert s = data.lines + s.shift # headers + assert s.many? + ids = s.map{|l| l.split(',').first.to_i} + assert_equal ids, ids.sort{|a, b| a <=> b} + end + +end + -- 2.11.0