From 974bb077ec2a3ef16214d13b2af288f6356d6950 Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Tue, 8 Oct 2024 12:39:02 +0200 Subject: [PATCH 01/11] aux function --- fractal_server/app/routes/aux/_task_group.py | 51 ++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 fractal_server/app/routes/aux/_task_group.py diff --git a/fractal_server/app/routes/aux/_task_group.py b/fractal_server/app/routes/aux/_task_group.py new file mode 100644 index 0000000000..760c90e13e --- /dev/null +++ b/fractal_server/app/routes/aux/_task_group.py @@ -0,0 +1,51 @@ +from typing import Optional + +from fastapi import HTTPException +from fastapi import status +from sqlmodel import select + +from fractal_server.app.db import AsyncSession +from fractal_server.app.models.v2 import TaskGroupV2 + + +async def _verify_non_duplication_constraints( + db: AsyncSession, + pkg_name: str, + user_id: int, + version: Optional[str], + user_group_id: Optional[int] = None, +): + stm = ( + select(TaskGroupV2) + .where(TaskGroupV2.pkg_name == pkg_name) + .where(TaskGroupV2.version == version) # FIXME test with None + .where(TaskGroupV2.user_id == user_id) + ) + res = await db.execute(stm) + duplicate = res.scalars().all() + if duplicate: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=( + "There is already a TaskGroupV2 with " + f"({pkg_name=}, {version=}, {user_id=})." + ), + ) + + if user_group_id is not None: + stm = ( + select(TaskGroupV2) + .where(TaskGroupV2.pkg_name == pkg_name) + .where(TaskGroupV2.version == version) + .where(TaskGroupV2.user_group_id == user_group_id) + ) + res = await db.execute(stm) + duplicate = res.scalars().all() + if duplicate: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=( + "There is already a TaskGroupV2 with " + f"({pkg_name=}, {version=}, {user_group_id=})." + ), + ) From 8b55b3228ddcfd281028eef8f6ab288e96208d68 Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Tue, 8 Oct 2024 12:44:37 +0200 Subject: [PATCH 02/11] move function [skip ci] --- fractal_server/app/routes/aux/_task_group.py | 51 -------------------- 1 file changed, 51 deletions(-) delete mode 100644 fractal_server/app/routes/aux/_task_group.py diff --git a/fractal_server/app/routes/aux/_task_group.py b/fractal_server/app/routes/aux/_task_group.py deleted file mode 100644 index 760c90e13e..0000000000 --- a/fractal_server/app/routes/aux/_task_group.py +++ /dev/null @@ -1,51 +0,0 @@ -from typing import Optional - -from fastapi import HTTPException -from fastapi import status -from sqlmodel import select - -from fractal_server.app.db import AsyncSession -from fractal_server.app.models.v2 import TaskGroupV2 - - -async def _verify_non_duplication_constraints( - db: AsyncSession, - pkg_name: str, - user_id: int, - version: Optional[str], - user_group_id: Optional[int] = None, -): - stm = ( - select(TaskGroupV2) - .where(TaskGroupV2.pkg_name == pkg_name) - .where(TaskGroupV2.version == version) # FIXME test with None - .where(TaskGroupV2.user_id == user_id) - ) - res = await db.execute(stm) - duplicate = res.scalars().all() - if duplicate: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - detail=( - "There is already a TaskGroupV2 with " - f"({pkg_name=}, {version=}, {user_id=})." - ), - ) - - if user_group_id is not None: - stm = ( - select(TaskGroupV2) - .where(TaskGroupV2.pkg_name == pkg_name) - .where(TaskGroupV2.version == version) - .where(TaskGroupV2.user_group_id == user_group_id) - ) - res = await db.execute(stm) - duplicate = res.scalars().all() - if duplicate: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - detail=( - "There is already a TaskGroupV2 with " - f"({pkg_name=}, {version=}, {user_group_id=})." - ), - ) From 717f3ed7b4db34eb9b741e02c575f849951e426b Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Tue, 8 Oct 2024 12:44:50 +0200 Subject: [PATCH 03/11] precommit --- .../app/routes/api/v2/_aux_functions_tasks.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py index 7f17c6da28..8b388465fa 100644 --- a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py +++ b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py @@ -207,3 +207,46 @@ async def _get_valid_user_group_id( user_id=user_id, user_group_id=user_group_id, db=db ) return user_group_id + + +async def _verify_non_duplication_constraints( + db: AsyncSession, + pkg_name: str, + user_id: int, + version: Optional[str], + user_group_id: Optional[int] = None, +): + stm = ( + select(TaskGroupV2) + .where(TaskGroupV2.pkg_name == pkg_name) + .where(TaskGroupV2.version == version) # FIXME test with None + .where(TaskGroupV2.user_id == user_id) + ) + res = await db.execute(stm) + duplicate = res.scalars().all() + if duplicate: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=( + "There is already a TaskGroupV2 with " + f"({pkg_name=}, {version=}, {user_id=})." + ), + ) + + if user_group_id is not None: + stm = ( + select(TaskGroupV2) + .where(TaskGroupV2.pkg_name == pkg_name) + .where(TaskGroupV2.version == version) + .where(TaskGroupV2.user_group_id == user_group_id) + ) + res = await db.execute(stm) + duplicate = res.scalars().all() + if duplicate: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=( + "There is already a TaskGroupV2 with " + f"({pkg_name=}, {version=}, {user_group_id=})." + ), + ) From 3986bcfd61fb50758a1c999f1c501bc7635db7d6 Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Tue, 8 Oct 2024 12:49:23 +0200 Subject: [PATCH 04/11] split into two functions [skip ci] --- .../app/routes/api/v2/_aux_functions_tasks.py | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py index 8b388465fa..a1fea91ad0 100644 --- a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py +++ b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py @@ -209,12 +209,11 @@ async def _get_valid_user_group_id( return user_group_id -async def _verify_non_duplication_constraints( +async def _verify_non_duplication_user_constraint( db: AsyncSession, pkg_name: str, user_id: int, version: Optional[str], - user_group_id: Optional[int] = None, ): stm = ( select(TaskGroupV2) @@ -233,20 +232,26 @@ async def _verify_non_duplication_constraints( ), ) - if user_group_id is not None: - stm = ( - select(TaskGroupV2) - .where(TaskGroupV2.pkg_name == pkg_name) - .where(TaskGroupV2.version == version) - .where(TaskGroupV2.user_group_id == user_group_id) + +async def _verify_non_duplication_group_constraint( + db: AsyncSession, + pkg_name: str, + version: Optional[str], + user_group_id: Optional[int], +): + stm = ( + select(TaskGroupV2) + .where(TaskGroupV2.pkg_name == pkg_name) + .where(TaskGroupV2.version == version) + .where(TaskGroupV2.user_group_id == user_group_id) + ) + res = await db.execute(stm) + duplicate = res.scalars().all() + if duplicate: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=( + "There is already a TaskGroupV2 with " + f"({pkg_name=}, {version=}, {user_group_id=})." + ), ) - res = await db.execute(stm) - duplicate = res.scalars().all() - if duplicate: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - detail=( - "There is already a TaskGroupV2 with " - f"({pkg_name=}, {version=}, {user_group_id=})." - ), - ) From 003ba5c103963425699dfcdf93d0a4100796cb9f Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Tue, 8 Oct 2024 14:13:16 +0200 Subject: [PATCH 05/11] protect POST task --- fractal_server/app/routes/api/v2/task.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/fractal_server/app/routes/api/v2/task.py b/fractal_server/app/routes/api/v2/task.py index eb1f73a083..29fa6fdd4a 100644 --- a/fractal_server/app/routes/api/v2/task.py +++ b/fractal_server/app/routes/api/v2/task.py @@ -14,6 +14,8 @@ from ._aux_functions_tasks import _get_task_full_access from ._aux_functions_tasks import _get_task_read_access from ._aux_functions_tasks import _get_valid_user_group_id +from ._aux_functions_tasks import _verify_non_duplication_group_constraint +from ._aux_functions_tasks import _verify_non_duplication_user_constraint from fractal_server.app.db import AsyncSession from fractal_server.app.db import get_async_db from fractal_server.app.models import LinkUserGroup @@ -221,13 +223,23 @@ async def create_task( ) # Add task db_task = TaskV2(**task.dict(), owner=owner, type=task_type) - + await _verify_non_duplication_user_constraint( + db=db, pkg_name=task.name, user_id=user.id, version=task.version + ) + if user_group_id is not None: + await _verify_non_duplication_group_constraint( + db=db, + pkg_name=task.name, + user_group_id=user_group_id, + version=task.version, + ) db_task_group = TaskGroupV2( user_id=user.id, user_group_id=user_group_id, active=True, task_list=[db_task], origin="other", + version=task.version, pkg_name=task.name, ) db.add(db_task_group) From a91a7f1342786324e7c32759585b7adfc1086d01 Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Tue, 8 Oct 2024 14:18:28 +0200 Subject: [PATCH 06/11] protect PATCH task group --- fractal_server/app/routes/api/v2/task_group.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fractal_server/app/routes/api/v2/task_group.py b/fractal_server/app/routes/api/v2/task_group.py index 4602152ee7..dc76826e4b 100644 --- a/fractal_server/app/routes/api/v2/task_group.py +++ b/fractal_server/app/routes/api/v2/task_group.py @@ -8,6 +8,7 @@ from ._aux_functions_tasks import _get_task_group_full_access from ._aux_functions_tasks import _get_task_group_read_access +from ._aux_functions_tasks import _verify_non_duplication_group_constraint from fractal_server.app.db import AsyncSession from fractal_server.app.db import get_async_db from fractal_server.app.models import LinkUserGroup @@ -124,7 +125,13 @@ async def patch_task_group( user_id=user.id, db=db, ) - + if task_group_update.user_group_id is not None: + await _verify_non_duplication_group_constraint( + db=db, + pkg_name=task_group.pkg_name, + version=task_group.version, + user_group_id=task_group_update.user_group_id, + ) for key, value in task_group_update.dict(exclude_unset=True).items(): if (key == "user_group_id") and (value is not None): await _verify_user_belongs_to_group( From df26807205fae6ed17f8dd68037dd17b1924892e Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Tue, 8 Oct 2024 14:28:41 +0200 Subject: [PATCH 07/11] user_group_id not optional --- .../app/routes/api/v2/_aux_functions_tasks.py | 16 ++++++++++++---- fractal_server/app/routes/api/v2/task.py | 11 ++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py index a1fea91ad0..6cfdb6bb01 100644 --- a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py +++ b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py @@ -211,15 +211,15 @@ async def _get_valid_user_group_id( async def _verify_non_duplication_user_constraint( db: AsyncSession, - pkg_name: str, user_id: int, + pkg_name: str, version: Optional[str], ): stm = ( select(TaskGroupV2) + .where(TaskGroupV2.user_id == user_id) .where(TaskGroupV2.pkg_name == pkg_name) .where(TaskGroupV2.version == version) # FIXME test with None - .where(TaskGroupV2.user_id == user_id) ) res = await db.execute(stm) duplicate = res.scalars().all() @@ -235,15 +235,23 @@ async def _verify_non_duplication_user_constraint( async def _verify_non_duplication_group_constraint( db: AsyncSession, + user_group_id: int, pkg_name: str, version: Optional[str], - user_group_id: Optional[int], ): + if user_group_id is None: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=( + "`_verify_non_duplication_group_constraint` cannot be called " + f"with {user_group_id=}." + ), + ) stm = ( select(TaskGroupV2) + .where(TaskGroupV2.user_group_id == user_group_id) .where(TaskGroupV2.pkg_name == pkg_name) .where(TaskGroupV2.version == version) - .where(TaskGroupV2.user_group_id == user_group_id) ) res = await db.execute(stm) duplicate = res.scalars().all() diff --git a/fractal_server/app/routes/api/v2/task.py b/fractal_server/app/routes/api/v2/task.py index 29fa6fdd4a..bbf96c15d9 100644 --- a/fractal_server/app/routes/api/v2/task.py +++ b/fractal_server/app/routes/api/v2/task.py @@ -223,15 +223,16 @@ async def create_task( ) # Add task db_task = TaskV2(**task.dict(), owner=owner, type=task_type) + pkg_name = db_task.name await _verify_non_duplication_user_constraint( - db=db, pkg_name=task.name, user_id=user.id, version=task.version + db=db, pkg_name=pkg_name, user_id=user.id, version=db_task.version ) if user_group_id is not None: await _verify_non_duplication_group_constraint( db=db, - pkg_name=task.name, + pkg_name=pkg_name, user_group_id=user_group_id, - version=task.version, + version=db_task.version, ) db_task_group = TaskGroupV2( user_id=user.id, @@ -239,8 +240,8 @@ async def create_task( active=True, task_list=[db_task], origin="other", - version=task.version, - pkg_name=task.name, + version=db_task.version, + pkg_name=pkg_name, ) db.add(db_task_group) await db.commit() From bb1679a9601e24ea7405ea909b2e7d8078b25fed Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Wed, 9 Oct 2024 09:24:44 +0200 Subject: [PATCH 08/11] fix ci --- tests/v2/03_api/test_api_task.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/v2/03_api/test_api_task.py b/tests/v2/03_api/test_api_task.py index 47ea0ceb10..2a272741eb 100644 --- a/tests/v2/03_api/test_api_task.py +++ b/tests/v2/03_api/test_api_task.py @@ -136,7 +136,7 @@ async def test_post_task(client, MockCurrentUser): assert res.json()["tags"] == ["compound", "test", "post", "api"] task = TaskCreateV2( - name="task_name", + name="task_name2", # Parallel source=f"{TASK_SOURCE}-parallel", command_parallel="task_command_parallel", @@ -147,7 +147,7 @@ async def test_post_task(client, MockCurrentUser): assert res.status_code == 201 assert res.json()["type"] == "parallel" task = TaskCreateV2( - name="task_name", + name="task_name3", # Non Parallel source=f"{TASK_SOURCE}-non_parallel", command_non_parallel="task_command_non_parallel", @@ -155,7 +155,6 @@ async def test_post_task(client, MockCurrentUser): res = await client.post( f"{PREFIX}/", json=task.dict(exclude_unset=True) ) - debug(res.json()) assert res.status_code == 201 assert res.json()["type"] == "non_parallel" @@ -181,8 +180,9 @@ async def test_post_task(client, MockCurrentUser): # Case 1: (username, slurm_user) = (None, None) user_kwargs = dict(username=None, is_verified=True) user_settings_dict = dict(slurm_user=None) - payload = dict(name="task", command_parallel="cmd") + payload = dict(command_parallel="cmd") payload["source"] = "source_x" + payload["name"] = "x" async with MockCurrentUser( user_kwargs=user_kwargs, user_settings_dict=user_settings_dict ): @@ -205,6 +205,7 @@ async def test_post_task(client, MockCurrentUser): user_kwargs = dict(username=None, is_verified=True) user_settings_dict = dict(slurm_user=SLURM_USER) payload["source"] = "source_z" + payload["name"] = "z" async with MockCurrentUser( user_kwargs=user_kwargs, user_settings_dict=user_settings_dict ): @@ -214,6 +215,7 @@ async def test_post_task(client, MockCurrentUser): user_kwargs = dict(username=USERNAME, is_verified=True) user_settings_dict = dict(slurm_user=None) payload["source"] = "source_xyz" + payload["name"] = "xyz" async with MockCurrentUser( user_kwargs=user_kwargs, user_settings_dict=user_settings_dict ): @@ -282,19 +284,21 @@ async def test_post_task_user_group_id( await db.commit() await db.refresh(team1_group) - args = dict(name="a", command_non_parallel="cmd") + args = dict(command_non_parallel="cmd") async with MockCurrentUser(user_kwargs=dict(is_verified=True)): # No query parameter - res = await client.post(f"{PREFIX}/", json=dict(source="1", **args)) + res = await client.post( + f"{PREFIX}/", json=dict(name="a", source="1", **args) + ) assert res.status_code == 201 taskgroup = await db.get(TaskGroupV2, res.json()["taskgroupv2_id"]) assert taskgroup.user_group_id == default_user_group.id # Private task res = await client.post( - f"{PREFIX}/?private=true", json=dict(source="2", **args) + f"{PREFIX}/?private=true", json=dict(name="b", source="2", **args) ) assert res.status_code == 201 taskgroup = await db.get(TaskGroupV2, res.json()["taskgroupv2_id"]) @@ -303,7 +307,7 @@ async def test_post_task_user_group_id( # Specific usergroup id / OK res = await client.post( f"{PREFIX}/?user_group_id={default_user_group.id}", - json=dict(source="3", **args), + json=dict(name="c", source="3", **args), ) assert res.status_code == 201 taskgroup = await db.get(TaskGroupV2, res.json()["taskgroupv2_id"]) @@ -312,7 +316,7 @@ async def test_post_task_user_group_id( # Specific usergroup id / not belonging res = await client.post( f"{PREFIX}/?user_group_id={team1_group.id}", - json=dict(source="4", **args), + json=dict(name="d", source="4", **args), ) assert res.status_code == 403 debug(res.json()) @@ -320,7 +324,7 @@ async def test_post_task_user_group_id( # Conflicting query parameters res = await client.post( f"{PREFIX}/?private=true&user_group_id={default_user_group.id}", - json=dict(source="5", **args), + json=dict(name="e", source="5", **args), ) assert res.status_code == 422 debug(res.json()) @@ -333,7 +337,9 @@ async def test_post_task_user_group_id( ), "MONKEY", ) - res = await client.post(f"{PREFIX}/", json=dict(source="4", **args)) + res = await client.post( + f"{PREFIX}/", json=dict(name="f", source="4", **args) + ) assert res.status_code == 404 From 226bcac1fb8dd2febcf0e3663e7be0bcd42c08c8 Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Wed, 9 Oct 2024 10:21:00 +0200 Subject: [PATCH 09/11] coverage --- tests/v2/03_api/test_api_task.py | 11 +++++++++++ tests/v2/03_api/test_api_task_group.py | 12 +++++++++++- tests/v2/03_api/test_unit_aux_functions_tasks.py | 14 ++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/v2/03_api/test_api_task.py b/tests/v2/03_api/test_api_task.py index 2a272741eb..ff981585f0 100644 --- a/tests/v2/03_api/test_api_task.py +++ b/tests/v2/03_api/test_api_task.py @@ -135,6 +135,17 @@ async def test_post_task(client, MockCurrentUser): assert res.json()["authors"] == "Foo Bar + Fractal Team" assert res.json()["tags"] == ["compound", "test", "post", "api"] + task = TaskCreateV2( + name="task_name", + source=f"{TASK_SOURCE}-parallel", + command_parallel="task_command_parallel", + ) + res = await client.post( + f"{PREFIX}/", json=task.dict(exclude_unset=True) + ) + # TaskGroupV2 with same (pkg_name, version, user_id) + assert res.status_code == 422 + task = TaskCreateV2( name="task_name2", # Parallel diff --git a/tests/v2/03_api/test_api_task_group.py b/tests/v2/03_api/test_api_task_group.py index 3028095e86..c632c620d7 100644 --- a/tests/v2/03_api/test_api_task_group.py +++ b/tests/v2/03_api/test_api_task_group.py @@ -126,7 +126,9 @@ async def test_delete_task_group_fail( assert res.status_code == 422 -async def test_patch_task_group(client, MockCurrentUser, task_factory_v2): +async def test_patch_task_group( + client, MockCurrentUser, task_factory_v2, default_user_group +): async with MockCurrentUser() as user1: task = await task_factory_v2(user_id=user1.id, source="source") @@ -157,6 +159,14 @@ async def test_patch_task_group(client, MockCurrentUser, task_factory_v2): ) assert res.status_code == 404 + # Already linked UserGroup + + res = await client.patch( + f"{PREFIX}/{task.taskgroupv2_id}/", + json=dict(user_group_id=default_user_group.id), + ) + assert res.status_code == 422 + async with MockCurrentUser(): # Unauthorized diff --git a/tests/v2/03_api/test_unit_aux_functions_tasks.py b/tests/v2/03_api/test_unit_aux_functions_tasks.py index 81cef34a5d..aab606fa71 100644 --- a/tests/v2/03_api/test_unit_aux_functions_tasks.py +++ b/tests/v2/03_api/test_unit_aux_functions_tasks.py @@ -14,6 +14,9 @@ from fractal_server.app.routes.api.v2._aux_functions_tasks import ( _get_task_read_access, ) +from fractal_server.app.routes.api.v2._aux_functions_tasks import ( + _verify_non_duplication_group_constraint, +) from fractal_server.app.security import FRACTAL_DEFAULT_GROUP_NAME @@ -150,3 +153,14 @@ async def test_get_task_require_active(db, task_factory_v2): await _get_task_read_access( task_id=task.id, user_id=user.id, db=db, require_active=True ) + + +async def test_unit_verify_non_duplication_group_constraint(db): + with pytest.raises(HTTPException): + # fail because `user_group_id=None` + await _verify_non_duplication_group_constraint( + db=db, + user_group_id=None, + pkg_name="foo", + version=None, + ) From 3ef1a66336d478e9b963ad9e2d22fd95d15e18a9 Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Wed, 9 Oct 2024 14:10:16 +0200 Subject: [PATCH 10/11] accept user_group_id=None --- .../app/routes/api/v2/_aux_functions_tasks.py | 11 +++-------- fractal_server/app/routes/api/v2/task.py | 13 ++++++------- fractal_server/app/routes/api/v2/task_group.py | 13 ++++++------- tests/v2/03_api/test_unit_aux_functions_tasks.py | 14 -------------- 4 files changed, 15 insertions(+), 36 deletions(-) diff --git a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py index 6cfdb6bb01..b571eb35b4 100644 --- a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py +++ b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py @@ -235,18 +235,13 @@ async def _verify_non_duplication_user_constraint( async def _verify_non_duplication_group_constraint( db: AsyncSession, - user_group_id: int, + user_group_id: Optional[int], pkg_name: str, version: Optional[str], ): if user_group_id is None: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - detail=( - "`_verify_non_duplication_group_constraint` cannot be called " - f"with {user_group_id=}." - ), - ) + return + stm = ( select(TaskGroupV2) .where(TaskGroupV2.user_group_id == user_group_id) diff --git a/fractal_server/app/routes/api/v2/task.py b/fractal_server/app/routes/api/v2/task.py index bbf96c15d9..8a6cac237d 100644 --- a/fractal_server/app/routes/api/v2/task.py +++ b/fractal_server/app/routes/api/v2/task.py @@ -227,13 +227,12 @@ async def create_task( await _verify_non_duplication_user_constraint( db=db, pkg_name=pkg_name, user_id=user.id, version=db_task.version ) - if user_group_id is not None: - await _verify_non_duplication_group_constraint( - db=db, - pkg_name=pkg_name, - user_group_id=user_group_id, - version=db_task.version, - ) + await _verify_non_duplication_group_constraint( + db=db, + pkg_name=pkg_name, + user_group_id=user_group_id, + version=db_task.version, + ) db_task_group = TaskGroupV2( user_id=user.id, user_group_id=user_group_id, diff --git a/fractal_server/app/routes/api/v2/task_group.py b/fractal_server/app/routes/api/v2/task_group.py index dc76826e4b..a586582d89 100644 --- a/fractal_server/app/routes/api/v2/task_group.py +++ b/fractal_server/app/routes/api/v2/task_group.py @@ -125,13 +125,12 @@ async def patch_task_group( user_id=user.id, db=db, ) - if task_group_update.user_group_id is not None: - await _verify_non_duplication_group_constraint( - db=db, - pkg_name=task_group.pkg_name, - version=task_group.version, - user_group_id=task_group_update.user_group_id, - ) + await _verify_non_duplication_group_constraint( + db=db, + pkg_name=task_group.pkg_name, + version=task_group.version, + user_group_id=task_group_update.user_group_id, + ) for key, value in task_group_update.dict(exclude_unset=True).items(): if (key == "user_group_id") and (value is not None): await _verify_user_belongs_to_group( diff --git a/tests/v2/03_api/test_unit_aux_functions_tasks.py b/tests/v2/03_api/test_unit_aux_functions_tasks.py index aab606fa71..81cef34a5d 100644 --- a/tests/v2/03_api/test_unit_aux_functions_tasks.py +++ b/tests/v2/03_api/test_unit_aux_functions_tasks.py @@ -14,9 +14,6 @@ from fractal_server.app.routes.api.v2._aux_functions_tasks import ( _get_task_read_access, ) -from fractal_server.app.routes.api.v2._aux_functions_tasks import ( - _verify_non_duplication_group_constraint, -) from fractal_server.app.security import FRACTAL_DEFAULT_GROUP_NAME @@ -153,14 +150,3 @@ async def test_get_task_require_active(db, task_factory_v2): await _get_task_read_access( task_id=task.id, user_id=user.id, db=db, require_active=True ) - - -async def test_unit_verify_non_duplication_group_constraint(db): - with pytest.raises(HTTPException): - # fail because `user_group_id=None` - await _verify_non_duplication_group_constraint( - db=db, - user_group_id=None, - pkg_name="foo", - version=None, - ) From fa2b907c9f7136698aac77191e0ce9f9d0947466 Mon Sep 17 00:00:00 2001 From: Yuri Chiucconi Date: Wed, 9 Oct 2024 14:35:12 +0200 Subject: [PATCH 11/11] changelog [skip ci] --- CHANGELOG.md | 7 ++++++- fractal_server/app/routes/api/v2/_aux_functions_tasks.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f38375e0c7..719c8e445c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,14 @@ into pre-release sections below. > WARNING: This release requires running `fractalctl update-db-data` (after > `fractalctl set-db`). +# 2.7.0a4 (unreleased) + +* API: + * Enforce non-duplication constraints on `TaskGroupV2` (\#1865). + # 2.7.0a3 -* API +* API: * Introduce additional intormation in PATCH-workflow endpoint, concerning non-active or non-accessible tasks (\#1868, \#1869). # 2.7.0a2 diff --git a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py index b571eb35b4..d50124da88 100644 --- a/fractal_server/app/routes/api/v2/_aux_functions_tasks.py +++ b/fractal_server/app/routes/api/v2/_aux_functions_tasks.py @@ -219,7 +219,7 @@ async def _verify_non_duplication_user_constraint( select(TaskGroupV2) .where(TaskGroupV2.user_id == user_id) .where(TaskGroupV2.pkg_name == pkg_name) - .where(TaskGroupV2.version == version) # FIXME test with None + .where(TaskGroupV2.version == version) ) res = await db.execute(stm) duplicate = res.scalars().all()