From 9200ef2ee9d5f40636eab972318248fb5b28915b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 18 Feb 2025 11:08:05 +0100 Subject: [PATCH 01/44] api --- .../garbage_collector/_tasks_trash.py | 4 ++-- .../server/src/simcore_service_webserver/trash/_service.py | 6 +++--- .../src/simcore_service_webserver/trash/trash_service.py | 4 ++++ 3 files changed, 9 insertions(+), 5 deletions(-) create mode 100644 services/web/server/src/simcore_service_webserver/trash/trash_service.py diff --git a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py index 47d5e7212f23..e22c87da5003 100644 --- a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py +++ b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py @@ -12,7 +12,7 @@ from tenacity.before_sleep import before_sleep_log from tenacity.wait import wait_exponential -from ..trash._service import prune_trash +from ..trash import trash_service _logger = logging.getLogger(__name__) @@ -28,7 +28,7 @@ before_sleep=before_sleep_log(_logger, logging.WARNING), ) async def _run_task(app: web.Application): - if deleted := await prune_trash(app): + if deleted := await trash_service.delete_expired_trash(app): for name in deleted: _logger.info("Trash item %s expired and was deleted", f"{name}") else: diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index 5e3ec8ed6dd2..091177aa5aea 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -62,7 +62,7 @@ async def _empty_explicitly_trashed_projects( ) -async def _empty_trashed_folders( +async def _empty_explicitly_trashed_folders_and_content( app: web.Application, product_name: ProductName, user_id: UserID ): trashed_folders_ids = await folders_trash_service.list_explicitly_trashed_folders( @@ -104,10 +104,10 @@ async def empty_trash_safe( ): await _empty_explicitly_trashed_projects(app, product_name, user_id) - await _empty_trashed_folders(app, product_name, user_id) + await _empty_explicitly_trashed_folders_and_content(app, product_name, user_id) -async def prune_trash(app: web.Application) -> list[str]: +async def delete_expired_trash(app: web.Application) -> list[str]: """Deletes expired items in the trash""" settings = get_plugin_settings(app) diff --git a/services/web/server/src/simcore_service_webserver/trash/trash_service.py b/services/web/server/src/simcore_service_webserver/trash/trash_service.py new file mode 100644 index 000000000000..44da62aba7f1 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/trash/trash_service.py @@ -0,0 +1,4 @@ +from ._service import delete_expired_trash + +__all__: tuple[str, ...] = ("delete_expired_trash",) +# nopycln: file From e78e912c91b1c9fc9715cc97cb952a8d40df114a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 18 Feb 2025 15:02:11 +0100 Subject: [PATCH 02/44] drafts tests --- .../tests/unit/with_dbs/03/trash/conftest.py | 81 ++++++++++++ .../with_dbs/03/{ => trash}/test_trash.py | 52 +------- .../with_dbs/03/trash/test_trash_service.py | 116 ++++++++++++++++++ .../server/tests/unit/with_dbs/conftest.py | 2 +- 4 files changed, 201 insertions(+), 50 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/03/trash/conftest.py rename services/web/server/tests/unit/with_dbs/03/{ => trash}/test_trash.py (95%) create mode 100644 services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py diff --git a/services/web/server/tests/unit/with_dbs/03/trash/conftest.py b/services/web/server/tests/unit/with_dbs/03/trash/conftest.py new file mode 100644 index 000000000000..fef8cd774437 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/trash/conftest.py @@ -0,0 +1,81 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +from collections.abc import AsyncIterable, Callable +from pathlib import Path + +import pytest +from aiohttp.test_utils import TestClient +from aioresponses import aioresponses +from models_library.products import ProductName +from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict +from pytest_simcore.helpers.typing_env import EnvVarsDict +from pytest_simcore.helpers.webserver_login import NewUser, UserInfoDict +from pytest_simcore.helpers.webserver_parametrizations import MockedStorageSubsystem +from pytest_simcore.helpers.webserver_projects import NewProject +from simcore_service_webserver.projects.models import ProjectDict + + +@pytest.fixture +def app_environment( + app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch +) -> EnvVarsDict: + return app_environment | setenvs_from_dict( + monkeypatch, {"WEBSERVER_DEV_FEATURES_ENABLED": "1"} + ) + + +@pytest.fixture +async def other_user( + client: TestClient, logged_user: UserInfoDict +) -> AsyncIterable[UserInfoDict]: + # new user different from logged_user + async with NewUser( + { + "name": f"other_user_than_{logged_user['name']}", + "role": "USER", + }, + client.app, + ) as user: + yield user + + +@pytest.fixture +async def other_user_project( + client: TestClient, + fake_project: ProjectDict, + other_user: UserInfoDict, + tests_data_dir: Path, + osparc_product_name: ProductName, +) -> AsyncIterable[ProjectDict]: + async with NewProject( + fake_project, + client.app, + user_id=other_user["id"], + product_name=osparc_product_name, + tests_data_dir=tests_data_dir, + ) as project: + yield project + + +@pytest.fixture +def mocked_catalog( + user_project: ProjectDict, + catalog_subsystem_mock: Callable[[list[ProjectDict]], None], +): + catalog_subsystem_mock([user_project]) + + +@pytest.fixture +def mocked_director_v2(director_v2_service_mock: aioresponses): + ... + + +@pytest.fixture +def mocked_storage(storage_subsystem_mock: MockedStorageSubsystem): + ... diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py similarity index 95% rename from services/web/server/tests/unit/with_dbs/03/test_trash.py rename to services/web/server/tests/unit/with_dbs/03/trash/test_trash.py index a1ec3bd8c977..b9d28d694661 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py @@ -7,24 +7,20 @@ import asyncio -from collections.abc import AsyncIterable, Callable +from collections.abc import AsyncIterable from unittest.mock import MagicMock from uuid import UUID import arrow import pytest from aiohttp.test_utils import TestClient -from aioresponses import aioresponses from models_library.api_schemas_webserver.folders_v2 import FolderGet from models_library.api_schemas_webserver.projects import ProjectGet, ProjectListItem from models_library.api_schemas_webserver.workspaces import WorkspaceGet from models_library.rest_pagination import Page from pytest_mock import MockerFixture from pytest_simcore.helpers.assert_checks import assert_status -from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict -from pytest_simcore.helpers.typing_env import EnvVarsDict -from pytest_simcore.helpers.webserver_login import NewUser, UserInfoDict -from pytest_simcore.helpers.webserver_parametrizations import MockedStorageSubsystem +from pytest_simcore.helpers.webserver_login import UserInfoDict from servicelib.aiohttp import status from simcore_service_webserver.db.models import UserRole from simcore_service_webserver.projects._groups_api import ProjectGroupGet @@ -33,38 +29,11 @@ from yarl import URL -@pytest.fixture -def app_environment( - app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch -) -> EnvVarsDict: - return app_environment | setenvs_from_dict( - monkeypatch, {"WEBSERVER_DEV_FEATURES_ENABLED": "1"} - ) - - @pytest.fixture def user_role() -> UserRole: return UserRole.USER -@pytest.fixture -def mocked_catalog( - user_project: ProjectDict, - catalog_subsystem_mock: Callable[[list[ProjectDict]], None], -): - catalog_subsystem_mock([user_project]) - - -@pytest.fixture -def mocked_director_v2(director_v2_service_mock: aioresponses): - ... - - -@pytest.fixture -def mocked_storage(storage_subsystem_mock: MockedStorageSubsystem): - ... - - @pytest.mark.acceptance_test( "For https://github.com/ITISFoundation/osparc-simcore/pull/6579" ) @@ -190,21 +159,6 @@ async def test_trash_projects( # noqa: PLR0915 mock_remove_dynamic_services.assert_awaited() -@pytest.fixture -async def other_user( - client: TestClient, logged_user: UserInfoDict -) -> AsyncIterable[UserInfoDict]: - # new user different from logged_user - async with NewUser( - { - "name": f"other_user_than_{logged_user['name']}", - "role": "USER", - }, - client.app, - ) as user: - yield user - - async def test_trash_projects_shared_among_users( client: TestClient, logged_user: UserInfoDict, @@ -432,7 +386,7 @@ async def test_trash_folder_with_content( await assert_status(resp, status.HTTP_200_OK) page = Page[FolderGet].model_validate(await resp.json()) assert page.meta.total == 1 - assert page.data[0].folder_id == folder.folder_id + assert page.data[0] == folder resp = await client.get( "/v0/folders", diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py new file mode 100644 index 000000000000..8885672fea72 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py @@ -0,0 +1,116 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +import contextlib +from collections.abc import AsyncIterator + +import pytest +from aiohttp.test_utils import TestClient +from models_library.api_schemas_webserver.projects import ProjectGet +from pytest_simcore.helpers.assert_checks import assert_status +from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict +from pytest_simcore.helpers.typing_env import EnvVarsDict +from pytest_simcore.helpers.webserver_login import UserInfoDict +from servicelib.aiohttp import status +from simcore_service_webserver.db.models import UserRole +from simcore_service_webserver.projects import _trash_service +from simcore_service_webserver.projects.models import ProjectDict +from simcore_service_webserver.trash._service import delete_expired_trash + + +@pytest.fixture +def app_environment( + app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch +) -> EnvVarsDict: + return app_environment | setenvs_from_dict( + monkeypatch, {"TRASH_RETENTION_DAYS": "0"} + ) + + +@pytest.fixture +def user_role() -> UserRole: + return UserRole.USER + + +@contextlib.asynccontextmanager +async def _client_session_with_user( + client: TestClient, user: UserInfoDict +) -> AsyncIterator[TestClient]: + assert client.app + + url = client.app.router["logout"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, status.HTTP_200_OK) + + url = client.app.router["login"].url_for() + resp = await client.post( + f"{url}", + json={ + "email": user["email"], + "password": user["raw_password"], + }, + ) + await assert_status(resp, status.HTTP_200_OK) + + yield client + + url = client.app.router["logout"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, status.HTTP_200_OK) + + +async def test_trash_service__delete_expired_trash( + client: TestClient, + logged_user: UserInfoDict, + user_project: ProjectDict, + other_user: UserInfoDict, + other_user_project: ProjectDict, + mocked_catalog: None, + mocked_director_v2: None, +): + assert client.app + assert logged_user["id"] != other_user["id"] + + # TRASH projects + # logged_user trashes his project + user_project_id = user_project["uuid"] + await _trash_service.trash_project( + client.app, + product_name="osparc", + user_id=logged_user["id"], + project_id=user_project_id, + force_stop_first=True, + explicit=True, + ) + + # other_user trashes his project + other_user_project_id = other_user_project["uuid"] + await _trash_service.trash_project( + client.app, + product_name="osparc", + user_id=other_user["id"], + project_id=other_user_project_id, + force_stop_first=True, + explicit=True, + ) + + resp = await client.get(f"/v0/projects/{user_project_id}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + assert ProjectGet.model_validate(data).trashed_by == logged_user["primary_gid"] + + # UNDER TEST: Run delete_expired_trash + await delete_expired_trash(client.app) + + # ASSERT: logged_user tries to get the project and expects 404 + resp = await client.get(f"/v0/projects/{user_project_id}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) + + # ASSERT: other_user tries to get the project and expects 404 + async with _client_session_with_user(client, other_user): + resp = await client.get(f"/v0/projects/{other_user_project_id}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) diff --git a/services/web/server/tests/unit/with_dbs/conftest.py b/services/web/server/tests/unit/with_dbs/conftest.py index d583e3c783e4..5b40152bea5a 100644 --- a/services/web/server/tests/unit/with_dbs/conftest.py +++ b/services/web/server/tests/unit/with_dbs/conftest.py @@ -631,7 +631,7 @@ async def user_project( fake_project: ProjectDict, logged_user: UserInfoDict, tests_data_dir: Path, - osparc_product_name: str, + osparc_product_name: ProductName, ) -> AsyncIterator[ProjectDict]: async with NewProject( fake_project, From 96949f9e25c3d033929e50d8793c9e8faf05b3aa Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 18 Feb 2025 23:21:37 +0100 Subject: [PATCH 03/44] change example --- .../src/models_library/licenses.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/models-library/src/models_library/licenses.py b/packages/models-library/src/models_library/licenses.py index c9d927ff9004..b65b7f9d6feb 100644 --- a/packages/models-library/src/models_library/licenses.py +++ b/packages/models-library/src/models_library/licenses.py @@ -26,16 +26,18 @@ class LicensedResourceType(StrAutoEnum): VIP_MODEL = auto() -VIP_FEATURES_EXAMPLE = { - "name": "Duke", - "version": "V2.0", - "sex": "Mas bien poco", +_VIP_FEATURES_EXAMPLE = { + # NOTE: this view is how it would be after parsed and validated "age": "34 years", - "weight": "70.2 Kg", - "height": "1.77 m", "date": "2015-03-01", "ethnicity": "Caucasian", "functionality": "Static", + "height": "1.77 m", + "name": "Duke", + "sex": "Male", + "version": "V2.0", + "weight": "70.2 Kg", + # other "additional_field": "allowed", } @@ -58,7 +60,7 @@ class FeaturesDict(TypedDict): "id": 1, "description": "A detailed description of the VIP model", "thumbnail": "https://example.com/thumbnail.jpg", - "features": VIP_FEATURES_EXAMPLE, + "features": _VIP_FEATURES_EXAMPLE, "doi": "10.1000/xyz123", "license_key": "ABC123XYZ", "license_version": "1.0", From eac98c4944d658acf4533cb50c15faa70bc2957d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 14:21:33 +0100 Subject: [PATCH 04/44] adds mock for bg --- .../garbage_collector/plugin.py | 22 ++++++++++--------- .../tests/unit/with_dbs/03/trash/conftest.py | 21 ++++++++++++++++++ .../with_dbs/03/trash/test_trash_service.py | 14 ++++++++---- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py b/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py index 3e76c6c947ce..663d94166cdf 100644 --- a/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py +++ b/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py @@ -3,17 +3,12 @@ from aiohttp import web from servicelib.aiohttp.application_setup import ModuleCategory, app_module_setup from servicelib.logging_utils import set_parent_module_log_level -from simcore_service_webserver.garbage_collector._tasks_trash import ( - create_background_task_to_prune_trash, -) from ..application_settings import get_application_settings from ..login.plugin import setup_login_storage from ..projects.db import setup_projects_db from ..socketio.plugin import setup_socketio -from ._tasks_api_keys import create_background_task_to_prune_api_keys -from ._tasks_core import run_background_task -from ._tasks_users import create_background_task_for_trial_accounts +from . import _tasks_api_keys, _tasks_core, _tasks_trash, _tasks_users from .settings import get_plugin_settings _logger = logging.getLogger(__name__) @@ -35,7 +30,7 @@ def setup_garbage_collector(app: web.Application) -> None: settings = get_plugin_settings(app) - app.cleanup_ctx.append(run_background_task) + app.cleanup_ctx.append(_tasks_core.run_background_task) set_parent_module_log_level( _logger.name, min(logging.INFO, get_application_settings(app).log_level) @@ -48,10 +43,17 @@ def setup_garbage_collector(app: web.Application) -> None: # If more tasks of this nature are needed, we should setup some sort of registration mechanism # with a interface such that plugins can pass tasks to the GC plugin to handle them interval_s = settings.GARBAGE_COLLECTOR_EXPIRED_USERS_CHECK_INTERVAL_S - app.cleanup_ctx.append(create_background_task_for_trial_accounts(interval_s)) + app.cleanup_ctx.append( + _tasks_users.create_background_task_for_trial_accounts(interval_s) + ) # SEE https://github.com/ITISFoundation/osparc-issues/issues/705 wait_period_s = settings.GARBAGE_COLLECTOR_PRUNE_APIKEYS_INTERVAL_S - app.cleanup_ctx.append(create_background_task_to_prune_api_keys(wait_period_s)) + app.cleanup_ctx.append( + _tasks_api_keys.create_background_task_to_prune_api_keys(wait_period_s) + ) - app.cleanup_ctx.append(create_background_task_to_prune_trash(wait_period_s)) + # SEE https://github.com/ITISFoundation/osparc-issues#468 + app.cleanup_ctx.append( + _tasks_trash.create_background_task_to_prune_trash(wait_period_s) + ) diff --git a/services/web/server/tests/unit/with_dbs/03/trash/conftest.py b/services/web/server/tests/unit/with_dbs/03/trash/conftest.py index fef8cd774437..5c742b12144d 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/conftest.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/conftest.py @@ -6,13 +6,17 @@ # pylint: disable=unused-variable +import logging from collections.abc import AsyncIterable, Callable from pathlib import Path import pytest +from aiohttp import web from aiohttp.test_utils import TestClient from aioresponses import aioresponses from models_library.products import ProductName +from pytest_mock import MockerFixture +from pytest_simcore.helpers.logging_tools import log_context from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict from pytest_simcore.helpers.webserver_login import NewUser, UserInfoDict @@ -20,6 +24,8 @@ from pytest_simcore.helpers.webserver_projects import NewProject from simcore_service_webserver.projects.models import ProjectDict +_logger = logging.getLogger(__name__) + @pytest.fixture def app_environment( @@ -79,3 +85,18 @@ def mocked_director_v2(director_v2_service_mock: aioresponses): @pytest.fixture def mocked_storage(storage_subsystem_mock: MockedStorageSubsystem): ... + + +@pytest.fixture +def with_disabled_background_task_to_prune_trash(mocker: MockerFixture) -> None: + async def _empty_lifespan(app: web.Application): + with log_context( + logging.INFO, "Fake background_task_to_prune_trash event", logger=_logger + ): + yield + + mocker.patch( + "simcore_service_webserver.garbage_collector._tasks_trash.create_background_task_to_prune_trash", + autospec=True, + return_value=_empty_lifespan, + ) diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py index 8885672fea72..586f7a7f6660 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py @@ -20,15 +20,21 @@ from simcore_service_webserver.db.models import UserRole from simcore_service_webserver.projects import _trash_service from simcore_service_webserver.projects.models import ProjectDict -from simcore_service_webserver.trash._service import delete_expired_trash +from simcore_service_webserver.trash import trash_service @pytest.fixture def app_environment( - app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch + app_environment: EnvVarsDict, + monkeypatch: pytest.MonkeyPatch, + with_disabled_background_task_to_prune_trash: None, ) -> EnvVarsDict: return app_environment | setenvs_from_dict( - monkeypatch, {"TRASH_RETENTION_DAYS": "0"} + monkeypatch, + { + "TRASH_RETENTION_DAYS": "0", + "WEBSERVER_GARBAGE_COLLECTOR": "null", + }, ) @@ -104,7 +110,7 @@ async def test_trash_service__delete_expired_trash( assert ProjectGet.model_validate(data).trashed_by == logged_user["primary_gid"] # UNDER TEST: Run delete_expired_trash - await delete_expired_trash(client.app) + await trash_service.delete_expired_trash(client.app) # ASSERT: logged_user tries to get the project and expects 404 resp = await client.get(f"/v0/projects/{user_project_id}") From 7dee6181fb3d2a1b5f4ae1af25f1bbc6304c905d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 15:19:38 +0100 Subject: [PATCH 05/44] minor --- .../simcore_service_webserver/folders/_folders_repository.py | 4 +--- .../src/simcore_service_webserver/folders/_trash_service.py | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 0d2842c878ed..e9fa358af300 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -1,6 +1,6 @@ import logging from datetime import datetime -from typing import Final, cast +from typing import cast import sqlalchemy as sa from aiohttp import web @@ -41,8 +41,6 @@ _logger = logging.getLogger(__name__) -_unset: Final = UnSet() - _FOLDER_DB_MODEL_COLS = get_columns_from_db_model(folders_v2, FolderDB) diff --git a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py index 4b5c6f3cdf00..51c0990c6d89 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py @@ -227,7 +227,7 @@ async def list_explicitly_trashed_folders( user_id=user_id, product_name=product_name, text=None, - trashed=True, + trashed=True, # NOTE: lists only expliclty trashed! offset=page_params.offset, limit=page_params.limit, order_by=OrderBy(field=IDStr("trashed"), direction=OrderDirection.ASC), From 64f4af768e8abfd10632b4ad6d73fe8bfe43a8a9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:02:28 +0100 Subject: [PATCH 06/44] updates folders query --- .../src/common_library/exclude.py | 4 ++ .../folders/_folders_repository.py | 65 +++++++++++++++++-- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/packages/common-library/src/common_library/exclude.py b/packages/common-library/src/common_library/exclude.py index 7f2392dec338..e24efb998c4e 100644 --- a/packages/common-library/src/common_library/exclude.py +++ b/packages/common-library/src/common_library/exclude.py @@ -12,6 +12,10 @@ def is_unset(v: Any) -> bool: return isinstance(v, UnSet) +def is_set(v: Any) -> bool: + return not isinstance(v, UnSet) + + def as_dict_exclude_unset(**params) -> dict[str, Any]: return {k: v for k, v in params.items() if not isinstance(v, UnSet)} diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index e9fa358af300..5f7f549926e4 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -4,7 +4,7 @@ import sqlalchemy as sa from aiohttp import web -from common_library.exclude import UnSet, as_dict_exclude_unset +from common_library.exclude import UnSet, as_dict_exclude_unset, is_set from models_library.folders import ( FolderDB, FolderID, @@ -35,6 +35,7 @@ from sqlalchemy.ext.asyncio import AsyncConnection from sqlalchemy.orm import aliased from sqlalchemy.sql import ColumnElement, CompoundSelect, Select, asc, desc, select +from sqlalchemy.sql.selectable import GenerativeSelect from ..db.plugin import get_asyncpg_engine from .errors import FolderAccessForbiddenError, FolderNotFoundError @@ -155,6 +156,17 @@ def _create_shared_workspace_query( return shared_workspace_query +def _set_ordering( + base_query: GenerativeSelect, + order_by: OrderBy, +) -> GenerativeSelect: + if order_by.direction == OrderDirection.ASC: + list_query = base_query.order_by(asc(getattr(folders_v2.c, order_by.field))) + else: + list_query = base_query.order_by(desc(getattr(folders_v2.c, order_by.field))) + return list_query + + async def list_( # pylint: disable=too-many-arguments,too-many-branches app: web.Application, connection: AsyncConnection | None = None, @@ -235,12 +247,7 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches count_query = select(func.count()).select_from(combined_query.subquery()) # Ordering and pagination - if order_by.direction == OrderDirection.ASC: - list_query = combined_query.order_by(asc(getattr(folders_v2.c, order_by.field))) - else: - list_query = combined_query.order_by( - desc(getattr(folders_v2.c, order_by.field)) - ) + list_query = _set_ordering(combined_query, order_by=order_by) list_query = list_query.offset(offset).limit(limit) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: @@ -253,6 +260,50 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches return cast(int, total_count), folders +async def list_trashed_folders( + app: web.Application, + connection: AsyncConnection | None = None, + *, + # filter + trashed_explicitly: bool | UnSet = UnSet.VALUE, + trashed_until: datetime | UnSet = UnSet.VALUE, + # pagination + offset: NonNegativeInt, + limit: int, + # order + order_by: OrderBy, +) -> tuple[int, list[FolderDB]]: + """ + NOTE: this is app-wide i.e. no product, user or workspace filtered + TODO: check with MD about workspaces + """ + base_query = select(_FOLDER_DB_MODEL_COLS).where(folders_v2.c.trashed.is_not(None)) + + if is_set(trashed_explicitly): + assert isinstance(trashed_explicitly, bool) # nosec + base_query = base_query.where( + folders_v2.c.trashed_explicitly.is_(trashed_explicitly) + ) + + if is_set(trashed_until): + assert isinstance(trashed_until, datetime) # nosec + base_query = base_query.where(folders_v2.c.trashed < trashed_until) + + # Select total count from base_query + count_query = select(func.count()).select_from(base_query.subquery()) + + # Ordering and pagination + list_query = _set_ordering(base_query, order_by) + list_query = list_query.offset(offset).limit(limit) + + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + total_count = await conn.scalar(count_query) + + result = await conn.stream(list_query) + folders: list[FolderDB] = [FolderDB.model_validate(row) async for row in result] + return cast(int, total_count), folders + + def _create_base_select_query(folder_id: FolderID, product_name: ProductName) -> Select: return select(*_FOLDER_DB_MODEL_COLS,).where( (folders_v2.c.product_name == product_name) From 8dd82a85df9f0f2af9f9926f0a79303f1b321ce0 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:43:08 +0100 Subject: [PATCH 07/44] batch delete folders --- .../folders/_folders_repository.py | 8 ++-- .../folders/_trash_service.py | 46 ++++++++++++++++++- .../folders/errors.py | 4 ++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 5f7f549926e4..024114f30f18 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -266,7 +266,7 @@ async def list_trashed_folders( *, # filter trashed_explicitly: bool | UnSet = UnSet.VALUE, - trashed_until: datetime | UnSet = UnSet.VALUE, + trashed_before: datetime | UnSet = UnSet.VALUE, # pagination offset: NonNegativeInt, limit: int, @@ -285,9 +285,9 @@ async def list_trashed_folders( folders_v2.c.trashed_explicitly.is_(trashed_explicitly) ) - if is_set(trashed_until): - assert isinstance(trashed_until, datetime) # nosec - base_query = base_query.where(folders_v2.c.trashed < trashed_until) + if is_set(trashed_before): + assert isinstance(trashed_before, datetime) # nosec + base_query = base_query.where(folders_v2.c.trashed < trashed_before) # Select total count from base_query count_query = select(func.count()).select_from(base_query.subquery()) diff --git a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py index 51c0990c6d89..531b6d66c46e 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py @@ -19,7 +19,7 @@ from ..projects._trash_service import trash_project, untrash_project from ..workspaces.api import check_user_workspace_access from . import _folders_repository, _folders_service -from .errors import FolderNotTrashedError +from .errors import FolderBatchDeleteError, FolderNotTrashedError _logger = logging.getLogger(__name__) @@ -278,3 +278,47 @@ async def delete_trashed_folder( await _folders_service.delete_folder( app, user_id=user_id, folder_id=folder_id, product_name=product_name ) + + +async def batch_delete_trashed_folders_as_admin( + app: web.Application, + trashed_before: datetime, + *, + product_name: ProductName, + fail_fast: bool, +) -> None: + """ + Raises: + FolderBatchDeleteError: if error and fail_fast=False + Exception: any other exception during delete_recursively + """ + + for page_params in iter_pagination_params(limit=MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE): + ( + page_params.total_number_of_items, + expired_trashed_folders, + ) = await _folders_repository.list_trashed_folders( + app, + trashed_explicitly=True, + trashed_before=trashed_before, + offset=page_params.offset, + limit=page_params.limit, + order_by=OrderBy(field=IDStr("trashed"), direction=OrderDirection.ASC), + ) + + # batch delete + errors: list[tuple[FolderID, Exception]] = [] + for folder in expired_trashed_folders: + try: + await _folders_repository.delete_recursively( + app, folder_id=folder.folder_id, product_name=product_name + ) + except Exception as err: # pylint: disable=broad-exception-caught + if fail_fast: + raise + errors.append((folder.folder_id, err)) + + if errors: + raise FolderBatchDeleteError( + errors=errors, trashed_before=trashed_before, product_name=product_name + ) diff --git a/services/web/server/src/simcore_service_webserver/folders/errors.py b/services/web/server/src/simcore_service_webserver/folders/errors.py index 6dc9b93cc54c..e8f2e346868b 100644 --- a/services/web/server/src/simcore_service_webserver/folders/errors.py +++ b/services/web/server/src/simcore_service_webserver/folders/errors.py @@ -29,3 +29,7 @@ class FolderNotTrashedError(FoldersRuntimeError): msg_template = ( "Cannot delete folder {folder_id} since it was not trashed first: {reason}" ) + + +class FolderBatchDeleteError(FoldersRuntimeError): + msg_template = "One or more folders could not be deleted: {errors}" From 7399b3d3f012b1aaf9b66f89c161389829990434 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:52:41 +0100 Subject: [PATCH 08/44] conneting with trash.service --- .../folders/folders_trash_service.py | 7 ++++- .../garbage_collector/_tasks_trash.py | 2 +- .../trash/_service.py | 30 ++++++++++++++----- .../trash/trash_service.py | 4 +-- .../with_dbs/03/trash/test_trash_service.py | 2 +- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/folders_trash_service.py b/services/web/server/src/simcore_service_webserver/folders/folders_trash_service.py index 9d1b012731e9..505f72f257f3 100644 --- a/services/web/server/src/simcore_service_webserver/folders/folders_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/folders_trash_service.py @@ -1,6 +1,11 @@ -from ._trash_service import delete_trashed_folder, list_explicitly_trashed_folders +from ._trash_service import ( + batch_delete_trashed_folders_as_admin, + delete_trashed_folder, + list_explicitly_trashed_folders, +) __all__: tuple[str, ...] = ( + "batch_delete_trashed_folders_as_admin", "delete_trashed_folder", "list_explicitly_trashed_folders", ) diff --git a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py index e22c87da5003..bb6ab51f2919 100644 --- a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py +++ b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py @@ -28,7 +28,7 @@ before_sleep=before_sleep_log(_logger, logging.WARNING), ) async def _run_task(app: web.Application): - if deleted := await trash_service.delete_expired_trash(app): + if deleted := await trash_service.delete_expired_trash_as_admin(app): for name in deleted: _logger.info("Trash item %s expired and was deleted", f"{name}") else: diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index 091177aa5aea..4632728b346d 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -1,4 +1,3 @@ -import asyncio import logging from datetime import timedelta from typing import Final @@ -107,19 +106,34 @@ async def empty_trash_safe( await _empty_explicitly_trashed_folders_and_content(app, product_name, user_id) -async def delete_expired_trash(app: web.Application) -> list[str]: - """Deletes expired items in the trash""" +async def delete_expired_trash_as_admin(app: web.Application) -> list[str]: settings = get_plugin_settings(app) # app-wide retention = timedelta(days=settings.TRASH_RETENTION_DAYS) - expiration_dt = arrow.now().datetime - retention + delete_until = arrow.now().datetime - retention - _logger.debug( + with log_context( + _logger, + logging.DEBUG, "CODE PLACEHOLDER: **ALL** items marked as trashed during %s days are deleted (those marked before %s)", retention, - expiration_dt, - ) - await asyncio.sleep(5) + delete_until, + ): + try: + await folders_trash_service.batch_delete_trashed_folders_as_admin( + app, trashed_before=delete_until, product_name="TODO", fail_fast=False + ) + except Exception as exc: # pylint: disable=broad-exception-caught + _logger.warning( + **create_troubleshotting_log_kwargs( + "Error deleting a trashed folders (and content) that were expired.", + error=exc, + error_context={ + "delete_until": delete_until, + "retention": retention, + }, + ) + ) return [] diff --git a/services/web/server/src/simcore_service_webserver/trash/trash_service.py b/services/web/server/src/simcore_service_webserver/trash/trash_service.py index 44da62aba7f1..1fbea54ac990 100644 --- a/services/web/server/src/simcore_service_webserver/trash/trash_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/trash_service.py @@ -1,4 +1,4 @@ -from ._service import delete_expired_trash +from ._service import delete_expired_trash_as_admin -__all__: tuple[str, ...] = ("delete_expired_trash",) +__all__: tuple[str, ...] = ("delete_expired_trash_as_admin",) # nopycln: file diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py index 586f7a7f6660..3802093c1df5 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py @@ -110,7 +110,7 @@ async def test_trash_service__delete_expired_trash( assert ProjectGet.model_validate(data).trashed_by == logged_user["primary_gid"] # UNDER TEST: Run delete_expired_trash - await trash_service.delete_expired_trash(client.app) + await trash_service.delete_expired_trash_as_admin(client.app) # ASSERT: logged_user tries to get the project and expects 404 resp = await client.get(f"/v0/projects/{user_project_id}") From 7225cd6269a469056e3de6dbd5162111dcd953e7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:10:10 +0100 Subject: [PATCH 09/44] drafts test --- .../with_dbs/03/trash/test_trash_service.py | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py index 3802093c1df5..ba54085aa0b1 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py @@ -120,3 +120,100 @@ async def test_trash_service__delete_expired_trash( async with _client_session_with_user(client, other_user): resp = await client.get(f"/v0/projects/{other_user_project_id}") await assert_status(resp, status.HTTP_404_NOT_FOUND) + + +async def test_trash_nested_folders_and_projects( + client: TestClient, + logged_user: UserInfoDict, + user_project: ProjectDict, + other_user: UserInfoDict, + other_user_project: ProjectDict, + mocked_catalog: None, + mocked_director_v2: None, +): + assert client.app + assert logged_user["id"] != other_user["id"] + + # Create folders hierarchy for logged_user + resp = await client.post("/v0/folders", json={"name": "Root Folder"}) + data, _ = await assert_status(resp, status.HTTP_201_CREATED) + logged_user_root_folder = data + + resp = await client.post( + "/v0/folders", + json={ + "name": "Sub Folder", + "parentFolderId": logged_user_root_folder["folderId"], + }, + ) + data, _ = await assert_status(resp, status.HTTP_201_CREATED) + logged_user_sub_folder = data + + # Move project to subfolder + resp = await client.put( + f"/v0/projects/{user_project['uuid']}/folders/{logged_user_sub_folder['folderId']}" + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # Trash root folders + resp = await client.post(f"/v0/folders/{logged_user_root_folder['folderId']}:trash") + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # Create folders hierarchy for other_user + async with _client_session_with_user(client, other_user): + resp = await client.post("/v0/folders", json={"name": "Root Folder"}) + data, _ = await assert_status(resp, status.HTTP_201_CREATED) + other_user_root_folder = data + + resp = await client.post( + "/v0/folders", + json={ + "name": "Sub Folder", + "parentFolderId": other_user_root_folder["folderId"], + }, + ) + data, _ = await assert_status(resp, status.HTTP_201_CREATED) + other_user_sub_folder = data + + # Move project to subfolder + resp = await client.put( + f"/v0/projects/{other_user_project['uuid']}/folders/{other_user_sub_folder['folderId']}" + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # Trash root folders + resp = await client.post( + f"/v0/folders/{logged_user_root_folder['folderId']}:trash" + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + async with _client_session_with_user(client, other_user): + resp = await client.post( + f"/v0/folders/{other_user_root_folder['folderId']}:trash" + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # UNDER TEST + await trash_service.delete_expired_trash_as_admin(client.app) + + async with _client_session_with_user(client, logged_user): + # Verify logged_user's resources are gone + resp = await client.get(f"/v0/folders/{logged_user_root_folder['folderId']}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) + + resp = await client.get(f"/v0/folders/{logged_user_sub_folder['folderId']}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) + + resp = await client.get(f"/v0/projects/{user_project['uuid']}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) + + # Verify other_user's resources are gone + async with _client_session_with_user(client, other_user): + resp = await client.get(f"/v0/folders/{other_user_root_folder['folderId']}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) + + resp = await client.get(f"/v0/folders/{other_user_sub_folder['folderId']}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) + + resp = await client.get(f"/v0/projects/{other_user_project['uuid']}") + await assert_status(resp, status.HTTP_404_NOT_FOUND) From 63f09d3d1af26d0d55059284a0485231511be9a4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:16:03 +0100 Subject: [PATCH 10/44] periodic task and products --- .../garbage_collector/_tasks_trash.py | 8 +-- .../trash/_service.py | 60 +++++++++++-------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py index bb6ab51f2919..c6366110e372 100644 --- a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py +++ b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py @@ -8,6 +8,7 @@ from collections.abc import AsyncIterator, Callable from aiohttp import web +from servicelib.logging_utils import log_context from tenacity import retry from tenacity.before_sleep import before_sleep_log from tenacity.wait import wait_exponential @@ -28,11 +29,8 @@ before_sleep=before_sleep_log(_logger, logging.WARNING), ) async def _run_task(app: web.Application): - if deleted := await trash_service.delete_expired_trash_as_admin(app): - for name in deleted: - _logger.info("Trash item %s expired and was deleted", f"{name}") - else: - _logger.info("No trash items expired") + with log_context(_logger, logging.INFO, "Deleting expired trashed items"): + await trash_service.delete_expired_trash_as_admin(app) async def _run_periodically(app: web.Application, wait_interval_s: float): diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index 4632728b346d..7784925d4b5a 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -8,6 +8,7 @@ from models_library.users import UserID from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.logging_utils import log_context +from simcore_service_webserver.products import _api as products_service from ..folders import folders_trash_service from ..projects import projects_trash_service @@ -106,34 +107,43 @@ async def empty_trash_safe( await _empty_explicitly_trashed_folders_and_content(app, product_name, user_id) -async def delete_expired_trash_as_admin(app: web.Application) -> list[str]: +async def delete_expired_trash_as_admin(app: web.Application) -> list: settings = get_plugin_settings(app) - - # app-wide retention = timedelta(days=settings.TRASH_RETENTION_DAYS) delete_until = arrow.now().datetime - retention - with log_context( - _logger, - logging.DEBUG, - "CODE PLACEHOLDER: **ALL** items marked as trashed during %s days are deleted (those marked before %s)", - retention, - delete_until, - ): - try: - await folders_trash_service.batch_delete_trashed_folders_as_admin( - app, trashed_before=delete_until, product_name="TODO", fail_fast=False - ) - except Exception as exc: # pylint: disable=broad-exception-caught - _logger.warning( - **create_troubleshotting_log_kwargs( - "Error deleting a trashed folders (and content) that were expired.", - error=exc, - error_context={ - "delete_until": delete_until, - "retention": retention, - }, + deleted = [] # TODO: delete count of all items? ids? delete-stats? + for product in products_service.list_products(app): + with log_context( + _logger, + logging.DEBUG, + "Deleting items marked as trashed before %s in %s [retention=%s]", + retention, + product.display_name, + delete_until, + ): + try: + + await folders_trash_service.batch_delete_trashed_folders_as_admin( + app, + trashed_before=delete_until, + product_name=product.name, + fail_fast=False, + ) + + except Exception as exc: # pylint: disable=broad-exception-caught + _logger.warning( + **create_troubleshotting_log_kwargs( + "Error batch deleting expired trashed folders as admin.", + error=exc, + error_context={ + "delete_until": delete_until, + "retention": retention, + "product_name": product.name, + }, + ) ) - ) - return [] + # TODO: batch delete trashed projects here + + return deleted From 16253253ecc7b36a0452e88d46187a0b57c6204a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:32:30 +0100 Subject: [PATCH 11/44] cleanup --- .../folders/_folders_repository.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 024114f30f18..e6c479bd51fa 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -1,6 +1,6 @@ import logging from datetime import datetime -from typing import cast +from typing import Callable, cast import sqlalchemy as sa from aiohttp import web @@ -160,11 +160,11 @@ def _set_ordering( base_query: GenerativeSelect, order_by: OrderBy, ) -> GenerativeSelect: - if order_by.direction == OrderDirection.ASC: - list_query = base_query.order_by(asc(getattr(folders_v2.c, order_by.field))) - else: - list_query = base_query.order_by(desc(getattr(folders_v2.c, order_by.field))) - return list_query + direction_func: Callable = {OrderDirection.ASC: asc, OrderDirection: desc}[ + order_by.direction + ] + column = getattr(folders_v2.c, order_by.field) + return base_query.order_by(direction_func(column)) async def list_( # pylint: disable=too-many-arguments,too-many-branches @@ -247,8 +247,9 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches count_query = select(func.count()).select_from(combined_query.subquery()) # Ordering and pagination - list_query = _set_ordering(combined_query, order_by=order_by) - list_query = list_query.offset(offset).limit(limit) + list_query = ( + _set_ordering(combined_query, order_by=order_by).offset(offset).limit(limit) + ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: total_count = await conn.scalar(count_query) @@ -293,8 +294,7 @@ async def list_trashed_folders( count_query = select(func.count()).select_from(base_query.subquery()) # Ordering and pagination - list_query = _set_ordering(base_query, order_by) - list_query = list_query.offset(offset).limit(limit) + list_query = _set_ordering(base_query, order_by).offset(offset).limit(limit) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: total_count = await conn.scalar(count_query) From d26921ddb51203d3b8fc47f0fc46b7da1a7af834 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:56:14 +0100 Subject: [PATCH 12/44] cleanup --- .../folders/_folders_repository.py | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index e6c479bd51fa..7246285f732a 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -1,6 +1,7 @@ import logging +from collections.abc import Callable from datetime import datetime -from typing import Callable, cast +from typing import cast import sqlalchemy as sa from aiohttp import web @@ -31,10 +32,10 @@ from simcore_postgres_database.utils_workspaces_sql import ( create_my_workspace_access_rights_subquery, ) -from sqlalchemy import func +from sqlalchemy import sql from sqlalchemy.ext.asyncio import AsyncConnection from sqlalchemy.orm import aliased -from sqlalchemy.sql import ColumnElement, CompoundSelect, Select, asc, desc, select +from sqlalchemy.sql import ColumnElement, CompoundSelect, Select from sqlalchemy.sql.selectable import GenerativeSelect from ..db.plugin import get_asyncpg_engine @@ -71,8 +72,8 @@ async def create( user_id=user_id, workspace_id=workspace_id, created_by_gid=created_by_gid, - created=func.now(), - modified=func.now(), + created=sql.func.now(), + modified=sql.func.now(), ) .returning(*_FOLDER_DB_MODEL_COLS) ) @@ -91,9 +92,9 @@ def _create_private_workspace_query( WorkspaceScope.ALL, ) return ( - select( + sql.select( *_FOLDER_DB_MODEL_COLS, - func.json_build_object( + sql.func.json_build_object( "read", sa.text("true"), "write", @@ -128,7 +129,7 @@ def _create_shared_workspace_query( ) shared_workspace_query = ( - select( + sql.select( *_FOLDER_DB_MODEL_COLS, workspace_access_rights_subquery.c.my_access_rights, ) @@ -160,7 +161,7 @@ def _set_ordering( base_query: GenerativeSelect, order_by: OrderBy, ) -> GenerativeSelect: - direction_func: Callable = {OrderDirection.ASC: asc, OrderDirection: desc}[ + direction_func: Callable = {OrderDirection.ASC: sql.asc, OrderDirection: sql.asc}[ order_by.direction ] column = getattr(folders_v2.c, order_by.field) @@ -244,7 +245,7 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches raise ValueError(msg) # Select total count from base_query - count_query = select(func.count()).select_from(combined_query.subquery()) + count_query = sql.select(sql.func.count()).select_from(combined_query.subquery()) # Ordering and pagination list_query = ( @@ -278,7 +279,9 @@ async def list_trashed_folders( NOTE: this is app-wide i.e. no product, user or workspace filtered TODO: check with MD about workspaces """ - base_query = select(_FOLDER_DB_MODEL_COLS).where(folders_v2.c.trashed.is_not(None)) + base_query = sql.select(_FOLDER_DB_MODEL_COLS).where( + folders_v2.c.trashed.is_not(None) + ) if is_set(trashed_explicitly): assert isinstance(trashed_explicitly, bool) # nosec @@ -291,7 +294,7 @@ async def list_trashed_folders( base_query = base_query.where(folders_v2.c.trashed < trashed_before) # Select total count from base_query - count_query = select(func.count()).select_from(base_query.subquery()) + count_query = sql.select(sql.func.count()).select_from(base_query.subquery()) # Ordering and pagination list_query = _set_ordering(base_query, order_by).offset(offset).limit(limit) @@ -305,7 +308,7 @@ async def list_trashed_folders( def _create_base_select_query(folder_id: FolderID, product_name: ProductName) -> Select: - return select(*_FOLDER_DB_MODEL_COLS,).where( + return sql.select(*_FOLDER_DB_MODEL_COLS,).where( (folders_v2.c.product_name == product_name) & (folders_v2.c.folder_id == folder_id) ) @@ -392,7 +395,7 @@ async def update( ) query = ( - (folders_v2.update().values(modified=func.now(), **updated)) + (folders_v2.update().values(modified=sql.func.now(), **updated)) .where(folders_v2.c.product_name == product_name) .returning(*_FOLDER_DB_MODEL_COLS) ) @@ -421,7 +424,7 @@ async def delete_recursively( ) -> None: async with transaction_context(get_asyncpg_engine(app), connection) as conn: # Step 1: Define the base case for the recursive CTE - base_query = select( + base_query = sql.select( folders_v2.c.folder_id, folders_v2.c.parent_folder_id ).where( (folders_v2.c.folder_id == folder_id) # <-- specified folder id @@ -431,7 +434,7 @@ async def delete_recursively( # Step 2: Define the recursive case folder_alias = aliased(folders_v2) - recursive_query = select( + recursive_query = sql.select( folder_alias.c.folder_id, folder_alias.c.parent_folder_id ).select_from( folder_alias.join( @@ -444,7 +447,7 @@ async def delete_recursively( folder_hierarchy_cte = folder_hierarchy_cte.union_all(recursive_query) # Step 4: Execute the query to get all descendants - final_query = select(folder_hierarchy_cte) + final_query = sql.select(folder_hierarchy_cte) result = await conn.stream(final_query) # list of tuples [(folder_id, parent_folder_id), ...] ex. [(1, None), (2, 1)] rows = [row async for row in result] @@ -479,7 +482,7 @@ async def get_projects_recursively_only_if_user_is_owner( async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: # Step 1: Define the base case for the recursive CTE - base_query = select( + base_query = sql.select( folders_v2.c.folder_id, folders_v2.c.parent_folder_id ).where( (folders_v2.c.folder_id == folder_id) # <-- specified folder id @@ -489,7 +492,7 @@ async def get_projects_recursively_only_if_user_is_owner( # Step 2: Define the recursive case folder_alias = aliased(folders_v2) - recursive_query = select( + recursive_query = sql.select( folder_alias.c.folder_id, folder_alias.c.parent_folder_id ).select_from( folder_alias.join( @@ -502,13 +505,13 @@ async def get_projects_recursively_only_if_user_is_owner( folder_hierarchy_cte = folder_hierarchy_cte.union_all(recursive_query) # Step 4: Execute the query to get all descendants - final_query = select(folder_hierarchy_cte) + final_query = sql.select(folder_hierarchy_cte) result = await conn.stream(final_query) # list of tuples [(folder_id, parent_folder_id), ...] ex. [(1, None), (2, 1)] folder_ids = [item[0] async for item in result] query = ( - select(projects_to_folders.c.project_uuid) + sql.select(projects_to_folders.c.project_uuid) .join(projects) .where( (projects_to_folders.c.folder_id.in_(folder_ids)) @@ -537,7 +540,7 @@ async def get_all_folders_and_projects_ids_recursively( async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: # Step 1: Define the base case for the recursive CTE - base_query = select( + base_query = sql.select( folders_v2.c.folder_id, folders_v2.c.parent_folder_id ).where( (folders_v2.c.folder_id == folder_id) # <-- specified folder id @@ -547,7 +550,7 @@ async def get_all_folders_and_projects_ids_recursively( # Step 2: Define the recursive case folder_alias = aliased(folders_v2) - recursive_query = select( + recursive_query = sql.select( folder_alias.c.folder_id, folder_alias.c.parent_folder_id ).select_from( folder_alias.join( @@ -560,12 +563,12 @@ async def get_all_folders_and_projects_ids_recursively( folder_hierarchy_cte = folder_hierarchy_cte.union_all(recursive_query) # Step 4: Execute the query to get all descendants - final_query = select(folder_hierarchy_cte) + final_query = sql.select(folder_hierarchy_cte) result = await conn.stream(final_query) # list of tuples [(folder_id, parent_folder_id), ...] ex. [(1, None), (2, 1)] folder_ids = [item.folder_id async for item in result] - query = select(projects_to_folders.c.project_uuid).where( + query = sql.select(projects_to_folders.c.project_uuid).where( (projects_to_folders.c.folder_id.in_(folder_ids)) & (projects_to_folders.c.user_id == private_workspace_user_id_or_none) ) @@ -586,7 +589,7 @@ async def get_folders_recursively( async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: # Step 1: Define the base case for the recursive CTE - base_query = select( + base_query = sql.select( folders_v2.c.folder_id, folders_v2.c.parent_folder_id ).where( (folders_v2.c.folder_id == folder_id) # <-- specified folder id @@ -596,7 +599,7 @@ async def get_folders_recursively( # Step 2: Define the recursive case folder_alias = aliased(folders_v2) - recursive_query = select( + recursive_query = sql.select( folder_alias.c.folder_id, folder_alias.c.parent_folder_id ).select_from( folder_alias.join( @@ -609,13 +612,13 @@ async def get_folders_recursively( folder_hierarchy_cte = folder_hierarchy_cte.union_all(recursive_query) # Step 4: Execute the query to get all descendants - final_query = select(folder_hierarchy_cte) + final_query = sql.select(folder_hierarchy_cte) result = await conn.stream(final_query) return cast(list[FolderID], [row.folder_id async for row in result]) def _select_trashed_by_primary_gid_query(): - return sa.select( + return sa.sql.select( folders_v2.c.folder_id, users.c.primary_gid.label("trashed_by_primary_gid"), ).select_from( From b7c634e7a9e698b32bf2592061ed9ed5b143bb14 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 18:05:20 +0100 Subject: [PATCH 13/44] drafts batch delete projects --- .../garbage_collector/_tasks_trash.py | 2 +- .../projects/_trash_service.py | 11 ++++++ .../projects/projects_trash_service.py | 2 ++ .../simcore_service_webserver/trash/_rest.py | 2 +- .../trash/_service.py | 35 ++++++++++++++----- .../trash/trash_service.py | 4 +-- .../with_dbs/03/trash/test_trash_service.py | 4 +-- 7 files changed, 46 insertions(+), 14 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py index c6366110e372..46df72c0a708 100644 --- a/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py +++ b/services/web/server/src/simcore_service_webserver/garbage_collector/_tasks_trash.py @@ -30,7 +30,7 @@ ) async def _run_task(app: web.Application): with log_context(_logger, logging.INFO, "Deleting expired trashed items"): - await trash_service.delete_expired_trash_as_admin(app) + await trash_service.safe_delete_expired_trash_as_admin(app) async def _run_periodically(app: web.Application, wait_interval_s: float): diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py index df0e342946dc..1016cac25f57 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py @@ -237,3 +237,14 @@ async def delete_explicitly_trashed_project( user_id=user_id, project_uuid=project_id, ) + + +async def batch_delete_trashed_projects_as_admin( + app: web.Application, + trashed_before: datetime, + *, + product_name: ProductName, + fail_fast: bool, +) -> None: + + raise NotImplementedError diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_trash_service.py index 6548eae4567c..2270ca66e6c8 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_trash_service.py @@ -1,9 +1,11 @@ from ._trash_service import ( + batch_delete_trashed_projects_as_admin, delete_explicitly_trashed_project, list_explicitly_trashed_projects, ) __all__: tuple[str, ...] = ( + "batch_delete_trashed_projects_as_admin", "delete_explicitly_trashed_project", "list_explicitly_trashed_projects", ) diff --git a/services/web/server/src/simcore_service_webserver/trash/_rest.py b/services/web/server/src/simcore_service_webserver/trash/_rest.py index 9cb7bd62d9cc..ac61d4c735ff 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_rest.py +++ b/services/web/server/src/simcore_service_webserver/trash/_rest.py @@ -58,7 +58,7 @@ async def empty_trash(request: web.Request): product_name = get_product_name(request) fire_and_forget_task( - _service.empty_trash_safe( + _service.safe_empty_trash( request.app, product_name=product_name, user_id=user_id ), task_suffix_name="rest.empty_trash", diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index 7784925d4b5a..5e7bc37750d9 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -99,7 +99,7 @@ async def _empty_explicitly_trashed_folders_and_content( ) -async def empty_trash_safe( +async def safe_empty_trash( app: web.Application, *, product_name: ProductName, user_id: UserID ): await _empty_explicitly_trashed_projects(app, product_name, user_id) @@ -107,13 +107,20 @@ async def empty_trash_safe( await _empty_explicitly_trashed_folders_and_content(app, product_name, user_id) -async def delete_expired_trash_as_admin(app: web.Application) -> list: +async def safe_delete_expired_trash_as_admin(app: web.Application) -> list: settings = get_plugin_settings(app) retention = timedelta(days=settings.TRASH_RETENTION_DAYS) delete_until = arrow.now().datetime - retention deleted = [] # TODO: delete count of all items? ids? delete-stats? for product in products_service.list_products(app): + + ctx = { + "delete_until": delete_until, + "retention": retention, + "product_name": product.name, + } + with log_context( _logger, logging.DEBUG, @@ -136,14 +143,26 @@ async def delete_expired_trash_as_admin(app: web.Application) -> list: **create_troubleshotting_log_kwargs( "Error batch deleting expired trashed folders as admin.", error=exc, - error_context={ - "delete_until": delete_until, - "retention": retention, - "product_name": product.name, - }, + error_context=ctx, ) ) - # TODO: batch delete trashed projects here + try: + + await projects_trash_service.batch_delete_trashed_projects_as_admin( + app, + trashed_before=delete_until, + product_name=product.name, + fail_fast=False, + ) + + except Exception as exc: # pylint: disable=broad-exception-caught + _logger.warning( + **create_troubleshotting_log_kwargs( + "Error batch deleting expired projects as admin.", + error=exc, + error_context=ctx, + ) + ) return deleted diff --git a/services/web/server/src/simcore_service_webserver/trash/trash_service.py b/services/web/server/src/simcore_service_webserver/trash/trash_service.py index 1fbea54ac990..3cd438f4e1c4 100644 --- a/services/web/server/src/simcore_service_webserver/trash/trash_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/trash_service.py @@ -1,4 +1,4 @@ -from ._service import delete_expired_trash_as_admin +from ._service import safe_delete_expired_trash_as_admin -__all__: tuple[str, ...] = ("delete_expired_trash_as_admin",) +__all__: tuple[str, ...] = ("safe_delete_expired_trash_as_admin",) # nopycln: file diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py index ba54085aa0b1..5481f9df1db0 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py @@ -110,7 +110,7 @@ async def test_trash_service__delete_expired_trash( assert ProjectGet.model_validate(data).trashed_by == logged_user["primary_gid"] # UNDER TEST: Run delete_expired_trash - await trash_service.delete_expired_trash_as_admin(client.app) + await trash_service.safe_delete_expired_trash_as_admin(client.app) # ASSERT: logged_user tries to get the project and expects 404 resp = await client.get(f"/v0/projects/{user_project_id}") @@ -194,7 +194,7 @@ async def test_trash_nested_folders_and_projects( await assert_status(resp, status.HTTP_204_NO_CONTENT) # UNDER TEST - await trash_service.delete_expired_trash_as_admin(client.app) + await trash_service.safe_delete_expired_trash_as_admin(client.app) async with _client_session_with_user(client, logged_user): # Verify logged_user's resources are gone From c59411d6ed9e678d9ad69f40562ef50599d27a1b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 19:55:13 +0100 Subject: [PATCH 14/44] fix tests --- .../web/server/tests/integration/01/test_garbage_collection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/tests/integration/01/test_garbage_collection.py b/services/web/server/tests/integration/01/test_garbage_collection.py index 62075ff6ba09..57f23d093029 100644 --- a/services/web/server/tests/integration/01/test_garbage_collection.py +++ b/services/web/server/tests/integration/01/test_garbage_collection.py @@ -198,7 +198,7 @@ async def _fake_background_task(app: web.Application): await asyncio.sleep(0.1) return mocker.patch( - "simcore_service_webserver.garbage_collector.plugin.run_background_task", + "simcore_service_webserver.garbage_collector.plugin._tasks_core.run_background_task", side_effect=_fake_background_task, ) From eaeae5aa88a629b4ba9a0382e862434af69a04d9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 21:11:34 +0100 Subject: [PATCH 15/44] cleanup --- .../server/src/simcore_service_webserver/trash/_service.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index 5e7bc37750d9..661753e5c45b 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -107,12 +107,11 @@ async def safe_empty_trash( await _empty_explicitly_trashed_folders_and_content(app, product_name, user_id) -async def safe_delete_expired_trash_as_admin(app: web.Application) -> list: +async def safe_delete_expired_trash_as_admin(app: web.Application) -> None: settings = get_plugin_settings(app) retention = timedelta(days=settings.TRASH_RETENTION_DAYS) delete_until = arrow.now().datetime - retention - deleted = [] # TODO: delete count of all items? ids? delete-stats? for product in products_service.list_products(app): ctx = { @@ -164,5 +163,3 @@ async def safe_delete_expired_trash_as_admin(app: web.Application) -> list: error_context=ctx, ) ) - - return deleted From bd78272e9300f758603b71ef2a09e7b38b946a2a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 21:15:29 +0100 Subject: [PATCH 16/44] fxi --- .../folders/_folders_repository.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 7246285f732a..42f83ef104dd 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -161,10 +161,11 @@ def _set_ordering( base_query: GenerativeSelect, order_by: OrderBy, ) -> GenerativeSelect: - direction_func: Callable = {OrderDirection.ASC: sql.asc, OrderDirection: sql.asc}[ - order_by.direction - ] - column = getattr(folders_v2.c, order_by.field) + direction_func: Callable = { + OrderDirection.ASC: sql.asc, + OrderDirection.DESC: sql.desc, + }[order_by.direction] + column = folders_v2.columns[order_by.field] return base_query.order_by(direction_func(column)) From f0da1ad4345b7ff10c689c4da146aea9cc7db2c5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 21:37:54 +0100 Subject: [PATCH 17/44] drafts list_trashed_projects --- .../folders/_folders_repository.py | 15 ++-- .../projects/_projects_db.py | 68 +++++++++++++++++-- 2 files changed, 70 insertions(+), 13 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 42f83ef104dd..072e944688ea 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -36,7 +36,6 @@ from sqlalchemy.ext.asyncio import AsyncConnection from sqlalchemy.orm import aliased from sqlalchemy.sql import ColumnElement, CompoundSelect, Select -from sqlalchemy.sql.selectable import GenerativeSelect from ..db.plugin import get_asyncpg_engine from .errors import FolderAccessForbiddenError, FolderNotFoundError @@ -157,16 +156,12 @@ def _create_shared_workspace_query( return shared_workspace_query -def _set_ordering( - base_query: GenerativeSelect, - order_by: OrderBy, -) -> GenerativeSelect: +def _to_expression(order_by: OrderBy): direction_func: Callable = { OrderDirection.ASC: sql.asc, OrderDirection.DESC: sql.desc, }[order_by.direction] - column = folders_v2.columns[order_by.field] - return base_query.order_by(direction_func(column)) + return direction_func(folders_v2.columns[order_by.field]) async def list_( # pylint: disable=too-many-arguments,too-many-branches @@ -250,7 +245,7 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches # Ordering and pagination list_query = ( - _set_ordering(combined_query, order_by=order_by).offset(offset).limit(limit) + combined_query.order_by(_to_expression(order_by)).offset(offset).limit(limit) ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: @@ -298,7 +293,9 @@ async def list_trashed_folders( count_query = sql.select(sql.func.count()).select_from(base_query.subquery()) # Ordering and pagination - list_query = _set_ordering(base_query, order_by).offset(offset).limit(limit) + list_query = ( + base_query.order_by(_to_expression(order_by)).offset(offset).limit(limit) + ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: total_count = await conn.scalar(count_query) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index c9571de658d7..3eb842e5e7dc 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -1,9 +1,15 @@ import logging +from datetime import datetime +from typing import Callable, cast -import sqlalchemy as sa from aiohttp import web +from common_library.exclude import UnSet, is_set +from models_library.basic_types import IDStr from models_library.groups import GroupID from models_library.projects import ProjectID +from models_library.rest_ordering import OrderBy, OrderDirection +from models_library.rest_pagination import MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE +from pydantic import NonNegativeInt, PositiveInt from simcore_postgres_database.models.projects import projects from simcore_postgres_database.models.users import users from simcore_postgres_database.utils_repos import ( @@ -29,6 +35,60 @@ ProjectDBGet, ) +_OLDEST_TRASHED_FIRST = OrderBy(field=IDStr("trashed"), direction=OrderDirection.ASC) + + +def _to_expression(order_by: OrderBy): + direction_func: Callable = { + OrderDirection.ASC: sql.asc, + OrderDirection.DESC: sql.desc, + }[order_by.direction] + return direction_func(projects.columns[order_by.field]) + + +async def list_trashed_projects( + app: web.Application, + connection: AsyncConnection | None = None, + *, + # filter + trashed_explicitly: bool | UnSet = UnSet.VALUE, + trashed_before: datetime | UnSet = UnSet.VALUE, + # pagination + offset: NonNegativeInt = 0, + limit: PositiveInt = MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE, + # order + order_by: OrderBy = _OLDEST_TRASHED_FIRST, +) -> tuple[int, list[ProjectDBGet]]: + + base_query = sql.select(PROJECT_DB_COLS).where(projects.c.trashed.is_not(None)) + + if is_set(trashed_explicitly): + assert isinstance(trashed_explicitly, bool) # nosec + base_query = base_query.where( + projects.c.trashed_explicitly.is_(trashed_explicitly) + ) + + if is_set(trashed_before): + assert isinstance(trashed_before, datetime) # nosec + base_query = base_query.where(projects.c.trashed < trashed_before) + + # Select total count from base_query + count_query = sql.select(sql.func.count()).select_from(base_query.subquery()) + + # Ordering and pagination + list_query = ( + base_query.order_by(_to_expression(order_by)).offset(offset).limit(limit) + ) + + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + total_count = await conn.scalar(count_query) + + result = await conn.stream(list_query) + folders: list[ProjectDBGet] = [ + ProjectDBGet.model_validate(row) async for row in result + ] + return cast(int, total_count), folders + async def patch_project( app: web.Application, @@ -41,7 +101,7 @@ async def patch_project( async with transaction_context(get_asyncpg_engine(app), connection) as conn: result = await conn.stream( projects.update() - .values(last_change_date=sa.func.now(), **new_partial_project_data) + .values(last_change_date=sql.func.now(), **new_partial_project_data) .where(projects.c.uuid == f"{project_uuid}") .returning(*PROJECT_DB_COLS) ) @@ -52,7 +112,7 @@ async def patch_project( def _select_trashed_by_primary_gid_query() -> sql.Select: - return sa.select( + return sql.select( projects.c.uuid, users.c.primary_gid.label("trashed_by_primary_gid"), ).select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) @@ -97,7 +157,7 @@ async def batch_get_trashed_by_primary_gid( ).order_by( # Preserves the order of folders_ids # SEE https://docs.sqlalchemy.org/en/20/core/sqlelement.html#sqlalchemy.sql.expression.case - sa.case( + sql.case( { project_uuid: index for index, project_uuid in enumerate(projects_uuids_str) From 62b2f6ec85a38c05d2c5bb27d5b66c40fb920446 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 21:49:23 +0100 Subject: [PATCH 18/44] implements batch_delete_trashed_projects_as_admin --- .../folders/_trash_service.py | 12 ++--- .../projects/_trash_service.py | 54 +++++++++++++++++-- .../projects/exceptions.py | 4 ++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py index 531b6d66c46e..179becd7b01b 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py @@ -292,6 +292,7 @@ async def batch_delete_trashed_folders_as_admin( FolderBatchDeleteError: if error and fail_fast=False Exception: any other exception during delete_recursively """ + errors: list[tuple[FolderID, Exception]] = [] for page_params in iter_pagination_params(limit=MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE): ( @@ -306,8 +307,7 @@ async def batch_delete_trashed_folders_as_admin( order_by=OrderBy(field=IDStr("trashed"), direction=OrderDirection.ASC), ) - # batch delete - errors: list[tuple[FolderID, Exception]] = [] + # BATCH delete for folder in expired_trashed_folders: try: await _folders_repository.delete_recursively( @@ -318,7 +318,7 @@ async def batch_delete_trashed_folders_as_admin( raise errors.append((folder.folder_id, err)) - if errors: - raise FolderBatchDeleteError( - errors=errors, trashed_before=trashed_before, product_name=product_name - ) + if errors: + raise FolderBatchDeleteError( + errors=errors, trashed_before=trashed_before, product_name=product_name + ) diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py index 1016cac25f57..fec52da9a90d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py @@ -17,12 +17,16 @@ from ..director_v2 import api as director_v2_api from ..dynamic_scheduler import api as dynamic_scheduler_api -from . import _crud_api_read, projects_service +from . import _crud_api_read +from . import _projects_db as _projects_repository +from . import projects_service from ._access_rights_api import check_user_project_permission +from ._projects_db import _OLDEST_TRASHED_FIRST from .exceptions import ( ProjectNotFoundError, ProjectNotTrashedError, ProjectRunningConflictError, + ProjectsBatchDeleteError, ) from .models import ProjectDict, ProjectPatchInternalExtended @@ -241,10 +245,52 @@ async def delete_explicitly_trashed_project( async def batch_delete_trashed_projects_as_admin( app: web.Application, - trashed_before: datetime, *, product_name: ProductName, + trashed_before: datetime, fail_fast: bool, -) -> None: +) -> list[ProjectID]: + + deleted_project_ids: list[ProjectID] = [] + errors: list[tuple[ProjectID, Exception]] = [] + + for page_params in iter_pagination_params(limit=MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE): + ( + page_params.total_number_of_items, + expired_trashed_projects, + ) = await _projects_repository.list_trashed_projects( + app, + trashed_explicitly=True, + trashed_before=trashed_before, + offset=page_params.offset, + limit=page_params.limit, + order_by=_OLDEST_TRASHED_FIRST, + ) + # BATCH delete + for project in expired_trashed_projects: + + assert project.trashed # nosec + assert project.trashed_explicitly # nosec + + try: + _logger.debug( + # TODO: _projects_service_delete.delete_project_as_admin + "await _projects_service_delete.delete_project_as_admin(app, project_id=%s, product_name=%s)", + project.uuid, + product_name, + ) + deleted_project_ids.append(project.uuid) + except Exception as err: # pylint: disable=broad-exception-caught + if fail_fast: + raise + errors.append((project.uuid, err)) + + if errors: + raise ProjectsBatchDeleteError( + errors=errors, + trashed_before=trashed_before, + product_name=product_name, + deleted_project_ids=deleted_project_ids, + ) - raise NotImplementedError + return deleted_project_ids diff --git a/services/web/server/src/simcore_service_webserver/projects/exceptions.py b/services/web/server/src/simcore_service_webserver/projects/exceptions.py index 0cec5bb84b66..8c270f99df5f 100644 --- a/services/web/server/src/simcore_service_webserver/projects/exceptions.py +++ b/services/web/server/src/simcore_service_webserver/projects/exceptions.py @@ -79,6 +79,10 @@ def __init__(self, *, project_uuid, reason, **ctx): self.reason = reason +class ProjectsBatchDeleteError(BaseProjectError): + msg_template = "One or more projects could not be deleted in the batch: {errors}" + + class ProjectTrashError(BaseProjectError): ... From cb774ae252aa637a51a71da8b0651b40aebef0cf Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 22:28:09 +0100 Subject: [PATCH 19/44] db and repo --- .../projects/_projects_db.py | 56 ++++++++++++---- .../projects/_projects_service_delete.py | 66 +++++++++++++++++++ 2 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 3eb842e5e7dc..58f82cc7f98e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -90,22 +90,16 @@ async def list_trashed_projects( return cast(int, total_count), folders -async def patch_project( +async def get_project( app: web.Application, connection: AsyncConnection | None = None, *, project_uuid: ProjectID, - new_partial_project_data: dict, ) -> ProjectDBGet: - - async with transaction_context(get_asyncpg_engine(app), connection) as conn: - result = await conn.stream( - projects.update() - .values(last_change_date=sql.func.now(), **new_partial_project_data) - .where(projects.c.uuid == f"{project_uuid}") - .returning(*PROJECT_DB_COLS) - ) - row = await result.first() + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + query = sql.select(PROJECT_DB_COLS).where(projects.c.uuid == f"{project_uuid}") + result = await conn.execute(query) + row = result.one_or_none() if row is None: raise ProjectNotFoundError(project_uuid=project_uuid) return ProjectDBGet.model_validate(row) @@ -170,3 +164,43 @@ async def batch_get_trashed_by_primary_gid( rows = {row.uuid: row.trashed_by_primary_gid async for row in result} return [rows.get(uuid) for uuid in projects_uuids_str] + + +async def patch_project( + app: web.Application, + connection: AsyncConnection | None = None, + *, + project_uuid: ProjectID, + new_partial_project_data: dict, +) -> None: + + async with transaction_context(get_asyncpg_engine(app), connection) as conn: + result = await conn.stream( + projects.update() + .values( + **new_partial_project_data, + last_change_date=sql.func.now(), + ) + .where(projects.c.uuid == f"{project_uuid}") + ) + row = await result.one_or_none() + if row is None: + raise ProjectNotFoundError(project_uuid=project_uuid) + + +async def delete_project( + app: web.Application, + connection: AsyncConnection | None = None, + *, + project_uuid: ProjectID, +) -> ProjectDBGet: + async with transaction_context(get_asyncpg_engine(app), connection) as conn: + result = await conn.stream( + projects.delete() + .where(projects.c.uuid == f"{project_uuid}") + .returning(*PROJECT_DB_COLS) + ) + row = await result.one_or_none() + if row is None: + raise ProjectNotFoundError(project_uuid=project_uuid) + return ProjectDBGet.model_validate(row) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py new file mode 100644 index 000000000000..e852b1485240 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py @@ -0,0 +1,66 @@ +# NOTE: should replace _crud_api_delete.py:delete_project + +import logging +from typing import Protocol + +from aiohttp import web +from models_library.projects import ProjectID +from servicelib.redis._errors import ProjectLockError +from simcore_service_director_v2.core.errors import ProjectNotFoundError +from simcore_service_webserver.projects.exceptions import ProjectDeleteError + +from . import _projects_db as _projects_repository + +_logger = logging.getLogger(__name__) + + +class StopServicesCallback(Protocol): + async def __call__(self, app: web.Application, project_uuid: ProjectID) -> None: + ... + + +async def delete_project_as_admin( + app: web.Application, + *, + project_uuid: ProjectID, + stop_project_services_as_admin: StopServicesCallback | None, +): + hidden = False + stopped = not stop_project_services_as_admin + deleted = False + + try: + # hide + await _projects_repository.patch_project( + app, + project_uuid=project_uuid, + new_partial_project_data={"hidden": True}, + ) + hidden = True + + if stop_project_services_as_admin: + # NOTE: this callback could take long or raise whatever! + await stop_project_services_as_admin(app, project_uuid) + stopped = True + + await _projects_repository.delete_project(app, project_uuid=project_uuid) + deleted = True + + except ProjectNotFoundError as err: + _logger.debug( + "Project %s being deleted is already gone. IGNORING error. Details: %s", + project_uuid, + err, + ) + + except ProjectLockError as err: + raise ProjectDeleteError( + project_uuid=project_uuid, + reason=f"Cannot delete project {project_uuid} because it is currently in use. Details: {err}", + ) from err + + except Exception as err: + raise ProjectDeleteError( + project_uuid=project_uuid, + reason=f"Unexpected error. Deletion sequence: {hidden=}, {stopped=}, {deleted=} ", + ) from err From b80980bf83bdccd9b99e45c6b0f46ba4c8b7d424 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 23:06:17 +0100 Subject: [PATCH 20/44] connect with service --- .../projects/_trash_service.py | 14 ++++++-------- .../simcore_service_webserver/trash/_service.py | 1 - 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py index fec52da9a90d..ecdbd74bcb9e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py @@ -19,7 +19,7 @@ from ..dynamic_scheduler import api as dynamic_scheduler_api from . import _crud_api_read from . import _projects_db as _projects_repository -from . import projects_service +from . import _projects_service_delete, projects_service from ._access_rights_api import check_user_project_permission from ._projects_db import _OLDEST_TRASHED_FIRST from .exceptions import ( @@ -246,7 +246,6 @@ async def delete_explicitly_trashed_project( async def batch_delete_trashed_projects_as_admin( app: web.Application, *, - product_name: ProductName, trashed_before: datetime, fail_fast: bool, ) -> list[ProjectID]: @@ -273,11 +272,11 @@ async def batch_delete_trashed_projects_as_admin( assert project.trashed_explicitly # nosec try: - _logger.debug( - # TODO: _projects_service_delete.delete_project_as_admin - "await _projects_service_delete.delete_project_as_admin(app, project_id=%s, product_name=%s)", - project.uuid, - product_name, + await _projects_service_delete.delete_project_as_admin( + app, + project_uuid=project.uuid, + # FIXME: trashed project does not have services running? + stop_project_services_as_admin=None, ) deleted_project_ids.append(project.uuid) except Exception as err: # pylint: disable=broad-exception-caught @@ -289,7 +288,6 @@ async def batch_delete_trashed_projects_as_admin( raise ProjectsBatchDeleteError( errors=errors, trashed_before=trashed_before, - product_name=product_name, deleted_project_ids=deleted_project_ids, ) diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index 661753e5c45b..b1bc4fbe801b 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -151,7 +151,6 @@ async def safe_delete_expired_trash_as_admin(app: web.Application) -> None: await projects_trash_service.batch_delete_trashed_projects_as_admin( app, trashed_before=delete_until, - product_name=product.name, fail_fast=False, ) From 4fb6a2d5bfb5b743941aa911088a4da2642bbb9e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 23:37:51 +0100 Subject: [PATCH 21/44] stop services --- .../projects/_projects_db.py | 5 +- .../projects/_projects_service_delete.py | 82 ++++++++++++++----- .../projects/_trash_service.py | 3 +- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 58f82cc7f98e..93e416df746b 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -172,8 +172,7 @@ async def patch_project( *, project_uuid: ProjectID, new_partial_project_data: dict, -) -> None: - +) -> ProjectDBGet: async with transaction_context(get_asyncpg_engine(app), connection) as conn: result = await conn.stream( projects.update() @@ -182,10 +181,12 @@ async def patch_project( last_change_date=sql.func.now(), ) .where(projects.c.uuid == f"{project_uuid}") + .returning(*PROJECT_DB_COLS) ) row = await result.one_or_none() if row is None: raise ProjectNotFoundError(project_uuid=project_uuid) + return ProjectDBGet.model_validate(row) async def delete_project( diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py index e852b1485240..f19428077da3 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py @@ -1,50 +1,91 @@ -# NOTE: should replace _crud_api_delete.py:delete_project - +import asyncio import logging -from typing import Protocol +import time +from contextlib import contextmanager +from typing import Any, Protocol from aiohttp import web from models_library.projects import ProjectID +from models_library.users import UserID +from servicelib.common_headers import UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE from servicelib.redis._errors import ProjectLockError from simcore_service_director_v2.core.errors import ProjectNotFoundError from simcore_service_webserver.projects.exceptions import ProjectDeleteError +from ..director_v2 import api as director_v2_service from . import _projects_db as _projects_repository +from . import projects_service _logger = logging.getLogger(__name__) +@contextmanager +def _monitor_step(steps: dict[str, Any], *, name: str, elapsed: bool = False): + # util + start_time = time.perf_counter() + steps[name] = {"status": "starting"} + try: + yield + except Exception as e: + steps[name]["status"] = "failed" + steps[name]["exception"] = str(e) + raise + else: + steps[name]["status"] = "success" + finally: + if elapsed: + steps[name]["elapsed"] = time.perf_counter() - start_time + + class StopServicesCallback(Protocol): async def __call__(self, app: web.Application, project_uuid: ProjectID) -> None: ... +async def batch_stop_services_in_project( + app: web.Application, *, user_id: UserID, project_uuid: ProjectID +) -> None: + await asyncio.gather( + director_v2_service.stop_pipeline( + app, user_id=user_id, project_id=project_uuid + ), + projects_service.remove_project_dynamic_services( + user_id=user_id, + project_uuid=f"{project_uuid}", + app=app, + simcore_user_agent=UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE, + notify_users=False, + ), + ) + + async def delete_project_as_admin( app: web.Application, *, project_uuid: ProjectID, - stop_project_services_as_admin: StopServicesCallback | None, ): - hidden = False - stopped = not stop_project_services_as_admin - deleted = False + + state: dict[str, Any] = {} try: - # hide - await _projects_repository.patch_project( - app, - project_uuid=project_uuid, - new_partial_project_data={"hidden": True}, - ) - hidden = True + # 1. hide + with _monitor_step(state, name="hide"): + project = await _projects_repository.patch_project( + app, + project_uuid=project_uuid, + new_partial_project_data={"hidden": True}, + ) - if stop_project_services_as_admin: + # 2. stop + with _monitor_step(state, name="stop", elapsed=True): # NOTE: this callback could take long or raise whatever! - await stop_project_services_as_admin(app, project_uuid) - stopped = True + await batch_stop_services_in_project( + app, user_id=project.prj_owner, project_uuid=project_uuid + ) - await _projects_repository.delete_project(app, project_uuid=project_uuid) - deleted = True + # 3. delete + with _monitor_step(state, name="delete"): + await _projects_repository.delete_project(app, project_uuid=project_uuid) except ProjectNotFoundError as err: _logger.debug( @@ -62,5 +103,6 @@ async def delete_project_as_admin( except Exception as err: raise ProjectDeleteError( project_uuid=project_uuid, - reason=f"Unexpected error. Deletion sequence: {hidden=}, {stopped=}, {deleted=} ", + reason=f"Unexpected error. Deletion sequence: {state=}", + state=state, ) from err diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py index ecdbd74bcb9e..827435ad4449 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py @@ -76,6 +76,7 @@ async def trash_project( if force_stop_first: async def _schedule(): + # TODO: use batch_stop_services_in_project instead. Change mocks! await asyncio.gather( director_v2_api.stop_pipeline( app, user_id=user_id, project_id=project_id @@ -275,8 +276,6 @@ async def batch_delete_trashed_projects_as_admin( await _projects_service_delete.delete_project_as_admin( app, project_uuid=project.uuid, - # FIXME: trashed project does not have services running? - stop_project_services_as_admin=None, ) deleted_project_ids.append(project.uuid) except Exception as err: # pylint: disable=broad-exception-caught From d48aedb70a8ff3616ef604daeaefe95fea82dbc9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 23:47:33 +0100 Subject: [PATCH 22/44] minor --- .../projects/_projects_service_delete.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py index f19428077da3..2a12795f00a2 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py @@ -26,9 +26,9 @@ def _monitor_step(steps: dict[str, Any], *, name: str, elapsed: bool = False): steps[name] = {"status": "starting"} try: yield - except Exception as e: - steps[name]["status"] = "failed" - steps[name]["exception"] = str(e) + except Exception as exc: + steps[name]["status"] = "raised" + steps[name]["exception"] = f"{exc.__class__.__name__}:{exc}" raise else: steps[name]["status"] = "success" @@ -98,6 +98,7 @@ async def delete_project_as_admin( raise ProjectDeleteError( project_uuid=project_uuid, reason=f"Cannot delete project {project_uuid} because it is currently in use. Details: {err}", + state=state, ) from err except Exception as err: From 7b9a2a9695900de3cd5e767889facd1e9caa88e6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Feb 2025 23:51:48 +0100 Subject: [PATCH 23/44] fix import --- .../projects/_projects_service_delete.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py index 2a12795f00a2..bd021a4810ff 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_service_delete.py @@ -9,12 +9,11 @@ from models_library.users import UserID from servicelib.common_headers import UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE from servicelib.redis._errors import ProjectLockError -from simcore_service_director_v2.core.errors import ProjectNotFoundError -from simcore_service_webserver.projects.exceptions import ProjectDeleteError from ..director_v2 import api as director_v2_service from . import _projects_db as _projects_repository from . import projects_service +from .exceptions import ProjectDeleteError, ProjectNotFoundError _logger = logging.getLogger(__name__) From 11412bfb0a4e1eb5539e2409a233e9b9e670b849 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 09:54:13 +0100 Subject: [PATCH 24/44] fix tests --- .../web/server/tests/unit/with_dbs/03/trash/test_trash.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py index b9d28d694661..8f1d159cb8b2 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py @@ -338,6 +338,7 @@ async def test_trash_folder_with_content( resp = await client.post("/v0/folders", json={"name": "My first folder"}) data, _ = await assert_status(resp, status.HTTP_201_CREATED) folder = FolderGet.model_validate(data) + assert folder.trashed_at is None # CREATE a SUB-folder resp = await client.post( @@ -381,12 +382,13 @@ async def test_trash_folder_with_content( resp = await client.post(f"/v0/folders/{folder.folder_id}:trash") await assert_status(resp, status.HTTP_204_NO_CONTENT) - # ONLY folder listed in trash. The rest is not listed anymore! + # ONLY folder listed in trash. The rest is not listed anymore since they are implicitly trashed! resp = await client.get("/v0/folders", params={"filters": '{"trashed": true}'}) await assert_status(resp, status.HTTP_200_OK) page = Page[FolderGet].model_validate(await resp.json()) assert page.meta.total == 1 - assert page.data[0] == folder + assert page.data[0].trashed_at is not None + assert page.data[0].folder_id == folder.folder_id resp = await client.get( "/v0/folders", From a1cd382aeb4075e400a887dbc484d515ea94f977 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 10:27:03 +0100 Subject: [PATCH 25/44] fix tests --- .../pytest_simcore/helpers/assert_checks.py | 6 +- .../folders/_folders_repository.py | 2 +- .../folders/_trash_service.py | 2 + .../projects/_trash_service.py | 3 +- .../with_dbs/03/trash/test_trash_service.py | 91 +++++++++---------- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py b/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py index 1fd0f9f71d48..fc931cbebd59 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py @@ -29,8 +29,10 @@ async def assert_status( data, error = unwrap_envelope(json_response) assert response.status == expected_status_code, ( - f"received {response.status}: ({data},{error})" - f", expected {get_code_display_name(expected_status_code)} : {expected_msg or ''}" + f"Expected: {get_code_display_name(expected_status_code)} : {expected_msg or ''}" + f"Got: {response.status}:\n" + f" - data :{pformat(data)}\n" + f" - error:{pformat(error)}\n)" ) if is_error(expected_status_code): diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 072e944688ea..fb4bed9f8d1e 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -275,7 +275,7 @@ async def list_trashed_folders( NOTE: this is app-wide i.e. no product, user or workspace filtered TODO: check with MD about workspaces """ - base_query = sql.select(_FOLDER_DB_MODEL_COLS).where( + base_query = sql.select(*_FOLDER_DB_MODEL_COLS).where( folders_v2.c.trashed.is_not(None) ) diff --git a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py index 179becd7b01b..f5003ee3dba1 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_trash_service.py @@ -313,6 +313,8 @@ async def batch_delete_trashed_folders_as_admin( await _folders_repository.delete_recursively( app, folder_id=folder.folder_id, product_name=product_name ) + # NOTE: projects in folders are NOT deleted + except Exception as err: # pylint: disable=broad-exception-caught if fail_fast: raise diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py index 827435ad4449..ae3598c96b4b 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py @@ -260,7 +260,7 @@ async def batch_delete_trashed_projects_as_admin( expired_trashed_projects, ) = await _projects_repository.list_trashed_projects( app, - trashed_explicitly=True, + # both implicit and explicitly trashed trashed_before=trashed_before, offset=page_params.offset, limit=page_params.limit, @@ -270,7 +270,6 @@ async def batch_delete_trashed_projects_as_admin( for project in expired_trashed_projects: assert project.trashed # nosec - assert project.trashed_explicitly # nosec try: await _projects_service_delete.delete_project_as_admin( diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py index 5481f9df1db0..94d67e5ec567 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py @@ -8,6 +8,7 @@ import contextlib from collections.abc import AsyncIterator +from unittest.mock import MagicMock import pytest from aiohttp.test_utils import TestClient @@ -44,18 +45,16 @@ def user_role() -> UserRole: @contextlib.asynccontextmanager -async def _client_session_with_user( +async def _switch_client_session_to( client: TestClient, user: UserInfoDict ) -> AsyncIterator[TestClient]: assert client.app - url = client.app.router["logout"].url_for() - resp = await client.post(f"{url}") - await assert_status(resp, status.HTTP_200_OK) + await client.post(f'{client.app.router["auth_logout"].url_for()}') + # sometimes 4xx if user already logged out. Ignore - url = client.app.router["login"].url_for() resp = await client.post( - f"{url}", + f'{client.app.router["auth_login"].url_for()}', json={ "email": user["email"], "password": user["raw_password"], @@ -65,8 +64,7 @@ async def _client_session_with_user( yield client - url = client.app.router["logout"].url_for() - resp = await client.post(f"{url}") + resp = await client.post(f'{client.app.router["auth_logout"].url_for()}') await assert_status(resp, status.HTTP_200_OK) @@ -78,6 +76,7 @@ async def test_trash_service__delete_expired_trash( other_user_project: ProjectDict, mocked_catalog: None, mocked_director_v2: None, + mocked_dynamic_services_interface: dict[str, MagicMock], ): assert client.app assert logged_user["id"] != other_user["id"] @@ -117,7 +116,7 @@ async def test_trash_service__delete_expired_trash( await assert_status(resp, status.HTTP_404_NOT_FOUND) # ASSERT: other_user tries to get the project and expects 404 - async with _client_session_with_user(client, other_user): + async with _switch_client_session_to(client, other_user): resp = await client.get(f"/v0/projects/{other_user_project_id}") await assert_status(resp, status.HTTP_404_NOT_FOUND) @@ -130,37 +129,41 @@ async def test_trash_nested_folders_and_projects( other_user_project: ProjectDict, mocked_catalog: None, mocked_director_v2: None, + mocked_dynamic_services_interface: dict[str, MagicMock], ): assert client.app assert logged_user["id"] != other_user["id"] - # Create folders hierarchy for logged_user - resp = await client.post("/v0/folders", json={"name": "Root Folder"}) - data, _ = await assert_status(resp, status.HTTP_201_CREATED) - logged_user_root_folder = data + async with _switch_client_session_to(client, logged_user): + # CREATE folders hierarchy for logged_user + resp = await client.post("/v0/folders", json={"name": "Root Folder"}) + data, _ = await assert_status(resp, status.HTTP_201_CREATED) + logged_user_root_folder = data - resp = await client.post( - "/v0/folders", - json={ - "name": "Sub Folder", - "parentFolderId": logged_user_root_folder["folderId"], - }, - ) - data, _ = await assert_status(resp, status.HTTP_201_CREATED) - logged_user_sub_folder = data + resp = await client.post( + "/v0/folders", + json={ + "name": "Sub Folder", + "parentFolderId": logged_user_root_folder["folderId"], + }, + ) + data, _ = await assert_status(resp, status.HTTP_201_CREATED) + logged_user_sub_folder = data - # Move project to subfolder - resp = await client.put( - f"/v0/projects/{user_project['uuid']}/folders/{logged_user_sub_folder['folderId']}" - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) + # MOVE project to subfolder + resp = await client.put( + f"/v0/projects/{user_project['uuid']}/folders/{logged_user_sub_folder['folderId']}" + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) - # Trash root folders - resp = await client.post(f"/v0/folders/{logged_user_root_folder['folderId']}:trash") - await assert_status(resp, status.HTTP_204_NO_CONTENT) + # TRASH root folders + resp = await client.post( + f"/v0/folders/{logged_user_root_folder['folderId']}:trash" + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) - # Create folders hierarchy for other_user - async with _client_session_with_user(client, other_user): + async with _switch_client_session_to(client, other_user): + # CREATE folders hierarchy for other_user resp = await client.post("/v0/folders", json={"name": "Root Folder"}) data, _ = await assert_status(resp, status.HTTP_201_CREATED) other_user_root_folder = data @@ -168,26 +171,20 @@ async def test_trash_nested_folders_and_projects( resp = await client.post( "/v0/folders", json={ - "name": "Sub Folder", + "name": "Sub Folder (other)", "parentFolderId": other_user_root_folder["folderId"], }, ) data, _ = await assert_status(resp, status.HTTP_201_CREATED) other_user_sub_folder = data - # Move project to subfolder + # MOVE project to subfolder resp = await client.put( f"/v0/projects/{other_user_project['uuid']}/folders/{other_user_sub_folder['folderId']}" ) await assert_status(resp, status.HTTP_204_NO_CONTENT) - # Trash root folders - resp = await client.post( - f"/v0/folders/{logged_user_root_folder['folderId']}:trash" - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - async with _client_session_with_user(client, other_user): + # TRASH root folders resp = await client.post( f"/v0/folders/{other_user_root_folder['folderId']}:trash" ) @@ -196,24 +193,24 @@ async def test_trash_nested_folders_and_projects( # UNDER TEST await trash_service.safe_delete_expired_trash_as_admin(client.app) - async with _client_session_with_user(client, logged_user): + async with _switch_client_session_to(client, logged_user): # Verify logged_user's resources are gone resp = await client.get(f"/v0/folders/{logged_user_root_folder['folderId']}") - await assert_status(resp, status.HTTP_404_NOT_FOUND) + await assert_status(resp, status.HTTP_403_FORBIDDEN) resp = await client.get(f"/v0/folders/{logged_user_sub_folder['folderId']}") - await assert_status(resp, status.HTTP_404_NOT_FOUND) + await assert_status(resp, status.HTTP_403_FORBIDDEN) resp = await client.get(f"/v0/projects/{user_project['uuid']}") await assert_status(resp, status.HTTP_404_NOT_FOUND) # Verify other_user's resources are gone - async with _client_session_with_user(client, other_user): + async with _switch_client_session_to(client, other_user): resp = await client.get(f"/v0/folders/{other_user_root_folder['folderId']}") - await assert_status(resp, status.HTTP_404_NOT_FOUND) + await assert_status(resp, status.HTTP_403_FORBIDDEN) resp = await client.get(f"/v0/folders/{other_user_sub_folder['folderId']}") - await assert_status(resp, status.HTTP_404_NOT_FOUND) + await assert_status(resp, status.HTTP_403_FORBIDDEN) resp = await client.get(f"/v0/projects/{other_user_project['uuid']}") await assert_status(resp, status.HTTP_404_NOT_FOUND) From e4525b0393bb9d11353b2cb18acb2da43f4528c8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 10:33:15 +0100 Subject: [PATCH 26/44] @matusdrobuliak66 review: rename --- .../src/simcore_service_webserver/projects/_projects_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 93e416df746b..a00f11ea6e69 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -84,10 +84,10 @@ async def list_trashed_projects( total_count = await conn.scalar(count_query) result = await conn.stream(list_query) - folders: list[ProjectDBGet] = [ + projects_list: list[ProjectDBGet] = [ ProjectDBGet.model_validate(row) async for row in result ] - return cast(int, total_count), folders + return cast(int, total_count), projects_list async def get_project( From c570336554e92c3ec283cecdea8effb9831447d8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 11:14:45 +0100 Subject: [PATCH 27/44] @matusdrobuliak66 review: rename --- .../folders/_folders_repository.py | 15 ++++++++++----- .../projects/_projects_db.py | 9 ++++++--- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index fb4bed9f8d1e..b74242eac4eb 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -93,6 +93,8 @@ def _create_private_workspace_query( return ( sql.select( *_FOLDER_DB_MODEL_COLS, + # NOTE: design INVARIANT: + # a user in his private workspace owns his folders sql.func.json_build_object( "read", sa.text("true"), @@ -130,6 +132,8 @@ def _create_shared_workspace_query( shared_workspace_query = ( sql.select( *_FOLDER_DB_MODEL_COLS, + # NOTE: design INVARIANT: + # a user access rights to a folder in a SHARED workspace is inherited from the workspace workspace_access_rights_subquery.c.my_access_rights, ) .select_from( @@ -156,12 +160,12 @@ def _create_shared_workspace_query( return shared_workspace_query -def _to_expression(order_by: OrderBy): +def _to_sql_expression(table: sa.Table, order_by: OrderBy): direction_func: Callable = { OrderDirection.ASC: sql.asc, OrderDirection.DESC: sql.desc, }[order_by.direction] - return direction_func(folders_v2.columns[order_by.field]) + return direction_func(table.columns[order_by.field]) async def list_( # pylint: disable=too-many-arguments,too-many-branches @@ -245,7 +249,9 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches # Ordering and pagination list_query = ( - combined_query.order_by(_to_expression(order_by)).offset(offset).limit(limit) + combined_query.order_by(_to_sql_expression(folders_v2, order_by)) + .offset(offset) + .limit(limit) ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: @@ -273,7 +279,6 @@ async def list_trashed_folders( ) -> tuple[int, list[FolderDB]]: """ NOTE: this is app-wide i.e. no product, user or workspace filtered - TODO: check with MD about workspaces """ base_query = sql.select(*_FOLDER_DB_MODEL_COLS).where( folders_v2.c.trashed.is_not(None) @@ -294,7 +299,7 @@ async def list_trashed_folders( # Ordering and pagination list_query = ( - base_query.order_by(_to_expression(order_by)).offset(offset).limit(limit) + base_query.order_by(_to_sql_expression(order_by)).offset(offset).limit(limit) ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index a00f11ea6e69..8054bdb86a68 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -2,6 +2,7 @@ from datetime import datetime from typing import Callable, cast +import sqlalchemy as sa from aiohttp import web from common_library.exclude import UnSet, is_set from models_library.basic_types import IDStr @@ -38,12 +39,12 @@ _OLDEST_TRASHED_FIRST = OrderBy(field=IDStr("trashed"), direction=OrderDirection.ASC) -def _to_expression(order_by: OrderBy): +def _to_sql_expression(table: sa.Table, order_by: OrderBy): direction_func: Callable = { OrderDirection.ASC: sql.asc, OrderDirection.DESC: sql.desc, }[order_by.direction] - return direction_func(projects.columns[order_by.field]) + return direction_func(table.columns[order_by.field]) async def list_trashed_projects( @@ -77,7 +78,9 @@ async def list_trashed_projects( # Ordering and pagination list_query = ( - base_query.order_by(_to_expression(order_by)).offset(offset).limit(limit) + base_query.order_by(_to_sql_expression(projects, order_by)) + .offset(offset) + .limit(limit) ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: From 2b00220e341233b27cb055aaa6aba5cc7d400276 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 11:27:40 +0100 Subject: [PATCH 28/44] reuse and fix mocks --- .../projects/_trash_service.py | 21 +++---------------- .../unit/with_dbs/03/trash/test_trash.py | 4 ++-- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py index ae3598c96b4b..1ec1c81d5341 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py @@ -1,4 +1,3 @@ -import asyncio import logging from datetime import datetime @@ -12,7 +11,6 @@ from models_library.rest_pagination import MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE from models_library.users import UserID from servicelib.aiohttp.application_keys import APP_FIRE_AND_FORGET_TASKS_KEY -from servicelib.common_headers import UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE from servicelib.utils import fire_and_forget_task from ..director_v2 import api as director_v2_api @@ -75,23 +73,10 @@ async def trash_project( if force_stop_first: - async def _schedule(): - # TODO: use batch_stop_services_in_project instead. Change mocks! - await asyncio.gather( - director_v2_api.stop_pipeline( - app, user_id=user_id, project_id=project_id - ), - projects_service.remove_project_dynamic_services( - user_id=user_id, - project_uuid=f"{project_id}", - app=app, - simcore_user_agent=UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE, - notify_users=False, - ), - ) - fire_and_forget_task( - _schedule(), + _projects_service_delete.batch_stop_services_in_project( + app, user_id=user_id, project_uuid=project_id + ), task_suffix_name=f"trash_project_force_stop_first_{user_id=}_{project_id=}", fire_and_forget_tasks_collection=app[APP_FIRE_AND_FORGET_TASKS_KEY], ) diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py index 8f1d159cb8b2..86cfb24dd24c 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash.py @@ -53,11 +53,11 @@ async def test_trash_projects( # noqa: PLR0915 # this test should emulate NO errors stopping services mock_remove_dynamic_services = mocker.patch( - "simcore_service_webserver.projects._trash_service.projects_service.remove_project_dynamic_services", + "simcore_service_webserver.projects._trash_service._projects_service_delete.projects_service.remove_project_dynamic_services", autospec=True, ) mock_stop_pipeline = mocker.patch( - "simcore_service_webserver.projects._trash_service.director_v2_api.stop_pipeline", + "simcore_service_webserver.projects._trash_service._projects_service_delete.director_v2_service.stop_pipeline", autospec=True, ) mocker.patch( From 228b774f443040da2838ca4fa08365ec72e104af Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 11:34:59 +0100 Subject: [PATCH 29/44] drafts tests --- .../with_dbs/02/test_projects_repository.py | 46 +++++++++++++++++++ .../02/test_projects_service_delete.py | 4 ++ 2 files changed, 50 insertions(+) create mode 100644 services/web/server/tests/unit/with_dbs/02/test_projects_repository.py create mode 100644 services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py new file mode 100644 index 000000000000..40c98ba7af73 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py @@ -0,0 +1,46 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + +from collections.abc import Awaitable, Callable +from typing import Any +from uuid import UUID + +import pytest +from aiohttp.test_utils import TestClient +from pytest_simcore.helpers.webserver_login import UserInfoDict +from simcore_service_webserver.projects import ( + _projects_db as projects_service_repository, +) +from simcore_service_webserver.projects.exceptions import ProjectNotFoundError +from simcore_service_webserver.projects.models import ProjectDict + + +async def test_get_project( + client: TestClient, + logged_user: UserInfoDict, + user_project: ProjectDict, + insert_project_in_db: Callable[..., Awaitable[dict[str, Any]]], +): + assert client.app + + # Insert a project into the database + new_project = await insert_project_in_db(user_project, user_id=logged_user["id"]) + + # Retrieve the project using the repository function + retrieved_project = await projects_service_repository.get_project( + client.app, project_uuid=UUID(new_project["uuid"]) + ) + + # Validate the retrieved project + assert retrieved_project.uuid == new_project["uuid"] + assert retrieved_project.name == new_project["name"] + assert retrieved_project.description == new_project["description"] + + # Test retrieving a non-existent project + non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") + with pytest.raises(ProjectNotFoundError): + await projects_service_repository.get_project( + client.app, project_uuid=non_existent_project_uuid + ) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py b/services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py new file mode 100644 index 000000000000..4591cb0c13d4 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py @@ -0,0 +1,4 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments From 16a562235c5e74f1c163477374fc401bd12d3b49 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 11:39:35 +0100 Subject: [PATCH 30/44] fix --- .../simcore_service_webserver/folders/_folders_repository.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index b74242eac4eb..8c98a0f8dc76 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -299,7 +299,9 @@ async def list_trashed_folders( # Ordering and pagination list_query = ( - base_query.order_by(_to_sql_expression(order_by)).offset(offset).limit(limit) + base_query.order_by(_to_sql_expression(folders_v2, order_by)) + .offset(offset) + .limit(limit) ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: From cdb153254d7cc29bce69b81161a6633a9b74de7c Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 11:57:01 +0100 Subject: [PATCH 31/44] fix tests --- .../with_dbs/02/test_projects_repository.py | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py index 40c98ba7af73..85aeb77b4935 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py @@ -3,12 +3,11 @@ # pylint: disable=unused-variable # pylint: disable=too-many-arguments -from collections.abc import Awaitable, Callable -from typing import Any from uuid import UUID import pytest from aiohttp.test_utils import TestClient +from common_library.users_enums import UserRole from pytest_simcore.helpers.webserver_login import UserInfoDict from simcore_service_webserver.projects import ( _projects_db as projects_service_repository, @@ -17,28 +16,28 @@ from simcore_service_webserver.projects.models import ProjectDict +@pytest.fixture +def user_role() -> UserRole: + return UserRole.USER + + async def test_get_project( client: TestClient, logged_user: UserInfoDict, user_project: ProjectDict, - insert_project_in_db: Callable[..., Awaitable[dict[str, Any]]], ): assert client.app - # Insert a project into the database - new_project = await insert_project_in_db(user_project, user_id=logged_user["id"]) - - # Retrieve the project using the repository function - retrieved_project = await projects_service_repository.get_project( - client.app, project_uuid=UUID(new_project["uuid"]) + # Get valid project + got = await projects_service_repository.get_project( + client.app, project_uuid=user_project["uuid"] ) - # Validate the retrieved project - assert retrieved_project.uuid == new_project["uuid"] - assert retrieved_project.name == new_project["name"] - assert retrieved_project.description == new_project["description"] + assert got.uuid == UUID(user_project["uuid"]) + assert got.name == user_project["name"] + assert got.description == user_project["description"] - # Test retrieving a non-existent project + # Get non-existent project non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") with pytest.raises(ProjectNotFoundError): await projects_service_repository.get_project( From da2866838444a213b7ac1c117a8426afe9730035 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 12:00:34 +0100 Subject: [PATCH 32/44] new tests --- .../with_dbs/02/test_projects_repository.py | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py index 85aeb77b4935..366c6a9b7038 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py @@ -43,3 +43,53 @@ async def test_get_project( await projects_service_repository.get_project( client.app, project_uuid=non_existent_project_uuid ) + + +async def test_patch_project( + client: TestClient, + logged_user: UserInfoDict, + user_project: ProjectDict, +): + assert client.app + + # Patch valid project + patch_data = {"name": "Updated Project Name"} + patched_project = await projects_service_repository.patch_project( + client.app, + project_uuid=user_project["uuid"], + new_partial_project_data=patch_data, + ) + + assert patched_project.uuid == UUID(user_project["uuid"]) + assert patched_project.name == patch_data["name"] + + # Patch non-existent project + non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") + with pytest.raises(ProjectNotFoundError): + await projects_service_repository.patch_project( + client.app, + project_uuid=non_existent_project_uuid, + new_partial_project_data=patch_data, + ) + + +async def test_delete_project( + client: TestClient, + logged_user: UserInfoDict, + user_project: ProjectDict, +): + assert client.app + + # Delete valid project + deleted_project = await projects_service_repository.delete_project( + client.app, project_uuid=user_project["uuid"] + ) + + assert deleted_project.uuid == UUID(user_project["uuid"]) + + # Delete non-existent project + non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") + with pytest.raises(ProjectNotFoundError): + await projects_service_repository.delete_project( + client.app, project_uuid=non_existent_project_uuid + ) From 116cb4e0b48ac258f189b46d27eace85ef83b2ef Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 13:42:46 +0100 Subject: [PATCH 33/44] fix tsets --- .../projects/_projects_db.py | 2 +- .../with_dbs/02/test_projects_repository.py | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 8054bdb86a68..5717e47ce370 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -100,7 +100,7 @@ async def get_project( project_uuid: ProjectID, ) -> ProjectDBGet: async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - query = sql.select(PROJECT_DB_COLS).where(projects.c.uuid == f"{project_uuid}") + query = sql.select(*PROJECT_DB_COLS).where(projects.c.uuid == f"{project_uuid}") result = await conn.execute(query) row = result.one_or_none() if row is None: diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py index 366c6a9b7038..e772d4919089 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py @@ -29,13 +29,13 @@ async def test_get_project( assert client.app # Get valid project - got = await projects_service_repository.get_project( + got_project = await projects_service_repository.get_project( client.app, project_uuid=user_project["uuid"] ) - assert got.uuid == UUID(user_project["uuid"]) - assert got.name == user_project["name"] - assert got.description == user_project["description"] + assert got_project.uuid == UUID(user_project["uuid"]) + assert got_project.name == user_project["name"] + assert got_project.description == user_project["description"] # Get non-existent project non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") @@ -52,6 +52,9 @@ async def test_patch_project( ): assert client.app + # Thie will change after in patched_project + assert user_project["creationDate"] == user_project["lastChangeDate"] + # Patch valid project patch_data = {"name": "Updated Project Name"} patched_project = await projects_service_repository.patch_project( @@ -62,6 +65,7 @@ async def test_patch_project( assert patched_project.uuid == UUID(user_project["uuid"]) assert patched_project.name == patch_data["name"] + assert patched_project.creation_date < patched_project.last_change_date # Patch non-existent project non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") @@ -87,6 +91,12 @@ async def test_delete_project( assert deleted_project.uuid == UUID(user_project["uuid"]) + # Check deleted + with pytest.raises(ProjectNotFoundError): + await projects_service_repository.delete_project( + client.app, project_uuid=user_project["uuid"] + ) + # Delete non-existent project non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") with pytest.raises(ProjectNotFoundError): From c7c7e24bf186ac642894109c20bee6805594baa1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 14:04:30 +0100 Subject: [PATCH 34/44] fix tests --- .../projects/_projects_db.py | 5 +- .../with_dbs/02/test_projects_repository.py | 63 ++++++++++++++++++- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 5717e47ce370..c9c67160648e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -1,6 +1,7 @@ import logging +from collections.abc import Callable from datetime import datetime -from typing import Callable, cast +from typing import cast import sqlalchemy as sa from aiohttp import web @@ -28,7 +29,7 @@ _logger = logging.getLogger(__name__) -PROJECT_DB_COLS = get_columns_from_db_model( # noqa: RUF012 +PROJECT_DB_COLS = get_columns_from_db_model( # NOTE: MD: I intentionally didn't include the workbench. There is a special interface # for the workbench, and at some point, this column should be removed from the table. # The same holds true for access_rights/ui/classifiers/quality, but we have decided to proceed step by step. diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py index e772d4919089..749d27207faa 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py @@ -3,8 +3,10 @@ # pylint: disable=unused-variable # pylint: disable=too-many-arguments +from datetime import timedelta from uuid import UUID +import arrow import pytest from aiohttp.test_utils import TestClient from common_library.users_enums import UserRole @@ -13,7 +15,7 @@ _projects_db as projects_service_repository, ) from simcore_service_webserver.projects.exceptions import ProjectNotFoundError -from simcore_service_webserver.projects.models import ProjectDict +from simcore_service_webserver.projects.models import ProjectDBGet, ProjectDict @pytest.fixture @@ -52,7 +54,7 @@ async def test_patch_project( ): assert client.app - # Thie will change after in patched_project + # This will change after in patched_project assert user_project["creationDate"] == user_project["lastChangeDate"] # Patch valid project @@ -103,3 +105,60 @@ async def test_delete_project( await projects_service_repository.delete_project( client.app, project_uuid=non_existent_project_uuid ) + + +@pytest.fixture +async def trashed_project( + client: TestClient, + logged_user: UserInfoDict, + user_project: ProjectDict, +) -> ProjectDBGet: + assert client.app + + # Patch project to be trashed + trashed_at = arrow.utcnow().datetime + patch_data = { + "trashed": trashed_at, + "trashed_by": logged_user["id"], + "trashed_explicitly": True, + } + return await projects_service_repository.patch_project( + client.app, + project_uuid=user_project["uuid"], + new_partial_project_data=patch_data, + ) + + +async def test_list_trashed_projects(client: TestClient, trashed_project: ProjectDBGet): + assert client.app + + ( + total_count, + trashed_projects, + ) = await projects_service_repository.list_trashed_projects( + client.app, + trashed_explicitly=True, + trashed_before=arrow.utcnow().datetime + timedelta(days=1), + ) + + assert total_count == 1 + assert len(trashed_projects) == 1 + assert trashed_projects[0] == trashed_project + + +async def test_get_trashed_by_primary_gid( + client: TestClient, + logged_user: UserInfoDict, + trashed_project: ProjectDBGet, +): + assert client.app + + # Get trashed by primary gid + trashed_by_primary_gid = ( + await projects_service_repository.get_trashed_by_primary_gid( + client.app, + projects_uuid=trashed_project.uuid, + ) + ) + + assert trashed_by_primary_gid == logged_user["primary_gid"] From adb6359f6b699b113de3bbcb42b5ec3bd920a918 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 14:12:53 +0100 Subject: [PATCH 35/44] fix tests --- .../projects/_projects_db.py | 6 ++-- .../with_dbs/02/test_projects_repository.py | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index c9c67160648e..8787699fe832 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -123,12 +123,12 @@ async def get_trashed_by_primary_gid( projects_uuid: ProjectID, ) -> GroupID | None: query = _select_trashed_by_primary_gid_query().where( - projects.c.uuid == projects_uuid + projects.c.uuid == f"{projects_uuid}" ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: result = await conn.execute(query) - row = result.first() + row = result.one_or_none() return row.trashed_by_primary_gid if row else None @@ -167,7 +167,7 @@ async def batch_get_trashed_by_primary_gid( result = await conn.stream(query) rows = {row.uuid: row.trashed_by_primary_gid async for row in result} - return [rows.get(uuid) for uuid in projects_uuids_str] + return [rows.get(project_uuid) for project_uuid in projects_uuids_str] async def patch_project( diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py index 749d27207faa..ec71a03ad02b 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py @@ -162,3 +162,31 @@ async def test_get_trashed_by_primary_gid( ) assert trashed_by_primary_gid == logged_user["primary_gid"] + + +async def test_batch_get_trashed_by_primary_gid( + client: TestClient, + logged_user: UserInfoDict, + trashed_project: ProjectDBGet, +): + assert client.app + + non_existent_project_uuid = UUID("00000000-0000-0000-0000-000000000000") + + # Batch get trashed by primary gid + trashed_by_primary_gid = ( + await projects_service_repository.batch_get_trashed_by_primary_gid( + client.app, + projects_uuids=[ + trashed_project.uuid, + non_existent_project_uuid, # invalid + trashed_project.uuid, # repeated + ], + ) + ) + + assert trashed_by_primary_gid == [ + logged_user["primary_gid"], + None, + logged_user["primary_gid"], + ] From 896f2bc35bfaf2d6a9434dd71170ce17146c8dff Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 14:17:04 +0100 Subject: [PATCH 36/44] cleanup --- .../with_dbs/02/test_projects_repository.py | 2 +- .../02/test_projects_service_delete.py | 4 -- .../tests/unit/with_dbs/03/test_project_db.py | 43 ------------------- 3 files changed, 1 insertion(+), 48 deletions(-) delete mode 100644 services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py index ec71a03ad02b..4141f9527965 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_repository.py @@ -179,7 +179,7 @@ async def test_batch_get_trashed_by_primary_gid( client.app, projects_uuids=[ trashed_project.uuid, - non_existent_project_uuid, # invalid + non_existent_project_uuid, # non-existent trashed_project.uuid, # repeated ], ) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py b/services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py deleted file mode 100644 index 4591cb0c13d4..000000000000 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_service_delete.py +++ /dev/null @@ -1,4 +0,0 @@ -# pylint: disable=redefined-outer-name -# pylint: disable=unused-argument -# pylint: disable=unused-variable -# pylint: disable=too-many-arguments diff --git a/services/web/server/tests/unit/with_dbs/03/test_project_db.py b/services/web/server/tests/unit/with_dbs/03/test_project_db.py index f946c2a8f207..d62cac76a519 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_project_db.py +++ b/services/web/server/tests/unit/with_dbs/03/test_project_db.py @@ -13,7 +13,6 @@ from uuid import UUID, uuid5 import aiopg.sa -import arrow import pytest import sqlalchemy as sa from aiohttp.test_utils import TestClient @@ -32,9 +31,6 @@ from simcore_postgres_database.utils_projects_nodes import ProjectNodesRepo from simcore_service_webserver.projects._db_utils import PermissionStr from simcore_service_webserver.projects._groups_db import update_or_insert_project_group -from simcore_service_webserver.projects._projects_db import ( - batch_get_trashed_by_primary_gid, -) from simcore_service_webserver.projects.api import has_user_project_access_rights from simcore_service_webserver.projects.db import ProjectAccessRights, ProjectDBAPI from simcore_service_webserver.projects.exceptions import ( @@ -966,42 +962,3 @@ async def test_check_project_node_has_all_required_inputs_ok( project_uuid=UUID(inserted_project["uuid"]), node_id=UUID("324d6ef2-a82c-414d-9001-dc84da1cbea3"), ) - - -@pytest.mark.parametrize( - "user_role", - [UserRole.USER], -) -async def test_batch_get_trashed_by_primary_gid( - client: TestClient, - logged_user: UserInfoDict, - insert_project_in_db: Callable[..., Awaitable[dict[str, Any]]], - fake_project: ProjectDict, -): - assert client.app - - # Insert two different projects - fake_project_1 = deepcopy(fake_project) - fake_project_1["trashed_by"] = logged_user["id"] - fake_project_1["trashed"] = arrow.now().datetime - fake_project_1["trashed_explicitly"] = True - - project_1 = await insert_project_in_db(fake_project_1, user_id=logged_user["id"]) - project_2 = await insert_project_in_db(fake_project, user_id=logged_user["id"]) - - # Test with two different project UUIDs - trashed_by_primary_gid = await batch_get_trashed_by_primary_gid( - client.app, - projects_uuids=[project_1["uuid"], project_2["uuid"]], - ) - assert trashed_by_primary_gid == [logged_user["primary_gid"], None] - - # Test with two identical project UUIDs - trashed_by_primary_gid = await batch_get_trashed_by_primary_gid( - client.app, - projects_uuids=[project_1["uuid"], project_1["uuid"]], - ) - assert trashed_by_primary_gid == [ - logged_user["primary_gid"], - logged_user["primary_gid"], - ] From 30bec2105a5187c5cfa582d8858978bc49a974f1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 14:58:07 +0100 Subject: [PATCH 37/44] new envs --- .env-devel | 2 +- services/docker-compose.yml | 66 +++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/.env-devel b/.env-devel index e4f191a0c1ec..f808111fb3d9 100644 --- a/.env-devel +++ b/.env-devel @@ -288,7 +288,7 @@ WB_GC_LOGIN=null WB_GC_LOGLEVEL=INFO WB_GC_NOTIFICATIONS=0 WB_GC_PAYMENTS=null -WB_GC_PRODUCTS=0 +WB_GC_PRODUCTS=1 WB_GC_PROJECTS=null WB_GC_PUBLICATIONS=0 WB_GC_RESOURCE_MANAGER_RESOURCE_TTL_S=60 diff --git a/services/docker-compose.yml b/services/docker-compose.yml index 59601b8e6226..0c32076f8eb2 100644 --- a/services/docker-compose.yml +++ b/services/docker-compose.yml @@ -971,16 +971,15 @@ services: init: true hostname: "gc-{{.Node.Hostname}}-{{.Task.Slot}}" # the hostname is used in conjonction with other services and must be unique see https://github.com/ITISFoundation/osparc-simcore/pull/5931 environment: - WEBSERVER_LOGLEVEL: ${WB_GC_LOGLEVEL} - WEBSERVER_HOST: ${WEBSERVER_HOST} - WEBSERVER_PORT: ${WEBSERVER_PORT} + # WEBSERVER_DIRECTOR_V2 + DIRECTOR_V2_HOST: ${DIRECTOR_V2_HOST} + DIRECTOR_V2_PORT: ${DIRECTOR_V2_PORT} - # WEBSERVER_RESOURCE_USAGE_TRACKER - RESOURCE_USAGE_TRACKER_HOST: ${RESOURCE_USAGE_TRACKER_HOST} - RESOURCE_USAGE_TRACKER_PORT: ${RESOURCE_USAGE_TRACKER_EXTERNAL_PORT} + GUNICORN_CMD_ARGS: ${WEBSERVER_GUNICORN_CMD_ARGS} - REST_SWAGGER_API_DOC_ENABLED: ${WB_GC_REST_SWAGGER_API_DOC_ENABLED} + LOG_FILTER_MAPPING : ${LOG_FILTER_MAPPING} + LOG_FORMAT_LOCAL_DEV_ENABLED: ${LOG_FORMAT_LOCAL_DEV_ENABLED} # WEBSERVER_DB POSTGRES_DB: ${POSTGRES_DB} @@ -990,47 +989,61 @@ services: POSTGRES_PORT: ${POSTGRES_PORT} POSTGRES_USER: ${POSTGRES_USER} - DIRECTOR_V2_HOST: ${DIRECTOR_V2_HOST} - DIRECTOR_V2_PORT: ${DIRECTOR_V2_PORT} - - GUNICORN_CMD_ARGS: ${WEBSERVER_GUNICORN_CMD_ARGS} - - LOG_FORMAT_LOCAL_DEV_ENABLED: ${LOG_FORMAT_LOCAL_DEV_ENABLED} - LOG_FILTER_MAPPING : ${LOG_FILTER_MAPPING} - - STORAGE_HOST: ${STORAGE_HOST} - STORAGE_PORT: ${STORAGE_PORT} + # WEBSERVER_RABBITMQ + RABBIT_HOST: ${RABBIT_HOST} + RABBIT_PASSWORD: ${RABBIT_PASSWORD} + RABBIT_PORT: ${RABBIT_PORT} + RABBIT_SECURE: ${RABBIT_SECURE} + RABBIT_USER: ${RABBIT_USER} + # WEBSERVER_REDIS REDIS_HOST: ${REDIS_HOST} + REDIS_PASSWORD: ${REDIS_PASSWORD} REDIS_PORT: ${REDIS_PORT} REDIS_SECURE: ${REDIS_SECURE} REDIS_USER: ${REDIS_USER} - REDIS_PASSWORD: ${REDIS_PASSWORD} - - SWARM_STACK_NAME: ${SWARM_STACK_NAME} - WEBSERVER_DB_LISTENER: ${WB_GC_DB_LISTENER} + # WEBSERVER_RESOURCE_MANAGER + RESOURCE_MANAGER_RESOURCE_TTL_S: ${WB_GC_RESOURCE_MANAGER_RESOURCE_TTL_S} - WEBSERVER_GARBAGE_COLLECTOR: ${WB_GC_GARBAGE_COLLECTOR} + # WEBSERVER_RESOURCE_USAGE_TRACKER + RESOURCE_USAGE_TRACKER_HOST: ${RESOURCE_USAGE_TRACKER_HOST} + RESOURCE_USAGE_TRACKER_PORT: ${RESOURCE_USAGE_TRACKER_EXTERNAL_PORT} - RESOURCE_MANAGER_RESOURCE_TTL_S: ${WB_GC_RESOURCE_MANAGER_RESOURCE_TTL_S} + REST_SWAGGER_API_DOC_ENABLED: ${WB_GC_REST_SWAGGER_API_DOC_ENABLED} + # WEBSERVER_SESSION SESSION_SECRET_KEY: ${WEBSERVER_SESSION_SECRET_KEY} + + # WEBSERVER_STORAGE + STORAGE_HOST: ${STORAGE_HOST} + STORAGE_PORT: ${STORAGE_PORT} + + SWARM_STACK_NAME: ${SWARM_STACK_NAME} + + # WEBSERVER_TRASH + TRASH_RETENTION_DAYS: ${TRASH_RETENTION_DAYS} + WEBSERVER_ACTIVITY: ${WB_GC_ACTIVITY} WEBSERVER_ANNOUNCEMENTS: ${WB_GC_ANNOUNCEMENTS} WEBSERVER_CATALOG: ${WB_GC_CATALOG} WEBSERVER_CLUSTERS: ${WB_GC_CLUSTERS} + WEBSERVER_DB_LISTENER: ${WB_GC_DB_LISTENER} WEBSERVER_DIAGNOSTICS: ${WB_GC_DIAGNOSTICS} WEBSERVER_EMAIL: ${WB_GC_EMAIL} WEBSERVER_EXPORTER: ${WB_GC_EXPORTER} WEBSERVER_FOLDERS: ${WB_GC_FOLDERS} WEBSERVER_FRONTEND: ${WB_GC_FRONTEND} + WEBSERVER_GARBAGE_COLLECTOR: ${WB_GC_GARBAGE_COLLECTOR} WEBSERVER_GROUPS: ${WB_GC_GROUPS} + WEBSERVER_HOST: ${WEBSERVER_HOST} WEBSERVER_INVITATIONS: ${WB_GC_INVITATIONS} WEBSERVER_LICENSES: null WEBSERVER_LOGIN: ${WB_GC_LOGIN} + WEBSERVER_LOGLEVEL: ${WB_GC_LOGLEVEL} WEBSERVER_NOTIFICATIONS: ${WB_GC_NOTIFICATIONS} WEBSERVER_PAYMENTS: ${WB_GC_PAYMENTS} + WEBSERVER_PORT: ${WEBSERVER_PORT} WEBSERVER_PRODUCTS: ${WB_GC_PRODUCTS} WEBSERVER_PROJECTS: ${WB_GC_PROJECTS} WEBSERVER_PUBLICATIONS: ${WB_GC_PUBLICATIONS} @@ -1043,12 +1056,7 @@ services: WEBSERVER_USERS: ${WB_GC_USERS} WEBSERVER_WALLETS: ${WB_GC_WALLETS} - # WEBSERVER_RABBITMQ - RABBIT_HOST: ${RABBIT_HOST} - RABBIT_PASSWORD: ${RABBIT_PASSWORD} - RABBIT_PORT: ${RABBIT_PORT} - RABBIT_SECURE: ${RABBIT_SECURE} - RABBIT_USER: ${RABBIT_USER} + networks: - default - interactive_services_subnet From 10a561290c02342d7d1358d3134fbf99c1c004fd Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 15:07:36 +0100 Subject: [PATCH 38/44] adds plugin --- .../src/simcore_service_webserver/garbage_collector/plugin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py b/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py index 663d94166cdf..7374eccd5611 100644 --- a/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py +++ b/services/web/server/src/simcore_service_webserver/garbage_collector/plugin.py @@ -3,6 +3,7 @@ from aiohttp import web from servicelib.aiohttp.application_setup import ModuleCategory, app_module_setup from servicelib.logging_utils import set_parent_module_log_level +from simcore_service_webserver.products.plugin import setup_products from ..application_settings import get_application_settings from ..login.plugin import setup_login_storage @@ -21,6 +22,9 @@ logger=_logger, ) def setup_garbage_collector(app: web.Application) -> None: + # for trashing + setup_products(app) + # - project-api needs access to db setup_projects_db(app) # - project needs access to socketio via notify_project_state_update From 0ccdc9fa6763cbbfc3131b52d04338f1b30c0135 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 15:33:47 +0100 Subject: [PATCH 39/44] adds logs --- .../simcore_service_webserver/trash/_service.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index b1bc4fbe801b..7f558dcfd3b0 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -123,7 +123,7 @@ async def safe_delete_expired_trash_as_admin(app: web.Application) -> None: with log_context( _logger, logging.DEBUG, - "Deleting items marked as trashed before %s in %s [retention=%s]", + "Deleting items marked as trashed before %s in %s [trashed_at < %s will be deleted]", retention, product.display_name, delete_until, @@ -148,12 +148,16 @@ async def safe_delete_expired_trash_as_admin(app: web.Application) -> None: try: - await projects_trash_service.batch_delete_trashed_projects_as_admin( - app, - trashed_before=delete_until, - fail_fast=False, + deleted_project_ids = ( + await projects_trash_service.batch_delete_trashed_projects_as_admin( + app, + trashed_before=delete_until, + fail_fast=False, + ) ) + _logger.info("Deleted %d trashed projects", len(deleted_project_ids)) + except Exception as exc: # pylint: disable=broad-exception-caught _logger.warning( **create_troubleshotting_log_kwargs( From 1433f199d6c8a61cafc2a7bd0cb968acb2af41ed Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 16:39:18 +0100 Subject: [PATCH 40/44] rm dependency with products domain --- .env-devel | 2 +- .../src/simcore_service_webserver/products/_api.py | 5 +++++ .../src/simcore_service_webserver/products/_db.py | 7 +++++++ .../src/simcore_service_webserver/trash/_service.py | 10 ++++++---- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/.env-devel b/.env-devel index f808111fb3d9..e4f191a0c1ec 100644 --- a/.env-devel +++ b/.env-devel @@ -288,7 +288,7 @@ WB_GC_LOGIN=null WB_GC_LOGLEVEL=INFO WB_GC_NOTIFICATIONS=0 WB_GC_PAYMENTS=null -WB_GC_PRODUCTS=1 +WB_GC_PRODUCTS=0 WB_GC_PROJECTS=null WB_GC_PUBLICATIONS=0 WB_GC_RESOURCE_MANAGER_RESOURCE_TTL_S=60 diff --git a/services/web/server/src/simcore_service_webserver/products/_api.py b/services/web/server/src/simcore_service_webserver/products/_api.py index ce2c03b87966..f7b516dfb738 100644 --- a/services/web/server/src/simcore_service_webserver/products/_api.py +++ b/services/web/server/src/simcore_service_webserver/products/_api.py @@ -42,6 +42,11 @@ def list_products(app: web.Application) -> list[Product]: return products +async def list_products_names(app: web.Application) -> list[ProductName]: + repo = ProductRepository.create_from_app(app) + return await repo.list_products_names() + + async def get_current_product_credit_price_info( request: web.Request, ) -> ProductPriceInfo | None: diff --git a/services/web/server/src/simcore_service_webserver/products/_db.py b/services/web/server/src/simcore_service_webserver/products/_db.py index a481c0f993e1..311d90bba069 100644 --- a/services/web/server/src/simcore_service_webserver/products/_db.py +++ b/services/web/server/src/simcore_service_webserver/products/_db.py @@ -86,6 +86,13 @@ async def iter_products(conn: SAConnection) -> AsyncIterator[ResultProxy]: class ProductRepository(BaseRepository): + async def list_products_names(self) -> list[ProductName]: + async with self.engine.acquire() as conn: + query = sa.select(products.c.name).order_by(products.c.priority) + result = await conn.execute(query) + rows = await result.fetchall() + return [ProductName(row.name) for row in rows] + async def get_product(self, product_name: str) -> Product | None: async with self.engine.acquire() as conn: result: ResultProxy = await conn.execute( diff --git a/services/web/server/src/simcore_service_webserver/trash/_service.py b/services/web/server/src/simcore_service_webserver/trash/_service.py index 7f558dcfd3b0..42a5f6f0626c 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_service.py +++ b/services/web/server/src/simcore_service_webserver/trash/_service.py @@ -112,12 +112,14 @@ async def safe_delete_expired_trash_as_admin(app: web.Application) -> None: retention = timedelta(days=settings.TRASH_RETENTION_DAYS) delete_until = arrow.now().datetime - retention - for product in products_service.list_products(app): + app_products_names = await products_service.list_products_names(app) + + for product_name in app_products_names: ctx = { "delete_until": delete_until, "retention": retention, - "product_name": product.name, + "product_name": product_name, } with log_context( @@ -125,7 +127,7 @@ async def safe_delete_expired_trash_as_admin(app: web.Application) -> None: logging.DEBUG, "Deleting items marked as trashed before %s in %s [trashed_at < %s will be deleted]", retention, - product.display_name, + product_name, delete_until, ): try: @@ -133,7 +135,7 @@ async def safe_delete_expired_trash_as_admin(app: web.Application) -> None: await folders_trash_service.batch_delete_trashed_folders_as_admin( app, trashed_before=delete_until, - product_name=product.name, + product_name=product_name, fail_fast=False, ) From b3536dd4039df7513ce551b5774fef8ff3206466 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 16:54:54 +0100 Subject: [PATCH 41/44] sonar --- .../folders/_folders_repository.py | 50 ++++++++----------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 8c98a0f8dc76..b5769d4e458f 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -467,6 +467,24 @@ async def delete_recursively( ) +def _create_folder_hierarchy_cte(base_query: Select): + folder_hierarchy_cte = base_query.cte(name="folder_hierarchy", recursive=True) + + # Step 2: Define the recursive case + folder_alias = aliased(folders_v2) + recursive_query = sql.select( + folder_alias.c.folder_id, folder_alias.c.parent_folder_id + ).select_from( + folder_alias.join( + folder_hierarchy_cte, + folder_alias.c.parent_folder_id == folder_hierarchy_cte.c.folder_id, + ) + ) + + # Step 3: Combine base and recursive cases into a CTE + return folder_hierarchy_cte.union_all(recursive_query) + + async def get_projects_recursively_only_if_user_is_owner( app: web.Application, connection: AsyncConnection | None = None, @@ -493,21 +511,9 @@ async def get_projects_recursively_only_if_user_is_owner( (folders_v2.c.folder_id == folder_id) # <-- specified folder id & (folders_v2.c.product_name == product_name) ) - folder_hierarchy_cte = base_query.cte(name="folder_hierarchy", recursive=True) - # Step 2: Define the recursive case - folder_alias = aliased(folders_v2) - recursive_query = sql.select( - folder_alias.c.folder_id, folder_alias.c.parent_folder_id - ).select_from( - folder_alias.join( - folder_hierarchy_cte, - folder_alias.c.parent_folder_id == folder_hierarchy_cte.c.folder_id, - ) - ) - - # Step 3: Combine base and recursive cases into a CTE - folder_hierarchy_cte = folder_hierarchy_cte.union_all(recursive_query) + # Step 2,3 + folder_hierarchy_cte = _create_folder_hierarchy_cte(base_query) # Step 4: Execute the query to get all descendants final_query = sql.select(folder_hierarchy_cte) @@ -551,21 +557,9 @@ async def get_all_folders_and_projects_ids_recursively( (folders_v2.c.folder_id == folder_id) # <-- specified folder id & (folders_v2.c.product_name == product_name) ) - folder_hierarchy_cte = base_query.cte(name="folder_hierarchy", recursive=True) - # Step 2: Define the recursive case - folder_alias = aliased(folders_v2) - recursive_query = sql.select( - folder_alias.c.folder_id, folder_alias.c.parent_folder_id - ).select_from( - folder_alias.join( - folder_hierarchy_cte, - folder_alias.c.parent_folder_id == folder_hierarchy_cte.c.folder_id, - ) - ) - - # Step 3: Combine base and recursive cases into a CTE - folder_hierarchy_cte = folder_hierarchy_cte.union_all(recursive_query) + # Step 2, 3 + folder_hierarchy_cte = _create_folder_hierarchy_cte(base_query) # Step 4: Execute the query to get all descendants final_query = sql.select(folder_hierarchy_cte) From 64a2b17d634ddb68450d799a82c29833c781e50f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 16:57:47 +0100 Subject: [PATCH 42/44] mypy --- .../web/server/src/simcore_service_webserver/products/_api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/products/_api.py b/services/web/server/src/simcore_service_webserver/products/_api.py index f7b516dfb738..5b7a3532ea00 100644 --- a/services/web/server/src/simcore_service_webserver/products/_api.py +++ b/services/web/server/src/simcore_service_webserver/products/_api.py @@ -44,7 +44,8 @@ def list_products(app: web.Application) -> list[Product]: async def list_products_names(app: web.Application) -> list[ProductName]: repo = ProductRepository.create_from_app(app) - return await repo.list_products_names() + names: list[ProductName] = await repo.list_products_names() + return names async def get_current_product_credit_price_info( From 3a57c083083387d691accb8f6930b5faa26d72fb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 16:59:49 +0100 Subject: [PATCH 43/44] sonar --- .../folders/_folders_repository.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index b5769d4e458f..ae524a97c2b9 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -596,19 +596,8 @@ async def get_folders_recursively( ) folder_hierarchy_cte = base_query.cte(name="folder_hierarchy", recursive=True) - # Step 2: Define the recursive case - folder_alias = aliased(folders_v2) - recursive_query = sql.select( - folder_alias.c.folder_id, folder_alias.c.parent_folder_id - ).select_from( - folder_alias.join( - folder_hierarchy_cte, - folder_alias.c.parent_folder_id == folder_hierarchy_cte.c.folder_id, - ) - ) - - # Step 3: Combine base and recursive cases into a CTE - folder_hierarchy_cte = folder_hierarchy_cte.union_all(recursive_query) + # Step 2, 3 + folder_hierarchy_cte = _create_folder_hierarchy_cte(base_query) # Step 4: Execute the query to get all descendants final_query = sql.select(folder_hierarchy_cte) From 2548ba8a3120d5c03ab1976e46ad17c279582761 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Feb 2025 17:17:06 +0100 Subject: [PATCH 44/44] sonar --- .../src/simcore_service_webserver/folders/_folders_repository.py | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index ae524a97c2b9..31ce1b0fc8e0 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -594,7 +594,6 @@ async def get_folders_recursively( (folders_v2.c.folder_id == folder_id) # <-- specified folder id & (folders_v2.c.product_name == product_name) ) - folder_hierarchy_cte = base_query.cte(name="folder_hierarchy", recursive=True) # Step 2, 3 folder_hierarchy_cte = _create_folder_hierarchy_cte(base_query)