introduce a presisted status for delayed jobs (#8299)

This is currently only employed for work package export jobs but is
intended to be used more broadly, e.g. to signal the status of copying a
project.

[ci skip]

app/controllers/work_packages/exports_controller.rb

@@ -34,27 +34,52 @@ class WorkPackages::ExportsController < ApplicationController
   before_action :find_model_object,
                 :authorize_current_user
 
+  before_action :find_job_status, only: :status
+
   def show
     if @export.ready?
       redirect_to attachment_content_path
     else
-      headers['Link'] = "<#{status_work_packages_export_path(@export.id)}> rel=\"status\""
+      headers['Link'] = "<#{status_work_packages_export_path(@export.id, format: :json)}> rel=\"status\""
       head 202
     end
   end
 
   def status
-    if @export.ready?
+    if @job_status.success?
       headers['Link'] = "<#{attachment_content_path}> rel=\"download\""
+    end
 
-      render plain: 'Completed'
-    else
-      render plain: 'Processing'
+    respond_to do |format|
+      format.json do
+        render json: status_json_body(@job_status),
+               status: 200
+      end
     end
   end
 
   private
 
+  def status_json_body(job)
+    body = { status: job_status(job), message: job.message }
+
+    if job.success?
+      body[:link] = attachment_content_path
+    end
+
+    body
+  end
+
+  def job_status(job)
+    if job.success?
+      'Completed'
+    elsif job.failure? || job.error?
+      'Error'
+    else
+      'Processing'
+    end
+  end
+
   def authorize_current_user
     deny_access(not_found: true) unless @export.visible?(current_user)
   end
@@ -64,4 +89,8 @@ class WorkPackages::ExportsController < ApplicationController
     # including the url helper again.
     ::API::V3::Utilities::PathHelper::ApiV3Path.attachment_content(@export.attachments.first.id)
   end
+
+  def find_job_status
+    @job_status = Delayed::Job::Status.of_reference(@export).first
+  end
 end

app/models/delayed/job/status.rb

@@ -0,0 +1,20 @@
+module Delayed
+  class Job
+    class Status < ApplicationRecord
+      self.table_name = 'delayed_job_statuses'
+
+      belongs_to :reference, polymorphic: true
+      belongs_to :job, class_name: '::Delayed::Job'
+
+      enum status: { in_queue: 'in_queue',
+                     error: 'error',
+                     in_process: 'in_process',
+                     success: 'success',
+                     failure: 'failure' }
+
+      def self.of_reference(reference)
+        where(reference: reference)
+      end
+    end
+  end
+end

app/workers/application_job.rb

@@ -29,7 +29,6 @@
 require 'active_job'
 
 class ApplicationJob < ::ActiveJob::Base
-
   ##
   # Return a priority number on the given payload
   def self.priority_number(prio = :default)
@@ -57,6 +56,14 @@ class ApplicationJob < ::ActiveJob::Base
     child.prepend Setup
   end
 
+  # Delayed jobs can have a status:
+  # Delayed::Job::Status
+  # which is related to the job via a reference which is an AR model instance.
+  # If no such reference is defined, there is no status stored in the db.
+  def status_reference
+    nil
+  end
+
   module Setup
     def perform(*args)
       before_perform!

app/workers/cron/clear_old_job_status_job.rb

@@ -0,0 +1,44 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+module Cron
+  class ClearOldJobStatusJob < CronJob
+    # runs at 4:15 nightly
+    self.cron_expression = '15 4 * * *'
+
+    RETENTION_PERIOD = 2.days.freeze
+
+    def perform
+      Delayed::Job::Status
+        .where(Delayed::Job::Status.arel_table[:updated_at].lteq(Time.now - RETENTION_PERIOD))
+        .destroy_all
+    end
+  end
+end

app/workers/work_packages/exports/export_job.rb

@@ -10,6 +10,10 @@ module WorkPackages
         end
       end
 
+      def status_reference
+        arguments.first[:export]
+      end
+
       private
 
       def export_work_packages(export, mime_type, query, options)

config/initializers/cronjobs.rb

@@ -4,6 +4,7 @@ OpenProject::Application.configure do |application|
   application.config.to_prepare do
     ::Cron::CronJob.register! ::Cron::ClearOldSessionsJob,
                               ::Cron::ClearTmpCacheJob,
-                              ::Cron::ClearUploadedFilesJob
+                              ::Cron::ClearUploadedFilesJob,
+                              ::Cron::ClearOldJobStatusJob
   end
 end

config/initializers/job_status.rb

@@ -0,0 +1,77 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+# Extends the ActiveJob adapter in use (DelayedJob) by a Status which lives
+# indenpendently from the job itself (which is deleted once successful or after max attempts).
+# That way, the result of a background job is available even after the original job is gone.
+
+ActiveSupport::Notifications.subscribe "perform.active_job" do |job:, exception_object: nil, **_args|
+  next unless job.status_reference
+
+  # job.provider_job_id is not filled at this point as
+  # the ActiveJob adapter for DelayedJob is only setting it
+  # on enqueue and enqueue_at.
+  if exception_object
+    dj_job_attempts = Delayed::Job.where(id: Delayed::Job::Status.of_reference(job.status_reference).select(:job_id))
+                      .pluck(:attempts)
+                      .first || 1
+
+    new_status = if dj_job_attempts + 1 >= Delayed::Worker.max_attempts
+                   :failure
+                 else
+                   :error
+                 end
+
+    Delayed::Job::Status
+      .of_reference(job.status_reference)
+      .update(status: new_status,
+              message: exception_object)
+  else
+    Delayed::Job::Status
+      .of_reference(job.status_reference)
+      .update(status: :success)
+  end
+end
+
+ActiveSupport::Notifications.subscribe "enqueue.active_job" do |job:, **_args|
+  if job.status_reference
+    Delayed::Job::Status.create(status: :in_queue,
+                                reference: job.status_reference,
+                                job_id: job.provider_job_id)
+  end
+end
+
+ActiveSupport::Notifications.subscribe "enqueue_at.active_job" do |job:, **_args|
+  if job.status_reference
+    Delayed::Job::Status.create(status: :in_queue,
+                                reference: job.status_reference,
+                                job_id: job.provider_job_id)
+  end
+end

db/migrate/20200422105623_add_job_status.rb

@@ -0,0 +1,25 @@
+class AddJobStatus < ActiveRecord::Migration[6.0]
+  def up
+    execute <<-SQL
+      CREATE TYPE delayed_job_status AS ENUM ('in_queue', 'error', 'in_process', 'success', 'failure');
+    SQL
+
+    create_table :delayed_job_statuses do |t|
+      t.references :job
+      t.references :reference, polymorphic: true, index: { unique: true }
+      t.string :message
+
+      t.timestamps
+    end
+
+    add_column :delayed_job_statuses, :status, :delayed_job_status, default: 'in_queue'
+  end
+
+  def down
+    drop_table :delayed_job_statuses
+
+    execute <<-SQL
+      DROP TYPE delayed_job_status;
+    SQL
+  end
+end

frontend/src/app/components/modals/export-modal/wp-table-export.modal.ts

@@ -13,7 +13,7 @@ import {
   LoadingIndicatorService,
   withDelayedLoadingIndicator
 } from "core-app/modules/common/loading-indicator/loading-indicator.service";
-import {switchMap, takeWhile} from 'rxjs/operators';
+import {switchMap, takeWhile, map} from 'rxjs/operators';
 import {interval, Observable, Subscription} from 'rxjs';
 import {NotificationsService} from "core-app/modules/common/notifications/notifications.service";
 
@@ -58,7 +58,6 @@ export class WpTableExportModal extends OpModalComponent implements OnInit, OnDe
   public downloadHref:string;
   public isLoading = false;
   private subscription?:Subscription;
-  private finished?:Function;
 
   @ViewChild('downloadLink') downloadLink:ElementRef;
 
@@ -122,7 +121,7 @@ export class WpTableExportModal extends OpModalComponent implements OnInit, OnDe
             this.pollUntilDownload(this.linkHeaderUrl(data)!);
           }
         },
-        (error:HttpErrorResponse) => this.handleError(error));
+        (error:HttpErrorResponse) => this.handleError(error.message));
   }
 
   private pollUntilDownload(url:string) {
@@ -131,14 +130,18 @@ export class WpTableExportModal extends OpModalComponent implements OnInit, OnDe
     this.subscription = interval(1000)
                         .pipe(
                           switchMap(() => this.performRequest(url)),
-                          takeWhile(response => response.status === 200 && !this.linkHeaderUrl(response), true),
+                          takeWhile(response => response.status === 200, true),
+                          map(response => JSON.parse(response.body)),
+                          takeWhile(body => body.status === 'Processing', true),
                           withDelayedLoadingIndicator(this.loadingIndicator.getter('modal')),
-                        ).subscribe(response => {
-                            if (response.status === 200 && this.linkHeaderUrl(response)) {
-                              this.download(this.linkHeaderUrl(response)!);
+                        ).subscribe(body => {
+                            if (body.status === 'Completed') {
+                              this.download(body.link!);
+                            } else if (body.status === 'Error') {
+                              this.handleError(body.message);
                             }
                           },
-                          error => this.handleError(error),
+                          error => this.handleError(error.message),
                           () => this.isLoading = false
                         );
   }
@@ -149,9 +152,9 @@ export class WpTableExportModal extends OpModalComponent implements OnInit, OnDe
       .get(url, { observe: 'response', responseType: 'text' });
   }
 
-  private handleError(error:HttpErrorResponse) {
+  private handleError(error:string) {
     this.isLoading = false;
-    this.notifications.addError(error.message || this.I18n.t('js.error.internal'));
+    this.notifications.addError(error || this.I18n.t('js.error.internal'));
   }
 
   private addColumnsToHref(href:string) {
@@ -175,9 +178,7 @@ export class WpTableExportModal extends OpModalComponent implements OnInit, OnDe
 
   private download(url:string) {
     this.downloadHref = url;
-    if (this.finished) {
-      this.finished();
-    }
+
     setTimeout(() => {
       this.downloadLink.nativeElement.click();
       this.service.close();

spec/controllers/work_packages/exports_controller_spec.rb

@@ -1,4 +1,3 @@
-
 #-- encoding: UTF-8
 
 #-- copyright
@@ -61,6 +60,22 @@ describe WorkPackages::ExportsController, type: :controller do
         .and_return(export_attachments)
     end
   end
+
+  let(:status) { 'in_queue' }
+  let(:job_message) { '' }
+  let!(:job_status) do
+    FactoryBot.build_stubbed(:delayed_job_status, status: status, message: job_message).tap do |s|
+      relation = double('AR relation')
+      allow(Delayed::Job::Status)
+        .to receive(:of_reference)
+        .with(export)
+        .and_return(relation)
+
+      allow(relation)
+        .to receive(:first)
+        .and_return(s)
+    end
+  end
   let(:attachment_done) { true }
 
   describe 'show' do
@@ -86,7 +101,7 @@ describe WorkPackages::ExportsController, type: :controller do
 
         it 'returns the status location via the link header' do
           expect(response.headers['Link'])
-            .to eql "</work_packages/exports/#{export.id}/status> rel=\"status\""
+            .to eql "</work_packages/exports/#{export.id}/status.json> rel=\"status\""
         end
       end
 
@@ -115,10 +130,12 @@ describe WorkPackages::ExportsController, type: :controller do
   describe 'status' do
     context 'with an existing id' do
       before do
-        get 'status', params: { id: export.id }
+        get 'status', params: { id: export.id, format: :json }
       end
 
-      context 'with the attachment being ready' do
+      context 'with the associated job status signaling success' do
+        let(:status) { :success }
+
         it 'returns 200 OK' do
           expect(response.status)
             .to eql 200
@@ -131,12 +148,34 @@ describe WorkPackages::ExportsController, type: :controller do
 
         it 'reads "Completed"' do
           expect(response.body)
-            .to eql 'Completed'
+            .to be_json_eql({ "status": "Completed",
+                              "message": "",
+                              "link": api_v3_paths.attachment_content(attachment.id) }.to_json)
         end
       end
 
-      context 'with the attachment not being ready' do
-        let(:attachment_done) { false }
+      context 'with the associated job status being in_queue' do
+        let(:status) { 'in_queue' }
+
+        it 'returns 200 OK' do
+          expect(response.status)
+            .to eql 200
+        end
+
+        it 'has no link to the download' do
+          expect(response.headers['Link'])
+            .to be nil
+        end
+
+        it 'reads "Processing"' do
+          expect(response.body)
+            .to be_json_eql({ "status": "Processing",
+                              "message": "" }.to_json)
+        end
+      end
+
+      context 'with the associated job status being in_process' do
+        let(:status) { :in_process }
 
         it 'returns 200 OK' do
           expect(response.status)
@@ -150,7 +189,50 @@ describe WorkPackages::ExportsController, type: :controller do
 
         it 'reads "Processing"' do
           expect(response.body)
-            .to eql 'Processing'
+            .to be_json_eql({ "status": "Processing",
+                              "message": "" }.to_json)
+        end
+      end
+
+      context 'with the associated job status being error' do
+        let(:status) { :error }
+        let(:job_message) { 'This errored.' }
+
+        it 'returns 200 OK' do
+          expect(response.status)
+            .to eql 200
+        end
+
+        it 'has no link to the download' do
+          expect(response.headers['Link'])
+            .to be nil
+        end
+
+        it 'reads "Error" with the error message' do
+          expect(response.body)
+            .to be_json_eql({ "status": "Error",
+                              "message": job_message }.to_json)
+        end
+      end
+
+      context 'with the associated job status being failure' do
+        let(:status) { :failure }
+        let(:job_message) { 'This failed.' }
+
+        it 'returns 200 OK' do
+          expect(response.status)
+            .to eql 200
+        end
+
+        it 'has no link to the download' do
+          expect(response.headers['Link'])
+            .to be nil
+        end
+
+        it 'reads "Error" with the error message' do
+          expect(response.body)
+            .to be_json_eql({ "status": "Error",
+                              "message": job_message }.to_json)
         end
       end
 
@@ -166,7 +248,7 @@ describe WorkPackages::ExportsController, type: :controller do
 
     context 'with an inexisting id' do
       before do
-        get 'show', params: { id: export.id + 5 }
+        get 'show', params: { id: export.id + 5, format: :json }
       end
 
       it 'returns 404 NOT FOUND' do

spec/factories/delayed_job_status_factory.rb

@@ -0,0 +1,33 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+FactoryBot.define do
+  factory :delayed_job_status, class: Delayed::Job::Status do
+
+  end
+end

spec/features/work_packages/export_spec.rb

@@ -58,34 +58,36 @@ describe 'work package export', type: :feature do
     login_as(current_user)
   end
 
+  let(:export_type) { 'CSV' }
   subject { DownloadedFile.download_content }
 
-  def export!
+  def export!(wait_for_downloads = true)
     DownloadedFile::clear_downloads
 
     work_packages_page.ensure_loaded
 
     settings_menu.open_and_choose 'Export ...'
-    click_on 'CSV'
+    click_on export_type
 
+    begin
       perform_enqueued_jobs
+    rescue
+      # nothing
+    end
+
+
+    if wait_for_downloads
       # Wait for the file to download
       ::DownloadedFile.wait_for_download
       ::DownloadedFile.wait_for_download_content
     end
-
-  #before do
-  #  # render the CSV as plain text so we can run expectations against the output
-  #  expect_any_instance_of(WorkPackagesController)
-  #    .to receive(:send_data) do |receiver, serialized_work_packages, _opts|
-  #    receiver.render plain: serialized_work_packages
-  #  end
-  #end
+  end
 
   after do
     DownloadedFile::clear_downloads
   end
 
+  context 'CSV export' do
     context 'with default filter' do
       before do
         work_packages_page.visit_index
@@ -198,3 +200,38 @@ describe 'work package export', type: :feature do
       end
     end
   end
+
+  context 'PDF export', js: true do
+    let(:export_type) { 'PDF' }
+    let(:query) do
+      FactoryBot.create :query,
+                        user: current_user,
+                        project: project
+    end
+
+    context 'with many columns' do
+      before do
+        query.column_names = query.available_columns.map { |c| c.name.to_s } - ['bcf_thumbnail']
+        query.save!
+
+        # Despite attempts to provoke the error by having a lot of columns, the pdf
+        # is still being drawn successfully. We thus have to fake the error.
+        allow_any_instance_of(WorkPackage::PDFExport::WorkPackageListToPdf)
+          .to receive(:render!)
+          .and_return(WorkPackage::Exporter::Result::Error.new(I18n.t(:error_pdf_export_too_many_columns)))
+      end
+
+      it 'returns the error' do
+        wp_table.visit_query query
+
+        export!(false)
+
+        expect(page)
+          .not_to have_content I18n.t('js.label_export_preparing')
+
+        expect(page)
+          .to have_content I18n.t(:error_pdf_export_too_many_columns)
+      end
+    end
+  end
+end