From 4de1a9945675f56c20d00a04b11d2da116fc3cb8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 5 Jun 2025 17:22:09 +0200 Subject: [PATCH 01/10] =?UTF-8?q?=F0=9F=90=9B=20refactor:=20rename=20servi?= =?UTF-8?q?ce=20availability=20check=20functions=20for=20clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../projects/_controller/projects_rest.py | 30 +++++++++++-------- .../_projects_repository_legacy_utils.py | 4 +-- .../projects/utils.py | 15 +++++----- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py index a403a96ecf4..d66a0c05f3e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py @@ -2,6 +2,7 @@ from aiohttp import web from common_library.json_serialization import json_dumps +from models_library.api_schemas_catalog.services import MyServiceGet from models_library.api_schemas_webserver.projects import ( EmptyModel, ProjectCopyOverride, @@ -39,7 +40,6 @@ from .. import _crud_api_create, _crud_api_read, _projects_service from .._permalink_service import update_or_pop_permalink_in_project from ..models import ProjectDict -from ..utils import get_project_unavailable_services, project_uses_available_services from . import _rest_utils from ._rest_exceptions import handle_plugin_requests_exceptions from ._rest_schemas import ( @@ -277,12 +277,6 @@ async def get_project(request: web.Request): req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) - user_available_services: list[dict] = ( - await catalog_service.get_services_for_user_in_product( - request.app, req_ctx.user_id, req_ctx.product_name, only_key_versions=True - ) - ) - project = await _projects_service.get_project_for_user( request.app, project_uuid=f"{path_params.project_id}", @@ -290,12 +284,24 @@ async def get_project(request: web.Request): include_state=True, include_trashed_by_primary_gid=True, ) - if not await project_uses_available_services(project, user_available_services): - unavilable_services = get_project_unavailable_services( - project, user_available_services - ) + + services_in_project = { + (srv["key"], srv["version"]) for _, srv in project.get("workbench", {}).items() + } + + my_services: list[MyServiceGet] = await catalog_service.batch_get_my_services( + request.app, + product_name=req_ctx.product_name, + user_id=req_ctx.user_id, + services_ids=list(services_in_project), + ) + not_my_services = services_in_project.difference( + {(srv.key, srv.release.version) for srv in my_services} + ) + + if not_my_services: formatted_services = ", ".join( - f"{service}:{version}" for service, version in unavilable_services + f"{service}:{version}" for service, version in not_my_services ) # TODO: lack of permissions should be notified with https://httpstatuses.com/403 web.HTTPForbidden raise web.HTTPNotFound( diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_repository_legacy_utils.py b/services/web/server/src/simcore_service_webserver/projects/_projects_repository_legacy_utils.py index a6ef69f9361..bc38625255e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_repository_legacy_utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_repository_legacy_utils.py @@ -32,7 +32,7 @@ ProjectNotFoundError, ) from .models import ProjectDict -from .utils import find_changed_node_keys, project_uses_available_services +from .utils import are_project_services_available, find_changed_node_keys logger = logging.getLogger(__name__) @@ -222,7 +222,7 @@ async def _execute_without_permission_check( filter_by_services is not None # This checks only old projects that are not in the projects_to_products table. and row[projects_to_products.c.product_name] is None - and not await project_uses_available_services(prj, filter_by_services) + and not are_project_services_available(prj, filter_by_services) ): logger.warning( "Project %s will not be listed for user %s since it has no access rights" diff --git a/services/web/server/src/simcore_service_webserver/projects/utils.py b/services/web/server/src/simcore_service_webserver/projects/utils.py index 18a02a5fb3c..03d35ba9c51 100644 --- a/services/web/server/src/simcore_service_webserver/projects/utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/utils.py @@ -189,20 +189,21 @@ def is_graph_equal( return True -async def project_uses_available_services( +def are_project_services_available( project: dict[str, Any], available_services: list[dict[str, Any]] ) -> bool: if not project["workbench"]: # empty project return True - # get project services - needed_services: set[tuple[str, str]] = { - (s["key"], s["version"]) for _, s in project["workbench"].items() + + # list services in project + needed_services = { + (srv["key"], srv["version"]) for _, srv in project["workbench"].items() } - # get available services - available_services_set: set[tuple[str, str]] = { - (s["key"], s["version"]) for s in available_services + # list available services + available_services_set = { + (srv["key"], srv["version"]) for srv in available_services } return needed_services.issubset(available_services_set) From 65c96279d340edb4cb800dd4e1fcdc6d91534297 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 5 Jun 2025 17:34:46 +0200 Subject: [PATCH 02/10] cleanup --- .../projects/_controller/projects_rest.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py index d66a0c05f3e..33311a4ecd2 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py @@ -55,12 +55,6 @@ ProjectsSearchQueryParams, ) -# When the user requests a project with a repo, the working copy might differ from -# the repo project. A middleware in the meta module (if active) will resolve -# the working copy and redirect to the appropriate project entrypoint. Nonetheless, the -# response needs to refer to the uuid of the request and this is passed through this request key -RQ_REQUESTED_REPO_PROJECT_UUID_KEY = f"{__name__}.RQT_REQUESTED_REPO_PROJECT_UUID_KEY" - _logger = logging.getLogger(__name__) @@ -277,6 +271,7 @@ async def get_project(request: web.Request): req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) + # 1. Get project if user has access project = await _projects_service.get_project_for_user( request.app, project_uuid=f"{path_params.project_id}", @@ -285,6 +280,7 @@ async def get_project(request: web.Request): include_trashed_by_primary_gid=True, ) + # 2. Check if user has access to all services within the project services_in_project = { (srv["key"], srv["version"]) for _, srv in project.get("workbench", {}).items() } @@ -295,6 +291,7 @@ async def get_project(request: web.Request): user_id=req_ctx.user_id, services_ids=list(services_in_project), ) + not_my_services = services_in_project.difference( {(srv.key, srv.release.version) for srv in my_services} ) @@ -311,10 +308,7 @@ async def get_project(request: web.Request): ) ) - if new_uuid := request.get(RQ_REQUESTED_REPO_PROJECT_UUID_KEY): - project["uuid"] = new_uuid - - # Adds permalink + # 3. Adds permalink await update_or_pop_permalink_in_project(request, project) data = ProjectGet.from_domain_model(project).data(exclude_unset=True) From f316fa3dd94d2e5d582bbd8407f797bfbff9f2d5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 5 Jun 2025 20:01:14 +0200 Subject: [PATCH 03/10] =?UTF-8?q?=F0=9F=90=9B=20refactor:=20replace=20get?= =?UTF-8?q?=5Fservices=5Ffor=5Fuser=5Fin=5Fproduct=20with=20list=5Fuser=5F?= =?UTF-8?q?services=5Fwith=5Fversions=20for=20improved=20clarity=20and=20c?= =?UTF-8?q?aching?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../catalog/_catalog_rest_client_service.py | 44 ++++++++++++------- .../catalog/_models.py | 7 ++- .../catalog/catalog_service.py | 6 ++- .../projects/_crud_api_read.py | 12 ++--- .../projects/utils.py | 2 +- .../with_dbs/01/test_catalog_rest_client.py | 5 +-- 6 files changed, 46 insertions(+), 30 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py index 20095cc72af..7a8a4658865 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py @@ -2,7 +2,7 @@ import logging import urllib.parse -from collections.abc import Callable, Iterator +from collections.abc import Iterator from contextlib import contextmanager from typing import Any, Final @@ -30,20 +30,11 @@ from yarl import URL from .._meta import api_version_prefix +from ._models import ServiceKeyVersionDict from .settings import CatalogSettings, get_plugin_settings _logger = logging.getLogger(__name__) -# Cache settings -_SECOND = 1 # in seconds -_MINUTE = 60 * _SECOND -_CACHE_TTL: Final = 1 * _MINUTE - - -def _create_service_cache_key(_f: Callable[..., Any], *_args, **kw): - assert len(_args) == 1, f"Expected only app, got {_args}" # nosec - return f"get_service_{kw['user_id']}_{kw['service_key']}_{kw['service_version']}_{kw['product_name']}" - @contextmanager def _handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]: @@ -96,10 +87,24 @@ def to_backend_service(rel_url: URL, origin: URL, version_prefix: str) -> URL: return origin.with_path(new_path).with_query(rel_url.query) -async def get_services_for_user_in_product( - app: web.Application, user_id: UserID, product_name: str, *, only_key_versions: bool -) -> list[dict]: +# Cache settings for services rest API +_SECOND = 1 # in seconds +_MINUTE = 60 * _SECOND +_CACHE_TTL: Final = 1 * _MINUTE + + +@cached( + ttl=_CACHE_TTL, + key_builder=lambda _f, *_args, **kw: f"get_services_for_user_in_product_{kw['user_id']}_{kw['product_name']}", + cache=Cache.MEMORY, +) +async def list_user_services_with_versions( + app: web.Application, *, user_id: UserID, product_name: str +) -> list[ServiceKeyVersionDict]: + settings: CatalogSettings = get_plugin_settings(app) + only_key_versions = True + url = (URL(settings.api_base_url) / "services").with_query( {"user_id": user_id, "details": f"{not only_key_versions}"} ) @@ -115,13 +120,18 @@ async def get_services_for_user_in_product( user_id, ) return [] - body: list[dict] = await response.json() - return body + services: list[dict] = await response.json() + + # This reduces the size cached in the memory + return [ + ServiceKeyVersionDict(key=service["key"], version=service["version"]) + for service in services + ] @cached( ttl=_CACHE_TTL, - key_builder=_create_service_cache_key, + key_builder=lambda _f, *_args, **kw: f"get_service_{kw['user_id']}_{kw['service_key']}_{kw['service_version']}_{kw['product_name']}", cache=Cache.MEMORY, # SEE https://github.com/ITISFoundation/osparc-simcore/pull/7802 ) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_models.py b/services/web/server/src/simcore_service_webserver/catalog/_models.py index b589e629a9f..18dd24dad6f 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_models.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_models.py @@ -1 +1,6 @@ -# NOTE: missing. @bisgaard-itis will follow up here +from typing import TypedDict + + +class ServiceKeyVersionDict(TypedDict): + key: str + version: str diff --git a/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py b/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py index d6e42b376dd..c6b46082787 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py @@ -2,10 +2,11 @@ get_service, get_service_access_rights, get_service_resources, - get_services_for_user_in_product, is_catalog_service_responsive, + list_user_services_with_versions, to_backend_service, ) +from ._models import ServiceKeyVersionDict from ._service import batch_get_my_services __all__: tuple[str, ...] = ( @@ -13,8 +14,9 @@ "get_service", "get_service_access_rights", "get_service_resources", - "get_services_for_user_in_product", + "list_user_services_with_versions", "is_catalog_service_responsive", "to_backend_service", + "ServiceKeyVersionDict", ) # nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py index 15cf85ed591..ec26e7062c2 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py @@ -120,9 +120,9 @@ async def list_projects( # pylint: disable=too-many-arguments ) -> tuple[list[ProjectDict], int]: db = ProjectDBAPI.get_from_app_context(app) - user_available_services: list[dict] = ( - await catalog_service.get_services_for_user_in_product( - app, user_id, product_name, only_key_versions=True + user_available_services: list[ServiceKeyVersionDict] = ( + await catalog_service.list_user_services_with_versions( + app, user_id=user_id, product_name=product_name ) ) @@ -204,9 +204,9 @@ async def list_projects_full_depth( ) -> tuple[list[ProjectDict], int]: db = ProjectDBAPI.get_from_app_context(app) - user_available_services: list[dict] = ( - await catalog_service.get_services_for_user_in_product( - app, user_id, product_name, only_key_versions=True + user_available_services: list[ServiceKeyVersionDict] = ( + await catalog_service.list_user_services_with_versions( + app, user_id=user_id, product_name=product_name ) ) diff --git a/services/web/server/src/simcore_service_webserver/projects/utils.py b/services/web/server/src/simcore_service_webserver/projects/utils.py index 03d35ba9c51..7f9514812e7 100644 --- a/services/web/server/src/simcore_service_webserver/projects/utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/utils.py @@ -190,7 +190,7 @@ def is_graph_equal( def are_project_services_available( - project: dict[str, Any], available_services: list[dict[str, Any]] + project: dict[str, Any], available_services: list[dict[str, str]] ) -> bool: if not project["workbench"]: # empty project diff --git a/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py b/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py index 452ecfd76ec..0a7ebeedf17 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py +++ b/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py @@ -15,8 +15,8 @@ ) from simcore_service_webserver.catalog.catalog_service import ( get_service_access_rights, - get_services_for_user_in_product, is_catalog_service_responsive, + list_user_services_with_versions, ) @@ -62,11 +62,10 @@ async def test_get_services_for_user_in_product( status=backend_status_code, ) assert client.app - _ = await get_services_for_user_in_product( + await list_user_services_with_versions( app=client.app, user_id=logged_user["id"], product_name="osparc", - only_key_versions=False, ) From 3a58957297c66e15d3a47f702c97b7e10089314d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 5 Jun 2025 20:04:15 +0200 Subject: [PATCH 04/10] revered --- .../projects/_controller/projects_rest.py | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py index 33311a4ecd2..a403a96ecf4 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py @@ -2,7 +2,6 @@ from aiohttp import web from common_library.json_serialization import json_dumps -from models_library.api_schemas_catalog.services import MyServiceGet from models_library.api_schemas_webserver.projects import ( EmptyModel, ProjectCopyOverride, @@ -40,6 +39,7 @@ from .. import _crud_api_create, _crud_api_read, _projects_service from .._permalink_service import update_or_pop_permalink_in_project from ..models import ProjectDict +from ..utils import get_project_unavailable_services, project_uses_available_services from . import _rest_utils from ._rest_exceptions import handle_plugin_requests_exceptions from ._rest_schemas import ( @@ -55,6 +55,12 @@ ProjectsSearchQueryParams, ) +# When the user requests a project with a repo, the working copy might differ from +# the repo project. A middleware in the meta module (if active) will resolve +# the working copy and redirect to the appropriate project entrypoint. Nonetheless, the +# response needs to refer to the uuid of the request and this is passed through this request key +RQ_REQUESTED_REPO_PROJECT_UUID_KEY = f"{__name__}.RQT_REQUESTED_REPO_PROJECT_UUID_KEY" + _logger = logging.getLogger(__name__) @@ -271,7 +277,12 @@ async def get_project(request: web.Request): req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) - # 1. Get project if user has access + user_available_services: list[dict] = ( + await catalog_service.get_services_for_user_in_product( + request.app, req_ctx.user_id, req_ctx.product_name, only_key_versions=True + ) + ) + project = await _projects_service.get_project_for_user( request.app, project_uuid=f"{path_params.project_id}", @@ -279,26 +290,12 @@ async def get_project(request: web.Request): include_state=True, include_trashed_by_primary_gid=True, ) - - # 2. Check if user has access to all services within the project - services_in_project = { - (srv["key"], srv["version"]) for _, srv in project.get("workbench", {}).items() - } - - my_services: list[MyServiceGet] = await catalog_service.batch_get_my_services( - request.app, - product_name=req_ctx.product_name, - user_id=req_ctx.user_id, - services_ids=list(services_in_project), - ) - - not_my_services = services_in_project.difference( - {(srv.key, srv.release.version) for srv in my_services} - ) - - if not_my_services: + if not await project_uses_available_services(project, user_available_services): + unavilable_services = get_project_unavailable_services( + project, user_available_services + ) formatted_services = ", ".join( - f"{service}:{version}" for service, version in not_my_services + f"{service}:{version}" for service, version in unavilable_services ) # TODO: lack of permissions should be notified with https://httpstatuses.com/403 web.HTTPForbidden raise web.HTTPNotFound( @@ -308,7 +305,10 @@ async def get_project(request: web.Request): ) ) - # 3. Adds permalink + if new_uuid := request.get(RQ_REQUESTED_REPO_PROJECT_UUID_KEY): + project["uuid"] = new_uuid + + # Adds permalink await update_or_pop_permalink_in_project(request, project) data = ProjectGet.from_domain_model(project).data(exclude_unset=True) From 8a99b8fa2c4f170e038c826f5fcdfed29748d44b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 5 Jun 2025 20:07:40 +0200 Subject: [PATCH 05/10] uses projects --- .../projects/_controller/projects_rest.py | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py index a403a96ecf4..2c0e9818bd9 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py @@ -39,7 +39,7 @@ from .. import _crud_api_create, _crud_api_read, _projects_service from .._permalink_service import update_or_pop_permalink_in_project from ..models import ProjectDict -from ..utils import get_project_unavailable_services, project_uses_available_services +from ..utils import are_project_services_available, get_project_unavailable_services from . import _rest_utils from ._rest_exceptions import handle_plugin_requests_exceptions from ._rest_schemas import ( @@ -55,12 +55,6 @@ ProjectsSearchQueryParams, ) -# When the user requests a project with a repo, the working copy might differ from -# the repo project. A middleware in the meta module (if active) will resolve -# the working copy and redirect to the appropriate project entrypoint. Nonetheless, the -# response needs to refer to the uuid of the request and this is passed through this request key -RQ_REQUESTED_REPO_PROJECT_UUID_KEY = f"{__name__}.RQT_REQUESTED_REPO_PROJECT_UUID_KEY" - _logger = logging.getLogger(__name__) @@ -277,10 +271,8 @@ async def get_project(request: web.Request): req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) - user_available_services: list[dict] = ( - await catalog_service.get_services_for_user_in_product( - request.app, req_ctx.user_id, req_ctx.product_name, only_key_versions=True - ) + user_available_services = await catalog_service.list_user_services_with_versions( + request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name ) project = await _projects_service.get_project_for_user( @@ -290,7 +282,8 @@ async def get_project(request: web.Request): include_state=True, include_trashed_by_primary_gid=True, ) - if not await project_uses_available_services(project, user_available_services): + + if not are_project_services_available(project, user_available_services): unavilable_services = get_project_unavailable_services( project, user_available_services ) @@ -305,9 +298,6 @@ async def get_project(request: web.Request): ) ) - if new_uuid := request.get(RQ_REQUESTED_REPO_PROJECT_UUID_KEY): - project["uuid"] = new_uuid - # Adds permalink await update_or_pop_permalink_in_project(request, project) From 8f5c2dfe321f2e251152f58e6db45a7ab9fd7df4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 5 Jun 2025 20:31:13 +0200 Subject: [PATCH 06/10] revert rename --- .../catalog/_catalog_rest_client_service.py | 15 +++++++++++---- .../catalog/catalog_service.py | 4 ++-- .../projects/_controller/projects_rest.py | 2 +- .../projects/_crud_api_read.py | 4 ++-- .../unit/with_dbs/01/test_catalog_rest_client.py | 5 +++-- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py index 7a8a4658865..6f221213f60 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py @@ -98,10 +98,13 @@ def to_backend_service(rel_url: URL, origin: URL, version_prefix: str) -> URL: key_builder=lambda _f, *_args, **kw: f"get_services_for_user_in_product_{kw['user_id']}_{kw['product_name']}", cache=Cache.MEMORY, ) -async def list_user_services_with_versions( +async def get_services_for_user_in_product( app: web.Application, *, user_id: UserID, product_name: str ) -> list[ServiceKeyVersionDict]: - + """ + DEPRECATED: see instead RPC interface. + SEE https://github.com/ITISFoundation/osparc-simcore/issues/7838 + """ settings: CatalogSettings = get_plugin_settings(app) only_key_versions = True @@ -143,6 +146,10 @@ async def get_service( service_version: ServiceVersion, product_name: ProductName, ) -> dict[str, Any]: + """ + DEPRECATED: see instead RPC interface. + SEE https://github.com/ITISFoundation/osparc-simcore/issues/7838 + """ settings: CatalogSettings = get_plugin_settings(app) url = URL( f"{settings.api_base_url}/services/{urllib.parse.quote_plus(service_key)}/{service_version}", @@ -154,8 +161,8 @@ async def get_service( url, headers={X_PRODUCT_NAME_HEADER: product_name} ) as response: response.raise_for_status() - body: dict[str, Any] = await response.json() - return body + service: dict[str, Any] = await response.json() + return service async def get_service_resources( diff --git a/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py b/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py index c6b46082787..3bce6a91386 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py @@ -2,8 +2,8 @@ get_service, get_service_access_rights, get_service_resources, + get_services_for_user_in_product, is_catalog_service_responsive, - list_user_services_with_versions, to_backend_service, ) from ._models import ServiceKeyVersionDict @@ -14,7 +14,7 @@ "get_service", "get_service_access_rights", "get_service_resources", - "list_user_services_with_versions", + "get_services_for_user_in_product", "is_catalog_service_responsive", "to_backend_service", "ServiceKeyVersionDict", diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py index 2c0e9818bd9..60d77aa9c3a 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py @@ -271,7 +271,7 @@ async def get_project(request: web.Request): req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) - user_available_services = await catalog_service.list_user_services_with_versions( + user_available_services = await catalog_service.get_services_for_user_in_product( request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name ) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py index ec26e7062c2..25b995a0632 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py @@ -121,7 +121,7 @@ async def list_projects( # pylint: disable=too-many-arguments db = ProjectDBAPI.get_from_app_context(app) user_available_services: list[ServiceKeyVersionDict] = ( - await catalog_service.list_user_services_with_versions( + await catalog_service.get_services_for_user_in_product( app, user_id=user_id, product_name=product_name ) ) @@ -205,7 +205,7 @@ async def list_projects_full_depth( db = ProjectDBAPI.get_from_app_context(app) user_available_services: list[ServiceKeyVersionDict] = ( - await catalog_service.list_user_services_with_versions( + await catalog_service.get_services_for_user_in_product( app, user_id=user_id, product_name=product_name ) ) diff --git a/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py b/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py index 0a7ebeedf17..77adeda93d8 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py +++ b/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py @@ -15,8 +15,8 @@ ) from simcore_service_webserver.catalog.catalog_service import ( get_service_access_rights, + get_services_for_user_in_product, is_catalog_service_responsive, - list_user_services_with_versions, ) @@ -62,7 +62,8 @@ async def test_get_services_for_user_in_product( status=backend_status_code, ) assert client.app - await list_user_services_with_versions( + # tests it does not raise an exception + await get_services_for_user_in_product( app=client.app, user_id=logged_user["id"], product_name="osparc", From c413b6be5dcc11d99fffddcdd39163f087059d65 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 5 Jun 2025 20:35:20 +0200 Subject: [PATCH 07/10] refactor: streamline exception handling and improve service retrieval logic --- .../catalog/_controller_rest.py | 2 +- .../catalog/_controller_rest_exceptions.py | 13 +++---------- .../projects/_controller/_rest_exceptions.py | 11 ++++++++--- .../projects/_crud_api_read.py | 12 ++++-------- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest.py b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest.py index 644edb14d9a..1ed2b50ac6b 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest.py @@ -37,7 +37,6 @@ from ..utils_aiohttp import envelope_json_response from . import _catalog_rest_client_service, _service from ._controller_rest_exceptions import ( - DefaultPricingUnitForServiceNotFoundError, handle_plugin_requests_exceptions, ) from ._controller_rest_schemas import ( @@ -50,6 +49,7 @@ ServiceTagPathParams, ToServiceInputsQueryParams, ) +from .errors import DefaultPricingUnitForServiceNotFoundError _logger = logging.getLogger(__name__) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py index ae763d342ef..98603af76c1 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py @@ -105,19 +105,12 @@ async def _handler_catalog_client_errors( } -_exceptions_handlers_map: ExceptionHandlersMap = { +catalog_exceptions_handlers_map: ExceptionHandlersMap = { CatalogResponseError: _handler_catalog_client_errors, CatalogConnectionError: _handler_catalog_client_errors, } -_exceptions_handlers_map.update(to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP)) +catalog_exceptions_handlers_map.update(to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP)) handle_plugin_requests_exceptions = exception_handling_decorator( - _exceptions_handlers_map -) - - -__all__: tuple[str, ...] = ( - "CatalogForbiddenError", - "CatalogItemNotFoundError", - "DefaultPricingUnitForServiceNotFoundError", + catalog_exceptions_handlers_map ) diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py index f986af6a1f8..9cf77642e12 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py @@ -8,7 +8,9 @@ CatalogItemNotFoundError, CatalogNotAvailableError, ) +from simcore_service_webserver.exception_handling._base import ExceptionHandlersMap +from ...catalog._controller_rest_exceptions import catalog_exceptions_handlers_map from ...conversations.errors import ( ConversationErrorNotFoundError, ConversationMessageErrorNotFoundError, @@ -239,6 +241,9 @@ def _assert_duplicate(): } -handle_plugin_requests_exceptions = exception_handling_decorator( - to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP) -) +_handlers: ExceptionHandlersMap = { + **catalog_exceptions_handlers_map, + **to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP), +} + +handle_plugin_requests_exceptions = exception_handling_decorator(_handlers) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py index 25b995a0632..d8931c5da8c 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py @@ -120,10 +120,8 @@ async def list_projects( # pylint: disable=too-many-arguments ) -> tuple[list[ProjectDict], int]: db = ProjectDBAPI.get_from_app_context(app) - user_available_services: list[ServiceKeyVersionDict] = ( - await catalog_service.get_services_for_user_in_product( - app, user_id=user_id, product_name=product_name - ) + user_available_services = await catalog_service.get_services_for_user_in_product( + app, user_id=user_id, product_name=product_name ) workspace_is_private = True @@ -204,10 +202,8 @@ async def list_projects_full_depth( ) -> tuple[list[ProjectDict], int]: db = ProjectDBAPI.get_from_app_context(app) - user_available_services: list[ServiceKeyVersionDict] = ( - await catalog_service.get_services_for_user_in_product( - app, user_id=user_id, product_name=product_name - ) + user_available_services = await catalog_service.get_services_for_user_in_product( + app, user_id=user_id, product_name=product_name ) db_projects, db_project_types, total_number_projects = await db.list_projects_dicts( From dfa81aa0281aac96e9883ad5036313f970a32456 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 6 Jun 2025 13:56:56 +0200 Subject: [PATCH 08/10] fixes tests --- .../tests/unit/with_dbs/01/test_catalog_rest_client.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py b/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py index 77adeda93d8..fcce12c154f 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py +++ b/services/web/server/tests/unit/with_dbs/01/test_catalog_rest_client.py @@ -8,6 +8,8 @@ from models_library.api_schemas_catalog.service_access_rights import ( ServiceAccessRightsGet, ) +from models_library.api_schemas_catalog.services import ServiceGet +from pydantic import TypeAdapter from pytest_simcore.helpers.webserver_login import UserInfoDict from servicelib.aiohttp import status from simcore_service_webserver.catalog._controller_rest_exceptions import ( @@ -38,9 +40,9 @@ async def test_server_responsive( assert client.app is_responsive = await is_catalog_service_responsive(app=client.app) if backend_status_code == status.HTTP_200_OK: - assert is_responsive == True + assert is_responsive is True else: - assert is_responsive == False + assert is_responsive is False @pytest.mark.parametrize( @@ -56,10 +58,13 @@ async def test_get_services_for_user_in_product( aioresponses_mocker: AioResponsesMock, backend_status_code: int, ): + examples = ServiceGet.model_json_schema()["examples"] + url_pattern = re.compile(r"http://catalog:8000/.*") aioresponses_mocker.get( url_pattern, status=backend_status_code, + payload=TypeAdapter(list[ServiceGet]).dump_python(examples, mode="json"), ) assert client.app # tests it does not raise an exception From 24b30aa66af99587486e8b41264ccd702116bd62 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 6 Jun 2025 16:03:51 +0200 Subject: [PATCH 09/10] refactor: rename mock function for clarity and consistency across tests --- .../02/test_projects_crud_handlers__patch.py | 6 +++--- .../with_dbs/02/test_projects_groups_handlers.py | 10 +++++----- .../02/test_projects_nodes_handlers__patch.py | 14 +++++++------- .../tests/unit/with_dbs/04/workspaces/conftest.py | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__patch.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__patch.py index dbb33ea5ecd..195b6b9c4cc 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__patch.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__patch.py @@ -32,9 +32,9 @@ def mock_catalog_api_get_services_for_user_in_product(mocker: MockerFixture): @pytest.fixture -def mock_project_uses_available_services(mocker: MockerFixture): +def mock_are_project_services_available(mocker: MockerFixture): mocker.patch( - "simcore_service_webserver.projects._controller.projects_rest.project_uses_available_services", + "simcore_service_webserver.projects._controller.projects_rest.are_project_services_available", spec=True, return_value=True, ) @@ -82,7 +82,7 @@ async def test_patch_project( user_project: ProjectDict, expected: HTTPStatus, mock_catalog_api_get_services_for_user_in_product, - mock_project_uses_available_services, + mock_are_project_services_available, ): assert client.app base_url = client.app.router["patch_project"].url_for( diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_groups_handlers.py b/services/web/server/tests/unit/with_dbs/02/test_projects_groups_handlers.py index ecbbeffc334..72aa178ecfc 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_groups_handlers.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_groups_handlers.py @@ -33,9 +33,9 @@ def mock_catalog_api_get_services_for_user_in_product( @pytest.fixture -def mock_project_uses_available_services(mocker: MockerFixture) -> MockType: +def mock_are_project_services_available(mocker: MockerFixture) -> MockType: return mocker.patch( - "simcore_service_webserver.projects._controller.projects_rest.project_uses_available_services", + "simcore_service_webserver.projects._controller.projects_rest.are_project_services_available", spec=True, return_value=True, ) @@ -51,7 +51,7 @@ async def test_projects_groups_full_workflow( # noqa: PLR0915 user_project: ProjectDict, expected: HTTPStatus, mock_catalog_api_get_services_for_user_in_product: MockType, - mock_project_uses_available_services: MockType, + mock_are_project_services_available: MockType, ): assert client.app # check the default project permissions @@ -265,7 +265,7 @@ async def test_share_project( logged_user: UserInfoDict, user_project: ProjectDict, mock_catalog_api_get_services_for_user_in_product: MockType, - mock_project_uses_available_services: MockType, + mock_are_project_services_available: MockType, ): assert client.app @@ -333,7 +333,7 @@ async def test_share_project_with_roles( logged_user: UserInfoDict, user_project: ProjectDict, mock_catalog_api_get_services_for_user_in_product: MockType, - mock_project_uses_available_services: MockType, + mock_are_project_services_available: MockType, user_role: UserRole, expected_status: HTTPStatus, ): diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__patch.py b/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__patch.py index 3bba1eaf118..cfbc1ecdcd1 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__patch.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__patch.py @@ -36,9 +36,9 @@ def mock_catalog_api_get_services_for_user_in_product(mocker: MockerFixture): @pytest.fixture -def mock_project_uses_available_services(mocker: MockerFixture): +def mock_are_project_services_available(mocker: MockerFixture): mocker.patch( - "simcore_service_webserver.projects._controller.projects_rest.project_uses_available_services", + "simcore_service_webserver.projects._controller.projects_rest.are_project_services_available", spec=True, return_value=True, ) @@ -100,7 +100,7 @@ async def test_patch_project_node( user_project: ProjectDict, expected: HTTPStatus, mock_catalog_api_get_services_for_user_in_product: None, - mock_project_uses_available_services: None, + mock_are_project_services_available: None, mock_catalog_rpc_check_for_service: None, ): node_id = next(iter(user_project["workbench"])) @@ -222,7 +222,7 @@ async def test_patch_project_node_notifies( user_project: ProjectDict, expected: HTTPStatus, mock_catalog_api_get_services_for_user_in_product, - mock_project_uses_available_services, + mock_are_project_services_available, mock_catalog_rpc_check_for_service, mocked_notify_project_node_update, ): @@ -258,7 +258,7 @@ async def test_patch_project_node_inputs_notifies( user_project: ProjectDict, expected: HTTPStatus, mock_catalog_api_get_services_for_user_in_product, - mock_project_uses_available_services, + mock_are_project_services_available, mocked_notify_project_node_update, ): node_id = next(iter(user_project["workbench"])) @@ -298,7 +298,7 @@ async def test_patch_project_node_inputs_with_data_type_change( user_project: ProjectDict, expected: HTTPStatus, mock_catalog_api_get_services_for_user_in_product, - mock_project_uses_available_services, + mock_are_project_services_available, ): node_id = next(iter(user_project["workbench"])) assert client.app @@ -351,7 +351,7 @@ async def test_patch_project_node_service_key_with_error( user_project: ProjectDict, expected: HTTPStatus, mock_catalog_api_get_services_for_user_in_product, - mock_project_uses_available_services, + mock_are_project_services_available, mocker: MockerFixture, ): node_id = next(iter(user_project["workbench"])) diff --git a/services/web/server/tests/unit/with_dbs/04/workspaces/conftest.py b/services/web/server/tests/unit/with_dbs/04/workspaces/conftest.py index 2f2e06af8fb..5ae933f8e9e 100644 --- a/services/web/server/tests/unit/with_dbs/04/workspaces/conftest.py +++ b/services/web/server/tests/unit/with_dbs/04/workspaces/conftest.py @@ -26,7 +26,7 @@ def mock_catalog_api_get_services_for_user_in_product(mocker: MockerFixture): return_value=[], ) mocker.patch( - "simcore_service_webserver.projects._controller.projects_rest.project_uses_available_services", + "simcore_service_webserver.projects._controller.projects_rest.are_project_services_available", spec=True, return_value=True, ) From f9904d333f7a0038764530b74b11f753454db583 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 6 Jun 2025 16:39:23 +0200 Subject: [PATCH 10/10] fixes wrong mock --- services/web/server/tests/unit/with_dbs/02/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/02/conftest.py b/services/web/server/tests/unit/with_dbs/02/conftest.py index 339e154e95e..41da494808e 100644 --- a/services/web/server/tests/unit/with_dbs/02/conftest.py +++ b/services/web/server/tests/unit/with_dbs/02/conftest.py @@ -324,7 +324,7 @@ def mock_catalog_service_api_responses(client, aioresponses_mocker): aioresponses_mocker.get( url_pattern, - payload={"data": {}}, + payload={}, repeat=True, ) aioresponses_mocker.post(