diff --git a/services/catalog/src/simcore_service_catalog/core/background_tasks.py b/services/catalog/src/simcore_service_catalog/core/background_tasks.py index 657f255bc85..ed5cd27240e 100644 --- a/services/catalog/src/simcore_service_catalog/core/background_tasks.py +++ b/services/catalog/src/simcore_service_catalog/core/background_tasks.py @@ -70,29 +70,36 @@ def _by_version(t: tuple[ServiceKey, ServiceVersion]) -> Version: (service_key, service_version) ] try: - ## Set deprecation date to null (is valid date value for postgres) - - # DEFAULT policies + # 1. Evaluate DEFAULT ownership and access rights ( owner_gid, service_access_rights, - ) = await access_rights.evaluate_default_policy(app, service_metadata) + ) = await access_rights.evaluate_default_service_ownership_and_rights( + app, + service=service_metadata, + product_name=app.state.default_product_name, + ) - # AUTO-UPGRADE PATCH policy - inherited_access_rights = await access_rights.evaluate_auto_upgrade_policy( - service_metadata, services_repo + # 2. Inherit access rights from the latest compatible release + inherited_data = await access_rights.inherit_from_latest_compatible_release( + service_metadata=service_metadata, + services_repo=services_repo, ) - service_access_rights += inherited_access_rights + # 3. Aggregates access rights and metadata updates + service_access_rights += inherited_data["access_rights"] service_access_rights = access_rights.reduce_access_rights( service_access_rights ) - # set the service in the DB + metadata_updates = { + **service_metadata.model_dump(exclude_unset=True), + **inherited_data["metadata_updates"], + } + + # 4. Upsert values in database await services_repo.create_or_update_service( - ServiceMetaDataDBCreate( - **service_metadata.model_dump(exclude_unset=True), owner=owner_gid - ), + ServiceMetaDataDBCreate(**metadata_updates, owner=owner_gid), service_access_rights, ) diff --git a/services/catalog/src/simcore_service_catalog/service/access_rights.py b/services/catalog/src/simcore_service_catalog/service/access_rights.py index 8e3adbeb429..43547319798 100644 --- a/services/catalog/src/simcore_service_catalog/service/access_rights.py +++ b/services/catalog/src/simcore_service_catalog/service/access_rights.py @@ -4,10 +4,12 @@ import operator from collections.abc import Callable from datetime import UTC, datetime -from typing import cast +from typing import Any, TypedDict, cast import arrow from fastapi import FastAPI +from models_library.groups import GroupID +from models_library.products import ProductName from models_library.services import ServiceMetaDataPublished from models_library.services_types import ServiceKey, ServiceVersion from packaging.version import Version @@ -15,7 +17,7 @@ from sqlalchemy.ext.asyncio import AsyncEngine from ..api._dependencies.director import get_director_client -from ..models.services_db import ServiceAccessRightsDB +from ..models.services_db import ServiceAccessRightsDB, ServiceMetaDataDBGet from ..repository.groups import GroupsRepository from ..repository.services import ServicesRepository from ..utils.versioning import as_version, is_patch_release @@ -25,6 +27,11 @@ _LEGACY_SERVICES_DATE: datetime = datetime(year=2020, month=8, day=19, tzinfo=UTC) +class InheritedData(TypedDict): + access_rights: list[ServiceAccessRightsDB] + metadata_updates: dict[str, Any] + + def _is_frontend_service(service: ServiceMetaDataPublished) -> bool: return "/frontend/" in service.key @@ -41,22 +48,28 @@ async def _is_old_service(app: FastAPI, service: ServiceMetaDataPublished) -> bo return bool(service_build_data < _LEGACY_SERVICES_DATE) -async def evaluate_default_policy( - app: FastAPI, service: ServiceMetaDataPublished -) -> tuple[PositiveInt | None, list[ServiceAccessRightsDB]]: - """Given a service, it returns the owner's group-id (gid) and a list of access rights following - default access-rights policies +async def evaluate_default_service_ownership_and_rights( + app: FastAPI, *, service: ServiceMetaDataPublished, product_name: ProductName +) -> tuple[GroupID | None, list[ServiceAccessRightsDB]]: + """Evaluates the owner (group_id) and the access rights for a service - - DEFAULT Access Rights policies: - 1. All services published in osparc prior 19.08.2020 will be visible to everyone (refered as 'old service'). - 2. Services published after 19.08.2020 will be visible ONLY to his/her owner - 3. Front-end services are have execute-access to everyone + This function determines: + 1. Who owns the service (based on contact or author email) + 2. Who can access the service based on the following rules: + - All services published before August 19, 2020 (_LEGACY_SERVICES_DATE) are accessible to everyone + - Services published after August 19, 2020 are only accessible to their owner + - Frontend services are accessible to everyone regardless of publication date + Returns: + A tuple containing: + - The owner's group ID (gid) if found, None otherwise + - A list of ServiceAccessRightsDB objects representing the default access rights + for the service, including who can execute and/or modify the service Raises: - HTTPException: from calls to director's rest API. Maps director errors into catalog's server error - SQLAlchemyError: from access to pg database - ValidationError: from pydantic model errors + HTTPException: If there's an error communicating with the director API + SQLAlchemyError: If there's an error accessing the database + ValidationError: If there's an error validating the Pydantic models """ db_engine: AsyncEngine = app.state.engine @@ -64,13 +77,17 @@ async def evaluate_default_policy( owner_gid = None group_ids: list[PositiveInt] = [] + # 1. If service is old or frontend, we add the everyone group if _is_frontend_service(service) or await _is_old_service(app, service): everyone_gid = (await groups_repo.get_everyone_group()).gid - _logger.debug("service %s:%s is old or frontend", service.key, service.version) - # let's make that one available to everyone - group_ids.append(everyone_gid) + group_ids.append(everyone_gid) # let's make that one available to everyone + _logger.debug( + "service %s:%s is old or frontend. Set available to everyone", + service.key, + service.version, + ) - # try to find the owner + # 2. Deducing the owner gid possible_owner_email = [service.contact] + [ author.email for author in service.authors ] @@ -80,19 +97,21 @@ async def evaluate_default_policy( if possible_gid and not owner_gid: owner_gid = possible_gid if not owner_gid: - _logger.warning("service %s:%s has no owner", service.key, service.version) + _logger.warning("Service %s:%s has no owner", service.key, service.version) else: group_ids.append(owner_gid) - # we add the owner with full rights, unless it's everyone + # 3. Aplying default access rights default_access_rights = [ ServiceAccessRightsDB( key=service.key, version=service.version, gid=gid, execute_access=True, - write_access=(gid == owner_gid), - product_name=app.state.default_product_name, + write_access=( + gid == owner_gid + ), # we add the owner with full rights, unless it's everyone + product_name=product_name, ) for gid in set(group_ids) ] @@ -100,58 +119,107 @@ async def evaluate_default_policy( return (owner_gid, default_access_rights) -async def evaluate_auto_upgrade_policy( - service_metadata: ServiceMetaDataPublished, services_repo: ServicesRepository -) -> list[ServiceAccessRightsDB]: - # AUTO-UPGRADE PATCH policy: - # - # - Any new patch released, inherits the access rights from previous compatible version - # - IDEA: add as option in the publication contract, i.e. in ServiceDockerData? - # - Does NOT apply to front-end services - # - # SEE https://github.com/ITISFoundation/osparc-simcore/issues/2244) - # +async def _find_latest_patch_compatible_release( + services_repo: ServicesRepository, *, service_metadata: ServiceMetaDataPublished +) -> ServiceMetaDataDBGet | None: + """ + Finds the previous patched release for a service. + + Args: + services_repo: Instance of ServicesRepository for database access. + service_metadata: Metadata of the service being evaluated. + + Returns: + Latest patch release of the service if it exists, otherwise None. + """ if _is_frontend_service(service_metadata): - return [] + return None - service_access_rights = [] new_version: Version = as_version(service_metadata.version) - latest_releases = await services_repo.list_service_releases( + patch_releases_latest_first = await services_repo.list_service_releases( service_metadata.key, major=new_version.major, minor=new_version.minor, ) - previous_release = None - for release in latest_releases: - # NOTE: latest_release is sorted from newer to older - # Here we search for the previous version patched by new-version + # latest_releases is sorted from newer to older + for release in patch_releases_latest_first: + # COMPATIBILITY RULE: + # - a patch release is compatible with the previous patch release + # - WARNING: this does not account for custom compatibility policies!!!! if is_patch_release(new_version, release.version): - previous_release = release - break + return release + + return None + + +async def inherit_from_latest_compatible_release( + services_repo: ServicesRepository, *, service_metadata: ServiceMetaDataPublished +) -> InheritedData: + """ + Inherits metadata and access rights from a previous compatible release. + + This function applies inheritance policies: + - AUTO-UPGRADE PATCH policy: new patch releases inherit access rights from previous compatible versions + - Metadata inheritance: icon and thumbnail fields are inherited if not specified in the new version + + Args: + services_repo: Instance of ServicesRepository for database access. + service_metadata: Metadata of the service being evaluated. - if previous_release: - previous_access_rights = await services_repo.get_service_access_rights( - previous_release.key, previous_release.version + Returns: + An InheritedData object containing: + - access_rights: List of ServiceAccessRightsDB objects inherited from the previous release + - metadata_updates: Dict of metadata fields that should be updated in the new service + + Notes: + - The policy is described in https://github.com/ITISFoundation/osparc-simcore/issues/2244 + - Inheritance is only for patch releases (i.e., same major and minor version). + """ + inherited_data: InheritedData = { + "access_rights": [], + "metadata_updates": {}, + } + + previous_release = await _find_latest_patch_compatible_release( + services_repo, service_metadata=service_metadata + ) + + if not previous_release: + return inherited_data + + # 1. ACCESS-RIGHTS: + # Inherit access rights (from all products) from the previous release + previous_access_rights = await services_repo.get_service_access_rights( + previous_release.key, previous_release.version + ) + + inherited_data["access_rights"] = [ + access.model_copy( + update={"version": service_metadata.version}, + deep=True, ) + for access in previous_access_rights + ] + + # 2. ServiceMetaDataPublished + # Inherit some fields if not specified in the new service + if not service_metadata.icon and previous_release.icon: + inherited_data["metadata_updates"]["icon"] = previous_release.icon + if not service_metadata.thumbnail and previous_release.thumbnail: + inherited_data["metadata_updates"]["thumbnail"] = previous_release.thumbnail - service_access_rights = [ - access.model_copy( - update={"version": service_metadata.version}, - deep=True, - ) - for access in previous_access_rights - ] - return service_access_rights + return inherited_data def reduce_access_rights( access_rights: list[ServiceAccessRightsDB], reduce_operation: Callable = operator.ior, ) -> list[ServiceAccessRightsDB]: - """ - Reduces a list of access-rights per target + """Reduces a list of access-rights per target + By default, the reduction is OR (i.e. preserves True flags) + """ # TODO: probably a lot of room to optimize # helper functions to simplify operation of access rights diff --git a/services/catalog/tests/unit/with_dbs/conftest.py b/services/catalog/tests/unit/with_dbs/conftest.py index 7be70290d6f..15bc8ac5b74 100644 --- a/services/catalog/tests/unit/with_dbs/conftest.py +++ b/services/catalog/tests/unit/with_dbs/conftest.py @@ -168,13 +168,13 @@ async def other_user( faker: Faker, ) -> AsyncIterator[dict[str, Any]]: - _user = random_user(fake=faker, id=user_id + 1) + _other_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, + values=_other_user, pk_col=users.c.id, - pk_value=_user["id"], + pk_value=_other_user["id"], ) as row: yield row @@ -247,22 +247,27 @@ async def services_db_tables_injector( async def _inject_in_db(fake_catalog: list[tuple]): # [(service, ar1, ...), (service2, ar1, ...) ] + iter_services = (items[0] for items in fake_catalog) + iter_access_rights = itertools.chain(items[1:] for items in fake_catalog) async with sqlalchemy_async_engine.begin() as conn: # NOTE: The 'default' dialect with current database version settings does not support in-place multirow inserts - for service in [items[0] for items in fake_catalog]: - insert_meta = pg_insert(services_meta_data).values(**service) - upsert_meta = insert_meta.on_conflict_do_update( - index_elements=[ - services_meta_data.c.key, - services_meta_data.c.version, - ], - set_=service, + for service in iter_services: + + insert_stmt = pg_insert(services_meta_data).values(**service) + + update_stmt = insert_stmt.on_conflict_do_update( + index_elements=["key", "version"], + set_={ + column_name: insert_stmt.excluded[column_name] + for column_name in service + if column_name not in ("key", "version") + }, ) - await conn.execute(upsert_meta) + await conn.execute(update_stmt) inserted_services.add((service["key"], service["version"])) - for access_rights in itertools.chain(items[1:] for items in fake_catalog): + for access_rights in iter_access_rights: stmt_access = services_access_rights.insert().values(access_rights) await conn.execute(stmt_access) 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 9f942cca334..f99002fb2c7 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 @@ -14,6 +14,7 @@ from models_library.services import ServiceMetaDataPublished from models_library.users import UserID from pydantic import TypeAdapter +from pytest_simcore.helpers.catalog_services import CreateFakeServiceDataCallable from respx.router import MockRouter from starlette import status from starlette.testclient import TestClient @@ -33,7 +34,7 @@ async def test_list_services_with_details( mocked_director_rest_api_base: MockRouter, user_id: UserID, target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, client: TestClient, benchmark, @@ -89,7 +90,7 @@ async def test_list_services_without_details( rabbitmq_and_rpc_setup_disabled: None, user_id: int, target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, client: TestClient, benchmark, @@ -133,7 +134,7 @@ async def test_list_services_without_details_with_wrong_user_id_returns_403( rabbitmq_and_rpc_setup_disabled: None, user_id: int, target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, client: TestClient, ): @@ -166,7 +167,7 @@ async def test_list_services_without_details_with_another_product_returns_other_ user_id: int, target_product: ProductName, other_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, client: TestClient, ): @@ -198,7 +199,7 @@ async def test_list_services_without_details_with_wrong_product_returns_0_servic rabbitmq_and_rpc_setup_disabled: None, user_id: int, target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, client: TestClient, ): @@ -234,7 +235,7 @@ async def test_list_services_that_are_deprecated( mocked_director_rest_api_base: MockRouter, user_id: int, target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, client: TestClient, ): diff --git a/services/catalog/tests/unit/with_dbs/test_api_rest_services_access_rights.py b/services/catalog/tests/unit/with_dbs/test_api_rest_services_access_rights.py index 7804f6de60d..e3ffd2248f5 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rest_services_access_rights.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rest_services_access_rights.py @@ -15,6 +15,7 @@ ) from models_library.products import ProductName from pydantic import TypeAdapter +from pytest_simcore.helpers.catalog_services import CreateFakeServiceDataCallable from respx.router import MockRouter from starlette.testclient import TestClient from yarl import URL @@ -33,7 +34,7 @@ async def test_get_service_access_rights( rabbitmq_and_rpc_setup_disabled: None, user: dict[str, Any], target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, client: TestClient, ): @@ -80,7 +81,7 @@ async def test_get_service_access_rights_with_more_gids( rabbitmq_and_rpc_setup_disabled: None, user: dict[str, Any], other_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, user_groups_ids: list[int], client: TestClient, diff --git a/services/catalog/tests/unit/with_dbs/test_api_rest_services_specifications.py b/services/catalog/tests/unit/with_dbs/test_api_rest_services_specifications.py index 554167b48d9..b32e1873a02 100644 --- a/services/catalog/tests/unit/with_dbs/test_api_rest_services_specifications.py +++ b/services/catalog/tests/unit/with_dbs/test_api_rest_services_specifications.py @@ -32,6 +32,7 @@ ) from models_library.products import ProductName from models_library.users import UserID +from pytest_simcore.helpers.catalog_services import CreateFakeServiceDataCallable from simcore_postgres_database.models.groups import user_to_groups from simcore_postgres_database.models.services_specifications import ( services_specifications, @@ -183,7 +184,7 @@ async def test_get_service_specifications( user: dict[str, Any], user_groups_ids: list[int], target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, services_specifications_injector: Callable, sqlalchemy_async_engine: AsyncEngine, @@ -278,7 +279,7 @@ async def test_get_service_specifications_are_passed_to_newer_versions_of_servic user: dict[str, Any], user_groups_ids: list[int], target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, services_specifications_injector: Callable, create_service_specifications: Callable[..., ServiceSpecificationsAtDB], 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 5b43b728261..4cc4c452fdb 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 @@ from models_library.users import UserID from packaging import version from pydantic import ValidationError +from pytest_simcore.helpers.catalog_services import CreateFakeServiceDataCallable from pytest_simcore.helpers.faker_factories import random_icon_url from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict @@ -70,7 +71,7 @@ def num_versions_per_service() -> int: @pytest.fixture def fake_data_for_services( target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, num_services: int, num_versions_per_service: int, ) -> list: @@ -169,7 +170,7 @@ async def test_rpc_list_services_paginated_with_filter_combinations( product_name: ProductName, user_id: UserID, app: FastAPI, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, ): """Tests all combinations of filters for list_services_paginated""" @@ -565,7 +566,7 @@ async def test_rpc_batch_get_my_services( user: dict[str, Any], user_id: UserID, app: FastAPI, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, ): # Create fake services data @@ -645,7 +646,7 @@ async def test_rpc_list_my_service_history_paginated( product_name: ProductName, user_id: UserID, app: FastAPI, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, ): assert app @@ -775,7 +776,7 @@ async def test_rpc_get_service_ports_permission_denied( user_id: UserID, other_user: dict[str, Any], app: FastAPI, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, services_db_tables_injector: Callable, ): """Tests that appropriate error is raised when user doesn't have permission""" diff --git a/services/catalog/tests/unit/with_dbs/test_service_access_rights.py b/services/catalog/tests/unit/with_dbs/test_service_access_rights.py index eeef6bc8bb5..cc2ae2c7621 100644 --- a/services/catalog/tests/unit/with_dbs/test_service_access_rights.py +++ b/services/catalog/tests/unit/with_dbs/test_service_access_rights.py @@ -3,18 +3,23 @@ # pylint: disable=unused-variable from collections.abc import Callable +from typing import Any, Protocol +import pytest import simcore_service_catalog.service.access_rights from fastapi import FastAPI -from models_library.groups import GroupAtDB +from models_library.groups import GroupID from models_library.products import ProductName from models_library.services import ServiceMetaDataPublished, ServiceVersion +from models_library.services_authoring import Author from pydantic import TypeAdapter +from pytest_mock import MockerFixture +from pytest_simcore.helpers.catalog_services import CreateFakeServiceDataCallable from simcore_service_catalog.models.services_db import ServiceAccessRightsDB from simcore_service_catalog.repository.services import ServicesRepository from simcore_service_catalog.service.access_rights import ( - evaluate_auto_upgrade_policy, - evaluate_default_policy, + evaluate_default_service_ownership_and_rights, + inherit_from_latest_compatible_release, reduce_access_rights, ) from sqlalchemy.ext.asyncio import AsyncEngine @@ -27,6 +32,91 @@ ] +@pytest.fixture +def new_service_metadata_published(user: dict[str, Any]) -> ServiceMetaDataPublished: + MOST_UPDATED_EXAMPLE = -1 + metadata = ServiceMetaDataPublished.model_validate( + ServiceMetaDataPublished.model_json_schema()["examples"][MOST_UPDATED_EXAMPLE] + ) + metadata.contact = user["email"] + metadata.authors = [ + Author(name=user["name"], email=user["email"], affiliation=None) + ] + metadata.version = TypeAdapter(ServiceVersion).validate_python("1.0.11") + # Set all inheritable fields to None initially + metadata.icon = None + metadata.thumbnail = None + return metadata + + +@pytest.fixture +def app_with_repo( + sqlalchemy_async_engine: AsyncEngine, + target_product: ProductName, + mocker: MockerFixture, +) -> tuple[FastAPI, ServicesRepository]: + """Creates FastAPI app with services repository setup.""" + app = FastAPI() + app.state.engine = sqlalchemy_async_engine + app.state.settings = mocker.Mock() + app.state.default_product_name = target_product + + services_repo = ServicesRepository(app.state.engine) + return app, services_repo + + +class CreateLatetReleaseCallable(Protocol): + """Callable to create a latest release with specified metadata fields.""" + + async def __call__( + self, metadata_fields: dict[str, Any], product: ProductName + ) -> dict[str, Any]: ... + + +@pytest.fixture +def create_latest_release( + create_fake_service_data: CreateFakeServiceDataCallable, + new_service_metadata_published: ServiceMetaDataPublished, + services_db_tables_injector: Callable, +) -> CreateLatetReleaseCallable: + """Creates a latest release with specified metadata fields for a product. + + Args: + metadata_fields: Dictionary of metadata fields to set + product: Product to create the service for + + Returns: + The created service metadata + """ + + from packaging.version import Version + + new_version = Version(new_service_metadata_published.version) + + async def _create( + metadata_fields: dict[str, Any], product: ProductName + ) -> dict[str, Any]: + latest_release_service, *latest_release_service_access_rights = ( + create_fake_service_data( + new_service_metadata_published.key, + f"{new_version.major}.{new_version.minor}.{new_version.micro-1}", + team_access="x", + everyone_access=None, + product=product, + ) + ) + + # Update with provided metadata fields + for field, value in metadata_fields.items(): + latest_release_service[field] = value + + latest_release = (latest_release_service, *latest_release_service_access_rights) + await services_db_tables_injector([latest_release]) + return latest_release_service + + return _create + + def test_reduce_access_rights(): sample = ServiceAccessRightsDB.model_validate( { @@ -82,98 +172,239 @@ def test_reduce_access_rights(): } -async def test_auto_upgrade_policy( - sqlalchemy_async_engine: AsyncEngine, - user_groups_ids: list[int], +async def test_metadata_inheritance_variations( + new_service_metadata_published: ServiceMetaDataPublished, + app_with_repo: tuple[FastAPI, ServicesRepository], + create_latest_release: CreateLatetReleaseCallable, target_product: ProductName, - other_product: ProductName, - services_db_tables_injector: Callable, - create_fake_service_data: Callable, - mocker, ): + """Test different variations of metadata inheritance with complete previous release.""" + app, services_repo = app_with_repo + + # Set up a previous release with all metadata fields + latest_release_service = await create_latest_release( + { + "icon": "https://foo/previous_icon.svg", + "thumbnail": "https://foo/previous_thumbnail.jpg", + }, + target_product, + ) + + # Case 1: All fields missing in new service - only icon and thumbnail should be inherited + new_service = new_service_metadata_published.model_copy(deep=True) + inherited_data = await inherit_from_latest_compatible_release( + services_repo, service_metadata=new_service + ) + + assert inherited_data["metadata_updates"] == { + "icon": latest_release_service["icon"], + "thumbnail": latest_release_service["thumbnail"], + } + + # Case 2: Some fields present, others missing + new_service = new_service_metadata_published.model_copy( + deep=True, + update={ + "thumbnail": None, # Missing field + "icon": "https://foo/new_icon.svg", + }, + ) + + inherited_data = await inherit_from_latest_compatible_release( + services_repo, service_metadata=new_service + ) + + # Only thumbnail should be inherited + assert "icon" not in inherited_data["metadata_updates"] + assert inherited_data["metadata_updates"] == { + "thumbnail": latest_release_service["thumbnail"], + } + + # Case 3: All fields present in new service - nothing should be inherited + new_service = new_service_metadata_published.model_copy( + deep=True, + update={ + "icon": "https://foo/new_icon.svg", + "thumbnail": "https://foo/new_thumbnail.jpg", + }, + ) + + inherited_data = await inherit_from_latest_compatible_release( + services_repo, service_metadata=new_service + ) + + # No metadata should be inherited + assert inherited_data["metadata_updates"] == {} + + +async def test_metadata_inheritance_with_incomplete_previous_release( + new_service_metadata_published: ServiceMetaDataPublished, + app_with_repo: tuple[FastAPI, ServicesRepository], + create_latest_release: CreateLatetReleaseCallable, + target_product: ProductName, +): + """Test metadata inheritance when previous release has incomplete metadata fields.""" + app, services_repo = app_with_repo + + # Case 4: Previous release missing some fields + latest_release_service = await create_latest_release( + { + "icon": "https://foo/previous_icon.svg", + "thumbnail": "https://foo/previous_thumbnail.jpg", + }, + target_product, + ) + + new_service = new_service_metadata_published.model_copy(deep=True) + inherited_data = await inherit_from_latest_compatible_release( + services_repo, service_metadata=new_service + ) + + # Only icon and thumbnail should be inherited + assert inherited_data["metadata_updates"] == { + "icon": latest_release_service["icon"], + "thumbnail": latest_release_service["thumbnail"], + } + assert "description" not in inherited_data["metadata_updates"] + + +async def test_service_upgrade_metadata_inheritance_old_service( + user_groups_ids: list[GroupID], + target_product: ProductName, + mocker: MockerFixture, + new_service_metadata_published: ServiceMetaDataPublished, + app_with_repo: tuple[FastAPI, ServicesRepository], + create_latest_release: CreateLatetReleaseCallable, +): + """Test inheritance behavior when the service is considered old""" everyone_gid, user_gid, team_gid = user_groups_ids + app, services_repo = app_with_repo - # Avoids calls to director API + # Mock to make the service appear as old mocker.patch.object( simcore_service_catalog.service.access_rights, "_is_old_service", - return_value=False, + return_value=True, ) - # Avoids creating a users + user_to_group table - data = GroupAtDB.model_json_schema()["example"] - data["gid"] = everyone_gid - mocker.patch.object( - simcore_service_catalog.service.access_rights.GroupsRepository, - "get_everyone_group", - return_value=GroupAtDB.model_validate(data), + + # Create latest-release service for testing inheritance with all metadata fields + latest_release_service = await create_latest_release( + { + "icon": "https://foo/previous_icon.svg", + "description": "Previous description", # This won't be inherited + "thumbnail": "https://foo/previous_thumbnail.jpg", + }, + target_product, + ) + + # DEFAULT policies for old service + owner_gid, service_access_rights = ( + await evaluate_default_service_ownership_and_rights( + app, service=new_service_metadata_published, product_name=target_product + ) ) + + # For old services, everyone should have access + assert owner_gid == user_gid + assert len(service_access_rights) == 2 # Owner + everyone + assert {a.gid for a in service_access_rights} == {owner_gid, everyone_gid} + + # Check owner access + owner_access = next(a for a in service_access_rights if a.gid == owner_gid) + assert owner_access.model_dump(include={"execute_access", "write_access"}) == { + "execute_access": True, + "write_access": True, + } + + # Check everyone access + everyone_access = next(a for a in service_access_rights if a.gid == everyone_gid) + assert everyone_access.model_dump(include={"execute_access", "write_access"}) == { + "execute_access": True, + "write_access": False, # Everyone can execute but not modify + } + + # Inheritance policy (both access rights and metadata) + inherited_data = await inherit_from_latest_compatible_release( + services_repo, service_metadata=new_service_metadata_published + ) + + # Check metadata inheritance - only icon and thumbnail should be inherited + inherited_metadata = inherited_data["metadata_updates"] + assert "description" not in inherited_metadata + assert inherited_metadata == { + "icon": latest_release_service["icon"], + "thumbnail": latest_release_service["thumbnail"], + } + + +async def test_service_upgrade_metadata_inheritance_new_service_multi_product( + user_groups_ids: list[GroupID], + target_product: ProductName, + other_product: ProductName, + services_db_tables_injector: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, + mocker: MockerFixture, + new_service_metadata_published: ServiceMetaDataPublished, + app_with_repo: tuple[FastAPI, ServicesRepository], + create_latest_release: CreateLatetReleaseCallable, +): + """Test inheritance behavior when the service is new and latest version exists in multiple products""" + everyone_gid, user_gid, team_gid = user_groups_ids + app, services_repo = app_with_repo + + # Avoids calls to director API - service is new mocker.patch.object( - simcore_service_catalog.service.access_rights.GroupsRepository, - "get_user_gid_from_email", - return_value=user_gid, + simcore_service_catalog.service.access_rights, + "_is_old_service", + return_value=False, ) - # SETUP --- - MOST_UPDATED_EXAMPLE = -1 - new_service_metadata = ServiceMetaDataPublished.model_validate( - ServiceMetaDataPublished.model_json_schema()["examples"][MOST_UPDATED_EXAMPLE] + # Create latest-release service with metadata for target product + metadata_fields = { + "icon": "https://foo/previous_icon.svg", + "thumbnail": "https://foo/previous_thumbnail.jpg", + } + + # Create in target product + latest_release_service = await create_latest_release( + metadata_fields, target_product ) - new_service_metadata.version = TypeAdapter(ServiceVersion).validate_python("1.0.11") - # we have three versions of the service in the database for which the sorting matters: (1.0.11 should inherit from 1.0.10 not 1.0.9) + # Create in other product + await create_latest_release(metadata_fields, other_product) + + # Create older versions in target product to test proper version ordering await services_db_tables_injector( [ create_fake_service_data( - new_service_metadata.key, + new_service_metadata_published.key, "1.0.1", team_access=None, everyone_access=None, product=target_product, ), create_fake_service_data( - new_service_metadata.key, + new_service_metadata_published.key, "1.0.9", team_access=None, everyone_access=None, product=target_product, ), - # new release is a patch on released 1.0.X - # which were released in two different product - create_fake_service_data( - new_service_metadata.key, - "1.0.10", - team_access="x", - everyone_access=None, - product=target_product, - ), - create_fake_service_data( - new_service_metadata.key, - "1.0.10", - team_access="x", - everyone_access=None, - product=other_product, - ), ] ) - # ------------ - - app = FastAPI() - app.state.engine = sqlalchemy_async_engine - app.state.settings = mocker.Mock() - app.state.default_product_name = target_product - - services_repo = ServicesRepository(app.state.engine) # DEFAULT policies - owner_gid, service_access_rights = await evaluate_default_policy( - app, new_service_metadata + owner_gid, service_access_rights = ( + await evaluate_default_service_ownership_and_rights( + app, service=new_service_metadata_published, product_name=target_product + ) ) assert owner_gid == user_gid - assert len(service_access_rights) == 1 + assert len(service_access_rights) == 1 # Only owner for new service assert {a.gid for a in service_access_rights} == {owner_gid} assert service_access_rights[0].model_dump() == { - "key": new_service_metadata.key, - "version": new_service_metadata.version, + "key": new_service_metadata_published.key, + "version": new_service_metadata_published.version, "gid": user_gid, "product_name": target_product, "execute_access": True, @@ -181,11 +412,13 @@ async def test_auto_upgrade_policy( } assert service_access_rights[0].product_name == target_product - # AUTO-UPGRADE PATCH policy - inherited_access_rights = await evaluate_auto_upgrade_policy( - new_service_metadata, services_repo + # Inheritance policy (both access rights and metadata) + inherited_data = await inherit_from_latest_compatible_release( + services_repo, service_metadata=new_service_metadata_published ) + # Check access rights inheritance + inherited_access_rights = inherited_data["access_rights"] assert len(inherited_access_rights) == 4 assert {a.gid for a in inherited_access_rights} == {team_gid, owner_gid} assert {a.product_name for a in inherited_access_rights} == { @@ -193,13 +426,23 @@ async def test_auto_upgrade_policy( other_product, } + # Check metadata inheritance + inherited_metadata = inherited_data["metadata_updates"] + assert "icon" in inherited_metadata + assert inherited_metadata["icon"] == latest_release_service["icon"] + assert "thumbnail" in inherited_metadata + assert inherited_metadata["thumbnail"] == latest_release_service["thumbnail"] + # ALL service_access_rights += inherited_access_rights service_access_rights = reduce_access_rights(service_access_rights) assert len(service_access_rights) == 4 - assert {a.gid for a in service_access_rights} == {team_gid, owner_gid} + assert {a.gid for a in service_access_rights} == { + team_gid, + owner_gid, + }, "last version was exposed to a team" assert {a.product_name for a in service_access_rights} == { target_product, other_product, - } + }, "last version was exposed to two products" diff --git a/services/catalog/tests/unit/with_dbs/test_service_catalog_services.py b/services/catalog/tests/unit/with_dbs/test_service_catalog_services.py index 5faaf6b384d..a7f106483a1 100644 --- a/services/catalog/tests/unit/with_dbs/test_service_catalog_services.py +++ b/services/catalog/tests/unit/with_dbs/test_service_catalog_services.py @@ -54,7 +54,7 @@ def num_versions_per_service() -> int: @pytest.fixture def fake_services_data( target_product: ProductName, - create_fake_service_data: Callable, + create_fake_service_data: CreateFakeServiceDataCallable, num_services: int, num_versions_per_service: int, ) -> list: