From 299f63e902d276af41a6c23ada0647bc87e6dc9c Mon Sep 17 00:00:00 2001 From: Giancarlo Romeo Date: Tue, 9 Sep 2025 22:45:38 +0200 Subject: [PATCH 1/5] feat: check associated jobs when deleting a function --- api/specs/web-server/_functions.py | 2 ++ .../src/models_library/functions_errors.py | 7 +++++++ .../functions/_controller/_functions_rest.py | 9 ++++++++- .../_controller/_functions_rest_schemas.py | 9 +++++++++ .../functions/_functions_repository.py | 16 ++++++++++++++++ .../functions/_functions_service.py | 2 ++ 6 files changed, 44 insertions(+), 1 deletion(-) diff --git a/api/specs/web-server/_functions.py b/api/specs/web-server/_functions.py index 47c11cb6d43..a58450703ff 100644 --- a/api/specs/web-server/_functions.py +++ b/api/specs/web-server/_functions.py @@ -23,6 +23,7 @@ FunctionGroupPathParams, ) from simcore_service_webserver.functions._controller._functions_rest_schemas import ( + FunctionDeleteQueryParams, FunctionGetQueryParams, FunctionPathParams, FunctionsListQueryParams, @@ -80,6 +81,7 @@ async def update_function( ) async def delete_function( _path: Annotated[FunctionPathParams, Depends()], + _query: Annotated[as_query(FunctionDeleteQueryParams), Depends()], ): ... diff --git a/packages/models-library/src/models_library/functions_errors.py b/packages/models-library/src/models_library/functions_errors.py index 6c112591c07..629c3dc1c7e 100644 --- a/packages/models-library/src/models_library/functions_errors.py +++ b/packages/models-library/src/models_library/functions_errors.py @@ -17,6 +17,13 @@ class FunctionIDNotFoundError(FunctionBaseError): status_code: int = 404 # Not Found +class FunctionHasJobsCannotDeleteError(FunctionBaseError): + msg_template: str = ( + "Cannot delete function {function_id} because it has {jobs_count} associated job(s)." + ) + status_code: int = 409 # Conflict + + class FunctionJobIDNotFoundError(FunctionBaseError): msg_template: str = "Function job {function_job_id} not found" status_code: int = 404 # Not Found diff --git a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest.py b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest.py index 3179231c4a5..d1be5c5928c 100644 --- a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest.py +++ b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest.py @@ -44,6 +44,7 @@ from .._services_metadata.proxy import ServiceMetadata from ._functions_rest_exceptions import handle_rest_requests_exceptions from ._functions_rest_schemas import ( + FunctionDeleteQueryParams, FunctionFilters, FunctionGetQueryParams, FunctionGroupPathParams, @@ -370,12 +371,18 @@ async def update_function(request: web.Request) -> web.Response: async def delete_function(request: web.Request) -> web.Response: path_params = parse_request_path_parameters_as(FunctionPathParams, request) function_id = path_params.function_id + + query_params: FunctionDeleteQueryParams = parse_request_query_parameters_as( + FunctionDeleteQueryParams, request + ) + req_ctx = AuthenticatedRequestContext.model_validate(request) await _functions_service.delete_function( app=request.app, - function_id=function_id, user_id=req_ctx.user_id, product_name=req_ctx.product_name, + function_id=function_id, + force=query_params.force, ) return web.json_response(status=status.HTTP_204_NO_CONTENT) diff --git a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py index 3fd09ab93fd..d67e4bab3e2 100644 --- a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py +++ b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py @@ -79,4 +79,13 @@ class FunctionsListQueryParams( ): ... +class FunctionDeleteQueryParams(BaseModel): + force: Annotated[ + bool, + Field( + description="If true, deletes the function even if it has associated jobs.", + ), + ] = False + + __all__: tuple[str, ...] = ("AuthenticatedRequestContext",) diff --git a/services/web/server/src/simcore_service_webserver/functions/_functions_repository.py b/services/web/server/src/simcore_service_webserver/functions/_functions_repository.py index cf80e7cf7d2..8a6543eb9a1 100644 --- a/services/web/server/src/simcore_service_webserver/functions/_functions_repository.py +++ b/services/web/server/src/simcore_service_webserver/functions/_functions_repository.py @@ -34,6 +34,7 @@ from models_library.functions_errors import ( FunctionBaseError, FunctionExecuteAccessDeniedError, + FunctionHasJobsCannotDeleteError, FunctionIDNotFoundError, FunctionJobCollectionExecuteAccessDeniedError, FunctionJobCollectionIDNotFoundError, @@ -819,6 +820,7 @@ async def delete_function( user_id: UserID, product_name: ProductName, function_id: FunctionID, + force: bool = False, ) -> None: async with transaction_context(get_asyncpg_engine(app), connection) as transaction: await check_user_permissions( @@ -840,6 +842,20 @@ async def delete_function( if row is None: raise FunctionIDNotFoundError(function_id=function_id) + # Check for existing function jobs if force is not True + if not force: + jobs_result = await transaction.execute( + function_jobs_table.select() + .with_only_columns(func.count()) + .where(function_jobs_table.c.function_uuid == function_id) + ) + jobs_count = jobs_result.scalar() or 0 + + if jobs_count > 0: + raise FunctionHasJobsCannotDeleteError( + function_id=function_id, jobs_count=jobs_count + ) + # Proceed with deletion await transaction.execute( functions_table.delete().where(functions_table.c.uuid == function_id) diff --git a/services/web/server/src/simcore_service_webserver/functions/_functions_service.py b/services/web/server/src/simcore_service_webserver/functions/_functions_service.py index 74cbf189484..d7e767f101d 100644 --- a/services/web/server/src/simcore_service_webserver/functions/_functions_service.py +++ b/services/web/server/src/simcore_service_webserver/functions/_functions_service.py @@ -353,12 +353,14 @@ async def delete_function( user_id: UserID, product_name: ProductName, function_id: FunctionID, + force: bool = False, ) -> None: await _functions_repository.delete_function( app=app, user_id=user_id, product_name=product_name, function_id=function_id, + force=force, ) From 59a4d504b3527292e0933c8077fdc7a5eb58b2e5 Mon Sep 17 00:00:00 2001 From: Giancarlo Romeo Date: Wed, 10 Sep 2025 09:39:46 +0200 Subject: [PATCH 2/5] fix: openapi-spec --- .../src/simcore_service_webserver/api/v0/openapi.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index bb23bd92f0e..a7fdc02ab4b 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -3788,6 +3788,13 @@ paths: type: string format: uuid title: Function Id + - name: force + in: query + required: false + schema: + type: boolean + default: false + title: Force responses: '204': description: Successful Response From ce104a449e65e517007d1c3375a43728cd1fd6e0 Mon Sep 17 00:00:00 2001 From: Giancarlo Romeo Date: Wed, 10 Sep 2025 11:51:57 +0200 Subject: [PATCH 3/5] tests: add deletion tests --- .../unit/with_dbs/04/functions/conftest.py | 77 ++++++++++++++++++- .../test_functions_controller_rest.py | 42 +++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/04/functions/conftest.py b/services/web/server/tests/unit/with_dbs/04/functions/conftest.py index f507d9bdf5c..b59d0dc2903 100644 --- a/services/web/server/tests/unit/with_dbs/04/functions/conftest.py +++ b/services/web/server/tests/unit/with_dbs/04/functions/conftest.py @@ -6,7 +6,7 @@ from collections.abc import AsyncIterator, Awaitable, Callable from contextlib import AsyncExitStack from typing import Any -from uuid import uuid4 +from uuid import UUID, uuid4 import pytest import sqlalchemy @@ -31,6 +31,13 @@ from simcore_postgres_database.models.funcapi_api_access_rights_table import ( funcapi_api_access_rights_table, ) +from simcore_postgres_database.models.funcapi_function_jobs_table import ( + function_jobs_table, +) +from simcore_postgres_database.models.funcapi_functions_access_rights_table import ( + functions_access_rights_table, +) +from simcore_postgres_database.models.funcapi_functions_table import functions_table from simcore_service_webserver.application_settings import ApplicationSettings from simcore_service_webserver.statics._constants import FRONTEND_APP_DEFAULT from sqlalchemy.ext.asyncio import AsyncEngine @@ -253,3 +260,71 @@ async def logged_user_function_api_access_rights( async with AsyncExitStack() as stack: row = await stack.enter_async_context(cm) yield row + + +@pytest.fixture +async def fake_function_with_associated_job( + asyncpg_engine: AsyncEngine, + logged_user: UserInfoDict, +) -> AsyncIterator[UUID]: + async with AsyncExitStack() as stack: + function_row = await stack.enter_async_context( + insert_and_get_row_lifespan( + asyncpg_engine, + table=functions_table, + values={ + "title": "Test Function", + "function_class": FunctionClass.PROJECT.value, + "description": "A test function", + "input_schema": { + "schema_class": "application/schema+json", + "schema_content": { + "type": "object", + "properties": {"input1": {"type": "string"}}, + }, + }, + "output_schema": { + "schema_class": "application/schema+json", + "schema_content": { + "type": "object", + "properties": {"output1": {"type": "string"}}, + }, + }, + "class_specific_data": {"project_id": f"{uuid4()}"}, + }, + pk_col=functions_table.c.uuid, + ) + ) + + await stack.enter_async_context( + insert_and_get_row_lifespan( + asyncpg_engine, + table=functions_access_rights_table, + values={ + "function_uuid": function_row["uuid"], + "group_id": logged_user["primary_gid"], + "product_name": "osparc", # Default product name + "read": True, + "write": True, + "execute": True, + }, + pk_cols=[ + functions_access_rights_table.c.function_uuid, + functions_access_rights_table.c.group_id, + functions_access_rights_table.c.product_name, + ], + ) + ) + + await stack.enter_async_context( + insert_and_get_row_lifespan( + asyncpg_engine, + table=function_jobs_table, + values={ + "function_uuid": function_row["uuid"], + "status": "pending", + }, + pk_col=function_jobs_table.c.uuid, + ) + ) + yield function_row["uuid"] diff --git a/services/web/server/tests/unit/with_dbs/04/functions/test_functions_controller_rest.py b/services/web/server/tests/unit/with_dbs/04/functions/test_functions_controller_rest.py index 0ca5a1158b0..17bbc9afe42 100644 --- a/services/web/server/tests/unit/with_dbs/04/functions/test_functions_controller_rest.py +++ b/services/web/server/tests/unit/with_dbs/04/functions/test_functions_controller_rest.py @@ -7,7 +7,7 @@ from collections.abc import AsyncIterator from http import HTTPStatus from typing import Any -from uuid import uuid4 +from uuid import UUID, uuid4 import pytest from aiohttp.test_utils import TestClient @@ -306,3 +306,43 @@ async def test_list_user_functions_permissions( function_permissions = MyFunctionPermissionsGet.model_validate(data) assert function_permissions.read_functions == expected_read_functions assert function_permissions.write_functions == expected_write_functions + + +@pytest.mark.parametrize( + "user_role,expected_read_functions,expected_write_functions", + [(UserRole.USER, True, True)], +) +async def test_delete_function_with_associated_jobs( + client: TestClient, + logged_user: UserInfoDict, + fake_function_with_associated_job: UUID, + logged_user_function_api_access_rights: dict[str, Any], +) -> None: + function_id = fake_function_with_associated_job + + url = client.app.router["get_function"].url_for(function_id=f"{function_id}") + response = await client.get(url) + data, error = await assert_status(response, status.HTTP_200_OK) + assert not error + function = TypeAdapter(RegisteredFunctionGet).validate_python(data) + assert function.uid == function_id + + url = client.app.router["delete_function"].url_for(function_id=f"{function_id}") + response = await client.delete(url) + data, error = await assert_status(response, status.HTTP_409_CONFLICT) + assert error is not None + + url = client.app.router["get_function"].url_for(function_id=f"{function_id}") + response = await client.get(url) + data, error = await assert_status(response, status.HTTP_200_OK) + assert not error + + url = client.app.router["delete_function"].url_for(function_id=f"{function_id}") + response = await client.delete(url, params={"force": "true"}) + data, error = await assert_status(response, status.HTTP_204_NO_CONTENT) + assert not error + + url = client.app.router["get_function"].url_for(function_id=f"{function_id}") + response = await client.get(url) + data, error = await assert_status(response, status.HTTP_404_NOT_FOUND) + assert error is not None From 33883bc9116674546f0b1e59527323d975e7c913 Mon Sep 17 00:00:00 2001 From: Giancarlo Romeo Date: Wed, 10 Sep 2025 12:19:38 +0200 Subject: [PATCH 4/5] =?UTF-8?q?services/webserver=20api=20version:=200.77.?= =?UTF-8?q?2=20=E2=86=92=200.78.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/web/server/VERSION | 2 +- services/web/server/setup.cfg | 2 +- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/VERSION b/services/web/server/VERSION index 5d3aa577862..bd14e8533ef 100644 --- a/services/web/server/VERSION +++ b/services/web/server/VERSION @@ -1 +1 @@ -0.77.2 +0.78.0 diff --git a/services/web/server/setup.cfg b/services/web/server/setup.cfg index 76a3da7ee2b..e9435fb043d 100644 --- a/services/web/server/setup.cfg +++ b/services/web/server/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.77.2 +current_version = 0.78.0 commit = True message = services/webserver api version: {current_version} → {new_version} tag = False diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index a7fdc02ab4b..a52205fc60d 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -2,7 +2,7 @@ openapi: 3.1.0 info: title: simcore-service-webserver description: Main service with an interface (http-API & websockets) to the web front-end - version: 0.77.2 + version: 0.78.0 servers: - url: '' description: webserver From 5ba1c98bcc2e1c5e3b9c55fe9819db2d9f7f9549 Mon Sep 17 00:00:00 2001 From: Giancarlo Romeo Date: Wed, 10 Sep 2025 14:45:43 +0200 Subject: [PATCH 5/5] fix: description --- .../functions/_controller/_functions_rest_schemas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py index d67e4bab3e2..2e0e8e8b9d9 100644 --- a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py +++ b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_schemas.py @@ -83,7 +83,7 @@ class FunctionDeleteQueryParams(BaseModel): force: Annotated[ bool, Field( - description="If true, deletes the function even if it has associated jobs.", + description="If true, deletes the function even if it has associated jobs; otherwise, returns HTTP_409_CONFLICT if jobs exist.", ), ] = False