diff --git a/api/apps/restful_apis/file_api.py b/api/apps/restful_apis/file_api.py index a015cd7b4a..306c64c651 100644 --- a/api/apps/restful_apis/file_api.py +++ b/api/apps/restful_apis/file_api.py @@ -24,8 +24,10 @@ from api.utils.api_utils import ( add_tenant_id_to_kwargs, get_error_argument_result, get_error_data_result, + get_json_result, get_result, ) +from common.constants import RetCode from api.utils.validation_utils import ( CreateFolderReq, DeleteFileReq, @@ -189,6 +191,16 @@ async def delete(tenant_id: str = None): if success: return get_result(data=result) else: + if isinstance(result, dict): + success_count = result.get("success_count", 0) + errors = result.get("errors", []) + return get_json_result( + code=RetCode.DATA_ERROR, + message=f"Partially deleted {success_count} files with {len(errors)} errors" + if success_count > 0 + else f"Deleted files failed with {len(errors)} errors", + data=result, + ) return get_error_data_result(message=result) except Exception as e: logging.exception(e) @@ -360,5 +372,3 @@ async def ancestors(tenant_id: str = None, file_id: str = None): except Exception as e: logging.exception(e) return get_error_data_result(message="Internal server error") - - diff --git a/api/apps/services/file_api_service.py b/api/apps/services/file_api_service.py index 700be9559f..da4df98773 100644 --- a/api/apps/services/file_api_service.py +++ b/api/apps/services/file_api_service.py @@ -211,54 +211,92 @@ async def delete_files(uid: str, file_ids: list): :param file_ids: list of file IDs to delete :return: (success, result) or (success, error_message) """ - def _delete_single_file(file): + errors: list[str] = [] + success_count = 0 + + def _delete_single_file(file) -> int: try: if file.location: settings.STORAGE_IMPL.rm(file.parent_id, file.location) except Exception as e: logging.exception(f"Fail to remove object: {file.parent_id}/{file.location}, error: {e}") + errors.append(f"Failed to remove object {file.parent_id}/{file.location}: {e}") informs = File2DocumentService.get_by_file_id(file.id) for inform in informs: doc_id = inform.document_id e, doc = DocumentService.get_by_id(doc_id) - if e and doc: - tenant_id = DocumentService.get_tenant_id(doc_id) - if tenant_id: - DocumentService.remove_document(doc, tenant_id) - File2DocumentService.delete_by_file_id(file.id) + if not e or not doc: + errors.append(f"Document not found for file {file.id}: {doc_id}") + continue - FileService.delete(file) + tenant_id = DocumentService.get_tenant_id(doc_id) + if not tenant_id: + errors.append(f"Tenant not found for document {doc_id}") + continue + + if not DocumentService.remove_document(doc, tenant_id): + errors.append(f"Failed to remove document {doc_id} for file {file.id}") + + try: + File2DocumentService.delete_by_file_id(file.id) + except Exception as e: + logging.exception(f"Fail to remove file-document relations for file {file.id}, error: {e}") + errors.append(f"Failed to remove file-document relations for file {file.id}: {e}") + + try: + FileService.delete(file) + except Exception as e: + logging.exception(f"Fail to delete file record {file.id}, error: {e}") + errors.append(f"Failed to delete file record {file.id}: {e}") + else: + return 1 + + return 0 def _delete_folder_recursive(folder, tenant_id): + deleted = 0 sub_files = FileService.list_all_files_by_parent_id(folder.id) for sub_file in sub_files: if sub_file.type == FileType.FOLDER.value: - _delete_folder_recursive(sub_file, tenant_id) + deleted += _delete_folder_recursive(sub_file, tenant_id) else: - _delete_single_file(sub_file) - FileService.delete(folder) + deleted += _delete_single_file(sub_file) + try: + FileService.delete(folder) + except Exception as e: + logging.exception(f"Fail to delete folder record {folder.id}, error: {e}") + errors.append(f"Failed to delete folder record {folder.id}: {e}") + else: + deleted += 1 + return deleted def _rm_sync(): + nonlocal success_count for file_id in file_ids: e, file = FileService.get_by_id(file_id) if not e or not file: - return False, "File or Folder not found!" + errors.append(f"File or Folder not found: {file_id}") + continue if not file.tenant_id: - return False, "Tenant not found!" + errors.append(f"Tenant not found for file {file_id}") + continue if not check_file_team_permission(file, uid): - return False, "No authorization." + errors.append(f"No authorization for file {file_id}") + continue if file.source_type == FileSource.KNOWLEDGEBASE: continue if file.type == FileType.FOLDER.value: - _delete_folder_recursive(file, uid) + success_count += _delete_folder_recursive(file, uid) continue - _delete_single_file(file) + success_count += _delete_single_file(file) - return True, True + if errors: + return False, {"success_count": success_count, "errors": errors} + return True, {"success_count": success_count} return await thread_pool_exec(_rm_sync) diff --git a/docs/references/http_api_reference.md b/docs/references/http_api_reference.md index 54eb32e917..7dfab3bdb2 100644 --- a/docs/references/http_api_reference.md +++ b/docs/references/http_api_reference.md @@ -7257,7 +7257,9 @@ Success: ```json { "code": 0, - "data": true + "data": { + "success_count": 2 + } } ``` @@ -7265,8 +7267,14 @@ Failure: ```json { - "code": 404, - "message": "File or Folder not found!" + "code": 102, + "message": "Partially deleted 1 files with 1 errors", + "data": { + "success_count": 1, + "errors": [ + "No authorization for file file1" + ] + } } ``` diff --git a/test/testcases/test_http_api/test_file_app/test_file_routes.py b/test/testcases/test_http_api/test_file_app/test_file_routes.py index 85fa264b42..e0cbe5f84d 100644 --- a/test/testcases/test_http_api/test_file_app/test_file_routes.py +++ b/test/testcases/test_http_api/test_file_app/test_file_routes.py @@ -258,7 +258,7 @@ def test_delete_files_checks_team_permission(monkeypatch): ok, message = _run(module.delete_files("tenant1", ["file1"])) assert ok is False - assert message == "No authorization." + assert message == {"success_count": 0, "errors": ["No authorization for file file1"]} @pytest.mark.p2 diff --git a/test/testcases/test_web_api/test_file_app/test_file_routes_unit.py b/test/testcases/test_web_api/test_file_app/test_file_routes_unit.py index 7e263b9325..87c37d4667 100644 --- a/test/testcases/test_web_api/test_file_app/test_file_routes_unit.py +++ b/test/testcases/test_web_api/test_file_app/test_file_routes_unit.py @@ -158,6 +158,7 @@ def _load_file_api_module(monkeypatch): api_utils_mod.get_error_argument_result = lambda message: {"code": 400, "data": None, "message": message} api_utils_mod.get_error_data_result = lambda message: {"code": 500, "data": None, "message": message} api_utils_mod.get_result = lambda data=None: {"code": 0, "data": data, "message": ""} + api_utils_mod.get_json_result = lambda code=0, message="success", data=None: {"code": code, "data": data, "message": message} monkeypatch.setitem(sys.modules, "api.utils.api_utils", api_utils_mod) validation_mod = ModuleType("api.utils.validation_utils") @@ -338,4 +339,3 @@ def test_parent_and_ancestors_use_new_routes(monkeypatch): assert ancestors_res["code"] == 0 assert ancestors_res["data"]["parent_folders"][0]["id"] == "root" - diff --git a/web/src/hooks/use-file-request.ts b/web/src/hooks/use-file-request.ts index abd2972ea9..7a6999d306 100644 --- a/web/src/hooks/use-file-request.ts +++ b/web/src/hooks/use-file-request.ts @@ -69,7 +69,9 @@ export const useUploadFile = () => { }); } return ret?.data?.code; - } catch (error) {} + } catch { + return; + } }, }); @@ -213,7 +215,6 @@ export const useFetchFileList = () => { }; export const useDeleteFile = () => { - const { setPaginationParams } = useSetPaginationParams(); const queryClient = useQueryClient(); const { t } = useTranslation(); @@ -229,11 +230,10 @@ export const useDeleteFile = () => { }); if (data.code === 0) { message.success(t('message.deleted')); - setPaginationParams(1); // TODO: There should be a better way to paginate the request list - queryClient.invalidateQueries({ - queryKey: [FileApiAction.FetchFileList], - }); } + queryClient.invalidateQueries({ + queryKey: [FileApiAction.FetchFileList], + }); return data.code; }, });