opensource/dify
3
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-09-24 23:22:26 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor: verticalize tag management and batch bindings (#35840)

Browse Source 
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
yyh
2026-05-07 01:36:10 +00:00
committed by GitHub
co-authored by autofix-ci[bot]
parent 7e6745e105
commit 00bf3f83f2
58 changed files with 1851 additions and 2007 deletions
Download Patch File Download Diff File Expand all files Collapse all files
api/controllers/console/tag/tags.py
+8 -53
View File
@@ -32,12 +32,7 @@ class TagBindingPayload(BaseModel):
class TagBindingRemovePayload(BaseModel):
tag_id: str = Field(description="Tag ID to remove")
target_id: str = Field(description="Target ID to unbind tag from")
type: TagType = Field(description="Tag type")
class TagBindingItemDeletePayload(BaseModel):
tag_ids: list[str] = Field(description="Tag IDs to remove", min_length=1)
target_id: str = Field(description="Target ID to unbind tag from")
type: TagType = Field(description="Tag type")
@@ -75,7 +70,6 @@ register_schema_models(
TagBasePayload,
TagBindingPayload,
TagBindingRemovePayload,
TagBindingItemDeletePayload,
TagListQueryParam,
TagResponse,
)
@@ -184,13 +178,13 @@ def _create_tag_bindings() -> tuple[dict[str, str], int]:
return {"result": "success"}, 200
def _remove_tag_binding() -> tuple[dict[str, str], int]:
def _remove_tag_bindings() -> tuple[dict[str, str], int]:
_require_tag_binding_edit_permission()
payload = TagBindingRemovePayload.model_validate(console_ns.payload or {})
TagService.delete_tag_binding(
TagBindingDeletePayload(
tag_id=payload.tag_id,
tag_ids=payload.tag_ids,
target_id=payload.target_id,
type=payload.type,
)
@@ -211,54 +205,15 @@ class TagBindingCollectionApi(Resource):
return _create_tag_bindings()
@console_ns.route("/tag-bindings/<uuid:id>")
class TagBindingItemApi(Resource):
"""Canonical item resource for tag binding deletion."""
@console_ns.doc("delete_tag_binding")
@console_ns.doc(params={"id": "Tag ID"})
@console_ns.expect(console_ns.models[TagBindingItemDeletePayload.__name__])
@setup_required
@login_required
@account_initialization_required
def delete(self, id):
_require_tag_binding_edit_permission()
payload = TagBindingItemDeletePayload.model_validate(console_ns.payload or {})
TagService.delete_tag_binding(
TagBindingDeletePayload(
tag_id=str(id),
target_id=payload.target_id,
type=payload.type,
)
)
return {"result": "success"}, 200
@console_ns.route("/tag-bindings/create")
class DeprecatedTagBindingCreateApi(Resource):
"""Deprecated verb-based alias for tag binding creation."""
@console_ns.doc("create_tag_binding_deprecated")
@console_ns.doc(deprecated=True)
@console_ns.doc(description="Deprecated legacy alias. Use POST /tag-bindings instead.")
@console_ns.expect(console_ns.models[TagBindingPayload.__name__])
@setup_required
@login_required
@account_initialization_required
def post(self):
return _create_tag_bindings()
@console_ns.route("/tag-bindings/remove")
class DeprecatedTagBindingRemoveApi(Resource):
"""Deprecated verb-based alias for tag binding deletion."""
class TagBindingRemoveApi(Resource):
"""Batch resource for tag binding deletion."""
@console_ns.doc("delete_tag_binding_deprecated")
@console_ns.doc(deprecated=True)
@console_ns.doc(description="Deprecated legacy alias. Use DELETE /tag-bindings/{id} instead.")
@console_ns.doc("remove_tag_bindings")
@console_ns.doc(description="Remove one or more tag bindings from a target.")
@console_ns.expect(console_ns.models[TagBindingRemovePayload.__name__])
@setup_required
@login_required
@account_initialization_required
def post(self):
return _remove_tag_binding()
return _remove_tag_bindings()
api/controllers/service_api/dataset/dataset.py
+24 -6
View File
@@ -2,7 +2,7 @@ from typing import Any, Literal, cast
from flask import request
from flask_restx import marshal
from pydantic import BaseModel, Field, TypeAdapter, field_validator
from pydantic import BaseModel, Field, TypeAdapter, field_validator, model_validator
from werkzeug.exceptions import Forbidden, NotFound
import services
@@ -100,9 +100,27 @@ class TagBindingPayload(BaseModel):
class TagUnbindingPayload(BaseModel):
tag_id: str
"""Accept the legacy single-tag Service API payload while exposing a normalized tag_ids list internally."""
tag_ids: list[str] = Field(default_factory=list)
tag_id: str | None = None
target_id: str
@model_validator(mode="before")
@classmethod
def normalize_legacy_tag_id(cls, data: object) -> object:
if not isinstance(data, dict):
return data
if not data.get("tag_ids") and data.get("tag_id"):
return {**data, "tag_ids": [data["tag_id"]]}
return data
@model_validator(mode="after")
def validate_tag_ids(self) -> "TagUnbindingPayload":
if not self.tag_ids:
raise ValueError("Tag IDs is required.")
return self
class DatasetListQuery(BaseModel):
page: int = Field(default=1, description="Page number")
@@ -601,11 +619,11 @@ class DatasetTagBindingApi(DatasetApiResource):
@service_api_ns.route("/datasets/tags/unbinding")
class DatasetTagUnbindingApi(DatasetApiResource):
@service_api_ns.expect(service_api_ns.models[TagUnbindingPayload.__name__])
@service_api_ns.doc("unbind_dataset_tag")
@service_api_ns.doc(description="Unbind a tag from a dataset")
@service_api_ns.doc("unbind_dataset_tags")
@service_api_ns.doc(description="Unbind tags from a dataset")
@service_api_ns.doc(
responses={
204: "Tag unbound successfully",
204: "Tags unbound successfully",
401: "Unauthorized - invalid API token",
403: "Forbidden - insufficient permissions",
}
@@ -618,7 +636,7 @@ class DatasetTagUnbindingApi(DatasetApiResource):
payload = TagUnbindingPayload.model_validate(service_api_ns.payload or {})
TagService.delete_tag_binding(
TagBindingDeletePayload(tag_id=payload.tag_id, target_id=payload.target_id, type=TagType.KNOWLEDGE)
TagBindingDeletePayload(tag_ids=payload.tag_ids, target_id=payload.target_id, type=TagType.KNOWLEDGE)
)
return "", 204
api/services/tag_service.py
+15 -8
View File
@@ -1,9 +1,11 @@
import uuid
from typing import cast
import sqlalchemy as sa
from flask_login import current_user
from pydantic import BaseModel, Field
from sqlalchemy import func, select
from sqlalchemy import delete, func, select
from sqlalchemy.engine import CursorResult
from werkzeug.exceptions import NotFound
from extensions.ext_database import db
@@ -29,7 +31,7 @@ class TagBindingCreatePayload(BaseModel):
class TagBindingDeletePayload(BaseModel):
tag_id: str
tag_ids: list[str] = Field(min_length=1)
target_id: str
type: TagType
@@ -178,13 +180,18 @@ class TagService:
@staticmethod
def delete_tag_binding(payload: TagBindingDeletePayload):
TagService.check_target_exists(payload.type, payload.target_id)
tag_binding = db.session.scalar(
select(TagBinding)
.where(TagBinding.target_id == payload.target_id, TagBinding.tag_id == payload.tag_id)
.limit(1)
result = cast(
CursorResult,
db.session.execute(
delete(TagBinding).where(
TagBinding.target_id == payload.target_id,
TagBinding.tag_id.in_(payload.tag_ids),
TagBinding.tenant_id == current_user.current_tenant_id,
)
),
)
if tag_binding:
db.session.delete(tag_binding)
if result.rowcount:
db.session.commit()
@staticmethod
api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py
+44 -4
View File
@@ -217,10 +217,20 @@ class TestTagUnbindingPayload:
"""Test suite for TagUnbindingPayload Pydantic model."""
def test_payload_with_valid_data(self):
payload = TagUnbindingPayload(tag_id="tag_123", target_id="dataset_456")
assert payload.tag_id == "tag_123"
payload = TagUnbindingPayload(tag_ids=["tag_123"], target_id="dataset_456")
assert payload.tag_ids == ["tag_123"]
assert payload.target_id == "dataset_456"
def test_payload_normalizes_legacy_tag_id(self):
payload = TagUnbindingPayload(tag_id="tag_123", target_id="dataset_456")
assert payload.tag_ids == ["tag_123"]
assert payload.target_id == "dataset_456"
def test_payload_rejects_empty_tag_ids(self):
with pytest.raises(ValueError) as exc_info:
TagUnbindingPayload(tag_ids=[], target_id="dataset_456")
assert "Tag IDs is required" in str(exc_info.value)
# ---------------------------------------------------------------------------
# Helpers
@@ -1012,6 +1022,36 @@ class TestDatasetTagUnbindingApiPost:
mock_current_user.is_dataset_editor = True
mock_tag_svc.delete_tag_binding.return_value = None
with app.test_request_context(
"/datasets/tags/unbinding",
method="POST",
json={"tag_ids": ["tag-1"], "target_id": "ds-1"},
):
api = DatasetTagUnbindingApi()
result = api.post(_=None)
assert result == ("", 204)
from services.tag_service import TagBindingDeletePayload
mock_tag_svc.delete_tag_binding.assert_called_once_with(
TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type="knowledge")
)
@patch("controllers.service_api.dataset.dataset.TagService")
@patch("controllers.service_api.dataset.dataset.current_user")
def test_unbind_legacy_tag_id_success(
self,
mock_current_user,
mock_tag_svc,
app,
):
from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi
mock_current_user.__class__ = Account
mock_current_user.has_edit_permission = True
mock_current_user.is_dataset_editor = True
mock_tag_svc.delete_tag_binding.return_value = None
with app.test_request_context(
"/datasets/tags/unbinding",
method="POST",
@@ -1024,7 +1064,7 @@ class TestDatasetTagUnbindingApiPost:
from services.tag_service import TagBindingDeletePayload
mock_tag_svc.delete_tag_binding.assert_called_once_with(
TagBindingDeletePayload(tag_id="tag-1", target_id="ds-1", type="knowledge")
TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type="knowledge")
)
@patch("controllers.service_api.dataset.dataset.current_user")
@@ -1038,7 +1078,7 @@ class TestDatasetTagUnbindingApiPost:
with app.test_request_context(
"/datasets/tags/unbinding",
method="POST",
json={"tag_id": "tag-1", "target_id": "ds-1"},
json={"tag_ids": ["tag-1"], "target_id": "ds-1"},
):
api = DatasetTagUnbindingApi()
with pytest.raises(Forbidden):
api/tests/test_containers_integration_tests/services/test_tag_service.py
+20 -19
View File
@@ -1099,38 +1099,39 @@ class TestTagService:
db_session_with_containers, mock_external_service_dependencies
)
# Create tag
tag = self._create_test_tags(
db_session_with_containers, mock_external_service_dependencies, tenant.id, "knowledge", 1
)[0]
# Create tags
tags = self._create_test_tags(
db_session_with_containers, mock_external_service_dependencies, tenant.id, "knowledge", 2
)
# Create dataset and bind tag
# Create dataset and bind tags
dataset = self._create_test_dataset(db_session_with_containers, mock_external_service_dependencies, tenant.id)
self._create_test_tag_bindings(
db_session_with_containers, mock_external_service_dependencies, [tag], dataset.id, tenant.id
db_session_with_containers, mock_external_service_dependencies, tags, dataset.id, tenant.id
)
# Verify binding exists before deletion
binding_before = (
# Verify bindings exist before deletion
bindings_before = (
db_session_with_containers.query(TagBinding)
.where(TagBinding.tag_id == tag.id, TagBinding.target_id == dataset.id)
.first()
.where(TagBinding.tag_id.in_([tag.id for tag in tags]), TagBinding.target_id == dataset.id)
.all()
)
assert binding_before is not None
assert len(bindings_before) == 2
# Act: Execute the method under test
delete_payload = TagBindingDeletePayload(type="knowledge", target_id=dataset.id, tag_id=tag.id)
delete_payload = TagBindingDeletePayload(
type="knowledge", target_id=dataset.id, tag_ids=[tag.id for tag in tags]
)
TagService.delete_tag_binding(delete_payload)
# Assert: Verify the expected outcomes
# Verify tag binding was deleted
binding_after = (
# Verify tag bindings were deleted
bindings_after = (
db_session_with_containers.query(TagBinding)
.where(TagBinding.tag_id == tag.id, TagBinding.target_id == dataset.id)
.first()
.where(TagBinding.tag_id.in_([tag.id for tag in tags]), TagBinding.target_id == dataset.id)
.all()
)
assert binding_after is None
assert len(bindings_after) == 0
def test_delete_tag_binding_non_existent_binding(
self, db_session_with_containers: Session, mock_external_service_dependencies
@@ -1156,7 +1157,7 @@ class TestTagService:
app = self._create_test_app(db_session_with_containers, mock_external_service_dependencies, tenant.id)
# Act: Try to delete non-existent binding
delete_payload = TagBindingDeletePayload(type="app", target_id=app.id, tag_id=tag.id)
delete_payload = TagBindingDeletePayload(type="app", target_id=app.id, tag_ids=[tag.id])
TagService.delete_tag_binding(delete_payload)
# Assert: Verify the expected outcomes
api/tests/unit_tests/controllers/console/tag/test_tags.py
+20 -91
View File
@@ -8,10 +8,8 @@ from werkzeug.exceptions import Forbidden
import controllers.console.tag.tags as module
from controllers.console import console_ns
from controllers.console.tag.tags import (
DeprecatedTagBindingCreateApi,
DeprecatedTagBindingRemoveApi,
TagBindingCollectionApi,
TagBindingItemApi,
TagBindingRemoveApi,
TagListApi,
TagUpdateDeleteApi,
)
@@ -249,39 +247,13 @@ class TestTagBindingCollectionApi:
method(api)
class TestDeprecatedTagBindingCreateApi:
def test_create_success(self, app, admin_user, payload_patch):
api = DeprecatedTagBindingCreateApi()
class TestTagBindingRemoveApi:
def test_remove_success(self, app, admin_user, payload_patch):
api = TagBindingRemoveApi()
method = unwrap(api.post)
payload = {
"tag_ids": ["tag-1"],
"target_id": "target-1",
"type": "knowledge",
}
with app.test_request_context("/", json=payload):
with (
patch(
"controllers.console.tag.tags.current_account_with_tenant",
return_value=(admin_user, None),
),
payload_patch(payload),
patch("controllers.console.tag.tags.TagService.save_tag_binding") as save_mock,
):
result, status = method(api)
save_mock.assert_called_once()
assert status == 200
assert result["result"] == "success"
class TestTagBindingItemApi:
def test_delete_success(self, app, admin_user, payload_patch):
api = TagBindingItemApi()
method = unwrap(api.delete)
payload = {
"tag_ids": ["tag-1", "tag-2"],
"target_id": "target-1",
"type": "knowledge",
}
@@ -295,57 +267,16 @@ class TestTagBindingItemApi:
payload_patch(payload),
patch("controllers.console.tag.tags.TagService.delete_tag_binding") as delete_mock,
):
result, status = method(api, "tag-1")
result, status = method(api)
delete_mock.assert_called_once()
delete_payload = delete_mock.call_args.args[0]
assert delete_payload.tag_id == "tag-1"
assert delete_payload.target_id == "target-1"
assert delete_payload.type == TagType.KNOWLEDGE
assert status == 200
assert result["result"] == "success"
def test_delete_forbidden(self, app, readonly_user):
api = TagBindingItemApi()
method = unwrap(api.delete)
with app.test_request_context("/"):
with patch(
"controllers.console.tag.tags.current_account_with_tenant",
return_value=(readonly_user, None),
):
with pytest.raises(Forbidden):
method(api, "tag-1")
class TestDeprecatedTagBindingRemoveApi:
def test_remove_success(self, app, admin_user, payload_patch):
api = DeprecatedTagBindingRemoveApi()
method = unwrap(api.post)
payload = {
"tag_id": "tag-1",
"target_id": "target-1",
"type": "knowledge",
}
with app.test_request_context("/", json=payload):
with (
patch(
"controllers.console.tag.tags.current_account_with_tenant",
return_value=(admin_user, None),
),
payload_patch(payload),
patch("controllers.console.tag.tags.TagService.delete_tag_binding") as delete_mock,
):
result, status = method(api)
delete_mock.assert_called_once()
assert delete_payload.tag_ids == ["tag-1", "tag-2"]
assert status == 200
assert result["result"] == "success"
def test_remove_forbidden(self, app, readonly_user, payload_patch):
api = DeprecatedTagBindingRemoveApi()
api = TagBindingRemoveApi()
method = unwrap(api.post)
with app.test_request_context("/", json={}):
@@ -371,32 +302,30 @@ class TestTagResponseModel:
class TestTagBindingRouteMetadata:
def test_legacy_write_routes_are_marked_deprecated(self):
assert DeprecatedTagBindingCreateApi.post.__apidoc__["deprecated"] is True
assert DeprecatedTagBindingRemoveApi.post.__apidoc__["deprecated"] is True
def test_write_routes_are_not_deprecated(self):
assert TagBindingCollectionApi.post.__apidoc__.get("deprecated") is not True
assert TagBindingItemApi.delete.__apidoc__.get("deprecated") is not True
assert TagBindingRemoveApi.post.__apidoc__.get("deprecated") is not True
def test_write_routes_have_stable_operation_ids(self):
assert TagBindingCollectionApi.post.__apidoc__["id"] == "create_tag_binding"
assert TagBindingItemApi.delete.__apidoc__["id"] == "delete_tag_binding"
assert DeprecatedTagBindingCreateApi.post.__apidoc__["id"] == "create_tag_binding_deprecated"
assert DeprecatedTagBindingRemoveApi.post.__apidoc__["id"] == "delete_tag_binding_deprecated"
assert TagBindingRemoveApi.post.__apidoc__["id"] == "remove_tag_bindings"
def test_canonical_and_legacy_write_routes_are_registered(self):
def test_write_routes_are_registered(self):
route_map = {
resource.__name__: urls
for resource, urls, _route_doc, _kwargs in console_ns.resources
if resource.__name__
in {
"TagBindingCollectionApi",
"TagBindingItemApi",
"DeprecatedTagBindingCreateApi",
"DeprecatedTagBindingRemoveApi",
"TagBindingRemoveApi",
}
}
assert route_map["TagBindingCollectionApi"] == ("/tag-bindings",)
assert route_map["TagBindingItemApi"] == ("/tag-bindings/<uuid:id>",)
assert route_map["DeprecatedTagBindingCreateApi"] == ("/tag-bindings/create",)
assert route_map["DeprecatedTagBindingRemoveApi"] == ("/tag-bindings/remove",)
assert route_map["TagBindingRemoveApi"] == ("/tag-bindings/remove",)
def test_legacy_write_routes_are_not_registered(self):
urls = {url for _resource, resource_urls, _route_doc, _kwargs in console_ns.resources for url in resource_urls}
assert "/tag-bindings/create" not in urls
assert "/tag-bindings/<uuid:id>" not in urls