From 7288b3890fec8acf66b5193697153556cb19b44a Mon Sep 17 00:00:00 2001 From: belthlemar Date: Thu, 19 Sep 2024 11:12:53 +0200 Subject: [PATCH] refactor --- antarest/core/exceptions.py | 6 +-- antarest/study/service.py | 30 +++++++---- antarest/study/web/raw_studies_blueprint.py | 4 +- .../test_fetch_raw_data.py | 48 +++++++++++++++++ tests/integration/test_integration.py | 53 ------------------- 5 files changed, 74 insertions(+), 67 deletions(-) diff --git a/antarest/core/exceptions.py b/antarest/core/exceptions.py index 46fd6e38a0..af5639b5ff 100644 --- a/antarest/core/exceptions.py +++ b/antarest/core/exceptions.py @@ -343,14 +343,14 @@ def __init__(self, is_variant: bool) -> None: super().__init__(HTTPStatus.EXPECTATION_FAILED, "Upgrade not supported for parent of variants") -class FileOrFolderDeletionFailed(HTTPException): +class FileDeletionNotAllowed(HTTPException): """ - Exception raised when deleting a file or a folder inside the User folder. + Exception raised when deleting a file or a folder which isn't inside the 'User' folder. """ def __init__(self, message: str) -> None: msg = f"Raw deletion failed because {message}" - super().__init__(HTTPStatus.EXPECTATION_FAILED, msg) + super().__init__(HTTPStatus.FORBIDDEN, msg) class ReferencedObjectDeletionNotAllowed(HTTPException): diff --git a/antarest/study/service.py b/antarest/study/service.py index e525660a58..dea1c83d85 100644 --- a/antarest/study/service.py +++ b/antarest/study/service.py @@ -34,7 +34,7 @@ BadEditInstructionException, ChildNotFoundError, CommandApplicationError, - FileOrFolderDeletionFailed, + FileDeletionNotAllowed, IncorrectPathError, NotAManagedStudyException, ReferencedObjectDeletionNotAllowed, @@ -2642,18 +2642,30 @@ def asserts_no_thermal_in_binding_constraints( raise ReferencedObjectDeletionNotAllowed(cluster_id, binding_ids, object_type="Cluster") def delete_file_or_folder(self, study_id: str, path: str, current_user: JWTUser) -> None: + """ + Deletes a file or a folder of the study. + The data must be located inside the 'User' folder. + Also, it can not be inside the 'expansion' folder. + + Args: + study_id: UUID of the concerned study + path: Path corresponding to the resource to be deleted + current_user: User that called the endpoint + + Raises: + FileDeletionNotAllowed: if the path does not comply with the above rules + """ study = self.get_study(study_id) assert_permission(current_user, study, StudyPermissionType.WRITE) + url = [item for item in path.split("/") if item] - if len(url) < 2: - raise FileOrFolderDeletionFailed(f"the given path cannot be treated: {path}") - if url[0] == "user" and url[1] == "expansion": - raise FileOrFolderDeletionFailed("you cannot delete a file/folder inside expansion folder") + if len(url) < 2 or url[0] != "user": + raise FileDeletionNotAllowed(f"the targeted data isn't inside the 'User' folder: {path}") + if url[1] == "expansion": + raise FileDeletionNotAllowed(f"you cannot delete a file/folder inside 'expansion' folder : {path}") + study_tree = self.storage_service.raw_study_service.get_raw(study, True).tree.build() try: study_tree["user"].delete(url[1:]) except ChildNotFoundError as e: - raise FileOrFolderDeletionFailed( - f"the file doesn't exist or you're not inside the 'User' folder: {e.detail}" - ) - # todo: I don't really like the way exception are handled. I would prefer having a FORBIDDEN if so and an 404 otherwise. + raise FileDeletionNotAllowed(f"the given path doesn't exist: {e.detail}") diff --git a/antarest/study/web/raw_studies_blueprint.py b/antarest/study/web/raw_studies_blueprint.py index 41bbc21e27..4efd141d6d 100644 --- a/antarest/study/web/raw_studies_blueprint.py +++ b/antarest/study/web/raw_studies_blueprint.py @@ -188,12 +188,12 @@ def get_study( @bp.delete( "/studies/{uuid}/raw", tags=[APITag.study_raw_data], - summary="Delete files or folders from Study inside User folder", + summary="Delete files or folders located inside the 'User' folder", response_model=None, ) def delete_file( uuid: str, - path: str = Param("/", examples=get_path_examples()), # type: ignore + path: str = Param("/", examples=["user/wind_solar/synthesis_windSolar.xlsx"]), # type: ignore current_user: JWTUser = Depends(auth.get_current_user), ) -> t.Any: uuid = sanitize_uuid(uuid) diff --git a/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py b/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py index 2b5fe6de4a..8b294cadc5 100644 --- a/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py +++ b/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py @@ -264,3 +264,51 @@ def test_get_study( headers=headers, ) assert res.status_code == 200, f"Error for path={path} and depth={depth}" + + +def test_delete_raw(client: TestClient, user_access_token: str, internal_study_id: str) -> None: + client.headers = {"Authorization": f"Bearer {user_access_token}"} + + # ============================= + # SET UP + NOMINAL CASES + # ============================= + + content = io.BytesIO(b"This is the end!") + file_1_path = "user/file_1.txt" + file_2_path = "user/folder/file_2.txt" + for f in [file_1_path, file_2_path]: + # Creates a file / folder inside user folder. + res = client.put( + f"/v1/studies/{internal_study_id}/raw", params={"path": f, "create_missing": True}, files={"file": content} + ) + assert res.status_code == 204, res.json() + + # Deletes the file / folder + res = client.delete(f"/v1/studies/{internal_study_id}/raw?path={f}") + assert res.status_code == 200 + # Asserts it doesn't exist anymore + res = client.get(f"/v1/studies/{internal_study_id}/raw?path={f}") + assert res.status_code == 404 + assert "not a child of" in res.json()["description"] + + # ============================= + # ERRORS + # ============================= + + # try to delete expansion folder + res = client.delete(f"/v1/studies/{internal_study_id}/raw?path=/user/expansion") + assert res.status_code == 403 + assert res.json()["exception"] == "FileDeletionNotAllowed" + assert "you cannot delete a file/folder inside 'expansion' folder" in res.json()["description"] + + # try to delete a file which isn't inside the 'User' folder + res = client.delete(f"/v1/studies/{internal_study_id}/raw?path=/input/thermal") + assert res.status_code == 403 + assert res.json()["exception"] == "FileDeletionNotAllowed" + assert "the targeted data isn't inside the 'User' folder" in res.json()["description"] + + # With a path that doesn't exist + res = client.delete(f"/v1/studies/{internal_study_id}/raw?path=user/fake_folder/fake_file.txt") + assert res.status_code == 403 + assert res.json()["exception"] == "FileDeletionNotAllowed" + assert "the given path doesn't exist" in res.json()["description"] diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index eeace56032..a136184969 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -1913,56 +1913,3 @@ def test_links_deletion_with_binding_constraints( # delete the link res = client.delete(f"/v1/studies/{internal_study_id}/links/area_1/area_2") assert res.status_code == 200, res.json() - - -def test_delete_raw(client: TestClient, user_access_token: str, internal_study_id: str) -> None: - client.headers = {"Authorization": f"Bearer {user_access_token}"} - - # ============================= - # SET UP + NOMINAL CASES - # ============================= - - content = io.BytesIO(b"This is the end!") - file_1_path = "user/file_1.txt" - file_2_path = "user/folder/file_2.txt" - for f in [file_1_path, file_2_path]: - # Creates a file / folder inside user folder. - res = client.put( - f"/v1/studies/{internal_study_id}/raw", - params={"path": f, "create_missing": True}, - files={"file": content} - ) - assert res.status_code == 204, res.json() - - # Deletes the file / folder - res = client.delete(f"/v1/studies/{internal_study_id}/raw?path={f}") - assert res.status_code == 200 - # Asserts it doesn't exist anymore - res = client.get(f"/v1/studies/{internal_study_id}/raw?path={f}") - assert res.status_code == 404 - assert "not a child of User" in res.json()["description"] - - # ============================= - # ERRORS - # ============================= - - # try to delete expansion folder - res = client.delete(f"/v1/studies/{internal_study_id}/raw?path=/user/expansion") - assert res.status_code == 417 - assert res.json()["exception"] == "FileOrFolderDeletionFailed" - assert "you cannot delete a file/folder inside expansion folder" in res.json()["description"] - # try to delete a file which isn't inside the 'User' folder - res = client.delete(f"/v1/studies/{internal_study_id}/raw?path=/input/thermal") - assert res.status_code == 417 - assert res.json()["exception"] == "FileOrFolderDeletionFailed" - assert "the file doesn't exist or you're not inside the 'User' folder" in res.json()["description"] - # With an empty path - res = client.delete(f"/v1/studies/{internal_study_id}/raw?path=fake_path") - assert res.status_code == 417 - assert res.json()["exception"] == "FileOrFolderDeletionFailed" - assert "the given path cannot be treated" in res.json()["description"] - # With a path that doesn't exist - res = client.delete(f"/v1/studies/{internal_study_id}/raw?path=fake_folder/fake_file") - assert res.status_code == 417 - assert res.json()["exception"] == "FileOrFolderDeletionFailed" - assert "the file doesn't exist or you're not inside the 'User' folder" in res.json()["description"]