🐛 fixes issue with interactive services removal (#2507)

* fixes issue with interactive services removal

* adding regression test and fixing issue

* refactor

* added missin braket

* reverting change

* added new possible error raised during operations

* changed import

Co-authored-by: Andrei Neagu <[email protected]>

Author: 2 people

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

@@ -182,6 +182,14 @@ def _find_changed_dict_keys(
 # FIXME: not clear when data is schema-compliant and db-compliant
 
 
+def _assemble_array_groups(user_groups: List[RowProxy]) -> str:
+    return (
+        "array[]::text[]"
+        if len(user_groups) == 0
+        else f"""array[{', '.join(f"'{group.gid}'" for group in user_groups)}]"""
+    )
+
+
 class ProjectDBAPI:
     def __init__(self, app: web.Application):
         # TODO: shall be a weak pointer since it is also contained by app??
@@ -319,7 +327,6 @@ async def load_projects(
 
         async with self.engine.acquire() as conn:
             user_groups: List[RowProxy] = await self.__load_user_groups(conn, user_id)
-            groups_array = ", ".join(f"'{group.gid}'" for group in user_groups)
 
             query = (
                 select([projects])
@@ -341,7 +348,7 @@ async def load_projects(
                     )
                     & (
                         sa.text(
-                            f"jsonb_exists_any(projects.access_rights, array[{groups_array}])"
+                            f"jsonb_exists_any(projects.access_rights, {_assemble_array_groups(user_groups)})"
                         )
                         | (projects.c.prj_owner == user_id)
                     )
@@ -443,14 +450,15 @@ async def _get_project(
         user_groups: List[RowProxy] = await self.__load_user_groups(connection, user_id)
 
         # NOTE: in order to use specific postgresql function jsonb_exists_any we use raw call here
+
         query = textwrap.dedent(
             f"""\
             SELECT *
             FROM projects
             WHERE
             {"" if include_templates else "projects.type != 'TEMPLATE' AND"}
             uuid = '{project_uuid}'
-            AND (jsonb_exists_any(projects.access_rights, array[{', '.join(f"'{group.gid}'" for group in user_groups)}])
+            AND (jsonb_exists_any(projects.access_rights, {_assemble_array_groups(user_groups)})
             OR prj_owner = {user_id})
             {"FOR UPDATE" if for_update else ""}
             """

services/web/server/src/simcore_service_webserver/resource_manager/garbage_collector.py

@@ -4,11 +4,11 @@
 from typing import Any, Dict, List, Optional, Set, Tuple
 
 import asyncpg.exceptions
-import psycopg2
 from aiohttp import web
 from aiopg.sa.result import RowProxy
 from aioredlock import Aioredlock
 from servicelib.utils import logged_gather
+from simcore_postgres_database.errors import DatabaseError
 
 from .. import users_exceptions
 from ..db_models import GroupType
@@ -37,7 +37,11 @@
 from .registry import RedisResourceRegistry, get_registry
 
 logger = logging.getLogger(__name__)
-database_errors = (psycopg2.DatabaseError, asyncpg.exceptions.PostgresError)
+database_errors = (
+    DatabaseError,
+    asyncpg.exceptions.PostgresError,
+    ProjectNotFoundError,
+)
 
 TASK_NAME = f"{__name__}.collect_garbage_periodically"
 TASK_CONFIG = f"{TASK_NAME}.config"
@@ -264,11 +268,22 @@ async def remove_disconnected_user_resources(
                 if resource_name == "project_id":
                     # inform that the project can be closed on the backend side
                     #
-                    await remove_project_interactive_services(
-                        user_id=int(dead_key["user_id"]),
-                        project_uuid=resource_value,
-                        app=app,
-                    )
+                    try:
+                        await remove_project_interactive_services(
+                            user_id=int(dead_key["user_id"]),
+                            project_uuid=resource_value,
+                            app=app,
+                        )
+                    except ProjectNotFoundError as err:
+                        logger.warning(
+                            (
+                                "Could not remove project interactive services user_id=%s "
+                                "project_uuid=%s. Check the logs above for details [%s]"
+                            ),
+                            user_id,
+                            resource_value,
+                            err,
+                        )
 
                 # ONLY GUESTS: if this user was a GUEST also remove it from the database
                 # with the only associated project owned

services/web/server/tests/unit/with_dbs/10/test_resource_manager.py

@@ -6,7 +6,7 @@
 import asyncio
 import json
 import logging
-from asyncio import Future, sleep
+from asyncio import Future
 from copy import deepcopy
 from typing import Any, Callable, Dict
 from unittest.mock import call
@@ -20,8 +20,6 @@
 from aioredis import Redis
 from aioresponses import aioresponses
 from pytest_mock.plugin import MockerFixture
-from tenacity import after_log, retry_if_exception_type, stop_after_attempt, wait_fixed
-
 from pytest_simcore.helpers.utils_assert import assert_status
 from pytest_simcore.helpers.utils_projects import NewProject
 from servicelib.application import create_safe_application
@@ -30,6 +28,11 @@
 from simcore_service_webserver.director_v2 import setup_director_v2
 from simcore_service_webserver.login import setup_login
 from simcore_service_webserver.projects import setup_projects
+from simcore_service_webserver.projects.projects_api import (
+    delete_project_from_db,
+    remove_project_interactive_services,
+)
+from simcore_service_webserver.projects.projects_exceptions import ProjectNotFoundError
 from simcore_service_webserver.resource_manager import (
     config,
     garbage_collector,
@@ -46,6 +49,8 @@
 from simcore_service_webserver.socketio import setup_socketio
 from simcore_service_webserver.socketio.events import SOCKET_IO_PROJECT_UPDATED_EVENT
 from simcore_service_webserver.users import setup_users
+from simcore_service_webserver.users_api import delete_user
+from tenacity import after_log, retry_if_exception_type, stop_after_attempt, wait_fixed
 
 logger = logging.getLogger(__name__)
 
@@ -71,6 +76,14 @@ def mock_garbage_collector_task(mocker):
     )
 
 
+@pytest.fixture
+def mock_delete_data_folders_for_project(mocker):
+    mocker.patch(
+        "simcore_service_webserver.projects.projects_api.delete_data_folders_of_project",
+        return_value=None,
+    )
+
+
 @pytest.fixture
 def client(
     mock_garbage_collector_task,
@@ -707,3 +720,30 @@ async def test_websocket_disconnected_remove_or_maintain_files_based_on_role(
         storage_subsystem_mock[1].assert_not_called()
         # make sure `delete_user` not called
         # asyncpg_storage_system_mock.assert_not_called()
+
+
+@pytest.mark.parametrize("user_role", [UserRole.USER, UserRole.TESTER, UserRole.GUEST])
+async def test_regression_removing_unexisting_user(
+    client,
+    logged_user,
+    empty_user_project,
+    user_role,
+    mock_delete_data_folders_for_project,
+) -> None:
+    # regression test for https://github.com/ITISFoundation/osparc-simcore/issues/2504
+
+    # remove project
+    await delete_project_from_db(
+        app=client.server.app,
+        project_uuid=empty_user_project["uuid"],
+        user_id=logged_user["id"],
+    )
+    # remove user
+    await delete_user(app=client.server.app, user_id=logged_user["id"])
+
+    with pytest.raises(ProjectNotFoundError):
+        await remove_project_interactive_services(
+            user_id=logged_user["id"],
+            project_uuid=empty_user_project["uuid"],
+            app=client.server.app,
+        )