BugFix: Fix/catalog model (#1826)

* Fixes validation of ServiceAccessRightsAtDB

* autoformat

* Fixes using core settings

* Fixes a hard-coded header

* Fixes API in catalog subsystem at webserver

* Fix wrong import

* Adds setup-product to client test fixtures

Author: pcrespov

services/catalog/src/simcore_service_catalog/api/routes/services.py

@@ -11,11 +11,11 @@
 from ...db.repositories.services import ServicesRepository
 from ...models.domain.service import (
     KEY_RE,
-    ServiceType,
     VERSION_RE,
     ServiceAccessRightsAtDB,
     ServiceMetaDataAtDB,
     ServiceOut,
+    ServiceType,
     ServiceUpdate,
 )
 from ...services.frontend_services import get_services as get_frontend_services

services/catalog/src/simcore_service_catalog/core/background_tasks.py

@@ -21,7 +21,6 @@
 from pydantic import ValidationError
 from pydantic.types import PositiveInt
 
-
 from ..api.dependencies.director import get_director_api
 from ..db.repositories.groups import GroupsRepository
 from ..db.repositories.projects import ProjectsRepository
@@ -31,13 +30,14 @@
     ServiceDockerData,
     ServiceMetaDataAtDB,
 )
+from ..services.frontend_services import get_services as get_frontend_services
 
 logger = logging.getLogger(__name__)
 
 ServiceKey = str
 ServiceVersion = str
 
-from ..services.frontend_services import get_services as get_frontend_services
+OLD_SERVICES_DATE: datetime = datetime(2020, 8, 19)
 
 
 async def _list_registry_services(
@@ -74,9 +74,6 @@ async def _list_db_services(
     }
 
 
-OLD_SERVICES_DATE: datetime = datetime(2020, 8, 19)
-
-
 async def _create_service_default_access_rights(
     app: FastAPI, service: ServiceDockerData, connection: SAConnection
 ) -> Tuple[Optional[PositiveInt], List[ServiceAccessRightsAtDB]]:
@@ -187,7 +184,9 @@ async def _ensure_registry_insync_with_db(
         )
 
 
-async def _ensure_published_templates_accessible(connection: SAConnection) -> None:
+async def _ensure_published_templates_accessible(
+    connection: SAConnection, default_product_name: str
+) -> None:
     # Rationale: if a project template was published, its services must be available to everyone.
     # a published template has a column Published that is set to True
     projects_repo = ProjectsRepository(connection)
@@ -210,7 +209,11 @@ async def _ensure_published_templates_accessible(connection: SAConnection) -> No
     missing_services = published_services - available_services
     missing_services_access_rights = [
         ServiceAccessRightsAtDB(
-            key=service[0], version=service[1], gid=everyone_gid, execute_access=True
+            key=service[0],
+            version=service[1],
+            gid=everyone_gid,
+            execute_access=True,
+            product_name=default_product_name,
         )
         for service in missing_services
     ]
@@ -224,6 +227,7 @@ async def _ensure_published_templates_accessible(connection: SAConnection) -> No
 
 async def sync_registry_task(app: FastAPI) -> None:
     # get list of services from director
+    default_product: str = app.state.settings.access_rights_default_product_name
     engine: Engine = app.state.engine
     while True:
         try:
@@ -233,20 +237,23 @@ async def sync_registry_task(app: FastAPI) -> None:
                 # check that the list of services is in sync with the registry
                 await _ensure_registry_insync_with_db(app, conn)
 
-                # check that the published services are available to everyone (templates are published to GUESTs, so their services must be also accessible)
-                await _ensure_published_templates_accessible(conn)
+                # check that the published services are available to everyone
+                # (templates are published to GUESTs, so their services must be also accessible)
+                await _ensure_published_templates_accessible(conn, default_product)
 
             await asyncio.sleep(app.state.settings.background_task_rest_time)
 
         except CancelledError:
             # task is stopped
             logger.debug("Catalog background task cancelled", exc_info=True)
             return
+
         except Exception:  # pylint: disable=broad-except
             logger.exception("Error while processing services entry")
+            # wait a bit before retrying, so it does not block everything until the director is up
             await asyncio.sleep(
-                5
-            )  # wait a bit before retrying, so it does not block everything until the director is up
+                app.state.settings.background_task_wait_after_failure
+            )
 
 
 async def start_registry_sync_task(app: FastAPI) -> None:

services/catalog/src/simcore_service_catalog/core/settings.py

@@ -92,16 +92,17 @@ def loglevel(self) -> int:
     # POSTGRES
     postgres: PostgresSettings
 
-    # Director service
+    # DIRECTOR SERVICE
     director: DirectorSettings
 
     # SERVICE SERVER (see : https://www.uvicorn.org/settings/)
     host: str = "0.0.0.0"  # nosec
     port: int = 8000
     debug: bool = False  # If True, debug tracebacks should be returned on errors.
 
-    # background task
+    # BACKGROUND TASK
     background_task_rest_time: PositiveInt = 60
+    background_task_wait_after_failure: PositiveInt = 5 # secs
     access_rights_default_product_name: str = "osparc"
 
     class Config(_CommonConfig):

services/catalog/src/simcore_service_catalog/db/repositories/services.py

@@ -131,7 +131,10 @@ async def update_service(
         return updated_service
 
     async def get_service_access_rights(
-        self, key: str, version: str, product_name: str,
+        self,
+        key: str,
+        version: str,
+        product_name: str,
     ) -> List[ServiceAccessRightsAtDB]:
         services_in_db = []
         query = sa.select([services_access_rights]).where(

services/catalog/tests/unit/test_openapi_specs.py

@@ -2,12 +2,16 @@
 # pylint:disable=unused-argument
 # pylint:disable=redefined-outer-name
 
-import pytest
 import json
+
+import pytest
+
 from simcore_service_catalog.core.application import init_app
 
 
-@pytest.mark.skip(reason="FIXME: fails with make tests but does not fail with pytest test_!???")
+@pytest.mark.skip(
+    reason="FIXME: fails with make tests but does not fail with pytest test_!???"
+)
 def test_openapi_json_is_updated(project_slug_dir, devel_environ):
     # devel_environ needed to build app
 

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

@@ -75,6 +75,9 @@ async def _request_catalog(
         return web.json_response(data, status=resp.status)
 
 
+## HANDLERS  ------------------------
+
+
 @login_required
 @permission_required("services.catalog.*")
 async def _reverse_proxy_handler(request: web.Request) -> web.Response:
@@ -114,6 +117,28 @@ async def _reverse_proxy_handler(request: web.Request) -> web.Response:
     )
 
 
+## API ------------------------
+
+
+async def get_services_for_user_in_product(
+    app: web.Application, user_id: int, product_name: str, *, only_key_versions: bool
+) -> Optional[List[Dict]]:
+    url = (
+        URL(app[f"{__name__}.catalog_origin"])
+        .with_path(app[f"{__name__}.catalog_version_prefix"] + "/services")
+        .with_query({"user_id": user_id, "details": f"{not only_key_versions}"})
+    )
+    session = get_client_session(app)
+    async with session.get(url, headers={X_PRODUCT_NAME_HEADER: product_name}) as resp:
+        if resp.status >= 400:
+            logger.error("Error while retrieving services for user %s", user_id)
+            return
+        return await resp.json()
+
+
+## SETUP ------------------------
+
+
 @app_module_setup(
     __name__,
     ModuleCategory.ADDON,
@@ -144,21 +169,3 @@ def setup_catalog(app: web.Application, *, disable_auth=False):
 
     # reverse proxy to catalog's API
     app.router.add_routes(routes)
-
-
-async def get_services_for_user(
-    app: web.Application, user_id: int, *, only_key_versions: bool
-) -> Optional[List[Dict]]:
-    url = (
-        URL(app[f"{__name__}.catalog_origin"])
-        .with_path(app[f"{__name__}.catalog_version_prefix"] + "/services")
-        .with_query({"user_id": user_id, "details": f"{not only_key_versions}"})
-    )
-
-    headers = {"X-Simcore-Products-Name": "osparc"}
-    session = get_client_session(app)
-    async with session.get(url, headers=headers) as resp:
-        if resp.status >= 400:
-            logger.error("Error while retrieving services for user %s", user_id)
-            return
-        return await resp.json()

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

@@ -5,14 +5,15 @@
 import logging
 from typing import Dict, List, Optional, Set
 
+import aioredlock
 from aiohttp import web
 from jsonschema import ValidationError
 
-import aioredlock
 from servicelib.utils import fire_and_forget_task, logged_gather
 
 from .. import catalog
 from ..computation_api import update_pipeline_db
+from ..constants import RQ_PRODUCT_KEY
 from ..login.decorators import RQT_USERID_KEY, login_required
 from ..resource_manager.websocket_manager import managed_resource
 from ..security_api import check_permission
@@ -130,7 +131,7 @@ async def list_projects(request: web.Request):
     # TODO: implement all query parameters as
     # in https://www.ibm.com/support/knowledgecenter/en/SSCRJU_3.2.0/com.ibm.swg.im.infosphere.streams.rest.api.doc/doc/restapis-queryparms-list.html
 
-    user_id = request[RQT_USERID_KEY]
+    user_id, product_name = request[RQT_USERID_KEY], request[RQ_PRODUCT_KEY]
     ptype = request.query.get("type", "all")  # TODO: get default for oaspecs
     db = request.config_dict[APP_PROJECT_DBAPI]
 
@@ -147,8 +148,10 @@ async def list_projects(request: web.Request):
 
     stop = min(start + count, len(projects_list))
     projects_list = projects_list[start:stop]
-    user_available_services: List[Dict] = await catalog.get_services_for_user(
-        request.app, user_id, only_key_versions=True
+    user_available_services: List[
+        Dict
+    ] = await catalog.get_services_for_user_in_product(
+        request.app, user_id, product_name, only_key_versions=True
     )
 
     # validate response
@@ -185,9 +188,11 @@ async def validate_project(prj: Dict) -> Optional[Dict]:
 async def get_project(request: web.Request):
     """Returns all projects accessible to a user (not necesarly owned)"""
     # TODO: temporary hidden until get_handlers_from_namespace refactor to seek marked functions instead!
-    user_id = request[RQT_USERID_KEY]
-    user_available_services: List[Dict] = await catalog.get_services_for_user(
-        request.app, user_id, only_key_versions=True
+    user_id, product_name = request[RQT_USERID_KEY], request[RQ_PRODUCT_KEY]
+    user_available_services: List[
+        Dict
+    ] = await catalog.get_services_for_user_in_product(
+        request.app, user_id, product_name, only_key_versions=True
     )
     from .projects_api import get_project_for_user
 

services/web/server/tests/integration/test_project_workflow.py

@@ -26,6 +26,7 @@
 from simcore_service_webserver.catalog import setup_catalog
 from simcore_service_webserver.db import setup_db
 from simcore_service_webserver.login import setup_login
+from simcore_service_webserver.products import setup_products
 from simcore_service_webserver.projects import setup_projects
 from simcore_service_webserver.projects.projects_fakes import Fake
 from simcore_service_webserver.projects.projects_models import ProjectState
@@ -74,6 +75,7 @@ def client(
     setup_resource_manager(app)
     assert setup_projects(app)
     setup_catalog(app)
+    setup_products(app)
 
     yield loop.run_until_complete(
         aiohttp_client(
@@ -242,7 +244,9 @@ def creator(projects: Optional[Union[List[Dict], Dict]] = None) -> None:
     async def mocked_get_services_for_user(*args, **kwargs):
         return services_in_project
 
-    monkeypatch.setattr(catalog, "get_services_for_user", mocked_get_services_for_user)
+    monkeypatch.setattr(
+        catalog, "get_services_for_user_in_product", mocked_get_services_for_user
+    )
 
     return creator
 

services/web/server/tests/unit/with_dbs/fast/test_access_to_studies.py

@@ -25,6 +25,7 @@
 from simcore_service_webserver.db import setup_db
 from simcore_service_webserver.login import setup_login
 from simcore_service_webserver.projects import setup_projects
+from simcore_service_webserver.products import setup_products
 from simcore_service_webserver.projects.projects_api import delete_project_from_db
 from simcore_service_webserver.projects.projects_models import (
     Owner,
@@ -109,9 +110,11 @@ def client(
     setup_rest(app)  # TODO: why should we need this??
     setup_login(app)
     setup_users(app)
+    setup_products(app)
     assert setup_projects(app), "Shall not skip this setup"
     assert setup_studies_access(app), "Shall not skip this setup"
 
+
     # server and client
     yield loop.run_until_complete(
         aiohttp_client(
@@ -249,7 +252,7 @@ async def catalog_subsystem_mock(monkeypatch, published_project):
     async def mocked_get_services_for_user(*args, **kwargs):
         return services_in_project
 
-    monkeypatch.setattr(catalog, "get_services_for_user", mocked_get_services_for_user)
+    monkeypatch.setattr(catalog, "get_services_for_user_in_product", mocked_get_services_for_user)
 
 
 async def test_access_study_anonymously(

services/web/server/tests/unit/with_dbs/slow/test_projects.py

@@ -28,6 +28,7 @@
 from simcore_service_webserver.db_models import UserRole
 from simcore_service_webserver.director import setup_director
 from simcore_service_webserver.login import setup_login
+from simcore_service_webserver.products import setup_products
 from simcore_service_webserver.projects import setup_projects
 from simcore_service_webserver.projects.projects_handlers import (
     OVERRIDABLE_DOCUMENT_KEYS,
@@ -64,7 +65,6 @@ def client(
     mocked_director_subsystem,
     mock_orphaned_services,
 ):
-    # def client(loop, aiohttp_client, app_cfg): # <<<< FOR DEVELOPMENT. DO NOT REMOVE.
 
     # config app
     cfg = deepcopy(app_cfg)
@@ -90,6 +90,7 @@ def client(
     setup_director(app)
     setup_tags(app)
     assert setup_projects(app)
+    setup_products(app)
 
     # server and client
     yield loop.run_until_complete(
@@ -243,7 +244,9 @@ def creator(projects: Optional[Union[List[Dict], Dict]] = None) -> None:
     async def mocked_get_services_for_user(*args, **kwargs):
         return services_in_project
 
-    monkeypatch.setattr(catalog, "get_services_for_user", mocked_get_services_for_user)
+    monkeypatch.setattr(
+        catalog, "get_services_for_user_in_product", mocked_get_services_for_user
+    )
 
     return creator
 
@@ -304,7 +307,9 @@ async def test_list_projects(
         ).locked.value, "Templates are not locked"
 
 
-async def _assert_get_same_project(client, project: Dict, expected: web.Response) -> Dict:
+async def _assert_get_same_project(
+    client, project: Dict, expected: web.Response
+) -> Dict:
     # GET /v0/projects/{project_id}
 
     # with a project owned by user
@@ -1575,6 +1580,8 @@ async def test_open_shared_project_at_same_time(
                 project_status = ProjectState(**data.pop("state"))
                 assert data == shared_project
                 assert project_status.locked.value
-                assert project_status.locked.owner.first_name in [c["user"]["name"] for c in clients]
+                assert project_status.locked.owner.first_name in [
+                    c["user"]["name"] for c in clients
+                ]
 
         assert num_assertions == NUMBER_OF_ADDITIONAL_CLIENTS