Migrate POST /setup/tag endpoint (#64)

docs/migration.md

@@ -107,6 +107,11 @@ For example, after tagging dataset 21 with the tag `"foo"`:
 }
 ```
 
+## Setups
+
+### `POST /setup/tag` and `POST /setup/untag`
+When successful, the "tag" property in the returned response is now always a list, even if only one tag exists for the entity. When removing the last tag, the "tag" property will be an empty list `[]` instead of being omitted from the response.
+
 ## Studies
 
 ### `GET /{id_or_alias}`

src/database/setups.py

@@ -46,3 +46,16 @@ async def untag(setup_id: int, tag: str, connection: AsyncConnection) -> None:
         ),
         parameters={"setup_id": setup_id, "tag": tag},
     )
+
+
+async def tag(setup_id: int, tag: str, user_id: int, connection: AsyncConnection) -> None:
+    """Add tag `tag` to setup with id `setup_id`."""
+    await connection.execute(
+        text(
+            """
+            INSERT INTO setup_tag (id, tag, uploader)
+            VALUES (:setup_id, :tag, :user_id)
+            """,
+        ),
+        parameters={"setup_id": setup_id, "tag": tag, "user_id": user_id},
+    )

src/routers/openml/setups.py

@@ -6,14 +6,43 @@
 from sqlalchemy.ext.asyncio import AsyncConnection
 
 import database.setups
-from core.errors import SetupNotFoundError, TagNotFoundError, TagNotOwnedError
+from core.errors import (
+    SetupNotFoundError,
+    TagAlreadyExistsError,
+    TagNotFoundError,
+    TagNotOwnedError,
+)
 from database.users import User, UserGroup
 from routers.dependencies import expdb_connection, fetch_user_or_raise
 from routers.types import SystemString64
 
 router = APIRouter(prefix="/setup", tags=["setup"])
 
 
+@router.post(path="/tag")
+async def tag_setup(
+    setup_id: Annotated[int, Body()],
+    tag: Annotated[str, SystemString64],
+    user: Annotated[User, Depends(fetch_user_or_raise)],
+    expdb_db: Annotated[AsyncConnection, Depends(expdb_connection)],
+) -> dict[str, dict[str, str | list[str]]]:
+    """Add tag `tag` to setup with id `setup_id`."""
+    if not await database.setups.get(setup_id, expdb_db):
+        msg = f"Setup {setup_id} not found."
+        raise SetupNotFoundError(msg)
+
+    setup_tags = await database.setups.get_tags(setup_id, expdb_db)
+    matched_tag_row = next((t for t in setup_tags if t.tag.casefold() == tag.casefold()), None)
+
+    if matched_tag_row:
+        msg = f"Setup {setup_id} already has tag {tag!r}."
+        raise TagAlreadyExistsError(msg)
+
+    await database.setups.tag(setup_id, tag, user.user_id, expdb_db)
+    all_tags = [t.tag.casefold() for t in setup_tags] + [tag.casefold()]
+    return {"setup_tag": {"id": str(setup_id), "tag": all_tags}}
+
+
 @router.post(path="/untag")
 async def untag_setup(
     setup_id: Annotated[int, Body()],

tests/routers/openml/migration/setups_migration_test.py

@@ -147,3 +147,152 @@ async def test_setup_untag_response_is_identical_tag_doesnt_exist(
         r"Setup \d+ does not have tag '\S+'.",
         new.json()["detail"],
     )
+
+
+@pytest.mark.parametrize(
+    "api_key",
+    [ApiKey.ADMIN, ApiKey.SOME_USER],
+    ids=["Administrator", "non-owner"],
+)
+@pytest.mark.parametrize(
+    "other_tags",
+    [[], ["some_other_tag"], ["foo_some_other_tag", "bar_some_other_tag"]],
+    ids=["none", "one tag", "two tags"],
+)
+async def test_setup_tag_response_is_identical_when_tag_doesnt_exist(
+    api_key: str,
+    other_tags: list[str],
+    py_api: httpx.AsyncClient,
+    php_api: httpx.AsyncClient,
+    expdb_test: AsyncConnection,
+) -> None:
+    setup_id = 1
+    tag = "totally_new_tag_for_migration_testing"
+
+    @contextlib.asynccontextmanager
+    async def temporary_tags(
+        tags: Iterable[str], setup_id: int, *, persist: bool = False
+    ) -> AsyncGenerator[None]:
+        for tag in tags:
+            await expdb_test.execute(
+                text(
+                    "INSERT INTO setup_tag(`id`,`tag`,`uploader`) "
+                    "VALUES (:setup_id, :tag, :user_id);"
+                ),
+                parameters={"setup_id": setup_id, "tag": tag, "user_id": OWNER_USER.user_id},
+            )
+        if persist:
+            await expdb_test.commit()
+        yield
+        for tag in tags:
+            await expdb_test.execute(
+                text("DELETE FROM setup_tag WHERE `id`=:setup_id AND `tag`=:tag"),
+                parameters={"setup_id": setup_id, "tag": tag},
+            )
+        if persist:
+            await expdb_test.commit()
+
+    all_tags = [*other_tags, tag]
+
+    async with temporary_tags(tags=all_tags, setup_id=setup_id, persist=True):
+        await expdb_test.execute(
+            text("DELETE FROM setup_tag WHERE `id`=:setup_id AND `tag`=:tag"),
+            parameters={"setup_id": setup_id, "tag": tag},
+        )
+        await expdb_test.commit()
+
+        original = await php_api.post(
+            "/setup/tag",
+            data={"api_key": api_key, "tag": tag, "setup_id": setup_id},
+        )
+
+    async with temporary_tags(tags=all_tags, setup_id=setup_id):
+        await expdb_test.execute(
+            text("DELETE FROM setup_tag WHERE `id`=:setup_id AND `tag`=:tag"),
+            parameters={"setup_id": setup_id, "tag": tag},
+        )
+        new = await py_api.post(
+            f"/setup/tag?api_key={api_key}",
+            json={"setup_id": setup_id, "tag": tag},
+        )
+
+    assert new.status_code == HTTPStatus.OK
+    assert original.status_code == new.status_code
+    original_tag = original.json()["setup_tag"]
+    new_tag = new.json()["setup_tag"]
+    assert original_tag["id"] == new_tag["id"]
+    if tags := original_tag.get("tag"):
+        if isinstance(tags, str):
+            assert tags == new_tag["tag"][0]
+        else:
+            assert set(tags) == set(new_tag["tag"])
+    else:
+        assert new_tag["tag"] == []
+
+
+async def test_setup_tag_response_is_identical_setup_doesnt_exist(
+    py_api: httpx.AsyncClient,
+    php_api: httpx.AsyncClient,
+) -> None:
+    setup_id = 999999
+    tag = "totally_new_tag_for_migration_testing"
+    api_key = ApiKey.SOME_USER
+
+    original = await php_api.post(
+        "/setup/tag",
+        data={"api_key": api_key, "tag": tag, "setup_id": setup_id},
+    )
+
+    new = await py_api.post(
+        f"/setup/tag?api_key={api_key}",
+        json={"setup_id": setup_id, "tag": tag},
+    )
+
+    assert original.status_code == HTTPStatus.PRECONDITION_FAILED
+    assert new.status_code == HTTPStatus.NOT_FOUND
+    assert original.json()["error"]["message"] == "Entity not found."
+    assert original.json()["error"]["code"] == new.json()["code"]
+    assert re.match(
+        r"Setup \d+ not found.",
+        new.json()["detail"],
+    )
+
+
+async def test_setup_tag_response_is_identical_tag_already_exists(
+    py_api: httpx.AsyncClient,
+    php_api: httpx.AsyncClient,
+    expdb_test: AsyncConnection,
+) -> None:
+    setup_id = 1
+    tag = "totally_new_tag_for_migration_testing"
+    api_key = ApiKey.SOME_USER
+
+    await expdb_test.execute(
+        text("INSERT INTO setup_tag(`id`,`tag`,`uploader`) VALUES (:setup_id, :tag, 2);"),
+        parameters={"setup_id": setup_id, "tag": tag},
+    )
+    await expdb_test.commit()
+
+    try:
+        original = await php_api.post(
+            "/setup/tag",
+            data={"api_key": api_key, "tag": tag, "setup_id": setup_id},
+        )
+
+        # In Python, since PHP committed it, it's also there for Python test context
+        new = await py_api.post(
+            f"/setup/tag?api_key={api_key}",
+            json={"setup_id": setup_id, "tag": tag},
+        )
+    finally:
+        await expdb_test.execute(
+            text("DELETE FROM setup_tag WHERE `id`=:setup_id AND `tag`=:tag"),
+            parameters={"setup_id": setup_id, "tag": tag},
+        )
+        await expdb_test.commit()
+
+    assert original.status_code == HTTPStatus.INTERNAL_SERVER_ERROR
+    assert new.status_code == HTTPStatus.CONFLICT
+    assert original.json()["error"]["code"] == new.json()["code"]
+    assert original.json()["error"]["message"] == "Entity already tagged by this tag."
+    assert new.json()["detail"] == f"Setup {setup_id} already has tag {tag!r}."

tests/routers/openml/setups_test.py

@@ -83,3 +83,50 @@ async def test_setup_untag_success(
         text("SELECT * FROM setup_tag WHERE id = 1 AND tag = 'test_success_tag'")
     )
     assert len(rows.all()) == 0
+
+
+async def test_setup_tag_missing_auth(py_api: httpx.AsyncClient) -> None:
+    response = await py_api.post("/setup/tag", json={"setup_id": 1, "tag": "test_tag"})
+    assert response.status_code == HTTPStatus.UNAUTHORIZED
+    assert response.json()["code"] == "103"
+    assert response.json()["detail"] == "Authentication failed"
+
+
+async def test_setup_tag_unknown_setup(py_api: httpx.AsyncClient) -> None:
+    response = await py_api.post(
+        f"/setup/tag?api_key={ApiKey.SOME_USER}",
+        json={"setup_id": 999999, "tag": "test_tag"},
+    )
+    assert response.status_code == HTTPStatus.NOT_FOUND
+    assert re.match(r"Setup \d+ not found.", response.json()["detail"])
+
+
+@pytest.mark.mut
+async def test_setup_tag_already_exists(
+    py_api: httpx.AsyncClient, expdb_test: AsyncConnection
+) -> None:
+    await expdb_test.execute(
+        text("INSERT INTO setup_tag (id, tag, uploader) VALUES (1, 'existing_tag_123', 2);")
+    )
+    response = await py_api.post(
+        f"/setup/tag?api_key={ApiKey.SOME_USER}",
+        json={"setup_id": 1, "tag": "existing_tag_123"},
+    )
+    assert response.status_code == HTTPStatus.CONFLICT
+    assert response.json()["detail"] == "Setup 1 already has tag 'existing_tag_123'."
+
+
+@pytest.mark.mut
+async def test_setup_tag_success(py_api: httpx.AsyncClient, expdb_test: AsyncConnection) -> None:
+    response = await py_api.post(
+        f"/setup/tag?api_key={ApiKey.SOME_USER}",
+        json={"setup_id": 1, "tag": "my_new_success_tag"},
+    )
+
+    assert response.status_code == HTTPStatus.OK
+    assert "my_new_success_tag" in response.json()["setup_tag"]["tag"]
+
+    rows = await expdb_test.execute(
+        text("SELECT * FROM setup_tag WHERE id = 1 AND tag = 'my_new_success_tag'")
+    )
+    assert len(rows.all()) == 1