mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-07-25 01:43:27 +08:00
## 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.
209 lines
6.8 KiB
Go
209 lines
6.8 KiB
Go
package service
|
|
|
|
import (
|
|
"testing"
|
|
)
|
|
|
|
func TestValidateParseTypeMode_Nil(t *testing.T) {
|
|
_, _, err := ValidateParseTypeMode(nil, nil, nil)
|
|
if err == nil || err.Error() != "parse_type is required" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestValidateParseTypeMode_InvalidZero(t *testing.T) {
|
|
v := 0
|
|
_, _, err := ValidateParseTypeMode(&v, nil, nil)
|
|
if err == nil || err.Error() != "invalid parse_type: 0 (must be 1 or 2)" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestValidateParseTypeMode_InvalidThree(t *testing.T) {
|
|
v := 3
|
|
_, _, err := ValidateParseTypeMode(&v, nil, nil)
|
|
if err == nil || err.Error() != "invalid parse_type: 3 (must be 1 or 2)" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestValidateParseTypeMode_BuiltinMissingParserID(t *testing.T) {
|
|
t.Run("nil parserID", func(t *testing.T) {
|
|
pt := 1
|
|
_, _, err := ValidateParseTypeMode(&pt, nil, nil)
|
|
if err == nil || err.Error() != "parser_id is required when parse_type is BuiltIn" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
})
|
|
t.Run("empty parserID", func(t *testing.T) {
|
|
pt := 1
|
|
empty := ""
|
|
_, _, err := ValidateParseTypeMode(&pt, &empty, nil)
|
|
if err == nil || err.Error() != "parser_id is required when parse_type is BuiltIn" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
})
|
|
t.Run("whitespace parserID", func(t *testing.T) {
|
|
pt := 1
|
|
ws := " "
|
|
_, _, err := ValidateParseTypeMode(&pt, &ws, nil)
|
|
if err == nil || err.Error() != "parser_id is required when parse_type is BuiltIn" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
})
|
|
}
|
|
|
|
func TestValidateParseTypeMode_BuiltinOK(t *testing.T) {
|
|
pt := 1
|
|
pid := "laws"
|
|
builtin, pipeline, err := ValidateParseTypeMode(&pt, &pid, nil)
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
if !builtin || pipeline {
|
|
t.Fatalf("expected builtin=true pipeline=false, got builtin=%v pipeline=%v", builtin, pipeline)
|
|
}
|
|
}
|
|
|
|
func TestValidateParseTypeMode_PipelineMissingPipelineID(t *testing.T) {
|
|
t.Run("nil pipelineID", func(t *testing.T) {
|
|
pt := 2
|
|
_, _, err := ValidateParseTypeMode(&pt, nil, nil)
|
|
if err == nil || err.Error() != "pipeline_id is required when parse_type is Pipeline" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
})
|
|
t.Run("empty pipelineID", func(t *testing.T) {
|
|
pt := 2
|
|
empty := ""
|
|
_, _, err := ValidateParseTypeMode(&pt, nil, &empty)
|
|
if err == nil || err.Error() != "pipeline_id is required when parse_type is Pipeline" {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
})
|
|
}
|
|
|
|
func TestValidateParseTypeMode_PipelineOK(t *testing.T) {
|
|
pt := 2
|
|
pipe := "abc123"
|
|
builtin, pipeline, err := ValidateParseTypeMode(&pt, nil, &pipe)
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
if builtin || !pipeline {
|
|
t.Fatalf("expected builtin=false pipeline=true, got builtin=%v pipeline=%v", builtin, pipeline)
|
|
}
|
|
}
|
|
|
|
func TestResolveParseMode_BuiltinIgnoresPipelineID(t *testing.T) {
|
|
pt := 1
|
|
parserID := "manual"
|
|
pipelineID := "should-be-ignored"
|
|
cur := ParseModeState{ParserID: "naive", PipelineID: strPtr("prior-canvas")}
|
|
isPipeline, effParserID, effPipelineID := ResolveParseMode(&pt, &parserID, &pipelineID, cur)
|
|
if isPipeline {
|
|
t.Fatalf("isPipeline = true, want false (builtin mode)")
|
|
}
|
|
if effParserID != "manual" {
|
|
t.Fatalf("effParserID = %q, want manual", effParserID)
|
|
}
|
|
if effPipelineID != nil {
|
|
t.Fatalf("effPipelineID = %v, want nil (canvas cleared)", effPipelineID)
|
|
}
|
|
}
|
|
|
|
// TestResolveParseMode_PipelineIgnoresParserID reproduces the comment-3 bug
|
|
// contract: parse_type=2 must ignore a dirty req.ParserID and keep isPipeline
|
|
// true so parser_config is cleaned against the canvas DSL, not the builtin DSL.
|
|
func TestResolveParseMode_PipelineIgnoresParserID(t *testing.T) {
|
|
pt := 2
|
|
parserID := "should-be-ignored"
|
|
pipelineID := "1234567890abcdef1234567890abcdef"
|
|
cur := ParseModeState{ParserID: "naive", PipelineID: nil}
|
|
isPipeline, effParserID, effPipelineID := ResolveParseMode(&pt, &parserID, &pipelineID, cur)
|
|
if !isPipeline {
|
|
t.Fatalf("isPipeline = false, want true (pipeline mode)")
|
|
}
|
|
if effParserID != "naive" {
|
|
t.Fatalf("effParserID = %q, want current naive (parser_id not applicable in pipeline mode)", effParserID)
|
|
}
|
|
if effPipelineID == nil || *effPipelineID != pipelineID {
|
|
t.Fatalf("effPipelineID = %v, want %q", effPipelineID, pipelineID)
|
|
}
|
|
}
|
|
|
|
func TestResolveParseMode_BuiltinFallsBackToCurrentParserID(t *testing.T) {
|
|
pt := 1
|
|
cur := ParseModeState{ParserID: "naive", PipelineID: strPtr("prior-canvas")}
|
|
isPipeline, effParserID, effPipelineID := ResolveParseMode(&pt, nil, nil, cur)
|
|
if isPipeline {
|
|
t.Fatalf("isPipeline = true, want false")
|
|
}
|
|
if effParserID != "naive" {
|
|
t.Fatalf("effParserID = %q, want naive", effParserID)
|
|
}
|
|
if effPipelineID != nil {
|
|
t.Fatalf("effPipelineID = %v, want nil", effPipelineID)
|
|
}
|
|
}
|
|
|
|
func TestResolveParseMode_PipelineFallsBackToCurrentPipelineID(t *testing.T) {
|
|
pt := 2
|
|
cur := ParseModeState{ParserID: "naive", PipelineID: strPtr("prior-canvas")}
|
|
isPipeline, effParserID, effPipelineID := ResolveParseMode(&pt, nil, nil, cur)
|
|
if !isPipeline {
|
|
t.Fatalf("isPipeline = false, want true")
|
|
}
|
|
if effParserID != "naive" {
|
|
t.Fatalf("effParserID = %q, want naive", effParserID)
|
|
}
|
|
if effPipelineID == nil || *effPipelineID != "prior-canvas" {
|
|
t.Fatalf("effPipelineID = %v, want prior-canvas", effPipelineID)
|
|
}
|
|
}
|
|
|
|
// TestResolveParseMode_NilParseTypeInheritsCurrent covers the "only
|
|
// parser_config changed" path: no mode switch, inherit current state.
|
|
func TestResolveParseMode_NilParseTypeInheritsCurrent(t *testing.T) {
|
|
t.Run("current is pipeline", func(t *testing.T) {
|
|
cur := ParseModeState{ParserID: "naive", PipelineID: strPtr("canvas-1")}
|
|
isPipeline, effParserID, effPipelineID := ResolveParseMode(nil, nil, nil, cur)
|
|
if !isPipeline {
|
|
t.Fatalf("isPipeline = false, want true")
|
|
}
|
|
if effParserID != "naive" {
|
|
t.Fatalf("effParserID = %q, want naive", effParserID)
|
|
}
|
|
if effPipelineID == nil || *effPipelineID != "canvas-1" {
|
|
t.Fatalf("effPipelineID = %v, want canvas-1", effPipelineID)
|
|
}
|
|
})
|
|
t.Run("current is builtin", func(t *testing.T) {
|
|
cur := ParseModeState{ParserID: "manual", PipelineID: nil}
|
|
isPipeline, effParserID, effPipelineID := ResolveParseMode(nil, nil, nil, cur)
|
|
if isPipeline {
|
|
t.Fatalf("isPipeline = true, want false")
|
|
}
|
|
if effParserID != "manual" {
|
|
t.Fatalf("effParserID = %q, want manual", effParserID)
|
|
}
|
|
if effPipelineID != nil {
|
|
t.Fatalf("effPipelineID = %v, want nil", effPipelineID)
|
|
}
|
|
})
|
|
t.Run("incremental parser_id update applied", func(t *testing.T) {
|
|
cur := ParseModeState{ParserID: "naive", PipelineID: nil}
|
|
newParser := "laws"
|
|
isPipeline, effParserID, effPipelineID := ResolveParseMode(nil, &newParser, nil, cur)
|
|
if isPipeline {
|
|
t.Fatalf("isPipeline = true, want false")
|
|
}
|
|
if effParserID != "laws" {
|
|
t.Fatalf("effParserID = %q, want laws", effParserID)
|
|
}
|
|
if effPipelineID != nil {
|
|
t.Fatalf("effPipelineID = %v, want nil", effPipelineID)
|
|
}
|
|
})
|
|
}
|