Merge branch 'master' into mai/docker-compose-refs

Author: pcrespov

packages/models-library/src/models_library/batch_operations.py

@@ -1,3 +1,28 @@
+"""
+
+# Batch Operations Rationale:
+
+Please preserve the following behaviors when implementing batch operations:
+
+| Case           | Behavior                                   | Justification               |
+| -------------- | ------------------------------------------ | --------------------------- |
+| Empty `names`  | `400 Bad Request`                          | Invalid input               |
+| Some missing   | `200 OK`, with `missing` field             | Partial success             |
+| Duplicates     | Silently deduplicate                       | Idempotent, client-friendly |
+| Response order | Preserve request order (excluding missing) | Deterministic, ergonomic    |
+
+
+- `BatchGet` is semantically distinct from `List`.
+  - `List` means “give me everything you have, maybe filtered.”
+  - `BatchGet` means “give me these specific known resources.”
+- Passing an empty list means you’re not actually identifying anything to fetch — so it’s a client error (bad request), not a legitimate “empty result.”
+- This aligns with the principle: If the request parameters are syntactically valid but semantically meaningless, return 400 Bad Request.
+
+# References:
+    - https://google.aip.dev/130
+    - https://google.aip.dev/231
+"""
+
 from typing import Annotated, Generic, TypeVar
 
 from common_library.basic_types import DEFAULT_FACTORY

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

@@ -703,6 +703,7 @@ async def batch_get_user_services(
 
     Raises:
         CatalogItemNotFoundError: When no services are found at all
+        ValidationError: if the ids are empty
     """
     unique_service_identifiers = _BatchIdsValidator.validate_python(ids)
 

services/dask-sidecar/src/simcore_service_dask_sidecar/utils/files.py

@@ -166,8 +166,9 @@ async def pull_file_from_remote(
     s3_settings: S3Settings | None,
 ) -> None:
     assert src_url.path  # nosec
+    src_location_str = f"{src_url.path.strip('/')} on {src_url.host}:{src_url.port}"
     await log_publishing_cb(
-        f"Downloading '{src_url}' into local file '{dst_path}'...",
+        f"Downloading '{src_location_str}' into local file '{dst_path}'...",
         logging.INFO,
     )
     if not dst_path.parent.exists():
@@ -196,11 +197,11 @@ async def pull_file_from_remote(
             TypeAdapter(FileUrl).validate_python(f"{download_dst_path.as_uri()}"),
             src_storage_cfg=cast(dict[str, Any], storage_kwargs),
             log_publishing_cb=log_publishing_cb,
-            text_prefix=f"Downloading '{src_url.path.strip('/')}':",
+            text_prefix=f"Downloading '{src_location_str}':",
         )
 
         await log_publishing_cb(
-            f"Download of '{src_url}' into local file '{download_dst_path}' complete.",
+            f"Download of '{src_location_str}' into local file '{download_dst_path}' complete.",
             logging.INFO,
         )
 

services/web/server/src/simcore_service_webserver/catalog/_service.py

@@ -10,6 +10,7 @@
     ServiceOutputGet,
     ServiceOutputKey,
 )
+from models_library.batch_operations import create_batch_ids_validator
 from models_library.products import ProductName
 from models_library.rest_pagination import (
     PageLimitInt,
@@ -25,6 +26,7 @@
 from models_library.users import UserID
 from models_library.utils.fastapi_encoders import jsonable_encoder
 from pint import UnitRegistry
+from pydantic import ValidationError
 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 (
@@ -94,6 +96,11 @@ async def list_latest_services(
     return page_data, page.meta
 
 
+_BatchServicesIdsValidator = create_batch_ids_validator(
+    tuple[ServiceKey, ServiceVersion]
+)
+
+
 async def batch_get_my_services(
     app: web.Application,
     *,
@@ -102,12 +109,17 @@ async def batch_get_my_services(
     services_ids: list[tuple[ServiceKey, ServiceVersion]],
 ) -> MyServicesBatchGetResult:
     try:
+        ids = _BatchServicesIdsValidator.validate_python(services_ids)
+    except ValidationError as err:
+        msg = f"Invalid 'service_ids' parameter:\n{err}"
+        raise ValueError(msg) from err
 
+    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,
+            ids=ids,
         )
 
     except RPCServerError as err:

services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py

@@ -546,28 +546,32 @@ async def get_project_services(request: web.Request) -> web.Response:
         )
     )
 
-    batch_got = await catalog_service.batch_get_my_services(
-        request.app,
-        product_name=req_ctx.product_name,
-        user_id=req_ctx.user_id,
-        services_ids=services_in_project,
-    )
+    services = []
+    missing = None
+
+    if services_in_project:
+        batch_got = await catalog_service.batch_get_my_services(
+            request.app,
+            product_name=req_ctx.product_name,
+            user_id=req_ctx.user_id,
+            services_ids=services_in_project,
+        )
+        services = [
+            NodeServiceGet.model_validate(sv, from_attributes=True)
+            for sv in batch_got.found_items
+        ]
+        missing = (
+            [
+                ServiceKeyVersion(key=k, version=v)
+                for k, v in batch_got.missing_identifiers
+            ]
+            if batch_got.missing_identifiers
+            else None
+        )
 
     return envelope_json_response(
         ProjectNodeServicesGet(
-            project_uuid=path_params.project_id,
-            services=[
-                NodeServiceGet.model_validate(sv, from_attributes=True)
-                for sv in batch_got.found_items
-            ],
-            missing=(
-                [
-                    ServiceKeyVersion(key=k, version=v)
-                    for k, v in batch_got.missing_identifiers
-                ]
-                if batch_got.missing_identifiers
-                else None
-            ),
+            project_uuid=path_params.project_id, services=services, missing=missing
         )
     )
 

services/web/server/src/simcore_service_webserver/projects/_projects_nodes_repository.py

@@ -1,7 +1,6 @@
 import logging
 
 import sqlalchemy as sa
-
 from aiohttp import web
 from models_library.projects import ProjectID
 from models_library.projects_nodes import Node, PartialNode
@@ -10,8 +9,8 @@
 from simcore_postgres_database.webserver_models import projects_nodes
 from sqlalchemy.ext.asyncio import AsyncConnection
 
-from .exceptions import NodeNotFoundError
 from ..db.plugin import get_asyncpg_engine
+from .exceptions import NodeNotFoundError
 
 _logger = logging.getLogger(__name__)
 
@@ -44,21 +43,18 @@ async def get(
     node_id: NodeID,
 ) -> Node:
     async with transaction_context(get_asyncpg_engine(app), connection) as conn:
-        get_stmt = sa.select(
-            *_SELECTION_PROJECTS_NODES_DB_ARGS
-        ).where(
+        get_stmt = sa.select(*_SELECTION_PROJECTS_NODES_DB_ARGS).where(
             (projects_nodes.c.project_uuid == f"{project_id}")
             & (projects_nodes.c.node_id == f"{node_id}")
         )
 
-        result = await conn.stream(get_stmt)
+        result = await conn.execute(get_stmt)
         assert result  # nosec
 
-        row = await result.first()
+        row = result.one_or_none()
         if row is None:
             raise NodeNotFoundError(
-                project_uuid=f"{project_id}",
-                node_uuid=f"{node_id}"
+                project_uuid=f"{project_id}", node_uuid=f"{node_id}"
             )
         assert row  # nosec
         return Node.model_validate(row, from_attributes=True)
@@ -75,7 +71,7 @@ async def update(
     values = partial_node.model_dump(mode="json", exclude_unset=True)
 
     async with transaction_context(get_asyncpg_engine(app), connection) as conn:
-        await conn.stream(
+        await conn.execute(
             projects_nodes.update()
             .values(**values)
             .where(

services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__services_access.py

@@ -4,8 +4,10 @@
 # pylint: disable=unused-variable
 # type: ignore
 
+from collections.abc import AsyncIterator
 from copy import deepcopy
 from http import HTTPStatus
+from pathlib import Path
 from typing import Any
 
 import pytest
@@ -20,6 +22,7 @@
 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_projects import NewProject
 from pytest_simcore.helpers.webserver_users import UserInfoDict
 from servicelib.aiohttp import status
 from servicelib.rabbitmq import RPCServerError
@@ -590,3 +593,66 @@ async def test_get_project_services_service_unavailable(
 
     assert error
     assert not data
+
+
+@pytest.fixture
+async def empty_project(
+    client,
+    fake_project: dict,
+    logged_user: dict,
+    tests_data_dir: Path,
+    osparc_product_name: str,
+) -> AsyncIterator[dict]:
+    fake_project["prjOwner"] = logged_user["name"]
+    fake_project["workbench"] = {}
+    async with NewProject(
+        fake_project,
+        client.app,
+        user_id=logged_user["id"],
+        product_name=osparc_product_name,
+        tests_data_dir=tests_data_dir,
+    ) as project:
+        yield project
+
+
+@pytest.mark.parametrize("user_role", [UserRole.USER])
+async def test_get_project_services_empty_project(
+    client: TestClient,
+    empty_project: ProjectDict,
+    mocker: MockerFixture,
+):
+    # NOTE: Tests bug described in https://github.com/ITISFoundation/osparc-simcore/pull/8501
+    assert empty_project["workbench"] == {}
+
+    # MOCK CATALOG TO RAISE IF CALLED (it should not be called)
+    from simcore_service_webserver.catalog._service import catalog_rpc  # noqa: PLC0415
+
+    mocker.patch.object(
+        catalog_rpc,
+        "batch_get_my_services",
+        spec=True,
+        side_effect=ValueError(
+            "Bad request: cannot batch-get an empty list of name-ids"
+        ),
+    )
+
+    assert client.app
+
+    # ACT
+    project_id = empty_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")
+
+    # ASSERT
+    data, _ = await assert_status(resp, status.HTTP_200_OK)
+
+    assert data == {
+        "projectUuid": project_id,
+        "services": [],
+        "missing": None,
+    }