From 31b0125a1bf94c2ac4d772041cce0c295062bc31 Mon Sep 17 00:00:00 2001 From: PGijsbers Date: Sat, 11 Jul 2026 14:44:23 +0200 Subject: [PATCH 1/2] Provide a more conventional endpoint for dataset tag --- docs/migration.md | 7 +++++ src/routers/datasets.py | 38 ++++++++++++++++-------- tests/routers/dataset_tag_test.py | 49 +++++++++++++++++++++---------- 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index f2daa49..7cb7f0f 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -129,6 +129,13 @@ The `limit` and `offset` parameters can now be used independently, you no longer to provide both if you wish to set only one. #### `POST /datasets/tag` + +???+ warning "This endpoint is deprecated" + + The new tag endpoint is `POST` `/datasets/{identifier}/tags`. + The dataset identifier is provided as part of the path, the tag is still provided in the body. + This endpoint is provided for easier migration in case the response matters to you. + When successful, the "tag" property in the returned response is now always a list, even if only one tag exists for the entity. For example, after tagging dataset 21 with the tag `"foo"`: ```diff diff --git a/src/routers/datasets.py b/src/routers/datasets.py index b23babd..1155ebb 100644 --- a/src/routers/datasets.py +++ b/src/routers/datasets.py @@ -13,7 +13,7 @@ from http import HTTPStatus from typing import TYPE_CHECKING, Annotated, Any, Literal, NamedTuple -from fastapi import APIRouter, Body, Depends, Query +from fastapi import APIRouter, Body, Depends, Path, Query from loguru import logger from sqlalchemy import bindparam, text @@ -89,30 +89,44 @@ def _format_dataset_url(dataset: UntypedRow) -> str: @router.post( path="/tag", + deprecated=True, ) async def tag_dataset( data_id: Annotated[Identifier, Body()], tag: Annotated[TagString, Body()], user: Annotated[User, Depends(fetch_user_or_raise)], - expdb_db: Annotated[AsyncSession, Depends(expdb_session)], + expdb: Annotated[AsyncSession, Depends(expdb_session)], ) -> dict[str, dict[str, Any]]: + """Add a tag to the dataset, this tag is publicly visible to all users.""" + await tag_dataset_new(data_id, tag, user, expdb) + + tags = await database.datasets.get_tags_for(data_id, expdb) + + return { + "data_tag": {"id": str(data_id), "tag": tags}, + } + + +@router.post( + path="/{identifier}/tags", +) +async def tag_dataset_new( + identifier: Annotated[Identifier, Path()], + tag: Annotated[TagString, Body(embed=True)], + user: Annotated[User, Depends(fetch_user_or_raise)], + expdb: Annotated[AsyncSession, Depends(expdb_session)], +) -> None: """Add a tag to the dataset, this tag is publicly visible to all users.""" try: - await database.datasets.tag(data_id, tag, user_id=user.user_id, session=expdb_db) + await database.datasets.tag(identifier, tag, user_id=user.user_id, session=expdb) except ForeignKeyConstraintError: - msg = f"Dataset {data_id} not found." + msg = f"Dataset {identifier} not found." raise DatasetNotFoundError(msg, code=472) from None except DuplicatePrimaryKeyError: - msg = f"Dataset {data_id} already tagged with {tag!r}." + msg = f"Dataset {identifier} already tagged with {tag!r}." raise TagAlreadyExistsError(msg) from None - logger.info("Dataset {data_id} tagged '{tag}'.", data_id=data_id, tag=tag) - - tags = await database.datasets.get_tags_for(data_id, expdb_db) - - return { - "data_tag": {"id": str(data_id), "tag": tags}, - } + logger.info("Dataset {identifier} tagged '{tag}'.", identifier=identifier, tag=tag) @router.post(path="/untag", deprecated=True) diff --git a/tests/routers/dataset_tag_test.py b/tests/routers/dataset_tag_test.py index 851e395..7ab07be 100644 --- a/tests/routers/dataset_tag_test.py +++ b/tests/routers/dataset_tag_test.py @@ -18,22 +18,41 @@ from sqlalchemy.ext.asyncio import AsyncSession -@pytest.mark.parametrize( - "key", - [None, ApiKey.INVALID], - ids=["no authentication", "invalid key"], -) -async def test_dataset_tag_rejects_unauthorized(key: ApiKey, py_api: httpx.AsyncClient) -> None: - apikey = "" if key is None else f"?api_key={key}" +async def test_dataset_tag_requires_authorization(py_api: httpx.AsyncClient) -> None: any_dataset_identifier = 1 response = await py_api.post( - f"/datasets/tag{apikey}", + "/datasets/tag", json={"data_id": any_dataset_identifier, "tag": "test"}, ) assert response.status_code == HTTPStatus.UNAUTHORIZED -# ── Direct call tests: tag_dataset ── +async def test_dataset_tag_json(py_api: httpx.AsyncClient, dataset_factory: DatasetFactory) -> None: + dataset_id = await dataset_factory() + response = await py_api.post( + f"/datasets/tag?api_key={ApiKey.SOME_USER}", + json={"data_id": dataset_id, "tag": "test"}, + ) + assert response.status_code == HTTPStatus.OK + expected_json = { + "data_tag": { + "id": str(dataset_id), + "tag": ["test"], + } + } + assert response.json() == expected_json + + +async def test_dataset_tag_new_json( + py_api: httpx.AsyncClient, dataset_factory: DatasetFactory +) -> None: + dataset_id = await dataset_factory() + response = await py_api.post( + f"/datasets/{dataset_id}/tags?api_key={ApiKey.SOME_USER}", + json={"tag": "test"}, + ) + assert response.status_code == HTTPStatus.OK, response.json() + assert response.json() is None @pytest.mark.mut @@ -47,7 +66,7 @@ async def test_dataset_tag( ) -> None: dataset_id = await dataset_factory() tag = "test_tag" - result = await tag_dataset(data_id=dataset_id, tag=tag, user=user, expdb_db=expdb_session) + result = await tag_dataset(data_id=dataset_id, tag=tag, user=user, expdb=expdb_session) assert result == {"data_tag": {"id": str(dataset_id), "tag": [tag]}} tags = await get_tags_for(dataset_id=dataset_id, session=expdb_session) @@ -59,9 +78,9 @@ async def test_dataset_tag_returns_existing_tags( expdb_session: AsyncSession, dataset_factory: DatasetFactory ) -> None: dataset_id = await dataset_factory() - await tag_dataset(data_id=dataset_id, tag="first", user=OWNER_USER, expdb_db=expdb_session) + await tag_dataset(data_id=dataset_id, tag="first", user=OWNER_USER, expdb=expdb_session) result = await tag_dataset( - data_id=dataset_id, tag="second", user=ADMIN_USER, expdb_db=expdb_session + data_id=dataset_id, tag="second", user=ADMIN_USER, expdb=expdb_session ) assert result == {"data_tag": {"id": str(dataset_id), "tag": ["first", "second"]}} @@ -72,10 +91,10 @@ async def test_dataset_tag_fails_if_tag_exists( ) -> None: tag = "repeated_tag" dataset_id = await dataset_factory() - await tag_dataset(data_id=dataset_id, tag=tag, user=OWNER_USER, expdb_db=expdb_session) + await tag_dataset(data_id=dataset_id, tag=tag, user=OWNER_USER, expdb=expdb_session) with pytest.raises(TagAlreadyExistsError) as e: - await tag_dataset(data_id=dataset_id, tag=tag, user=ADMIN_USER, expdb_db=expdb_session) + await tag_dataset(data_id=dataset_id, tag=tag, user=ADMIN_USER, expdb=expdb_session) assert str(dataset_id) in e.value.detail assert tag in e.value.detail @@ -87,7 +106,7 @@ async def test_dataset_tag_fails_if_dataset_does_not_exist(expdb_session: AsyncS data_id=dataset_id, tag="foo", user=ADMIN_USER, - expdb_db=expdb_session, + expdb=expdb_session, ) assert str(dataset_id) in e.value.detail dataset_not_found_in_tag_endpoint = 472 From 68f603e5852fb2c775b07ec8a573b8c6dd85b1b4 Mon Sep 17 00:00:00 2001 From: PGijsbers Date: Sun, 12 Jul 2026 11:50:37 +0200 Subject: [PATCH 2/2] Be slightly more specific about the changes we make --- docs/roadmap.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index 3fabcb1..2052332 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -6,8 +6,12 @@ The sunset date of the PHP-based REST API will depend on the progress of this re ## Phase 1: Achieve Feature Parity The first goal is _rough_ feature parity. -This means that we want to expose the same endpoints with the same general responses. -We may deviate in case the original implementation had an oversight or a bug, and fix those along the way. +This means that we want to expose the same endpoints with the same general responses, but: + +- we may deviate in case the original implementation had an oversight or a bug, and fix those along the way. +- we may drop support for some admin-only or previously undocumented endpoints. +- we may make other alterations at our discretion, e.g., improve consistency in responses or [RFC9457](https://www.rfc-editor.org/rfc/rfc9457.html) error handling. But we do not significantly restructure "happy path" responses or endpoint structure. + We can be pragmatic about this, for example, copying over a complex query from the PHP API. This phase is considered complete when the output of the new REST API can be mapped to output of the old REST API for all endpoints the PHP-based API has and the [migration guide](migration.md) is done. @@ -15,6 +19,7 @@ This phase is considered complete when the output of the new REST API can be map ## Phase 2: Modernizing the API We are also working on incorporating modern standards like [RFC9457](https://www.rfc-editor.org/rfc/rfc9457.html) for error reporting, better use of JSON types, including authentication in the request's HTTP header instead of the URL, or changing HTTP methods or paths to better reflect the behavior of the endpoint. +We may also consider schema changes to responses to improve e.g., usability or performance. ## Phase 3: Revisiting the Data(base)