From 289dcd929628101b1acc1720f5b2ebe0e7f52d31 Mon Sep 17 00:00:00 2001 From: yassinnouh21 Date: Sat, 21 Feb 2026 16:21:50 +0100 Subject: [PATCH 1/5] Add tag/untag database functions for tasks, flows, and runs Add tag(), get_tag(), and delete_tag() to tasks.py and flows.py. Create new runs.py with get_tags(), tag(), get_tag(), and delete_tag(). --- src/database/flows.py | 37 ++++++++++++++++++++++++++++++ src/database/runs.py | 52 +++++++++++++++++++++++++++++++++++++++++++ src/database/tasks.py | 37 ++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 src/database/runs.py diff --git a/src/database/flows.py b/src/database/flows.py index 3129e91e..17410799 100644 --- a/src/database/flows.py +++ b/src/database/flows.py @@ -50,6 +50,43 @@ def get_parameters(flow_id: int, expdb: Connection) -> Sequence[Row]: ) +def tag(id_: int, tag_: str, *, user_id: int, connection: Connection) -> None: + connection.execute( + text( + """ + INSERT INTO implementation_tag(`id`, `tag`, `uploader`) + VALUES (:flow_id, :tag, :user_id) + """, + ), + parameters={"flow_id": id_, "tag": tag_, "user_id": user_id}, + ) + + +def get_tag(id_: int, tag_: str, connection: Connection) -> Row | None: + return connection.execute( + text( + """ + SELECT `id`, `tag`, `uploader` + FROM implementation_tag + WHERE `id` = :flow_id AND `tag` = :tag + """, + ), + parameters={"flow_id": id_, "tag": tag_}, + ).one_or_none() + + +def delete_tag(id_: int, tag_: str, connection: Connection) -> None: + connection.execute( + text( + """ + DELETE FROM implementation_tag + WHERE `id` = :flow_id AND `tag` = :tag + """, + ), + parameters={"flow_id": id_, "tag": tag_}, + ) + + def get_by_name(name: str, external_version: str, expdb: Connection) -> Row | None: """Gets flow by name and external version.""" return expdb.execute( diff --git a/src/database/runs.py b/src/database/runs.py new file mode 100644 index 00000000..d263c23d --- /dev/null +++ b/src/database/runs.py @@ -0,0 +1,52 @@ +from sqlalchemy import Connection, Row, text + + +def get_tags(id_: int, expdb: Connection) -> list[str]: + tag_rows = expdb.execute( + text( + """ + SELECT `tag` + FROM run_tag + WHERE `id` = :run_id + """, + ), + parameters={"run_id": id_}, + ) + return [row.tag for row in tag_rows] + + +def tag(id_: int, tag_: str, *, user_id: int, connection: Connection) -> None: + connection.execute( + text( + """ + INSERT INTO run_tag(`id`, `tag`, `uploader`) + VALUES (:run_id, :tag, :user_id) + """, + ), + parameters={"run_id": id_, "tag": tag_, "user_id": user_id}, + ) + + +def get_tag(id_: int, tag_: str, connection: Connection) -> Row | None: + return connection.execute( + text( + """ + SELECT `id`, `tag`, `uploader` + FROM run_tag + WHERE `id` = :run_id AND `tag` = :tag + """, + ), + parameters={"run_id": id_, "tag": tag_}, + ).one_or_none() + + +def delete_tag(id_: int, tag_: str, connection: Connection) -> None: + connection.execute( + text( + """ + DELETE FROM run_tag + WHERE `id` = :run_id AND `tag` = :tag + """, + ), + parameters={"run_id": id_, "tag": tag_}, + ) diff --git a/src/database/tasks.py b/src/database/tasks.py index 97caef3b..56c1f658 100644 --- a/src/database/tasks.py +++ b/src/database/tasks.py @@ -104,3 +104,40 @@ def get_tags(id_: int, expdb: Connection) -> list[str]: parameters={"task_id": id_}, ) return [row.tag for row in tag_rows] + + +def tag(id_: int, tag_: str, *, user_id: int, connection: Connection) -> None: + connection.execute( + text( + """ + INSERT INTO task_tag(`id`, `tag`, `uploader`) + VALUES (:task_id, :tag, :user_id) + """, + ), + parameters={"task_id": id_, "tag": tag_, "user_id": user_id}, + ) + + +def get_tag(id_: int, tag_: str, connection: Connection) -> Row | None: + return connection.execute( + text( + """ + SELECT `id`, `tag`, `uploader` + FROM task_tag + WHERE `id` = :task_id AND `tag` = :tag + """, + ), + parameters={"task_id": id_, "tag": tag_}, + ).one_or_none() + + +def delete_tag(id_: int, tag_: str, connection: Connection) -> None: + connection.execute( + text( + """ + DELETE FROM task_tag + WHERE `id` = :task_id AND `tag` = :tag + """, + ), + parameters={"task_id": id_, "tag": tag_}, + ) From 2297b44f233bb3d4d4d92584835a574eaade15a9 Mon Sep 17 00:00:00 2001 From: yassinnouh21 Date: Sat, 21 Feb 2026 16:21:55 +0100 Subject: [PATCH 2/5] Add tag/untag POST endpoints for tasks, flows, and runs - POST /tasks/tag, /tasks/untag - POST /flows/tag, /flows/untag - POST /runs/tag, /runs/untag (new router, registered in main.py) Untag checks tag ownership (error 478) with admin bypass. --- src/main.py | 2 + src/routers/openml/flows.py | 70 +++++++++++++++++++++++++++++++++-- src/routers/openml/runs.py | 74 +++++++++++++++++++++++++++++++++++++ src/routers/openml/tasks.py | 70 +++++++++++++++++++++++++++++++++-- 4 files changed, 210 insertions(+), 6 deletions(-) create mode 100644 src/routers/openml/runs.py diff --git a/src/main.py b/src/main.py index 560b4c50..97187342 100644 --- a/src/main.py +++ b/src/main.py @@ -11,6 +11,7 @@ from routers.openml.evaluations import router as evaluationmeasures_router from routers.openml.flows import router as flows_router from routers.openml.qualities import router as qualities_router +from routers.openml.runs import router as runs_router from routers.openml.study import router as study_router from routers.openml.tasks import router as task_router from routers.openml.tasktype import router as ttype_router @@ -54,6 +55,7 @@ def create_api() -> FastAPI: app.include_router(estimationprocedure_router) app.include_router(task_router) app.include_router(flows_router) + app.include_router(runs_router) app.include_router(study_router) return app diff --git a/src/routers/openml/flows.py b/src/routers/openml/flows.py index cb6df5d9..d8e208a7 100644 --- a/src/routers/openml/flows.py +++ b/src/routers/openml/flows.py @@ -1,17 +1,81 @@ from http import HTTPStatus -from typing import Annotated, Literal +from typing import Annotated, Any, Literal -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Body, Depends, HTTPException from sqlalchemy import Connection import database.flows from core.conversions import _str_to_num -from routers.dependencies import expdb_connection +from database.users import User, UserGroup +from routers.dependencies import expdb_connection, fetch_user +from routers.types import SystemString64 from schemas.flows import Flow, Parameter, Subflow router = APIRouter(prefix="/flows", tags=["flows"]) +@router.post(path="/tag") +def tag_flow( + flow_id: Annotated[int, Body()], + tag: Annotated[str, SystemString64], + user: Annotated[User | None, Depends(fetch_user)] = None, + expdb: Annotated[Connection, Depends(expdb_connection)] = None, +) -> dict[str, dict[str, Any]]: + if user is None: + raise HTTPException( + status_code=HTTPStatus.PRECONDITION_FAILED, + detail={"code": "103", "message": "Authentication failed"}, + ) + tags = database.flows.get_tags(flow_id, expdb) + if tag.casefold() in [t.casefold() for t in tags]: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "473", + "message": "Entity already tagged by this tag.", + "additional_information": f"id={flow_id}; tag={tag}", + }, + ) + database.flows.tag(flow_id, tag, user_id=user.user_id, connection=expdb) + return {"flow_tag": {"id": str(flow_id), "tag": [*tags, tag]}} + + +@router.post(path="/untag") +def untag_flow( + flow_id: Annotated[int, Body()], + tag: Annotated[str, SystemString64], + user: Annotated[User | None, Depends(fetch_user)] = None, + expdb: Annotated[Connection, Depends(expdb_connection)] = None, +) -> dict[str, dict[str, Any]]: + if user is None: + raise HTTPException( + status_code=HTTPStatus.PRECONDITION_FAILED, + detail={"code": "103", "message": "Authentication failed"}, + ) + existing = database.flows.get_tag(flow_id, tag, expdb) + if existing is None: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "477", + "message": "Tag not found.", + "additional_information": f"id={flow_id}; tag={tag}", + }, + ) + if existing.uploader != user.user_id and UserGroup.ADMIN not in user.groups: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "478", + "message": "Tag is not owned by you.", + "additional_information": f"id={flow_id}; tag={tag}", + }, + ) + database.flows.delete_tag(flow_id, tag, expdb) + tags = database.flows.get_tags(flow_id, expdb) + return {"flow_tag": {"id": str(flow_id), "tag": tags}} + + @router.get("/exists/{name}/{external_version}") def flow_exists( name: str, diff --git a/src/routers/openml/runs.py b/src/routers/openml/runs.py new file mode 100644 index 00000000..a81875ae --- /dev/null +++ b/src/routers/openml/runs.py @@ -0,0 +1,74 @@ +from http import HTTPStatus +from typing import Annotated, Any + +from fastapi import APIRouter, Body, Depends, HTTPException +from sqlalchemy import Connection + +import database.runs +from database.users import User, UserGroup +from routers.dependencies import expdb_connection, fetch_user +from routers.types import SystemString64 + +router = APIRouter(prefix="/runs", tags=["runs"]) + + +@router.post(path="/tag") +def tag_run( + run_id: Annotated[int, Body()], + tag: Annotated[str, SystemString64], + user: Annotated[User | None, Depends(fetch_user)] = None, + expdb: Annotated[Connection, Depends(expdb_connection)] = None, +) -> dict[str, dict[str, Any]]: + if user is None: + raise HTTPException( + status_code=HTTPStatus.PRECONDITION_FAILED, + detail={"code": "103", "message": "Authentication failed"}, + ) + tags = database.runs.get_tags(run_id, expdb) + if tag.casefold() in [t.casefold() for t in tags]: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "473", + "message": "Entity already tagged by this tag.", + "additional_information": f"id={run_id}; tag={tag}", + }, + ) + database.runs.tag(run_id, tag, user_id=user.user_id, connection=expdb) + return {"run_tag": {"id": str(run_id), "tag": [*tags, tag]}} + + +@router.post(path="/untag") +def untag_run( + run_id: Annotated[int, Body()], + tag: Annotated[str, SystemString64], + user: Annotated[User | None, Depends(fetch_user)] = None, + expdb: Annotated[Connection, Depends(expdb_connection)] = None, +) -> dict[str, dict[str, Any]]: + if user is None: + raise HTTPException( + status_code=HTTPStatus.PRECONDITION_FAILED, + detail={"code": "103", "message": "Authentication failed"}, + ) + existing = database.runs.get_tag(run_id, tag, expdb) + if existing is None: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "477", + "message": "Tag not found.", + "additional_information": f"id={run_id}; tag={tag}", + }, + ) + if existing.uploader != user.user_id and UserGroup.ADMIN not in user.groups: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "478", + "message": "Tag is not owned by you.", + "additional_information": f"id={run_id}; tag={tag}", + }, + ) + database.runs.delete_tag(run_id, tag, expdb) + tags = database.runs.get_tags(run_id, expdb) + return {"run_tag": {"id": str(run_id), "tag": tags}} diff --git a/src/routers/openml/tasks.py b/src/routers/openml/tasks.py index 8397f1da..4b1f8911 100644 --- a/src/routers/openml/tasks.py +++ b/src/routers/openml/tasks.py @@ -1,16 +1,18 @@ import json import re from http import HTTPStatus -from typing import Annotated, cast +from typing import Annotated, Any, cast import xmltodict -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Body, Depends, HTTPException from sqlalchemy import Connection, RowMapping, text import config import database.datasets import database.tasks -from routers.dependencies import expdb_connection +from database.users import User, UserGroup +from routers.dependencies import expdb_connection, fetch_user +from routers.types import SystemString64 from schemas.datasets.openml import Task router = APIRouter(prefix="/tasks", tags=["tasks"]) @@ -149,6 +151,68 @@ def _fill_json_template( return template.replace("[CONSTANT:base_url]", server_url) +@router.post(path="/tag") +def tag_task( + task_id: Annotated[int, Body()], + tag: Annotated[str, SystemString64], + user: Annotated[User | None, Depends(fetch_user)] = None, + expdb: Annotated[Connection, Depends(expdb_connection)] = None, +) -> dict[str, dict[str, Any]]: + if user is None: + raise HTTPException( + status_code=HTTPStatus.PRECONDITION_FAILED, + detail={"code": "103", "message": "Authentication failed"}, + ) + tags = database.tasks.get_tags(task_id, expdb) + if tag.casefold() in [t.casefold() for t in tags]: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "473", + "message": "Entity already tagged by this tag.", + "additional_information": f"id={task_id}; tag={tag}", + }, + ) + database.tasks.tag(task_id, tag, user_id=user.user_id, connection=expdb) + return {"task_tag": {"id": str(task_id), "tag": [*tags, tag]}} + + +@router.post(path="/untag") +def untag_task( + task_id: Annotated[int, Body()], + tag: Annotated[str, SystemString64], + user: Annotated[User | None, Depends(fetch_user)] = None, + expdb: Annotated[Connection, Depends(expdb_connection)] = None, +) -> dict[str, dict[str, Any]]: + if user is None: + raise HTTPException( + status_code=HTTPStatus.PRECONDITION_FAILED, + detail={"code": "103", "message": "Authentication failed"}, + ) + existing = database.tasks.get_tag(task_id, tag, expdb) + if existing is None: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "477", + "message": "Tag not found.", + "additional_information": f"id={task_id}; tag={tag}", + }, + ) + if existing.uploader != user.user_id and UserGroup.ADMIN not in user.groups: + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail={ + "code": "478", + "message": "Tag is not owned by you.", + "additional_information": f"id={task_id}; tag={tag}", + }, + ) + database.tasks.delete_tag(task_id, tag, expdb) + tags = database.tasks.get_tags(task_id, expdb) + return {"task_tag": {"id": str(task_id), "tag": tags}} + + @router.get("/{task_id}") def get_task( task_id: int, From c7d717cac9dd80e3d67ce3d203de2b93d068f221 Mon Sep 17 00:00:00 2001 From: yassinnouh21 Date: Sat, 21 Feb 2026 16:22:00 +0100 Subject: [PATCH 3/5] Add tests for task, flow, and run tag/untag endpoints Covers auth rejection, tagging, duplicate detection, untagging, ownership enforcement, and tag-not-found errors. --- tests/routers/openml/flow_tag_test.py | 123 +++++++++++++++++++++++ tests/routers/openml/run_tag_test.py | 134 ++++++++++++++++++++++++++ tests/routers/openml/task_tag_test.py | 117 ++++++++++++++++++++++ 3 files changed, 374 insertions(+) create mode 100644 tests/routers/openml/flow_tag_test.py create mode 100644 tests/routers/openml/run_tag_test.py create mode 100644 tests/routers/openml/task_tag_test.py diff --git a/tests/routers/openml/flow_tag_test.py b/tests/routers/openml/flow_tag_test.py new file mode 100644 index 00000000..76521604 --- /dev/null +++ b/tests/routers/openml/flow_tag_test.py @@ -0,0 +1,123 @@ +from http import HTTPStatus + +import pytest +from sqlalchemy import Connection +from starlette.testclient import TestClient + +from database.flows import get_tags +from tests.conftest import Flow +from tests.users import ApiKey + + +@pytest.mark.parametrize( + "key", + [None, ApiKey.INVALID], + ids=["no authentication", "invalid key"], +) +def test_flow_tag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: + apikey = "" if key is None else f"?api_key={key}" + response = py_api.post( + f"/flows/tag{apikey}", + json={"flow_id": 1, "tag": "test"}, + ) + assert response.status_code == HTTPStatus.PRECONDITION_FAILED + assert response.json()["detail"] == {"code": "103", "message": "Authentication failed"} + + +def test_flow_tag(flow: Flow, expdb_test: Connection, py_api: TestClient) -> None: + tag = "test" + response = py_api.post( + f"/flows/tag?api_key={ApiKey.ADMIN}", + json={"flow_id": flow.id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + assert response.json() == {"flow_tag": {"id": str(flow.id), "tag": [tag]}} + + tags = get_tags(flow_id=flow.id, expdb=expdb_test) + assert tag in tags + + +def test_flow_tag_returns_existing_tags(py_api: TestClient) -> None: + flow_id, tag = 1, "test" + response = py_api.post( + f"/flows/tag?api_key={ApiKey.ADMIN}", + json={"flow_id": flow_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + result = response.json() + assert result["flow_tag"]["id"] == str(flow_id) + assert "OpenmlWeka" in result["flow_tag"]["tag"] + assert "weka" in result["flow_tag"]["tag"] + assert tag in result["flow_tag"]["tag"] + + +def test_flow_tag_fails_if_tag_exists(py_api: TestClient) -> None: + flow_id, tag = 1, "OpenmlWeka" + response = py_api.post( + f"/flows/tag?api_key={ApiKey.ADMIN}", + json={"flow_id": flow_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + expected = { + "detail": { + "code": "473", + "message": "Entity already tagged by this tag.", + "additional_information": f"id={flow_id}; tag={tag}", + }, + } + assert expected == response.json() + + +@pytest.mark.parametrize( + "key", + [None, ApiKey.INVALID], + ids=["no authentication", "invalid key"], +) +def test_flow_untag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: + apikey = "" if key is None else f"?api_key={key}" + response = py_api.post( + f"/flows/untag{apikey}", + json={"flow_id": 1, "tag": "test"}, + ) + assert response.status_code == HTTPStatus.PRECONDITION_FAILED + assert response.json()["detail"] == {"code": "103", "message": "Authentication failed"} + + +def test_flow_untag(flow: Flow, expdb_test: Connection, py_api: TestClient) -> None: + tag = "test" + py_api.post( + f"/flows/tag?api_key={ApiKey.ADMIN}", + json={"flow_id": flow.id, "tag": tag}, + ) + response = py_api.post( + f"/flows/untag?api_key={ApiKey.ADMIN}", + json={"flow_id": flow.id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + assert response.json() == {"flow_tag": {"id": str(flow.id), "tag": []}} + + tags = get_tags(flow_id=flow.id, expdb=expdb_test) + assert tag not in tags + + +def test_flow_untag_fails_if_tag_not_found(py_api: TestClient) -> None: + response = py_api.post( + f"/flows/untag?api_key={ApiKey.ADMIN}", + json={"flow_id": 1, "tag": "nonexistent"}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.json()["detail"]["code"] == "477" + + +def test_flow_untag_fails_if_not_owner(flow: Flow, py_api: TestClient) -> None: + tag = "test" + py_api.post( + f"/flows/tag?api_key={ApiKey.ADMIN}", + json={"flow_id": flow.id, "tag": tag}, + ) + response = py_api.post( + f"/flows/untag?api_key={ApiKey.SOME_USER}", + json={"flow_id": flow.id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.json()["detail"]["code"] == "478" diff --git a/tests/routers/openml/run_tag_test.py b/tests/routers/openml/run_tag_test.py new file mode 100644 index 00000000..4dd7aff0 --- /dev/null +++ b/tests/routers/openml/run_tag_test.py @@ -0,0 +1,134 @@ +from http import HTTPStatus + +import pytest +from sqlalchemy import Connection, text +from starlette.testclient import TestClient + +from database.runs import get_tags +from tests.users import ApiKey + + +@pytest.fixture +def run_id(expdb_test: Connection) -> int: + expdb_test.execute( + text( + """ + INSERT INTO run(`uploader`, `task_id`, `setup`) + VALUES (1, 59, 1); + """, + ), + ) + (rid,) = expdb_test.execute(text("SELECT LAST_INSERT_ID();")).one() + return rid + + +@pytest.mark.parametrize( + "key", + [None, ApiKey.INVALID], + ids=["no authentication", "invalid key"], +) +def test_run_tag_rejects_unauthorized( + key: ApiKey, + run_id: int, + py_api: TestClient, +) -> None: + apikey = "" if key is None else f"?api_key={key}" + response = py_api.post( + f"/runs/tag{apikey}", + json={"run_id": run_id, "tag": "test"}, + ) + assert response.status_code == HTTPStatus.PRECONDITION_FAILED + assert response.json()["detail"] == {"code": "103", "message": "Authentication failed"} + + +def test_run_tag(run_id: int, expdb_test: Connection, py_api: TestClient) -> None: + tag = "test" + response = py_api.post( + f"/runs/tag?api_key={ApiKey.ADMIN}", + json={"run_id": run_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + assert response.json() == {"run_tag": {"id": str(run_id), "tag": [tag]}} + + tags = get_tags(id_=run_id, expdb=expdb_test) + assert tag in tags + + +def test_run_tag_fails_if_tag_exists(run_id: int, py_api: TestClient) -> None: + tag = "test" + py_api.post( + f"/runs/tag?api_key={ApiKey.ADMIN}", + json={"run_id": run_id, "tag": tag}, + ) + response = py_api.post( + f"/runs/tag?api_key={ApiKey.ADMIN}", + json={"run_id": run_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + expected = { + "detail": { + "code": "473", + "message": "Entity already tagged by this tag.", + "additional_information": f"id={run_id}; tag={tag}", + }, + } + assert expected == response.json() + + +@pytest.mark.parametrize( + "key", + [None, ApiKey.INVALID], + ids=["no authentication", "invalid key"], +) +def test_run_untag_rejects_unauthorized( + key: ApiKey, + run_id: int, + py_api: TestClient, +) -> None: + apikey = "" if key is None else f"?api_key={key}" + response = py_api.post( + f"/runs/untag{apikey}", + json={"run_id": run_id, "tag": "test"}, + ) + assert response.status_code == HTTPStatus.PRECONDITION_FAILED + assert response.json()["detail"] == {"code": "103", "message": "Authentication failed"} + + +def test_run_untag(run_id: int, expdb_test: Connection, py_api: TestClient) -> None: + tag = "test" + py_api.post( + f"/runs/tag?api_key={ApiKey.ADMIN}", + json={"run_id": run_id, "tag": tag}, + ) + response = py_api.post( + f"/runs/untag?api_key={ApiKey.ADMIN}", + json={"run_id": run_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + assert response.json() == {"run_tag": {"id": str(run_id), "tag": []}} + + tags = get_tags(id_=run_id, expdb=expdb_test) + assert tag not in tags + + +def test_run_untag_fails_if_tag_not_found(run_id: int, py_api: TestClient) -> None: + response = py_api.post( + f"/runs/untag?api_key={ApiKey.ADMIN}", + json={"run_id": run_id, "tag": "nonexistent"}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.json()["detail"]["code"] == "477" + + +def test_run_untag_fails_if_not_owner(run_id: int, py_api: TestClient) -> None: + tag = "test" + py_api.post( + f"/runs/tag?api_key={ApiKey.ADMIN}", + json={"run_id": run_id, "tag": tag}, + ) + response = py_api.post( + f"/runs/untag?api_key={ApiKey.SOME_USER}", + json={"run_id": run_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.json()["detail"]["code"] == "478" diff --git a/tests/routers/openml/task_tag_test.py b/tests/routers/openml/task_tag_test.py new file mode 100644 index 00000000..8da17b70 --- /dev/null +++ b/tests/routers/openml/task_tag_test.py @@ -0,0 +1,117 @@ +from http import HTTPStatus + +import pytest +from sqlalchemy import Connection +from starlette.testclient import TestClient + +from database.tasks import get_tags +from tests.users import ApiKey + + +@pytest.mark.parametrize( + "key", + [None, ApiKey.INVALID], + ids=["no authentication", "invalid key"], +) +def test_task_tag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: + apikey = "" if key is None else f"?api_key={key}" + response = py_api.post( + f"/tasks/tag{apikey}", + json={"task_id": 59, "tag": "test"}, + ) + assert response.status_code == HTTPStatus.PRECONDITION_FAILED + assert response.json()["detail"] == {"code": "103", "message": "Authentication failed"} + + +@pytest.mark.parametrize( + "key", + [ApiKey.ADMIN, ApiKey.SOME_USER, ApiKey.OWNER_USER], + ids=["administrator", "non-owner", "owner"], +) +def test_task_tag(key: ApiKey, expdb_test: Connection, py_api: TestClient) -> None: + task_id, tag = 59, "test" + response = py_api.post( + f"/tasks/tag?api_key={key}", + json={"task_id": task_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + assert response.json() == {"task_tag": {"id": str(task_id), "tag": [tag]}} + + tags = get_tags(id_=task_id, expdb=expdb_test) + assert tag in tags + + +def test_task_tag_fails_if_tag_exists(py_api: TestClient) -> None: + task_id, tag = 59, "test" + py_api.post( + f"/tasks/tag?api_key={ApiKey.ADMIN}", + json={"task_id": task_id, "tag": tag}, + ) + response = py_api.post( + f"/tasks/tag?api_key={ApiKey.ADMIN}", + json={"task_id": task_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + expected = { + "detail": { + "code": "473", + "message": "Entity already tagged by this tag.", + "additional_information": f"id={task_id}; tag={tag}", + }, + } + assert expected == response.json() + + +@pytest.mark.parametrize( + "key", + [None, ApiKey.INVALID], + ids=["no authentication", "invalid key"], +) +def test_task_untag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: + apikey = "" if key is None else f"?api_key={key}" + response = py_api.post( + f"/tasks/untag{apikey}", + json={"task_id": 59, "tag": "test"}, + ) + assert response.status_code == HTTPStatus.PRECONDITION_FAILED + assert response.json()["detail"] == {"code": "103", "message": "Authentication failed"} + + +def test_task_untag(expdb_test: Connection, py_api: TestClient) -> None: + task_id, tag = 59, "test" + py_api.post( + f"/tasks/tag?api_key={ApiKey.ADMIN}", + json={"task_id": task_id, "tag": tag}, + ) + response = py_api.post( + f"/tasks/untag?api_key={ApiKey.ADMIN}", + json={"task_id": task_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + assert response.json() == {"task_tag": {"id": str(task_id), "tag": []}} + + tags = get_tags(id_=task_id, expdb=expdb_test) + assert tag not in tags + + +def test_task_untag_fails_if_tag_not_found(py_api: TestClient) -> None: + response = py_api.post( + f"/tasks/untag?api_key={ApiKey.ADMIN}", + json={"task_id": 59, "tag": "nonexistent"}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.json()["detail"]["code"] == "477" + + +def test_task_untag_fails_if_not_owner(py_api: TestClient) -> None: + task_id, tag = 59, "test" + py_api.post( + f"/tasks/tag?api_key={ApiKey.ADMIN}", + json={"task_id": task_id, "tag": tag}, + ) + response = py_api.post( + f"/tasks/untag?api_key={ApiKey.SOME_USER}", + json={"task_id": task_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.json()["detail"]["code"] == "478" From 59a79273e64e7ca788b1dd0647e17fa9dfda5bf4 Mon Sep 17 00:00:00 2001 From: yassinnouh21 Date: Sat, 21 Feb 2026 16:25:29 +0100 Subject: [PATCH 4/5] Fix mypy no-any-return in run_tag_test fixture --- tests/routers/openml/run_tag_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/routers/openml/run_tag_test.py b/tests/routers/openml/run_tag_test.py index 4dd7aff0..b3e41460 100644 --- a/tests/routers/openml/run_tag_test.py +++ b/tests/routers/openml/run_tag_test.py @@ -19,7 +19,7 @@ def run_id(expdb_test: Connection) -> int: ), ) (rid,) = expdb_test.execute(text("SELECT LAST_INSERT_ID();")).one() - return rid + return int(rid) @pytest.mark.parametrize( From 18e713f145f83568951fe674da86eb9bc1b0ad8e Mon Sep 17 00:00:00 2001 From: yassinnouh21 Date: Sat, 21 Feb 2026 22:16:54 +0100 Subject: [PATCH 5/5] Address review feedback - Re-read tags from DB after insert instead of appending locally - Fix type annotations: key param is ApiKey | None when parametrize includes None - Assert setup POST responses in tests to catch silent failures - Add test for non-admin user untagging their own tag --- src/routers/openml/flows.py | 3 ++- src/routers/openml/runs.py | 3 ++- src/routers/openml/tasks.py | 3 ++- tests/routers/openml/flow_tag_test.py | 10 +++++---- tests/routers/openml/run_tag_test.py | 13 +++++++----- tests/routers/openml/task_tag_test.py | 30 ++++++++++++++++++++++----- 6 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/routers/openml/flows.py b/src/routers/openml/flows.py index d8e208a7..b6bfa887 100644 --- a/src/routers/openml/flows.py +++ b/src/routers/openml/flows.py @@ -37,7 +37,8 @@ def tag_flow( }, ) database.flows.tag(flow_id, tag, user_id=user.user_id, connection=expdb) - return {"flow_tag": {"id": str(flow_id), "tag": [*tags, tag]}} + tags = database.flows.get_tags(flow_id, expdb) + return {"flow_tag": {"id": str(flow_id), "tag": tags}} @router.post(path="/untag") diff --git a/src/routers/openml/runs.py b/src/routers/openml/runs.py index a81875ae..68bc75c4 100644 --- a/src/routers/openml/runs.py +++ b/src/routers/openml/runs.py @@ -35,7 +35,8 @@ def tag_run( }, ) database.runs.tag(run_id, tag, user_id=user.user_id, connection=expdb) - return {"run_tag": {"id": str(run_id), "tag": [*tags, tag]}} + tags = database.runs.get_tags(run_id, expdb) + return {"run_tag": {"id": str(run_id), "tag": tags}} @router.post(path="/untag") diff --git a/src/routers/openml/tasks.py b/src/routers/openml/tasks.py index 4b1f8911..32ebda21 100644 --- a/src/routers/openml/tasks.py +++ b/src/routers/openml/tasks.py @@ -174,7 +174,8 @@ def tag_task( }, ) database.tasks.tag(task_id, tag, user_id=user.user_id, connection=expdb) - return {"task_tag": {"id": str(task_id), "tag": [*tags, tag]}} + tags = database.tasks.get_tags(task_id, expdb) + return {"task_tag": {"id": str(task_id), "tag": tags}} @router.post(path="/untag") diff --git a/tests/routers/openml/flow_tag_test.py b/tests/routers/openml/flow_tag_test.py index 76521604..f9f3e1e4 100644 --- a/tests/routers/openml/flow_tag_test.py +++ b/tests/routers/openml/flow_tag_test.py @@ -14,7 +14,7 @@ [None, ApiKey.INVALID], ids=["no authentication", "invalid key"], ) -def test_flow_tag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: +def test_flow_tag_rejects_unauthorized(key: ApiKey | None, py_api: TestClient) -> None: apikey = "" if key is None else f"?api_key={key}" response = py_api.post( f"/flows/tag{apikey}", @@ -73,7 +73,7 @@ def test_flow_tag_fails_if_tag_exists(py_api: TestClient) -> None: [None, ApiKey.INVALID], ids=["no authentication", "invalid key"], ) -def test_flow_untag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: +def test_flow_untag_rejects_unauthorized(key: ApiKey | None, py_api: TestClient) -> None: apikey = "" if key is None else f"?api_key={key}" response = py_api.post( f"/flows/untag{apikey}", @@ -85,10 +85,11 @@ def test_flow_untag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> Non def test_flow_untag(flow: Flow, expdb_test: Connection, py_api: TestClient) -> None: tag = "test" - py_api.post( + setup = py_api.post( f"/flows/tag?api_key={ApiKey.ADMIN}", json={"flow_id": flow.id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/flows/untag?api_key={ApiKey.ADMIN}", json={"flow_id": flow.id, "tag": tag}, @@ -111,10 +112,11 @@ def test_flow_untag_fails_if_tag_not_found(py_api: TestClient) -> None: def test_flow_untag_fails_if_not_owner(flow: Flow, py_api: TestClient) -> None: tag = "test" - py_api.post( + setup = py_api.post( f"/flows/tag?api_key={ApiKey.ADMIN}", json={"flow_id": flow.id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/flows/untag?api_key={ApiKey.SOME_USER}", json={"flow_id": flow.id, "tag": tag}, diff --git a/tests/routers/openml/run_tag_test.py b/tests/routers/openml/run_tag_test.py index b3e41460..1fe2fb9c 100644 --- a/tests/routers/openml/run_tag_test.py +++ b/tests/routers/openml/run_tag_test.py @@ -28,7 +28,7 @@ def run_id(expdb_test: Connection) -> int: ids=["no authentication", "invalid key"], ) def test_run_tag_rejects_unauthorized( - key: ApiKey, + key: ApiKey | None, run_id: int, py_api: TestClient, ) -> None: @@ -56,10 +56,11 @@ def test_run_tag(run_id: int, expdb_test: Connection, py_api: TestClient) -> Non def test_run_tag_fails_if_tag_exists(run_id: int, py_api: TestClient) -> None: tag = "test" - py_api.post( + setup = py_api.post( f"/runs/tag?api_key={ApiKey.ADMIN}", json={"run_id": run_id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/runs/tag?api_key={ApiKey.ADMIN}", json={"run_id": run_id, "tag": tag}, @@ -81,7 +82,7 @@ def test_run_tag_fails_if_tag_exists(run_id: int, py_api: TestClient) -> None: ids=["no authentication", "invalid key"], ) def test_run_untag_rejects_unauthorized( - key: ApiKey, + key: ApiKey | None, run_id: int, py_api: TestClient, ) -> None: @@ -96,10 +97,11 @@ def test_run_untag_rejects_unauthorized( def test_run_untag(run_id: int, expdb_test: Connection, py_api: TestClient) -> None: tag = "test" - py_api.post( + setup = py_api.post( f"/runs/tag?api_key={ApiKey.ADMIN}", json={"run_id": run_id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/runs/untag?api_key={ApiKey.ADMIN}", json={"run_id": run_id, "tag": tag}, @@ -122,10 +124,11 @@ def test_run_untag_fails_if_tag_not_found(run_id: int, py_api: TestClient) -> No def test_run_untag_fails_if_not_owner(run_id: int, py_api: TestClient) -> None: tag = "test" - py_api.post( + setup = py_api.post( f"/runs/tag?api_key={ApiKey.ADMIN}", json={"run_id": run_id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/runs/untag?api_key={ApiKey.SOME_USER}", json={"run_id": run_id, "tag": tag}, diff --git a/tests/routers/openml/task_tag_test.py b/tests/routers/openml/task_tag_test.py index 8da17b70..33afa9d2 100644 --- a/tests/routers/openml/task_tag_test.py +++ b/tests/routers/openml/task_tag_test.py @@ -13,7 +13,7 @@ [None, ApiKey.INVALID], ids=["no authentication", "invalid key"], ) -def test_task_tag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: +def test_task_tag_rejects_unauthorized(key: ApiKey | None, py_api: TestClient) -> None: apikey = "" if key is None else f"?api_key={key}" response = py_api.post( f"/tasks/tag{apikey}", @@ -43,10 +43,11 @@ def test_task_tag(key: ApiKey, expdb_test: Connection, py_api: TestClient) -> No def test_task_tag_fails_if_tag_exists(py_api: TestClient) -> None: task_id, tag = 59, "test" - py_api.post( + setup = py_api.post( f"/tasks/tag?api_key={ApiKey.ADMIN}", json={"task_id": task_id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/tasks/tag?api_key={ApiKey.ADMIN}", json={"task_id": task_id, "tag": tag}, @@ -67,7 +68,7 @@ def test_task_tag_fails_if_tag_exists(py_api: TestClient) -> None: [None, ApiKey.INVALID], ids=["no authentication", "invalid key"], ) -def test_task_untag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: +def test_task_untag_rejects_unauthorized(key: ApiKey | None, py_api: TestClient) -> None: apikey = "" if key is None else f"?api_key={key}" response = py_api.post( f"/tasks/untag{apikey}", @@ -79,10 +80,11 @@ def test_task_untag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> Non def test_task_untag(expdb_test: Connection, py_api: TestClient) -> None: task_id, tag = 59, "test" - py_api.post( + setup = py_api.post( f"/tasks/tag?api_key={ApiKey.ADMIN}", json={"task_id": task_id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/tasks/untag?api_key={ApiKey.ADMIN}", json={"task_id": task_id, "tag": tag}, @@ -103,12 +105,30 @@ def test_task_untag_fails_if_tag_not_found(py_api: TestClient) -> None: assert response.json()["detail"]["code"] == "477" +def test_task_untag_non_admin_own_tag(expdb_test: Connection, py_api: TestClient) -> None: + task_id, tag = 59, "user_tag" + setup = py_api.post( + f"/tasks/tag?api_key={ApiKey.SOME_USER}", + json={"task_id": task_id, "tag": tag}, + ) + assert setup.status_code == HTTPStatus.OK + response = py_api.post( + f"/tasks/untag?api_key={ApiKey.SOME_USER}", + json={"task_id": task_id, "tag": tag}, + ) + assert response.status_code == HTTPStatus.OK + + tags = get_tags(id_=task_id, expdb=expdb_test) + assert tag not in tags + + def test_task_untag_fails_if_not_owner(py_api: TestClient) -> None: task_id, tag = 59, "test" - py_api.post( + setup = py_api.post( f"/tasks/tag?api_key={ApiKey.ADMIN}", json={"task_id": task_id, "tag": tag}, ) + assert setup.status_code == HTTPStatus.OK response = py_api.post( f"/tasks/untag?api_key={ApiKey.SOME_USER}", json={"task_id": task_id, "tag": tag},