mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-08-01 21:37:33 +08:00
Test: release Go-proxy RESTful contract tests verified passing in Go mode (#17468)
### Summary Aligns Go and Python error codes/messages so both backends honor the same RESTful API contract, removing implementation-specific error leaks (MySQL errors, `ValueError`, `AttributeError`, Gin validator format) in favor of clean business error codes. **Chat list** — invalid `orderby` now returns code 101 (was: raw Python `AttributeError` code 100); invalid `page`/`page_size` values fall back to defaults (was: raw `ValueError`/`ProgrammingError` code 100). **Dataset create/update/delete** — adds UUID validation (101), extra-field rejection (101), duplicate-id detection (101), content-type / JSON-syntax / object-shape checks (101), and "lacks permission" for nonexistent datasets (IDOR). Create auto-deduplicates dataset names. Pagerank updates tolerate a missing ES index. List response includes `parser_config` and `pagerank`. **Session list/update** — adds filtering, sorting, and pagination support. Empty payloads are valid no-ops. Authorization errors map to code 109. **Chunk list** — doc object uses Python key names (`chunk_count`, `dataset_id`, `chunk_method`, run text status). Add validates list element types. **Document update** — adds `chunk_method` alias, pydantic-style Field error messages, metadata index auto-create with refresh, and "These documents do not belong to dataset" messages. List validates `metadata_condition` and reports ownership errors for unmatched name/id filters. **Search completion** — `kb_ids` ownership failure returns code 102 instead of 109. Released 31 contract tests from `GO_ONLY_SKIPS` (all verified passing on both Go and Python backends with real LLM keys).
This commit is contained in:
@@ -113,8 +113,10 @@ type ArtifactResponse struct {
|
||||
}
|
||||
|
||||
type UpdateDatasetDocumentRequest struct {
|
||||
Name *string `json:"name"`
|
||||
ParserID *string `json:"parser_id"`
|
||||
Name *string `json:"name"`
|
||||
ParserID *string `json:"parser_id"`
|
||||
// ChunkMethod is the public alias for parser_id (Python UpdateDocumentReq).
|
||||
ChunkMethod *string `json:"chunk_method"`
|
||||
ChunkCount *int64 `json:"chunk_count"`
|
||||
TokenCount *int64 `json:"token_count"`
|
||||
PipelineID *string `json:"pipeline_id"`
|
||||
|
||||
@@ -194,7 +194,7 @@ func (s *DocumentService) DeleteDocument(ctx context.Context, id string) error {
|
||||
func (s *DocumentService) DeleteDocuments(ctx context.Context, ids []string, deleteAll bool, datasetID, userID string) (int, error) {
|
||||
// 1. Check dataset is accessible by the user
|
||||
if !s.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) {
|
||||
return 0, fmt.Errorf("you don't own the dataset %s", datasetID)
|
||||
return 0, fmt.Errorf("You don't own the dataset %s.", datasetID)
|
||||
}
|
||||
|
||||
// 2. Resolve document IDs
|
||||
|
||||
@@ -195,6 +195,17 @@ func (s *DocumentService) UpdateDatasetDocument(ctx context.Context, userID, dat
|
||||
reparsePipelineID = req.PipelineID
|
||||
}
|
||||
}
|
||||
} else if present["chunk_method"] && req.ChunkMethod != nil {
|
||||
// chunk_method is the public alias for a direct parser change; it does
|
||||
// not require parse_type (mirrors Python's update_chunk_method).
|
||||
if p := strings.TrimSpace(*req.ChunkMethod); !strings.EqualFold(p, doc.ParserID) {
|
||||
reparseParserID = &p
|
||||
empty := ""
|
||||
reparsePipelineID = &empty
|
||||
} else if doc.PipelineID != nil && *doc.PipelineID != "" {
|
||||
empty := ""
|
||||
reparsePipelineID = &empty
|
||||
}
|
||||
}
|
||||
if reparseParserID != nil || reparsePipelineID != nil {
|
||||
if err = s.resetDocumentForReparse(ctx, doc, kb.TenantID, reparseParserID, reparsePipelineID); err != nil {
|
||||
@@ -229,17 +240,17 @@ func (s *DocumentService) validateDatasetDocumentUpdate(ctx context.Context, dat
|
||||
return common.CodeDataError, errors.New("invalid request payload")
|
||||
}
|
||||
if present["chunk_count"] && req.ChunkCount != nil && *req.ChunkCount != 0 && *req.ChunkCount != doc.ChunkNum {
|
||||
return common.CodeDataError, errors.New("can't change `chunk_count`")
|
||||
return common.CodeDataError, errors.New("Can't change `chunk_count`.")
|
||||
}
|
||||
if present["token_count"] && req.TokenCount != nil && *req.TokenCount != 0 && *req.TokenCount != doc.TokenNum {
|
||||
return common.CodeDataError, errors.New("can't change `token_count`")
|
||||
return common.CodeDataError, errors.New("Can't change `token_count`.")
|
||||
}
|
||||
if present["progress"] && req.Progress != nil {
|
||||
if *req.Progress > 1 {
|
||||
return common.CodeDataError, fmt.Errorf("Field: <progress> - Message: <Input should be less than or equal to 1> - Value: <%v>", *req.Progress)
|
||||
return common.CodeDataError, fmt.Errorf("Field: <progress> - Message: <Input should be less than or equal to 1> - Value: <%s>", pythonFloatRepr(*req.Progress))
|
||||
}
|
||||
if *req.Progress != 0 && math.Abs(*req.Progress-doc.Progress) > 1e-9 {
|
||||
return common.CodeDataError, errors.New("can't change `progress`")
|
||||
return common.CodeDataError, errors.New("Can't change `progress`.")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -249,6 +260,19 @@ func (s *DocumentService) validateDatasetDocumentUpdate(ctx context.Context, dat
|
||||
}
|
||||
}
|
||||
|
||||
if present["chunk_method"] {
|
||||
if req.ChunkMethod == nil || strings.TrimSpace(*req.ChunkMethod) == "" {
|
||||
return common.CodeDataError, errors.New("`chunk_method` (empty string) is not valid")
|
||||
}
|
||||
cm := strings.TrimSpace(*req.ChunkMethod)
|
||||
if !validDocumentChunkMethods[cm] {
|
||||
return common.CodeDataError, fmt.Errorf("Field: <chunk_method> - Message: <`chunk_method` %s doesn't exist> - Value: <%s>", cm, cm)
|
||||
}
|
||||
if (doc.Type == "visual" && cm != "picture") || (isPresentationFile(doc.Name) && cm != "presentation") {
|
||||
return common.CodeDataError, errors.New("not supported yet")
|
||||
}
|
||||
}
|
||||
|
||||
if present["parse_type"] || present["parser_id"] || present["pipeline_id"] {
|
||||
isBuiltin, _, err := service.ValidateParseTypeMode(req.ParseType, req.ParserID, req.PipelineID)
|
||||
if err != nil {
|
||||
@@ -263,9 +287,13 @@ func (s *DocumentService) validateDatasetDocumentUpdate(ctx context.Context, dat
|
||||
}
|
||||
}
|
||||
}
|
||||
if present["name"] && req.Name != nil {
|
||||
if err := s.validateDocumentName(ctx, doc, *req.Name); err != nil {
|
||||
return common.CodeDataError, err
|
||||
if present["name"] {
|
||||
// An explicit null name is a type error, not an unset field.
|
||||
if req.Name == nil {
|
||||
return common.CodeDataError, errors.New("Field: <name> - Message: <Input should be a valid string> - Value: <None>")
|
||||
}
|
||||
if code, err := s.validateDocumentName(ctx, doc, *req.Name); err != nil {
|
||||
return code, err
|
||||
}
|
||||
}
|
||||
|
||||
@@ -278,12 +306,11 @@ func (s *DocumentService) validateDatasetDocumentUpdate(ctx context.Context, dat
|
||||
return common.CodeSuccess, nil
|
||||
}
|
||||
|
||||
func (s *DocumentService) validateDocumentName(ctx context.Context, doc *entity.Document, newName string) error {
|
||||
if strings.TrimSpace(newName) == "" {
|
||||
return errors.New("file name can't be empty")
|
||||
}
|
||||
// validateDocumentName mirrors Python's validate_document_name: length check
|
||||
// (101), then extension check (101), then duplicate check (102).
|
||||
func (s *DocumentService) validateDocumentName(ctx context.Context, doc *entity.Document, newName string) (common.ErrorCode, error) {
|
||||
if len([]byte(newName)) > 255 {
|
||||
return errors.New("file name must be 255 bytes or less")
|
||||
return common.CodeArgumentError, errors.New("File name must be 255 bytes or less.")
|
||||
}
|
||||
|
||||
oldName := ""
|
||||
@@ -292,20 +319,20 @@ func (s *DocumentService) validateDocumentName(ctx context.Context, doc *entity.
|
||||
}
|
||||
|
||||
if strings.ToLower(filepath.Ext(newName)) != strings.ToLower(filepath.Ext(oldName)) {
|
||||
return errors.New("the extension of file can't be changed")
|
||||
return common.CodeArgumentError, errors.New("The extension of file can't be changed")
|
||||
}
|
||||
|
||||
docs, err := s.documentDAO.GetByNameAndKBID(ctx, dao.DB, newName, doc.KbID)
|
||||
if err != nil {
|
||||
return err
|
||||
return common.CodeServerError, err
|
||||
}
|
||||
for _, d := range docs {
|
||||
if d.ID != doc.ID && d.Name != nil && *d.Name == newName {
|
||||
return errors.New("duplicated document name in the same dataset")
|
||||
return common.CodeDataError, errors.New("Duplicated document name in the same dataset.")
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
return common.CodeSuccess, nil
|
||||
}
|
||||
|
||||
func isPresentationFile(name *string) bool {
|
||||
@@ -331,11 +358,11 @@ func validateMetaFields(meta map[string]any) error {
|
||||
case string, float64, int, int64, float32:
|
||||
continue
|
||||
default:
|
||||
return fmt.Errorf("the type is not supported in list: %v", typed)
|
||||
return fmt.Errorf("Field: <meta_fields> - Message: <The type is not supported in list: %s> - Value: <%s>", pyRepr(typed), pyRepr(map[string]interface{}(meta)))
|
||||
}
|
||||
}
|
||||
default:
|
||||
return fmt.Errorf("the type is not supported: %v", v)
|
||||
return fmt.Errorf("Field: <meta_fields> - Message: <The type is not supported: %s> - Value: <%s>", pyRepr(v), pyRepr(map[string]interface{}(meta)))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -447,3 +474,59 @@ func mapDocumentRunStatus(run *string) string {
|
||||
return "UNSTART"
|
||||
}
|
||||
}
|
||||
|
||||
// validDocumentChunkMethods mirrors Python's UpdateDocumentReq chunk_method set.
|
||||
var validDocumentChunkMethods = map[string]bool{
|
||||
"naive": true, "manual": true, "qa": true, "table": true, "paper": true,
|
||||
"book": true, "laws": true, "presentation": true, "picture": true,
|
||||
"one": true, "knowledge_graph": true, "email": true, "tag": true,
|
||||
}
|
||||
|
||||
// pythonFloatRepr formats a float the way Python's str()/repr() does:
|
||||
// integral floats keep a trailing ".0".
|
||||
func pythonFloatRepr(v float64) string {
|
||||
if v == math.Trunc(v) {
|
||||
return fmt.Sprintf("%.1f", v)
|
||||
}
|
||||
return strconv.FormatFloat(v, 'g', -1, 64)
|
||||
}
|
||||
|
||||
// pyRepr renders a decoded JSON value the way Python's repr() does, for
|
||||
// pydantic-style "Field: <f> - Message: <m> - Value: <v>" contract messages.
|
||||
func pyRepr(v interface{}) string {
|
||||
switch typed := v.(type) {
|
||||
case nil:
|
||||
return "None"
|
||||
case string:
|
||||
return "'" + strings.ReplaceAll(typed, "'", "\\'") + "'"
|
||||
case bool:
|
||||
if typed {
|
||||
return "True"
|
||||
}
|
||||
return "False"
|
||||
case float64:
|
||||
if typed == math.Trunc(typed) {
|
||||
return strconv.FormatInt(int64(typed), 10)
|
||||
}
|
||||
return strconv.FormatFloat(typed, 'g', -1, 64)
|
||||
case float32:
|
||||
return pyRepr(float64(typed))
|
||||
case int:
|
||||
return strconv.Itoa(typed)
|
||||
case int64:
|
||||
return strconv.FormatInt(typed, 10)
|
||||
case []interface{}:
|
||||
parts := make([]string, len(typed))
|
||||
for i, item := range typed {
|
||||
parts[i] = pyRepr(item)
|
||||
}
|
||||
return "[" + strings.Join(parts, ", ") + "]"
|
||||
case map[string]interface{}:
|
||||
parts := make([]string, 0, len(typed))
|
||||
for k, item := range typed {
|
||||
parts = append(parts, pyRepr(k)+": "+pyRepr(item))
|
||||
}
|
||||
return "{" + strings.Join(parts, ", ") + "}"
|
||||
}
|
||||
return fmt.Sprintf("%v", v)
|
||||
}
|
||||
|
||||
@@ -48,6 +48,12 @@ func (s *DocumentService) SetDocumentMetadata(ctx context.Context, docID string,
|
||||
return fmt.Errorf("failed to get tenant ID: %w", err)
|
||||
}
|
||||
|
||||
// Ensure the metadata store exists before writing (service-layer
|
||||
// create-on-first-write logic; the engine layer assumes it exists).
|
||||
if err := s.metadataSvc.EnsureMetadataStore(ctx, tenantID); err != nil {
|
||||
return fmt.Errorf("failed to ensure metadata store: %w", err)
|
||||
}
|
||||
|
||||
if err = s.docEngine.UpdateMetadata(ctx, docID, doc.KbID, meta, tenantID); err != nil {
|
||||
return fmt.Errorf("failed to update metadata: %w", err)
|
||||
}
|
||||
@@ -527,16 +533,27 @@ func (s *DocumentService) patchDocumentMetadata(ctx context.Context, docID strin
|
||||
}
|
||||
}
|
||||
|
||||
updateFields := make(map[string]interface{})
|
||||
// Check if anything actually changed.
|
||||
changed := false
|
||||
for key, value := range after {
|
||||
if !reflect.DeepEqual(before[key], value) {
|
||||
updateFields[key] = value
|
||||
changed = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if len(updateFields) == 0 {
|
||||
if !changed && len(deleteKeys) == 0 {
|
||||
return nil
|
||||
}
|
||||
return s.SetDocumentMetadata(ctx, docID, updateFields)
|
||||
|
||||
// If 'after' is empty, all keys were deleted — the record has already
|
||||
// been cleaned up by DeleteDocumentMetadata above; skip the write.
|
||||
if len(after) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Send the complete 'after' map — UpdateMetadata does a full replace,
|
||||
// not a merge, so a partial delta would wipe unchanged keys.
|
||||
return s.SetDocumentMetadata(ctx, docID, after)
|
||||
}
|
||||
|
||||
// BatchUpdateDocumentMetadatas implements the shared logic for
|
||||
@@ -576,7 +593,7 @@ func (s *DocumentService) BatchUpdateDocumentMetadatas(
|
||||
}
|
||||
}
|
||||
if len(invalidIDs) > 0 {
|
||||
return nil, common.CodeDataError, fmt.Errorf("these documents do not belong to dataset %s: %s",
|
||||
return nil, common.CodeDataError, fmt.Errorf("These documents do not belong to dataset %s: %s",
|
||||
datasetID, strings.Join(invalidIDs, ", "))
|
||||
}
|
||||
for _, id := range selector.DocumentIDs {
|
||||
|
||||
@@ -257,6 +257,15 @@ func (s *DocumentService) StopParseDocuments(ctx context.Context, datasetID stri
|
||||
|
||||
docs, err := s.validateDocsInDataset(ctx, deduped, datasetID)
|
||||
if err != nil {
|
||||
// Mirror the Python parse/stop endpoint's "Documents not found" message.
|
||||
var notInDataset *documentsNotInDatasetError
|
||||
if errors.As(err, ¬InDataset) {
|
||||
quoted := make([]string, len(notInDataset.ids))
|
||||
for i, id := range notInDataset.ids {
|
||||
quoted[i] = "'" + id + "'"
|
||||
}
|
||||
return nil, fmt.Errorf("Documents not found: [%s]", strings.Join(quoted, ", "))
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -277,6 +286,18 @@ func (s *DocumentService) StopParseDocuments(ctx context.Context, datasetID stri
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// documentsNotInDatasetError carries the ids that are missing from (or do not
|
||||
// belong to) a dataset so each endpoint can format its own contract message.
|
||||
type documentsNotInDatasetError struct {
|
||||
datasetID string
|
||||
ids []string
|
||||
}
|
||||
|
||||
// Error mirrors the Python delete endpoint's message.
|
||||
func (e *documentsNotInDatasetError) Error() string {
|
||||
return fmt.Sprintf("These documents do not belong to dataset %s or Document not found: %s", e.datasetID, strings.Join(e.ids, ", "))
|
||||
}
|
||||
|
||||
// validateDocsInDataset deduplicates IDs, fetches the documents, and ensures
|
||||
// every document exists and belongs to the given dataset. Returns the resolved
|
||||
// documents.
|
||||
@@ -285,17 +306,26 @@ func (s *DocumentService) validateDocsInDataset(ctx context.Context, docIDs []st
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to fetch documents: %w", err)
|
||||
}
|
||||
invalid := make([]string, 0)
|
||||
if len(docs) != len(docIDs) {
|
||||
return nil, fmt.Errorf("some document IDs not found in dataset %s", datasetID)
|
||||
}
|
||||
var invalid []string
|
||||
for _, d := range docs {
|
||||
if d.KbID != datasetID {
|
||||
invalid = append(invalid, d.ID)
|
||||
found := make(map[string]struct{}, len(docs))
|
||||
for _, d := range docs {
|
||||
found[d.ID] = struct{}{}
|
||||
}
|
||||
for _, id := range docIDs {
|
||||
if _, ok := found[id]; !ok {
|
||||
invalid = append(invalid, id)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
for _, d := range docs {
|
||||
if d.KbID != datasetID {
|
||||
invalid = append(invalid, d.ID)
|
||||
}
|
||||
}
|
||||
}
|
||||
if len(invalid) > 0 {
|
||||
return nil, fmt.Errorf("These documents do not belong to dataset %s: %v", datasetID, invalid)
|
||||
return nil, &documentsNotInDatasetError{datasetID: datasetID, ids: invalid}
|
||||
}
|
||||
return docs, nil
|
||||
}
|
||||
|
||||
@@ -1643,7 +1643,7 @@ func TestUpdateDatasetDocumentRejectsCounterMutation(t *testing.T) {
|
||||
if code != common.CodeDataError {
|
||||
t.Fatalf("code = %v, want %v", code, common.CodeDataError)
|
||||
}
|
||||
if err.Error() != "can't change `chunk_count`" {
|
||||
if err.Error() != "Can't change `chunk_count`." {
|
||||
t.Fatalf("err = %q", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user