From ae346180bf80ae0dbbc5d0613dc27c30a27d4dca Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 17 Jun 2020 14:40:22 +0200 Subject: [PATCH 01/71] write permissions needed to remove a user not delete permission --- services/web/server/src/simcore_service_webserver/groups_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/groups_api.py b/services/web/server/src/simcore_service_webserver/groups_api.py index 722fb264997..13613808540 100644 --- a/services/web/server/src/simcore_service_webserver/groups_api.py +++ b/services/web/server/src/simcore_service_webserver/groups_api.py @@ -153,7 +153,7 @@ async def delete_user_group(app: web.Application, user_id: int, gid: int) -> Non engine = app[APP_DB_ENGINE_KEY] async with engine.acquire() as conn: group = await _get_user_group(conn, user_id, gid) - check_group_permissions(group, user_id, gid, "delete") + check_group_permissions(group, user_id, gid, "write") await conn.execute( # pylint: disable=no-value-for-parameter From 2d8149bafcbdd8cf281ceeebb8b8482972b3aa0c Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 19 Jun 2020 17:54:49 +0200 Subject: [PATCH 02/71] adding group testing with more than 1 user --- .../server/tests/unit/with_dbs/test_groups.py | 112 ++++++++++++++++-- 1 file changed, 105 insertions(+), 7 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index d6da3bb01d3..6407e2d511c 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -59,13 +59,6 @@ def client(loop, aiohttp_client, app_cfg, postgres_service): return client -# WARNING: pytest-asyncio and pytest-aiohttp are not compatible -# -# https://github.com/aio-libs/pytest-aiohttp/issues/8#issuecomment-405602020 -# https://github.com/pytest-dev/pytest-asyncio/issues/76 -# - - @pytest.fixture async def logged_user(client, role: UserRole): """ adds a user in db and logs in with client @@ -567,3 +560,108 @@ async def test_add_remove_users_from_group( ) resp = await client.get(get_group_user_url) data, error = await assert_status(resp, expected_not_found) + + +@pytest.mark.parametrize( + "role, expected_created,expected_ok,expected_not_found,expected_no_content", + [ + ( + UserRole.ANONYMOUS, + web.HTTPUnauthorized, + web.HTTPUnauthorized, + web.HTTPUnauthorized, + web.HTTPUnauthorized, + ), + ( + UserRole.GUEST, + web.HTTPForbidden, + web.HTTPForbidden, + web.HTTPForbidden, + web.HTTPForbidden, + ), + ( + UserRole.USER, + web.HTTPCreated, + web.HTTPOk, + web.HTTPNotFound, + web.HTTPNoContent, + ), + ( + UserRole.TESTER, + web.HTTPCreated, + web.HTTPOk, + web.HTTPNotFound, + web.HTTPNoContent, + ), + ], +) +async def test_group_access_rights( + client, + logged_user, + role, + expected_created, + expected_ok, + expected_not_found, + expected_no_content, +): + # Use-case: + + + # 1. create a group + url = client.app.router["create_group"].url_for() + assert str(url) == f"{PREFIX}" + + new_group = { + "gid": "4564", + "label": f"this is user {logged_user['id']} group", + "description": f"user {logged_user['email']} is the owner of that one", + "thumbnail": None, + } + + resp = await client.post(url, json=new_group) + data, error = await assert_status(resp, expected_ok) + if not data: + # role cannot create a group so stop here + return + assigned_group = data + + # 1. have 2 users + users = [await create_user()]*2 + import pdb; pdb.set_trace() + # 2. add the users to the group + add_group_user_url = client.app.router["add_group_user"].url_for( + gid=str(assigned_group["gid"]) + ) + assert str(add_group_user_url) == f"{PREFIX}/{assigned_group['gid']}/users" + for i in users: + params = ( + {"uid": users[i]["id"]} + if i % 2 == 0 + else {"email": users[i]["email"]} + ) + resp = await client.post(add_group_user_url, json=params) + data, error = await assert_status(resp, expected_no_content) + # 3. user 1 shall be a manager + patch_group_user_url = client.app.router["update_group_user"].url_for( + gid=str(assigned_group["gid"]), uid=str(users[0]["id"]) + ) + assert str(patch_group_user_url) == f"{PREFIX}/{assigned_group['gid']}/users/{users[0]['id']}" + params={ + "read": True, + "write": True, + "delete": False + } + resp = await client.patch(patch_group_user_url, json=params) + data, error = await assert_status(resp, expected_ok) + # 4. user 2 shall be a member + patch_group_user_url = client.app.router["update_group_user"].url_for( + gid=str(assigned_group["gid"]), uid=users[1]["id"] + ) + assert str(patch_group_user_url) == f"{PREFIX}/{assigned_group['gid']}/users/{users[1]['id']}" + params={ + "read": True, + "write": False, + "delete": False + } + resp = await client.patch(patch_group_user_url, json=params) + data, error = await assert_status(resp, expected_ok) From 61f9261e2d0b7cef3d083fc3ad534d4604826011 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 11:24:39 +0200 Subject: [PATCH 03/71] small fixes --- .../web/server/tests/unit/with_dbs/test_groups.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index 6407e2d511c..ed8583fe862 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -605,8 +605,6 @@ async def test_group_access_rights( expected_no_content, ): # Use-case: - - # 1. create a group url = client.app.router["create_group"].url_for() assert str(url) == f"{PREFIX}" @@ -619,25 +617,25 @@ async def test_group_access_rights( } resp = await client.post(url, json=new_group) - data, error = await assert_status(resp, expected_ok) + data, error = await assert_status(resp, expected_created) if not data: # role cannot create a group so stop here return assigned_group = data # 1. have 2 users - users = [await create_user()]*2 - import pdb; pdb.set_trace() + users = [await create_user() for i in range(2)] + # 2. add the users to the group add_group_user_url = client.app.router["add_group_user"].url_for( gid=str(assigned_group["gid"]) ) assert str(add_group_user_url) == f"{PREFIX}/{assigned_group['gid']}/users" - for i in users: + for i, user in enumerate(users): params = ( - {"uid": users[i]["id"]} + {"uid": user["id"]} if i % 2 == 0 - else {"email": users[i]["email"]} + else {"email": user["email"]} ) resp = await client.post(add_group_user_url, json=params) data, error = await assert_status(resp, expected_no_content) From 2922a558ef9afa8d8fbd34c01553815c294615f9 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 11:49:17 +0200 Subject: [PATCH 04/71] fixing user in group patch endpoint --- api/specs/webserver/openapi-groups.yaml | 7 ++- .../api/v0/openapi.yaml | 51 ++++++++++--------- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/api/specs/webserver/openapi-groups.yaml b/api/specs/webserver/openapi-groups.yaml index 5de07b012bf..0379e779eb9 100644 --- a/api/specs/webserver/openapi-groups.yaml +++ b/api/specs/webserver/openapi-groups.yaml @@ -181,7 +181,12 @@ paths: content: application/json: schema: - $ref: "./components/schemas/group.yaml#/GroupAccessRights" + - type: object + required: + - accessRights + properties: + accessRights: + $ref: "./components/schemas/group.yaml#/GroupAccessRights" responses: "200": description: modified user 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 7e9f28a8dc5..17b1e325fcb 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 @@ -4269,29 +4269,34 @@ paths: content: application/json: schema: - description: defines acesss rights for the user - type: object - properties: - read: - type: boolean - write: - type: boolean - delete: - type: boolean - required: - - read - - write - - delete - example: - - read: true - write: false - delete: false - - read: true - write: true - delete: false - - read: true - write: true - delete: true + - type: object + required: + - accessRights + properties: + accessRights: + description: defines acesss rights for the user + type: object + properties: + read: + type: boolean + write: + type: boolean + delete: + type: boolean + required: + - read + - write + - delete + example: + - read: true + write: false + delete: false + - read: true + write: true + delete: false + - read: true + write: true + delete: true responses: '200': description: modified user From 890a9bcf9ae5fb2287682813081d3cf07f211c36 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 11:55:18 +0200 Subject: [PATCH 05/71] fix patch users in group endpoint --- api/specs/webserver/openapi-groups.yaml | 15 +++-- .../api/v0/openapi.yaml | 56 +++++++++---------- .../simcore_service_webserver/groups_api.py | 3 +- .../server/tests/unit/with_dbs/test_groups.py | 30 ++++------ 4 files changed, 51 insertions(+), 53 deletions(-) diff --git a/api/specs/webserver/openapi-groups.yaml b/api/specs/webserver/openapi-groups.yaml index 0379e779eb9..fc7fd24fb83 100644 --- a/api/specs/webserver/openapi-groups.yaml +++ b/api/specs/webserver/openapi-groups.yaml @@ -181,12 +181,15 @@ paths: content: application/json: schema: - - type: object - required: - - accessRights - properties: - accessRights: - $ref: "./components/schemas/group.yaml#/GroupAccessRights" + type: object + properties: + accessRights: + $ref: "./components/schemas/group.yaml#/GroupAccessRights" + required: + - accessRights + + + responses: "200": description: modified user 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 17b1e325fcb..882eff85c99 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 @@ -4269,34 +4269,34 @@ paths: content: application/json: schema: - - type: object - required: - - accessRights - properties: - accessRights: - description: defines acesss rights for the user - type: object - properties: - read: - type: boolean - write: - type: boolean - delete: - type: boolean - required: - - read - - write - - delete - example: - - read: true - write: false - delete: false - - read: true - write: true - delete: false - - read: true - write: true - delete: true + type: object + properties: + accessRights: + description: defines acesss rights for the user + type: object + properties: + read: + type: boolean + write: + type: boolean + delete: + type: boolean + required: + - read + - write + - delete + example: + - read: true + write: false + delete: false + - read: true + write: true + delete: false + - read: true + write: true + delete: true + required: + - accessRights responses: '200': description: modified user diff --git a/services/web/server/src/simcore_service_webserver/groups_api.py b/services/web/server/src/simcore_service_webserver/groups_api.py index 13613808540..0a6bde52515 100644 --- a/services/web/server/src/simcore_service_webserver/groups_api.py +++ b/services/web/server/src/simcore_service_webserver/groups_api.py @@ -271,10 +271,11 @@ async def update_user_in_group( conn, gid, the_user_id_in_group ) # modify the user access rights + new_db_values = {"access_rights": new_values_for_user_in_group["accessRights"]} await conn.execute( # pylint: disable=no-value-for-parameter user_to_groups.update() - .values(**new_values_for_user_in_group) + .values(**new_db_values) .where( and_( user_to_groups.c.uid == the_user_id_in_group, diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index ed8583fe862..632f46add22 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -632,34 +632,28 @@ async def test_group_access_rights( ) assert str(add_group_user_url) == f"{PREFIX}/{assigned_group['gid']}/users" for i, user in enumerate(users): - params = ( - {"uid": user["id"]} - if i % 2 == 0 - else {"email": user["email"]} - ) + params = {"uid": user["id"]} if i % 2 == 0 else {"email": user["email"]} resp = await client.post(add_group_user_url, json=params) data, error = await assert_status(resp, expected_no_content) # 3. user 1 shall be a manager patch_group_user_url = client.app.router["update_group_user"].url_for( gid=str(assigned_group["gid"]), uid=str(users[0]["id"]) ) - assert str(patch_group_user_url) == f"{PREFIX}/{assigned_group['gid']}/users/{users[0]['id']}" - params={ - "read": True, - "write": True, - "delete": False - } + assert ( + str(patch_group_user_url) + == f"{PREFIX}/{assigned_group['gid']}/users/{users[0]['id']}" + ) + params = {"accessRights": {"read": True, "write": True, "delete": False}} resp = await client.patch(patch_group_user_url, json=params) data, error = await assert_status(resp, expected_ok) # 4. user 2 shall be a member patch_group_user_url = client.app.router["update_group_user"].url_for( - gid=str(assigned_group["gid"]), uid=users[1]["id"] + gid=str(assigned_group["gid"]), uid=str(users[1]["id"]) ) - assert str(patch_group_user_url) == f"{PREFIX}/{assigned_group['gid']}/users/{users[1]['id']}" - params={ - "read": True, - "write": False, - "delete": False - } + assert ( + str(patch_group_user_url) + == f"{PREFIX}/{assigned_group['gid']}/users/{users[1]['id']}" + ) + params = {"accessRights": {"read": True, "write": False, "delete": False}} resp = await client.patch(patch_group_user_url, json=params) data, error = await assert_status(resp, expected_ok) From fd8f0d44f2c47f0e4a0e493903f106f5975688a2 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 12:24:20 +0200 Subject: [PATCH 06/71] access_rights renamed to accessRights --- .../webserver/components/schemas/group.yaml | 4 +- .../api/v0/openapi.yaml | 44 +++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/api/specs/webserver/components/schemas/group.yaml b/api/specs/webserver/components/schemas/group.yaml index ebb7bfde29e..12459b729da 100644 --- a/api/specs/webserver/components/schemas/group.yaml +++ b/api/specs/webserver/components/schemas/group.yaml @@ -42,13 +42,13 @@ UsersGroup: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: $ref: "#/GroupAccessRights" required: - gid - label - description - - access_rights + - accessRights example: - gid: "27" label: "A user" 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 882eff85c99..6738aafb055 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 @@ -2136,7 +2136,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -2164,7 +2164,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -2196,7 +2196,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -2224,7 +2224,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -2254,7 +2254,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -2282,7 +2282,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -2867,7 +2867,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -2895,7 +2895,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -2927,7 +2927,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -2955,7 +2955,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -2985,7 +2985,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -3013,7 +3013,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -3139,7 +3139,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -3167,7 +3167,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -3207,7 +3207,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -3235,7 +3235,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -3373,7 +3373,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -3401,7 +3401,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -3527,7 +3527,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -3555,7 +3555,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user @@ -3595,7 +3595,7 @@ paths: description: url to the group thumbnail type: string format: uri - access_rights: + accessRights: description: defines acesss rights for the user type: object properties: @@ -3623,7 +3623,7 @@ paths: - gid - label - description - - access_rights + - accessRights example: - gid: '27' label: A user From cec68fa3e5dd0fea5e564873044755e2348858d1 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 13:27:05 +0200 Subject: [PATCH 07/71] renamed access_rights to accessRights following openapi --- .../source/class/osparc/component/export/Permissions.js | 4 ++-- .../osparc/desktop/preferences/pages/OrganizationsPage.js | 6 +++--- .../server/src/simcore_service_webserver/groups_utils.py | 2 +- services/web/server/tests/unit/with_dbs/test_groups.py | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/services/web/client/source/class/osparc/component/export/Permissions.js b/services/web/client/source/class/osparc/component/export/Permissions.js index 8c00053b4c7..90bda71e574 100644 --- a/services/web/client/source/class/osparc/component/export/Permissions.js +++ b/services/web/client/source/class/osparc/component/export/Permissions.js @@ -157,7 +157,7 @@ qx.Class.define("osparc.component.export.Permissions", { ctrl.bindProperty("login", "subtitle", null, item, id); // user ctrl.bindProperty("description", "subtitle", null, item, id); // organization ctrl.bindProperty("isOrg", "isOrganization", null, item, id); - ctrl.bindProperty("access_rights", "accessRights", null, item, id); + ctrl.bindProperty("accessRights", "accessRights", null, item, id); ctrl.bindProperty("showOptions", "showOptions", null, item, id); }, configureItem: item => { @@ -229,7 +229,7 @@ qx.Class.define("osparc.component.export.Permissions", { collaborator["thumbnail"] = osparc.utils.Avatar.getUrl(collaborator["login"], 32); collaborator["name"] = osparc.utils.Utils.firstsUp(collaborator["first_name"], collaborator["last_name"]); } - collaborator["access_rights"] = aceessRights[gid]; + collaborator["accessRights"] = aceessRights[gid]; if (this.__isUserOwner()) { collaborator["showOptions"] = true; } diff --git a/services/web/client/source/class/osparc/desktop/preferences/pages/OrganizationsPage.js b/services/web/client/source/class/osparc/desktop/preferences/pages/OrganizationsPage.js index 5613368f77e..5675578ce85 100644 --- a/services/web/client/source/class/osparc/desktop/preferences/pages/OrganizationsPage.js +++ b/services/web/client/source/class/osparc/desktop/preferences/pages/OrganizationsPage.js @@ -88,7 +88,7 @@ qx.Class.define("osparc.desktop.preferences.pages.OrganizationsPage", { ctrl.bindProperty("label", "title", null, item, id); ctrl.bindProperty("description", "subtitle", null, item, id); ctrl.bindProperty("nMembers", "contact", null, item, id); - ctrl.bindProperty("access_rights", "accessRights", null, item, id); + ctrl.bindProperty("accessRights", "accessRights", null, item, id); }, configureItem: item => { const thumbanil = item.getChildControl("thumbnail"); @@ -159,7 +159,7 @@ qx.Class.define("osparc.desktop.preferences.pages.OrganizationsPage", { ctrl.bindProperty("id", "key", null, item, id); ctrl.bindProperty("thumbnail", "thumbnail", null, item, id); ctrl.bindProperty("name", "title", null, item, id); - ctrl.bindProperty("access_rights", "accessRights", null, item, id); + ctrl.bindProperty("accessRights", "accessRights", null, item, id); ctrl.bindProperty("login", "subtitle", null, item, id); ctrl.bindProperty("showOptions", "showOptions", null, item, id); }, @@ -365,7 +365,7 @@ qx.Class.define("osparc.desktop.preferences.pages.OrganizationsPage", { "uid": orgMember["key"] }, data: { - "access_rights": { + "accessRights": { "read": true, "write": true, "delete": false diff --git a/services/web/server/src/simcore_service_webserver/groups_utils.py b/services/web/server/src/simcore_service_webserver/groups_utils.py index 5ac3a0eafa8..da49d5f0f51 100644 --- a/services/web/server/src/simcore_service_webserver/groups_utils.py +++ b/services/web/server/src/simcore_service_webserver/groups_utils.py @@ -13,7 +13,7 @@ "label": "name", "description": "description", "thumbnail": "thumbnail", - "access_rights": "access_rights", + "accessRights": "access_rights", } diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index 632f46add22..d8337cefee7 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -76,9 +76,9 @@ async def logged_user(client, role: UserRole): def _assert_group(group: Dict[str, str]): - properties = ["gid", "label", "description", "thumbnail", "access_rights"] + properties = ["gid", "label", "description", "thumbnail", "accessRights"] assert all(x in group for x in properties) - access_rights = group["access_rights"] + access_rights = group["accessRights"] access_rights_properties = ["read", "write", "delete"] assert all(x in access_rights for x in access_rights_properties) @@ -96,7 +96,7 @@ def _assert__group_user( assert "gravatar_id" in actual_user assert actual_user["gravatar_id"] == gravatar_hash(expected_user["email"]) assert "access_rights" in actual_user - assert actual_user["access_rights"] == expected_access_rights + assert actual_user["accessRights"] == expected_access_rights assert "id" in actual_user assert actual_user["id"] == expected_user["id"] assert "gid" in actual_user From 782d95dc1cd019d774c10caa995d69f8e3f1bdd8 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 16:21:41 +0200 Subject: [PATCH 08/71] fix rename access_rights to accessRights added test for groups access rights --- .../simcore_service_webserver/groups_api.py | 8 +-- .../simcore_service_webserver/groups_utils.py | 2 +- .../server/tests/unit/with_dbs/test_groups.py | 60 +++++++++++++++++-- 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups_api.py b/services/web/server/src/simcore_service_webserver/groups_api.py index 0a6bde52515..256cfe6f86f 100644 --- a/services/web/server/src/simcore_service_webserver/groups_api.py +++ b/services/web/server/src/simcore_service_webserver/groups_api.py @@ -120,7 +120,7 @@ async def create_user_group( ) ) return convert_groups_db_to_schema( - group, access_rights=DEFAULT_GROUP_OWNER_ACCESS_RIGHTS + group, accessRights=DEFAULT_GROUP_OWNER_ACCESS_RIGHTS ) @@ -145,7 +145,7 @@ async def update_user_group( ) updated_group = await result.fetchone() return convert_groups_db_to_schema( - updated_group, access_rights=group.access_rights + updated_group, accessRights=group.access_rights ) @@ -153,7 +153,7 @@ async def delete_user_group(app: web.Application, user_id: int, gid: int) -> Non engine = app[APP_DB_ENGINE_KEY] async with engine.acquire() as conn: group = await _get_user_group(conn, user_id, gid) - check_group_permissions(group, user_id, gid, "write") + check_group_permissions(group, user_id, gid, "delete") await conn.execute( # pylint: disable=no-value-for-parameter @@ -284,7 +284,7 @@ async def update_user_in_group( ) ) the_user = dict(the_user) - the_user.update(**new_values_for_user_in_group) + the_user.update(**new_db_values) return convert_user_in_group_to_schema(the_user) diff --git a/services/web/server/src/simcore_service_webserver/groups_utils.py b/services/web/server/src/simcore_service_webserver/groups_utils.py index da49d5f0f51..f2183b2602c 100644 --- a/services/web/server/src/simcore_service_webserver/groups_utils.py +++ b/services/web/server/src/simcore_service_webserver/groups_utils.py @@ -49,6 +49,6 @@ def convert_groups_schema_to_db(schema: Dict) -> Dict: def convert_user_in_group_to_schema(row: Union[RowProxy, Dict]) -> Dict[str, str]: group_user = convert_user_db_to_schema(row) group_user.pop("role") - group_user["access_rights"] = row["access_rights"] + group_user["accessRights"] = row["access_rights"] group_user["gid"] = row["primary_gid"] return group_user diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index d8337cefee7..b8647c239a5 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -11,7 +11,7 @@ from aiohttp import web from pytest_simcore.helpers.utils_assert import assert_status -from pytest_simcore.helpers.utils_login import LoggedUser, create_user +from pytest_simcore.helpers.utils_login import LoggedUser, create_user, log_client_in from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.groups import setup_groups @@ -95,7 +95,7 @@ def _assert__group_user( assert actual_user["login"] == expected_user["email"] assert "gravatar_id" in actual_user assert actual_user["gravatar_id"] == gravatar_hash(expected_user["email"]) - assert "access_rights" in actual_user + assert "accessRights" in actual_user assert actual_user["accessRights"] == expected_access_rights assert "id" in actual_user assert actual_user["id"] == expected_user["id"] @@ -310,7 +310,7 @@ async def test_group_creation_workflow( for prop in ["label", "description", "thumbnail"]: assert assigned_group[prop] == new_group[prop] # we get all rights on the group since we are the creator - assert assigned_group["access_rights"] == { + assert assigned_group["accessRights"] == { "read": True, "write": True, "delete": True, @@ -447,7 +447,7 @@ async def test_add_remove_users_from_group( for prop in ["label", "description", "thumbnail"]: assert assigned_group[prop] == new_group[prop] # we get all rights on the group since we are the creator - assert assigned_group["access_rights"] == { + assert assigned_group["accessRights"] == { "read": True, "write": True, "delete": True, @@ -531,7 +531,7 @@ async def test_add_remove_users_from_group( gid=str(assigned_group["gid"]), uid=str(created_users_list[i]["id"]) ) resp = await client.patch( - update_group_user_url, json={"access_rights": MANAGER_ACCESS_RIGHTS} + update_group_user_url, json={"accessRights": MANAGER_ACCESS_RIGHTS} ) data, error = await assert_status(resp, expected) if not error: @@ -657,3 +657,53 @@ async def test_group_access_rights( params = {"accessRights": {"read": True, "write": False, "delete": False}} resp = await client.patch(patch_group_user_url, json=params) data, error = await assert_status(resp, expected_ok) + + # let's login as user 1 + # login + url = client.app.router["auth_login"].url_for() + resp = await client.post( + url, json={"email": users[0]["email"], "password": users[0]["raw_password"],} + ) + await assert_status(resp, expected_ok) + # check as a manager I can remove user 2 + delete_group_user_url = client.app.router["delete_group_user"].url_for( + gid=str(assigned_group["gid"]), uid=str(users[1]["id"]) + ) + assert ( + str(delete_group_user_url) + == f"{PREFIX}/{assigned_group['gid']}/users/{users[1]['id']}" + ) + resp = await client.delete(delete_group_user_url) + data, error = await assert_status(resp, expected_no_content) + # as a manager I can add user 2 again + resp = await client.post(add_group_user_url, json={"uid": users[1]["id"]}) + data, error = await assert_status(resp, expected_no_content) + # as a manager I cannot delete the group + url = client.app.router["delete_group"].url_for(gid=str(assigned_group["gid"])) + resp = await client.delete(url) + data, error = await assert_status(resp, web.HTTPForbidden) + + # now log in as user 2 + # login + url = client.app.router["auth_login"].url_for() + resp = await client.post( + url, json={"email": users[1]["email"], "password": users[1]["raw_password"],} + ) + await assert_status(resp, expected_ok) + # as a member I cannot remove user 1 + delete_group_user_url = client.app.router["delete_group_user"].url_for( + gid=str(assigned_group["gid"]), uid=str(users[0]["id"]) + ) + assert ( + str(delete_group_user_url) + == f"{PREFIX}/{assigned_group['gid']}/users/{users[0]['id']}" + ) + resp = await client.delete(delete_group_user_url) + data, error = await assert_status(resp, web.HTTPForbidden) + # as a member I cannot add user 1 + resp = await client.post(add_group_user_url, json={"uid": users[0]["id"]}) + data, error = await assert_status(resp, web.HTTPForbidden) + # as a member I cannot delete the grouop + url = client.app.router["delete_group"].url_for(gid=str(assigned_group["gid"])) + resp = await client.delete(url) + data, error = await assert_status(resp, web.HTTPForbidden) From 307d3c2bf264512a401554bca49c662cbade9541 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 17:49:52 +0200 Subject: [PATCH 09/71] simplify tests --- .../server/tests/unit/with_dbs/test_groups.py | 264 +++++------------- 1 file changed, 77 insertions(+), 187 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index b8647c239a5..9bbc81f80da 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -102,14 +102,52 @@ def _assert__group_user( assert "gid" in actual_user +from collections import namedtuple + +ExpectedResponse = namedtuple( + "ExpectedResponse", ["ok", "created", "no_content", "not_found"] +) + + +def _standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse]]]: + return ( + "role,expected", + [ + ( + UserRole.ANONYMOUS, + ExpectedResponse( + web.HTTPUnauthorized, + web.HTTPUnauthorized, + web.HTTPUnauthorized, + web.HTTPUnauthorized, + ), + ), + ( + UserRole.GUEST, + ExpectedResponse( + web.HTTPForbidden, + web.HTTPForbidden, + web.HTTPForbidden, + web.HTTPForbidden, + ), + ), + ( + UserRole.USER, + ExpectedResponse( + web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, + ), + ), + ( + UserRole.TESTER, + ExpectedResponse( + web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, + ), + ), + ], + ) + @pytest.mark.parametrize( - "role,expected", - [ - (UserRole.ANONYMOUS, web.HTTPUnauthorized), - (UserRole.GUEST, web.HTTPForbidden), - (UserRole.USER, web.HTTPOk), - (UserRole.TESTER, web.HTTPOk), - ], + *_standard_role_response() ) async def test_list_groups( client, @@ -124,7 +162,7 @@ async def test_list_groups( assert str(url) == f"{PREFIX}" resp = await client.get(url) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.ok) if not error: assert isinstance(data, dict) @@ -142,53 +180,7 @@ async def test_list_groups( assert data["all"] == all_group -def _standard_role_response() -> Tuple[ - str, List[Tuple[UserRole, web.Response, web.Response, web.Response]] -]: - return ( - "role,expected_ok, expected_created, expected_no_contents, expected_not_found", - [ - ( - UserRole.ANONYMOUS, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - ), - ( - UserRole.GUEST, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - ), - ( - UserRole.USER, - web.HTTPOk, - web.HTTPCreated, - web.HTTPNoContent, - web.HTTPNotFound, - ), - ( - UserRole.TESTER, - web.HTTPOk, - web.HTTPCreated, - web.HTTPNoContent, - web.HTTPNotFound, - ), - ], - ) - - -@pytest.mark.parametrize( - "role,expected", - [ - (UserRole.ANONYMOUS, web.HTTPUnauthorized), - (UserRole.GUEST, web.HTTPForbidden), - (UserRole.USER, web.HTTPOk), - (UserRole.TESTER, web.HTTPOk), - ], -) +@pytest.mark.parametrize(*_standard_role_response(),) async def test_group_access_rights( client, logged_user, @@ -202,7 +194,7 @@ async def test_group_access_rights( assert str(url) == f"{PREFIX}" resp = await client.get(url) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.ok) if not error: assert isinstance(data, dict) @@ -245,47 +237,13 @@ async def test_group_access_rights( data, error = await assert_status(resp, web.HTTPForbidden) -@pytest.mark.parametrize( - "role,expected,expected_read,expected_delete,expected_not_found", - [ - ( - UserRole.ANONYMOUS, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - ), - ( - UserRole.GUEST, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - ), - ( - UserRole.USER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNoContent, - web.HTTPNotFound, - ), - ( - UserRole.TESTER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNoContent, - web.HTTPNotFound, - ), - ], +@pytest.mark.parametrize(*_standard_role_response() ) async def test_group_creation_workflow( client, logged_user, role, - expected, - expected_read, - expected_delete, - expected_not_found, + expected ): url = client.app.router["create_group"].url_for() assert str(url) == f"{PREFIX}" @@ -298,7 +256,7 @@ async def test_group_creation_workflow( } resp = await client.post(url, json=new_group) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.created) assigned_group = new_group if not error: @@ -321,7 +279,7 @@ async def test_group_creation_workflow( assert str(url) == f"{PREFIX}" resp = await client.get(url) - data, error = await assert_status(resp, expected_read) + data, error = await assert_status(resp, expected.ok) if not error: assert len(data["organizations"]) == 1 assert data["organizations"][0] == assigned_group @@ -330,7 +288,7 @@ async def test_group_creation_workflow( url = client.app.router["get_group"].url_for(gid=str(assigned_group["gid"])) assert str(url) == f"{PREFIX}/{assigned_group['gid']}" resp = await client.get(url) - data, error = await assert_status(resp, expected_read) + data, error = await assert_status(resp, expected.ok) if not error: assert data == assigned_group @@ -339,7 +297,7 @@ async def test_group_creation_workflow( url = client.app.router["update_group"].url_for(gid=str(assigned_group["gid"])) assert str(url) == f"{PREFIX}/{assigned_group['gid']}" resp = await client.patch(url, json=modified_group) - data, error = await assert_status(resp, expected_read) + data, error = await assert_status(resp, expected.ok) if not error: assert data != assigned_group _assert_group(data) @@ -349,7 +307,7 @@ async def test_group_creation_workflow( url = client.app.router["get_group"].url_for(gid=str(assigned_group["gid"])) assert str(url) == f"{PREFIX}/{assigned_group['gid']}" resp = await client.get(url) - data, error = await assert_status(resp, expected_read) + data, error = await assert_status(resp, expected.ok) if not error: _assert_group(data) assert data == assigned_group @@ -358,7 +316,7 @@ async def test_group_creation_workflow( url = client.app.router["delete_group"].url_for(gid=str(assigned_group["gid"])) assert str(url) == f"{PREFIX}/{assigned_group['gid']}" resp = await client.delete(url) - data, error = await assert_status(resp, expected_delete) + data, error = await assert_status(resp, expected.no_content) if not error: assert not data @@ -366,56 +324,22 @@ async def test_group_creation_workflow( url = client.app.router["delete_group"].url_for(gid=str(assigned_group["gid"])) assert str(url) == f"{PREFIX}/{assigned_group['gid']}" resp = await client.delete(url) - data, error = await assert_status(resp, expected_not_found) + data, error = await assert_status(resp, expected.not_found) # check getting the group fails url = client.app.router["get_group"].url_for(gid=str(assigned_group["gid"])) assert str(url) == f"{PREFIX}/{assigned_group['gid']}" resp = await client.get(url) - data, error = await assert_status(resp, expected_not_found) + data, error = await assert_status(resp, expected.not_found) -@pytest.mark.parametrize( - "role, expected_created,expected,expected_not_found,expected_no_content", - [ - ( - UserRole.ANONYMOUS, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - ), - ( - UserRole.GUEST, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - ), - ( - UserRole.USER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNotFound, - web.HTTPNoContent, - ), - ( - UserRole.TESTER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNotFound, - web.HTTPNoContent, - ), - ], +@pytest.mark.parametrize(*_standard_role_response() ) async def test_add_remove_users_from_group( client, logged_user, role, - expected_created, expected, - expected_not_found, - expected_no_content, ): new_group = { @@ -429,13 +353,13 @@ async def test_add_remove_users_from_group( url = client.app.router["get_group_users"].url_for(gid=new_group["gid"]) assert str(url) == f"{PREFIX}/{new_group['gid']}/users" resp = await client.get(url) - data, error = await assert_status(resp, expected_not_found) + data, error = await assert_status(resp, expected.not_found) url = client.app.router["create_group"].url_for() assert str(url) == f"{PREFIX}" resp = await client.post(url, json=new_group) - data, error = await assert_status(resp, expected_created) + data, error = await assert_status(resp, expected.created) assigned_group = new_group if not error: @@ -459,7 +383,7 @@ async def test_add_remove_users_from_group( ) assert str(get_group_users_url) == f"{PREFIX}/{assigned_group['gid']}/users" resp = await client.get(get_group_users_url) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.ok) if not error: list_of_users = data @@ -484,7 +408,7 @@ async def test_add_remove_users_from_group( else {"email": created_users_list[i]["email"]} ) resp = await client.post(add_group_user_url, json=params) - data, error = await assert_status(resp, expected_no_content) + data, error = await assert_status(resp, expected.no_content) get_group_user_url = client.app.router["get_group_user"].url_for( gid=str(assigned_group["gid"]), uid=str(created_users_list[i]["id"]) @@ -494,7 +418,7 @@ async def test_add_remove_users_from_group( == f"{PREFIX}/{assigned_group['gid']}/users/{created_users_list[i]['id']}" ) resp = await client.get(get_group_user_url) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.ok) if not error: _assert__group_user( created_users_list[i], DEFAULT_GROUP_READ_ACCESS_RIGHTS, data @@ -549,60 +473,26 @@ async def test_add_remove_users_from_group( gid=str(assigned_group["gid"]), uid=str(created_users_list[i]["id"]) ) resp = await client.delete(delete_group_user_url) - data, error = await assert_status(resp, expected_no_content) + data, error = await assert_status(resp, expected.no_content) # do it again to check it is not found anymore resp = await client.delete(delete_group_user_url) - data, error = await assert_status(resp, expected_not_found) + data, error = await assert_status(resp, expected.not_found) # check it is not there anymore get_group_user_url = client.app.router["get_group_user"].url_for( gid=str(assigned_group["gid"]), uid=str(created_users_list[i]["id"]) ) resp = await client.get(get_group_user_url) - data, error = await assert_status(resp, expected_not_found) + data, error = await assert_status(resp, expected.not_found) -@pytest.mark.parametrize( - "role, expected_created,expected_ok,expected_not_found,expected_no_content", - [ - ( - UserRole.ANONYMOUS, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - ), - ( - UserRole.GUEST, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - ), - ( - UserRole.USER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNotFound, - web.HTTPNoContent, - ), - ( - UserRole.TESTER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNotFound, - web.HTTPNoContent, - ), - ], +@pytest.mark.parametrize(*_standard_role_response() ) async def test_group_access_rights( client, logged_user, role, - expected_created, - expected_ok, - expected_not_found, - expected_no_content, + expected, ): # Use-case: # 1. create a group @@ -617,7 +507,7 @@ async def test_group_access_rights( } resp = await client.post(url, json=new_group) - data, error = await assert_status(resp, expected_created) + data, error = await assert_status(resp, expected.created) if not data: # role cannot create a group so stop here return @@ -634,7 +524,7 @@ async def test_group_access_rights( for i, user in enumerate(users): params = {"uid": user["id"]} if i % 2 == 0 else {"email": user["email"]} resp = await client.post(add_group_user_url, json=params) - data, error = await assert_status(resp, expected_no_content) + data, error = await assert_status(resp, expected.no_content) # 3. user 1 shall be a manager patch_group_user_url = client.app.router["update_group_user"].url_for( gid=str(assigned_group["gid"]), uid=str(users[0]["id"]) @@ -645,7 +535,7 @@ async def test_group_access_rights( ) params = {"accessRights": {"read": True, "write": True, "delete": False}} resp = await client.patch(patch_group_user_url, json=params) - data, error = await assert_status(resp, expected_ok) + data, error = await assert_status(resp, expected.ok) # 4. user 2 shall be a member patch_group_user_url = client.app.router["update_group_user"].url_for( gid=str(assigned_group["gid"]), uid=str(users[1]["id"]) @@ -656,7 +546,7 @@ async def test_group_access_rights( ) params = {"accessRights": {"read": True, "write": False, "delete": False}} resp = await client.patch(patch_group_user_url, json=params) - data, error = await assert_status(resp, expected_ok) + data, error = await assert_status(resp, expected.ok) # let's login as user 1 # login @@ -664,7 +554,7 @@ async def test_group_access_rights( resp = await client.post( url, json={"email": users[0]["email"], "password": users[0]["raw_password"],} ) - await assert_status(resp, expected_ok) + await assert_status(resp, expected.ok) # check as a manager I can remove user 2 delete_group_user_url = client.app.router["delete_group_user"].url_for( gid=str(assigned_group["gid"]), uid=str(users[1]["id"]) @@ -674,10 +564,10 @@ async def test_group_access_rights( == f"{PREFIX}/{assigned_group['gid']}/users/{users[1]['id']}" ) resp = await client.delete(delete_group_user_url) - data, error = await assert_status(resp, expected_no_content) + data, error = await assert_status(resp, expected.no_content) # as a manager I can add user 2 again resp = await client.post(add_group_user_url, json={"uid": users[1]["id"]}) - data, error = await assert_status(resp, expected_no_content) + data, error = await assert_status(resp, expected.no_content) # as a manager I cannot delete the group url = client.app.router["delete_group"].url_for(gid=str(assigned_group["gid"])) resp = await client.delete(url) @@ -689,7 +579,7 @@ async def test_group_access_rights( resp = await client.post( url, json={"email": users[1]["email"], "password": users[1]["raw_password"],} ) - await assert_status(resp, expected_ok) + await assert_status(resp, expected.ok) # as a member I cannot remove user 1 delete_group_user_url = client.app.router["delete_group_user"].url_for( gid=str(assigned_group["gid"]), uid=str(users[0]["id"]) From b2b7641c69e3ebe9c4991df0a4bfdbed81b12800 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 10:45:03 +0200 Subject: [PATCH 10/71] refactor --- .../server/tests/unit/with_dbs/conftest.py | 4 +- .../unit/with_dbs/helpers/handler_utils.py | 49 +++++++++++ .../server/tests/unit/with_dbs/test_groups.py | 84 +++---------------- 3 files changed, 65 insertions(+), 72 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/helpers/handler_utils.py diff --git a/services/web/server/tests/unit/with_dbs/conftest.py b/services/web/server/tests/unit/with_dbs/conftest.py index eefc459f8c0..50b635f893d 100644 --- a/services/web/server/tests/unit/with_dbs/conftest.py +++ b/services/web/server/tests/unit/with_dbs/conftest.py @@ -12,6 +12,7 @@ import os import sys from asyncio import Future + from copy import deepcopy from pathlib import Path from typing import Dict, List @@ -23,11 +24,11 @@ import socketio import sqlalchemy as sa import trafaret_config +from pytest_simcore.helpers.utils_login import NewUser from yarl import URL import simcore_service_webserver.db_models as orm import simcore_service_webserver.utils -from pytest_simcore.helpers.utils_login import NewUser from servicelib.aiopg_utils import DSN from servicelib.rest_responses import unwrap_envelope from simcore_service_webserver.application import create_application @@ -39,6 +40,7 @@ list_user_groups, ) + # current directory current_dir = Path(sys.argv[0] if __name__ == "__main__" else __file__).resolve().parent diff --git a/services/web/server/tests/unit/with_dbs/helpers/handler_utils.py b/services/web/server/tests/unit/with_dbs/helpers/handler_utils.py new file mode 100644 index 00000000000..0791eab0c0f --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/helpers/handler_utils.py @@ -0,0 +1,49 @@ +from collections import namedtuple +from typing import List, Tuple + +from aiohttp import web + +from simcore_service_webserver.security_roles import UserRole + +ExpectedResponse = namedtuple( + "ExpectedResponse", ["ok", "created", "no_content", "not_found"] +) + + + +def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse]]]: + return ( + "role,expected", + [ + ( + UserRole.ANONYMOUS, + ExpectedResponse( + web.HTTPUnauthorized, + web.HTTPUnauthorized, + web.HTTPUnauthorized, + web.HTTPUnauthorized, + ), + ), + ( + UserRole.GUEST, + ExpectedResponse( + web.HTTPForbidden, + web.HTTPForbidden, + web.HTTPForbidden, + web.HTTPForbidden, + ), + ), + ( + UserRole.USER, + ExpectedResponse( + web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, + ), + ), + ( + UserRole.TESTER, + ExpectedResponse( + web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, + ), + ), + ], + ) diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index 9bbc81f80da..99b747584e3 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -5,13 +5,13 @@ import random from copy import deepcopy -from typing import Dict, List, Tuple +from typing import Dict, List import pytest from aiohttp import web - from pytest_simcore.helpers.utils_assert import assert_status -from pytest_simcore.helpers.utils_login import LoggedUser, create_user, log_client_in +from pytest_simcore.helpers.utils_login import LoggedUser, create_user + from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.groups import setup_groups @@ -29,6 +29,8 @@ ## BUG FIXES ####################################################### from simcore_service_webserver.utils import gravatar_hash +from .helpers import standard_role_response + API_VERSION = "v0" @@ -102,53 +104,7 @@ def _assert__group_user( assert "gid" in actual_user -from collections import namedtuple - -ExpectedResponse = namedtuple( - "ExpectedResponse", ["ok", "created", "no_content", "not_found"] -) - - -def _standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse]]]: - return ( - "role,expected", - [ - ( - UserRole.ANONYMOUS, - ExpectedResponse( - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - ), - ), - ( - UserRole.GUEST, - ExpectedResponse( - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPForbidden, - ), - ), - ( - UserRole.USER, - ExpectedResponse( - web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, - ), - ), - ( - UserRole.TESTER, - ExpectedResponse( - web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, - ), - ), - ], - ) - -@pytest.mark.parametrize( - *_standard_role_response() -) +@pytest.mark.parametrize(*standard_role_response()) async def test_list_groups( client, logged_user, @@ -180,7 +136,7 @@ async def test_list_groups( assert data["all"] == all_group -@pytest.mark.parametrize(*_standard_role_response(),) +@pytest.mark.parametrize(*standard_role_response(),) async def test_group_access_rights( client, logged_user, @@ -237,14 +193,8 @@ async def test_group_access_rights( data, error = await assert_status(resp, web.HTTPForbidden) -@pytest.mark.parametrize(*_standard_role_response() -) -async def test_group_creation_workflow( - client, - logged_user, - role, - expected -): +@pytest.mark.parametrize(*standard_role_response()) +async def test_group_creation_workflow(client, logged_user, role, expected): url = client.app.router["create_group"].url_for() assert str(url) == f"{PREFIX}" @@ -333,13 +283,9 @@ async def test_group_creation_workflow( data, error = await assert_status(resp, expected.not_found) -@pytest.mark.parametrize(*_standard_role_response() -) +@pytest.mark.parametrize(*standard_role_response()) async def test_add_remove_users_from_group( - client, - logged_user, - role, - expected, + client, logged_user, role, expected, ): new_group = { @@ -486,13 +432,9 @@ async def test_add_remove_users_from_group( data, error = await assert_status(resp, expected.not_found) -@pytest.mark.parametrize(*_standard_role_response() -) +@pytest.mark.parametrize(*standard_role_response()) async def test_group_access_rights( - client, - logged_user, - role, - expected, + client, logged_user, role, expected, ): # Use-case: # 1. create a group From 91da8b7a8a48505fb728047e58d2802b6deecdf0 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 22 Jun 2020 11:14:13 +0200 Subject: [PATCH 11/71] added test for opening a shared project 2 times --- .../tests/unit/with_dbs/test_projects.py | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 26976d1abe0..0980cb725a4 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -10,10 +10,10 @@ import pytest from aiohttp import web from mock import call - from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import LoggedUser, log_client_in from pytest_simcore.helpers.utils_projects import NewProject, delete_all_projects + from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.db_models import UserRole @@ -148,6 +148,21 @@ async def user_project(client, fake_project, logged_user): print("<----- removed project", project["name"]) +@pytest.fixture +async def shared_project(client, fake_project, logged_user, all_group): + async with NewProject( + fake_project, + client.app, + user_id=logged_user["id"], + accessRights={ + f"{all_group['gid']}": {"read": True, "write": False, "delete": False} + }, + ) as project: + print("-----> added project", project["name"]) + yield project + print("<----- removed project", project["name"]) + + @pytest.fixture async def template_project( client, fake_project, logged_user, all_group: Dict[str, str] @@ -1026,7 +1041,7 @@ async def test_get_active_project( (UserRole.TESTER, web.HTTPForbidden), ], ) -async def test_delete_shared_project_forbidden( +async def test_delete_multiple_opened_project_forbidden( client, logged_user, user_project, @@ -1220,3 +1235,32 @@ async def test_tags_to_studies( url = client.app.router["delete_tag"].url_for(tag_id=str(added_tags[1].get("id"))) resp = await client.delete(url) await assert_status(resp, web.HTTPNoContent) + + +@pytest.mark.parametrize("user_role,expected", [(UserRole.USER, web.HTTPOk)]) +async def test_open_shared_project_2_users_forbidden( + client, + logged_user, + shared_project, + socketio_client, + client_session_id, + user_role, + expected, +): + # Use-case: user 1 opens a shared project, user 2 tries to open it as well + # 1. log as user 1 and open the project + client_session_id1 = client_session_id() + sio1 = await socketio_client(client_session_id1) + url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) + resp = await client.post(url, json=client_session_id1) + await assert_status(resp, web.HTTPOk) + # 2. log as user 2 and open the project since it's shared with everyone + # get another user logged in now + user_2 = await log_client_in( + client, {"role": user_role.name}, enable_check=user_role != UserRole.ANONYMOUS + ) + client_session_id2 = client_session_id() + sio2 = await socketio_client(client_session_id1) + url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) + resp = await client.post(url, json=client_session_id1) + assert resp.status_code == 423 # the locked HTTP code does not exist in aiohttp... From b91cf463fbc75e572453079f01c51a895536dc3f Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 08:26:33 +0200 Subject: [PATCH 12/71] moved helpers in a module --- .../unit/with_dbs/{helpers/handler_utils.py => _helpers.py} | 0 services/web/server/tests/unit/with_dbs/test_groups.py | 3 +-- 2 files changed, 1 insertion(+), 2 deletions(-) rename services/web/server/tests/unit/with_dbs/{helpers/handler_utils.py => _helpers.py} (100%) diff --git a/services/web/server/tests/unit/with_dbs/helpers/handler_utils.py b/services/web/server/tests/unit/with_dbs/_helpers.py similarity index 100% rename from services/web/server/tests/unit/with_dbs/helpers/handler_utils.py rename to services/web/server/tests/unit/with_dbs/_helpers.py diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index 99b747584e3..cbe517e6cd5 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -12,6 +12,7 @@ from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import LoggedUser, create_user +from _helpers import standard_role_response from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.groups import setup_groups @@ -29,8 +30,6 @@ ## BUG FIXES ####################################################### from simcore_service_webserver.utils import gravatar_hash -from .helpers import standard_role_response - API_VERSION = "v0" From 74dd47f2f1ba5098d454c2608fe474312577936d Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 08:31:52 +0200 Subject: [PATCH 13/71] simplify tests --- .../server/tests/unit/with_dbs/_helpers.py | 17 ++++-- .../tests/unit/with_dbs/test_projects.py | 54 +++---------------- 2 files changed, 21 insertions(+), 50 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/_helpers.py b/services/web/server/tests/unit/with_dbs/_helpers.py index 0791eab0c0f..2b3593e3942 100644 --- a/services/web/server/tests/unit/with_dbs/_helpers.py +++ b/services/web/server/tests/unit/with_dbs/_helpers.py @@ -6,11 +6,10 @@ from simcore_service_webserver.security_roles import UserRole ExpectedResponse = namedtuple( - "ExpectedResponse", ["ok", "created", "no_content", "not_found"] + "ExpectedResponse", ["ok", "created", "no_content", "not_found", "forbidden"] ) - def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse]]]: return ( "role,expected", @@ -22,6 +21,7 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse web.HTTPUnauthorized, web.HTTPUnauthorized, web.HTTPUnauthorized, + web.HTTPUnauthorized, ), ), ( @@ -31,18 +31,27 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse web.HTTPForbidden, web.HTTPForbidden, web.HTTPForbidden, + web.HTTPForbidden, ), ), ( UserRole.USER, ExpectedResponse( - web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, + web.HTTPOk, + web.HTTPCreated, + web.HTTPNoContent, + web.HTTPNotFound, + web.HTTPForbidden, ), ), ( UserRole.TESTER, ExpectedResponse( - web.HTTPOk, web.HTTPCreated, web.HTTPNoContent, web.HTTPNotFound, + web.HTTPOk, + web.HTTPCreated, + web.HTTPNoContent, + web.HTTPNotFound, + web.HTTPForbidden, ), ), ], diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 0980cb725a4..2b4b472d014 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -617,43 +617,9 @@ async def test_new_template_from_project( except ValueError: pytest.fail("Invalid uuid in workbench node {}".format(node_name)) - -@pytest.mark.parametrize( +from _helpers import standard_role_response +@pytest.mark.parametrize(*standard_role_response() "user_role,expected_created,expected_ok,expected_notfound,expected_nocontents,expected_forbidden", - [ - ( - UserRole.ANONYMOUS, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - web.HTTPUnauthorized, - ), - ( - UserRole.GUEST, - web.HTTPForbidden, - web.HTTPForbidden, - web.HTTPNotFound, - web.HTTPForbidden, - web.HTTPForbidden, - ), - ( - UserRole.USER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNotFound, - web.HTTPNoContent, - web.HTTPForbidden, - ), - ( - UserRole.TESTER, - web.HTTPCreated, - web.HTTPOk, - web.HTTPNotFound, - web.HTTPNoContent, - web.HTTPForbidden, - ), - ], ) @pytest.mark.parametrize( "share_rights", @@ -671,11 +637,7 @@ async def test_share_project( standard_groups: List[Dict[str, str]], all_group: Dict[str, str], user_role, - expected_created, - expected_ok, - expected_notfound, - expected_nocontents, - expected_forbidden, + expected, storage_subsystem_mock, mocked_director_subsystem, computational_system_mock, @@ -699,7 +661,7 @@ async def test_share_project( } # user 1 can always get to his project - await _get_project(client, new_project, expected_ok) + await _get_project(client, new_project, expected.ok) # get another user logged in now user_2 = await log_client_in( @@ -710,10 +672,10 @@ async def test_share_project( await _get_project( client, new_project, - expected_ok if share_rights["read"] else expected_forbidden, + expected.ok if share_rights["read"] else expected.forbidden, ) # user 2 can only list projects if user 2 has read access - list_projects = await _list_projects(client, expected_ok) + list_projects = await _list_projects(client, expected.ok) assert len(list_projects) == (1 if share_rights["read"] else 0) # user 2 can only update the project is user 2 has write access project_update = deepcopy(new_project) @@ -721,13 +683,13 @@ async def test_share_project( await _replace_project( client, project_update, - expected_ok if share_rights["write"] else expected_forbidden, + expected.ok if share_rights["write"] else expected.forbidden, ) # user 2 can only delete projects if user 2 has delete access await _delete_project( client, new_project, - expected_nocontents if share_rights["delete"] else expected_forbidden, + expected.no_content if share_rights["delete"] else expected.forbidden, ) From e0f0a5b58ce63b6254b5bf5cf83bdeef2c0076bb Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 08:42:58 +0200 Subject: [PATCH 14/71] refactor --- .../server/tests/unit/with_dbs/conftest.py | 20 +++++------ .../tests/unit/with_dbs/test_projects.py | 34 +++++++++++-------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/conftest.py b/services/web/server/tests/unit/with_dbs/conftest.py index 50b635f893d..5a985e488b9 100644 --- a/services/web/server/tests/unit/with_dbs/conftest.py +++ b/services/web/server/tests/unit/with_dbs/conftest.py @@ -12,10 +12,9 @@ import os import sys from asyncio import Future - from copy import deepcopy from pathlib import Path -from typing import Dict, List +from typing import Callable, Dict, List from uuid import uuid4 import aioredis @@ -32,14 +31,12 @@ from servicelib.aiopg_utils import DSN from servicelib.rest_responses import unwrap_envelope from simcore_service_webserver.application import create_application -from simcore_service_webserver.application_config import app_schema as app_schema -from simcore_service_webserver.groups_api import ( - add_user_in_group, - create_user_group, - delete_user_group, - list_user_groups, -) - +from simcore_service_webserver.application_config import \ + app_schema as app_schema +from simcore_service_webserver.groups_api import (add_user_in_group, + create_user_group, + delete_user_group, + list_user_groups) # current directory current_dir = Path(sys.argv[0] if __name__ == "__main__" else __file__).resolve().parent @@ -261,9 +258,8 @@ async def security_cookie(client) -> str: cookie = resp.request_info.headers["Cookie"] yield cookie - @pytest.fixture() -async def socketio_client(socketio_url: str, security_cookie: str): +async def socketio_client(socketio_url: str, security_cookie: str) -> Callable: clients = [] async def connect(client_session_id) -> socketio.AsyncClient: diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 2b4b472d014..e39617ac21e 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -12,17 +12,18 @@ from mock import call from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import LoggedUser, log_client_in -from pytest_simcore.helpers.utils_projects import NewProject, delete_all_projects +from pytest_simcore.helpers.utils_projects import (NewProject, + delete_all_projects) +from _helpers import ExpectedResponse, standard_role_response from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.db_models import UserRole from simcore_service_webserver.director import setup_director from simcore_service_webserver.login import setup_login from simcore_service_webserver.projects import setup_projects -from simcore_service_webserver.projects.projects_handlers import ( - OVERRIDABLE_DOCUMENT_KEYS, -) +from simcore_service_webserver.projects.projects_handlers import \ + OVERRIDABLE_DOCUMENT_KEYS from simcore_service_webserver.resource_manager import setup_resource_manager from simcore_service_webserver.rest import setup_rest from simcore_service_webserver.security import setup_security @@ -60,6 +61,12 @@ def mocked_director_subsystem(mocker): } return mock_director_api +DEFAULT_GARBAGE_COLLECTOR_INTERVAL_SECONDS: int = 3 +DEFAULT_GARBAGE_COLLECTOR_DELETION_TIMEOUT_SECONDS: int = 3 + +@pytest.fixture +def gc_long_deletion_timeout(): + DEFAULT_GARBAGE_COLLECTOR_DELETION_TIMEOUT_SECONDS = 900 @pytest.fixture def client( @@ -80,10 +87,10 @@ def client( cfg["director"]["enabled"] = True cfg["resource_manager"][ "garbage_collection_interval_seconds" - ] = 3 # increase speed of garbage collection + ] = DEFAULT_GARBAGE_COLLECTOR_INTERVAL_SECONDS # increase speed of garbage collection cfg["resource_manager"][ "resource_deletion_timeout_seconds" - ] = 3 # reduce deletion delay + ] = DEFAULT_GARBAGE_COLLECTOR_DELETION_TIMEOUT_SECONDS # reduce deletion delay app = create_safe_application(cfg) # setup app @@ -617,7 +624,6 @@ async def test_new_template_from_project( except ValueError: pytest.fail("Invalid uuid in workbench node {}".format(node_name)) -from _helpers import standard_role_response @pytest.mark.parametrize(*standard_role_response() "user_role,expected_created,expected_ok,expected_notfound,expected_nocontents,expected_forbidden", ) @@ -1198,16 +1204,16 @@ async def test_tags_to_studies( resp = await client.delete(url) await assert_status(resp, web.HTTPNoContent) - @pytest.mark.parametrize("user_role,expected", [(UserRole.USER, web.HTTPOk)]) async def test_open_shared_project_2_users_forbidden( + gc_long_deletion_timeout, client, - logged_user, - shared_project, - socketio_client, - client_session_id, - user_role, - expected, + logged_user: Dict, + shared_project: Dict, + socketio_client: Callable, + client_session_id: str, + user_role: UserRole, + expected: ExpectedResponse, ): # Use-case: user 1 opens a shared project, user 2 tries to open it as well # 1. log as user 1 and open the project From 40f27e04538d50f9189e949ca02b3326245aaecd Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 08:53:57 +0200 Subject: [PATCH 15/71] ongoing tests --- .../tests/unit/with_dbs/test_projects.py | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index e39617ac21e..50d1376d515 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -11,9 +11,8 @@ from aiohttp import web from mock import call from pytest_simcore.helpers.utils_assert import assert_status -from pytest_simcore.helpers.utils_login import LoggedUser, log_client_in -from pytest_simcore.helpers.utils_projects import (NewProject, - delete_all_projects) +from pytest_simcore.helpers.utils_login import LoggedUser, log_client_in, create_user +from pytest_simcore.helpers.utils_projects import NewProject, delete_all_projects from _helpers import ExpectedResponse, standard_role_response from servicelib.application import create_safe_application @@ -22,8 +21,9 @@ from simcore_service_webserver.director import setup_director from simcore_service_webserver.login import setup_login from simcore_service_webserver.projects import setup_projects -from simcore_service_webserver.projects.projects_handlers import \ - OVERRIDABLE_DOCUMENT_KEYS +from simcore_service_webserver.projects.projects_handlers import ( + OVERRIDABLE_DOCUMENT_KEYS, +) from simcore_service_webserver.resource_manager import setup_resource_manager from simcore_service_webserver.rest import setup_rest from simcore_service_webserver.security import setup_security @@ -61,13 +61,16 @@ def mocked_director_subsystem(mocker): } return mock_director_api + DEFAULT_GARBAGE_COLLECTOR_INTERVAL_SECONDS: int = 3 DEFAULT_GARBAGE_COLLECTOR_DELETION_TIMEOUT_SECONDS: int = 3 + @pytest.fixture def gc_long_deletion_timeout(): DEFAULT_GARBAGE_COLLECTOR_DELETION_TIMEOUT_SECONDS = 900 + @pytest.fixture def client( loop, @@ -624,9 +627,8 @@ async def test_new_template_from_project( except ValueError: pytest.fail("Invalid uuid in workbench node {}".format(node_name)) -@pytest.mark.parametrize(*standard_role_response() - "user_role,expected_created,expected_ok,expected_notfound,expected_nocontents,expected_forbidden", -) + +@pytest.mark.parametrize(*standard_role_response()) @pytest.mark.parametrize( "share_rights", [ @@ -1204,9 +1206,10 @@ async def test_tags_to_studies( resp = await client.delete(url) await assert_status(resp, web.HTTPNoContent) + @pytest.mark.parametrize("user_role,expected", [(UserRole.USER, web.HTTPOk)]) async def test_open_shared_project_2_users_forbidden( - gc_long_deletion_timeout, + gc_long_deletion_timeout, # ensure garbage collector does not delete the project too early client, logged_user: Dict, shared_project: Dict, @@ -1223,12 +1226,17 @@ async def test_open_shared_project_2_users_forbidden( resp = await client.post(url, json=client_session_id1) await assert_status(resp, web.HTTPOk) # 2. log as user 2 and open the project since it's shared with everyone - # get another user logged in now - user_2 = await log_client_in( - client, {"role": user_role.name}, enable_check=user_role != UserRole.ANONYMOUS + # get another user logged in with another client + user_2 = await create_user({"role": user_role.name}) + import requests + + url = client.app.router["auth_login"].url_for() + resp = requests.post( + url, json={"email": user["email"], "password": user["raw_password"],} ) + assert resp.status_code == expected.ok.status_count client_session_id2 = client_session_id() - sio2 = await socketio_client(client_session_id1) + sio2 = await socketio_client(client_session_id2) url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) resp = await client.post(url, json=client_session_id1) assert resp.status_code == 423 # the locked HTTP code does not exist in aiohttp... From 09b6e111d994536e8a03bc6c3cc272da6f79790f Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 08:54:30 +0200 Subject: [PATCH 16/71] missing import --- services/web/server/tests/unit/with_dbs/test_projects.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 50d1376d515..0699f073302 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -5,13 +5,13 @@ import uuid as uuidlib from asyncio import Future, sleep from copy import deepcopy -from typing import Dict, List, Optional +from typing import Callable, Dict, List, Optional import pytest from aiohttp import web from mock import call from pytest_simcore.helpers.utils_assert import assert_status -from pytest_simcore.helpers.utils_login import LoggedUser, log_client_in, create_user +from pytest_simcore.helpers.utils_login import LoggedUser, create_user, log_client_in from pytest_simcore.helpers.utils_projects import NewProject, delete_all_projects from _helpers import ExpectedResponse, standard_role_response From 7b54a98e03d7eda61bb758fa9ba83a84e2b93bb6 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 10:38:36 +0200 Subject: [PATCH 17/71] adding test for shared project concurrency --- .../server/tests/unit/with_dbs/_helpers.py | 2 +- .../server/tests/unit/with_dbs/conftest.py | 16 +-- .../server/tests/unit/with_dbs/test_groups.py | 34 +----- .../tests/unit/with_dbs/test_projects.py | 100 +++++++++++------- 4 files changed, 73 insertions(+), 79 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/_helpers.py b/services/web/server/tests/unit/with_dbs/_helpers.py index 2b3593e3942..2485a761337 100644 --- a/services/web/server/tests/unit/with_dbs/_helpers.py +++ b/services/web/server/tests/unit/with_dbs/_helpers.py @@ -12,7 +12,7 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse]]]: return ( - "role,expected", + "user_role,expected", [ ( UserRole.ANONYMOUS, diff --git a/services/web/server/tests/unit/with_dbs/conftest.py b/services/web/server/tests/unit/with_dbs/conftest.py index 5a985e488b9..7de3936444f 100644 --- a/services/web/server/tests/unit/with_dbs/conftest.py +++ b/services/web/server/tests/unit/with_dbs/conftest.py @@ -31,12 +31,13 @@ from servicelib.aiopg_utils import DSN from servicelib.rest_responses import unwrap_envelope from simcore_service_webserver.application import create_application -from simcore_service_webserver.application_config import \ - app_schema as app_schema -from simcore_service_webserver.groups_api import (add_user_in_group, - create_user_group, - delete_user_group, - list_user_groups) +from simcore_service_webserver.application_config import app_schema as app_schema +from simcore_service_webserver.groups_api import ( + add_user_in_group, + create_user_group, + delete_user_group, + list_user_groups, +) # current directory current_dir = Path(sys.argv[0] if __name__ == "__main__" else __file__).resolve().parent @@ -89,7 +90,7 @@ def docker_compose_file(default_app_cfg): os.environ["TEST_POSTGRES_USER"] = cfg["user"] os.environ["TEST_POSTGRES_PASSWORD"] = cfg["password"] - dc_path = current_dir / "docker-compose.yml" + dc_path = current_dir / "docker-compose-devel.yml" assert dc_path.exists() yield str(dc_path) @@ -258,6 +259,7 @@ async def security_cookie(client) -> str: cookie = resp.request_info.headers["Cookie"] yield cookie + @pytest.fixture() async def socketio_client(socketio_url: str, security_cookie: str) -> Callable: clients = [] diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index cbe517e6cd5..a12f275ba77 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -103,40 +103,8 @@ def _assert__group_user( assert "gid" in actual_user -@pytest.mark.parametrize(*standard_role_response()) -async def test_list_groups( - client, - logged_user, - role, - expected, - primary_group: Dict[str, str], - standard_groups: List[Dict[str, str]], - all_group: Dict[str, str], -): - url = client.app.router["list_groups"].url_for() - assert str(url) == f"{PREFIX}" - - resp = await client.get(url) - data, error = await assert_status(resp, expected.ok) - - if not error: - assert isinstance(data, dict) - assert "me" in data - _assert_group(data["me"]) - assert data["me"] == primary_group - - assert "organizations" in data - assert isinstance(data["organizations"], list) - for group in data["organizations"]: - _assert_group(group) - assert data["organizations"] == standard_groups - assert "all" in data - _assert_group(data["all"]) - assert data["all"] == all_group - - @pytest.mark.parametrize(*standard_role_response(),) -async def test_group_access_rights( +async def test_list_groups( client, logged_user, role, diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 0699f073302..466be25fe0a 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -10,11 +10,11 @@ import pytest from aiohttp import web from mock import call + +from _helpers import ExpectedResponse, standard_role_response from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import LoggedUser, create_user, log_client_in from pytest_simcore.helpers.utils_projects import NewProject, delete_all_projects - -from _helpers import ExpectedResponse, standard_role_response from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.db_models import UserRole @@ -160,13 +160,15 @@ async def user_project(client, fake_project, logged_user): @pytest.fixture async def shared_project(client, fake_project, logged_user, all_group): - async with NewProject( - fake_project, - client.app, - user_id=logged_user["id"], - accessRights={ - f"{all_group['gid']}": {"read": True, "write": False, "delete": False} + fake_project.update( + { + "accessRights": { + f"{all_group['gid']}": {"read": True, "write": False, "delete": False} + }, }, + ) + async with NewProject( + fake_project, client.app, user_id=logged_user["id"], ) as project: print("-----> added project", project["name"]) yield project @@ -640,16 +642,16 @@ async def test_new_template_from_project( ) async def test_share_project( client, - logged_user, + logged_user: Dict, primary_group: Dict[str, str], standard_groups: List[Dict[str, str]], all_group: Dict[str, str], - user_role, - expected, + user_role: UserRole, + expected: ExpectedResponse, storage_subsystem_mock, mocked_director_subsystem, computational_system_mock, - share_rights, + share_rights: Dict, project_db_cleaner, ): # Use-case: the user shares some projects with a group @@ -657,7 +659,7 @@ async def test_share_project( # create a few projects new_project = await _new_project( client, - expected_created, + expected.created, logged_user, primary_group, project={"accessRights": {str(all_group["gid"]): share_rights}}, @@ -939,10 +941,13 @@ async def test_close_project( mocked_director_subsystem["stop_service"].has_calls(calls) +from socketio.exceptions import ConnectionError + + @pytest.mark.parametrize( "user_role, expected", [ - # (UserRole.ANONYMOUS, web.HTTPUnauthorized), + (UserRole.ANONYMOUS, web.HTTPUnauthorized), (UserRole.GUEST, web.HTTPOk), (UserRole.USER, web.HTTPOk), (UserRole.TESTER, web.HTTPOk), @@ -959,8 +964,14 @@ async def test_get_active_project( ): # login with socket using client session id client_id1 = client_session_id() - sio = await socketio_client(client_id1) - assert sio.sid + sio = None + try: + sio = await socketio_client(client_id1) + assert sio.sid + except ConnectionError: + if expected == web.HTTPOk: + pytest.fail("socket io connection should not fail") + # get active projects -> empty get_active_projects_url = ( client.app.router["get_active_project"] @@ -987,8 +998,12 @@ async def test_get_active_project( # login with socket using client session id2 client_id2 = client_session_id() - sio = await socketio_client(client_id2) - assert sio.sid + try: + sio = await socketio_client(client_id2) + assert sio.sid + except ConnectionError: + if expected == web.HTTPOk: + pytest.fail("socket io connection should not fail") # get active projects -> empty get_active_projects_url = ( client.app.router["get_active_project"] @@ -1207,36 +1222,45 @@ async def test_tags_to_studies( await assert_status(resp, web.HTTPNoContent) -@pytest.mark.parametrize("user_role,expected", [(UserRole.USER, web.HTTPOk)]) +@pytest.mark.parametrize(*standard_role_response()) async def test_open_shared_project_2_users_forbidden( - gc_long_deletion_timeout, # ensure garbage collector does not delete the project too early client, logged_user: Dict, shared_project: Dict, socketio_client: Callable, + # mocked_director_subsystem, client_session_id: str, user_role: UserRole, expected: ExpectedResponse, + aiohttp_client, ): # Use-case: user 1 opens a shared project, user 2 tries to open it as well # 1. log as user 1 and open the project - client_session_id1 = client_session_id() - sio1 = await socketio_client(client_session_id1) + # import pdb + + # pdb.set_trace() + # login with socket using client session id + client_id1 = client_session_id() + try: + sio = await socketio_client(client_id1) + assert sio.sid + except ConnectionError: + if user_role != UserRole.ANONYMOUS: + pytest.fail("socket io connection should not fail") url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.post(url, json=client_session_id1) - await assert_status(resp, web.HTTPOk) - # 2. log as user 2 and open the project since it's shared with everyone - # get another user logged in with another client - user_2 = await create_user({"role": user_role.name}) - import requests - - url = client.app.router["auth_login"].url_for() - resp = requests.post( - url, json={"email": user["email"], "password": user["raw_password"],} + resp = await client.post(url, json=client_id1) + await assert_status( + resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk ) - assert resp.status_code == expected.ok.status_count - client_session_id2 = client_session_id() - sio2 = await socketio_client(client_session_id2) - url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.post(url, json=client_session_id1) - assert resp.status_code == 423 # the locked HTTP code does not exist in aiohttp... + + # create a separate client now + # client_2 = await aiohttp_client(client.app) + # user_2 = await log_client_in( + # client_2, {"role": user_role.name}, enable_check=user_role != UserRole.ANONYMOUS + # ) + # client_id2 = client_session_id() + # sio2 = await socketio_client(client_id2) + # assert sio2.sid + # url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) + # resp = await client.post(url, json=client_id2) + # assert resp.status_code == 423 # the locked HTTP code does not exist in aiohttp... From df4c404e6647f5a4fd72c02711594507001f0f0e Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 10:57:45 +0200 Subject: [PATCH 18/71] test ready for concurrency access --- .../server/tests/unit/with_dbs/conftest.py | 59 ++++++++++++------- .../tests/unit/with_dbs/test_projects.py | 26 ++++---- .../unit/with_dbs/test_resource_manager.py | 14 +++-- 3 files changed, 63 insertions(+), 36 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/conftest.py b/services/web/server/tests/unit/with_dbs/conftest.py index 7de3936444f..594f1431807 100644 --- a/services/web/server/tests/unit/with_dbs/conftest.py +++ b/services/web/server/tests/unit/with_dbs/conftest.py @@ -14,7 +14,7 @@ from asyncio import Future from copy import deepcopy from pathlib import Path -from typing import Callable, Dict, List +from typing import Callable, Dict, List, Optional from uuid import uuid4 import aioredis @@ -23,11 +23,11 @@ import socketio import sqlalchemy as sa import trafaret_config -from pytest_simcore.helpers.utils_login import NewUser from yarl import URL import simcore_service_webserver.db_models as orm import simcore_service_webserver.utils +from pytest_simcore.helpers.utils_login import NewUser from servicelib.aiopg_utils import DSN from servicelib.rest_responses import unwrap_envelope from simcore_service_webserver.application import create_application @@ -239,41 +239,56 @@ async def redis_client(loop, redis_service): @pytest.fixture() -async def socketio_url(client) -> str: - SOCKET_IO_PATH = "/socket.io/" - return str(client.make_url(SOCKET_IO_PATH)) +def socketio_url(client) -> Callable: + def create_url(client_override: Optional = None) -> str: + SOCKET_IO_PATH = "/socket.io/" + return str((client_override or client).make_url(SOCKET_IO_PATH)) + + yield create_url @pytest.fixture() -async def security_cookie(client) -> str: - # get the cookie by calling the root entrypoint - resp = await client.get("/v0/") - payload = await resp.json() - assert resp.status == 200, str(payload) - data, error = unwrap_envelope(payload) - assert data - assert not error +async def security_cookie(client) -> Callable: + async def creator(client_override: Optional = None) -> str: + # get the cookie by calling the root entrypoint + resp = await (client_override or client).get("/v0/") + payload = await resp.json() + assert resp.status == 200, str(payload) + data, error = unwrap_envelope(payload) + assert data + assert not error + + cookie = ( + resp.request_info.headers["Cookie"] + if "Cookie" in resp.request_info.headers + else "" + ) + return cookie - cookie = "" - if "Cookie" in resp.request_info.headers: - cookie = resp.request_info.headers["Cookie"] - yield cookie + yield creator @pytest.fixture() -async def socketio_client(socketio_url: str, security_cookie: str) -> Callable: +async def socketio_client( + socketio_url: Callable, security_cookie: Callable +) -> Callable: clients = [] - async def connect(client_session_id) -> socketio.AsyncClient: + async def connect( + client_session_id: str, client: Optional = None + ) -> socketio.AsyncClient: sio = socketio.AsyncClient(ssl_verify=False) # enginio 3.10.0 introduced ssl verification url = str( - URL(socketio_url).with_query({"client_session_id": client_session_id}) + URL(socketio_url(client)).with_query( + {"client_session_id": client_session_id} + ) ) headers = {} - if security_cookie: + cookie = await security_cookie(client) + if cookie: # WARNING: engineio fails with empty cookies. Expects "key=value" - headers.update({"Cookie": security_cookie}) + headers.update({"Cookie": cookie}) await sio.connect(url, headers=headers) assert sio.sid diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 466be25fe0a..f474cbdacdb 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -1254,13 +1254,19 @@ async def test_open_shared_project_2_users_forbidden( ) # create a separate client now - # client_2 = await aiohttp_client(client.app) - # user_2 = await log_client_in( - # client_2, {"role": user_role.name}, enable_check=user_role != UserRole.ANONYMOUS - # ) - # client_id2 = client_session_id() - # sio2 = await socketio_client(client_id2) - # assert sio2.sid - # url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) - # resp = await client.post(url, json=client_id2) - # assert resp.status_code == 423 # the locked HTTP code does not exist in aiohttp... + client_2 = await aiohttp_client(client.app) + user_2 = await log_client_in( + client_2, {"role": user_role.name}, enable_check=user_role != UserRole.ANONYMOUS + ) + client_id2 = client_session_id() + try: + sio2 = await socketio_client(client_id2, client_2) + assert sio2.sid + except ConnectionError: + if user_role != UserRole.ANONYMOUS: + pytest.fail("socket io connection should not fail") + url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) + resp = await client.post(url, json=client_id2) + assert ( + resp.status == 423 if user_role != UserRole.ANONYMOUS else web.HTTPUnauthorized + ) # the locked HTTP code does not exist in aiohttp... diff --git a/services/web/server/tests/unit/with_dbs/test_resource_manager.py b/services/web/server/tests/unit/with_dbs/test_resource_manager.py index 2b3cf1ff176..5b91e4929cf 100644 --- a/services/web/server/tests/unit/with_dbs/test_resource_manager.py +++ b/services/web/server/tests/unit/with_dbs/test_resource_manager.py @@ -143,19 +143,25 @@ async def close_project(client, project_uuid: str, client_session_id: str) -> No # ------------------------ TESTS ------------------------------- +from typing import Callable + + async def test_anonymous_websocket_connection( - client_session_id, socketio_url: str, security_cookie, mocker, + client_session_id: str, socketio_url: Callable, security_cookie: Callable, mocker, ): from yarl import URL sio = socketio.AsyncClient( ssl_verify=False ) # enginio 3.10.0 introduced ssl verification - url = str(URL(socketio_url).with_query({"client_session_id": client_session_id()})) + url = str( + URL(socketio_url()).with_query({"client_session_id": client_session_id()}) + ) headers = {} - if security_cookie: + cookie = await security_cookie(client) + if cookie: # WARNING: engineio fails with empty cookies. Expects "key=value" - headers.update({"Cookie": security_cookie}) + headers.update({"Cookie": cookie}) socket_connect_error = mocker.Mock() sio.on("connect_error", handler=socket_connect_error) From 3fceb21f1820146fd3ce026295642067ae5e08ba Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 11:00:06 +0200 Subject: [PATCH 19/71] after user 1 closes the project user 2 shall be able to open --- .../web/server/tests/unit/with_dbs/test_projects.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index f474cbdacdb..d5b9dc10bae 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -1270,3 +1270,16 @@ async def test_open_shared_project_2_users_forbidden( assert ( resp.status == 423 if user_role != UserRole.ANONYMOUS else web.HTTPUnauthorized ) # the locked HTTP code does not exist in aiohttp... + + # user 1 closes the project + url = client.app.router["close_project"].url_for(project_id=shared_project["uuid"]) + resp = await client.post(url, json=client_id1) + await assert_status( + resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk + ) + # user 2 now should be able to open the project + url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) + resp = await client.post(url, json=client_id2) + await assert_status( + resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk + ) From 5d903776d5533ec03d6a5a9ed1cd6062fb650109 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 11:10:25 +0200 Subject: [PATCH 20/71] prevent opening of same project by 2 users --- .../projects/projects_handlers.py | 10 +++++++++ .../tests/unit/with_dbs/test_projects.py | 22 ++++++++++--------- 2 files changed, 22 insertions(+), 10 deletions(-) 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 9507566d572..e426b4614ea 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 @@ -273,6 +273,11 @@ async def delete_project(request: web.Request): raise web.HTTPNoContent(content_type="application/json") +class HTTPConflict(web.HTTPClientError): + # pylint: disable=too-many-ancestors + status_code = 423 + + @login_required @permission_required("project.open") async def open_project(request: web.Request) -> web.Response: @@ -291,6 +296,11 @@ async def open_project(request: web.Request) -> web.Response: user_id=user_id, include_templates=True, ) + + # let's check if that project is already opened by someone + other_users = await rt.find_users_of_resource("project_id", project_uuid) + if other_users: + raise HTTPConflict(reason="Project is already opened by another user") await rt.add("project_id", project_uuid) # user id opened project uuid diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index d5b9dc10bae..839a95e1314 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -1223,7 +1223,7 @@ async def test_tags_to_studies( @pytest.mark.parametrize(*standard_role_response()) -async def test_open_shared_project_2_users_forbidden( +async def test_open_shared_project_2_users_locked( client, logged_user: Dict, shared_project: Dict, @@ -1274,12 +1274,14 @@ async def test_open_shared_project_2_users_forbidden( # user 1 closes the project url = client.app.router["close_project"].url_for(project_id=shared_project["uuid"]) resp = await client.post(url, json=client_id1) - await assert_status( - resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk - ) - # user 2 now should be able to open the project - url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.post(url, json=client_id2) - await assert_status( - resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk - ) + data, error = await assert_status(resp, expected.no_content) + + if not error: + # user 2 now should be able to open the project + url = client.app.router["open_project"].url_for( + project_id=shared_project["uuid"] + ) + resp = await client.post(url, json=client_id2) + await assert_status( + resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk + ) From c579d75528ae7a6ea05ddf4f94893986dccfdeec Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 11:11:27 +0200 Subject: [PATCH 21/71] typos --- .../web/server/tests/unit/with_dbs/test_groups.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index a12f275ba77..176751dd8af 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -61,13 +61,15 @@ def client(loop, aiohttp_client, app_cfg, postgres_service): @pytest.fixture -async def logged_user(client, role: UserRole): +async def logged_user(client, user_role: UserRole): """ adds a user in db and logs in with client NOTE: role fixture is defined as a parametrization below """ async with LoggedUser( - client, {"role": role.name}, check_if_succeeds=role != UserRole.ANONYMOUS + client, + {"role": user_role.name}, + check_if_succeeds=user_role != UserRole.ANONYMOUS, ) as user: yield user @@ -107,7 +109,7 @@ def _assert__group_user( async def test_list_groups( client, logged_user, - role, + user_role, expected, primary_group: Dict[str, str], standard_groups: List[Dict[str, str]], @@ -161,7 +163,7 @@ async def test_list_groups( @pytest.mark.parametrize(*standard_role_response()) -async def test_group_creation_workflow(client, logged_user, role, expected): +async def test_group_creation_workflow(client, logged_user, user_role, expected): url = client.app.router["create_group"].url_for() assert str(url) == f"{PREFIX}" @@ -252,7 +254,7 @@ async def test_group_creation_workflow(client, logged_user, role, expected): @pytest.mark.parametrize(*standard_role_response()) async def test_add_remove_users_from_group( - client, logged_user, role, expected, + client, logged_user, user_role, expected, ): new_group = { @@ -401,7 +403,7 @@ async def test_add_remove_users_from_group( @pytest.mark.parametrize(*standard_role_response()) async def test_group_access_rights( - client, logged_user, role, expected, + client, logged_user, user_role, expected, ): # Use-case: # 1. create a group From 93f336a4faee887792ec8f441908395f0ed9bf88 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 11:22:00 +0200 Subject: [PATCH 22/71] wrong syntax --- .../web/server/src/simcore_service_webserver/users_api.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 ea35c123b37..63fc7f3a837 100644 --- a/services/web/server/src/simcore_service_webserver/users_api.py +++ b/services/web/server/src/simcore_service_webserver/users_api.py @@ -44,20 +44,20 @@ async def get_user_profile(app: web.Application, user_id: int) -> Dict[str, Any] all_group = convert_groups_db_to_schema( row, prefix="groups_", - access_rights=row["user_to_groups_access_rights"], + accessRights=row["user_to_groups_access_rights"], ) elif row["groups_type"] == GroupType.PRIMARY: user_primary_group = convert_groups_db_to_schema( row, prefix="groups_", - access_rights=row["user_to_groups_access_rights"], + accessRights=row["user_to_groups_access_rights"], ) else: user_standard_groups.append( convert_groups_db_to_schema( row, prefix="groups_", - access_rights=row["user_to_groups_access_rights"], + accessRights=row["user_to_groups_access_rights"], ) ) if not user_profile: @@ -94,7 +94,6 @@ async def update_user_profile( assert resp.rowcount == 1 # nosec - async def is_user_guest(app: web.Application, user_id: int) -> bool: """Returns True if the user exists and is a GUEST""" db = get_storage(app) From 003dac7905e85a9950ee92a22da59d695a2dfc27 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 11:22:06 +0200 Subject: [PATCH 23/71] typo --- services/web/server/tests/unit/with_dbs/test_groups.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_groups.py b/services/web/server/tests/unit/with_dbs/test_groups.py index 176751dd8af..884cfea7d53 100644 --- a/services/web/server/tests/unit/with_dbs/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/test_groups.py @@ -340,7 +340,7 @@ async def test_add_remove_users_from_group( ) # check list is correct resp = await client.get(get_group_users_url) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.ok) if not error: list_of_users = data # now we should have all the users in the group + the owner @@ -372,7 +372,7 @@ async def test_add_remove_users_from_group( resp = await client.patch( update_group_user_url, json={"accessRights": MANAGER_ACCESS_RIGHTS} ) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.ok) if not error: _assert__group_user(created_users_list[i], MANAGER_ACCESS_RIGHTS, data) # check it is there @@ -380,7 +380,7 @@ async def test_add_remove_users_from_group( gid=str(assigned_group["gid"]), uid=str(created_users_list[i]["id"]) ) resp = await client.get(get_group_user_url) - data, error = await assert_status(resp, expected) + data, error = await assert_status(resp, expected.ok) if not error: _assert__group_user(created_users_list[i], MANAGER_ACCESS_RIGHTS, data) # remove the user from the group From d36b0810742bb8713b96946fe9b72dba5ffe28a3 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 11:34:00 +0200 Subject: [PATCH 24/71] fixed test --- .../web/server/tests/unit/with_dbs/test_resource_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/test_resource_manager.py b/services/web/server/tests/unit/with_dbs/test_resource_manager.py index 5b91e4929cf..90398e7640c 100644 --- a/services/web/server/tests/unit/with_dbs/test_resource_manager.py +++ b/services/web/server/tests/unit/with_dbs/test_resource_manager.py @@ -158,7 +158,7 @@ async def test_anonymous_websocket_connection( URL(socketio_url()).with_query({"client_session_id": client_session_id()}) ) headers = {} - cookie = await security_cookie(client) + cookie = await security_cookie() if cookie: # WARNING: engineio fails with empty cookies. Expects "key=value" headers.update({"Cookie": cookie}) From d14811ed9530c43fbaa2bc825dcfad84a8e3e0d1 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 16:46:01 +0200 Subject: [PATCH 25/71] added endpoint to return state of a project --- api/specs/common/schemas/project.yaml | 25 +++- api/specs/webserver/openapi-projects.yaml | 26 ++++ api/specs/webserver/openapi.yaml | 3 + .../api/v0/openapi.yaml | 116 ++++++++++++++++++ 4 files changed, 167 insertions(+), 3 deletions(-) diff --git a/api/specs/common/schemas/project.yaml b/api/specs/common/schemas/project.yaml index b71b11006df..a4a264978ca 100644 --- a/api/specs/common/schemas/project.yaml +++ b/api/specs/common/schemas/project.yaml @@ -1,14 +1,14 @@ components: schemas: Project: - $ref: './project-v0.0.1-converted.yaml' + $ref: "./project-v0.0.1-converted.yaml" ProjectEnveloped: type: object required: - data properties: data: - $ref: '#/components/schemas/Project' + $ref: "#/components/schemas/Project" error: nullable: true default: null @@ -20,7 +20,26 @@ components: data: type: array items: - $ref: '#/components/schemas/Project' + $ref: "#/components/schemas/Project" + error: + nullable: true + default: null + ProjectState: + type: object + required: + - locked + properties: + locked: + type: boolean + description: true if project currently locked + + ProjectStateEnveloped: + type: object + required: + - data + properties: + data: + $ref: "#/components/schemas/ProjectState" error: nullable: true default: null diff --git a/api/specs/webserver/openapi-projects.yaml b/api/specs/webserver/openapi-projects.yaml index 197fdbb1438..3efcd1f1f88 100644 --- a/api/specs/webserver/openapi-projects.yaml +++ b/api/specs/webserver/openapi-projects.yaml @@ -171,6 +171,29 @@ paths: default: $ref: "#/components/responses/DefaultErrorResponse" + /projects/{project_id}/state: + parameters: + - name: project_id + in: path + required: true + schema: + type: string + get: + tags: + - project + summary: returns the state of a project + operationId: state_project + responses: + "200": + description: returns the project current state + content: + application/json: + schema: + $ref: "#/components/schemas/ProjectStateEnveloped" + + default: + $ref: "#/components/responses/DefaultErrorResponse" + /projects/{project_id}/close: parameters: - name: project_id @@ -362,6 +385,9 @@ components: ProjectArrayEnveloped: $ref: "../common/schemas/project.yaml#/components/schemas/ProjectArrayEnveloped" + ProjectStateEnveloped: + $ref: "../common/schemas/project.yaml#/components/schemas/ProjectStateEnveloped" + RunningServiceEnveloped: $ref: "../common/schemas/running_service.yaml#/components/schemas/RunningServiceEnveloped" diff --git a/api/specs/webserver/openapi.yaml b/api/specs/webserver/openapi.yaml index 34d54bd2e85..685da8c34a0 100644 --- a/api/specs/webserver/openapi.yaml +++ b/api/specs/webserver/openapi.yaml @@ -159,6 +159,9 @@ paths: /projects/{project_id}:open: $ref: "./openapi-projects.yaml#/paths/~1projects~1{project_id}~1open" + /projects/{project_id}:state: + $ref: "./openapi-projects.yaml#/paths/~1projects~1{project_id}~1state" + /projects/{project_id}:close: $ref: "./openapi-projects.yaml#/paths/~1projects~1{project_id}~1close" 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 6738aafb055..ce229ca22ba 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 @@ -8646,6 +8646,122 @@ paths: message: Password is not secure field: pasword status: 400 + '/projects/{project_id}:state': + parameters: + - name: project_id + in: path + required: true + schema: + type: string + get: + tags: + - project + summary: returns the state of a project + operationId: state_project + responses: + '200': + description: returns the project current state + content: + application/json: + schema: + type: object + required: + - data + properties: + data: + type: object + required: + - locked + properties: + locked: + type: boolean + description: true if project currently locked + error: + nullable: true + default: null + default: + description: Default http error response body + content: + application/json: + schema: + type: object + required: + - error + properties: + data: + nullable: true + default: null + error: + type: object + nullable: true + properties: + logs: + description: log messages + type: array + items: + type: object + properties: + level: + description: log level + type: string + default: INFO + enum: + - DEBUG + - WARNING + - INFO + - ERROR + message: + description: 'log message. If logger is USER, then it MUST be human readable' + type: string + logger: + description: name of the logger receiving this message + type: string + required: + - message + example: + message: 'Hi there, Mr user' + level: INFO + logger: user-logger + errors: + description: errors metadata + type: array + items: + type: object + required: + - code + - message + properties: + code: + type: string + description: Typically the name of the exception that produced it otherwise some known error code + message: + type: string + description: Error message specific to this item + resource: + type: string + description: API resource affected by this error + field: + type: string + description: Specific field within the resource + status: + description: HTTP error code + type: integer + example: + BadRequestError: + logs: + - message: Requested information is incomplete or malformed + level: ERROR + - message: Invalid email and password + level: ERROR + logger: USER + errors: + - code: InvalidEmail + message: Email is malformed + field: email + - code: UnsavePassword + message: Password is not secure + field: pasword + status: 400 '/projects/{project_id}:close': parameters: - name: project_id From 17e6c6f6e4b285b4068ed08a8ab0cea468ee045f Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 17:18:36 +0200 Subject: [PATCH 26/71] adding state endpoint --- .../projects/projects_handlers.py | 26 +++++++++++++++++++ .../tests/unit/with_dbs/test_projects.py | 19 ++++++++++++++ 2 files changed, 45 insertions(+) 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 e426b4614ea..f9fcee13bf5 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 @@ -345,6 +345,32 @@ async def close_project(request: web.Request) -> web.Response: raise web.HTTPNotFound(reason=f"Project {project_uuid} not found") +@login_required +@permission_required("project.read") +async def state_project(request: web.Request) -> web.Response: + user_id = request[RQT_USERID_KEY] + project_uuid = request.match_info.get("project_id") + with managed_resource(user_id, None, request.app) as rt: + # TODO: temporary hidden until get_handlers_from_namespace refactor to seek marked functions instead! + from .projects_api import get_project_for_user + + project = await get_project_for_user( + request.app, + project_uuid=project_uuid, + user_id=user_id, + include_templates=True, + ) + + return { + "data": { + "locked": len( + await rt.find_users_of_resource("project_id", project_uuid) + ) + > 0 + } + } + + @login_required @permission_required("project.read") async def get_active_project(request: web.Request) -> web.Response: diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 839a95e1314..e334fe5f202 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -1253,6 +1253,16 @@ async def test_open_shared_project_2_users_locked( resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk ) + # test state + url = client.app.router["state_project"].url_for(project_id=shared_project["uuid"]) + resp = await client.get(url) + data, error = await assert_status( + resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk + ) + if not error: + assert "locked" in data + assert data["locked"] == True + # create a separate client now client_2 = await aiohttp_client(client.app) user_2 = await log_client_in( @@ -1275,6 +1285,15 @@ async def test_open_shared_project_2_users_locked( url = client.app.router["close_project"].url_for(project_id=shared_project["uuid"]) resp = await client.post(url, json=client_id1) data, error = await assert_status(resp, expected.no_content) + # test state + url = client.app.router["state_project"].url_for(project_id=shared_project["uuid"]) + resp = await client.get(url) + data, error = await assert_status( + resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk + ) + if not error: + assert "locked" in data + assert data["locked"] == False if not error: # user 2 now should be able to open the project From ddcccda1e073aa236ee9d1c7efb1f85181874250 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 23 Jun 2020 14:00:56 +0200 Subject: [PATCH 27/71] fixed endpoint behaviour --- api/specs/webserver/openapi.yaml | 2 +- .../api/v0/openapi.yaml | 2 +- .../projects/projects_handlers.py | 13 +++--------- .../tests/unit/with_dbs/test_projects.py | 20 ++++++++++--------- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/api/specs/webserver/openapi.yaml b/api/specs/webserver/openapi.yaml index 685da8c34a0..06a566c05e6 100644 --- a/api/specs/webserver/openapi.yaml +++ b/api/specs/webserver/openapi.yaml @@ -159,7 +159,7 @@ paths: /projects/{project_id}:open: $ref: "./openapi-projects.yaml#/paths/~1projects~1{project_id}~1open" - /projects/{project_id}:state: + /projects/{project_id}/state: $ref: "./openapi-projects.yaml#/paths/~1projects~1{project_id}~1state" /projects/{project_id}:close: 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 ce229ca22ba..9226d53fe93 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 @@ -8646,7 +8646,7 @@ paths: message: Password is not secure field: pasword status: 400 - '/projects/{project_id}:state': + '/projects/{project_id}/state': parameters: - name: project_id in: path 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 f9fcee13bf5..07d65391744 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 @@ -354,21 +354,14 @@ async def state_project(request: web.Request) -> web.Response: # TODO: temporary hidden until get_handlers_from_namespace refactor to seek marked functions instead! from .projects_api import get_project_for_user - project = await get_project_for_user( + await get_project_for_user( request.app, project_uuid=project_uuid, user_id=user_id, include_templates=True, ) - - return { - "data": { - "locked": len( - await rt.find_users_of_resource("project_id", project_uuid) - ) - > 0 - } - } + users_of_project = await rt.find_users_of_resource("project_id", project_uuid) + return {"data": {"locked": len(users_of_project) > 0}} @login_required diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index e334fe5f202..7e970df6477 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -1285,17 +1285,19 @@ async def test_open_shared_project_2_users_locked( url = client.app.router["close_project"].url_for(project_id=shared_project["uuid"]) resp = await client.post(url, json=client_id1) data, error = await assert_status(resp, expected.no_content) - # test state - url = client.app.router["state_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.get(url) - data, error = await assert_status( - resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk - ) - if not error: - assert "locked" in data - assert data["locked"] == False if not error: + # test state + url = client.app.router["state_project"].url_for( + project_id=shared_project["uuid"] + ) + resp = await client.get(url) + data, error = await assert_status( + resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk + ) + if not error: + assert "locked" in data + assert data["locked"] == False # user 2 now should be able to open the project url = client.app.router["open_project"].url_for( project_id=shared_project["uuid"] From 84f181bfc589850009fd2846ef5a5094316d54ff Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 24 Jun 2020 10:41:51 +0200 Subject: [PATCH 28/71] refactor --- .../server/tests/unit/with_dbs/_helpers.py | 7 ++ .../tests/unit/with_dbs/test_projects.py | 84 ++++--------------- 2 files changed, 24 insertions(+), 67 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/_helpers.py b/services/web/server/tests/unit/with_dbs/_helpers.py index 2485a761337..0320a990a84 100644 --- a/services/web/server/tests/unit/with_dbs/_helpers.py +++ b/services/web/server/tests/unit/with_dbs/_helpers.py @@ -1,3 +1,4 @@ +from asyncio import Future from collections import namedtuple from typing import List, Tuple @@ -56,3 +57,9 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse ), ], ) + + +def future_with_result(result) -> Future: + f = Future() + f.set_result(result) + return f diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 7e970df6477..377e3c44c16 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -10,11 +10,12 @@ import pytest from aiohttp import web from mock import call - -from _helpers import ExpectedResponse, standard_role_response from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import LoggedUser, create_user, log_client_in from pytest_simcore.helpers.utils_projects import NewProject, delete_all_projects +from socketio.exceptions import ConnectionError + +from _helpers import ExpectedResponse, future_with_result, standard_role_response from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.db_models import UserRole @@ -37,12 +38,6 @@ API_PREFIX = "/" + API_VERSION -def future_with_result(result) -> Future: - f = Future() - f.set_result(result) - return f - - @pytest.fixture def mocked_director_subsystem(mocker): mock_director_api = { @@ -132,22 +127,6 @@ async def logged_user(client, user_role: UserRole): print("<----- logged out user", user_role) -@pytest.fixture() -async def logged_user2(client, user_role: UserRole): - """ adds a user in db and logs in with client - - NOTE: `user_role` fixture is defined as a parametrization below!!! - """ - async with LoggedUser( - client, - {"role": user_role.name}, - check_if_succeeds=user_role != UserRole.ANONYMOUS, - ) as user: - print("-----> logged in user", user_role) - yield user - print("<----- logged out user", user_role) - - @pytest.fixture async def user_project(client, fake_project, logged_user): async with NewProject( @@ -399,15 +378,7 @@ async def _new_project( # POST -------- -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, web.HTTPUnauthorized), - (UserRole.GUEST, web.HTTPForbidden), - (UserRole.USER, web.HTTPCreated), - (UserRole.TESTER, web.HTTPCreated), - ], -) +@pytest.mark.parametrize(*standard_role_response()) async def test_new_project( client, logged_user, @@ -417,18 +388,12 @@ async def test_new_project( storage_subsystem_mock, project_db_cleaner, ): - new_project = await _new_project(client, expected, logged_user, primary_group) + new_project = await _new_project( + client, expected.created, logged_user, primary_group + ) -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, web.HTTPUnauthorized), - (UserRole.GUEST, web.HTTPForbidden), - (UserRole.USER, web.HTTPCreated), - (UserRole.TESTER, web.HTTPCreated), - ], -) +@pytest.mark.parametrize(*standard_role_response()) async def test_new_project_from_template( client, logged_user, @@ -440,7 +405,11 @@ async def test_new_project_from_template( project_db_cleaner, ): new_project = await _new_project( - client, expected, logged_user, primary_group, from_template=template_project + client, + expected.created, + logged_user, + primary_group, + from_template=template_project, ) if new_project: @@ -452,15 +421,7 @@ async def test_new_project_from_template( pytest.fail("Invalid uuid in workbench node {}".format(node_name)) -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, web.HTTPUnauthorized), - (UserRole.GUEST, web.HTTPForbidden), - (UserRole.USER, web.HTTPCreated), - (UserRole.TESTER, web.HTTPCreated), - ], -) +@pytest.mark.parametrize(*standard_role_response()) async def test_new_project_from_template_with_body( client, logged_user, @@ -492,7 +453,7 @@ async def test_new_project_from_template_with_body( } project = await _new_project( client, - expected, + expected.created, logged_user, primary_group, project=predefined, @@ -890,15 +851,7 @@ async def test_open_project( mocked_director_subsystem["start_service"].assert_has_calls(calls) -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, web.HTTPUnauthorized), - (UserRole.GUEST, web.HTTPForbidden), - (UserRole.USER, web.HTTPNoContent), - (UserRole.TESTER, web.HTTPNoContent), - ], -) +@pytest.mark.parametrize(*standard_role_response()) async def test_close_project( client, logged_user, @@ -930,7 +883,7 @@ async def test_close_project( # close project url = client.app.router["close_project"].url_for(project_id=user_project["uuid"]) resp = await client.post(url, json=client_id) - await assert_status(resp, expected) + await assert_status(resp, expected.no_content) if resp.status == web.HTTPNoContent.status_code: calls = [ call(client.server.app, user_project["uuid"], None), @@ -941,9 +894,6 @@ async def test_close_project( mocked_director_subsystem["stop_service"].has_calls(calls) -from socketio.exceptions import ConnectionError - - @pytest.mark.parametrize( "user_role, expected", [ From fdd540b57667ff80fd2e36bf4b9f8978eb53cc43 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 24 Jun 2020 10:59:50 +0200 Subject: [PATCH 29/71] mypy --- .../resource_manager/websocket_manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 5ea83ebd0af..9e6d2f659f0 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 @@ -16,7 +16,7 @@ import logging from contextlib import contextmanager -from typing import Dict, List +from typing import Dict, Iterator, List, Optional import attr from aiohttp import web @@ -32,7 +32,7 @@ @attr.s(auto_attribs=True) class WebsocketRegistry: user_id: str - client_session_id: str + client_session_id: Optional[str] app: web.Application def _resource_key(self) -> Dict[str, str]: @@ -150,8 +150,8 @@ async def find_users_of_resource(self, key: str, value: str) -> List[str]: @contextmanager def managed_resource( - user_id: str, client_session_id: str, app: web.Application -) -> WebsocketRegistry: + user_id: str, client_session_id: Optional[str], app: web.Application +) -> Iterator[WebsocketRegistry]: registry = WebsocketRegistry(user_id, client_session_id, app) try: yield registry From b2b48d025558f670f007d30ea6454c71688468db Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 24 Jun 2020 13:45:48 +0200 Subject: [PATCH 30/71] locked property added --- .../source/class/osparc/dashboard/StudyBrowser.js | 7 +++++-- .../osparc/dashboard/StudyBrowserButtonItem.js | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index f07bc02e376..4eff5cc917d 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -420,7 +420,8 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { this.__userStudyContainer.removeAll(); this.self().sortStudyList(userStudyList); for (let i=0; i { - this.__itemClicked(item, isTemplate); + if (!item.getLocked()) { + this.__itemClicked(item, isTemplate); + } }, this); return item; diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index 55e4e796eb1..a14bb41d260 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -102,6 +102,13 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { tags: { check: "Array", apply: "_applyTags" + }, + + locked: { + check: "Boolean", + init: false, + nullable: false, + apply: "_applyLocked" } }, @@ -142,7 +149,7 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { break; case "tick-unselected": control = new qx.ui.basic.Image("@FontAwesome5Solid/circle/16").set({ - zIndex: this.self().MENU_BTN_Z -1 + zIndex: this.self().MENU_BTN_Z-1 }); this._add(control, { top: 4, @@ -151,7 +158,7 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { break; case "tick-selected": control = new qx.ui.basic.Image("@FontAwesome5Solid/check-circle/16").set({ - zIndex: this.self().MENU_BTN_Z -1 + zIndex: this.self().MENU_BTN_Z-1 }); this._add(control, { top: 4, @@ -311,6 +318,9 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { } }, + _applyLocked: function(locked) { + }, + _shouldApplyFilter: function(data) { if (data.text) { const checks = [ From d896c62a990e9693dc5514a17d443e118193a03a Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 24 Jun 2020 14:29:14 +0200 Subject: [PATCH 31/71] Show lock when locked --- .../osparc/dashboard/StudyBrowserButtonItem.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index a14bb41d260..ffb7b7c1847 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -165,6 +165,15 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { right: 4 }); break; + case "lock": + control = new osparc.component.widget.Thumbnail("@FontAwesome5Solid/lock/70"); + this._add(control, { + top: 0, + right: 0, + bottom: 0, + left: 0 + }); + break; } return control || this.base(arguments, id); @@ -319,6 +328,15 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { }, _applyLocked: function(locked) { + this.setCursor(locked ? "not-allowed" : "pointer"); + + this._getChildren().forEach(item => { + item.setOpacity(locked ? 0.4 : 1.0); + }); + + const lock = this.getChildControl("lock"); + lock.setOpacity(1.0); + lock.setVisibility(locked ? "visible" : "excluded"); }, _shouldApplyFilter: function(data) { From 1c61e5b43067baddadfe41181b94f41f97c59c30 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 24 Jun 2020 14:52:40 +0200 Subject: [PATCH 32/71] cleanup --- .../src/pytest_simcore/websocket_client.py | 14 ++++++++++---- .../tests/unit/with_dbs/test_resource_manager.py | 16 ---------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/websocket_client.py b/packages/pytest-simcore/src/pytest_simcore/websocket_client.py index 5e4d49ba0eb..3b6e2f7e185 100644 --- a/packages/pytest-simcore/src/pytest_simcore/websocket_client.py +++ b/packages/pytest-simcore/src/pytest_simcore/websocket_client.py @@ -32,17 +32,23 @@ async def socketio_url(loop, client) -> str: @pytest.fixture() -async def socketio_client(socketio_url: str, security_cookie: str): +async def socketio_client( + socketio_url: str, security_cookie: str +) -> socketio.AsyncClient: clients = [] async def connect(client_session_id) -> socketio.AsyncClient: - sio = socketio.AsyncClient(ssl_verify=False) # enginio 3.10.0 introduced ssl verification - url = str(URL(socketio_url).with_query({'client_session_id': client_session_id})) + sio = socketio.AsyncClient( + ssl_verify=False + ) # enginio 3.10.0 introduced ssl verification + url = str( + URL(socketio_url).with_query({"client_session_id": client_session_id}) + ) headers = {} if security_cookie: # WARNING: engineio fails with empty cookies. Expects "key=value" - headers.update({'Cookie': security_cookie}) + headers.update({"Cookie": security_cookie}) await sio.connect(url, headers=headers) assert sio.sid diff --git a/services/web/server/tests/unit/with_dbs/test_resource_manager.py b/services/web/server/tests/unit/with_dbs/test_resource_manager.py index 90398e7640c..f105cd911b1 100644 --- a/services/web/server/tests/unit/with_dbs/test_resource_manager.py +++ b/services/web/server/tests/unit/with_dbs/test_resource_manager.py @@ -89,22 +89,6 @@ async def logged_user(client, user_role: UserRole): print("<----- logged out user", user_role) -@pytest.fixture() -async def logged_user2(client, user_role: UserRole): - """ adds a user in db and logs in with client - - NOTE: `user_role` fixture is defined as a parametrization below!!! - """ - async with LoggedUser( - client, - {"role": user_role.name}, - check_if_succeeds=user_role != UserRole.ANONYMOUS, - ) as user: - print("-----> logged in user", user_role) - yield user - print("<----- logged out user", user_role) - - @pytest.fixture async def empty_user_project(client, empty_project, logged_user): project = empty_project() From ea66b70875bbee015d139c94c5150364211378a4 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 24 Jun 2020 14:52:53 +0200 Subject: [PATCH 33/71] now check project is locked --- .../projects/projects_handlers.py | 4 +- .../server/tests/unit/with_dbs/_helpers.py | 8 +- .../tests/unit/with_dbs/test_projects.py | 213 ++++++++++++------ 3 files changed, 149 insertions(+), 76 deletions(-) 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 07d65391744..17f1a015b71 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 @@ -273,7 +273,7 @@ async def delete_project(request: web.Request): raise web.HTTPNoContent(content_type="application/json") -class HTTPConflict(web.HTTPClientError): +class HTTPLocked(web.HTTPClientError): # pylint: disable=too-many-ancestors status_code = 423 @@ -300,7 +300,7 @@ async def open_project(request: web.Request) -> web.Response: # let's check if that project is already opened by someone other_users = await rt.find_users_of_resource("project_id", project_uuid) if other_users: - raise HTTPConflict(reason="Project is already opened by another user") + raise HTTPLocked(reason="Project is already opened by another user") await rt.add("project_id", project_uuid) # user id opened project uuid diff --git a/services/web/server/tests/unit/with_dbs/_helpers.py b/services/web/server/tests/unit/with_dbs/_helpers.py index 0320a990a84..ed826046a94 100644 --- a/services/web/server/tests/unit/with_dbs/_helpers.py +++ b/services/web/server/tests/unit/with_dbs/_helpers.py @@ -4,10 +4,12 @@ from aiohttp import web +from simcore_service_webserver.projects.projects_handlers import HTTPLocked from simcore_service_webserver.security_roles import UserRole ExpectedResponse = namedtuple( - "ExpectedResponse", ["ok", "created", "no_content", "not_found", "forbidden"] + "ExpectedResponse", + ["ok", "created", "no_content", "not_found", "forbidden", "locked"], ) @@ -23,6 +25,7 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse web.HTTPUnauthorized, web.HTTPUnauthorized, web.HTTPUnauthorized, + web.HTTPUnauthorized, ), ), ( @@ -33,6 +36,7 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse web.HTTPForbidden, web.HTTPForbidden, web.HTTPForbidden, + web.HTTPForbidden, ), ), ( @@ -43,6 +47,7 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse web.HTTPNoContent, web.HTTPNotFound, web.HTTPForbidden, + HTTPLocked, ), ), ( @@ -53,6 +58,7 @@ def standard_role_response() -> Tuple[str, List[Tuple[UserRole, ExpectedResponse web.HTTPNoContent, web.HTTPNotFound, web.HTTPForbidden, + HTTPLocked, ), ), ], diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 377e3c44c16..146db52656d 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -8,6 +8,7 @@ from typing import Callable, Dict, List, Optional import pytest +import socketio from aiohttp import web from mock import call from pytest_simcore.helpers.utils_assert import assert_status @@ -15,7 +16,12 @@ from pytest_simcore.helpers.utils_projects import NewProject, delete_all_projects from socketio.exceptions import ConnectionError -from _helpers import ExpectedResponse, future_with_result, standard_role_response +from _helpers import ( + ExpectedResponse, + HTTPLocked, + future_with_result, + standard_role_response, +) from servicelib.application import create_safe_application from simcore_service_webserver.db import setup_db from simcore_service_webserver.db_models import UserRole @@ -968,12 +974,12 @@ async def test_get_active_project( @pytest.mark.parametrize( - "user_role, expected", + "user_role, expected_ok, expected_forbidden", [ - # (UserRole.ANONYMOUS), - (UserRole.GUEST, web.HTTPForbidden), - (UserRole.USER, web.HTTPForbidden), - (UserRole.TESTER, web.HTTPForbidden), + (UserRole.ANONYMOUS, web.HTTPUnauthorized, web.HTTPUnauthorized), + (UserRole.GUEST, web.HTTPOk, web.HTTPForbidden), + (UserRole.USER, web.HTTPOk, web.HTTPForbidden), + (UserRole.TESTER, web.HTTPOk, web.HTTPForbidden), ], ) async def test_delete_multiple_opened_project_forbidden( @@ -984,21 +990,31 @@ async def test_delete_multiple_opened_project_forbidden( mocked_dynamic_service, socketio_client, client_session_id, - expected, + user_role, + expected_ok, + expected_forbidden, mocked_director_subsystem, ): # service in project = await mocked_dynamic_service(logged_user["id"], empty_user_project["uuid"]) service = await mocked_dynamic_service(logged_user["id"], user_project["uuid"]) # open project in tab1 client_session_id1 = client_session_id() - sio1 = await socketio_client(client_session_id1) + try: + sio1 = await socketio_client(client_session_id1) + except ConnectionError: + if user_role != UserRole.ANONYMOUS: + pytest.fail("socket io connection should not fail") url = client.app.router["open_project"].url_for(project_id=user_project["uuid"]) resp = await client.post(url, json=client_session_id1) - await assert_status(resp, web.HTTPOk) + await assert_status(resp, expected_ok) # delete project in tab2 client_session_id2 = client_session_id() - sio2 = await socketio_client(client_session_id2) - await _delete_project(client, user_project, expected) + try: + sio2 = await socketio_client(client_session_id2) + except ConnectionError: + if user_role != UserRole.ANONYMOUS: + pytest.fail("socket io connection should not fail") + await _delete_project(client, user_project, expected_forbidden) @pytest.mark.parametrize( @@ -1172,6 +1188,53 @@ async def test_tags_to_studies( await assert_status(resp, web.HTTPNoContent) +async def _connect_websocket( + socketio_client: Callable, + check_connection: bool, + client_id: str, + events: Optional[Dict[str, Callable]] = None, +) -> socketio.AsyncClient: + try: + sio = await socketio_client(client_id) + assert sio.sid + if events: + for event, handler in events.items(): + sio.on(event, handler=handler) + return sio + except ConnectionError: + if check_connection: + pytest.fail("socket io connection should not fail") + + +async def _open_project( + client, client_id: str, project: Dict, expected: web.HTTPException +): + url = client.app.router["open_project"].url_for(project_id=project["uuid"]) + resp = await client.post(url, json=client_id) + await assert_status(resp, expected) + + +async def _close_project( + client, client_id: str, project: Dict, expected: web.HTTPException +): + url = client.app.router["close_project"].url_for(project_id=project["uuid"]) + resp = await client.post(url, json=client_id) + data, error = await assert_status(resp, expected) + + +async def _state_project( + client, project: Dict, expected: web.HTTPException, params: Dict +) -> Dict[str, str]: + url = client.app.router["state_project"].url_for(project_id=project["uuid"]) + resp = await client.get(url) + data, error = await assert_status(resp, expected) + if not error: + # the project is locked + for k, v in params.items(): + assert k in data + assert data[k] == v + + @pytest.mark.parametrize(*standard_role_response()) async def test_open_shared_project_2_users_locked( client, @@ -1183,76 +1246,80 @@ async def test_open_shared_project_2_users_locked( user_role: UserRole, expected: ExpectedResponse, aiohttp_client, + mocker, ): # Use-case: user 1 opens a shared project, user 2 tries to open it as well - # 1. log as user 1 and open the project - # import pdb - - # pdb.set_trace() - # login with socket using client session id + mock_project_state_updated_handler = mocker.Mock() + project_state_updated_event = "projectUpdated" + client_1 = client client_id1 = client_session_id() - try: - sio = await socketio_client(client_id1) - assert sio.sid - except ConnectionError: - if user_role != UserRole.ANONYMOUS: - pytest.fail("socket io connection should not fail") - url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.post(url, json=client_id1) - await assert_status( - resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk - ) + client_2 = await aiohttp_client(client.app) + client_id2 = client_session_id() - # test state - url = client.app.router["state_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.get(url) - data, error = await assert_status( - resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk + # 1. user 1 opens project + sio_1 = await _connect_websocket( + socketio_client, + user_role != UserRole.ANONYMOUS, + client_id1, + {project_state_updated_event: mock_project_state_updated_handler}, + ) + await _state_project( + client_1, + shared_project, + expected.ok if user_role != UserRole.GUEST else web.HTTPOk, + params={"locked": False}, + ) + await _open_project( + client_1, + client_id1, + shared_project, + expected.ok if user_role != UserRole.GUEST else web.HTTPOk, + ) + await _state_project( + client_1, + shared_project, + expected.ok if user_role != UserRole.GUEST else web.HTTPOk, + params={"locked": True}, ) - if not error: - assert "locked" in data - assert data["locked"] == True - # create a separate client now - client_2 = await aiohttp_client(client.app) + # 2. create a separate client now and log in user2, try to open the same shared project user_2 = await log_client_in( client_2, {"role": user_role.name}, enable_check=user_role != UserRole.ANONYMOUS ) - client_id2 = client_session_id() - try: - sio2 = await socketio_client(client_id2, client_2) - assert sio2.sid - except ConnectionError: - if user_role != UserRole.ANONYMOUS: - pytest.fail("socket io connection should not fail") - url = client.app.router["open_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.post(url, json=client_id2) - assert ( - resp.status == 423 if user_role != UserRole.ANONYMOUS else web.HTTPUnauthorized - ) # the locked HTTP code does not exist in aiohttp... + sio_2 = await _connect_websocket( + socketio_client, + user_role != UserRole.ANONYMOUS, + client_id2, + {project_state_updated_event: mock_project_state_updated_handler}, + ) + await _open_project( + client_2, + client_id2, + shared_project, + expected.locked if user_role != UserRole.GUEST else HTTPLocked, + ) + await _state_project( + client_2, + shared_project, + expected.ok if user_role != UserRole.GUEST else web.HTTPOk, + params={"locked": True}, + ) - # user 1 closes the project - url = client.app.router["close_project"].url_for(project_id=shared_project["uuid"]) - resp = await client.post(url, json=client_id1) - data, error = await assert_status(resp, expected.no_content) + # 3. user 1 closes the project + await _close_project(client_1, client_id1, shared_project, expected.no_content) + await _state_project( + client_1, + shared_project, + expected.ok if user_role != UserRole.GUEST else web.HTTPOk, + params={"locked": user_role == UserRole.GUEST}, # Guests cannot close projects + ) - if not error: - # test state - url = client.app.router["state_project"].url_for( - project_id=shared_project["uuid"] - ) - resp = await client.get(url) - data, error = await assert_status( - resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk - ) - if not error: - assert "locked" in data - assert data["locked"] == False - # user 2 now should be able to open the project - url = client.app.router["open_project"].url_for( - project_id=shared_project["uuid"] - ) - resp = await client.post(url, json=client_id2) - await assert_status( - resp, expected.ok if user_role != UserRole.GUEST else web.HTTPOk - ) + # we should receive an event that the project lock state changed + + # 4. user 2 now should be able to open the project + await _open_project( + client_2, + client_id2, + shared_project, + expected.ok if user_role != UserRole.GUEST else HTTPLocked, + ) From 88893acbdfaa8599ee428d152568767b0849f9db Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 24 Jun 2020 15:21:16 +0200 Subject: [PATCH 34/71] add error status to throwing Error object --- services/web/client/source/class/osparc/data/Resources.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/services/web/client/source/class/osparc/data/Resources.js b/services/web/client/source/class/osparc/data/Resources.js index d1d4006f00a..64f6f2e7b56 100644 --- a/services/web/client/source/class/osparc/data/Resources.js +++ b/services/web/client/source/class/osparc/data/Resources.js @@ -546,14 +546,20 @@ qx.Class.define("osparc.data.Resources", { res.addListenerOnce(endpoint + "Error", e => { let message = null; + let status = null; if (e.getData().error) { const logs = e.getData().error.logs || null; if (logs && logs.length) { message = logs[0].message; } + status = e.getData().error.status; } res.dispose(); - reject(Error(message ? message : `Error while fetching ${resource}`)); + const err = Error(message ? message : `Error while fetching ${resource}`); + if (status) { + err.status = status; + } + reject(err); }); res[endpoint](params.url || null, params.data || null); From 3ea048082a392e272d3d219580eb4c4db1ebe90d Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 24 Jun 2020 16:40:37 +0200 Subject: [PATCH 35/71] openStudy callback logic moved to StudyEditor --- .../web/client/source/class/osparc/data/model/Study.js | 4 +--- .../client/source/class/osparc/desktop/StudyEditor.js | 9 +++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/services/web/client/source/class/osparc/data/model/Study.js b/services/web/client/source/class/osparc/data/model/Study.js index 378667cc960..6fbe630466e 100644 --- a/services/web/client/source/class/osparc/data/model/Study.js +++ b/services/web/client/source/class/osparc/data/model/Study.js @@ -166,9 +166,7 @@ qx.Class.define("osparc.data.model.Study", { }, data: osparc.utils.Utils.getClientSessionID() }; - osparc.data.Resources.fetch("studies", "open", params) - .then(data => this.getWorkbench().initWorkbench()) - .catch(err => console.error(err)); + return osparc.data.Resources.fetch("studies", "open", params); }, closeStudy: function() { diff --git a/services/web/client/source/class/osparc/desktop/StudyEditor.js b/services/web/client/source/class/osparc/desktop/StudyEditor.js index dfd6a02a123..50eb4b3ec1c 100644 --- a/services/web/client/source/class/osparc/desktop/StudyEditor.js +++ b/services/web/client/source/class/osparc/desktop/StudyEditor.js @@ -69,11 +69,16 @@ qx.Class.define("osparc.desktop.StudyEditor", { _applyStudy: function(study) { osparc.store.Store.getInstance().setCurrentStudy(study); study.buildWorkbench(); - study.openStudy(); + study.openStudy() + .then(() => { + study.getWorkbench().initWorkbench(); + }) + .catch(err => { + console.error(err); + }); this.__initViews(); this.__connectEvents(); this.__startAutoSaveTimer(); - this.__openOneNode(); }, From 6f27b9399bf4f75338acebd19c9f61f621180e12 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 24 Jun 2020 16:42:53 +0200 Subject: [PATCH 36/71] back to dashboard if study is locked --- .../web/client/source/class/osparc/desktop/MainPage.js | 3 +++ .../web/client/source/class/osparc/desktop/StudyEditor.js | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/services/web/client/source/class/osparc/desktop/MainPage.js b/services/web/client/source/class/osparc/desktop/MainPage.js index d4548a4fec8..26326c5f20b 100644 --- a/services/web/client/source/class/osparc/desktop/MainPage.js +++ b/services/web/client/source/class/osparc/desktop/MainPage.js @@ -135,6 +135,9 @@ qx.Class.define("osparc.desktop.MainPage", { const elements = ev.getData(); this.__navBar.setPathButtons(elements); }, this); + this.__studyEditor.addListener("studyIsLocked", () => { + this.__showDashboard(); + }, this); this.__studyEditor.addListener("studySaved", ev => { const wasSaved = ev.getData(); diff --git a/services/web/client/source/class/osparc/desktop/StudyEditor.js b/services/web/client/source/class/osparc/desktop/StudyEditor.js index 50eb4b3ec1c..bc540da088a 100644 --- a/services/web/client/source/class/osparc/desktop/StudyEditor.js +++ b/services/web/client/source/class/osparc/desktop/StudyEditor.js @@ -49,6 +49,7 @@ qx.Class.define("osparc.desktop.StudyEditor", { events: { "changeMainViewCaption": "qx.event.type.Data", + "studyIsLocked": "qx.event.type.Event", "studySaved": "qx.event.type.Data" }, @@ -74,7 +75,13 @@ qx.Class.define("osparc.desktop.StudyEditor", { study.getWorkbench().initWorkbench(); }) .catch(err => { + if ("status" in err && err["status"] == 423) { // Locked + const msg = study.getName() + this.tr(" is already opened"); + osparc.component.message.FlashMessenger.getInstance().logAs(msg, "ERROR"); + this.fireEvent("studyIsLocked"); + } else { console.error(err); + } }); this.__initViews(); this.__connectEvents(); From 0b9e9ffc68ad15f52504211b4e00f93035ba5cd0 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 24 Jun 2020 16:43:47 +0200 Subject: [PATCH 37/71] state resource added --- services/web/client/source/class/osparc/data/Resources.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/web/client/source/class/osparc/data/Resources.js b/services/web/client/source/class/osparc/data/Resources.js index 64f6f2e7b56..d33faec0161 100644 --- a/services/web/client/source/class/osparc/data/Resources.js +++ b/services/web/client/source/class/osparc/data/Resources.js @@ -93,6 +93,11 @@ qx.Class.define("osparc.data.Resources", { method: "POST", url: statics.API + "/projects/{projectId}:close" }, + state: { + useCache: false, + method: "GET", + url: statics.API + "/projects/{projectId}/state" + }, post: { method: "POST", url: statics.API + "/projects" From 46de9a0223ead7197d435fe759aafaae5b2a90d1 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 24 Jun 2020 16:44:20 +0200 Subject: [PATCH 38/71] init study and template arrays --- .../web/client/source/class/osparc/dashboard/StudyBrowser.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index 4eff5cc917d..af3cfffd2d6 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -146,6 +146,8 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { __initResources: function() { this.__showLoadingPage(this.tr("Loading studies")); + this.__userStudies = []; + this.__templateStudies = []; this.__getTags() .then(() => { this.__hideLoadingPage(); From 7eed0f3c491e5fc0c893bf274f8156d2f0e38b4a Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 11:06:39 +0200 Subject: [PATCH 39/71] getStudiesWState added --- .../class/osparc/dashboard/StudyBrowser.js | 3 +- .../client/source/class/osparc/store/Store.js | 43 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index af3cfffd2d6..8719f16206d 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -112,7 +112,7 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { */ reloadUserStudies: function() { if (osparc.data.Permissions.getInstance().canDo("studies.user.read")) { - osparc.data.Resources.get("studies") + osparc.store.Store.getInstance().getStudiesWState() .then(studies => { this.__resetStudyList(studies); this.__itemSelected(null); @@ -469,6 +469,7 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { accessRights: study.accessRights ? study.accessRights : null, lastChangeDate: study.lastChangeDate ? new Date(study.lastChangeDate) : null, icon: study.thumbnail || (isTemplate ? "@FontAwesome5Solid/copy/50" : "@FontAwesome5Solid/file-alt/50"), + locked: study.locked ? study.locked : false, tags }); const menu = this.__getStudyItemMenu(item, study, isTemplate); diff --git a/services/web/client/source/class/osparc/store/Store.js b/services/web/client/source/class/osparc/store/Store.js index 941aadd94f7..0ecf9ff6022 100644 --- a/services/web/client/source/class/osparc/store/Store.js +++ b/services/web/client/source/class/osparc/store/Store.js @@ -197,6 +197,49 @@ qx.Class.define("osparc.store.Store", { } }, + /** + * This function provides the list of studies with their state + * @param {Boolean} reload ? + */ + getStudiesWState: function(reload = false) { + return new Promise((resolve, reject) => { + const studiesWStateCache = this.getStudies(); + if (!reload && studiesWStateCache.length !== 0) { + resolve(studiesWStateCache); + } + studiesWStateCache.length = 0; + osparc.data.Resources.get("studies") + .then(studies => { + const studiesWStatePromises = []; + studies.forEach(study => { + const params = { + url: { + "projectId": study.uuid + } + }; + studiesWStatePromises.push(osparc.data.Resources.fetch("studies", "state", params)); + }); + Promise.all(studiesWStatePromises) + .then(studyStates => { + studyStates.forEach((studyState, idx) => { + const study = studies[idx]; + study["locked"] = studyState["locked"]; + studiesWStateCache.push(study); + }); + resolve(studiesWStateCache); + }) + .catch(er => { + console.error(er); + reject(); + }); + }) + .catch(err => { + console.error(err); + reject(); + }); + }); + }, + /** * This functions does the needed processing in order to have a working list of services and DAGs. * @param {Boolean} reload ? From e72b70842df8170944b6a96cb9c469eb76d04e91 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 11:06:52 +0200 Subject: [PATCH 40/71] minor --- .../client/source/class/osparc/dashboard/StudyBrowser.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index 8719f16206d..92e1bc56e81 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -411,10 +411,12 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { __resetStudyItem: function(studyData) { const userStudyList = this.__userStudies; const index = userStudyList.findIndex(userStudy => userStudy["uuid"] === studyData["uuid"]); - if (index !== -1) { - this.__userStudies[index] = studyData; - this.__resetStudyList(userStudyList); + if (index === -1) { + userStudyList.push(studyData); + } else { + userStudyList[index] = studyData; } + this.__resetStudyList(userStudyList); }, __resetStudyList: function(userStudyList) { From ad11621503700573b59752727a70812f8af09eb3 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 11:22:24 +0200 Subject: [PATCH 41/71] getStudyWState added to Store --- .../class/osparc/dashboard/StudyBrowser.js | 18 +++------- .../client/source/class/osparc/store/Store.js | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index 92e1bc56e81..641b7fde547 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -91,13 +91,8 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { this.__itemSelected(null); }, - __reloadUserStudy: function(studyId) { - const params = { - url: { - projectId: studyId - } - }; - osparc.data.Resources.getOne("studies", params) + __reloadUserStudy: function(studyId, reload) { + osparc.store.Store.getInstance().getStudyWState(studyId, reload) .then(studyData => { this.__resetStudyItem(studyData); }) @@ -279,12 +274,7 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { }, __getStudyAndStart: function(loadStudyId) { - const params = { - url: { - projectId: loadStudyId - } - }; - osparc.data.Resources.getOne("studies", params) + osparc.store.Store.getStudyWState(loadStudyId, true) .then(studyData => { this.__startStudy(studyData); }) @@ -542,7 +532,7 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { const permissionsView = new osparc.component.export.Permissions(studyData); permissionsView.addListener("updateStudy", e => { const studyId = e.getData(); - this.__reloadUserStudy(studyId); + this.__reloadUserStudy(studyId, true); }, this); const window = permissionsView.createWindow(); permissionsView.addListener("finished", e => { diff --git a/services/web/client/source/class/osparc/store/Store.js b/services/web/client/source/class/osparc/store/Store.js index 0ecf9ff6022..dcf89473732 100644 --- a/services/web/client/source/class/osparc/store/Store.js +++ b/services/web/client/source/class/osparc/store/Store.js @@ -197,6 +197,42 @@ qx.Class.define("osparc.store.Store", { } }, + getStudyWState: function(studyId, reload = false) { + return new Promise((resolve, reject) => { + const studiesWStateCache = this.getStudies(); + const idx = studiesWStateCache.findIndex(studyWStateCache => studyWStateCache["uuid"] === studyId); + if (!reload && idx !== -1) { + resolve(studiesWStateCache[idx]); + } + const params = { + url: { + "projectId": studyId + } + }; + osparc.data.Resources.getOne("studies", params) + .then(study => { + osparc.data.Resources.fetch("studies", "state", params) + .then(state => { + study["locked"] = state["locked"]; + if (idx === -1) { + studiesWStateCache.push(study); + } else { + studiesWStateCache[idx] = study; + } + resolve(study); + }) + .catch(er => { + console.error(er); + reject(); + }); + }) + .catch(err => { + console.error(err); + reject(); + }); + }); + }, + /** * This function provides the list of studies with their state * @param {Boolean} reload ? From 9c19449410705b42ad497c5ce83b3f4a438ec8a6 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 11:45:57 +0200 Subject: [PATCH 42/71] minor --- services/web/client/source/class/osparc/store/Store.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/client/source/class/osparc/store/Store.js b/services/web/client/source/class/osparc/store/Store.js index dcf89473732..f0c5de14a81 100644 --- a/services/web/client/source/class/osparc/store/Store.js +++ b/services/web/client/source/class/osparc/store/Store.js @@ -240,7 +240,7 @@ qx.Class.define("osparc.store.Store", { getStudiesWState: function(reload = false) { return new Promise((resolve, reject) => { const studiesWStateCache = this.getStudies(); - if (!reload && studiesWStateCache.length !== 0) { + if (!reload && studiesWStateCache.length) { resolve(studiesWStateCache); } studiesWStateCache.length = 0; From b841aa45f381c0ab5e36d90ba957a44fa6324e77 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 11:56:50 +0200 Subject: [PATCH 43/71] minor --- services/web/client/source/class/osparc/store/Store.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/web/client/source/class/osparc/store/Store.js b/services/web/client/source/class/osparc/store/Store.js index f0c5de14a81..a086c09c668 100644 --- a/services/web/client/source/class/osparc/store/Store.js +++ b/services/web/client/source/class/osparc/store/Store.js @@ -203,6 +203,7 @@ qx.Class.define("osparc.store.Store", { const idx = studiesWStateCache.findIndex(studyWStateCache => studyWStateCache["uuid"] === studyId); if (!reload && idx !== -1) { resolve(studiesWStateCache[idx]); + return; } const params = { url: { @@ -242,6 +243,7 @@ qx.Class.define("osparc.store.Store", { const studiesWStateCache = this.getStudies(); if (!reload && studiesWStateCache.length) { resolve(studiesWStateCache); + return; } studiesWStateCache.length = 0; osparc.data.Resources.get("studies") From a4a62c8b281c711271245bea738d5dd239f52902 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 11:57:23 +0200 Subject: [PATCH 44/71] use cache if available --- services/web/client/source/class/osparc/store/Store.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/services/web/client/source/class/osparc/store/Store.js b/services/web/client/source/class/osparc/store/Store.js index a086c09c668..0304c36be48 100644 --- a/services/web/client/source/class/osparc/store/Store.js +++ b/services/web/client/source/class/osparc/store/Store.js @@ -350,9 +350,13 @@ qx.Class.define("osparc.store.Store", { }); }, - getVisibleMembers: function() { - const reachableMembers = this.getReachableMembers(); + getVisibleMembers: function(reload = false) { return new Promise((resolve, reject) => { + const reachableMembers = this.getReachableMembers(); + if (!reload && Object.keys(reachableMembers).length) { + resolve(reachableMembers); + return; + } osparc.data.Resources.get("organizations") .then(resp => { const orgMembersPromises = []; From 80e94d1a90fd42ed9140c3bd7605d1ea35e02b92 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 13:30:13 +0200 Subject: [PATCH 45/71] optimize calls to backend --- .../class/osparc/dashboard/StudyBrowser.js | 23 ++++++++----------- .../client/source/class/osparc/store/Store.js | 19 +++++---------- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index 641b7fde547..d53a169b094 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -143,7 +143,13 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { this.__userStudies = []; this.__templateStudies = []; - this.__getTags() + const resourcePromises = []; + const store = osparc.store.Store.getInstance(); + resourcePromises.push(store.getVisibleMembers()); + if (osparc.data.Permissions.getInstance().canDo("study.tag")) { + resourcePromises.push(osparc.data.Resources.get("tags")); + } + Promise.all(resourcePromises) .then(() => { this.__hideLoadingPage(); this.__createStudiesLayout(); @@ -153,7 +159,8 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { if (loadStudyId) { this.__getStudyAndStart(loadStudyId); } - }); + }) + .catch(console.error); }, __reloadStudies: function() { @@ -181,18 +188,6 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { }); }, - __getTags: function() { - return new Promise((resolve, reject) => { - if (osparc.data.Permissions.getInstance().canDo("study.tag")) { - osparc.data.Resources.get("tags") - .catch(console.error) - .finally(() => resolve()); - } else { - resolve(); - } - }); - }, - __createStudiesLayout: function() { const studyFilters = this.__studyFilters = new osparc.component.filter.group.StudyFilterGroup("studyBrowser").set({ paddingTop: 5 diff --git a/services/web/client/source/class/osparc/store/Store.js b/services/web/client/source/class/osparc/store/Store.js index 0304c36be48..8ea84e82889 100644 --- a/services/web/client/source/class/osparc/store/Store.js +++ b/services/web/client/source/class/osparc/store/Store.js @@ -362,19 +362,12 @@ qx.Class.define("osparc.store.Store", { const orgMembersPromises = []; const orgs = resp["organizations"]; orgs.forEach(org => { - orgMembersPromises.push( - new Promise((resolve2, reject2) => { - const params = { - url: { - "gid": org["gid"] - } - }; - osparc.data.Resources.get("organizationMembers", params) - .then(orgMembers => { - resolve2(orgMembers); - }); - }) - ); + const params = { + url: { + "gid": org["gid"] + } + }; + orgMembersPromises.push(osparc.data.Resources.get("organizationMembers", params)); }); Promise.all(orgMembersPromises) .then(orgMemberss => { From b6246beffa63a337829696c09ea8e123c5f6d96f Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 13:44:17 +0200 Subject: [PATCH 46/71] minor --- services/web/client/source/class/osparc/store/Store.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/client/source/class/osparc/store/Store.js b/services/web/client/source/class/osparc/store/Store.js index 8ea84e82889..c94a142a5bf 100644 --- a/services/web/client/source/class/osparc/store/Store.js +++ b/services/web/client/source/class/osparc/store/Store.js @@ -258,10 +258,10 @@ qx.Class.define("osparc.store.Store", { studiesWStatePromises.push(osparc.data.Resources.fetch("studies", "state", params)); }); Promise.all(studiesWStatePromises) - .then(studyStates => { - studyStates.forEach((studyState, idx) => { + .then(states => { + states.forEach((state, idx) => { const study = studies[idx]; - study["locked"] = studyState["locked"]; + study["locked"] = state["locked"]; studiesWStateCache.push(study); }); resolve(studiesWStateCache); From 1c7b4f3b433eb05f8c63762b46cf0e2bdd010dee Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 14:03:43 +0200 Subject: [PATCH 47/71] minor --- .../client/source/class/osparc/data/model/Study.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/services/web/client/source/class/osparc/data/model/Study.js b/services/web/client/source/class/osparc/data/model/Study.js index 6fbe630466e..4ae136ab759 100644 --- a/services/web/client/source/class/osparc/data/model/Study.js +++ b/services/web/client/source/class/osparc/data/model/Study.js @@ -141,6 +141,7 @@ qx.Class.define("osparc.data.model.Study", { tags: [] }; }, + updateStudy: function(params) { return osparc.data.Resources.fetch("studies", "put", { url: { @@ -151,6 +152,10 @@ qx.Class.define("osparc.data.model.Study", { qx.event.message.Bus.getInstance().dispatchByName("updateStudy", data); return data; }); + }, + + getProperties: function() { + return Object.keys(qx.util.PropertyUtil.getProperties(osparc.data.model.Study)); } }, @@ -188,14 +193,14 @@ qx.Class.define("osparc.data.model.Study", { serializeStudy: function() { let jsonObject = {}; - const properties = this.constructor.$$properties; - for (let key in properties) { + const propertyKeys = this.self().getProperties(); + propertyKeys.forEach(key => { const value = key === "workbench" ? this.getWorkbench().serializeWorkbench() : this.get(key); if (value !== null) { // only put the value in the payload if there is a value jsonObject[key] = value; } - } + }); return jsonObject; }, From f4640f12b605e2d3efd8e7233af1e0f7dc94b803 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 14:12:13 +0200 Subject: [PATCH 48/71] deepCloneStudyObject added --- .../client/source/class/osparc/data/model/Study.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/services/web/client/source/class/osparc/data/model/Study.js b/services/web/client/source/class/osparc/data/model/Study.js index 4ae136ab759..3c1770f3cff 100644 --- a/services/web/client/source/class/osparc/data/model/Study.js +++ b/services/web/client/source/class/osparc/data/model/Study.js @@ -156,6 +156,18 @@ qx.Class.define("osparc.data.model.Study", { getProperties: function() { return Object.keys(qx.util.PropertyUtil.getProperties(osparc.data.model.Study)); + }, + + // deep clones object with study-only properties + deepCloneStudyObject: function(src) { + const studyObject = osparc.utils.Utils.deepCloneObject(src); + const studyPropKeys = osparc.data.model.Study.getProperties(); + Object.keys(studyObject).forEach(key => { + if (!studyPropKeys.includes(key)) { + delete studyObject[key]; + } + }); + return studyObject; } }, From ba6ad5b14b4c87637598cbc28663ecc1a542dd8b Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 14:15:32 +0200 Subject: [PATCH 49/71] minor fix --- .../client/source/class/osparc/component/export/Permissions.js | 2 +- .../source/class/osparc/component/export/SaveAsTemplate.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/client/source/class/osparc/component/export/Permissions.js b/services/web/client/source/class/osparc/component/export/Permissions.js index 90bda71e574..4d8f71b9733 100644 --- a/services/web/client/source/class/osparc/component/export/Permissions.js +++ b/services/web/client/source/class/osparc/component/export/Permissions.js @@ -33,7 +33,7 @@ qx.Class.define("osparc.component.export.Permissions", { construct: function(studyData) { this.base(arguments); - this.__studyData = osparc.utils.Utils.deepCloneObject(studyData); + this.__studyData = osparc.data.model.Study.deepCloneStudyObject(studyData); this._setLayout(new qx.ui.layout.VBox(15)); diff --git a/services/web/client/source/class/osparc/component/export/SaveAsTemplate.js b/services/web/client/source/class/osparc/component/export/SaveAsTemplate.js index eca7b4a5728..caba14bedbe 100644 --- a/services/web/client/source/class/osparc/component/export/SaveAsTemplate.js +++ b/services/web/client/source/class/osparc/component/export/SaveAsTemplate.js @@ -34,7 +34,7 @@ qx.Class.define("osparc.component.export.SaveAsTemplate", { this._setLayout(new qx.ui.layout.VBox(5)); this.__studyId = studyId; - this.__formData = osparc.utils.Utils.deepCloneObject(studyData); + this.__formData = osparc.data.model.Study.deepCloneStudyObject(studyData); this.__buildLayout(); From 0a5760f9d0baae08a88ba946a9b2c0475d167f77 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 14:59:34 +0200 Subject: [PATCH 50/71] minor --- .../dashboard/StudyBrowserButtonItem.js | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index ffb7b7c1847..e4a680a8acf 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -243,20 +243,18 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { const store = osparc.store.Store.getInstance(); Promise.all([ store.getGroupsAll(), - store.getGroupsMe(), store.getVisibleMembers(), store.getGroupsOrganizations() ]) .then(values => { const all = values[0]; - const me = values[1]; const orgMembs = []; - const orgMembers = values[2]; + const orgMembers = values[1]; for (const gid of Object.keys(orgMembers)) { orgMembs.push(orgMembers[gid]); } - const orgs = values.length === 4 ? values[3] : []; - const groups = [[me], orgMembs, orgs, [all]]; + const orgs = values.length === 3 ? values[2] : []; + const groups = [orgMembs, orgs, [all]]; this.__setSharedIcon(image, value, groups); }); } @@ -285,13 +283,12 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { } switch (i) { case 0: - case 1: image.setSource(this.self().SHARED_USER); break; - case 2: + case 1: image.setSource(this.self().SHARED_ORGS); break; - case 3: + case 2: image.setSource(this.self().SHARED_ALL); break; } @@ -302,14 +299,19 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { return; } - let hintText = ""; + const sharedGrpLabels = []; + const maxItems = 6; for (let i=0; i 6) { - hintText += "..."; + if (i > maxItems) { + sharedGrpLabels.push("..."); break; } - hintText += (sharedGrps[i]["label"] + "
"); + const sharedGrpLabel = sharedGrps[i]["label"]; + if (!sharedGrpLabels.includes(sharedGrpLabel)) { + sharedGrpLabels.push(sharedGrpLabel); + } } + const hintText = sharedGrpLabels.join("
"); const hint = new osparc.ui.hint.Hint(image, hintText); image.addListener("mouseover", () => hint.show(), this); image.addListener("mouseout", () => hint.exclude(), this); From ffb92a2d33d26febee1ca72f0ed3be542398ee74 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 25 Jun 2020 16:06:50 +0200 Subject: [PATCH 51/71] aesthetic and logic improvements --- .../dashboard/StudyBrowserButtonItem.js | 60 +++++++++++++------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index e4a680a8acf..530370b5c7f 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -38,15 +38,7 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { qx.locale.Date.getTimeFormat("short") ); - this.addListener("changeValue", e => { - const val = this.getValue(); - - const tick = this.getChildControl("tick-selected"); - tick.setVisibility(val ? "visible" : "excluded"); - - const untick = this.getChildControl("tick-unselected"); - untick.setVisibility(val ? "excluded" : "visible"); - }); + this.addListener("changeValue", this.__itemSelected, this); }, properties: { @@ -113,7 +105,6 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { }, statics: { - MENU_BTN_Z: 20, MENU_BTN_WIDTH: 25, SHARED_USER: "@FontAwesome5Solid/user/14", SHARED_ORGS: "@FontAwesome5Solid/users/14", @@ -126,7 +117,30 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { multiSelection: function(on) { const menuButton = this.getChildControl("menu-button"); - menuButton.setVisibility(on ? "excluded" : "visible"); + if (on) { + menuButton.setVisibility("excluded"); + this.__itemSelected(); + } else { + menuButton.setVisibility("visible"); + const tick = this.getChildControl("tick-selected"); + tick.setVisibility("excluded"); + const untick = this.getChildControl("tick-unselected"); + untick.setVisibility("excluded"); + } + }, + + __itemSelected: function() { + const selected = this.getValue(); + + if (this.getLocked() && selected) { + this.setValue(false); + } + + const tick = this.getChildControl("tick-selected"); + tick.setVisibility(selected ? "visible" : "excluded"); + + const untick = this.getChildControl("tick-unselected"); + untick.setVisibility(selected ? "excluded" : "visible"); }, // overridden @@ -138,7 +152,6 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { width: this.self().MENU_BTN_WIDTH, height: this.self().MENU_BTN_WIDTH, icon: "@FontAwesome5Solid/ellipsis-v/14", - zIndex: this.self().MENU_BTN_Z, focusable: false }); osparc.utils.Utils.setIdToWidget(control, "studyItemMenuButton"); @@ -148,18 +161,14 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { }); break; case "tick-unselected": - control = new qx.ui.basic.Image("@FontAwesome5Solid/circle/16").set({ - zIndex: this.self().MENU_BTN_Z-1 - }); + control = new qx.ui.basic.Image("@FontAwesome5Solid/circle/16"); this._add(control, { top: 4, right: 4 }); break; case "tick-selected": - control = new qx.ui.basic.Image("@FontAwesome5Solid/check-circle/16").set({ - zIndex: this.self().MENU_BTN_Z-1 - }); + control = new qx.ui.basic.Image("@FontAwesome5Solid/check-circle/16"); this._add(control, { top: 4, right: 4 @@ -330,7 +339,9 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { }, _applyLocked: function(locked) { - this.setCursor(locked ? "not-allowed" : "pointer"); + this.set({ + cursor: locked ? "not-allowed" : "pointer" + }); this._getChildren().forEach(item => { item.setOpacity(locked ? 0.4 : 1.0); @@ -339,6 +350,17 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { const lock = this.getChildControl("lock"); lock.setOpacity(1.0); lock.setVisibility(locked ? "visible" : "excluded"); + + [ + "menu-button", + "tick-unselected", + "tick-selected" + ].forEach(childName => { + const child = this.getChildControl(childName); + child.set({ + enabled: !locked + }); + }); }, _shouldApplyFilter: function(data) { From 80e958ae781c4499bf358403f3d73b51464f6cb9 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 09:07:44 +0200 Subject: [PATCH 52/71] added ProjectState model --- .../src/simcore_service_webserver/projects/projects_api.py | 5 +++++ 1 file changed, 5 insertions(+) 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 35896cdaa03..7c7d4a245be 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 @@ -14,6 +14,7 @@ from uuid import uuid4 from aiohttp import web +from pydantic import BaseModel from servicelib.application_keys import APP_JSONSCHEMA_SPECS_KEY from servicelib.jsonschema_validation import validate_instance @@ -326,3 +327,7 @@ async def is_node_id_present_in_any_project_workbench( """If the node_id is presnet in one of the projects' workbenche returns True""" db = app[APP_PROJECT_DBAPI] return node_id in await db.get_all_node_ids_from_workbenches() + + +class ProjectState(BaseModel): + locked: bool From 094f4574bb20e5b96b33cb5b44381dab2962e7d2 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 09:07:59 +0200 Subject: [PATCH 53/71] added projectStateUpdated event --- .../src/simcore_service_webserver/socketio/events.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/socketio/events.py b/services/web/server/src/simcore_service_webserver/socketio/events.py index d7477aca481..9ffae614981 100644 --- a/services/web/server/src/simcore_service_webserver/socketio/events.py +++ b/services/web/server/src/simcore_service_webserver/socketio/events.py @@ -15,6 +15,8 @@ log = logging.getLogger(__name__) +SOCKET_IO_PROJECT_UPDATED_EVENT: str = "projectStateUpdated" + async def post_messages( app: Application, user_id: str, messages: Dict[str, Any] @@ -28,3 +30,11 @@ async def post_messages( # Notice that there might be several tabs open for event_name, data in messages.items(): fire_and_forget_task(sio.emit(event_name, json.dumps(data), room=sid)) + + +async def post_group_messages( + app: Application, room: str, messages: Dict[str, Any] +) -> None: + sio: AsyncServer = get_socket_server(app) + for event_name, data in messages.items(): + fire_and_forget_task(sio.emit(event_name, json.dumps(data), room=room)) From 0861b42d3637a8efdd88d1ff784fce4fe363f083 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 09:08:25 +0200 Subject: [PATCH 54/71] user is automatically enters room upon successful login --- .../simcore_service_webserver/socketio/handlers.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/socketio/handlers.py b/services/web/server/src/simcore_service_webserver/socketio/handlers.py index 51681a1b992..3a0fa3bdc6f 100644 --- a/services/web/server/src/simcore_service_webserver/socketio/handlers.py +++ b/services/web/server/src/simcore_service_webserver/socketio/handlers.py @@ -16,6 +16,7 @@ from servicelib.observer import observe from servicelib.utils import fire_and_forget_task, logged_gather +from ..groups_api import list_user_groups from ..login.decorators import RQT_USERID_KEY, login_required from ..resource_manager.config import get_service_deletion_timeout from ..resource_manager.websocket_manager import managed_resource @@ -45,6 +46,7 @@ async def connect(sid: str, environ: Dict, app: web.Application) -> bool: request = environ[_SOCKET_IO_AIOHTTP_REQUEST_KEY] try: await authenticate_user(sid, app, request) + await set_user_in_rooms(sid, app, request) except web.HTTPUnauthorized: raise SocketIOConnectionError("authentification failed") except Exception as exc: # pylint: disable=broad-except @@ -86,6 +88,18 @@ async def authenticate_user( await rt.set_socket_id(sid) +async def set_user_in_rooms( + sid: str, app: web.Application, request: web.Request +) -> None: + user_id = request.get(RQT_USERID_KEY, ANONYMOUS_USER_ID) + primary_group, user_groups, all_group = await list_user_groups(app, user_id) + groups = [primary_group] + user_groups + [all_group] + sio = get_socket_server(app) + # TODO: check if it is necessary to leave_room when socket disconnects + for group in groups: + sio.enter_room(sid, f"{group['gid']}") + + async def disconnect_other_sockets(sio, sockets: List[str]) -> None: log.debug("disconnecting sockets %s", sockets) logout_tasks = [ From 7126a9cae264f8beea9a92f18807bf2c80d3bc3b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 09:13:47 +0200 Subject: [PATCH 55/71] use projectState class --- .../tests/unit/with_dbs/test_projects.py | 91 ++++++++++++++++--- 1 file changed, 80 insertions(+), 11 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 146db52656d..9b548626407 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -2,6 +2,8 @@ # pylint:disable=unused-argument # pylint:disable=redefined-outer-name +import asyncio +import json import uuid as uuidlib from asyncio import Future, sleep from copy import deepcopy @@ -28,6 +30,7 @@ from simcore_service_webserver.director import setup_director from simcore_service_webserver.login import setup_login from simcore_service_webserver.projects import setup_projects +from simcore_service_webserver.projects.projects_api import ProjectState from simcore_service_webserver.projects.projects_handlers import ( OVERRIDABLE_DOCUMENT_KEYS, ) @@ -36,6 +39,7 @@ from simcore_service_webserver.security import setup_security from simcore_service_webserver.session import setup_session from simcore_service_webserver.socketio import setup_sockets +from simcore_service_webserver.socketio.events import SOCKET_IO_PROJECT_UPDATED_EVENT from simcore_service_webserver.tags import setup_tags from simcore_service_webserver.utils import now_str, to_datetime @@ -1223,16 +1227,17 @@ async def _close_project( async def _state_project( - client, project: Dict, expected: web.HTTPException, params: Dict + client, + project: Dict, + expected: web.HTTPException, + expected_project_state: ProjectState, ) -> Dict[str, str]: url = client.app.router["state_project"].url_for(project_id=project["uuid"]) resp = await client.get(url) data, error = await assert_status(resp, expected) if not error: # the project is locked - for k, v in params.items(): - assert k in data - assert data[k] == v + assert data == expected_project_state @pytest.mark.parametrize(*standard_role_response()) @@ -1250,7 +1255,7 @@ async def test_open_shared_project_2_users_locked( ): # Use-case: user 1 opens a shared project, user 2 tries to open it as well mock_project_state_updated_handler = mocker.Mock() - project_state_updated_event = "projectUpdated" + client_1 = client client_id1 = client_session_id() client_2 = await aiohttp_client(client.app) @@ -1261,13 +1266,14 @@ async def test_open_shared_project_2_users_locked( socketio_client, user_role != UserRole.ANONYMOUS, client_id1, - {project_state_updated_event: mock_project_state_updated_handler}, + {SOCKET_IO_PROJECT_UPDATED_EVENT: mock_project_state_updated_handler}, ) + expected_project_state = ProjectState(locked=False) await _state_project( client_1, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - params={"locked": False}, + expected_project_state, ) await _open_project( client_1, @@ -1275,11 +1281,30 @@ async def test_open_shared_project_2_users_locked( shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, ) + expected_project_state.locked = True + if user_role != UserRole.ANONYMOUS: + await asyncio.sleep(3) + # NOTE: there are 2 calls since we are part of the primary group and the all group + calls = [ + call( + json.dumps( + { + "project_uuid": shared_project["uuid"], + "data": expected_project_state.dict(), + } + ) + ) + ] * 2 + mock_project_state_updated_handler.assert_has_calls(calls) + mock_project_state_updated_handler.reset_mock() + else: + mock_project_state_updated_handler.assert_not_called() + await _state_project( client_1, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - params={"locked": True}, + expected_project_state, ) # 2. create a separate client now and log in user2, try to open the same shared project @@ -1290,7 +1315,7 @@ async def test_open_shared_project_2_users_locked( socketio_client, user_role != UserRole.ANONYMOUS, client_id2, - {project_state_updated_event: mock_project_state_updated_handler}, + {SOCKET_IO_PROJECT_UPDATED_EVENT: mock_project_state_updated_handler}, ) await _open_project( client_2, @@ -1302,19 +1327,39 @@ async def test_open_shared_project_2_users_locked( client_2, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - params={"locked": True}, + expected_project_state, ) # 3. user 1 closes the project await _close_project(client_1, client_id1, shared_project, expected.no_content) + expected_project_state.locked = ( + True if user_role == UserRole.GUEST else False + ) # Guests cannot close projects await _state_project( client_1, shared_project, expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - params={"locked": user_role == UserRole.GUEST}, # Guests cannot close projects + expected_project_state, ) # we should receive an event that the project lock state changed + await asyncio.sleep(3) + if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]): + mock_project_state_updated_handler.assert_not_called() + else: + # NOTE: there are 4 calls since both users are registered now we are part of the primary group and the all group + calls = [ + call( + json.dumps( + { + "project_uuid": shared_project["uuid"], + "data": expected_project_state.dict(), + } + ) + ) + ] * 4 + mock_project_state_updated_handler.assert_has_calls(calls) + mock_project_state_updated_handler.reset_mock() # 4. user 2 now should be able to open the project await _open_project( @@ -1323,3 +1368,27 @@ async def test_open_shared_project_2_users_locked( shared_project, expected.ok if user_role != UserRole.GUEST else HTTPLocked, ) + expected_project_state.locked = True + await asyncio.sleep(3) + if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]): + mock_project_state_updated_handler.assert_not_called() + else: + # NOTE: there are 4 calls since both users are registered now we are part of the primary group and the all group + calls = [ + call( + json.dumps( + { + "project_uuid": shared_project["uuid"], + "data": expected_project_state.dict(), + } + ) + ) + ] * 4 + mock_project_state_updated_handler.assert_has_calls(calls) + mock_project_state_updated_handler.reset_mock() + await _state_project( + client_1, + shared_project, + expected.ok if user_role != UserRole.GUEST else web.HTTPOk, + expected_project_state, + ) From 15feca3e5a85385ecf71e07baf8eca8e292bf803 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 11:42:14 +0200 Subject: [PATCH 56/71] move notification to project_api --- .../projects/projects_api.py | 18 +++++++++++++++++- .../projects/projects_handlers.py | 3 +++ 2 files changed, 20 insertions(+), 1 deletion(-) 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 7c7d4a245be..0e6efee027d 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 @@ -23,6 +23,7 @@ from ..computation_api import delete_pipeline_db from ..director import director_api +from ..socketio.events import SOCKET_IO_PROJECT_UPDATED_EVENT, post_group_messages from ..storage_api import copy_data_folders_from_project # mocked in unit-tests from ..storage_api import ( delete_data_folders_of_project, @@ -50,7 +51,7 @@ async def get_project_for_user( project_uuid: str, user_id: int, *, - include_templates: bool = False + include_templates: bool = False, ) -> Dict: """ Returns a project accessible to user @@ -331,3 +332,18 @@ async def is_node_id_present_in_any_project_workbench( class ProjectState(BaseModel): locked: bool + + +async def notify_project_state_update(app: web.Application, project: Dict) -> None: + rooms_to_notify = [ + f"{gid}" for gid, rights in project["accessRights"].items() if rights["read"] + ] + messages = { + SOCKET_IO_PROJECT_UPDATED_EVENT: { + "project_uuid": project["uuid"], + "data": ProjectState(locked=True).dict(), + } + } + + for room in rooms_to_notify: + await post_group_messages(app, room, messages) 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 17f1a015b71..cfe6e27a4b8 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 @@ -300,11 +300,14 @@ async def open_project(request: web.Request) -> web.Response: # let's check if that project is already opened by someone other_users = await rt.find_users_of_resource("project_id", project_uuid) if other_users: + # project is already locked raise HTTPLocked(reason="Project is already opened by another user") await rt.add("project_id", project_uuid) # user id opened project uuid await projects_api.start_project_interactive_services(request, project, user_id) + # notify users that project is now locked + await projects_api.notify_project_state_update(request.app, project) return {"data": project} except ProjectNotFoundError: From 74843db69b06a05d37c35bc2bf92de2d42393131 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 14:01:38 +0200 Subject: [PATCH 57/71] notify users on closing project not 4 calls but 3 --- .../projects/projects_api.py | 6 +- .../projects/projects_handlers.py | 20 ++- .../tests/unit/with_dbs/test_projects.py | 120 ++++++++++-------- 3 files changed, 87 insertions(+), 59 deletions(-) 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 0e6efee027d..70aa428cee3 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 @@ -334,14 +334,16 @@ class ProjectState(BaseModel): locked: bool -async def notify_project_state_update(app: web.Application, project: Dict) -> None: +async def notify_project_state_update( + app: web.Application, project: Dict, opened: bool +) -> None: rooms_to_notify = [ f"{gid}" for gid, rights in project["accessRights"].items() if rights["read"] ] messages = { SOCKET_IO_PROJECT_UPDATED_EVENT: { "project_uuid": project["uuid"], - "data": ProjectState(locked=True).dict(), + "data": ProjectState(locked=opened).dict(), } } 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 cfe6e27a4b8..be4b4151b24 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 @@ -3,6 +3,7 @@ """ import json import logging +from typing import Dict from aiohttp import web from jsonschema import ValidationError @@ -307,7 +308,9 @@ async def open_project(request: web.Request) -> web.Response: # user id opened project uuid await projects_api.start_project_interactive_services(request, project, user_id) # notify users that project is now locked - await projects_api.notify_project_state_update(request.app, project) + await projects_api.notify_project_state_update( + request.app, project, opened=True + ) return {"data": project} except ProjectNotFoundError: @@ -327,21 +330,27 @@ async def close_project(request: web.Request) -> web.Response: from .projects_api import get_project_for_user with managed_resource(user_id, client_session_id, request.app) as rt: - await get_project_for_user( + project: Dict = await get_project_for_user( request.app, project_uuid=project_uuid, user_id=user_id, include_templates=True, ) await rt.remove("project_id") + other_users = await rt.find_users_of_resource("project_id", project_uuid) if not other_users: # only remove the services if no one else is using them now - fire_and_forget_task( - projects_api.remove_project_interactive_services( + async def close_project_task(): + await projects_api.remove_project_interactive_services( user_id, project_uuid, request.app ) - ) + + await projects_api.notify_project_state_update( + request.app, project, opened=False + ) + + fire_and_forget_task(close_project_task()) raise web.HTTPNoContent(content_type="application/json") except ProjectNotFoundError: @@ -363,6 +372,7 @@ async def state_project(request: web.Request) -> web.Response: user_id=user_id, include_templates=True, ) + users_of_project = await rt.find_users_of_resource("project_id", project_uuid) return {"data": {"locked": len(users_of_project) > 0}} diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 9b548626407..3b6ec0ce972 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -1195,11 +1195,12 @@ async def test_tags_to_studies( async def _connect_websocket( socketio_client: Callable, check_connection: bool, + client, client_id: str, events: Optional[Dict[str, Callable]] = None, ) -> socketio.AsyncClient: try: - sio = await socketio_client(client_id) + sio = await socketio_client(client_id, client) assert sio.sid if events: for event, handler in events.items(): @@ -1240,6 +1241,45 @@ async def _state_project( assert data == expected_project_state +import mock +import time + + +async def _assert_project_state_updated( + handler: mock.Mock, + shared_project: Dict, + expected_project_state: ProjectState, + num_calls: int, +) -> None: + if num_calls == 0: + handler.assert_not_called() + else: + # wait for the calls + now = time.monotonic() + MAX_WAITING_TIME = 15 + while time.monotonic() - now < MAX_WAITING_TIME: + await asyncio.sleep(1) + if handler.call_count == num_calls: + 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" + ) + + calls = [ + call( + json.dumps( + { + "project_uuid": shared_project["uuid"], + "data": expected_project_state.dict(), + } + ) + ) + ] * num_calls + handler.assert_has_calls(calls) + handler.reset_mock() + + @pytest.mark.parametrize(*standard_role_response()) async def test_open_shared_project_2_users_locked( client, @@ -1265,6 +1305,7 @@ async def test_open_shared_project_2_users_locked( sio_1 = await _connect_websocket( socketio_client, user_role != UserRole.ANONYMOUS, + client_1, client_id1, {SOCKET_IO_PROJECT_UPDATED_EVENT: mock_project_state_updated_handler}, ) @@ -1282,23 +1323,13 @@ async def test_open_shared_project_2_users_locked( expected.ok if user_role != UserRole.GUEST else web.HTTPOk, ) expected_project_state.locked = True - if user_role != UserRole.ANONYMOUS: - await asyncio.sleep(3) - # NOTE: there are 2 calls since we are part of the primary group and the all group - calls = [ - call( - json.dumps( - { - "project_uuid": shared_project["uuid"], - "data": expected_project_state.dict(), - } - ) - ) - ] * 2 - mock_project_state_updated_handler.assert_has_calls(calls) - mock_project_state_updated_handler.reset_mock() - else: - mock_project_state_updated_handler.assert_not_called() + # 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, + ) await _state_project( client_1, @@ -1314,6 +1345,7 @@ async def test_open_shared_project_2_users_locked( sio_2 = await _connect_websocket( socketio_client, user_role != UserRole.ANONYMOUS, + client_2, client_id2, {SOCKET_IO_PROJECT_UPDATED_EVENT: mock_project_state_updated_handler}, ) @@ -1343,23 +1375,15 @@ async def test_open_shared_project_2_users_locked( ) # we should receive an event that the project lock state changed - await asyncio.sleep(3) - if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]): - mock_project_state_updated_handler.assert_not_called() - else: - # NOTE: there are 4 calls since both users are registered now we are part of the primary group and the all group - calls = [ - call( - json.dumps( - { - "project_uuid": shared_project["uuid"], - "data": expected_project_state.dict(), - } - ) - ) - ] * 4 - mock_project_state_updated_handler.assert_has_calls(calls) - mock_project_state_updated_handler.reset_mock() + # 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 + 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, + ) # 4. user 2 now should be able to open the project await _open_project( @@ -1369,23 +1393,15 @@ async def test_open_shared_project_2_users_locked( expected.ok if user_role != UserRole.GUEST else HTTPLocked, ) expected_project_state.locked = True - await asyncio.sleep(3) - if any(user_role == role for role in [UserRole.ANONYMOUS, UserRole.GUEST]): - mock_project_state_updated_handler.assert_not_called() - else: - # NOTE: there are 4 calls since both users are registered now we are part of the primary group and the all group - calls = [ - call( - json.dumps( - { - "project_uuid": shared_project["uuid"], - "data": expected_project_state.dict(), - } - ) - ) - ] * 4 - mock_project_state_updated_handler.assert_has_calls(calls) - mock_project_state_updated_handler.reset_mock() + # 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, + ) await _state_project( client_1, shared_project, From bea0945dc9681f1b2373ce5e33555800938aef31 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 14:54:36 +0200 Subject: [PATCH 58/71] ensure resource is removed --- .../projects/projects_handlers.py | 37 +++++++++++-------- .../resource_manager/websocket_manager.py | 6 +-- .../tests/unit/with_dbs/test_projects.py | 13 +++---- 3 files changed, 30 insertions(+), 26 deletions(-) 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 be4b4151b24..fd0d5b9fe94 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 @@ -329,28 +329,33 @@ async def close_project(request: web.Request) -> web.Response: # TODO: temporary hidden until get_handlers_from_namespace refactor to seek marked functions instead! from .projects_api import get_project_for_user + project = await get_project_for_user( + request.app, + project_uuid=project_uuid, + user_id=user_id, + include_templates=True, + ) + project_opened_by_others: bool = False with managed_resource(user_id, client_session_id, request.app) as rt: - project: Dict = await get_project_for_user( - request.app, - project_uuid=project_uuid, - user_id=user_id, - include_templates=True, - ) await rt.remove("project_id") - - other_users = await rt.find_users_of_resource("project_id", project_uuid) - if not other_users: - # only remove the services if no one else is using them now - async def close_project_task(): + project_opened_by_others = ( + len(await rt.find_users_of_resource("project_id", project_uuid)) > 0 + ) + # if we are the only user left we can safely remove the services + async def close_project_task(): + try: + 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: + # ensure we notify the user whatever happens, the GC should take care of dangling services in case of issue + await projects_api.notify_project_state_update( + request.app, project, opened=False + ) - await projects_api.notify_project_state_update( - request.app, project, opened=False - ) - - fire_and_forget_task(close_project_task()) + fire_and_forget_task(close_project_task()) raise web.HTTPNoContent(content_type="application/json") except ProjectNotFoundError: 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 9e6d2f659f0..1deff4df369 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 @@ -16,7 +16,7 @@ import logging from contextlib import contextmanager -from typing import Dict, Iterator, List, Optional +from typing import Dict, Iterator, List, Optional, Union import attr from aiohttp import web @@ -150,9 +150,9 @@ async def find_users_of_resource(self, key: str, value: str) -> List[str]: @contextmanager def managed_resource( - user_id: str, client_session_id: Optional[str], app: web.Application + user_id: Union[str, int], client_session_id: Optional[str], app: web.Application ) -> Iterator[WebsocketRegistry]: - registry = WebsocketRegistry(user_id, client_session_id, app) + registry = WebsocketRegistry(str(user_id), client_session_id, app) try: yield registry except Exception: diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index 3b6ec0ce972..b7373eed35f 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -1367,13 +1367,6 @@ async def test_open_shared_project_2_users_locked( expected_project_state.locked = ( True if user_role == UserRole.GUEST else False ) # Guests cannot close projects - await _state_project( - client_1, - shared_project, - expected.ok if user_role != UserRole.GUEST else web.HTTPOk, - expected_project_state, - ) - # 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 await _assert_project_state_updated( @@ -1384,6 +1377,12 @@ async def test_open_shared_project_2_users_locked( 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, + ) # 4. user 2 now should be able to open the project await _open_project( From dc6b74a738c363d0d016b1d7e3a9cbd6506e2fb6 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 15:29:29 +0200 Subject: [PATCH 59/71] add recipe for mypy --- scripts/common.Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/common.Makefile b/scripts/common.Makefile index 3fc94e1ead0..e1349a61cf8 100644 --- a/scripts/common.Makefile +++ b/scripts/common.Makefile @@ -100,6 +100,9 @@ autoformat: ## runs black python formatter on this service's code. Use AFTER mak --exclude "/(\.eggs|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|\.svn|_build|buck-out|build|dist|migration|client-sdk|generated_code)/" \ $(CURDIR) +.PHONY: mypy +mypy: $(REPO_BASE_DIR)/mypy.ini ## runs mypy python static type checker on this services's code. Use AFTER make install-* + docker run -ti -v $(REPO_BASE_DIR)/mypy.ini:/config/mypy.ini -v $(CURDIR):/src --workdir=/src kiwicom/mypy mypy --config-file /config/mypy.ini src .PHONY: version-patch version-minor version-major version-patch: ## commits version with bug fixes not affecting the cookiecuter config From 54c5923bcea586328599ec07f29162bd59c9e50a Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 16:11:24 +0200 Subject: [PATCH 60/71] fix open a project by same user --- .../simcore_service_webserver/projects/projects_handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 fd0d5b9fe94..be4e60d19cd 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 @@ -298,9 +298,9 @@ async def open_project(request: web.Request) -> web.Response: include_templates=True, ) - # let's check if that project is already opened by someone + # let's check if that project is already opened by someone else other_users = await rt.find_users_of_resource("project_id", project_uuid) - if other_users: + if other_users and any(f"{user_id}" != user for user in other_users): # project is already locked raise HTTPLocked(reason="Project is already opened by another user") await rt.add("project_id", project_uuid) From 55ab98c0bf63c1316d75f899c760cbe40dc0bc36 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 16:12:10 +0200 Subject: [PATCH 61/71] lint --- .../src/simcore_service_webserver/projects/projects_handlers.py | 1 - 1 file changed, 1 deletion(-) 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 be4e60d19cd..b4d8a7048df 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 @@ -3,7 +3,6 @@ """ import json import logging -from typing import Dict from aiohttp import web from jsonschema import ValidationError From 61219a5f427333174f077d815f68e1293610bfd2 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 26 Jun 2020 17:45:47 +0200 Subject: [PATCH 62/71] for now ignore missing imports --- mypy.ini | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/mypy.ini b/mypy.ini index bd8c2057cf8..54108569424 100644 --- a/mypy.ini +++ b/mypy.ini @@ -3,7 +3,9 @@ python_version = 3.6 warn_return_any = True warn_unused_configs = True - +namespace_packages = True +ignore_missing_imports = True +; mypy_path = /packages/service-library/src, ./packages/postgres-database/src # Per-module options: [mypy-aio-pika.*] ignore_missing_imports = True @@ -17,6 +19,8 @@ ignore_missing_imports = True ignore_missing_imports = True [mypy-aiosmtplib.*] ignore_missing_imports = True +[mypy-aiozipkin.*] +ignore_missing_imports = True [mypy-asyncpg.*] ignore_missing_imports = True [mypy-celery.*] @@ -27,6 +31,10 @@ ignore_missing_imports = True ignore_missing_imports = True [mypy-jsondiff.*] ignore_missing_imports = True +[mypy-jsonschema.*] +ignore_missing_imports = True +[mypy-openapi_core.*] +ignore_missing_imports = True [mypy-passlib.*] ignore_missing_imports = True [mypy-prometheus_client.*] @@ -39,3 +47,7 @@ ignore_missing_imports = True ignore_missing_imports = True [mypy-trafaret.*] ignore_missing_imports = True +[mypy-trafaret_config.*] +ignore_missing_imports = True +[mypy-yarl.*] +ignore_missing_imports = True From 20eefda21468e4edf4549dff2e4586c923fec26b Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 1 Jul 2020 09:43:59 +0200 Subject: [PATCH 63/71] bad merge --- mypy.ini | 1 + .../src/simcore_service_webserver/projects/projects_api.py | 1 - services/web/server/tests/unit/with_dbs/test_projects.py | 1 - 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/mypy.ini b/mypy.ini index 74073358939..e343faa54ff 100644 --- a/mypy.ini +++ b/mypy.ini @@ -4,6 +4,7 @@ python_version = 3.6 warn_return_any = True warn_unused_configs = True namespace_packages = True +; ignore_missing_imports = True # Per-module options: [mypy-aio-pika.*] ignore_missing_imports = True 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 09f4ae01551..ff615334a80 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 @@ -14,7 +14,6 @@ from uuid import uuid4 from aiohttp import web -from pydantic import BaseModel from servicelib.application_keys import APP_JSONSCHEMA_SPECS_KEY from servicelib.jsonschema_validation import validate_instance diff --git a/services/web/server/tests/unit/with_dbs/test_projects.py b/services/web/server/tests/unit/with_dbs/test_projects.py index df4c2b1784a..4feb5fab05c 100644 --- a/services/web/server/tests/unit/with_dbs/test_projects.py +++ b/services/web/server/tests/unit/with_dbs/test_projects.py @@ -33,7 +33,6 @@ from simcore_service_webserver.director import setup_director from simcore_service_webserver.login import setup_login from simcore_service_webserver.projects import setup_projects -from simcore_service_webserver.projects.projects_api import ProjectState from simcore_service_webserver.projects.projects_handlers import ( OVERRIDABLE_DOCUMENT_KEYS, ) From 03fb7541623956a91bd4dd7cbffceeb87715aa83 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Wed, 1 Jul 2020 10:01:53 +0200 Subject: [PATCH 64/71] listen projectStateUpdated socket event --- .../source/class/osparc/dashboard/StudyBrowser.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index d53a169b094..2cd51f34c62 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -302,6 +302,20 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { }, __attachEventHandlers: function() { + // Listen to socket + const socket = osparc.wrapper.WebSocket.getInstance(); + // callback for incoming logs + const slotName = "projectStateUpdated"; + socket.removeSlot(slotName); + socket.on(slotName, function(jsonString) { + const data = JSON.parse(jsonString); + if (data) { + const studyId = data["project_uuid"]; + const locked = (data["data"] && "locked" in data["data"]) ? data["data"]["locked"] : false; + console.log(studyId, locked); + } + }, this); + const textfield = this.__studyFilters.getTextFilter().getChildControl("textfield"); textfield.addListener("appear", () => { textfield.focus(); From 86a8ce4d80fe6f03345ba1e288d16e18559fbfbf Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 2 Jul 2020 10:52:48 +0200 Subject: [PATCH 65/71] new state object --- .../class/osparc/dashboard/StudyBrowser.js | 11 ++++--- .../dashboard/StudyBrowserButtonItem.js | 32 ++++++++++++++++++- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index 2cd51f34c62..015273c8ee4 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -311,8 +311,11 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { const data = JSON.parse(jsonString); if (data) { const studyId = data["project_uuid"]; - const locked = (data["data"] && "locked" in data["data"]) ? data["data"]["locked"] : false; - console.log(studyId, locked); + const state = ("data" in data) ? data["data"] : {}; + const studyItem = this.__userStudyContainer.getChildren().find(card => card.getUuid() === studyId); + if (studyItem) { + studyItem.setState(state); + } } }, this); @@ -470,7 +473,7 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { accessRights: study.accessRights ? study.accessRights : null, lastChangeDate: study.lastChangeDate ? new Date(study.lastChangeDate) : null, icon: study.thumbnail || (isTemplate ? "@FontAwesome5Solid/copy/50" : "@FontAwesome5Solid/file-alt/50"), - locked: study.locked ? study.locked : false, + state: study.state ? study.state : {}, tags }); const menu = this.__getStudyItemMenu(item, study, isTemplate); @@ -478,7 +481,7 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { item.subscribeToFilterGroup("studyBrowser"); item.addListener("execute", () => { - if (!item.getLocked()) { + if (!item.isLocked()) { this.__itemClicked(item, isTemplate); } }, this); diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index 530370b5c7f..b7c56af91f0 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -96,11 +96,23 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { apply: "_applyTags" }, + state: { + check: "Object", + nullable: false, + apply: "_applyState" + }, + locked: { check: "Boolean", init: false, nullable: false, apply: "_applyLocked" + }, + + lockedBy: { + check: "String", + nullable: true, + apply: "_applyLockedBy" } }, @@ -132,7 +144,7 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { __itemSelected: function() { const selected = this.getValue(); - if (this.getLocked() && selected) { + if (this.isLocked() && selected) { this.setValue(false); } @@ -338,6 +350,18 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { } }, + _applyState: function(state) { + const locked = ("locked" in state) ? state["locked"]["value"] : false; + if (locked) { + this.setLocked(state["locked"]["value"]); + const owner = state["locked"]["owner"]; + this.setLockedBy(osparc.utils.Utils.firstsUp(owner["first_name"], owner["last_name"])); + } else { + this.setLocked(false); + this.setLockedBy(null); + } + }, + _applyLocked: function(locked) { this.set({ cursor: locked ? "not-allowed" : "pointer" @@ -363,6 +387,12 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { }); }, + _applyLockedBy: function(lockedBy) { + this.set({ + toolTipText: lockedBy + this.tr(" is using it") + }); + }, + _shouldApplyFilter: function(data) { if (data.text) { const checks = [ From caca98da4cd7ccf1f51f916f3f27e636a2648202 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 2 Jul 2020 11:05:10 +0200 Subject: [PATCH 66/71] minor --- .../source/class/osparc/dashboard/StudyBrowserButtonItem.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index b7c56af91f0..7471110602b 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -376,9 +376,9 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { lock.setVisibility(locked ? "visible" : "excluded"); [ - "menu-button", + "tick-selected", "tick-unselected", - "tick-selected" + "menu-button" ].forEach(childName => { const child = this.getChildControl(childName); child.set({ From 776db3a95c60409ee292be451f701b7dd4edb64c Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 2 Jul 2020 11:09:56 +0200 Subject: [PATCH 67/71] minor --- .../source/class/osparc/dashboard/StudyBrowserButtonItem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index 7471110602b..18ffeeb43ff 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -389,7 +389,7 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { _applyLockedBy: function(lockedBy) { this.set({ - toolTipText: lockedBy + this.tr(" is using it") + toolTipText: lockedBy ? (lockedBy + this.tr(" is using it")) : null }); }, From f19685106a1cc68a74c6088475c5865755bb4df0 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 2 Jul 2020 11:30:03 +0200 Subject: [PATCH 68/71] skip system-test-e2e --- .github/workflows/ci-testing-deploy.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci-testing-deploy.yml b/.github/workflows/ci-testing-deploy.yml index 54c10604e1b..62dbf57e040 100644 --- a/.github/workflows/ci-testing-deploy.yml +++ b/.github/workflows/ci-testing-deploy.yml @@ -748,6 +748,9 @@ jobs: run: ./ci/github/system-testing/swarm-deploy.bash clean_up system-test-e2e: + # skip the job until make it faster and more reliable + # https://github.com/ITISFoundation/osparc-simcore/issues/1594 + if: "false" name: System-testing e2e needs: [build-test-images] runs-on: ${{ matrix.os }} From 314dfcf0a4e91659aec065dab56ca34a2b8e58f7 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 2 Jul 2020 11:35:20 +0200 Subject: [PATCH 69/71] fixme added --- .github/workflows/ci-testing-deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-testing-deploy.yml b/.github/workflows/ci-testing-deploy.yml index 62dbf57e040..56e6856c724 100644 --- a/.github/workflows/ci-testing-deploy.yml +++ b/.github/workflows/ci-testing-deploy.yml @@ -748,7 +748,7 @@ jobs: run: ./ci/github/system-testing/swarm-deploy.bash clean_up system-test-e2e: - # skip the job until make it faster and more reliable + # FIXME: skip the job until make it faster and more reliable # https://github.com/ITISFoundation/osparc-simcore/issues/1594 if: "false" name: System-testing e2e From f3a365459908915cb656a6fe731df8a5973d8f51 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 2 Jul 2020 16:25:09 +0200 Subject: [PATCH 70/71] minor --- .../web/client/source/class/osparc/dashboard/ExploreBrowser.js | 2 ++ .../web/client/source/class/osparc/dashboard/StudyBrowser.js | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/client/source/class/osparc/dashboard/ExploreBrowser.js b/services/web/client/source/class/osparc/dashboard/ExploreBrowser.js index 9936c124f3d..7d12f10d405 100644 --- a/services/web/client/source/class/osparc/dashboard/ExploreBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/ExploreBrowser.js @@ -130,6 +130,8 @@ qx.Class.define("osparc.dashboard.ExploreBrowser", { __initResources: function() { this.__showLoadingPage(this.tr("Discovering Templates and Apps")); + this.__templateStudies = []; + this.__services = []; const servicesTags = this.__getTags(); const store = osparc.store.Store.getInstance(); const servicesPromise = store.getServicesDAGs(true); diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index 9a08961f6ac..eaa3a4869f0 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -123,7 +123,6 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { this.__showLoadingPage(this.tr("Loading Studies")); this.__userStudies = []; - this.__templateStudies = []; const resourcePromises = []; const store = osparc.store.Store.getInstance(); resourcePromises.push(store.getVisibleMembers()); From 38241ad98c8fefa8a18e0a756507dd0c299e4048 Mon Sep 17 00:00:00 2001 From: odeimaiz Date: Thu, 2 Jul 2020 17:30:55 +0200 Subject: [PATCH 71/71] minor fixes --- .../class/osparc/dashboard/StudyBrowser.js | 2 +- .../dashboard/StudyBrowserButtonItem.js | 33 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js index eaa3a4869f0..ef30b74ec18 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowser.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowser.js @@ -272,7 +272,7 @@ qx.Class.define("osparc.dashboard.StudyBrowser", { if (data) { const studyId = data["project_uuid"]; const state = ("data" in data) ? data["data"] : {}; - const studyItem = this.__userStudyContainer.getChildren().find(card => card.getUuid() === studyId); + const studyItem = this.__userStudyContainer.getChildren().find(card => (card instanceof osparc.dashboard.StudyBrowserButtonItem) && (card.getUuid() === studyId)); if (studyItem) { studyItem.setState(state); } diff --git a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js index 8029f4eb2d7..1901cabee08 100644 --- a/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js +++ b/services/web/client/source/class/osparc/dashboard/StudyBrowserButtonItem.js @@ -131,31 +131,40 @@ qx.Class.define("osparc.dashboard.StudyBrowserButtonItem", { }, multiSelection: function(on) { - const menuButton = this.getChildControl("menu-button"); if (on) { + const menuButton = this.getChildControl("menu-button"); menuButton.setVisibility("excluded"); this.__itemSelected(); } else { - menuButton.setVisibility("visible"); - const tick = this.getChildControl("tick-selected"); - tick.setVisibility("excluded"); - const untick = this.getChildControl("tick-unselected"); - untick.setVisibility("excluded"); + this.__showMenuOnly(); } }, __itemSelected: function() { - const selected = this.getValue(); + if (this.isResourceType("study")) { + const selected = this.getValue(); + + if (this.isLocked() && selected) { + this.setValue(false); + } + + const tick = this.getChildControl("tick-selected"); + tick.setVisibility(selected ? "visible" : "excluded"); - if (this.isLocked() && selected) { - this.setValue(false); + const untick = this.getChildControl("tick-unselected"); + untick.setVisibility(selected ? "excluded" : "visible"); + } else { + this.__showMenuOnly(); } + }, + __showMenuOnly: function() { + const menuButton = this.getChildControl("menu-button"); + menuButton.setVisibility("visible"); const tick = this.getChildControl("tick-selected"); - tick.setVisibility(selected ? "visible" : "excluded"); - + tick.setVisibility("excluded"); const untick = this.getChildControl("tick-unselected"); - untick.setVisibility(selected ? "excluded" : "visible"); + untick.setVisibility("excluded"); }, // overridden