diff --git a/Makefile b/Makefile index a6f2ffe1cfb..51489a6762e 100644 --- a/Makefile +++ b/Makefile @@ -189,11 +189,24 @@ endif define _show_endpoints # The following endpoints are available -echo "http://$(if $(IS_WSL2),$(get_my_ip),127.0.0.1):9081 - oSparc platform" -echo "http://$(if $(IS_WSL2),$(get_my_ip),127.0.0.1):18080/?pgsql=postgres&username=scu&db=simcoredb&ns=public - Postgres DB" -echo "http://$(if $(IS_WSL2),$(get_my_ip),127.0.0.1):9000 - Portainer" +set -o allexport; \ +source $(CURDIR)/.env; \ +set +o allexport; \ +separator=------------------------------------------;\ +separator=$${separator}$${separator}$${separator};\ +rows="%-80s| %22s| %12s| %12s\n";\ +TableWidth=140;\ +printf "%80s| %22s| %12s| %12s\n" Endpoint Name User Password;\ +printf "%.$${TableWidth}s\n" "$$separator";\ +printf "$$rows" 'http://$(if $(IS_WSL2),$(get_my_ip),127.0.0.1):9081' 'oSparc platform';\ +printf "$$rows" 'http://$(if $(IS_WSL2),$(get_my_ip),127.0.0.1):18080/?pgsql=postgres&username=$${POSTGRES_USER}&db=$${POSTGRES_DB}&ns=public' 'Postgres DB' $${POSTGRES_USER} $${POSTGRES_PASSWORD};\ +printf "$$rows" 'http://$(if $(IS_WSL2),$(get_my_ip),127.0.0.1):9000' Portainer admin adminadmin;\ +printf "$$rows" 'http://$(if $(IS_WSL2),$(get_my_ip),127.0.0.1):18081' Redis endef +show-endpoints: + @$(_show_endpoints) + up-devel: .stack-simcore-development.yml .init-swarm $(CLIENT_WEB_OUTPUT) ## Deploys local development stack, qx-compile+watch and ops stack (pass 'make ops_disabled=1 up-...' to disable) # Start compile+watch front-end container [front-end] @$(MAKE_C) services/web/client down compile-dev flags=--watch diff --git a/README.md b/README.md index 508ce29bb26..6e31c12800c 100644 --- a/README.md +++ b/README.md @@ -10,8 +10,7 @@ [![Requires.io]](https://requires.io/github/ITISFoundation/osparc-simcore/requirements/?branch=master "State of third party python dependencies") [![Github-CI Push/PR]](https://github.com/ITISFoundation/osparc-simcore/actions?query=workflow%3A%22Github-CI+Push%2FPR%22+branch%3Amaster) -[![coveralls.io]](https://coveralls.io/github/ITISFoundation/osparc-simcore?branch=master) -[![codecov.io]](https://codecov.io/gh/ITISFoundation/osparc-simcore) +[![codecov](https://codecov.io/gh/ITISFoundation/osparc-simcore/branch/master/graph/badge.svg?token=h1rOE8q7ic)](https://codecov.io/gh/ITISFoundation/osparc-simcore) [![github.io]](https://itisfoundation.github.io/) [![itis.dockerhub]](https://hub.docker.com/u/itisfoundation) [![license]](./LICENSE) @@ -23,8 +22,6 @@ [travis-ci]:https://travis-ci.org/ITISFoundation/osparc-simcore.svg?branch=master [github.io]:https://img.shields.io/website-up-down-green-red/https/itisfoundation.github.io.svg?label=documentation [itis.dockerhub]:https://img.shields.io/website/https/hub.docker.com/u/itisfoundation.svg?down_color=red&label=dockerhub%20repos&up_color=green -[coveralls.io]:https://coveralls.io/repos/github/ITISFoundation/osparc-simcore/badge.svg?branch=master -[codecov.io]:https://codecov.io/gh/ITISFoundation/osparc-simcore/branch/master/graph/badge.svg [license]:https://img.shields.io/github/license/ITISFoundation/osparc-simcore [Github-CI Push/PR]:https://github.com/ITISFoundation/osparc-simcore/workflows/Github-CI%20Push/PR/badge.svg diff --git a/api/specs/common/schemas/node-meta-v0.0.1-converted.yaml b/api/specs/common/schemas/node-meta-v0.0.1-converted.yaml index a1e0bb08951..912b978a09a 100644 --- a/api/specs/common/schemas/node-meta-v0.0.1-converted.yaml +++ b/api/specs/common/schemas/node-meta-v0.0.1-converted.yaml @@ -16,7 +16,7 @@ properties: key: type: string description: distinctive name for the node based on the docker registry path - pattern: ^(simcore)/(services)/(comp|dynamic)(/[\w/-]+)+$ + pattern: ^(simcore)/(services)/(comp|dynamic|frontend)(/[\w/-]+)+$ example: simcore/services/comp/itis/sleeper integration-version: type: string diff --git a/api/specs/common/schemas/node-meta-v0.0.1.json b/api/specs/common/schemas/node-meta-v0.0.1.json index f303bb45ec7..660045befab 100644 --- a/api/specs/common/schemas/node-meta-v0.0.1.json +++ b/api/specs/common/schemas/node-meta-v0.0.1.json @@ -19,7 +19,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/itis/sleeper", "simcore/services/dynamic/3dviewer" diff --git a/api/specs/common/schemas/project-v0.0.1-converted.yaml b/api/specs/common/schemas/project-v0.0.1-converted.yaml index afba7b52a5b..59f7222720c 100644 --- a/api/specs/common/schemas/project-v0.0.1-converted.yaml +++ b/api/specs/common/schemas/project-v0.0.1-converted.yaml @@ -405,11 +405,11 @@ properties: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: If locked, the user that owns the lock allOf: - title: Owner type: object @@ -439,8 +439,20 @@ properties: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state diff --git a/api/specs/common/schemas/project-v0.0.1.json b/api/specs/common/schemas/project-v0.0.1.json index 29e79aefaf8..fb06837091e 100644 --- a/api/specs/common/schemas/project-v0.0.1.json +++ b/api/specs/common/schemas/project-v0.0.1.json @@ -116,7 +116,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/sleeper", "simcore/services/dynamic/3dviewer", @@ -556,12 +556,12 @@ "properties": { "value": { "title": "Value", - "description": "True if the project is locked by another user", + "description": "True if the project is locked", "type": "boolean" }, "owner": { "title": "Owner", - "description": "The user that owns the lock", + "description": "If locked, the user that owns the lock", "allOf": [ { "title": "Owner", @@ -600,10 +600,24 @@ ] } ] + }, + "status": { + "title": "Status", + "description": "The status of the project", + "enum": [ + "CLOSED", + "CLOSING", + "CLONING", + "OPENING", + "EXPORTING", + "OPENED" + ], + "type": "string" } }, "required": [ - "value" + "value", + "status" ] } ] diff --git a/packages/models-library/src/models_library/projects_state.py b/packages/models-library/src/models_library/projects_state.py index a5c1c959187..ad9227f519e 100644 --- a/packages/models-library/src/models_library/projects_state.py +++ b/packages/models-library/src/models_library/projects_state.py @@ -5,7 +5,7 @@ from enum import Enum, unique from typing import Optional -from pydantic import BaseModel, Extra, Field +from pydantic import BaseModel, Extra, Field, validator from .projects_access import Owner @@ -29,14 +29,60 @@ class DataState(str, Enum): OUTDATED = "OUTDATED" +@unique +class ProjectStatus(str, Enum): + CLOSED = "CLOSED" + CLOSING = "CLOSING" + CLONING = "CLONING" + EXPORTING = "EXPORTING" + OPENING = "OPENING" + OPENED = "OPENED" + + class ProjectLocked(BaseModel): - value: bool = Field( - ..., description="True if the project is locked by another user" + value: bool = Field(..., description="True if the project is locked") + owner: Optional[Owner] = Field( + None, description="If locked, the user that owns the lock" ) - owner: Optional[Owner] = Field(None, description="The user that owns the lock") + status: ProjectStatus = Field(..., description="The status of the project") class Config: extra = Extra.forbid + use_enum_values = True + schema_extra = { + "examples": [ + {"value": False, "status": ProjectStatus.CLOSED}, + { + "value": True, + "status": ProjectStatus.OPENED, + "owner": { + "user_id": 123, + "first_name": "Johnny", + "last_name": "Cash", + }, + }, + ] + } + + @validator("owner", pre=True, always=True) + @classmethod + def check_not_null(v, values): + if values["value"] is True and v is None: + raise ValueError("value cannot be None when project is locked") + return v + + @validator("status", always=True) + @classmethod + def check_status_compatible(v, values): + if values["value"] is False and v not in ["CLOSED", "OPENED"]: + raise ValueError( + f"status is set to {v} and lock is set to {values['value']}!" + ) + if values["value"] is True and v == "CLOSED": + raise ValueError( + f"status is set to {v} and lock is set to {values['value']}!" + ) + return v class ProjectRunningState(BaseModel): diff --git a/packages/models-library/tests/test_projects_state.py b/packages/models-library/tests/test_projects_state.py new file mode 100644 index 00000000000..d4b73b689a1 --- /dev/null +++ b/packages/models-library/tests/test_projects_state.py @@ -0,0 +1,35 @@ +from pprint import pformat + +import pytest +from models_library.projects_state import ProjectLocked, ProjectStatus + + +@pytest.mark.parametrize( + "model_cls", + (ProjectLocked,), +) +def test_projects_state_model_examples(model_cls, model_cls_examples): + for name, example in model_cls_examples.items(): + print(name, ":", pformat(example)) + model_instance = model_cls(**example) + assert model_instance, f"Failed with {name}" + + +def test_project_locked_with_missing_owner_raises(): + with pytest.raises(ValueError): + ProjectLocked(**{"value": True, "status": ProjectStatus.OPENED}) + ProjectLocked.parse_obj({"value": False, "status": ProjectStatus.OPENED}) + + +@pytest.mark.parametrize( + "lock, status", + [ + (False, x) + for x in ProjectStatus + if x not in [ProjectStatus.CLOSED, ProjectStatus.OPENED] + ] + + [(True, ProjectStatus.CLOSED)], +) +def test_project_locked_with_allowed_values(lock: bool, status: ProjectStatus): + with pytest.raises(ValueError): + ProjectLocked.parse_obj({"value": lock, "status": status}) diff --git a/packages/models-library/tests/test_services.py b/packages/models-library/tests/test_services.py index 568ab6a48b6..0b05824ef39 100644 --- a/packages/models-library/tests/test_services.py +++ b/packages/models-library/tests/test_services.py @@ -4,10 +4,11 @@ import re from pprint import pformat -from typing import Any, Dict +from typing import Any, Callable, Dict, List import pytest import yaml +from models_library.basic_regex import VERSION_RE from models_library.services import ( SERVICE_KEY_RE, ServiceAccessRightsAtDB, @@ -103,7 +104,6 @@ def test_service_models_examples(model_cls, model_cls_examples): assert model_instance, f"Failed with {name}" -@pytest.mark.skip(reason="dev") @pytest.mark.parametrize( "service_key", [ @@ -159,7 +159,7 @@ def test_service_models_examples(model_cls, model_cls_examples): [SERVICE_KEY_RE, r"^(simcore)/(services)/(comp|dynamic|frontend)(/[^\s/]+)+$"], ids=["pattern_with_w", "pattern_with_s"], ) -def test_service_key_regex_patterns(service_key, regex_pattern): +def test_service_key_regex_patterns(service_key: str, regex_pattern: str): match = re.match(regex_pattern, service_key) assert match @@ -178,3 +178,36 @@ def test_services_model_examples(model_cls, model_cls_examples): print(name, ":", pformat(example)) model_instance = model_cls(**example) assert model_instance, f"Failed with {name}" + + +@pytest.mark.parametrize( + "python_regex_pattern, json_schema_file_name, json_schema_entry_paths", + [ + (SERVICE_KEY_RE, "project-v0.0.1.json", ["key"]), + (VERSION_RE, "project-v0.0.1.json", ["version"]), + (VERSION_RE, "node-meta-v0.0.1.json", ["version"]), + (SERVICE_KEY_RE, "node-meta-v0.0.1.json", ["key"]), + ], +) +def test_regex_pattern_same_in_jsonschema_and_python( + python_regex_pattern: str, + json_schema_file_name: str, + json_schema_entry_paths: List[str], + json_schema_dict: Callable, +): + # read file in + json_schema_config = json_schema_dict(json_schema_file_name) + # go to keys + def _find_pattern_entry(obj: Dict[str, Any], key: str) -> Any: + if key in obj: + return obj[key]["pattern"] + for v in obj.values(): + if isinstance(v, dict): + item = _find_pattern_entry(v, key) + if item is not None: + return item + return None + + for x_path in json_schema_entry_paths: + json_pattern = _find_pattern_entry(json_schema_config, x_path) + assert json_pattern == python_regex_pattern diff --git a/packages/pytest-simcore/src/pytest_simcore/redis_service.py b/packages/pytest-simcore/src/pytest_simcore/redis_service.py index 7f19072c5d2..4694dda44f7 100644 --- a/packages/pytest-simcore/src/pytest_simcore/redis_service.py +++ b/packages/pytest-simcore/src/pytest_simcore/redis_service.py @@ -49,7 +49,7 @@ async def redis_service(redis_config: RedisConfig, monkeypatch) -> RedisConfig: return redis_config -@pytest.fixture(scope="module") +@pytest.fixture(scope="function") async def redis_client(loop, redis_config: RedisConfig) -> Iterator[aioredis.Redis]: client = await aioredis.create_redis_pool(redis_config.dsn, encoding="utf-8") diff --git a/services/api-server/tests/unit/test_utils_solver_job_models_converters.py b/services/api-server/tests/unit/test_utils_solver_job_models_converters.py index 1a6063c6818..49de0989e97 100644 --- a/services/api-server/tests/unit/test_utils_solver_job_models_converters.py +++ b/services/api-server/tests/unit/test_utils_solver_job_models_converters.py @@ -173,7 +173,13 @@ def test_create_job_from_project(): }, "quality": {}, "tags": [], - "state": {"locked": {"value": False}, "state": {"value": "SUCCESS"}}, + "state": { + "locked": { + "value": False, + "status": "CLOSED", + }, + "state": {"value": "SUCCESS"}, + }, }, ) diff --git a/services/director/src/simcore_service_director/api/v0/openapi.yaml b/services/director/src/simcore_service_director/api/v0/openapi.yaml index 93f90afb366..ac0254f26cf 100644 --- a/services/director/src/simcore_service_director/api/v0/openapi.yaml +++ b/services/director/src/simcore_service_director/api/v0/openapi.yaml @@ -159,7 +159,7 @@ paths: key: type: string description: distinctive name for the node based on the docker registry path - pattern: '^(simcore)/(services)/(comp|dynamic)(/[\w/-]+)+$' + pattern: '^(simcore)/(services)/(comp|dynamic|frontend)(/[\w/-]+)+$' example: simcore/services/comp/itis/sleeper integration-version: type: string @@ -552,7 +552,7 @@ paths: key: type: string description: distinctive name for the node based on the docker registry path - pattern: '^(simcore)/(services)/(comp|dynamic)(/[\w/-]+)+$' + pattern: '^(simcore)/(services)/(comp|dynamic|frontend)(/[\w/-]+)+$' example: simcore/services/comp/itis/sleeper integration-version: type: string @@ -2199,7 +2199,7 @@ components: key: type: string description: distinctive name for the node based on the docker registry path - pattern: '^(simcore)/(services)/(comp|dynamic)(/[\w/-]+)+$' + pattern: '^(simcore)/(services)/(comp|dynamic|frontend)(/[\w/-]+)+$' example: simcore/services/comp/itis/sleeper integration-version: type: string diff --git a/services/director/src/simcore_service_director/api/v0/schemas/node-meta-v0.0.1.json b/services/director/src/simcore_service_director/api/v0/schemas/node-meta-v0.0.1.json index f303bb45ec7..660045befab 100644 --- a/services/director/src/simcore_service_director/api/v0/schemas/node-meta-v0.0.1.json +++ b/services/director/src/simcore_service_director/api/v0/schemas/node-meta-v0.0.1.json @@ -19,7 +19,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/itis/sleeper", "simcore/services/dynamic/3dviewer" diff --git a/services/director/src/simcore_service_director/api/v0/schemas/project-v0.0.1.json b/services/director/src/simcore_service_director/api/v0/schemas/project-v0.0.1.json index 29e79aefaf8..fb06837091e 100644 --- a/services/director/src/simcore_service_director/api/v0/schemas/project-v0.0.1.json +++ b/services/director/src/simcore_service_director/api/v0/schemas/project-v0.0.1.json @@ -116,7 +116,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/sleeper", "simcore/services/dynamic/3dviewer", @@ -556,12 +556,12 @@ "properties": { "value": { "title": "Value", - "description": "True if the project is locked by another user", + "description": "True if the project is locked", "type": "boolean" }, "owner": { "title": "Owner", - "description": "The user that owns the lock", + "description": "If locked, the user that owns the lock", "allOf": [ { "title": "Owner", @@ -600,10 +600,24 @@ ] } ] + }, + "status": { + "title": "Status", + "description": "The status of the project", + "enum": [ + "CLOSED", + "CLOSING", + "CLONING", + "OPENING", + "EXPORTING", + "OPENED" + ], + "type": "string" } }, "required": [ - "value" + "value", + "status" ] } ] diff --git a/services/storage/src/simcore_service_storage/api/v0/openapi.yaml b/services/storage/src/simcore_service_storage/api/v0/openapi.yaml index edb34ddc72d..58e07c84edc 100644 --- a/services/storage/src/simcore_service_storage/api/v0/openapi.yaml +++ b/services/storage/src/simcore_service_storage/api/v0/openapi.yaml @@ -1157,11 +1157,11 @@ components: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -1189,8 +1189,20 @@ components: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state diff --git a/services/storage/src/simcore_service_storage/api/v0/schemas/node-meta-v0.0.1.json b/services/storage/src/simcore_service_storage/api/v0/schemas/node-meta-v0.0.1.json index f303bb45ec7..660045befab 100644 --- a/services/storage/src/simcore_service_storage/api/v0/schemas/node-meta-v0.0.1.json +++ b/services/storage/src/simcore_service_storage/api/v0/schemas/node-meta-v0.0.1.json @@ -19,7 +19,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/itis/sleeper", "simcore/services/dynamic/3dviewer" diff --git a/services/storage/src/simcore_service_storage/api/v0/schemas/project-v0.0.1.json b/services/storage/src/simcore_service_storage/api/v0/schemas/project-v0.0.1.json index 29e79aefaf8..fb06837091e 100644 --- a/services/storage/src/simcore_service_storage/api/v0/schemas/project-v0.0.1.json +++ b/services/storage/src/simcore_service_storage/api/v0/schemas/project-v0.0.1.json @@ -116,7 +116,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/sleeper", "simcore/services/dynamic/3dviewer", @@ -556,12 +556,12 @@ "properties": { "value": { "title": "Value", - "description": "True if the project is locked by another user", + "description": "True if the project is locked", "type": "boolean" }, "owner": { "title": "Owner", - "description": "The user that owns the lock", + "description": "If locked, the user that owns the lock", "allOf": [ { "title": "Owner", @@ -600,10 +600,24 @@ ] } ] + }, + "status": { + "title": "Status", + "description": "The status of the project", + "enum": [ + "CLOSED", + "CLOSING", + "CLONING", + "OPENING", + "EXPORTING", + "OPENED" + ], + "type": "string" } }, "required": [ - "value" + "value", + "status" ] } ] diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index d35ac74cc97..c3cbfda354b 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -473,7 +473,8 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { if (locked) { this.setLocked(state["locked"]["value"]); const owner = state["locked"]["owner"]; - this.__setLockedBy(osparc.utils.Utils.firstsUp(owner["first_name"], owner["last_name"])); + const status = state["locked"]["status"]; + this.__setLockedBy(osparc.utils.Utils.firstsUp(owner["first_name"], owner["last_name"]), status); } else { this.setLocked(false); } @@ -510,10 +511,28 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { }); }, - __setLockedBy: function(lockedBy) { + __setLockedBy: function(lockedBy, status) { const lock = this.getChildControl("lock"); + let toolTipText = null; + switch (status) { + case "CLOSING": + toolTipText = lockedBy + this.tr(" is closing it..."); + break; + case "CLONING": + toolTipText = lockedBy + this.tr(" is cloning it..."); + break; + case "EXPORTING": + toolTipText = lockedBy + this.tr(" is exporting it..."); + break; + case "OPENING": + toolTipText = lockedBy + this.tr(" is opening it..."); + break; + case "OPENED": + toolTipText = lockedBy + this.tr(" is using it."); + break; + } lock.set({ - toolTipText: lockedBy ? (lockedBy + this.tr(" is using it")) : null + toolTipText: toolTipText }); }, diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 4899374db78..1df7bd1ef4b 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -6525,11 +6525,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -6557,8 +6557,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -7126,11 +7138,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -7158,8 +7170,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -7601,11 +7625,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -7633,8 +7657,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -8194,11 +8230,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -8226,8 +8262,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -8793,11 +8841,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -8825,8 +8873,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -9383,11 +9443,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -9415,8 +9475,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -9858,11 +9930,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -9890,8 +9962,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -10473,11 +10557,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -10505,8 +10589,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -12221,11 +12317,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -12253,8 +12349,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state @@ -12813,11 +12921,11 @@ paths: properties: value: title: Value - description: True if the project is locked by another user + description: True if the project is locked type: boolean owner: title: Owner - description: The user that owns the lock + description: 'If locked, the user that owns the lock' allOf: - title: Owner type: object @@ -12845,8 +12953,20 @@ paths: - user_id - first_name - last_name + status: + title: Status + description: The status of the project + enum: + - CLOSED + - CLOSING + - CLONING + - OPENING + - EXPORTING + - OPENED + type: string required: - value + - status state: title: State description: The project running state diff --git a/services/web/server/src/simcore_service_webserver/api/v0/schemas/node-meta-v0.0.1.json b/services/web/server/src/simcore_service_webserver/api/v0/schemas/node-meta-v0.0.1.json index f303bb45ec7..660045befab 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/schemas/node-meta-v0.0.1.json +++ b/services/web/server/src/simcore_service_webserver/api/v0/schemas/node-meta-v0.0.1.json @@ -19,7 +19,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/itis/sleeper", "simcore/services/dynamic/3dviewer" diff --git a/services/web/server/src/simcore_service_webserver/api/v0/schemas/project-v0.0.1.json b/services/web/server/src/simcore_service_webserver/api/v0/schemas/project-v0.0.1.json index 29e79aefaf8..fb06837091e 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/schemas/project-v0.0.1.json +++ b/services/web/server/src/simcore_service_webserver/api/v0/schemas/project-v0.0.1.json @@ -116,7 +116,7 @@ "key": { "type": "string", "description": "distinctive name for the node based on the docker registry path", - "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[^\\s/]+)+$", + "pattern": "^(simcore)/(services)/(comp|dynamic|frontend)(/[\\w/-]+)+$", "examples": [ "simcore/services/comp/sleeper", "simcore/services/dynamic/3dviewer", @@ -556,12 +556,12 @@ "properties": { "value": { "title": "Value", - "description": "True if the project is locked by another user", + "description": "True if the project is locked", "type": "boolean" }, "owner": { "title": "Owner", - "description": "The user that owns the lock", + "description": "If locked, the user that owns the lock", "allOf": [ { "title": "Owner", @@ -600,10 +600,24 @@ ] } ] + }, + "status": { + "title": "Status", + "description": "The status of the project", + "enum": [ + "CLOSED", + "CLOSING", + "CLONING", + "OPENING", + "EXPORTING", + "OPENED" + ], + "type": "string" } }, "required": [ - "value" + "value", + "status" ] } ] diff --git a/services/web/server/src/simcore_service_webserver/exporter/export_import.py b/services/web/server/src/simcore_service_webserver/exporter/export_import.py index 49bbbf66ce0..5e395cf9ac0 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/export_import.py +++ b/services/web/server/src/simcore_service_webserver/exporter/export_import.py @@ -29,6 +29,7 @@ async def study_export( returns: directory if archive is True else a compressed archive is returned """ + # storage area for the project data base_temp_dir = Path(tmp_dir) destination = base_temp_dir / project_id @@ -45,7 +46,6 @@ async def study_export( return destination # an archive is always produced when compression is active - archive_path = await zip_folder( folder_to_zip=base_temp_dir, destination_folder=base_temp_dir ) @@ -105,4 +105,4 @@ async def study_duplicate( app: web.Application, user_id: int, exported_project_path: Path ) -> str: formatter: BaseFormatter = await validate_manifest(exported_project_path) - return await formatter.validate_and_import_directory(app=app, user_id=user_id) \ No newline at end of file + return await formatter.validate_and_import_directory(app=app, user_id=user_id) diff --git a/services/web/server/src/simcore_service_webserver/exporter/formatters/formatter_v1.py b/services/web/server/src/simcore_service_webserver/exporter/formatters/formatter_v1.py index e940f613a98..3da2b5acfcf 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/formatters/formatter_v1.py +++ b/services/web/server/src/simcore_service_webserver/exporter/formatters/formatter_v1.py @@ -13,21 +13,18 @@ from models_library.projects import AccessRights, Project from models_library.projects_nodes_io import BaseFileLink, NodeID from models_library.utils.nodes import compute_node_hash, project_node_io_payload_cb -from simcore_service_webserver.director_v2 import create_or_update_pipeline -from simcore_service_webserver.projects.projects_api import ( - delete_project, - get_project_for_user, -) -from simcore_service_webserver.projects.projects_db import APP_PROJECT_DBAPI -from simcore_service_webserver.projects.projects_exceptions import ProjectsException -from simcore_service_webserver.storage_handlers import ( + +from ...director_v2 import create_or_update_pipeline +from ...projects.projects_api import delete_project, get_project_for_user +from ...projects.projects_db import APP_PROJECT_DBAPI +from ...projects.projects_exceptions import ProjectsException +from ...storage_handlers import ( get_file_download_url, get_file_upload_url, get_project_files_metadata, ) -from simcore_service_webserver.users_api import get_user -from simcore_service_webserver.utils import now_str - +from ...users_api import get_user +from ...utils import now_str from ..exceptions import ExporterException from ..file_downloader import ParallelDownloader from ..utils import path_getsize @@ -42,7 +39,7 @@ async def download_all_files_from_storage( app: web.Application, download_links: Deque[LinkAndPath2] ) -> None: - """ Downloads links to files in their designed storage_path_to_file """ + """Downloads links to files in their designed storage_path_to_file""" parallel_downloader = ParallelDownloader() for link_and_path in download_links: log.debug( diff --git a/services/web/server/src/simcore_service_webserver/exporter/formatters/models.py b/services/web/server/src/simcore_service_webserver/exporter/formatters/models.py index e5a156f3a36..7cb92039edb 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/formatters/models.py +++ b/services/web/server/src/simcore_service_webserver/exporter/formatters/models.py @@ -5,6 +5,7 @@ import aiofiles from models_library.projects import Project +from models_library.projects_state import ProjectStatus from pydantic import BaseModel, DirectoryPath, Field, validator from ..utils import makedirs @@ -131,3 +132,26 @@ def new_instance_from_shuffled_data( new_obj.storage_path = self.storage_path return new_obj + + # migration validators -------------------------------- + # NOTE: these migration validator are necessary when the base Project class is modified + # this allows importing an older project to the newest state + _MIGRATION_FLAGS = dict(pre=True, always=True) + + @validator("state", **_MIGRATION_FLAGS) + @classmethod + def optional_project_state_added_locked_status(cls, v): + """{"state":Optional[{"locked": {"value": bool}}}] + --> + {"state":Optional[{"locked": {"value": bool, "status": ProjectStatus}}}]""" + + # ProjectStatus is optional + if v is not None: + # get locked. old is {"locked": {"value": bool}} + locked = v.get("locked") + if not locked: + raise ValueError(f"missing locked field in {v}") + if not locked.get("status"): + locked["status"] = ProjectStatus.CLOSED.value + + return v diff --git a/services/web/server/src/simcore_service_webserver/exporter/request_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/request_handlers.py index e2c3ba25707..da835de25ca 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/request_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/request_handlers.py @@ -3,15 +3,19 @@ from aiohttp import web from aiohttp.web_request import FileField +from models_library.projects_state import ProjectStatus +from simcore_service_webserver.projects.project_lock import lock_project +from simcore_service_webserver.users_api import get_user_name +from ..constants import RQ_PRODUCT_KEY from ..login.decorators import RQT_USERID_KEY, login_required +from ..projects.projects_api import retrieve_and_notify_project_locked_state from ..security_decorators import permission_required -from ..constants import RQ_PRODUCT_KEY from .config import get_settings from .exceptions import ExporterException -from .export_import import study_export, study_import, study_duplicate -from .utils import CleanupFileResponse, get_empty_tmp_dir, remove_dir +from .export_import import study_duplicate, study_export, study_import from .formatters import FormatterV1 +from .utils import CleanupFileResponse, get_empty_tmp_dir, remove_dir ONE_GB: int = 1024 * 1024 * 1024 @@ -38,24 +42,38 @@ async def export_project(request: web.Request): temp_dir: str = await get_empty_tmp_dir() try: - file_to_download = await study_export( - app=request.app, - tmp_dir=temp_dir, - project_id=project_uuid, - user_id=user_id, - product_name=request[RQ_PRODUCT_KEY], - archive=True, - ) - log.info("File to download '%s'", file_to_download) - - if not file_to_download.is_file(): - raise ExporterException( - f"Must provide a file to download, not {str(file_to_download)}" + async with await lock_project( + request.app, + project_uuid, + ProjectStatus.EXPORTING, + user_id, + await get_user_name(request.app, user_id), + ): + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, request.app + ) + file_to_download = await study_export( + app=request.app, + tmp_dir=temp_dir, + project_id=project_uuid, + user_id=user_id, + product_name=request[RQ_PRODUCT_KEY], + archive=True, ) + log.info("File to download '%s'", file_to_download) + + if not file_to_download.is_file(): + raise ExporterException( + f"Must provide a file to download, not {str(file_to_download)}" + ) except Exception as e: # make sure all errors are trapped and the directory where the file is sotred is removed await remove_dir(temp_dir) raise e + finally: + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, request.app + ) headers = {"Content-Disposition": f'attachment; filename="{file_to_download.name}"'} @@ -97,28 +115,42 @@ async def import_project(request: web.Request): async def duplicate_project(request: web.Request): user_id = request[RQT_USERID_KEY] project_uuid = request.match_info.get("project_id") - - with TemporaryDirectory() as temp_dir: - exported_project_path = await study_export( - app=request.app, - tmp_dir=temp_dir, - project_id=project_uuid, - user_id=user_id, - product_name=request[RQ_PRODUCT_KEY], - archive=False, - formatter_class=FormatterV1, - ) - log.info("Study to duplicate '%s'", exported_project_path) - - # return the duplicated study ID - duplicated_project_uuid = await study_duplicate( - app=request.app, - user_id=user_id, - exported_project_path=exported_project_path, + try: + with TemporaryDirectory() as temp_dir: + async with await lock_project( + request.app, + project_uuid, + ProjectStatus.CLONING, + user_id, + await get_user_name(request.app, user_id), + ): + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, request.app + ) + exported_project_path = await study_export( + app=request.app, + tmp_dir=temp_dir, + project_id=project_uuid, + user_id=user_id, + product_name=request[RQ_PRODUCT_KEY], + archive=False, + formatter_class=FormatterV1, + ) + log.info("Study to duplicate '%s'", exported_project_path) + + # return the duplicated study ID + duplicated_project_uuid = await study_duplicate( + app=request.app, + user_id=user_id, + exported_project_path=exported_project_path, + ) + return dict(uuid=duplicated_project_uuid) + finally: + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, request.app ) - return dict(uuid=duplicated_project_uuid) rest_handler_functions = { fun.__name__: fun for fun in {export_project, import_project, duplicate_project} -} \ No newline at end of file +} diff --git a/services/web/server/src/simcore_service_webserver/projects/project_lock.py b/services/web/server/src/simcore_service_webserver/projects/project_lock.py new file mode 100644 index 00000000000..b9843ee2553 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/projects/project_lock.py @@ -0,0 +1,65 @@ +from typing import Dict, Optional, Union + +import aioredlock +from aiohttp import web +from models_library.projects import ProjectID +from models_library.projects_state import Owner, ProjectLocked, ProjectStatus +from simcore_service_webserver.resource_manager.redis import ( + get_redis_lock_manager, + get_redis_lock_manager_client, +) + +PROJECT_REDIS_LOCK_KEY: str = "project:{}" + +ProjectLock = aioredlock.Lock +ProjectLockError = aioredlock.LockError + + +async def lock_project( + app: web.Application, + project_uuid: Union[str, ProjectID], + status: ProjectStatus, + user_id: int, + user_name: Dict[str, str], +) -> ProjectLock: + """returns a distributed redis lock on the project defined by its UUID. + NOTE: can be used as a context manager + + try: + async with await lock_project(app, project_uuid, ProjectStatus.CLOSING, user_id, user_name): + close_project(project_uuid) # do something with the project that requires the project to be locked + + + except aioredlock.LockError: + pass # the lock could not be acquired + + """ + return await get_redis_lock_manager(app).lock( + PROJECT_REDIS_LOCK_KEY.format(project_uuid), + lock_timeout=None, + lock_identifier=ProjectLocked( + value=True, + owner=Owner(user_id=user_id, **user_name), + status=status, + ).json(), + ) + + +async def is_project_locked( + app: web.Application, project_uuid: Union[str, ProjectID] +) -> bool: + return await get_redis_lock_manager(app).is_locked( + PROJECT_REDIS_LOCK_KEY.format(project_uuid) + ) + + +async def get_project_locked_state( + app: web.Application, project_uuid: Union[str, ProjectID] +) -> Optional[ProjectLocked]: + """returns the ProjectLocked object if the project is locked""" + if await is_project_locked(app, project_uuid): + project_locked: Optional[str] = await get_redis_lock_manager_client(app).get( + PROJECT_REDIS_LOCK_KEY.format(project_uuid) + ) + if project_locked: + return ProjectLocked.parse_raw(project_locked) diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index 4ef072e0eca..6218151613e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -17,17 +17,16 @@ from uuid import uuid4 from aiohttp import web - from models_library.projects_state import ( Owner, ProjectLocked, ProjectRunningState, ProjectState, + ProjectStatus, RunningState, ) from servicelib.application_keys import APP_JSONSCHEMA_SPECS_KEY from servicelib.jsonschema_validation import validate_instance -from servicelib.observer import observe from servicelib.utils import fire_and_forget_task, logged_gather from simcore_service_webserver.director import director_exceptions @@ -37,7 +36,7 @@ get_computation_task, request_retrieve_dyn_service, ) -from ..resource_manager.websocket_manager import managed_resource +from ..resource_manager.websocket_manager import PROJECT_ID_KEY, managed_resource from ..socketio.events import ( SOCKET_IO_NODE_UPDATED_EVENT, SOCKET_IO_PROJECT_UPDATED_EVENT, @@ -49,10 +48,13 @@ ) from ..users_api import get_user_name, is_user_guest from .config import CONFIG_SECTION_NAME +from .project_lock import ProjectLockError, get_project_locked_state, lock_project from .projects_db import APP_PROJECT_DBAPI log = logging.getLogger(__name__) +PROJECT_REDIS_LOCK_KEY: str = "project:{}" + def _is_node_dynamic(node_key: str) -> bool: return "/dynamic/" in node_key @@ -68,8 +70,8 @@ async def get_project_for_user( project_uuid: str, user_id: int, *, - include_templates: bool = False, - include_state: bool = False, + include_templates: Optional[bool] = False, + include_state: Optional[bool] = False, ) -> Dict: """Returns a VALID project accessible to user @@ -175,7 +177,9 @@ async def delete_project(app: web.Application, project_uuid: str, user_id: int) await delete_project_from_db(app, project_uuid, user_id) async def remove_services_and_data(): - await remove_project_interactive_services(user_id, project_uuid, app) + await remove_project_interactive_services( + user_id, project_uuid, app, notify_users=False + ) await delete_project_data(app, project_uuid, user_id) fire_and_forget_task(remove_services_and_data()) @@ -184,19 +188,94 @@ async def remove_services_and_data(): ## PROJECT NODES ----------------------------------------------------- -@observe(event="SIGNAL_PROJECT_CLOSE") +async def retrieve_and_notify_project_locked_state( + user_id: int, project_uuid: str, app: web.Application +): + project = await get_project_for_user(app, project_uuid, user_id, include_state=True) + await notify_project_state_update(app, project) + + +# TODO: Once python 3.8 is in use this +# @contextlib.asynccontextmanager +# async def lock_with_notification( +# app: web.Application, +# project_uuid: str, +# status: ProjectStatus, +# user_id: int, +# user_name: Dict[str, str], +# notify_users: bool = True, +# ): +# try: +# async with await lock_project( +# app, +# project_uuid, +# status, +# user_id, +# user_name, +# ): +# if notify_users: +# await retrieve_and_notify_project_locked_state( +# user_id, project_uuid, app +# ) +# yield + +# finally: +# if notify_users: +# await retrieve_and_notify_project_locked_state(user_id, project_uuid, app) + + async def remove_project_interactive_services( - user_id: Optional[int], project_uuid: Optional[str], app: web.Application + user_id: int, project_uuid: str, app: web.Application, notify_users: bool = True ) -> None: - # save the state if the user is not a guest. if we do not know we save in any case. - with suppress(director_exceptions.DirectorException): - # here director exceptions are suppressed. in case the service is not found to preserve old behavior - await director_api.stop_services( - app=app, - user_id=user_id, - project_id=project_uuid, - save_state=not await is_user_guest(app, user_id) if user_id else True, + # NOTE: during the closing process, which might take awhile, + # the project is locked so no one opens it at the same time + try: + log.debug( + "removing project interactive services for project [%s] and user [%s]", + project_uuid, + user_id, ) + async with await lock_project( + app, + project_uuid, + ProjectStatus.CLOSING, + user_id, + await get_user_name(app, user_id), + ): + if notify_users: + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, app + ) + + # save the state if the user is not a guest. if we do not know we save in any case. + with suppress(director_exceptions.DirectorException): + # here director exceptions are suppressed. in case the service is not found to preserve old behavior + await director_api.stop_services( + app=app, + user_id=user_id, + project_id=project_uuid, + save_state=not await is_user_guest(app, user_id) + if user_id + else True, + ) + except ProjectLockError: + # maybe the someone else is already closing + prj_states: ProjectState = await get_project_states_for_user( + user_id, project_uuid, app + ) + if prj_states.locked.status not in [ + ProjectStatus.CLOSED, + ProjectStatus.CLOSING, + ]: + log.error( + "lock for project [%s] was already taken, current state is %s. project could not be closed please check.", + project_uuid, + prj_states.locked.status, + ) + finally: + # notify when done and the project is closed + if notify_users: + await retrieve_and_notify_project_locked_state(user_id, project_uuid, app) async def delete_project_data( @@ -477,32 +556,213 @@ async def trigger_connected_service_retrieve( # PROJECT STATE ------------------------------------------------------------------- +async def _user_has_another_client_open( + user_session_id_list: List[Tuple[int, str]], app: web.Application +) -> bool: + # NOTE if there is an active socket in use, that means the client is active + for user_id, client_session_id in user_session_id_list: + with managed_resource(user_id, client_session_id, app) as rt: + if await rt.get_socket_id() is not None: + return True + return False + + +async def _clean_user_disconnected_clients( + user_session_id_list: List[Tuple[int, str]], app: web.Application +): + for user_id, client_session_id in user_session_id_list: + with managed_resource(user_id, client_session_id, app) as rt: + if await rt.get_socket_id() is None: + log.debug( + "removing disconnected project of user %s/%s", + user_id, + client_session_id, + ) + await rt.remove(PROJECT_ID_KEY) + + +async def try_open_project_for_user( + user_id: int, project_uuid: str, client_session_id: str, app: web.Application +) -> bool: + try: + async with await lock_project( + app, + project_uuid, + ProjectStatus.OPENING, + user_id, + await get_user_name(app, user_id), + ): + log.debug( + "project [%s] lock acquired, now checking if project is available", + project_uuid, + ) + with managed_resource(user_id, client_session_id, app) as rt: + user_session_id_list: List[ + Tuple[int, str] + ] = await rt.find_users_of_resource(PROJECT_ID_KEY, project_uuid) + + if not user_session_id_list: + # no one has the project so we lock it + await rt.add(PROJECT_ID_KEY, project_uuid) + return True + + set_user_ids = {uid for uid, _ in user_session_id_list} + if set_user_ids.issubset({user_id}): + # we are the only user + if not await _user_has_another_client_open( + user_session_id_list, app + ): + # steal the project + await rt.add(PROJECT_ID_KEY, project_uuid) + await _clean_user_disconnected_clients( + user_session_id_list, app + ) + return True + return False + + except ProjectLockError: + # the project is currently locked + return False + + +async def try_close_project_for_user( + user_id: int, + project_uuid: str, + client_session_id: str, + app: web.Application, +): + with managed_resource(user_id, client_session_id, app) as rt: + user_to_session_ids: List[Tuple(int, str)] = await rt.find_users_of_resource( + PROJECT_ID_KEY, project_uuid + ) + # first check we have it opened now + if (user_id, client_session_id) not in user_to_session_ids: + # nothing to do the project is already closed + log.warning( + "project [%s] is already closed for user [%s].", + project_uuid, + user_id, + ) + return + # remove the project from our list of opened ones + log.debug( + "removing project [%s] from user [%s] resources", project_uuid, user_id + ) + await rt.remove(PROJECT_ID_KEY) + # check it is not opened by someone else + user_to_session_ids.remove((user_id, client_session_id)) + log.debug("remaining user_to_session_ids: %s", user_to_session_ids) + if not user_to_session_ids: + # NOTE: depending on the garbage collector speed, it might already be removing it + fire_and_forget_task( + remove_project_interactive_services(user_id, project_uuid, app) + ) + else: + log.warning( + "project [%s] is used by other users: [%s]. This should not be possible", + project_uuid, + {uid for uid, _ in user_to_session_ids}, + ) + + async def _get_project_lock_state( - user_id: int, project_uuid: str, app: web.Application + user_id: int, + project_uuid: str, + app: web.Application, ) -> ProjectLocked: + """returns the lock state of a project + 1. If a project is locked for any reason, first return the project as locked and STATUS defined by lock + 2. If a client_session_id is passed, then first check to see if the project is currently opened by this very user/tab combination, if yes returns the project as Locked and OPENED. + 3. If any other user that user_id is using the project (even disconnected before the TTL is finished) then the project is Locked and OPENED. + 4. If the same user is using the project with a valid socket id (meaning a tab is currently active) then the project is Locked and OPENED. + 5. If the same user is using the project with NO socket id (meaning there is no current tab active) then the project is Unlocked and OPENED. which means the user can open it again. + """ + log.debug("getting project [%s] lock state for user [%s]...", project_uuid, user_id) + prj_locked_state: Optional[ProjectLocked] = await get_project_locked_state( + app, project_uuid + ) + if prj_locked_state: + return prj_locked_state + + # let's now check if anyone has the project in use somehow with managed_resource(user_id, None, app) as rt: - # checks who is using it - users_of_project = await rt.find_users_of_resource("project_id", project_uuid) - usernames = [await get_user_name(app, uid) for uid in set(users_of_project)] - assert ( - len(usernames) <= 1 - ) # nosec # currently not possible to have more than 1 - - # based on usage, sets an state - is_locked: bool = len(usernames) > 0 - if is_locked: + user_session_id_list: List[Tuple[int, str]] = await rt.find_users_of_resource( + PROJECT_ID_KEY, project_uuid + ) + set_user_ids = {x for x, _ in user_session_id_list} + + assert ( + len(set_user_ids) <= 1 + ) # nosec # NOTE: A project can only be opened by one user in one tab at the moment + + if not set_user_ids: + # no one has the project, so it is unlocked and closed. + log.debug("project [%s] is not in use", project_uuid) + return ProjectLocked(value=False, status=ProjectStatus.CLOSED) + + log.debug( + "project [%s] might be used by the following users: [%s]", + project_uuid, + set_user_ids, + ) + usernames: List[Dict[str, str]] = [ + await get_user_name(app, uid) for uid in set_user_ids + ] + # let's check if the project is opened by the same user, maybe already opened or closed in a orphaned session + if set_user_ids.issubset({user_id}): + if not await _user_has_another_client_open(user_session_id_list, app): + # in this case the project is re-openable by the same user until it gets closed + log.debug( + "project [%s] is in use by the same user [%s] that is currently disconnected, so it is unlocked for this specific user and opened", + project_uuid, + set_user_ids, + ) return ProjectLocked( - value=is_locked, - owner=Owner(user_id=users_of_project[0], **usernames[0]), + value=False, + owner=Owner(user_id=list(set_user_ids)[0], **usernames[0]), + status=ProjectStatus.OPENED, ) - return ProjectLocked(value=is_locked) + # the project is opened in another tab or browser, or by another user, both case resolves to the project being locked, and opened + log.debug( + "project [%s] is in use by another user [%s], so it is locked", + project_uuid, + set_user_ids, + ) + return ProjectLocked( + value=True, + owner=Owner(user_id=list(set_user_ids)[0], **usernames[0]), + status=ProjectStatus.OPENED, + ) + + +async def get_project_states_for_user( + user_id: int, project_uuid: str, app: web.Application +) -> ProjectState: + # for templates: the project is never locked and never opened. also the running state is always unknown + lock_state = ProjectLocked(value=False, status=ProjectStatus.CLOSED) + running_state = RunningState.UNKNOWN + lock_state, computation_task = await logged_gather( + _get_project_lock_state(user_id, project_uuid, app), + get_computation_task(app, user_id, project_uuid), + ) + if computation_task: + # get the running state + running_state = computation_task.state + + return ProjectState( + locked=lock_state, state=ProjectRunningState(value=running_state) + ) async def add_project_states_for_user( - user_id: int, project: Dict[str, Any], is_template: bool, app: web.Application + user_id: int, + project: Dict[str, Any], + is_template: bool, + app: web.Application, ) -> Dict[str, Any]: - lock_state = ProjectLocked(value=False) + # for templates: the project is never locked and never opened. also the running state is always unknown + lock_state = ProjectLocked(value=False, status=ProjectStatus.CLOSED) running_state = RunningState.UNKNOWN if not is_template: lock_state, computation_task = await logged_gather( diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_handlers.py b/services/web/server/src/simcore_service_webserver/projects/projects_handlers.py index 68bf85e418b..f81a01ccd7d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_handlers.py @@ -7,18 +7,17 @@ import logging from typing import Any, Coroutine, Dict, List, Optional, Set -import aioredlock from aiohttp import web from jsonschema import ValidationError from models_library.projects_state import ProjectState from servicelib.rest_pagination_utils import PageResponseLimitOffset -from servicelib.utils import fire_and_forget_task, logged_gather +from servicelib.utils import logged_gather from simcore_postgres_database.webserver_models import ProjectType as ProjectTypeDB from .. import catalog, director_v2 from ..constants import RQ_PRODUCT_KEY from ..login.decorators import RQT_USERID_KEY, login_required -from ..resource_manager.websocket_manager import managed_resource +from ..resource_manager.websocket_manager import PROJECT_ID_KEY, managed_resource from ..rest_utils import RESPONSE_MODEL_POLICY from ..security_api import check_permission from ..security_decorators import permission_required @@ -342,9 +341,12 @@ async def delete_project(request: web.Request): ) project_users: Set[int] = {} with managed_resource(user_id, None, request.app) as rt: - project_users = set( - await rt.find_users_of_resource("project_id", project_uuid) - ) + project_users = { + uid + for uid, _ in await rt.find_users_of_resource( + PROJECT_ID_KEY, project_uuid + ) + } # that project is still in use if user_id in project_users: raise web.HTTPForbidden( @@ -385,42 +387,22 @@ async def open_project(request: web.Request) -> web.Response: request.app, project_uuid=project_uuid, user_id=user_id, - include_templates=True, + include_templates=False, include_state=True, ) - async def try_add_project() -> Optional[Set[int]]: - with managed_resource(user_id, client_session_id, request.app) as rt: - try: - # NOTE: we need to lock the access to the project so that no one else opens it - # at the same time. - async with await rt.get_registry_lock(project_uuid): - other_users: Set[int] = set( - await rt.find_users_of_resource("project_id", project_uuid) - ) - if user_id in other_users: - other_users.remove(user_id) - if other_users: - return other_users - await rt.add("project_id", project_uuid) - except aioredlock.LockError as exc: - # TODO: this lock is not a good solution for long term - # maybe a project key in redis might improve spped of checking - raise HTTPLocked(reason="Project is locked") from exc - - other_users = await try_add_project() - if other_users: - # project is already locked - usernames = [ - await projects_api.get_user_name(request.app, uid) - for uid in other_users - ] - raise HTTPLocked(reason=f"Project is already opened by {usernames}") + if not await projects_api.try_open_project_for_user( + user_id, + project_uuid=project_uuid, + client_session_id=client_session_id, + app=request.app, + ): + raise HTTPLocked(reason="Project is locked, try later") # user id opened project uuid await projects_api.start_project_interactive_services(request, project, user_id) - # notify users that project is now locked + # notify users that project is now opened project = await projects_api.add_project_states_for_user( user_id=user_id, project=project, @@ -445,43 +427,16 @@ async def close_project(request: web.Request) -> web.Response: try: # ensure the project exists - project = await projects_api.get_project_for_user( + await projects_api.get_project_for_user( request.app, project_uuid=project_uuid, user_id=user_id, - include_templates=True, + include_templates=False, include_state=False, ) - # if we are the only user left we can safely remove the services - async def _close_project_task(project: Dict[str, Any]) -> None: - try: - project_opened_by_others: bool = False - with managed_resource(user_id, client_session_id, request.app) as rt: - project_users: List[int] = await rt.find_users_of_resource( - "project_id", project_uuid - ) - project_opened_by_others = len(project_users) > 1 - - if not project_opened_by_others: - # only remove the services if no one else is using them now - await projects_api.remove_project_interactive_services( - user_id, project_uuid, request.app - ) - finally: - with managed_resource(user_id, client_session_id, request.app) as rt: - # now we can remove the lock - await rt.remove("project_id") - # ensure we notify the user whatever happens, the GC should take care of dangling services in case of issue - project = await projects_api.add_project_states_for_user( - user_id=user_id, - project=project, - is_template=False, - app=request.app, - ) - await projects_api.notify_project_state_update(request.app, project) - - fire_and_forget_task(_close_project_task(project)) - + await projects_api.try_close_project_for_user( + user_id, project_uuid, client_session_id, request.app + ) raise web.HTTPNoContent(content_type="application/json") except ProjectNotFoundError as exc: raise web.HTTPNotFound(reason=f"Project {project_uuid} not found") from exc @@ -490,8 +445,13 @@ async def _close_project_task(project: Dict[str, Any]) -> None: @login_required @permission_required("project.read") async def state_project(request: web.Request) -> web.Response: + from servicelib.rest_utils import extract_and_validate + user_id = request[RQT_USERID_KEY] - project_uuid = request.match_info.get("project_id") + path, _, _ = await extract_and_validate(request) + + user_id = request[RQT_USERID_KEY] + project_uuid = path.get("project_id") # check that project exists and queries state validated_project = await projects_api.get_project_for_user( @@ -516,7 +476,7 @@ async def get_active_project(request: web.Request) -> web.Response: user_active_projects = [] with managed_resource(user_id, client_session_id, request.app) as rt: # get user's projects - user_active_projects = await rt.find("project_id") + user_active_projects = await rt.find(PROJECT_ID_KEY) if user_active_projects: project = await projects_api.get_project_for_user( diff --git a/services/web/server/src/simcore_service_webserver/resource_manager/config.py b/services/web/server/src/simcore_service_webserver/resource_manager/config.py index bd88ff5e9a6..01f4446f367 100644 --- a/services/web/server/src/simcore_service_webserver/resource_manager/config.py +++ b/services/web/server/src/simcore_service_webserver/resource_manager/config.py @@ -14,6 +14,9 @@ CONFIG_SECTION_NAME = "resource_manager" APP_CLIENT_REDIS_CLIENT_KEY = __name__ + ".resource_manager.redis_client" APP_CLIENT_REDIS_LOCK_MANAGER_KEY = __name__ + ".resource_manager.redis_lock" +APP_CLIENT_REDIS_LOCK_MANAGER_CLIENT_KEY = ( + __name__ + ".resource_manager.redis_lock_client" +) APP_CLIENT_SOCKET_REGISTRY_KEY = __name__ + ".resource_manager.registry" APP_RESOURCE_MANAGER_TASKS_KEY = __name__ + ".resource_manager.tasks.key" diff --git a/services/web/server/src/simcore_service_webserver/resource_manager/garbage_collector.py b/services/web/server/src/simcore_service_webserver/resource_manager/garbage_collector.py index d148c24c45d..dc61e89834a 100644 --- a/services/web/server/src/simcore_service_webserver/resource_manager/garbage_collector.py +++ b/services/web/server/src/simcore_service_webserver/resource_manager/garbage_collector.py @@ -9,7 +9,6 @@ from aiohttp import web from aiopg.sa.result import RowProxy from aioredlock import Aioredlock -from servicelib.observer import emit from servicelib.utils import logged_gather from .. import users_exceptions @@ -22,6 +21,7 @@ get_project_for_user, get_workbench_node_ids_from_project_uuid, is_node_id_present_in_any_project_workbench, + remove_project_interactive_services, ) from ..projects.projects_db import APP_PROJECT_DBAPI, ProjectAccessRights from ..projects.projects_exceptions import ProjectNotFoundError @@ -230,11 +230,7 @@ async def remove_disconnected_user_resources( if resource_name == "project_id": # inform that the project can be closed on the backend side # - # FIXME: slot functions are "whatever" and can e.g. raise any exception or - # delay or block execution here in many different ways - # - await emit( - event="SIGNAL_PROJECT_CLOSE", + await remove_project_interactive_services( user_id=int(dead_key["user_id"]), project_uuid=resource_value, app=app, @@ -326,7 +322,7 @@ async def remove_orphaned_services( If the service is a dynamic service """ - logger.info("Starting orphaned services removal...") + logger.debug("Starting orphaned services removal...") currently_opened_projects_node_ids = set() alive_keys, _ = await registry.get_all_resource_keys() @@ -342,7 +338,7 @@ async def remove_orphaned_services( running_interactive_services: List[ Dict[str, Any] ] = await get_running_interactive_services(app) - logger.info( + logger.debug( "Will collect the following: %s", [x["service_host"] for x in running_interactive_services], ) @@ -364,7 +360,6 @@ async def remove_orphaned_services( # if the node is not present in any of the currently opened project it shall be closed if node_id not in currently_opened_projects_node_ids: - service_host = interactive_service["service_host"] if interactive_service.get("service_state") in [ "pulling", "starting", @@ -406,7 +401,7 @@ async def remove_orphaned_services( except (ServiceNotFoundError, DirectorException) as err: logger.warning("Error while stopping service: %s", err) - logger.info("Finished orphaned services removal") + logger.debug("Finished orphaned services removal") async def remove_guest_user_with_all_its_resources( diff --git a/services/web/server/src/simcore_service_webserver/resource_manager/redis.py b/services/web/server/src/simcore_service_webserver/resource_manager/redis.py index 53d6fe3aae9..4263f6df855 100644 --- a/services/web/server/src/simcore_service_webserver/resource_manager/redis.py +++ b/services/web/server/src/simcore_service_webserver/resource_manager/redis.py @@ -4,11 +4,13 @@ import aioredis from aiohttp import web from aioredlock import Aioredlock -from servicelib.application_keys import APP_CONFIG_KEY from tenacity import AsyncRetrying, before_log, stop_after_attempt, wait_fixed +from servicelib.application_keys import APP_CONFIG_KEY + from .config import ( APP_CLIENT_REDIS_CLIENT_KEY, + APP_CLIENT_REDIS_LOCK_MANAGER_CLIENT_KEY, APP_CLIENT_REDIS_LOCK_MANAGER_KEY, CONFIG_SECTION_NAME, ) @@ -30,33 +32,43 @@ async def redis_client(app: web.Application): cfg = app[APP_CONFIG_KEY][CONFIG_SECTION_NAME] url = DSN.format(**cfg["redis"]) - # create redis client - client: Optional[aioredis.Redis] = None - async for attempt in AsyncRetrying(**retry_upon_init_policy): - with attempt: - client = await aioredis.create_redis_pool(url, encoding="utf-8") - if not client: - raise ValueError("Expected aioredis client instance, got {client}") - - # create lock manager but use DB 1 - lock_manager = Aioredlock([url + "/1"]) - + async def create_client(url) -> aioredis.Redis: + # create redis client + client: Optional[aioredis.Redis] = None + async for attempt in AsyncRetrying(**retry_upon_init_policy): + with attempt: + client = await aioredis.create_redis_pool(url, encoding="utf-8") + if not client: + raise ValueError("Expected aioredis client instance, got {client}") + return client + + app[APP_CLIENT_REDIS_CLIENT_KEY] = client = await create_client(url) assert client # nosec - app[APP_CLIENT_REDIS_CLIENT_KEY] = client + # create lock manager but use DB 1 + lock_db_url = url + "/1" + # create a client for it as well + app[ + APP_CLIENT_REDIS_LOCK_MANAGER_CLIENT_KEY + ] = client_lock_db = await create_client(lock_db_url) + assert client_lock_db # nosec + app[APP_CLIENT_REDIS_LOCK_MANAGER_KEY] = lock_manager = Aioredlock([lock_db_url]) assert lock_manager # nosec - app[APP_CLIENT_REDIS_LOCK_MANAGER_KEY] = lock_manager yield if client is not app[APP_CLIENT_REDIS_CLIENT_KEY]: log.critical("Invalid redis client in app") + if client_lock_db is not app[APP_CLIENT_REDIS_LOCK_MANAGER_CLIENT_KEY]: + log.critical("Invalid redis client for lock db in app") if lock_manager is not app[APP_CLIENT_REDIS_LOCK_MANAGER_KEY]: log.critical("Invalid redis lock manager in app") - # close client + # close clients client.close() await client.wait_closed() + client_lock_db.close() + await client_lock_db.wait_closed() # delete lock manager await lock_manager.destroy() @@ -81,3 +93,7 @@ def get_redis_client(app: web.Application) -> aioredis.Redis: def get_redis_lock_manager(app: web.Application) -> Aioredlock: return app[APP_CLIENT_REDIS_LOCK_MANAGER_KEY] + + +def get_redis_lock_manager_client(app: web.Application) -> aioredis.Redis: + return app[APP_CLIENT_REDIS_LOCK_MANAGER_CLIENT_KEY] diff --git a/services/web/server/src/simcore_service_webserver/resource_manager/websocket_manager.py b/services/web/server/src/simcore_service_webserver/resource_manager/websocket_manager.py index 4b4a5a37c99..61f3f36d3f3 100644 --- a/services/web/server/src/simcore_service_webserver/resource_manager/websocket_manager.py +++ b/services/web/server/src/simcore_service_webserver/resource_manager/websocket_manager.py @@ -15,20 +15,23 @@ """ import logging +from collections import namedtuple from contextlib import contextmanager from typing import Dict, Iterator, List, Optional, Union -import aioredlock import attr from aiohttp import web from .config import get_service_deletion_timeout -from .redis import get_redis_lock_manager from .registry import get_registry log = logging.getLogger(__file__) SOCKET_ID_KEY = "socket_id" +PROJECT_ID_KEY = "project_id" + + +UserSessionID = namedtuple("UserSessionID", "user_id client_session_id") @attr.s(auto_attribs=True) @@ -72,7 +75,7 @@ async def set_socket_id(self, socket_id: str) -> None: async def get_socket_id(self) -> Optional[str]: log.debug( - "user %s/tab %s removing socket from registry...", + "user %s/tab %s getting socket from registry...", self.user_id, self.client_session_id, ) @@ -150,7 +153,7 @@ async def remove(self, key: str) -> None: registry = get_registry(self.app) await registry.remove_resource(self._resource_key(), key) - async def find_users_of_resource(self, key: str, value: str) -> List[int]: + async def find_users_of_resource(self, key: str, value: str) -> List[UserSessionID]: log.debug( "user %s/tab %s finding %s:%s in registry...", self.user_id, @@ -160,26 +163,10 @@ async def find_users_of_resource(self, key: str, value: str) -> List[int]: ) registry = get_registry(self.app) registry_keys = await registry.find_keys((key, value)) - users = [int(x["user_id"]) for x in registry_keys] - return users - - async def get_registry_lock(self, resource_name: str) -> aioredlock.Lock: - """use in a context manager: - ```async with rt.get_gegistry_lock(resource_name="project_uuid") as lock: - # do some useful stuff - pass - ``` - """ - log.debug( - "user %s/tab %s getting registry lock...", - self.user_id, - self.client_session_id, - ) - # NOTE: passing a lock_timeout=None means the lock manager will auto-extend the timeout as long - # the context manager is on - return await get_redis_lock_manager(self.app).lock( - f"{__name__}.{resource_name}", lock_timeout=None - ) + user_session_id_list = [ + (int(x["user_id"]), x["client_session_id"]) for x in registry_keys + ] + return user_session_id_list @contextmanager diff --git a/services/web/server/src/simcore_service_webserver/users_api.py b/services/web/server/src/simcore_service_webserver/users_api.py index f0e81dc9f00..104399eb555 100644 --- a/services/web/server/src/simcore_service_webserver/users_api.py +++ b/services/web/server/src/simcore_service_webserver/users_api.py @@ -158,6 +158,8 @@ async def get_user_name(app: web.Application, user_id: int) -> Dict[str, str]: user_name = await conn.scalar( sa.select([users.c.name]).where(users.c.id == user_id) ) + if not user_name: + raise UserNotFoundError(uid=user_id) parts = user_name.split(".") + [""] return dict(first_name=parts[0], last_name=parts[1]) diff --git a/services/web/server/tests/unit/with_dbs/01/test_redis_registry.py b/services/web/server/tests/unit/with_dbs/01/test_redis_registry.py index ea606ebe3b5..92466304b64 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_redis_registry.py +++ b/services/web/server/tests/unit/with_dbs/01/test_redis_registry.py @@ -3,7 +3,7 @@ # pylint:disable=redefined-outer-name import time from random import randint -from typing import Dict, List +from typing import Dict, List, Tuple from uuid import uuid4 import pytest @@ -209,10 +209,10 @@ async def test_websocket_manager(loop, redis_enabled_app, redis_registry, user_i # resource key shall be filled assert await rt.find(res_key) == [res_value] list_of_same_resource_users: List[ - int + Tuple[int, str] ] = await rt.find_users_of_resource(res_key, res_value) assert list_user_ids[: (list_user_ids.index(user_id) + 1)] == sorted( - set(list_of_same_resource_users) + {uid for uid, _ in list_of_same_resource_users} ) # remove sockets diff --git a/services/web/server/tests/unit/with_dbs/02/test_access_to_studies.py b/services/web/server/tests/unit/with_dbs/02/test_access_to_studies.py index 2cdf5ba5c5d..746fc72c078 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_access_to_studies.py +++ b/services/web/server/tests/unit/with_dbs/02/test_access_to_studies.py @@ -17,7 +17,7 @@ from aiohttp import ClientResponse, ClientSession, web from aiohttp.test_utils import TestClient from aioresponses import aioresponses -from models_library.projects_state import ProjectLocked +from models_library.projects_state import ProjectLocked, ProjectStatus from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import UserRole from pytest_simcore.helpers.utils_mock import future_with_result @@ -200,7 +200,9 @@ def mocks_on_projects_api(mocker) -> None: """ mocker.patch( "simcore_service_webserver.projects.projects_api._get_project_lock_state", - return_value=future_with_result(ProjectLocked(value=False)), + return_value=future_with_result( + ProjectLocked(value=False, status=ProjectStatus.CLOSED) + ), ) diff --git a/services/web/server/tests/unit/with_dbs/02/test_studies_dispatcher_handlers.py b/services/web/server/tests/unit/with_dbs/02/test_studies_dispatcher_handlers.py index 4dd06189c97..daa8516eead 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_studies_dispatcher_handlers.py +++ b/services/web/server/tests/unit/with_dbs/02/test_studies_dispatcher_handlers.py @@ -13,7 +13,7 @@ import sqlalchemy as sa from aiohttp import ClientResponse, ClientSession, web from aioresponses import aioresponses -from models_library.projects_state import ProjectLocked +from models_library.projects_state import ProjectLocked, ProjectStatus from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import UserRole from pytest_simcore.helpers.utils_mock import future_with_result @@ -290,7 +290,9 @@ def mocks_on_projects_api(mocker): """ mocker.patch( "simcore_service_webserver.projects.projects_api._get_project_lock_state", - return_value=future_with_result(ProjectLocked(value=False)), + return_value=future_with_result( + ProjectLocked(value=False, status=ProjectStatus.CLOSED) + ), ) diff --git a/services/web/server/tests/unit/with_dbs/06/test_projects_02.py b/services/web/server/tests/unit/with_dbs/06/test_projects_02.py index 04977ffbd30..f7d5ab3632e 100644 --- a/services/web/server/tests/unit/with_dbs/06/test_projects_02.py +++ b/services/web/server/tests/unit/with_dbs/06/test_projects_02.py @@ -21,6 +21,7 @@ ProjectLocked, ProjectRunningState, ProjectState, + ProjectStatus, RunningState, ) from pytest_simcore.helpers.utils_assert import assert_status @@ -67,6 +68,7 @@ def client( postgres_db, mocked_director_subsystem, mock_orphaned_services, + redis_client, # this ensure redis is properly cleaned ): # config app @@ -104,32 +106,6 @@ def client( # teardown here ... -@pytest.fixture -def mocks_on_projects_api(mocker, logged_user) -> None: - """ - All projects in this module are UNLOCKED - - Emulates that it found logged_user as the SOLE user of this project - and returns the ProjectState indicating his as owner - """ - nameparts = logged_user["name"].split(".") + [""] - state = ProjectState( - locked=ProjectLocked( - value=False, - owner=Owner( - user_id=logged_user["id"], - first_name=nameparts[0], - last_name=nameparts[1], - ), - ), - state=ProjectRunningState(value=RunningState.NOT_STARTED), - ) - mocker.patch( - "simcore_service_webserver.projects.projects_api._get_project_lock_state", - return_value=future_with_result(state), - ) - - @pytest.fixture async def user_project(client, fake_project, logged_user): async with NewProject( @@ -455,10 +431,9 @@ async def _state_project( async def _assert_project_state_updated( handler: mock.Mock, shared_project: Dict, - expected_project_state: ProjectState, - num_calls: int, + expected_project_state_updates: List[ProjectState], ) -> None: - if num_calls == 0: + if not expected_project_state_updates: handler.assert_not_called() else: # wait for the calls @@ -466,11 +441,11 @@ async def _assert_project_state_updated( MAX_WAITING_TIME = 15 while time.monotonic() - now < MAX_WAITING_TIME: await asyncio.sleep(1) - if handler.call_count == num_calls: + if handler.call_count == len(expected_project_state_updates): break if time.monotonic() - now > MAX_WAITING_TIME: pytest.fail( - f"waited more than {MAX_WAITING_TIME}s and got only {handler.call_count}/{num_calls} calls" + f"waited more than {MAX_WAITING_TIME}s and got only {handler.call_count}/{len(expected_project_state_updates)} calls" ) calls = [ @@ -478,13 +453,12 @@ async def _assert_project_state_updated( json.dumps( { "project_uuid": shared_project["uuid"], - "data": expected_project_state.dict( - by_alias=True, exclude_unset=True - ), + "data": p_state.dict(by_alias=True, exclude_unset=True), } ) ) - ] * num_calls + for p_state in expected_project_state_updates + ] handler.assert_has_calls(calls) handler.reset_mock() @@ -975,41 +949,45 @@ async def test_open_shared_project_2_users_locked( client_id1, {SOCKET_IO_PROJECT_UPDATED_EVENT: mock_project_state_updated_handler}, ) - expected_project_state = ProjectState( - locked={"value": False}, + # expected is that the project is closed and unlocked + expected_project_state_client_1 = ProjectState( + locked=ProjectLocked(value=False, status=ProjectStatus.CLOSED), state=ProjectRunningState(value=RunningState.NOT_STARTED), ) - await _state_project( - client_1, - shared_project, - expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - expected_project_state, - ) + for client_id in [client_id1, None]: + await _state_project( + client_1, + shared_project, + expected.ok if user_role != UserRole.GUEST else web.HTTPOk, + expected_project_state_client_1, + ) await _open_project( client_1, client_id1, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, ) - expected_project_state.locked.value = True - expected_project_state.locked.owner = Owner( + # now the expected result is that the project is locked and opened by client 1 + owner1 = Owner( user_id=logged_user["id"], first_name=(logged_user["name"].split(".") + [""])[0], last_name=(logged_user["name"].split(".") + [""])[1], ) + expected_project_state_client_1.locked.value = True + expected_project_state_client_1.locked.status = ProjectStatus.OPENED + expected_project_state_client_1.locked.owner = owner1 # NOTE: there are 2 calls since we are part of the primary group and the all group await _assert_project_state_updated( mock_project_state_updated_handler, shared_project, - expected_project_state, - 0 if user_role == UserRole.ANONYMOUS else 2, + [expected_project_state_client_1] + * (0 if user_role == UserRole.ANONYMOUS else 2), ) - await _state_project( client_1, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - expected_project_state, + expected_project_state_client_1, ) # 2. create a separate client now and log in user2, try to open the same shared project @@ -1029,37 +1007,57 @@ async def test_open_shared_project_2_users_locked( shared_project, expected.locked if user_role != UserRole.GUEST else HTTPLocked, ) + expected_project_state_client_2 = deepcopy(expected_project_state_client_1) + expected_project_state_client_2.locked.status = ProjectStatus.OPENED + await _state_project( client_2, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - expected_project_state, + expected_project_state_client_2, ) # 3. user 1 closes the project await _close_project(client_1, client_id1, shared_project, expected.no_content) if not any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]): # Guests cannot close projects - expected_project_state = ProjectState( - locked=ProjectLocked(value=False), + expected_project_state_client_1 = ProjectState( + locked=ProjectLocked(value=False, status=ProjectStatus.CLOSED), state=ProjectRunningState(value=RunningState.NOT_STARTED), ) # we should receive an event that the project lock state changed - # NOTE: there are 3 calls since we are part of the primary group and the all group and user 2 is part of the all group + # NOTE: there are 2x3 calls since we are part of the primary group and the all group and user 2 is part of the all group + # first CLOSING, then CLOSED await _assert_project_state_updated( mock_project_state_updated_handler, shared_project, - expected_project_state, - 0 - if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]) - else 3, + [ + expected_project_state_client_1.copy( + update={ + "locked": ProjectLocked( + value=True, status=ProjectStatus.CLOSING, owner=owner1 + ) + } + ) + ] + * ( + 0 + if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]) + else 3 + ) + + [expected_project_state_client_1] + * ( + 0 + if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]) + else 3 + ), ) await _state_project( client_1, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - expected_project_state, + expected_project_state_client_1, ) # 4. user 2 now should be able to open the project @@ -1070,26 +1068,33 @@ async def test_open_shared_project_2_users_locked( expected.ok if user_role != UserRole.GUEST else HTTPLocked, ) if not any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]): - expected_project_state.locked.value = True - expected_project_state.locked.owner = Owner( + expected_project_state_client_2.locked.value = True + expected_project_state_client_2.locked.status = ProjectStatus.OPENED + owner2 = Owner( user_id=user_2["id"], first_name=(user_2["name"].split(".") + [""])[0], last_name=(user_2["name"].split(".") + [""])[1], ) + expected_project_state_client_2.locked.owner = owner2 + expected_project_state_client_1.locked.value = True + expected_project_state_client_1.locked.status = ProjectStatus.OPENED + expected_project_state_client_1.locked.owner = owner2 # NOTE: there are 3 calls since we are part of the primary group and the all group await _assert_project_state_updated( mock_project_state_updated_handler, shared_project, - expected_project_state, - 0 - if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]) - else 3, + [expected_project_state_client_1] + * ( + 0 + if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]) + else 3 + ), ) await _state_project( client_1, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - expected_project_state, + expected_project_state_client_1, ) diff --git a/services/web/server/tests/unit/with_dbs/09/test_projects_01.py b/services/web/server/tests/unit/with_dbs/09/test_projects_01.py index bc912f6d7e0..58b66886526 100644 --- a/services/web/server/tests/unit/with_dbs/09/test_projects_01.py +++ b/services/web/server/tests/unit/with_dbs/09/test_projects_01.py @@ -3,7 +3,6 @@ # pylint:disable=redefined-outer-name # pylint:disable=too-many-arguments import asyncio -import unittest.mock as mock import uuid as uuidlib from copy import deepcopy from math import ceil @@ -12,7 +11,6 @@ import aiohttp import pytest -import socketio from _helpers import ExpectedResponse, standard_role_response from aiohttp import web from aiohttp.test_utils import TestClient @@ -22,6 +20,7 @@ ProjectLocked, ProjectRunningState, ProjectState, + ProjectStatus, RunningState, ) from pytest_simcore.helpers.utils_assert import assert_status @@ -67,6 +66,7 @@ def client( postgres_db, mocked_director_subsystem, mock_orphaned_services, + redis_client, ): # config app @@ -126,6 +126,7 @@ def mocks_on_projects_api(mocker, logged_user) -> Dict: first_name=nameparts[0], last_name=nameparts[1], ), + status=ProjectStatus.CLOSED, ), state=ProjectRunningState(value=RunningState.NOT_STARTED), ) diff --git a/services/web/server/tests/unit/with_dbs/10/test_resource_manager.py b/services/web/server/tests/unit/with_dbs/10/test_resource_manager.py index 9f352789d7e..6081f9885e4 100644 --- a/services/web/server/tests/unit/with_dbs/10/test_resource_manager.py +++ b/services/web/server/tests/unit/with_dbs/10/test_resource_manager.py @@ -3,16 +3,19 @@ # pylint:disable=redefined-outer-name -from asyncio import sleep +import asyncio +from asyncio import Future, sleep from copy import deepcopy -from typing import Callable +from typing import Any, Callable, Dict from unittest.mock import call import pytest import socketio import socketio.exceptions +import sqlalchemy as sa from aiohttp import web from aiohttp.test_utils import TestClient +from aioredis import Redis from aioresponses import aioresponses from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_projects import NewProject @@ -22,7 +25,11 @@ from simcore_service_webserver.director_v2 import setup_director_v2 from simcore_service_webserver.login import setup_login from simcore_service_webserver.projects import setup_projects -from simcore_service_webserver.resource_manager import config, setup_resource_manager +from simcore_service_webserver.resource_manager import ( + config, + garbage_collector, + setup_resource_manager, +) from simcore_service_webserver.resource_manager.registry import ( RedisResourceRegistry, get_registry, @@ -40,7 +47,24 @@ @pytest.fixture -def client(loop, aiohttp_client, app_cfg, postgres_db, mock_orphaned_services): +def mock_garbage_collector_task(mocker): + """patch the setup of the garbage collector so we can call it manually""" + mocker.patch( + "simcore_service_webserver.resource_manager.module_setup.setup_garbage_collector", + return_value="", + ) + + +@pytest.fixture +def client( + mock_garbage_collector_task, + loop: asyncio.AbstractEventLoop, + aiohttp_client: TestClient, + app_cfg: Dict[str, Any], + postgres_db: sa.engine.Engine, + mock_orphaned_services, + redis_client: Redis, +): cfg = deepcopy(app_cfg) assert cfg["rest"]["version"] == API_VERSION @@ -348,7 +372,8 @@ async def test_interactive_services_removed_after_logout( assert r.url_obj.path == logout_url.path await assert_status(r, web.HTTPOk) # ensure sufficient time is wasted here - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL + 1) + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) # assert dynamic service is removed calls = [call(client.server.app, service["service_uuid"], exp_save_state)] mocked_director_api["stop_service"].assert_has_calls(calls) @@ -373,7 +398,6 @@ async def test_interactive_services_remain_after_websocket_reconnection_from_2_t storage_subsystem_mock, # when guest user logs out garbage is collected exp_save_state: bool, ): - set_service_deletion_delay(SERVICE_DELETION_DELAY, client.server.app) # login - logged_user fixture @@ -392,13 +416,14 @@ async def test_interactive_services_remain_after_websocket_reconnection_from_2_t client_session_id2 = client_session_id_factory() sio2 = await socketio_client_factory(client_session_id2) assert sio.sid != sio2.sid - # open project in second client - await open_project(client, empty_user_project["uuid"], client_session_id2) # disconnect first websocket await sio.disconnect() assert not sio.sid + # open project in second client + await open_project(client, empty_user_project["uuid"], client_session_id2) # ensure sufficient time is wasted here - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL) + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) # assert dynamic service is still around mocked_director_api["stop_service"].assert_not_called() # disconnect second websocket @@ -408,21 +433,34 @@ async def test_interactive_services_remain_after_websocket_reconnection_from_2_t mocked_director_api["stop_service"].assert_not_called() # reconnect websocket sio2 = await socketio_client_factory(client_session_id2) - # assert dynamic service is still around - mocked_director_api["stop_service"].assert_not_called() - # event after waiting some time + # it should still be there even after waiting for auto deletion from garbage collector await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) mocked_director_api["stop_service"].assert_not_called() # now really disconnect await sio2.disconnect() assert not sio2.sid - # we need to wait for the service deletion delay - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL + 1) + # run the garbage collector + # event after waiting some time + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) # assert dynamic service is gone calls = [call(client.server.app, service["service_uuid"], exp_save_state)] mocked_director_api["stop_service"].assert_has_calls(calls) +@pytest.fixture +async def mocked_notification_system(mocker): + mocks = {} + mocked_notification_system = mocker.patch( + "simcore_service_webserver.projects.projects_api.retrieve_and_notify_project_locked_state", + return_value=Future(), + ) + mocked_notification_system.return_value.set_result("") + mocks["mocked_notification_system"] = mocked_notification_system + yield mocks + + @pytest.mark.parametrize( "user_role, exp_save_state", [ @@ -438,6 +476,7 @@ async def test_interactive_services_removed_per_project( empty_user_project2, mocked_director_api, mocked_dynamic_service, + mocked_notification_system, socketio_client_factory: Callable, client_session_id_factory: Callable[[], str], asyncpg_storage_system_mock, @@ -475,7 +514,8 @@ async def test_interactive_services_removed_per_project( # assert dynamic service is still around mocked_director_api["stop_service"].assert_not_called() # wait the defined delay - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL) + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) # assert dynamic service 1 is removed calls = [call(client.server.app, service["service_uuid"], exp_save_state)] mocked_director_api["stop_service"].assert_has_calls(calls) @@ -487,7 +527,8 @@ async def test_interactive_services_removed_per_project( # assert dynamic services are still around mocked_director_api["stop_service"].assert_not_called() # wait the defined delay - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL + 2) + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) # assert dynamic service 2,3 is removed calls = [ call(client.server.app, service2["service_uuid"], exp_save_state), @@ -497,6 +538,9 @@ async def test_interactive_services_removed_per_project( mocked_director_api["stop_service"].reset_mock() +@pytest.mark.xfail( + reason="it is currently not permitted to open the same project from 2 different tabs" +) @pytest.mark.parametrize( "user_role, exp_save_state", [ @@ -536,13 +580,15 @@ async def test_services_remain_after_closing_one_out_of_two_tabs( # close project in tab1 await close_project(client, empty_user_project["uuid"], client_session_id1) # wait the defined delay - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL) + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) # assert dynamic service is still around mocked_director_api["stop_service"].assert_not_called() # close project in tab2 await close_project(client, empty_user_project["uuid"], client_session_id2) # wait the defined delay - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL) + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) mocked_director_api["stop_service"].assert_has_calls( [call(client.server.app, service["service_uuid"], exp_save_state)] ) @@ -588,7 +634,8 @@ async def test_websocket_disconnected_remove_or_maintain_files_based_on_role( await assert_status(r, web.HTTPOk) # ensure sufficient time is wasted here - await sleep(SERVICE_DELETION_DELAY + GARBAGE_COLLECTOR_INTERVAL + 1) + await sleep(SERVICE_DELETION_DELAY + 1) + await garbage_collector.collect_garbage(client.app) # assert dynamic service is removed calls = [call(client.server.app, service["service_uuid"], exp_save_state)]