From 3e753db4715115486299eadf0a20819833de4c36 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 18:02:53 +0100 Subject: [PATCH 01/23] drafts catalog service --- .../services_http/catalog.py | 34 +++++--- .../services_rpc/catalog.py | 83 +++++++++++++++++++ .../tests/unit/test_services_catalog.py | 52 ++++++++++++ 3 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 services/api-server/src/simcore_service_api_server/services_rpc/catalog.py create mode 100644 services/api-server/tests/unit/test_services_catalog.py diff --git a/services/api-server/src/simcore_service_api_server/services_http/catalog.py b/services/api-server/src/simcore_service_api_server/services_http/catalog.py index f20264087e10..b6607342724c 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_http/catalog.py @@ -9,7 +9,9 @@ from fastapi import FastAPI, status from models_library.emails import LowerCaseEmailStr +from models_library.products import ProductName from models_library.services import ServiceMetaDataPublished, ServiceType +from models_library.users import UserID from pydantic import ConfigDict, TypeAdapter, ValidationError from settings_library.catalog import CatalogSettings from settings_library.tracing import TracingSettings @@ -70,9 +72,9 @@ def to_solver(self) -> Solver: _exception_mapper = partial(service_exception_mapper, service_name="Catalog") -TruncatedCatalogServiceOutAdapter: Final[ - TypeAdapter[TruncatedCatalogServiceOut] -] = TypeAdapter(TruncatedCatalogServiceOut) +TruncatedCatalogServiceOutAdapter: Final[TypeAdapter[TruncatedCatalogServiceOut]] = ( + TypeAdapter(TruncatedCatalogServiceOut) +) TruncatedCatalogServiceOutListAdapter: Final[ TypeAdapter[list[TruncatedCatalogServiceOut]] ] = TypeAdapter(list[TruncatedCatalogServiceOut]) @@ -97,8 +99,8 @@ class CatalogApi(BaseServiceClientApi): async def list_solvers( self, *, - user_id: int, - product_name: str, + user_id: UserID, + product_name: ProductName, predicate: Callable[[Solver], bool] | None = None, ) -> list[Solver]: @@ -140,7 +142,12 @@ async def list_solvers( http_status_map={status.HTTP_404_NOT_FOUND: SolverOrStudyNotFoundError} ) async def get_service( - self, *, user_id: int, name: SolverKeyId, version: VersionStr, product_name: str + self, + *, + user_id: UserID, + name: SolverKeyId, + version: VersionStr, + product_name: ProductName, ) -> Solver: assert version != LATEST_VERSION # nosec @@ -171,8 +178,13 @@ async def get_service( http_status_map={status.HTTP_404_NOT_FOUND: SolverOrStudyNotFoundError} ) async def get_service_ports( - self, *, user_id: int, name: SolverKeyId, version: VersionStr, product_name: str - ): + self, + *, + user_id: UserID, + name: SolverKeyId, + version: VersionStr, + product_name: ProductName, + ) -> list[SolverPort]: assert version != LATEST_VERSION # nosec @@ -190,7 +202,7 @@ async def get_service_ports( return TypeAdapter(list[SolverPort]).validate_python(response.json()) async def list_latest_releases( - self, *, user_id: int, product_name: str + self, *, user_id: UserID, product_name: ProductName ) -> list[Solver]: solvers: list[Solver] = await self.list_solvers( user_id=user_id, product_name=product_name @@ -205,7 +217,7 @@ async def list_latest_releases( return list(latest_releases.values()) async def list_solver_releases( - self, *, user_id: int, solver_key: SolverKeyId, product_name: str + self, *, user_id: UserID, solver_key: SolverKeyId, product_name: ProductName ) -> list[Solver]: def _this_solver(solver: Solver) -> bool: return solver.id == solver_key @@ -216,7 +228,7 @@ def _this_solver(solver: Solver) -> bool: return releases async def get_latest_release( - self, *, user_id: int, solver_key: SolverKeyId, product_name: str + self, *, user_id: UserID, solver_key: SolverKeyId, product_name: ProductName ) -> Solver: releases = await self.list_solver_releases( user_id=user_id, solver_key=solver_key, product_name=product_name diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py new file mode 100644 index 000000000000..3ef3264cd2af --- /dev/null +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -0,0 +1,83 @@ +from models_library.basic_types import VersionStr +from models_library.products import ProductName +from models_library.rest_pagination import ( + DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, + PageLimitInt, + PageOffsetInt, +) +from models_library.users import UserID + +from ..models.schemas.solvers import Solver, SolverKeyId, SolverPort + +_FAKE: list[Solver] = [ + Solver.model_validate(Solver.model_json_schema()["example"]), +] +_FAKE2: list[SolverPort] = [ + SolverPort.model_validate(SolverPort.model_json_schema()["example"]), +] + + +async def list_latest_releases( + *, + product_name: ProductName, + user_id: UserID, + offset: PageOffsetInt = 0, + limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, +) -> list[Solver]: + assert product_name # nosec + assert user_id # nosec + + return _FAKE[offset : offset + limit] + + +async def list_solver_releases( + *, + product_name: ProductName, + user_id: UserID, + solver_id: SolverKeyId, + offset: PageOffsetInt = 0, + limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, +) -> list[Solver]: + assert product_name # nosec + assert user_id # nosec + return [solver for solver in _FAKE if solver.id == solver_id][ + offset : offset + limit + ] + + +async def get_solver( + *, + product_name: ProductName, + user_id: UserID, + solver_id: SolverKeyId, + solver_version: VersionStr, +) -> Solver | None: + assert product_name # nosec + assert user_id # nosec + + return next( + ( + solver + for solver in _FAKE + if solver.id == solver_id and solver.version == solver_version + ), + None, + ) + + +async def get_solver_ports( + *, + product_name: ProductName, + user_id: int, + solver_id: SolverKeyId, + solver_version: VersionStr, +) -> list[SolverPort]: + + if await get_solver( + product_name=product_name, + user_id=user_id, + solver_id=solver_id, + solver_version=solver_version, + ): + return _FAKE2 + return [] diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py new file mode 100644 index 000000000000..140c2ad000ea --- /dev/null +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -0,0 +1,52 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=unused-argument +# pylint: disable=unused-variable + +import pytest +from models_library.products import ProductName +from models_library.users import UserID +from simcore_service_api_server.models.schemas.solvers import Solver, SolverPort +from simcore_service_api_server.services_rpc import catalog as catalog_service + + +@pytest.fixture +def product_name() -> ProductName: + return "osparc" + + +async def test_catalog_service_read_solvers(product_name: ProductName, user_id: UserID): + # Step 1: List latest releases in a page + solver_releases_page: list[Solver] = await catalog_service.list_latest_releases( + product_name=product_name, user_id=user_id + ) + assert solver_releases_page, "Releases page should not be empty" + + # Step 2: Select one release and list solver releases + selected_release = solver_releases_page[0] + solver_releases = await catalog_service.list_solver_releases( + product_name=product_name, + user_id=user_id, + solver_id=selected_release.id, + ) + assert solver_releases, "Solver releases should not be empty" + + # Step 3: Take the latest solver release and get solver details + latest_solver_release = solver_releases[0] + solver_details: Solver | None = await catalog_service.get_solver( + product_name=product_name, + user_id=user_id, + solver_id=latest_solver_release.id, + solver_version=latest_solver_release.version, + ) + assert solver_details, "Solver details should not be empty" + + # Step 4: Get solver ports + solver_ports: list[SolverPort] = await catalog_service.get_solver_ports( + product_name=product_name, + user_id=user_id, + solver_id=latest_solver_release.id, + solver_version=latest_solver_release.version, + ) + assert solver_ports, "Solver ports should not be empty" From 240da753db16afdeb5bd63bfc0c0ff76aa32c9cf Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 19:01:33 +0100 Subject: [PATCH 02/23] pagination --- .../models-library/src/models_library/rest_pagination.py | 4 +++- .../servicelib/rabbitmq/rpc_interfaces/catalog/services.py | 7 ++++--- .../src/simcore_service_catalog/api/rpc/_services.py | 5 +++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/models-library/src/models_library/rest_pagination.py b/packages/models-library/src/models_library/rest_pagination.py index 0b7e68e52227..e4a815777420 100644 --- a/packages/models-library/src/models_library/rest_pagination.py +++ b/packages/models-library/src/models_library/rest_pagination.py @@ -28,6 +28,8 @@ PageOffsetInt: TypeAlias = NonNegativeInt +PageTotalCount: TypeAlias = NonNegativeInt + DEFAULT_NUMBER_OF_ITEMS_PER_PAGE: Final[PageLimitInt] = TypeAdapter( PageLimitInt ).validate_python(20) @@ -70,7 +72,7 @@ class PageQueryParameters(RequestParameters): class PageMetaInfoLimitOffset(BaseModel): limit: PositiveInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE - total: NonNegativeInt + total: PageTotalCount offset: NonNegativeInt = 0 count: NonNegativeInt diff --git a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py index ca4f8876f597..837ca129638c 100644 --- a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py +++ b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py @@ -12,6 +12,7 @@ ) from models_library.products import ProductName from models_library.rabbitmq_basic_types import RPCMethodName +from models_library.rest_pagination import PageOffsetInt from models_library.rpc_pagination import ( DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, PageLimitInt, @@ -19,7 +20,7 @@ ) from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID -from pydantic import NonNegativeInt, TypeAdapter, validate_call +from pydantic import TypeAdapter, validate_call from servicelib.logging_utils import log_decorator from servicelib.rabbitmq._constants import RPC_REQUEST_DEFAULT_TIMEOUT_S @@ -34,7 +35,7 @@ async def list_services_paginated( # pylint: disable=too-many-arguments product_name: ProductName, user_id: UserID, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, - offset: NonNegativeInt = 0, + offset: PageOffsetInt = 0, ) -> PageRpc[LatestServiceGet]: """ Raises: @@ -47,7 +48,7 @@ async def _call( product_name: ProductName, user_id: UserID, limit: PageLimitInt, - offset: NonNegativeInt, + offset: PageOffsetInt, ): return await rpc_client.request( CATALOG_RPC_NAMESPACE, diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py index 7568b18e351b..815b23981fbe 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -10,10 +10,11 @@ ServiceUpdateV2, ) from models_library.products import ProductName +from models_library.rest_pagination import PageOffsetInt from models_library.rpc_pagination import DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, PageLimitInt from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID -from pydantic import NonNegativeInt, ValidationError, validate_call +from pydantic import ValidationError, validate_call from pyinstrument import Profiler from servicelib.logging_utils import log_decorator from servicelib.rabbitmq import RPCRouter @@ -62,7 +63,7 @@ async def list_services_paginated( product_name: ProductName, user_id: UserID, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, - offset: NonNegativeInt = 0, + offset: PageOffsetInt = 0, ) -> PageRpcServicesGetV2: assert app.state.engine # nosec From 41844d53cd8a24474af33c2b027b98b43386204f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 19:04:24 +0100 Subject: [PATCH 03/23] udpates api --- .../services_rpc/catalog.py | 35 ++++++++++++++++--- .../tests/unit/test_services_catalog.py | 6 ++-- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index 3ef3264cd2af..593c54b2fb0e 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -3,6 +3,7 @@ from models_library.rest_pagination import ( DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, PageLimitInt, + PageMetaInfoLimitOffset, PageOffsetInt, ) from models_library.users import UserID @@ -15,6 +16,12 @@ _FAKE2: list[SolverPort] = [ SolverPort.model_validate(SolverPort.model_json_schema()["example"]), ] +# from models_library.api_schemas_catalog.services import ( +# LatestServiceGet, +# MyServiceGet, +# ServiceGetV2, +# ServiceUpdateV2, +# ) async def list_latest_releases( @@ -23,11 +30,15 @@ async def list_latest_releases( user_id: UserID, offset: PageOffsetInt = 0, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, -) -> list[Solver]: +) -> tuple[list[Solver], PageMetaInfoLimitOffset]: assert product_name # nosec assert user_id # nosec - return _FAKE[offset : offset + limit] + data = _FAKE[offset : offset + limit] + meta = PageMetaInfoLimitOffset( + limit=limit, offset=offset, total=len(_FAKE), count=len(data) + ) + return data, meta async def list_solver_releases( @@ -37,13 +48,19 @@ async def list_solver_releases( solver_id: SolverKeyId, offset: PageOffsetInt = 0, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, -) -> list[Solver]: +) -> tuple[list[Solver], PageMetaInfoLimitOffset]: assert product_name # nosec assert user_id # nosec - return [solver for solver in _FAKE if solver.id == solver_id][ + + data = [solver for solver in _FAKE if solver.id == solver_id][ offset : offset + limit ] + meta = PageMetaInfoLimitOffset( + limit=limit, offset=offset, total=len(_FAKE), count=len(data) + ) + return data, meta + async def get_solver( *, @@ -55,6 +72,16 @@ async def get_solver( assert product_name # nosec assert user_id # nosec + # service: ServiceGetV2 = await catalog_rpc.get_service( + # get_rabbitmq_rpc_client(app), + # product_name=product_name, + # user_id=user_id, + # service_key=solver_id, + # service_version=solver_version, + # ) + + # solver = Solver(id=service.key, version=service.version, title=) ServiceGetV2)(service) + return next( ( solver diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index 140c2ad000ea..c950868767d4 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -18,19 +18,21 @@ def product_name() -> ProductName: async def test_catalog_service_read_solvers(product_name: ProductName, user_id: UserID): # Step 1: List latest releases in a page - solver_releases_page: list[Solver] = await catalog_service.list_latest_releases( + solver_releases_page, meta = await catalog_service.list_latest_releases( product_name=product_name, user_id=user_id ) assert solver_releases_page, "Releases page should not be empty" + assert meta.offset == 0 # Step 2: Select one release and list solver releases selected_release = solver_releases_page[0] - solver_releases = await catalog_service.list_solver_releases( + solver_releases, meta = await catalog_service.list_solver_releases( product_name=product_name, user_id=user_id, solver_id=selected_release.id, ) assert solver_releases, "Solver releases should not be empty" + assert meta.offset == 0 # Step 3: Take the latest solver release and get solver details latest_solver_release = solver_releases[0] From d514cbab2a3c0a3ca4f91005e5bd0c8a31de07cb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 20:30:24 +0100 Subject: [PATCH 04/23] draft catalog tests --- .../tests/unit/with_dbs/test_api_rpc.py | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/services/catalog/tests/unit/with_dbs/test_api_rpc.py b/services/catalog/tests/unit/with_dbs/test_api_rpc.py index 830650729baa..d3c7395f5680 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -13,6 +13,7 @@ from fastapi import FastAPI from models_library.products import ProductName from models_library.rest_pagination import MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE +from models_library.services import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import ValidationError @@ -29,6 +30,7 @@ batch_get_my_services, check_for_service, get_service, + get_service_release_history, list_services_paginated, update_service, ) @@ -477,3 +479,69 @@ async def test_rpc_batch_get_my_services( "write": True, } assert my_services[1].owner == user["primary_gid"] + assert my_services[1].key == other_service_key + assert my_services[1].release.version == other_service_version + + +async def test_rpc_get_service_release_history( + background_sync_task_mocked: None, + mocked_director_service_api: MockRouter, + rpc_client: RabbitMQRPCClient, + product_name: ProductName, + user_id: UserID, + app: FastAPI, +): + assert app + + service_key = "simcore/services/comp/test-service-release-history" + service_version_1 = "1.0.0" + service_version_2 = "1.1.0" + + # Inject fake service releases for the target service + fake_releases = [ + ServiceRelease(key=service_key, version=service_version_1), + ServiceRelease(key=service_key, version=service_version_2), + ] + + # Inject unrelated fake service releases + unrelated_service_key_1 = "simcore/services/comp/unrelated-service-1" + unrelated_service_key_2 = "simcore/services/comp/unrelated-service-2" + unrelated_releases = [ + ServiceRelease(key=unrelated_service_key_1, version="1.0.0"), + ServiceRelease(key=unrelated_service_key_1, version="1.1.0"), + ServiceRelease(key=unrelated_service_key_2, version="2.0.0"), + ] + + mocked_director_service_api.post( + f"/services/{service_key}/releases", + json=[release.model_dump() for release in fake_releases], + ) + mocked_director_service_api.post( + f"/services/{unrelated_service_key_1}/releases", + json=[release.model_dump() for release in unrelated_releases[:2]], + ) + mocked_director_service_api.post( + f"/services/{unrelated_service_key_2}/releases", + json=[release.model_dump() for release in unrelated_releases[2:]], + ) + + # Call the RPC function + release_history = await get_service_release_history( + rpc_client, + product_name=product_name, + user_id=user_id, + service_key=service_key, + ) + + # Validate the response + assert isinstance(release_history, list) + assert len(release_history) == 2 + assert release_history[0].key == service_key + assert release_history[0].version == service_version_1 + assert release_history[1].key == service_key + assert release_history[1].version == service_version_2 + + # Ensure unrelated services do not appear in the release history + unrelated_keys = {unrelated_service_key_1, unrelated_service_key_2} + for release in release_history: + assert release.key not in unrelated_keys From aeb9c9ad669ba741ca6fbe684e0b7b2f9422bd72 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 20:49:08 +0100 Subject: [PATCH 05/23] draft implementation --- .../api/rpc/_services.py | 21 ++++++++++ .../services/services_api.py | 38 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py index 815b23981fbe..d3e11a08a840 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -12,6 +12,7 @@ from models_library.products import ProductName from models_library.rest_pagination import PageOffsetInt from models_library.rpc_pagination import DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, PageLimitInt +from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import ValidationError, validate_call @@ -220,3 +221,23 @@ async def batch_get_my_services( assert [(sv.key, sv.release.version) for sv in services] == ids # nosec return services + + +@router.expose(reraise_if_error_type=(CatalogForbiddenError, ValidationError)) +@log_decorator(_logger, level=logging.DEBUG) +@validate_call(config={"arbitrary_types_allowed": True}) +async def get_my_service_history( + app: FastAPI, + *, + product_name: ProductName, + user_id: UserID, + service_key: ServiceKey, +) -> list[ServiceRelease]: + assert app.state.engine # nosec + + return await services_api.get_service_history( + repo=ServicesRepository(app.state.engine), + product_name=product_name, + user_id=user_id, + service_key=service_key, + ) diff --git a/services/catalog/src/simcore_service_catalog/services/services_api.py b/services/catalog/src/simcore_service_catalog/services/services_api.py index 843a91fc713c..d11cdd9bfabc 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -464,3 +464,41 @@ async def batch_get_my_services( ) return my_services + + +async def get_service_history( + repo: ServicesRepository, + *, + product_name: ProductName, + user_id: UserID, + service_key: ServiceKey, + include_compatibility: bool = False, +) -> list[ServiceRelease]: + + history = ( + await repo.get_service_history( + # NOTE: that the service history might be different for each user + # since access-rights are defined on a k:v basis + product_name=product_name, + user_id=user_id, + key=service_key, + ) + or [] + ) + + compatibility_map = {} + if include_compatibility: + msg = "This operation is heavy and for the moment is not necessary" + raise NotImplementedError(msg) + + return [ + # domain -> domain + ServiceRelease.model_construct( + version=h.version, + version_display=h.version_display, + released=h.created, + retired=h.deprecated, + compatibility=compatibility_map.get(h.version), + ) + for h in history + ] From f6c7859baa16182807f3102b3651d8274fbd7448 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 20:54:39 +0100 Subject: [PATCH 06/23] draft implementation --- .../api_schemas_catalog/services.py | 3 ++ .../rpc_interfaces/catalog/services.py | 38 +++++++++++++++++++ .../api/rpc/_services.py | 2 +- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_catalog/services.py b/packages/models-library/src/models_library/api_schemas_catalog/services.py index a80490fad36a..6bb52fa970f9 100644 --- a/packages/models-library/src/models_library/api_schemas_catalog/services.py +++ b/packages/models-library/src/models_library/api_schemas_catalog/services.py @@ -365,3 +365,6 @@ class MyServiceGet(CatalogOutputSchema): owner: GroupID | None my_access_rights: ServiceGroupAccessRightsV2 + + +__all__: tuple[str, ...] = ("ServiceRelease",) diff --git a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py index 837ca129638c..35091e0b0ca8 100644 --- a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py +++ b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py @@ -8,6 +8,7 @@ LatestServiceGet, MyServiceGet, ServiceGetV2, + ServiceRelease, ServiceUpdateV2, ) from models_library.products import ProductName @@ -236,3 +237,40 @@ async def _call( result = await _call(product_name=product_name, user_id=user_id, ids=ids) assert TypeAdapter(list[MyServiceGet]).validate_python(result) is not None # nosec return cast(list[MyServiceGet], result) + + +async def get_my_service_history( # pylint: disable=too-many-arguments + rpc_client: RabbitMQRPCClient, + *, + product_name: ProductName, + user_id: UserID, + service_key: ServiceKey, +) -> list[ServiceRelease]: + """ + Raises: + ValidationError: on invalid arguments + """ + + @validate_call() + async def _call( + product_name: ProductName, + user_id: UserID, + service_key: ServiceKey, + ): + return await rpc_client.request( + CATALOG_RPC_NAMESPACE, + TypeAdapter(RPCMethodName).validate_python("get_my_service_history"), + product_name=product_name, + user_id=user_id, + service_key=service_key, + ) + + result = await _call( + product_name=product_name, + user_id=user_id, + service_key=service_key, + ) + assert ( # nosec + TypeAdapter(list[ServiceRelease]).validate_python(result) is not None + ) + return cast(list[ServiceRelease], result) diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py index d3e11a08a840..724cc45711f3 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -223,7 +223,7 @@ async def batch_get_my_services( return services -@router.expose(reraise_if_error_type=(CatalogForbiddenError, ValidationError)) +@router.expose(reraise_if_error_type=(ValidationError,)) @log_decorator(_logger, level=logging.DEBUG) @validate_call(config={"arbitrary_types_allowed": True}) async def get_my_service_history( From c72030dd9b7cb4f90469a8af66493d86bd45c4ab Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 20:58:22 +0100 Subject: [PATCH 07/23] deprecates get operations --- services/catalog/openapi.json | 4 +++ .../api/rest/_services.py | 32 +++++++++++-------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/services/catalog/openapi.json b/services/catalog/openapi.json index 9fbd49878986..9fc1481d2b46 100644 --- a/services/catalog/openapi.json +++ b/services/catalog/openapi.json @@ -473,7 +473,9 @@ "services" ], "summary": "List Services", + "description": "Use instead rpc._service.list_services_paginated -> PageRpcServicesGetV2", "operationId": "list_services_v0_services_get", + "deprecated": true, "parameters": [ { "name": "user_id", @@ -540,7 +542,9 @@ "services" ], "summary": "Get Service", + "description": "Use instead rpc._service.get_service -> ServiceGetV2", "operationId": "get_service_v0_services__service_key___service_version__get", + "deprecated": true, "parameters": [ { "name": "service_key", diff --git a/services/catalog/src/simcore_service_catalog/api/rest/_services.py b/services/catalog/src/simcore_service_catalog/api/rest/_services.py index 4f77c0f8a492..c97a5545d8c5 100644 --- a/services/catalog/src/simcore_service_catalog/api/rest/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rest/_services.py @@ -64,21 +64,23 @@ def _build_cache_key(fct, *_, **kwargs): return f"{fct.__name__}_{kwargs['user_id']}_{kwargs['x_simcore_products_name']}_{kwargs['details']}" -# -# Routes -# - router = APIRouter() -# NOTE: this call is pretty expensive and can be called several times -# (when e2e runs or by the webserver when listing projects) therefore -# a cache is setup here -@router.get("", response_model=list[ServiceGet], **RESPONSE_MODEL_POLICY) +@router.get( + "", + response_model=list[ServiceGet], + **RESPONSE_MODEL_POLICY, + deprecated=True, + description="Use instead rpc._service.list_services_paginated -> PageRpcServicesGetV2", +) @cancel_on_disconnect @cached( ttl=LIST_SERVICES_CACHING_TTL, key_builder=_build_cache_key, + # NOTE: this call is pretty expensive and can be called several times + # (when e2e runs or by the webserver when listing projects) therefore + # a cache is setup here ) async def list_services( request: Request, # pylint:disable=unused-argument @@ -192,6 +194,8 @@ async def cached_registry_services() -> dict[str, Any]: "/{service_key:path}/{service_version}", response_model=ServiceGet, **RESPONSE_MODEL_POLICY, + deprecated=True, + description="Use instead rpc._service.get_service -> ServiceGetV2", ) async def get_service( user_id: int, @@ -226,12 +230,12 @@ async def get_service( ) if service_in_db: # we have full access, let's add the access to the output - service_access_rights: list[ - ServiceAccessRightsAtDB - ] = await services_repo.get_service_access_rights( - service_in_manifest.key, - service_in_manifest.version, - product_name=x_simcore_products_name, + service_access_rights: list[ServiceAccessRightsAtDB] = ( + await services_repo.get_service_access_rights( + service_in_manifest.key, + service_in_manifest.version, + product_name=x_simcore_products_name, + ) ) service_data["access_rights"] = { rights.gid: rights for rights in service_access_rights From 9fc678b85d3c22b3be272007d5fca42e7ee09618 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 20:59:13 +0100 Subject: [PATCH 08/23] =?UTF-8?q?services/catalog=20version:=200.8.0=20?= =?UTF-8?q?=E2=86=92=200.8.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/catalog/VERSION | 2 +- services/catalog/setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/catalog/VERSION b/services/catalog/VERSION index 8adc70fdd9d6..c18d72be3037 100644 --- a/services/catalog/VERSION +++ b/services/catalog/VERSION @@ -1 +1 @@ -0.8.0 \ No newline at end of file +0.8.1 \ No newline at end of file diff --git a/services/catalog/setup.cfg b/services/catalog/setup.cfg index 031198b2fdd1..ac92051993c2 100644 --- a/services/catalog/setup.cfg +++ b/services/catalog/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.8.0 +current_version = 0.8.1 commit = True message = services/catalog version: {current_version} → {new_version} tag = False From 38c1915eba029b358aeda435cb93c7e3f397ba40 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 21:24:13 +0100 Subject: [PATCH 09/23] sideeffects --- .../helpers/catalog_rpc_server.py | 86 ++++++++++++++++++ .../01/test_catalog_handlers__services.py | 90 +++---------------- 2 files changed, 98 insertions(+), 78 deletions(-) create mode 100644 packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py new file mode 100644 index 000000000000..2bda553aecbf --- /dev/null +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py @@ -0,0 +1,86 @@ +# pylint: disable=not-context-manager +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +from models_library.api_schemas_catalog.services import LatestServiceGet, ServiceGetV2 +from models_library.api_schemas_webserver.catalog import ( + CatalogServiceUpdate, +) +from models_library.products import ProductName +from models_library.rpc_pagination import PageLimitInt, PageRpc +from models_library.services_types import ServiceKey, ServiceVersion +from models_library.users import UserID +from pydantic import NonNegativeInt, TypeAdapter +from servicelib.rabbitmq._client_rpc import RabbitMQRPCClient + + +class CatalogRpcSideEffects: + async def list_services_paginated( + self, + rpc_client: RabbitMQRPCClient, + *, + product_name: ProductName, + user_id: UserID, + limit: PageLimitInt, + offset: NonNegativeInt, + ): + assert rpc_client + assert product_name + assert user_id + + items = TypeAdapter(list[LatestServiceGet]).validate_python( + LatestServiceGet.model_json_schema()["examples"], + ) + total_count = len(items) + + return PageRpc[LatestServiceGet].create( + items[offset : offset + limit], + total=total_count, + limit=limit, + offset=offset, + ) + + async def get_service( + self, + rpc_client: RabbitMQRPCClient, + *, + product_name: ProductName, + user_id: UserID, + service_key: ServiceKey, + service_version: ServiceVersion, + ): + assert rpc_client + assert product_name + assert user_id + + got = ServiceGetV2.model_validate( + ServiceGetV2.model_json_schema()["examples"][0] + ) + got.version = service_version + got.key = service_key + + return got + + async def update_service( + self, + rpc_client: RabbitMQRPCClient, + *, + product_name: ProductName, + user_id: UserID, + service_key: ServiceKey, + service_version: ServiceVersion, + update: CatalogServiceUpdate, + ): + assert rpc_client + assert product_name + assert user_id + + got = ServiceGetV2.model_validate( + ServiceGetV2.model_json_schema()["examples"][0] + ) + got.version = service_version + got.key = service_key + return got.model_copy(update=update.model_dump(exclude_unset=True)) diff --git a/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py b/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py index e005192edaea..881282abe6fe 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py +++ b/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py @@ -4,27 +4,22 @@ import re import urllib.parse -from unittest.mock import MagicMock import pytest -from aiohttp import web from aiohttp.test_utils import TestClient from aioresponses import aioresponses as AioResponsesMock from faker import Faker -from models_library.api_schemas_catalog.services import LatestServiceGet, ServiceGetV2 +from models_library.api_schemas_catalog.services import ServiceGetV2 from models_library.api_schemas_webserver.catalog import ( CatalogServiceGet, CatalogServiceUpdate, ) -from models_library.products import ProductName from models_library.rest_pagination import Page -from models_library.rpc_pagination import PageLimitInt, PageRpc -from models_library.services_types import ServiceKey, ServiceVersion -from models_library.users import UserID from models_library.utils.fastapi_encoders import jsonable_encoder -from pydantic import NonNegativeInt, TypeAdapter -from pytest_mock import MockerFixture +from pydantic import TypeAdapter +from pytest_mock import MockerFixture, MockType from pytest_simcore.helpers.assert_checks import assert_status +from pytest_simcore.helpers.catalog_rpc_server import CatalogRpcSideEffects from pytest_simcore.helpers.faker_factories import random_icon_url from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict @@ -50,86 +45,25 @@ def app_environment( @pytest.fixture -def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MagicMock]: - async def _list( - app: web.Application, - *, - product_name: ProductName, - user_id: UserID, - limit: PageLimitInt, - offset: NonNegativeInt, - ): - assert app - assert product_name - assert user_id - - items = TypeAdapter(list[LatestServiceGet]).validate_python( - LatestServiceGet.model_json_schema()["examples"], - ) - total_count = len(items) - - return PageRpc[LatestServiceGet].create( - items[offset : offset + limit], - total=total_count, - limit=limit, - offset=offset, - ) +def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType]: - async def _get( - app: web.Application, - *, - product_name: ProductName, - user_id: UserID, - service_key: ServiceKey, - service_version: ServiceVersion, - ): - assert app - assert product_name - assert user_id - - got = ServiceGetV2.model_validate( - ServiceGetV2.model_json_schema()["examples"][0] - ) - got.version = service_version - got.key = service_key - - return got - - async def _update( - app: web.Application, - *, - product_name: ProductName, - user_id: UserID, - service_key: ServiceKey, - service_version: ServiceVersion, - update: CatalogServiceUpdate, - ): - assert app - assert product_name - assert user_id - - got = ServiceGetV2.model_validate( - ServiceGetV2.model_json_schema()["examples"][0] - ) - got.version = service_version - got.key = service_key - return got.model_copy(update=update.model_dump(exclude_unset=True)) + side_effects = CatalogRpcSideEffects() return { "list_services_paginated": mocker.patch( "simcore_service_webserver.catalog._service.catalog_rpc.list_services_paginated", autospec=True, - side_effect=_list, + side_effect=side_effects.list_services_paginated, ), "get_service": mocker.patch( "simcore_service_webserver.catalog._service.catalog_rpc.get_service", autospec=True, - side_effect=_get, + side_effect=side_effects.get_service, ), "update_service": mocker.patch( "simcore_service_webserver.catalog._service.catalog_rpc.update_service", autospec=True, - side_effect=_update, + side_effect=side_effects.update_service, ), } @@ -141,7 +75,7 @@ async def _update( async def test_list_services_latest( client: TestClient, logged_user: UserInfoDict, - mocked_rpc_catalog_service_api: dict[str, MagicMock], + mocked_rpc_catalog_service_api: dict[str, MockType], ): assert client.app assert client.app.router @@ -359,7 +293,7 @@ async def test_get_compatible_outputs_given_target_inptuts( async def test_get_and_patch_service( client: TestClient, logged_user: UserInfoDict, - mocked_rpc_catalog_service_api: dict[str, MagicMock], + mocked_rpc_catalog_service_api: dict[str, MockType], faker: Faker, ): assert client.app @@ -426,7 +360,7 @@ async def test_get_and_patch_service( async def test_tags_in_services( client: TestClient, logged_user: UserInfoDict, - mocked_rpc_catalog_service_api: dict[str, MagicMock], + mocked_rpc_catalog_service_api: dict[str, MockType], ): assert client.app assert client.app.router From e6bf689cfeeb8c2321d4268956ed3cdad6b847f4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 22:33:55 +0100 Subject: [PATCH 10/23] extending test --- .../services_rpc/catalog.py | 183 ++++++++++-------- .../tests/unit/test_services_catalog.py | 43 +++- 2 files changed, 139 insertions(+), 87 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index 593c54b2fb0e..e11afbc7d812 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -1,3 +1,5 @@ +from dataclasses import dataclass + from models_library.basic_types import VersionStr from models_library.products import ProductName from models_library.rest_pagination import ( @@ -7,6 +9,9 @@ PageOffsetInt, ) from models_library.users import UserID +from servicelib.fastapi.app_state import SingletonInAppStateMixin +from servicelib.rabbitmq import RabbitMQRPCClient +from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc from ..models.schemas.solvers import Solver, SolverKeyId, SolverPort @@ -23,88 +28,96 @@ # ServiceUpdateV2, # ) - -async def list_latest_releases( - *, - product_name: ProductName, - user_id: UserID, - offset: PageOffsetInt = 0, - limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, -) -> tuple[list[Solver], PageMetaInfoLimitOffset]: - assert product_name # nosec - assert user_id # nosec - - data = _FAKE[offset : offset + limit] - meta = PageMetaInfoLimitOffset( - limit=limit, offset=offset, total=len(_FAKE), count=len(data) - ) - return data, meta - - -async def list_solver_releases( - *, - product_name: ProductName, - user_id: UserID, - solver_id: SolverKeyId, - offset: PageOffsetInt = 0, - limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, -) -> tuple[list[Solver], PageMetaInfoLimitOffset]: - assert product_name # nosec - assert user_id # nosec - - data = [solver for solver in _FAKE if solver.id == solver_id][ - offset : offset + limit - ] - - meta = PageMetaInfoLimitOffset( - limit=limit, offset=offset, total=len(_FAKE), count=len(data) - ) - return data, meta - - -async def get_solver( - *, - product_name: ProductName, - user_id: UserID, - solver_id: SolverKeyId, - solver_version: VersionStr, -) -> Solver | None: - assert product_name # nosec - assert user_id # nosec - - # service: ServiceGetV2 = await catalog_rpc.get_service( - # get_rabbitmq_rpc_client(app), - # product_name=product_name, - # user_id=user_id, - # service_key=solver_id, - # service_version=solver_version, - # ) - - # solver = Solver(id=service.key, version=service.version, title=) ServiceGetV2)(service) - - return next( - ( - solver - for solver in _FAKE - if solver.id == solver_id and solver.version == solver_version - ), - None, - ) - - -async def get_solver_ports( - *, - product_name: ProductName, - user_id: int, - solver_id: SolverKeyId, - solver_version: VersionStr, -) -> list[SolverPort]: - - if await get_solver( - product_name=product_name, - user_id=user_id, - solver_id=solver_id, - solver_version=solver_version, - ): - return _FAKE2 - return [] +assert catalog_rpc # nosec + + +@dataclass +class CatalogService(SingletonInAppStateMixin): + app_state_name = "CatalogService" + _client: RabbitMQRPCClient + + async def list_latest_releases( + self, + *, + product_name: ProductName, + user_id: UserID, + offset: PageOffsetInt = 0, + limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, + ) -> tuple[list[Solver], PageMetaInfoLimitOffset]: + assert product_name # nosec + assert user_id # nosec + + data = _FAKE[offset : offset + limit] + meta = PageMetaInfoLimitOffset( + limit=limit, offset=offset, total=len(_FAKE), count=len(data) + ) + return data, meta + + async def list_solver_releases( + self, + *, + product_name: ProductName, + user_id: UserID, + solver_id: SolverKeyId, + offset: PageOffsetInt = 0, + limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, + ) -> tuple[list[Solver], PageMetaInfoLimitOffset]: + assert product_name # nosec + assert user_id # nosec + + data = [solver for solver in _FAKE if solver.id == solver_id][ + offset : offset + limit + ] + + meta = PageMetaInfoLimitOffset( + limit=limit, offset=offset, total=len(_FAKE), count=len(data) + ) + return data, meta + + async def get_solver( + self, + *, + product_name: ProductName, + user_id: UserID, + solver_id: SolverKeyId, + solver_version: VersionStr, + ) -> Solver | None: + assert product_name # nosec + assert user_id # nosec + + # service: ServiceGetV2 = await catalog_rpc.get_service( + # get_rabbitmq_rpc_client(app), + # product_name=product_name, + # user_id=user_id, + # service_key=solver_id, + # service_version=solver_version, + # ) + + # solver = Solver(id=service.key, version=service.version, title=) ServiceGetV2)(service) + + return next( + ( + solver + for solver in _FAKE + if solver.id == solver_id and solver.version == solver_version + ), + None, + ) + + async def get_solver_ports( + self, + *, + product_name: ProductName, + user_id: int, + solver_id: SolverKeyId, + solver_version: VersionStr, + ) -> list[SolverPort]: + + if await self.get_solver( + product_name=product_name, + user_id=user_id, + solver_id=solver_id, + solver_version=solver_version, + ): + return _FAKE2 + return [] diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index c950868767d4..c5b7b46e42f6 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -5,10 +5,13 @@ # pylint: disable=unused-variable import pytest +from fastapi import FastAPI from models_library.products import ProductName from models_library.users import UserID +from pytest_mock import MockerFixture, MockType +from pytest_simcore.helpers.catalog_rpc_server import CatalogRpcSideEffects from simcore_service_api_server.models.schemas.solvers import Solver, SolverPort -from simcore_service_api_server.services_rpc import catalog as catalog_service +from simcore_service_api_server.services_rpc.catalog import CatalogService, catalog_rpc @pytest.fixture @@ -16,7 +19,43 @@ def product_name() -> ProductName: return "osparc" -async def test_catalog_service_read_solvers(product_name: ProductName, user_id: UserID): +@pytest.fixture +def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType]: + + side_effects = CatalogRpcSideEffects() + + return { + "list_services_paginated": mocker.patch.object( + catalog_rpc, + "list_services_paginated", + autospec=True, + side_effect=side_effects.list_services_paginated, + ), + "get_service": mocker.patch.object( + catalog_rpc, + "get_service", + autospec=True, + side_effect=side_effects.get_service, + ), + "update_service": mocker.patch.object( + catalog_rpc, + "update_service", + autospec=True, + side_effect=side_effects.update_service, + ), + } + + +async def test_catalog_service_read_solvers( + product_name: ProductName, + user_id: UserID, + mocker: MockerFixture, + mocked_rpc_catalog_service_api: dict[str, MockType], +): + + catalog_service = CatalogService(_client=mocker.MagicMock()) + catalog_service.set_to_app_state(app=FastAPI()) + # Step 1: List latest releases in a page solver_releases_page, meta = await catalog_service.list_latest_releases( product_name=product_name, user_id=user_id From eec071fd944754664c7ae1bdd2fb8c863d97ba9a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 23:07:15 +0100 Subject: [PATCH 11/23] refactor --- .../services_rpc/catalog.py | 114 ++++++------------ .../tests/unit/test_services_catalog.py | 52 +++++--- 2 files changed, 72 insertions(+), 94 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index e11afbc7d812..8e868a2d595c 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -1,6 +1,5 @@ from dataclasses import dataclass -from models_library.basic_types import VersionStr from models_library.products import ProductName from models_library.rest_pagination import ( DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, @@ -8,28 +7,14 @@ PageMetaInfoLimitOffset, PageOffsetInt, ) +from models_library.services_history import ServiceRelease +from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID +from pytest_simcore.helpers.catalog_rpc_server import LatestServiceGet, ServiceGetV2 from servicelib.fastapi.app_state import SingletonInAppStateMixin from servicelib.rabbitmq import RabbitMQRPCClient from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc -from ..models.schemas.solvers import Solver, SolverKeyId, SolverPort - -_FAKE: list[Solver] = [ - Solver.model_validate(Solver.model_json_schema()["example"]), -] -_FAKE2: list[SolverPort] = [ - SolverPort.model_validate(SolverPort.model_json_schema()["example"]), -] -# from models_library.api_schemas_catalog.services import ( -# LatestServiceGet, -# MyServiceGet, -# ServiceGetV2, -# ServiceUpdateV2, -# ) - -assert catalog_rpc # nosec - @dataclass class CatalogService(SingletonInAppStateMixin): @@ -43,81 +28,62 @@ async def list_latest_releases( user_id: UserID, offset: PageOffsetInt = 0, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, - ) -> tuple[list[Solver], PageMetaInfoLimitOffset]: - assert product_name # nosec - assert user_id # nosec + ) -> tuple[list[LatestServiceGet], PageMetaInfoLimitOffset]: - data = _FAKE[offset : offset + limit] + page = await catalog_rpc.list_services_paginated( + self._client, + product_name=product_name, + user_id=user_id, + offset=offset, + limit=limit, + ) meta = PageMetaInfoLimitOffset( - limit=limit, offset=offset, total=len(_FAKE), count=len(data) + limit=page.meta.limit, + offset=page.meta.offset, + total=page.meta.total, + count=page.meta.count, ) - return data, meta + return page.data, meta - async def list_solver_releases( + async def list_release_history( self, *, product_name: ProductName, user_id: UserID, - solver_id: SolverKeyId, + service_key: ServiceKey, offset: PageOffsetInt = 0, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, - ) -> tuple[list[Solver], PageMetaInfoLimitOffset]: - assert product_name # nosec - assert user_id # nosec - - data = [solver for solver in _FAKE if solver.id == solver_id][ - offset : offset + limit - ] + ) -> tuple[list[ServiceRelease], PageMetaInfoLimitOffset]: + data = await catalog_rpc.get_my_service_history( + self._client, + product_name=product_name, + user_id=user_id, + service_key=service_key, + # TODO: offset=offset, + # TODO: limit=limit, + ) meta = PageMetaInfoLimitOffset( - limit=limit, offset=offset, total=len(_FAKE), count=len(data) + limit=limit, + offset=offset, + total=len(data), + count=len(data), ) return data, meta - async def get_solver( + async def get( self, *, product_name: ProductName, user_id: UserID, - solver_id: SolverKeyId, - solver_version: VersionStr, - ) -> Solver | None: - assert product_name # nosec - assert user_id # nosec - - # service: ServiceGetV2 = await catalog_rpc.get_service( - # get_rabbitmq_rpc_client(app), - # product_name=product_name, - # user_id=user_id, - # service_key=solver_id, - # service_version=solver_version, - # ) - - # solver = Solver(id=service.key, version=service.version, title=) ServiceGetV2)(service) - - return next( - ( - solver - for solver in _FAKE - if solver.id == solver_id and solver.version == solver_version - ), - None, - ) + service_key: ServiceKey, + service_version: ServiceVersion, + ) -> ServiceGetV2: - async def get_solver_ports( - self, - *, - product_name: ProductName, - user_id: int, - solver_id: SolverKeyId, - solver_version: VersionStr, - ) -> list[SolverPort]: - - if await self.get_solver( + return await catalog_rpc.get_service( + self._client, product_name=product_name, user_id=user_id, - solver_id=solver_id, - solver_version=solver_version, - ): - return _FAKE2 - return [] + service_key=service_key, + service_version=service_version, + ) diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index c5b7b46e42f6..5463ee8adc2d 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -6,11 +6,14 @@ import pytest from fastapi import FastAPI +from models_library.api_schemas_catalog.services import LatestServiceGet, ServiceGetV2 from models_library.products import ProductName +from models_library.services_history import ServiceRelease from models_library.users import UserID +from pydantic import HttpUrl from pytest_mock import MockerFixture, MockType from pytest_simcore.helpers.catalog_rpc_server import CatalogRpcSideEffects -from simcore_service_api_server.models.schemas.solvers import Solver, SolverPort +from simcore_service_api_server.models.schemas.solvers import Solver from simcore_service_api_server.services_rpc.catalog import CatalogService, catalog_rpc @@ -46,48 +49,57 @@ def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType] } +def to_solver( + service: LatestServiceGet | ServiceGetV2, href_self: HttpUrl | None = None +) -> Solver: + # NOTE: this is an adapter around models on CatalogService interface + return Solver( + id=service.key, + version=service.version, + title=service.name, + maintainer=service.owner or service.contact or "UNKNOWN", + url=href_self, + description=service.description, + ) + + async def test_catalog_service_read_solvers( product_name: ProductName, user_id: UserID, mocker: MockerFixture, mocked_rpc_catalog_service_api: dict[str, MockType], ): - catalog_service = CatalogService(_client=mocker.MagicMock()) catalog_service.set_to_app_state(app=FastAPI()) # Step 1: List latest releases in a page - solver_releases_page, meta = await catalog_service.list_latest_releases( + latest_releases, meta = await catalog_service.list_latest_releases( product_name=product_name, user_id=user_id ) + solver_releases_page = [to_solver(srv) for srv in latest_releases] + assert solver_releases_page, "Releases page should not be empty" assert meta.offset == 0 # Step 2: Select one release and list solver releases - selected_release = solver_releases_page[0] - solver_releases, meta = await catalog_service.list_solver_releases( + selected_solver = solver_releases_page[0] + releases, meta = await catalog_service.list_release_history( product_name=product_name, user_id=user_id, - solver_id=selected_release.id, + service_key=selected_solver.id, ) - assert solver_releases, "Solver releases should not be empty" + assert releases, "Solver releases should not be empty" assert meta.offset == 0 # Step 3: Take the latest solver release and get solver details - latest_solver_release = solver_releases[0] - solver_details: Solver | None = await catalog_service.get_solver( - product_name=product_name, - user_id=user_id, - solver_id=latest_solver_release.id, - solver_version=latest_solver_release.version, - ) - assert solver_details, "Solver details should not be empty" + oldest_release: ServiceRelease = releases[-1] - # Step 4: Get solver ports - solver_ports: list[SolverPort] = await catalog_service.get_solver_ports( + service: ServiceGetV2 = await catalog_service.get( product_name=product_name, user_id=user_id, - solver_id=latest_solver_release.id, - solver_version=latest_solver_release.version, + service_key=selected_solver.id, + service_version=oldest_release.version, ) - assert solver_ports, "Solver ports should not be empty" + solver = to_solver(service) + assert solver.id == selected_solver.id + assert solver.version == oldest_release.version From 6a9b28947c076d27fd97b30177d2e1c169b65791 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 23:42:07 +0100 Subject: [PATCH 12/23] adding pagination to history --- .../api_schemas_catalog/services.py | 7 +++- .../helpers/catalog_rpc_server.py | 32 +++++++++++++++++++ .../rpc_interfaces/catalog/services.py | 19 ++++++++--- .../services_rpc/catalog.py | 16 +++++----- .../api/rpc/_services.py | 16 ++++++---- 5 files changed, 70 insertions(+), 20 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_catalog/services.py b/packages/models-library/src/models_library/api_schemas_catalog/services.py index 6bb52fa970f9..5e5e6de88143 100644 --- a/packages/models-library/src/models_library/api_schemas_catalog/services.py +++ b/packages/models-library/src/models_library/api_schemas_catalog/services.py @@ -323,11 +323,16 @@ def _update_json_schema_extra(schema: JsonDict) -> None: ) -PageRpcServicesGetV2: TypeAlias = PageRpc[ +PageRpcLatestServiceGet: TypeAlias = PageRpc[ # WARNING: keep this definition in models_library and not in the RPC interface LatestServiceGet ] +PageRpcServiceRelease: TypeAlias = PageRpc[ + # WARNING: keep this definition in models_library and not in the RPC interface + ServiceRelease +] + ServiceResourcesGet: TypeAlias = ServiceResourcesDict diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py index 2bda553aecbf..6c13babadb04 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py @@ -10,7 +10,9 @@ CatalogServiceUpdate, ) from models_library.products import ProductName +from models_library.rest_pagination import PageOffsetInt from models_library.rpc_pagination import PageLimitInt, PageRpc +from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import NonNegativeInt, TypeAdapter @@ -84,3 +86,33 @@ async def update_service( got.version = service_version got.key = service_key return got.model_copy(update=update.model_dump(exclude_unset=True)) + + async def get_my_service_history( + self, + rpc_client: RabbitMQRPCClient, + *, + product_name: ProductName, + user_id: UserID, + service_key: ServiceKey, + limit: PageLimitInt, + offset: PageOffsetInt, + ) -> PageRpc[ServiceRelease]: + + assert rpc_client + assert product_name + assert user_id + assert service_key + + items = TypeAdapter(list[ServiceRelease]).validate_python( + [ + ServiceRelease.model_json_schema()["example"], + ] + ) + total_count = len(items) + + return PageRpc[ServiceRelease].create( + items[offset : offset + limit], + total=total_count, + limit=limit, + offset=offset, + ) diff --git a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py index 35091e0b0ca8..9f49c4aea87f 100644 --- a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py +++ b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py @@ -7,6 +7,8 @@ from models_library.api_schemas_catalog.services import ( LatestServiceGet, MyServiceGet, + PageRpcLatestServiceGet, + PageRpcServiceRelease, ServiceGetV2, ServiceRelease, ServiceUpdateV2, @@ -37,7 +39,7 @@ async def list_services_paginated( # pylint: disable=too-many-arguments user_id: UserID, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, offset: PageOffsetInt = 0, -) -> PageRpc[LatestServiceGet]: +) -> PageRpcLatestServiceGet: """ Raises: ValidationError: on invalid arguments @@ -245,7 +247,9 @@ async def get_my_service_history( # pylint: disable=too-many-arguments product_name: ProductName, user_id: UserID, service_key: ServiceKey, -) -> list[ServiceRelease]: + limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, + offset: PageOffsetInt = 0, +) -> PageRpcServiceRelease: """ Raises: ValidationError: on invalid arguments @@ -256,6 +260,8 @@ async def _call( product_name: ProductName, user_id: UserID, service_key: ServiceKey, + limit: PageLimitInt, + offset: PageOffsetInt, ): return await rpc_client.request( CATALOG_RPC_NAMESPACE, @@ -263,14 +269,19 @@ async def _call( product_name=product_name, user_id=user_id, service_key=service_key, + limit=limit, + offset=offset, ) result = await _call( product_name=product_name, user_id=user_id, service_key=service_key, + limit=limit, + offset=offset, ) + assert ( # nosec - TypeAdapter(list[ServiceRelease]).validate_python(result) is not None + TypeAdapter(PageRpcServiceRelease).validate_python(result) is not None ) - return cast(list[ServiceRelease], result) + return cast(PageRpc[ServiceRelease], result) diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index 8e868a2d595c..2882ae30ae3e 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -55,21 +55,21 @@ async def list_release_history( limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, ) -> tuple[list[ServiceRelease], PageMetaInfoLimitOffset]: - data = await catalog_rpc.get_my_service_history( + page = await catalog_rpc.get_my_service_history( self._client, product_name=product_name, user_id=user_id, service_key=service_key, - # TODO: offset=offset, - # TODO: limit=limit, + offset=offset, + limit=limit, ) meta = PageMetaInfoLimitOffset( - limit=limit, - offset=offset, - total=len(data), - count=len(data), + limit=page.meta.limit, + offset=page.meta.offset, + total=page.meta.total, + count=page.meta.count, ) - return data, meta + return page.data, meta async def get( self, diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py index 724cc45711f3..c5d15616efea 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -5,14 +5,14 @@ from fastapi import FastAPI from models_library.api_schemas_catalog.services import ( MyServiceGet, - PageRpcServicesGetV2, + PageRpcLatestServiceGet, + PageRpcServiceRelease, ServiceGetV2, ServiceUpdateV2, ) from models_library.products import ProductName from models_library.rest_pagination import PageOffsetInt from models_library.rpc_pagination import DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, PageLimitInt -from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import ValidationError, validate_call @@ -65,7 +65,7 @@ async def list_services_paginated( user_id: UserID, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, offset: PageOffsetInt = 0, -) -> PageRpcServicesGetV2: +) -> PageRpcLatestServiceGet: assert app.state.engine # nosec total_count, items = await services_api.list_latest_services( @@ -81,8 +81,8 @@ async def list_services_paginated( assert len(items) <= limit # nosec return cast( - PageRpcServicesGetV2, - PageRpcServicesGetV2.create( + PageRpcLatestServiceGet, + PageRpcLatestServiceGet.create( items, total=total_count, limit=limit, @@ -232,10 +232,12 @@ async def get_my_service_history( product_name: ProductName, user_id: UserID, service_key: ServiceKey, -) -> list[ServiceRelease]: + limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, + offset: PageOffsetInt = 0, +) -> PageRpcServiceRelease: assert app.state.engine # nosec - return await services_api.get_service_history( + history = await services_api.get_service_history( repo=ServicesRepository(app.state.engine), product_name=product_name, user_id=user_id, From f762de71a3a670ce84b54c39eb2fa068ade158e8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Mar 2025 23:56:46 +0100 Subject: [PATCH 13/23] adapting catalog --- .../api/rpc/_services.py | 17 +++++++++- .../db/repositories/_services_sql.py | 6 +++- .../db/repositories/services.py | 26 ++++++++++----- .../services/services_api.py | 32 ++++++++++--------- .../unit/test_db_repositories_services_sql.py | 4 +-- 5 files changed, 58 insertions(+), 27 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py index c5d15616efea..d419c01ec9e3 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -237,9 +237,24 @@ async def get_my_service_history( ) -> PageRpcServiceRelease: assert app.state.engine # nosec - history = await services_api.get_service_history( + total_count, items = await services_api.get_service_history( repo=ServicesRepository(app.state.engine), product_name=product_name, user_id=user_id, service_key=service_key, + limit=limit, + offset=offset, + ) + + assert len(items) <= total_count # nosec + assert len(items) <= limit # nosec + + return cast( + PageRpcServiceRelease, + PageRpcServiceRelease.create( + items, + total=total_count, + limit=limit, + offset=offset, + ), ) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py b/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py index d83dffbdb220..34637fd12dc2 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py @@ -111,7 +111,7 @@ def _has_access_rights( ) -def total_count_stmt( +def latest_services_total_count_stmt( *, product_name: ProductName, user_id: UserID, @@ -312,7 +312,11 @@ def get_service_history_stmt( user_id: UserID, access_rights: sa.sql.ClauseElement, service_key: ServiceKey, + limit: int | None, + offset: int | None, ): + assert offset is None, "UNDER DEV" + assert limit is None, "UNDER DEV" _sq = ( sa.select( diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index 509a23d68d69..b7f5f25c5147 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -38,9 +38,9 @@ can_get_service_stmt, get_service_history_stmt, get_service_stmt, + latest_services_total_count_stmt, list_latest_services_stmt, list_services_stmt, - total_count_stmt, ) _logger = logging.getLogger(__name__) @@ -340,6 +340,8 @@ async def get_service_with_history( user_id=user_id, access_rights=AccessRightsClauses.can_read, service_key=key, + limit=None, + offset=None, ) async with self.db_engine.begin() as conn: result = await conn.execute(stmt_history) @@ -381,7 +383,7 @@ async def list_latest_services( ) -> tuple[PositiveInt, list[ServiceWithHistoryDBGet]]: # get page - stmt_total = total_count_stmt( + stmt_total = latest_services_total_count_stmt( product_name=product_name, user_id=user_id, access_rights=AccessRightsClauses.can_read, @@ -438,24 +440,32 @@ async def get_service_history( user_id: UserID, # get args key: ServiceKey, - ) -> list[ReleaseDBGet] | None: + # list args: pagination + limit: int | None = None, + offset: int | None = None, + ) -> tuple[PositiveInt, list[ReleaseDBGet]]: - stmt_history = get_service_history_stmt( + stmt_total, stmt_history = get_service_history_stmt( product_name=product_name, user_id=user_id, access_rights=AccessRightsClauses.can_read, service_key=key, + offset=offset, + limit=limit, ) async with self.db_engine.connect() as conn: + result = await conn.execute(stmt_total) + total_count = result.scalar() or 0 + result = await conn.execute(stmt_history) row = result.one_or_none() - return ( - TypeAdapter(list[ReleaseDBGet]).validate_python(row.history) - if row - else None + items = ( + TypeAdapter(list[ReleaseDBGet]).validate_python(row.history) if row else [] ) + return total_count, items + # Service Access Rights ---- async def get_service_access_rights( diff --git a/services/catalog/src/simcore_service_catalog/services/services_api.py b/services/catalog/src/simcore_service_catalog/services/services_api.py index d11cdd9bfabc..3ea550f6dda3 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -9,7 +9,7 @@ ) from models_library.groups import GroupID from models_library.products import ProductName -from models_library.rest_pagination import PageLimitInt +from models_library.rest_pagination import PageLimitInt, PageTotalCount from models_library.services_access import ServiceGroupAccessRightsV2 from models_library.services_history import Compatibility, ServiceRelease from models_library.services_metadata_published import ServiceMetaDataPublished @@ -125,7 +125,7 @@ async def list_latest_services( user_id: UserID, limit: PageLimitInt | None, offset: NonNegativeInt = 0, -) -> tuple[NonNegativeInt, list[LatestServiceGet]]: +) -> tuple[PageTotalCount, list[LatestServiceGet]]: # defines the order total_count, services = await repo.list_latest_services( @@ -472,26 +472,26 @@ async def get_service_history( product_name: ProductName, user_id: UserID, service_key: ServiceKey, + limit: PageLimitInt | None = None, + offset: NonNegativeInt | None = None, include_compatibility: bool = False, -) -> list[ServiceRelease]: - - history = ( - await repo.get_service_history( - # NOTE: that the service history might be different for each user - # since access-rights are defined on a k:v basis - product_name=product_name, - user_id=user_id, - key=service_key, - ) - or [] - ) +) -> tuple[PageTotalCount, list[ServiceRelease]]: + total_count, history = await repo.get_service_history( + # NOTE: that the service history might be different for each user + # since access-rights are defined on a k:v basis + product_name=product_name, + user_id=user_id, + key=service_key, + limit=limit, + offset=offset, + ) or (0, []) compatibility_map = {} if include_compatibility: msg = "This operation is heavy and for the moment is not necessary" raise NotImplementedError(msg) - return [ + items = [ # domain -> domain ServiceRelease.model_construct( version=h.version, @@ -502,3 +502,5 @@ async def get_service_history( ) for h in history ] + + return total_count, items diff --git a/services/catalog/tests/unit/test_db_repositories_services_sql.py b/services/catalog/tests/unit/test_db_repositories_services_sql.py index 09891d5fe5e1..27b8db9e770d 100644 --- a/services/catalog/tests/unit/test_db_repositories_services_sql.py +++ b/services/catalog/tests/unit/test_db_repositories_services_sql.py @@ -9,8 +9,8 @@ can_get_service_stmt, get_service_history_stmt, get_service_stmt, + latest_services_total_count_stmt, list_latest_services_stmt, - total_count_stmt, ) @@ -70,7 +70,7 @@ def _check(func_smt, **kwargs): ) _check( - total_count_stmt, + latest_services_total_count_stmt, product_name=product_name, user_id=user_id, access_rights=AccessRightsClauses.can_read, From 927dc9c00c33dda3738f943b475dff79506b4b26 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 00:50:04 +0100 Subject: [PATCH 14/23] draft tests --- .../db/repositories/_services_sql.py | 11 +- .../db/repositories/services.py | 102 +++++++++++++++--- .../services/services_api.py | 4 +- .../unit/with_dbs/test_db_repositories.py | 66 ++++++++++++ 4 files changed, 158 insertions(+), 25 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py b/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py index 34637fd12dc2..935a38349761 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/_services_sql.py @@ -66,7 +66,7 @@ def list_services_stmt( return stmt -def _version(column_or_value): +def by_version(column_or_value): # converts version value string to array[integer] that can be compared # i.e. '1.2.3' -> [1, 2, 3] return sa.func.string_to_array(column_or_value, ".").cast(ARRAY(INTEGER)) @@ -165,7 +165,7 @@ def list_latest_services_stmt( .where(access_rights) .order_by( services_meta_data.c.key, - sa.desc(_version(services_meta_data.c.version)), # latest first + sa.desc(by_version(services_meta_data.c.version)), # latest first ) .distinct(services_meta_data.c.key) # get only first .limit(limit) @@ -312,12 +312,7 @@ def get_service_history_stmt( user_id: UserID, access_rights: sa.sql.ClauseElement, service_key: ServiceKey, - limit: int | None, - offset: int | None, ): - assert offset is None, "UNDER DEV" - assert limit is None, "UNDER DEV" - _sq = ( sa.select( services_meta_data.c.key, @@ -356,7 +351,7 @@ def get_service_history_stmt( history_subquery = ( sa.select(_sq) .order_by( - sa.desc(_version(_sq.c.version)), # latest version first + sa.desc(by_version(_sq.c.version)), # latest version first ) .alias("history_subquery") ) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index b7f5f25c5147..2ac819177be3 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -16,7 +16,9 @@ from models_library.users import UserID from psycopg2.errors import ForeignKeyViolation from pydantic import PositiveInt, TypeAdapter, ValidationError +from simcore_postgres_database.utils_repos import pass_or_acquire_connection from simcore_postgres_database.utils_services import create_select_latest_services_query +from sqlalchemy import sql from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.sql import and_, or_ from sqlalchemy.sql.expression import tuple_ @@ -30,11 +32,18 @@ ServiceWithHistoryDBGet, ) from ...models.services_specifications import ServiceSpecificationsAtDB -from ..tables import services_access_rights, services_meta_data, services_specifications +from ..tables import ( + services_access_rights, + services_compatibility, + services_meta_data, + services_specifications, + user_to_groups, +) from ._base import BaseRepository from ._services_sql import ( SERVICES_META_DATA_COLS, AccessRightsClauses, + by_version, can_get_service_stmt, get_service_history_stmt, get_service_stmt, @@ -340,8 +349,6 @@ async def get_service_with_history( user_id=user_id, access_rights=AccessRightsClauses.can_read, service_key=key, - limit=None, - offset=None, ) async with self.db_engine.begin() as conn: result = await conn.execute(stmt_history) @@ -440,30 +447,95 @@ async def get_service_history( user_id: UserID, # get args key: ServiceKey, - # list args: pagination - limit: int | None = None, - offset: int | None = None, - ) -> tuple[PositiveInt, list[ReleaseDBGet]]: - - stmt_total, stmt_history = get_service_history_stmt( + ) -> list[ReleaseDBGet]: + """ + DEPRECATED: use get_service_history_page instead! + """ + stmt_history = get_service_history_stmt( product_name=product_name, user_id=user_id, access_rights=AccessRightsClauses.can_read, service_key=key, - offset=offset, - limit=limit, ) async with self.db_engine.connect() as conn: - result = await conn.execute(stmt_total) - total_count = result.scalar() or 0 - result = await conn.execute(stmt_history) row = result.one_or_none() - items = ( + return ( TypeAdapter(list[ReleaseDBGet]).validate_python(row.history) if row else [] ) + async def get_service_history_page( + self, + # access-rights + product_name: ProductName, + user_id: UserID, + # get args + key: ServiceKey, + # list args: pagination + limit: int | None = None, + offset: int | None = None, + ) -> tuple[PositiveInt, list[ReleaseDBGet]]: + + base_query = ( + sql.select( + services_meta_data.c.key, + services_meta_data.c.version, + services_meta_data.c.version_display, + services_meta_data.c.deprecated, + services_meta_data.c.created, + services_compatibility.c.custom_policy.label( + "compatibility_policy" + ), # CompatiblePolicyDict | None + ) + .select_from( + # joins because access-rights might change per version + services_meta_data.join( + services_access_rights, + (services_meta_data.c.key == services_access_rights.c.key) + & ( + services_meta_data.c.version == services_access_rights.c.version + ), + ) + .join( + user_to_groups, + (user_to_groups.c.gid == services_access_rights.c.gid), + ) + .outerjoin( + services_compatibility, + (services_meta_data.c.key == services_compatibility.c.key) + & ( + services_meta_data.c.version == services_compatibility.c.version + ), + ) + ) + .where( + (services_meta_data.c.key == key) + & (services_access_rights.c.product_name == product_name) + & (user_to_groups.c.uid == user_id) + & AccessRightsClauses.can_read + ) + .distinct() + ) + + subquery = base_query.subquery() + count_query = sql.select(sql.func.count()).select_from(subquery) + + page_query = ( + base_query.order_by(sql.desc(by_version(base_query.c.version))) + .offset(offset) + .limit(limit) + ) + + async with pass_or_acquire_connection(self.db_engine) as conn: + total_count: PositiveInt = await conn.scalar(count_query) or 0 + + result = await conn.stream(page_query) + items: list[ReleaseDBGet] = [ + ReleaseDBGet.model_validate(row, from_attributes=True) + async for row in result + ] + return total_count, items # Service Access Rights ---- diff --git a/services/catalog/src/simcore_service_catalog/services/services_api.py b/services/catalog/src/simcore_service_catalog/services/services_api.py index 3ea550f6dda3..ed7aaf88892c 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -476,7 +476,7 @@ async def get_service_history( offset: NonNegativeInt | None = None, include_compatibility: bool = False, ) -> tuple[PageTotalCount, list[ServiceRelease]]: - total_count, history = await repo.get_service_history( + total_count, history = await repo.get_service_history_page( # NOTE: that the service history might be different for each user # since access-rights are defined on a k:v basis product_name=product_name, @@ -484,7 +484,7 @@ async def get_service_history( key=service_key, limit=limit, offset=offset, - ) or (0, []) + ) compatibility_map = {} if include_compatibility: diff --git a/services/catalog/tests/unit/with_dbs/test_db_repositories.py b/services/catalog/tests/unit/with_dbs/test_db_repositories.py index e8990527c047..fdcfb9f182e9 100644 --- a/services/catalog/tests/unit/with_dbs/test_db_repositories.py +++ b/services/catalog/tests/unit/with_dbs/test_db_repositories.py @@ -476,3 +476,69 @@ async def test_can_get_service( key=service_key, version=service_version, ) + + +async def test_get_service_history_page( + target_product: ProductName, + create_fake_service_data: Callable, + services_db_tables_injector: Callable, + services_repo: ServicesRepository, + user_id: UserID, +): + # inject services with multiple versions + service_key = "simcore/services/dynamic/test-service" + num_versions = 10 + await services_db_tables_injector( + [ + create_fake_service_data( + service_key, + f"{v}.0.0", + team_access=None, + everyone_access=None, + product=target_product, + ) + for v in range(num_versions) + ] + ) + + # fetch full history using get_service_history_page + total_count, history = await services_repo.get_service_history_page( + product_name=target_product, + user_id=user_id, + key=service_key, + ) + assert total_count == num_versions + assert len(history) == num_versions + assert [release.version for release in history] == [ + f"{v}.0.0" for v in reversed(range(num_versions)) + ] + + # fetch full history using deprecated get_service_history + deprecated_history = await services_repo.get_service_history( + product_name=target_product, + user_id=user_id, + key=service_key, + ) + assert len(deprecated_history) == len(history) + assert [release.version for release in deprecated_history] == [ + release.version for release in history + ] + + # fetch paginated history + limit = 3 + offset = 2 + total_count, paginated_history = await services_repo.get_service_history_page( + product_name=target_product, + user_id=user_id, + key=service_key, + limit=limit, + offset=offset, + ) + assert total_count == num_versions + assert len(paginated_history) == limit + assert [release.version for release in paginated_history] == [ + f"{v}.0.0" for v in reversed(range(offset, offset + limit)) + ] + + # compare paginated results with the corresponding slice of the full history + assert paginated_history == history[offset : offset + limit] From 8cfa6a925389253ab95de040818d5444df5d50ec Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 00:55:23 +0100 Subject: [PATCH 15/23] fixes tests --- .../api/rpc/_services.py | 2 +- .../db/repositories/services.py | 1 + .../services/services_api.py | 10 ++- .../tests/unit/with_dbs/test_api_rpc.py | 61 ++++++++++--------- 4 files changed, 43 insertions(+), 31 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py index d419c01ec9e3..f5b5b1c98677 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -237,7 +237,7 @@ async def get_my_service_history( ) -> PageRpcServiceRelease: assert app.state.engine # nosec - total_count, items = await services_api.get_service_history( + total_count, items = await services_api.list_my_service_release_history( repo=ServicesRepository(app.state.engine), product_name=product_name, user_id=user_id, diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index 2ac819177be3..9a2a3507676f 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -467,6 +467,7 @@ async def get_service_history( async def get_service_history_page( self, + *, # access-rights product_name: ProductName, user_id: UserID, diff --git a/services/catalog/src/simcore_service_catalog/services/services_api.py b/services/catalog/src/simcore_service_catalog/services/services_api.py index ed7aaf88892c..43a70f45bf27 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -121,6 +121,7 @@ def _to_get_schema( async def list_latest_services( repo: ServicesRepository, director_api: DirectorApi, + *, product_name: ProductName, user_id: UserID, limit: PageLimitInt | None, @@ -466,16 +467,21 @@ async def batch_get_my_services( return my_services -async def get_service_history( +async def list_my_service_release_history( repo: ServicesRepository, *, + # access-rights product_name: ProductName, user_id: UserID, + # target service service_key: ServiceKey, + # pagination limit: PageLimitInt | None = None, offset: NonNegativeInt | None = None, + # options include_compatibility: bool = False, ) -> tuple[PageTotalCount, list[ServiceRelease]]: + total_count, history = await repo.get_service_history_page( # NOTE: that the service history might be different for each user # since access-rights are defined on a k:v basis @@ -486,7 +492,7 @@ async def get_service_history( offset=offset, ) - compatibility_map = {} + compatibility_map: dict[ServiceVersion, Compatibility | None] = {} if include_compatibility: msg = "This operation is heavy and for the moment is not necessary" raise NotImplementedError(msg) diff --git a/services/catalog/tests/unit/with_dbs/test_api_rpc.py b/services/catalog/tests/unit/with_dbs/test_api_rpc.py index d3c7395f5680..4d4af698aa2f 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -13,7 +13,7 @@ from fastapi import FastAPI from models_library.products import ProductName from models_library.rest_pagination import MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE -from models_library.services import ServiceRelease +from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import ValidationError @@ -29,8 +29,8 @@ from servicelib.rabbitmq.rpc_interfaces.catalog.services import ( batch_get_my_services, check_for_service, + get_my_service_history, get_service, - get_service_release_history, list_services_paginated, update_service, ) @@ -483,13 +483,15 @@ async def test_rpc_batch_get_my_services( assert my_services[1].release.version == other_service_version -async def test_rpc_get_service_release_history( +async def test_rpc_get_my_service_history( background_sync_task_mocked: None, mocked_director_service_api: MockRouter, rpc_client: RabbitMQRPCClient, product_name: ProductName, user_id: UserID, app: FastAPI, + create_fake_service_data: Callable, + services_db_tables_injector: Callable, ): assert app @@ -499,49 +501,52 @@ async def test_rpc_get_service_release_history( # Inject fake service releases for the target service fake_releases = [ - ServiceRelease(key=service_key, version=service_version_1), - ServiceRelease(key=service_key, version=service_version_2), + create_fake_service_data( + service_key, + srv_version, + team_access=None, + everyone_access=None, + product=product_name, + ) + for srv_version in (service_version_1, service_version_2) ] # Inject unrelated fake service releases unrelated_service_key_1 = "simcore/services/comp/unrelated-service-1" unrelated_service_key_2 = "simcore/services/comp/unrelated-service-2" unrelated_releases = [ - ServiceRelease(key=unrelated_service_key_1, version="1.0.0"), - ServiceRelease(key=unrelated_service_key_1, version="1.1.0"), - ServiceRelease(key=unrelated_service_key_2, version="2.0.0"), + *[ + create_fake_service_data( + unrelated_service_key_1, + srv_version, + team_access=None, + everyone_access=None, + product=product_name, + ) + for srv_version in (service_version_1, service_version_2) + ], + create_fake_service_data( + unrelated_service_key_2, + "2.0.0", + team_access=None, + everyone_access=None, + product=product_name, + ), ] - mocked_director_service_api.post( - f"/services/{service_key}/releases", - json=[release.model_dump() for release in fake_releases], - ) - mocked_director_service_api.post( - f"/services/{unrelated_service_key_1}/releases", - json=[release.model_dump() for release in unrelated_releases[:2]], - ) - mocked_director_service_api.post( - f"/services/{unrelated_service_key_2}/releases", - json=[release.model_dump() for release in unrelated_releases[2:]], - ) + await services_db_tables_injector(fake_releases + unrelated_releases) # Call the RPC function - release_history = await get_service_release_history( + page = await get_my_service_history( rpc_client, product_name=product_name, user_id=user_id, service_key=service_key, ) + release_history: list[ServiceRelease] = page.data # Validate the response assert isinstance(release_history, list) assert len(release_history) == 2 - assert release_history[0].key == service_key assert release_history[0].version == service_version_1 - assert release_history[1].key == service_key assert release_history[1].version == service_version_2 - - # Ensure unrelated services do not appear in the release history - unrelated_keys = {unrelated_service_key_1, unrelated_service_key_2} - for release in release_history: - assert release.key not in unrelated_keys From 339b3446c9e94d34a2832ca6ea45dcfb28b8be0a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 01:12:40 +0100 Subject: [PATCH 16/23] renaming --- .../servicelib/rabbitmq/rpc_interfaces/catalog/services.py | 6 ++++-- .../src/simcore_service_catalog/api/rpc/_services.py | 2 +- services/catalog/tests/unit/with_dbs/test_api_rpc.py | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py index 9f49c4aea87f..3a5392cfdd83 100644 --- a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py +++ b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py @@ -241,7 +241,7 @@ async def _call( return cast(list[MyServiceGet], result) -async def get_my_service_history( # pylint: disable=too-many-arguments +async def list_my_service_history_paginated( # pylint: disable=too-many-arguments rpc_client: RabbitMQRPCClient, *, product_name: ProductName, @@ -265,7 +265,9 @@ async def _call( ): return await rpc_client.request( CATALOG_RPC_NAMESPACE, - TypeAdapter(RPCMethodName).validate_python("get_my_service_history"), + TypeAdapter(RPCMethodName).validate_python( + "list_my_service_history_paginated" + ), product_name=product_name, user_id=user_id, service_key=service_key, diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py index f5b5b1c98677..874ac9bb45fa 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -226,7 +226,7 @@ async def batch_get_my_services( @router.expose(reraise_if_error_type=(ValidationError,)) @log_decorator(_logger, level=logging.DEBUG) @validate_call(config={"arbitrary_types_allowed": True}) -async def get_my_service_history( +async def list_my_service_history_paginated( app: FastAPI, *, product_name: ProductName, diff --git a/services/catalog/tests/unit/with_dbs/test_api_rpc.py b/services/catalog/tests/unit/with_dbs/test_api_rpc.py index 4d4af698aa2f..0db9c156504e 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -29,8 +29,8 @@ from servicelib.rabbitmq.rpc_interfaces.catalog.services import ( batch_get_my_services, check_for_service, - get_my_service_history, get_service, + list_my_service_history_paginated, list_services_paginated, update_service, ) @@ -537,7 +537,7 @@ async def test_rpc_get_my_service_history( await services_db_tables_injector(fake_releases + unrelated_releases) # Call the RPC function - page = await get_my_service_history( + page = await list_my_service_history_paginated( rpc_client, product_name=product_name, user_id=user_id, From 397aecda875fc14d661534b11178a724db1037a5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 01:15:46 +0100 Subject: [PATCH 17/23] cleanup --- .../pytest_simcore/helpers/catalog_rpc_server.py | 4 ++-- .../services_rpc/catalog.py | 2 +- .../api-server/tests/unit/test_services_catalog.py | 13 +++++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py index 6c13babadb04..917aa6e67a95 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py @@ -87,15 +87,15 @@ async def update_service( got.key = service_key return got.model_copy(update=update.model_dump(exclude_unset=True)) - async def get_my_service_history( + async def list_my_service_history_paginated( self, rpc_client: RabbitMQRPCClient, *, product_name: ProductName, user_id: UserID, service_key: ServiceKey, - limit: PageLimitInt, offset: PageOffsetInt, + limit: PageLimitInt, ) -> PageRpc[ServiceRelease]: assert rpc_client diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index 2882ae30ae3e..4b93ecb50af9 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -55,7 +55,7 @@ async def list_release_history( limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, ) -> tuple[list[ServiceRelease], PageMetaInfoLimitOffset]: - page = await catalog_rpc.get_my_service_history( + page = await catalog_rpc.list_my_service_history_paginated( self._client, product_name=product_name, user_id=user_id, diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index 5463ee8adc2d..99c5dc58b4cd 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -46,6 +46,12 @@ def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType] autospec=True, side_effect=side_effects.update_service, ), + "list_my_service_history_paginated": mocker.patch.object( + catalog_rpc, + "list_my_service_history_paginated", + autospec=True, + side_effect=side_effects.list_my_service_history_paginated, + ), } @@ -103,3 +109,10 @@ async def test_catalog_service_read_solvers( solver = to_solver(service) assert solver.id == selected_solver.id assert solver.version == oldest_release.version + + # checks calls to rpc + mocked_rpc_catalog_service_api["list_services_paginated"].assert_called_once() + mocked_rpc_catalog_service_api[ + "list_my_service_history_paginated" + ].assert_called_once() + mocked_rpc_catalog_service_api["get_service"].assert_called_once() From d91a670e4e1ffbad793d021ccd1da95bd8f8650f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 01:48:47 +0100 Subject: [PATCH 18/23] fixes tests --- .../src/pytest_simcore/helpers/catalog_rpc_server.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py index 917aa6e67a95..aa6db574410a 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py @@ -20,6 +20,7 @@ class CatalogRpcSideEffects: + # pylint: disable=no-self-use async def list_services_paginated( self, rpc_client: RabbitMQRPCClient, @@ -104,9 +105,7 @@ async def list_my_service_history_paginated( assert service_key items = TypeAdapter(list[ServiceRelease]).validate_python( - [ - ServiceRelease.model_json_schema()["example"], - ] + ServiceRelease.model_json_schema()["examples"], ) total_count = len(items) From 6e4df248b63fd07aa9d78c9198cfe0218f6d4892 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 01:57:38 +0100 Subject: [PATCH 19/23] updates OAS --- services/catalog/openapi.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/openapi.json b/services/catalog/openapi.json index 9fc1481d2b46..0173c51913d0 100644 --- a/services/catalog/openapi.json +++ b/services/catalog/openapi.json @@ -3,7 +3,7 @@ "info": { "title": "simcore-service-catalog", "description": "Manages and maintains a catalog of all published components (e.g. macro-algorithms, scripts, etc)", - "version": "0.8.0" + "version": "0.8.1" }, "paths": { "/": { From 47add772c617b5fec7ed6cde8c7cc4a4fd8935a8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 10:24:17 +0100 Subject: [PATCH 20/23] fixes mypy --- .../src/simcore_service_api_server/services_rpc/catalog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index 4b93ecb50af9..0a9fde924dee 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -1,5 +1,6 @@ from dataclasses import dataclass +from models_library.api_schemas_catalog.services import LatestServiceGet, ServiceGetV2 from models_library.products import ProductName from models_library.rest_pagination import ( DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, @@ -10,7 +11,6 @@ from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID -from pytest_simcore.helpers.catalog_rpc_server import LatestServiceGet, ServiceGetV2 from servicelib.fastapi.app_state import SingletonInAppStateMixin from servicelib.rabbitmq import RabbitMQRPCClient from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc From 3702e717f2bbd8d5f1ca7f58ede3d19ab5775f75 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 11:01:48 +0100 Subject: [PATCH 21/23] fixing queries --- .../db/repositories/services.py | 56 +++++++++++-------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index 9a2a3507676f..06a166659981 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -16,6 +16,7 @@ from models_library.users import UserID from psycopg2.errors import ForeignKeyViolation from pydantic import PositiveInt, TypeAdapter, ValidationError +from simcore_postgres_database.utils import as_postgres_sql_query_str from simcore_postgres_database.utils_repos import pass_or_acquire_connection from simcore_postgres_database.utils_services import create_select_latest_services_query from sqlalchemy import sql @@ -478,37 +479,23 @@ async def get_service_history_page( offset: int | None = None, ) -> tuple[PositiveInt, list[ReleaseDBGet]]: - base_query = ( + base_subquery = ( + # Search on service (key, *) for (product_name, user_id w/ access) sql.select( services_meta_data.c.key, services_meta_data.c.version, - services_meta_data.c.version_display, - services_meta_data.c.deprecated, - services_meta_data.c.created, - services_compatibility.c.custom_policy.label( - "compatibility_policy" - ), # CompatiblePolicyDict | None ) .select_from( - # joins because access-rights might change per version services_meta_data.join( services_access_rights, (services_meta_data.c.key == services_access_rights.c.key) & ( services_meta_data.c.version == services_access_rights.c.version ), - ) - .join( + ).join( user_to_groups, (user_to_groups.c.gid == services_access_rights.c.gid), ) - .outerjoin( - services_compatibility, - (services_meta_data.c.key == services_compatibility.c.key) - & ( - services_meta_data.c.version == services_compatibility.c.version - ), - ) ) .where( (services_meta_data.c.key == key) @@ -516,17 +503,42 @@ async def get_service_history_page( & (user_to_groups.c.uid == user_id) & AccessRightsClauses.can_read ) - .distinct() - ) + ).subquery() - subquery = base_query.subquery() - count_query = sql.select(sql.func.count()).select_from(subquery) + # Query to count the TOTAL number of rows + count_query = sql.select(sql.func.count()).select_from(base_subquery) + _logger.debug("count_query=\n%s", as_postgres_sql_query_str(count_query)) + # Query to retrieve page with additional columns, ordering, offset, and limit page_query = ( - base_query.order_by(sql.desc(by_version(base_query.c.version))) + sql.select( + services_meta_data.c.key, + services_meta_data.c.version, + services_meta_data.c.version_display, + services_meta_data.c.deprecated, + services_meta_data.c.created, + # CompatiblePolicyDict | None + services_compatibility.c.custom_policy.label("compatibility_policy"), + ) + .select_from( + # NOTE: these joins are avoided in count_query + base_subquery.join( + services_meta_data, + (base_subquery.c.key == services_meta_data.c.key) + & (base_subquery.c.version == services_meta_data.c.version), + ).outerjoin( + services_compatibility, + (services_meta_data.c.key == services_compatibility.c.key) + & ( + services_meta_data.c.version == services_compatibility.c.version + ), + ) + ) + .order_by(sql.desc(by_version(services_meta_data.c.version))) .offset(offset) .limit(limit) ) + _logger.debug("page_query=\n%s", as_postgres_sql_query_str(page_query)) async with pass_or_acquire_connection(self.db_engine) as conn: total_count: PositiveInt = await conn.scalar(count_query) or 0 From 1b60257803cb9e92d9bf10500628570625abeed6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 14:18:59 +0100 Subject: [PATCH 22/23] fixing tests --- .../tests/unit/with_dbs/test_api_rpc.py | 7 +++++-- .../unit/with_dbs/test_db_repositories.py | 21 ++++++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/services/catalog/tests/unit/with_dbs/test_api_rpc.py b/services/catalog/tests/unit/with_dbs/test_api_rpc.py index 0db9c156504e..b0faa1734d69 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -16,6 +16,7 @@ from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID +from packaging import version from pydantic import ValidationError from pytest_simcore.helpers.faker_factories import random_icon_url from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict @@ -499,6 +500,8 @@ async def test_rpc_get_my_service_history( service_version_1 = "1.0.0" service_version_2 = "1.1.0" + assert version.Version(service_version_1) < version.Version(service_version_2) + # Inject fake service releases for the target service fake_releases = [ create_fake_service_data( @@ -548,5 +551,5 @@ async def test_rpc_get_my_service_history( # Validate the response assert isinstance(release_history, list) assert len(release_history) == 2 - assert release_history[0].version == service_version_1 - assert release_history[1].version == service_version_2 + assert release_history[0].version == service_version_2, "expected newest first" + assert release_history[1].version == service_version_1 diff --git a/services/catalog/tests/unit/with_dbs/test_db_repositories.py b/services/catalog/tests/unit/with_dbs/test_db_repositories.py index fdcfb9f182e9..8618a67c25e4 100644 --- a/services/catalog/tests/unit/with_dbs/test_db_repositories.py +++ b/services/catalog/tests/unit/with_dbs/test_db_repositories.py @@ -3,6 +3,7 @@ # pylint: disable=unused-variable # pylint: disable=too-many-arguments +import random from collections import Counter from collections.abc import Callable from dataclasses import dataclass, field @@ -488,18 +489,26 @@ async def test_get_service_history_page( # inject services with multiple versions service_key = "simcore/services/dynamic/test-service" num_versions = 10 + + release_versions = [ + f"{random.randint(0, 2)}.{random.randint(0, 9)}.{random.randint(0, 9)}" # noqa: S311 + for _ in range(num_versions) + ] await services_db_tables_injector( [ create_fake_service_data( service_key, - f"{v}.0.0", + service_version, team_access=None, everyone_access=None, product=target_product, ) - for v in range(num_versions) + for service_version in release_versions ] ) + # sorted AFTER injecting + release_versions = sorted(release_versions, key=version.Version, reverse=True) + assert version.Version(release_versions[0]) > version.Version(release_versions[-1]) # fetch full history using get_service_history_page total_count, history = await services_repo.get_service_history_page( @@ -509,9 +518,7 @@ async def test_get_service_history_page( ) assert total_count == num_versions assert len(history) == num_versions - assert [release.version for release in history] == [ - f"{v}.0.0" for v in reversed(range(num_versions)) - ] + assert [release.version for release in history] == release_versions # fetch full history using deprecated get_service_history deprecated_history = await services_repo.get_service_history( @@ -536,8 +543,8 @@ async def test_get_service_history_page( ) assert total_count == num_versions assert len(paginated_history) == limit - assert [release.version for release in paginated_history] == [ - f"{v}.0.0" for v in reversed(range(offset, offset + limit)) + assert [release.version for release in paginated_history] == release_versions[ + offset : offset + limit ] # compare paginated results with the corresponding slice of the full history From 28039ae3214adb51d0ae19d307a819968bbbd29c Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Mar 2025 14:36:03 +0100 Subject: [PATCH 23/23] @GitHK review:doc --- .../src/models_library/api_schemas_catalog/services.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/models-library/src/models_library/api_schemas_catalog/services.py b/packages/models-library/src/models_library/api_schemas_catalog/services.py index 5e5e6de88143..363b74c84768 100644 --- a/packages/models-library/src/models_library/api_schemas_catalog/services.py +++ b/packages/models-library/src/models_library/api_schemas_catalog/services.py @@ -325,11 +325,15 @@ def _update_json_schema_extra(schema: JsonDict) -> None: PageRpcLatestServiceGet: TypeAlias = PageRpc[ # WARNING: keep this definition in models_library and not in the RPC interface + # otherwise the metaclass PageRpc[*] will create *different* classes in server/client side + # and will fail to serialize/deserialize these parameters when transmitted/received LatestServiceGet ] PageRpcServiceRelease: TypeAlias = PageRpc[ # WARNING: keep this definition in models_library and not in the RPC interface + # otherwise the metaclass PageRpc[*] will create *different* classes in server/client side + # and will fail to serialize/deserialize these parameters when transmitted/received ServiceRelease ]