Refactor: Consolidation WEB API & HTTP API for document delete api (#14254)

### What problem does this PR solve?

Before consolidation
Web API: POST /v1/document/rm
Http API - DELETE /api/v1/datasets/<dataset_id>/documents

After consolidation, Restful API -- DELETE
/api/v1/datasets/<dataset_id>/documents

### Type of change

- [x] Refactoring

test/testcases/test_http_api/test_file_management_within_dataset/test_delete_documents.py

@@ -26,11 +26,11 @@ class TestAuthorization:
     @pytest.mark.parametrize(
         "invalid_auth, expected_code, expected_message",
         [
-            (None, 0, "`Authorization` can't be empty"),
+            (None, 401, "<Unauthorized '401: Unauthorized'>"),
             (
                 RAGFlowHttpApiAuth(INVALID_API_TOKEN),
-                109,
-                "Authentication error: API key is invalid!",
+                401,
+                "<Unauthorized '401: Unauthorized'>",
             ),
         ],
     )
@@ -45,19 +45,19 @@ class TestDocumentsDeletion:
     @pytest.mark.parametrize(
         "payload, expected_code, expected_message, remaining",
         [
-            (None, 0, "", 3),
-            ({"ids": []}, 0, "", 3),
-            ({"ids": ["invalid_id"]}, 102, "Documents not found: ['invalid_id']", 3),
+            ({}, 102, "should either provide doc ids or set delete_all(true), dataset", 3),
+            ({"ids": []}, 102, "should either provide doc ids or set delete_all(true), dataset", 3),
+            ({"ids": ["invalid_id"]}, 101, "Field: <ids> - Message: <Invalid UUID1 format> - Value: <['invalid_id']>", 3),
             (
                 {"ids": ["\n!?。；！？\"'"]},
-                102,
-                """Documents not found: [\'\\n!?。；！？"\\\'\']""",
+                101,
+                "Field: <ids> - Message: <Invalid UUID1 format> - Value:",
                 3,
             ),
             (
                 "not json",
-                100,
-                "AttributeError(\"'str' object has no attribute 'get'\")",
+                101,
+                "Invalid request payload: expected object, got str",
                 3,
             ),
             (lambda r: {"ids": r[:1]}, 0, "", 2),
@@ -79,7 +79,7 @@ class TestDocumentsDeletion:
         res = delete_documents(HttpApiAuth, dataset_id, payload)
         assert res["code"] == expected_code
         if res["code"] != 0:
-            assert res["message"] == expected_message
+            assert expected_message in res["message"]
 
         res = list_documents(HttpApiAuth, dataset_id)
         assert len(res["data"]["docs"]) == remaining
@@ -117,12 +117,12 @@ class TestDocumentsDeletion:
         if callable(payload):
             payload = payload(document_ids)
         res = delete_documents(HttpApiAuth, dataset_id, payload)
-        assert res["code"] == 102
-        assert res["message"] == "Documents not found: ['invalid_id']"
+        assert res["code"] == 101
+        assert "Field: <ids> - Message: <Invalid UUID1 format> - Value" in res["message"]
 
         res = list_documents(HttpApiAuth, dataset_id)
-        assert len(res["data"]["docs"]) == 0
-        assert res["data"]["total"] == 0
+        assert len(res["data"]["docs"]) == 3
+        assert res["data"]["total"] == 3
 
     @pytest.mark.p2
     def test_repeated_deletion(self, HttpApiAuth, add_documents_func):
@@ -132,19 +132,18 @@ class TestDocumentsDeletion:
 
         res = delete_documents(HttpApiAuth, dataset_id, {"ids": document_ids})
         assert res["code"] == 102
-        assert "Documents not found" in res["message"]
+        assert "Document not found" in res["message"]
 
     @pytest.mark.p2
     def test_duplicate_deletion(self, HttpApiAuth, add_documents_func):
         dataset_id, document_ids = add_documents_func
         res = delete_documents(HttpApiAuth, dataset_id, {"ids": document_ids + document_ids})
-        assert res["code"] == 0
-        assert "Duplicate document ids" in res["data"]["errors"][0]
-        assert res["data"]["success_count"] == 3
+        assert res["code"] == 101, res
+        assert "Field: <ids> - Message: <Duplicate ids:" in res["message"]
 
         res = list_documents(HttpApiAuth, dataset_id)
-        assert len(res["data"]["docs"]) == 0
-        assert res["data"]["total"] == 0
+        assert len(res["data"]["docs"]) == 3
+        assert res["data"]["total"] == 3
 
 
 @pytest.mark.p3

test/testcases/test_http_api/test_file_management_within_dataset/test_doc_sdk_routes_unit.py

@@ -478,46 +478,6 @@ class TestDocRoutesUnit:
         assert res["data"]["matched_docs"] == 1
 
 
-    def test_delete_branches(self, monkeypatch):
-        module = _load_doc_module(monkeypatch)
-        monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False)
-        res = _run(module.delete.__wrapped__("tenant-1", "ds-1"))
-        assert "don't own the dataset" in res["message"]
-
-        monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: True)
-        monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({}))
-        res = _run(module.delete.__wrapped__("tenant-1", "ds-1"))
-        assert res["code"] == module.RetCode.SUCCESS
-
-        monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"ids": ["doc-1"]}))
-        monkeypatch.setattr(module, "check_duplicate_ids", lambda ids, _kind: (ids, []))
-        monkeypatch.setattr(module.FileService, "get_root_folder", lambda _tenant: {"id": "pf-1"})
-        monkeypatch.setattr(module.FileService, "init_knowledgebase_docs", lambda *_args, **_kwargs: None)
-        monkeypatch.setattr(module.DocumentService, "get_by_id", lambda _id: (True, _DummyDoc()))
-        monkeypatch.setattr(module.DocumentService, "get_tenant_id", lambda _id: None)
-        res = _run(module.delete.__wrapped__("tenant-1", "ds-1"))
-        assert res["message"] == "Tenant not found!"
-
-        monkeypatch.setattr(module.DocumentService, "get_tenant_id", lambda _id: "tenant-1")
-        monkeypatch.setattr(module.File2DocumentService, "get_storage_address", lambda **_kwargs: ("b", "n"))
-        monkeypatch.setattr(module.DocumentService, "remove_document", lambda *_args, **_kwargs: False)
-        res = _run(module.delete.__wrapped__("tenant-1", "ds-1"))
-        assert "Document removal" in res["message"]
-
-        def _raise_get_by_id(_id):
-            raise RuntimeError("boom")
-
-        monkeypatch.setattr(module.DocumentService, "get_by_id", _raise_get_by_id)
-        res = _run(module.delete.__wrapped__("tenant-1", "ds-1"))
-        assert res["code"] == module.RetCode.SERVER_ERROR
-        assert "boom" in res["message"]
-
-        monkeypatch.setattr(module, "check_duplicate_ids", lambda _ids, _kind: ([], ["Duplicate document ids: doc-1"]))
-        monkeypatch.setattr(module.DocumentService, "get_by_id", lambda _id: (False, None))
-        res = _run(module.delete.__wrapped__("tenant-1", "ds-1"))
-        assert res["code"] == module.RetCode.DATA_ERROR
-        assert "Duplicate document ids" in res["message"]
-
     def test_parse_branches(self, monkeypatch):
         module = _load_doc_module(monkeypatch)
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False)

test/testcases/test_sdk_api/test_file_management_within_dataset/test_delete_documents.py

@@ -24,10 +24,10 @@ class TestDocumentsDeletion:
     @pytest.mark.parametrize(
         "payload, expected_message, remaining",
         [
-            ({"ids": None}, "", 3),
-            ({"ids": []}, "", 3),
-            ({"ids": ["invalid_id"]}, "Documents not found: ['invalid_id']", 3),
-            ({"ids": ["\n!?。；！？\"'"]}, "Documents not found: ['\\n!?。；！？\"\\'']", 3),
+            ({"ids": None}, "should either provide doc ids or set delete_all(true), dataset:", 3),
+            ({"ids": []}, "should either provide doc ids or set delete_all(true), dataset:", 3),
+            ({"ids": ["invalid_id"]}, "Field: <ids> - Message: <Invalid UUID1 format> - Value: <['invalid_id']>", 3),
+            ({"ids": ["\n!?。；！？\"'"]}, "Field: <ids> - Message: <Invalid UUID1 format> - Value:", 3),
             ("not json", "must be a mapping", 3),
             (lambda r: {"ids": r[:1]}, "", 2),
             (lambda r: {"ids": r}, "", 0),
@@ -69,10 +69,10 @@ class TestDocumentsDeletion:
 
         with pytest.raises(Exception) as exception_info:
             dataset.delete_documents(**payload)
-        assert "Documents not found: ['invalid_id']" in str(exception_info.value), str(exception_info.value)
+        assert "Field: <ids> - Message: <Invalid UUID1 format> - Value: <" in str(exception_info.value), str(exception_info.value)
 
         documents = dataset.list_documents()
-        assert len(documents) == 0, str(documents)
+        assert len(documents) == 3, str(documents)
 
     @pytest.mark.p2
     def test_repeated_deletion(self, add_documents_func):
@@ -81,14 +81,16 @@ class TestDocumentsDeletion:
         dataset.delete_documents(ids=document_ids)
         with pytest.raises(Exception) as exception_info:
             dataset.delete_documents(ids=document_ids)
-        assert "Documents not found" in str(exception_info.value), str(exception_info.value)
+        assert "Document not found" in str(exception_info.value), str(exception_info.value)
 
     @pytest.mark.p2
     def test_duplicate_deletion(self, add_documents_func):
         dataset, documents = add_documents_func
         document_ids = [document.id for document in documents]
-        dataset.delete_documents(ids=document_ids + document_ids)
-        assert len(dataset.list_documents()) == 0, str(dataset.list_documents())
+        with pytest.raises(Exception) as exception_info:
+            dataset.delete_documents(ids=document_ids + document_ids)
+        assert "Field: <ids> - Message: <Duplicate ids:" in str(exception_info.value), str(exception_info.value)
+        assert len(dataset.list_documents()) == 3, str(dataset.list_documents())
 
 
 @pytest.mark.p3

test/testcases/test_web_api/test_chunk_app/test_create_chunk.py

@@ -218,8 +218,8 @@ class TestAddChunk:
 
     @pytest.mark.p2
     def test_add_chunk_to_deleted_document(self, WebApiAuth, add_document):
-        _, doc_id = add_document
-        delete_document(WebApiAuth, {"doc_id": doc_id})
+        kb_id, doc_id = add_document
+        delete_document(WebApiAuth, kb_id, {"ids": [doc_id]})
         res = add_chunk(WebApiAuth, {"doc_id": doc_id, "content_with_weight": "chunk test"})
         assert res["code"] == 102, res
         assert res["message"] == "Document not found!", res

test/testcases/test_web_api/test_chunk_app/test_update_chunk.py

@@ -251,8 +251,8 @@ class TestUpdateChunk:
 
     @pytest.mark.p3
     def test_update_chunk_to_deleted_document(self, WebApiAuth, add_chunks):
-        _, doc_id, chunk_ids = add_chunks
-        delete_document(WebApiAuth, {"doc_id": doc_id})
+        kb_id, doc_id, chunk_ids = add_chunks
+        delete_document(WebApiAuth, kb_id, {"ids": [doc_id]})
         payload = {"doc_id": doc_id, "chunk_id": chunk_ids[0], "content_with_weight": "test content"}
         res = update_chunk(WebApiAuth, payload)
         assert res["code"] == 102, res

test/testcases/test_web_api/test_common.py

@@ -382,8 +382,10 @@ def list_documents(auth, params=None, payload=None, *, headers=HEADERS, data=Non
     return res.json()
 
 
-def delete_document(auth, payload=None, *, headers=HEADERS, data=None):
-    res = requests.post(url=f"{HOST_ADDRESS}{DOCUMENT_APP_URL}/rm", headers=headers, auth=auth, json=payload, data=data)
+def delete_document(auth, dataset_id, payload=None, *, headers=HEADERS, data=None):
+    # New API: DELETE /api/v1/datasets/<dataset_id>/documents
+    url = f"{HOST_ADDRESS}{DATASETS_URL}/{dataset_id}/documents"
+    res = requests.delete(url=url, headers=headers, auth=auth, json=payload, data=data)
     return res.json()
 
 

test/testcases/test_web_api/test_document_app/conftest.py

@@ -36,7 +36,7 @@ def add_document_func(request, WebApiAuth, add_dataset, ragflow_tmp_dir):
     def cleanup():
         res = list_documents(WebApiAuth, {"kb_id": dataset_id})
         for doc in res["data"]["docs"]:
-            delete_document(WebApiAuth, {"doc_id": doc["id"]})
+            delete_document(WebApiAuth, dataset_id, {"ids": [doc["id"]]})
 
     request.addfinalizer(cleanup)
 
@@ -49,7 +49,7 @@ def add_documents(request, WebApiAuth, add_dataset, ragflow_tmp_dir):
     def cleanup():
         res = list_documents(WebApiAuth, {"kb_id": dataset_id})
         for doc in res["data"]["docs"]:
-            delete_document(WebApiAuth, {"doc_id": doc["id"]})
+            delete_document(WebApiAuth, dataset_id, {"ids": [doc["id"]]})
 
     request.addfinalizer(cleanup)
 
@@ -62,7 +62,7 @@ def add_documents_func(request, WebApiAuth, add_dataset_func, ragflow_tmp_dir):
     def cleanup():
         res = list_documents(WebApiAuth, {"kb_id": dataset_id})
         for doc in res["data"]["docs"]:
-            delete_document(WebApiAuth, {"doc_id": doc["id"]})
+            delete_document(WebApiAuth, dataset_id, {"ids": [doc["id"]]})
 
     request.addfinalizer(cleanup)
 

test/testcases/test_web_api/test_document_app/test_rm_documents.py

@@ -36,7 +36,7 @@ class TestAuthorization:
         ],
     )
     def test_invalid_auth(self, invalid_auth, expected_code, expected_message):
-        res = delete_document(invalid_auth)
+        res = delete_document(invalid_auth, "kb_id")
         assert res["code"] == expected_code, res
         assert res["message"] == expected_message, res
 
@@ -46,22 +46,23 @@ class TestDocumentsDeletion:
     @pytest.mark.parametrize(
         "payload, expected_code, expected_message, remaining",
         [
-            (None, 101, "required argument are missing: doc_id; ", 3),
-            ({"doc_id": ""}, 109, "No authorization.", 3),
-            ({"doc_id": "invalid_id"}, 109, "No authorization.", 3),
-            ({"doc_id": "\n!?。；！？\"'"}, 109, "No authorization.", 3),
-            ("not json", 101, "required argument are missing: doc_id; ", 3),
-            (lambda r: {"doc_id": r[0]}, 0, "", 2),
+            ({}, 102, "should either provide doc ids or set delete_all(true), dataset:", 3),
+            ({"invalid_key":[]}, 101, "Field: <invalid_key> - Message: <Extra inputs are not permitted> - Value: <[]>", 3),
+            ({"ids": ""}, 101, "Field: <ids> - Message: <Input should be a valid list> - Value: <>", 3),
+            ({"ids": ["invalid_id"]}, 101, "Field: <ids> - Message: <Invalid UUID1 format> - Value:", 3),
+            ("not json", 101, "Invalid request payload: expected object, got str", 3),
+            (lambda r: {"ids": r[0]}, 101, "Field: <ids> - Message: <Input should be a valid list> - Value", 3),
+            (lambda r: {"ids": r}, 0, "", 0),
         ],
     )
     def test_basic_scenarios(self, WebApiAuth, add_documents_func, payload, expected_code, expected_message, remaining):
         kb_id, document_ids = add_documents_func
         if callable(payload):
             payload = payload(document_ids)
-        res = delete_document(WebApiAuth, payload)
+        res = delete_document(WebApiAuth, kb_id, payload)
         assert res["code"] == expected_code, res
         if res["code"] != 0:
-            assert res["message"] == expected_message, res
+            assert expected_message in res["message"], res
 
         res = list_documents(WebApiAuth, {"kb_id": kb_id})
         assert len(res["data"]["docs"]) == remaining, res
@@ -69,57 +70,46 @@ class TestDocumentsDeletion:
 
     @pytest.mark.p2
     def test_repeated_deletion(self, WebApiAuth, add_documents_func):
-        _, document_ids = add_documents_func
+        kb_id, document_ids = add_documents_func
         for doc_id in document_ids:
-            res = delete_document(WebApiAuth, {"doc_id": doc_id})
+            res = delete_document(WebApiAuth, kb_id, {"ids": [doc_id]})
             assert res["code"] == 0, res
 
         for doc_id in document_ids:
-            res = delete_document(WebApiAuth, {"doc_id": doc_id})
-            assert res["code"] == 109, res
-            assert res["message"] == "No authorization.", res
+            res = delete_document(WebApiAuth, kb_id, {"ids": [doc_id]})
+            assert res["code"] == 102, res
+            assert res["message"] == "Document not found!", res
+
+    @pytest.mark.p2
+    def test_delete_all(self, WebApiAuth, add_documents_func):
+        kb_id, document_ids = add_documents_func
+
+        res = delete_document(WebApiAuth, kb_id, {"delete_all": True})
+        assert res["code"] == 0, res
+
+        res = list_documents(WebApiAuth, {"kb_id": kb_id})
+        assert len(res["data"]["docs"]) == 0, res
+        assert res["data"]["total"] == 0, res
 
 
 @pytest.mark.p2
-class TestDocumentsDeletionUnit:
-    def test_rm_string_doc_id_normalization_success_unit(self, document_app_module, monkeypatch):
-        module = document_app_module
-        captured = {}
-
-        async def fake_request_json():
-            return {"doc_id": "doc1"}
-
-        async def fake_thread_pool_exec(func, doc_ids, user_id):
-            captured["func"] = func
-            captured["doc_ids"] = doc_ids
-            captured["user_id"] = user_id
-            return None
-
-        monkeypatch.setattr(module, "get_request_json", fake_request_json)
-        monkeypatch.setattr(module.DocumentService, "accessible4deletion", lambda *_args, **_kwargs: True)
-        monkeypatch.setattr(module, "thread_pool_exec", fake_thread_pool_exec)
-        res = _run(module.rm.__wrapped__())
-        assert res["code"] == 0
-        assert res["data"] is True
-        assert captured["func"] == module.FileService.delete_docs
-        assert captured["doc_ids"] == ["doc1"]
-        assert captured["user_id"] == module.current_user.id
-
-
-@pytest.mark.p3
 def test_concurrent_deletion(WebApiAuth, add_dataset, tmp_path):
     count = 100
     kb_id = add_dataset
     document_ids = bulk_upload_documents(WebApiAuth, kb_id, count, tmp_path)
 
     with ThreadPoolExecutor(max_workers=5) as executor:
-        futures = [executor.submit(delete_document, WebApiAuth, {"doc_id": document_ids[i]}) for i in range(count)]
+        futures = [executor.submit(delete_document, WebApiAuth, kb_id, {"ids": [document_ids[i]]}) for i in range(count)]
     responses = list(as_completed(futures))
     assert len(responses) == count, responses
     assert all(future.result()["code"] == 0 for future in futures), responses
 
+    res = list_documents(WebApiAuth, {"kb_id": kb_id})
+    assert len(res["data"]["docs"]) == 0, res
+    assert res["data"]["total"] == 0, res
 
-@pytest.mark.p3
+
+@pytest.mark.p2
 def test_delete_100(WebApiAuth, add_dataset, tmp_path):
     documents_num = 100
     kb_id = add_dataset
@@ -128,7 +118,7 @@ def test_delete_100(WebApiAuth, add_dataset, tmp_path):
     assert res["data"]["total"] == documents_num, res
 
     for doc_id in document_ids:
-        res = delete_document(WebApiAuth, {"doc_id": doc_id})
+        res = delete_document(WebApiAuth, kb_id, {"ids": [doc_id]})
         assert res["code"] == 0, res
 
     res = list_documents(WebApiAuth, {"kb_id": kb_id})