✨ Refactor service filters application and enhance service key prefix handling in SQL queries

Author: pcrespov

services/catalog/src/simcore_service_catalog/repository/_services_sql.py

@@ -125,8 +125,9 @@ def apply_services_filters(
         if prefix is None:
             msg = f"Undefined service type {filters.service_type}. Please update prefix expressions"
             raise ValueError(msg)
-        prefix = prefix.rstrip("/")  # safety
-        stmt = stmt.where(services_meta_data.c.service_key.like(f"{prefix}/%"))
+
+        assert not prefix.endswith("/")  # nosec
+        return stmt.where(services_meta_data.c.key.like(f"{prefix}/%"))
     return stmt
 
 
@@ -155,7 +156,7 @@ def latest_services_total_count_stmt(
     )
 
     if filters:
-        apply_services_filters(stmt, filters)
+        stmt = apply_services_filters(stmt, filters)
 
     return stmt
 
@@ -199,7 +200,7 @@ def list_latest_services_stmt(
     )
 
     if filters:
-        apply_services_filters(cte_stmt, filters)
+        cte_stmt = apply_services_filters(cte_stmt, filters)
 
     cte = cte_stmt.cte("cte")
 

services/catalog/src/simcore_service_catalog/repository/services.py

@@ -411,6 +411,11 @@ async def list_latest_services(
             filters=filters,
         )
 
+        from simcore_postgres_database.utils import as_postgres_sql_query_str
+
+        print(as_postgres_sql_query_str(stmt_total))
+        print(as_postgres_sql_query_str(stmt_page))
+
         async with self.db_engine.connect() as conn:
             result = await conn.execute(stmt_total)
             total_count = result.scalar() or 0
@@ -514,7 +519,7 @@ async def get_service_history_page(
         )
 
         if filters:
-            apply_services_filters(base_stmt, filters)
+            base_stmt = apply_services_filters(base_stmt, filters)
 
         base_subquery = base_stmt.subquery()
 

services/catalog/tests/unit/with_dbs/test_repositories.py

@@ -14,6 +14,11 @@
 import pytest
 from models_library.products import ProductName
 from models_library.services_enums import ServiceType  # Import ServiceType enum
+from models_library.services_regex import (
+    COMPUTATIONAL_SERVICE_KEY_PREFIX,
+    DYNAMIC_SERVICE_KEY_PREFIX,
+    SERVICE_TYPE_PREFIXES,
+)
 from models_library.users import UserID
 from packaging import version
 from pydantic import EmailStr, HttpUrl, TypeAdapter
@@ -416,49 +421,46 @@ async def test_list_latest_services_with_filters(
     await services_db_tables_injector(
         [
             create_fake_service_data(
-                f"simcore/services/dynamic/service-type-a-{i}",
+                f"{DYNAMIC_SERVICE_KEY_PREFIX}/service-name-a-{i}",
                 "1.0.0",
                 team_access=None,
                 everyone_access=None,
                 product=target_product,
-                service_type=ServiceType.DYNAMIC.value,
             )
             for i in range(3)
         ]
         + [
             create_fake_service_data(
-                f"simcore/services/dynamic/service-type-b-{i}",
+                f"{COMPUTATIONAL_SERVICE_KEY_PREFIX}/service-name-b-{i}",
                 "1.0.0",
                 team_access=None,
                 everyone_access=None,
                 product=target_product,
-                service_type=ServiceType.COMPUTATIONAL.value,
             )
             for i in range(2)
         ]
     )
 
-    # Test: Apply filter for service_type=ServiceType.DYNAMIC
+    # Test: Apply filter for ServiceType.DYNAMIC
     filters = ServiceFiltersDB(service_type=ServiceType.DYNAMIC)
     total_count, services_items = await services_repo.list_latest_services(
         product_name=target_product, user_id=user_id, filters=filters
     )
     assert total_count == 3
     assert len(services_items) == 3
     assert all(
-        service.key.startswith("simcore/services/dynamic/service-type-a")
-        for service in services_items
+        service.key.startswith(DYNAMIC_SERVICE_KEY_PREFIX) for service in services_items
     )
 
-    # Test: Apply filter for service_type=ServiceType.COMPUTATIONAL
+    # Test: Apply filter for ServiceType.COMPUTATIONAL
     filters = ServiceFiltersDB(service_type=ServiceType.COMPUTATIONAL)
     total_count, services_items = await services_repo.list_latest_services(
         product_name=target_product, user_id=user_id, filters=filters
     )
     assert total_count == 2
     assert len(services_items) == 2
     assert all(
-        service.key.startswith("simcore/services/dynamic/service-type-b")
+        service.key.startswith(COMPUTATIONAL_SERVICE_KEY_PREFIX)
         for service in services_items
     )
 
@@ -551,6 +553,15 @@ async def test_can_get_service(
     )
 
 
+def _create_fake_release_versions(num_versions: int) -> set[str]:
+    release_versions = set()
+    while len(release_versions) < num_versions:
+        release_versions.add(
+            f"{random.randint(0, 2)}.{random.randint(0, 9)}.{random.randint(0, 9)}"  # noqa: S311
+        )
+    return release_versions
+
+
 async def test_get_service_history_page(
     target_product: ProductName,
     create_fake_service_data: Callable,
@@ -562,12 +573,7 @@ async def test_get_service_history_page(
     service_key = "simcore/services/dynamic/test-some-service"
     num_versions = 10
 
-    release_versions = set()
-    while len(release_versions) < num_versions:
-        release_versions.add(
-            f"{random.randint(0, 2)}.{random.randint(0, 9)}.{random.randint(0, 9)}"  # noqa: S311
-        )
-
+    release_versions = _create_fake_release_versions(num_versions)
     await services_db_tables_injector(
         [
             create_fake_service_data(
@@ -626,22 +632,23 @@ async def test_get_service_history_page(
     assert paginated_history == history[offset : offset + limit]
 
 
+@pytest.mark.parametrize(
+    "expected_service_type,service_prefix", SERVICE_TYPE_PREFIXES.items()
+)
 async def test_get_service_history_page_with_filters(
     target_product: ProductName,
     create_fake_service_data: Callable,
     services_db_tables_injector: Callable,
     services_repo: ServicesRepository,
     user_id: UserID,
+    expected_service_type: ServiceType,
+    service_prefix: str,
 ):
     # Setup: Inject services with multiple versions and types
-    service_key = "simcore/services/dynamic/test-service"
+    service_key = f"{service_prefix}/test-service"
     num_versions = 10
 
-    release_versions = set()
-    while len(release_versions) < num_versions:
-        release_versions.add(
-            f"{random.randint(0, 2)}.{random.randint(0, 9)}.{random.randint(0, 9)}"  # noqa: S311
-        )
+    release_versions = _create_fake_release_versions(num_versions)
 
     await services_db_tables_injector(
         [
@@ -651,13 +658,8 @@ async def test_get_service_history_page_with_filters(
                 team_access=None,
                 everyone_access=None,
                 product=target_product,
-                service_type=(
-                    ServiceType.DYNAMIC.value
-                    if i % 2 == 0
-                    else ServiceType.COMPUTATIONAL.value
-                ),
             )
-            for i, service_version in enumerate(release_versions)
+            for _, service_version in enumerate(release_versions)
         ]
     )
     # Sort versions after injecting
@@ -673,33 +675,33 @@ async def test_get_service_history_page_with_filters(
     assert len(history) == num_versions
     assert [release.version for release in history] == release_versions
 
-    # Test: Apply filter for service_type=ServiceType.DYNAMIC
-    filters = ServiceFiltersDB(service_type=ServiceType.DYNAMIC)
+    # Test: Apply filter for
+    filters = ServiceFiltersDB(service_type=expected_service_type)
     total_count, filtered_history = await services_repo.get_service_history_page(
         product_name=target_product,
         user_id=user_id,
         key=service_key,
         filters=filters,
     )
-    assert total_count == num_versions // 2
-    assert len(filtered_history) == num_versions // 2
-    assert all(
-        int(release.version.split(".")[0]) % 2 == 0 for release in filtered_history
-    )
+    assert total_count == num_versions
+    assert len(filtered_history) == num_versions
+    assert [release.version for release in filtered_history] == release_versions
 
-    # Test: Apply filter for service_type=ServiceType.COMPUTATIONAL
-    filters = ServiceFiltersDB(service_type=ServiceType.COMPUTATIONAL)
-    total_count, filtered_history = await services_repo.get_service_history_page(
+    # Final check: filter by a different service type expecting no results
+    different_service_type = (
+        ServiceType.COMPUTATIONAL
+        if expected_service_type != ServiceType.COMPUTATIONAL
+        else ServiceType.DYNAMIC
+    )
+    filters = ServiceFiltersDB(service_type=different_service_type)
+    total_count, no_history = await services_repo.get_service_history_page(
         product_name=target_product,
         user_id=user_id,
         key=service_key,
         filters=filters,
     )
-    assert total_count == num_versions // 2
-    assert len(filtered_history) == num_versions // 2
-    assert all(
-        int(release.version.split(".")[0]) % 2 != 0 for release in filtered_history
-    )
+    assert total_count == 0
+    assert no_history == []
 
 
 async def test_list_services_from_published_templates(