Publish objects using a background job

Objects that have several hundred files may take several minutes to publish, so make this an asynchronous job.  In our testing we found a 600 page book took over 82s to publish

app/controllers/background_job_results_controller.rb

@@ -11,10 +11,7 @@ class BackgroundJobResultsController < ApplicationController
   end
 
   def show
-    result = BackgroundJobResult.find(params[:id])
-    @output = result.output
-    @status = result.status
-
-    render status: result.code
+    @result = BackgroundJobResult.find(params[:id])
+    render status: @result.complete? ? :ok : :accepted
   end
 end

app/controllers/objects_controller.rb

@@ -41,16 +41,9 @@ def show
   end
 
   def publish
-    begin
-      PublishMetadataService.publish(@item)
-    rescue Dor::DataError => e
-      return render json: {
-        errors: [
-          { title: 'Data error', detail: e.message }
-        ]
-      }, status: :unprocessable_entity
-    end
-    head :created
+    result = BackgroundJobResult.create
+    PublishJob.perform_later(druid: params[:id], background_job_result: result)
+    head :created, location: result
   end
 
   def update_marc_record

app/jobs/create_virtual_objects_job.rb

@@ -20,7 +20,6 @@ def perform(virtual_objects:, background_job_result:)
     end
 
     background_job_result.output = { errors: errors } if errors.any?
-    background_job_result.code = 200
 
     background_job_result.complete!
   end

app/jobs/publish_job.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+# Create virtual objects in the background
+class PublishJob < ApplicationJob
+  queue_as :default
+
+  # @param [String] druid the identifier of the item to be published
+  # @param [BackgroundJobResult] background_job_result identifier of a background job result to store status info
+  def perform(druid:, background_job_result:)
+    background_job_result.processing!
+
+    errors = []
+
+    begin
+      item = Dor.find(druid)
+      PublishMetadataService.publish(item)
+    rescue Dor::DataError => e
+      errors << { title: 'Data error', detail: e.message }
+    end
+
+    background_job_result.output = { errors: errors } if errors.any?
+
+    background_job_result.complete!
+  end
+end

app/views/background_job_results/show.json.jbuilder

@@ -1,4 +1,4 @@
 # frozen_string_literal: true
 
-json.output @output
-json.status @status
+json.output @result.output
+json.status @result.status

db/migrate/20191015193638_remove_background_job_results_code.rb

@@ -0,0 +1,5 @@
+class RemoveBackgroundJobResultsCode < ActiveRecord::Migration[5.2]
+  def change
+    remove_column :background_job_results, :code
+  end
+end

db/structure.sql

@@ -5,10 +5,23 @@ SET client_encoding = 'UTF8';
 SET standard_conforming_strings = on;
 SELECT pg_catalog.set_config('search_path', '', false);
 SET check_function_bodies = false;
-SET xmloption = content;
 SET client_min_messages = warning;
 SET row_security = off;
 
+--
+-- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: -
+--
+
+CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;
+
+
+--
+-- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner: -
+--
+
+COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';
+
+
 --
 -- Name: background_job_result_status; Type: TYPE; Schema: public; Owner: -
 --
@@ -43,7 +56,6 @@ CREATE TABLE public.ar_internal_metadata (
 CREATE TABLE public.background_job_results (
     id bigint NOT NULL,
     output json DEFAULT '{}'::json,
-    code integer DEFAULT 202,
     status public.background_job_result_status DEFAULT 'pending'::public.background_job_result_status,
     created_at timestamp without time zone NOT NULL,
     updated_at timestamp without time zone NOT NULL
@@ -116,6 +128,7 @@ ALTER TABLE ONLY public.schema_migrations
 SET search_path TO "$user", public;
 
 INSERT INTO "schema_migrations" (version) VALUES
-('20190917215521');
+('20190917215521'),
+('20191015193638');
 
 

openapi.json

@@ -316,12 +316,18 @@
         "description": "Combines a parent object with child objects to create an object like an Atlas (composed of several images)",
         "operationId": "virtual_objects#create",
         "responses": {
-          "204": {
-            "description": "objects created"
+          "201": {
+            "description": "Virtual merge action started",
+            "headers": {
+              "Location": {
+                "description": "the status of the action is found at this URI",
+                "schema": {
+                  "type": "string",
+                  "format": "uri"
+                }
+              }
+            }
           },
-          "422": {
-            "description": "unable to process request"
-          }
         },
         "requestBody": {
           "content": {
@@ -351,15 +357,14 @@
         "description": "",
         "operationId": "objects#publish",
         "responses": {
-          "200": {
-            "description": "OK"
-          },
-          "422": {
-            "description": "There is a problem with the data that makes is impossible to publish",
-            "content": {
-              "application/json": {
+          "201": {
+            "description": "Publishing action started",
+            "headers": {
+              "Location": {
+                "description": "the status of the action is found at this URI",
                 "schema": {
-                  "$ref": "#/components/schemas/ErrorResponse"
+                  "type": "string",
+                  "format": "uri"
                 }
               }
             }

spec/factories/background_job_results.rb

@@ -4,6 +4,5 @@
   factory :background_job_result do
     output { {} }
     status { 'pending' }
-    code { 202 }
   end
 end

spec/jobs/create_virtual_objects_job_spec.rb

@@ -34,10 +34,6 @@
       expect(result).to be_complete
     end
 
-    it 'sets the HTTP status code to 200' do
-      expect(result.code).to eq(200)
-    end
-
     it 'has no output' do
       expect(result.output).to be_blank
     end
@@ -62,10 +58,6 @@
       expect(result).to be_complete
     end
 
-    it 'sets the HTTP status code to 200' do
-      expect(result.code).to eq(200)
-    end
-
     it 'has output with errors' do
       expect(result.output[:errors].first[parent_id]).to match_array(['One thing was not combinable', 'And another'])
     end

spec/jobs/publish_job_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe PublishJob, type: :job do
+  subject(:perform) { described_class.perform_now(druid: druid, background_job_result: result) }
+
+  let(:druid) { 'druid:mk420bs7601' }
+  let(:result) { create(:background_job_result) }
+  let(:item) { instance_double(Dor::Item) }
+
+  before do
+    allow(Dor).to receive(:find).with(druid).and_return(item)
+    allow(result).to receive(:processing!)
+  end
+
+  context 'with no errors' do
+    before do
+      allow(PublishMetadataService).to receive(:publish)
+      perform
+    end
+
+    it 'marks the job as processing' do
+      expect(result).to have_received(:processing!).once
+    end
+
+    it 'invokes the PublishMetadataService' do
+      expect(PublishMetadataService).to have_received(:publish).with(item).once
+    end
+
+    it 'marks the job as complete' do
+      expect(result).to be_complete
+    end
+
+    it 'has no output' do
+      expect(result.output).to be_blank
+    end
+  end
+
+  context 'with errors returned by PublishMetadataService' do
+    let(:error_message) { "DublinCoreService#ng_xml produced incorrect xml (no children):\n<xml/>" }
+
+    before do
+      allow(PublishMetadataService).to receive(:publish).and_raise(Dor::DataError, error_message)
+      perform
+    end
+
+    it 'marks the job as processing' do
+      expect(result).to have_received(:processing!).once
+    end
+
+    it 'invokes the PublishMetadataService' do
+      expect(PublishMetadataService).to have_received(:publish).with(item).once
+    end
+
+    it 'marks the job as complete' do
+      expect(result).to be_complete
+    end
+
+    it 'has output with errors' do
+      expect(result.output[:errors]).to eq [{ 'detail' => error_message, 'title' => 'Data error' }]
+    end
+  end
+end

spec/requests/background_job_results_spec.rb

@@ -56,7 +56,7 @@
   end
 
   context 'when it is complete' do
-    let(:background_job_result) { create(:background_job_result, code: code, output: output) }
+    let(:background_job_result) { create(:background_job_result, output: output) }
 
     before do
       background_job_result.complete!
@@ -65,7 +65,6 @@
     end
 
     context 'without errors' do
-      let(:code) { 200 }
       let(:output) { { result: 'succeeded!' } }
 
       it 'renders an HTTP 200 status code' do
@@ -82,7 +81,6 @@
     end
 
     context 'with errors' do
-      let(:code) { 200 }
       let(:output) { { errors: [{ detail: 'failed!' }] } }
 
       it 'renders an HTTP 200 status code' do

spec/requests/publish_object_spec.rb

@@ -7,33 +7,13 @@
 
   before do
     allow(Dor).to receive(:find).and_return(object)
+    allow(PublishJob).to receive(:perform_later)
   end
 
-  context 'with bad metadata' do
-    let(:error_message) { "DublinCoreService#ng_xml produced incorrect xml (no children):\n<xml/>" }
+  it 'calls PublishMetadataService and returns 201' do
+    post '/v1/objects/druid:1234/publish', headers: { 'Authorization' => "Bearer #{jwt}" }
 
-    before do
-      allow(PublishMetadataService).to receive(:publish).and_raise(Dor::DataError, error_message)
-    end
-
-    it 'returns a 422 error with location header' do
-      post '/v1/objects/druid:1234/publish', headers: { 'Authorization' => "Bearer #{jwt}" }
-      expect(response.status).to eq(422)
-      json = JSON.parse(response.body)
-      expect(json.fetch('errors').first.fetch('detail')).to eq(error_message)
-    end
-  end
-
-  context 'when the request is successful' do
-    before do
-      allow(PublishMetadataService).to receive(:publish)
-    end
-
-    it 'calls PublishMetadataService and returns 201' do
-      post '/v1/objects/druid:1234/publish', headers: { 'Authorization' => "Bearer #{jwt}" }
-
-      expect(PublishMetadataService).to have_received(:publish)
-      expect(response.status).to eq(201)
-    end
+    expect(PublishJob).to have_received(:perform_later).with(druid: 'druid:1234', background_job_result: BackgroundJobResult)
+    expect(response.status).to eq(201)
   end
 end