From 4032ed6c6a3d807f682b7b6e9d0ca7bfd818e12b Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:45 -0400 Subject: [PATCH 01/17] enh: Add Dandiset DOIs - Dandiset DOI will redirect to the DLP - Example: 10.80507/dandi.000004 - Dandiset DOI is stored in the doi field of the draft version - Dandiset DOI metadata (on Datacite) will match the draft version until first publication - Once a Dandiset is published, the Dandiset DOI metadata will match the latest publication See the design document for more details: https://github.com/dandi/dandi-archive/pull/2012 --- dandiapi/api/checks.py | 2 +- dandiapi/api/datacite.py | 246 +++++++++++++ dandiapi/api/doi.py | 136 ++++--- dandiapi/api/services/dandiset/__init__.py | 82 ++++- dandiapi/api/services/embargo/__init__.py | 9 + dandiapi/api/services/publish/__init__.py | 57 ++- dandiapi/api/tasks/__init__.py | 4 +- dandiapi/api/tests/test_asset_paths.py | 2 +- dandiapi/api/tests/test_audit.py | 8 +- dandiapi/api/tests/test_dandiset.py | 143 +++++++- dandiapi/api/tests/test_datacite.py | 405 +++++++++++++++++++++ dandiapi/api/tests/test_tasks.py | 5 +- dandiapi/api/tests/test_unembargo.py | 2 + dandiapi/api/tests/test_version.py | 94 ++++- dandiapi/api/views/version.py | 16 + 15 files changed, 1098 insertions(+), 113 deletions(-) create mode 100644 dandiapi/api/datacite.py create mode 100644 dandiapi/api/tests/test_datacite.py diff --git a/dandiapi/api/checks.py b/dandiapi/api/checks.py index a3339490e..0f17ac47f 100644 --- a/dandiapi/api/checks.py +++ b/dandiapi/api/checks.py @@ -3,7 +3,7 @@ from django.conf import settings from django.core.checks import Error, register -from dandiapi.api.doi import DANDI_DOI_SETTINGS +from dandiapi.api.datacite import DANDI_DOI_SETTINGS @register() diff --git a/dandiapi/api/datacite.py b/dandiapi/api/datacite.py new file mode 100644 index 000000000..70dca4bfd --- /dev/null +++ b/dandiapi/api/datacite.py @@ -0,0 +1,246 @@ +""" +DataCite API client implementation. + +This module provides the implementation details for interacting with the DataCite API. +The public interface is exposed through doi.py. +""" + +from __future__ import annotations + +import copy +import logging +from typing import TYPE_CHECKING + +from django.conf import settings +import requests + +if TYPE_CHECKING: + from dandiapi.api.models import Version + +# All of the required DOI configuration settings +# Cannot be in doi.py to avoid circular imports +DANDI_DOI_SETTINGS = [ + (settings.DANDI_DOI_API_URL, 'DANDI_DOI_API_URL'), + (settings.DANDI_DOI_API_USER, 'DANDI_DOI_API_USER'), + (settings.DANDI_DOI_API_PASSWORD, 'DANDI_DOI_API_PASSWORD'), + (settings.DANDI_DOI_API_PREFIX, 'DANDI_DOI_API_PREFIX'), +] + +logger = logging.getLogger(__name__) + + +class DataCiteClient: + """Client for interacting with the DataCite API.""" + + def __init__(self): + self.api_url = settings.DANDI_DOI_API_URL + self.api_user = settings.DANDI_DOI_API_USER + self.api_password = settings.DANDI_DOI_API_PASSWORD + self.api_prefix = settings.DANDI_DOI_API_PREFIX + self.auth = requests.auth.HTTPBasicAuth(self.api_user, self.api_password) + self.headers = {'Accept': 'application/vnd.api+json'} + self.timeout = 30 + + def is_configured(self) -> bool: + """Check if the DOI client is properly configured.""" + return all(setting is not None for setting, _ in DANDI_DOI_SETTINGS) + + def format_doi(self, dandiset_id: str, version_id: str | None = None) -> str: + """ + Format a DOI string for a dandiset or version. + + Args: + dandiset_id: The dandiset identifier. + version_id: Optional version identifier. If provided, creates a Version DOI. + If omitted, creates a Dandiset DOI. + + Returns: + Formatted DOI string. + """ + if version_id: + # TODO(asmaco) replace "dandi" with non-hardcoded ID_PATTERN + # https://github.com/dandi/dandi-schema/pull/294/files#diff-43c9cc813638d87fd33e527a7baccb2fd7dff85595a7e686bfaf61f0409bd403R47 + return f'{self.api_prefix}/dandi.{dandiset_id}/{version_id}' + return f'{self.api_prefix}/dandi.{dandiset_id}' + + def generate_doi_data( + self, version: Version, version_doi: bool = True, event: str | None = None + ) -> tuple[str, dict]: + """ + Generate DOI data for a version or dandiset. + + Args: + version: Version object containing metadata. + version_doi: If True, generate a Version DOI, otherwise generate a Dandiset DOI. + event: The DOI event type. + - None: Creates a Draft DOI. + - "publish": Creates or promotes to a Findable DOI. + - "hide": Converts to a Registered DOI. + + Returns: + Tuple of (doi_string, datacite_payload) + """ + # TODO(asmacdo) if not datacite configured make sure we dont save any dois to model + from dandischema.datacite import to_datacite + dandiset_id = version.dandiset.identifier + version_id = version.version + metadata = copy.deepcopy(version.metadata) + + # Generate the appropriate DOI string + if version_doi: + doi = self.format_doi(dandiset_id, version_id) + else: + doi = self.format_doi(dandiset_id) + # Dandiset DOI is the same as version url without version + metadata['url'] = metadata['url'].rsplit('/', 1)[0] + + metadata['doi'] = doi + + # Generate the datacite payload with the appropriate event + datacite_payload = to_datacite(metadata, event=event) + + return (doi, datacite_payload) + + def create_or_update_doi(self, original_datacite_payload: dict) -> str | None: + """ + Create or update a DOI with the DataCite API. + + Args: + datacite_payload: The DOI payload to send to DataCite. + + Returns: + The DOI string on success, None on failure when not configured. + + Raises: + requests.exceptions.HTTPError: If the API request fails. + """ + datacite_payload = copy.deepcopy(original_datacite_payload) + doi = datacite_payload['data']['attributes']['doi'] + + if not self.is_configured(): + logger.warning('DOI API not configured. Skipping operations for %s', doi) + return None + + # Check if we're trying to create a non-draft DOI when it's not allowed + event = datacite_payload['data']['attributes'].get('event') + if not settings.DANDI_DOI_PUBLISH and event in ['publish', 'hide']: + # Remove the event to make it a draft DOI + if 'event' in datacite_payload['data']['attributes']: + del datacite_payload['data']['attributes']['event'] + + logger.warning( + 'DANDI_DOI_PUBLISH is not enabled. DOI %s will be created as draft.', doi + ) + + try: + response = requests.post( + self.api_url, + json=datacite_payload, + auth=self.auth, + headers=self.headers, + timeout=self.timeout, + ) + response.raise_for_status() + # Return early on success + return doi + except requests.exceptions.HTTPError as e: + # HTTP 422 status code means DOI already exists + already_exists_code = 422 + if e.response is not None and e.response.status_code == already_exists_code: + # Retry with PUT if DOI already exists + update_url = f'{self.api_url}/{doi}' + try: + update_response = requests.put( + update_url, + json=datacite_payload, + auth=self.auth, + headers=self.headers, + timeout=self.timeout, + ) + update_response.raise_for_status() + # Success with update + return doi + except Exception: + error_details = f'Failed to update existing DOI {doi}' + if e.response and hasattr(e.response, 'text'): + error_details += f'\nResponse: {e.response.text}' + error_details += f'\nPayload: {datacite_payload}' + logger.exception(error_details) + raise + else: + error_details = f'Failed to create DOI {doi}' + if e.response and hasattr(e.response, 'text'): + error_details += f'\nResponse: {e.response.text}' + error_details += f'\nPayload: {datacite_payload}' + logger.exception(error_details) + raise + + def delete_or_hide_doi(self, doi: str) -> None: + """ + Delete a draft DOI or hide a findable DOI depending on its state. + + This method first checks the DOI's state and then either deletes it (if it's a draft) + or hides it (if it's findable). Hiding a DOI requires DANDI_DOI_PUBLISH to be enabled. + + Args: + doi: The DOI to delete or hide. + + Raises: + requests.exceptions.HTTPError: If the API request fails. + """ + if not self.is_configured(): + logger.warning('DOI API not configured. Skipping operations for %s', doi) + return + + doi_url = f'{self.api_url}/{doi}' + + try: + # First, get DOI information to check its state + response = requests.get( + doi_url, auth=self.auth, headers=self.headers, timeout=self.timeout + ) + response.raise_for_status() + + doi_data = response.json() + # Get the state, defaulting to 'draft' if absent + doi_state = doi_data.get('data', {}).get('attributes', {}).get('state', 'draft') + + if doi_state == 'draft': + # Draft DOIs can be deleted + delete_response = requests.delete( + doi_url, auth=self.auth, headers=self.headers, timeout=self.timeout + ) + delete_response.raise_for_status() + logger.info('Successfully deleted draft DOI: %s', doi) + else: + # Findable DOIs must be hidden + # Check if DANDI_DOI_PUBLISH is enabled for hiding + if not settings.DANDI_DOI_PUBLISH: + logger.warning( + 'DANDI_DOI_PUBLISH is not enabled. DOI %s will remain findable.', doi + ) + return + + # Create hide payload + hide_payload = { + 'data': {'id': doi, 'type': 'dois', 'attributes': {'event': 'hide'}} + } + + hide_response = requests.put( + doi_url, + json=hide_payload, + auth=self.auth, + headers=self.headers, + timeout=self.timeout, + ) + hide_response.raise_for_status() + logger.info('Successfully hid findable DOI: %s', doi) + + except requests.exceptions.HTTPError as e: + if e.response and e.response.status_code == requests.codes.not_found: + logger.warning('Tried to get data for nonexistent DOI %s', doi) + return + logger.exception('Failed to delete or hide DOI %s', doi) + raise + + diff --git a/dandiapi/api/doi.py b/dandiapi/api/doi.py index 100fdb725..b43459e93 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -1,86 +1,76 @@ +""" +DOI management interface for the DANDI Archive. + +This module provides the public interface for DOI operations, +while the implementation details are in datacite.py. +""" + from __future__ import annotations import logging from typing import TYPE_CHECKING from django.conf import settings -import requests + +from dandiapi.api.datacite import DataCiteClient if TYPE_CHECKING: from dandiapi.api.models import Version -# All of the required DOI configuration settings -DANDI_DOI_SETTINGS = [ - (settings.DANDI_DOI_API_URL, 'DANDI_DOI_API_URL'), - (settings.DANDI_DOI_API_URL, 'DANDI_DOI_API_USER'), - (settings.DANDI_DOI_API_PASSWORD, 'DANDI_DOI_API_PASSWORD'), - (settings.DANDI_DOI_API_PREFIX, 'DANDI_DOI_API_PREFIX'), -] - logger = logging.getLogger(__name__) -def doi_configured() -> bool: - return any(setting is not None for setting, _ in DANDI_DOI_SETTINGS) - - -def _generate_doi_data(version: Version): - from dandischema.datacite import to_datacite - - publish = settings.DANDI_DOI_PUBLISH - # Use the DANDI test datacite instance as a placeholder if PREFIX isn't set - prefix = settings.DANDI_DOI_API_PREFIX or '10.80507' - dandiset_id = version.dandiset.identifier - version_id = version.version - doi = f'{prefix}/dandi.{dandiset_id}/{version_id}' - metadata = version.metadata - metadata['doi'] = doi - return (doi, to_datacite(metadata, publish=publish)) - - -def create_doi(version: Version) -> str: - doi, request_body = _generate_doi_data(version) - # If DOI isn't configured, skip the API call - if doi_configured(): - try: - requests.post( - settings.DANDI_DOI_API_URL, - json=request_body, - auth=requests.auth.HTTPBasicAuth( - settings.DANDI_DOI_API_USER, - settings.DANDI_DOI_API_PASSWORD, - ), - timeout=30, - ).raise_for_status() - except requests.exceptions.HTTPError as e: - logger.exception('Failed to create DOI %s', doi) - logger.exception(request_body) - if e.response: - logger.exception(e.response.text) - raise - return doi - - -def delete_doi(doi: str) -> None: - # If DOI isn't configured, skip the API call - if doi_configured(): - doi_url = settings.DANDI_DOI_API_URL.rstrip('/') + '/' + doi - with requests.Session() as s: - s.auth = (settings.DANDI_DOI_API_USER, settings.DANDI_DOI_API_PASSWORD) - try: - r = s.get(doi_url, headers={'Accept': 'application/vnd.api+json'}) - r.raise_for_status() - except requests.exceptions.HTTPError as e: - if e.response and e.response.status_code == requests.codes.not_found: - logger.warning('Tried to get data for nonexistent DOI %s', doi) - return - logger.exception('Failed to fetch data for DOI %s', doi) - raise - if r.json()['data']['attributes']['state'] == 'draft': - try: - s.delete(doi_url).raise_for_status() - except requests.exceptions.HTTPError: - logger.exception('Failed to delete DOI %s', doi) - raise - else: - logger.debug('Skipping DOI deletion for %s since not configured', doi) +# Singleton instance +datacite_client = DataCiteClient() + + +def generate_doi_data( + version: Version, version_doi: bool = True, event: str | None = None +) -> tuple[str, dict]: + """ + Generate DOI data for a version or dandiset. + + Args: + version: Version object containing metadata. + version_doi: If True, generate a Version DOI, otherwise generate a Dandiset DOI. + event: The DOI event type. + - None: Creates a Draft DOI. + - "publish": Creates or promotes to a Findable DOI. + - "hide": Converts to a Registered DOI. + + Returns: + Tuple of (doi_string, datacite_payload) + """ + return datacite_client.generate_doi_data(version, version_doi, event) + + +def create_or_update_doi(datacite_payload: dict) -> str | None: + """ + Create or update a DOI with the DataCite API. + + Args: + datacite_payload: The DOI payload to send to DataCite. + + Returns: + The DOI string on success, None on failure when not configured. + + Raises: + requests.exceptions.HTTPError: If the API request fails. + """ + return datacite_client.create_or_update_doi(datacite_payload) + + +def delete_or_hide_doi(doi: str) -> None: + """ + Delete a draft DOI or hide a findable DOI depending on its state. + + This method first checks the DOI's state and then either deletes it (if it's a draft) + or hides it (if it's findable). Hiding a DOI requires DANDI_DOI_PUBLISH to be enabled. + + Args: + doi: The DOI to delete or hide. + + Raises: + requests.exceptions.HTTPError: If the API request fails. + """ + datacite_client.delete_or_hide_doi(doi) diff --git a/dandiapi/api/services/dandiset/__init__.py b/dandiapi/api/services/dandiset/__init__.py index 274b892a3..fda550dc9 100644 --- a/dandiapi/api/services/dandiset/__init__.py +++ b/dandiapi/api/services/dandiset/__init__.py @@ -1,7 +1,10 @@ from __future__ import annotations +import logging + from django.db import transaction +from dandiapi.api import doi from dandiapi.api.models.dandiset import Dandiset, DandisetStar from dandiapi.api.models.version import Version from dandiapi.api.services import audit @@ -15,6 +18,8 @@ from dandiapi.api.services.permissions.dandiset import add_dandiset_owner, is_dandiset_owner from dandiapi.api.services.version.metadata import _normalize_version_metadata +logger = logging.getLogger(__name__) + def create_dandiset( *, @@ -56,9 +61,42 @@ def create_dandiset( dandiset=dandiset, user=user, metadata=draft_version.metadata, embargoed=embargo ) + # For public dandisets, create a Draft DOI + if not embargo: + try: + _create_dandiset_draft_doi(draft_version) + except Exception: + # Log error but allow dandiset creation to proceed + logger.exception('Failed to create Draft DOI for dandiset %s', dandiset.identifier) + return dandiset, draft_version +def _create_dandiset_draft_doi(draft_version: Version) -> None: + """ + Create a Draft DOI for a dandiset. + + This is called during dandiset creation for public dandisets. + For embargoed dandisets, no DOI is created until unembargo. + + Args: + draft_version: The draft version of the dandiset. + """ + # Generate a Draft DOI (event=None) + dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( + draft_version, + version_doi=False, + event=None, # Draft DOI + ) + + # Create the DOI + doi.create_or_update_doi(dandiset_doi_payload) + + # Store the DOI in the draft version + draft_version.doi = dandiset_doi + draft_version.save() + + def delete_dandiset(*, user, dandiset: Dandiset) -> None: if not is_dandiset_owner(dandiset, user): raise NotAllowedError('Cannot delete dandisets which you do not own.') @@ -69,13 +107,22 @@ def delete_dandiset(*, user, dandiset: Dandiset) -> None: if dandiset.unembargo_in_progress: raise DandisetUnembargoInProgressError + # Check if there's a DOI that needs to be handled + draft_version = dandiset.versions.filter(version='draft').first() + # Delete all versions first, so that AssetPath deletion is cascaded # through versions, rather than through zarrs directly with transaction.atomic(): # Record the audit event first so that the AuditRecord instance has a # chance to grab the Dandiset information before it is destroyed. audit.delete_dandiset(dandiset=dandiset, user=user) - + # Dandisets with published versions cannot be deleted + # so only the Dandiset DOI needs to be delete. + if draft_version and draft_version.doi is not None: + try: + doi.delete_or_hide_doi(draft_version.doi) + except Exception: + pass # doi operation should not stop deletion. delete_or_hide will log the exception. dandiset.versions.all().delete() dandiset.delete() @@ -114,3 +161,36 @@ def unstar_dandiset(*, user, dandiset: Dandiset) -> int: DandisetStar.objects.filter(user=user, dandiset=dandiset).delete() return dandiset.star_count + + +def update_draft_version_doi(draft_version: Version) -> None: + """ + Update or create a Draft DOI for a dandiset with the latest metadata. + + This is called when a draft version's metadata is updated for a dandiset + that has never been published. + + Args: + draft_version: The draft version of the dandiset with updated metadata. + """ + # Skip for dandisets that have published versions + if draft_version.dandiset.versions.exclude(version='draft').exists(): + return + + # Generate DOI payload with updated metadata + dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( + draft_version, + version_doi=False, # Generate a Dandiset DOI, not a Version DOI + event=None, # Keep as Draft DOI + ) + + # Create or update the DOI + doi.create_or_update_doi(dandiset_doi_payload) + + # If the version doesn't have a DOI yet, store it + if draft_version.doi is None: + draft_version.doi = dandiset_doi + draft_version.save() + logger.info('Created new Draft DOI %s', dandiset_doi) + else: + logger.info('Updated Draft DOI %s with new metadata', draft_version.doi) diff --git a/dandiapi/api/services/embargo/__init__.py b/dandiapi/api/services/embargo/__init__.py index 494640c60..35fc6e08d 100644 --- a/dandiapi/api/services/embargo/__init__.py +++ b/dandiapi/api/services/embargo/__init__.py @@ -10,6 +10,7 @@ from dandiapi.api.models.asset import Asset from dandiapi.api.services import audit from dandiapi.api.services.asset.exceptions import DandisetOwnerRequiredError +from dandiapi.api.services.dandiset import _create_dandiset_draft_doi from dandiapi.api.services.embargo.utils import remove_dandiset_embargo_tags from dandiapi.api.services.exceptions import DandiError from dandiapi.api.services.metadata import validate_version_metadata @@ -73,6 +74,14 @@ def unembargo_dandiset(ds: Dandiset, user: User): validate_version_metadata(version=v) logger.info('Version metadata validated') + # Create a Draft DOI now that the dandiset is public + try: + _create_dandiset_draft_doi(v) + logger.info('Draft DOI created for unembargoed dandiset') + except Exception: + # Log error but continue with unembargo + logger.exception('Failed to create DOI during unembargo for dandiset %s', ds.identifier) + # Notify owners of completed unembargo send_dandiset_unembargoed_message(ds) logger.info('Dandiset owners notified.') diff --git a/dandiapi/api/services/publish/__init__.py b/dandiapi/api/services/publish/__init__.py index 59042d9a5..abd025126 100644 --- a/dandiapi/api/services/publish/__init__.py +++ b/dandiapi/api/services/publish/__init__.py @@ -106,6 +106,56 @@ def _build_publishable_version_from_draft(draft_version: Version) -> Version: return publishable_version +def _handle_publication_dois(version_id: int) -> None: + """ + Create and update DOIs for a published version. + + Args: + version_id: ID of the published version + """ + version = Version.objects.get(id=version_id) + draft_version = version.dandiset.draft_version + + # Check if this is the first publication (no prior DOI in draft version) + # TODO(asmacdo) not true anymore, we need to check the db. + # if draft_version.dandiset.versions.exclude(version='draft').exists(): + is_first_publication = draft_version.doi is None + + # Create Version DOI as Findable + version_doi, version_doi_payload = doi.generate_doi_data( + version, version_doi=True, event='publish' + ) + + # Either create or update the Dandiset DOI based on whether it's the first publication + if is_first_publication: + # For first publication: generate Dandiset DOI and promote from Draft to Findable + dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( + version, + version_doi=False, + event='publish', # Promote to Findable on first publication + ) + else: + # For subsequent publications: update the metadata but keep as Findable + dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( + version, + version_doi=False, + event='publish', # Update existing Findable DOI + ) + + # Create or update the DOIs + # TODO(asmacdo) we need to try:except here, so dandiset doi doesnt block version doi + doi.create_or_update_doi(dandiset_doi_payload) + doi.create_or_update_doi(version_doi_payload) + + # Store the DOI values + version.doi = version_doi + draft_version.doi = dandiset_doi + + # Save the values + draft_version.save() + version.save() + + def _publish_dandiset(dandiset_id: int, user_id: int) -> None: """ Publish a dandiset. @@ -187,13 +237,8 @@ def _publish_dandiset(dandiset_id: int, user_id: int) -> None: # Write updated manifest files and create DOI after # published version has been committed to DB. transaction.on_commit(lambda: write_manifest_files.delay(new_version.id)) + transaction.on_commit(lambda: _handle_publication_dois.delay(new_version.id)) - def _create_doi(version_id: int): - version = Version.objects.get(id=version_id) - version.doi = doi.create_doi(version) - version.save() - - transaction.on_commit(lambda: _create_doi(new_version.id)) user = User.objects.get(id=user_id) audit.publish_dandiset( diff --git a/dandiapi/api/tasks/__init__.py b/dandiapi/api/tasks/__init__.py index 300e8569e..9365dacd5 100644 --- a/dandiapi/api/tasks/__init__.py +++ b/dandiapi/api/tasks/__init__.py @@ -7,7 +7,7 @@ from celery.utils.log import get_task_logger from django.contrib.auth.models import User -from dandiapi.api.doi import delete_doi +from dandiapi.api.doi import delete_or_hide_doi from dandiapi.api.mail import send_dandiset_unembargo_failed_message from dandiapi.api.manifests import ( write_assets_jsonld, @@ -83,7 +83,7 @@ def validate_version_metadata_task(version_id: int) -> None: @shared_task def delete_doi_task(doi: str) -> None: - delete_doi(doi) + delete_or_hide_doi(doi) @shared_task diff --git a/dandiapi/api/tests/test_asset_paths.py b/dandiapi/api/tests/test_asset_paths.py index 01385d35e..70155fb64 100644 --- a/dandiapi/api/tests/test_asset_paths.py +++ b/dandiapi/api/tests/test_asset_paths.py @@ -304,7 +304,7 @@ def test_asset_path_search_asset_paths(draft_version_factory, asset_factory): @pytest.mark.django_db -def test_asset_path_publish_version(draft_version_factory, asset_factory, user): +def test_asset_path_publish_version(draft_version_factory, asset_factory, user, mocker): version: Version = draft_version_factory() asset = asset_factory(path='foo/bar.txt', status=Asset.Status.VALID) version.assets.add(asset) diff --git a/dandiapi/api/tests/test_audit.py b/dandiapi/api/tests/test_audit.py index 1d52a09de..281f12fcc 100644 --- a/dandiapi/api/tests/test_audit.py +++ b/dandiapi/api/tests/test_audit.py @@ -118,11 +118,12 @@ def user_info(u): @pytest.mark.django_db -def test_audit_update_metadata(api_client, draft_version, user): +def test_audit_update_metadata(api_client, draft_version, user, mocker): # Create a Dandiset. dandiset = draft_version.dandiset add_dandiset_owner(dandiset, user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') # Edit its metadata. metadata = draft_version.metadata metadata['foo'] = 'bar' @@ -144,6 +145,7 @@ def test_audit_update_metadata(api_client, draft_version, user): metadata = rec.details['metadata'] assert metadata['name'] == 'baz' assert metadata['foo'] == 'bar' + mock_update_doi.assert_called_once() @pytest.mark.django_db @@ -275,7 +277,7 @@ def test_audit_remove_asset( @pytest.mark.django_db(transaction=True) def test_audit_publish_dandiset( - api_client, user, dandiset_factory, draft_version_factory, draft_asset_factory + api_client, user, dandiset_factory, draft_version_factory, draft_asset_factory, mocker ): # Create a Dandiset whose draft version has one asset. dandiset = dandiset_factory() @@ -289,6 +291,7 @@ def test_audit_publish_dandiset( # through the API). validate_asset_metadata(asset=draft_asset) validate_version_metadata(version=draft_version) + mock_handle_dois = mocker.patch('dandiapi.api.services.publish._handle_publication_dois') # Publish the Dandiset. api_client.force_authenticate(user=user) @@ -301,6 +304,7 @@ def test_audit_publish_dandiset( rec = get_latest_audit_record(dandiset=dandiset, record_type='publish_dandiset') verify_model_properties(rec, user) assert rec.details['version'] == dandiset.most_recent_published_version.version + mock_handle_dois.delay.assert_called_once() @pytest.mark.django_db diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index f130d6b82..61aaeabe0 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -8,6 +8,7 @@ from dandiapi.api.asset_paths import add_asset_paths, add_version_asset_paths from dandiapi.api.models import Dandiset, Version +from dandiapi.api.services.dandiset import _create_dandiset_draft_doi, update_draft_version_doi from dandiapi.api.services.permissions.dandiset import ( add_dandiset_owner, get_dandiset_owners, @@ -355,7 +356,7 @@ def test_dandiset_rest_embargo_access( @pytest.mark.django_db -def test_dandiset_rest_create(api_client, user): +def test_dandiset_rest_create(api_client, user, mocker): user.first_name = 'John' user.last_name = 'Doe' user.save() @@ -363,9 +364,13 @@ def test_dandiset_rest_create(api_client, user): name = 'Test Dandiset' metadata = {'foo': 'bar'} + mock_create_doi = mocker.patch('dandiapi.api.services.dandiset._create_dandiset_draft_doi') + response = api_client.post( '/api/dandisets/', {'name': name, 'metadata': metadata}, format='json' ) + + mock_create_doi.assert_called_once() assert response.data == { 'identifier': DANDISET_ID_RE, 'created': TIMESTAMP_RE, @@ -631,17 +636,19 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): @pytest.mark.django_db -def test_dandiset_rest_create_embargoed(api_client, user): +def test_dandiset_rest_create_embargoed(api_client, user, mocker): user.first_name = 'John' user.last_name = 'Doe' user.save() api_client.force_authenticate(user=user) name = 'Test Dandiset' metadata = {'foo': 'bar'} + mock_create_doi = mocker.patch('dandiapi.api.services.dandiset._create_dandiset_draft_doi') response = api_client.post( '/api/dandisets/?embargo=true', {'name': name, 'metadata': metadata}, format='json' ) + mock_create_doi.assert_not_called() assert response.data == { 'identifier': DANDISET_ID_RE, 'created': TIMESTAMP_RE, @@ -751,27 +758,41 @@ def test_dandiset_rest_create_with_invalid_identifier(api_client, admin_user): @pytest.mark.django_db @pytest.mark.parametrize( - ('embargo_status', 'success'), + ('embargo_status', 'success', 'doi'), [ - (Dandiset.EmbargoStatus.OPEN, True), - (Dandiset.EmbargoStatus.EMBARGOED, True), - (Dandiset.EmbargoStatus.UNEMBARGOING, False), + (Dandiset.EmbargoStatus.OPEN, True, '10.48324/dandi.000123'), + (Dandiset.EmbargoStatus.OPEN, True, None), + (Dandiset.EmbargoStatus.EMBARGOED, True, '10.48324/dandi.000123'), + (Dandiset.EmbargoStatus.UNEMBARGOING, False, '10.48324/dandi.000123'), ], ) -def test_dandiset_rest_delete(api_client, draft_version_factory, user, embargo_status, success): +def test_dandiset_rest_delete(api_client, draft_version_factory, user, embargo_status, success, doi, mocker): api_client.force_authenticate(user=user) + mock_delete_doi = mocker.patch('dandiapi.api.doi.delete_or_hide_doi') + # Ensure that open or embargoed dandisets can be deleted draft_version = draft_version_factory(dandiset__embargo_status=embargo_status) + # Set a DOI on the draft version + if doi is not None: + draft_version.doi = doi + draft_version.save() + add_dandiset_owner(draft_version.dandiset, user) response = api_client.delete(f'/api/dandisets/{draft_version.dandiset.identifier}/') if success: assert response.status_code == 204 assert not Dandiset.objects.all() + if doi is not None: + mock_delete_doi.assert_called_once_with(draft_version.doi) + else: + mock_delete_doi.assert_not_called() else: assert response.status_code >= 400 assert Dandiset.objects.count() == 1 + # Verify that delete_or_hide_doi was not called + mock_delete_doi.assert_not_called() @pytest.mark.django_db @@ -781,11 +802,15 @@ def test_dandiset_rest_delete_with_zarrs( user, zarr_archive_factory, draft_asset_factory, + mocker, ): api_client.force_authenticate(user=user) add_dandiset_owner(draft_version.dandiset, user) zarr = zarr_archive_factory(dandiset=draft_version.dandiset) asset = draft_asset_factory(blob=None, zarr=zarr) + mock_delete_doi = mocker.patch('dandiapi.api.doi.delete_or_hide_doi') + draft_version.doi = '10.48324/dandi.000123' + draft_version.save() # Add paths add_asset_paths(asset=asset, version=draft_version) @@ -794,39 +819,46 @@ def test_dandiset_rest_delete_with_zarrs( response = api_client.delete(f'/api/dandisets/{draft_version.dandiset.identifier}/') assert response.status_code == 204 assert not Dandiset.objects.all() + mock_delete_doi.assert_called_once() @pytest.mark.django_db -def test_dandiset_rest_delete_not_an_owner(api_client, draft_version, user): +def test_dandiset_rest_delete_not_an_owner(api_client, draft_version, user, mocker): api_client.force_authenticate(user=user) + mock_delete_doi = mocker.patch('dandiapi.api.doi.delete_or_hide_doi') response = api_client.delete(f'/api/dandisets/{draft_version.dandiset.identifier}/') assert response.status_code == 403 assert draft_version.dandiset in Dandiset.objects.all() + mock_delete_doi.assert_not_called() @pytest.mark.django_db -def test_dandiset_rest_delete_published(api_client, published_version, user): +def test_dandiset_rest_delete_published(api_client, published_version, user, mocker): api_client.force_authenticate(user=user) add_dandiset_owner(published_version.dandiset, user) + mock_delete_doi = mocker.patch('dandiapi.api.doi.delete_or_hide_doi') response = api_client.delete(f'/api/dandisets/{published_version.dandiset.identifier}/') assert response.status_code == 403 assert response.data == 'Cannot delete dandisets with published versions.' assert published_version.dandiset in Dandiset.objects.all() + mock_delete_doi.assert_not_called() @pytest.mark.django_db -def test_dandiset_rest_delete_published_admin(api_client, published_version, admin_user): +def test_dandiset_rest_delete_published_admin(api_client, published_version, admin_user, mocker): api_client.force_authenticate(user=admin_user) + mock_delete_doi = mocker.patch('dandiapi.api.doi.delete_or_hide_doi') response = api_client.delete(f'/api/dandisets/{published_version.dandiset.identifier}/') assert response.status_code == 403 assert response.data == 'Cannot delete dandisets with published versions.' assert published_version.dandiset in Dandiset.objects.all() + mock_delete_doi.assert_not_called() @pytest.mark.django_db @@ -1359,3 +1391,94 @@ def test_dandiset_list_order_size(api_client, user, draft_version_factory, asset def test_dandiset_list_starred_unauthenticated(api_client): response = api_client.get('/api/dandisets/', {'starred': True}) assert response.status_code == 401 + + +@pytest.mark.django_db +def test__create_dandiset_draft_doi(draft_version, mocker): + """Test the _create_dandiset_draft_doi function directly.""" + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi.return_value = '10.48324/dandi.000123' + + # Call the function directly + _create_dandiset_draft_doi(draft_version) + + # Verify the mocks were called correctly + mock_generate_doi.assert_called_once_with( + draft_version, + version_doi=False, + event=None # Draft DOI + ) + mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) + + # Verify the DOI was stored in the draft version + assert draft_version.doi == '10.48324/dandi.000123' + + +@pytest.mark.django_db +def test_update_draft_version_doi_no_previous_doi(draft_version, mocker): + """Test updating a draft DOI when none exists yet.""" + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi.return_value = '10.48324/dandi.000123' + + update_draft_version_doi(draft_version) + + # Verify the mocks were called correctly + mock_generate_doi.assert_called_once_with( + draft_version, + version_doi=False, + event=None + ) + mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) + + # Verify the DOI was stored in the draft version + assert draft_version.doi == '10.48324/dandi.000123' + + +@pytest.mark.django_db +def test_update_draft_version_doi_existing_doi(draft_version, mocker): + """Test updating a draft DOI when one already exists.""" + # Set existing DOI + draft_version.doi = '10.48324/dandi.000123' + draft_version.save() + + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi.return_value = '10.48324/dandi.000123' + + update_draft_version_doi(draft_version) + + # Verify the mocks were called correctly + mock_generate_doi.assert_called_once_with( + draft_version, + version_doi=False, + event=None + ) + mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) + + # Verify the DOI is still the same + assert draft_version.doi == '10.48324/dandi.000123' + + +@pytest.mark.django_db +def test_update_draft_version_doi_published_version(draft_version, published_version, mocker): + """Test that update_draft_version_doi is a no-op for dandisets with published versions.""" + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + + update_draft_version_doi(draft_version) + + # Verify no DOI operations were performed + mock_generate_doi.assert_not_called() + mock_create_doi.assert_not_called() diff --git a/dandiapi/api/tests/test_datacite.py b/dandiapi/api/tests/test_datacite.py new file mode 100644 index 000000000..c3fedcd50 --- /dev/null +++ b/dandiapi/api/tests/test_datacite.py @@ -0,0 +1,405 @@ +from __future__ import annotations + +import pytest +import requests +from django.conf import settings +from requests.exceptions import HTTPError + +from dandiapi.api.datacite import DataCiteClient, DANDI_DOI_SETTINGS + +# Mock individual HTTP methods for safety +@pytest.fixture(autouse=True) +def mock_requests(mocker): + """Mock individual request methods for all tests to prevent actual HTTP calls.""" + # Create a mock object to hold all the request methods + mock_obj = mocker.Mock() + + # Patch the individual methods + mock_obj.get = mocker.patch('requests.get') + mock_obj.post = mocker.patch('requests.post') + mock_obj.put = mocker.patch('requests.put') + mock_obj.delete = mocker.patch('requests.delete') + + # Return the mock object with all methods + return mock_obj + +@pytest.fixture +def datacite_client(): + return DataCiteClient() + +def test_is_configured(datacite_client, mocker): + """Test the is_configured method.""" + # Test when all settings are configured + # We need to patch the DANDI_DOI_SETTINGS list directly since it's imported at module level + mocked_settings = [ + ('https://api.test', 'DANDI_DOI_API_URL'), + ('user', 'DANDI_DOI_API_USER'), + ('pass', 'DANDI_DOI_API_PASSWORD'), + ('10.12345', 'DANDI_DOI_API_PREFIX'), + ] + mocker.patch('dandiapi.api.datacite.DANDI_DOI_SETTINGS', mocked_settings) + + assert datacite_client.is_configured() is True + + # Test when one setting is not configured + mocked_settings_with_none = [ + (None, 'DANDI_DOI_API_URL'), + ('user', 'DANDI_DOI_API_USER'), + ('pass', 'DANDI_DOI_API_PASSWORD'), + ('10.12345', 'DANDI_DOI_API_PREFIX'), + ] + mocker.patch('dandiapi.api.datacite.DANDI_DOI_SETTINGS', mocked_settings_with_none) + assert datacite_client.is_configured() is False + + +def test_format_doi(datacite_client, mocker): + """Test formatting DOI strings.""" + mocker.patch.object(datacite_client, 'api_prefix', '10.12345') + + # Test Version DOI format + version_doi = datacite_client.format_doi('000123', '1.2.3') + assert version_doi == '10.12345/dandi.000123/1.2.3' + + # Test Dandiset DOI format + dandiset_doi = datacite_client.format_doi('000123') + assert dandiset_doi == '10.12345/dandi.000123' + + +@pytest.mark.django_db +def test_generate_doi_data(datacite_client, published_version, mocker): + """Test generating DOI data for a version.""" + # Mock to_datacite to avoid actual validation + mock_to_datacite = mocker.patch('dandischema.datacite.to_datacite') + mock_to_datacite.return_value = {'data': {'attributes': {}}} + + # Test Version DOI + doi_string, payload = datacite_client.generate_doi_data(published_version, version_doi=True) + dandiset_id = published_version.dandiset.identifier + version_id = published_version.version + expected_doi = f'{datacite_client.api_prefix}/dandi.{dandiset_id}/{version_id}' + assert doi_string == expected_doi + assert 'doi' in published_version.metadata + # Make sure metadata is copied, not modified + assert id(published_version.metadata) != id(datacite_client.generate_doi_data(published_version)[1]) + + # Test Dandiset DOI + doi_string, payload = datacite_client.generate_doi_data(published_version, version_doi=False) + expected_doi = f'{datacite_client.api_prefix}/dandi.{dandiset_id}' + assert doi_string == expected_doi + + +def test_create_or_update_doi_not_configured(datacite_client, mock_requests, mocker): + """Test create_or_update_doi when API is not configured.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=False) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + payload = {'data': {'attributes': {'doi': '10.12345/test'}}} + result = datacite_client.create_or_update_doi(payload) + + assert result is None + + # Verify no requests methods were called + assert not mock_requests.get.called + assert not mock_requests.post.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + # Check that only the warning was logged + mock_logger.warning.assert_called_once() + + +def test_create_or_update_doi_publish_disabled_event_publish(datacite_client, mock_requests, mocker): + """Test create_or_update_doi when DANDI_DOI_PUBLISH is False.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', False) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Configure mock response + mock_response = mocker.Mock() + mock_response.raise_for_status = mocker.Mock() + mock_requests.post.return_value = mock_response + + # Test with publish event + payload = { + 'data': { + 'attributes': { + 'doi': '10.12345/test', + 'event': 'publish', + } + } + } + datacite_client.create_or_update_doi(payload) + + expected_payload = { + 'data': { + 'attributes': { + 'doi': '10.12345/test', + # event should be removed by the code + } + } + } + mock_requests.post.assert_called_once() + assert mock_requests.post.call_args[1]['json'] == expected_payload + mock_logger.warning.assert_called_once() + # Verify no other requests methods were called + assert not mock_requests.get.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + + +def test_create_or_update_doi_new_doi(datacite_client, mock_requests, mocker): + """Test creating a new DOI successfully.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) + + # Configure mock response + mock_response = mocker.Mock() + mock_response.raise_for_status = mocker.Mock() + mock_requests.post.return_value = mock_response + + payload = {'data': {'attributes': {'doi': '10.12345/test', 'event': 'publish'}}} + result = datacite_client.create_or_update_doi(payload) + + # Verify POST was called with correct params + assert mock_requests.post.called + assert mock_requests.post.call_args[1]['json'] == payload + assert mock_requests.post.call_args[1]['auth'] == datacite_client.auth + assert mock_requests.post.call_args[1]['headers'] == datacite_client.headers + assert mock_requests.post.call_args[1]['timeout'] == datacite_client.timeout + # Verify no other requests methods were called + assert not mock_requests.get.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + assert result == '10.12345/test' + + +def test_create_or_update_doi_existing_doi(datacite_client, mock_requests, mocker): + """Test updating an existing DOI.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) + + # Mock POST to fail with 422 (already exists) + mock_post_response = mocker.Mock() + http_error = HTTPError("DOI already exists") + http_error.response = mocker.Mock() + http_error.response.status_code = 422 + mock_post_response.raise_for_status.side_effect = http_error + mock_requests.post.return_value = mock_post_response + + # Mock PUT to succeed + mock_put_response = mocker.Mock() + mock_put_response.raise_for_status = mocker.Mock() + mock_requests.put.return_value = mock_put_response + + payload = {'data': {'attributes': {'doi': '10.12345/test'}}} + result = datacite_client.create_or_update_doi(payload) + + assert mock_requests.post.called + # Verify PUT was called with correct params + assert mock_requests.put.called + assert mock_requests.put.call_args[1]['json'] == payload + # Verify no other HTTP methods besides post and put were called + assert not mock_requests.get.called + assert not mock_requests.delete.called + assert result == '10.12345/test' + + +def test_create_or_update_doi_post_error(datacite_client, mock_requests, mocker): + """Test error handling when POST fails with non-422 error.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Mock POST to fail with 400 + http_error = HTTPError("Bad request") + http_error.response = mocker.Mock() + http_error.response.status_code = 400 + http_error.response.text = "Bad request" + mock_requests.post.side_effect = http_error + + payload = {'data': {'attributes': {'doi': '10.12345/test'}}} + with pytest.raises(HTTPError): + datacite_client.create_or_update_doi(payload) + + # Verify logger was called + mock_logger.exception.assert_called_once() + # Verify no other HTTP methods were called + assert not mock_requests.get.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + + +def test_create_or_update_doi_put_error(datacite_client, mock_requests, mocker): + """Test error handling when PUT fails.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Mock POST to fail with 422 (already exists) + mock_post_response = mocker.Mock() + http_error = HTTPError("DOI already exists") + http_error.response = mocker.Mock() + http_error.response.status_code = 422 + mock_post_response.raise_for_status.side_effect = http_error + mock_requests.post.return_value = mock_post_response + + # Mock PUT to fail + put_error = HTTPError("Update failed") + put_error.response = mocker.Mock() + put_error.response.text = "Update failed" + mock_requests.put.side_effect = put_error + + payload = {'data': {'attributes': {'doi': '10.12345/test'}}} + with pytest.raises(HTTPError): + datacite_client.create_or_update_doi(payload) + + # Verify both methods were called in the right order + assert mock_requests.post.called + assert mock_requests.put.called + # Verify no other HTTP methods were called + assert not mock_requests.get.called + assert not mock_requests.delete.called + # Verify logger was called + mock_logger.exception.assert_called_once() + + +def test_delete_or_hide_doi_not_configured(datacite_client, mock_requests, mocker): + """Test delete_or_hide_doi when API is not configured.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=False) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + datacite_client.delete_or_hide_doi('10.12345/test') + + # Verify no HTTP methods were called + assert not mock_requests.get.called + assert not mock_requests.post.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + mock_logger.warning.assert_called_once() + + +def test_delete_or_hide_doi_draft(datacite_client, mock_requests, mocker): + """Test deleting a draft DOI.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Mock GET to return a draft DOI + mock_get_response = mocker.Mock() + mock_get_response.json.return_value = { + 'data': {'attributes': {'state': 'draft'}} + } + mock_get_response.raise_for_status = mocker.Mock() + mock_requests.get.return_value = mock_get_response + + # Mock DELETE to succeed + mock_delete_response = mocker.Mock() + mock_delete_response.raise_for_status = mocker.Mock() + mock_requests.delete.return_value = mock_delete_response + + datacite_client.delete_or_hide_doi('10.12345/test') + + # Verify GET and DELETE were called + assert mock_requests.get.called + assert mock_requests.delete.called + assert '10.12345/test' in mock_requests.delete.call_args[0][0] + # Verify no other HTTP methods were called + assert not mock_requests.post.called + assert not mock_requests.put.called + mock_logger.info.assert_called_once() + + +def test_delete_or_hide_doi_findable_publish_enabled(datacite_client, mock_requests, mocker): + """Test hiding a findable DOI when DANDI_DOI_PUBLISH is True.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Mock GET to return a findable DOI + mock_get_response = mocker.Mock() + mock_get_response.json.return_value = { + 'data': {'attributes': {'state': 'findable'}} + } + mock_get_response.raise_for_status = mocker.Mock() + mock_requests.get.return_value = mock_get_response + + # Mock PUT to succeed + mock_put_response = mocker.Mock() + mock_put_response.raise_for_status = mocker.Mock() + mock_requests.put.return_value = mock_put_response + + datacite_client.delete_or_hide_doi('10.12345/test') + + # Verify GET and PUT were called, but not other methods + assert mock_requests.get.called + assert mock_requests.put.called + assert not mock_requests.post.called + assert not mock_requests.delete.called + + # Verify correct parameters + assert '10.12345/test' in mock_requests.put.call_args[0][0] + assert mock_requests.put.call_args[1]['json']['data']['attributes']['event'] == 'hide' + mock_logger.info.assert_called_once() + + +def test_delete_or_hide_doi_findable_publish_disabled(datacite_client, mock_requests, mocker): + """Test not hiding a findable DOI when DANDI_DOI_PUBLISH is False.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', False) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Mock GET to return a findable DOI + mock_get_response = mocker.Mock() + mock_get_response.json.return_value = { + 'data': {'attributes': {'state': 'findable'}} + } + mock_get_response.raise_for_status = mocker.Mock() + mock_requests.get.return_value = mock_get_response + + datacite_client.delete_or_hide_doi('10.12345/test') + + # Verify only GET was called, but no other methods + assert mock_requests.get.called + assert not mock_requests.post.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + mock_logger.warning.assert_called_once() + + +def test_delete_or_hide_doi_nonexistent(datacite_client, mock_requests, mocker): + """Test handling a nonexistent DOI.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Mock GET to fail with 404 + get_error = HTTPError("Not found") + get_error.response = mocker.Mock() + get_error.response.status_code = 404 + mock_requests.get.side_effect = get_error + + datacite_client.delete_or_hide_doi('10.12345/test') + + # Verify only GET was attempted, but no other methods + assert mock_requests.get.called + assert not mock_requests.post.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + mock_logger.warning.assert_called_once() + + +def test_delete_or_hide_doi_get_error(datacite_client, mock_requests, mocker): + """Test error handling when GET fails with non-404 error.""" + mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + # Mock GET to fail with 500 + get_error = HTTPError("Server error") + get_error.response = mocker.Mock() + get_error.response.status_code = 500 + mock_requests.get.side_effect = get_error + + with pytest.raises(HTTPError): + datacite_client.delete_or_hide_doi('10.12345/test') + + # Verify only GET was attempted, but no other methods + assert mock_requests.get.called + assert not mock_requests.post.called + assert not mock_requests.put.called + assert not mock_requests.delete.called + mock_logger.exception.assert_called_once() diff --git a/dandiapi/api/tests/test_tasks.py b/dandiapi/api/tests/test_tasks.py index 6e818598b..3713a2ad9 100644 --- a/dandiapi/api/tests/test_tasks.py +++ b/dandiapi/api/tests/test_tasks.py @@ -355,7 +355,9 @@ def test_publish_task( published_asset_factory, draft_version_factory, django_capture_on_commit_callbacks, + mocker, ): + mock_handle_dois = mocker.patch('dandiapi.api.services.publish._handle_publication_dois') # Create a draft_version in PUBLISHING state draft_version: Version = draft_version_factory(status=Version.Status.PUBLISHING) @@ -411,7 +413,7 @@ def test_publish_task( f'/{published_version.version}' ), 'citation': published_version.citation(published_version.metadata), - 'doi': f'10.80507/dandi.{draft_version.dandiset.identifier}/{published_version.version}', + # We have mocked the doi generation, so we will not have a DOI on the model. # Once the assets are linked, assetsSummary should be computed properly 'assetsSummary': { 'schemaKey': 'AssetsSummary', @@ -468,3 +470,4 @@ def test_publish_task( assert new_published_asset.path == old_published_asset.path assert new_published_asset.blob == old_published_asset.blob assert new_published_asset.metadata == old_published_asset.metadata + mock_handle_dois.delay.assert_called_once() diff --git a/dandiapi/api/tests/test_unembargo.py b/dandiapi/api/tests/test_unembargo.py index 3408887b6..8c18ac3ca 100644 --- a/dandiapi/api/tests/test_unembargo.py +++ b/dandiapi/api/tests/test_unembargo.py @@ -335,6 +335,7 @@ def test_unembargo_dandiset_validate_version_metadata( draft_version.save() draft_version.assets.add(asset_factory()) + mock_create_doi = mocker.patch('dandiapi.api.services.embargo._create_dandiset_draft_doi') # Spy on the imported function in the embargo service validate_version_spy = mocker.spy(embargo_service, 'validate_version_metadata') @@ -343,6 +344,7 @@ def test_unembargo_dandiset_validate_version_metadata( assert validate_version_spy.call_count == 1 draft_version.refresh_from_db() assert not draft_version.validation_errors + mock_create_doi.assert_called_once() @pytest.mark.django_db diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index 66b508012..02891b59b 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -534,9 +534,10 @@ def test_version_rest_info_with_asset( @pytest.mark.django_db -def test_version_rest_update(api_client, user, draft_version): +def test_version_rest_update(api_client, user, draft_version, mocker): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_name = 'A unique and special name!' new_metadata = { @@ -620,10 +621,60 @@ def test_version_rest_update(api_client, user, draft_version): assert draft_version.metadata == saved_metadata assert draft_version.name == new_name assert draft_version.status == Version.Status.PENDING + mock_update_doi.assert_called_once_with(draft_version) @pytest.mark.django_db -def test_version_rest_update_unembargo_in_progress(api_client, user, draft_version_factory): +def test_version_rest_update_embargoed(api_client, user, draft_version_factory, mocker): + draft_version = draft_version_factory( + dandiset__embargo_status=Dandiset.EmbargoStatus.EMBARGOED + ) + add_dandiset_owner(draft_version.dandiset, user) + api_client.force_authenticate(user=user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') + + new_name = 'Embargoed dandiset name' + new_metadata = { + 'schemaVersion': settings.DANDI_SCHEMA_VERSION, + 'name': new_name, + 'description': 'Test description', + } + + resp = api_client.put( + f'/api/dandisets/{draft_version.dandiset.identifier}/versions/{draft_version.version}/', + {'metadata': new_metadata, 'name': new_name}, + format='json', + ) + assert resp.status_code == 200 + mock_update_doi.assert_not_called() + + +@pytest.mark.django_db +def test_version_rest_update_doi_error(api_client, user, draft_version, mocker): + add_dandiset_owner(draft_version.dandiset, user) + api_client.force_authenticate(user=user) + + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') + mock_update_doi.side_effect = ValueError('DOI error') + mock_logger = mocker.patch('dandiapi.api.views.version.logger') + + new_name = 'Test error handling' + new_metadata = {'schemaVersion': settings.DANDI_SCHEMA_VERSION, 'description': 'Test'} + + # Update should succeed even if DOI update fails + resp = api_client.put( + f'/api/dandisets/{draft_version.dandiset.identifier}/versions/{draft_version.version}/', + {'metadata': new_metadata, 'name': new_name}, + format='json', + ) + assert resp.status_code == 200 + mock_update_doi.assert_called_once() + mock_logger.exception.assert_called_once() + + +@pytest.mark.django_db +def test_version_rest_update_unembargo_in_progress(api_client, user, draft_version_factory, mocker): + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') draft_version = draft_version_factory( dandiset__embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING ) @@ -646,12 +697,14 @@ def test_version_rest_update_unembargo_in_progress(api_client, user, draft_versi format='json', ) assert resp.status_code == 400 + mock_update_doi.assert_not_called() @pytest.mark.django_db -def test_version_rest_update_published_version(api_client, user, published_version): +def test_version_rest_update_published_version(api_client, user, published_version, mocker): add_dandiset_owner(published_version.dandiset, user) api_client.force_authenticate(user=user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_name = 'A unique and special name!' new_metadata = {'foo': 'bar', 'num': 123, 'list': ['a', 'b', 'c']} @@ -664,11 +717,13 @@ def test_version_rest_update_published_version(api_client, user, published_versi ) assert resp.status_code == 405 assert resp.data == 'Only draft versions can be modified.' + mock_update_doi.assert_not_called() @pytest.mark.django_db -def test_version_rest_update_not_an_owner(api_client, user, version): +def test_version_rest_update_not_an_owner(api_client, user, version, mocker): api_client.force_authenticate(user=user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_name = 'A unique and special name!' new_metadata = {'foo': 'bar', 'num': 123, 'list': ['a', 'b', 'c']} @@ -681,24 +736,26 @@ def test_version_rest_update_not_an_owner(api_client, user, version): ).status_code == 403 ) + mock_update_doi.assert_not_called() @pytest.mark.parametrize( - ('access'), + ('access', 'triggers_update'), [ - 'some value', - 123, - None, - [], - ['a', 'b'], - ['a', 'b', {}], - [{'schemaKey': 'AccessRequirements', 'status': 'foobar'}], + ('some value', True), + (123, True), + (None, True), + ([], True), + (['a', 'b'], True), + (['a', 'b', {}], True), + ([{'schemaKey': 'AccessRequirements', 'status': 'foobar'}], False) ], ) @pytest.mark.django_db -def test_version_rest_update_access_values(api_client, user, draft_version, access): +def test_version_rest_update_access_values(api_client, user, draft_version, access, triggers_update, mocker): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_metadata = {**draft_version.metadata, 'access': access} resp = api_client.put( @@ -706,7 +763,6 @@ def test_version_rest_update_access_values(api_client, user, draft_version, acce {'metadata': new_metadata, 'name': draft_version.name}, format='json', ) - assert resp.status_code == 200 draft_version.refresh_from_db() access = draft_version.metadata['access'] @@ -717,12 +773,15 @@ def test_version_rest_update_access_values(api_client, user, draft_version, acce if draft_version.dandiset.embargoed else AccessType.OpenAccess.value ) + if triggers_update: + mock_update_doi.assert_called_once() @pytest.mark.django_db -def test_version_rest_update_access_missing(api_client, user, draft_version): +def test_version_rest_update_access_missing(api_client, user, draft_version, mocker): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') # Check that the field missing entirely is also okay new_metadata = {**draft_version.metadata} @@ -743,12 +802,14 @@ def test_version_rest_update_access_missing(api_client, user, draft_version): if draft_version.dandiset.embargoed else AccessType.OpenAccess.value ) + mock_update_doi.assert_called_once() @pytest.mark.django_db -def test_version_rest_update_access_valid(api_client, user, draft_version): +def test_version_rest_update_access_valid(api_client, user, draft_version, mocker): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) + mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') # Check that extra fields persist new_metadata = {**draft_version.metadata, 'access': [{'extra': 'field'}]} @@ -769,6 +830,7 @@ def test_version_rest_update_access_valid(api_client, user, draft_version): ) assert access[0]['extra'] == 'field' + mock_update_doi.assert_called_once() @pytest.mark.django_db diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index bcd1f9d4d..9b2ab8ca1 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -1,4 +1,5 @@ from __future__ import annotations +import logging from django.db import transaction from django_filters import rest_framework as filters @@ -19,6 +20,7 @@ require_dandiset_owner_or_403, ) from dandiapi.api.services.publish import publish_dandiset +from dandiapi.api.services.dandiset import update_draft_version_doi from dandiapi.api.tasks import delete_doi_task from dandiapi.api.views.common import DANDISET_PK_PARAM, VERSION_PARAM from dandiapi.api.views.pagination import DandiPagination @@ -29,6 +31,9 @@ ) +logger = logging.getLogger(__name__) + + class VersionFilter(filters.FilterSet): order = filters.OrderingFilter(fields=['created']) @@ -131,6 +136,17 @@ def update(self, request, **kwargs): metadata=locked_version.metadata, ) + # For unpublished dandisets, update or create the draft DOI + # to keep it in sync with the latest metadata + if not locked_version.dandiset.embargoed: + try: + update_draft_version_doi(locked_version) + except ValueError: + logger.exception('Failed to update Draft DOI for dandiset %s', locked_version.dandiset.identifier) + else: + logger.debug("Skipping DOI update for embargoed Dandiset %s.", + locked_version.dandiset.identifier) + serializer = VersionDetailSerializer(instance=locked_version) return Response(serializer.data, status=status.HTTP_200_OK) From 101471aca35287416b394e3dc918a1832d3e0e77 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:45 -0400 Subject: [PATCH 02/17] Move DOI handling for publish to celery task --- dandiapi/api/doi.py | 52 ++++++++++++++++++++++ dandiapi/api/services/publish/__init__.py | 53 +---------------------- dandiapi/api/tasks/__init__.py | 7 +++ dandiapi/api/tests/test_audit.py | 2 +- dandiapi/api/tests/test_tasks.py | 2 +- 5 files changed, 63 insertions(+), 53 deletions(-) diff --git a/dandiapi/api/doi.py b/dandiapi/api/doi.py index b43459e93..26b8cf9b7 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -74,3 +74,55 @@ def delete_or_hide_doi(doi: str) -> None: requests.exceptions.HTTPError: If the API request fails. """ datacite_client.delete_or_hide_doi(doi) + + +def _handle_publication_dois(version_id: int) -> None: + """ + Create and update DOIs for a published version. + + Args: + version_id: ID of the published version + """ + from dandiapi.api.models import Version + + version = Version.objects.get(id=version_id) + draft_version = version.dandiset.draft_version + + # Check if this is the first publication (no prior DOI in draft version) + # TODO(asmacdo) not true anymore, we need to check the db. + # if draft_version.dandiset.versions.exclude(version='draft').exists(): + is_first_publication = draft_version.doi is None + + # Create Version DOI as Findable + version_doi, version_doi_payload = generate_doi_data( + version, version_doi=True, event='publish' + ) + + # Either create or update the Dandiset DOI based on whether it's the first publication + if is_first_publication: + # For first publication: generate Dandiset DOI and promote from Draft to Findable + dandiset_doi, dandiset_doi_payload = generate_doi_data( + version, + version_doi=False, + event='publish', # Promote to Findable on first publication + ) + else: + # For subsequent publications: update the metadata but keep as Findable + dandiset_doi, dandiset_doi_payload = generate_doi_data( + version, + version_doi=False, + event='publish', # Update existing Findable DOI + ) + + # Create or update the DOIs + # TODO(asmacdo) we need to try:except here, so dandiset doi doesnt block version doi + create_or_update_doi(dandiset_doi_payload) + create_or_update_doi(version_doi_payload) + + # Store the DOI values + version.doi = version_doi + draft_version.doi = dandiset_doi + + # Save the values + draft_version.save() + version.save() diff --git a/dandiapi/api/services/publish/__init__.py b/dandiapi/api/services/publish/__init__.py index abd025126..66fa76eaf 100644 --- a/dandiapi/api/services/publish/__init__.py +++ b/dandiapi/api/services/publish/__init__.py @@ -22,7 +22,7 @@ DandisetNotLockedError, DandisetValidationPendingError, ) -from dandiapi.api.tasks import write_manifest_files +from dandiapi.api.tasks import handle_publication_dois_task, write_manifest_files if TYPE_CHECKING: from django.db.models import QuerySet @@ -106,54 +106,6 @@ def _build_publishable_version_from_draft(draft_version: Version) -> Version: return publishable_version -def _handle_publication_dois(version_id: int) -> None: - """ - Create and update DOIs for a published version. - - Args: - version_id: ID of the published version - """ - version = Version.objects.get(id=version_id) - draft_version = version.dandiset.draft_version - - # Check if this is the first publication (no prior DOI in draft version) - # TODO(asmacdo) not true anymore, we need to check the db. - # if draft_version.dandiset.versions.exclude(version='draft').exists(): - is_first_publication = draft_version.doi is None - - # Create Version DOI as Findable - version_doi, version_doi_payload = doi.generate_doi_data( - version, version_doi=True, event='publish' - ) - - # Either create or update the Dandiset DOI based on whether it's the first publication - if is_first_publication: - # For first publication: generate Dandiset DOI and promote from Draft to Findable - dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( - version, - version_doi=False, - event='publish', # Promote to Findable on first publication - ) - else: - # For subsequent publications: update the metadata but keep as Findable - dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( - version, - version_doi=False, - event='publish', # Update existing Findable DOI - ) - - # Create or update the DOIs - # TODO(asmacdo) we need to try:except here, so dandiset doi doesnt block version doi - doi.create_or_update_doi(dandiset_doi_payload) - doi.create_or_update_doi(version_doi_payload) - - # Store the DOI values - version.doi = version_doi - draft_version.doi = dandiset_doi - - # Save the values - draft_version.save() - version.save() def _publish_dandiset(dandiset_id: int, user_id: int) -> None: @@ -237,8 +189,7 @@ def _publish_dandiset(dandiset_id: int, user_id: int) -> None: # Write updated manifest files and create DOI after # published version has been committed to DB. transaction.on_commit(lambda: write_manifest_files.delay(new_version.id)) - transaction.on_commit(lambda: _handle_publication_dois.delay(new_version.id)) - + transaction.on_commit(lambda: handle_publication_dois_task.delay(new_version.id)) user = User.objects.get(id=user_id) audit.publish_dandiset( diff --git a/dandiapi/api/tasks/__init__.py b/dandiapi/api/tasks/__init__.py index 9365dacd5..99992e37d 100644 --- a/dandiapi/api/tasks/__init__.py +++ b/dandiapi/api/tasks/__init__.py @@ -106,3 +106,10 @@ def unembargo_dandiset_task(dandiset_id: int, user_id: int): except Exception: send_dandiset_unembargo_failed_message(ds) raise + + +@shared_task(soft_time_limit=60) +def handle_publication_dois_task(version_id: int) -> None: + from dandiapi.api.doi import _handle_publication_dois + + _handle_publication_dois(version_id) diff --git a/dandiapi/api/tests/test_audit.py b/dandiapi/api/tests/test_audit.py index 281f12fcc..0af71a469 100644 --- a/dandiapi/api/tests/test_audit.py +++ b/dandiapi/api/tests/test_audit.py @@ -291,7 +291,7 @@ def test_audit_publish_dandiset( # through the API). validate_asset_metadata(asset=draft_asset) validate_version_metadata(version=draft_version) - mock_handle_dois = mocker.patch('dandiapi.api.services.publish._handle_publication_dois') + mock_handle_dois = mocker.patch('dandiapi.api.services.publish.handle_publication_dois_task') # Publish the Dandiset. api_client.force_authenticate(user=user) diff --git a/dandiapi/api/tests/test_tasks.py b/dandiapi/api/tests/test_tasks.py index 3713a2ad9..b566e5b8d 100644 --- a/dandiapi/api/tests/test_tasks.py +++ b/dandiapi/api/tests/test_tasks.py @@ -357,7 +357,7 @@ def test_publish_task( django_capture_on_commit_callbacks, mocker, ): - mock_handle_dois = mocker.patch('dandiapi.api.services.publish._handle_publication_dois') + mock_handle_dois = mocker.patch('dandiapi.api.services.publish.handle_publication_dois_task') # Create a draft_version in PUBLISHING state draft_version: Version = draft_version_factory(status=Version.Status.PUBLISHING) From 9e5879d89d959550a86dae43110c0654958a0a8b Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 03/17] Add safety: disable calls to Datacite during tests The datacite_client_unsafe bypasses the safety check, but can still be used safely when mocking requests. --- dandiapi/api/datacite.py | 17 ++++++ dandiapi/api/tests/test_datacite.py | 94 +++++++++++++++++------------ 2 files changed, 72 insertions(+), 39 deletions(-) diff --git a/dandiapi/api/datacite.py b/dandiapi/api/datacite.py index 70dca4bfd..61e68c16f 100644 --- a/dandiapi/api/datacite.py +++ b/dandiapi/api/datacite.py @@ -8,7 +8,9 @@ from __future__ import annotations import copy +from functools import wraps import logging +import sys from typing import TYPE_CHECKING from django.conf import settings @@ -29,6 +31,19 @@ logger = logging.getLogger(__name__) + +def block_during_test(fn): + """ + Datacite API should not be called + """ + @wraps(fn) + def wrapper(*args, **kwargs): + if "pytest" in sys.modules: + raise RuntimeError(f"DOI calls to {fn.__name__} blocked during test.") + return fn(*args, **kwargs) + return wrapper + + class DataCiteClient: """Client for interacting with the DataCite API.""" @@ -101,6 +116,7 @@ def generate_doi_data( return (doi, datacite_payload) + @block_during_test def create_or_update_doi(self, original_datacite_payload: dict) -> str | None: """ Create or update a DOI with the DataCite API. @@ -175,6 +191,7 @@ def create_or_update_doi(self, original_datacite_payload: dict) -> str | None: logger.exception(error_details) raise + @block_during_test def delete_or_hide_doi(self, doi: str) -> None: """ Delete a draft DOI or hide a findable DOI depending on its state. diff --git a/dandiapi/api/tests/test_datacite.py b/dandiapi/api/tests/test_datacite.py index c3fedcd50..994efaa96 100644 --- a/dandiapi/api/tests/test_datacite.py +++ b/dandiapi/api/tests/test_datacite.py @@ -23,10 +23,26 @@ def mock_requests(mocker): # Return the mock object with all methods return mock_obj + @pytest.fixture def datacite_client(): return DataCiteClient() + +@pytest.fixture +def datacite_client_unsafe(): + """ + Bypass safety feature that prevents API calls to datacite during tests. + """ + client = DataCiteClient() + if hasattr(client.create_or_update_doi, '__wrapped__'): + client.create_or_update_doi = client.create_or_update_doi.__wrapped__.__get__(client) + if hasattr(client.delete_or_hide_doi, '__wrapped__'): + client.delete_or_hide_doi = client.delete_or_hide_doi.__wrapped__.__get__(client) + + return client + + def test_is_configured(datacite_client, mocker): """Test the is_configured method.""" # Test when all settings are configured @@ -88,13 +104,13 @@ def test_generate_doi_data(datacite_client, published_version, mocker): assert doi_string == expected_doi -def test_create_or_update_doi_not_configured(datacite_client, mock_requests, mocker): +def test_create_or_update_doi_not_configured(datacite_client_unsafe, mock_requests, mocker): """Test create_or_update_doi when API is not configured.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=False) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=False) mock_logger = mocker.patch('dandiapi.api.datacite.logger') payload = {'data': {'attributes': {'doi': '10.12345/test'}}} - result = datacite_client.create_or_update_doi(payload) + result = datacite_client_unsafe.create_or_update_doi(payload) assert result is None @@ -107,9 +123,9 @@ def test_create_or_update_doi_not_configured(datacite_client, mock_requests, moc mock_logger.warning.assert_called_once() -def test_create_or_update_doi_publish_disabled_event_publish(datacite_client, mock_requests, mocker): +def test_create_or_update_doi_publish_disabled_event_publish(datacite_client_unsafe, mock_requests, mocker): """Test create_or_update_doi when DANDI_DOI_PUBLISH is False.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', False) mock_logger = mocker.patch('dandiapi.api.datacite.logger') @@ -127,7 +143,7 @@ def test_create_or_update_doi_publish_disabled_event_publish(datacite_client, mo } } } - datacite_client.create_or_update_doi(payload) + datacite_client_unsafe.create_or_update_doi(payload) expected_payload = { 'data': { @@ -146,9 +162,9 @@ def test_create_or_update_doi_publish_disabled_event_publish(datacite_client, mo assert not mock_requests.delete.called -def test_create_or_update_doi_new_doi(datacite_client, mock_requests, mocker): +def test_create_or_update_doi_new_doi(datacite_client_unsafe, mock_requests, mocker): """Test creating a new DOI successfully.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) # Configure mock response @@ -157,14 +173,14 @@ def test_create_or_update_doi_new_doi(datacite_client, mock_requests, mocker): mock_requests.post.return_value = mock_response payload = {'data': {'attributes': {'doi': '10.12345/test', 'event': 'publish'}}} - result = datacite_client.create_or_update_doi(payload) + result = datacite_client_unsafe.create_or_update_doi(payload) # Verify POST was called with correct params assert mock_requests.post.called assert mock_requests.post.call_args[1]['json'] == payload - assert mock_requests.post.call_args[1]['auth'] == datacite_client.auth - assert mock_requests.post.call_args[1]['headers'] == datacite_client.headers - assert mock_requests.post.call_args[1]['timeout'] == datacite_client.timeout + assert mock_requests.post.call_args[1]['auth'] == datacite_client_unsafe.auth + assert mock_requests.post.call_args[1]['headers'] == datacite_client_unsafe.headers + assert mock_requests.post.call_args[1]['timeout'] == datacite_client_unsafe.timeout # Verify no other requests methods were called assert not mock_requests.get.called assert not mock_requests.put.called @@ -172,9 +188,9 @@ def test_create_or_update_doi_new_doi(datacite_client, mock_requests, mocker): assert result == '10.12345/test' -def test_create_or_update_doi_existing_doi(datacite_client, mock_requests, mocker): +def test_create_or_update_doi_existing_doi(datacite_client_unsafe, mock_requests, mocker): """Test updating an existing DOI.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) # Mock POST to fail with 422 (already exists) @@ -191,7 +207,7 @@ def test_create_or_update_doi_existing_doi(datacite_client, mock_requests, mocke mock_requests.put.return_value = mock_put_response payload = {'data': {'attributes': {'doi': '10.12345/test'}}} - result = datacite_client.create_or_update_doi(payload) + result = datacite_client_unsafe.create_or_update_doi(payload) assert mock_requests.post.called # Verify PUT was called with correct params @@ -203,9 +219,9 @@ def test_create_or_update_doi_existing_doi(datacite_client, mock_requests, mocke assert result == '10.12345/test' -def test_create_or_update_doi_post_error(datacite_client, mock_requests, mocker): +def test_create_or_update_doi_post_error(datacite_client_unsafe, mock_requests, mocker): """Test error handling when POST fails with non-422 error.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock POST to fail with 400 @@ -217,7 +233,7 @@ def test_create_or_update_doi_post_error(datacite_client, mock_requests, mocker) payload = {'data': {'attributes': {'doi': '10.12345/test'}}} with pytest.raises(HTTPError): - datacite_client.create_or_update_doi(payload) + datacite_client_unsafe.create_or_update_doi(payload) # Verify logger was called mock_logger.exception.assert_called_once() @@ -227,9 +243,9 @@ def test_create_or_update_doi_post_error(datacite_client, mock_requests, mocker) assert not mock_requests.delete.called -def test_create_or_update_doi_put_error(datacite_client, mock_requests, mocker): +def test_create_or_update_doi_put_error(datacite_client_unsafe, mock_requests, mocker): """Test error handling when PUT fails.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock POST to fail with 422 (already exists) @@ -248,7 +264,7 @@ def test_create_or_update_doi_put_error(datacite_client, mock_requests, mocker): payload = {'data': {'attributes': {'doi': '10.12345/test'}}} with pytest.raises(HTTPError): - datacite_client.create_or_update_doi(payload) + datacite_client_unsafe.create_or_update_doi(payload) # Verify both methods were called in the right order assert mock_requests.post.called @@ -260,12 +276,12 @@ def test_create_or_update_doi_put_error(datacite_client, mock_requests, mocker): mock_logger.exception.assert_called_once() -def test_delete_or_hide_doi_not_configured(datacite_client, mock_requests, mocker): +def test_delete_or_hide_doi_not_configured(datacite_client_unsafe, mock_requests, mocker): """Test delete_or_hide_doi when API is not configured.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=False) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=False) mock_logger = mocker.patch('dandiapi.api.datacite.logger') - datacite_client.delete_or_hide_doi('10.12345/test') + datacite_client_unsafe.delete_or_hide_doi('10.12345/test') # Verify no HTTP methods were called assert not mock_requests.get.called @@ -275,9 +291,9 @@ def test_delete_or_hide_doi_not_configured(datacite_client, mock_requests, mocke mock_logger.warning.assert_called_once() -def test_delete_or_hide_doi_draft(datacite_client, mock_requests, mocker): +def test_delete_or_hide_doi_draft(datacite_client_unsafe, mock_requests, mocker): """Test deleting a draft DOI.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock GET to return a draft DOI @@ -293,7 +309,7 @@ def test_delete_or_hide_doi_draft(datacite_client, mock_requests, mocker): mock_delete_response.raise_for_status = mocker.Mock() mock_requests.delete.return_value = mock_delete_response - datacite_client.delete_or_hide_doi('10.12345/test') + datacite_client_unsafe.delete_or_hide_doi('10.12345/test') # Verify GET and DELETE were called assert mock_requests.get.called @@ -305,9 +321,9 @@ def test_delete_or_hide_doi_draft(datacite_client, mock_requests, mocker): mock_logger.info.assert_called_once() -def test_delete_or_hide_doi_findable_publish_enabled(datacite_client, mock_requests, mocker): +def test_delete_or_hide_doi_findable_publish_enabled(datacite_client_unsafe, mock_requests, mocker): """Test hiding a findable DOI when DANDI_DOI_PUBLISH is True.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) mock_logger = mocker.patch('dandiapi.api.datacite.logger') @@ -324,7 +340,7 @@ def test_delete_or_hide_doi_findable_publish_enabled(datacite_client, mock_reque mock_put_response.raise_for_status = mocker.Mock() mock_requests.put.return_value = mock_put_response - datacite_client.delete_or_hide_doi('10.12345/test') + datacite_client_unsafe.delete_or_hide_doi('10.12345/test') # Verify GET and PUT were called, but not other methods assert mock_requests.get.called @@ -338,9 +354,9 @@ def test_delete_or_hide_doi_findable_publish_enabled(datacite_client, mock_reque mock_logger.info.assert_called_once() -def test_delete_or_hide_doi_findable_publish_disabled(datacite_client, mock_requests, mocker): +def test_delete_or_hide_doi_findable_publish_disabled(datacite_client_unsafe, mock_requests, mocker): """Test not hiding a findable DOI when DANDI_DOI_PUBLISH is False.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', False) mock_logger = mocker.patch('dandiapi.api.datacite.logger') @@ -352,7 +368,7 @@ def test_delete_or_hide_doi_findable_publish_disabled(datacite_client, mock_requ mock_get_response.raise_for_status = mocker.Mock() mock_requests.get.return_value = mock_get_response - datacite_client.delete_or_hide_doi('10.12345/test') + datacite_client_unsafe.delete_or_hide_doi('10.12345/test') # Verify only GET was called, but no other methods assert mock_requests.get.called @@ -362,9 +378,9 @@ def test_delete_or_hide_doi_findable_publish_disabled(datacite_client, mock_requ mock_logger.warning.assert_called_once() -def test_delete_or_hide_doi_nonexistent(datacite_client, mock_requests, mocker): +def test_delete_or_hide_doi_nonexistent(datacite_client_unsafe, mock_requests, mocker): """Test handling a nonexistent DOI.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock GET to fail with 404 @@ -373,7 +389,7 @@ def test_delete_or_hide_doi_nonexistent(datacite_client, mock_requests, mocker): get_error.response.status_code = 404 mock_requests.get.side_effect = get_error - datacite_client.delete_or_hide_doi('10.12345/test') + datacite_client_unsafe.delete_or_hide_doi('10.12345/test') # Verify only GET was attempted, but no other methods assert mock_requests.get.called @@ -383,9 +399,9 @@ def test_delete_or_hide_doi_nonexistent(datacite_client, mock_requests, mocker): mock_logger.warning.assert_called_once() -def test_delete_or_hide_doi_get_error(datacite_client, mock_requests, mocker): +def test_delete_or_hide_doi_get_error(datacite_client_unsafe, mock_requests, mocker): """Test error handling when GET fails with non-404 error.""" - mocker.patch.object(datacite_client, 'is_configured', return_value=True) + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock GET to fail with 500 @@ -395,7 +411,7 @@ def test_delete_or_hide_doi_get_error(datacite_client, mock_requests, mocker): mock_requests.get.side_effect = get_error with pytest.raises(HTTPError): - datacite_client.delete_or_hide_doi('10.12345/test') + datacite_client_unsafe.delete_or_hide_doi('10.12345/test') # Verify only GET was attempted, but no other methods assert mock_requests.get.called From 95931ba2b705928c55cf7cf93d2b28160733123d Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 04/17] fixup: safety feature caught one test using datacite, should mock instead --- dandiapi/api/tests/test_version.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index 02891b59b..ef6f9d842 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -1038,14 +1038,16 @@ def test_version_rest_delete_published_not_admin(api_client, user, published_ver @pytest.mark.django_db -def test_version_rest_delete_published_admin(api_client, admin_user, published_version): +def test_version_rest_delete_published_admin(api_client, admin_user, published_version, mocker): api_client.force_authenticate(user=admin_user) + mock_delete_doi = mocker.patch('dandiapi.api.doi.datacite_client.delete_or_hide_doi') response = api_client.delete( f'/api/dandisets/{published_version.dandiset.identifier}' f'/versions/{published_version.version}/' ) assert response.status_code == 204 assert not Version.objects.all() + mock_delete_doi.assert_called_once() @pytest.mark.django_db From f7e00f9b45279ac26d4c2a5d2033096be8808f48 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 05/17] move doi handling for creation to celery task --- dandiapi/api/datacite.py | 2 +- dandiapi/api/doi.py | 25 ++++++++++++++++ dandiapi/api/services/dandiset/__init__.py | 33 ++-------------------- dandiapi/api/services/embargo/__init__.py | 12 ++------ dandiapi/api/tasks/__init__.py | 12 ++++++++ 5 files changed, 42 insertions(+), 42 deletions(-) diff --git a/dandiapi/api/datacite.py b/dandiapi/api/datacite.py index 61e68c16f..14e60e1d4 100644 --- a/dandiapi/api/datacite.py +++ b/dandiapi/api/datacite.py @@ -34,7 +34,7 @@ def block_during_test(fn): """ - Datacite API should not be called + Datacite API should not be called """ @wraps(fn) def wrapper(*args, **kwargs): diff --git a/dandiapi/api/doi.py b/dandiapi/api/doi.py index 26b8cf9b7..858124fa7 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -76,6 +76,31 @@ def delete_or_hide_doi(doi: str) -> None: datacite_client.delete_or_hide_doi(doi) +def _create_dandiset_draft_doi(draft_version: Version) -> None: + """ + Create a Draft DOI for a dandiset. + + This is called during dandiset creation for public dandisets. + For embargoed dandisets, no DOI is created until unembargo. + + Args: + draft_version: The draft version of the dandiset. + """ + # Generate a Draft DOI (event=None) + dandiset_doi, dandiset_doi_payload = generate_doi_data( + draft_version, + version_doi=False, + event=None, # Draft DOI + ) + + # Create the DOI + create_or_update_doi(dandiset_doi_payload) + + # Store the DOI in the draft version + draft_version.doi = dandiset_doi + draft_version.save() + + def _handle_publication_dois(version_id: int) -> None: """ Create and update DOIs for a published version. diff --git a/dandiapi/api/services/dandiset/__init__.py b/dandiapi/api/services/dandiset/__init__.py index fda550dc9..adc49346d 100644 --- a/dandiapi/api/services/dandiset/__init__.py +++ b/dandiapi/api/services/dandiset/__init__.py @@ -17,6 +17,7 @@ ) from dandiapi.api.services.permissions.dandiset import add_dandiset_owner, is_dandiset_owner from dandiapi.api.services.version.metadata import _normalize_version_metadata +from dandiapi.api.tasks import create_dandiset_draft_doi_task logger = logging.getLogger(__name__) @@ -61,42 +62,12 @@ def create_dandiset( dandiset=dandiset, user=user, metadata=draft_version.metadata, embargoed=embargo ) - # For public dandisets, create a Draft DOI if not embargo: - try: - _create_dandiset_draft_doi(draft_version) - except Exception: - # Log error but allow dandiset creation to proceed - logger.exception('Failed to create Draft DOI for dandiset %s', dandiset.identifier) + transaction.on_commit(lambda: create_dandiset_draft_doi_task.delay(draft_version.id)) return dandiset, draft_version -def _create_dandiset_draft_doi(draft_version: Version) -> None: - """ - Create a Draft DOI for a dandiset. - - This is called during dandiset creation for public dandisets. - For embargoed dandisets, no DOI is created until unembargo. - - Args: - draft_version: The draft version of the dandiset. - """ - # Generate a Draft DOI (event=None) - dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( - draft_version, - version_doi=False, - event=None, # Draft DOI - ) - - # Create the DOI - doi.create_or_update_doi(dandiset_doi_payload) - - # Store the DOI in the draft version - draft_version.doi = dandiset_doi - draft_version.save() - - def delete_dandiset(*, user, dandiset: Dandiset) -> None: if not is_dandiset_owner(dandiset, user): raise NotAllowedError('Cannot delete dandisets which you do not own.') diff --git a/dandiapi/api/services/embargo/__init__.py b/dandiapi/api/services/embargo/__init__.py index 35fc6e08d..e155affb3 100644 --- a/dandiapi/api/services/embargo/__init__.py +++ b/dandiapi/api/services/embargo/__init__.py @@ -10,12 +10,11 @@ from dandiapi.api.models.asset import Asset from dandiapi.api.services import audit from dandiapi.api.services.asset.exceptions import DandisetOwnerRequiredError -from dandiapi.api.services.dandiset import _create_dandiset_draft_doi from dandiapi.api.services.embargo.utils import remove_dandiset_embargo_tags from dandiapi.api.services.exceptions import DandiError from dandiapi.api.services.metadata import validate_version_metadata from dandiapi.api.services.permissions.dandiset import is_dandiset_owner -from dandiapi.api.tasks import unembargo_dandiset_task +from dandiapi.api.tasks import create_dandiset_draft_doi_task, unembargo_dandiset_task from .exceptions import ( AssetBlobEmbargoedError, @@ -74,14 +73,6 @@ def unembargo_dandiset(ds: Dandiset, user: User): validate_version_metadata(version=v) logger.info('Version metadata validated') - # Create a Draft DOI now that the dandiset is public - try: - _create_dandiset_draft_doi(v) - logger.info('Draft DOI created for unembargoed dandiset') - except Exception: - # Log error but continue with unembargo - logger.exception('Failed to create DOI during unembargo for dandiset %s', ds.identifier) - # Notify owners of completed unembargo send_dandiset_unembargoed_message(ds) logger.info('Dandiset owners notified.') @@ -89,6 +80,7 @@ def unembargo_dandiset(ds: Dandiset, user: User): logger.info('...Done') audit.unembargo_dandiset(dandiset=ds, user=user) + transaction.on_commit(lambda: create_dandiset_draft_doi_task.delay(v.id)) def remove_asset_blob_embargoed_tag(asset_blob: AssetBlob) -> None: diff --git a/dandiapi/api/tasks/__init__.py b/dandiapi/api/tasks/__init__.py index 99992e37d..609b1c34e 100644 --- a/dandiapi/api/tasks/__init__.py +++ b/dandiapi/api/tasks/__init__.py @@ -113,3 +113,15 @@ def handle_publication_dois_task(version_id: int) -> None: from dandiapi.api.doi import _handle_publication_dois _handle_publication_dois(version_id) + + +@shared_task(soft_time_limit=60) +def create_dandiset_draft_doi_task(version_id: int) -> None: + from dandiapi.api.doi import _create_dandiset_draft_doi + + version = Version.objects.get(id=version_id) + try: + _create_dandiset_draft_doi(version) + except Exception: + # Log error but allow dandiset creation to proceed + logger.exception('Failed to create Draft DOI for dandiset %s', version.dandiset.identifier) From ab2e82e00b99060886f57236025b0cf1bff846e6 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 06/17] fixup tests for moved _create_dandiset_draft_doi --- dandiapi/api/tests/test_dandiset.py | 34 ++-------------------------- dandiapi/api/tests/test_doi.py | 30 ++++++++++++++++++++++++ dandiapi/api/tests/test_unembargo.py | 2 -- 3 files changed, 32 insertions(+), 34 deletions(-) create mode 100644 dandiapi/api/tests/test_doi.py diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 61aaeabe0..18e8a0e8b 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -8,7 +8,7 @@ from dandiapi.api.asset_paths import add_asset_paths, add_version_asset_paths from dandiapi.api.models import Dandiset, Version -from dandiapi.api.services.dandiset import _create_dandiset_draft_doi, update_draft_version_doi +from dandiapi.api.services.dandiset import update_draft_version_doi from dandiapi.api.services.permissions.dandiset import ( add_dandiset_owner, get_dandiset_owners, @@ -356,7 +356,7 @@ def test_dandiset_rest_embargo_access( @pytest.mark.django_db -def test_dandiset_rest_create(api_client, user, mocker): +def test_dandiset_rest_create(api_client, user): user.first_name = 'John' user.last_name = 'Doe' user.save() @@ -364,13 +364,10 @@ def test_dandiset_rest_create(api_client, user, mocker): name = 'Test Dandiset' metadata = {'foo': 'bar'} - mock_create_doi = mocker.patch('dandiapi.api.services.dandiset._create_dandiset_draft_doi') - response = api_client.post( '/api/dandisets/', {'name': name, 'metadata': metadata}, format='json' ) - mock_create_doi.assert_called_once() assert response.data == { 'identifier': DANDISET_ID_RE, 'created': TIMESTAMP_RE, @@ -643,12 +640,10 @@ def test_dandiset_rest_create_embargoed(api_client, user, mocker): api_client.force_authenticate(user=user) name = 'Test Dandiset' metadata = {'foo': 'bar'} - mock_create_doi = mocker.patch('dandiapi.api.services.dandiset._create_dandiset_draft_doi') response = api_client.post( '/api/dandisets/?embargo=true', {'name': name, 'metadata': metadata}, format='json' ) - mock_create_doi.assert_not_called() assert response.data == { 'identifier': DANDISET_ID_RE, 'created': TIMESTAMP_RE, @@ -1393,31 +1388,6 @@ def test_dandiset_list_starred_unauthenticated(api_client): assert response.status_code == 401 -@pytest.mark.django_db -def test__create_dandiset_draft_doi(draft_version, mocker): - """Test the _create_dandiset_draft_doi function directly.""" - # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') - mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) - - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') - mock_create_doi.return_value = '10.48324/dandi.000123' - - # Call the function directly - _create_dandiset_draft_doi(draft_version) - - # Verify the mocks were called correctly - mock_generate_doi.assert_called_once_with( - draft_version, - version_doi=False, - event=None # Draft DOI - ) - mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) - - # Verify the DOI was stored in the draft version - assert draft_version.doi == '10.48324/dandi.000123' - - @pytest.mark.django_db def test_update_draft_version_doi_no_previous_doi(draft_version, mocker): """Test updating a draft DOI when none exists yet.""" diff --git a/dandiapi/api/tests/test_doi.py b/dandiapi/api/tests/test_doi.py new file mode 100644 index 000000000..efb96b874 --- /dev/null +++ b/dandiapi/api/tests/test_doi.py @@ -0,0 +1,30 @@ +from __future__ import annotations + +import pytest + +from dandiapi.api.doi import _create_dandiset_draft_doi + + +@pytest.mark.django_db +def test__create_dandiset_draft_doi(draft_version, mocker): + """Test the _create_dandiset_draft_doi function directly.""" + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi.return_value = '10.48324/dandi.000123' + + # Call the function directly + _create_dandiset_draft_doi(draft_version) + + # Verify the mocks were called correctly + mock_generate_doi.assert_called_once_with( + draft_version, + version_doi=False, + event=None # Draft DOI + ) + mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) + + # Verify the DOI was stored in the draft version + assert draft_version.doi == '10.48324/dandi.000123' \ No newline at end of file diff --git a/dandiapi/api/tests/test_unembargo.py b/dandiapi/api/tests/test_unembargo.py index 8c18ac3ca..3408887b6 100644 --- a/dandiapi/api/tests/test_unembargo.py +++ b/dandiapi/api/tests/test_unembargo.py @@ -335,7 +335,6 @@ def test_unembargo_dandiset_validate_version_metadata( draft_version.save() draft_version.assets.add(asset_factory()) - mock_create_doi = mocker.patch('dandiapi.api.services.embargo._create_dandiset_draft_doi') # Spy on the imported function in the embargo service validate_version_spy = mocker.spy(embargo_service, 'validate_version_metadata') @@ -344,7 +343,6 @@ def test_unembargo_dandiset_validate_version_metadata( assert validate_version_spy.call_count == 1 draft_version.refresh_from_db() assert not draft_version.validation_errors - mock_create_doi.assert_called_once() @pytest.mark.django_db From c645c75403e885c0a45a26b872eb1f2b3df2b6fe Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 07/17] move doi handling for update to celery task --- dandiapi/api/doi.py | 33 ++++++++++ dandiapi/api/services/dandiset/__init__.py | 33 ---------- dandiapi/api/tasks/__init__.py | 12 ++++ dandiapi/api/tests/test_audit.py | 2 - dandiapi/api/tests/test_dandiset.py | 67 -------------------- dandiapi/api/tests/test_doi.py | 70 ++++++++++++++++++++- dandiapi/api/tests/test_version.py | 72 +++++----------------- dandiapi/api/views/version.py | 8 +-- 8 files changed, 131 insertions(+), 166 deletions(-) diff --git a/dandiapi/api/doi.py b/dandiapi/api/doi.py index 858124fa7..8db491b64 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -101,6 +101,39 @@ def _create_dandiset_draft_doi(draft_version: Version) -> None: draft_version.save() +def _update_draft_version_doi(draft_version: Version) -> None: + """ + Update or create a Draft DOI for a dandiset with the latest metadata. + + This is called when a draft version's metadata is updated for a dandiset + that has never been published. + + Args: + draft_version: The draft version of the dandiset with updated metadata. + """ + # Skip for dandisets that have published versions + if draft_version.dandiset.versions.exclude(version='draft').exists(): + return + + # Generate DOI payload with updated metadata + dandiset_doi, dandiset_doi_payload = generate_doi_data( + draft_version, + version_doi=False, # Generate a Dandiset DOI, not a Version DOI + event=None, # Keep as Draft DOI + ) + + # Create or update the DOI + create_or_update_doi(dandiset_doi_payload) + + # If the version doesn't have a DOI yet, store it + if draft_version.doi is None: + draft_version.doi = dandiset_doi + draft_version.save() + logger.info('Created new Draft DOI %s', dandiset_doi) + else: + logger.info('Updated Draft DOI %s with new metadata', draft_version.doi) + + def _handle_publication_dois(version_id: int) -> None: """ Create and update DOIs for a published version. diff --git a/dandiapi/api/services/dandiset/__init__.py b/dandiapi/api/services/dandiset/__init__.py index adc49346d..3bed7cba2 100644 --- a/dandiapi/api/services/dandiset/__init__.py +++ b/dandiapi/api/services/dandiset/__init__.py @@ -132,36 +132,3 @@ def unstar_dandiset(*, user, dandiset: Dandiset) -> int: DandisetStar.objects.filter(user=user, dandiset=dandiset).delete() return dandiset.star_count - - -def update_draft_version_doi(draft_version: Version) -> None: - """ - Update or create a Draft DOI for a dandiset with the latest metadata. - - This is called when a draft version's metadata is updated for a dandiset - that has never been published. - - Args: - draft_version: The draft version of the dandiset with updated metadata. - """ - # Skip for dandisets that have published versions - if draft_version.dandiset.versions.exclude(version='draft').exists(): - return - - # Generate DOI payload with updated metadata - dandiset_doi, dandiset_doi_payload = doi.generate_doi_data( - draft_version, - version_doi=False, # Generate a Dandiset DOI, not a Version DOI - event=None, # Keep as Draft DOI - ) - - # Create or update the DOI - doi.create_or_update_doi(dandiset_doi_payload) - - # If the version doesn't have a DOI yet, store it - if draft_version.doi is None: - draft_version.doi = dandiset_doi - draft_version.save() - logger.info('Created new Draft DOI %s', dandiset_doi) - else: - logger.info('Updated Draft DOI %s with new metadata', draft_version.doi) diff --git a/dandiapi/api/tasks/__init__.py b/dandiapi/api/tasks/__init__.py index 609b1c34e..6b862427b 100644 --- a/dandiapi/api/tasks/__init__.py +++ b/dandiapi/api/tasks/__init__.py @@ -125,3 +125,15 @@ def create_dandiset_draft_doi_task(version_id: int) -> None: except Exception: # Log error but allow dandiset creation to proceed logger.exception('Failed to create Draft DOI for dandiset %s', version.dandiset.identifier) + + +@shared_task(soft_time_limit=60) +def update_draft_version_doi_task(version_id: int) -> None: + from dandiapi.api.doi import _update_draft_version_doi + + version = Version.objects.get(id=version_id) + try: + _update_draft_version_doi(version) + except Exception: + # Log error but allow version update to proceed + logger.exception('Failed to update Draft DOI for dandiset %s', version.dandiset.identifier) diff --git a/dandiapi/api/tests/test_audit.py b/dandiapi/api/tests/test_audit.py index 0af71a469..a687d0e5b 100644 --- a/dandiapi/api/tests/test_audit.py +++ b/dandiapi/api/tests/test_audit.py @@ -123,7 +123,6 @@ def test_audit_update_metadata(api_client, draft_version, user, mocker): dandiset = draft_version.dandiset add_dandiset_owner(dandiset, user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') # Edit its metadata. metadata = draft_version.metadata metadata['foo'] = 'bar' @@ -145,7 +144,6 @@ def test_audit_update_metadata(api_client, draft_version, user, mocker): metadata = rec.details['metadata'] assert metadata['name'] == 'baz' assert metadata['foo'] == 'bar' - mock_update_doi.assert_called_once() @pytest.mark.django_db diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 18e8a0e8b..4618e262f 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -8,7 +8,6 @@ from dandiapi.api.asset_paths import add_asset_paths, add_version_asset_paths from dandiapi.api.models import Dandiset, Version -from dandiapi.api.services.dandiset import update_draft_version_doi from dandiapi.api.services.permissions.dandiset import ( add_dandiset_owner, get_dandiset_owners, @@ -1386,69 +1385,3 @@ def test_dandiset_list_order_size(api_client, user, draft_version_factory, asset def test_dandiset_list_starred_unauthenticated(api_client): response = api_client.get('/api/dandisets/', {'starred': True}) assert response.status_code == 401 - - -@pytest.mark.django_db -def test_update_draft_version_doi_no_previous_doi(draft_version, mocker): - """Test updating a draft DOI when none exists yet.""" - # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') - mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) - - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') - mock_create_doi.return_value = '10.48324/dandi.000123' - - update_draft_version_doi(draft_version) - - # Verify the mocks were called correctly - mock_generate_doi.assert_called_once_with( - draft_version, - version_doi=False, - event=None - ) - mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) - - # Verify the DOI was stored in the draft version - assert draft_version.doi == '10.48324/dandi.000123' - - -@pytest.mark.django_db -def test_update_draft_version_doi_existing_doi(draft_version, mocker): - """Test updating a draft DOI when one already exists.""" - # Set existing DOI - draft_version.doi = '10.48324/dandi.000123' - draft_version.save() - - # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') - mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) - - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') - mock_create_doi.return_value = '10.48324/dandi.000123' - - update_draft_version_doi(draft_version) - - # Verify the mocks were called correctly - mock_generate_doi.assert_called_once_with( - draft_version, - version_doi=False, - event=None - ) - mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) - - # Verify the DOI is still the same - assert draft_version.doi == '10.48324/dandi.000123' - - -@pytest.mark.django_db -def test_update_draft_version_doi_published_version(draft_version, published_version, mocker): - """Test that update_draft_version_doi is a no-op for dandisets with published versions.""" - # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') - - update_draft_version_doi(draft_version) - - # Verify no DOI operations were performed - mock_generate_doi.assert_not_called() - mock_create_doi.assert_not_called() diff --git a/dandiapi/api/tests/test_doi.py b/dandiapi/api/tests/test_doi.py index efb96b874..055b713dc 100644 --- a/dandiapi/api/tests/test_doi.py +++ b/dandiapi/api/tests/test_doi.py @@ -2,7 +2,7 @@ import pytest -from dandiapi.api.doi import _create_dandiset_draft_doi +from dandiapi.api.doi import _create_dandiset_draft_doi, _update_draft_version_doi @pytest.mark.django_db @@ -27,4 +27,70 @@ def test__create_dandiset_draft_doi(draft_version, mocker): mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) # Verify the DOI was stored in the draft version - assert draft_version.doi == '10.48324/dandi.000123' \ No newline at end of file + assert draft_version.doi == '10.48324/dandi.000123' + + +@pytest.mark.django_db +def test_update_draft_version_doi_no_previous_doi(draft_version, mocker): + """Test updating a draft DOI when none exists yet.""" + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi.return_value = '10.48324/dandi.000123' + + _update_draft_version_doi(draft_version) + + # Verify the mocks were called correctly + mock_generate_doi.assert_called_once_with( + draft_version, + version_doi=False, + event=None + ) + mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) + + # Verify the DOI was stored in the draft version + assert draft_version.doi == '10.48324/dandi.000123' + + +@pytest.mark.django_db +def test_update_draft_version_doi_existing_doi(draft_version, mocker): + """Test updating a draft DOI when one already exists.""" + # Set existing DOI + draft_version.doi = '10.48324/dandi.000123' + draft_version.save() + + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi.return_value = '10.48324/dandi.000123' + + _update_draft_version_doi(draft_version) + + # Verify the mocks were called correctly + mock_generate_doi.assert_called_once_with( + draft_version, + version_doi=False, + event=None + ) + mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) + + # Verify the DOI is still the same + assert draft_version.doi == '10.48324/dandi.000123' + + +@pytest.mark.django_db +def test_update_draft_version_doi_published_version(draft_version, published_version, mocker): + """Test that update_draft_version_doi is a no-op for dandisets with published versions.""" + # Set up mocks + mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + + _update_draft_version_doi(draft_version) + + # Verify no DOI operations were performed + mock_generate_doi.assert_not_called() + mock_create_doi.assert_not_called() \ No newline at end of file diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index ef6f9d842..3a34f0e5a 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -534,10 +534,9 @@ def test_version_rest_info_with_asset( @pytest.mark.django_db -def test_version_rest_update(api_client, user, draft_version, mocker): +def test_version_rest_update(api_client, user, draft_version): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_name = 'A unique and special name!' new_metadata = { @@ -621,17 +620,15 @@ def test_version_rest_update(api_client, user, draft_version, mocker): assert draft_version.metadata == saved_metadata assert draft_version.name == new_name assert draft_version.status == Version.Status.PENDING - mock_update_doi.assert_called_once_with(draft_version) @pytest.mark.django_db -def test_version_rest_update_embargoed(api_client, user, draft_version_factory, mocker): +def test_version_rest_update_embargoed(api_client, user, draft_version_factory): draft_version = draft_version_factory( dandiset__embargo_status=Dandiset.EmbargoStatus.EMBARGOED ) add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_name = 'Embargoed dandiset name' new_metadata = { @@ -646,35 +643,10 @@ def test_version_rest_update_embargoed(api_client, user, draft_version_factory, format='json', ) assert resp.status_code == 200 - mock_update_doi.assert_not_called() @pytest.mark.django_db -def test_version_rest_update_doi_error(api_client, user, draft_version, mocker): - add_dandiset_owner(draft_version.dandiset, user) - api_client.force_authenticate(user=user) - - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') - mock_update_doi.side_effect = ValueError('DOI error') - mock_logger = mocker.patch('dandiapi.api.views.version.logger') - - new_name = 'Test error handling' - new_metadata = {'schemaVersion': settings.DANDI_SCHEMA_VERSION, 'description': 'Test'} - - # Update should succeed even if DOI update fails - resp = api_client.put( - f'/api/dandisets/{draft_version.dandiset.identifier}/versions/{draft_version.version}/', - {'metadata': new_metadata, 'name': new_name}, - format='json', - ) - assert resp.status_code == 200 - mock_update_doi.assert_called_once() - mock_logger.exception.assert_called_once() - - -@pytest.mark.django_db -def test_version_rest_update_unembargo_in_progress(api_client, user, draft_version_factory, mocker): - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') +def test_version_rest_update_unembargo_in_progress(api_client, user, draft_version_factory): draft_version = draft_version_factory( dandiset__embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING ) @@ -697,14 +669,12 @@ def test_version_rest_update_unembargo_in_progress(api_client, user, draft_versi format='json', ) assert resp.status_code == 400 - mock_update_doi.assert_not_called() @pytest.mark.django_db -def test_version_rest_update_published_version(api_client, user, published_version, mocker): +def test_version_rest_update_published_version(api_client, user, published_version): add_dandiset_owner(published_version.dandiset, user) api_client.force_authenticate(user=user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_name = 'A unique and special name!' new_metadata = {'foo': 'bar', 'num': 123, 'list': ['a', 'b', 'c']} @@ -717,13 +687,11 @@ def test_version_rest_update_published_version(api_client, user, published_versi ) assert resp.status_code == 405 assert resp.data == 'Only draft versions can be modified.' - mock_update_doi.assert_not_called() @pytest.mark.django_db -def test_version_rest_update_not_an_owner(api_client, user, version, mocker): +def test_version_rest_update_not_an_owner(api_client, user, version): api_client.force_authenticate(user=user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_name = 'A unique and special name!' new_metadata = {'foo': 'bar', 'num': 123, 'list': ['a', 'b', 'c']} @@ -736,26 +704,24 @@ def test_version_rest_update_not_an_owner(api_client, user, version, mocker): ).status_code == 403 ) - mock_update_doi.assert_not_called() @pytest.mark.parametrize( - ('access', 'triggers_update'), + 'access', [ - ('some value', True), - (123, True), - (None, True), - ([], True), - (['a', 'b'], True), - (['a', 'b', {}], True), - ([{'schemaKey': 'AccessRequirements', 'status': 'foobar'}], False) + 'some value', + 123, + None, + [], + ['a', 'b'], + ['a', 'b', {}], + [{'schemaKey': 'AccessRequirements', 'status': 'foobar'}] ], ) @pytest.mark.django_db -def test_version_rest_update_access_values(api_client, user, draft_version, access, triggers_update, mocker): +def test_version_rest_update_access_values(api_client, user, draft_version, access): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') new_metadata = {**draft_version.metadata, 'access': access} resp = api_client.put( @@ -773,15 +739,12 @@ def test_version_rest_update_access_values(api_client, user, draft_version, acce if draft_version.dandiset.embargoed else AccessType.OpenAccess.value ) - if triggers_update: - mock_update_doi.assert_called_once() @pytest.mark.django_db -def test_version_rest_update_access_missing(api_client, user, draft_version, mocker): +def test_version_rest_update_access_missing(api_client, user, draft_version): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') # Check that the field missing entirely is also okay new_metadata = {**draft_version.metadata} @@ -802,14 +765,12 @@ def test_version_rest_update_access_missing(api_client, user, draft_version, moc if draft_version.dandiset.embargoed else AccessType.OpenAccess.value ) - mock_update_doi.assert_called_once() @pytest.mark.django_db -def test_version_rest_update_access_valid(api_client, user, draft_version, mocker): +def test_version_rest_update_access_valid(api_client, user, draft_version): add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) - mock_update_doi = mocker.patch('dandiapi.api.views.version.update_draft_version_doi') # Check that extra fields persist new_metadata = {**draft_version.metadata, 'access': [{'extra': 'field'}]} @@ -830,7 +791,6 @@ def test_version_rest_update_access_valid(api_client, user, draft_version, mocke ) assert access[0]['extra'] == 'field' - mock_update_doi.assert_called_once() @pytest.mark.django_db diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index 9b2ab8ca1..250cc0370 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -20,8 +20,7 @@ require_dandiset_owner_or_403, ) from dandiapi.api.services.publish import publish_dandiset -from dandiapi.api.services.dandiset import update_draft_version_doi -from dandiapi.api.tasks import delete_doi_task +from dandiapi.api.tasks import delete_doi_task, update_draft_version_doi_task from dandiapi.api.views.common import DANDISET_PK_PARAM, VERSION_PARAM from dandiapi.api.views.pagination import DandiPagination from dandiapi.api.views.serializers import ( @@ -139,10 +138,7 @@ def update(self, request, **kwargs): # For unpublished dandisets, update or create the draft DOI # to keep it in sync with the latest metadata if not locked_version.dandiset.embargoed: - try: - update_draft_version_doi(locked_version) - except ValueError: - logger.exception('Failed to update Draft DOI for dandiset %s', locked_version.dandiset.identifier) + transaction.on_commit(lambda: update_draft_version_doi_task.delay(locked_version.id)) else: logger.debug("Skipping DOI update for embargoed Dandiset %s.", locked_version.dandiset.identifier) From 8b2796707d4d1fd386fddd459ee481d692f69a21 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 08/17] Cleanup unnecessary changes --- dandiapi/api/services/publish/__init__.py | 2 -- dandiapi/api/tests/test_asset_paths.py | 2 +- dandiapi/api/tests/test_audit.py | 2 +- dandiapi/api/tests/test_dandiset.py | 4 +--- dandiapi/api/tests/test_version.py | 5 +++-- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/dandiapi/api/services/publish/__init__.py b/dandiapi/api/services/publish/__init__.py index 66fa76eaf..0b74bb4b0 100644 --- a/dandiapi/api/services/publish/__init__.py +++ b/dandiapi/api/services/publish/__init__.py @@ -106,8 +106,6 @@ def _build_publishable_version_from_draft(draft_version: Version) -> Version: return publishable_version - - def _publish_dandiset(dandiset_id: int, user_id: int) -> None: """ Publish a dandiset. diff --git a/dandiapi/api/tests/test_asset_paths.py b/dandiapi/api/tests/test_asset_paths.py index 70155fb64..01385d35e 100644 --- a/dandiapi/api/tests/test_asset_paths.py +++ b/dandiapi/api/tests/test_asset_paths.py @@ -304,7 +304,7 @@ def test_asset_path_search_asset_paths(draft_version_factory, asset_factory): @pytest.mark.django_db -def test_asset_path_publish_version(draft_version_factory, asset_factory, user, mocker): +def test_asset_path_publish_version(draft_version_factory, asset_factory, user): version: Version = draft_version_factory() asset = asset_factory(path='foo/bar.txt', status=Asset.Status.VALID) version.assets.add(asset) diff --git a/dandiapi/api/tests/test_audit.py b/dandiapi/api/tests/test_audit.py index a687d0e5b..26ef521d5 100644 --- a/dandiapi/api/tests/test_audit.py +++ b/dandiapi/api/tests/test_audit.py @@ -118,7 +118,7 @@ def user_info(u): @pytest.mark.django_db -def test_audit_update_metadata(api_client, draft_version, user, mocker): +def test_audit_update_metadata(api_client, draft_version, user): # Create a Dandiset. dandiset = draft_version.dandiset add_dandiset_owner(dandiset, user) diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 4618e262f..d2fd0ca57 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -366,7 +366,6 @@ def test_dandiset_rest_create(api_client, user): response = api_client.post( '/api/dandisets/', {'name': name, 'metadata': metadata}, format='json' ) - assert response.data == { 'identifier': DANDISET_ID_RE, 'created': TIMESTAMP_RE, @@ -632,7 +631,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): @pytest.mark.django_db -def test_dandiset_rest_create_embargoed(api_client, user, mocker): +def test_dandiset_rest_create_embargoed(api_client, user): user.first_name = 'John' user.last_name = 'Doe' user.save() @@ -785,7 +784,6 @@ def test_dandiset_rest_delete(api_client, draft_version_factory, user, embargo_s else: assert response.status_code >= 400 assert Dandiset.objects.count() == 1 - # Verify that delete_or_hide_doi was not called mock_delete_doi.assert_not_called() diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index 3a34f0e5a..d0059c05b 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -707,7 +707,7 @@ def test_version_rest_update_not_an_owner(api_client, user, version): @pytest.mark.parametrize( - 'access', + ('access'), [ 'some value', 123, @@ -715,7 +715,7 @@ def test_version_rest_update_not_an_owner(api_client, user, version): [], ['a', 'b'], ['a', 'b', {}], - [{'schemaKey': 'AccessRequirements', 'status': 'foobar'}] + [{'schemaKey': 'AccessRequirements', 'status': 'foobar'}], ], ) @pytest.mark.django_db @@ -729,6 +729,7 @@ def test_version_rest_update_access_values(api_client, user, draft_version, acce {'metadata': new_metadata, 'name': draft_version.name}, format='json', ) + assert resp.status_code == 200 draft_version.refresh_from_db() access = draft_version.metadata['access'] From 2acf10a69240018c7284a9924c66184067d4c6a4 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 09/17] Add Dandiset DOI to vue --- web/src/views/DandisetLandingView/DandisetMain.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/views/DandisetLandingView/DandisetMain.vue b/web/src/views/DandisetLandingView/DandisetMain.vue index e946897ab..1afa0d033 100644 --- a/web/src/views/DandisetLandingView/DandisetMain.vue +++ b/web/src/views/DandisetLandingView/DandisetMain.vue @@ -24,7 +24,7 @@ class="ml-2" /> From a25ad0bd4449cc5eb32d55146191a85deb4c9f04 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:46 -0400 Subject: [PATCH 10/17] Prevent dandiset deletion if doi delete fails --- dandiapi/api/services/dandiset/__init__.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/dandiapi/api/services/dandiset/__init__.py b/dandiapi/api/services/dandiset/__init__.py index 3bed7cba2..0855a9ba9 100644 --- a/dandiapi/api/services/dandiset/__init__.py +++ b/dandiapi/api/services/dandiset/__init__.py @@ -78,22 +78,19 @@ def delete_dandiset(*, user, dandiset: Dandiset) -> None: if dandiset.unembargo_in_progress: raise DandisetUnembargoInProgressError - # Check if there's a DOI that needs to be handled - draft_version = dandiset.versions.filter(version='draft').first() - # Delete all versions first, so that AssetPath deletion is cascaded # through versions, rather than through zarrs directly with transaction.atomic(): # Record the audit event first so that the AuditRecord instance has a # chance to grab the Dandiset information before it is destroyed. audit.delete_dandiset(dandiset=dandiset, user=user) + # Dandisets with published versions cannot be deleted - # so only the Dandiset DOI needs to be delete. + # so only the Dandiset DOI needs to be deleted. + draft_version = dandiset.versions.filter(version='draft').first() if draft_version and draft_version.doi is not None: - try: - doi.delete_or_hide_doi(draft_version.doi) - except Exception: - pass # doi operation should not stop deletion. delete_or_hide will log the exception. + # Call DOI deletion prior to delete so if this raises, we do not proceed + doi.delete_or_hide_doi(draft_version.doi) dandiset.versions.all().delete() dandiset.delete() From 5d8bea23b10239801e5937f456dd3e92b470a1ed Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:47 -0400 Subject: [PATCH 11/17] remove unnecessary compatability layer --- dandiapi/api/doi.py | 54 ++++++---------------------------- dandiapi/api/tests/test_doi.py | 16 +++++----- 2 files changed, 17 insertions(+), 53 deletions(-) diff --git a/dandiapi/api/doi.py b/dandiapi/api/doi.py index 8db491b64..868c3e769 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -24,42 +24,6 @@ datacite_client = DataCiteClient() -def generate_doi_data( - version: Version, version_doi: bool = True, event: str | None = None -) -> tuple[str, dict]: - """ - Generate DOI data for a version or dandiset. - - Args: - version: Version object containing metadata. - version_doi: If True, generate a Version DOI, otherwise generate a Dandiset DOI. - event: The DOI event type. - - None: Creates a Draft DOI. - - "publish": Creates or promotes to a Findable DOI. - - "hide": Converts to a Registered DOI. - - Returns: - Tuple of (doi_string, datacite_payload) - """ - return datacite_client.generate_doi_data(version, version_doi, event) - - -def create_or_update_doi(datacite_payload: dict) -> str | None: - """ - Create or update a DOI with the DataCite API. - - Args: - datacite_payload: The DOI payload to send to DataCite. - - Returns: - The DOI string on success, None on failure when not configured. - - Raises: - requests.exceptions.HTTPError: If the API request fails. - """ - return datacite_client.create_or_update_doi(datacite_payload) - - def delete_or_hide_doi(doi: str) -> None: """ Delete a draft DOI or hide a findable DOI depending on its state. @@ -87,14 +51,14 @@ def _create_dandiset_draft_doi(draft_version: Version) -> None: draft_version: The draft version of the dandiset. """ # Generate a Draft DOI (event=None) - dandiset_doi, dandiset_doi_payload = generate_doi_data( + dandiset_doi, dandiset_doi_payload = datacite_client.generate_doi_data( draft_version, version_doi=False, event=None, # Draft DOI ) # Create the DOI - create_or_update_doi(dandiset_doi_payload) + datacite_client.create_or_update_doi(dandiset_doi_payload) # Store the DOI in the draft version draft_version.doi = dandiset_doi @@ -116,14 +80,14 @@ def _update_draft_version_doi(draft_version: Version) -> None: return # Generate DOI payload with updated metadata - dandiset_doi, dandiset_doi_payload = generate_doi_data( + dandiset_doi, dandiset_doi_payload = datacite_client.generate_doi_data( draft_version, version_doi=False, # Generate a Dandiset DOI, not a Version DOI event=None, # Keep as Draft DOI ) # Create or update the DOI - create_or_update_doi(dandiset_doi_payload) + datacite_client.create_or_update_doi(dandiset_doi_payload) # If the version doesn't have a DOI yet, store it if draft_version.doi is None: @@ -152,21 +116,21 @@ def _handle_publication_dois(version_id: int) -> None: is_first_publication = draft_version.doi is None # Create Version DOI as Findable - version_doi, version_doi_payload = generate_doi_data( + version_doi, version_doi_payload = datacite_client.generate_doi_data( version, version_doi=True, event='publish' ) # Either create or update the Dandiset DOI based on whether it's the first publication if is_first_publication: # For first publication: generate Dandiset DOI and promote from Draft to Findable - dandiset_doi, dandiset_doi_payload = generate_doi_data( + dandiset_doi, dandiset_doi_payload = datacite_client.generate_doi_data( version, version_doi=False, event='publish', # Promote to Findable on first publication ) else: # For subsequent publications: update the metadata but keep as Findable - dandiset_doi, dandiset_doi_payload = generate_doi_data( + dandiset_doi, dandiset_doi_payload = datacite_client.generate_doi_data( version, version_doi=False, event='publish', # Update existing Findable DOI @@ -174,8 +138,8 @@ def _handle_publication_dois(version_id: int) -> None: # Create or update the DOIs # TODO(asmacdo) we need to try:except here, so dandiset doi doesnt block version doi - create_or_update_doi(dandiset_doi_payload) - create_or_update_doi(version_doi_payload) + datacite_client.create_or_update_doi(dandiset_doi_payload) + datacite_client.create_or_update_doi(version_doi_payload) # Store the DOI values version.doi = version_doi diff --git a/dandiapi/api/tests/test_doi.py b/dandiapi/api/tests/test_doi.py index 055b713dc..275152b4d 100644 --- a/dandiapi/api/tests/test_doi.py +++ b/dandiapi/api/tests/test_doi.py @@ -9,10 +9,10 @@ def test__create_dandiset_draft_doi(draft_version, mocker): """Test the _create_dandiset_draft_doi function directly.""" # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi = mocker.patch('dandiapi.api.doi.datacite_client.generate_doi_data') mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi = mocker.patch('dandiapi.api.doi.datacite_client.create_or_update_doi') mock_create_doi.return_value = '10.48324/dandi.000123' # Call the function directly @@ -34,10 +34,10 @@ def test__create_dandiset_draft_doi(draft_version, mocker): def test_update_draft_version_doi_no_previous_doi(draft_version, mocker): """Test updating a draft DOI when none exists yet.""" # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi = mocker.patch('dandiapi.api.doi.datacite_client.generate_doi_data') mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi = mocker.patch('dandiapi.api.doi.datacite_client.create_or_update_doi') mock_create_doi.return_value = '10.48324/dandi.000123' _update_draft_version_doi(draft_version) @@ -62,10 +62,10 @@ def test_update_draft_version_doi_existing_doi(draft_version, mocker): draft_version.save() # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') + mock_generate_doi = mocker.patch('dandiapi.api.doi.datacite_client.generate_doi_data') mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_create_doi = mocker.patch('dandiapi.api.doi.datacite_client.create_or_update_doi') mock_create_doi.return_value = '10.48324/dandi.000123' _update_draft_version_doi(draft_version) @@ -86,8 +86,8 @@ def test_update_draft_version_doi_existing_doi(draft_version, mocker): def test_update_draft_version_doi_published_version(draft_version, published_version, mocker): """Test that update_draft_version_doi is a no-op for dandisets with published versions.""" # Set up mocks - mock_generate_doi = mocker.patch('dandiapi.api.doi.generate_doi_data') - mock_create_doi = mocker.patch('dandiapi.api.doi.create_or_update_doi') + mock_generate_doi = mocker.patch('dandiapi.api.doi.datacite_client.generate_doi_data') + mock_create_doi = mocker.patch('dandiapi.api.doi.datacite_client.create_or_update_doi') _update_draft_version_doi(draft_version) From bb76239b5bda895fd3340e7fd16ffc234067ae85 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:47 -0400 Subject: [PATCH 12/17] fixup linting --- dandiapi/api/datacite.py | 25 ++++----- dandiapi/api/doi.py | 6 +-- dandiapi/api/services/dandiset/__init__.py | 4 -- dandiapi/api/services/publish/__init__.py | 1 - dandiapi/api/tests/test_dandiset.py | 4 +- dandiapi/api/tests/test_datacite.py | 61 ++++++++++------------ dandiapi/api/tests/test_doi.py | 16 ++---- dandiapi/api/tests/test_version.py | 4 +- dandiapi/api/views/version.py | 12 +++-- 9 files changed, 58 insertions(+), 75 deletions(-) diff --git a/dandiapi/api/datacite.py b/dandiapi/api/datacite.py index 14e60e1d4..befd886fd 100644 --- a/dandiapi/api/datacite.py +++ b/dandiapi/api/datacite.py @@ -31,16 +31,15 @@ logger = logging.getLogger(__name__) - def block_during_test(fn): - """ - Datacite API should not be called - """ + """Datacite API should not be called.""" + @wraps(fn) def wrapper(*args, **kwargs): - if "pytest" in sys.modules: - raise RuntimeError(f"DOI calls to {fn.__name__} blocked during test.") + if 'pytest' in sys.modules: + raise RuntimeError(f'DOI calls to {fn.__name__} blocked during test.') return fn(*args, **kwargs) + return wrapper @@ -73,13 +72,13 @@ def format_doi(self, dandiset_id: str, version_id: str | None = None) -> str: Formatted DOI string. """ if version_id: - # TODO(asmaco) replace "dandi" with non-hardcoded ID_PATTERN + # TODO(asmaco): replace "dandi" with non-hardcoded ID_PATTERN # https://github.com/dandi/dandi-schema/pull/294/files#diff-43c9cc813638d87fd33e527a7baccb2fd7dff85595a7e686bfaf61f0409bd403R47 return f'{self.api_prefix}/dandi.{dandiset_id}/{version_id}' return f'{self.api_prefix}/dandi.{dandiset_id}' def generate_doi_data( - self, version: Version, version_doi: bool = True, event: str | None = None + self, version: Version, *, version_doi: bool = True, event: str | None = None ) -> tuple[str, dict]: """ Generate DOI data for a version or dandiset. @@ -95,8 +94,9 @@ def generate_doi_data( Returns: Tuple of (doi_string, datacite_payload) """ - # TODO(asmacdo) if not datacite configured make sure we dont save any dois to model + # TODO(asmacdo): if not datacite configured make sure we dont save any dois to model from dandischema.datacite import to_datacite + dandiset_id = version.dandiset.identifier version_id = version.version metadata = copy.deepcopy(version.metadata) @@ -158,7 +158,7 @@ def create_or_update_doi(self, original_datacite_payload: dict) -> str | None: ) response.raise_for_status() # Return early on success - return doi + return doi # noqa: TRY300 except requests.exceptions.HTTPError as e: # HTTP 422 status code means DOI already exists already_exists_code = 422 @@ -174,8 +174,7 @@ def create_or_update_doi(self, original_datacite_payload: dict) -> str | None: timeout=self.timeout, ) update_response.raise_for_status() - # Success with update - return doi + return doi # noqa: TRY300 except Exception: error_details = f'Failed to update existing DOI {doi}' if e.response and hasattr(e.response, 'text'): @@ -259,5 +258,3 @@ def delete_or_hide_doi(self, doi: str) -> None: return logger.exception('Failed to delete or hide DOI %s', doi) raise - - diff --git a/dandiapi/api/doi.py b/dandiapi/api/doi.py index 868c3e769..35c3ef690 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -10,8 +10,6 @@ import logging from typing import TYPE_CHECKING -from django.conf import settings - from dandiapi.api.datacite import DataCiteClient if TYPE_CHECKING: @@ -111,7 +109,7 @@ def _handle_publication_dois(version_id: int) -> None: draft_version = version.dandiset.draft_version # Check if this is the first publication (no prior DOI in draft version) - # TODO(asmacdo) not true anymore, we need to check the db. + # TODO(asmacdo): not true anymore, we need to check the db. # if draft_version.dandiset.versions.exclude(version='draft').exists(): is_first_publication = draft_version.doi is None @@ -137,7 +135,7 @@ def _handle_publication_dois(version_id: int) -> None: ) # Create or update the DOIs - # TODO(asmacdo) we need to try:except here, so dandiset doi doesnt block version doi + # TODO(asmacdo): we need to try:except here, so dandiset doi doesn't block version doi datacite_client.create_or_update_doi(dandiset_doi_payload) datacite_client.create_or_update_doi(version_doi_payload) diff --git a/dandiapi/api/services/dandiset/__init__.py b/dandiapi/api/services/dandiset/__init__.py index 0855a9ba9..5872c9d51 100644 --- a/dandiapi/api/services/dandiset/__init__.py +++ b/dandiapi/api/services/dandiset/__init__.py @@ -1,7 +1,5 @@ from __future__ import annotations -import logging - from django.db import transaction from dandiapi.api import doi @@ -19,8 +17,6 @@ from dandiapi.api.services.version.metadata import _normalize_version_metadata from dandiapi.api.tasks import create_dandiset_draft_doi_task -logger = logging.getLogger(__name__) - def create_dandiset( *, diff --git a/dandiapi/api/services/publish/__init__.py b/dandiapi/api/services/publish/__init__.py index 0b74bb4b0..d94e4362d 100644 --- a/dandiapi/api/services/publish/__init__.py +++ b/dandiapi/api/services/publish/__init__.py @@ -8,7 +8,6 @@ from django.db import transaction from more_itertools import ichunked -from dandiapi.api import doi from dandiapi.api.asset_paths import add_version_asset_paths from dandiapi.api.models import Asset, Dandiset, Version from dandiapi.api.services import audit diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index d2fd0ca57..c66edf8b4 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -759,7 +759,9 @@ def test_dandiset_rest_create_with_invalid_identifier(api_client, admin_user): (Dandiset.EmbargoStatus.UNEMBARGOING, False, '10.48324/dandi.000123'), ], ) -def test_dandiset_rest_delete(api_client, draft_version_factory, user, embargo_status, success, doi, mocker): +def test_dandiset_rest_delete( + api_client, draft_version_factory, user, embargo_status, success, doi, mocker +): api_client.force_authenticate(user=user) mock_delete_doi = mocker.patch('dandiapi.api.doi.delete_or_hide_doi') diff --git a/dandiapi/api/tests/test_datacite.py b/dandiapi/api/tests/test_datacite.py index 994efaa96..8b1aeae80 100644 --- a/dandiapi/api/tests/test_datacite.py +++ b/dandiapi/api/tests/test_datacite.py @@ -1,13 +1,12 @@ from __future__ import annotations -import pytest -import requests from django.conf import settings +import pytest from requests.exceptions import HTTPError -from dandiapi.api.datacite import DataCiteClient, DANDI_DOI_SETTINGS +from dandiapi.api.datacite import DataCiteClient + -# Mock individual HTTP methods for safety @pytest.fixture(autouse=True) def mock_requests(mocker): """Mock individual request methods for all tests to prevent actual HTTP calls.""" @@ -31,9 +30,7 @@ def datacite_client(): @pytest.fixture def datacite_client_unsafe(): - """ - Bypass safety feature that prevents API calls to datacite during tests. - """ + """Bypass safety feature that prevents API calls to datacite during tests.""" client = DataCiteClient() if hasattr(client.create_or_update_doi, '__wrapped__'): client.create_or_update_doi = client.create_or_update_doi.__wrapped__.__get__(client) @@ -96,7 +93,9 @@ def test_generate_doi_data(datacite_client, published_version, mocker): assert doi_string == expected_doi assert 'doi' in published_version.metadata # Make sure metadata is copied, not modified - assert id(published_version.metadata) != id(datacite_client.generate_doi_data(published_version)[1]) + assert id(published_version.metadata) != id( + datacite_client.generate_doi_data(published_version)[1] + ) # Test Dandiset DOI doi_string, payload = datacite_client.generate_doi_data(published_version, version_doi=False) @@ -123,10 +122,12 @@ def test_create_or_update_doi_not_configured(datacite_client_unsafe, mock_reques mock_logger.warning.assert_called_once() -def test_create_or_update_doi_publish_disabled_event_publish(datacite_client_unsafe, mock_requests, mocker): +def test_create_or_update_doi_publish_disabled_event_publish( + datacite_client_unsafe, mock_requests, mocker +): """Test create_or_update_doi when DANDI_DOI_PUBLISH is False.""" mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) - mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', False) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', new=False) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Configure mock response @@ -165,7 +166,7 @@ def test_create_or_update_doi_publish_disabled_event_publish(datacite_client_uns def test_create_or_update_doi_new_doi(datacite_client_unsafe, mock_requests, mocker): """Test creating a new DOI successfully.""" mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) - mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', new=True) # Configure mock response mock_response = mocker.Mock() @@ -191,11 +192,11 @@ def test_create_or_update_doi_new_doi(datacite_client_unsafe, mock_requests, moc def test_create_or_update_doi_existing_doi(datacite_client_unsafe, mock_requests, mocker): """Test updating an existing DOI.""" mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) - mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', new=True) # Mock POST to fail with 422 (already exists) mock_post_response = mocker.Mock() - http_error = HTTPError("DOI already exists") + http_error = HTTPError('DOI already exists') http_error.response = mocker.Mock() http_error.response.status_code = 422 mock_post_response.raise_for_status.side_effect = http_error @@ -225,10 +226,10 @@ def test_create_or_update_doi_post_error(datacite_client_unsafe, mock_requests, mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock POST to fail with 400 - http_error = HTTPError("Bad request") + http_error = HTTPError('Bad request') http_error.response = mocker.Mock() http_error.response.status_code = 400 - http_error.response.text = "Bad request" + http_error.response.text = 'Bad request' mock_requests.post.side_effect = http_error payload = {'data': {'attributes': {'doi': '10.12345/test'}}} @@ -250,16 +251,16 @@ def test_create_or_update_doi_put_error(datacite_client_unsafe, mock_requests, m # Mock POST to fail with 422 (already exists) mock_post_response = mocker.Mock() - http_error = HTTPError("DOI already exists") + http_error = HTTPError('DOI already exists') http_error.response = mocker.Mock() http_error.response.status_code = 422 mock_post_response.raise_for_status.side_effect = http_error mock_requests.post.return_value = mock_post_response # Mock PUT to fail - put_error = HTTPError("Update failed") + put_error = HTTPError('Update failed') put_error.response = mocker.Mock() - put_error.response.text = "Update failed" + put_error.response.text = 'Update failed' mock_requests.put.side_effect = put_error payload = {'data': {'attributes': {'doi': '10.12345/test'}}} @@ -298,9 +299,7 @@ def test_delete_or_hide_doi_draft(datacite_client_unsafe, mock_requests, mocker) # Mock GET to return a draft DOI mock_get_response = mocker.Mock() - mock_get_response.json.return_value = { - 'data': {'attributes': {'state': 'draft'}} - } + mock_get_response.json.return_value = {'data': {'attributes': {'state': 'draft'}}} mock_get_response.raise_for_status = mocker.Mock() mock_requests.get.return_value = mock_get_response @@ -324,14 +323,12 @@ def test_delete_or_hide_doi_draft(datacite_client_unsafe, mock_requests, mocker) def test_delete_or_hide_doi_findable_publish_enabled(datacite_client_unsafe, mock_requests, mocker): """Test hiding a findable DOI when DANDI_DOI_PUBLISH is True.""" mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) - mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', True) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', new=True) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock GET to return a findable DOI mock_get_response = mocker.Mock() - mock_get_response.json.return_value = { - 'data': {'attributes': {'state': 'findable'}} - } + mock_get_response.json.return_value = {'data': {'attributes': {'state': 'findable'}}} mock_get_response.raise_for_status = mocker.Mock() mock_requests.get.return_value = mock_get_response @@ -354,17 +351,17 @@ def test_delete_or_hide_doi_findable_publish_enabled(datacite_client_unsafe, moc mock_logger.info.assert_called_once() -def test_delete_or_hide_doi_findable_publish_disabled(datacite_client_unsafe, mock_requests, mocker): +def test_delete_or_hide_doi_findable_publish_disabled( + datacite_client_unsafe, mock_requests, mocker +): """Test not hiding a findable DOI when DANDI_DOI_PUBLISH is False.""" mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=True) - mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', False) + mocker.patch.object(settings, 'DANDI_DOI_PUBLISH', new=False) mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock GET to return a findable DOI mock_get_response = mocker.Mock() - mock_get_response.json.return_value = { - 'data': {'attributes': {'state': 'findable'}} - } + mock_get_response.json.return_value = {'data': {'attributes': {'state': 'findable'}}} mock_get_response.raise_for_status = mocker.Mock() mock_requests.get.return_value = mock_get_response @@ -384,7 +381,7 @@ def test_delete_or_hide_doi_nonexistent(datacite_client_unsafe, mock_requests, m mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock GET to fail with 404 - get_error = HTTPError("Not found") + get_error = HTTPError('Not found') get_error.response = mocker.Mock() get_error.response.status_code = 404 mock_requests.get.side_effect = get_error @@ -405,7 +402,7 @@ def test_delete_or_hide_doi_get_error(datacite_client_unsafe, mock_requests, moc mock_logger = mocker.patch('dandiapi.api.datacite.logger') # Mock GET to fail with 500 - get_error = HTTPError("Server error") + get_error = HTTPError('Server error') get_error.response = mocker.Mock() get_error.response.status_code = 500 mock_requests.get.side_effect = get_error diff --git a/dandiapi/api/tests/test_doi.py b/dandiapi/api/tests/test_doi.py index 275152b4d..fd85aae60 100644 --- a/dandiapi/api/tests/test_doi.py +++ b/dandiapi/api/tests/test_doi.py @@ -22,7 +22,7 @@ def test__create_dandiset_draft_doi(draft_version, mocker): mock_generate_doi.assert_called_once_with( draft_version, version_doi=False, - event=None # Draft DOI + event=None, # Draft DOI ) mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) @@ -43,11 +43,7 @@ def test_update_draft_version_doi_no_previous_doi(draft_version, mocker): _update_draft_version_doi(draft_version) # Verify the mocks were called correctly - mock_generate_doi.assert_called_once_with( - draft_version, - version_doi=False, - event=None - ) + mock_generate_doi.assert_called_once_with(draft_version, version_doi=False, event=None) mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) # Verify the DOI was stored in the draft version @@ -71,11 +67,7 @@ def test_update_draft_version_doi_existing_doi(draft_version, mocker): _update_draft_version_doi(draft_version) # Verify the mocks were called correctly - mock_generate_doi.assert_called_once_with( - draft_version, - version_doi=False, - event=None - ) + mock_generate_doi.assert_called_once_with(draft_version, version_doi=False, event=None) mock_create_doi.assert_called_once_with({'data': {'attributes': {}}}) # Verify the DOI is still the same @@ -93,4 +85,4 @@ def test_update_draft_version_doi_published_version(draft_version, published_ver # Verify no DOI operations were performed mock_generate_doi.assert_not_called() - mock_create_doi.assert_not_called() \ No newline at end of file + mock_create_doi.assert_not_called() diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index d0059c05b..a66ae6087 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -624,9 +624,7 @@ def test_version_rest_update(api_client, user, draft_version): @pytest.mark.django_db def test_version_rest_update_embargoed(api_client, user, draft_version_factory): - draft_version = draft_version_factory( - dandiset__embargo_status=Dandiset.EmbargoStatus.EMBARGOED - ) + draft_version = draft_version_factory(dandiset__embargo_status=Dandiset.EmbargoStatus.EMBARGOED) add_dandiset_owner(draft_version.dandiset, user) api_client.force_authenticate(user=user) diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index 250cc0370..336c96a92 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -1,4 +1,5 @@ from __future__ import annotations + import logging from django.db import transaction @@ -29,7 +30,6 @@ VersionSerializer, ) - logger = logging.getLogger(__name__) @@ -138,10 +138,14 @@ def update(self, request, **kwargs): # For unpublished dandisets, update or create the draft DOI # to keep it in sync with the latest metadata if not locked_version.dandiset.embargoed: - transaction.on_commit(lambda: update_draft_version_doi_task.delay(locked_version.id)) + transaction.on_commit( + lambda: update_draft_version_doi_task.delay(locked_version.id) + ) else: - logger.debug("Skipping DOI update for embargoed Dandiset %s.", - locked_version.dandiset.identifier) + logger.debug( + 'Skipping DOI update for embargoed Dandiset %s.', + locked_version.dandiset.identifier, + ) serializer = VersionDetailSerializer(instance=locked_version) return Response(serializer.data, status=status.HTTP_200_OK) From 67d78e5658c0c1c6f3791d10a68e85a9895e3b81 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:47 -0400 Subject: [PATCH 13/17] Remove duplication --- dandiapi/api/doi.py | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/dandiapi/api/doi.py b/dandiapi/api/doi.py index 35c3ef690..9b7cd19f6 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -108,31 +108,17 @@ def _handle_publication_dois(version_id: int) -> None: version = Version.objects.get(id=version_id) draft_version = version.dandiset.draft_version - # Check if this is the first publication (no prior DOI in draft version) - # TODO(asmacdo): not true anymore, we need to check the db. - # if draft_version.dandiset.versions.exclude(version='draft').exists(): - is_first_publication = draft_version.doi is None - # Create Version DOI as Findable version_doi, version_doi_payload = datacite_client.generate_doi_data( version, version_doi=True, event='publish' ) - # Either create or update the Dandiset DOI based on whether it's the first publication - if is_first_publication: - # For first publication: generate Dandiset DOI and promote from Draft to Findable - dandiset_doi, dandiset_doi_payload = datacite_client.generate_doi_data( - version, - version_doi=False, - event='publish', # Promote to Findable on first publication - ) - else: - # For subsequent publications: update the metadata but keep as Findable - dandiset_doi, dandiset_doi_payload = datacite_client.generate_doi_data( - version, - version_doi=False, - event='publish', # Update existing Findable DOI - ) + # Update Dandiset DOI metadata and promote Findable (ok if already findable) + dandiset_doi, dandiset_doi_payload = datacite_client.generate_doi_data( + version, + version_doi=False, + event='publish', + ) # Create or update the DOIs # TODO(asmacdo): we need to try:except here, so dandiset doi doesn't block version doi @@ -142,7 +128,5 @@ def _handle_publication_dois(version_id: int) -> None: # Store the DOI values version.doi = version_doi draft_version.doi = dandiset_doi - - # Save the values draft_version.save() version.save() From 00af13b22ff8f79861a5588dd726d9aa70d84e58 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:47 -0400 Subject: [PATCH 14/17] add delete management command --- .../commands/test_dandiset_delete.py | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 dandiapi/api/management/commands/test_dandiset_delete.py diff --git a/dandiapi/api/management/commands/test_dandiset_delete.py b/dandiapi/api/management/commands/test_dandiset_delete.py new file mode 100644 index 000000000..7b590098d --- /dev/null +++ b/dandiapi/api/management/commands/test_dandiset_delete.py @@ -0,0 +1,124 @@ +import requests +from django.conf import settings +from django.contrib.auth.models import User +from django.core.management.base import BaseCommand +from rest_framework.authtoken.models import Token + +from dandiapi.api.models import Dandiset + + +class Command(BaseCommand): + help = "Test dandiset deletion via REST API to verify DOI deletion behavior" + + def add_arguments(self, parser): + parser.add_argument( + "--dandiset-id", + type=str, + required=True, + help="Dandiset ID to delete (e.g., 000354)", + ) + parser.add_argument( + "--username", + type=str, + default="admin", + help="Username to authenticate as (default: admin)", + ) + parser.add_argument( + "--base-url", + type=str, + default="http://localhost:8000", + help="Base URL for the API (default: http://localhost:8000)", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="Show what would be deleted without actually deleting", + ) + + def handle(self, *args, **options): + dandiset_id = options["dandiset_id"] + username = options["username"] + base_url = options["base_url"].rstrip("/") + dry_run = options["dry_run"] + + # Get the dandiset + try: + dandiset = Dandiset.objects.get(id=dandiset_id) + except Dandiset.DoesNotExist: + self.stderr.write(self.style.ERROR(f"Dandiset {dandiset_id} not found")) + return + + self.stdout.write(f"Found dandiset: {dandiset.id}") + self.stdout.write(f"Embargo status: {dandiset.embargo_status}") + + # Get draft version and check for DOI + draft_version = dandiset.versions.filter(version='draft').first() + if draft_version: + self.stdout.write(f"Draft version exists") + if draft_version.doi: + self.stdout.write(f"Draft version has DOI: {draft_version.doi}") + else: + self.stdout.write("Draft version has no DOI") + else: + self.stdout.write("No draft version found") + + # Check for published versions + published_versions = dandiset.versions.exclude(version='draft') + if published_versions.exists(): + self.stderr.write(self.style.ERROR( + f"Dandiset has {published_versions.count()} published versions. Cannot delete." + )) + return + + if dry_run: + self.stdout.write(self.style.WARNING("DRY RUN: Would delete dandiset via REST API")) + return + + # Get user and create/get auth token + try: + user = User.objects.get(username=username) + except User.DoesNotExist: + self.stderr.write(self.style.ERROR(f"User {username} not found")) + return + + # Get or create auth token + token, created = Token.objects.get_or_create(user=user) + if created: + self.stdout.write(f"Created new auth token for {username}") + + # Make the DELETE request + delete_url = f"{base_url}/api/dandisets/{dandiset_id}/" + headers = { + "Authorization": f"Token {token.key}", + "Content-Type": "application/json", + } + + self.stdout.write(f"Making DELETE request to: {delete_url}") + + try: + response = requests.delete(delete_url, headers=headers, timeout=30) + + if response.status_code == 204: + self.stdout.write(self.style.SUCCESS( + f"Successfully deleted dandiset {dandiset_id}" + )) + self.stdout.write("DOI deletion should have been triggered if DOI existed") + elif response.status_code == 404: + self.stderr.write(self.style.ERROR(f"Dandiset {dandiset_id} not found via API")) + elif response.status_code == 403: + self.stderr.write(self.style.ERROR(f"Permission denied - user {username} cannot delete this dandiset")) + elif response.status_code == 400: + self.stderr.write(self.style.ERROR(f"Bad request: {response.text}")) + else: + self.stderr.write(self.style.ERROR( + f"Unexpected response: {response.status_code} - {response.text}" + )) + except requests.exceptions.RequestException as e: + self.stderr.write(self.style.ERROR(f"Request failed: {e}")) + + # Verify deletion + try: + Dandiset.objects.get(id=dandiset_id) + self.stderr.write(self.style.ERROR("Dandiset still exists after deletion attempt")) + except Dandiset.DoesNotExist: + self.stdout.write(self.style.SUCCESS("Confirmed: Dandiset no longer exists in database")) From 97df5c02c72a71425834b9e0a3fce56ea868e19c Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:47 -0400 Subject: [PATCH 15/17] test and fix integration with dandi schema changes --- dandiapi/api/datacite.py | 1 + dandiapi/api/tests/test_datacite.py | 41 ++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/dandiapi/api/datacite.py b/dandiapi/api/datacite.py index befd886fd..26d23514b 100644 --- a/dandiapi/api/datacite.py +++ b/dandiapi/api/datacite.py @@ -108,6 +108,7 @@ def generate_doi_data( doi = self.format_doi(dandiset_id) # Dandiset DOI is the same as version url without version metadata['url'] = metadata['url'].rsplit('/', 1)[0] + metadata['version'] = version.dandiset.draft_version.metadata['id'] metadata['doi'] = doi diff --git a/dandiapi/api/tests/test_datacite.py b/dandiapi/api/tests/test_datacite.py index 8b1aeae80..64a6eff84 100644 --- a/dandiapi/api/tests/test_datacite.py +++ b/dandiapi/api/tests/test_datacite.py @@ -79,12 +79,15 @@ def test_format_doi(datacite_client, mocker): @pytest.mark.django_db -def test_generate_doi_data(datacite_client, published_version, mocker): +def test_generate_doi_data(datacite_client, published_version, draft_version_factory, mocker): """Test generating DOI data for a version.""" # Mock to_datacite to avoid actual validation mock_to_datacite = mocker.patch('dandischema.datacite.to_datacite') mock_to_datacite.return_value = {'data': {'attributes': {}}} + # Create a draft version for Dandiset DOI generation + draft_version_factory(dandiset=published_version.dandiset) + # Test Version DOI doi_string, payload = datacite_client.generate_doi_data(published_version, version_doi=True) dandiset_id = published_version.dandiset.identifier @@ -416,3 +419,39 @@ def test_delete_or_hide_doi_get_error(datacite_client_unsafe, mock_requests, moc assert not mock_requests.put.called assert not mock_requests.delete.called mock_logger.exception.assert_called_once() + + +@pytest.mark.django_db +def test_generate_doi_data_schema_integration( + datacite_client, published_version, draft_version_factory +): + """Test generating DOI data with real dandischema.datacite integration.""" + # This test exercises the actual dandischema.datacite.to_datacite function + # without mocking, ensuring our schema changes work with the archive + + # Create a draft version for Dandiset DOI generation + draft_version_factory(dandiset=published_version.dandiset) + + # Test Version DOI with real schema integration + doi_string, payload = datacite_client.generate_doi_data(published_version, version_doi=True) + dandiset_id = published_version.dandiset.identifier + version_id = published_version.version + expected_doi = f'{datacite_client.api_prefix}/dandi.{dandiset_id}/{version_id}' + + assert doi_string == expected_doi + assert 'doi' in published_version.metadata + + # Verify the payload structure from real to_datacite + assert 'data' in payload + assert 'attributes' in payload['data'] + assert 'doi' in payload['data']['attributes'] + assert payload['data']['attributes']['doi'] == expected_doi + + # Test Dandiset DOI with real schema integration + doi_string, payload = datacite_client.generate_doi_data(published_version, version_doi=False) + expected_doi = f'{datacite_client.api_prefix}/dandi.{dandiset_id}' + + assert doi_string == expected_doi + assert 'data' in payload + assert 'attributes' in payload['data'] + assert payload['data']['attributes']['doi'] == expected_doi From 3d41c8c893f5af4c1f99c6abeb900138fdb35a4a Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:47 -0400 Subject: [PATCH 16/17] remove dandiset delete mgmnt command, not ready, will add later --- .../commands/test_dandiset_delete.py | 124 ------------------ 1 file changed, 124 deletions(-) delete mode 100644 dandiapi/api/management/commands/test_dandiset_delete.py diff --git a/dandiapi/api/management/commands/test_dandiset_delete.py b/dandiapi/api/management/commands/test_dandiset_delete.py deleted file mode 100644 index 7b590098d..000000000 --- a/dandiapi/api/management/commands/test_dandiset_delete.py +++ /dev/null @@ -1,124 +0,0 @@ -import requests -from django.conf import settings -from django.contrib.auth.models import User -from django.core.management.base import BaseCommand -from rest_framework.authtoken.models import Token - -from dandiapi.api.models import Dandiset - - -class Command(BaseCommand): - help = "Test dandiset deletion via REST API to verify DOI deletion behavior" - - def add_arguments(self, parser): - parser.add_argument( - "--dandiset-id", - type=str, - required=True, - help="Dandiset ID to delete (e.g., 000354)", - ) - parser.add_argument( - "--username", - type=str, - default="admin", - help="Username to authenticate as (default: admin)", - ) - parser.add_argument( - "--base-url", - type=str, - default="http://localhost:8000", - help="Base URL for the API (default: http://localhost:8000)", - ) - parser.add_argument( - "--dry-run", - action="store_true", - help="Show what would be deleted without actually deleting", - ) - - def handle(self, *args, **options): - dandiset_id = options["dandiset_id"] - username = options["username"] - base_url = options["base_url"].rstrip("/") - dry_run = options["dry_run"] - - # Get the dandiset - try: - dandiset = Dandiset.objects.get(id=dandiset_id) - except Dandiset.DoesNotExist: - self.stderr.write(self.style.ERROR(f"Dandiset {dandiset_id} not found")) - return - - self.stdout.write(f"Found dandiset: {dandiset.id}") - self.stdout.write(f"Embargo status: {dandiset.embargo_status}") - - # Get draft version and check for DOI - draft_version = dandiset.versions.filter(version='draft').first() - if draft_version: - self.stdout.write(f"Draft version exists") - if draft_version.doi: - self.stdout.write(f"Draft version has DOI: {draft_version.doi}") - else: - self.stdout.write("Draft version has no DOI") - else: - self.stdout.write("No draft version found") - - # Check for published versions - published_versions = dandiset.versions.exclude(version='draft') - if published_versions.exists(): - self.stderr.write(self.style.ERROR( - f"Dandiset has {published_versions.count()} published versions. Cannot delete." - )) - return - - if dry_run: - self.stdout.write(self.style.WARNING("DRY RUN: Would delete dandiset via REST API")) - return - - # Get user and create/get auth token - try: - user = User.objects.get(username=username) - except User.DoesNotExist: - self.stderr.write(self.style.ERROR(f"User {username} not found")) - return - - # Get or create auth token - token, created = Token.objects.get_or_create(user=user) - if created: - self.stdout.write(f"Created new auth token for {username}") - - # Make the DELETE request - delete_url = f"{base_url}/api/dandisets/{dandiset_id}/" - headers = { - "Authorization": f"Token {token.key}", - "Content-Type": "application/json", - } - - self.stdout.write(f"Making DELETE request to: {delete_url}") - - try: - response = requests.delete(delete_url, headers=headers, timeout=30) - - if response.status_code == 204: - self.stdout.write(self.style.SUCCESS( - f"Successfully deleted dandiset {dandiset_id}" - )) - self.stdout.write("DOI deletion should have been triggered if DOI existed") - elif response.status_code == 404: - self.stderr.write(self.style.ERROR(f"Dandiset {dandiset_id} not found via API")) - elif response.status_code == 403: - self.stderr.write(self.style.ERROR(f"Permission denied - user {username} cannot delete this dandiset")) - elif response.status_code == 400: - self.stderr.write(self.style.ERROR(f"Bad request: {response.text}")) - else: - self.stderr.write(self.style.ERROR( - f"Unexpected response: {response.status_code} - {response.text}" - )) - except requests.exceptions.RequestException as e: - self.stderr.write(self.style.ERROR(f"Request failed: {e}")) - - # Verify deletion - try: - Dandiset.objects.get(id=dandiset_id) - self.stderr.write(self.style.ERROR("Dandiset still exists after deletion attempt")) - except Dandiset.DoesNotExist: - self.stdout.write(self.style.SUCCESS("Confirmed: Dandiset no longer exists in database")) From 988bb7b270913a63a213ad96221f7d648ab9f70d Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Thu, 4 Sep 2025 16:24:47 -0400 Subject: [PATCH 17/17] Add vue test for DOI on draft dandiset --- .../api/management/commands/inject_doi.py | 56 +++++++++++++++++++ e2e/tests/dandisetLandingPage.spec.ts | 34 +++++++++++ 2 files changed, 90 insertions(+) create mode 100644 dandiapi/api/management/commands/inject_doi.py diff --git a/dandiapi/api/management/commands/inject_doi.py b/dandiapi/api/management/commands/inject_doi.py new file mode 100644 index 000000000..1cb384838 --- /dev/null +++ b/dandiapi/api/management/commands/inject_doi.py @@ -0,0 +1,56 @@ +from __future__ import annotations + +from django.core.management.base import BaseCommand + +from dandiapi.api.models import Dandiset, Version + + +class Command(BaseCommand): + help = 'Inject a DOI into a dandiset version for testing' + + def add_arguments(self, parser): + parser.add_argument( + 'dandiset_identifier', type=str, help='Dandiset identifier (e.g., 000001)' + ) + parser.add_argument( + '--dandiset-version', type=str, default='draft', help='Version (default: draft)' + ) + parser.add_argument( + '--doi', type=str, help='DOI to inject (if not provided, will generate one)' + ) + + def handle(self, *args, **options): + dandiset_identifier = options['dandiset_identifier'] + version = options['dandiset_version'] + doi = options['doi'] + + try: + try: + dandiset = Dandiset.objects.get(id=int(dandiset_identifier)) + except (ValueError, Dandiset.DoesNotExist): + numeric_id = ( + int(dandiset_identifier.lstrip('0')) if dandiset_identifier.lstrip('0') else 0 + ) + dandiset = Dandiset.objects.get(id=numeric_id) + + version_obj = Version.objects.get(dandiset=dandiset, version=version) + + if not doi: + # TODO: this prefix needs to be updated for non-dandi deployments + doi = f'10.80507/dandi.{dandiset_identifier}' + + version_obj.metadata['doi'] = doi + version_obj.save() + + self.stdout.write( + self.style.SUCCESS( + f'Successfully injected DOI "{doi}" into {dandiset_identifier}/{version}' + ) + ) + + except Dandiset.DoesNotExist: + self.stdout.write(self.style.ERROR(f'Dandiset {dandiset_identifier} not found')) + except Version.DoesNotExist: + self.stdout.write( + self.style.ERROR(f'Version {version} not found for dandiset {dandiset_identifier}') + ) diff --git a/e2e/tests/dandisetLandingPage.spec.ts b/e2e/tests/dandisetLandingPage.spec.ts index 95314d263..eb1a897ff 100644 --- a/e2e/tests/dandisetLandingPage.spec.ts +++ b/e2e/tests/dandisetLandingPage.spec.ts @@ -1,6 +1,7 @@ import { expect, test } from "@playwright/test"; import { clientUrl, registerNewUser, LOGOUT_BUTTON_TEXT, registerDandiset } from "../utils.ts"; import { faker } from "@faker-js/faker"; +import { execSync } from "child_process"; test.describe("dandiset landing page", async () => { test("add an owner to the dandiset", async ({ page, browser }) => { @@ -40,9 +41,42 @@ test.describe("dandiset landing page", async () => { await expect(newPage.getByText(otherUserName)).toHaveCount(1); await context.close(); }); + test("navigate to an invalid dandiset URL", async ({ page }) => { await page.goto(`${clientUrl}/dandiset/1`); await page.waitForLoadState("networkidle"); await expect(page.getByText("Error: Dandiset does not exist")).toHaveCount(1); }); + + test("draft dandiset shows dandiset DOI", async ({ page }) => { + // Register a new user and create a dandiset + await registerNewUser(page); + const dandisetName = faker.lorem.words(); + const dandisetDescription = faker.lorem.sentences(); + const dandisetId = await registerDandiset(page, dandisetName, dandisetDescription); + + // Inject a DOI directly using Django management command + const testDoi = `10.80507/dandi.${dandisetId}`; + + // Execute the Django management command to inject DOI + try { + execSync(`cd .. && python manage.py inject_doi ${dandisetId} --dandiset-version=draft --doi="${testDoi}"`, { + stdio: 'inherit', + timeout: 10000 + }); + } catch (error) { + console.error('Failed to inject DOI:', error); + } + + // Refresh the page to see the updated DOI + await page.reload(); + await page.waitForLoadState("networkidle"); + + // The draft version should show the injected Dandiset DOI + await expect(page.getByText(testDoi)).toHaveCount(1); + + // Should not show a version DOI (since it's a draft) + const versionDoiPattern = new RegExp(`10\\.(48324|80507)/dandi\\.${dandisetId}/`); + await expect(page.getByText(versionDoiPattern)).toHaveCount(0); + }); });