Refa: migrate chunk APIs to RESTful routes (#14291)

### What problem does this PR solve?

migrate chunk APIs to RESTful routes

### Type of change
- [x] Refactoring

test/testcases/test_http_api/common.py

@@ -173,9 +173,15 @@ def list_chunks(auth, dataset_id, document_id, params=None):
     return res.json()
 
 
+def get_chunk(auth, dataset_id, document_id, chunk_id):
+    url = f"{HOST_ADDRESS}{CHUNK_API_URL}/{chunk_id}".format(dataset_id=dataset_id, document_id=document_id)
+    res = requests.get(url=url, headers=HEADERS, auth=auth)
+    return res.json()
+
+
 def update_chunk(auth, dataset_id, document_id, chunk_id, payload=None):
     url = f"{HOST_ADDRESS}{CHUNK_API_URL}/{chunk_id}".format(dataset_id=dataset_id, document_id=document_id)
-    res = requests.put(url=url, headers=HEADERS, auth=auth, json=payload)
+    res = requests.patch(url=url, headers=HEADERS, auth=auth, json=payload)
     return res.json()
 
 

test/testcases/test_http_api/test_chunk_management_within_dataset/conftest.py

@@ -18,17 +18,20 @@
 from time import sleep
 
 import pytest
-from common import batch_add_chunks, delete_all_chunks, list_documents, parse_documents
-from utils import wait_for
+from common import add_chunk, batch_add_chunks, delete_all_chunks
 
 
-@wait_for(30, 1, "Document parsing timeout")
-def condition(_auth, _dataset_id):
-    res = list_documents(_auth, _dataset_id)
-    for doc in res["data"]["docs"]:
-        if doc["run"] != "DONE":
-            return False
-    return True
+def _add_baseline_chunk(auth, dataset_id, document_id):
+    add_chunk(auth, dataset_id, document_id, {"content": "ragflow test upload"})
+
+
+@pytest.fixture(scope="class")
+def add_chunks(HttpApiAuth, add_document):
+    dataset_id, document_id = add_document
+    _add_baseline_chunk(HttpApiAuth, dataset_id, document_id)
+    chunk_ids = batch_add_chunks(HttpApiAuth, dataset_id, document_id, 4)
+    sleep(1)  # issues/6487
+    return dataset_id, document_id, chunk_ids
 
 
 @pytest.fixture(scope="function")
@@ -39,8 +42,7 @@ def add_chunks_func(request, HttpApiAuth, add_document):
     request.addfinalizer(cleanup)
 
     dataset_id, document_id = add_document
-    parse_documents(HttpApiAuth, dataset_id, {"document_ids": [document_id]})
-    condition(HttpApiAuth, dataset_id)
+    _add_baseline_chunk(HttpApiAuth, dataset_id, document_id)
     chunk_ids = batch_add_chunks(HttpApiAuth, dataset_id, document_id, 4)
     # issues/6487
     sleep(1)

test/testcases/test_http_api/test_chunk_management_within_dataset/test_add_chunk.py

@@ -39,12 +39,8 @@ class TestAuthorization:
     @pytest.mark.parametrize(
         "invalid_auth, expected_code, expected_message",
         [
-            (None, 0, "`Authorization` can't be empty"),
-            (
-                RAGFlowHttpApiAuth(INVALID_API_TOKEN),
-                109,
-                "Authentication error: API key is invalid!",
-            ),
+            (None, 401, "<Unauthorized '401: Unauthorized'>"),
+            (RAGFlowHttpApiAuth(INVALID_API_TOKEN), 401, "<Unauthorized '401: Unauthorized'>"),
         ],
     )
     def test_invalid_auth(self, invalid_auth, expected_code, expected_message):

test/testcases/test_http_api/test_chunk_management_within_dataset/test_delete_chunks.py

@@ -26,12 +26,8 @@ class TestAuthorization:
     @pytest.mark.parametrize(
         "invalid_auth, expected_code, expected_message",
         [
-            (None, 0, "`Authorization` can't be empty"),
-            (
-                RAGFlowHttpApiAuth(INVALID_API_TOKEN),
-                109,
-                "Authentication error: API key is invalid!",
-            ),
+            (None, 401, "<Unauthorized '401: Unauthorized'>"),
+            (RAGFlowHttpApiAuth(INVALID_API_TOKEN), 401, "<Unauthorized '401: Unauthorized'>"),
         ],
     )
     def test_invalid_auth(self, invalid_auth, expected_code, expected_message):
@@ -58,7 +54,7 @@ class TestChunksDeletion:
     @pytest.mark.parametrize(
         "document_id, expected_code, expected_message",
         [
-            (INVALID_ID_32, 100, f"""LookupError("Can't find the document with ID {INVALID_ID_32}!")"""),
+            (INVALID_ID_32, 102, f"You don't own the document {INVALID_ID_32}."),
         ],
     )
     def test_invalid_document_id(self, HttpApiAuth, add_chunks_func, document_id, expected_code, expected_message):

test/testcases/test_http_api/test_chunk_management_within_dataset/test_list_chunks.py

@@ -17,7 +17,7 @@ import os
 from concurrent.futures import ThreadPoolExecutor, as_completed
 
 import pytest
-from common import batch_add_chunks, list_chunks
+from common import batch_add_chunks, get_chunk, list_chunks
 from configs import INVALID_API_TOKEN, INVALID_ID_32
 from libs.auth import RAGFlowHttpApiAuth
 
@@ -27,12 +27,8 @@ class TestAuthorization:
     @pytest.mark.parametrize(
         "invalid_auth, expected_code, expected_message",
         [
-            (None, 0, "`Authorization` can't be empty"),
-            (
-                RAGFlowHttpApiAuth(INVALID_API_TOKEN),
-                109,
-                "Authentication error: API key is invalid!",
-            ),
+            (None, 401, "<Unauthorized '401: Unauthorized'>"),
+            (RAGFlowHttpApiAuth(INVALID_API_TOKEN), 401, "<Unauthorized '401: Unauthorized'>"),
         ],
     )
     def test_invalid_auth(self, invalid_auth, expected_code, expected_message):
@@ -139,6 +135,15 @@ class TestChunksList:
         else:
             assert res["message"] == expected_message
 
+    @pytest.mark.p1
+    @pytest.mark.skipif(os.getenv("DOC_ENGINE") == "infinity", reason="issues/6499")
+    def test_get_chunk(self, HttpApiAuth, add_chunks):
+        dataset_id, document_id, chunk_ids = add_chunks
+        res = get_chunk(HttpApiAuth, dataset_id, document_id, chunk_ids[0])
+        assert res["code"] == 0
+        assert res["data"]["id"] == chunk_ids[0]
+        assert res["data"]["doc_id"] == document_id
+
     @pytest.mark.p3
     def test_invalid_params(self, HttpApiAuth, add_chunks):
         dataset_id, document_id, _ = add_chunks

test/testcases/test_http_api/test_chunk_management_within_dataset/test_update_chunk.py

@@ -28,12 +28,8 @@ class TestAuthorization:
     @pytest.mark.parametrize(
         "invalid_auth, expected_code, expected_message",
         [
-            (None, 0, "`Authorization` can't be empty"),
-            (
-                RAGFlowHttpApiAuth(INVALID_API_TOKEN),
-                109,
-                "Authentication error: API key is invalid!",
-            ),
+            (None, 401, "<Unauthorized '401: Unauthorized'>"),
+            (RAGFlowHttpApiAuth(INVALID_API_TOKEN), 401, "<Unauthorized '401: Unauthorized'>"),
         ],
     )
     def test_invalid_auth(self, invalid_auth, expected_code, expected_message):

test/testcases/test_http_api/test_file_management_within_dataset/test_doc_sdk_routes_unit.py

@@ -14,6 +14,7 @@
 #  limitations under the License.
 #
 import asyncio
+import inspect
 import importlib.util
 import sys
 from pathlib import Path
@@ -309,6 +310,19 @@ def _load_doc_module(monkeypatch):
     return module
 
 
+def _load_restful_chunk_module(monkeypatch):
+    repo_root = Path(__file__).resolve().parents[4]
+    helper_path = repo_root / "test" / "testcases" / "test_web_api" / "test_chunk_app" / "test_chunk_routes_unit.py"
+    spec = importlib.util.spec_from_file_location("test_restful_chunk_route_helpers", helper_path)
+    helper = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(helper)
+    return helper._load_chunk_api_module(monkeypatch)
+
+
+def _route_core(func):
+    return inspect.unwrap(func)
+
+
 def _patch_send_file(monkeypatch, module):
     async def _fake_send_file(file_obj, **kwargs):
         return {"file": file_obj, "filename": kwargs.get("attachment_filename")}
@@ -336,7 +350,7 @@ def _patch_docstore(monkeypatch, module, **kwargs):
 @pytest.mark.p2
 class TestDocRoutesUnit:
     def test_chunk_positions_validation_error(self, monkeypatch):
-        module = _load_doc_module(monkeypatch)
+        module = _load_restful_chunk_module(monkeypatch)
         with pytest.raises(ValueError) as exc_info:
             module.Chunk(positions=[[1, 2, 3, 4]])
         assert "length of 5" in str(exc_info.value)
@@ -484,25 +498,44 @@ class TestDocRoutesUnit:
         assert res["code"] == 0
 
     def test_list_chunks_branches(self, monkeypatch):
-        module = _load_doc_module(monkeypatch)
+        module = _load_restful_chunk_module(monkeypatch)
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False)
-        res = _run(module.list_chunks.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.list_chunks)("tenant-1", "ds-1", "doc-1"))
         assert "don't own the dataset" in res["message"]
 
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: True)
         monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [])
-        res = _run(module.list_chunks.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.list_chunks)("tenant-1", "ds-1", "doc-1"))
         assert "don't own the document" in res["message"]
 
         monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [_DummyDoc()])
+        monkeypatch.setattr(module, "request", SimpleNamespace(args=_DummyArgs({})))
+        _patch_docstore(monkeypatch, module, index_exist=lambda *_args, **_kwargs: False)
+        res = _run(_route_core(module.list_chunks)("tenant-1", "ds-1", "doc-1"))
+        assert res["code"] == 0
+        assert res["data"]["total"] == 0
+        assert res["data"]["chunks"] == []
+
         monkeypatch.setattr(module, "request", SimpleNamespace(args=_DummyArgs({"id": "chunk-1"})))
         _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: None)
-        res = _run(module.list_chunks.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.list_chunks)("tenant-1", "ds-1", "doc-1"))
+        assert res["code"] == module.RetCode.DATA_ERROR
         assert "Chunk not found" in res["message"]
 
-        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: {"id_vec": [1], "content_with_weight_vec": [2]})
-        res = _run(module.list_chunks.__wrapped__("tenant-1", "ds-1", "doc-1"))
-        assert "Chunk `chunk-1` not found." in res["message"]
+        _patch_docstore(
+            monkeypatch,
+            module,
+            get=lambda *_args, **_kwargs: {
+                "chunk_id": "chunk-1",
+                "content_with_weight": "x",
+                "doc_id": "other-doc",
+                "docnm_kwd": "doc",
+                "position_int": [[1, 2, 3, 4, 5]],
+            },
+        )
+        res = _run(_route_core(module.list_chunks)("tenant-1", "ds-1", "doc-1"))
+        assert res["code"] == module.RetCode.DATA_ERROR
+        assert "Chunk not found" in res["message"]
 
         _patch_docstore(
             monkeypatch,
@@ -515,29 +548,29 @@ class TestDocRoutesUnit:
                 "position_int": [[1, 2, 3, 4, 5]],
             },
         )
-        res = _run(module.list_chunks.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.list_chunks)("tenant-1", "ds-1", "doc-1"))
         assert res["code"] == 0
         assert res["data"]["total"] == 1
         assert res["data"]["chunks"][0]["id"] == "chunk-1"
 
     def test_add_chunk_access_guard(self, monkeypatch):
-        module = _load_doc_module(monkeypatch)
+        module = _load_restful_chunk_module(monkeypatch)
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False)
-        res = _run(module.add_chunk.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.add_chunk)("tenant-1", "ds-1", "doc-1"))
         assert "don't own the dataset" in res["message"]
 
     def test_rm_chunk_branches(self, monkeypatch):
-        module = _load_doc_module(monkeypatch)
+        module = _load_restful_chunk_module(monkeypatch)
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False)
-        res = _run(module.rm_chunk.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.rm_chunk)("tenant-1", "ds-1", "doc-1"))
         assert "don't own the dataset" in res["message"]
 
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: True)
-        monkeypatch.setattr(module.DocumentService, "get_by_ids", lambda _ids: [])
-        with pytest.raises(LookupError):
-            _run(module.rm_chunk.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [])
+        res = _run(_route_core(module.rm_chunk)("tenant-1", "ds-1", "doc-1"))
+        assert "don't own the document" in res["message"]
 
-        monkeypatch.setattr(module.DocumentService, "get_by_ids", lambda _ids: [_DummyDoc()])
+        monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [_DummyDoc()])
         monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({}))
         _patch_docstore(
             monkeypatch,
@@ -545,32 +578,37 @@ class TestDocRoutesUnit:
             delete=lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("delete must not run for empty chunk ids")),
         )
         monkeypatch.setattr(module.DocumentService, "decrement_chunk_num", lambda *_args, **_kwargs: None)
-        res = _run(module.rm_chunk.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.rm_chunk)("tenant-1", "ds-1", "doc-1"))
         assert res["code"] == 0
 
         monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"chunk_ids": ["c1", "c1"]}))
         monkeypatch.setattr(module, "check_duplicate_ids", lambda _ids, _kind: (["c1"], ["Duplicate chunk ids: c1"]))
         _patch_docstore(monkeypatch, module, delete=lambda *_args, **_kwargs: 1)
-        res = _run(module.rm_chunk.__wrapped__("tenant-1", "ds-1", "doc-1"))
+        res = _run(_route_core(module.rm_chunk)("tenant-1", "ds-1", "doc-1"))
         assert res["code"] == 0
         assert res["data"]["errors"] == ["Duplicate chunk ids: c1"]
 
     def test_update_chunk_branches(self, monkeypatch):
-        module = _load_doc_module(monkeypatch)
-        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: None)
-        res = _run(module.update_chunk.__wrapped__("tenant-1", "ds-1", "doc-1", "chunk-1"))
-        assert "Can't find this chunk" in res["message"]
-
-        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: {"content_with_weight": "q\na"})
+        module = _load_restful_chunk_module(monkeypatch)
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False)
-        res = _run(module.update_chunk.__wrapped__("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("chunk lookup must not run before access check")))
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
         assert "don't own the dataset" in res["message"]
 
         monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: True)
         monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [])
-        res = _run(module.update_chunk.__wrapped__("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
         assert "don't own the document" in res["message"]
 
+        monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [_DummyDoc()])
+        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: None)
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        assert "Can't find this chunk" in res["message"]
+
+        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: {"doc_id": "other-doc", "content_with_weight": "q\na"})
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        assert "Can't find this chunk" in res["message"]
+
         doc = _DummyDoc(parser_id="naive")
         monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [doc])
         monkeypatch.setattr(module.rag_tokenizer, "tokenize", lambda text: text or "")
@@ -584,25 +622,25 @@ class TestDocRoutesUnit:
                 return [np.array([0.2, 0.8]), np.array([0.3, 0.7])], 1
 
         monkeypatch.setattr(module.TenantLLMService, "model_instance", lambda *_args, **_kwargs: _EmbedModel())
+        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: {"doc_id": "doc-1", "content_with_weight": "x"}, update=lambda *_args, **_kwargs: None)
         monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"positions": "bad"}))
-        res = _run(module.update_chunk.__wrapped__("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
         assert "`positions` should be a list" in res["message"]
 
-        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: {"content_with_weight": "x"}, update=lambda *_args, **_kwargs: None)
         monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"positions": [[1, 2, 3, 4, 5]]}))
-        res = _run(module.update_chunk.__wrapped__("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
         assert res["code"] == 0
 
         qa_doc = _DummyDoc(parser_id=module.ParserType.QA)
         monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [qa_doc])
         monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"content": "no-separator"}))
-        res = _run(module.update_chunk.__wrapped__("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
         assert "Q&A must be separated" in res["message"]
 
         monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"content": "Q?\nA!"}))
-        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: {"content_with_weight": "Q?\nA!"}, update=lambda *_args, **_kwargs: None)
+        _patch_docstore(monkeypatch, module, get=lambda *_args, **_kwargs: {"doc_id": "doc-1", "content_with_weight": "Q?\nA!"}, update=lambda *_args, **_kwargs: None)
         monkeypatch.setattr(module, "beAdoc", lambda d, *_args, **_kwargs: d)
-        res = _run(module.update_chunk.__wrapped__("tenant-1", "ds-1", "doc-1", "chunk-1"))
+        res = _run(_route_core(module.update_chunk)("tenant-1", "ds-1", "doc-1", "chunk-1"))
         assert res["code"] == 0
 
     def test_retrieval_validation_matrix(self, monkeypatch):