Skip to content

Commit

Permalink
Merge pull request #4055 from open-formulieren/feature/4041-robustness
Browse files Browse the repository at this point in the history
 [#4041] Improve API documents registration robustness
  • Loading branch information
sergei-maertens authored Mar 25, 2024
2 parents 755c3e8 + b86e990 commit e406ef2
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,10 @@ def register_submission_attachment(


@contextmanager
def save_and_raise(registration_data: ObjectsAPIRegistrationData) -> Iterator[None]:
def save_and_raise(
registration_data: ObjectsAPIRegistrationData,
submission_attachments: list[ObjectsAPISubmissionAttachment],
) -> Iterator[None]:
"""Save the registration data before raising a :class:`~openforms.registrations.exceptions.RegistrationFailed` exception."""

try:
Expand All @@ -174,6 +177,7 @@ def save_and_raise(registration_data: ObjectsAPIRegistrationData) -> Iterator[No
raise RegistrationFailed() from e
finally:
registration_data.save()
ObjectsAPISubmissionAttachment.objects.bulk_create(submission_attachments)


OptionsT = TypeVar(
Expand Down Expand Up @@ -210,10 +214,11 @@ def save_registration_data(
registration_data, _ = ObjectsAPIRegistrationData.objects.get_or_create(
submission=submission
)
submission_attachments: list[ObjectsAPISubmissionAttachment] = []

with (
get_documents_client() as documents_client,
save_and_raise(registration_data),
save_and_raise(registration_data, submission_attachments),
):
if not registration_data.pdf_url:
registration_data.pdf_url = register_submission_pdf(
Expand All @@ -233,11 +238,9 @@ def save_registration_data(
)
]

objs: list[ObjectsAPISubmissionAttachment] = []

for attachment in submission.attachments:
if attachment not in existing:
objs.append(
submission_attachments.append(
ObjectsAPISubmissionAttachment(
submission_file_attachment=attachment,
document_url=register_submission_attachment(
Expand All @@ -246,8 +249,6 @@ def save_registration_data(
)
)

ObjectsAPISubmissionAttachment.objects.bulk_create(objs)

@abstractmethod
def get_object_data(
self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@

from openforms.registrations.contrib.objects_api.models import (
ObjectsAPIRegistrationData,
ObjectsAPISubmissionAttachment,
)
from openforms.registrations.exceptions import RegistrationFailed
from openforms.submissions.tests.factories import SubmissionFactory
from openforms.submissions.tests.factories import (
SubmissionFactory,
SubmissionFileAttachmentFactory,
)

from ..models import ObjectsAPIConfig
from ..plugin import PLUGIN_IDENTIFIER, ObjectsAPIRegistration
Expand Down Expand Up @@ -104,6 +108,77 @@ def test_csv_creation_fails_pdf_still_saved(self, m: requests_mock.Mocker):
self.assertEqual(registration_data.pdf_url, pdf["url"])
self.assertEqual(registration_data.csv_url, "")

def test_attachment_fails_other_attachments_still_saved(
self, m: requests_mock.Mocker
):
"""Test the behavior when one of the API calls to register an attachment fails.
The exception should be caught, the first attachment saved, and a
``RegistrationFailed`` should be raised in the end.
"""

submission = SubmissionFactory.from_components(
[
{
"key": "file_1",
"type": "file",
},
{
"key": "file_2",
"type": "file",
},
],
completed=True,
)
submission_step = submission.steps[0]
attachment_1 = SubmissionFileAttachmentFactory.create(
submission_step=submission_step,
file_name="attachment1.jpg",
form_key="file_1",
)
SubmissionFileAttachmentFactory.create(
submission_step=submission_step,
file_name="attachment2.png",
form_key="file_2",
)

jpg_resp = generate_oas_component(
"documenten",
"schemas/EnkelvoudigInformatieObject",
url="https://documenten.nl/api/v1/enkelvoudiginformatieobjecten/1",
)

# OK on JPG attachment request
m.post(
"https://documenten.nl/api/v1/enkelvoudiginformatieobjecten",
status_code=201,
json=jpg_resp,
additional_matcher=lambda req: req.json()["bestandsnaam"].endswith(".jpg"),
)

# Failure on PNG attachment request (which is dispatched after the JPG one)
m.post(
"https://documenten.nl/api/v1/enkelvoudiginformatieobjecten",
status_code=500,
additional_matcher=lambda req: req.json()["bestandsnaam"].endswith(".png"),
)

plugin = ObjectsAPIRegistration(PLUGIN_IDENTIFIER)

with self.assertRaises(RegistrationFailed):
plugin.register_submission(
submission,
{
"upload_submission_csv": False,
"informatieobjecttype_attachment": "dummy",
},
)

attachment = ObjectsAPISubmissionAttachment.objects.filter(
submission_file_attachment=attachment_1
)
self.assertTrue(attachment.exists())

def test_registration_works_after_failure(self, m: requests_mock.Mocker):
"""Test the registration behavior after a failure.
Expand Down

0 comments on commit e406ef2

Please sign in to comment.