Give a descriptive error when a DOI cannot be resolved to citation metadata

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,15 +17,15 @@
 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
 
 from .base import ChoiceList, instance_option, map_to_click_exceptions
 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", "<unset>")
+        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)

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="<!DOCTYPE html><html><body>Dandiset 001827</body></html>",
+        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