From 92cbc8a0e10223e235dc779db03ea215370a985c Mon Sep 17 00:00:00 2001 From: Zach Mullen Date: Thu, 24 Aug 2023 15:32:48 -0400 Subject: [PATCH 1/2] More robust DOI commitment flow --- isic/core/services/collection/doi.py | 71 +++++++++++++++++++++------- isic/core/tests/test_doi.py | 4 +- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/isic/core/services/collection/doi.py b/isic/core/services/collection/doi.py index 2bd04730..40038350 100644 --- a/isic/core/services/collection/doi.py +++ b/isic/core/services/collection/doi.py @@ -1,5 +1,6 @@ import logging import random +from urllib import parse from django.conf import settings from django.contrib.auth.models import User @@ -57,6 +58,17 @@ def collection_build_doi(*, collection: Collection, doi_id: str) -> dict: } +def collection_build_draft_doi(*, doi_id: str) -> dict: + return { + "data": { + "type": "dois", + "attributes": { + "doi": doi_id, + }, + } + } + + def collection_generate_random_doi_id(): # pad DOI with leading zeros so all DOIs are prefix/6 digits return f"{settings.ISIC_DATACITE_DOI_PREFIX}/{random.randint(10_000, 999_999):06}" @@ -84,27 +96,54 @@ def _datacite_create_doi(doi: dict) -> None: timeout=5, json=doi, ) - # TODO: check for already exists - r.raise_for_status() + + try: + r.raise_for_status() + except HTTPError: + logger.exception(f"DOI draft creation failed: {r.json()}") + raise ValidationError("Something went wrong creating the DOI.") + + +def _datacite_update_doi(doi: dict, doi_id: str): + doi_quoted = parse.quote(doi_id, safe="") # escape the / for path + r = requests.put( + f"{settings.ISIC_DATACITE_API_URL}/dois/{doi_quoted}", + auth=(settings.ISIC_DATACITE_USERNAME, settings.ISIC_DATACITE_PASSWORD), + timeout=5, + json=doi, + ) + + try: + r.raise_for_status() + except HTTPError: + logger.exception(f"DOI update failed: {r.json()}") + raise ValidationError("Something went wrong publishing the DOI.") def collection_create_doi(*, user: User, collection: Collection) -> Doi: collection_check_create_doi_allowed(user=user, collection=collection) doi_id = collection_generate_random_doi_id() - doi = collection_build_doi(collection=collection, doi_id=doi_id) + draft_doi_dict = collection_build_draft_doi(doi_id=doi_id) + doi_dict = collection_build_doi(collection=collection, doi_id=doi_id) - try: - _datacite_create_doi(doi) - except HTTPError as e: - logger.error(e) - raise ValidationError("Something went wrong creating the DOI.") - else: - with transaction.atomic(): - doi = Doi(id=doi_id, creator=user, url=f"https://doi.org/{doi_id}") - doi.full_clean() - doi.save() - collection_lock(collection=collection) - collection_update(collection=collection, doi=doi, ignore_lock=True) - logger.info("User %d created DOI %s for collection %d", user.id, doi.id, collection.id) + with transaction.atomic(): + # First, create the local DOI record to validate uniqueness within our known set + doi = Doi(id=doi_id, creator=user, url=f"https://doi.org/{doi_id}") + doi.full_clean() + doi.save() + + # Lock the collection, set the DOI on it + collection_lock(collection=collection) + collection_update(collection=collection, doi=doi, ignore_lock=True) + + # Reserve the DOI using the draft mechanism. + # If it fails, transaction will rollback, nothing in our database will change. + _datacite_create_doi(draft_doi_dict) + + # Convert to a published DOI. If this fails, someone will have to come along later and + # retry to publish it. (May want a django-admin action for this if it ever happens.) + _datacite_update_doi(doi_dict, doi_id) + + logger.info("User %d created DOI %s for collection %d", user.id, doi.id, collection.id) return doi diff --git a/isic/core/tests/test_doi.py b/isic/core/tests/test_doi.py index a8fcd7ed..ae63356c 100644 --- a/isic/core/tests/test_doi.py +++ b/isic/core/tests/test_doi.py @@ -8,9 +8,11 @@ @pytest.fixture def mock_datacite_create_doi(mocker): - yield mocker.patch( + mocker.patch( "isic.core.services.collection.doi._datacite_create_doi", lambda doi: {"doi": "123456"} ) + mocker.patch("isic.core.services.collection.doi._datacite_update_doi") + yield @pytest.fixture From 9c0407331f32f29bcfcb5008d36860bfaf7b901a Mon Sep 17 00:00:00 2001 From: Zach Mullen Date: Fri, 25 Aug 2023 10:52:38 -0400 Subject: [PATCH 2/2] Make assertions about mocked DOI publish calls --- isic/core/tests/test_doi.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/isic/core/tests/test_doi.py b/isic/core/tests/test_doi.py index ae63356c..32327c6d 100644 --- a/isic/core/tests/test_doi.py +++ b/isic/core/tests/test_doi.py @@ -8,11 +8,12 @@ @pytest.fixture def mock_datacite_create_doi(mocker): - mocker.patch( - "isic.core.services.collection.doi._datacite_create_doi", lambda doi: {"doi": "123456"} - ) - mocker.patch("isic.core.services.collection.doi._datacite_update_doi") - yield + yield mocker.patch("isic.core.services.collection.doi._datacite_create_doi") + + +@pytest.fixture +def mock_datacite_update_doi(mocker): + yield mocker.patch("isic.core.services.collection.doi._datacite_update_doi") @pytest.fixture @@ -29,7 +30,10 @@ def staff_user_request(staff_user, mocker): @pytest.mark.django_db def test_collection_create_doi( - public_collection_with_public_images, staff_user, mock_datacite_create_doi + public_collection_with_public_images, + staff_user, + mock_datacite_create_doi, + mock_datacite_update_doi, ): collection_create_doi(user=staff_user, collection=public_collection_with_public_images) @@ -37,6 +41,8 @@ def test_collection_create_doi( assert public_collection_with_public_images.locked assert public_collection_with_public_images.doi assert public_collection_with_public_images.doi.creator == staff_user + mock_datacite_create_doi.assert_called_once() + mock_datacite_update_doi.assert_called_once() @pytest.mark.django_db @@ -60,7 +66,10 @@ def test_doi_form_requires_no_existing_doi(public_collection, staff_user_request @pytest.mark.django_db def test_doi_form_creation( - public_collection_with_public_images, staff_user_request, mock_datacite_create_doi + public_collection_with_public_images, + staff_user_request, + mock_datacite_create_doi, + mock_datacite_update_doi, ): form = CreateDoiForm( data={}, @@ -73,6 +82,8 @@ def test_doi_form_creation( public_collection_with_public_images.refresh_from_db() assert public_collection_with_public_images.doi is not None assert public_collection_with_public_images.locked + mock_datacite_create_doi.assert_called_once() + mock_datacite_update_doi.assert_called_once() @pytest.fixture