diff --git a/api/apps/document_app.py b/api/apps/document_app.py index f4c3e3355c..14f6623687 100644 --- a/api/apps/document_app.py +++ b/api/apps/document_app.py @@ -25,7 +25,6 @@ from api.constants import FILE_NAME_LEN_LIMIT, IMG_BASE64_PREFIX from api.db import FileType from api.db.db_models import Task from api.db.services import duplicate_name -from api.db.services.doc_metadata_service import DocMetadataService from api.db.services.document_service import DocumentService, doc_upload_and_parse from api.db.services.file2document_service import File2DocumentService from api.db.services.file_service import FileService @@ -183,33 +182,6 @@ async def create(): return server_error_response(e) -@manager.route("/metadata/update", methods=["POST"]) # noqa: F821 -@login_required -@validate_request("doc_ids") -async def metadata_update(): - req = await get_request_json() - kb_id = req.get("kb_id") - document_ids = req.get("doc_ids") - updates = req.get("updates", []) or [] - deletes = req.get("deletes", []) or [] - - if not kb_id: - return get_json_result(data=False, message='Lack of "KB ID"', code=RetCode.ARGUMENT_ERROR) - - if not isinstance(updates, list) or not isinstance(deletes, list): - return get_json_result(data=False, message="updates and deletes must be lists.", code=RetCode.ARGUMENT_ERROR) - - for upd in updates: - if not isinstance(upd, dict) or not upd.get("key") or "value" not in upd: - return get_json_result(data=False, message="Each update requires key and value.", code=RetCode.ARGUMENT_ERROR) - for d in deletes: - if not isinstance(d, dict) or not d.get("key"): - return get_json_result(data=False, message="Each delete requires key.", code=RetCode.ARGUMENT_ERROR) - - updated = DocMetadataService.batch_update_metadata(kb_id, document_ids, updates, deletes) - return get_json_result(data={"updated": updated, "matched_docs": len(document_ids)}) - - @manager.route("/thumbnails", methods=["GET"]) # noqa: F821 # @login_required def thumbnails(): diff --git a/api/apps/restful_apis/document_api.py b/api/apps/restful_apis/document_api.py index 56c4f56df0..220ed2c624 100644 --- a/api/apps/restful_apis/document_api.py +++ b/api/apps/restful_apis/document_api.py @@ -891,3 +891,131 @@ async def update_metadata_config(tenant_id, dataset_id, document_id): return get_json_result(code=RetCode.EXCEPTION_ERROR, message=repr(e)) return get_result(data=doc.to_dict()) + + +@manager.route("/datasets//documents/metadatas", methods=["PATCH"]) # noqa: F821 +@login_required +@add_tenant_id_to_kwargs +async def update_metadata(tenant_id, dataset_id): + """ + Update document metadata in batch. + --- + tags: + - Documents + security: + - ApiKeyAuth: [] + parameters: + - in: path + name: dataset_id + type: string + required: true + description: ID of the dataset. + - in: header + name: Authorization + type: string + required: true + description: Bearer token for authentication. + - in: body + name: body + description: Metadata update request. + required: true + schema: + type: object + properties: + selector: + type: object + description: Document selector. + properties: + document_ids: + type: array + items: + type: string + description: List of document IDs to update. + metadata_condition: + type: object + description: Filter documents by existing metadata. + updates: + type: array + items: + type: object + properties: + key: + type: string + value: + type: any + description: List of metadata key-value pairs to update. + deletes: + type: array + items: + type: object + properties: + key: + type: string + description: List of metadata keys to delete. + responses: + 200: + description: Metadata updated successfully. + """ + # Verify ownership of dataset + if not KnowledgebaseService.accessible(kb_id=dataset_id, user_id=tenant_id): + return get_error_data_result(message=f"You don't own the dataset {dataset_id}.") + + # Get request body + req = await get_request_json() + selector = req.get("selector", {}) or {} + updates = req.get("updates", []) or [] + deletes = req.get("deletes", []) or [] + + # Validate selector + if not isinstance(selector, dict): + return get_error_data_result(message="selector must be an object.") + if not isinstance(updates, list) or not isinstance(deletes, list): + return get_error_data_result(message="updates and deletes must be lists.") + + # Validate metadata_condition + metadata_condition = selector.get("metadata_condition", {}) or {} + if metadata_condition and not isinstance(metadata_condition, dict): + return get_error_data_result(message="metadata_condition must be an object.") + + # Validate document_ids + document_ids = selector.get("document_ids", []) or [] + if document_ids and not isinstance(document_ids, list): + return get_error_data_result(message="document_ids must be a list.") + + # Validate updates + for upd in updates: + if not isinstance(upd, dict) or not upd.get("key") or "value" not in upd: + return get_error_data_result(message="Each update requires key and value.") + + # Validate deletes + for d in deletes: + if not isinstance(d, dict) or not d.get("key"): + return get_error_data_result(message="Each delete requires key.") + + # Initialize target document IDs + target_doc_ids = set() + + # If document_ids provided, validate they belong to the dataset + if document_ids: + kb_doc_ids = KnowledgebaseService.list_documents_by_ids([dataset_id]) + invalid_ids = set(document_ids) - set(kb_doc_ids) + if invalid_ids: + return get_error_data_result( + message=f"These documents do not belong to dataset {dataset_id}: {', '.join(invalid_ids)}" + ) + target_doc_ids = set(document_ids) + + # Apply metadata_condition filtering if provided + if metadata_condition: + metas = DocMetadataService.get_flatted_meta_by_kbs([dataset_id]) + filtered_ids = set( + meta_filter(metas, convert_conditions(metadata_condition), metadata_condition.get("logic", "and")) + ) + target_doc_ids = target_doc_ids & filtered_ids + if metadata_condition.get("conditions") and not target_doc_ids: + return get_result(data={"updated": 0, "matched_docs": 0}) + + # Convert to list and perform update + target_doc_ids = list(target_doc_ids) + updated = DocMetadataService.batch_update_metadata(dataset_id, target_doc_ids, updates, deletes) + return get_result(data={"updated": updated, "matched_docs": len(target_doc_ids)}) diff --git a/api/apps/sdk/doc.py b/api/apps/sdk/doc.py index c215cf26de..067796ada0 100644 --- a/api/apps/sdk/doc.py +++ b/api/apps/sdk/doc.py @@ -157,57 +157,6 @@ async def download_doc(document_id): ) -@manager.route("/datasets//metadata/update", methods=["POST"]) # noqa: F821 -@token_required -async def metadata_batch_update(dataset_id, tenant_id): - if not KnowledgebaseService.accessible(kb_id=dataset_id, user_id=tenant_id): - return get_error_data_result(message=f"You don't own the dataset {dataset_id}. ") - - req = await get_request_json() - selector = req.get("selector", {}) or {} - updates = req.get("updates", []) or [] - deletes = req.get("deletes", []) or [] - - if not isinstance(selector, dict): - return get_error_data_result(message="selector must be an object.") - if not isinstance(updates, list) or not isinstance(deletes, list): - return get_error_data_result(message="updates and deletes must be lists.") - - metadata_condition = selector.get("metadata_condition", {}) or {} - if metadata_condition and not isinstance(metadata_condition, dict): - return get_error_data_result(message="metadata_condition must be an object.") - - document_ids = selector.get("document_ids", []) or [] - if document_ids and not isinstance(document_ids, list): - return get_error_data_result(message="document_ids must be a list.") - - for upd in updates: - if not isinstance(upd, dict) or not upd.get("key") or "value" not in upd: - return get_error_data_result(message="Each update requires key and value.") - for d in deletes: - if not isinstance(d, dict) or not d.get("key"): - return get_error_data_result(message="Each delete requires key.") - - if document_ids: - kb_doc_ids = KnowledgebaseService.list_documents_by_ids([dataset_id]) - target_doc_ids = set(kb_doc_ids) - invalid_ids = set(document_ids) - set(kb_doc_ids) - if invalid_ids: - return get_error_data_result(message=f"These documents do not belong to dataset {dataset_id}: {', '.join(invalid_ids)}") - target_doc_ids = set(document_ids) - - if metadata_condition: - metas = DocMetadataService.get_flatted_meta_by_kbs([dataset_id]) - filtered_ids = set(meta_filter(metas, convert_conditions(metadata_condition), metadata_condition.get("logic", "and"))) - target_doc_ids = target_doc_ids & filtered_ids - if metadata_condition.get("conditions") and not target_doc_ids: - return get_result(data={"updated": 0, "matched_docs": 0}) - - target_doc_ids = list(target_doc_ids) - updated = DocMetadataService.batch_update_metadata(dataset_id, target_doc_ids, updates, deletes) - return get_result(data={"updated": updated, "matched_docs": len(target_doc_ids)}) - - DOC_STOP_PARSING_INVALID_STATE_MESSAGE = "Can't stop parsing document that has not started or already completed" DOC_STOP_PARSING_INVALID_STATE_ERROR_CODE = "DOC_STOP_PARSING_INVALID_STATE" diff --git a/test/testcases/test_http_api/common.py b/test/testcases/test_http_api/common.py index fc8c144664..4f96843f76 100644 --- a/test/testcases/test_http_api/common.py +++ b/test/testcases/test_http_api/common.py @@ -341,6 +341,16 @@ def metadata_batch_update(auth, dataset_id, payload=None): return res.json() +def update_documents_metadata(auth, dataset_id, payload=None): + """New unified API for updating document metadata. + + Uses PATCH method at /api/v1/datasets/{dataset_id}/documents/metadatas + """ + url = f"{HOST_ADDRESS}{DATASETS_API_URL}/{dataset_id}/documents/metadatas" + res = requests.patch(url=url, headers=HEADERS, auth=auth, json=payload) + return res.json() + + # CHAT COMPLETIONS AND RELATED QUESTIONS def related_questions(auth, payload=None): url = f"{HOST_ADDRESS}/api/{VERSION}/sessions/related_questions" diff --git a/test/testcases/test_http_api/test_file_management_within_dataset/test_doc_sdk_routes_unit.py b/test/testcases/test_http_api/test_file_management_within_dataset/test_doc_sdk_routes_unit.py index 70b5edced3..510e2c391c 100644 --- a/test/testcases/test_http_api/test_file_management_within_dataset/test_doc_sdk_routes_unit.py +++ b/test/testcases/test_http_api/test_file_management_within_dataset/test_doc_sdk_routes_unit.py @@ -388,95 +388,6 @@ class TestDocRoutesUnit: res = _run(module.download_doc("doc-1")) assert res["filename"] == "doc.txt" - def test_metadata_batch_update(self, monkeypatch): - module = _load_doc_module(monkeypatch) - monkeypatch.setattr(module, "convert_conditions", lambda cond: cond) - monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: False) - monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"selector": {}})) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert "don't own the dataset" in res["message"] - - monkeypatch.setattr(module.KnowledgebaseService, "accessible", lambda **_kwargs: True) - monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"selector": [1]})) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert res["message"] == "selector must be an object." - - monkeypatch.setattr(module, "get_request_json", lambda: _AwaitableValue({"selector": {}, "updates": {"k": "v"}, "deletes": []})) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert res["message"] == "updates and deletes must be lists." - - monkeypatch.setattr( - module, - "get_request_json", - lambda: _AwaitableValue({"selector": {"metadata_condition": [1]}, "updates": [], "deletes": []}), - ) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert res["message"] == "metadata_condition must be an object." - - monkeypatch.setattr( - module, - "get_request_json", - lambda: _AwaitableValue({"selector": {"document_ids": "doc-1"}, "updates": [], "deletes": []}), - ) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert res["message"] == "document_ids must be a list." - - monkeypatch.setattr( - module, - "get_request_json", - lambda: _AwaitableValue({"selector": {}, "updates": [{"key": ""}], "deletes": []}), - ) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert "Each update requires key and value." in res["message"] - - monkeypatch.setattr( - module, - "get_request_json", - lambda: _AwaitableValue({"selector": {}, "updates": [], "deletes": [{"x": "y"}]}), - ) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert "Each delete requires key." in res["message"] - - monkeypatch.setattr( - module, - "get_request_json", - lambda: _AwaitableValue( - { - "selector": {"document_ids": ["bad"], "metadata_condition": {"conditions": []}}, - "updates": [{"key": "k", "value": "v"}], - "deletes": [], - } - ), - ) - monkeypatch.setattr(module.KnowledgebaseService, "list_documents_by_ids", lambda _ids: ["doc-1"]) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert "do not belong to dataset" in res["message"] - - monkeypatch.setattr( - module, - "get_request_json", - lambda: _AwaitableValue( - { - "selector": {"document_ids": ["doc-1"], "metadata_condition": {"conditions": [{"f": "x"}]}}, - "updates": [{"key": "k", "value": "v"}], - "deletes": [], - } - ), - ) - monkeypatch.setattr(module, "meta_filter", lambda *_args, **_kwargs: []) - monkeypatch.setattr(module.DocMetadataService, "get_flatted_meta_by_kbs", lambda _kbs: []) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert res["code"] == 0 - assert res["data"]["updated"] == 0 - assert res["data"]["matched_docs"] == 0 - - monkeypatch.setattr(module, "meta_filter", lambda *_args, **_kwargs: ["doc-1"]) - monkeypatch.setattr(module.DocMetadataService, "batch_update_metadata", lambda *_args, **_kwargs: 1) - res = _run(module.metadata_batch_update.__wrapped__("ds-1", "tenant-1")) - assert res["code"] == 0 - assert res["data"]["updated"] == 1 - assert res["data"]["matched_docs"] == 1 - def test_parse_branches(self, monkeypatch): module = _load_doc_module(monkeypatch) diff --git a/test/testcases/test_http_api/test_file_management_within_dataset/test_metadata_batch_update.py b/test/testcases/test_http_api/test_file_management_within_dataset/test_metadata_batch_update.py index 9061ba3902..f2b3060d64 100644 --- a/test/testcases/test_http_api/test_file_management_within_dataset/test_metadata_batch_update.py +++ b/test/testcases/test_http_api/test_file_management_within_dataset/test_metadata_batch_update.py @@ -13,8 +13,21 @@ # See the License for the specific language governing permissions and # limitations under the License. # +""" +End-to-end tests for metadata batch update API. + +This test file converts the unit test test_metadata_batch_update from test_doc_sdk_routes_unit.py +to end-to-end tests that call the actual HTTP API. +""" import pytest -from common import metadata_batch_update, list_documents, delete_documents, upload_documents +from common import ( + update_documents_metadata, + list_documents, + delete_documents, + upload_documents, +) +from configs import INVALID_API_TOKEN +from libs.auth import RAGFlowHttpApiAuth def _create_and_upload_in_batches(auth, dataset_id, num_docs, tmp_path, batch_size=100): @@ -33,6 +46,31 @@ def _create_and_upload_in_batches(auth, dataset_id, num_docs, tmp_path, batch_si return document_ids +@pytest.fixture(scope="class") +def dataset_with_docs(request, HttpApiAuth, add_dataset, ragflow_tmp_dir): + """Create a dataset with test documents and clean up after test class.""" + dataset_id = add_dataset + + # Upload test documents + fps = [] + for i in range(5): + fp = ragflow_tmp_dir / f"test_doc_{i}.txt" + fp.write_text(f"Test document content {i}\n" * 10) + fps.append(fp) + + upload_res = upload_documents(HttpApiAuth, dataset_id, fps) + assert upload_res["code"] == 0, f"Failed to upload documents: {upload_res}" + + document_ids = [doc["id"] for doc in upload_res["data"]] + + def cleanup(): + delete_documents(HttpApiAuth, dataset_id, {"ids": document_ids}) + + request.addfinalizer(cleanup) + + return dataset_id, document_ids + + @pytest.mark.p3 class TestMetadataBatchUpdate: def test_batch_update_metadata(self, HttpApiAuth, add_dataset, ragflow_tmp_dir): @@ -47,7 +85,7 @@ class TestMetadataBatchUpdate: # Update metadata via batch update API updates = [{"key": "author", "value": "new_author"}, {"key": "status", "value": "processed"}] - res = metadata_batch_update(HttpApiAuth, dataset_id, {"selector": {"document_ids": document_ids}, "updates": updates}) + res = update_documents_metadata(HttpApiAuth, dataset_id, {"selector": {"document_ids": document_ids}, "updates": updates}) # Verify the API call succeeded assert res["code"] == 0, f"Expected code 0, got {res.get('code')}: {res.get('message')}" @@ -64,3 +102,287 @@ class TestMetadataBatchUpdate: # Cleanup delete_documents(HttpApiAuth, dataset_id, {"ids": document_ids}) + + +@pytest.mark.p2 +class TestMetadataBatchUpdateValidation: + """Test validation scenarios for metadata batch update API.""" + + def test_invalid_auth(self): + """Test that invalid authentication returns 401.""" + res = update_documents_metadata( + RAGFlowHttpApiAuth(INVALID_API_TOKEN), + "dataset_id", + {"selector": {"document_ids": []}, "updates": [], "deletes": []}, + ) + assert res["code"] == 401 + + def test_invalid_dataset_id(self, HttpApiAuth): + """Test that invalid dataset ID returns error.""" + res = update_documents_metadata( + HttpApiAuth, + "invalid_dataset_id", + {"selector": {"document_ids": []}, "updates": [], "deletes": []}, + ) + assert res["code"] == 102 + assert "You don't own the dataset" in res["message"] + + def test_selector_not_object(self, HttpApiAuth, dataset_with_docs): + """Test that selector must be an object.""" + dataset_id, _ = dataset_with_docs + + # Pass selector as a list instead of object + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": [1], "updates": [], "deletes": []}, + ) + assert res["code"] == 102 + assert "selector must be an object" in res["message"] + + def test_updates_and_deletes_must_be_lists(self, HttpApiAuth, dataset_with_docs): + """Test that updates and deletes must be lists.""" + dataset_id, _ = dataset_with_docs + + # Pass updates and deletes as objects instead of lists + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {}, "updates": {"key": "value"}, "deletes": []}, + ) + assert res["code"] == 102 + assert "updates and deletes must be lists" in res["message"] + + def test_metadata_condition_must_be_object(self, HttpApiAuth, dataset_with_docs): + """Test that metadata_condition must be an object.""" + dataset_id, _ = dataset_with_docs + + # Pass metadata_condition as a list instead of object + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"metadata_condition": [1]}, "updates": [], "deletes": []}, + ) + assert res["code"] == 102 + assert "metadata_condition must be an object" in res["message"] + + def test_document_ids_must_be_list(self, HttpApiAuth, dataset_with_docs): + """Test that document_ids must be a list.""" + dataset_id, _ = dataset_with_docs + + # Pass document_ids as a string instead of list + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"document_ids": "doc-1"}, "updates": [], "deletes": []}, + ) + assert res["code"] == 102 + assert "document_ids must be a list" in res["message"] + + def test_each_update_requires_key_and_value(self, HttpApiAuth, dataset_with_docs): + """Test that each update requires key and value.""" + dataset_id, _ = dataset_with_docs + + # Pass update without key + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {}, "updates": [{"key": ""}], "deletes": []}, + ) + assert res["code"] == 102 + assert "Each update requires key and value" in res["message"] + + def test_each_delete_requires_key(self, HttpApiAuth, dataset_with_docs): + """Test that each delete requires key.""" + dataset_id, _ = dataset_with_docs + + # Pass delete without key + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {}, "updates": [], "deletes": [{"x": "y"}]}, + ) + assert res["code"] == 102 + assert "Each delete requires key" in res["message"] + + def test_documents_not_belong_to_dataset(self, HttpApiAuth, dataset_with_docs): + """Test that documents must belong to the dataset.""" + dataset_id, _ = dataset_with_docs + + # Pass document IDs that don't belong to the dataset + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + { + "selector": {"document_ids": ["doc-does-not-exist-1", "doc-does-not-exist-2"]}, + "updates": [{"key": "author", "value": "test"}], + "deletes": [], + }, + ) + assert res["code"] == 102 + assert "do not belong to dataset" in res["message"] + + +@pytest.mark.p2 +class TestMetadataBatchUpdateSuccess: + """Test successful scenarios for metadata batch update API.""" + + def test_batch_update_by_document_ids(self, HttpApiAuth, dataset_with_docs): + """Test batch update metadata by document IDs.""" + dataset_id, document_ids = dataset_with_docs + + # Update metadata for specific documents + updates = [{"key": "author", "value": "test_author"}, {"key": "status", "value": "processed"}] + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"document_ids": document_ids}, "updates": updates, "deletes": []}, + ) + + assert res["code"] == 0, f"Expected code 0, got {res.get('code')}: {res.get('message')}" + assert res["data"]["updated"] == 5 + assert res["data"]["matched_docs"] == 5 + + # Verify metadata was updated + list_res = list_documents(HttpApiAuth, dataset_id, {"ids": document_ids}) + assert list_res["code"] == 0 + + for doc in list_res["data"]["docs"]: + assert doc["meta_fields"].get("author") == "test_author" + assert doc["meta_fields"].get("status") == "processed" + + def test_batch_update_with_metadata_condition(self, HttpApiAuth, dataset_with_docs): + """Test batch update metadata using metadata_condition filter.""" + dataset_id, document_ids = dataset_with_docs + + # First, set initial metadata + updates = [{"key": "category", "value": "test_category"}] + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"document_ids": document_ids}, "updates": updates, "deletes": []}, + ) + + assert res["code"] == 0 + assert res["data"]["updated"] == 5 + assert res["data"]["matched_docs"] == 5 + + # Now update only documents with category="test_category" + updates = [{"key": "author", "value": "filtered_author"}] + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + { + "selector": { + "document_ids": document_ids, + "metadata_condition": {"conditions": [{"comparison_operator": "is", "name": "category", "value": "test_category"}]}, + }, + "updates": updates, + "deletes": [], + }, + ) + + assert res["code"] == 0, f"Expected code 0, got {res.get('code')}: {res.get('message')}" + assert res["data"]["updated"] == 5 + assert res["data"]["matched_docs"] == 5 + + def test_batch_delete_metadata(self, HttpApiAuth, dataset_with_docs): + """Test batch delete metadata keys.""" + dataset_id, document_ids = dataset_with_docs + + # First, set some metadata + updates = [{"key": "author", "value": "test_author"}, {"key": "status", "value": "processed"}] + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"document_ids": document_ids}, "updates": updates, "deletes": []}, + ) + assert res["code"] == 0 + + # Now delete the "author" key + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"document_ids": document_ids}, "updates": [], "deletes": [{"key": "author"}]}, + ) + + assert res["code"] == 0, f"Expected code 0, got {res.get('code')}: {res.get('message')}" + assert res["data"]["updated"] == 5 + + # Verify author was deleted but status remains + list_res = list_documents(HttpApiAuth, dataset_id, {"ids": document_ids}) + assert list_res["code"] == 0 + + for doc in list_res["data"]["docs"]: + assert "author" not in doc["meta_fields"] or doc["meta_fields"].get("author") is None + assert doc["meta_fields"].get("status") == "processed" + + def test_batch_update_and_delete_combined(self, HttpApiAuth, dataset_with_docs): + """Test batch update and delete metadata in same request.""" + dataset_id, document_ids = dataset_with_docs + + # First, set initial metadata + updates = [{"key": "author", "value": "old_author"}, {"key": "status", "value": "old_status"}] + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"document_ids": document_ids}, "updates": updates, "deletes": []}, + ) + assert res["code"] == 0 + + # Now update and delete in same request + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + { + "selector": {"document_ids": document_ids}, + "updates": [{"key": "author", "value": "new_author"}], + "deletes": [{"key": "status"}], + }, + ) + + assert res["code"] == 0, f"Expected code 0, got {res.get('code')}: {res.get('message')}" + assert res["data"]["updated"] == 5 + + # Verify the changes + list_res = list_documents(HttpApiAuth, dataset_id, {"ids": document_ids}) + assert list_res["code"] == 0 + + for doc in list_res["data"]["docs"]: + assert doc["meta_fields"].get("author") == "new_author" + assert "status" not in doc["meta_fields"] or doc["meta_fields"].get("status") is None + + def test_update_with_empty_document_ids(self, HttpApiAuth, dataset_with_docs): + """Test that empty document_ids returns success with 0 matched.""" + dataset_id, _ = dataset_with_docs + + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + {"selector": {"document_ids": []}, "updates": [{"key": "author", "value": "test"}], "deletes": []}, + ) + + assert res["code"] == 0 + assert res["data"]["updated"] == 0 + assert res["data"]["matched_docs"] == 0 + + def test_update_with_no_matching_metadata_condition(self, HttpApiAuth, dataset_with_docs): + """Test that non-matching metadata_condition returns 0 matched.""" + dataset_id, document_ids = dataset_with_docs + + res = update_documents_metadata( + HttpApiAuth, + dataset_id, + { + "selector": { + "document_ids": document_ids, + "metadata_condition": {"conditions": [{"comparison_operator":"is", "name": "nonexistent_key", "value": "nonexistent_value"}]}, + }, + "updates": [{"key": "author", "value": "test"}], + "deletes": [], + }, + ) + + assert res["code"] == 0 + assert res["data"]["updated"] == 0 + assert res["data"]["matched_docs"] == 0 diff --git a/test/testcases/test_web_api/test_common.py b/test/testcases/test_web_api/test_common.py index 06754956d3..b2edcd9171 100644 --- a/test/testcases/test_web_api/test_common.py +++ b/test/testcases/test_web_api/test_common.py @@ -409,8 +409,12 @@ def document_metadata_summary(auth, payload=None, *, headers=HEADERS, data=None) return res.json() -def document_metadata_update(auth, payload=None, *, headers=HEADERS, data=None): - res = requests.post(url=f"{HOST_ADDRESS}{DOCUMENT_APP_URL}/metadata/update", headers=headers, auth=auth, json=payload, data=data) +def document_metadata_update(auth, dataset_id, payload=None, *, headers=HEADERS, data=None): + """New unified API for updating document metadata. + + Uses PATCH method at /api/v1/datasets/{dataset_id}/documents/metadatas + """ + res = requests.patch(url=f"{HOST_ADDRESS}{DATASETS_URL}/{dataset_id}/documents/metadatas", headers=headers, auth=auth, json=payload, data=data) return res.json() diff --git a/test/testcases/test_web_api/test_document_app/test_document_metadata.py b/test/testcases/test_web_api/test_document_app/test_document_metadata.py index 6976765478..1fd6486948 100644 --- a/test/testcases/test_web_api/test_document_app/test_document_metadata.py +++ b/test/testcases/test_web_api/test_document_app/test_document_metadata.py @@ -23,6 +23,7 @@ from test_common import ( document_filter, document_infos, document_metadata_summary, + document_metadata_update, document_update_metadata_setting, ) from configs import INVALID_API_TOKEN @@ -245,39 +246,44 @@ class TestDocumentMetadataUnit: monkeypatch.setattr(module.UserTenantService, "query", lambda **_kwargs: [SimpleNamespace(tenant_id=tenant_id)]) monkeypatch.setattr(module.KnowledgebaseService, "query", lambda **_kwargs: True if _kwargs.get("id") == kb_id else False) - def test_metadata_update_missing_kb_id(self, document_app_module, monkeypatch): - module = document_app_module + @pytest.mark.p3 + def test_update_metadata_missing_dataset_id(self, WebApiAuth, add_document_func): + """Test the new unified update_metadata API - missing dataset_id.""" + # Call with empty dataset_id (should fail validation) + res = document_metadata_update(WebApiAuth, "", {"dataset_id": "", "selector": {"document_ids": ["doc1"]}, "updates": []}) + assert res["code"] == 404 + assert res["message"] == "Not Found: /api/v1/datasets//documents/metadatas", res - async def fake_request_json(): - return {"doc_ids": ["doc1"], "updates": [], "deletes": []} + @pytest.mark.p3 + def test_update_metadata_success(self, WebApiAuth, add_document_func): + """Test the new unified update_metadata API - success case.""" + kb_id, doc_id = add_document_func + res = document_metadata_update( + WebApiAuth, kb_id, + { + "selector": {"document_ids": [doc_id]}, + "updates": [{"key": "author", "value": "test_author"}], + "deletes": [] + } + ) + assert res["code"] == 0, res - monkeypatch.setattr(module, "get_request_json", fake_request_json) - res = _run(module.metadata_update.__wrapped__()) - assert res["code"] == 101 - assert "KB ID" in res["message"] - def test_metadata_update_success(self, document_app_module, monkeypatch): - module = document_app_module - monkeypatch.setattr(module.DocMetadataService, "batch_update_metadata", lambda *_args, **_kwargs: 1) + @pytest.mark.p3 + def test_update_metadata_invalid_delete_item(self, WebApiAuth, add_document_func): + """Test the new unified update_metadata API - invalid delete item.""" + kb_id, doc_id = add_document_func + res = document_metadata_update( + WebApiAuth, kb_id, + { + "selector": {"document_ids": [doc_id]}, + "updates": [], + "deletes": [{}] # Invalid - missing key + } + ) + assert res["code"] == 102 + assert "Each delete requires key" in res["message"], res - async def fake_request_json(): - return {"kb_id": "kb1", "doc_ids": ["doc1"], "updates": [{"key": "author", "value": "alice"}], "deletes": []} - - monkeypatch.setattr(module, "get_request_json", fake_request_json) - res = _run(module.metadata_update.__wrapped__()) - assert res["code"] == 0 - assert res["data"]["matched_docs"] == 1 - - def test_metadata_update_invalid_delete_item_unit(self, document_app_module, monkeypatch): - module = document_app_module - - async def fake_request_json(): - return {"kb_id": "kb1", "doc_ids": ["doc1"], "updates": [], "deletes": [{}]} - - monkeypatch.setattr(module, "get_request_json", fake_request_json) - res = _run(module.metadata_update.__wrapped__()) - assert res["code"] == module.RetCode.ARGUMENT_ERROR - assert "Each delete requires key." in res["message"] def test_thumbnails_missing_ids_rewrite_and_exception_unit(self, document_app_module, monkeypatch): module = document_app_module diff --git a/web/src/pages/dataset/components/metedata/hooks/use-manage-modal.ts b/web/src/pages/dataset/components/metedata/hooks/use-manage-modal.ts index 1cbb38fad7..b2778eb69c 100644 --- a/web/src/pages/dataset/components/metedata/hooks/use-manage-modal.ts +++ b/web/src/pages/dataset/components/metedata/hooks/use-manage-modal.ts @@ -5,7 +5,7 @@ import { DocumentApiAction } from '@/hooks/use-document-request'; import kbService, { getMetaDataService, updateDocumentMetaDataConfig, - updateMetaData, + updateDocumentsMetadata, } from '@/services/knowledge-service'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { RowSelectionState } from '@tanstack/react-table'; @@ -375,10 +375,11 @@ export const useManageMetaDataModal = ( const handleSaveManage = useCallback( async (callback: () => void) => { console.log('handleSaveManage', tableData); - const { data: res } = await updateMetaData({ - kb_id: id as string, - data: operations, - doc_ids: documentIds, + const { data: res } = await updateDocumentsMetadata({ + dataset_id: id as string, + selector: { document_ids: documentIds }, + updates: operations.updates, + deletes: operations.deletes, }); if (res.code === 0) { queryClient.invalidateQueries({ diff --git a/web/src/services/knowledge-service.ts b/web/src/services/knowledge-service.ts index 760248efdb..9d64e43e8e 100644 --- a/web/src/services/knowledge-service.ts +++ b/web/src/services/knowledge-service.ts @@ -270,15 +270,23 @@ export const getMetaDataService = ({ request.get(api.getMetaData(kb_id), { params: doc_ids?.length ? { doc_ids: doc_ids.join(',') } : undefined, }); -export const updateMetaData = ({ - kb_id, - doc_ids, - data, +export const updateDocumentsMetadata = ({ + dataset_id, + selector, + updates, + deletes, }: { - kb_id: string; - doc_ids?: string[]; - data: any; -}) => request.post(api.updateMetaData, { data: { kb_id, doc_ids, ...data } }); + dataset_id: string; + selector?: { + document_ids?: string[]; + metadata_condition?: any; + }; + updates?: any[]; + deletes?: any[]; +}) => + request.patch(api.updateDocumentsMetadata(dataset_id), { + data: { selector, updates, deletes }, + }); export const updateDocumentMetaDataConfig = ({ kb_id, diff --git a/web/src/utils/api.ts b/web/src/utils/api.ts index 8da592cfaf..982a24871e 100644 --- a/web/src/utils/api.ts +++ b/web/src/utils/api.ts @@ -86,7 +86,8 @@ export default { pipelineRerun: `${webAPI}/canvas/rerun`, getMetaData: (datasetId: string) => `${restAPIv1}/datasets/${datasetId}/metadata/summary`, - updateMetaData: `${webAPI}/document/metadata/update`, + updateDocumentsMetadata: (datasetId: string) => + `${restAPIv1}/datasets/${datasetId}/documents/metadatas`, kbUpdateMetaData: `${webAPI}/kb/update_metadata_setting`, documentUpdateMetaDataConfig: (datasetId: string, documentId: string) => `${restAPIv1}/datasets/${datasetId}/documents/${documentId}/metadata/config`,