From 885399d89ff795f221150d12290e6faff2f16a43 Mon Sep 17 00:00:00 2001 From: igennova Date: Mon, 20 Apr 2026 03:21:05 +0530 Subject: [PATCH 1/3] Add DELETE /users/{id} with no-resources rule --- src/core/errors.py | 16 ++ src/database/users.py | 38 +++++ src/main.py | 2 + src/routers/openml/users.py | 54 ++++++ tests/routers/openml/users_delete_test.py | 191 ++++++++++++++++++++++ 5 files changed, 301 insertions(+) create mode 100644 src/routers/openml/users.py create mode 100644 tests/routers/openml/users_delete_test.py diff --git a/src/core/errors.py b/src/core/errors.py index 69d7e0c7..fe111c58 100644 --- a/src/core/errors.py +++ b/src/core/errors.py @@ -228,6 +228,22 @@ class ForbiddenError(ProblemDetailError): _default_status_code = HTTPStatus.FORBIDDEN +class UserNotFoundError(ProblemDetailError): + """Raised when a user id does not exist in the user database.""" + + uri = "https://openml.org/problems/user-not-found" + title = "User Not Found" + _default_status_code = HTTPStatus.NOT_FOUND + + +class AccountHasResourcesError(ProblemDetailError): + """Raised when account deletion is blocked because the user still owns resources.""" + + uri = "https://openml.org/problems/account-has-resources" + title = "Account Has Active Resources" + _default_status_code = HTTPStatus.CONFLICT + + # ============================================================================= # Tag Errors # ============================================================================= diff --git a/src/database/users.py b/src/database/users.py index 0b09fb0d..2507abfd 100644 --- a/src/database/users.py +++ b/src/database/users.py @@ -76,3 +76,41 @@ async def get_groups(self) -> list[UserGroup]: async def is_admin(self) -> bool: return UserGroup.ADMIN in await self.get_groups() + + +async def exists_by_id(*, user_id: int, connection: AsyncConnection) -> bool: + row = await connection.execute( + text("SELECT 1 FROM users WHERE id = :user_id LIMIT 1"), + parameters={"user_id": user_id}, + ) + return row.one_or_none() is not None + + +async def count_uploaded_resources(*, user_id: int, expdb: AsyncConnection) -> int: + """Count datasets, flows, runs, and studies uploaded or created by this user (expdb).""" + row = await expdb.execute( + text( + """ + SELECT ( + (SELECT COUNT(*) FROM dataset WHERE uploader = :uid) + + (SELECT COUNT(*) FROM implementation WHERE uploader = :uid) + + (SELECT COUNT(*) FROM `run` WHERE uploader = :uid) + + (SELECT COUNT(*) FROM study WHERE creator = :uid) + ) AS total + """, + ), + parameters={"uid": user_id}, + ) + return int(row.scalar_one()) + + +async def delete_user_rows(*, user_id: int, userdb: AsyncConnection) -> None: + """Remove group memberships then the user row (openml user database).""" + await userdb.execute( + text("DELETE FROM users_groups WHERE user_id = :user_id"), + parameters={"user_id": user_id}, + ) + await userdb.execute( + text("DELETE FROM users WHERE id = :user_id"), + parameters={"user_id": user_id}, + ) diff --git a/src/main.py b/src/main.py index 46cd79cf..0219db8b 100644 --- a/src/main.py +++ b/src/main.py @@ -34,6 +34,7 @@ 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 +from routers.openml.users import router as users_router @asynccontextmanager @@ -106,6 +107,7 @@ def create_api(configuration_file: Path | None = None) -> FastAPI: app.include_router(study_router) app.include_router(setup_router) app.include_router(run_router) + app.include_router(users_router) logger.info("App setup completed.") logger.remove(setup_sink) diff --git a/src/routers/openml/users.py b/src/routers/openml/users.py new file mode 100644 index 00000000..838f8703 --- /dev/null +++ b/src/routers/openml/users.py @@ -0,0 +1,54 @@ +"""User account HTTP endpoints.""" + +from typing import Annotated + +from fastapi import APIRouter, Depends, Path, Response +from sqlalchemy.ext.asyncio import AsyncConnection + +import database.users +from core.errors import AccountHasResourcesError, ForbiddenError, UserNotFoundError +from database.users import User +from routers.dependencies import expdb_connection, fetch_user_or_raise, userdb_connection + +router = APIRouter(prefix="/users", tags=["users"]) + + +@router.delete( + "/{user_id}", + responses={ + 204: {"description": "User account deleted."}, + 401: {"description": "Authentication failed or missing."}, + 403: {"description": "Not allowed to delete this account."}, + 404: {"description": "User id not found."}, + 409: {"description": "User still has datasets, flows, runs, or studies."}, + }, +) +async def delete_user_account( + user_id: Annotated[int, Path(description="Numeric user id to delete.", gt=0)], + current_user: Annotated[User, Depends(fetch_user_or_raise)], + expdb: Annotated[AsyncConnection, Depends(expdb_connection)], + userdb: Annotated[AsyncConnection, Depends(userdb_connection)], +) -> Response: + """Delete a user account when they have no uploaded resources (Phase 1). + + Callers may delete their own account. Administrators may delete any account + that satisfies the no-resources rule. + """ + if current_user.user_id != user_id and not await current_user.is_admin(): + msg = "You may only delete your own user account." + raise ForbiddenError(msg) + + if not await database.users.exists_by_id(user_id=user_id, connection=userdb): + msg = f"User {user_id} not found." + raise UserNotFoundError(msg) + + resource_count = await database.users.count_uploaded_resources(user_id=user_id, expdb=expdb) + if resource_count > 0: + msg = ( + "Cannot delete this account while datasets, flows, runs, or studies " + "are still associated with the user. Remove or transfer them first." + ) + raise AccountHasResourcesError(msg) + + await database.users.delete_user_rows(user_id=user_id, userdb=userdb) + return Response(status_code=204) diff --git a/tests/routers/openml/users_delete_test.py b/tests/routers/openml/users_delete_test.py new file mode 100644 index 00000000..7903c69b --- /dev/null +++ b/tests/routers/openml/users_delete_test.py @@ -0,0 +1,191 @@ +"""Tests for DELETE /users/{user_id} (Phase 1: no resources, self or admin).""" + +import uuid +from http import HTTPStatus + +import httpx +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import AsyncConnection + +from core.errors import AccountHasResourcesError, ForbiddenError, UserNotFoundError +from routers.openml.users import delete_user_account +from tests.users import ADMIN_USER, SOME_USER, ApiKey + + +async def test_delete_user_missing_auth(py_api: httpx.AsyncClient) -> None: + response = await py_api.delete("/users/1") + assert response.status_code == HTTPStatus.UNAUTHORIZED + body = response.json() + assert body["code"] == "103" + assert body["detail"] == "No API key provided." + + +async def test_delete_user_not_found(py_api: httpx.AsyncClient) -> None: + response = await py_api.delete( + "/users/999999999", + params={"api_key": ApiKey.ADMIN}, + ) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.headers["content-type"] == "application/problem+json" + body = response.json() + assert body["type"] == UserNotFoundError.uri + assert body["detail"] == "User 999999999 not found." + + +async def test_delete_user_forbidden_non_admin_deletes_other( + py_api: httpx.AsyncClient, +) -> None: + response = await py_api.delete( + f"/users/{ADMIN_USER.user_id}", + params={"api_key": ApiKey.SOME_USER}, + ) + assert response.status_code == HTTPStatus.FORBIDDEN + body = response.json() + assert body["type"] == ForbiddenError.uri + assert body["detail"] == "You may only delete your own user account." + + +async def test_delete_user_conflict_when_user_has_resources( + py_api: httpx.AsyncClient, +) -> None: + """User 16 owns dataset 130 in the test database.""" + response = await py_api.delete( + "/users/16", + params={"api_key": ApiKey.ADMIN}, + ) + assert response.status_code == HTTPStatus.CONFLICT + assert response.headers["content-type"] == "application/problem+json" + body = response.json() + assert body["type"] == AccountHasResourcesError.uri + assert "datasets" in body["detail"] + + +@pytest.mark.mut +async def test_delete_user_api_success_self_delete( + py_api: httpx.AsyncClient, + user_test: AsyncConnection, +) -> None: + """Disposable user deletes their own account using their API key (session_hash).""" + api_key = "fedcba9876543210fedcba9876543210" + suffix = uuid.uuid4().hex[:10] + username = f"tmp_self_{suffix}" + email = f"{suffix}@openml-self-delete.test" + await user_test.execute( + text( + """ + INSERT INTO users ( + ip_address, username, password, email, created_on, + company, country, bio, session_hash + ) VALUES ( + '127.0.0.1', :username, 'x', :email, UNIX_TIMESTAMP(), + '', '', '', :api_key + ) + """, + ), + parameters={"username": username, "email": email, "api_key": api_key}, + ) + uid_row = await user_test.execute(text("SELECT LAST_INSERT_ID() AS id")) + (new_id,) = uid_row.one() + await user_test.execute( + text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, 2)"), + parameters={"uid": new_id}, + ) + + response = await py_api.delete( + f"/users/{new_id}", + params={"api_key": api_key}, + ) + assert response.status_code == HTTPStatus.NO_CONTENT + assert response.content == b"" + + exists = await user_test.execute( + text("SELECT 1 FROM users WHERE id = :id LIMIT 1"), + parameters={"id": new_id}, + ) + assert exists.one_or_none() is None + + +@pytest.mark.mut +async def test_delete_user_api_success_admin_deletes_disposable_user( + py_api: httpx.AsyncClient, + user_test: AsyncConnection, +) -> None: + suffix = uuid.uuid4().hex[:12] + username = f"tmp_del_{suffix}" + email = f"{suffix}@openml-delete.test" + await user_test.execute( + text( + """ + INSERT INTO users ( + ip_address, username, password, email, created_on, + company, country, bio + ) VALUES ( + '127.0.0.1', :username, 'x', :email, UNIX_TIMESTAMP(), + '', '', '' + ) + """, + ), + parameters={"username": username, "email": email}, + ) + uid_row = await user_test.execute(text("SELECT LAST_INSERT_ID() AS id")) + (new_id,) = uid_row.one() + await user_test.execute( + text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, 2)"), + parameters={"uid": new_id}, + ) + + response = await py_api.delete( + f"/users/{new_id}", + params={"api_key": ApiKey.ADMIN}, + ) + assert response.status_code == HTTPStatus.NO_CONTENT + assert response.content == b"" + + exists = await user_test.execute( + text("SELECT 1 FROM users WHERE id = :id LIMIT 1"), + parameters={"id": new_id}, + ) + assert exists.one_or_none() is None + + +# ── Direct handler tests ── + + +async def test_delete_user_direct_not_found( + user_test: AsyncConnection, + expdb_test: AsyncConnection, +) -> None: + with pytest.raises(UserNotFoundError, match=r"User 888888888 not found\."): + await delete_user_account( + user_id=888888888, + current_user=ADMIN_USER, + expdb=expdb_test, + userdb=user_test, + ) + + +async def test_delete_user_direct_forbidden( + user_test: AsyncConnection, + expdb_test: AsyncConnection, +) -> None: + with pytest.raises(ForbiddenError, match=r"You may only delete your own user account\."): + await delete_user_account( + user_id=ADMIN_USER.user_id, + current_user=SOME_USER, + expdb=expdb_test, + userdb=user_test, + ) + + +async def test_delete_user_direct_conflict_has_resources( + user_test: AsyncConnection, + expdb_test: AsyncConnection, +) -> None: + with pytest.raises(AccountHasResourcesError, match="Cannot delete this account"): + await delete_user_account( + user_id=16, + current_user=ADMIN_USER, + expdb=expdb_test, + userdb=user_test, + ) From e3b016dae79d918e8549aa6963229ded61905090 Mon Sep 17 00:00:00 2001 From: igennova Date: Mon, 20 Apr 2026 03:42:08 +0530 Subject: [PATCH 2/3] query update --- src/database/users.py | 29 ++++++++++++++++------- src/routers/openml/users.py | 23 +++++++++++------- tests/routers/openml/users_delete_test.py | 9 +++---- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/database/users.py b/src/database/users.py index 2507abfd..4f539685 100644 --- a/src/database/users.py +++ b/src/database/users.py @@ -86,22 +86,33 @@ async def exists_by_id(*, user_id: int, connection: AsyncConnection) -> bool: return row.one_or_none() is not None -async def count_uploaded_resources(*, user_id: int, expdb: AsyncConnection) -> int: - """Count datasets, flows, runs, and studies uploaded or created by this user (expdb).""" +async def has_user_references(*, user_id: int, expdb: AsyncConnection) -> bool: + """Return ``True`` if any ``expdb`` row still references ``user_id``.""" row = await expdb.execute( text( """ - SELECT ( - (SELECT COUNT(*) FROM dataset WHERE uploader = :uid) - + (SELECT COUNT(*) FROM implementation WHERE uploader = :uid) - + (SELECT COUNT(*) FROM `run` WHERE uploader = :uid) - + (SELECT COUNT(*) FROM study WHERE creator = :uid) - ) AS total + SELECT EXISTS ( + SELECT 1 FROM dataset WHERE uploader = :uid + UNION ALL SELECT 1 FROM dataset_description WHERE uploader = :uid + UNION ALL SELECT 1 FROM dataset_status WHERE user_id = :uid + UNION ALL SELECT 1 FROM dataset_tag WHERE uploader = :uid + UNION ALL SELECT 1 FROM dataset_topic WHERE uploader = :uid + UNION ALL SELECT 1 FROM implementation WHERE uploader = :uid + UNION ALL SELECT 1 FROM implementation_tag WHERE uploader = :uid + UNION ALL SELECT 1 FROM `run` WHERE uploader = :uid + UNION ALL SELECT 1 FROM run_study WHERE uploader = :uid + UNION ALL SELECT 1 FROM run_tag WHERE uploader = :uid + UNION ALL SELECT 1 FROM setup_tag WHERE uploader = :uid + UNION ALL SELECT 1 FROM study WHERE creator = :uid + UNION ALL SELECT 1 FROM task WHERE creator = :uid + UNION ALL SELECT 1 FROM task_study WHERE uploader = :uid + UNION ALL SELECT 1 FROM task_tag WHERE uploader = :uid + ) AS has_refs """, ), parameters={"uid": user_id}, ) - return int(row.scalar_one()) + return bool(row.scalar_one()) async def delete_user_rows(*, user_id: int, userdb: AsyncConnection) -> None: diff --git a/src/routers/openml/users.py b/src/routers/openml/users.py index 838f8703..40a0baa3 100644 --- a/src/routers/openml/users.py +++ b/src/routers/openml/users.py @@ -3,6 +3,7 @@ from typing import Annotated from fastapi import APIRouter, Depends, Path, Response +from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncConnection import database.users @@ -10,6 +11,11 @@ from database.users import User from routers.dependencies import expdb_connection, fetch_user_or_raise, userdb_connection +_ACCOUNT_HAS_RESOURCES_MSG = ( + "Cannot delete this account while records still reference the user " + "(datasets, flows, runs, studies, tags, etc.). Remove or transfer them first." +) + router = APIRouter(prefix="/users", tags=["users"]) @@ -42,13 +48,14 @@ async def delete_user_account( msg = f"User {user_id} not found." raise UserNotFoundError(msg) - resource_count = await database.users.count_uploaded_resources(user_id=user_id, expdb=expdb) - if resource_count > 0: - msg = ( - "Cannot delete this account while datasets, flows, runs, or studies " - "are still associated with the user. Remove or transfer them first." - ) - raise AccountHasResourcesError(msg) + if await database.users.has_user_references(user_id=user_id, expdb=expdb): + raise AccountHasResourcesError(_ACCOUNT_HAS_RESOURCES_MSG) + + try: + await database.users.delete_user_rows(user_id=user_id, userdb=userdb) + except IntegrityError as exc: + # Safety net: a user-reference table was added without updating + # ``has_user_references``. Surface a clean 409 instead of a 500. + raise AccountHasResourcesError(_ACCOUNT_HAS_RESOURCES_MSG) from exc - await database.users.delete_user_rows(user_id=user_id, userdb=userdb) return Response(status_code=204) diff --git a/tests/routers/openml/users_delete_test.py b/tests/routers/openml/users_delete_test.py index 7903c69b..407a427b 100644 --- a/tests/routers/openml/users_delete_test.py +++ b/tests/routers/openml/users_delete_test.py @@ -9,6 +9,7 @@ from sqlalchemy.ext.asyncio import AsyncConnection from core.errors import AccountHasResourcesError, ForbiddenError, UserNotFoundError +from database.users import UserGroup from routers.openml.users import delete_user_account from tests.users import ADMIN_USER, SOME_USER, ApiKey @@ -88,8 +89,8 @@ async def test_delete_user_api_success_self_delete( uid_row = await user_test.execute(text("SELECT LAST_INSERT_ID() AS id")) (new_id,) = uid_row.one() await user_test.execute( - text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, 2)"), - parameters={"uid": new_id}, + text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, :gid)"), + parameters={"uid": new_id, "gid": UserGroup.READ_WRITE.value}, ) response = await py_api.delete( @@ -131,8 +132,8 @@ async def test_delete_user_api_success_admin_deletes_disposable_user( uid_row = await user_test.execute(text("SELECT LAST_INSERT_ID() AS id")) (new_id,) = uid_row.one() await user_test.execute( - text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, 2)"), - parameters={"uid": new_id}, + text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, :gid)"), + parameters={"uid": new_id, "gid": UserGroup.READ_WRITE.value}, ) response = await py_api.delete( From 96f31223d7c0535fd3ffacd4d1074eb091784c61 Mon Sep 17 00:00:00 2001 From: igennova Date: Tue, 28 Apr 2026 22:53:51 +0530 Subject: [PATCH 3/3] suggested changes --- src/routers/openml/users.py | 15 +- tests/routers/openml/users_delete_test.py | 178 ++++++++++++---------- 2 files changed, 109 insertions(+), 84 deletions(-) diff --git a/src/routers/openml/users.py b/src/routers/openml/users.py index 40a0baa3..821ea5e8 100644 --- a/src/routers/openml/users.py +++ b/src/routers/openml/users.py @@ -3,6 +3,7 @@ from typing import Annotated from fastapi import APIRouter, Depends, Path, Response +from loguru import logger from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncConnection @@ -35,10 +36,11 @@ async def delete_user_account( expdb: Annotated[AsyncConnection, Depends(expdb_connection)], userdb: Annotated[AsyncConnection, Depends(userdb_connection)], ) -> Response: - """Delete a user account when they have no uploaded resources (Phase 1). + """Delete the user account if they have no associated resources. - Callers may delete their own account. Administrators may delete any account - that satisfies the no-resources rule. + The account to be deleted must not have associated resources (such as + datasets, tasks, or tags). Users may only delete their own account. + Administrators may delete any account that satisfies the no-resources rule. """ if current_user.user_id != user_id and not await current_user.is_admin(): msg = "You may only delete your own user account." @@ -54,8 +56,11 @@ async def delete_user_account( try: await database.users.delete_user_rows(user_id=user_id, userdb=userdb) except IntegrityError as exc: - # Safety net: a user-reference table was added without updating - # ``has_user_references``. Surface a clean 409 instead of a 500. + logger.error( + "Delete of user {user_id} failed with integrity error after pre-check.", + user_id=user_id, + ) raise AccountHasResourcesError(_ACCOUNT_HAS_RESOURCES_MSG) from exc + logger.info("User account {user_id} was removed.", user_id=user_id) return Response(status_code=204) diff --git a/tests/routers/openml/users_delete_test.py b/tests/routers/openml/users_delete_test.py index 407a427b..7780ca1e 100644 --- a/tests/routers/openml/users_delete_test.py +++ b/tests/routers/openml/users_delete_test.py @@ -1,11 +1,15 @@ """Tests for DELETE /users/{user_id} (Phase 1: no resources, self or admin).""" import uuid +from collections.abc import AsyncGenerator from http import HTTPStatus +from typing import NamedTuple import httpx import pytest +import pytest_mock from sqlalchemy import text +from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncConnection from core.errors import AccountHasResourcesError, ForbiddenError, UserNotFoundError @@ -22,56 +26,18 @@ async def test_delete_user_missing_auth(py_api: httpx.AsyncClient) -> None: assert body["detail"] == "No API key provided." -async def test_delete_user_not_found(py_api: httpx.AsyncClient) -> None: - response = await py_api.delete( - "/users/999999999", - params={"api_key": ApiKey.ADMIN}, - ) - assert response.status_code == HTTPStatus.NOT_FOUND - assert response.headers["content-type"] == "application/problem+json" - body = response.json() - assert body["type"] == UserNotFoundError.uri - assert body["detail"] == "User 999999999 not found." - - -async def test_delete_user_forbidden_non_admin_deletes_other( - py_api: httpx.AsyncClient, -) -> None: - response = await py_api.delete( - f"/users/{ADMIN_USER.user_id}", - params={"api_key": ApiKey.SOME_USER}, - ) - assert response.status_code == HTTPStatus.FORBIDDEN - body = response.json() - assert body["type"] == ForbiddenError.uri - assert body["detail"] == "You may only delete your own user account." +class DisposableUser(NamedTuple): + user_id: int + api_key: str -async def test_delete_user_conflict_when_user_has_resources( - py_api: httpx.AsyncClient, -) -> None: - """User 16 owns dataset 130 in the test database.""" - response = await py_api.delete( - "/users/16", - params={"api_key": ApiKey.ADMIN}, - ) - assert response.status_code == HTTPStatus.CONFLICT - assert response.headers["content-type"] == "application/problem+json" - body = response.json() - assert body["type"] == AccountHasResourcesError.uri - assert "datasets" in body["detail"] - - -@pytest.mark.mut -async def test_delete_user_api_success_self_delete( - py_api: httpx.AsyncClient, - user_test: AsyncConnection, -) -> None: - """Disposable user deletes their own account using their API key (session_hash).""" - api_key = "fedcba9876543210fedcba9876543210" +@pytest.fixture +async def disposable_user(user_test: AsyncConnection) -> AsyncGenerator[DisposableUser]: + api_key = uuid.uuid4().hex suffix = uuid.uuid4().hex[:10] - username = f"tmp_self_{suffix}" - email = f"{suffix}@openml-self-delete.test" + username = f"tmp_user_{suffix}" + email = f"{suffix}@openml-delete.test" + await user_test.execute( text( """ @@ -92,17 +58,33 @@ async def test_delete_user_api_success_self_delete( text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, :gid)"), parameters={"uid": new_id, "gid": UserGroup.READ_WRITE.value}, ) + yield DisposableUser(user_id=new_id, api_key=api_key) + await user_test.execute( + text("DELETE FROM users_groups WHERE user_id = :uid"), + parameters={"uid": new_id}, + ) + await user_test.execute( + text("DELETE FROM users WHERE id = :uid"), + parameters={"uid": new_id}, + ) + +@pytest.mark.mut +async def test_delete_user_api_success_self_delete( + py_api: httpx.AsyncClient, + user_test: AsyncConnection, + disposable_user: DisposableUser, +) -> None: response = await py_api.delete( - f"/users/{new_id}", - params={"api_key": api_key}, + f"/users/{disposable_user.user_id}", + params={"api_key": disposable_user.api_key}, ) assert response.status_code == HTTPStatus.NO_CONTENT assert response.content == b"" exists = await user_test.execute( text("SELECT 1 FROM users WHERE id = :id LIMIT 1"), - parameters={"id": new_id}, + parameters={"id": disposable_user.user_id}, ) assert exists.one_or_none() is None @@ -111,33 +93,10 @@ async def test_delete_user_api_success_self_delete( async def test_delete_user_api_success_admin_deletes_disposable_user( py_api: httpx.AsyncClient, user_test: AsyncConnection, + disposable_user: DisposableUser, ) -> None: - suffix = uuid.uuid4().hex[:12] - username = f"tmp_del_{suffix}" - email = f"{suffix}@openml-delete.test" - await user_test.execute( - text( - """ - INSERT INTO users ( - ip_address, username, password, email, created_on, - company, country, bio - ) VALUES ( - '127.0.0.1', :username, 'x', :email, UNIX_TIMESTAMP(), - '', '', '' - ) - """, - ), - parameters={"username": username, "email": email}, - ) - uid_row = await user_test.execute(text("SELECT LAST_INSERT_ID() AS id")) - (new_id,) = uid_row.one() - await user_test.execute( - text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, :gid)"), - parameters={"uid": new_id, "gid": UserGroup.READ_WRITE.value}, - ) - response = await py_api.delete( - f"/users/{new_id}", + f"/users/{disposable_user.user_id}", params={"api_key": ApiKey.ADMIN}, ) assert response.status_code == HTTPStatus.NO_CONTENT @@ -145,7 +104,7 @@ async def test_delete_user_api_success_admin_deletes_disposable_user( exists = await user_test.execute( text("SELECT 1 FROM users WHERE id = :id LIMIT 1"), - parameters={"id": new_id}, + parameters={"id": disposable_user.user_id}, ) assert exists.one_or_none() is None @@ -157,36 +116,97 @@ async def test_delete_user_direct_not_found( user_test: AsyncConnection, expdb_test: AsyncConnection, ) -> None: - with pytest.raises(UserNotFoundError, match=r"User 888888888 not found\."): + with pytest.raises(UserNotFoundError, match=r"User 888888888 not found\.") as exc_info: await delete_user_account( user_id=888888888, current_user=ADMIN_USER, expdb=expdb_test, userdb=user_test, ) + assert exc_info.value.status_code == HTTPStatus.NOT_FOUND + assert exc_info.value.uri == UserNotFoundError.uri async def test_delete_user_direct_forbidden( user_test: AsyncConnection, expdb_test: AsyncConnection, ) -> None: - with pytest.raises(ForbiddenError, match=r"You may only delete your own user account\."): + with pytest.raises( + ForbiddenError, match=r"You may only delete your own user account\." + ) as exc_info: await delete_user_account( user_id=ADMIN_USER.user_id, current_user=SOME_USER, expdb=expdb_test, userdb=user_test, ) + assert exc_info.value.status_code == HTTPStatus.FORBIDDEN + assert exc_info.value.uri == ForbiddenError.uri async def test_delete_user_direct_conflict_has_resources( user_test: AsyncConnection, expdb_test: AsyncConnection, ) -> None: - with pytest.raises(AccountHasResourcesError, match="Cannot delete this account"): + with pytest.raises(AccountHasResourcesError, match="Cannot delete this account") as exc_info: await delete_user_account( user_id=16, current_user=ADMIN_USER, expdb=expdb_test, userdb=user_test, ) + assert exc_info.value.status_code == HTTPStatus.CONFLICT + assert exc_info.value.uri == AccountHasResourcesError.uri + + +@pytest.mark.mut +async def test_delete_user_direct_success_logs_info( + user_test: AsyncConnection, + expdb_test: AsyncConnection, + disposable_user: DisposableUser, + mocker: pytest_mock.MockerFixture, +) -> None: + log_info = mocker.patch("routers.openml.users.logger.info") + + response = await delete_user_account( + user_id=disposable_user.user_id, + current_user=ADMIN_USER, + expdb=expdb_test, + userdb=user_test, + ) + + assert response.status_code == HTTPStatus.NO_CONTENT + log_info.assert_called_once_with( + "User account {user_id} was removed.", + user_id=disposable_user.user_id, + ) + + +@pytest.mark.mut +async def test_delete_user_integrity_error_logs_and_raises_conflict( + user_test: AsyncConnection, + expdb_test: AsyncConnection, + disposable_user: DisposableUser, + mocker: pytest_mock.MockerFixture, +) -> None: + mocker.patch( + "database.users.delete_user_rows", + side_effect=IntegrityError( + "DELETE FROM users", {"user_id": disposable_user.user_id}, Exception("fk") + ), + ) + log_error = mocker.patch("routers.openml.users.logger.error") + + with pytest.raises(AccountHasResourcesError, match="Cannot delete this account") as exc_info: + await delete_user_account( + user_id=disposable_user.user_id, + current_user=ADMIN_USER, + expdb=expdb_test, + userdb=user_test, + ) + + assert exc_info.value.status_code == HTTPStatus.CONFLICT + log_error.assert_called_once_with( + "Delete of user {user_id} failed with integrity error after pre-check.", + user_id=disposable_user.user_id, + )