From 8fc20dd9caac0f0b1d7f86ac6d700cf41ea0944d Mon Sep 17 00:00:00 2001 From: euvre <93761161+euvre@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:58:49 +0800 Subject: [PATCH] Test: release Go-proxy RESTful contract tests verified passing in Go mode (#17468) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 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). --- api/apps/restful_apis/chat_api.py | 39 +++- api/apps/restful_apis/chunk_api.py | 14 +- api/apps/restful_apis/document_api.py | 23 +- cmd/ragflow_server.go | 4 + internal/common/error_code.go | 14 ++ internal/dao/chat_session.go | 27 ++- internal/dao/kb.go | 3 +- internal/engine/elasticsearch/chunk.go | 2 +- internal/engine/elasticsearch/common.go | 4 +- internal/engine/elasticsearch/metadata.go | 75 +++---- internal/engine/infinity/chunk.go | 6 + internal/engine/infinity/metadata.go | 25 +-- internal/engine/types/types.go | 5 + internal/entity/dataset.go | 8 +- internal/handler/api_token.go | 7 +- internal/handler/auth.go | 9 +- internal/handler/auth_test.go | 16 +- internal/handler/chat.go | 8 +- internal/handler/chat_session.go | 65 ++++-- internal/handler/chat_session_test.go | 59 ----- internal/handler/chunk.go | 6 +- internal/handler/dataset.go | 208 ++++++++++++++++-- internal/handler/document.go | 154 ++++++++++++- internal/handler/search.go | 2 +- internal/handler/searchbot.go | 4 +- internal/handler/tenant.go | 8 + internal/router/router_test.go | 4 +- internal/service/chat_session.go | 125 +++++++---- .../service/chat_session_contract_test.go | 4 +- internal/service/chat_session_test.go | 95 +++++++- internal/service/chunk/chunk.go | 35 ++- internal/service/chunk_types.go | 35 ++- internal/service/dataset/create_test.go | 17 +- internal/service/dataset/crud.go | 33 ++- internal/service/dataset/helpers.go | 11 +- internal/service/dataset/update.go | 23 +- internal/service/dataset/update_test.go | 7 +- internal/service/dataset/utils.go | 16 +- internal/service/document/document.go | 6 +- internal/service/document/document_crud.go | 2 +- .../document/document_dataset_update.go | 119 ++++++++-- .../service/document/document_metadata.go | 27 ++- internal/service/document/document_parse.go | 44 +++- internal/service/document/document_test.go | 2 +- internal/service/metadata.go | 21 ++ internal/service/search.go | 3 +- rag/app/picture.py | 14 +- test/testcases/restful_api/conftest.py | 59 +---- test/testcases/restful_api/test_chats.py | 8 +- test/testcases/restful_api/test_chunks.py | 4 +- test/testcases/restful_api/test_documents.py | 22 +- test/testcases/restful_api/test_searches.py | 2 +- test/testcases/restful_api/test_sessions.py | 31 +-- 53 files changed, 1138 insertions(+), 426 deletions(-) diff --git a/api/apps/restful_apis/chat_api.py b/api/apps/restful_apis/chat_api.py index b922d68c9b..13eee8c46f 100644 --- a/api/apps/restful_apis/chat_api.py +++ b/api/apps/restful_apis/chat_api.py @@ -24,6 +24,7 @@ from copy import deepcopy from types import SimpleNamespace from quart import Response, request +from werkzeug.exceptions import BadRequest from api.apps import current_user, login_required from api.apps.restful_apis._generation_params import merge_generation_config, pop_generation_config @@ -454,9 +455,21 @@ async def list_chats(): if chat_id or name: keywords = "" + if orderby not in ("create_time", "update_time", "name"): + return get_json_result(code=RetCode.ARGUMENT_ERROR, message=f"invalid orderby field: {orderby}") + try: - page_number = int(request.args.get("page", 0)) - items_per_page = validate_rest_api_page_size(int(request.args.get("page_size", 0))) + # Invalid or negative pagination values fall back to defaults + # instead of leaking internal conversion/SQL errors. + try: + page_number = max(int(request.args.get("page", 0)), 0) + except (TypeError, ValueError): + page_number = 0 + try: + parsed_page_size = int(request.args.get("page_size", 0)) + except (TypeError, ValueError): + parsed_page_size = 0 + items_per_page = validate_rest_api_page_size(parsed_page_size) if owner_ids: chats, total = await thread_pool_exec( @@ -794,9 +807,22 @@ async def list_sessions(chat_id): message="no authorization", code=RetCode.AUTHENTICATION_ERROR, ) - page_number = int(request.args.get("page", 1)) - items_per_page = validate_rest_api_page_size(int(request.args.get("page_size", 30))) + # Invalid or negative pagination values fall back to defaults + # instead of leaking internal conversion/SQL errors. + try: + page_number = int(request.args.get("page", 1)) + except (TypeError, ValueError): + page_number = 1 + if page_number < 1: + page_number = 1 + try: + parsed_page_size = int(request.args.get("page_size", 30)) + except (TypeError, ValueError): + parsed_page_size = 30 + items_per_page = validate_rest_api_page_size(parsed_page_size) orderby = request.args.get("orderby", "create_time") + if orderby not in ("create_time", "update_time", "name"): + return get_json_result(code=RetCode.ARGUMENT_ERROR, message=f"invalid orderby field: {orderby}") desc = request.args.get("desc", "true").lower() != "false" session_id = request.args.get("id") name = request.args.get("name") @@ -868,7 +894,10 @@ async def delete_sessions(chat_id): if not await _ensure_owned_chat(chat_id): return get_json_result(data=False, message="no authorization", code=RetCode.AUTHENTICATION_ERROR) try: - req = await get_request_json() + try: + req = await get_request_json() + except BadRequest: + return get_json_result(code=RetCode.ARGUMENT_ERROR, message="Malformed JSON syntax: Missing commas/brackets or invalid encoding") if not req: return get_json_result(data={}) diff --git a/api/apps/restful_apis/chunk_api.py b/api/apps/restful_apis/chunk_api.py index c639521624..5dd1ec33e9 100644 --- a/api/apps/restful_apis/chunk_api.py +++ b/api/apps/restful_apis/chunk_api.py @@ -953,10 +953,16 @@ async def add_chunk(tenant_id, dataset_id, document_id): req = await get_request_json() if is_content_empty(req.get("content")): return get_error_data_result(message="`content` is required") - if "important_keywords" in req and not isinstance(req["important_keywords"], list): - return get_error_data_result("`important_keywords` is required to be a list") - if "questions" in req and not isinstance(req["questions"], list): - return get_error_data_result("`questions` is required to be a list") + if "important_keywords" in req: + if not isinstance(req["important_keywords"], list): + return get_error_data_result("`important_keywords` is required to be a list") + if not all(isinstance(k, str) for k in req["important_keywords"]): + return get_error_data_result("`important_keywords` must be a list of strings") + if "questions" in req: + if not isinstance(req["questions"], list): + return get_error_data_result("`questions` is required to be a list") + if not all(isinstance(q, str) for q in req["questions"]): + return get_error_data_result("`questions` must be a list of strings") chunk_id = xxhash.xxh64((req["content"] + document_id).encode("utf-8")).hexdigest() d = { diff --git a/api/apps/restful_apis/document_api.py b/api/apps/restful_apis/document_api.py index 192064101f..3b9b532698 100644 --- a/api/apps/restful_apis/document_api.py +++ b/api/apps/restful_apis/document_api.py @@ -213,6 +213,10 @@ async def update_document(tenant_id, dataset_id, document_id): """ req = await get_request_json() + # An explicit null name is a type error, not an unset field. + if "name" in req and req["name"] is None: + return get_error_data_result(message="Field: - Message: - Value: ") + # Verify ownership and existence of dataset and document if not KnowledgebaseService.query(id=dataset_id, tenant_id=tenant_id): return get_error_data_result(message="you don't own the dataset") @@ -856,10 +860,25 @@ def _get_docs_with_request(req, dataset_id: str): """ q = req.args - page = int(q.get("page", 1)) - page_size = validate_rest_api_page_size(int(q.get("page_size", 30))) + # Invalid or negative pagination values fall back to defaults + # instead of leaking internal conversion/SQL errors. + try: + page = int(q.get("page", 1)) + except (TypeError, ValueError): + page = 1 + if page < 1: + page = 1 + try: + parsed_page_size = int(q.get("page_size", 30)) + except (TypeError, ValueError): + parsed_page_size = 30 + if parsed_page_size < 0: + parsed_page_size = 30 + page_size = validate_rest_api_page_size(parsed_page_size) orderby = q.get("orderby", "create_time") + if orderby not in ("create_time", "update_time", "name"): + return RetCode.ARGUMENT_ERROR, f"invalid orderby field: {orderby}", [], 0 desc = str(q.get("desc", "true")).strip().lower() != "false" keywords = q.get("keywords", "") diff --git a/cmd/ragflow_server.go b/cmd/ragflow_server.go index 043a7498dc..77f4bc5e16 100644 --- a/cmd/ragflow_server.go +++ b/cmd/ragflow_server.go @@ -444,6 +444,8 @@ func runAdmin(ctx context.Context, args *serverArgs, config *server.Config) erro // Create Gin engine ginEngine := gin.New() + // Mirror Quart's merge_slashes: collapse duplicate slashes before routing. + ginEngine.RemoveExtraSlash = true // Middleware ginEngine.Use(common.GinLogger()) @@ -848,6 +850,8 @@ func startServer(ctx context.Context, config *server.Config) { // Create Gin engine ginEngine := gin.New() + // Mirror Quart's merge_slashes: collapse duplicate slashes before routing. + ginEngine.RemoveExtraSlash = true // Middleware // Note: common.GinLogger() is registered inside router.Setup so the diff --git a/internal/common/error_code.go b/internal/common/error_code.go index 2d35306c39..e3bd460755 100644 --- a/internal/common/error_code.go +++ b/internal/common/error_code.go @@ -89,6 +89,20 @@ func (e ErrorCode) Message() string { return "Unknown error" } +// CodedError pairs a business ErrorCode with a message so handlers can return +// the established {code, message} contract (HTTP 200) instead of a generic +// HTTP 500 for expected domain failures. +type CodedError struct { + Code ErrorCode + Message string +} + +func (e *CodedError) Error() string { return e.Message } + +func NewCodedError(code ErrorCode, message string) *CodedError { + return &CodedError{Code: code, Message: message} +} + var ( ErrInvalidToken = errors.New("invalid token") ErrNotAdmin = errors.New("user is not admin") diff --git a/internal/dao/chat_session.go b/internal/dao/chat_session.go index 29c4533d13..78faafbf8f 100644 --- a/internal/dao/chat_session.go +++ b/internal/dao/chat_session.go @@ -108,11 +108,30 @@ func (dao *ChatSessionDAO) DeleteByID(ctx context.Context, db *gorm.DB, id strin } // ListByChatID lists chat sessions by chat ID -func (dao *ChatSessionDAO) ListByChatID(ctx context.Context, db *gorm.DB, chatID string) ([]*entity.ChatSession, error) { +func (dao *ChatSessionDAO) ListByChatID(ctx context.Context, db *gorm.DB, chatID, sessionID, name, orderby string, desc bool, page, pageSize int) ([]*entity.ChatSession, error) { var convs []*entity.ChatSession - err := db.WithContext(ctx).Where("dialog_id = ?", chatID). - Order("create_time DESC"). - Find(&convs).Error + query := db.WithContext(ctx).Where("dialog_id = ?", chatID) + if sessionID != "" { + query = query.Where("id = ?", sessionID) + } + if name != "" { + query = query.Where("name = ?", name) + } + if orderby == "" { + orderby = "create_time" + } + if desc { + query = query.Order(orderby + " DESC") + } else { + query = query.Order(orderby + " ASC") + } + if pageSize > 0 { + if page < 1 { + page = 1 + } + query = query.Offset((page - 1) * pageSize).Limit(pageSize) + } + err := query.Find(&convs).Error return convs, err } diff --git a/internal/dao/kb.go b/internal/dao/kb.go index e1a315e1dd..1eda211460 100644 --- a/internal/dao/kb.go +++ b/internal/dao/kb.go @@ -181,7 +181,8 @@ func (dao *KnowledgebaseDAO) GetByTenantIDs(ctx context.Context, db *gorm.DB, te Select(`knowledgebase.id, knowledgebase.avatar, knowledgebase.name, knowledgebase.language, knowledgebase.description, knowledgebase.tenant_id, knowledgebase.permission, knowledgebase.doc_num, knowledgebase.token_num, - knowledgebase.chunk_num, knowledgebase.parser_id, knowledgebase.embd_id, + knowledgebase.chunk_num, knowledgebase.parser_id, knowledgebase.parser_config, + knowledgebase.pagerank, knowledgebase.embd_id, knowledgebase.tenant_embd_id, user.nickname, user.avatar as tenant_avatar, knowledgebase.update_time`). Joins("LEFT JOIN user ON knowledgebase.tenant_id = user.id"). diff --git a/internal/engine/elasticsearch/chunk.go b/internal/engine/elasticsearch/chunk.go index 538e1bca8a..1b1a4f1436 100644 --- a/internal/engine/elasticsearch/chunk.go +++ b/internal/engine/elasticsearch/chunk.go @@ -287,7 +287,7 @@ func (e *elasticsearchEngine) UpdateChunks(ctx context.Context, condition map[st return fmt.Errorf("failed to check index existence: %w", err) } if !exists { - return fmt.Errorf("index '%s' does not exist", fullIndexName) + return fmt.Errorf("%w: '%s'", types.ErrIndexNotFound, fullIndexName) } if strings.HasPrefix(fullIndexName, "memory_") { diff --git a/internal/engine/elasticsearch/common.go b/internal/engine/elasticsearch/common.go index 56f0b59f1a..63e5f44702 100644 --- a/internal/engine/elasticsearch/common.go +++ b/internal/engine/elasticsearch/common.go @@ -36,7 +36,9 @@ func (e *elasticsearchEngine) dropIndex(ctx context.Context, indexName string) e return fmt.Errorf("failed to check index existence: %w", err) } if !exists { - return fmt.Errorf("index '%s' does not exist", indexName) + // Tolerate missing index (mirrors Python's docStoreConn which + // silently returns False on a non-existent index). + return nil } // Delete index diff --git a/internal/engine/elasticsearch/metadata.go b/internal/engine/elasticsearch/metadata.go index 4cf462aed4..acc64d8dd6 100644 --- a/internal/engine/elasticsearch/metadata.go +++ b/internal/engine/elasticsearch/metadata.go @@ -89,19 +89,6 @@ func (e *elasticsearchEngine) InsertMetadata(ctx context.Context, metadata []map return nil, fmt.Errorf("index name cannot be empty") } - // Check if index exists, create if not - exists, err := e.indexExists(ctx, indexName) - if err != nil { - common.Error("Failed to check index existence", err) - return nil, fmt.Errorf("failed to check index existence: %w", err) - } - if !exists { - // Create metadata index - if createErr := e.CreateMetadataStore(ctx, tenantID); createErr != nil { - return nil, fmt.Errorf("failed to create metadata index: %w", createErr) - } - } - // Build bulk request body var buf bytes.Buffer for _, doc := range metadata { @@ -114,12 +101,12 @@ func (e *elasticsearchEngine) InsertMetadata(ctx context.Context, metadata []map continue } - // Action line: use json.Marshal to properly escape string values - compositeID := fmt.Sprintf("%d:%s|%d:%s", len(docID), docID, len(kbID), kbID) + // Action line: use the plain document id as _id, matching Python's + // es_conn.insert (readers shim hit._id back into the "id" field). action, err := json.Marshal(map[string]interface{}{ "index": map[string]interface{}{ "_index": indexName, - "_id": compositeID, + "_id": docID, }, }) if err != nil { @@ -170,30 +157,19 @@ func (e *elasticsearchEngine) InsertMetadata(ctx context.Context, metadata []map return []string{}, nil } -// UpdateMetadata updates or inserts document metadata in tenant's metadata index. +// UpdateMetadata fully replaces the meta_fields for a document in tenant's metadata index. // -// Examples (existing row → input → resulting meta_fields): +// UpdateMetadata fully replaces the meta_fields for a document in the +// document engine. Callers must send the complete desired meta_fields map — +// unchanged keys are NOT preserved (unlike a merge). This mirrors Python's +// replace_meta_fields semantics: stale keys must not survive an update. // -// {character:["曹操","孙权"], year:2025} -// + {author:["John","Tom"], category:"tech"} -// = {character:["曹操","孙权"], year:2025, author:["John","Tom"], category:"tech"} -// -// {character:["曹操","孙权"], year:2025} -// + {year:2026} -// = {character:["曹操","孙权"], year:2026} +// The metadata index must already exist; the service layer is responsible +// for creating it before writing. func (e *elasticsearchEngine) UpdateMetadata(ctx context.Context, docID string, datasetID string, metaFields map[string]interface{}, tenantID string) error { indexName := buildMetadataIndexName(tenantID) common.Info("ElasticsearchConnection.UpdateMetadata called", zap.String("index_name", indexName), zap.String("docID", docID), zap.String("datasetID", datasetID)) - // Check if index exists - exists, err := e.indexExists(ctx, indexName) - if err != nil { - return fmt.Errorf("failed to check index existence: %w", err) - } - if !exists { - return fmt.Errorf("index '%s' does not exist", indexName) - } - // Build the document ID for update docIDStr := strings.ReplaceAll(docID, "'", "''") datasetIDStr := strings.ReplaceAll(datasetID, "'", "''") @@ -208,14 +184,13 @@ func (e *elasticsearchEngine) UpdateMetadata(ctx context.Context, docID string, }, } - // Painless script: for every (key, value) in params.meta_fields, - // set ctx._source.meta_fields[key] = value. Existing keys not - // present in params.meta_fields are preserved. If the row has no - // meta_fields at all yet, initialize it to an empty map first. + // Painless script: fully replace meta_fields (mirrors Python's + // replace_meta_fields — stale keys must not survive the update). updateReq := map[string]interface{}{ - "query": query, + "query": query, + "conflicts": "proceed", "script": map[string]interface{}{ - "source": "if (ctx._source.meta_fields == null) { ctx._source.meta_fields = new HashMap(); } for (entry in params.meta_fields.entrySet()) { ctx._source.meta_fields[entry.getKey()] = entry.getValue(); }", + "source": "ctx._source.meta_fields = params.meta_fields;", "lang": "painless", "params": map[string]interface{}{ "meta_fields": metaFields, @@ -229,8 +204,9 @@ func (e *elasticsearchEngine) UpdateMetadata(ctx context.Context, docID string, } req := esapi.UpdateByQueryRequest{ - Index: []string{indexName}, - Body: bytes.NewReader(updateBytes), + Index: []string{indexName}, + Body: bytes.NewReader(updateBytes), + Refresh: func(b bool) *bool { return &b }(true), } res, err := req.Do(ctx, e.client) @@ -300,10 +276,12 @@ func (e *elasticsearchEngine) DeleteMetadata(ctx context.Context, condition map[ return 0, fmt.Errorf("failed to marshal delete body: %w", err) } - // Execute delete by query + // Execute delete by query. Refresh so follow-up reads/writes do not see + // the stale pre-delete state (mirrors Python's refresh semantics). req := esapi.DeleteByQueryRequest{ - Index: []string{indexName}, - Body: bytes.NewReader(bodyBytes), + Index: []string{indexName}, + Body: bytes.NewReader(bodyBytes), + Refresh: func(b bool) *bool { return &b }(true), } res, err := req.Do(ctx, e.client) @@ -346,7 +324,7 @@ func (e *elasticsearchEngine) DeleteMetadataKeys(ctx context.Context, docID stri return fmt.Errorf("failed to check index existence: %w", err) } if !exists { - return fmt.Errorf("index '%s' does not exist", indexName) + return fmt.Errorf("%w: '%s'", types.ErrIndexNotFound, indexName) } // Build the document ID for query (no escaping needed for ES term queries) @@ -508,8 +486,9 @@ func (e *elasticsearchEngine) DeleteMetadataKeys(ctx context.Context, docID stri } req := esapi.UpdateByQueryRequest{ - Index: []string{indexName}, - Body: bytes.NewReader(updateBytes), + Index: []string{indexName}, + Body: bytes.NewReader(updateBytes), + Refresh: func(b bool) *bool { return &b }(true), } res, err := req.Do(ctx, e.client) diff --git a/internal/engine/infinity/chunk.go b/internal/engine/infinity/chunk.go index 092b7adb92..d7ba963334 100644 --- a/internal/engine/infinity/chunk.go +++ b/internal/engine/infinity/chunk.go @@ -400,6 +400,12 @@ func (e *infinityEngine) UpdateChunks(ctx context.Context, condition map[string] table, err := db.GetTable(tableName) if err != nil { + // Tolerate missing table (mirrors Python's docStoreConn which + // silently returns False on a non-existent table). + errMsg := strings.ToLower(err.Error()) + if strings.Contains(errMsg, "not found") || strings.Contains(errMsg, "doesn't exist") { + return nil + } return fmt.Errorf("Failed to get table %s: %w", tableName, err) } diff --git a/internal/engine/infinity/metadata.go b/internal/engine/infinity/metadata.go index 2d5b8691c4..ad1f17b908 100644 --- a/internal/engine/infinity/metadata.go +++ b/internal/engine/infinity/metadata.go @@ -135,7 +135,8 @@ func (e *infinityEngine) createMetadataStoreWithDB(db *infinity.Database, tenant } // InsertMetadata inserts document metadata into tenant's metadata table -// Auto-create the table if it doesn't exist +// The metadata table must already exist; the service layer is responsible +// for creating it before writing. // Replace existing metadata with same id and kb_id func (e *infinityEngine) InsertMetadata(ctx context.Context, metadata []map[string]interface{}, tenantID string) ([]string, error) { tableName := buildMetadataTableName(tenantID) @@ -149,21 +150,7 @@ func (e *infinityEngine) InsertMetadata(ctx context.Context, metadata []map[stri table, err := db.GetTable(tableName) if err != nil { - // Table doesn't exist, try to create it - errMsg := strings.ToLower(err.Error()) - if !strings.Contains(errMsg, "not found") && !strings.Contains(errMsg, "doesn't exist") { - return nil, fmt.Errorf("failed to get table %s: %w", tableName, err) - } - - // Create metadata table - if createErr := e.createMetadataStoreWithDB(db, tenantID); createErr != nil { - return nil, fmt.Errorf("failed to create metadata table: %w", createErr) - } - - table, err = db.GetTable(tableName) - if err != nil { - return nil, fmt.Errorf("failed to get table after creation: %w", err) - } + return nil, fmt.Errorf("failed to get table %s: %w", tableName, err) } // Transform metadata - convert meta_fields map to JSON string @@ -421,6 +408,12 @@ func (e *infinityEngine) DeleteMetadataKeys(ctx context.Context, docID string, d table, err := db.GetTable(tableName) if err != nil { + // Tolerate missing metadata table (mirrors Python's docStoreConn + // which silently returns False on a non-existent table). + errMsg := strings.ToLower(err.Error()) + if strings.Contains(errMsg, "not found") || strings.Contains(errMsg, "doesn't exist") { + return nil + } return fmt.Errorf("failed to get metadata table %s: %w", tableName, err) } diff --git a/internal/engine/types/types.go b/internal/engine/types/types.go index 6c460f1745..e96cee1b1d 100644 --- a/internal/engine/types/types.go +++ b/internal/engine/types/types.go @@ -27,6 +27,11 @@ import ( var ErrDocumentNotFound = errors.New("document not found") +// ErrIndexNotFound marks operations against a document-engine index that does +// not exist yet (e.g. no chunks were ever indexed). Callers may treat it as a +// tolerable no-op, mirroring Python's docStoreConn behavior. +var ErrIndexNotFound = errors.New("index does not exist") + // SearchRequest unified search request for all engines type SearchRequest struct { // Search target diff --git a/internal/entity/dataset.go b/internal/entity/dataset.go index 8fa2d490c3..0a4cc6299f 100644 --- a/internal/entity/dataset.go +++ b/internal/entity/dataset.go @@ -224,16 +224,18 @@ type KnowledgebaseDetail struct { // KnowledgebaseListItem represents a knowledge base item in list responses type KnowledgebaseListItem struct { ID string `json:"id"` - Avatar *string `json:"avatar,omitempty"` + Avatar *string `json:"avatar"` Name string `json:"name"` - Language *string `json:"language,omitempty"` - Description *string `json:"description,omitempty"` + Language *string `json:"language"` + Description *string `json:"description"` TenantID string `json:"tenant_id"` Permission string `json:"permission"` DocNum int64 `json:"doc_num"` TokenNum int64 `json:"token_num"` ChunkNum int64 `json:"chunk_num"` ParserID string `json:"parser_id"` + ParserConfig JSONMap `json:"parser_config"` + Pagerank int64 `json:"pagerank"` EmbdID string `json:"embd_id"` TenantEmbdID *string `json:"tenant_embd_id,omitempty"` Nickname string `json:"nickname"` diff --git a/internal/handler/api_token.go b/internal/handler/api_token.go index d74facbf64..85514d01f2 100644 --- a/internal/handler/api_token.go +++ b/internal/handler/api_token.go @@ -17,6 +17,8 @@ package handler import ( + "errors" + "io" "net/http" "ragflow/internal/common" "ragflow/internal/dao" @@ -89,9 +91,10 @@ func (h *SystemHandler) CreateKey(c *gin.Context) { tenantID := tenants[0].TenantID - // Parse request + // Parse request. An empty body is valid (all fields are optional); + // ShouldBind reports io.EOF for it. var req service.CreateAPIKeyRequest - if err = c.ShouldBind(&req); err != nil { + if err = c.ShouldBind(&req); err != nil && !errors.Is(err, io.EOF) { common.ResponseWithHttpCodeData(c, http.StatusBadRequest, 400, nil, "Invalid request") return } diff --git a/internal/handler/auth.go b/internal/handler/auth.go index 904a4abbd7..cb3a6c788b 100644 --- a/internal/handler/auth.go +++ b/internal/handler/auth.go @@ -60,7 +60,7 @@ func NewAuthHandler() *AuthHandler { // 1. Beta API token → GetUserByBetaAPIToken // 2. JWT (regular session) → existing UserService.GetUserByToken // 3. API token → GetUserByAPIToken -// 4. Fall through → 401 +// 4. Fall through → code 102 "Authorization is not valid!" // // IMPORTANT: the regular-user branch is NOT gated on a "Bearer " // prefix. UserService.GetUserByToken accepts the raw Authorization @@ -79,7 +79,9 @@ func (h *AuthHandler) BetaAuthMiddleware() gin.HandlerFunc { } if auth == "" { - common.ResponseWithCodeData(c, common.CodeUnauthorized, nil, "Authorization required") + // Mirror Python's login_required(auth_types=AUTH_BETA): any auth + // failure on beta endpoints is a business error, not an HTTP 401. + common.ResponseWithCodeData(c, common.CodeDataError, nil, "Authorization is not valid!") c.Abort() return } @@ -105,7 +107,8 @@ func (h *AuthHandler) BetaAuthMiddleware() gin.HandlerFunc { c.Next() return } - common.ResponseWithCodeData(c, common.CodeUnauthorized, nil, "Invalid auth credentials") + // Mirror Python's login_required(auth_types=AUTH_BETA). + common.ResponseWithCodeData(c, common.CodeDataError, nil, "Authorization is not valid!") c.Abort() } } diff --git a/internal/handler/auth_test.go b/internal/handler/auth_test.go index 4e2b62ac91..d6b92243c8 100644 --- a/internal/handler/auth_test.go +++ b/internal/handler/auth_test.go @@ -49,18 +49,20 @@ func TestBetaAuthMiddleware_MissingHeader(t *testing.T) { if rec.Code != http.StatusOK { t.Errorf("status = %d, want %d", rec.Code, http.StatusOK) } - // jsonError writes 200 with a CodeUnauthorized body. Confirm the - // body shape matches the wire contract used by the rest of the - // bot handlers by decoding the JSON envelope and asserting the - // code field rather than just checking for a non-empty body. + // Beta endpoints follow Python's login_required(auth_types=AUTH_BETA): + // auth failures are business errors (code 102), not HTTP 401. var resp struct { - Code common.ErrorCode `json:"code"` + Code common.ErrorCode `json:"code"` + Message string `json:"message"` } if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { t.Fatalf("unmarshal response: %v; body = %s", err, rec.Body.String()) } - if resp.Code != common.CodeUnauthorized { + if resp.Code != common.CodeDataError { t.Errorf("code = %d, want %d; body = %s", - resp.Code, common.CodeUnauthorized, rec.Body.String()) + resp.Code, common.CodeDataError, rec.Body.String()) + } + if resp.Message != "Authorization is not valid!" { + t.Errorf("message = %q; body = %s", resp.Message, rec.Body.String()) } } diff --git a/internal/handler/chat.go b/internal/handler/chat.go index 4f4ac62b60..2b55455368 100644 --- a/internal/handler/chat.go +++ b/internal/handler/chat.go @@ -96,10 +96,16 @@ func (h *ChatHandler) ListChats(c *gin.Context) { } orderby := c.DefaultQuery("orderby", "create_time") + switch orderby { + case "create_time", "update_time", "name": + default: + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("invalid orderby field: %s", orderby)) + return + } desc := true if descStr := c.Query("desc"); descStr != "" { - desc = descStr != "false" + desc = !strings.EqualFold(descStr, "false") } ownerIDs := getOwnerIDs(c) diff --git a/internal/handler/chat_session.go b/internal/handler/chat_session.go index 5b2a1ac91a..306f948b7d 100644 --- a/internal/handler/chat_session.go +++ b/internal/handler/chat_session.go @@ -19,9 +19,11 @@ package handler import ( "encoding/json" "errors" + "fmt" "io" "net/http" "ragflow/internal/common" + "strconv" "strings" "github.com/gin-gonic/gin" @@ -67,13 +69,45 @@ func (h *ChatSessionHandler) ListChatSessions(c *gin.Context) { return } + // Mirror Python's list_sessions query handling: invalid/negative page + // values fall back to the default; page_size 0 yields an empty list and a + // negative page_size disables pagination. + page := 1 + if pageStr := c.Query("page"); pageStr != "" { + if p, err := strconv.Atoi(pageStr); err == nil && p > 0 { + page = p + } + } + pageSize := 30 + if pageSizeStr := c.Query("page_size"); pageSizeStr != "" { + if ps, err := strconv.Atoi(pageSizeStr); err == nil { + pageSize = ps + } + } + + orderby := "create_time" + if queryOrderby := c.Query("orderby"); queryOrderby != "" { + switch queryOrderby { + case "create_time", "update_time", "name": + orderby = queryOrderby + default: + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("invalid orderby field: %s", queryOrderby)) + return + } + } + + desc := true + if descStr := c.Query("desc"); descStr != "" { + desc = !strings.EqualFold(descStr, "false") + } + // Call service to list chat sessions ctx := c.Request.Context() - result, err := h.chatSessionService.ListChatSessions(ctx, userID, chatID) + result, err := h.chatSessionService.ListChatSessions(ctx, userID, chatID, c.Query("id"), c.Query("name"), orderby, desc, page, pageSize) if err != nil { - // Check if it's an authorization error - if err.Error() == "Only owner of dialog authorized for this operation" { - common.ResponseWithHttpCodeData(c, http.StatusForbidden, 403, false, err.Error()) + // Mirror Python: ownership failures return code 109 "No authorization." + if strings.Contains(err.Error(), "No authorization") { + common.ResponseWithCodeData(c, common.CodeAuthenticationError, false, "No authorization.") return } common.ResponseWithHttpCodeData(c, http.StatusInternalServerError, 500, nil, err.Error()) @@ -231,6 +265,11 @@ func (h *ChatSessionHandler) ChatCompletions(c *gin.Context) { false, nil, ) if err != nil { + var codedErr *common.CodedError + if errors.As(err, &codedErr) { + common.ErrorWithCode(c, codedErr.Code, codedErr.Message) + return + } common.ResponseWithHttpCodeData(c, http.StatusInternalServerError, 500, nil, err.Error()) return } @@ -282,7 +321,8 @@ func (h *ChatSessionHandler) CreateSession(c *gin.Context) { if errors.Is(err, io.EOF) { req = map[string]interface{}{} } else { - common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error()) + // Mirror Python's malformed-JSON contract message. + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Malformed JSON syntax: Missing commas/brackets or invalid encoding") return } } @@ -329,7 +369,8 @@ func (h *ChatSessionHandler) DeleteSessions(c *gin.Context) { if errors.Is(err, io.EOF) { req = map[string]interface{}{} } else { - common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error()) + // Mirror Python's malformed-JSON contract message. + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Malformed JSON syntax: Missing commas/brackets or invalid encoding") return } } @@ -361,20 +402,12 @@ func (h *ChatSessionHandler) UpdateSession(c *gin.Context) { userID := user.ID chatID, sessionID := c.Param("chat_id"), c.Param("session_id") + // An empty payload is a valid no-op (mirrors Python's update_session). req := map[string]any{} - if err := c.ShouldBindJSON(&req); err != nil { - if errors.Is(err, io.EOF) { - common.ResponseWithCodeData(c, common.CodeArgumentError, nil, - "Request body cannot be empty") - return - } + if err := c.ShouldBindJSON(&req); err != nil && !errors.Is(err, io.EOF) { common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Invalid request: "+err.Error()) return } - if len(req) == 0 { - common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Request body cannot be empty") - return - } ctx := c.Request.Context() result, code, err := h.chatSessionService.UpdateSession(ctx, userID, chatID, sessionID, req) diff --git a/internal/handler/chat_session_test.go b/internal/handler/chat_session_test.go index 6a2e05dd05..4913703b04 100644 --- a/internal/handler/chat_session_test.go +++ b/internal/handler/chat_session_test.go @@ -14,65 +14,6 @@ import ( "github.com/gin-gonic/gin" ) -func TestChatSessionHandlerUpdateSession_RejectsEmptyBody(t *testing.T) { - gin.SetMode(gin.TestMode) - - recorder := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(recorder) - ctx.Request = httptest.NewRequest(http.MethodPatch, "/api/v1/chats/chat-1/sessions/session-1", nil) - ctx.Params = gin.Params{ - {Key: "chat_id", Value: "chat-1"}, - {Key: "session_id", Value: "session-1"}, - } - ctx.Set("user", &entity.User{ID: "user-1"}) - - handler := NewChatSessionHandler(service.NewChatSessionService(), nil) - handler.UpdateSession(ctx) - - if recorder.Code != http.StatusOK { - t.Fatalf("status=%d", recorder.Code) - } - - var body map[string]interface{} - if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil { - t.Fatalf("decode response body: %v", err) - } - if got := body["code"]; got != float64(common.CodeArgumentError) { - t.Fatalf("code=%v", got) - } - if got := body["message"]; got != "Request body cannot be empty" { - t.Fatalf("message=%v", got) - } -} - -func TestChatSessionHandlerUpdateSession_RejectsEmptyJSONObject(t *testing.T) { - gin.SetMode(gin.TestMode) - - recorder := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(recorder) - ctx.Request = httptest.NewRequest(http.MethodPatch, "/api/v1/chats/chat-1/sessions/session-1", strings.NewReader(`{}`)) - ctx.Request.Header.Set("Content-Type", "application/json") - ctx.Params = gin.Params{ - {Key: "chat_id", Value: "chat-1"}, - {Key: "session_id", Value: "session-1"}, - } - ctx.Set("user", &entity.User{ID: "user-1"}) - - handler := NewChatSessionHandler(service.NewChatSessionService(), nil) - handler.UpdateSession(ctx) - - var body map[string]interface{} - if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil { - t.Fatalf("decode response body: %v", err) - } - if got := body["code"]; got != float64(common.CodeArgumentError) { - t.Fatalf("code=%v", got) - } - if got := body["message"]; got != "Request body cannot be empty" { - t.Fatalf("message=%v", got) - } -} - func TestChatSessionHandlerUpdateMessageFeedback_RejectsEmptyBody(t *testing.T) { gin.SetMode(gin.TestMode) diff --git a/internal/handler/chunk.go b/internal/handler/chunk.go index f9584c9bc9..5cc4aa170c 100644 --- a/internal/handler/chunk.go +++ b/internal/handler/chunk.go @@ -720,17 +720,17 @@ func (h *ChunkHandler) AddChunk(c *gin.Context) { } importantKeywords, err := addChunkStringListField(rawBody, "important_keywords", "`important_keywords` is required to be a list", "`important_keywords` must be a list of strings") if err != nil { - common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error()) + common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } questions, err := addChunkStringListField(rawBody, "questions", "`questions` is required to be a list", "`questions` must be a list of strings") if err != nil { - common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error()) + common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } tagKwd, err := addChunkStringListField(rawBody, "tag_kwd", "`tag_kwd` is required to be a list", "`tag_kwd` must be a list of strings") if err != nil { - common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error()) + common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } imageBase64, err := addChunkStringPtrField(rawBody, "image_base64") diff --git a/internal/handler/dataset.go b/internal/handler/dataset.go index 7c949d1882..cb2748c86b 100644 --- a/internal/handler/dataset.go +++ b/internal/handler/dataset.go @@ -88,28 +88,60 @@ func (h *DatasetsHandler) ListDatasets(c *gin.Context) { return } + // Mirror pydantic's extra="forbid" on ListDatasetReq. + for param := range c.Request.URL.Query() { + if !listDatasetsAllowedParams[param] { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("Extra inputs are not permitted: %s", param)) + return + } + } + + // Mirror the pydantic validation of Python's ListDatasetReq. page := 1 if pageStr := c.Query("page"); pageStr != "" { - if p, err := strconv.Atoi(pageStr); err == nil && p > 0 { - page = p + p, err := strconv.Atoi(pageStr) + if err != nil { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Input should be a valid integer, unable to parse string as an integer") + return } + if p < 1 { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Input should be greater than or equal to 1") + return + } + page = p } pageSize := 30 if pageSizeStr := c.Query("page_size"); pageSizeStr != "" { - if ps, err := strconv.Atoi(pageSizeStr); err == nil && ps > 0 { - pageSize = ps + ps, err := strconv.Atoi(pageSizeStr) + if err != nil { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Input should be a valid integer, unable to parse string as an integer") + return } + if ps < 1 { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Input should be greater than or equal to 1") + return + } + pageSize = ps } orderby := "create_time" - if queryOrderby := c.Query("orderby"); queryOrderby != "" { + if queryOrderby, exists := c.GetQuery("orderby"); exists { + if queryOrderby != "create_time" && queryOrderby != "update_time" { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Input should be 'create_time' or 'update_time'") + return + } orderby = queryOrderby } desc := true if descStr := c.Query("desc"); descStr != "" { - desc = strings.ToLower(descStr) == "true" + parsed, ok := parsePythonBool(descStr) + if !ok { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Input should be a valid boolean, unable to interpret input") + return + } + desc = parsed } keywords := "" @@ -128,6 +160,14 @@ func (h *DatasetsHandler) ListDatasets(c *gin.Context) { ownerIDs = ext.OwnerIDs } + // Mirror pydantic: a present-but-empty id fails UUID validation. + if rawID, exists := c.GetQuery("id"); exists { + if _, err := dataset.NormalizeDatasetID(strings.TrimSpace(rawID)); err != nil { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error()) + return + } + } + ctx := c.Request.Context() data, total, code, err := h.datasetsService.ListDatasets( ctx, @@ -162,11 +202,21 @@ func (h *DatasetsHandler) CreateDataset(c *gin.Context) { return } + bodyBytes, _, ok := parseJSONRequestObject(c) + if !ok { + return + } + var req service.CreateDatasetRequest - if err := c.ShouldBindJSON(&req); err != nil { + if err := json.Unmarshal(bodyBytes, &req); err != nil { common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } + // Mirror Python's pydantic required validation. + if req.Name == "" || (len(bodyBytes) > 0 && jsonNullValue(bodyBytes, "name")) { + common.ResponseWithCodeData(c, common.CodeDataError, nil, "Field validation for 'name' failed on the 'required' tag") + return + } ctx := c.Request.Context() @@ -200,6 +250,96 @@ func (h *DatasetsHandler) GetDataset(c *gin.Context) { } // UpdateDataset Update a dataset. +// parsePythonBool mirrors pydantic's boolean coercion for query params: +// accepts true/false/1/0/yes/no/y/n (case-insensitive). +func parsePythonBool(s string) (bool, bool) { + switch strings.ToLower(s) { + case "true", "1", "yes", "y", "on": + return true, true + case "false", "0", "no", "n", "off": + return false, true + } + return false, false +} + +// parseJSONRequestObject mirrors Python's validate_and_parse_json_request: +// content-type check first, then JSON syntax, then object shape. On failure it +// writes the error response and returns ok=false. +func parseJSONRequestObject(c *gin.Context) (body []byte, raw map[string]interface{}, ok bool) { + if c.ContentType() != "application/json" { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("Unsupported content type: Expected application/json, got %s", c.GetHeader("Content-Type"))) + return nil, nil, false + } + + body, err := c.GetRawData() + if err != nil || len(body) == 0 { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Malformed JSON syntax: Missing commas/brackets or invalid encoding") + return nil, nil, false + } + + var payload interface{} + if err := json.Unmarshal(body, &payload); err != nil { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, "Malformed JSON syntax: Missing commas/brackets or invalid encoding") + return nil, nil, false + } + raw, isObject := payload.(map[string]interface{}) + if !isObject { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("Invalid request payload: expected object, got %s", pythonJSONTypeName(payload))) + return nil, nil, false + } + return body, raw, true +} + +// jsonNullValue checks if a field is explicitly null in the JSON body. +func jsonNullValue(body []byte, field string) bool { + var m map[string]json.RawMessage + if err := json.Unmarshal(body, &m); err != nil { + return false + } + raw, ok := m[field] + if !ok { + return false + } + return strings.TrimSpace(string(raw)) == "null" +} + +// pythonJSONTypeName maps a decoded JSON value to the Python type name used in +// the "Invalid request payload" contract message. +func pythonJSONTypeName(v interface{}) string { + switch v.(type) { + case string: + return "str" + case []interface{}: + return "list" + case float64: + return "float" + case bool: + return "bool" + case nil: + return "NoneType" + default: + return "object" + } +} + +// listDatasetsAllowedParams mirrors the query field set of Python's +// ListDatasetReq (BaseListReq + include_parsing_status/ext; `type` is handled +// before validation in the Python endpoint). +var listDatasetsAllowedParams = map[string]bool{ + "id": true, "name": true, "page": true, "page_size": true, + "orderby": true, "desc": true, "include_parsing_status": true, + "ext": true, "type": true, +} + +// updateDatasetAllowedFields mirrors the field set of Python's UpdateDatasetReq +// (CreateDatasetReq fields + dataset_id/pagerank/language/connectors). +var updateDatasetAllowedFields = map[string]bool{ + "name": true, "avatar": true, "description": true, "embedding_model": true, + "permission": true, "parse_type": true, "pipeline_id": true, "chunk_method": true, + "parser_id": true, "parser_config": true, "auto_metadata_config": true, "ext": true, + "dataset_id": true, "pagerank": true, "language": true, "connectors": true, +} + func (h *DatasetsHandler) UpdateDataset(c *gin.Context) { user, errorCode, errorMessage := GetUser(c) if errorCode != common.CodeSuccess { @@ -218,15 +358,19 @@ func (h *DatasetsHandler) UpdateDataset(c *gin.Context) { common.ResponseWithCodeData(c, common.CodeBadRequest, nil, "dataset id is required") return } + // Mirror the pydantic UUID validation of Python's UpdateDatasetReq. + if _, err := dataset.NormalizeDatasetID(datasetID); err != nil { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error()) + return + } - bodyBytes, err := c.GetRawData() - if err != nil { - common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) + bodyBytes, _, ok := parseJSONRequestObject(c) + if !ok { return } var req service.UpdateDatasetRequest - if err = json.Unmarshal(bodyBytes, &req); err != nil { + if err := json.Unmarshal(bodyBytes, &req); err != nil { common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } @@ -234,8 +378,15 @@ func (h *DatasetsHandler) UpdateDataset(c *gin.Context) { // Detect an explicitly provided parser_config key (even {} or null) so it is not // rejected as "no properties were modified", mirroring the Python contract. var providedFields map[string]json.RawMessage - if err = json.Unmarshal(bodyBytes, &providedFields); err == nil { + if err := json.Unmarshal(bodyBytes, &providedFields); err == nil { _, req.ParserConfigProvided = providedFields["parser_config"] + // Mirror pydantic's extra="forbid" on UpdateDatasetReq. + for field := range providedFields { + if !updateDatasetAllowedFields[field] { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("Extra inputs are not permitted: %s", field)) + return + } + } } ctx := c.Request.Context() @@ -396,13 +547,40 @@ func (h *DatasetsHandler) DeleteDatasets(c *gin.Context) { return } + bodyBytes, rawPayload, ok := parseJSONRequestObject(c) + if !ok { + return + } + + // Mirror pydantic's extra="forbid" on DeleteDatasetReq. + for field := range rawPayload { + if field != "ids" && field != "delete_all" { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("Extra inputs are not permitted: %s", field)) + return + } + } + var req struct { IDs *[]string `json:"ids"` DeleteAll bool `json:"delete_all,omitempty"` } - if c.Request.ContentLength > 0 { - if err := c.ShouldBindJSON(&req); err != nil { - common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) + if err := json.Unmarshal(bodyBytes, &req); err != nil { + common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) + return + } + + // Mirror DeleteReq duplicate detection. + if req.IDs != nil { + seen := make(map[string]int, len(*req.IDs)) + duplicates := make([]string, 0) + for _, id := range *req.IDs { + seen[id]++ + if seen[id] == 2 { + duplicates = append(duplicates, id) + } + } + if len(duplicates) > 0 { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("Duplicate ids: '%s'", strings.Join(duplicates, ", "))) return } } diff --git a/internal/handler/document.go b/internal/handler/document.go index 3d1f35d240..b1197dca5f 100644 --- a/internal/handler/document.go +++ b/internal/handler/document.go @@ -31,6 +31,7 @@ import ( "ragflow/internal/common" "ragflow/internal/entity" "ragflow/internal/utility" + "reflect" "strconv" "strings" "time" @@ -392,13 +393,32 @@ func (h *DocumentHandler) DeleteDocuments(c *gin.Context) { return } + bodyBytes, _, ok := parseJSONRequestObject(c) + if !ok { + return + } + var req struct { IDs *[]string `json:"ids"` DeleteAll bool `json:"delete_all,omitempty"` } - if c.Request.ContentLength > 0 { - if err := c.ShouldBindJSON(&req); err != nil { - common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) + if err := json.Unmarshal(bodyBytes, &req); err != nil { + common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) + return + } + + // Mirror pydantic's DeleteDocumentReq duplicate detection. + if req.IDs != nil { + seen := make(map[string]int, len(*req.IDs)) + duplicates := make([]string, 0) + for _, id := range *req.IDs { + seen[id]++ + if seen[id] == 2 { + duplicates = append(duplicates, id) + } + } + if len(duplicates) > 0 { + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("Field: - Message: - Value: <%s>", strings.Join(duplicates, ", "), string(bodyBytes))) return } } @@ -505,6 +525,15 @@ func (h *DocumentHandler) ListDocuments(c *gin.Context) { userID := c.GetString("user_id") + if orderby := c.Query("orderby"); orderby != "" { + switch orderby { + case "create_time", "update_time", "name": + default: + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("invalid orderby field: %s", orderby)) + return + } + } + ctx := c.Request.Context() if !h.datasetService.Accessible(ctx, datasetID, userID) { common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("You don't own the dataset %s.", datasetID)) @@ -548,6 +577,30 @@ func (h *DocumentHandler) ListDocuments(c *gin.Context) { return } + // Mirror Python: an explicit id/name filter that matches nothing is an + // ownership error rather than an empty page. Each filter's existence is + // checked independently of the other. + if docID := c.Query("id"); docID != "" { + idOpts := opts + idOpts.DocIDs = []string{docID} + idOpts.Name = "" + _, idTotal, idErr := h.documentService.ListDocumentsByDatasetIDWithOptions(ctx, idOpts, 1, 1) + if idErr == nil && idTotal == 0 { + common.ResponseWithCodeData(c, common.CodeDataError, map[string]interface{}{"total": 0, "docs": []interface{}{}}, fmt.Sprintf("you don't own the document %s", docID)) + return + } + } + if opts.Name != "" { + nameOpts := opts + nameOpts.DocIDs = nil + nameOpts.DocIDFilterApplied = false + _, nameTotal, nameErr := h.documentService.ListDocumentsByDatasetIDWithOptions(ctx, nameOpts, 1, 1) + if nameErr == nil && nameTotal == 0 { + common.ResponseWithCodeData(c, common.CodeDataError, map[string]interface{}{"total": 0, "docs": []interface{}{}}, fmt.Sprintf("you don't own the document %s", opts.Name)) + return + } + } + docs := make([]map[string]interface{}, 0, len(documents)) for _, doc := range documents { if opts.CreateTimeFrom > 0 && doc.CreateTime != nil && *doc.CreateTime < opts.CreateTimeFrom { @@ -579,7 +632,7 @@ func parseDocumentListOptions(c *gin.Context, datasetID string) (dao.DocumentLis opts.RunStatuses = normalizeRunStatusFilter(queryValues(c, "run", "run_status")) if len(queryValues(c, "run", "run_status")) > 0 && len(opts.RunStatuses) == 0 { - return opts, "Invalid filter run status conditions" + return opts, fmt.Sprintf("Invalid filter run status conditions: %s", strings.Join(invalidRunStatuses(queryValues(c, "run", "run_status")), ", ")) } opts.Name = c.Query("name") @@ -615,6 +668,17 @@ func parseDocumentListOptions(c *gin.Context, datasetID string) (dao.DocumentLis } func (h *DocumentHandler) applyDocumentMetadataFilter(c *gin.Context, opts dao.DocumentListOptions) (dao.DocumentListOptions, string) { + // Mirror Python's metadata_condition query-param validation. + if raw := strings.TrimSpace(c.Query("metadata_condition")); raw != "" { + var mc interface{} + if err := json.Unmarshal([]byte(raw), &mc); err != nil { + return opts, fmt.Sprintf("metadata_condition must be valid JSON: %s.", raw) + } + if _, ok := mc.(map[string]interface{}); !ok { + return opts, "metadata_condition must be an object." + } + } + metadata, err := parseMetadataQuery(c.Request.URL.Query()) if err != nil { return opts, err.Error() @@ -820,6 +884,19 @@ func normalizeRunStatusFilter(statuses []string) []string { return out } +// invalidRunStatuses returns the raw filter values that do not map to a valid +// run status, mirroring Python's "Invalid filter run status conditions: ...". +func invalidRunStatuses(statuses []string) []string { + valid := map[string]bool{"UNSTART": true, "RUNNING": true, "CANCEL": true, "DONE": true, "FAIL": true} + invalid := make([]string, 0) + for _, status := range statuses { + if !valid[strings.ToUpper(status)] { + invalid = append(invalid, status) + } + } + return invalid +} + func (h *DocumentHandler) UploadDocuments(c *gin.Context) { user, errorCode, errorMessage := GetUser(c) if errorCode != common.CodeSuccess { @@ -1421,8 +1498,22 @@ func (h *DocumentHandler) StartIngestionTask(c *gin.Context) { common.ResponseWithCodeData(c, common.CodeDataError, nil, "`document_ids` is required") return } + // Gin's `required` accepts an empty non-nil slice; reject it explicitly. + if len(req.DocumentIDs) == 0 { + common.ResponseWithCodeData(c, common.CodeDataError, nil, "`document_ids` is required") + return + } userID := c.GetString("user_id") + if orderby := c.Query("orderby"); orderby != "" { + switch orderby { + case "create_time", "update_time", "name": + default: + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("invalid orderby field: %s", orderby)) + return + } + } + ctx := c.Request.Context() if !h.datasetService.Accessible(ctx, datasetID, userID) { common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("You don't own the dataset %s.", datasetID)) @@ -1529,16 +1620,25 @@ func (h *DocumentHandler) StopParseDocuments(c *gin.Context) { var req StopParseDocumentRequest if err := c.ShouldBindJSON(&req); err != nil { - common.ErrorWithCode(c, common.CodeBadRequest, err.Error()) + common.ErrorWithCode(c, common.CodeDataError, "`document_ids` is required") return } if len(req.DocumentIDs) == 0 { - common.ErrorWithCode(c, common.CodeBadRequest, "`document_ids` is required") + common.ErrorWithCode(c, common.CodeDataError, "`document_ids` is required") return } userID := c.GetString("user_id") + if orderby := c.Query("orderby"); orderby != "" { + switch orderby { + case "create_time", "update_time", "name": + default: + common.ResponseWithCodeData(c, common.CodeArgumentError, nil, fmt.Sprintf("invalid orderby field: %s", orderby)) + return + } + } + ctx := c.Request.Context() if !h.datasetService.Accessible(ctx, datasetID, userID) { common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("You don't own the dataset %s.", datasetID)) @@ -1619,6 +1719,25 @@ func (h *DocumentHandler) UpdateDatasetDocument(c *gin.Context) { } var req document.UpdateDatasetDocumentRequest if err = json.Unmarshal(body, &req); err != nil { + // Mirror pydantic's "Field: - Message: - Value: " format. + var typeErr *json.UnmarshalTypeError + if errors.As(err, &typeErr) { + value := typeErr.Value + if rawValue, ok := raw[typeErr.Field]; ok { + // Render scalars the way Python's str() does: strings + // unquoted, other JSON values verbatim. + var scalar interface{} + if jsonErr := json.Unmarshal(rawValue, &scalar); jsonErr == nil { + if str, isStr := scalar.(string); isStr { + value = str + } else { + value = string(rawValue) + } + } + } + common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("Field: <%s> - Message: - Value: <%s>", typeErr.Field, pythonJSONKindName(typeErr), value)) + return + } common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } @@ -1845,3 +1964,26 @@ func parseMetadataDeletes(raw interface{}) ([]document.DocumentMetadataDelete, s } return deletes, "" } + +// pythonJSONKindName maps a JSON unmarshal type error to the pydantic type +// name used in the "Input should be a valid ..." contract message. +func pythonJSONKindName(typeErr *json.UnmarshalTypeError) string { + if typeErr.Type == nil { + return "value" + } + switch typeErr.Type.Kind() { + case reflect.String: + return "string" + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return "integer" + case reflect.Float32, reflect.Float64: + return "number" + case reflect.Bool: + return "boolean" + case reflect.Slice, reflect.Array: + return "list" + case reflect.Map: + return "dictionary" + } + return "value" +} diff --git a/internal/handler/search.go b/internal/handler/search.go index df4c371cbb..5477c54d41 100644 --- a/internal/handler/search.go +++ b/internal/handler/search.go @@ -334,7 +334,7 @@ func (h *SearchHandler) UpdateSearch(c *gin.Context) { errMsg := err.Error() switch errMsg { case "no authorization": - common.ResponseWithCodeData(c, common.CodeAuthenticationError, false, "no authorization") + common.ResponseWithCodeData(c, common.CodeAuthenticationError, false, "No authorization.") case "duplicated search name": common.ResponseWithCodeData(c, common.CodeDataError, nil, "Duplicated search name.") default: diff --git a/internal/handler/searchbot.go b/internal/handler/searchbot.go index 069fd36558..d37f8eadd4 100644 --- a/internal/handler/searchbot.go +++ b/internal/handler/searchbot.go @@ -89,8 +89,10 @@ func (r *SearchBotRetrievalTestRequest) UnmarshalJSON(data []byte) error { } // SearchBotRequest is the request body for POST /api/v1/searchbots/related_questions. +// Question is validated manually below so the error message follows the +// established API contract instead of the Gin validator format. type SearchBotRequest struct { - Question string `json:"question" binding:"required"` + Question string `json:"question"` SearchID string `json:"search_id,omitempty"` } diff --git a/internal/handler/tenant.go b/internal/handler/tenant.go index 70bdbcd413..21471121c3 100644 --- a/internal/handler/tenant.go +++ b/internal/handler/tenant.go @@ -456,6 +456,14 @@ func (h *TenantHandler) InsertMetadataFromFile(c *gin.Context) { // Use user.ID as tenant ID (user IS the tenant in user mode) tenantID := user.ID + // Ensure the metadata store exists before writing (service-layer + // create-on-first-write logic; the engine layer assumes it exists). + metadataSvc := service.NewMetadataService() + if err := metadataSvc.EnsureMetadataStore(c.Request.Context(), tenantID); err != nil { + common.ResponseWithHttpCodeData(c, http.StatusBadRequest, 400, nil, "failed to ensure metadata store: "+err.Error()) + return + } + // Get the document engine and insert docEngine := engine.Get() result, err := docEngine.InsertMetadata(c.Request.Context(), inputFormat.Chunks, tenantID) diff --git a/internal/router/router_test.go b/internal/router/router_test.go index 23ea1a859e..891294393f 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -128,7 +128,7 @@ func TestRouterSetupRegistersSearchbotMindMapRoute(t *testing.T) { if err := json.Unmarshal(resp.Body.Bytes(), &body); err != nil { t.Fatalf("failed to decode response body: %v", err) } - if body.Code != common.CodeUnauthorized { + if body.Code != common.CodeDataError { t.Fatalf("status=%d body=%s; want beta auth middleware to handle registered MindMap route", resp.Code, resp.Body.String()) } } @@ -156,7 +156,7 @@ func TestRouterSetupRegistersChatbotInfoOnce(t *testing.T) { if err := json.Unmarshal(resp.Body.Bytes(), &body); err != nil { t.Fatalf("failed to decode response body: %v", err) } - if body.Code != common.CodeUnauthorized { + if body.Code != common.CodeDataError { t.Fatalf("status=%d body=%s; want beta auth middleware to handle registered ChatbotInfo route", resp.Code, resp.Body.String()) } } diff --git a/internal/service/chat_session.go b/internal/service/chat_session.go index a4f92d9bae..77ef1445e1 100644 --- a/internal/service/chat_session.go +++ b/internal/service/chat_session.go @@ -47,7 +47,7 @@ type chatSessionStore interface { Create(ctx context.Context, db *gorm.DB, conv *entity.ChatSession) error UpdateByID(ctx context.Context, db *gorm.DB, id string, updates map[string]interface{}) error DeleteByID(ctx context.Context, db *gorm.DB, id string) error - ListByChatID(ctx context.Context, db *gorm.DB, chatID string) ([]*entity.ChatSession, error) + ListByChatID(ctx context.Context, db *gorm.DB, chatID, sessionID, name, orderby string, desc bool, page, pageSize int) ([]*entity.ChatSession, error) GetDialogByID(ctx context.Context, db *gorm.DB, chatID string) (*entity.Chat, error) CheckDialogExists(ctx context.Context, db *gorm.DB, tenantID, chatID string) (bool, error) } @@ -243,7 +243,7 @@ type ChatSessionPayload struct { } // ListChatSessions lists chat sessions for a dialog -func (s *ChatSessionService) ListChatSessions(ctx context.Context, userID string, chatID string) (*ListChatSessionsResponse, error) { +func (s *ChatSessionService) ListChatSessions(ctx context.Context, userID, chatID, sessionID, name, orderby string, desc bool, page, pageSize int) (*ListChatSessionsResponse, error) { // Get user's tenants tenantIDs, err := s.userTenantDAO.GetTenantIDsByUserID(ctx, dao.DB, userID) if err != nil { @@ -275,11 +275,16 @@ func (s *ChatSessionService) ListChatSessions(ctx context.Context, userID string } if !isOwner { - return nil, errors.New("only owner of dialog authorized for this operation") + return nil, errors.New("No authorization.") + } + + // items_per_page == 0 returns an empty list (mirrors Python's list_sessions). + if pageSize == 0 { + return &ListChatSessionsResponse{Sessions: []*entity.ChatSession{}}, nil } // List chat sessions - sessions, err := s.chatSessionDAO.ListByChatID(ctx, dao.DB, chatID) + sessions, err := s.chatSessionDAO.ListByChatID(ctx, dao.DB, chatID, sessionID, name, orderby, desc, page, pageSize) if err != nil { return nil, err } @@ -294,7 +299,7 @@ func (s *ChatSessionService) GetSession(ctx context.Context, userID, chatID, ses return nil, common.CodeServerError, err } if !ok { - return nil, common.CodeAuthenticationError, errors.New("no authorization") + return nil, common.CodeAuthenticationError, errors.New("No authorization.") } session, err := s.chatSessionDAO.GetByID(ctx, dao.DB, sessionID) @@ -323,7 +328,7 @@ func (s *ChatSessionService) CreateSession(ctx context.Context, userID, chatID s return nil, common.CodeServerError, err } if !ok { - return nil, common.CodeAuthenticationError, errors.New("no authorization") + return nil, common.CodeAuthenticationError, errors.New("No authorization.") } dialog, err := s.chatSessionDAO.GetDialogByID(ctx, dao.DB, chatID) @@ -389,7 +394,7 @@ func (s *ChatSessionService) DeleteSessions(ctx context.Context, userID, chatID return nil, "", common.CodeServerError, err } if !ok { - return false, "no authorization", common.CodeAuthenticationError, errors.New("no authorization") + return false, "No authorization.", common.CodeAuthenticationError, errors.New("No authorization.") } if len(req) == 0 { @@ -400,7 +405,7 @@ func (s *ChatSessionService) DeleteSessions(ctx context.Context, userID, chatID if !hasIDs || len(sessionIDs) == 0 { deleteAll, _ := req["delete_all"].(bool) if deleteAll { - sessions, err := s.chatSessionDAO.ListByChatID(ctx, dao.DB, chatID) + sessions, err := s.chatSessionDAO.ListByChatID(ctx, dao.DB, chatID, "", "", "create_time", true, 0, -1) if err != nil { return nil, "", common.CodeServerError, err } @@ -547,10 +552,7 @@ func (s *ChatSessionService) UpdateSession(ctx context.Context, userID, chatID, return nil, common.CodeServerError, err } if !ok { - return nil, common.CodeAuthenticationError, errors.New("no authorization") - } - if len(req) == 0 { - return nil, common.CodeArgumentError, errors.New("request body cannot be empty") + return nil, common.CodeAuthenticationError, errors.New("No authorization.") } if _, err = s.chatSessionDAO.GetBySessionIDAndChatID(ctx, dao.DB, sessionID, chatID); err != nil { @@ -592,11 +594,14 @@ func (s *ChatSessionService) UpdateSession(ctx context.Context, userID, chatID, } } - if err = s.chatSessionDAO.UpdateByID(ctx, dao.DB, sessionID, updateFields); err != nil { - if isChatSessionNotFound(err) { - return nil, common.CodeDataError, errors.New("session not found") + // An empty payload is a no-op: skip the write and return the current state. + if len(updateFields) > 0 { + if err = s.chatSessionDAO.UpdateByID(ctx, dao.DB, sessionID, updateFields); err != nil { + if isChatSessionNotFound(err) { + return nil, common.CodeDataError, errors.New("Session not found!") + } + return nil, common.CodeServerError, err } - return nil, common.CodeServerError, err } session, err := s.chatSessionDAO.GetByID(ctx, dao.DB, sessionID) @@ -616,7 +621,7 @@ func (s *ChatSessionService) DeleteSessionMessage(ctx context.Context, userID, c return nil, common.CodeServerError, err } if !ok { - return nil, common.CodeAuthenticationError, errors.New("no authorization") + return nil, common.CodeAuthenticationError, errors.New("No authorization.") } session, err := s.chatSessionDAO.GetByID(ctx, dao.DB, sessionID) @@ -706,7 +711,7 @@ func (s *ChatSessionService) UpdateMessageFeedback(ctx context.Context, userID, } ok := ownerTenantID != "" if !ok { - return nil, common.CodeAuthenticationError, errors.New("no authorization") + return nil, common.CodeAuthenticationError, errors.New("No authorization.") } session, err := s.chatSessionDAO.GetByID(ctx, dao.DB, sessionID) @@ -1472,12 +1477,12 @@ func (s *ChatSessionService) ChatCompletions( // --- 1. Normalize messages --- requestMessages, requestMsg, messageID, err := s.normalizeCompletionMessages(messages, question, files) if err != nil { - return fail(err) + return fail(common.NewCodedError(common.CodeArgumentError, err.Error())) } // --- 2. Validate --- if sessionID != "" && chatID == "" { - return fail(errors.New("`chat_id` is required when `session_id` is provided")) + return fail(common.NewCodedError(common.CodeDataError, "`chat_id` is required when `session_id` is provided.")) } // --- 3. Resolve dialog and session --- @@ -1485,19 +1490,19 @@ func (s *ChatSessionService) ChatCompletions( var session *entity.ChatSession if chatID != "" { if err = s.checkDialogOwnership(ctx, userID, chatID); err != nil { - return fail(err) + return fail(common.NewCodedError(common.CodeAuthenticationError, "No authorization.")) } dialog, err = s.chatSessionDAO.GetDialogByID(ctx, dao.DB, chatID) if err != nil { - return fail(errors.New("chat not found")) + return fail(common.NewCodedError(common.CodeDataError, "Chat not found!")) } if sessionID != "" { session, err = s.chatSessionDAO.GetByID(ctx, dao.DB, sessionID) if err != nil { - return fail(errors.New("session not found")) + return fail(common.NewCodedError(common.CodeDataError, "Session not found!")) } if session.DialogID != chatID { - return fail(errors.New("session does not belong to this chat")) + return fail(common.NewCodedError(common.CodeDataError, "Session does not belong to this chat!")) } } else { session, err = s.createSessionForCompletion(ctx, chatID, dialog, userID) @@ -1534,7 +1539,7 @@ func (s *ChatSessionService) ChatCompletions( if llmID != "" { hasKey, err := s.checkTenantLLMAPIKey(ctx, dialog.TenantID, llmID) if err != nil || !hasKey { - return fail(fmt.Errorf("cannot use specified model %s", llmID)) + return fail(common.NewCodedError(common.CodeDataError, fmt.Sprintf("Cannot use specified model %s.", llmID))) } dialog.LLMID = llmID dialog.LLMSetting = genConfig @@ -1668,26 +1673,7 @@ func (s *ChatSessionService) ChatCompletions( s.updateSessionMessages(ctx, session, s.getSessionMessagesAsSlice(session), reference) } } else { - var answer strings.Builder - var finalRef map[string]interface{} - for result := range resultChan { - if result.Final && result.Answer != "" { - // The final event carries the complete (decorated) answer; - // it replaces any accumulated deltas rather than appending. - answer.Reset() - answer.WriteString(result.Answer) - } else if result.Answer != "" { - answer.WriteString(result.Answer) - } - if result.Reference != nil { - finalRef = result.Reference - } - } - ans := map[string]interface{}{ - "answer": answer.String(), - "reference": finalRef, - "final": true, - } + ans := accumulateNonStreamAnswer(resultChan) if session != nil { result := s.structureAnswerWithConv(session, ans, messageID, sessionID, reference) if chatID != "" { @@ -1709,6 +1695,53 @@ func (s *ChatSessionService) ChatCompletions( // --- Helpers for ChatCompletions --- +// accumulateNonStreamAnswer drains the pipeline result channel of a non-stream +// completion and builds the response answer map. +// +// Response metadata (audio_binary, prompt, created_at) is captured only from +// the final event, assigned as-is. Intermediate events may carry conflicting +// or placeholder metadata; accepting it would mix values from different events +// into one response. This mirrors the stream path, which reads these fields +// from result.Final. +func accumulateNonStreamAnswer(resultChan <-chan AsyncChatResult) map[string]interface{} { + var answer strings.Builder + var finalRef map[string]interface{} + var audioBinary interface{} + var prompt string + var createdAt float64 + for result := range resultChan { + if result.Final { + // The final event carries the complete (decorated) answer; + // it replaces any accumulated deltas rather than appending. + if result.Answer != "" { + answer.Reset() + answer.WriteString(result.Answer) + } + audioBinary = result.AudioBinary + prompt = result.Prompt + createdAt = result.CreatedAt + } else if result.Answer != "" { + answer.WriteString(result.Answer) + } + if result.Reference != nil { + finalRef = result.Reference + } + } + // Mirror Python's non-stream response shape: decorate_answer's + // {answer, reference, prompt, created_at} plus audio_binary. + ans := map[string]interface{}{ + "answer": answer.String(), + "reference": finalRef, + "audio_binary": audioBinary, + "prompt": prompt, + "final": true, + } + if createdAt != 0 { + ans["created_at"] = createdAt + } + return ans +} + // normalizeCompletionMessages mirrors Python _normalize_completion_messages. func (s *ChatSessionService) normalizeCompletionMessages( messages []map[string]interface{}, question string, files []interface{}, @@ -1776,7 +1809,7 @@ func (s *ChatSessionService) checkDialogOwnership(ctx context.Context, userID, c return err } if !ok { - return errors.New("no authorization") + return errors.New("No authorization.") } return nil } diff --git a/internal/service/chat_session_contract_test.go b/internal/service/chat_session_contract_test.go index 1682f8d681..5e700c3c7c 100644 --- a/internal/service/chat_session_contract_test.go +++ b/internal/service/chat_session_contract_test.go @@ -118,7 +118,7 @@ func TestCreateSession_NotOwner(t *testing.T) { ctx := t.Context() _, code, err := svc.CreateSession(ctx, "user-1", "chat-1", map[string]interface{}{"name": "x"}) - if err == nil || err.Error() != "no authorization" { + if err == nil || err.Error() != "No authorization." { t.Fatalf("err=%v", err) } if code != common.CodeAuthenticationError { @@ -208,7 +208,7 @@ func TestDeleteSessions_NotOwner(t *testing.T) { ctx := t.Context() _, _, code, err := svc.DeleteSessions(ctx, "user-1", "chat-1", map[string]interface{}{"ids": []interface{}{"s1"}}) - if err == nil || err.Error() != "no authorization" { + if err == nil || err.Error() != "No authorization." { t.Fatalf("err=%v", err) } if code != common.CodeAuthenticationError { diff --git a/internal/service/chat_session_test.go b/internal/service/chat_session_test.go index d766471e6f..93b4da5a03 100644 --- a/internal/service/chat_session_test.go +++ b/internal/service/chat_session_test.go @@ -118,12 +118,25 @@ func (f *fakeSessionStore) DeleteByID(ctx context.Context, db *gorm.DB, id strin return nil } -func (f *fakeSessionStore) ListByChatID(ctx context.Context, db *gorm.DB, chatID string) ([]*entity.ChatSession, error) { +func (f *fakeSessionStore) ListByChatID(ctx context.Context, db *gorm.DB, chatID, sessionID, name, orderby string, desc bool, page, pageSize int) ([]*entity.ChatSession, error) { var result []*entity.ChatSession for _, s := range f.sessions { - if s.DialogID == chatID { - result = append(result, s) + if s.DialogID != chatID { + continue } + if sessionID != "" && s.ID != sessionID { + continue + } + if name != "" { + var sessionName string + if s.Name != nil { + sessionName = *s.Name + } + if sessionName != name { + continue + } + } + result = append(result, s) } return result, nil } @@ -291,7 +304,7 @@ func TestListChatSessions_Success(t *testing.T) { } ctx := t.Context() - resp, err := svc.ListChatSessions(ctx, "user-1", "chat-1") + resp, err := svc.ListChatSessions(ctx, "user-1", "chat-1", "", "", "create_time", true, 1, 30) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -310,9 +323,9 @@ func TestListChatSessions_NotOwner(t *testing.T) { } ctx := t.Context() - _, err := svc.ListChatSessions(ctx, "user-1", "chat-1") - if err == nil || !strings.Contains(err.Error(), "only owner") { - t.Fatalf("expected 'only owner' error, got %v", err) + _, err := svc.ListChatSessions(ctx, "user-1", "chat-1", "", "", "create_time", true, 1, 30) + if err == nil || !strings.Contains(err.Error(), "No authorization") { + t.Fatalf("got %v", err) } } @@ -388,7 +401,7 @@ func TestGetSession_NotOwner(t *testing.T) { ctx := t.Context() _, code, err := svc.GetSession(ctx, "user-1", "chat-1", "session-1") - if err == nil || err.Error() != "no authorization" { + if err == nil || err.Error() != "No authorization." { t.Fatalf("err=%v", err) } if code != common.CodeAuthenticationError { @@ -480,7 +493,7 @@ func TestUpdateSession_ValidationErrors(t *testing.T) { message string code common.ErrorCode }{ - {name: "empty body", req: map[string]interface{}{}, message: "request body cannot be empty", code: common.CodeArgumentError}, + // Empty body is now a valid no-op per the contract. {name: "message", req: map[string]interface{}{"message": []interface{}{}}, message: "`messages` cannot be changed", code: common.CodeDataError}, {name: "messages", req: map[string]interface{}{"messages": []interface{}{}}, message: "`messages` cannot be changed", code: common.CodeDataError}, {name: "reference", req: map[string]interface{}{"reference": []interface{}{}}, message: "`reference` cannot be changed", code: common.CodeDataError}, @@ -1570,3 +1583,67 @@ func TestChunksFormat_UnsupportedTypeReturnsEmpty(t *testing.T) { t.Fatalf("expected empty for string type, got %d", len(result)) } } + +func drainResults(events []AsyncChatResult) <-chan AsyncChatResult { + ch := make(chan AsyncChatResult, len(events)) + for _, e := range events { + ch <- e + } + close(ch) + return ch +} + +func TestAccumulateNonStreamAnswer_MetadataFromFinalEventOnly(t *testing.T) { + // Intermediate events carry conflicting metadata; the response must + // reflect only the final event's values. + ans := accumulateNonStreamAnswer(drainResults([]AsyncChatResult{ + {Answer: "partial ", AudioBinary: "intermediate-audio", Prompt: "intermediate-prompt", CreatedAt: 111}, + {Answer: "answer", Final: true, AudioBinary: "final-audio", Prompt: "final-prompt", CreatedAt: 222}, + })) + if ans["audio_binary"] != "final-audio" { + t.Fatalf("audio_binary=%v, want final-audio", ans["audio_binary"]) + } + if ans["prompt"] != "final-prompt" { + t.Fatalf("prompt=%v, want final-prompt", ans["prompt"]) + } + if ans["created_at"] != float64(222) { + t.Fatalf("created_at=%v, want 222", ans["created_at"]) + } + if ans["answer"] != "answer" { + t.Fatalf("answer=%v, want final decorated answer", ans["answer"]) + } +} + +func TestAccumulateNonStreamAnswer_FinalEventOmitsMetadata(t *testing.T) { + // The final event omits metadata; it must be assigned as-is (nil/zero) + // rather than leaking values from intermediate events. + ans := accumulateNonStreamAnswer(drainResults([]AsyncChatResult{ + {Answer: "partial ", AudioBinary: "intermediate-audio", Prompt: "intermediate-prompt", CreatedAt: 111}, + {Answer: "full answer", Final: true}, + })) + if ans["audio_binary"] != nil { + t.Fatalf("audio_binary=%v, want nil", ans["audio_binary"]) + } + if ans["prompt"] != "" { + t.Fatalf("prompt=%v, want empty", ans["prompt"]) + } + if _, ok := ans["created_at"]; ok { + t.Fatalf("created_at should be omitted, got %v", ans["created_at"]) + } +} + +func TestAccumulateNonStreamAnswer_AccumulatesDeltasUntilFinal(t *testing.T) { + ans := accumulateNonStreamAnswer(drainResults([]AsyncChatResult{ + {Answer: "Hello, "}, + {Answer: "world"}, + })) + if ans["answer"] != "Hello, world" { + t.Fatalf("answer=%v, want accumulated deltas", ans["answer"]) + } + if ans["final"] != true { + t.Fatalf("final=%v, want true", ans["final"]) + } + if ref, _ := ans["reference"].(map[string]interface{}); ref != nil { + t.Fatalf("reference=%v, want nil", ref) + } +} diff --git a/internal/service/chunk/chunk.go b/internal/service/chunk/chunk.go index ef6f460a90..8edd37ae2f 100644 --- a/internal/service/chunk/chunk.go +++ b/internal/service/chunk/chunk.go @@ -961,13 +961,15 @@ func (s *ChunkService) List(ctx context.Context, req *service.ListChunksRequest, chunks = append(chunks, result) } - // Build document info + // Build document info, mirroring Python's _map_doc key renames: + // kb_id→dataset_id, parser_id→chunk_method, token_num→token_count, + // chunk_num→chunk_count, run→text status. timeFormat := "2006-01-02T15:04:05" docInfo := map[string]interface{}{ "id": doc.ID, "thumbnail": doc.Thumbnail, - "kb_id": doc.KbID, - "parser_id": doc.ParserID, + "dataset_id": doc.KbID, + "chunk_method": doc.ParserID, "pipeline_id": doc.PipelineID, "parser_config": doc.ParserConfig, "source_type": doc.SourceType, @@ -976,15 +978,15 @@ func (s *ChunkService) List(ctx context.Context, req *service.ListChunksRequest, "name": doc.Name, "location": doc.Location, "size": doc.Size, - "token_num": doc.TokenNum, - "chunk_num": doc.ChunkNum, + "token_count": doc.TokenNum, + "chunk_count": doc.ChunkNum, "progress": utility.JSONFloat64(doc.Progress), "progress_msg": doc.ProgressMsg, "process_begin_at": utility.FormatTimeToString(doc.ProcessBeginAt, timeFormat), "process_duration": doc.ProcessDuration, "content_hash": doc.ContentHash, "suffix": doc.Suffix, - "run": doc.Run, + "run": chunkDocRunText(doc.Run), "status": doc.Status, "create_time": doc.CreateTime, "create_date": utility.FormatTimeToString(doc.CreateDate, timeFormat), @@ -1714,3 +1716,24 @@ func releaseChunkImageMergeLock(key string) { delete(chunkImageMergeLocks.locks, key) } } + +// chunkDocRunText maps the document run code to its text form, mirroring +// Python's _map_doc run_mapping. +func chunkDocRunText(run *string) interface{} { + if run == nil { + return nil + } + switch *run { + case "0": + return "UNSTART" + case "1": + return "RUNNING" + case "2": + return "CANCEL" + case "3": + return "DONE" + case "4": + return "FAIL" + } + return *run +} diff --git a/internal/service/chunk_types.go b/internal/service/chunk_types.go index 88e6f21086..5be3b3195f 100644 --- a/internal/service/chunk_types.go +++ b/internal/service/chunk_types.go @@ -460,13 +460,15 @@ func (s *ChunkService) List(ctx context.Context, req *ListChunksRequest, userID chunks = append(chunks, result) } - // Build document info + // Build document info, mirroring Python's _map_doc key renames: + // kb_id→dataset_id, parser_id→chunk_method, token_num→token_count, + // chunk_num→chunk_count, run→text status. timeFormat := "2006-01-02T15:04:05" docInfo := map[string]interface{}{ "id": doc.ID, "thumbnail": doc.Thumbnail, - "kb_id": doc.KbID, - "parser_id": doc.ParserID, + "dataset_id": doc.KbID, + "chunk_method": doc.ParserID, "pipeline_id": doc.PipelineID, "parser_config": doc.ParserConfig, "source_type": doc.SourceType, @@ -475,15 +477,15 @@ func (s *ChunkService) List(ctx context.Context, req *ListChunksRequest, userID "name": doc.Name, "location": doc.Location, "size": doc.Size, - "token_num": doc.TokenNum, - "chunk_num": doc.ChunkNum, + "token_count": doc.TokenNum, + "chunk_count": doc.ChunkNum, "progress": utility.JSONFloat64(doc.Progress), "progress_msg": doc.ProgressMsg, "process_begin_at": utility.FormatTimeToString(doc.ProcessBeginAt, timeFormat), "process_duration": doc.ProcessDuration, "content_hash": doc.ContentHash, "suffix": doc.Suffix, - "run": doc.Run, + "run": chunkDocRunText(doc.Run), "status": doc.Status, "create_time": doc.CreateTime, "create_date": utility.FormatTimeToString(doc.CreateDate, timeFormat), @@ -931,6 +933,27 @@ func isInternalField(k string) bool { // applyCommonChunkMapping applies field mappings shared between GetChunk and // ListChunks. Returns true if the field was handled. +// chunkDocRunText maps the document run code to its text form, mirroring +// Python's _map_doc run_mapping. +func chunkDocRunText(run *string) interface{} { + if run == nil { + return nil + } + switch *run { + case "0": + return "UNSTART" + case "1": + return "RUNNING" + case "2": + return "CANCEL" + case "3": + return "DONE" + case "4": + return "FAIL" + } + return *run +} + func applyCommonChunkMapping(result map[string]interface{}, k string, v interface{}) bool { switch k { case "content": diff --git a/internal/service/dataset/create_test.go b/internal/service/dataset/create_test.go index 06b6513ef4..6132d0631d 100644 --- a/internal/service/dataset/create_test.go +++ b/internal/service/dataset/create_test.go @@ -162,7 +162,7 @@ func TestCreateDataset_ValidatesName(t *testing.T) { } } -func TestCreateDataset_RejectsDuplicateName(t *testing.T) { +func TestCreateDataset_DedupesDuplicateName(t *testing.T) { db := setupServiceTestDB(t) pushServiceDB(t, db) insertCreateDatasetTenant(t, "tenant-1") @@ -179,15 +179,16 @@ func TestCreateDataset_RejectsDuplicateName(t *testing.T) { } ctx := t.Context() - _, code, err := testDatasetCreateService(t).CreateDataset(ctx, &service.CreateDatasetRequest{Name: "Existing"}, "tenant-1") - if err == nil { - t.Fatal("expected duplicate name error") + // Mirror Python's duplicate_name: the create appends (1) instead of failing. + result, code, err := testDatasetCreateService(t).CreateDataset(ctx, &service.CreateDatasetRequest{Name: "Existing"}, "tenant-1") + if err != nil { + t.Fatalf("expected success, got %v", err) } - if code != common.CodeDataError { - t.Fatalf("expected data error code, got %d", code) + if code != common.CodeSuccess { + t.Fatalf("expected success code, got %d", code) } - if !strings.Contains(err.Error(), "already exists") { - t.Fatalf("unexpected error: %v", err) + if result["name"] != "Existing(1)" { + t.Fatalf("unexpected name: %v", result["name"]) } } diff --git a/internal/service/dataset/crud.go b/internal/service/dataset/crud.go index 50446bb9f2..7f88d34ed7 100644 --- a/internal/service/dataset/crud.go +++ b/internal/service/dataset/crud.go @@ -117,14 +117,9 @@ func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateD kbID := utility.GenerateToken() status := string(entity.StatusValid) - // Reject duplicate name within tenant to match the established API contract. - existing, err := d.kbDAO.GetByName(ctx, dao.DB, name, tenantID) - if err != nil && !dao.IsNotFoundErr(err) { - return nil, common.CodeServerError, errors.New("database operation failed") - } - if existing != nil { - return nil, common.CodeDataError, fmt.Errorf("dataset name '%s' already exists", name) - } + // Mirror Python's duplicate_name: append (1), (2), ... until the name is + // unique within the tenant. + name = d.dedupeDatasetName(ctx, name, tenantID) kb := &entity.Knowledgebase{ ID: kbID, @@ -159,15 +154,31 @@ func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateD return datasetToMap(createdKB), common.CodeSuccess, nil } +// dedupeDatasetName mirrors Python's duplicate_name: if the name already +// exists within the tenant, append (1), (2), ... until it is unique. +func (d *DatasetService) dedupeDatasetName(ctx context.Context, name, tenantID string) string { + candidate := name + for i := 1; i < 1000; i++ { + existing, err := d.kbDAO.GetByName(ctx, dao.DB, candidate, tenantID) + if err != nil || existing == nil { + return candidate + } + candidate = fmt.Sprintf("%s(%d)", name, i) + } + return candidate +} + func (d *DatasetService) GetDataset(ctx context.Context, datasetID, userID string) (map[string]interface{}, common.ErrorCode, error) { datasetID = strings.TrimSpace(datasetID) if datasetID == "" { return nil, common.CodeDataError, errors.New("lack of \"Dataset ID\"") } + // Mirror Python's get_dataset: no UUID validation up front — any unknown + // or malformed id simply fails the permission check. normalizedID, err := normalizeDatasetID(datasetID) if err != nil { - return nil, common.CodeDataError, err + return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", userID, datasetID) } datasetID = normalizedID @@ -203,7 +214,7 @@ func (d *DatasetService) DeleteDatasets(ctx context.Context, ids []string, delet for _, id := range ids { normalizedID, err := normalizeDatasetID(id) if err != nil { - return nil, common.CodeDataError, err + return nil, common.CodeArgumentError, err } if _, seen := seenIDs[normalizedID]; seen { continue @@ -322,7 +333,7 @@ func (d *DatasetService) ListDatasets(ctx context.Context, id, name string, page if id != "" { normalizedID, err := normalizeDatasetID(id) if err != nil { - return nil, 0, common.CodeDataError, err + return nil, 0, common.CodeArgumentError, err } id = normalizedID diff --git a/internal/service/dataset/helpers.go b/internal/service/dataset/helpers.go index db41589c94..36966489c2 100644 --- a/internal/service/dataset/helpers.go +++ b/internal/service/dataset/helpers.go @@ -162,13 +162,20 @@ func validateDatasetParserConfigSize(parserConfig map[string]interface{}) error return nil } +// NormalizeDatasetID validates the dataset ID format and returns its +// dash-less UUID form. Exported so HTTP handlers can mirror the pydantic +// UUID validation of the Python request models (error code 101). +func NormalizeDatasetID(id string) (string, error) { + return normalizeDatasetID(id) +} + func normalizeDatasetID(id string) (string, error) { parsedUUID, err := uuid.Parse(id) if err != nil { - return "", errors.New("invalid UUID format") + return "", errors.New("Invalid UUID format") } if parsedUUID == (uuid.UUID{}) { - return "", errors.New("invalid UUID format") + return "", errors.New("Invalid UUID format") } return strings.ReplaceAll(parsedUUID.String(), "-", ""), nil } diff --git a/internal/service/dataset/update.go b/internal/service/dataset/update.go index 57831eea9a..a2fbef107b 100644 --- a/internal/service/dataset/update.go +++ b/internal/service/dataset/update.go @@ -9,6 +9,7 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" + "ragflow/internal/engine/types" "ragflow/internal/entity" pipelinepkg "ragflow/internal/ingestion/pipeline" "ragflow/internal/service" @@ -31,7 +32,9 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID tenantID = strings.TrimSpace(tenantID) if _, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID); err != nil { if dao.IsNotFoundErr(err) { - return nil, common.CodeDataError, errors.New("dataset not found") + // Match Python: nonexistent and not-owned datasets share the + // "lacks permission" error so existence is not revealed (IDOR). + return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", tenantID, datasetID) } return nil, common.CodeServerError, errors.New("database operation failed") } @@ -61,7 +64,7 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID if req.Name != nil { name := strings.TrimSpace(*req.Name) if name == "" { - return nil, common.CodeDataError, errors.New("`name` is required") + return nil, common.CodeDataError, errors.New("String should have at least 1 character") } if len(name) > 128 { return nil, common.CodeDataError, errors.New("String should have at most 128 characters") @@ -194,7 +197,7 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID } if lookupErr == nil { txCode = common.CodeDataError - return fmt.Errorf("dataset name '%s' already exists", nameValue) + return fmt.Errorf("Dataset name '%s' already exists", nameValue) } } @@ -289,7 +292,7 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID if dao.IsDuplicateKeyErr(err) { if nameValue, ok := updates["name"].(string); ok { txCode = common.CodeDataError - return fmt.Errorf("dataset name '%s' already exists", nameValue) + return fmt.Errorf("Dataset name '%s' already exists", nameValue) } txCode = common.CodeDataError return errors.New("dataset name already exists") @@ -345,10 +348,18 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID func (d *DatasetService) updateDatasetPagerankChunks(update datasetPagerankUpdate) error { ctx, cancel := context.WithTimeout(context.Background(), datasetPagerankUpdateTimeout) defer cancel() + var err error if update.value > 0 { - return d.docEngine.UpdateChunks(ctx, map[string]interface{}{"kb_id": update.datasetID}, map[string]interface{}{common.PAGERANK_FLD: update.value}, update.index, update.datasetID) + err = d.docEngine.UpdateChunks(ctx, map[string]interface{}{"kb_id": update.datasetID}, map[string]interface{}{common.PAGERANK_FLD: update.value}, update.index, update.datasetID) + } else { + err = d.docEngine.UpdateChunks(ctx, map[string]interface{}{"exists": common.PAGERANK_FLD}, map[string]interface{}{"remove": common.PAGERANK_FLD}, update.index, update.datasetID) } - return d.docEngine.UpdateChunks(ctx, map[string]interface{}{"exists": common.PAGERANK_FLD}, map[string]interface{}{"remove": common.PAGERANK_FLD}, update.index, update.datasetID) + if errors.Is(err, types.ErrIndexNotFound) { + // Python's docStoreConn.update logs and returns False on a missing + // index; the dataset-level pagerank update tolerates it. + return nil + } + return err } func (d *DatasetService) lockAccessibleDatasetForUpdate(tx *gorm.DB, datasetID, userID string) (*entity.Knowledgebase, common.ErrorCode, error) { diff --git a/internal/service/dataset/update_test.go b/internal/service/dataset/update_test.go index 3fbeb8dc8e..dd2b396112 100644 --- a/internal/service/dataset/update_test.go +++ b/internal/service/dataset/update_test.go @@ -309,7 +309,10 @@ func TestDatasetServiceUpdateDatasetRejectsMissingDataset(t *testing.T) { if code != common.CodeDataError { t.Fatalf("expected data error code, got %d", code) } - if err.Error() != "dataset not found" { + // Nonexistent and not-owned datasets share the "lacks permission" + // error so existence is not revealed (IDOR), matching Python. + expected := "user 'tenant-1' lacks permission for dataset 'missing-kb'" + if err.Error() != expected { t.Fatalf("unexpected error: %v", err) } } @@ -382,7 +385,7 @@ func TestDatasetServiceUpdateDatasetValidatesName(t *testing.T) { if code != common.CodeDataError { t.Fatalf("expected data error code, got %d", code) } - if err.Error() != "`name` is required" { + if err.Error() != "String should have at least 1 character" { t.Fatalf("unexpected error: %v", err) } } diff --git a/internal/service/dataset/utils.go b/internal/service/dataset/utils.go index a9afbbda56..a9cb5eb126 100644 --- a/internal/service/dataset/utils.go +++ b/internal/service/dataset/utils.go @@ -15,27 +15,25 @@ import ( ) func datasetListItemToMap(kb *entity.KnowledgebaseListItem) map[string]interface{} { + // avatar/language/description keys are always present (null when unset), + // matching Python's full-row dict response. item := map[string]interface{}{ "id": kb.ID, "name": kb.Name, + "avatar": stringPointerValue(kb.Avatar), + "language": stringPointerValue(kb.Language), + "description": stringPointerValue(kb.Description), "tenant_id": kb.TenantID, "permission": kb.Permission, "document_count": kb.DocNum, "token_num": kb.TokenNum, "chunk_count": kb.ChunkNum, "parser_id": kb.ParserID, + "parser_config": jsonMapValue(kb.ParserConfig), + "pagerank": kb.Pagerank, "embedding_model": kb.EmbdID, "nickname": kb.Nickname, } - if kb.Avatar != nil { - item["avatar"] = *kb.Avatar - } - if kb.Language != nil { - item["language"] = *kb.Language - } - if kb.Description != nil { - item["description"] = *kb.Description - } if kb.TenantAvatar != nil { item["tenant_avatar"] = *kb.TenantAvatar } diff --git a/internal/service/document/document.go b/internal/service/document/document.go index 207ddcf4ae..206fdab8a3 100644 --- a/internal/service/document/document.go +++ b/internal/service/document/document.go @@ -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"` diff --git a/internal/service/document/document_crud.go b/internal/service/document/document_crud.go index 0034e2c12d..f5b0f5e10d 100644 --- a/internal/service/document/document_crud.go +++ b/internal/service/document/document_crud.go @@ -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 diff --git a/internal/service/document/document_dataset_update.go b/internal/service/document/document_dataset_update.go index 6823d762d4..c22717a728 100644 --- a/internal/service/document/document_dataset_update.go +++ b/internal/service/document/document_dataset_update.go @@ -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: - Message: - Value: <%v>", *req.Progress) + return common.CodeDataError, fmt.Errorf("Field: - Message: - 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: - 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: - Message: - Value: ") + } + 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: - Message: - Value: <%s>", pyRepr(typed), pyRepr(map[string]interface{}(meta))) } } default: - return fmt.Errorf("the type is not supported: %v", v) + return fmt.Errorf("Field: - Message: - 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: - Message: - Value: " 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) +} diff --git a/internal/service/document/document_metadata.go b/internal/service/document/document_metadata.go index fc4b3fda62..a749fd252c 100644 --- a/internal/service/document/document_metadata.go +++ b/internal/service/document/document_metadata.go @@ -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 { diff --git a/internal/service/document/document_parse.go b/internal/service/document/document_parse.go index 4356a4d1df..a3fbfce8d4 100644 --- a/internal/service/document/document_parse.go +++ b/internal/service/document/document_parse.go @@ -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 } diff --git a/internal/service/document/document_test.go b/internal/service/document/document_test.go index 19b97eed43..bfe4e3795d 100644 --- a/internal/service/document/document_test.go +++ b/internal/service/document/document_test.go @@ -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()) } } diff --git a/internal/service/metadata.go b/internal/service/metadata.go index 3e8abd5e52..622fbfb942 100644 --- a/internal/service/metadata.go +++ b/internal/service/metadata.go @@ -64,6 +64,27 @@ func BuildMetadataIndexName(tenantID string) string { return fmt.Sprintf("ragflow_doc_meta_%s", tenantID) } +// EnsureMetadataStore creates the metadata index/table for a tenant if it +// does not already exist. This is the create-on-first-write logic that +// belongs in the service layer; the engine layer should assume the store +// already exists when performing insert/update operations. +func (s *MetadataService) EnsureMetadataStore(ctx context.Context, tenantID string) error { + if s.docEngine == nil { + return fmt.Errorf("doc engine is not initialized") + } + exists, err := s.docEngine.MetadataStoreExists(ctx, tenantID) + if err != nil { + return fmt.Errorf("failed to check metadata store existence: %w", err) + } + if exists { + return nil + } + if err := s.docEngine.CreateMetadataStore(ctx, tenantID); err != nil { + return fmt.Errorf("failed to create metadata store: %w", err) + } + return nil +} + // GetTenantIDByKBID retrieves tenant ID from knowledge base ID func (s *MetadataService) GetTenantIDByKBID(ctx context.Context, kbID string) (string, error) { return dao.GetTenantIDByKBID(ctx, dao.DB, kbID) diff --git a/internal/service/search.go b/internal/service/search.go index 6362cd2af6..df4ed5d51f 100644 --- a/internal/service/search.go +++ b/internal/service/search.go @@ -395,7 +395,8 @@ func (s *SearchService) PrepareCompletion(ctx context.Context, userID, searchID for _, datasetID := range datasetIDs { accessible = s.datasetDAO.Accessible(ctx, dao.DB, datasetID, userID) if !accessible { - return nil, common.CodeAuthenticationError, fmt.Errorf("no authorization for dataset %s", datasetID) + // Mirror Python's search completion endpoint message. + return nil, common.CodeDataError, fmt.Errorf("You don't own the dataset %s", datasetID) } } diff --git a/rag/app/picture.py b/rag/app/picture.py index d248a9c717..4c6b3cc21c 100644 --- a/rag/app/picture.py +++ b/rag/app/picture.py @@ -32,7 +32,17 @@ from common.string_utils import clean_markdown_block from deepdoc.vision import OCR from rag.nlp import attach_media_context, rag_tokenizer, tokenize -ocr = OCR() +_ocr = None + + +def _get_ocr(): + """Lazy-init OCR to avoid downloading models at import time (breaks CI + collection when the network is unavailable).""" + global _ocr + if _ocr is None: + _ocr = OCR() + return _ocr + # Gemini supported MIME types VIDEO_EXTS = [".mp4", ".mov", ".avi", ".flv", ".mpeg", ".mpg", ".webm", ".wmv", ".3gp", ".3gpp", ".mkv"] @@ -78,7 +88,7 @@ def chunk(filename, binary, tenant_id, lang, callback=None, **kwargs): if not txt: # Fallback to local deepdoc OCR - bxs = ocr(np.array(img)) + bxs = _get_ocr()(np.array(img)) txt = "\n".join([t[0] for _, t in bxs if t[0]]) callback(0.4, "Finish OCR: (%s ...)" % txt[:12]) diff --git a/test/testcases/restful_api/conftest.py b/test/testcases/restful_api/conftest.py index c7b287b343..86a1a6a648 100644 --- a/test/testcases/restful_api/conftest.py +++ b/test/testcases/restful_api/conftest.py @@ -38,77 +38,42 @@ GO_ONLY_SKIPS = { "test_cancel_missing_task_sets_cancel_contract", }, "Go validation or response contract does not match the established API contract": { - "test_chat_list_page_and_page_size_contract", - "test_chat_list_sorting_contract", "test_dataset_update_parser_config_valid_matrix_contract", "test_dataset_update_parser_config_with_chunk_method_change_contract", - "test_dataset_update_pagerank_contract", - "test_dataset_update_pagerank_set_to_zero_contract", - "test_dataset_update_content_type_and_payload_contract", - "test_dataset_update_identifier_validation_contract", "test_dataset_update_parser_config_invalid_contract", - "test_dataset_update_field_unset_and_unsupported_contract", - "test_dataset_update_name_invalid_and_duplicate_contract", - "test_dataset_create_name_and_case_insensitive_contract", # Updating with `{"parser_config": {}}` / `None` is a valid no-op in Go (handled by # ParserConfigProvided). But the final GET asserts the stored parser_config equals Python's # DEFAULT_PARSER_CONFIG, which embeds a CI-specific tenant `llm_id` and a richer `graphrag` / # `parent_child` structure than Go's common.GetParserConfig produces. Exact equality is a # Python/CI-specific contract, not a meaningful Go behavior difference. "test_dataset_update_parser_config_defaults_contract", + # Go CreateDataset does not accept parser_config in the request body. "test_dataset_create_parser_config_different_chunk_methods_contract", "test_dataset_create_parser_config_missing_raptor_and_graphrag", - "test_dataset_create_embedding_model_contract", "test_dataset_create_parser_config_bugfix_contract", - "test_dataset_create_content_type_and_payload_bad_contract", "test_dataset_create_parser_config_invalid_contract", "test_dataset_create_parser_config_defaults_and_extra_fields_contract", - "test_dataset_list_query_contract_matrix", - "test_dataset_delete_contract_matrix", - # "test_memory_update_invalid_name", - # "test_memory_crud_cycle", - # "test_messages_add_list_recent_content_update_forget", - # "test_message_status_validation_requires_boolean", - # "test_message_search_route_contract", - # "test_memory_crud_and_config", - # "test_messages_list_and_search_validation_contracts", - # "test_message_update_forget_and_content_error_contracts", + "test_dataset_create_embedding_model_contract", + # Empty path (e.g. /chats//sessions) triggers a 405/404 framework + # response in Go rather than the Python contract's code-100 envelope. "test_session_create_validation_and_deleted_chat_contract", - "test_session_delete_basic_scenarios", - "test_session_list_filter_and_deleted_chat_contract", - "test_session_list_page_and_sort_contract", - "test_session_update_name_and_param_contract", "test_session_update_requires_auth_and_invalid_target_contract", - "test_chat_completion_validation_errors", - "test_chat_completion_nonstream_with_session", - "test_chat_completion_nonstream_with_chat_without_session", - "test_chat_completion_nonstream_without_chat", - "test_chat_completion_stream_events", - "test_search_completion_sse_shape_when_kb_ids_provided", - "test_system_tokens_auth_and_crud", - # --- exposed after meta_fields skip guard removal --- + # Go response data omits the parser_config key that the contract asserts. + "test_documents_update_parser_config_contract", + }, + "Go ingestion pipeline does not complete document parsing within the test timeout": { + # Chunk add requires an embedding model call; document update/delete + # touches ES/Infinity indices that are only created during parsing. + # Without a working Go ingestion pipeline, these indices never exist. "test_chunk_add_keyword_question_and_tag_contract", "test_chunk_add_repeated_and_deleted_document_contract", "test_chunk_concurrent_add_contract", - "test_documents_list_default_concurrent_and_filters_contract", - "test_documents_list_error_and_sorting_contract", "test_documents_update_patch_and_delete", "test_documents_update_name_contract", - "test_documents_update_chunk_method_contract", "test_documents_update_meta_fields_contract", - "test_documents_update_invalid_field_and_guard_contract", - "test_documents_update_parser_config_contract", "test_documents_metadata_batch_update_contract", "test_documents_metadata_update_path", - "test_documents_delete_contract_matrix", "test_documents_delete_invalid_dataset_partial_duplicate_repeat_and_cross_dataset", - "test_documents_delete_concurrent_and_bulk_contract", - "test_documents_download_requires_auth_and_invalid_id_contract", - }, - "Go LLM setup cannot exercise the configured model": { - "test_related_questions_contract", - }, - "Go ingestion pipeline does not complete document parsing within the test timeout": { "test_chat_list_concurrent_and_dataset_delete_contract", "test_chat_create_dataset_ids_contract", "test_chat_create_llm_contract", @@ -127,11 +92,9 @@ GO_ONLY_SKIPS = { "test_retrieval_document_ids_and_metadata_condition_contract", "test_retrieval_rerank_unknown_contract", "test_retrieval_concurrent_contract", - "test_documents_parse_and_stop", "test_documents_parse_contract_matrix", "test_documents_parse_invalid_dataset_partial_duplicate_and_repeated", "test_documents_parse_chunks_and_scaled_bulk_contract", - "test_documents_stop_parse_requires_auth", "test_documents_stop_parse_contract_matrix", "test_documents_stop_parse_invalid_dataset_partial_and_scaled_concurrency", "test_documents_table_parser_chat_patterns", diff --git a/test/testcases/restful_api/test_chats.py b/test/testcases/restful_api/test_chats.py index e4b5d2a65d..ad1c44dc1f 100644 --- a/test/testcases/restful_api/test_chats.py +++ b/test/testcases/restful_api/test_chats.py @@ -365,15 +365,15 @@ def test_chat_list_page_and_page_size_contract(rest_client, clear_chats): ("page two", {"page": 2, "page_size": 2}, 0, lambda total: min(max(total - 2, 0), 2), ""), ("page three", {"page": 3, "page_size": 2}, 0, lambda total: min(max(total - 4, 0), 2), ""), ("page string", {"page": "3", "page_size": 2}, 0, lambda total: min(max(total - 4, 0), 2), ""), - ("page negative", {"page": -1, "page_size": 2}, 100, None, "ProgrammingError(1064"), - ("page alpha", {"page": "a", "page_size": 2}, 100, None, "ValueError(\"invalid literal for int() with base 10: 'a'\")"), + ("page negative", {"page": -1, "page_size": 2}, 0, lambda total: total, ""), + ("page alpha", {"page": "a", "page_size": 2}, 0, lambda total: total, ""), ("page_size none", {"page_size": None}, 0, lambda total: total, ""), ("page_size zero", {"page_size": 0}, 0, lambda total: total, ""), ("page_size one", {"page_size": 1}, 0, lambda total: total, ""), ("page_size six", {"page_size": 6}, 0, lambda total: total, ""), ("page_size string", {"page_size": "1"}, 0, lambda total: total, ""), ("page_size negative", {"page_size": -1}, 0, lambda total: total, ""), - ("page_size alpha", {"page_size": "a"}, 100, None, "ValueError(\"invalid literal for int() with base 10: 'a'\")"), + ("page_size alpha", {"page_size": "a"}, 0, lambda total: total, ""), ] for scenario_name, params, expected_code, expected_count_fn, expected_message in cases: @@ -403,7 +403,7 @@ def test_chat_list_sorting_contract(rest_client, clear_chats): ("orderby create", {"orderby": "create_time"}, 0, descending_names, ""), ("orderby update", {"orderby": "update_time"}, 0, descending_names, ""), ("orderby name ascending", {"orderby": "name", "desc": "False"}, 0, ascending_names, ""), - ("orderby unknown", {"orderby": "unknown"}, 100, None, "AttributeError(\"type object 'Dialog' has no attribute 'unknown'\")"), + ("orderby unknown", {"orderby": "unknown"}, 101, None, "invalid orderby field"), ("desc none", {"desc": None}, 0, descending_names, ""), ("desc true", {"desc": "true"}, 0, descending_names, ""), ("desc True", {"desc": "True"}, 0, descending_names, ""), diff --git a/test/testcases/restful_api/test_chunks.py b/test/testcases/restful_api/test_chunks.py index 336621b524..30d03ab000 100644 --- a/test/testcases/restful_api/test_chunks.py +++ b/test/testcases/restful_api/test_chunks.py @@ -200,7 +200,7 @@ def test_chunk_add_keyword_question_and_tag_contract(rest_client, create_documen [ ({"content": "chunk test", "important_keywords": ["a", "b", "c"]}, 0, ""), ({"content": "chunk test", "important_keywords": [""]}, 0, ""), - ({"content": "chunk test", "important_keywords": [1]}, 100, "TypeError('sequence item 0: expected str instance, int found')"), + ({"content": "chunk test", "important_keywords": [1]}, 102, "`important_keywords` must be a list of strings"), ({"content": "chunk test", "important_keywords": ["a", "a"]}, 0, ""), ({"content": "chunk test", "important_keywords": "abc"}, 102, "`important_keywords` is required to be a list"), ({"content": "chunk test", "important_keywords": 123}, 102, "`important_keywords` is required to be a list"), @@ -211,7 +211,7 @@ def test_chunk_add_keyword_question_and_tag_contract(rest_client, create_documen [ ({"content": "chunk test", "questions": ["a", "b", "c"]}, 0, ""), ({"content": "chunk test", "questions": [""]}, 0, ""), - ({"content": "chunk test", "questions": [1]}, 100, "TypeError('sequence item 0: expected str instance, int found')"), + ({"content": "chunk test", "questions": [1]}, 102, "`questions` must be a list of strings"), ({"content": "chunk test", "questions": ["a", "a"]}, 0, ""), ({"content": "chunk test", "questions": "abc"}, 102, "`questions` is required to be a list"), ({"content": "chunk test", "questions": 123}, 102, "`questions` is required to be a list"), diff --git a/test/testcases/restful_api/test_documents.py b/test/testcases/restful_api/test_documents.py index 24d064d779..aaa1301b57 100644 --- a/test/testcases/restful_api/test_documents.py +++ b/test/testcases/restful_api/test_documents.py @@ -289,36 +289,36 @@ def test_documents_list_error_and_sorting_contract(rest_client, create_dataset, "orderby invalid", f"/datasets/{dataset_id}/documents", {"orderby": "unknown"}, - 100, - "Document' has no attribute 'unknown'", + 101, + "invalid orderby field", ), ( "page invalid number", f"/datasets/{dataset_id}/documents", {"page": -1, "page_size": 2}, - 100, - "1064", + 0, + "", ), ( "page invalid type", f"/datasets/{dataset_id}/documents", {"page": "a", "page_size": 2}, - 100, - "invalid literal for int()", + 0, + "", ), ( "page_size invalid number", f"/datasets/{dataset_id}/documents", {"page_size": -1}, - 100, - "1064", + 0, + "", ), ( "page_size invalid type", f"/datasets/{dataset_id}/documents", {"page_size": "a"}, - 100, - "invalid literal for int()", + 0, + "", ), ] for case_name, path, params, expected_code, expected_message in error_cases: @@ -547,7 +547,7 @@ def test_documents_update_name_contract(rest_client, create_dataset, tmp_path): ("new_name.txt", 0, ""), (long_name, 0, ""), (0, 102, "Field: - Message: - Value: <0>"), - (None, 100, "AttributeError('NoneType' object has no attribute 'encode')"), + (None, 102, "Field: - Message: - Value: "), ("", 101, "The extension of file can't be changed"), ("ragflow_test_upload_0", 101, "The extension of file can't be changed"), ("ragflow_test_upload_1.txt", 102, "Duplicated document name in the same dataset."), diff --git a/test/testcases/restful_api/test_searches.py b/test/testcases/restful_api/test_searches.py index ccd3be2ac2..afed81e328 100644 --- a/test/testcases/restful_api/test_searches.py +++ b/test/testcases/restful_api/test_searches.py @@ -101,7 +101,7 @@ def test_search_update_invalid_search_id(rest_client): assert res.status_code == 200 payload = res.json() assert payload["code"] == 109, payload - assert "no authorization" in payload["message"], payload + assert "no authorization" in payload["message"].lower(), payload @pytest.mark.p2 diff --git a/test/testcases/restful_api/test_sessions.py b/test/testcases/restful_api/test_sessions.py index 328f47fe2c..728e961fb8 100644 --- a/test/testcases/restful_api/test_sessions.py +++ b/test/testcases/restful_api/test_sessions.py @@ -146,7 +146,7 @@ def test_session_create_validation_and_deleted_chat_contract(rest_client, create assert invalid_chat_res.status_code == 200 invalid_chat_payload = invalid_chat_res.json() assert invalid_chat_payload["code"] == 109, invalid_chat_payload - assert invalid_chat_payload["message"] == "no authorization", invalid_chat_payload + assert invalid_chat_payload["message"].lower().strip().rstrip(".") == "no authorization", invalid_chat_payload for scenario_name, payload in ( ("valid", {"name": "valid_name"}), @@ -194,7 +194,7 @@ def test_session_create_validation_and_deleted_chat_contract(rest_client, create assert create_after_delete.status_code == 200 create_after_delete_payload = create_after_delete.json() assert create_after_delete_payload["code"] == 109, create_after_delete_payload - assert create_after_delete_payload["message"] == "no authorization", create_after_delete_payload + assert create_after_delete_payload["message"].lower().strip().rstrip(".") == "no authorization", create_after_delete_payload @pytest.mark.p2 @@ -234,7 +234,7 @@ def test_session_delete_requires_auth_and_invalid_target_contract(rest_client, c assert invalid_chat_res.status_code == 200 invalid_chat_payload = invalid_chat_res.json() assert invalid_chat_payload["code"] == 109, invalid_chat_payload - assert invalid_chat_payload["message"] == "no authorization", invalid_chat_payload + assert invalid_chat_payload["message"].lower().strip().rstrip(".") == "no authorization", invalid_chat_payload @pytest.mark.p2 @@ -242,7 +242,7 @@ def test_session_delete_basic_scenarios(rest_client, create_chat): cases = [ ("none payload", None, 0, 5, {}), ("invalid only", {"ids": ["invalid_id"]}, 102, 5, "The chat doesn't own the session invalid_id"), - ("not json", "not json", 100, 5, ""), + ("not json", "not json", 101, 5, "Malformed JSON syntax: Missing commas/brackets or invalid encoding"), ("single id", lambda sessions: {"ids": [sessions[0]["id"]]}, 0, 4, True), ("all ids", lambda sessions: {"ids": [session["id"] for session in sessions]}, 0, 0, True), ("delete all", {"delete_all": True}, 0, 0, True), @@ -394,7 +394,7 @@ def test_session_list_filter_and_deleted_chat_contract(rest_client, create_chat) assert invalid_chat_res.status_code == 200 invalid_chat_payload = invalid_chat_res.json() assert invalid_chat_payload["code"] == 109, invalid_chat_payload - assert invalid_chat_payload["message"] == "no authorization", invalid_chat_payload + assert invalid_chat_payload["message"].lower().strip().rstrip(".") == "no authorization", invalid_chat_payload delete_chat_res = rest_client.delete("/chats", json={"ids": [chat_id]}) assert delete_chat_res.status_code == 200 @@ -405,7 +405,7 @@ def test_session_list_filter_and_deleted_chat_contract(rest_client, create_chat) assert deleted_list_res.status_code == 200 deleted_list_payload = deleted_list_res.json() assert deleted_list_payload["code"] == 109, deleted_list_payload - assert deleted_list_payload["message"] == "no authorization", deleted_list_payload + assert deleted_list_payload["message"].lower().strip().rstrip(".") == "no authorization", deleted_list_payload @pytest.mark.p2 @@ -420,14 +420,14 @@ def test_session_list_page_and_sort_contract(rest_client, create_chat): ("page two", {"page": 2, "page_size": 2}, 0, 2, ""), ("page three", {"page": 3, "page_size": 2}, 0, 1, ""), ("page string", {"page": "3", "page_size": 2}, 0, 1, ""), - ("page negative", {"page": -1, "page_size": 2}, 100, 0, "ProgrammingError(1064"), - ("page alpha", {"page": "a", "page_size": 2}, 100, 0, "ValueError(\"invalid literal for int() with base 10: 'a'\")"), + ("page negative", {"page": -1, "page_size": 2}, 0, 2, ""), + ("page alpha", {"page": "a", "page_size": 2}, 0, 2, ""), ("page_size none", {"page_size": None}, 0, 5, ""), ("page_size zero", {"page_size": 0}, 0, 0, ""), ("page_size one", {"page_size": 1}, 0, 1, ""), ("page_size six", {"page_size": 6}, 0, 5, ""), ("page_size negative", {"page_size": -1}, 0, 5, ""), - ("page_size alpha", {"page_size": "a"}, 100, 0, "ValueError(\"invalid literal for int() with base 10: 'a'\")"), + ("page_size alpha", {"page_size": "a"}, 0, 5, ""), ] for scenario_name, params, expected_code, expected_count, expected_message in page_cases: res = rest_client.get(f"/chats/{chat_id}/sessions", params=params) @@ -444,7 +444,7 @@ def test_session_list_page_and_sort_contract(rest_client, create_chat): ("orderby create", {"orderby": "create_time", "page_size": 30}, "create_time", True, descending_names, ""), ("orderby update", {"orderby": "update_time", "page_size": 30}, "update_time", True, descending_names, ""), ("orderby name ascending", {"orderby": "name", "desc": "False", "page_size": 30}, "name", False, created_names, ""), - ("orderby unknown", {"orderby": "unknown", "page_size": 30}, None, None, None, "AttributeError(\"type object 'Conversation' has no attribute 'unknown'\")"), + ("orderby unknown", {"orderby": "unknown", "page_size": 30}, None, None, None, "invalid orderby field"), ("desc none", {"desc": None, "page_size": 30}, "create_time", True, descending_names, ""), ("desc true", {"desc": "true", "page_size": 30}, "create_time", True, descending_names, ""), ("desc True", {"desc": "True", "page_size": 30}, "create_time", True, descending_names, ""), @@ -457,7 +457,7 @@ def test_session_list_page_and_sort_contract(rest_client, create_chat): res = rest_client.get(f"/chats/{chat_id}/sessions", params=params) assert res.status_code == 200, (scenario_name, res.text) payload = res.json() - expected_code = 0 if expected_names is not None else 100 + expected_code = 0 if expected_names is not None else 101 assert payload["code"] == expected_code, (scenario_name, payload) if expected_code == 0: assert is_sorted(payload["data"], field, descending), (scenario_name, payload) @@ -498,7 +498,7 @@ def test_session_update_requires_auth_and_invalid_target_contract(rest_client, c assert invalid_chat_res.status_code == 200 invalid_chat_payload = invalid_chat_res.json() assert invalid_chat_payload["code"] == 109, invalid_chat_payload - assert invalid_chat_payload["message"] == "no authorization", invalid_chat_payload + assert invalid_chat_payload["message"].lower().strip().rstrip(".") == "no authorization", invalid_chat_payload empty_session_res = rest_client.patch(f"/chats/{chat_id}/sessions/", json={"name": "x"}) assert empty_session_res.status_code == 200 @@ -552,7 +552,7 @@ def test_session_update_name_and_param_contract(rest_client, create_chat): assert update_after_delete_res.status_code == 200 update_after_delete_payload = update_after_delete_res.json() assert update_after_delete_payload["code"] == 109, update_after_delete_payload - assert update_after_delete_payload["message"] == "no authorization", update_after_delete_payload + assert update_after_delete_payload["message"].lower().strip().rstrip(".") == "no authorization", update_after_delete_payload @pytest.mark.p2 @@ -602,7 +602,8 @@ def test_related_questions_compatibility_requires_auth(rest_client_noauth): assert res.status_code == 200 payload = res.json() if IS_GO_PROXY: - assert_auth_error(payload, "invalid token") + assert payload["code"] == 102, payload + assert payload["message"] == "Authorization is not valid!", payload else: assert payload["code"] == 102, payload assert payload["message"].strip() in { @@ -759,4 +760,4 @@ def test_chat_completion_validation_errors(rest_client, create_chat): assert invalid_chat.status_code == 200 invalid_chat_payload = invalid_chat.json() assert invalid_chat_payload["code"] == 109, invalid_chat_payload - assert "no authorization" in invalid_chat_payload["message"], invalid_chat_payload + assert "no authorization" in invalid_chat_payload["message"].lower(), invalid_chat_payload