From 4c3f0880dd841863f9dba4bbf637709e3f98109e Mon Sep 17 00:00:00 2001 From: jazairi <16103405+jazairi@users.noreply.github.com> Date: Tue, 21 Nov 2023 11:17:23 -0500 Subject: [PATCH] Refactor error handling in Preservation Submission Job Why these changes are being introduced: The Preservation Submission Job currently handles exceptions during SIP creation by setting the SIP's status to 'error' and saving it. This allows us to see failed SIPs in the administrate dashboard, but it also causes preservation to fail silently. Relevant ticket(s): https://mitlibraries.atlassian.net/browse/ETD-647 How this addresses that need: This refactors the Preservation Submission Job to create SIPs using `create!` instead of `create`, so that exceptions are raised during the object creation. When exceptions are raised, they are logged and added to the `results` array. Since no SIP is created when an exception is raised by `create!`, the logic in the `rescue` block around updating `preservation_status` has been removed. Side effects of this change: * Fixtures have been adjusted to accommodate testing. * A quasi-related test that had no assertions has been updated to actually test something. * The `preservation_status` of 'error' has been removed, as it is no longer used in the app. There are currently no SIPs in production that have this status. * Unrelated to this change, the `baggable` module has been moved from `app/models` to `app/lib`. This to be consistent with where we store our other module, and to make it clear that it is not a model. --- app/jobs/preservation_submission_job.rb | 4 +-- app/{models => lib}/baggable.rb | 0 app/models/submission_information_package.rb | 2 +- test/fixtures/archivematica_accessions.yml | 4 +++ test/fixtures/degree_periods.yml | 4 +++ test/fixtures/theses.yml | 1 + test/jobs/preservation_submission_job_test.rb | 32 +++++++++++++++---- 7 files changed, 36 insertions(+), 11 deletions(-) rename app/{models => lib}/baggable.rb (100%) diff --git a/app/jobs/preservation_submission_job.rb b/app/jobs/preservation_submission_job.rb index 053a1575..df6c83aa 100644 --- a/app/jobs/preservation_submission_job.rb +++ b/app/jobs/preservation_submission_job.rb @@ -6,15 +6,13 @@ def perform(theses) results = { total: theses.count, processed: 0, errors: [] } theses.each do |thesis| Rails.logger.info("Thesis #{thesis.id} is now being prepared for preservation") - sip = thesis.submission_information_packages.create + sip = thesis.submission_information_packages.create! preserve_sip(sip) Rails.logger.info("Thesis #{thesis.id} has been sent to preservation") results[:processed] += 1 rescue StandardError, Aws::Errors => e preservation_error = "Thesis #{thesis.id} could not be preserved: #{e}" Rails.logger.info(preservation_error) - sip.preservation_status = 'error' - sip.save results[:errors] << preservation_error end ReportMailer.preservation_results_email(results).deliver_now if results[:total].positive? diff --git a/app/models/baggable.rb b/app/lib/baggable.rb similarity index 100% rename from app/models/baggable.rb rename to app/lib/baggable.rb diff --git a/app/models/submission_information_package.rb b/app/models/submission_information_package.rb index 9b4b16a4..893d0699 100644 --- a/app/models/submission_information_package.rb +++ b/app/models/submission_information_package.rb @@ -31,7 +31,7 @@ class SubmissionInformationPackage < ApplicationRecord before_create :set_metadata, :set_bag_declaration, :set_manifest, :set_bag_name - enum preservation_status: %i[unpreserved preserved error] + enum preservation_status: %i[unpreserved preserved] def data file_locations = {} diff --git a/test/fixtures/archivematica_accessions.yml b/test/fixtures/archivematica_accessions.yml index a38e55ee..78788f48 100644 --- a/test/fixtures/archivematica_accessions.yml +++ b/test/fixtures/archivematica_accessions.yml @@ -28,3 +28,7 @@ june_2018_001: june_2021_001: accession_number: '2021_001' degree_period: june_2021 + +june_2022_001: + accession_number: '2022_001' + degree_period: june_2022 diff --git a/test/fixtures/degree_periods.yml b/test/fixtures/degree_periods.yml index ce4e81eb..2f1b71aa 100644 --- a/test/fixtures/degree_periods.yml +++ b/test/fixtures/degree_periods.yml @@ -28,3 +28,7 @@ june_2018: june_2021: grad_month: 'June' grad_year: '2021' + +june_2022: + grad_month: 'June' + grad_year: '2022' diff --git a/test/fixtures/theses.yml b/test/fixtures/theses.yml index e4201f1c..e201f901 100644 --- a/test/fixtures/theses.yml +++ b/test/fixtures/theses.yml @@ -275,6 +275,7 @@ engineer: metadata_complete: true issues_found: false publication_status: Published + dspace_handle: '3456/7890' proquest_exported: 'Full harvest' long_abstracts_are_fun: diff --git a/test/jobs/preservation_submission_job_test.rb b/test/jobs/preservation_submission_job_test.rb index 5da19745..55108272 100644 --- a/test/jobs/preservation_submission_job_test.rb +++ b/test/jobs/preservation_submission_job_test.rb @@ -33,7 +33,19 @@ def setup_thesis end test 'creates multiple SIPs' do - theses = [setup_thesis, theses(:published)] + thesis_one = setup_thesis + thesis_two = theses(:engineer) + assert_equal 0, thesis_one.submission_information_packages.count + assert_equal 0, thesis_two.submission_information_packages.count + + theses = [thesis_one, thesis_two] + PreservationSubmissionJob.perform_now(theses) + assert_equal 1, thesis_one.submission_information_packages.count + assert_equal 1, thesis_two.submission_information_packages.count + + PreservationSubmissionJob.perform_now(theses) + assert_equal 2, thesis_one.submission_information_packages.count + assert_equal 2, thesis_two.submission_information_packages.count end test 'updates preservation_status to "preserved" after successfully processing a thesis' do @@ -51,15 +63,21 @@ def setup_thesis end end - test 'rescues exceptions by updating preservation_status to "error"' do - thesis = theses(:one) - PreservationSubmissionJob.perform_now([thesis]) - assert_equal 'error', thesis.submission_information_packages.last.preservation_status + test 'throws exceptions when a thesis is unbaggable' do + assert_raises StandardError do + PreservationSubmissionJob.perform_now([theses[:one]]) + end + + assert_nothing_raised do + PreservationSubmissionJob.perform_now([setup_thesis]) + end end - test 'does not update preserved_at if the job enters an error state' do + test 'does not create a SIP if the job enters an error state' do thesis = theses(:one) + assert_empty thesis.submission_information_packages + PreservationSubmissionJob.perform_now([thesis]) - assert_nil thesis.submission_information_packages.last.preserved_at + assert_empty thesis.submission_information_packages end end