From 155bb918d86f83f4e3ea96dafa5a4db195482479 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Feb 2025 15:33:33 +0100 Subject: [PATCH 01/39] draft project/services api --- api/specs/web-server/_projects_nodes.py | 26 +++- .../api_schemas_webserver/projects_nodes.py | 18 ++- .../api/v0/openapi.yaml | 121 ++++++++++++++++++ 3 files changed, 162 insertions(+), 3 deletions(-) diff --git a/api/specs/web-server/_projects_nodes.py b/api/specs/web-server/_projects_nodes.py index 06189fa7cc3..9cd20c2af64 100644 --- a/api/specs/web-server/_projects_nodes.py +++ b/api/specs/web-server/_projects_nodes.py @@ -4,8 +4,10 @@ # pylint: disable=too-many-arguments -from _common import assert_handler_signature_against_model -from fastapi import APIRouter, status +from typing import Annotated + +from _common import as_query, assert_handler_signature_against_model +from fastapi import APIRouter, Depends, status from models_library.api_schemas_directorv2.dynamic_services import DynamicServiceGet from models_library.api_schemas_long_running_tasks.tasks import TaskGet from models_library.api_schemas_webserver.projects_nodes import ( @@ -18,12 +20,14 @@ NodePatch, NodeRetrieve, NodeRetrieved, + ProjectNodeServicesGet, ServiceResourcesDict, ) from models_library.generics import Envelope from models_library.groups import GroupID from models_library.projects import ProjectID from models_library.projects_nodes_io import NodeID +from models_library.rest_pagination import Page, PageQueryParameters from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.projects._crud_handlers import ProjectPathParams from simcore_service_webserver.projects._nodes_handlers import ( @@ -156,6 +160,24 @@ def replace_node_resources( # +@router.get( + "/projects/-/nodes/-/services", + response_model=Page[ProjectNodeServicesGet], +) +async def list_projects_services( + _query: Annotated[as_query(PageQueryParameters), Depends()] +): + """Lists all services used in the user's projects **grouped by project**""" + + +@router.get( + "/projects/{project_id}/nodes/-/services", + response_model=Envelope[ProjectNodeServicesGet], +) +async def get_project_services(project_id: ProjectID): + ... + + @router.get( "/projects/{project_id}/nodes/-/services:access", response_model=Envelope[_ProjectGroupAccess], diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py b/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py index 69aafba0962..f21c7688936 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py @@ -1,8 +1,12 @@ # mypy: disable-error-code=truthy-function from typing import Annotated, Any, Literal, TypeAlias +from models_library.groups import GroupID +from models_library.projects import ProjectID +from models_library.services_history import ServiceRelease from pydantic import ConfigDict, Field +from ..access_rights import AccessRights from ..api_schemas_directorv2.dynamic_services import RetrieveDataOut from ..basic_types import PortInt from ..projects_nodes import InputID, InputsDict, PartialNode @@ -40,7 +44,7 @@ class NodePatch(InputSchemaWithoutCamelCase): ] inputs_required: Annotated[ list[InputID] | None, - Field(alias="inputsRequired"), + Field(alias="inputsRequired"), ] = None input_nodes: Annotated[ list[NodeID] | None, @@ -197,3 +201,15 @@ class NodeRetrieve(InputSchemaWithoutCamelCase): class NodeRetrieved(RetrieveDataOut): model_config = OutputSchema.model_config + + +class NodeServiceGet(OutputSchema): + key: ServiceKey + release: ServiceRelease + owner: GroupID + my_access_rights: AccessRights + + +class ProjectNodeServicesGet(OutputSchema): + project_uuid: ProjectID + services: list[NodeServiceGet] diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 42573752b6f..418ec16f95c 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -4927,6 +4927,58 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_dict_Annotated_str__StringConstraints___ImageResources__' + /v0/projects/-/nodes/-/services: + get: + tags: + - projects + - nodes + summary: List Projects Services + description: Lists all services used in the user's projects **grouped by project** + operationId: list_projects_services + parameters: + - name: limit + in: query + required: false + schema: + type: integer + default: 20 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/Page_ProjectNodeServicesGet_' + /v0/projects/{project_id}/nodes/-/services: + get: + tags: + - projects + - nodes + summary: Get Project Services + operationId: get_project_services + parameters: + - name: project_id + in: path + required: true + schema: + type: string + format: uuid + title: Project Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/Envelope_ProjectNodeServicesGet_' /v0/projects/{project_id}/nodes/-/services:access: get: tags: @@ -9107,6 +9159,19 @@ components: title: Error type: object title: Envelope[ProjectMetadataGet] + Envelope_ProjectNodeServicesGet_: + properties: + data: + anyOf: + - $ref: '#/components/schemas/ProjectNodeServicesGet' + - type: 'null' + error: + anyOf: + - {} + - type: 'null' + title: Error + type: object + title: Envelope[ProjectNodeServicesGet] Envelope_ProjectState_: properties: data: @@ -12062,6 +12127,28 @@ components: - thumbnail_url - file_url title: NodeScreenshot + NodeServiceGet: + properties: + key: + type: string + pattern: ^simcore/services/((comp|dynamic|frontend))/([a-z0-9][a-z0-9_.-]*/)*([a-z0-9-_]+[a-z0-9])$ + title: Key + release: + $ref: '#/components/schemas/ServiceRelease' + owner: + type: integer + exclusiveMinimum: true + title: Owner + minimum: 0 + myAccessRights: + $ref: '#/components/schemas/AccessRights' + type: object + required: + - key + - release + - owner + - myAccessRights + title: NodeServiceGet NodeState: properties: modified: @@ -12352,6 +12439,24 @@ components: - _links - data title: Page[ProjectListItem] + Page_ProjectNodeServicesGet_: + properties: + _meta: + $ref: '#/components/schemas/PageMetaInfoLimitOffset' + _links: + $ref: '#/components/schemas/PageLinks' + data: + items: + $ref: '#/components/schemas/ProjectNodeServicesGet' + type: array + title: Data + additionalProperties: false + type: object + required: + - _meta + - _links + - data + title: Page[ProjectNodeServicesGet] Page_ServiceRunGet_: properties: _meta: @@ -13549,6 +13654,22 @@ components: required: - custom title: ProjectMetadataUpdate + ProjectNodeServicesGet: + properties: + projectUuid: + type: string + format: uuid + title: Projectuuid + services: + items: + $ref: '#/components/schemas/NodeServiceGet' + type: array + title: Services + type: object + required: + - projectUuid + - services + title: ProjectNodeServicesGet ProjectOutputGet: properties: key: From bcf3fa4fc2629be845702eb5b009f1dba6f063cf Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Feb 2025 17:41:46 +0100 Subject: [PATCH 02/39] draft test and api in catalog --- .../api_schemas_catalog/services.py | 8 +++ .../api/rest/_services.py | 2 +- .../api/rpc/_services.py | 19 ++++++ .../db/repositories/services.py | 2 +- .../services/services_api.py | 38 ++++++++++- .../with_dbs/test_services_services_api.py | 66 +++++++++++++++++++ 6 files changed, 131 insertions(+), 4 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 e4c39c4c158..c73f4e211a2 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 @@ -310,3 +310,11 @@ class ServiceUpdateV2(CatalogInputSchema): assert set(ServiceUpdateV2.model_fields.keys()) - set( # nosec ServiceGetV2.model_fields.keys() ) == {"deprecated"} + + +class MyServiceGet(CatalogOutputSchema): + key: ServiceKey + release: ServiceRelease + + owner: GroupID + my_access_rights: ServiceGroupAccessRightsV2 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 ec1b5dca6b1..4f77c0f8a49 100644 --- a/services/catalog/src/simcore_service_catalog/api/rest/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rest/_services.py @@ -152,7 +152,7 @@ async def cached_registry_services() -> dict[str, Any]: services_owner_emails, ) = await asyncio.gather( cached_registry_services(), - services_repo.list_services_access_rights( + services_repo.batch_get_services_access_rights( key_versions=services_in_db, product_name=x_simcore_products_name, ), 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 0cd2870fb0f..e06b00f92ee 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -4,6 +4,7 @@ from fastapi import FastAPI from models_library.api_schemas_catalog.services import ( + MyServiceGet, PageRpcServicesGetV2, ServiceGetV2, ServiceUpdateV2, @@ -165,3 +166,21 @@ async def check_for_service( service_key=service_key, service_version=service_version, ) + + +@router.expose(reraise_if_error_type=(CatalogForbiddenError,)) +@log_decorator(_logger, level=logging.DEBUG) +async def batch_get_my_services( + app: FastAPI, + *, + product_name: ProductName, + user_id: UserID, + ids: list[ + tuple[ + ServiceKey, + ServiceVersion, + ] + ], +) -> list[MyServiceGet]: + + raise NotImplementedError 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 1cae7e1c43b..7592ad28aea 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -481,7 +481,7 @@ async def get_service_access_rights( async for row in await conn.stream(query) ] - async def list_services_access_rights( + async def batch_get_services_access_rights( self, key_versions: Iterable[tuple[str, str]], product_name: str | None = None, 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 27368a7565f..c16c0006039 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -1,6 +1,10 @@ import logging -from models_library.api_schemas_catalog.services import ServiceGetV2, ServiceUpdateV2 +from models_library.api_schemas_catalog.services import ( + MyServiceGet, + ServiceGetV2, + ServiceUpdateV2, +) from models_library.products import ProductName from models_library.rest_pagination import PageLimitInt from models_library.services_access import ServiceGroupAccessRightsV2 @@ -93,7 +97,7 @@ async def list_services_paginated( # injects access-rights access_rights: dict[ tuple[str, str], list[ServiceAccessRightsAtDB] - ] = await repo.list_services_access_rights( + ] = await repo.batch_get_services_access_rights( ((s.key, s.version) for s in services), product_name=product_name ) if not access_rights: @@ -333,3 +337,33 @@ async def check_for_service( user_id=user_id, product_name=product_name, ) + + +async def batch_get_my_services( + *, + repo: ServicesRepository, + product_name: ProductName, + user_id: UserID, + ids: list[ + tuple[ + ServiceKey, + ServiceVersion, + ] + ], +) -> list[MyServiceGet]: + + raise NotImplementedError + + # user_groups = [] + + # result = [] + + # services_access_rights = await repo.batch_get_services_access_rights(key_versions=ids, product_name=product_name) + + # for service_key, service_version in ids: + # my_access_rights = {"read": False, "execute": False} + + # if (service_access_rights_db := services_access_rights.get((service_key,service_version))) + + # if service_access_rights_db.gid in user_groups: + # my_access_rights.append() diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index bcfae48d319..94126e4a86c 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -143,3 +143,69 @@ async def test_list_services_paginated( # since it is cached, it should only call it `limit` times assert mocked_director_service_api["get_service"].call_count == limit + + +async def test_batch_get_my_services( + background_sync_task_mocked: None, + rabbitmq_and_rpc_setup_disabled: None, + mocked_director_service_api: MockRouter, + target_product: ProductName, + services_repo: ServicesRepository, + user_id: UserID, + director_client: DirectorApi, + create_fake_service_data: Callable, + services_db_tables_injector: Callable, +): + # Create fake services data + service_key = "simcore/services/comp/some-service" + service_version_1 = "1.0.0" + service_version_2 = "2.0.0" + other_service_key = "simcore/services/comp/other-service" + other_service_version = "1.0.0" + + fake_service_1 = create_fake_service_data( + service_key, + service_version_1, + team_access=None, + everyone_access=None, + product=target_product, + ) + fake_service_2 = create_fake_service_data( + service_key, + service_version_2, + team_access="x", + everyone_access=None, + product=target_product, + ) + fake_service_3 = create_fake_service_data( + other_service_key, + other_service_version, + team_access=None, + everyone_access=None, + product=target_product, + ) + + # Inject fake services into the database + await services_db_tables_injector([fake_service_1, fake_service_2, fake_service_3]) + + # Batch get my services + ids = [ + (service_key, service_version_1), + (service_key, service_version_2), + (other_service_key, other_service_version), + ] + + my_services = await services_api.batch_get_my_services( + repo=services_repo, + product_name=target_product, + user_id=user_id, + ids=ids, + ) + + assert len(my_services) == 3 + + # Check access rights + assert my_services[0].my_access_rights == {} + assert my_services[1].my_access_rights is not None + assert my_services[2].my_access_rights is not None + assert my_services[2].owner is not None From 1ff3970b21b38836d8f0aa620c16c7ba43a65cb9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Feb 2025 18:50:16 +0100 Subject: [PATCH 03/39] updates OAS --- api/specs/web-server/_projects_nodes.py | 17 +----- .../api_schemas_catalog/services.py | 56 +++++++++++++------ .../api/v0/openapi.yaml | 48 ---------------- 3 files changed, 41 insertions(+), 80 deletions(-) diff --git a/api/specs/web-server/_projects_nodes.py b/api/specs/web-server/_projects_nodes.py index 9cd20c2af64..3d3d67f6056 100644 --- a/api/specs/web-server/_projects_nodes.py +++ b/api/specs/web-server/_projects_nodes.py @@ -4,10 +4,8 @@ # pylint: disable=too-many-arguments -from typing import Annotated - -from _common import as_query, assert_handler_signature_against_model -from fastapi import APIRouter, Depends, status +from _common import assert_handler_signature_against_model +from fastapi import APIRouter, status from models_library.api_schemas_directorv2.dynamic_services import DynamicServiceGet from models_library.api_schemas_long_running_tasks.tasks import TaskGet from models_library.api_schemas_webserver.projects_nodes import ( @@ -27,7 +25,6 @@ from models_library.groups import GroupID from models_library.projects import ProjectID from models_library.projects_nodes_io import NodeID -from models_library.rest_pagination import Page, PageQueryParameters from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.projects._crud_handlers import ProjectPathParams from simcore_service_webserver.projects._nodes_handlers import ( @@ -160,16 +157,6 @@ def replace_node_resources( # -@router.get( - "/projects/-/nodes/-/services", - response_model=Page[ProjectNodeServicesGet], -) -async def list_projects_services( - _query: Annotated[as_query(PageQueryParameters), Depends()] -): - """Lists all services used in the user's projects **grouped by project**""" - - @router.get( "/projects/{project_id}/nodes/-/services", response_model=Envelope[ProjectNodeServicesGet], 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 c73f4e211a2..a049bcbec55 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 @@ -1,6 +1,7 @@ from datetime import datetime -from typing import Any, TypeAlias +from typing import Annotated, Any, TypeAlias +from common_library.basic_types import DEFAULT_FACTORY from models_library.rpc_pagination import PageRpc from pydantic import ConfigDict, Field, HttpUrl, NonNegativeInt from pydantic.config import JsonDict @@ -154,9 +155,10 @@ class ServiceGet( ServiceMetaDataPublished, ServiceAccessRights, ServiceMetaDataEditable ): # pylint: disable=too-many-ancestors - owner: LowerCaseEmailStr | None = Field( - description="None when the owner email cannot be found in the database" - ) + owner: Annotated[ + LowerCaseEmailStr | None, + Field(description="None when the owner email cannot be found in the database"), + ] @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: @@ -169,7 +171,7 @@ def _update_json_schema_extra(schema: JsonDict) -> None: ) -class ServiceGetV2(CatalogOutputSchema): +class ServiceGetV2Base(CatalogOutputSchema): # Model used in catalog's rpc and rest interfaces key: ServiceKey version: ServiceVersion @@ -183,13 +185,14 @@ class ServiceGetV2(CatalogOutputSchema): version_display: str | None = None - service_type: ServiceType = Field(default=..., alias="type") + service_type: Annotated[ServiceType, Field(alias="type")] contact: LowerCaseEmailStr | None - authors: list[Author] = Field(..., min_length=1) - owner: LowerCaseEmailStr | None = Field( - description="None when the owner email cannot be found in the database" - ) + authors: Annotated[list[Author], Field(min_length=1)] + owner: Annotated[ + LowerCaseEmailStr | None, + Field(description="None when the owner email cannot be found in the database"), + ] inputs: ServiceInputsDict outputs: ServiceOutputsDict @@ -202,12 +205,18 @@ class ServiceGetV2(CatalogOutputSchema): classifiers: list[str] | None = [] quality: dict[str, Any] = {} - history: list[ServiceRelease] = Field( - default_factory=list, - description="history of releases for this service at this point in time, starting from the newest to the oldest." - " It includes current release.", - json_schema_extra={"default": []}, - ) + +class ServiceGetV2(ServiceGetV2Base): + # Model used in catalog's rpc and rest interfaces + history: Annotated[ + list[ServiceRelease], + Field( + default_factory=list, + description="history of releases for this service at this point in time, starting from the newest to the oldest." + " It includes current release.", + json_schema_extra={"default": []}, + ), + ] = DEFAULT_FACTORY @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: @@ -276,9 +285,22 @@ def _update_json_schema_extra(schema: JsonDict) -> None: ) +class ServiceItemList(ServiceGetV2Base): + release: ServiceRelease + history: Annotated[ + list[ServiceRelease], + Field( + default_factory=list, + deprecated=True, + description="History will be replaced by current 'release' instead", + json_schema_extra={"default": []}, + ), + ] = DEFAULT_FACTORY + + PageRpcServicesGetV2: TypeAlias = PageRpc[ # WARNING: keep this definition in models_library and not in the RPC interface - ServiceGetV2 + ServiceItemList ] ServiceResourcesGet: TypeAlias = ServiceResourcesDict diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 418ec16f95c..ef8c915fac9 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -4927,36 +4927,6 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_dict_Annotated_str__StringConstraints___ImageResources__' - /v0/projects/-/nodes/-/services: - get: - tags: - - projects - - nodes - summary: List Projects Services - description: Lists all services used in the user's projects **grouped by project** - operationId: list_projects_services - parameters: - - name: limit - in: query - required: false - schema: - type: integer - default: 20 - title: Limit - - name: offset - in: query - required: false - schema: - type: integer - default: 0 - title: Offset - responses: - '200': - description: Successful Response - content: - application/json: - schema: - $ref: '#/components/schemas/Page_ProjectNodeServicesGet_' /v0/projects/{project_id}/nodes/-/services: get: tags: @@ -12439,24 +12409,6 @@ components: - _links - data title: Page[ProjectListItem] - Page_ProjectNodeServicesGet_: - properties: - _meta: - $ref: '#/components/schemas/PageMetaInfoLimitOffset' - _links: - $ref: '#/components/schemas/PageLinks' - data: - items: - $ref: '#/components/schemas/ProjectNodeServicesGet' - type: array - title: Data - additionalProperties: false - type: object - required: - - _meta - - _links - - data - title: Page[ProjectNodeServicesGet] Page_ServiceRunGet_: properties: _meta: From 36d015070a34e814ae317b745078ea386b1f2e51 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Feb 2025 18:57:25 +0100 Subject: [PATCH 04/39] draft --- .../services/services_api.py | 54 +++++++++++++++---- 1 file changed, 43 insertions(+), 11 deletions(-) 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 c16c0006039..e75de9ba055 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -49,7 +49,7 @@ def _db_to_api_model( description=service_db.description, description_ui=service_db.description_ui, version_display=service_db.version_display, - type=service_manifest.service_type, + service_type=service_manifest.service_type, contact=service_manifest.contact, authors=service_manifest.authors, owner=(service_db.owner_email if service_db.owner_email else None), @@ -340,8 +340,9 @@ async def check_for_service( async def batch_get_my_services( - *, repo: ServicesRepository, + director_api: DirectorApi, + *, product_name: ProductName, user_id: UserID, ids: list[ @@ -352,18 +353,49 @@ async def batch_get_my_services( ], ) -> list[MyServiceGet]: - raise NotImplementedError + services_access_rights = await repo.batch_get_services_access_rights( + key_versions=ids, product_name=product_name + ) - # user_groups = [] + my_services = [] + for service_key, service_version in ids: + access_rights = services_access_rights.get((service_key, service_version), []) + my_access_rights = { + "read": any(ar.execute_access for ar in access_rights), + "write": any(ar.write_access for ar in access_rights), + } - # result = [] + service = await repo.get_service_with_history( + product_name=product_name, + user_id=user_id, + key=service_key, + version=service_version, + ) - # services_access_rights = await repo.batch_get_services_access_rights(key_versions=ids, product_name=product_name) + if service: + service_manifest = await manifest.get_service( + key=service_key, + version=service_version, + director_client=director_api, + ) - # for service_key, service_version in ids: - # my_access_rights = {"read": False, "execute": False} + compatibility_map = await evaluate_service_compatibility_map( + repo, + product_name=product_name, + user_id=user_id, + service_release_history=service.history, + ) - # if (service_access_rights_db := services_access_rights.get((service_key,service_version))) + my_services.append( + MyServiceGet( + **_db_to_api_model( + service_db=service, + access_rights_db=access_rights, + service_manifest=service_manifest, + compatibility_map=compatibility_map, + ).model_dump(), + my_access_rights=my_access_rights, + ) + ) - # if service_access_rights_db.gid in user_groups: - # my_access_rights.append() + return my_services From c1102496bd8ab6d8a1eb44b962ed29a3ff5f5845 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 26 Feb 2025 19:37:36 +0100 Subject: [PATCH 05/39] draft 2 --- .../models/services_db.py | 6 +- .../services/services_api.py | 73 ++++++++++++------- .../with_dbs/test_services_services_api.py | 3 +- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/models/services_db.py b/services/catalog/src/simcore_service_catalog/models/services_db.py index 902aae32993..5ddcbe239ef 100644 --- a/services/catalog/src/simcore_service_catalog/models/services_db.py +++ b/services/catalog/src/simcore_service_catalog/models/services_db.py @@ -3,6 +3,7 @@ from common_library.basic_types import DEFAULT_FACTORY from models_library.basic_types import IdInt +from models_library.groups import GroupID from models_library.products import ProductName from models_library.services_access import ServiceGroupAccessRights from models_library.services_base import ServiceKeyVersion @@ -10,7 +11,6 @@ from models_library.utils.common_validators import empty_str_to_none_pre_validator from pydantic import BaseModel, ConfigDict, Field, field_validator from pydantic.config import JsonDict -from pydantic.types import PositiveInt from simcore_postgres_database.models.services_compatibility import CompatiblePolicyDict @@ -20,7 +20,7 @@ class ServiceMetaDataDBGet(BaseModel): version: ServiceVersion # ownership - owner: IdInt | None + owner: GroupID | None # display name: str @@ -208,7 +208,7 @@ class ServiceWithHistoryDBGet(BaseModel): class ServiceAccessRightsAtDB(ServiceKeyVersion, ServiceGroupAccessRights): - gid: PositiveInt + gid: GroupID product_name: ProductName @staticmethod 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 e75de9ba055..beaaf2ddbaf 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -17,6 +17,7 @@ CatalogForbiddenError, CatalogItemNotFoundError, ) +from simcore_service_catalog.db.repositories.groups import GroupsRepository from ..db.repositories.services import ServicesRepository from ..models.services_db import ( @@ -341,7 +342,7 @@ async def check_for_service( async def batch_get_my_services( repo: ServicesRepository, - director_api: DirectorApi, + groups_repo: GroupsRepository, *, product_name: ProductName, user_id: UserID, @@ -357,45 +358,67 @@ async def batch_get_my_services( key_versions=ids, product_name=product_name ) + user_groups = await groups_repo.list_user_groups(user_id=user_id) + my_group_ids = {g.gid for g in user_groups} + my_services = [] for service_key, service_version in ids: access_rights = services_access_rights.get((service_key, service_version), []) + my_access_rights = { - "read": any(ar.execute_access for ar in access_rights), - "write": any(ar.write_access for ar in access_rights), + "execute": False, + "write": False, } - service = await repo.get_service_with_history( + for ar in access_rights: + if ar.gid in my_group_ids: + my_access_rights["execute"] |= ar.execute_access + my_access_rights["write"] |= ar.write_access + + service_db = await repo.get_service( product_name=product_name, - user_id=user_id, key=service_key, version=service_version, ) + assert service_db # nosec - if service: - service_manifest = await manifest.get_service( - key=service_key, - version=service_version, - director_client=director_api, + owner = service_db.owner + if not owner: + # TODO: raise error to indicate that no owner is registered for a given service + owner = next( + ar.gid for ar in access_rights if ar.write_access and ar.execute_access ) - compatibility_map = await evaluate_service_compatibility_map( - repo, - product_name=product_name, - user_id=user_id, - service_release_history=service.history, - ) + assert owner is not None # nosec - my_services.append( - MyServiceGet( - **_db_to_api_model( - service_db=service, - access_rights_db=access_rights, - service_manifest=service_manifest, - compatibility_map=compatibility_map, - ).model_dump(), - my_access_rights=my_access_rights, + compatibility_map = {} + if my_access_rights != {"execute": False, "write": False}: + history = await repo.get_service_history( + product_name=product_name, user_id=user_id, key=service_key + ) + if history: + compatibility_map = await evaluate_service_compatibility_map( + repo, + product_name=product_name, + user_id=user_id, + service_release_history=history, ) + + my_services.append( + MyServiceGet( + key=service_db.key, + release=ServiceRelease.model_construct( + version=service_db.version, + version_display=service_db.version_display, + released=service_db.created, + retired=service_db.deprecated, + compatibility=compatibility_map.get(service_db.version), + ), + owner=owner, + my_access_rights=my_access_rights, ) + ) + + # TODO: else error return my_services diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 94126e4a86c..1658658e60a 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -196,7 +196,8 @@ async def test_batch_get_my_services( ] my_services = await services_api.batch_get_my_services( - repo=services_repo, + services_repo, + director_client, product_name=target_product, user_id=user_id, ids=ids, From 1f9da9e554ae335fbd6875d063b4c48895255710 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 11:47:30 +0100 Subject: [PATCH 06/39] draft 3 --- .../services/compatibility.py | 14 +++-- .../services/services_api.py | 52 +++++++++++-------- .../with_dbs/test_services_services_api.py | 10 +++- 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/services/compatibility.py b/services/catalog/src/simcore_service_catalog/services/compatibility.py index f3d9b6c680a..db8483e11c9 100644 --- a/services/catalog/src/simcore_service_catalog/services/compatibility.py +++ b/services/catalog/src/simcore_service_catalog/services/compatibility.py @@ -96,9 +96,13 @@ async def evaluate_service_compatibility_map( user_id: UserID, service_release_history: list[ReleaseDBGet], ) -> dict[ServiceVersion, Compatibility | None]: - released_versions = _convert_to_versions(service_release_history) - result: dict[ServiceVersion, Compatibility | None] = {} + """ + Evaluates the compatibility among a list of service releases for a given product and user. + """ + compatibility_map: dict[ServiceVersion, Compatibility | None] = {} + + released_versions = _convert_to_versions(service_release_history) for release in service_release_history: compatibility = None if release.compatibility_policy: @@ -108,7 +112,7 @@ async def evaluate_service_compatibility_map( repo=repo, target_version=release.version, released_versions=released_versions, - compatibility_policy={**release.compatibility_policy}, + compatibility_policy=dict(release.compatibility_policy), ) elif latest_version := _get_latest_compatible_version( release.version, @@ -117,6 +121,6 @@ async def evaluate_service_compatibility_map( compatibility = Compatibility( can_update_to=CompatibleService(version=f"{latest_version}") ) - result[release.version] = compatibility + compatibility_map[release.version] = compatibility - return result + return compatibility_map 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 beaaf2ddbaf..424eb63493b 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -5,6 +5,7 @@ ServiceGetV2, ServiceUpdateV2, ) +from models_library.groups import GroupID from models_library.products import ProductName from models_library.rest_pagination import PageLimitInt from models_library.services_access import ServiceGroupAccessRightsV2 @@ -363,18 +364,16 @@ async def batch_get_my_services( my_services = [] for service_key, service_version in ids: - access_rights = services_access_rights.get((service_key, service_version), []) - - my_access_rights = { - "execute": False, - "write": False, - } + # Evaluate user's access-rights to this service key:version + access_rights = services_access_rights.get((service_key, service_version), []) + my_access_rights = ServiceGroupAccessRightsV2(execute=False, write=False) for ar in access_rights: if ar.gid in my_group_ids: - my_access_rights["execute"] |= ar.execute_access - my_access_rights["write"] |= ar.write_access + my_access_rights.execute |= ar.execute_access + my_access_rights.write |= ar.write_access + # Get service metadata service_db = await repo.get_service( product_name=product_name, key=service_key, @@ -382,43 +381,50 @@ async def batch_get_my_services( ) assert service_db # nosec - owner = service_db.owner + # Find service owner + owner: GroupID | None = service_db.owner if not owner: - # TODO: raise error to indicate that no owner is registered for a given service + # NOTE can be more than one. Just get first. owner = next( ar.gid for ar in access_rights if ar.write_access and ar.execute_access ) + # TODO: raise error to indicate that no owner is registered for a given service assert owner is not None # nosec - compatibility_map = {} - if my_access_rights != {"execute": False, "write": False}: + # Evaluate `compatibility` + compatibility: Compatibility | None = None + if my_access_rights.execute or my_access_rights.write: + # TODO: add cache to this section that evals compatibility_map based on service_key + + # NOTE: that the service history might be different for each user + # since access rights are defined on a k:v basis history = await repo.get_service_history( product_name=product_name, user_id=user_id, key=service_key ) - if history: - compatibility_map = await evaluate_service_compatibility_map( - repo, - product_name=product_name, - user_id=user_id, - service_release_history=history, - ) + assert history # nosec + + compatibility_map = await evaluate_service_compatibility_map( + repo, + product_name=product_name, + user_id=user_id, + service_release_history=history, + ) + compatibility = compatibility_map.get(service_db.version) my_services.append( MyServiceGet( key=service_db.key, - release=ServiceRelease.model_construct( + release=ServiceRelease( version=service_db.version, version_display=service_db.version_display, released=service_db.created, retired=service_db.deprecated, - compatibility=compatibility_map.get(service_db.version), + compatibility=compatibility, ), owner=owner, my_access_rights=my_access_rights, ) ) - # TODO: else error - return my_services diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 1658658e60a..1cafdc4ea60 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -11,6 +11,7 @@ from models_library.users import UserID from respx.router import MockRouter from simcore_service_catalog.api.dependencies.director import get_director_api +from simcore_service_catalog.db.repositories.groups import GroupsRepository from simcore_service_catalog.db.repositories.services import ServicesRepository from simcore_service_catalog.services import manifest, services_api from simcore_service_catalog.services.director import DirectorApi @@ -29,6 +30,11 @@ def services_repo(sqlalchemy_async_engine: AsyncEngine): return ServicesRepository(sqlalchemy_async_engine) +@pytest.fixture +def groups_repo(sqlalchemy_async_engine: AsyncEngine): + return GroupsRepository(sqlalchemy_async_engine) + + @pytest.fixture def num_services() -> int: return 5 @@ -151,8 +157,8 @@ async def test_batch_get_my_services( mocked_director_service_api: MockRouter, target_product: ProductName, services_repo: ServicesRepository, + groups_repo: GroupsRepository, user_id: UserID, - director_client: DirectorApi, create_fake_service_data: Callable, services_db_tables_injector: Callable, ): @@ -197,7 +203,7 @@ async def test_batch_get_my_services( my_services = await services_api.batch_get_my_services( services_repo, - director_client, + groups_repo, product_name=target_product, user_id=user_id, ids=ids, From 504a11d9ce2681865831cde333121dac114873c6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 14:22:11 +0100 Subject: [PATCH 07/39] fix tests --- .../catalog/tests/unit/with_dbs/conftest.py | 27 +++++++--- .../with_dbs/test_services_services_api.py | 49 +++++++++++++------ 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/services/catalog/tests/unit/with_dbs/conftest.py b/services/catalog/tests/unit/with_dbs/conftest.py index 1bd0bb27e50..467083a1b6f 100644 --- a/services/catalog/tests/unit/with_dbs/conftest.py +++ b/services/catalog/tests/unit/with_dbs/conftest.py @@ -8,7 +8,7 @@ from collections.abc import AsyncIterator, Awaitable, Callable from copy import deepcopy from datetime import datetime -from typing import Any +from typing import Any, Protocol import pytest import sqlalchemy as sa @@ -354,6 +354,19 @@ def _fake_factory(**overrides): return _fake_factory +class CreateFakeServiceData(Protocol): + def __call__( + self, + key, + version, + team_access: str | None = None, + everyone_access: str | None = None, + product: ProductName = "osparc", + deprecated: datetime | None = None, + ): + ... + + @pytest.fixture() async def create_fake_service_data( user_groups_ids: list[int], @@ -376,11 +389,11 @@ async def create_fake_service_data( owner_access, team_access, everyone_access = fake_access_rights """ - everyone_gid, user_gid, team_gid = user_groups_ids + everyone_gid, user_primary_gid, team_standard_gid = user_groups_ids def _random_service(**overrides) -> dict[str, Any]: return random_service_meta_data( - owner_primary_gid=user_gid, + owner_primary_gid=user_primary_gid, fake=faker, **overrides, ) @@ -396,9 +409,9 @@ def _random_access(service, **overrides) -> dict[str, Any]: def _fake_factory( key, version, - team_access=None, - everyone_access=None, - product=products_names[0], + team_access: str | None = None, + everyone_access: str | None = None, + product: ProductName = products_names[0], deprecated: datetime | None = None, ) -> tuple[dict[str, Any], ...]: service = _random_service(key=key, version=version, deprecated=deprecated) @@ -420,7 +433,7 @@ def _fake_factory( fakes.append( _random_access( service, - gid=team_gid, + gid=team_standard_gid, execute_access="x" in team_access, write_access="w" in team_access, product_name=product, diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 1cafdc4ea60..646f35aa764 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -8,6 +8,8 @@ import pytest from fastapi import FastAPI from models_library.products import ProductName +from models_library.services_access import ServiceGroupAccessRightsV2 +from models_library.services_history import CompatibleService from models_library.users import UserID from respx.router import MockRouter from simcore_service_catalog.api.dependencies.director import get_director_api @@ -152,22 +154,24 @@ async def test_list_services_paginated( async def test_batch_get_my_services( - background_sync_task_mocked: None, + background_tasks_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, mocked_director_service_api: MockRouter, target_product: ProductName, services_repo: ServicesRepository, groups_repo: GroupsRepository, user_id: UserID, + user: dict[str, Any], create_fake_service_data: Callable, services_db_tables_injector: Callable, ): - # Create fake services data + # catalog service_key = "simcore/services/comp/some-service" - service_version_1 = "1.0.0" - service_version_2 = "2.0.0" + service_version_1 = "1.0.0" # can upgrade to 1.0.1 + service_version_2 = "1.0.1" # latest + other_service_key = "simcore/services/comp/other-service" - other_service_version = "1.0.0" + other_service_version = "2.0.0" fake_service_1 = create_fake_service_data( service_key, @@ -194,10 +198,9 @@ async def test_batch_get_my_services( # Inject fake services into the database await services_db_tables_injector([fake_service_1, fake_service_2, fake_service_3]) - # Batch get my services - ids = [ + # Batch get services e.g. services in a project + services_ids = [ (service_key, service_version_1), - (service_key, service_version_2), (other_service_key, other_service_version), ] @@ -206,13 +209,29 @@ async def test_batch_get_my_services( groups_repo, product_name=target_product, user_id=user_id, - ids=ids, + ids=services_ids, + ) + + # assert returned order and length as ids + assert services_ids == [(sc.key, sc.release.version) for sc in my_services] + + # assert access: owns them + assert my_services[0].my_access_rights == ServiceGroupAccessRightsV2( + execute=True, write=True + ) + assert my_services[0].owner == user["primary_gid"] + + assert my_services[1].my_access_rights == ServiceGroupAccessRightsV2( + execute=True, write=True ) + assert my_services[1].owner == user["primary_gid"] - assert len(my_services) == 3 + # assert status: first can be updated but not second + assert my_services[0].release.retired is None + assert my_services[0].release.compatibility + assert my_services[0].release.compatibility.can_update_to == CompatibleService( + version=service_version_2 + ) # can be updated - # Check access rights - assert my_services[0].my_access_rights == {} - assert my_services[1].my_access_rights is not None - assert my_services[2].my_access_rights is not None - assert my_services[2].owner is not None + assert my_services[1].release.retired is None + assert my_services[2].release.compatibility is None # nothing to update From 19cc9d1f1ab57f06d91ad18e474f60320dfe2a08 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 14:42:09 +0100 Subject: [PATCH 08/39] expanding test --- .../helpers/catalog_services.py | 25 ++++++++ services/catalog/tests/unit/conftest.py | 1 + .../catalog/tests/unit/with_dbs/conftest.py | 20 ++---- .../with_dbs/test_services_services_api.py | 63 ++++++++++++------- 4 files changed, 72 insertions(+), 37 deletions(-) create mode 100644 packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py new file mode 100644 index 00000000000..6268931cd23 --- /dev/null +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py @@ -0,0 +1,25 @@ +# pylint: disable=not-context-manager +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable + +from datetime import datetime +from typing import Protocol + +from models_library.products import ProductName + + +class CreateFakeServiceDataCallable(Protocol): + """Signature for services/catalog/tests/unit/with_dbs/conftest.py::create_fake_service_data""" + + def __call__( + self, + key, + version, + team_access: str | None = None, + everyone_access: str | None = None, + product: ProductName = "osparc", + deprecated: datetime | None = None, # DB column + ): + ... diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index 4b0000cbac4..1608b93157b 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -34,6 +34,7 @@ pytest_plugins = [ "pytest_simcore.cli_runner", "pytest_simcore.docker_compose", + "pytest_simcore.simcore_catalog_service", "pytest_simcore.docker_registry", "pytest_simcore.docker_swarm", "pytest_simcore.environment_configs", diff --git a/services/catalog/tests/unit/with_dbs/conftest.py b/services/catalog/tests/unit/with_dbs/conftest.py index 467083a1b6f..9a8a259d828 100644 --- a/services/catalog/tests/unit/with_dbs/conftest.py +++ b/services/catalog/tests/unit/with_dbs/conftest.py @@ -8,7 +8,7 @@ from collections.abc import AsyncIterator, Awaitable, Callable from copy import deepcopy from datetime import datetime -from typing import Any, Protocol +from typing import Any import pytest import sqlalchemy as sa @@ -18,6 +18,7 @@ from models_library.services import ServiceMetaDataPublished from models_library.users import UserID from pydantic import ConfigDict, TypeAdapter +from pytest_simcore.helpers.catalog_services import CreateFakeServiceDataCallable from pytest_simcore.helpers.faker_factories import ( random_service_access_rights, random_service_meta_data, @@ -354,25 +355,12 @@ def _fake_factory(**overrides): return _fake_factory -class CreateFakeServiceData(Protocol): - def __call__( - self, - key, - version, - team_access: str | None = None, - everyone_access: str | None = None, - product: ProductName = "osparc", - deprecated: datetime | None = None, - ): - ... - - -@pytest.fixture() +@pytest.fixture async def create_fake_service_data( user_groups_ids: list[int], products_names: list[str], faker: Faker, -) -> Callable: +) -> CreateFakeServiceDataCallable: """Returns a fake factory that creates catalog DATA that can be used to fill both services_meta_data and services_access_rights tables diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 646f35aa764..c09da7c17f0 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -3,14 +3,17 @@ # pylint: disable=unused-variable from collections.abc import Callable +from datetime import timedelta from typing import Any +import arrow import pytest from fastapi import FastAPI +from models_library.api_schemas_catalog.services import MyServiceGet from models_library.products import ProductName -from models_library.services_access import ServiceGroupAccessRightsV2 -from models_library.services_history import CompatibleService from models_library.users import UserID +from pydantic import TypeAdapter +from pytest_simcore.helpers.catalog_services import CreateFakeServiceDataCallable from respx.router import MockRouter from simcore_service_catalog.api.dependencies.director import get_director_api from simcore_service_catalog.db.repositories.groups import GroupsRepository @@ -162,7 +165,7 @@ async def test_batch_get_my_services( groups_repo: GroupsRepository, user_id: UserID, user: dict[str, Any], - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, ): # catalog @@ -173,12 +176,15 @@ async def test_batch_get_my_services( other_service_key = "simcore/services/comp/other-service" other_service_version = "2.0.0" + expected_retirement = arrow.now().datetime + timedelta(days=1) + fake_service_1 = create_fake_service_data( service_key, service_version_1, team_access=None, everyone_access=None, product=target_product, + deprecated=expected_retirement, ) fake_service_2 = create_fake_service_data( service_key, @@ -215,23 +221,38 @@ async def test_batch_get_my_services( # assert returned order and length as ids assert services_ids == [(sc.key, sc.release.version) for sc in my_services] - # assert access: owns them - assert my_services[0].my_access_rights == ServiceGroupAccessRightsV2( - execute=True, write=True - ) - assert my_services[0].owner == user["primary_gid"] - - assert my_services[1].my_access_rights == ServiceGroupAccessRightsV2( - execute=True, write=True - ) - assert my_services[1].owner == user["primary_gid"] - - # assert status: first can be updated but not second - assert my_services[0].release.retired is None - assert my_services[0].release.compatibility - assert my_services[0].release.compatibility.can_update_to == CompatibleService( - version=service_version_2 - ) # can be updated - assert my_services[1].release.retired is None assert my_services[2].release.compatibility is None # nothing to update + + released = my_services[1].release + + assert my_services == TypeAdapter(list[MyServiceGet]).validate_python( + [ + { + "key": "simcore/services/comp/some-service", + "release": { + "version": "1.0.0", + "version_display": None, + "released": released, + "retired": expected_retirement, + "compatibility": { + "can_update_to": {"version": "1.0.1"} + }, # can be updated + }, + "owner": user["primary_gid"], + "my_access_rights": {"execute": True, "write": True}, # full access + }, + { + "key": "simcore/services/comp/other-service", + "release": { + "version": "2.0.0", + "version_display": None, + "released": released, + "retired": None, + "compatibility": None, # cannot be updated + }, + "owner": user["primary_gid"], + "my_access_rights": {"execute": True, "write": True}, # full acces + }, + ] + ) From 50f2de126f2d016926cfb24c8cb09152fef8dbbe Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 14:44:18 +0100 Subject: [PATCH 09/39] update --- .../catalog/tests/unit/with_dbs/test_services_services_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index c09da7c17f0..315aca628db 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -252,7 +252,7 @@ async def test_batch_get_my_services( "compatibility": None, # cannot be updated }, "owner": user["primary_gid"], - "my_access_rights": {"execute": True, "write": True}, # full acces + "my_access_rights": {"execute": True, "write": True}, # full access }, ] ) From d2f8ec94dfa16cde6c96051bd38bab724fb274b9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 14:52:21 +0100 Subject: [PATCH 10/39] connect to rpc --- .../simcore_service_catalog/api/rpc/_services.py | 15 ++++++++++++++- .../unit/with_dbs/test_services_services_api.py | 4 ++++ 2 files changed, 18 insertions(+), 1 deletion(-) 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 e06b00f92ee..9926d8b45f4 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -21,6 +21,7 @@ CatalogForbiddenError, CatalogItemNotFoundError, ) +from simcore_service_catalog.db.repositories.groups import GroupsRepository from ...db.repositories.services import ServicesRepository from ...services import services_api @@ -182,5 +183,17 @@ async def batch_get_my_services( ] ], ) -> list[MyServiceGet]: + assert app.state.engine # nosec + + # TODO: id not found? + services = await services_api.batch_get_my_services( + repo=ServicesRepository(app.state.engine), + groups_repo=GroupsRepository(app.state.engine), + product_name=product_name, + user_id=user_id, + ids=ids, + ) + + assert [(sv.key, sv.release.version) for sv in services] == ids # nosec - raise NotImplementedError + return services diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 315aca628db..c7e86ba2829 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -256,3 +256,7 @@ async def test_batch_get_my_services( }, ] ) + + # TODO: test user has no access to one and needs to ask owner (NOTE: owner can be a group? ) + # TODO: test user has no access to entire history + # From f5a7c4084613af5339b6baae90c351bb389a10ca Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 15:23:09 +0100 Subject: [PATCH 11/39] updates listing --- .../api_schemas_catalog/services.py | 15 +++-- .../src/models_library/services_history.py | 57 +++++++++---------- .../services/services_api.py | 34 ++++++++--- services/catalog/tests/unit/conftest.py | 1 - .../with_dbs/test_services_services_api.py | 7 ++- 5 files changed, 69 insertions(+), 45 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 a049bcbec55..880b7c5474e 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 @@ -171,7 +171,7 @@ def _update_json_schema_extra(schema: JsonDict) -> None: ) -class ServiceGetV2Base(CatalogOutputSchema): +class _BaseServiceGetV2(CatalogOutputSchema): # Model used in catalog's rpc and rest interfaces key: ServiceKey version: ServiceVersion @@ -205,8 +205,14 @@ class ServiceGetV2Base(CatalogOutputSchema): classifiers: list[str] | None = [] quality: dict[str, Any] = {} + model_config = ConfigDict( + extra="forbid", + populate_by_name=True, + alias_generator=snake_to_camel, + ) -class ServiceGetV2(ServiceGetV2Base): + +class ServiceGetV2(_BaseServiceGetV2): # Model used in catalog's rpc and rest interfaces history: Annotated[ list[ServiceRelease], @@ -278,14 +284,11 @@ def _update_json_schema_extra(schema: JsonDict) -> None: ) model_config = ConfigDict( - extra="forbid", - populate_by_name=True, - alias_generator=snake_to_camel, json_schema_extra=_update_json_schema_extra, ) -class ServiceItemList(ServiceGetV2Base): +class ServiceItemList(_BaseServiceGetV2): release: ServiceRelease history: Annotated[ list[ServiceRelease], diff --git a/packages/models-library/src/models_library/services_history.py b/packages/models-library/src/models_library/services_history.py index b38f5f2e783..91ed08fbe4b 100644 --- a/packages/models-library/src/models_library/services_history.py +++ b/packages/models-library/src/models_library/services_history.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import TypeAlias +from typing import Annotated, TypeAlias from pydantic import BaseModel, ConfigDict, Field @@ -8,41 +8,42 @@ class CompatibleService(BaseModel): - key: ServiceKey | None = Field( - default=None, - description="If None, it refer to current service. Used only for inter-service compatibility", - ) + key: Annotated[ + ServiceKey | None, + Field( + description="If None, it refer to current service. Used only for inter-service compatibility" + ), + ] = None version: ServiceVersion class Compatibility(BaseModel): - # NOTE: as an object it is more maintainable than a list - can_update_to: CompatibleService = Field( - ..., description="Latest compatible service at this moment" - ) + can_update_to: Annotated[ + CompatibleService, Field(description="Latest compatible service at this moment") + ] model_config = ConfigDict(alias_generator=snake_to_camel, populate_by_name=True) class ServiceRelease(BaseModel): - # from ServiceMetaDataPublished version: ServiceVersion - version_display: str | None = Field( - default=None, description="If None, then display `version`" - ) - released: datetime | None = Field( - default=None, description="When provided, it indicates the release timestamp" - ) - retired: datetime | None = Field( - default=None, - description="whether this service is planned to be retired. " - "If None, the service is still active. " - "If now tuple[NonNegativeInt, list[ServiceGetV2]]: +) -> tuple[NonNegativeInt, list[ServiceItemList]]: # defines the order total_count, services = await repo.list_latest_services( @@ -120,12 +121,15 @@ async def list_services_paginated( items = [ _db_to_api_model( - service_db=s, access_rights_db=ar, service_manifest=sm, compatibility_map=cm + service_db=sc, + access_rights_db=ar, + service_manifest=sm, + compatibility_map=cm, ) - for s in services + for sc in services if ( - (ar := access_rights.get((s.key, s.version))) - and (sm := service_manifest.get((s.key, s.version))) + (ar := access_rights.get((sc.key, sc.version))) + and (sm := service_manifest.get((sc.key, sc.version))) and ( # NOTE: This operation might be resource-intensive. # It is temporarily implemented on a trial basis. @@ -133,13 +137,29 @@ async def list_services_paginated( repo, product_name=product_name, user_id=user_id, - service_release_history=s.history, + service_release_history=sc.history, ) ) ) ] - return total_count, items + def _get_release(item: ServiceGetV2) -> ServiceRelease: + for rs in item.history: + if rs.version == item.version: + return rs + return ServiceRelease( + version=item.version, version_display=item.version_display, released=None + ) + + return total_count, [ + ServiceItemList.model_validate( + { + **it.model_dump(exclude_unset=True, by_alias=True), + "release": _get_release(it), + } + ) + for it in items + ] async def get_service( diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index 1608b93157b..4b0000cbac4 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -34,7 +34,6 @@ pytest_plugins = [ "pytest_simcore.cli_runner", "pytest_simcore.docker_compose", - "pytest_simcore.simcore_catalog_service", "pytest_simcore.docker_registry", "pytest_simcore.docker_swarm", "pytest_simcore.environment_configs", diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index c7e86ba2829..7c5adabca89 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -89,7 +89,10 @@ async def background_sync_task_mocked( services_db_tables_injector: Callable, fake_services_data: list, ) -> None: - # inject db services (typically done by the sync background task) + """ + Emulates a sync backgroundtask that injects + some services in the db + """ await services_db_tables_injector(fake_services_data) @@ -150,7 +153,7 @@ async def test_list_services_paginated( service_version=item.version, ) - assert got == item + assert got.model_dump() == item.model_dump(exclude={"release"}) # since it is cached, it should only call it `limit` times assert mocked_director_service_api["get_service"].call_count == limit From 43ae5b0fc22fe850b01f023eae2a0a3ecd8c0bd8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 16:52:04 +0100 Subject: [PATCH 12/39] fix tests --- .../with_dbs/test_api_rest_services__list.py | 4 +++- .../unit/with_dbs/test_services_services_api.py | 16 ++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/services/catalog/tests/unit/with_dbs/test_api_rest_services__list.py b/services/catalog/tests/unit/with_dbs/test_api_rest_services__list.py index 301c64bf7be..732dda730c1 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rest_services__list.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rest_services__list.py @@ -240,7 +240,9 @@ async def test_list_services_that_are_deprecated( ): # injects fake data in db - deprecation_date = datetime.utcnow() + timedelta(days=1) + deprecation_date = datetime.utcnow() + timedelta( # NOTE: old offset-naive column + days=1 + ) deprecated_service = create_fake_service_data( "simcore/services/dynamic/jupyterlab", "1.0.1", diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 7c5adabca89..4fbdd5a144d 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -3,10 +3,9 @@ # pylint: disable=unused-variable from collections.abc import Callable -from datetime import timedelta +from datetime import datetime, timedelta from typing import Any -import arrow import pytest from fastapi import FastAPI from models_library.api_schemas_catalog.services import MyServiceGet @@ -179,7 +178,9 @@ async def test_batch_get_my_services( other_service_key = "simcore/services/comp/other-service" other_service_version = "2.0.0" - expected_retirement = arrow.now().datetime + timedelta(days=1) + expected_retirement = datetime.utcnow() + timedelta( + days=1 + ) # NOTE: old offset-naive column fake_service_1 = create_fake_service_data( service_key, @@ -224,11 +225,6 @@ async def test_batch_get_my_services( # assert returned order and length as ids assert services_ids == [(sc.key, sc.release.version) for sc in my_services] - assert my_services[1].release.retired is None - assert my_services[2].release.compatibility is None # nothing to update - - released = my_services[1].release - assert my_services == TypeAdapter(list[MyServiceGet]).validate_python( [ { @@ -236,7 +232,7 @@ async def test_batch_get_my_services( "release": { "version": "1.0.0", "version_display": None, - "released": released, + "released": my_services[0].release.released, "retired": expected_retirement, "compatibility": { "can_update_to": {"version": "1.0.1"} @@ -250,7 +246,7 @@ async def test_batch_get_my_services( "release": { "version": "2.0.0", "version_display": None, - "released": released, + "released": my_services[1].release.released, "retired": None, "compatibility": None, # cannot be updated }, From 9479efd6d937fe98657a5a6225425773ce5bb108 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 17:02:29 +0100 Subject: [PATCH 13/39] cleanup --- .../api_schemas_catalog/services.py | 1 - .../services/services_api.py | 17 ++++------------- .../unit/with_dbs/test_services_services_api.py | 2 +- 3 files changed, 5 insertions(+), 15 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 880b7c5474e..980c5cbf24f 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 @@ -289,7 +289,6 @@ def _update_json_schema_extra(schema: JsonDict) -> None: class ServiceItemList(_BaseServiceGetV2): - release: ServiceRelease history: Annotated[ list[ServiceRelease], Field( 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 2c3f235d953..9f87efe16ca 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -98,10 +98,10 @@ async def list_services_paginated( if services: # injects access-rights - access_rights: dict[ - tuple[str, str], list[ServiceAccessRightsAtDB] - ] = await repo.batch_get_services_access_rights( - ((s.key, s.version) for s in services), product_name=product_name + access_rights: dict[tuple[str, str], list[ServiceAccessRightsAtDB]] = ( + await repo.batch_get_services_access_rights( + ((s.key, s.version) for s in services), product_name=product_name + ) ) if not access_rights: raise CatalogForbiddenError( @@ -143,19 +143,10 @@ async def list_services_paginated( ) ] - def _get_release(item: ServiceGetV2) -> ServiceRelease: - for rs in item.history: - if rs.version == item.version: - return rs - return ServiceRelease( - version=item.version, version_display=item.version_display, released=None - ) - return total_count, [ ServiceItemList.model_validate( { **it.model_dump(exclude_unset=True, by_alias=True), - "release": _get_release(it), } ) for it in items diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 4fbdd5a144d..94b8b3b514d 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -152,7 +152,7 @@ async def test_list_services_paginated( service_version=item.version, ) - assert got.model_dump() == item.model_dump(exclude={"release"}) + assert got.model_dump() == item.model_dump() # since it is cached, it should only call it `limit` times assert mocked_director_service_api["get_service"].call_count == limit From dfd365e8705b5e397e7f260dd797b5201d50a316 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 17:11:27 +0100 Subject: [PATCH 14/39] drop api changes --- api/specs/web-server/_projects_nodes.py | 17 ++-- services/storage/openapi.json | 2 +- .../api/v0/openapi.yaml | 80 +------------------ 3 files changed, 12 insertions(+), 87 deletions(-) diff --git a/api/specs/web-server/_projects_nodes.py b/api/specs/web-server/_projects_nodes.py index 3d3d67f6056..9ad716b77cd 100644 --- a/api/specs/web-server/_projects_nodes.py +++ b/api/specs/web-server/_projects_nodes.py @@ -77,8 +77,7 @@ def delete_node(project_id: str, node_id: str): # noqa: ARG001 ) def retrieve_node( project_id: str, node_id: str, _retrieve: NodeRetrieve # noqa: ARG001 -): - ... +): ... @router.post( @@ -148,8 +147,7 @@ def get_node_resources(project_id: str, node_id: str): # noqa: ARG001 ) def replace_node_resources( project_id: str, node_id: str, _new: ServiceResourcesDict # noqa: ARG001 -): - ... +): ... # @@ -160,9 +158,10 @@ def replace_node_resources( @router.get( "/projects/{project_id}/nodes/-/services", response_model=Envelope[ProjectNodeServicesGet], + # NOTE: will be activated on the follow up from https://github.com/ITISFoundation/osparc-simcore/pull/7287 + include_in_schema=False, ) -async def get_project_services(project_id: ProjectID): - ... +async def get_project_services(project_id: ProjectID): ... @router.get( @@ -172,8 +171,7 @@ async def get_project_services(project_id: ProjectID): ) async def get_project_services_access_for_gid( project_id: ProjectID, for_gid: GroupID # noqa: ARG001 -): - ... +): ... assert_handler_signature_against_model( @@ -206,8 +204,7 @@ async def list_project_nodes_previews(project_id: ProjectID): # noqa: ARG001 ) async def get_project_node_preview( project_id: ProjectID, node_id: NodeID # noqa: ARG001 -): - ... +): ... assert_handler_signature_against_model(get_project_node_preview, NodePathParams) diff --git a/services/storage/openapi.json b/services/storage/openapi.json index 79c56f056fa..f826e6b0526 100644 --- a/services/storage/openapi.json +++ b/services/storage/openapi.json @@ -2443,7 +2443,7 @@ "type": "string", "format": "path", "title": "Display Path", - "description": "the path to display with UUID replaced" + "description": "the path to display with UUID replaced (URL Encoded by parts as names may contain '/')" }, "file_meta_data": { "anyOf": [ diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index ef8c915fac9..d734d72c125 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -4927,28 +4927,6 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_dict_Annotated_str__StringConstraints___ImageResources__' - /v0/projects/{project_id}/nodes/-/services: - get: - tags: - - projects - - nodes - summary: Get Project Services - operationId: get_project_services - parameters: - - name: project_id - in: path - required: true - schema: - type: string - format: uuid - title: Project Id - responses: - '200': - description: Successful Response - content: - application/json: - schema: - $ref: '#/components/schemas/Envelope_ProjectNodeServicesGet_' /v0/projects/{project_id}/nodes/-/services:access: get: tags: @@ -9129,19 +9107,6 @@ components: title: Error type: object title: Envelope[ProjectMetadataGet] - Envelope_ProjectNodeServicesGet_: - properties: - data: - anyOf: - - $ref: '#/components/schemas/ProjectNodeServicesGet' - - type: 'null' - error: - anyOf: - - {} - - type: 'null' - title: Error - type: object - title: Envelope[ProjectNodeServicesGet] Envelope_ProjectState_: properties: data: @@ -12097,28 +12062,6 @@ components: - thumbnail_url - file_url title: NodeScreenshot - NodeServiceGet: - properties: - key: - type: string - pattern: ^simcore/services/((comp|dynamic|frontend))/([a-z0-9][a-z0-9_.-]*/)*([a-z0-9-_]+[a-z0-9])$ - title: Key - release: - $ref: '#/components/schemas/ServiceRelease' - owner: - type: integer - exclusiveMinimum: true - title: Owner - minimum: 0 - myAccessRights: - $ref: '#/components/schemas/AccessRights' - type: object - required: - - key - - release - - owner - - myAccessRights - title: NodeServiceGet NodeState: properties: modified: @@ -12446,7 +12389,8 @@ components: type: string format: path title: Display Path - description: the path to display with UUID replaced + description: the path to display with UUID replaced (URL Encoded by parts + as names may contain '/') file_meta_data: anyOf: - $ref: '#/components/schemas/FileMetaDataGet' @@ -13606,22 +13550,6 @@ components: required: - custom title: ProjectMetadataUpdate - ProjectNodeServicesGet: - properties: - projectUuid: - type: string - format: uuid - title: Projectuuid - services: - items: - $ref: '#/components/schemas/NodeServiceGet' - type: array - title: Services - type: object - required: - - projectUuid - - services - title: ProjectNodeServicesGet ProjectOutputGet: properties: key: @@ -14474,9 +14402,9 @@ components: format: date-time - type: 'null' title: Retired - description: 'whether this service is planned to be retired. If None, the + description: whether this service is planned to be retired. If None, the service is still active. If now Date: Thu, 27 Feb 2025 17:23:42 +0100 Subject: [PATCH 15/39] items --- .../rabbitmq/rpc_interfaces/catalog/services.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 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 ed7d4662a00..bdb76461e38 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 @@ -4,7 +4,11 @@ from typing import Any, cast from models_library.api_schemas_catalog import CATALOG_RPC_NAMESPACE -from models_library.api_schemas_catalog.services import ServiceGetV2, ServiceUpdateV2 +from models_library.api_schemas_catalog.services import ( + ServiceGetV2, + ServiceItemList, + ServiceUpdateV2, +) from models_library.products import ProductName from models_library.rabbitmq_basic_types import RPCMethodName from models_library.rpc_pagination import ( @@ -30,7 +34,7 @@ async def list_services_paginated( # pylint: disable=too-many-arguments user_id: UserID, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, offset: NonNegativeInt = 0, -) -> PageRpc[ServiceGetV2]: +) -> PageRpc[ServiceItemList]: """ Raises: ValidationError: on invalid arguments @@ -57,10 +61,10 @@ async def _call( result = await _call( product_name=product_name, user_id=user_id, limit=limit, offset=offset ) - assert ( - TypeAdapter(PageRpc[ServiceGetV2]).validate_python(result) is not None - ) # nosec - return cast(PageRpc[ServiceGetV2], result) + assert ( # nosec + TypeAdapter(PageRpc[ServiceItemList]).validate_python(result) is not None + ) + return cast(PageRpc[ServiceItemList], result) @log_decorator(_logger, level=logging.DEBUG) From 47d96c690f58307d7e7642e5b6252417168d9c9f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 27 Feb 2025 17:34:46 +0100 Subject: [PATCH 16/39] other user fixture --- .../catalog/tests/unit/with_dbs/conftest.py | 19 ++++++++++++++ .../tests/unit/with_dbs/test_api_rpc.py | 25 ++----------------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/services/catalog/tests/unit/with_dbs/conftest.py b/services/catalog/tests/unit/with_dbs/conftest.py index 9a8a259d828..74daa97804d 100644 --- a/services/catalog/tests/unit/with_dbs/conftest.py +++ b/services/catalog/tests/unit/with_dbs/conftest.py @@ -22,6 +22,7 @@ from pytest_simcore.helpers.faker_factories import ( random_service_access_rights, random_service_meta_data, + random_user, ) from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.postgres_tools import ( @@ -160,6 +161,24 @@ async def user( yield row +@pytest.fixture +async def other_user( + user_id: UserID, + sqlalchemy_async_engine: AsyncEngine, + faker: Faker, +) -> AsyncIterator[dict[str, Any]]: + + _user = random_user(fake=faker, id=user_id + 1) + async with insert_and_get_row_lifespan( # pylint:disable=contextmanager-generator-missing-cleanup + sqlalchemy_async_engine, + table=users, + values=_user, + pk_col=users.c.id, + pk_value=_user["id"], + ) as row: + yield row + + @pytest.fixture() async def user_groups_ids( sqlalchemy_async_engine: AsyncEngine, user: dict[str, Any] 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 3605eeae7f0..8b47045fad2 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -5,7 +5,7 @@ # pylint: disable=unused-variable -from collections.abc import AsyncIterator, Callable +from collections.abc import Callable from typing import Any import pytest @@ -16,9 +16,8 @@ from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import ValidationError -from pytest_simcore.helpers.faker_factories import random_icon_url, random_user +from pytest_simcore.helpers.faker_factories import random_icon_url from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict -from pytest_simcore.helpers.postgres_tools import insert_and_get_row_lifespan from pytest_simcore.helpers.typing_env import EnvVarsDict from respx.router import MockRouter from servicelib.rabbitmq import RabbitMQRPCClient @@ -32,8 +31,6 @@ list_services_paginated, update_service, ) -from simcore_postgres_database.models.users import users -from sqlalchemy.ext.asyncio import AsyncEngine pytest_simcore_core_services_selection = [ "rabbit", @@ -260,24 +257,6 @@ async def test_rpc_check_for_service( ) -@pytest.fixture -async def other_user( - user_id: UserID, - sqlalchemy_async_engine: AsyncEngine, - faker: Faker, -) -> AsyncIterator[dict[str, Any]]: - - _user = random_user(fake=faker, id=user_id + 1) - async with insert_and_get_row_lifespan( # pylint:disable=contextmanager-generator-missing-cleanup - sqlalchemy_async_engine, - table=users, - values=_user, - pk_col=users.c.id, - pk_value=_user["id"], - ) as row: - yield row - - async def test_rpc_get_service_access_rights( background_sync_task_mocked: None, mocked_director_service_api: MockRouter, From 3451d57443aaff44b80b4f4f189e83ec2614fa53 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 10:13:35 +0100 Subject: [PATCH 17/39] fix tests --- services/catalog/tests/unit/with_dbs/test_api_rpc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 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 8b47045fad2..02604230fdb 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -161,8 +161,8 @@ async def test_rpc_catalog_client( assert got.key == service_key assert got.version == service_version - assert got == next( - item + assert got.model_dump() == next( + item.model_dump() for item in page.data if (item.key == service_key and item.version == service_version) ) From e2bfac8764f80ca3515ac44a524941cf8e045bab Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 10:23:43 +0100 Subject: [PATCH 18/39] drafts test --- .../rpc_interfaces/catalog/services.py | 40 ++++++++++ .../tests/unit/with_dbs/test_api_rpc.py | 75 +++++++++++++++++++ 2 files changed, 115 insertions(+) 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 bdb76461e38..c312bea3b5a 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 @@ -5,6 +5,7 @@ from models_library.api_schemas_catalog import CATALOG_RPC_NAMESPACE from models_library.api_schemas_catalog.services import ( + MyServiceGet, ServiceGetV2, ServiceItemList, ServiceUpdateV2, @@ -195,3 +196,42 @@ async def _call( service_key=service_key, service_version=service_version, ) + + +@log_decorator(_logger, level=logging.DEBUG) +async def batch_get_my_services( + rpc_client: RabbitMQRPCClient, + *, + product_name: ProductName, + user_id: UserID, + ids: list[ + tuple[ + ServiceKey, + ServiceVersion, + ] + ], +) -> list[MyServiceGet]: + """ + Raises: + ValidationError: on invalid arguments + CatalogForbiddenError: no access-rights to list services + """ + + @validate_call() + async def _call( + product_name: ProductName, + user_id: UserID, + ids: list[tuple[ServiceKey, ServiceVersion]], + ): + return await rpc_client.request( + CATALOG_RPC_NAMESPACE, + TypeAdapter(RPCMethodName).validate_python("batch_get_my_services"), + product_name=product_name, + user_id=user_id, + ids=ids, + timeout_s=40 * RPC_REQUEST_DEFAULT_TIMEOUT_S, + ) + + 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) 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 02604230fdb..0893109b386 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -26,6 +26,7 @@ CatalogItemNotFoundError, ) from servicelib.rabbitmq.rpc_interfaces.catalog.services import ( + batch_get_my_services, check_for_service, get_service, list_services_paginated, @@ -397,3 +398,77 @@ async def test_rpc_get_service_access_rights( "name": "foo", "description": "bar", } + + +async def test_rpc_batch_get_my_services( + 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, +): + # Create fake services data + service_key = "simcore/services/comp/test-batch-service" + service_version_1 = "1.0.0" + service_version_2 = "2.0.0" + other_service_key = "simcore/services/comp/other-batch-service" + other_service_version = "1.0.0" + + fake_service_1 = create_fake_service_data( + service_key, + service_version_1, + team_access=None, + everyone_access=None, + product=product_name, + ) + fake_service_2 = create_fake_service_data( + service_key, + service_version_2, + team_access="x", + everyone_access=None, + product=product_name, + ) + fake_service_3 = create_fake_service_data( + other_service_key, + other_service_version, + team_access=None, + everyone_access=None, + product=product_name, + ) + + # Inject fake services into the database + await services_db_tables_injector([fake_service_1, fake_service_2, fake_service_3]) + + # Batch get my services + ids = [ + (service_key, service_version_1), + (service_key, service_version_2), + (other_service_key, other_service_version), + ] + + my_services = await batch_get_my_services( + rpc_client, + product_name=product_name, + user_id=user_id, + ids=ids, + ) + + assert len(my_services) == 3 + + # Check access rights + assert my_services[0].my_access_rights.model_dump() == { + "execute": False, + "write": False, + } + assert my_services[1].my_access_rights.model_dump() == { + "execute": True, + "write": False, + } + assert my_services[2].my_access_rights.model_dump() == { + "execute": False, + "write": False, + } + assert my_services[2].owner is not None From 18c0dd56cd560bb5dc29ce8888c37fd017f547a9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 14:37:04 +0100 Subject: [PATCH 19/39] fixes models --- api/specs/web-server/_catalog.py | 64 ++++++++----------- .../api_schemas_catalog/services.py | 4 +- .../api_schemas_webserver/catalog.py | 43 +++++++------ .../rpc_interfaces/catalog/services.py | 8 +-- .../services/services_api.py | 6 +- 5 files changed, 58 insertions(+), 67 deletions(-) diff --git a/api/specs/web-server/_catalog.py b/api/specs/web-server/_catalog.py index 90dd187c55b..153c2d8b968 100644 --- a/api/specs/web-server/_catalog.py +++ b/api/specs/web-server/_catalog.py @@ -4,6 +4,7 @@ from models_library.api_schemas_api_server.pricing_plans import ServicePricingPlanGet from models_library.api_schemas_webserver.catalog import ( CatalogServiceGet, + CatalogServiceListItem, CatalogServiceUpdate, ServiceInputGet, ServiceInputKey, @@ -31,16 +32,11 @@ ) -# -# /catalog/services/* COLLECTION -# - - @router.get( "/catalog/services/-/latest", - response_model=Page[CatalogServiceGet], + response_model=Page[CatalogServiceListItem], ) -def list_services_latest(_query_params: Annotated[ListServiceParams, Depends()]): +def list_services_latest(_query: Annotated[ListServiceParams, Depends()]): pass @@ -48,8 +44,7 @@ def list_services_latest(_query_params: Annotated[ListServiceParams, Depends()]) "/catalog/services/{service_key}/{service_version}", response_model=Envelope[CatalogServiceGet], ) -def get_service(_path_params: Annotated[ServicePathParams, Depends()]): - ... +def get_service(_path: Annotated[ServicePathParams, Depends()]): ... @router.patch( @@ -57,10 +52,9 @@ def get_service(_path_params: Annotated[ServicePathParams, Depends()]): response_model=Envelope[CatalogServiceGet], ) def update_service( - _path_params: Annotated[ServicePathParams, Depends()], - _update: CatalogServiceUpdate, -): - ... + _path: Annotated[ServicePathParams, Depends()], + _body: CatalogServiceUpdate, +): ... @router.get( @@ -68,9 +62,8 @@ def update_service( response_model=Envelope[list[ServiceInputGet]], ) def list_service_inputs( - _path_params: Annotated[ServicePathParams, Depends()], -): - ... + _path: Annotated[ServicePathParams, Depends()], +): ... @router.get( @@ -78,9 +71,8 @@ def list_service_inputs( response_model=Envelope[ServiceInputGet], ) def get_service_input( - _path_params: Annotated[_ServiceInputsPathParams, Depends()], -): - ... + _path: Annotated[_ServiceInputsPathParams, Depends()], +): ... @router.get( @@ -88,10 +80,9 @@ def get_service_input( response_model=Envelope[list[ServiceInputKey]], ) def get_compatible_inputs_given_source_output( - _path_params: Annotated[ServicePathParams, Depends()], - _query_params: Annotated[_FromServiceOutputParams, Depends()], -): - ... + _path: Annotated[ServicePathParams, Depends()], + _query: Annotated[_FromServiceOutputParams, Depends()], +): ... @router.get( @@ -99,9 +90,8 @@ def get_compatible_inputs_given_source_output( response_model=Envelope[list[ServiceOutputKey]], ) def list_service_outputs( - _path_params: Annotated[ServicePathParams, Depends()], -): - ... + _path: Annotated[ServicePathParams, Depends()], +): ... @router.get( @@ -109,9 +99,8 @@ def list_service_outputs( response_model=Envelope[list[ServiceOutputGet]], ) def get_service_output( - _path_params: Annotated[_ServiceOutputsPathParams, Depends()], -): - ... + _path: Annotated[_ServiceOutputsPathParams, Depends()], +): ... @router.get( @@ -119,10 +108,9 @@ def get_service_output( response_model=Envelope[list[ServiceOutputKey]], ) def get_compatible_outputs_given_target_input( - _path_params: Annotated[ServicePathParams, Depends()], - _query_params: Annotated[_ToServiceInputsParams, Depends()], -): - ... + _path: Annotated[ServicePathParams, Depends()], + _query: Annotated[_ToServiceInputsParams, Depends()], +): ... @router.get( @@ -130,9 +118,8 @@ def get_compatible_outputs_given_target_input( response_model=Envelope[ServiceResourcesGet], ) def get_service_resources( - _params: Annotated[ServicePathParams, Depends()], -): - ... + _path: Annotated[ServicePathParams, Depends()], +): ... @router.get( @@ -142,6 +129,5 @@ def get_service_resources( tags=["pricing-plans"], ) async def get_service_pricing_plan( - _params: Annotated[ServicePathParams, Depends()], -): - ... + _path: Annotated[ServicePathParams, Depends()], +): ... 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 980c5cbf24f..fd522655ded 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 @@ -288,7 +288,7 @@ def _update_json_schema_extra(schema: JsonDict) -> None: ) -class ServiceItemList(_BaseServiceGetV2): +class ServiceListItem(_BaseServiceGetV2): history: Annotated[ list[ServiceRelease], Field( @@ -302,7 +302,7 @@ class ServiceItemList(_BaseServiceGetV2): PageRpcServicesGetV2: TypeAlias = PageRpc[ # WARNING: keep this definition in models_library and not in the RPC interface - ServiceItemList + ServiceListItem ] ServiceResourcesGet: TypeAlias = ServiceResourcesDict diff --git a/packages/models-library/src/models_library/api_schemas_webserver/catalog.py b/packages/models-library/src/models_library/api_schemas_webserver/catalog.py index 1391ce19ce3..df48d45cdbf 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/catalog.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/catalog.py @@ -1,4 +1,4 @@ -from typing import Any, TypeAlias +from typing import Annotated, Any, TypeAlias from pydantic import ConfigDict, Field from pydantic.config import JsonDict @@ -32,9 +32,9 @@ class _BaseCommonApiExtension(BaseModel): class ServiceInputGet(ServiceInput, _BaseCommonApiExtension): """Extends fields of api_schemas_catalog.services.ServiceGet.outputs[*]""" - key_id: ServiceInputKey = Field( - ..., description="Unique name identifier for this input" - ) + key_id: Annotated[ + ServiceInputKey, Field(description="Unique name identifier for this input") + ] @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: @@ -78,9 +78,9 @@ def _update_json_schema_extra(schema: JsonDict) -> None: class ServiceOutputGet(ServiceOutput, _BaseCommonApiExtension): """Extends fields of api_schemas_catalog.services.ServiceGet.outputs[*]""" - key_id: ServiceOutputKey = Field( - ..., description="Unique name identifier for this input" - ) + key_id: Annotated[ + ServiceOutputKey, Field(description="Unique name identifier for this input") + ] @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: @@ -227,12 +227,12 @@ def _update_json_schema_extra(schema: JsonDict) -> None: class ServiceGet(api_schemas_catalog_services.ServiceGet): # pylint: disable=too-many-ancestors - inputs: ServiceInputsGetDict = Field( # type: ignore[assignment] - ..., description="inputs with extended information" - ) - outputs: ServiceOutputsGetDict = Field( # type: ignore[assignment] - ..., description="outputs with extended information" - ) + inputs: Annotated[ + ServiceInputsGetDict, Field(description="inputs with extended information") + ] + outputs: Annotated[ + ServiceOutputsGetDict, Field(description="outputs with extended information") + ] @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: @@ -247,16 +247,21 @@ def _update_json_schema_extra(schema: JsonDict) -> None: ServiceResourcesGet: TypeAlias = api_schemas_catalog_services.ServiceResourcesGet +class CatalogServiceListItem(api_schemas_catalog_services.ServiceListItem): + inputs: ServiceInputsGetDict + outputs: ServiceOutputsGetDict + + class CatalogServiceGet(api_schemas_catalog_services.ServiceGetV2): # NOTE: will replace ServiceGet! # pylint: disable=too-many-ancestors - inputs: ServiceInputsGetDict = Field( # type: ignore[assignment] - ..., description="inputs with extended information" - ) - outputs: ServiceOutputsGetDict = Field( # type: ignore[assignment] - ..., description="outputs with extended information" - ) + inputs: Annotated[ + ServiceInputsGetDict, Field(description="inputs with extended information") + ] + outputs: Annotated[ + ServiceOutputsGetDict, Field(description="outputs with extended information") + ] @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: 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 c312bea3b5a..ae137dcd1b9 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,7 +7,7 @@ from models_library.api_schemas_catalog.services import ( MyServiceGet, ServiceGetV2, - ServiceItemList, + ServiceListItem, ServiceUpdateV2, ) from models_library.products import ProductName @@ -35,7 +35,7 @@ async def list_services_paginated( # pylint: disable=too-many-arguments user_id: UserID, limit: PageLimitInt = DEFAULT_NUMBER_OF_ITEMS_PER_PAGE, offset: NonNegativeInt = 0, -) -> PageRpc[ServiceItemList]: +) -> PageRpc[ServiceListItem]: """ Raises: ValidationError: on invalid arguments @@ -63,9 +63,9 @@ async def _call( product_name=product_name, user_id=user_id, limit=limit, offset=offset ) assert ( # nosec - TypeAdapter(PageRpc[ServiceItemList]).validate_python(result) is not None + TypeAdapter(PageRpc[ServiceListItem]).validate_python(result) is not None ) - return cast(PageRpc[ServiceItemList], result) + return cast(PageRpc[ServiceListItem], result) @log_decorator(_logger, level=logging.DEBUG) 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 9f87efe16ca..d0f50e348f6 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -3,7 +3,7 @@ from models_library.api_schemas_catalog.services import ( MyServiceGet, ServiceGetV2, - ServiceItemList, + ServiceListItem, ServiceUpdateV2, ) from models_library.groups import GroupID @@ -89,7 +89,7 @@ async def list_services_paginated( user_id: UserID, limit: PageLimitInt | None, offset: NonNegativeInt = 0, -) -> tuple[NonNegativeInt, list[ServiceItemList]]: +) -> tuple[NonNegativeInt, list[ServiceListItem]]: # defines the order total_count, services = await repo.list_latest_services( @@ -144,7 +144,7 @@ async def list_services_paginated( ] return total_count, [ - ServiceItemList.model_validate( + ServiceListItem.model_validate( { **it.model_dump(exclude_unset=True, by_alias=True), } From b3aed7d89e3b5e5822d48935e577209dadd23673 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 14:37:45 +0100 Subject: [PATCH 20/39] minor oas changes --- .../api/v0/openapi.yaml | 123 +++++++++++++++++- 1 file changed, 119 insertions(+), 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index d734d72c125..436ffeceb40 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -2027,7 +2027,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Page_CatalogServiceGet_' + $ref: '#/components/schemas/Page_CatalogServiceListItem_' /v0/catalog/services/{service_key}/{service_version}: get: tags: @@ -8034,6 +8034,121 @@ components: type: computational version: 2.2.1 version_display: 2 Xtreme + CatalogServiceListItem: + properties: + key: + type: string + pattern: ^simcore/services/((comp|dynamic|frontend))/([a-z0-9][a-z0-9_.-]*/)*([a-z0-9-_]+[a-z0-9])$ + title: Key + version: + type: string + pattern: ^(0|[1-9]\d*)(\.(0|[1-9]\d*)){2}(-(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*)(\.(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*))*)?(\+[-\da-zA-Z]+(\.[-\da-zA-Z-]+)*)?$ + title: Version + name: + type: string + title: Name + thumbnail: + anyOf: + - type: string + - type: 'null' + title: Thumbnail + icon: + anyOf: + - type: string + - type: 'null' + title: Icon + description: + type: string + title: Description + descriptionUi: + type: boolean + title: Descriptionui + default: false + versionDisplay: + anyOf: + - type: string + - type: 'null' + title: Versiondisplay + type: + $ref: '#/components/schemas/ServiceType' + contact: + anyOf: + - type: string + format: email + - type: 'null' + title: Contact + authors: + items: + $ref: '#/components/schemas/Author' + type: array + minItems: 1 + title: Authors + owner: + anyOf: + - type: string + format: email + - type: 'null' + title: Owner + description: None when the owner email cannot be found in the database + inputs: + type: object + title: Inputs + outputs: + type: object + title: Outputs + bootOptions: + anyOf: + - type: object + - type: 'null' + title: Bootoptions + minVisibleInputs: + anyOf: + - type: integer + minimum: 0 + - type: 'null' + title: Minvisibleinputs + accessRights: + anyOf: + - additionalProperties: + $ref: '#/components/schemas/ServiceGroupAccessRightsV2' + type: object + - type: 'null' + title: Accessrights + classifiers: + anyOf: + - items: + type: string + type: array + - type: 'null' + title: Classifiers + default: [] + quality: + type: object + title: Quality + default: {} + history: + items: + $ref: '#/components/schemas/ServiceRelease' + type: array + title: History + description: History will be replaced by current 'release' instead + default: [] + deprecated: true + additionalProperties: false + type: object + required: + - key + - version + - name + - description + - type + - contact + - authors + - owner + - inputs + - outputs + - accessRights + title: CatalogServiceListItem CatalogServiceUpdate: properties: name: @@ -12208,7 +12323,7 @@ components: - total - count title: PageMetaInfoLimitOffset - Page_CatalogServiceGet_: + Page_CatalogServiceListItem_: properties: _meta: $ref: '#/components/schemas/PageMetaInfoLimitOffset' @@ -12216,7 +12331,7 @@ components: $ref: '#/components/schemas/PageLinks' data: items: - $ref: '#/components/schemas/CatalogServiceGet' + $ref: '#/components/schemas/CatalogServiceListItem' type: array title: Data additionalProperties: false @@ -12225,7 +12340,7 @@ components: - _meta - _links - data - title: Page[CatalogServiceGet] + title: Page[CatalogServiceListItem] Page_LicensedItemPurchaseGet_: properties: _meta: From 52319ab362f546b470ea2174da90e1b233d867de Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 15:11:31 +0100 Subject: [PATCH 21/39] adds get_project_services handle --- api/specs/web-server/_projects_nodes.py | 2 - .../api/v0/openapi.yaml | 73 +++++++++++++++++++ .../simcore_service_webserver/catalog/_api.py | 18 ++++- .../catalog/catalog_service.py | 5 ++ .../projects/_nodes_handlers.py | 43 +++++++++-- 5 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 services/web/server/src/simcore_service_webserver/catalog/catalog_service.py diff --git a/api/specs/web-server/_projects_nodes.py b/api/specs/web-server/_projects_nodes.py index 9ad716b77cd..61650993ab7 100644 --- a/api/specs/web-server/_projects_nodes.py +++ b/api/specs/web-server/_projects_nodes.py @@ -158,8 +158,6 @@ def replace_node_resources( @router.get( "/projects/{project_id}/nodes/-/services", response_model=Envelope[ProjectNodeServicesGet], - # NOTE: will be activated on the follow up from https://github.com/ITISFoundation/osparc-simcore/pull/7287 - include_in_schema=False, ) async def get_project_services(project_id: ProjectID): ... diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 436ffeceb40..3445894902b 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -4927,6 +4927,28 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_dict_Annotated_str__StringConstraints___ImageResources__' + /v0/projects/{project_id}/nodes/-/services: + get: + tags: + - projects + - nodes + summary: Get Project Services + operationId: get_project_services + parameters: + - name: project_id + in: path + required: true + schema: + type: string + format: uuid + title: Project Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/Envelope_ProjectNodeServicesGet_' /v0/projects/{project_id}/nodes/-/services:access: get: tags: @@ -9222,6 +9244,19 @@ components: title: Error type: object title: Envelope[ProjectMetadataGet] + Envelope_ProjectNodeServicesGet_: + properties: + data: + anyOf: + - $ref: '#/components/schemas/ProjectNodeServicesGet' + - type: 'null' + error: + anyOf: + - {} + - type: 'null' + title: Error + type: object + title: Envelope[ProjectNodeServicesGet] Envelope_ProjectState_: properties: data: @@ -12177,6 +12212,28 @@ components: - thumbnail_url - file_url title: NodeScreenshot + NodeServiceGet: + properties: + key: + type: string + pattern: ^simcore/services/((comp|dynamic|frontend))/([a-z0-9][a-z0-9_.-]*/)*([a-z0-9-_]+[a-z0-9])$ + title: Key + release: + $ref: '#/components/schemas/ServiceRelease' + owner: + type: integer + exclusiveMinimum: true + title: Owner + minimum: 0 + myAccessRights: + $ref: '#/components/schemas/AccessRights' + type: object + required: + - key + - release + - owner + - myAccessRights + title: NodeServiceGet NodeState: properties: modified: @@ -13665,6 +13722,22 @@ components: required: - custom title: ProjectMetadataUpdate + ProjectNodeServicesGet: + properties: + projectUuid: + type: string + format: uuid + title: Projectuuid + services: + items: + $ref: '#/components/schemas/NodeServiceGet' + type: array + title: Services + type: object + required: + - projectUuid + - services + title: ProjectNodeServicesGet ProjectOutputGet: properties: key: diff --git a/services/web/server/src/simcore_service_webserver/catalog/_api.py b/services/web/server/src/simcore_service_webserver/catalog/_api.py index 2c85d4007e6..5e8256a0c4c 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_api.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_api.py @@ -4,7 +4,7 @@ from aiohttp import web from aiohttp.web import Request -from models_library.api_schemas_catalog.services import ServiceUpdateV2 +from models_library.api_schemas_catalog.services import MyServiceGet, ServiceUpdateV2 from models_library.api_schemas_webserver.catalog import ( ServiceInputGet, ServiceInputKey, @@ -110,6 +110,22 @@ async def list_latest_services( return page_data, page.meta +async def batch_get_my_services( + app: web.Application, + *, + user_id: UserID, + product_name: ProductName, + services_ids: list[tuple[ServiceKey, ServiceVersion]], +) -> list[MyServiceGet]: + + return await catalog_rpc.batch_get_my_services( + get_rabbitmq_rpc_client(app), + user_id=user_id, + product_name=product_name, + ids=services_ids, + ) + + async def get_service_v2( app: web.Application, *, diff --git a/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py b/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py new file mode 100644 index 00000000000..e0dab71aaf2 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/catalog/catalog_service.py @@ -0,0 +1,5 @@ +from ._api import batch_get_my_services + +__all__: tuple[str, ...] = ("batch_get_my_services",) + +# nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py index 28fcb974511..bd52e446ff0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py @@ -1,6 +1,4 @@ -""" Handlers for CRUD operations on /projects/{*}/nodes/{*} - -""" +"""Handlers for CRUD operations on /projects/{*}/nodes/{*}""" import asyncio import logging @@ -10,6 +8,7 @@ from models_library.api_schemas_catalog.service_access_rights import ( ServiceAccessRightsGet, ) +from models_library.api_schemas_catalog.services import MyServiceGet from models_library.api_schemas_directorv2.dynamic_services import DynamicServiceGet from models_library.api_schemas_dynamic_scheduler.dynamic_services import ( DynamicServiceStop, @@ -23,12 +22,15 @@ NodeOutputs, NodePatch, NodeRetrieve, + NodeServiceGet, + ProjectNodeServicesGet, ) from models_library.groups import EVERYONE_GROUP_ID, Group, GroupID, GroupType from models_library.projects import Project, ProjectID from models_library.projects_nodes_io import NodeID, NodeIDStr from models_library.services import ServiceKeyVersion from models_library.services_resources import ServiceResourcesDict +from models_library.services_types import ServiceKey, ServiceVersion from models_library.utils.fastapi_encoders import jsonable_encoder from pydantic import BaseModel, Field from servicelib.aiohttp import status @@ -55,6 +57,7 @@ from simcore_postgres_database.models.users import UserRole from .._meta import API_VTAG as VTAG +from ..catalog import catalog_service from ..catalog import client as catalog_client from ..dynamic_scheduler import api as dynamic_scheduler_api from ..groups.api import get_group_from_gid, list_all_user_groups_ids @@ -464,6 +467,36 @@ class _ProjectGroupAccess(BaseModel): inaccessible_services: list[ServiceKeyVersion] | None = Field(default=None) +@routes.get( + f"/{VTAG}/projects/{{project_id}}/nodes/-/services", + name="get_project_services_access_for_gid", +) +@login_required +@permission_required("project.read") +@handle_plugin_requests_exceptions +async def get_project_services(request: web.Request) -> web.Response: + req_ctx = RequestContext.model_validate(request) + path_params = parse_request_path_parameters_as(ProjectPathParams, request) + + services_in_project: list[tuple[ServiceKey, ServiceVersion]] = [] + # TODO: call projects repository and get all services in the projects' node + # and fill services_in_project + + services: list[MyServiceGet] = await catalog_service.batch_get_my_services( + request.app, user_id=req_ctx.user_id, services_ids=services_in_project + ) + + return envelope_json_response( + ProjectNodeServicesGet( + project_uuid=path_params.project_id, + services=[ + NodeServiceGet.model_validate(sv, from_attributes=True) + for sv in services + ], + ) + ) + + @routes.get( f"/{VTAG}/projects/{{project_id}}/nodes/-/services:access", name="get_project_services_access_for_gid", @@ -471,9 +504,7 @@ class _ProjectGroupAccess(BaseModel): @login_required @permission_required("project.read") @handle_plugin_requests_exceptions -async def get_project_services_access_for_gid( - request: web.Request, -) -> web.Response: +async def get_project_services_access_for_gid(request: web.Request) -> web.Response: req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) query_params: _ServicesAccessQuery = parse_request_query_parameters_as( From 99e999c63b4c75cf1fded9fe84af8f9bdb4123a6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 15:19:55 +0100 Subject: [PATCH 22/39] drafts test --- .../src/models_library/access_rights.py | 15 ++- .../api_schemas_webserver/projects_nodes.py | 10 +- .../projects/_nodes_handlers.py | 7 +- .../server/tests/unit/with_dbs/02/conftest.py | 6 +- ...rojects_nodes_handlers__services_access.py | 105 ++++++++++++++++-- 5 files changed, 122 insertions(+), 21 deletions(-) diff --git a/packages/models-library/src/models_library/access_rights.py b/packages/models-library/src/models_library/access_rights.py index a6cea15a946..a78ba105ac8 100644 --- a/packages/models-library/src/models_library/access_rights.py +++ b/packages/models-library/src/models_library/access_rights.py @@ -1,9 +1,18 @@ +from typing import Annotated + from pydantic import BaseModel, ConfigDict, Field class AccessRights(BaseModel): - read: bool = Field(..., description="has read access") - write: bool = Field(..., description="has write access") - delete: bool = Field(..., description="has deletion rights") + read: Annotated[bool, Field(description="has read access")] + write: Annotated[bool, Field(description="has write access")] + delete: Annotated[bool, Field(description="has deletion rights")] + + model_config = ConfigDict(extra="forbid") + + +class ExecutableAccessRights(BaseModel): + write: Annotated[bool, Field(description="can change executable settings")] + execute: Annotated[bool, Field(description="can run executable")] model_config = ConfigDict(extra="forbid") diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py b/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py index f21c7688936..3932451cee5 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py @@ -6,7 +6,7 @@ from models_library.services_history import ServiceRelease from pydantic import ConfigDict, Field -from ..access_rights import AccessRights +from ..access_rights import ExecutableAccessRights from ..api_schemas_directorv2.dynamic_services import RetrieveDataOut from ..basic_types import PortInt from ..projects_nodes import InputID, InputsDict, PartialNode @@ -59,9 +59,9 @@ class NodePatch(InputSchemaWithoutCamelCase): ), ] = None boot_options: Annotated[BootOptions | None, Field(alias="bootOptions")] = None - outputs: dict[ - str, Any - ] | None = None # NOTE: it is used by frontend for File Picker + outputs: dict[str, Any] | None = ( + None # NOTE: it is used by frontend for File Picker + ) def to_domain_model(self) -> PartialNode: data = self.model_dump( @@ -207,7 +207,7 @@ class NodeServiceGet(OutputSchema): key: ServiceKey release: ServiceRelease owner: GroupID - my_access_rights: AccessRights + my_access_rights: ExecutableAccessRights class ProjectNodeServicesGet(OutputSchema): diff --git a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py index bd52e446ff0..780d87bcf52 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py @@ -469,7 +469,7 @@ class _ProjectGroupAccess(BaseModel): @routes.get( f"/{VTAG}/projects/{{project_id}}/nodes/-/services", - name="get_project_services_access_for_gid", + name="get_project_services", ) @login_required @permission_required("project.read") @@ -483,7 +483,10 @@ async def get_project_services(request: web.Request) -> web.Response: # and fill services_in_project services: list[MyServiceGet] = await catalog_service.batch_get_my_services( - request.app, user_id=req_ctx.user_id, services_ids=services_in_project + request.app, + product_name=req_ctx.product_name, + user_id=req_ctx.user_id, + services_ids=services_in_project, ) return envelope_json_response( diff --git a/services/web/server/tests/unit/with_dbs/02/conftest.py b/services/web/server/tests/unit/with_dbs/02/conftest.py index 25be7db87c8..2bd29316680 100644 --- a/services/web/server/tests/unit/with_dbs/02/conftest.py +++ b/services/web/server/tests/unit/with_dbs/02/conftest.py @@ -107,8 +107,8 @@ def mock_catalog_api( @pytest.fixture async def user_project( client: TestClient, - fake_project, - logged_user, + fake_project: ProjectDict, + logged_user: UserInfoDict, tests_data_dir: Path, osparc_product_name: str, ) -> AsyncIterator[ProjectDict]: @@ -223,7 +223,7 @@ async def _creator(**prj_kwargs) -> ProjectDict: @pytest.fixture def fake_services( - create_dynamic_service_mock: Callable[..., Awaitable[DynamicServiceGet]] + create_dynamic_service_mock: Callable[..., Awaitable[DynamicServiceGet]], ) -> Callable[..., Awaitable[list[DynamicServiceGet]]]: async def create_fakes(number_services: int) -> list[DynamicServiceGet]: return [await create_dynamic_service_mock() for _ in range(number_services)] diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py b/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py index 3e8a4d9e2b4..aa52a39681f 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py @@ -13,8 +13,11 @@ from models_library.api_schemas_catalog.service_access_rights import ( ServiceAccessRightsGet, ) +from models_library.api_schemas_catalog.services import MyServiceGet +from models_library.services_history import ServiceRelease from pytest_mock import MockerFixture from pytest_simcore.helpers.assert_checks import assert_status +from pytest_simcore.helpers.webserver_login import UserInfoDict from servicelib.aiohttp import status from simcore_service_webserver.db.models import UserRole from simcore_service_webserver.projects.models import ProjectDict @@ -95,9 +98,9 @@ def mock_catalog_api_get_service_access_rights_response(mocker: MockerFixture): async def test_user_role_access( client: TestClient, user_project: ProjectDict, - logged_user: dict, + logged_user: UserInfoDict, expected: HTTPStatus, - mock_catalog_api_get_service_access_rights_response, + mock_catalog_api_get_service_access_rights_response: None, ): assert client.app @@ -123,7 +126,7 @@ async def test_accessible_thanks_to_everyone_group_id( client: TestClient, user_project: ProjectDict, mocker: MockerFixture, - logged_user: dict, + logged_user: UserInfoDict, ): mocker.patch( "simcore_service_webserver.projects._nodes_handlers.catalog_client.get_service_access_rights", @@ -176,7 +179,7 @@ async def test_accessible_thanks_to_concrete_group_id( client: TestClient, user_project: ProjectDict, mocker: MockerFixture, - logged_user: dict, + logged_user: UserInfoDict, ): for_gid = logged_user["primary_gid"] @@ -229,7 +232,7 @@ async def test_accessible_through_product_group( client: TestClient, user_project: ProjectDict, mocker: MockerFixture, - logged_user: dict, + logged_user: UserInfoDict, ): for_gid = logged_user["primary_gid"] @@ -288,7 +291,7 @@ async def test_accessible_for_one_service( client: TestClient, user_project: ProjectDict, mocker: MockerFixture, - logged_user: dict, + logged_user: UserInfoDict, ): for_gid = logged_user["primary_gid"] @@ -348,7 +351,7 @@ async def test_not_accessible_for_more_services( client: TestClient, user_project: ProjectDict, mocker: MockerFixture, - logged_user: dict, + logged_user: UserInfoDict, ): mocker.patch( "simcore_service_webserver.projects._nodes_handlers.catalog_client.get_service_access_rights", @@ -412,7 +415,7 @@ async def test_not_accessible_for_service_because_of_execute_access_false( client: TestClient, user_project: ProjectDict, mocker: MockerFixture, - logged_user: dict, + logged_user: UserInfoDict, ): for_gid = logged_user["primary_gid"] @@ -461,3 +464,89 @@ async def test_not_accessible_for_service_because_of_execute_access_false( {"key": "simcore/services/comp/itis/sleeper", "version": "2.1.4"} ], } + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_get_project_services( + client: TestClient, + user_project: ProjectDict, + mocker: MockerFixture, + logged_user: UserInfoDict, +): + fake_services_in_project = [ + (sv["key"], sv["version"]) for sv in user_project["workbench"].values() + ] + + mocker.patch( + "simcore_service_webserver.catalog._api.catalog_rpc.batch_get_my_services", + spec=True, + return_value=[ + MyServiceGet( + key=service_key, + release=ServiceRelease( + version=service_version, + version_display=f"v{service_version}", + released="2023-01-01T00:00:00Z", + retired=None, + compatibility=None, + ), + owner=logged_user["primary_gid"], + my_access_rights={"execute": True, "write": False}, + ) + for service_key, service_version in fake_services_in_project + ], + ) + + assert client.app + + project_id = user_project["uuid"] + + expected_url = client.app.router["get_project_services"].url_for( + project_id=project_id + ) + assert URL(f"/v0/projects/{project_id}/nodes/-/services") == expected_url + + resp = await client.get(f"/v0/projects/{project_id}/nodes/-/services") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + assert data == { + "projectUuid": project_id, + "services": [ + { + "key": "simcore/services/comp/itis/sleeper", + "myAccessRights": {"execute": True, "write": False}, + "owner": logged_user["primary_gid"], + "release": { + "compatibility": None, + "released": "2023-01-01T00:00:00+00:00", + "retired": None, + "version": "2.1.4", + "versionDisplay": "v2.1.4", + }, + }, + { + "key": "simcore/services/frontend/parameter/integer", + "myAccessRights": {"execute": True, "write": False}, + "owner": logged_user["primary_gid"], + "release": { + "compatibility": None, + "released": "2023-01-01T00:00:00+00:00", + "retired": None, + "version": "1.0.0", + "versionDisplay": "v1.0.0", + }, + }, + { + "key": "simcore/services/comp/itis/sleeper", + "myAccessRights": {"execute": True, "write": False}, + "owner": logged_user["primary_gid"], + "release": { + "compatibility": None, + "released": "2023-01-01T00:00:00+00:00", + "retired": None, + "version": "2.1.5", + "versionDisplay": "v2.1.5", + }, + }, + ], + } From ead3316cf8878327ecd7eb220a6c53845898bf8b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 16:28:57 +0100 Subject: [PATCH 23/39] implementing get_project_nodes_services --- .../projects/_nodes_api.py | 11 +++++++++++ .../projects/_nodes_handlers.py | 11 ++++++++--- .../projects/_nodes_repository.py | 18 ++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 services/web/server/src/simcore_service_webserver/projects/_nodes_repository.py diff --git a/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py b/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py index 6b4b8df3f90..e3b84bd9ff4 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py @@ -12,6 +12,7 @@ from models_library.projects import ProjectID from models_library.projects_nodes import Node from models_library.projects_nodes_io import NodeID, SimCoreFileLink +from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import ( BaseModel, @@ -22,10 +23,12 @@ ValidationError, model_validator, ) +from pytest_simcore.faker_projects_data import project_id from servicelib.utils import logged_gather from ..application_settings import get_application_settings from ..storage.api import get_download_link, get_files_in_node_folder +from . import _nodes_repository from .exceptions import ProjectStartsTooManyDynamicNodesError _logger = logging.getLogger(__name__) @@ -71,6 +74,14 @@ def get_total_project_dynamic_nodes_creation_interval( return max_nodes * _NODE_START_INTERVAL_S.total_seconds() +async def get_project_nodes_services( + app: web.Application, *, project_uuid: ProjectID +) -> list[tuple[ServiceKey, ServiceVersion]]: + return await _nodes_repository.get_project_nodes_services( + app, project_uuid=project_uuid + ) + + # # PREVIEWS # diff --git a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py index 780d87bcf52..030e1ec60a0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py @@ -67,6 +67,7 @@ from ..security.decorators import permission_required from ..users.api import get_user_id_from_gid, get_user_role from ..utils_aiohttp import envelope_json_response +from . import _nodes_api as _nodes_service from . import nodes_utils, projects_service from ._common.exceptions_handlers import handle_plugin_requests_exceptions from ._common.models import ProjectPathParams, RequestContext @@ -478,9 +479,13 @@ async def get_project_services(request: web.Request) -> web.Response: req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) - services_in_project: list[tuple[ServiceKey, ServiceVersion]] = [] - # TODO: call projects repository and get all services in the projects' node - # and fill services_in_project + # FIXME: check user_id has read access to this project! + + services_in_project: list[tuple[ServiceKey, ServiceVersion]] = ( + await _nodes_service.get_project_nodes_services( + request.app, project_uuid=path_params.project_id + ) + ) services: list[MyServiceGet] = await catalog_service.batch_get_my_services( request.app, diff --git a/services/web/server/src/simcore_service_webserver/projects/_nodes_repository.py b/services/web/server/src/simcore_service_webserver/projects/_nodes_repository.py new file mode 100644 index 00000000000..e5060360265 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/projects/_nodes_repository.py @@ -0,0 +1,18 @@ +from aiohttp import web +from models_library.projects import ProjectID +from models_library.services_types import ServiceKey, ServiceVersion +from simcore_postgres_database.utils_projects_nodes import ProjectNodesRepo + +from ..db.plugin import get_database_engine + + +async def get_project_nodes_services( + app: web.Application, *, project_uuid: ProjectID +) -> list[tuple[ServiceKey, ServiceVersion]]: + repo = ProjectNodesRepo(project_uuid=project_uuid) + + async with get_database_engine(app).acquire() as conn: + nodes = await repo.list(conn) + + # removes duplicates by preserving order + return list(dict.fromkeys((node.key, node.version) for node in nodes)) From a79eb6ddc26bbe313463b65a6e0eb02601a80698 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 17:00:32 +0100 Subject: [PATCH 24/39] adds test --- .../rabbitmq/rpc_interfaces/catalog/errors.py | 4 +++ .../simcore_service_webserver/catalog/_api.py | 20 +++++++---- .../projects/_common/exceptions_handlers.py | 5 +++ ...rojects_nodes_handlers__services_access.py | 34 +++++++++++++++++++ 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py index d278bb350ba..9357758f057 100644 --- a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py +++ b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py @@ -11,3 +11,7 @@ class CatalogItemNotFoundError(CatalogApiBaseError): class CatalogForbiddenError(CatalogApiBaseError): msg_template = "Insufficient access rights for {name}" + + +class CatalogServiceError(CatalogApiBaseError): + msg_template = "Catalog service failed unexpectedly" diff --git a/services/web/server/src/simcore_service_webserver/catalog/_api.py b/services/web/server/src/simcore_service_webserver/catalog/_api.py index 5e8256a0c4c..aa2ae3bb044 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_api.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_api.py @@ -24,7 +24,9 @@ from pint import UnitRegistry from pydantic import BaseModel, ConfigDict from servicelib.aiohttp.requests_validation import handle_validation_as_http_error +from servicelib.rabbitmq._errors import RPCServerError from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc +from servicelib.rabbitmq.rpc_interfaces.catalog.errors import CatalogServiceError from servicelib.rest_constants import RESPONSE_MODEL_POLICY from ..constants import RQ_PRODUCT_KEY, RQT_USERID_KEY @@ -117,13 +119,19 @@ async def batch_get_my_services( product_name: ProductName, services_ids: list[tuple[ServiceKey, ServiceVersion]], ) -> list[MyServiceGet]: + try: - return await catalog_rpc.batch_get_my_services( - get_rabbitmq_rpc_client(app), - user_id=user_id, - product_name=product_name, - ids=services_ids, - ) + return await catalog_rpc.batch_get_my_services( + get_rabbitmq_rpc_client(app), + user_id=user_id, + product_name=product_name, + ids=services_ids, + ) + except RPCServerError as err: + raise CatalogServiceError( + user_id=user_id, + product_name=product_name, + ) from err async def get_service_v2( diff --git a/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py index d9116350236..92331497b91 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py @@ -6,6 +6,7 @@ from servicelib.rabbitmq.rpc_interfaces.catalog.errors import ( CatalogForbiddenError, CatalogItemNotFoundError, + CatalogServiceError, ) from ...exception_handling import ( @@ -163,6 +164,10 @@ _OTHER_ERRORS: ExceptionToHttpErrorMap = { + CatalogServiceError: HttpErrorInfo( + status.HTTP_503_SERVICE_UNAVAILABLE, + "This service is currently not available [supportID:{error_code}]", + ), ClustersKeeperNotAvailableError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, "Clusters-keeper service is not available", diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py b/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py index aa52a39681f..cedb64451bb 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py @@ -19,6 +19,7 @@ from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.webserver_login import UserInfoDict from servicelib.aiohttp import status +from servicelib.rabbitmq import RPCServerError from simcore_service_webserver.db.models import UserRole from simcore_service_webserver.projects.models import ProjectDict from yarl import URL @@ -550,3 +551,36 @@ async def test_get_project_services( }, ], } + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_get_project_services_service_unavailable( + client: TestClient, + user_project: ProjectDict, + mocker: MockerFixture, + logged_user: UserInfoDict, +): + mocker.patch( + "simcore_service_webserver.catalog._api.catalog_rpc.batch_get_my_services", + spec=True, + side_effect=RPCServerError( + exc_message="Service Unavailable", + method_name="batch_get_my_services", + exc_type="Exception", + ), + ) + + assert client.app + + project_id = user_project["uuid"] + + expected_url = client.app.router["get_project_services"].url_for( + project_id=project_id + ) + assert URL(f"/v0/projects/{project_id}/nodes/-/services") == expected_url + + resp = await client.get(f"/v0/projects/{project_id}/nodes/-/services") + data, error = await assert_status(resp, status.HTTP_503_SERVICE_UNAVAILABLE) + + assert error + assert not data From 4c5c868549775168201eaff1689b6f011cf1af83 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 17:28:39 +0100 Subject: [PATCH 25/39] fixes mypy --- .../api_schemas_webserver/catalog.py | 29 +++---------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/catalog.py b/packages/models-library/src/models_library/api_schemas_webserver/catalog.py index df48d45cdbf..76fdf4ac06e 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/catalog.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/catalog.py @@ -225,41 +225,20 @@ def _update_json_schema_extra(schema: JsonDict) -> None: } -class ServiceGet(api_schemas_catalog_services.ServiceGet): - # pylint: disable=too-many-ancestors - inputs: Annotated[ - ServiceInputsGetDict, Field(description="inputs with extended information") - ] - outputs: Annotated[ - ServiceOutputsGetDict, Field(description="outputs with extended information") - ] - - @staticmethod - def _update_json_schema_extra(schema: JsonDict) -> None: - schema.update({"examples": [_EXAMPLE_FILEPICKER, _EXAMPLE_SLEEPER]}) - - model_config = ConfigDict( - **OutputSchema.model_config, - json_schema_extra=_update_json_schema_extra, - ) - - ServiceResourcesGet: TypeAlias = api_schemas_catalog_services.ServiceResourcesGet class CatalogServiceListItem(api_schemas_catalog_services.ServiceListItem): - inputs: ServiceInputsGetDict - outputs: ServiceOutputsGetDict + inputs: ServiceInputsGetDict # type: ignore[assignment] + outputs: ServiceOutputsGetDict # type: ignore[assignment] class CatalogServiceGet(api_schemas_catalog_services.ServiceGetV2): - # NOTE: will replace ServiceGet! - # pylint: disable=too-many-ancestors - inputs: Annotated[ + inputs: Annotated[ # type: ignore[assignment] ServiceInputsGetDict, Field(description="inputs with extended information") ] - outputs: Annotated[ + outputs: Annotated[ # type: ignore[assignment] ServiceOutputsGetDict, Field(description="outputs with extended information") ] From e663629c579503e47851d3de4a0b2b53cdc8e976 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 17:32:30 +0100 Subject: [PATCH 26/39] fixes pylint --- .../src/simcore_service_webserver/projects/_nodes_api.py | 1 - .../web/server/src/simcore_service_webserver/tags/errors.py | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py b/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py index e3b84bd9ff4..0206e1315cc 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/_nodes_api.py @@ -23,7 +23,6 @@ ValidationError, model_validator, ) -from pytest_simcore.faker_projects_data import project_id from servicelib.utils import logged_gather from ..application_settings import get_application_settings diff --git a/services/web/server/src/simcore_service_webserver/tags/errors.py b/services/web/server/src/simcore_service_webserver/tags/errors.py index 95fa3185972..579ed5ef125 100644 --- a/services/web/server/src/simcore_service_webserver/tags/errors.py +++ b/services/web/server/src/simcore_service_webserver/tags/errors.py @@ -1,8 +1,9 @@ +# pylint: disable=too-many-ancestors + from ..errors import WebServerBaseError -class TagsPermissionError(WebServerBaseError, PermissionError): - ... +class TagsPermissionError(WebServerBaseError, PermissionError): ... class ShareTagWithEveryoneNotAllowedError(TagsPermissionError): From a6f3051b63a316b3191825d37163d9e5bff0bea5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 17:38:56 +0100 Subject: [PATCH 27/39] fixes tests --- .../db/repositories/services.py | 2 +- .../tests/unit/with_dbs/test_api_rpc.py | 31 +++++++++++-------- 2 files changed, 19 insertions(+), 14 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 7592ad28aea..96433c83dd1 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -179,7 +179,7 @@ async def get_service( product_name: str | None = None, ) -> ServiceMetaDataDBGet | None: - query = sa.select(SERVICES_META_DATA_COLS) + query = sa.select(*SERVICES_META_DATA_COLS) if gids or execute_access or write_access: conditions = [ 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 0893109b386..65b6dd70b11 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rpc.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rpc.py @@ -405,6 +405,7 @@ async def test_rpc_batch_get_my_services( mocked_director_service_api: MockRouter, rpc_client: RabbitMQRPCClient, product_name: ProductName, + user: dict[str, Any], user_id: UserID, app: FastAPI, create_fake_service_data: Callable, @@ -413,7 +414,8 @@ async def test_rpc_batch_get_my_services( # Create fake services data service_key = "simcore/services/comp/test-batch-service" service_version_1 = "1.0.0" - service_version_2 = "2.0.0" + service_version_2 = "1.0.5" + other_service_key = "simcore/services/comp/other-batch-service" other_service_version = "1.0.0" @@ -442,10 +444,9 @@ async def test_rpc_batch_get_my_services( # Inject fake services into the database await services_db_tables_injector([fake_service_1, fake_service_2, fake_service_3]) - # Batch get my services + # Batch get my services: project with two, not three ids = [ (service_key, service_version_1), - (service_key, service_version_2), (other_service_key, other_service_version), ] @@ -456,19 +457,23 @@ async def test_rpc_batch_get_my_services( ids=ids, ) - assert len(my_services) == 3 + assert len(my_services) == 2 - # Check access rights + # Check access rights to all of them assert my_services[0].my_access_rights.model_dump() == { - "execute": False, - "write": False, + "execute": True, + "write": True, } + assert my_services[0].owner == user["primary_gid"] + assert my_services[0].key == service_key + assert my_services[0].release.version == service_version_1 + assert my_services[0].release.compatibility + assert ( + my_services[0].release.compatibility.can_update_to.version == service_version_2 + ) + assert my_services[1].my_access_rights.model_dump() == { "execute": True, - "write": False, - } - assert my_services[2].my_access_rights.model_dump() == { - "execute": False, - "write": False, + "write": True, } - assert my_services[2].owner is not None + assert my_services[1].owner == user["primary_gid"] From 02bb0faae5c225c1be02216f7fb2c4bc616ed5e8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 17:41:42 +0100 Subject: [PATCH 28/39] updates OAS --- .../api/v0/openapi.yaml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 3445894902b..d357763402c 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -10301,6 +10301,22 @@ components: - resource - field title: ErrorItemType + ExecutableAccessRights: + properties: + write: + type: boolean + title: Write + description: can change executable settings + execute: + type: boolean + title: Execute + description: can run executable + additionalProperties: false + type: object + required: + - write + - execute + title: ExecutableAccessRights FeaturesDict: properties: age: @@ -12226,7 +12242,7 @@ components: title: Owner minimum: 0 myAccessRights: - $ref: '#/components/schemas/AccessRights' + $ref: '#/components/schemas/ExecutableAccessRights' type: object required: - key From 323ab38684bff7ae886e1d4790a498ecad9798df Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 17:41:47 +0100 Subject: [PATCH 29/39] =?UTF-8?q?services/webserver=20api=20version:=200.6?= =?UTF-8?q?0.0=20=E2=86=92=200.61.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/web/server/VERSION | 2 +- services/web/server/setup.cfg | 2 +- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/VERSION b/services/web/server/VERSION index 7e750b4ebf3..0b09455034e 100644 --- a/services/web/server/VERSION +++ b/services/web/server/VERSION @@ -1 +1 @@ -0.60.0 +0.61.0 diff --git a/services/web/server/setup.cfg b/services/web/server/setup.cfg index 6012cf501a1..65b8a29b46b 100644 --- a/services/web/server/setup.cfg +++ b/services/web/server/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.60.0 +current_version = 0.61.0 commit = True message = services/webserver api version: {current_version} → {new_version} tag = False diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index d357763402c..7521151e57c 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -2,7 +2,7 @@ openapi: 3.1.0 info: title: simcore-service-webserver description: Main service with an interface (http-API & websockets) to the web front-end - version: 0.60.0 + version: 0.61.0 servers: - url: '' description: webserver From d1ae3d9f33ddb27f7afc69067274562eb0fc0a08 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 17:53:32 +0100 Subject: [PATCH 30/39] adds access check --- .../projects/_nodes_handlers.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py index 030e1ec60a0..986090a224a 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_nodes_handlers.py @@ -67,6 +67,7 @@ from ..security.decorators import permission_required from ..users.api import get_user_id_from_gid, get_user_role from ..utils_aiohttp import envelope_json_response +from . import _access_rights_api as access_rights_service from . import _nodes_api as _nodes_service from . import nodes_utils, projects_service from ._common.exceptions_handlers import handle_plugin_requests_exceptions @@ -479,7 +480,13 @@ async def get_project_services(request: web.Request) -> web.Response: req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request) - # FIXME: check user_id has read access to this project! + await access_rights_service.check_user_project_permission( + request.app, + product_name=req_ctx.product_name, + user_id=req_ctx.user_id, + project_id=path_params.project_id, + permission="read", + ) services_in_project: list[tuple[ServiceKey, ServiceVersion]] = ( await _nodes_service.get_project_nodes_services( From b31ab1fad0f46ea8341461433e3bd99ddb153c12 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 18:15:32 +0100 Subject: [PATCH 31/39] accepts no ownership --- .../api_schemas_catalog/services.py | 2 +- .../api_schemas_webserver/projects_nodes.py | 7 ++++++- .../services/services_api.py | 15 ++++++++------- .../projects/_common/exceptions_handlers.py | 2 +- 4 files changed, 16 insertions(+), 10 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 fd522655ded..9e57924808b 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 @@ -340,5 +340,5 @@ class MyServiceGet(CatalogOutputSchema): key: ServiceKey release: ServiceRelease - owner: GroupID + owner: GroupID | None my_access_rights: ServiceGroupAccessRightsV2 diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py b/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py index 3932451cee5..a8932553201 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects_nodes.py @@ -206,7 +206,12 @@ class NodeRetrieved(RetrieveDataOut): class NodeServiceGet(OutputSchema): key: ServiceKey release: ServiceRelease - owner: GroupID + owner: Annotated[ + GroupID | None, + Field( + description="Service owner primary group id or None if ownership still not defined" + ), + ] my_access_rights: ExecutableAccessRights 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 d0f50e348f6..7f312846b12 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -1,4 +1,5 @@ import logging +from contextlib import suppress from models_library.api_schemas_catalog.services import ( MyServiceGet, @@ -392,16 +393,16 @@ async def batch_get_my_services( ) assert service_db # nosec - # Find service owner + # Find service owner (if defined!) owner: GroupID | None = service_db.owner if not owner: # NOTE can be more than one. Just get first. - owner = next( - ar.gid for ar in access_rights if ar.write_access and ar.execute_access - ) - - # TODO: raise error to indicate that no owner is registered for a given service - assert owner is not None # nosec + with suppress(StopIteration): + owner = next( + ar.gid + for ar in access_rights + if ar.write_access and ar.execute_access + ) # Evaluate `compatibility` compatibility: Compatibility | None = None diff --git a/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py index 92331497b91..d477dbb75e0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py @@ -166,7 +166,7 @@ _OTHER_ERRORS: ExceptionToHttpErrorMap = { CatalogServiceError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - "This service is currently not available [supportID:{error_code}]", + "This service is currently not available", ), ClustersKeeperNotAvailableError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, From f5c9a2823d758e36f3f1c4c3485245d156d2c410 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 18:16:09 +0100 Subject: [PATCH 32/39] updates OAS --- .../src/simcore_service_webserver/api/v0/openapi.yaml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 7521151e57c..75c6801d132 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -12237,10 +12237,14 @@ components: release: $ref: '#/components/schemas/ServiceRelease' owner: - type: integer - exclusiveMinimum: true + anyOf: + - type: integer + exclusiveMinimum: true + minimum: 0 + - type: 'null' title: Owner - minimum: 0 + description: Service owner primary group id or None if ownership still not + defined myAccessRights: $ref: '#/components/schemas/ExecutableAccessRights' type: object From 31d0f115b3b82921ceb640e58a82f80b0be50833 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 19:06:16 +0100 Subject: [PATCH 33/39] cleanup --- .../simcore_service_catalog/services/services_api.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 7f312846b12..11f8d57644a 100644 --- a/services/catalog/src/simcore_service_catalog/services/services_api.py +++ b/services/catalog/src/simcore_service_catalog/services/services_api.py @@ -407,12 +407,12 @@ async def batch_get_my_services( # Evaluate `compatibility` compatibility: Compatibility | None = None if my_access_rights.execute or my_access_rights.write: - # TODO: add cache to this section that evals compatibility_map based on service_key - - # NOTE: that the service history might be different for each user - # since access rights are defined on a k:v basis history = await repo.get_service_history( - product_name=product_name, user_id=user_id, key=service_key + # 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, ) assert history # nosec @@ -422,6 +422,7 @@ async def batch_get_my_services( user_id=user_id, service_release_history=history, ) + compatibility = compatibility_map.get(service_db.version) my_services.append( From ea7a0122530be6e68aae49060b7772a681067b39 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 19:30:42 +0100 Subject: [PATCH 34/39] extends test --- .../helpers/catalog_services.py | 21 ++++++++++-- .../with_dbs/test_services_services_api.py | 32 ++++++++++++------- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py index 6268931cd23..ffe4009b761 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py @@ -5,7 +5,7 @@ # pylint: disable=unused-variable from datetime import datetime -from typing import Protocol +from typing import Any, Protocol from models_library.products import ProductName @@ -21,5 +21,22 @@ def __call__( everyone_access: str | None = None, product: ProductName = "osparc", deprecated: datetime | None = None, # DB column - ): + ) -> tuple[dict[str, Any], ...]: + """ + Returns a fake factory that creates catalog DATA that can be used to fill + both services_meta_data and services_access_rights tables + + + Example: + fake_service, *fake_access_rights = create_fake_service_data( + "simcore/services/dynamic/jupyterlab", + "0.0.1", + team_access="xw", + everyone_access="x", + product=target_product, + ), + + owner_access, team_access, everyone_access = fake_access_rights + + """ ... diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index 94b8b3b514d..f534898caec 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -167,21 +167,23 @@ async def test_batch_get_my_services( groups_repo: GroupsRepository, user_id: UserID, user: dict[str, Any], + other_user: dict[str, Any], create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, ): # catalog service_key = "simcore/services/comp/some-service" service_version_1 = "1.0.0" # can upgrade to 1.0.1 - service_version_2 = "1.0.1" # latest + service_version_2 = "1.0.10" # latest other_service_key = "simcore/services/comp/other-service" - other_service_version = "2.0.0" + other_service_version = "2.1.2" expected_retirement = datetime.utcnow() + timedelta( days=1 ) # NOTE: old offset-naive column + # Owned by user fake_service_1 = create_fake_service_data( service_key, service_version_1, @@ -197,6 +199,8 @@ async def test_batch_get_my_services( everyone_access=None, product=target_product, ) + + # Owned by other-user fake_service_3 = create_fake_service_data( other_service_key, other_service_version, @@ -204,10 +208,15 @@ async def test_batch_get_my_services( everyone_access=None, product=target_product, ) + _service, _owner_access = fake_service_3 + _service["owner"] = other_user["primary_gid"] + _owner_access["gid"] = other_user["primary_gid"] # Inject fake services into the database await services_db_tables_injector([fake_service_1, fake_service_2, fake_service_3]) + # UNDER TEST ------------------------------- + # Batch get services e.g. services in a project services_ids = [ (service_key, service_version_1), @@ -222,6 +231,8 @@ async def test_batch_get_my_services( ids=services_ids, ) + # CHECKS ------------------------------- + # assert returned order and length as ids assert services_ids == [(sc.key, sc.release.version) for sc in my_services] @@ -230,12 +241,12 @@ async def test_batch_get_my_services( { "key": "simcore/services/comp/some-service", "release": { - "version": "1.0.0", + "version": service_version_1, "version_display": None, "released": my_services[0].release.released, "retired": expected_retirement, "compatibility": { - "can_update_to": {"version": "1.0.1"} + "can_update_to": {"version": service_version_2} }, # can be updated }, "owner": user["primary_gid"], @@ -244,18 +255,17 @@ async def test_batch_get_my_services( { "key": "simcore/services/comp/other-service", "release": { - "version": "2.0.0", + "version": other_service_version, "version_display": None, "released": my_services[1].release.released, "retired": None, "compatibility": None, # cannot be updated }, - "owner": user["primary_gid"], - "my_access_rights": {"execute": True, "write": True}, # full access + "owner": other_user["primary_gid"], # needs to request access + "my_access_rights": { + "execute": False, + "write": False, + }, }, ] ) - - # TODO: test user has no access to one and needs to ask owner (NOTE: owner can be a group? ) - # TODO: test user has no access to entire history - # From 818362284b10ff23b23e61ebf04c28f860c661d8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Feb 2025 20:14:02 +0100 Subject: [PATCH 35/39] fixes pylint --- .../catalog/tests/unit/with_dbs/test_services_services_api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/catalog/tests/unit/with_dbs/test_services_services_api.py b/services/catalog/tests/unit/with_dbs/test_services_services_api.py index f534898caec..a2d3c2551c7 100644 --- a/services/catalog/tests/unit/with_dbs/test_services_services_api.py +++ b/services/catalog/tests/unit/with_dbs/test_services_services_api.py @@ -1,6 +1,8 @@ # pylint: disable=redefined-outer-name # pylint: disable=unused-argument # pylint: disable=unused-variable +# pylint: disable=too-many-arguments + from collections.abc import Callable from datetime import datetime, timedelta From 056bedcf74225a4b3c970007ba10b779a54d6676 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 3 Mar 2025 09:32:38 +0100 Subject: [PATCH 36/39] fix pylint --- .../src/pytest_simcore/helpers/catalog_services.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py index ffe4009b761..90a2508111f 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_services.py @@ -21,7 +21,7 @@ def __call__( everyone_access: str | None = None, product: ProductName = "osparc", deprecated: datetime | None = None, # DB column - ) -> tuple[dict[str, Any], ...]: + ) -> tuple[dict[str, Any], ...]: # type: ignore """ Returns a fake factory that creates catalog DATA that can be used to fill both services_meta_data and services_access_rights tables @@ -39,4 +39,3 @@ def __call__( owner_access, team_access, everyone_access = fake_access_rights """ - ... From 0510b0b81ad075a5e662d7bdc00b9d87d835ed34 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 3 Mar 2025 09:34:05 +0100 Subject: [PATCH 37/39] fix sonar --- .../catalog/src/simcore_service_catalog/api/rpc/_services.py | 1 - 1 file changed, 1 deletion(-) 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 9926d8b45f4..6111398059d 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -185,7 +185,6 @@ async def batch_get_my_services( ) -> list[MyServiceGet]: assert app.state.engine # nosec - # TODO: id not found? services = await services_api.batch_get_my_services( repo=ServicesRepository(app.state.engine), groups_repo=GroupsRepository(app.state.engine), From c53cab9214026b5b5d1fea5d9a4edf6d2bfca311 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 3 Mar 2025 11:54:52 +0100 Subject: [PATCH 38/39] @GitHK review: validate on server side --- .../api/rpc/_services.py | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 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 6111398059d..6d95fbeb962 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py @@ -13,7 +13,7 @@ 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 +from pydantic import NonNegativeInt, ValidationError, validate_call from pyinstrument import Profiler from servicelib.logging_utils import log_decorator from servicelib.rabbitmq import RPCRouter @@ -53,8 +53,9 @@ async def _wrapper(app: FastAPI, **kwargs): return _wrapper -@router.expose(reraise_if_error_type=(CatalogForbiddenError,)) +@router.expose(reraise_if_error_type=(CatalogForbiddenError, ValidationError)) @_profile_rpc_call +@validate_call(config={"arbitrary_types_allowed": True}) async def list_services_paginated( app: FastAPI, *, @@ -88,9 +89,16 @@ async def list_services_paginated( ) -@router.expose(reraise_if_error_type=(CatalogItemNotFoundError, CatalogForbiddenError)) +@router.expose( + reraise_if_error_type=( + CatalogItemNotFoundError, + CatalogForbiddenError, + ValidationError, + ) +) @log_decorator(_logger, level=logging.DEBUG) @_profile_rpc_call +@validate_call(config={"arbitrary_types_allowed": True}) async def get_service( app: FastAPI, *, @@ -116,8 +124,15 @@ async def get_service( return service -@router.expose(reraise_if_error_type=(CatalogItemNotFoundError, CatalogForbiddenError)) +@router.expose( + reraise_if_error_type=( + CatalogItemNotFoundError, + CatalogForbiddenError, + ValidationError, + ) +) @log_decorator(_logger, level=logging.DEBUG) +@validate_call(config={"arbitrary_types_allowed": True}) async def update_service( app: FastAPI, *, @@ -147,8 +162,15 @@ async def update_service( return service -@router.expose(reraise_if_error_type=(CatalogItemNotFoundError, CatalogForbiddenError)) +@router.expose( + reraise_if_error_type=( + CatalogItemNotFoundError, + CatalogForbiddenError, + ValidationError, + ) +) @log_decorator(_logger, level=logging.DEBUG) +@validate_call(config={"arbitrary_types_allowed": True}) async def check_for_service( app: FastAPI, *, @@ -169,8 +191,9 @@ async def check_for_service( ) -@router.expose(reraise_if_error_type=(CatalogForbiddenError,)) +@router.expose(reraise_if_error_type=(CatalogForbiddenError, ValidationError)) @log_decorator(_logger, level=logging.DEBUG) +@validate_call(config={"arbitrary_types_allowed": True}) async def batch_get_my_services( app: FastAPI, *, From b4035d5d35f4330dbcec2acae17b131337d21099 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 4 Mar 2025 09:55:17 +0100 Subject: [PATCH 39/39] @sanderegg review: rename error --- .../src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py | 2 +- .../web/server/src/simcore_service_webserver/catalog/_api.py | 4 ++-- .../projects/_common/exceptions_handlers.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py index 9357758f057..be5e7c6c4e4 100644 --- a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py +++ b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/errors.py @@ -13,5 +13,5 @@ class CatalogForbiddenError(CatalogApiBaseError): msg_template = "Insufficient access rights for {name}" -class CatalogServiceError(CatalogApiBaseError): +class CatalogNotAvailableError(CatalogApiBaseError): msg_template = "Catalog service failed unexpectedly" diff --git a/services/web/server/src/simcore_service_webserver/catalog/_api.py b/services/web/server/src/simcore_service_webserver/catalog/_api.py index aa2ae3bb044..beec1366857 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_api.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_api.py @@ -26,7 +26,7 @@ from servicelib.aiohttp.requests_validation import handle_validation_as_http_error from servicelib.rabbitmq._errors import RPCServerError from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc -from servicelib.rabbitmq.rpc_interfaces.catalog.errors import CatalogServiceError +from servicelib.rabbitmq.rpc_interfaces.catalog.errors import CatalogNotAvailableError from servicelib.rest_constants import RESPONSE_MODEL_POLICY from ..constants import RQ_PRODUCT_KEY, RQT_USERID_KEY @@ -128,7 +128,7 @@ async def batch_get_my_services( ids=services_ids, ) except RPCServerError as err: - raise CatalogServiceError( + raise CatalogNotAvailableError( user_id=user_id, product_name=product_name, ) from err diff --git a/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py index d477dbb75e0..ad2db124670 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_common/exceptions_handlers.py @@ -6,7 +6,7 @@ from servicelib.rabbitmq.rpc_interfaces.catalog.errors import ( CatalogForbiddenError, CatalogItemNotFoundError, - CatalogServiceError, + CatalogNotAvailableError, ) from ...exception_handling import ( @@ -164,7 +164,7 @@ _OTHER_ERRORS: ExceptionToHttpErrorMap = { - CatalogServiceError: HttpErrorInfo( + CatalogNotAvailableError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, "This service is currently not available", ),