Refactor: Consolidation WEB API & HTTP API for document list_docs (#14176)

### What problem does this PR solve?

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

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

### Type of change

- [x] Refactoring
This commit is contained in:
Jack
2026-04-20 14:54:40 +08:00
committed by GitHub
parent d053317c4d
commit 939933649a
28 changed files with 627 additions and 712 deletions

View File

@@ -388,65 +388,6 @@ class TestDocRoutesUnit:
res = _run(module.download_doc("doc-1"))
assert res["filename"] == "doc.txt"
def test_list_docs_metadata_filters(self, monkeypatch):
module = _load_doc_module(monkeypatch)
monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False)
monkeypatch.setattr(module, "request", SimpleNamespace(args=_DummyArgs()))
res = module.list_docs.__wrapped__("ds-1", "tenant-1")
assert "don't own the dataset" in res["message"]
monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: True)
monkeypatch.setattr(
module,
"request",
SimpleNamespace(
args=_DummyArgs(
{
"metadata_condition": "{bad json",
}
)
),
)
res = module.list_docs.__wrapped__("ds-1", "tenant-1")
assert res["message"] == "metadata_condition must be valid JSON."
monkeypatch.setattr(module, "request", SimpleNamespace(args=_DummyArgs({"metadata_condition": "[1]"})))
res = module.list_docs.__wrapped__("ds-1", "tenant-1")
assert res["message"] == "metadata_condition must be an object."
monkeypatch.setattr(module.DocMetadataService, "get_flatted_meta_by_kbs", lambda _kbs: [{"doc_id": "x"}])
monkeypatch.setattr(module, "meta_filter", lambda *_args, **_kwargs: [])
monkeypatch.setattr(module, "convert_conditions", lambda cond: cond)
monkeypatch.setattr(
module,
"request",
SimpleNamespace(args=_DummyArgs({"metadata_condition": '{"conditions":[{"field":"x","op":"eq","value":"y"}]}'})),
)
res = module.list_docs.__wrapped__("ds-1", "tenant-1")
assert res["code"] == module.RetCode.SUCCESS
assert res["data"]["total"] == 0
monkeypatch.setattr(
module.DocumentService,
"get_list",
lambda *_args, **_kwargs: ([{"id": "doc-1", "create_time": 100, "run": "0"}], 1),
)
monkeypatch.setattr(
module,
"request",
SimpleNamespace(
args=_DummyArgs(
{
"create_time_from": "101",
"create_time_to": "200",
}
)
),
)
res = module.list_docs.__wrapped__("ds-1", "tenant-1")
assert res["code"] == 0
assert res["data"]["docs"] == []
def test_metadata_batch_update(self, monkeypatch):
module = _load_doc_module(monkeypatch)
monkeypatch.setattr(module, "convert_conditions", lambda cond: cond)

View File

@@ -27,11 +27,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'>",
),
],
)
@@ -72,7 +72,7 @@ class TestDocumentsList:
"params, expected_code, expected_page_size, expected_message",
[
({"page": None, "page_size": 2}, 0, 2, ""),
({"page": 0, "page_size": 2}, 0, 2, ""),
({"page": 1, "page_size": 2}, 0, 2, ""),
({"page": 2, "page_size": 2}, 0, 2, ""),
({"page": 3, "page_size": 2}, 0, 1, ""),
({"page": "3", "page_size": 2}, 0, 1, ""),
@@ -115,7 +115,6 @@ class TestDocumentsList:
"params, expected_code, expected_page_size, expected_message",
[
({"page_size": None}, 0, 5, ""),
({"page_size": 0}, 0, 0, ""),
({"page_size": 1}, 0, 1, ""),
({"page_size": 6}, 0, 5, ""),
({"page_size": "1"}, 0, 1, ""),
@@ -232,6 +231,7 @@ class TestDocumentsList:
assert len(res["data"]["docs"]) == expected_num
assert res["data"]["total"] == expected_num
@pytest.mark.p1
@pytest.mark.parametrize(
"params, expected_code, expected_num, expected_message",
@@ -240,21 +240,21 @@ class TestDocumentsList:
({"name": ""}, 0, 5, ""),
({"name": "ragflow_test_upload_0.txt"}, 0, 1, ""),
(
{"name": "unknown.txt"},
102,
0,
"You don't own the document unknown.txt.",
{"name": "unknown.txt"},
102,
0,
"You don't own the document unknown.txt.",
),
],
)
def test_name(
self,
HttpApiAuth,
add_documents,
params,
expected_code,
expected_num,
expected_message,
self,
HttpApiAuth,
add_documents,
params,
expected_code,
expected_num,
expected_message,
):
dataset_id, _ = add_documents
res = list_documents(HttpApiAuth, dataset_id, params=params)
@@ -267,6 +267,7 @@ class TestDocumentsList:
else:
assert res["message"] == expected_message
@pytest.mark.p1
@pytest.mark.parametrize(
"document_id, expected_code, expected_num, expected_message",
@@ -278,13 +279,13 @@ class TestDocumentsList:
],
)
def test_id(
self,
HttpApiAuth,
add_documents,
document_id,
expected_code,
expected_num,
expected_message,
self,
HttpApiAuth,
add_documents,
document_id,
expected_code,
expected_num,
expected_message,
):
dataset_id, document_ids = add_documents
if callable(document_id):
@@ -298,11 +299,13 @@ class TestDocumentsList:
if params["id"] in [None, ""]:
assert len(res["data"]["docs"]) == expected_num
else:
assert res["data"]["docs"][0]["id"] == params["id"]
doc = res["data"]["docs"][0]
assert doc["id"] == params["id"]
else:
assert res["message"] == expected_message
@pytest.mark.p3
@pytest.mark.p2
@pytest.mark.parametrize(
"document_id, name, expected_code, expected_num, expected_message",
[
@@ -310,23 +313,23 @@ class TestDocumentsList:
(lambda r: r[0], "ragflow_test_upload_1.txt", 0, 0, ""),
(lambda r: r[0], "unknown", 102, 0, "You don't own the document unknown."),
(
"id",
"ragflow_test_upload_0.txt",
102,
0,
"You don't own the document id.",
"id",
"ragflow_test_upload_0.txt",
102,
0,
"You don't own the document id.",
),
],
)
def test_name_and_id(
self,
HttpApiAuth,
add_documents,
document_id,
name,
expected_code,
expected_num,
expected_message,
self,
HttpApiAuth,
add_documents,
document_id,
name,
expected_code,
expected_num,
expected_message,
):
dataset_id, document_ids = add_documents
if callable(document_id):
@@ -340,6 +343,7 @@ class TestDocumentsList:
else:
assert res["message"] == expected_message
@pytest.mark.p3
def test_concurrent_list(self, HttpApiAuth, add_documents):
dataset_id, _ = add_documents
@@ -358,3 +362,83 @@ class TestDocumentsList:
res = list_documents(HttpApiAuth, dataset_id, params=params)
assert res["code"] == 0
assert len(res["data"]["docs"]) == 5
@pytest.mark.p2
@pytest.mark.parametrize(
"params, expected_code, expected_message",
[
(
{"metadata_condition": "{bad json"},
102,
"metadata_condition must be valid JSON",
),
(
{"metadata_condition": "[1]"},
102,
"metadata_condition must be an object",
),
],
)
def test_metadata_condition_validation(
self, HttpApiAuth, add_documents, params, expected_code, expected_message
):
dataset_id, _ = add_documents
res = list_documents(HttpApiAuth, dataset_id, params=params)
assert res["code"] == expected_code
assert expected_message in res["message"]
@pytest.mark.p2
@pytest.mark.parametrize(
"params, expected_code, expected_total",
[
# Filter with create_time_from in the future - should return 0 results
({"create_time_from": "9999999999000"}, 0, 0),
# Filter with create_time_to in the past - should return 0 results
({"create_time_to": "1"}, 0, 0),
# Filter with create_time_from and create_time_to covering all time
({"create_time_from": "0", "create_time_to": "9999999999000"}, 0, 5),
],
)
def test_create_time_filter(
self, HttpApiAuth, add_documents, params, expected_code, expected_total
):
dataset_id, _ = add_documents
res = list_documents(HttpApiAuth, dataset_id, params=params)
assert res["code"] == expected_code
assert len(res["data"]["docs"]) == expected_total
assert res["data"]["total"] == 5
@pytest.mark.p2
@pytest.mark.parametrize(
"params, expected_code, expected_message",
[
# Invalid run status - should return error
({"run": ["INVALID_STATUS"]}, 102, "Invalid filter run status conditions: INVALID_STATUS"),
],
)
def test_run_status_filter_invalid(
self, HttpApiAuth, add_documents, params, expected_code, expected_message
):
dataset_id, _ = add_documents
res = list_documents(HttpApiAuth, dataset_id, params=params)
assert res["code"] == expected_code
assert expected_message in res["message"]
@pytest.mark.p2
@pytest.mark.parametrize(
"params, expected_size",
[
# Invalid run status - should return error
({"run": ["UNSTART"]}, 5),
],
)
def test_run_status_filter_unstart(
self, HttpApiAuth, add_documents, params, expected_size
):
dataset_id, _ = add_documents
res = list_documents(HttpApiAuth, dataset_id, params=params)
assert res["code"] == 0
assert res["data"]["total"] == expected_size

View File

@@ -42,6 +42,7 @@ def condition(_auth, _dataset_id, _document_ids=None):
def validate_document_details(auth, dataset_id, document_ids):
# currently list_documents not support search by document id
for document_id in document_ids:
res = list_documents(auth, dataset_id, params={"id": document_id})
doc = res["data"]["docs"][0]

View File

@@ -42,7 +42,8 @@ class TestAuthorization:
class TestDocumentsUpdated:
@pytest.mark.p1
# GET /api/v1/datasets/<dataset_id>/documents no longer support find by id/name
@pytest.mark.p3
@pytest.mark.parametrize(
"name, expected_code, expected_message",
[
@@ -94,7 +95,8 @@ class TestDocumentsUpdated:
else:
assert res["message"] == expected_message
@pytest.mark.p2
# GET /api/v1/datasets/<dataset_id>/documents no longer support find by id/name
@pytest.mark.p3
@pytest.mark.parametrize(
"document_id, expected_code, expected_message",
[
@@ -206,10 +208,12 @@ class TestDocumentsUpdated:
assert res["code"] == expected_code
if expected_code == 0:
res = list_documents(HttpApiAuth, dataset_id, {"id": document_ids[0]})
doc_of_id = res["data"]["docs"][0]
if chunk_method == "":
assert res["data"]["docs"][0]["chunk_method"] == "naive"
assert doc_of_id["chunk_method"] == "naive"
else:
assert res["data"]["docs"][0]["chunk_method"] == chunk_method
print(f"doc:{doc_of_id}")
assert doc_of_id["chunk_method"] == chunk_method
else:
assert res["message"] == expected_message
@@ -597,10 +601,12 @@ class TestUpdateDocumentParserConfig:
assert res["code"] == expected_code
if expected_code == 0:
res = list_documents(HttpApiAuth, dataset_id, {"id": document_ids[0]})
doc_of_id = res["data"]["docs"][0]
if parser_config == {}:
assert res["data"]["docs"][0]["parser_config"] == DEFAULT_PARSER_CONFIG
assert doc_of_id["parser_config"] == DEFAULT_PARSER_CONFIG
else:
for k, v in parser_config.items():
assert res["data"]["docs"][0]["parser_config"][k] == v
assert doc_of_id["parser_config"][k] == v
if expected_code != 0 or expected_message:
assert res["message"] == expected_message