From 3248df3361181ef8b47e2e3e612d38be68671fad Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Wed, 2 Sep 2026 04:33:15 +0200 Subject: [PATCH 1/3] Add mirror-publish subcommand to publish an existing Dataverse draft Split the create-draft and publish steps of the Zenodo-to-Dataverse mirror into two deliberate actions. The new fwl-io mirror-publish subcommand and publish_existing_dataverse_draft() publish an existing draft by its persistent id and never create a dataset, so publishing is a separate, auditable step run after a draft has been reviewed. The persistent id must be of the form doi:/; a value that lacks the prefix, the slash, or either part is rejected locally with ValueError before any network call. The mirror-publish.yaml workflow targets the default Dataverse URL and binds every dispatch input to an environment variable, so a crafted input cannot redirect the API token or inject a shell command. Covered by unit tests for the never-creates-a-dataset property, the persistent-id validation, and the CLI wiring; docs/Reference/cli.md and docs/How-to/mirror_dataset.md document the subcommand. --- .github/workflows/mirror-publish.yaml | 35 +++++ docs/How-to/mirror_dataset.md | 4 + docs/Reference/cli.md | 11 +- src/fwl_io/cli.py | 35 ++++- src/fwl_io/mirror.py | 53 ++++++- tests/test_mirror.py | 198 ++++++++++++++++++++++++++ 6 files changed, 332 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/mirror-publish.yaml diff --git a/.github/workflows/mirror-publish.yaml b/.github/workflows/mirror-publish.yaml new file mode 100644 index 0000000..fcf75d3 --- /dev/null +++ b/.github/workflows/mirror-publish.yaml @@ -0,0 +1,35 @@ +name: Publish an existing Dataverse draft + +on: + workflow_dispatch: + inputs: + persistent_id: + description: "Persistent id (DOI) of the existing draft to publish, e.g. doi:10.34894/EXAMPLE" + required: true + version_type: + description: "Dataverse publish version bump" + required: true + default: "major" + +jobs: + mirror-publish: + runs-on: ubuntu-latest + environment: dataverse + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Install fwl-io + run: pip install -e . + - name: Publish the draft + env: + DATAVERSE_TOKEN: ${{ secrets.DATAVERSE_TOKEN }} + # Bind the user inputs to environment variables rather than + # interpolating them into the shell script, so a crafted persistent id + # cannot inject commands into a job that holds the token. + PERSISTENT_ID: ${{ inputs.persistent_id }} + VERSION_TYPE: ${{ inputs.version_type }} + run: | + fwl-io mirror-publish "${PERSISTENT_ID}" \ + --version-type "${VERSION_TYPE}" diff --git a/docs/How-to/mirror_dataset.md b/docs/How-to/mirror_dataset.md index 290510e..6d777e1 100644 --- a/docs/How-to/mirror_dataset.md +++ b/docs/How-to/mirror_dataset.md @@ -22,6 +22,10 @@ Mirroring runs from the **Mirror a Zenodo deposit to Dataverse** GitHub Actions Commit that change in a pull request, like any other data change. +## Publishing a reviewed draft + +A draft created with **publish** unchecked stays private until it is published. Run the **Publish an existing Dataverse draft** GitHub Actions workflow, supplying the draft's persistent id (the DOI printed by the mirror run, with a `doi:` prefix, for example `doi:10.34894/XXXXXX`) and the same Dataverse URL. It only publishes; it never creates a dataset, so it cannot mint a duplicate one. Add the DOI to the manifest as in step 3 above once it is published. + ## What the mirror does For the given Zenodo version DOI, the mirror downloads and checksum-verifies every file, creates a Dataverse dataset whose title, authors, and description come from the Zenodo record (with a note recording the source DOI), uploads the files byte-identically with tabular ingest disabled, and publishes the dataset unless asked not to. A concept DOI is rejected, so the mirror always tracks a specific pinned deposit. diff --git a/docs/Reference/cli.md b/docs/Reference/cli.md index e5ed135..4f8c5d1 100644 --- a/docs/Reference/cli.md +++ b/docs/Reference/cli.md @@ -1,6 +1,6 @@ # CLI reference -The `fwl-io` command has six subcommands. Failures are reported as concise messages on stderr (never a traceback) and exit with status 1; success exits 0. `sync` and `fetch` aggregate per-dataset failures into a multi-line report, and a download failure lists every mirror attempt. +The `fwl-io` command has seven subcommands. Failures are reported as concise messages on stderr (never a traceback) and exit with status 1; success exits 0. `sync` and `fetch` aggregate per-dataset failures into a multi-line report, and a download failure lists every mirror attempt. ## fwl-io sync @@ -68,6 +68,15 @@ DATAVERSE_TOKEN=... fwl-io mirror --collection \ Mirrors a pinned Zenodo deposit to a Dataverse collection: it downloads and checksum-verifies the deposit's files, creates a matching Dataverse dataset with citation metadata taken from the Zenodo record, uploads the files byte-identically (tabular ingest disabled), and by default publishes the dataset, then prints the Dataverse DOI to add to the consuming manifest. The API token is read from the `DATAVERSE_TOKEN` environment variable, never a command-line argument. A contact email (`--contact-email`) is required to create a dataset; only `--dry-run`, which makes no Dataverse writes, is exempt. `--subject` is validated by the server when the dataset is created, so a value outside the target installation's citation vocabulary is rejected then. `--dry-run` performs the download and metadata mapping only, making no Dataverse changes; `--no-publish` leaves the created dataset as a private draft. See [Mirror a deposit to Dataverse](../How-to/mirror_dataset.md). +## fwl-io mirror-publish + +```bash +DATAVERSE_TOKEN=... fwl-io mirror-publish \ + [--dataverse-url URL] [--version-type {major,minor}] +``` + +Publishes an existing Dataverse draft by its persistent id: it never creates a dataset, so it is the second step of a create-draft-then-publish workflow, run once a draft created by `fwl-io mirror --no-publish` has been reviewed. `` must be of the form `doi:/`, for example `doi:10.34894/EXAMPLE`. The API token is read from the `DATAVERSE_TOKEN` environment variable, never a command-line argument. `--version-type` is `major` by default and accepts only `major` or `minor`. Fails clearly if the dataset is already published or the persistent id does not resolve to a draft. See [Mirror a deposit to Dataverse](../How-to/mirror_dataset.md). + ## fwl-io --version Prints the installed version. diff --git a/src/fwl_io/cli.py b/src/fwl_io/cli.py index 5c4cd46..ac20450 100644 --- a/src/fwl_io/cli.py +++ b/src/fwl_io/cli.py @@ -1,4 +1,5 @@ -"""Command-line interface: ``fwl-io sync | list | fetch | check | relocate | mirror``. +"""Command-line interface for ``fwl-io``: sync, list, fetch, check, relocate, mirror, +mirror-publish. Failures from the package's own error types exit with status 1 and a one-line message on stderr instead of a traceback. @@ -99,6 +100,23 @@ def _cmd_mirror(args: argparse.Namespace) -> int: return 0 +def _cmd_mirror_publish(args: argparse.Namespace) -> int: + from fwl_io.mirror import publish_existing_dataverse_draft + + token = os.environ.get('DATAVERSE_TOKEN', '') + if not token: + print('fwl-io: set DATAVERSE_TOKEN to publish', file=sys.stderr) + return 1 + publish_existing_dataverse_draft( + args.persistent_id, + dataverse_url=args.dataverse_url, + token=token, + version_type=args.version_type, + ) + print(f'published {args.persistent_id}') + return 0 + + def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser( prog='fwl-io', @@ -160,6 +178,21 @@ def main(argv: list[str] | None = None) -> int: ) p_mirror.set_defaults(func=_cmd_mirror) + p_mirror_publish = sub.add_parser( + 'mirror-publish', help='publish an existing Dataverse draft (never creates a dataset)' + ) + p_mirror_publish.add_argument('persistent_id', help='persistent id (DOI) of the draft') + p_mirror_publish.add_argument( + '--dataverse-url', default='https://dataverse.nl', help='Dataverse base URL' + ) + p_mirror_publish.add_argument( + '--version-type', + default='major', + choices=['major', 'minor'], + help='Dataverse publish version bump', + ) + p_mirror_publish.set_defaults(func=_cmd_mirror_publish) + args = parser.parse_args(argv) try: return args.func(args) diff --git a/src/fwl_io/mirror.py b/src/fwl_io/mirror.py index 811e3b0..29ea8b3 100644 --- a/src/fwl_io/mirror.py +++ b/src/fwl_io/mirror.py @@ -1,11 +1,14 @@ """Mirror a pinned Zenodo deposit to a Dataverse.nl collection. Zenodo is the primary source of every dataset; Dataverse is a download -mirror used as the second link in the fetch fallback chain. This module +mirror used as the second link in the fetch fallback chain. :func:`mirror_to_dataverse` takes a Zenodo version DOI, downloads and checksum-verifies its files, then creates a matching Dataverse dataset, uploads the files byte-identically, and (optionally) publishes it, printing the Dataverse DOI to add to the -consuming manifest. +consuming manifest. Called with ``publish=False``, it leaves the created +dataset as a private draft instead; :func:`publish_existing_dataverse_draft` +is the second step of that workflow, publishing an existing draft by its +persistent id without ever creating a dataset. The Dataverse writes go through the native API (https://guides.dataverse.org/en/latest/api/native-api.html): @@ -410,3 +413,49 @@ def mirror_to_dataverse( ) raise return persistent_id + + +def publish_existing_dataverse_draft( + persistent_id: str, + *, + dataverse_url: str, + token: str, + version_type: str = 'major', +) -> None: + """Publish an existing Dataverse draft dataset by its persistent id. + + This never calls :meth:`DataverseClient.create_dataset`, so it cannot + mint a duplicate dataset: it is the second step of a create-draft -> + review -> publish workflow, run once the draft created by + :func:`mirror_to_dataverse` (with ``publish=False``) has been reviewed. + + Parameters + ---------- + persistent_id : str + Persistent id (DOI) of the existing draft, for example + ``'doi:10.34894/EXAMPLE'``. + dataverse_url : str + Base URL of the Dataverse installation (for example + ``https://dataverse.nl``). + token : str + Dataverse API token. + version_type : str + Dataverse publish version bump: ``'major'`` or ``'minor'``. + + Raises + ------ + ValueError + If ``persistent_id`` is not of the form ``'doi:/'``. + DataverseError + If the publish request fails: for example the dataset is already + published, does not exist, or the server returns a non-2xx status. + """ + prefix, sep, suffix = persistent_id.removeprefix('doi:').partition('/') + if not persistent_id.startswith('doi:') or not sep or not prefix or not suffix: + raise ValueError( + f'{persistent_id!r} is not a Dataverse persistent id of the form ' + "'doi:/'" + ) + client = DataverseClient(dataverse_url, token) + client.publish(persistent_id, version_type=version_type) + log.info('published %s', persistent_id) diff --git a/tests/test_mirror.py b/tests/test_mirror.py index f2c71c0..7a62c93 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -21,6 +21,7 @@ DataverseClient, DataverseError, mirror_to_dataverse, + publish_existing_dataverse_draft, zenodo_record_to_citation, ) @@ -498,6 +499,116 @@ def test_publish_raises_with_the_status_and_body_on_a_non_json_success_body(): requests.request = orig +@pytest.mark.unit +def test_publish_existing_draft_never_creates_a_dataset(monkeypatch): + """The publish-only wrapper calls publish() and never create_dataset().""" + + def _forbidden(*args, **kwargs): + raise AssertionError('publish_existing_dataverse_draft must never create a dataset') + + monkeypatch.setattr(DataverseClient, 'create_dataset', _forbidden) + monkeypatch.setattr(DataverseClient, 'publish', lambda self, pid, **k: None) + + publish_existing_dataverse_draft( + 'doi:10.34894/DEMO01', dataverse_url='http://unused', token='tok' + ) + + +@pytest.mark.unit +def test_publish_existing_draft_forwards_the_persistent_id_and_version_type(monkeypatch): + """The wrapper forwards persistent_id and version_type to DataverseClient.publish.""" + captured = {} + monkeypatch.setattr( + DataverseClient, + 'publish', + lambda self, pid, **k: captured.update(persistent_id=pid, **k), + ) + + publish_existing_dataverse_draft( + 'doi:10.34894/DEMO01', + dataverse_url='http://unused', + token='tok', + version_type='minor', + ) + assert captured == {'persistent_id': 'doi:10.34894/DEMO01', 'version_type': 'minor'} + + +@pytest.mark.unit +def test_publish_existing_draft_raises_clearly_on_an_already_published_dataset(): + """Publishing an already-published (or nonexistent) persistentId errors clearly. + + Dataverse answers a re-publish or an unknown persistentId with a non-2xx + status; the wrapper must let DataverseError propagate with that status and + body rather than silently no-op. + """ + import requests + + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response( + 403, b'{"status": "ERROR", "message": "Dataset already published"}' + ) + try: + with pytest.raises(DataverseError, match='already published') as exc_info: + publish_existing_dataverse_draft( + 'doi:10.34894/DEMO01', dataverse_url='http://unused', token='tok' + ) + assert '403' in str(exc_info.value) + finally: + requests.request = orig + + +@pytest.mark.unit +def test_publish_existing_draft_raises_with_the_status_and_body_on_a_non_json_success_body(): + """A 2xx publish response with a non-JSON body raises, naming the status and body.""" + import requests + + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'this is not json') + try: + with pytest.raises(DataverseError, match='this is not json') as exc_info: + publish_existing_dataverse_draft( + 'doi:10.34894/DEMO01', dataverse_url='http://unused', token='tok' + ) + assert '200' in str(exc_info.value) + finally: + requests.request = orig + + +@pytest.mark.unit +@pytest.mark.parametrize( + 'persistent_id', ['', '10.34894/DEMO01', 'doi:', 'doi:noSlashHere', 'doi:10.34894/'] +) +def test_publish_existing_draft_rejects_a_malformed_persistent_id(persistent_id): + """A persistent id that isn't 'doi:/' is rejected locally. + + No request is made: DataverseClient.publish is never reached, so this + raises ValueError even with an unreachable dataverse_url. + """ + with pytest.raises(ValueError, match='Dataverse persistent id'): + publish_existing_dataverse_draft(persistent_id, dataverse_url='http://unused', token='tok') + + +@pytest.mark.unit +def test_publish_existing_draft_raises_clearly_on_a_nonexistent_persistent_id(): + """Publishing a persistentId Dataverse doesn't recognize errors clearly.""" + import requests + + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response( + 404, + b'{"status": "ERROR", "message": "Dataset with Persistent ID doi:10.34894/' + b'NOPE could not be found."}', + ) + try: + with pytest.raises(DataverseError, match='could not be found') as exc_info: + publish_existing_dataverse_draft( + 'doi:10.34894/NOPE', dataverse_url='http://unused', token='tok' + ) + assert '404' in str(exc_info.value) + finally: + requests.request = orig + + @pytest.mark.unit def test_delete_draft_accepts_an_empty_success_body(): """delete_draft (the rollback path) does not raise when Dataverse returns an empty 2xx.""" @@ -996,3 +1107,90 @@ def test_cli_mirror_prints_manifest_ready_dataverse_doi(monkeypatch, capsys): # The manifest field takes a bare DOI, so the doi: prefix is stripped. assert 'dataverse = "10.34894/DEMO01"' in out assert 'doi:10.34894/DEMO01' not in out.split('add this to the manifest')[1] + + +@pytest.mark.unit +def test_cli_mirror_publish_requires_token(monkeypatch, capsys): + """mirror-publish refuses to run without a token, and never calls the wrapper.""" + import fwl_io.mirror as mirror_mod + from fwl_io.cli import main + + called = [] + monkeypatch.setattr( + mirror_mod, 'publish_existing_dataverse_draft', lambda *a, **k: called.append(1) + ) + monkeypatch.delenv('DATAVERSE_TOKEN', raising=False) + + rc = main(['mirror-publish', 'doi:10.34894/DEMO01']) + assert rc == 1 + assert 'DATAVERSE_TOKEN' in capsys.readouterr().err + assert called == [] + + +@pytest.mark.unit +def test_cli_mirror_publish_forwards_the_persistent_id_url_and_version_type(monkeypatch): + """mirror-publish forwards the persistent id and its flags, unmodified.""" + import fwl_io.mirror as mirror_mod + from fwl_io.cli import main + + captured = {} + monkeypatch.setattr( + mirror_mod, + 'publish_existing_dataverse_draft', + lambda pid, **k: captured.update(persistent_id=pid, **k), + ) + monkeypatch.setenv('DATAVERSE_TOKEN', 'tok') + + main( + [ + 'mirror-publish', + 'doi:10.34894/DEMO01', + '--dataverse-url', + 'https://demo.dataverse.org', + '--version-type', + 'minor', + ] + ) + assert captured['persistent_id'] == 'doi:10.34894/DEMO01' + assert captured['dataverse_url'] == 'https://demo.dataverse.org' + assert captured['version_type'] == 'minor' + + +@pytest.mark.unit +def test_cli_mirror_publish_prints_the_published_id_on_success(monkeypatch, capsys): + """On success mirror-publish reports the persistent id it published.""" + import fwl_io.mirror as mirror_mod + from fwl_io.cli import main + + monkeypatch.setattr(mirror_mod, 'publish_existing_dataverse_draft', lambda *a, **k: None) + monkeypatch.setenv('DATAVERSE_TOKEN', 'tok') + + rc = main(['mirror-publish', 'doi:10.34894/DEMO01']) + assert rc == 0 + assert 'published doi:10.34894/DEMO01' in capsys.readouterr().out + + +@pytest.mark.unit +def test_cli_mirror_publish_surfaces_a_dataverse_error(monkeypatch, capsys): + """A DataverseError from the wrapper (e.g. already published) reaches the CLI as text. + + The CLI's top-level error boundary formats it as a one-line message rather + than a traceback, so an already-published or unknown persistentId is + reported clearly instead of crashing the Actions job with a stack trace. + """ + import fwl_io.mirror as mirror_mod + from fwl_io.cli import main + from fwl_io.mirror import DataverseError + + def _raise(*args, **kwargs): + raise DataverseError( + 'Dataverse POST /api/datasets/:persistentId/actions/:publish failed ' + '(403): Dataset already published' + ) + + monkeypatch.setattr(mirror_mod, 'publish_existing_dataverse_draft', _raise) + monkeypatch.setenv('DATAVERSE_TOKEN', 'tok') + + rc = main(['mirror-publish', 'doi:10.34894/DEMO01']) + assert rc == 1 + assert 'already published' in capsys.readouterr().err From 71dc0fcb0c83f25f3037d217af93c821c9cf792c Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Wed, 2 Sep 2026 16:20:09 +0200 Subject: [PATCH 2/3] Reject invalid publish version types before the Dataverse call The --version-type argparse choice bypassed the CLI's own exit-1 error contract by exiting 2 straight from argument parsing. Validate it inside publish_existing_dataverse_draft instead, alongside the existing persistent_id check, and restrict the workflow_dispatch input to the same two values. Also fixes a stale mirror_dataset.md reference to a dataverse_url input the publish workflow no longer takes, and tightens the persistent_id test coverage to include an empty-prefix DOI. --- .github/workflows/mirror-publish.yaml | 4 ++++ docs/How-to/mirror_dataset.md | 2 +- docs/Reference/cli.md | 2 +- src/fwl_io/cli.py | 3 +-- src/fwl_io/mirror.py | 7 ++++++- tests/test_mirror.py | 20 +++++++++++++++++++- 6 files changed, 32 insertions(+), 6 deletions(-) diff --git a/.github/workflows/mirror-publish.yaml b/.github/workflows/mirror-publish.yaml index fcf75d3..ce9725b 100644 --- a/.github/workflows/mirror-publish.yaml +++ b/.github/workflows/mirror-publish.yaml @@ -10,6 +10,10 @@ on: description: "Dataverse publish version bump" required: true default: "major" + type: choice + options: + - major + - minor jobs: mirror-publish: diff --git a/docs/How-to/mirror_dataset.md b/docs/How-to/mirror_dataset.md index 6d777e1..7c7ff13 100644 --- a/docs/How-to/mirror_dataset.md +++ b/docs/How-to/mirror_dataset.md @@ -24,7 +24,7 @@ Mirroring runs from the **Mirror a Zenodo deposit to Dataverse** GitHub Actions ## Publishing a reviewed draft -A draft created with **publish** unchecked stays private until it is published. Run the **Publish an existing Dataverse draft** GitHub Actions workflow, supplying the draft's persistent id (the DOI printed by the mirror run, with a `doi:` prefix, for example `doi:10.34894/XXXXXX`) and the same Dataverse URL. It only publishes; it never creates a dataset, so it cannot mint a duplicate one. Add the DOI to the manifest as in step 3 above once it is published. +A draft created with **publish** unchecked stays private until it is published. Run the **Publish an existing Dataverse draft** GitHub Actions workflow, supplying the draft's persistent id (the DOI printed by the mirror run, with a `doi:` prefix, for example `doi:10.34894/XXXXXX`). It only publishes; it never creates a dataset, so it cannot mint a duplicate one. Add the DOI to the manifest as in step 3 above once it is published. ## What the mirror does diff --git a/docs/Reference/cli.md b/docs/Reference/cli.md index 4f8c5d1..e8e4b91 100644 --- a/docs/Reference/cli.md +++ b/docs/Reference/cli.md @@ -72,7 +72,7 @@ Mirrors a pinned Zenodo deposit to a Dataverse collection: it downloads and chec ```bash DATAVERSE_TOKEN=... fwl-io mirror-publish \ - [--dataverse-url URL] [--version-type {major,minor}] + [--dataverse-url URL] [--version-type VERSION_TYPE] ``` Publishes an existing Dataverse draft by its persistent id: it never creates a dataset, so it is the second step of a create-draft-then-publish workflow, run once a draft created by `fwl-io mirror --no-publish` has been reviewed. `` must be of the form `doi:/`, for example `doi:10.34894/EXAMPLE`. The API token is read from the `DATAVERSE_TOKEN` environment variable, never a command-line argument. `--version-type` is `major` by default and accepts only `major` or `minor`. Fails clearly if the dataset is already published or the persistent id does not resolve to a draft. See [Mirror a deposit to Dataverse](../How-to/mirror_dataset.md). diff --git a/src/fwl_io/cli.py b/src/fwl_io/cli.py index ac20450..903b5ff 100644 --- a/src/fwl_io/cli.py +++ b/src/fwl_io/cli.py @@ -188,8 +188,7 @@ def main(argv: list[str] | None = None) -> int: p_mirror_publish.add_argument( '--version-type', default='major', - choices=['major', 'minor'], - help='Dataverse publish version bump', + help="Dataverse publish version bump: 'major' or 'minor'", ) p_mirror_publish.set_defaults(func=_cmd_mirror_publish) diff --git a/src/fwl_io/mirror.py b/src/fwl_io/mirror.py index 29ea8b3..a0c86a1 100644 --- a/src/fwl_io/mirror.py +++ b/src/fwl_io/mirror.py @@ -445,11 +445,16 @@ def publish_existing_dataverse_draft( Raises ------ ValueError - If ``persistent_id`` is not of the form ``'doi:/'``. + If ``persistent_id`` is not of the form ``'doi:/'``, + or ``version_type`` is not ``'major'`` or ``'minor'``. DataverseError If the publish request fails: for example the dataset is already published, does not exist, or the server returns a non-2xx status. """ + if version_type not in ('major', 'minor'): + raise ValueError( + f"{version_type!r} is not a valid Dataverse version type: use 'major' or 'minor'" + ) prefix, sep, suffix = persistent_id.removeprefix('doi:').partition('/') if not persistent_id.startswith('doi:') or not sep or not prefix or not suffix: raise ValueError( diff --git a/tests/test_mirror.py b/tests/test_mirror.py index 7a62c93..ca7ecbb 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -576,7 +576,8 @@ def test_publish_existing_draft_raises_with_the_status_and_body_on_a_non_json_su @pytest.mark.unit @pytest.mark.parametrize( - 'persistent_id', ['', '10.34894/DEMO01', 'doi:', 'doi:noSlashHere', 'doi:10.34894/'] + 'persistent_id', + ['', '10.34894/DEMO01', 'doi:', 'doi:noSlashHere', 'doi:10.34894/', 'doi:/DEMO01'], ) def test_publish_existing_draft_rejects_a_malformed_persistent_id(persistent_id): """A persistent id that isn't 'doi:/' is rejected locally. @@ -588,6 +589,23 @@ def test_publish_existing_draft_rejects_a_malformed_persistent_id(persistent_id) publish_existing_dataverse_draft(persistent_id, dataverse_url='http://unused', token='tok') +@pytest.mark.unit +@pytest.mark.parametrize('version_type', ['', 'Major', 'patch', 'MAJOR']) +def test_publish_existing_draft_rejects_an_invalid_version_type(version_type): + """A version_type other than 'major' or 'minor' is rejected locally. + + No request is made: DataverseClient.publish is never reached, so this + raises ValueError even with an unreachable dataverse_url. + """ + with pytest.raises(ValueError, match='version type'): + publish_existing_dataverse_draft( + 'doi:10.34894/DEMO01', + dataverse_url='http://unused', + token='tok', + version_type=version_type, + ) + + @pytest.mark.unit def test_publish_existing_draft_raises_clearly_on_a_nonexistent_persistent_id(): """Publishing a persistentId Dataverse doesn't recognize errors clearly.""" From f92560df2852e6e321fe4a5982d9176d14330700 Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Wed, 2 Sep 2026 16:41:37 +0200 Subject: [PATCH 3/3] Tighten persistent-id validation and dedupe the Dataverse URL default A whitespace-corrupted persistent id like 'doi: 10.34894/DEMO01' passed local validation and only failed later against the live Dataverse API; publish_existing_dataverse_draft now rejects any persistent id that contains whitespace. Adds a test for the version_type/persistent_id check order (version_type is checked first, so an invalid persistent id combined with an invalid version_type surfaces the version_type error). Also folds the two identical --dataverse-url defaults in cli.py's mirror and mirror-publish subparsers into one DEFAULT_DATAVERSE_URL constant. --- src/fwl_io/cli.py | 6 ++++-- src/fwl_io/mirror.py | 8 +++++++- tests/test_mirror.py | 28 +++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/fwl_io/cli.py b/src/fwl_io/cli.py index 903b5ff..435bc14 100644 --- a/src/fwl_io/cli.py +++ b/src/fwl_io/cli.py @@ -13,6 +13,8 @@ from fwl_io import __version__ +DEFAULT_DATAVERSE_URL = 'https://dataverse.nl' + def _cmd_sync(args: argparse.Namespace) -> int: from fwl_io.sync import ZENODO_API, sync_manifest @@ -159,7 +161,7 @@ def main(argv: list[str] | None = None) -> int: p_mirror.add_argument('zenodo_doi', help='Zenodo version DOI to mirror') p_mirror.add_argument('--collection', required=True, help='target Dataverse collection alias') p_mirror.add_argument( - '--dataverse-url', default='https://dataverse.nl', help='Dataverse base URL' + '--dataverse-url', default=DEFAULT_DATAVERSE_URL, help='Dataverse base URL' ) p_mirror.add_argument('--contact-name', default='PROTEUS Framework', help='dataset contact') p_mirror.add_argument( @@ -183,7 +185,7 @@ def main(argv: list[str] | None = None) -> int: ) p_mirror_publish.add_argument('persistent_id', help='persistent id (DOI) of the draft') p_mirror_publish.add_argument( - '--dataverse-url', default='https://dataverse.nl', help='Dataverse base URL' + '--dataverse-url', default=DEFAULT_DATAVERSE_URL, help='Dataverse base URL' ) p_mirror_publish.add_argument( '--version-type', diff --git a/src/fwl_io/mirror.py b/src/fwl_io/mirror.py index a0c86a1..d8cefdb 100644 --- a/src/fwl_io/mirror.py +++ b/src/fwl_io/mirror.py @@ -456,7 +456,13 @@ def publish_existing_dataverse_draft( f"{version_type!r} is not a valid Dataverse version type: use 'major' or 'minor'" ) prefix, sep, suffix = persistent_id.removeprefix('doi:').partition('/') - if not persistent_id.startswith('doi:') or not sep or not prefix or not suffix: + if ( + not persistent_id.startswith('doi:') + or not sep + or not prefix + or not suffix + or any(ch.isspace() for ch in persistent_id) + ): raise ValueError( f'{persistent_id!r} is not a Dataverse persistent id of the form ' "'doi:/'" diff --git a/tests/test_mirror.py b/tests/test_mirror.py index ca7ecbb..8bbf68e 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -577,7 +577,17 @@ def test_publish_existing_draft_raises_with_the_status_and_body_on_a_non_json_su @pytest.mark.unit @pytest.mark.parametrize( 'persistent_id', - ['', '10.34894/DEMO01', 'doi:', 'doi:noSlashHere', 'doi:10.34894/', 'doi:/DEMO01'], + [ + '', + '10.34894/DEMO01', + 'doi:', + 'doi:noSlashHere', + 'doi:10.34894/', + 'doi:/DEMO01', + 'doi: 10.34894/DEMO01', + 'doi:10.34894/DEMO01 ', + 'doi:10.34894/DE MO01', + ], ) def test_publish_existing_draft_rejects_a_malformed_persistent_id(persistent_id): """A persistent id that isn't 'doi:/' is rejected locally. @@ -606,6 +616,22 @@ def test_publish_existing_draft_rejects_an_invalid_version_type(version_type): ) +@pytest.mark.unit +def test_publish_existing_draft_checks_version_type_before_persistent_id(): + """When both are invalid, the version_type error is raised first. + + No request is made either way: both checks run before DataverseClient + is ever constructed. + """ + with pytest.raises(ValueError, match='version type'): + publish_existing_dataverse_draft( + 'not-a-doi', + dataverse_url='http://unused', + token='tok', + version_type='bogus', + ) + + @pytest.mark.unit def test_publish_existing_draft_raises_clearly_on_a_nonexistent_persistent_id(): """Publishing a persistentId Dataverse doesn't recognize errors clearly."""