From 850335ca4f6eb736e7cc2002f2ed716ea1ba7191 Mon Sep 17 00:00:00 2001 From: Stefan Verhoeven Date: Tue, 30 Jan 2024 15:33:57 +0100 Subject: [PATCH 1/5] Add POST /job/{jobid}/name route Refs https://github.com/i-VRESSE/haddock3-webapp/issues/37 --- src/bartender/db/dao/job_dao.py | 17 +++++++++++++++++ src/bartender/web/api/job/views.py | 27 +++++++++++++++++++++++++++ tests/web/test_job.py | 26 ++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) diff --git a/src/bartender/db/dao/job_dao.py b/src/bartender/db/dao/job_dao.py index f18e295..6c25abe 100644 --- a/src/bartender/db/dao/job_dao.py +++ b/src/bartender/db/dao/job_dao.py @@ -118,5 +118,22 @@ async def update_internal_job_id( job.destination = destination await self.session.commit() + async def set_job_name(self, jobid: int, user: str, name: str) -> None: + """Set name of a job. + + Args: + jobid: name of job instance. + user: Which user to get jobs from. + name: new name of job instance. + + Raises: + IndexError: if job was not found or user is not the owner. + """ + job = await self.session.get(Job, jobid) + if job is None or job.submitter != user: + raise IndexError("Job not found") + job.name = name + await self.session.commit() + CurrentJobDAO = Annotated[JobDAO, Depends()] diff --git a/src/bartender/web/api/job/views.py b/src/bartender/web/api/job/views.py index 6d88856..4db1798 100644 --- a/src/bartender/web/api/job/views.py +++ b/src/bartender/web/api/job/views.py @@ -518,3 +518,30 @@ async def run_interactive_app( status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=exc.message, ) from exc + + +@router.post("/{jobid}/name") +async def rename_job_name( + jobid: int, + job_dao: CurrentJobDAO, + user: CurrentUser, + name: str, +) -> None: + """Rename the name of a job. + + Args: + jobid: The job identifier. + job_dao: The job DAO. + user: The current user. + name: The new name of the job. + + Raises: + HTTPException: When job is not found. Or when user is not owner of job. + """ + try: + await job_dao.set_job_name(jobid, user.username, name) + except IndexError as exc: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Job not found", + ) from exc diff --git a/tests/web/test_job.py b/tests/web/test_job.py index 01ea178..7071533 100644 --- a/tests/web/test_job.py +++ b/tests/web/test_job.py @@ -170,6 +170,7 @@ async def test_retrieve_job_new( assert job == expected +@pytest.mark.anyio async def test_retrieve_job_unknown( fastapi_app: FastAPI, client: AsyncClient, @@ -192,6 +193,7 @@ async def test_retrieve_job_stdout_unknown( assert response.status_code == status.HTTP_404_NOT_FOUND +@pytest.mark.anyio async def test_retrieve_job_queued2running( dbsession: AsyncSession, current_user: User, @@ -219,6 +221,7 @@ async def test_retrieve_job_queued2running( download_mock.assert_not_called() +@pytest.mark.anyio async def test_retrieve_job_completed( dbsession: AsyncSession, current_user: User, @@ -246,6 +249,7 @@ async def test_retrieve_job_completed( download_mock.assert_not_called() +@pytest.mark.anyio async def test_retrieve_job_running2ok( dbsession: AsyncSession, current_user: User, @@ -287,6 +291,7 @@ async def test_retrieve_job_running2ok( download_mock.assert_called_once() +@pytest.mark.anyio async def test_retrieve_jobs_queued2running( dbsession: AsyncSession, current_user: User, @@ -316,6 +321,7 @@ async def test_retrieve_jobs_queued2running( download_mock.assert_not_called() +@pytest.mark.anyio async def test_retrieve_jobs_running2staging_out( dbsession: AsyncSession, current_user: User, @@ -764,3 +770,23 @@ async def test_run_interactive_app_invalid_requestbody( assert response.json() == { "detail": "Additional properties are not allowed ('foo' was unexpected)", } + + +@pytest.mark.anyio +async def test_rename_job_name( + fastapi_app: FastAPI, + client: AsyncClient, + auth_headers: Dict[str, str], + mock_ok_job: int, +) -> None: + url = fastapi_app.url_path_for("rename_job_name", jobid=str(mock_ok_job)) + response = await client.post(url, headers=auth_headers, params={"name": "newname"}) + + assert response.status_code == status.HTTP_200_OK + + url = fastapi_app.url_path_for("retrieve_job", jobid=str(mock_ok_job)) + response2 = await client.get(url, headers=auth_headers) + assert response2.status_code == status.HTTP_200_OK + + renamed_job = response2.json() + assert renamed_job["name"] == "newname" From c0f613fb75a49cccbbb2490d5582c4678457da44 Mon Sep 17 00:00:00 2001 From: sverhoeven Date: Thu, 1 Feb 2024 10:02:43 +0100 Subject: [PATCH 2/5] Accept new name in body instead of query + add max length --- src/bartender/db/models/job_model.py | 6 ++++-- src/bartender/web/api/job/views.py | 14 +++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/bartender/db/models/job_model.py b/src/bartender/db/models/job_model.py index 2116bf6..fe6ae80 100644 --- a/src/bartender/db/models/job_model.py +++ b/src/bartender/db/models/job_model.py @@ -32,6 +32,8 @@ CompletedStates: set[State] = {"ok", "error"} +MAX_LENGTH_NAME = 200 + class Job(Base): """Model for the Job.""" @@ -39,9 +41,9 @@ class Job(Base): __tablename__ = "job" id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) - name: Mapped[str] = mapped_column(String(length=200)) # noqa: WPS432 + name: Mapped[str] = mapped_column(String(length=MAX_LENGTH_NAME)) application: Mapped[str] = mapped_column( - String(length=200), # noqa: WPS432 + String(length=MAX_LENGTH_NAME), ) state: Mapped[State] = mapped_column( String(length=20), # noqa: WPS432 diff --git a/src/bartender/web/api/job/views.py b/src/bartender/web/api/job/views.py index 4db1798..a1cfc39 100644 --- a/src/bartender/web/api/job/views.py +++ b/src/bartender/web/api/job/views.py @@ -2,7 +2,15 @@ from pathlib import Path from typing import Annotated, Literal, Optional, Tuple, Type, Union -from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query, Request +from fastapi import ( + APIRouter, + BackgroundTasks, + Body, + Depends, + HTTPException, + Query, + Request, +) from fastapi.responses import FileResponse, PlainTextResponse from fs.copy import copy_fs from fs.osfs import OSFS @@ -18,7 +26,7 @@ from bartender.config import CurrentConfig, InteractiveApplicationConfiguration from bartender.context import CurrentContext, get_job_root_dir from bartender.db.dao.job_dao import CurrentJobDAO -from bartender.db.models.job_model import CompletedStates, Job +from bartender.db.models.job_model import MAX_LENGTH_NAME, CompletedStates, Job from bartender.filesystem.walk_dir import DirectoryItem, walk_dir from bartender.filesystems.queue import CurrentFileOutStagingQueue from bartender.web.api.job.interactive_apps import InteractiveAppResult, run @@ -525,7 +533,7 @@ async def rename_job_name( jobid: int, job_dao: CurrentJobDAO, user: CurrentUser, - name: str, + name: Annotated[str, Body(max_length=MAX_LENGTH_NAME)], ) -> None: """Rename the name of a job. From 0f0d8abb8f90749b41b004ab6de643c4ffbfe9cf Mon Sep 17 00:00:00 2001 From: sverhoeven Date: Thu, 1 Feb 2024 10:07:54 +0100 Subject: [PATCH 3/5] Fix test --- tests/web/test_job.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/web/test_job.py b/tests/web/test_job.py index 7071533..02f7780 100644 --- a/tests/web/test_job.py +++ b/tests/web/test_job.py @@ -780,7 +780,7 @@ async def test_rename_job_name( mock_ok_job: int, ) -> None: url = fastapi_app.url_path_for("rename_job_name", jobid=str(mock_ok_job)) - response = await client.post(url, headers=auth_headers, params={"name": "newname"}) + response = await client.post(url, headers=auth_headers, json="newname") assert response.status_code == status.HTTP_200_OK From 2b28e44496b2842033b873f01afc6d0fc74bfb37 Mon Sep 17 00:00:00 2001 From: sverhoeven Date: Thu, 1 Feb 2024 11:44:13 +0100 Subject: [PATCH 4/5] Less magic numbers --- src/bartender/db/models/job_model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bartender/db/models/job_model.py b/src/bartender/db/models/job_model.py index fe6ae80..14d4151 100644 --- a/src/bartender/db/models/job_model.py +++ b/src/bartender/db/models/job_model.py @@ -52,10 +52,10 @@ class Job(Base): submitter: Mapped[str] = mapped_column(String(length=254)) # noqa: WPS432 # Identifier for job used by the scheduler internal_id: Mapped[Optional[str]] = mapped_column( - String(length=200), # noqa: WPS432 + String(length=MAX_LENGTH_NAME), ) destination: Mapped[Optional[str]] = mapped_column( - String(length=200), # noqa: WPS432 + String(length=MAX_LENGTH_NAME), ) created_on: Mapped[datetime] = mapped_column( DateTime(timezone=True), From 3a248697e58a8b96e59c6c424dfdfb5dee8f3f4c Mon Sep 17 00:00:00 2001 From: sverhoeven Date: Thu, 1 Feb 2024 11:56:47 +0100 Subject: [PATCH 5/5] More tests + min length 1 --- src/bartender/web/api/job/views.py | 2 +- tests/web/test_job.py | 43 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/bartender/web/api/job/views.py b/src/bartender/web/api/job/views.py index a1cfc39..e28ea4f 100644 --- a/src/bartender/web/api/job/views.py +++ b/src/bartender/web/api/job/views.py @@ -533,7 +533,7 @@ async def rename_job_name( jobid: int, job_dao: CurrentJobDAO, user: CurrentUser, - name: Annotated[str, Body(max_length=MAX_LENGTH_NAME)], + name: Annotated[str, Body(max_length=MAX_LENGTH_NAME, min_length=1)], ) -> None: """Rename the name of a job. diff --git a/tests/web/test_job.py b/tests/web/test_job.py index 02f7780..6b3619e 100644 --- a/tests/web/test_job.py +++ b/tests/web/test_job.py @@ -790,3 +790,46 @@ async def test_rename_job_name( renamed_job = response2.json() assert renamed_job["name"] == "newname" + + +@pytest.mark.anyio +async def test_rename_job_name_too_short( + fastapi_app: FastAPI, + client: AsyncClient, + auth_headers: Dict[str, str], + mock_ok_job: int, +) -> None: + jobid = str(mock_ok_job) + name = "" + url = fastapi_app.url_path_for("rename_job_name", jobid=jobid) + response = await client.post(url, headers=auth_headers, json=name) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + expected = { + "detail": [ + { + "ctx": {"limit_value": 1}, + "loc": ["body"], + "msg": "ensure this value has at least 1 characters", + "type": "value_error.any_str.min_length", + }, + ], + } + assert response.json() == expected + + +@pytest.mark.anyio +async def test_rename_job_name_wrong_user( + fastapi_app: FastAPI, + client: AsyncClient, + second_user_token: str, + mock_ok_job: int, +) -> None: + jobid = str(mock_ok_job) + name = "newname" + url = fastapi_app.url_path_for("rename_job_name", jobid=jobid) + headers = {"Authorization": f"Bearer {second_user_token}"} + response = await client.post(url, headers=headers, json=name) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json() == {"detail": "Job not found"}