diff --git a/dandi/cli/cmd_service_scripts.py b/dandi/cli/cmd_service_scripts.py index 8e55645c0..9e049d56e 100644 --- a/dandi/cli/cmd_service_scripts.py +++ b/dandi/cli/cmd_service_scripts.py @@ -7,6 +7,7 @@ import json import os from pathlib import PurePosixPath +import re from textwrap import indent from typing import Any, TypeVar import urllib.parse @@ -16,7 +17,7 @@ from dandischema.consts import DANDI_SCHEMA_VERSION from packaging.version import Version from requests.auth import HTTPBasicAuth -from requests.exceptions import HTTPError +from requests.exceptions import HTTPError, RequestException from dandi.consts import known_instances @@ -24,7 +25,7 @@ from .. import __version__, lgr from ..dandiapi import DandiAPIClient, RemoteBlobAsset, RESTFullAPIClient from ..dandiarchive import parse_dandi_url -from ..exceptions import NotFoundError +from ..exceptions import HTTP404Error, NotFoundError from ..utils import yaml_dump T = TypeVar("T") @@ -35,6 +36,112 @@ "https://api.datacite.org/dois": "https://doi.datacite.org/dois", } +#: Base URL of the DOI resolver used to look up citation metadata +DOI_RESOLVER_URL = "https://doi.org/" + +#: Content type requested from the DOI resolver for citation metadata +DOI_CSL_ACCEPT = "application/vnd.citationstyles.csl+json; charset=utf-8" + +#: Matches a bare DOI, e.g. ``10.48324/dandi.001827/0.260505.1322`` +DOI_REGEX = re.compile(r"10\.\d{4,9}/\S+") + +#: Prefixes a DOI may be spelled with, in the order they are stripped +DOI_PREFIX_REGEXES = (r"doi:", r"(?:https?://)?(?:dx\.)?doi\.org/") + + +def normalize_doi(doi: str) -> str: + """Reduce a DOI given in any of its usual spellings to the bare DOI. + + A bare DOI (``10.48324/dandi.001827/0.260505.1322``), a ``doi:`` URI, and a + resolver URL (``https://doi.org/...``, ``http://dx.doi.org/...``) are all + accepted and reduced to the bare form. + + Parameters + ---------- + doi : str + The DOI as given by the user + + Returns + ------- + str + The bare DOI + + Raises + ------ + ValueError + If `doi` is not a syntactically valid DOI in any accepted spelling + """ + value = doi.strip() + for prefix_regex in DOI_PREFIX_REGEXES: + if m := re.match(prefix_regex, value, flags=re.I): + value = value[m.end() :].strip() + break + if not DOI_REGEX.fullmatch(value): + raise ValueError( + f"{doi!r} does not look like a DOI. Expected something like " + "'10.48324/dandi.001827/0.260505.1322', optionally prefixed with " + "'doi:' or 'https://doi.org/'." + ) + return value + + +def fetch_doi_citation_metadata(doi: str) -> dict[str, Any]: + """Fetch the CSL JSON citation metadata for a bare `doi` from doi.org. + + Parameters + ---------- + doi : str + A bare DOI, as returned by `normalize_doi()` + + Returns + ------- + dict + The parsed CSL JSON record + + Raises + ------ + click.ClickException + If the DOI cannot be resolved, or if the resolver answers with + something other than a CSL JSON object. The exception message + describes what went wrong, so that the user is not left with a bare + `json.JSONDecodeError` traceback. + """ + url = f"{DOI_RESOLVER_URL}{doi}" + with RESTFullAPIClient( + DOI_RESOLVER_URL, headers={"Accept": DOI_CSL_ACCEPT} + ) as doiclient: + try: + r = doiclient.get(doi, json_resp=False) + except HTTP404Error: + raise click.ClickException( + f"DOI {doi} is not registered: {url} returned 404. Check the " + "DOI for typos and make sure it has already been published." + ) + except HTTPError as e: + status = e.response.status_code if e.response is not None else "?" + raise click.ClickException( + f"Failed to look up DOI {doi}: {url} returned HTTP {status}." + ) + except RequestException as e: + raise click.ClickException(f"Failed to look up DOI {doi} at {url}: {e}") + content_type = r.headers.get("Content-Type", "") + try: + doidata = r.json() + except ValueError: + raise click.ClickException( + f"DOI {doi} did not resolve to citation metadata: {url} answered " + f"with {content_type!r} instead of CSL JSON (final URL: {r.url}). " + "This usually means the DOI's registration agency does not serve " + "citation metadata for it, and doi.org fell back to redirecting " + "to the landing page." + ) + if not isinstance(doidata, dict): + raise click.ClickException( + f"DOI {doi} resolved to a JSON {type(doidata).__name__} rather than " + f"the expected CSL JSON object (final URL: {r.url})." + ) + return doidata + @click.group() def service_scripts() -> None: @@ -247,7 +354,15 @@ def update_dandiset_from_doi( """ Update the metadata for the draft version of a Dandiset with information from a given DOI record. + + DOI may be given bare (``10.48324/dandi.001827/0.260505.1322``), as a + ``doi:`` URI, or as a resolver URL (``https://doi.org/...``). """ + try: + doi = normalize_doi(doi) + except ValueError as e: + raise click.UsageError(str(e)) + known_instance_names = [k.upper() for k in known_instances.keys()] # Strip instance name prefix from dandiset ID, if present @@ -258,15 +373,10 @@ def update_dandiset_from_doi( break start_time = datetime.now().astimezone() + # Resolve the DOI before talking to the archive, so that a bad DOI fails + # fast and without requiring credentials + doidata = fetch_doi_citation_metadata(doi) with DandiAPIClient.for_dandi_instance(dandi_instance, authenticate=True) as client: - with RESTFullAPIClient( - "https://doi.org/", - headers={ - "Accept": "application/vnd.citationstyles.csl+json; charset=utf-8" - }, - ) as doiclient: - doidata = doiclient.get(doi) - d = client.get_dandiset(dandiset, "draft", lazy=False) original_metadata = d.get_raw_metadata() new_metadata = deepcopy(original_metadata) diff --git a/dandi/cli/tests/test_service_scripts.py b/dandi/cli/tests/test_service_scripts.py index 10a0a8713..18df6c8ed 100644 --- a/dandi/cli/tests/test_service_scripts.py +++ b/dandi/cli/tests/test_service_scripts.py @@ -8,14 +8,21 @@ import sys import anys +import click from click.testing import CliRunner from dandischema.models import ID_PATTERN import pytest +import responses from dandi import __version__ from dandi.tests.fixtures import SampleDandiset -from ..cmd_service_scripts import service_scripts +from ..cmd_service_scripts import ( + DOI_CSL_ACCEPT, + fetch_doi_citation_metadata, + normalize_doi, + service_scripts, +) DATA_DIR = Path(__file__).with_name("data") @@ -142,3 +149,113 @@ def test_update_dandiset_from_doi( else: expected["citation"] = citation assert metadata == expected + + +@pytest.mark.ai_generated +@pytest.mark.parametrize( + "given", + [ + "10.48324/dandi.001827/0.260505.1322", + " 10.48324/dandi.001827/0.260505.1322 ", + "doi:10.48324/dandi.001827/0.260505.1322", + "DOI:10.48324/dandi.001827/0.260505.1322", + "https://doi.org/10.48324/dandi.001827/0.260505.1322", + "http://doi.org/10.48324/dandi.001827/0.260505.1322", + "https://dx.doi.org/10.48324/dandi.001827/0.260505.1322", + "doi.org/10.48324/dandi.001827/0.260505.1322", + ], +) +def test_normalize_doi(given: str) -> None: + assert normalize_doi(given) == "10.48324/dandi.001827/0.260505.1322" + + +@pytest.mark.ai_generated +@pytest.mark.parametrize( + "given", + [ + "", + "not a doi", + "https://doi.org/", + "https://example.com/10.1234/foo", + "10.1/too-short-prefix", + ], +) +def test_normalize_doi_rejects_non_doi(given: str) -> None: + with pytest.raises(ValueError, match="does not look like a DOI"): + normalize_doi(given) + + +@pytest.mark.ai_generated +@responses.activate +def test_fetch_doi_citation_metadata_non_json() -> None: + # doi.org falls back to redirecting to the landing page when the + # registration agency cannot serve CSL JSON, so we get HTML with a 200. + # See https://github.com/dandi/dandi-cli/issues/1855 + doi = "10.48324/dandi.001827/0.260505.1322" + responses.add( + responses.GET, + f"https://doi.org/{doi}", + body="Dandiset 001827", + status=200, + content_type="text/html; charset=utf-8", + ) + with pytest.raises(click.ClickException) as excinfo: + fetch_doi_citation_metadata(doi) + message = str(excinfo.value) + assert doi in message + assert "did not resolve to citation metadata" in message + assert "text/html" in message + + +@pytest.mark.ai_generated +@responses.activate +def test_fetch_doi_citation_metadata_not_found() -> None: + doi = "10.48324/dandi.999999/0.000000.0000" + responses.add( + responses.GET, + f"https://doi.org/{doi}", + body="DOI Not Found", + status=404, + content_type="text/plain", + ) + with pytest.raises(click.ClickException) as excinfo: + fetch_doi_citation_metadata(doi) + assert "is not registered" in str(excinfo.value) + + +@pytest.mark.ai_generated +@responses.activate +def test_fetch_doi_citation_metadata_ok() -> None: + doi = "10.1101/2020.01.17.909838" + responses.add( + responses.GET, + f"https://doi.org/{doi}", + json={"title": "A paper", "author": []}, + status=200, + ) + assert fetch_doi_citation_metadata(doi) == {"title": "A paper", "author": []} + + +@pytest.mark.ai_generated +@responses.activate +def test_fetch_doi_citation_metadata_requests_csl_json() -> None: + # The CSL Accept header used to be set on the session only, where + # `RESTFullAPIClient.request()` overrode it with "application/json" while + # building a JSON request. See https://github.com/dandi/dandi-cli/issues/1855 + doi = "10.1101/2020.01.17.909838" + responses.add( + responses.GET, f"https://doi.org/{doi}", json={"title": "A paper"}, status=200 + ) + fetch_doi_citation_metadata(doi) + assert responses.calls[0].request.headers["Accept"] == DOI_CSL_ACCEPT + + +@pytest.mark.ai_generated +def test_update_dandiset_from_doi_bad_doi() -> None: + r = CliRunner().invoke( + service_scripts, + ["update-dandiset-from-doi", "-d", "000001", "not-a-doi"], + ) + assert r.exit_code == 2 + assert "does not look like a DOI" in r.output + assert "Traceback" not in r.output