Refactor: Merge document update API (#13962)

### What problem does this PR solve?

Refactor: merge document.rename into document.update_document

### Type of change

- [x] Refactoring


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added a unified document update API (PUT) supporting name, metadata,
parser/chunk settings, and status changes.

* **Breaking Changes**
* Legacy single-parameter rename endpoint removed; renames now require
dataset + document identifiers.
  * `/list` now reads dataset id from a different query parameter.

* **Validation / Bug Fixes**
* Stricter meta_fields and parser-config validation; unauthenticated
requests return 401.

* **Frontend**
  * UI now sends dataset id when saving document names.

* **Tests**
* Numerous unit and HTTP tests adjusted or removed to match new API and
validations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
Co-authored-by: Jin Hai <haijin.chn@gmail.com>
Co-authored-by: MkDev11 <94194147+MkDev11@users.noreply.github.com>
Co-authored-by: mkdev11 <YOUR_GITHUB_ID+MkDev11@users.noreply.github.com>
Co-authored-by: mkdev11 <MkDev11@users.noreply.github.com>
Co-authored-by: Qi Wang <wangq8@outlook.com>
Co-authored-by: dataCenter430 <161712630+dataCenter430@users.noreply.github.com>
Co-authored-by: balibabu <cike8899@users.noreply.github.com>
This commit is contained in:
Jack
2026-04-09 11:17:38 +08:00
committed by GitHub
parent c13f8856a1
commit 577c96bf2a
14 changed files with 651 additions and 710 deletions

View File

@@ -366,123 +366,6 @@ class TestDocRoutesUnit:
assert res["code"] == module.RetCode.SERVER_ERROR
assert res["message"] == "upload failed"
def test_update_doc_guards_and_error_paths(self, monkeypatch):
module = _load_doc_module(monkeypatch)
doc = _DummyDoc()
monkeypatch.setattr(module.KnowledgebaseService, "query", lambda **_kwargs: [])
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "You don't own the dataset."
monkeypatch.setattr(module.KnowledgebaseService, "query", lambda **_kwargs: [1])
monkeypatch.setattr(module.KnowledgebaseService, "get_by_id", lambda _id: (False, None))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Can't find this dataset!"
monkeypatch.setattr(module.KnowledgebaseService, "get_by_id", lambda _id: (True, SimpleNamespace(tenant_id="tenant-1")))
monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [])
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert "doesn't own the document" in res["message"]
monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [doc])
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"chunk_count": 100}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert "chunk_count" in res["message"]
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"token_count": 100}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert "token_count" in res["message"]
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"progress": 100}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert "progress" in res["message"]
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"meta_fields": []}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Field: <meta_fields> - Message: <Input should be a valid dictionary> - Value: <[]>"
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"meta_fields": {"k": "v"}}))
monkeypatch.setattr(module.DocMetadataService, "update_document_metadata", lambda *_args, **_kwargs: False)
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Failed to update metadata"
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"name": "a" * (module.FILE_NAME_LEN_LIMIT + 1)}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["code"] == module.RetCode.ARGUMENT_ERROR
assert "bytes or less" in res["message"]
monkeypatch.setattr(module.DocumentService, "update_by_id", lambda *_args, **_kwargs: False)
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"name": "new.txt"}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert "Document rename" in res["message"]
def test_update_doc_chunk_method_enabled_and_db_error(self, monkeypatch):
module = _load_doc_module(monkeypatch)
from api.db import FileType
visual_doc = _DummyDoc(parser_id="naive", doc_type=FileType.VISUAL)
kb = SimpleNamespace(tenant_id="tenant-1")
monkeypatch.setattr(module.KnowledgebaseService, "query", lambda **_kwargs: [1])
monkeypatch.setattr(module.KnowledgebaseService, "get_by_id", lambda _id: (True, kb))
monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [visual_doc])
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"chunk_method": "naive"}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Not supported yet!"
doc = _DummyDoc(token_num=2, chunk_num=1, parser_id="naive")
monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [doc])
monkeypatch.setattr(module.DocumentService, "update_by_id", lambda *_args, **_kwargs: False)
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"chunk_method": "manual"}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Document not found!"
monkeypatch.setattr(module.DocumentService, "update_by_id", lambda *_args, **_kwargs: True)
monkeypatch.setattr(module.DocumentService, "update_parser_config", lambda *_args, **_kwargs: None)
monkeypatch.setattr(module.DocumentService, "increment_chunk_num", lambda *_args, **_kwargs: False)
_patch_docstore(monkeypatch, module, delete=lambda *_args, **_kwargs: None)
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"chunk_method": "manual"}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Document not found!"
monkeypatch.setattr(module.DocumentService, "increment_chunk_num", lambda *_args, **_kwargs: True)
doc_for_enabled = _DummyDoc(status=False)
monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [doc_for_enabled])
monkeypatch.setattr(module.DocumentService, "get_by_id", lambda _id: (True, doc_for_enabled))
monkeypatch.setattr(module.DocumentService, "update_by_id", lambda *_args, **_kwargs: False)
_patch_docstore(monkeypatch, module, update=lambda *_args, **_kwargs: None, delete=lambda *_args, **_kwargs: None)
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"enabled": 1}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert "Document update" in res["message"]
monkeypatch.setattr(module.DocumentService, "update_by_id", lambda *_args, **_kwargs: True)
_patch_docstore(monkeypatch, module, update=lambda *_args, **_kwargs: (_ for _ in ()).throw(RuntimeError("boom")), delete=lambda *_args, **_kwargs: None)
monkeypatch.setattr(module, "server_error_response", lambda e: {"code": 500, "message": str(e)})
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["code"] == 500
assert "boom" in res["message"]
monkeypatch.setattr(module.DocumentService, "get_by_id", lambda _id: (False, None))
_patch_docstore(monkeypatch, module, update=lambda *_args, **_kwargs: None, delete=lambda *_args, **_kwargs: None)
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Dataset created failed"
# cover token reset + docStore deletion branch
doc_reset = _DummyDoc(token_num=3, chunk_num=2, parser_id="naive", run=0)
monkeypatch.setattr(module.DocumentService, "query", lambda **_kwargs: [doc_reset])
monkeypatch.setattr(module.DocumentService, "update_by_id", lambda *_args, **_kwargs: True)
monkeypatch.setattr(module.DocumentService, "increment_chunk_num", lambda *_args, **_kwargs: True)
monkeypatch.setattr(module.DocumentService, "get_by_id", lambda _id: (True, doc_reset))
_patch_docstore(monkeypatch, module, delete=lambda *_args, **_kwargs: None, update=lambda *_args, **_kwargs: None)
monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"chunk_method": "manual"}))
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["code"] == 0
def _raise_operational_error(_id):
raise module.OperationalError("db down")
monkeypatch.setattr(module.DocumentService, "get_by_id", _raise_operational_error)
res = _run(module.update_doc.__wrapped__("tenant-1", "ds-1", "doc-1"))
assert res["message"] == "Database operation failed"
def test_download_and_download_doc_errors(self, monkeypatch):
module = _load_doc_module(monkeypatch)
_patch_send_file(monkeypatch, module)

View File

@@ -21,16 +21,17 @@ from configs import DOCUMENT_NAME_LIMIT, INVALID_API_TOKEN, INVALID_ID_32
from libs.auth import RAGFlowHttpApiAuth
from configs import DEFAULT_PARSER_CONFIG
@pytest.mark.p1
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'>",
),
],
)
@@ -53,13 +54,13 @@ class TestDocumentsUpdated:
),
(
0,
100,
"""AttributeError(\'int\' object has no attribute \'encode\')""",
102,
"Field: <name> - Message: <Input should be a valid string> - Value: <0>",
),
(
None,
100,
"""AttributeError(\'NoneType\' object has no attribute \'encode\')""",
"AttributeError('NoneType' object has no attribute 'encode')",
),
(
"",
@@ -93,7 +94,7 @@ class TestDocumentsUpdated:
else:
assert res["message"] == expected_message
@pytest.mark.p3
@pytest.mark.p2
@pytest.mark.parametrize(
"document_id, expected_code, expected_message",
[
@@ -110,7 +111,7 @@ class TestDocumentsUpdated:
assert res["code"] == expected_code
assert res["message"] == expected_message
@pytest.mark.p3
@pytest.mark.p2
@pytest.mark.parametrize(
"dataset_id, expected_code, expected_message",
[
@@ -127,10 +128,28 @@ class TestDocumentsUpdated:
assert res["code"] == expected_code
assert res["message"] == expected_message
@pytest.mark.p3
@pytest.mark.p2
@pytest.mark.parametrize(
"meta_fields, expected_code, expected_message",
[({"test": "test"}, 0, ""), ("test", 102, "meta_fields must be a dictionary")],
[
# Valid meta_fields
({"test": "test"}, 0, ""),
# Valid meta_fields with various types
({"author": "alice", "year": 2024}, 0, ""),
({"tags": ["tag1", "tag2"]}, 0, ""),
({"count": 42, "price": 19.99}, 0, ""),
# Invalid type - string instead of dict
("test", 102, "Field: <meta_fields> - Message: <Input should be a valid dictionary> - Value: <test>"),
# Invalid type - list instead of dict
([], 102, "Field: <meta_fields> - Message: <Input should be a valid dictionary> - Value: <[]>"),
# Invalid - list containing objects (unsupported type in list)
({"tags": [{"x": {"a": "b"}}]}, 102, "Field: <meta_fields> - Message: <The type is not supported in list: [{'x': {'a': 'b'}}]> - Value: <{'tags': [{'x': {'a': 'b'}}]}>"),
({"tags": [{"x": 1}]}, 102, "Field: <meta_fields> - Message: <The type is not supported in list: [{'x': 1}]> - Value: <{'tags': [{'x': 1}]}>"),
# Invalid - nested object with unsupported type
({"obj": {"x": 1}}, 102, "Field: <meta_fields> - Message: <The type is not supported: {'x': 1}> - Value: <{'obj': {'x': 1}}>"),
# Valid types of list
({"tags": [2, 1]}, 0, ""),
],
)
def test_meta_fields(self, HttpApiAuth, add_documents, meta_fields, expected_code, expected_message):
dataset_id, document_ids = add_documents
@@ -139,7 +158,22 @@ class TestDocumentsUpdated:
res = list_documents(HttpApiAuth, dataset_id, {"id": document_ids[0]})
assert res["data"]["docs"][0]["meta_fields"] == meta_fields
else:
assert res["message"] == expected_message
assert expected_message in res["message"] or res["message"] == expected_message
@pytest.mark.p2
@pytest.mark.parametrize(
"meta_fields, expected_code, expected_message",
[
# Test with invalid document ID (not owned by dataset)
({"author": "alice"}, 102, "The dataset doesn't own the document."),
],
)
def test_meta_fields_invalid_document(self, HttpApiAuth, add_documents, meta_fields, expected_code, expected_message):
"""Test meta_fields update with invalid document ID"""
dataset_id, _ = add_documents
res = update_document(HttpApiAuth, dataset_id, "invalid_doc_id_12345678901234567890", {"meta_fields": meta_fields})
assert res["code"] == expected_code
assert expected_message in res["message"]
@pytest.mark.p2
@pytest.mark.parametrize(
@@ -297,6 +331,30 @@ class TestDocumentsUpdated:
assert res["code"] == expected_code
assert res["message"] == expected_message
@pytest.mark.p2
@pytest.mark.parametrize(
"payload, expected_code, expected_message",
[
({"chunk_count": 100}, 102, "Can't change `chunk_count`."),
({"token_count": 100}, 102, "Can't change `token_count`."),
({"progress": 2.0}, 102, "Field: <progress> - Message: <Input should be less than or equal to 1> - Value: <2.0>"),
({"progress": 1.0}, 102, "Can't change `progress`."),
({"meta_fields": []}, 102, "Field: <meta_fields> - Message: <Input should be a valid dictionary> - Value: <[]>"),
],
)
def test_update_doc_guards_and_error_paths(self, HttpApiAuth, add_documents, payload, expected_code, expected_message):
"""
Test various guard conditions and error paths for document update functionality.
This includes testing for invalid dataset ownership, document ownership,
immutable fields, and validation errors.
"""
dataset_id, document_ids = add_documents
document_id = document_ids[0]
res = update_document(HttpApiAuth, dataset_id, document_id, payload)
assert res["code"] == expected_code
if expected_message:
assert expected_message in res["message"] or res["message"] == expected_message
DEFAULT_PARSER_CONFIG_FOR_TEST = {
@@ -328,6 +386,7 @@ DEFAULT_PARSER_CONFIG_FOR_TEST = {
},
}
class TestUpdateDocumentParserConfig:
@pytest.mark.p2
@pytest.mark.parametrize(
@@ -343,37 +402,32 @@ class TestUpdateDocumentParserConfig:
pytest.param(
"naive",
{"chunk_token_num": -1},
100,
"AssertionError('chunk_token_num should be in range from 1 to 100000000')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.chunk_token_num> - Message: <Input should be greater than or equal to 1> - Value: <-1>",
),
pytest.param(
"naive",
{"chunk_token_num": 0},
100,
"AssertionError('chunk_token_num should be in range from 1 to 100000000')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.chunk_token_num> - Message: <Input should be greater than or equal to 1> - Value: <0>",
),
pytest.param(
"naive",
{"chunk_token_num": 100000000},
100,
"AssertionError('chunk_token_num should be in range from 1 to 100000000')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.chunk_token_num> - Message: <Input should be less than or equal to 2048> - Value: <100000000>",
),
pytest.param(
"naive",
{"chunk_token_num": 3.14},
102,
"",
marks=pytest.mark.skip(reason="issues/6098"),
"Field: <parser_config.chunk_token_num> - Message: <Input should be a valid integer> - Value: <3.14>",
),
pytest.param(
"naive",
{"chunk_token_num": "1024"},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.chunk_token_num> - Message: <Input should be a valid integer> - Value: <1024>",
),
(
"naive",
@@ -392,148 +446,135 @@ class TestUpdateDocumentParserConfig:
pytest.param(
"naive",
{"html4excel": 1},
100,
"AssertionError('html4excel should be True or False')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.html4excel> - Message: <Input should be a valid boolean> - Value: <1>",
),
("naive", {"delimiter": ""}, 102, "Field: <parser_config.delimiter> - Message: <String should have at least 1 character> - Value: <>"),
("naive", {"delimiter": "`##`"}, 0, ""),
pytest.param(
"naive",
{"delimiter": 1},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.delimiter> - Message: <Input should be a valid string> - Value: <1>",
),
pytest.param(
"naive",
{"task_page_size": -1},
100,
"AssertionError('task_page_size should be in range from 1 to 100000000')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.task_page_size> - Message: <Input should be greater than or equal to 1> - Value: <-1>",
),
pytest.param(
"naive",
{"task_page_size": 0},
100,
"AssertionError('task_page_size should be in range from 1 to 100000000')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.task_page_size> - Message: <Input should be greater than or equal to 1> - Value: <0>",
),
pytest.param(
"naive",
{"task_page_size": 100000000},
100,
"AssertionError('task_page_size should be in range from 1 to 100000000')",
marks=pytest.mark.skip(reason="issues/6098"),
0,
"",
),
pytest.param(
"naive",
{"task_page_size": 3.14},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.task_page_size> - Message: <Input should be a valid integer> - Value: <3.14>",
),
pytest.param(
"naive",
{"task_page_size": "1024"},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.task_page_size> - Message: <Input should be a valid integer> - Value: <1024>",
),
(
"naive",
{
"raptor": {
"use_raptor": {"a": "b"},
}
},
102,
"Field: <parser_config.raptor.use_raptor> - Message: <Input should be a valid boolean> - Value: <{'a': 'b'}>",
),
("naive", {"raptor": {"use_raptor": {
"a": "b"
},}}, 102, "Field: <parser_config.raptor.use_raptor> - Message: <Input should be a valid boolean> - Value: <{'a': 'b'}>"),
("naive", {"raptor": {"use_raptor": False}}, 0, ""),
pytest.param(
"naive",
{"invalid_key": "invalid_value"},
100,
"""AssertionError("Abnormal \'parser_config\'. Invalid key: invalid_key")""",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.invalid_key> - Message: <Extra inputs are not permitted> - Value: <invalid_value>",
),
pytest.param(
"naive",
{"auto_keywords": -1},
100,
"AssertionError('auto_keywords should be in range from 0 to 32')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.auto_keywords> - Message: <Input should be greater than or equal to 0> - Value: <-1>",
),
pytest.param(
"naive",
{"auto_keywords": 32},
100,
"AssertionError('auto_keywords should be in range from 0 to 32')",
marks=pytest.mark.skip(reason="issues/6098"),
),
pytest.param(
"naive",
{"auto_questions": 3.14},
100,
0,
"",
marks=pytest.mark.skip(reason="issues/6098"),
),
pytest.param(
"naive",
{"auto_keywords": "1024"},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.auto_keywords> - Message: <Input should be a valid integer> - Value: <1024>",
),
pytest.param(
"naive",
{"auto_keywords": 3.14},
102,
"Field: <parser_config.auto_keywords> - Message: <Input should be a valid integer> - Value: <3.14>",
),
pytest.param(
"naive",
{"auto_questions": -1},
100,
"AssertionError('auto_questions should be in range from 0 to 10')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.auto_questions> - Message: <Input should be greater than or equal to 0> - Value: <-1>",
),
pytest.param(
"naive",
{"auto_questions": 10},
100,
"AssertionError('auto_questions should be in range from 0 to 10')",
marks=pytest.mark.skip(reason="issues/6098"),
0,
"",
),
pytest.param(
"naive",
{"auto_questions": 3.14},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.auto_questions> - Message: <Input should be a valid integer> - Value: <3.14>",
),
pytest.param(
"naive",
{"auto_questions": "1024"},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.auto_questions> - Message: <Input should be a valid integer> - Value: <1024>",
),
pytest.param(
"naive",
{"topn_tags": -1},
100,
"AssertionError('topn_tags should be in range from 0 to 10')",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.topn_tags> - Message: <Input should be greater than or equal to 1> - Value: <-1>",
),
pytest.param(
"naive",
{"topn_tags": 10},
100,
"AssertionError('topn_tags should be in range from 0 to 10')",
marks=pytest.mark.skip(reason="issues/6098"),
0,
"",
),
pytest.param(
"naive",
{"topn_tags": 3.14},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.topn_tags> - Message: <Input should be a valid integer> - Value: <3.14>",
),
pytest.param(
"naive",
{"topn_tags": "1024"},
100,
"",
marks=pytest.mark.skip(reason="issues/6098"),
102,
"Field: <parser_config.topn_tags> - Message: <Input should be a valid integer> - Value: <1024>",
),
],
)