✨ catalog: new service inherit icon and thumbnail from latest patched compatible releases (#7769)

Author: pcrespov

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,
             )
 

services/catalog/src/simcore_service_catalog/service/access_rights.py

@@ -4,18 +4,20 @@
 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
 from pydantic.types import PositiveInt
 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,36 +48,46 @@ 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
 
     groups_repo = GroupsRepository(db_engine)
     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,78 +97,129 @@ 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)
     ]
 
     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

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)