diff --git a/src/documents/barcodes.py b/src/documents/barcodes.py index 75a196250..20d5028c7 100644 --- a/src/documents/barcodes.py +++ b/src/documents/barcodes.py @@ -116,6 +116,24 @@ class BarcodePlugin(ConsumeTaskPlugin): self._tiff_conversion_done = False self.barcodes: list[Barcode] = [] + def _apply_detected_asn(self, detected_asn: int) -> None: + """ + Apply a detected ASN to metadata if allowed. + """ + if ( + self.metadata.skip_asn_if_exists + and Document.global_objects.filter( + archive_serial_number=detected_asn, + ).exists() + ): + logger.info( + f"Found ASN in barcode {detected_asn} but skipping because it already exists.", + ) + return + + logger.info(f"Found ASN in barcode: {detected_asn}") + self.metadata.asn = detected_asn + def run(self) -> None: # Some operations may use PIL, override pixel setting if needed maybe_override_pixel_limit() @@ -188,18 +206,7 @@ class BarcodePlugin(ConsumeTaskPlugin): # Update/overwrite an ASN if possible # After splitting, as otherwise each split document gets the same ASN if self.settings.barcode_enable_asn and (located_asn := self.asn) is not None: - if ( - self.metadata.skip_asn_if_exists - and Document.global_objects.filter( - archive_serial_number=located_asn, - ).exists() - ): - logger.info( - f"Found ASN in barcode {located_asn} but skipping because it already exists.", - ) - else: - logger.info(f"Found ASN in barcode: {located_asn}") - self.metadata.asn = located_asn + self._apply_detected_asn(located_asn) def cleanup(self) -> None: self.temp_dir.cleanup() diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py index 347a372ac..ef6501acd 100644 --- a/src/documents/tests/test_bulk_edit.py +++ b/src/documents/tests/test_bulk_edit.py @@ -670,6 +670,85 @@ class TestPDFActions(DirectoriesMixin, TestCase): self.assertIsNone(self.doc2.archive_serial_number) self.assertIsNone(self.doc3.archive_serial_number) + @mock.patch("documents.bulk_edit.delete.si") + @mock.patch("documents.tasks.consume_file.s") + def test_merge_and_delete_originals_restore_on_failure( + self, + mock_consume_file, + mock_delete_documents, + ): + """ + GIVEN: + - Existing documents + WHEN: + - Merge action with deleting documents is called with 1 document + - Error occurs when queuing consume file task + THEN: + - Archive serial numbers are restored + """ + doc_ids = [self.doc1.id] + self.doc1.archive_serial_number = 111 + self.doc1.save() + sig = mock.Mock() + sig.apply_async.side_effect = Exception("boom") + mock_consume_file.return_value = sig + + with self.assertRaises(Exception): + bulk_edit.merge(doc_ids, delete_originals=True) + + self.doc1.refresh_from_db() + self.assertEqual(self.doc1.archive_serial_number, 111) + + @mock.patch("documents.bulk_edit.delete.si") + @mock.patch("documents.tasks.consume_file.s") + def test_merge_and_delete_originals_metadata_handoff( + self, + mock_consume_file, + mock_delete_documents, + ): + """ + GIVEN: + - Existing documents with ASNs + WHEN: + - Merge with delete_originals=True and metadata_document_id set + THEN: + - Handoff ASN uses metadata document ASN + """ + doc_ids = [self.doc1.id, self.doc2.id] + self.doc1.archive_serial_number = 101 + self.doc2.archive_serial_number = 202 + self.doc1.save() + self.doc2.save() + + result = bulk_edit.merge( + doc_ids, + metadata_document_id=self.doc2.id, + delete_originals=True, + ) + self.assertEqual(result, "OK") + + consume_file_args, _ = mock_consume_file.call_args + self.assertEqual(consume_file_args[1].asn, 202) + + def test_restore_archive_serial_numbers_task(self): + """ + GIVEN: + - Existing document with no archive serial number + WHEN: + - Restore archive serial number task is called with backup data + THEN: + - Document archive serial number is restored + """ + self.doc1.archive_serial_number = 444 + self.doc1.save() + Document.objects.filter(pk=self.doc1.id).update(archive_serial_number=None) + + backup = {self.doc1.id: 444} + bulk_edit.restore_archive_serial_numbers_task(backup) + + self.doc1.refresh_from_db() + self.assertEqual(self.doc1.archive_serial_number, 444) + @mock.patch("documents.tasks.consume_file.s") def test_merge_with_archive_fallback(self, mock_consume_file): """ @@ -785,6 +864,39 @@ class TestPDFActions(DirectoriesMixin, TestCase): self.doc2.refresh_from_db() self.assertIsNone(self.doc2.archive_serial_number) + @mock.patch("documents.bulk_edit.delete.si") + @mock.patch("documents.tasks.consume_file.s") + @mock.patch("documents.bulk_edit.chord") + def test_split_restore_on_failure( + self, + mock_chord, + mock_consume_file, + mock_delete_documents, + ): + """ + GIVEN: + - Existing documents + WHEN: + - Split action with deleting documents is called with 1 document and 2 page groups + - Error occurs when queuing chord task + THEN: + - Archive serial numbers are restored + """ + doc_ids = [self.doc2.id] + pages = [[1, 2]] + self.doc2.archive_serial_number = 222 + self.doc2.save() + + sig = mock.Mock() + sig.apply_async.side_effect = Exception("boom") + mock_chord.return_value = sig + + result = bulk_edit.split(doc_ids, pages, delete_originals=True) + self.assertEqual(result, "OK") + + self.doc2.refresh_from_db() + self.assertEqual(self.doc2.archive_serial_number, 222) + @mock.patch("documents.tasks.consume_file.delay") @mock.patch("pikepdf.Pdf.save") def test_split_with_errors(self, mock_save_pdf, mock_consume_file): @@ -996,6 +1108,39 @@ class TestPDFActions(DirectoriesMixin, TestCase): self.doc2.refresh_from_db() self.assertIsNone(self.doc2.archive_serial_number) + @mock.patch("documents.bulk_edit.delete.si") + @mock.patch("documents.tasks.consume_file.s") + @mock.patch("documents.bulk_edit.chord") + def test_edit_pdf_restore_on_failure( + self, + mock_chord, + mock_consume_file, + mock_delete_documents, + ): + """ + GIVEN: + - Existing document + WHEN: + - edit_pdf is called with delete_original=True + - Error occurs when queuing chord task + THEN: + - Archive serial numbers are restored + """ + doc_ids = [self.doc2.id] + operations = [{"page": 1}] + self.doc2.archive_serial_number = 333 + self.doc2.save() + + sig = mock.Mock() + sig.apply_async.side_effect = Exception("boom") + mock_chord.return_value = sig + + with self.assertRaises(Exception): + bulk_edit.edit_pdf(doc_ids, operations, delete_original=True) + + self.doc2.refresh_from_db() + self.assertEqual(self.doc2.archive_serial_number, 333) + @mock.patch("documents.tasks.update_document_content_maybe_archive_file.delay") def test_edit_pdf_with_update_document(self, mock_update_document): """ diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py index 6387b5e95..325279898 100644 --- a/src/documents/tests/test_consumer.py +++ b/src/documents/tests/test_consumer.py @@ -14,6 +14,7 @@ from django.test import override_settings from django.utils import timezone from guardian.core import ObjectPermissionChecker +from documents.barcodes import BarcodePlugin from documents.consumer import ConsumerError from documents.data_models import DocumentMetadataOverrides from documents.data_models import DocumentSource @@ -1232,3 +1233,46 @@ class PostConsumeTestCase(DirectoriesMixin, GetConsumerMixin, TestCase): r"sample\.pdf: Error while executing post-consume script: Command '\[.*\]' returned non-zero exit status \d+\.", ): consumer.run_post_consume_script(doc) + + +class TestMetadataOverrides(TestCase): + def test_update_skip_asn_if_exists(self): + base = DocumentMetadataOverrides() + incoming = DocumentMetadataOverrides(skip_asn_if_exists=True) + base.update(incoming) + self.assertTrue(base.skip_asn_if_exists) + + +class TestBarcodeApplyDetectedASN(TestCase): + """ + GIVEN: + - Existing Documents with ASN 123 + WHEN: + - A BarcodePlugin which detected an ASN + THEN: + - If skip_asn_if_exists is set, and ASN exists, do not set ASN + - If skip_asn_if_exists is set, and ASN does not exist, set ASN + """ + + def test_apply_detected_asn_skips_existing_when_flag_set(self): + doc = Document.objects.create( + checksum="X1", + title="D1", + archive_serial_number=123, + ) + metadata = DocumentMetadataOverrides(skip_asn_if_exists=True) + plugin = BarcodePlugin( + input_doc=mock.Mock(), + metadata=metadata, + status_mgr=mock.Mock(), + base_tmp_dir=tempfile.gettempdir(), + task_id="test-task", + ) + + plugin._apply_detected_asn(123) + self.assertIsNone(plugin.metadata.asn) + + doc.hard_delete() + + plugin._apply_detected_asn(123) + self.assertEqual(plugin.metadata.asn, 123)