refactor delete

Author: giancarloromeo

services/web/server/src/simcore_service_webserver/api_keys/_api.py

@@ -103,12 +103,12 @@ async def get_or_create_api_key(
 async def delete_api_key(
     app: web.Application,
     *,
-    name: str,
+    api_key_id: int,
     user_id: UserID,
     product_name: ProductName,
 ) -> None:
-    await db.delete_by_name(
-        app, display_name=name, user_id=user_id, product_name=product_name
+    await db.delete(
+        app, api_key_id=api_key_id, user_id=user_id, product_name=product_name
     )
 
 

services/web/server/src/simcore_service_webserver/api_keys/_db.py

@@ -123,6 +123,23 @@ async def get_or_create(
         return ApiKeyInDB.model_validate(row)
 
 
+async def delete(
+    app: web.Application,
+    connection: AsyncConnection | None = None,
+    *,
+    api_key_id: int,
+    user_id: UserID,
+    product_name: ProductName,
+) -> None:
+    async with transaction_context(get_asyncpg_engine(app), connection) as conn:
+        stmt = api_keys.delete().where(
+            (api_keys.c.user_id == user_id)
+            & (api_keys.c.id == api_key_id)
+            & (api_keys.c.product_name == product_name)
+        )
+        await conn.execute(stmt)
+
+
 async def delete_by_name(
     app: web.Application,
     connection: AsyncConnection | None = None,

services/web/server/src/simcore_service_webserver/api_keys/_models.py

@@ -0,0 +1,5 @@
+from models_library.rest_base import StrictRequestParameters
+
+
+class ApiKeysPathParams(StrictRequestParameters):
+    api_key_id: int

services/web/server/src/simcore_service_webserver/api_keys/_rest.py

@@ -4,9 +4,13 @@
 from aiohttp.web import RouteTableDef
 from models_library.api_schemas_webserver.auth import ApiKeyCreate
 from servicelib.aiohttp import status
-from servicelib.aiohttp.requests_validation import parse_request_body_as
+from servicelib.aiohttp.requests_validation import (
+    parse_request_body_as,
+    parse_request_path_parameters_as,
+)
 from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON
 from simcore_postgres_database.errors import DatabaseError
+from simcore_service_webserver.api_keys._models import ApiKeysPathParams
 
 from .._meta import API_VTAG
 from ..login.decorators import login_required
@@ -56,26 +60,25 @@ async def create_api_key(request: web.Request):
     return envelope_json_response(data)
 
 
-@routes.delete(f"/{API_VTAG}/auth/api-keys", name="delete_api_key")
+@routes.delete(f"/{API_VTAG}/auth/api-keys/{{api_key_id}}", name="delete_api_key")
 @login_required
 @permission_required("user.apikey.*")
 async def delete_api_key(request: web.Request):
     req_ctx = RequestContext.model_validate(request)
-
-    # NOTE: SEE https://github.com/ITISFoundation/osparc-simcore/issues/4920
-    body = await request.json()
-    name = body.get("display_name")
+    path_params = parse_request_path_parameters_as(ApiKeysPathParams, request)
 
     try:
         await _api.delete_api_key(
             request.app,
-            name=name,
+            api_key_id=path_params.api_key_id,
             user_id=req_ctx.user_id,
             product_name=req_ctx.product_name,
         )
     except DatabaseError as err:
         _logger.warning(
-            "Failed to delete API key %s. Ignoring error", name, exc_info=err
+            "Failed to delete API key with ID: %s. Ignoring error",
+            path_params.api_key_id,
+            exc_info=err,
         )
 
     return web.json_response(status=status.HTTP_204_NO_CONTENT)

services/web/server/tests/unit/with_dbs/01/test_api_keys.py

@@ -11,6 +11,7 @@
 import pytest
 import simcore_service_webserver.api_keys._db as db
 from aiohttp.test_utils import TestClient
+from faker import Faker
 from models_library.products import ProductName
 from pytest_simcore.helpers.assert_checks import assert_status
 from pytest_simcore.helpers.webserver_login import NewUser, UserInfoDict
@@ -28,27 +29,28 @@ async def fake_user_api_keys(
     client: TestClient,
     logged_user: UserInfoDict,
     osparc_product_name: ProductName,
+    faker: Faker,
 ) -> AsyncIterable[list[str]]:
     assert client.app
-    names = ["foo", "bar", "beta", "alpha"]
+    api_key_ids = []
 
-    for name in names:
-        await db.create(
+    for _ in range(5):
+        api_key_ids += await db.create(
             client.app,
             user_id=logged_user["id"],
             product_name=osparc_product_name,
-            display_name=name,
+            display_name=faker.pystr(),
             expiration=None,
-            api_key=f"{name}-key",
-            api_secret=f"{name}-secret",
+            api_key=faker.pystr(),
+            api_secret=faker.pystr(),
         )
 
-    yield names
+    yield api_key_ids
 
-    for name in names:
-        await db.delete_by_name(
+    for api_key_id in api_key_ids:
+        await db.delete(
             client.app,
-            display_name=name,
+            api_key_id=api_key_id,
             user_id=logged_user["id"],
             product_name=osparc_product_name,
         )
@@ -122,11 +124,11 @@ async def test_delete_api_keys(
     expected: HTTPStatus,
     disable_gc_manual_guest_users: None,
 ):
-    resp = await client.delete("/v0/auth/api-keys", json={"display_name": "foo"})
+    resp = await client.delete("/v0/auth/api-keys/0")
     await assert_status(resp, expected)
 
-    for name in fake_user_api_keys:
-        resp = await client.delete("/v0/auth/api-keys", json={"display_name": name})
+    for api_key_id in fake_user_api_keys:
+        resp = await client.delete(f"/v0/auth/api-keys/{api_key_id}")
         await assert_status(resp, expected)