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..26d23514b --- /dev/null +++ b/dandiapi/api/datacite.py @@ -0,0 +1,261 @@ +""" +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 +from functools import wraps +import logging +import sys +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__) + + +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.""" + + 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['version'] = version.dandiset.draft_version.metadata['id'] + + metadata['doi'] = doi + + # Generate the datacite payload with the appropriate event + datacite_payload = to_datacite(metadata, event=event) + + 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. + + 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 # noqa: TRY300 + 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() + return doi # noqa: TRY300 + 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 + + @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. + + 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..9b7cd19f6 100644 --- a/dandiapi/api/doi.py +++ b/dandiapi/api/doi.py @@ -1,86 +1,132 @@ +""" +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 +# Singleton instance +datacite_client = DataCiteClient() + + +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) + + +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 = datacite_client.generate_doi_data( + draft_version, + version_doi=False, + event=None, # Draft DOI + ) + + # Create the DOI + datacite_client.create_or_update_doi(dandiset_doi_payload) + + # Store the DOI in the draft version + draft_version.doi = dandiset_doi + 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 = 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 + 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: + draft_version.doi = dandiset_doi + draft_version.save() + logger.info('Created new Draft DOI %s', dandiset_doi) else: - logger.debug('Skipping DOI deletion for %s since not configured', doi) + 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. + + 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 + + # Create Version DOI as Findable + version_doi, version_doi_payload = datacite_client.generate_doi_data( + version, version_doi=True, event='publish' + ) + + # 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 + 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 + draft_version.doi = dandiset_doi + draft_version.save() + version.save() 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/dandiapi/api/services/dandiset/__init__.py b/dandiapi/api/services/dandiset/__init__.py index 274b892a3..5872c9d51 100644 --- a/dandiapi/api/services/dandiset/__init__.py +++ b/dandiapi/api/services/dandiset/__init__.py @@ -2,6 +2,7 @@ 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 @@ -14,6 +15,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 def create_dandiset( @@ -56,6 +58,9 @@ def create_dandiset( dandiset=dandiset, user=user, metadata=draft_version.metadata, embargoed=embargo ) + if not embargo: + transaction.on_commit(lambda: create_dandiset_draft_doi_task.delay(draft_version.id)) + return dandiset, draft_version @@ -76,6 +81,12 @@ def delete_dandiset(*, user, dandiset: Dandiset) -> None: # 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 deleted. + draft_version = dandiset.versions.filter(version='draft').first() + if draft_version and draft_version.doi is not None: + # 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() diff --git a/dandiapi/api/services/embargo/__init__.py b/dandiapi/api/services/embargo/__init__.py index 494640c60..e155affb3 100644 --- a/dandiapi/api/services/embargo/__init__.py +++ b/dandiapi/api/services/embargo/__init__.py @@ -14,7 +14,7 @@ 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, @@ -80,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/services/publish/__init__.py b/dandiapi/api/services/publish/__init__.py index 59042d9a5..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 @@ -22,7 +21,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 @@ -187,13 +186,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)) - - 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)) + 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 300e8569e..6b862427b 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 @@ -106,3 +106,34 @@ 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) + + +@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) + + +@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 1d52a09de..26ef521d5 100644 --- a/dandiapi/api/tests/test_audit.py +++ b/dandiapi/api/tests/test_audit.py @@ -275,7 +275,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 +289,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_task') # Publish the Dandiset. api_client.force_authenticate(user=user) @@ -301,6 +302,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..c66edf8b4 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -751,27 +751,42 @@ 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 + mock_delete_doi.assert_not_called() @pytest.mark.django_db @@ -781,11 +796,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 +813,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 diff --git a/dandiapi/api/tests/test_datacite.py b/dandiapi/api/tests/test_datacite.py new file mode 100644 index 000000000..64a6eff84 --- /dev/null +++ b/dandiapi/api/tests/test_datacite.py @@ -0,0 +1,457 @@ +from __future__ import annotations + +from django.conf import settings +import pytest +from requests.exceptions import HTTPError + +from dandiapi.api.datacite import DataCiteClient + + +@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() + + +@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 + # 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, 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 + 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_unsafe, mock_requests, mocker): + """Test create_or_update_doi when API is not configured.""" + 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_unsafe.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_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', new=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_unsafe.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_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', new=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_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_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 + assert not mock_requests.delete.called + assert result == '10.12345/test' + + +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', new=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_unsafe.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_unsafe, mock_requests, mocker): + """Test error handling when POST fails with non-422 error.""" + 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 + 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_unsafe.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_unsafe, mock_requests, mocker): + """Test error handling when PUT fails.""" + 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) + 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_unsafe.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_unsafe, mock_requests, mocker): + """Test delete_or_hide_doi when API is not configured.""" + mocker.patch.object(datacite_client_unsafe, 'is_configured', return_value=False) + mock_logger = mocker.patch('dandiapi.api.datacite.logger') + + datacite_client_unsafe.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_unsafe, mock_requests, mocker): + """Test deleting a draft DOI.""" + 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 + 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_unsafe.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_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', 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.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_unsafe.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_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', 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.raise_for_status = mocker.Mock() + mock_requests.get.return_value = mock_get_response + + datacite_client_unsafe.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_unsafe, mock_requests, mocker): + """Test handling a nonexistent DOI.""" + 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 + 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_unsafe.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_unsafe, mock_requests, mocker): + """Test error handling when GET fails with non-404 error.""" + 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 + 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_unsafe.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() + + +@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 diff --git a/dandiapi/api/tests/test_doi.py b/dandiapi/api/tests/test_doi.py new file mode 100644 index 000000000..fd85aae60 --- /dev/null +++ b/dandiapi/api/tests/test_doi.py @@ -0,0 +1,88 @@ +from __future__ import annotations + +import pytest + +from dandiapi.api.doi import _create_dandiset_draft_doi, _update_draft_version_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.datacite_client.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + 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 + _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.datacite_client.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + 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) + + # 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.datacite_client.generate_doi_data') + mock_generate_doi.return_value = ('10.48324/dandi.000123', {'data': {'attributes': {}}}) + + 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) + + # 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.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) + + # 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_tasks.py b/dandiapi/api/tests/test_tasks.py index 6e818598b..b566e5b8d 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_task') # 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_version.py b/dandiapi/api/tests/test_version.py index 66b508012..a66ae6087 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -622,6 +622,27 @@ def test_version_rest_update(api_client, user, draft_version): assert draft_version.status == Version.Status.PENDING +@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) + add_dandiset_owner(draft_version.dandiset, user) + api_client.force_authenticate(user=user) + + 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 + + @pytest.mark.django_db def test_version_rest_update_unembargo_in_progress(api_client, user, draft_version_factory): draft_version = draft_version_factory( @@ -976,14 +997,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 diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index bcd1f9d4d..336c96a92 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -1,5 +1,7 @@ from __future__ import annotations +import logging + from django.db import transaction from django_filters import rest_framework as filters from drf_yasg.utils import no_body, swagger_auto_schema @@ -19,7 +21,7 @@ require_dandiset_owner_or_403, ) from dandiapi.api.services.publish import publish_dandiset -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 ( @@ -28,6 +30,8 @@ VersionSerializer, ) +logger = logging.getLogger(__name__) + class VersionFilter(filters.FilterSet): order = filters.OrderingFilter(fields=['created']) @@ -131,6 +135,18 @@ 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: + 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, + ) + serializer = VersionDetailSerializer(instance=locked_version) return Response(serializer.data, status=status.HTTP_200_OK) 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); + }); }); 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" />