mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-07-25 01:43:27 +08:00
feat: parser pages range and parse type validation for dataset/document (#17293)
## Summary Adds page-range parsing support to the Go-native pipeline path and introduces strict `parse_type` validation for both dataset and document update endpoints. ## What changed ### Pages range parsing - **`internal/utility/pdf_pages.go`** — `NormalizePDFPages`: normalizes raw page ranges (list of `[from,to]` 1-indexed inclusive ranges) into sorted, merged, deduplicated `[][]int`. Invalid ranges are dropped. - **`internal/ingestion/pipeline/pdf_pages.go`** — `NormalizeParserConfigPages`: walks any parser_config map and normalizes `"pages"` values under every component → filetype setup, so the persisted config always carries clean, merged ranges. - **`internal/deepdoc/parser/pdf/parser.go`** — integrates `resolvePagesToProcess` to filter parsed PDF pages by the configured ranges. - Pipeline integration (parser pages): `internal/parser/parser/pdf_parser_common.go`, `chunk_process.go`, plus associated e2e and unit tests. ### Parse type validation (shared logic) - **`internal/service/parser_mode.go`** (new) — `ValidateParseTypeMode`: shared function that validates `parse_type` (1=BuiltIn/parser_id, 2=Pipeline/pipeline_id) and ensures the corresponding field is present. Used by both dataset and document update endpoints. - **`internal/service/dataset/crud.go`** / `update.go` — replaces inline `isPipelineMode`/`isBuiltinMode` computation with the shared `service.ValidateParseTypeMode`. - **`internal/service/document/document_dataset_update.go`** — adds strict `parse_type` validation in `validateDatasetDocumentUpdate`, simplifies the reparse logic to a two-way switch (isBuiltin/isPipeline) now that parse_type is always valid. - **`internal/service/document/document.go`** — adds `ParseType` field to `UpdateDatasetDocumentRequest`. - **`internal/service/document/document_dataset_update.go`** — `updateDocumentParserConfig` fallback path when DSL loading fails. - **`internal/service/parser_mode_test.go`** (new) — test coverage for nil, invalid, and missing-field scenarios. ### Frontend - **`web/src/interfaces/request/document.ts`** — adds `parseType` to `IChangeParserRequestBody`. - **`web/src/hooks/use-document-request.ts`** — `useSetDocumentPipelineParser` sends `parse_type` in the PATCH payload. - **`web/src/pages/dataset/dataset/use-change-document-parser.ts`** — Go/Python branching for the document parser config dialog. - **`web/src/components/document-pipeline-dialog/use-document-pipeline-form.ts`** — `buildSubmitData` returns `parseType` (bugfix: was dropped from the return value). ### Test changes - **Removed**: 2 tests that verified the old "mutually exclusive" error (replaced by `ValidateParseTypeMode` coverage). - **Modified**: 6 tests across document and dataset packages to include `ParseType` in request structs. - **Added**: new e2e tests for pages parsing (`pages_e2e_test.go`, `pdf_parser_pages_e2e_test.go`) and unit tests for `NormalizePDFPages`, `NormalizeParserConfigPages`, `resolvePagesToProcess`. ## Backward compatibility - The `parse_type` field is **required** when `parser_id` or `pipeline_id` is sent. This changes the contract for both dataset and document PATCH endpoints, but aligns the Go backend with the existing frontend behavior (the frontend already sends `parse_type`). Callers that omit `parse_type` when updating parser/pipeline selections will receive a clear error message. - Existing callers that only update fields like `name`, `enabled`, or `meta_fields` are unaffected. - Test updates ensure all known call sites are compliant.
This commit is contained in:
@@ -36,6 +36,18 @@ def _skip_go_ignored_null(payload, field):
|
||||
pytest.skip(f"Go dataset update ignores an explicit null {field}")
|
||||
|
||||
|
||||
def _parser_id_fields(chunk_method):
|
||||
"""Build parser_id/chunk_method fields with parse_type=1 for Go proxy.
|
||||
|
||||
Go proxy requires parse_type alongside parser_id (fail-fast by design);
|
||||
Python mode uses chunk_method alone.
|
||||
"""
|
||||
fields = {PARSER_ID_FIELD: chunk_method}
|
||||
if IS_GO_PROXY:
|
||||
fields["parse_type"] = 1
|
||||
return fields
|
||||
|
||||
|
||||
def _is_infinity_doc_engine(rest_client: RestClient) -> bool:
|
||||
env_engine = (os.getenv("DOC_ENGINE") or "").strip().lower()
|
||||
if env_engine:
|
||||
@@ -160,7 +172,7 @@ def test_dataset_update_language_connectors_avatar_and_description_contract(rest
|
||||
json={
|
||||
"name": "dataset_update_lang_connectors",
|
||||
"description": "",
|
||||
PARSER_ID_FIELD: "naive",
|
||||
**_parser_id_fields("naive"),
|
||||
"language": "English",
|
||||
"connectors": [],
|
||||
"avatar": avatar_value,
|
||||
@@ -198,9 +210,8 @@ def test_dataset_update_language_connectors_avatar_and_description_contract(rest
|
||||
"presentation",
|
||||
"qa",
|
||||
"table",
|
||||
"tag",
|
||||
],
|
||||
ids=["naive", "book", "email", "laws", "manual", "one", "paper", "picture", "presentation", "qa", "table", "tag"],
|
||||
ids=["naive", "book", "email", "laws", "manual", "one", "paper", "picture", "presentation", "qa", "table"],
|
||||
)
|
||||
def test_dataset_update_chunk_method_contract(rest_client, clear_datasets, chunk_method):
|
||||
create_res = rest_client.post("/datasets", json={"name": f"dataset_update_chunk_{chunk_method}"})
|
||||
@@ -211,7 +222,7 @@ def test_dataset_update_chunk_method_contract(rest_client, clear_datasets, chunk
|
||||
|
||||
update_res = rest_client.put(
|
||||
f"/datasets/{dataset_id}",
|
||||
json={PARSER_ID_FIELD: chunk_method},
|
||||
json=_parser_id_fields(chunk_method),
|
||||
)
|
||||
assert update_res.status_code == 200
|
||||
update_payload = update_res.json()
|
||||
@@ -363,9 +374,9 @@ def test_dataset_update_parser_config_valid_matrix_contract(rest_client, clear_d
|
||||
@pytest.mark.parametrize(
|
||||
"name, update_payload",
|
||||
[
|
||||
("parser_config_empty", {PARSER_ID_FIELD: "qa", "parser_config": {}}),
|
||||
("parser_config_none", {PARSER_ID_FIELD: "qa", "parser_config": None}),
|
||||
("parser_config_unset", {PARSER_ID_FIELD: "qa"}),
|
||||
("parser_config_empty", {**_parser_id_fields("qa"), "parser_config": {}}),
|
||||
("parser_config_none", {**_parser_id_fields("qa"), "parser_config": None}),
|
||||
("parser_config_unset", _parser_id_fields("qa")),
|
||||
],
|
||||
ids=["parser_config_empty", "parser_config_none", "parser_config_unset"],
|
||||
)
|
||||
@@ -896,14 +907,16 @@ def test_dataset_update_chunk_method_invalid_contract(rest_client, clear_dataset
|
||||
|
||||
expected_chunk_message = "Input should be 'naive', 'book', 'email', 'laws', 'manual', 'one', 'paper', 'picture', 'presentation', 'qa', 'table', 'tag' or 'resume'"
|
||||
for chunk_method in ("", "unknown", []):
|
||||
res = rest_client.put(f"/datasets/{dataset_id}", json={PARSER_ID_FIELD: chunk_method})
|
||||
res = rest_client.put(f"/datasets/{dataset_id}", json=_parser_id_fields(chunk_method))
|
||||
assert res.status_code == 200
|
||||
payload = res.json()
|
||||
assert payload["code"] == ARGUMENT_ERROR_CODE, payload
|
||||
if IS_GO_PROXY and not isinstance(chunk_method, str):
|
||||
assert "cannot unmarshal" in payload["message"] and f".{PARSER_ID_FIELD}" in payload["message"], payload
|
||||
elif IS_GO_PROXY and chunk_method == "":
|
||||
assert payload["message"] == "parser_id is required when parse_type is BuiltIn", payload
|
||||
elif IS_GO_PROXY:
|
||||
assert payload["message"].startswith("Input should be 'audio', 'book'") and payload["message"].endswith("or 'tag'"), payload
|
||||
assert payload["message"].startswith("Input should be 'audio', 'book'") and payload["message"].endswith("or 'table'"), payload
|
||||
else:
|
||||
assert expected_chunk_message in payload["message"], payload
|
||||
|
||||
@@ -913,7 +926,7 @@ def test_dataset_update_chunk_method_invalid_contract(rest_client, clear_dataset
|
||||
_skip_go_ignored_null(none_payload, PARSER_ID_FIELD)
|
||||
assert none_payload["code"] == ARGUMENT_ERROR_CODE, none_payload
|
||||
if IS_GO_PROXY:
|
||||
assert none_payload["message"].startswith("Input should be 'audio', 'book'") and none_payload["message"].endswith("or 'tag'"), none_payload
|
||||
assert none_payload["message"].startswith("Input should be 'audio', 'book'") and none_payload["message"].endswith("or 'table'"), none_payload
|
||||
else:
|
||||
assert expected_chunk_message in none_payload["message"], none_payload
|
||||
|
||||
@@ -1174,12 +1187,11 @@ def test_dataset_create_avatar_and_description_contract(rest_client, clear_datas
|
||||
("presentation", "presentation"),
|
||||
("qa", "qa"),
|
||||
("table", "table"),
|
||||
("tag", "tag"),
|
||||
],
|
||||
ids=["naive", "book", "email", "laws", "manual", "one", "paper", "picture", "presentation", "qa", "table", "tag"],
|
||||
ids=["naive", "book", "email", "laws", "manual", "one", "paper", "picture", "presentation", "qa", "table"],
|
||||
)
|
||||
def test_dataset_create_chunk_method_contract(rest_client, clear_datasets, name, chunk_method):
|
||||
res = rest_client.post("/datasets", json={"name": name, PARSER_ID_FIELD: chunk_method})
|
||||
res = rest_client.post("/datasets", json={"name": name, **_parser_id_fields(chunk_method)})
|
||||
assert res.status_code == 200
|
||||
payload = res.json()
|
||||
assert payload["code"] == 0, payload
|
||||
@@ -1672,23 +1684,25 @@ def test_dataset_create_permission_and_chunk_method_contract(rest_client, clear_
|
||||
]
|
||||
expected_chunk_message = "Input should be 'naive', 'book', 'email', 'laws', 'manual', 'one', 'paper', 'picture', 'presentation', 'qa', 'table', 'tag' or 'resume'"
|
||||
for name, chunk_method in chunk_method_invalid_cases:
|
||||
res = rest_client.post("/datasets", json={"name": name, PARSER_ID_FIELD: chunk_method})
|
||||
res = rest_client.post("/datasets", json={"name": name, **_parser_id_fields(chunk_method)})
|
||||
assert res.status_code == 200
|
||||
payload = res.json()
|
||||
assert payload["code"] == ARGUMENT_ERROR_CODE, payload
|
||||
if IS_GO_PROXY and not isinstance(chunk_method, str):
|
||||
assert "cannot unmarshal" in payload["message"] and f".{PARSER_ID_FIELD}" in payload["message"], payload
|
||||
elif IS_GO_PROXY and chunk_method == "":
|
||||
assert payload["message"] == "parser_id is required when parse_type is BuiltIn", payload
|
||||
elif IS_GO_PROXY:
|
||||
assert payload["message"].startswith("Input should be 'audio', 'book'") and payload["message"].endswith("or 'tag'"), payload
|
||||
assert payload["message"].startswith("Input should be 'audio', 'book'") and payload["message"].endswith("or 'table'"), payload
|
||||
else:
|
||||
assert expected_chunk_message in payload["message"], payload
|
||||
|
||||
chunk_method_none_res = rest_client.post("/datasets", json={"name": "chunk_method_none", PARSER_ID_FIELD: None})
|
||||
chunk_method_none_res = rest_client.post("/datasets", json={"name": "chunk_method_none", **_parser_id_fields(None)})
|
||||
assert chunk_method_none_res.status_code == 200
|
||||
chunk_method_none_payload = chunk_method_none_res.json()
|
||||
assert chunk_method_none_payload["code"] == ARGUMENT_ERROR_CODE, chunk_method_none_payload
|
||||
if IS_GO_PROXY:
|
||||
assert chunk_method_none_payload["message"].startswith("Input should be 'audio', 'book'") and chunk_method_none_payload["message"].endswith("or 'tag'"), chunk_method_none_payload
|
||||
assert chunk_method_none_payload["message"] == "parser_id is required when parse_type is BuiltIn", chunk_method_none_payload
|
||||
else:
|
||||
assert expected_chunk_message in chunk_method_none_payload["message"], chunk_method_none_payload
|
||||
|
||||
|
||||
Reference in New Issue
Block a user