More robust DOI commitment flow

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

isic/core/tests/test_doi.py

@@ -8,9 +8,12 @@
 
 @pytest.fixture
 def mock_datacite_create_doi(mocker):
-    yield mocker.patch(
-        "isic.core.services.collection.doi._datacite_create_doi", lambda doi: {"doi": "123456"}
-    )
+    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
@@ -27,14 +30,19 @@ 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)
 
     public_collection_with_public_images.refresh_from_db()
     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
@@ -58,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={},
@@ -71,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