From 70e7899d5384043a18c36576f3deaaa34de15518 Mon Sep 17 00:00:00 2001 From: belthlemar Date: Thu, 19 Sep 2024 16:42:58 +0200 Subject: [PATCH 1/7] feat(debug): allow files and folder deletion inside debug view --- antarest/core/exceptions.py | 10 ++++ antarest/study/service.py | 30 ++++++++++++ antarest/study/web/raw_studies_blueprint.py | 15 ++++++ .../test_fetch_raw_data.py | 48 +++++++++++++++++++ 4 files changed, 103 insertions(+) diff --git a/antarest/core/exceptions.py b/antarest/core/exceptions.py index dddcda014c..af5639b5ff 100644 --- a/antarest/core/exceptions.py +++ b/antarest/core/exceptions.py @@ -343,6 +343,16 @@ def __init__(self, is_variant: bool) -> None: super().__init__(HTTPStatus.EXPECTATION_FAILED, "Upgrade not supported for parent of variants") +class FileDeletionNotAllowed(HTTPException): + """ + 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.FORBIDDEN, msg) + + class ReferencedObjectDeletionNotAllowed(HTTPException): """ Exception raised when a binding constraint is not allowed to be deleted because it references diff --git a/antarest/study/service.py b/antarest/study/service.py index 2bff77b1ec..662f20670c 100644 --- a/antarest/study/service.py +++ b/antarest/study/service.py @@ -35,6 +35,7 @@ BadEditInstructionException, ChildNotFoundError, CommandApplicationError, + FileDeletionNotAllowed, IncorrectPathError, NotAManagedStudyException, ReferencedObjectDeletionNotAllowed, @@ -2640,3 +2641,32 @@ def asserts_no_thermal_in_binding_constraints( if ref_bcs: binding_ids = [bc.id for bc in ref_bcs] 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 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 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 dc8554e55d..738d2aa3ac 100644 --- a/antarest/study/web/raw_studies_blueprint.py +++ b/antarest/study/web/raw_studies_blueprint.py @@ -191,6 +191,21 @@ def get_study( ).encode("utf-8") return Response(content=json_response, media_type="application/json") + @bp.delete( + "/studies/{uuid}/raw", + tags=[APITag.study_raw_data], + summary="Delete files or folders located inside the 'User' folder", + response_model=None, + ) + def delete_file( + uuid: str, + 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) + logger.info(f"Deleting some data for study {uuid}", extra={"user": current_user.id}) + study_service.delete_file_or_folder(uuid, path, current_user) + @bp.get( "/studies/{uuid}/areas/aggregate/mc-ind/{output_id}", tags=[APITag.study_raw_data], 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"] From 20113b3f0eb081523ceaa5a33f9986fdb98ca2a7 Mon Sep 17 00:00:00 2001 From: Samir Kamal <1954121+skamril@users.noreply.github.com> Date: Fri, 20 Sep 2024 16:43:35 +0200 Subject: [PATCH 2/7] feat(ui-debug): allow to delete file --- .../Singlestudy/explore/Debug/Data/Folder.tsx | 113 +++++++++++++++--- webapp/src/services/api/studies/raw/index.ts | 15 ++- webapp/src/services/api/studies/raw/types.ts | 5 + 3 files changed, 118 insertions(+), 15 deletions(-) diff --git a/webapp/src/components/App/Singlestudy/explore/Debug/Data/Folder.tsx b/webapp/src/components/App/Singlestudy/explore/Debug/Data/Folder.tsx index c565a6a682..4de47da44b 100644 --- a/webapp/src/components/App/Singlestudy/explore/Debug/Data/Folder.tsx +++ b/webapp/src/components/App/Singlestudy/explore/Debug/Data/Folder.tsx @@ -1,12 +1,18 @@ import { Divider, + IconButton, List, + ListItem, ListItemButton, ListItemIcon, ListItemText, ListSubheader, + Menu, + MenuItem, } from "@mui/material"; import FolderIcon from "@mui/icons-material/Folder"; +import MoreVertIcon from "@mui/icons-material/MoreVert"; +import DeleteIcon from "@mui/icons-material/Delete"; import { getFileIcon, getFileType, @@ -14,13 +20,16 @@ import { type DataCompProps, isFolder, } from "../utils"; -import { Fragment } from "react"; +import { Fragment, useState } from "react"; import EmptyView from "../../../../../common/page/SimpleContent"; import { useTranslation } from "react-i18next"; import { Filename, Menubar } from "./styles"; import UploadFileButton from "../../../../../common/buttons/UploadFileButton"; import ConfirmationDialog from "../../../../../common/dialogs/ConfirmationDialog"; import useConfirm from "../../../../../../hooks/useConfirm"; +import { deleteFile } from "../../../../../../services/api/studies/raw"; +import useEnqueueErrorSnackbar from "../../../../../../hooks/useEnqueueErrorSnackbar"; +import { toError } from "../../../../../../utils/fnUtils"; function Folder(props: DataCompProps) { const { @@ -35,6 +44,13 @@ function Folder(props: DataCompProps) { const { t } = useTranslation(); const replaceFile = useConfirm(); + const removeFile = useConfirm(); + const [menuData, setMenuData] = useState(null); + const enqueueErrorSnackbar = useEnqueueErrorSnackbar(); + const treeFolder = treeData as TreeFolder; const list = Object.entries(treeFolder); @@ -53,6 +69,27 @@ function Folder(props: DataCompProps) { } }; + const handleMenuClose = () => { + setMenuData(null); + }; + + const handleDeleteClick = () => { + handleMenuClose(); + + removeFile.showConfirm().then((confirm) => { + const filePath = menuData?.filePath; + if (confirm && filePath) { + deleteFile({ studyId, path: filePath }) + .then((res) => { + reloadTreeData(); + }) + .catch((err) => { + enqueueErrorSnackbar("Delete failed", toError(err)); + }); + } + }); + }; + //////////////////////////////////////////////////////////////// // JSX //////////////////////////////////////////////////////////////// @@ -94,21 +131,45 @@ function Folder(props: DataCompProps) { return ( - - setSelectedFile({ - fileType, - filename, - filePath: `${filePath}/${filename}`, - treeData: data, - }) + { + setMenuData({ + anchorEl: event.currentTarget, + filePath: `${filePath}/${filename}`, + }); + }} + > + + } + disablePadding > - - - - - + + setSelectedFile({ + fileType, + filename, + filePath: `${filePath}/${filename}`, + treeData: data, + }) + } + > + + + + + + {!isLast && } ); @@ -117,6 +178,18 @@ function Folder(props: DataCompProps) { )} + {/* Items menu */} + + + + Delete + + + {/* Confim file replacement */} {t("study.debug.folder.upload.replaceFileConfirm.message")} + {/* Confim file deletion */} + + Delete the file? + ); } diff --git a/webapp/src/services/api/studies/raw/index.ts b/webapp/src/services/api/studies/raw/index.ts index f7350f1015..35240f1e39 100644 --- a/webapp/src/services/api/studies/raw/index.ts +++ b/webapp/src/services/api/studies/raw/index.ts @@ -1,9 +1,14 @@ import client from "../../client"; -import type { DownloadMatrixParams, ImportFileParams } from "./types"; +import type { + DeleteFileParams, + DownloadMatrixParams, + ImportFileParams, +} from "./types"; export async function downloadMatrix(params: DownloadMatrixParams) { const { studyId, ...queryParams } = params; const url = `v1/studies/${studyId}/raw/download`; + const res = await client.get(url, { params: queryParams, responseType: "blob", @@ -16,6 +21,7 @@ export async function importFile(params: ImportFileParams) { const { studyId, file, onUploadProgress, ...queryParams } = params; const url = `v1/studies/${studyId}/raw`; const body = { file }; + await client.putForm(url, body, { params: { ...queryParams, @@ -24,3 +30,10 @@ export async function importFile(params: ImportFileParams) { onUploadProgress, }); } + +export async function deleteFile(params: DeleteFileParams) { + const { studyId, path } = params; + const url = `v1/studies/${studyId}/raw`; + + await client.delete(url, { params: { path } }); +} diff --git a/webapp/src/services/api/studies/raw/types.ts b/webapp/src/services/api/studies/raw/types.ts index 2c62844115..a6a6a251be 100644 --- a/webapp/src/services/api/studies/raw/types.ts +++ b/webapp/src/services/api/studies/raw/types.ts @@ -16,3 +16,8 @@ export interface ImportFileParams { createMissing?: boolean; onUploadProgress?: AxiosRequestConfig["onUploadProgress"]; } + +export interface DeleteFileParams { + studyId: StudyMetadata["id"]; + path: string; +} From e0fd6b1b758504efc862fa05ad6509905f7cb83f Mon Sep 17 00:00:00 2001 From: belthlemar Date: Mon, 23 Sep 2024 11:44:30 +0200 Subject: [PATCH 3/7] fix(cache): update cache after deleting file --- antarest/study/service.py | 7 ++++++- .../test_fetch_raw_data.py | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/antarest/study/service.py b/antarest/study/service.py index 662f20670c..25f73e7469 100644 --- a/antarest/study/service.py +++ b/antarest/study/service.py @@ -48,7 +48,7 @@ ) from antarest.core.filetransfer.model import FileDownloadTaskDTO from antarest.core.filetransfer.service import FileTransferManager -from antarest.core.interfaces.cache import ICache +from antarest.core.interfaces.cache import CacheConstants, ICache from antarest.core.interfaces.eventbus import Event, EventType, IEventBus from antarest.core.jwt import DEFAULT_ADMIN_USER, JWTGroup, JWTUser from antarest.core.model import JSON, SUB_JSON, PermissionInfo, PublicMode, StudyPermissionType @@ -2670,3 +2670,8 @@ def delete_file_or_folder(self, study_id: str, path: str, current_user: JWTUser) study_tree["user"].delete(url[1:]) except ChildNotFoundError as e: raise FileDeletionNotAllowed(f"the given path doesn't exist: {e.detail}") + + # update cache + cache_id = f"{CacheConstants.RAW_STUDY}/{study.id}" + updated_tree = self.storage_service.raw_study_service.get_raw(study, False).tree.get() + self.storage_service.get_storage(study).cache.put(cache_id, updated_tree) # type: ignore 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 8b294cadc5..b36b33b720 100644 --- a/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py +++ b/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py @@ -276,7 +276,8 @@ def test_delete_raw(client: TestClient, user_access_token: str, internal_study_i 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]: + file_3_path = "user/folder_2/file_3.txt" + for f in [file_1_path, file_2_path, file_3_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} @@ -284,6 +285,8 @@ def test_delete_raw(client: TestClient, user_access_token: str, internal_study_i assert res.status_code == 204, res.json() # Deletes the file / folder + if f == file_2_path: + f = "user/folder" res = client.delete(f"/v1/studies/{internal_study_id}/raw?path={f}") assert res.status_code == 200 # Asserts it doesn't exist anymore @@ -291,6 +294,18 @@ def test_delete_raw(client: TestClient, user_access_token: str, internal_study_i assert res.status_code == 404 assert "not a child of" in res.json()["description"] + # checks debug view + res = client.get(f"/v1/studies/{internal_study_id}/raw?path=&depth=-1") + assert res.status_code == 200 + tree = res.json()["user"] + if f == file_3_path: + # asserts the folder that wasn't deleted is still here. + assert list(tree.keys()) == ["expansion", "folder_2"] + assert tree["folder_2"] == {} + else: + # asserts deleted files cannot be seen inside the debug view + assert list(tree.keys()) == ["expansion"] + # ============================= # ERRORS # ============================= From 0afd210520fbbd92d5bb1d39484399451486bfc8 Mon Sep 17 00:00:00 2001 From: belthlemar Date: Tue, 24 Sep 2024 14:49:06 +0200 Subject: [PATCH 4/7] refactor(raw): resolve easy comments --- antarest/study/service.py | 9 +++++---- antarest/study/web/raw_studies_blueprint.py | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/antarest/study/service.py b/antarest/study/service.py index 25f73e7469..7c7617dbf6 100644 --- a/antarest/study/service.py +++ b/antarest/study/service.py @@ -2665,13 +2665,14 @@ def delete_file_or_folder(self, study_id: str, path: str, current_user: JWTUser) 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() + study_tree = self.storage_service.raw_study_service.get_raw(study, True).tree + builded_tree = study_tree.build() try: - study_tree["user"].delete(url[1:]) + builded_tree["user"].delete(url[1:]) except ChildNotFoundError as e: - raise FileDeletionNotAllowed(f"the given path doesn't exist: {e.detail}") + raise FileDeletionNotAllowed("the given path doesn't exist") from e # update cache cache_id = f"{CacheConstants.RAW_STUDY}/{study.id}" - updated_tree = self.storage_service.raw_study_service.get_raw(study, False).tree.get() + updated_tree = study_tree.get() self.storage_service.get_storage(study).cache.put(cache_id, updated_tree) # type: ignore diff --git a/antarest/study/web/raw_studies_blueprint.py b/antarest/study/web/raw_studies_blueprint.py index 738d2aa3ac..177f071668 100644 --- a/antarest/study/web/raw_studies_blueprint.py +++ b/antarest/study/web/raw_studies_blueprint.py @@ -203,7 +203,7 @@ def delete_file( current_user: JWTUser = Depends(auth.get_current_user), ) -> t.Any: uuid = sanitize_uuid(uuid) - logger.info(f"Deleting some data for study {uuid}", extra={"user": current_user.id}) + logger.info(f"Deleting path {path} inside study {uuid}", extra={"user": current_user.id}) study_service.delete_file_or_folder(uuid, path, current_user) @bp.get( From 1513a10a8577bac996d50be7b3432af88a2fd439 Mon Sep 17 00:00:00 2001 From: belthlemar Date: Tue, 24 Sep 2024 15:07:09 +0200 Subject: [PATCH 5/7] refactor(raw): use registered files --- antarest/study/service.py | 11 +++++++---- .../raw_studies_blueprint/test_fetch_raw_data.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/antarest/study/service.py b/antarest/study/service.py index 7c7617dbf6..6ea8d7cc0f 100644 --- a/antarest/study/service.py +++ b/antarest/study/service.py @@ -131,6 +131,7 @@ from antarest.study.storage.rawstudy.model.filesystem.matrix.matrix import MatrixFrequency from antarest.study.storage.rawstudy.model.filesystem.matrix.output_series_matrix import OutputSeriesMatrix from antarest.study.storage.rawstudy.model.filesystem.raw_file_node import RawFileNode +from antarest.study.storage.rawstudy.model.filesystem.root.user.user import User from antarest.study.storage.rawstudy.raw_study_service import RawStudyService from antarest.study.storage.storage_service import StudyStorageService from antarest.study.storage.study_download_utils import StudyDownloader, get_output_variables_information @@ -2662,13 +2663,15 @@ def delete_file_or_folder(self, study_id: str, path: str, current_user: JWTUser) url = [item for item in path.split("/") if item] 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 - builded_tree = study_tree.build() + user_node = study_tree.build()["user"] + assert isinstance(user_node, User) + if url[1] in [file.filename for file in user_node.registered_files]: + raise FileDeletionNotAllowed(f"you are not allowed to delete this resource : {path}") + try: - builded_tree["user"].delete(url[1:]) + user_node.delete(url[1:]) except ChildNotFoundError as e: raise FileDeletionNotAllowed("the given path doesn't exist") from e 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 b36b33b720..8859a56469 100644 --- a/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py +++ b/tests/integration/raw_studies_blueprint/test_fetch_raw_data.py @@ -314,7 +314,7 @@ def test_delete_raw(client: TestClient, user_access_token: str, internal_study_i 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"] + assert "you are not allowed to delete this resource" 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") From f858da688713995b09996f50c8ec508a90a750a8 Mon Sep 17 00:00:00 2001 From: belthlemar Date: Tue, 24 Sep 2024 16:53:11 +0200 Subject: [PATCH 6/7] enhancement(raw): only build user folder to speed up the code --- antarest/study/service.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/antarest/study/service.py b/antarest/study/service.py index 6ea8d7cc0f..4f4e0360a0 100644 --- a/antarest/study/service.py +++ b/antarest/study/service.py @@ -2665,8 +2665,7 @@ def delete_file_or_folder(self, study_id: str, path: str, current_user: JWTUser) raise FileDeletionNotAllowed(f"the targeted data isn't inside the 'User' folder: {path}") study_tree = self.storage_service.raw_study_service.get_raw(study, True).tree - user_node = study_tree.build()["user"] - assert isinstance(user_node, User) + user_node = t.cast(User,study_tree.get_node(["user"])) if url[1] in [file.filename for file in user_node.registered_files]: raise FileDeletionNotAllowed(f"you are not allowed to delete this resource : {path}") From 033490c7bcb9fba6357c710f5420290d65df81ab Mon Sep 17 00:00:00 2001 From: belthlemar Date: Tue, 24 Sep 2024 16:53:44 +0200 Subject: [PATCH 7/7] fix typo --- antarest/study/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/antarest/study/service.py b/antarest/study/service.py index 4f4e0360a0..24cbe730f9 100644 --- a/antarest/study/service.py +++ b/antarest/study/service.py @@ -2665,7 +2665,7 @@ def delete_file_or_folder(self, study_id: str, path: str, current_user: JWTUser) raise FileDeletionNotAllowed(f"the targeted data isn't inside the 'User' folder: {path}") study_tree = self.storage_service.raw_study_service.get_raw(study, True).tree - user_node = t.cast(User,study_tree.get_node(["user"])) + user_node = t.cast(User, study_tree.get_node(["user"])) if url[1] in [file.filename for file in user_node.registered_files]: raise FileDeletionNotAllowed(f"you are not allowed to delete this resource : {path}")