diff --git a/api/services/app_service.py b/api/services/app_service.py index 0f346433265..941855b8321 100644 --- a/api/services/app_service.py +++ b/api/services/app_service.py @@ -8,6 +8,7 @@ import sqlalchemy as sa from flask_sqlalchemy.pagination import Pagination from pydantic import BaseModel, Field from sqlalchemy import ColumnElement, select +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session, scoped_session from configs import dify_config @@ -27,6 +28,7 @@ from models import Account, AppStar from models.agent import Agent, AgentIconType, AgentScope, AgentSource, AgentStatus from models.model import App, AppMode, AppModelConfig, IconType, Site from models.tools import ApiToolProvider +from services.agent.errors import AgentNameConflictError from services.billing_service import BillingService from services.enterprise import rbac_service as enterprise_rbac_service from services.enterprise.enterprise_service import EnterpriseService @@ -593,6 +595,16 @@ class AppService: if updated_at is not None: agent.updated_at = updated_at + @staticmethod + def _commit_app_identity_update(app: App) -> None: + try: + db.session.commit() + except IntegrityError as exc: + db.session.rollback() + if app.mode == AppMode.AGENT: + raise AgentNameConflictError() from exc + raise + def update_app(self, app: App, args: ArgsDict) -> App: """ Update app @@ -629,7 +641,7 @@ class AppService: account_id=current_user.id, updated_at=app.updated_at, ) - db.session.commit() + self._commit_app_identity_update(app) app_was_updated.send(app) @@ -652,7 +664,7 @@ class AppService: account_id=current_user.id, updated_at=app.updated_at, ) - db.session.commit() + self._commit_app_identity_update(app) app_was_updated.send(app) diff --git a/api/tests/unit_tests/services/test_app_service.py b/api/tests/unit_tests/services/test_app_service.py index e595721e169..c57fb6ed775 100644 --- a/api/tests/unit_tests/services/test_app_service.py +++ b/api/tests/unit_tests/services/test_app_service.py @@ -3,7 +3,11 @@ from __future__ import annotations from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pytest +from sqlalchemy.exc import IntegrityError + from models.model import App +from services.agent.errors import AgentNameConflictError from services.app_service import AppService @@ -317,6 +321,59 @@ class TestAgentAppType: assert backing_agent.role == "" + def test_update_agent_app_duplicate_name_rolls_back_and_raises_conflict(self): + from models.agent import AgentIconType + from models.model import AppMode, IconType + from services.app_service import AppService + + app = SimpleNamespace( + id="app-1", + tenant_id="tenant-1", + mode=AppMode.AGENT, + name="Old", + description="old", + role="draft", + icon_type=IconType.EMOJI, + icon="robot", + icon_background="#fff", + use_icon_as_answer_icon=False, + max_active_requests=None, + created_by="account-1", + ) + backing_agent = SimpleNamespace( + name="Old", + description="old", + role="research assistant", + icon_type=AgentIconType.EMOJI, + icon="robot", + icon_background="#fff", + updated_by=None, + updated_at=None, + ) + + with ( + patch("services.app_service.db") as mock_db, + patch("services.app_service.current_user", SimpleNamespace(id="account-2")), + ): + mock_db.session.scalar.return_value = backing_agent + mock_db.session.commit.side_effect = IntegrityError("duplicate", None, None) + with pytest.raises(AgentNameConflictError): + AppService().update_app( + app, # type: ignore[arg-type] + { + "name": "Existing Agent", + "description": "agent app", + "role": "research assistant", + "icon_type": "emoji", + "icon": "robot", + "icon_background": "#fff", + "use_icon_as_answer_icon": False, + "max_active_requests": 0, + }, + ) + + mock_db.session.rollback.assert_called_once() + def test_delete_agent_app_archives_backing_agent(self): from models.agent import AgentStatus from models.model import AppMode