From 1375dc3864caed44bcd8c8dffed09d182dde7c1f Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 7 Aug 2026 12:46:27 +0900 Subject: [PATCH] test: migrate console app sessions to SQLite (#40081) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .../console/agent/test_agent_controllers.py | 306 ++++++++++++------ .../console/agent/test_app_helpers.py | 13 +- .../console/app/test_agent_app_sandbox.py | 11 +- .../app/test_agent_config_inspector.py | 14 +- .../console/app/test_agent_drive_inspector.py | 88 +++-- .../console/app/test_agent_skills.py | 194 ++++++----- .../console/app/test_annotation_api.py | 68 ++-- .../controllers/console/app/test_app_apis.py | 12 +- .../console/app/test_app_response_models.py | 131 ++++---- .../console/app/test_conversation_api.py | 77 +++-- .../app/test_conversation_variables_api.py | 1 - .../console/app/test_mcp_server_response.py | 4 - .../console/app/test_message_api.py | 87 +++-- .../controllers/console/app/test_workflow.py | 3 +- .../controllers/console/app/test_wraps.py | 60 ++-- 15 files changed, 654 insertions(+), 415 deletions(-) diff --git a/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py b/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py index 6f2454b4dcb..7dfa3eb7106 100644 --- a/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py +++ b/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py @@ -1,3 +1,4 @@ +from datetime import datetime from inspect import getsource, unwrap from types import SimpleNamespace from typing import Any, cast @@ -5,6 +6,7 @@ from unittest.mock import MagicMock, Mock, call import pytest from flask import Flask +from sqlalchemy.orm import Session from werkzeug.exceptions import InternalServerError, NotFound from controllers.console import console_ns @@ -53,10 +55,73 @@ from controllers.console.app.message import ( AgentMessageFeedbackApi, AgentMessageSuggestedQuestionApi, ) -from models.agent import AgentConfigDraftType +from core.app.entities.app_invoke_entities import InvokeFrom +from models.agent import Agent, AgentConfigDraftType, AgentScope, AgentSource, AgentStatus +from models.enums import ConversationFromSource +from models.model import AppMode, Conversation, Message from services.entities.agent_entities import ComposerSaveStrategy, ComposerVariant +def _persist_conversation_message( + session: Session, + *, + app_id: str, + conversation_id: str, + message_id: str, + created_at: datetime, +) -> tuple[Conversation, Message]: + conversation = session.get(Conversation, conversation_id) + if conversation is None: + conversation = Conversation( + app_id=app_id, + app_model_config_id=None, + model_provider=None, + override_model_configs=None, + model_id=None, + mode=AppMode.CHAT, + name="Conversation", + inputs={}, + introduction="", + system_instruction="", + system_instruction_tokens=0, + status="normal", + invoke_from=InvokeFrom.DEBUGGER, + from_source=ConversationFromSource.CONSOLE, + from_end_user_id=None, + from_account_id="00000000-0000-0000-0000-000000000021", + ) + conversation.id = conversation_id + session.add(conversation) + session.flush() + message = Message( + app_id=app_id, + conversation_id=conversation.id, + inputs={}, + query="query", + message={}, + message_tokens=0, + message_unit_price=0, + message_price_unit=0, + answer="answer", + answer_tokens=0, + answer_unit_price=0, + answer_price_unit=0, + provider_response_latency=0, + total_price=0, + currency="USD", + invoke_from=InvokeFrom.DEBUGGER, + from_source=ConversationFromSource.CONSOLE, + from_end_user_id=None, + from_account_id="00000000-0000-0000-0000-000000000021", + app_mode=AppMode.CHAT, + created_at=created_at, + ) + message.id = message_id + session.add(message) + session.flush() + return conversation, message + + def _version_response(version_id: str = "version-1") -> dict: return { "id": version_id, @@ -430,18 +495,25 @@ def test_agent_app_create_omits_optional_role_as_empty_string( def test_agent_app_detail_update_delete_resolve_app_from_agent_id( - app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str + app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str, sqlite_session: Session ) -> None: agent_id = "00000000-0000-0000-0000-000000000001" - app_model = _app_detail_obj(id="app-1", bound_agent_id=agent_id) - agent = SimpleNamespace( - id=agent_id, - app_id="app-1", - backing_app_id=None, + tenant_id = "00000000-0000-0000-0000-000000000002" + app_id = "00000000-0000-0000-0000-000000000003" + app_model = _app_detail_obj(id=app_id, tenant_id=tenant_id, bound_agent_id=agent_id) + agent = Agent( + tenant_id=tenant_id, + name="Resolved agent", + description="", role="Resolved role", - debug_conversation_id="debug-conversation-detail", - active_config_snapshot_id=None, + scope=AgentScope.ROSTER, + source=AgentSource.AGENT_APP, + app_id=app_id, + status=AgentStatus.ACTIVE, ) + agent.id = agent_id + sqlite_session.add(agent) + sqlite_session.flush() captured: dict[str, object] = {} monkeypatch.setattr(roster_controller.AgentRosterService, "get_agent_app_model", lambda _self, **kwargs: app_model) monkeypatch.setattr(roster_controller, "_resolve_agent_runtime_app_model", lambda _session, **kwargs: app_model) @@ -472,17 +544,16 @@ def test_agent_app_detail_update_delete_resolve_app_from_agent_id( def update_app(self, app_obj: object, args: dict[str, object], *, session: object) -> object: captured["update"] = {"app": app_obj, "args": args} - return _app_detail_obj(id="app-1", name=args["name"], bound_agent_id=agent_id) + return _app_detail_obj(id=app_id, tenant_id=tenant_id, name=args["name"], bound_agent_id=agent_id) def delete_app(self, app_obj: object, *, session: object) -> None: captured["delete"] = app_obj monkeypatch.setattr(roster_controller, "AppService", FakeAppService) - session = Mock() - session.scalar.return_value = agent - detail = unwrap(AgentAppApi.get)(AgentAppApi(), session, "tenant-1", SimpleNamespace(id=account_id), agent_id) + session = sqlite_session + detail = unwrap(AgentAppApi.get)(AgentAppApi(), session, tenant_id, SimpleNamespace(id=account_id), agent_id) assert detail["id"] == agent_id - assert detail["app_id"] == "app-1" + assert detail["app_id"] == app_id assert detail["debug_conversation_id"] == "debug-conversation-detail" assert detail["debug_conversation_has_messages"] is True assert detail["debug_conversation_message_count"] == 2 @@ -495,10 +566,10 @@ def test_agent_app_detail_update_delete_resolve_app_from_agent_id( "/console/api/agent/00000000-0000-0000-0000-000000000001", json={"name": "Renamed", "description": "", "role": "Reviewer", "icon_type": "emoji", "icon": "R"}, ): - updated = unwrap(AgentAppApi.put)(AgentAppApi(), session, "tenant-1", SimpleNamespace(id=account_id), agent_id) + updated = unwrap(AgentAppApi.put)(AgentAppApi(), session, tenant_id, SimpleNamespace(id=account_id), agent_id) assert updated["name"] == "Renamed" assert updated["id"] == agent_id - assert updated["app_id"] == "app-1" + assert updated["app_id"] == app_id assert updated["debug_conversation_id"] == "debug-conversation-detail" assert updated["debug_conversation_has_messages"] is True assert updated["debug_conversation_message_count"] == 2 @@ -508,7 +579,7 @@ def test_agent_app_detail_update_delete_resolve_app_from_agent_id( update_call = cast(dict[str, object], captured["update"]) assert update_call["app"] is app_model assert cast(dict[str, object], update_call["args"])["role"] == "Reviewer" - deleted, status = unwrap(AgentAppApi.delete)(AgentAppApi(), session, "tenant-1", agent_id) + deleted, status = unwrap(AgentAppApi.delete)(AgentAppApi(), session, tenant_id, agent_id) assert (deleted, status) == ("", 204) assert captured["delete"] is app_model @@ -735,7 +806,9 @@ def test_agent_api_access_uses_agent_id_and_returns_service_api_metadata(monkeyp } -def test_agent_api_status_and_key_routes_resolve_backing_app(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def test_agent_api_status_and_key_routes_resolve_backing_app( + app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: agent_id = "00000000-0000-0000-0000-000000000001" api_key_id = "00000000-0000-0000-0000-000000000002" app_model = SimpleNamespace( @@ -747,7 +820,6 @@ def test_agent_api_status_and_key_routes_resolve_backing_app(app: Flask, monkeyp api_rph=0, ) captured: dict[str, object] = {} - session = MagicMock() resolve_app = Mock(return_value=app_model) monkeypatch.setattr(roster_controller, "_resolve_agent_app_model", resolve_app) monkeypatch.setattr(roster_controller, "_agent_api_key_count", lambda _session, app_id: 1) @@ -792,34 +864,42 @@ def test_agent_api_status_and_key_routes_resolve_backing_app(app: Flask, monkeyp with app.test_request_context( "/console/api/agent/00000000-0000-0000-0000-000000000001/api-enable", json={"enable_api": True} ): - enabled = unwrap(AgentApiStatusApi.post)(AgentApiStatusApi(), session, "tenant-1", agent_id) + enabled = unwrap(AgentApiStatusApi.post)(AgentApiStatusApi(), unbound_session, "tenant-1", agent_id) assert enabled["enabled"] is True assert captured["enable"] == {"app": app_model, "enable_api": True} - keys = unwrap(AgentApiKeyListApi.get)(AgentApiKeyListApi(), session, "tenant-1", agent_id) + keys = unwrap(AgentApiKeyListApi.get)(AgentApiKeyListApi(), unbound_session, "tenant-1", agent_id) assert keys == {"data": []} - assert captured["list_keys"] == {"session": session, "resource_id": "app-1", "tenant_id": "tenant-1"} - created, status = unwrap(AgentApiKeyListApi.post)(AgentApiKeyListApi(), session, "tenant-1", agent_id) + assert captured["list_keys"] == { + "session": unbound_session, + "resource_id": "app-1", + "tenant_id": "tenant-1", + } + created, status = unwrap(AgentApiKeyListApi.post)(AgentApiKeyListApi(), unbound_session, "tenant-1", agent_id) assert status == 201 assert created["id"] == api_key_id assert created["token"] == "app-test-token" - assert captured["create_key"] == {"session": session, "resource_id": "app-1", "tenant_id": "tenant-1"} + assert captured["create_key"] == { + "session": unbound_session, + "resource_id": "app-1", + "tenant_id": "tenant-1", + } current_user = SimpleNamespace(id="account-1", is_admin_or_owner=True) deleted, delete_status = unwrap(AgentApiKeyApi.delete)( - AgentApiKeyApi(), session, "tenant-1", current_user, agent_id, api_key_id + AgentApiKeyApi(), unbound_session, "tenant-1", current_user, agent_id, api_key_id ) assert (deleted, delete_status) == ("", 204) assert captured["delete_key"] == { - "session": session, + "session": unbound_session, "resource_id": "app-1", "api_key_id": api_key_id, "tenant_id": "tenant-1", "current_user": current_user, } assert resolve_app.call_args_list == [ - call(session, tenant_id="tenant-1", agent_id=agent_id), - call(session, tenant_id="tenant-1", agent_id=agent_id), - call(session, tenant_id="tenant-1", agent_id=agent_id), - call(session, tenant_id="tenant-1", agent_id=agent_id), + call(unbound_session, tenant_id="tenant-1", agent_id=agent_id), + call(unbound_session, tenant_id="tenant-1", agent_id=agent_id), + call(unbound_session, tenant_id="tenant-1", agent_id=agent_id), + call(unbound_session, tenant_id="tenant-1", agent_id=agent_id), ] @@ -1327,7 +1407,7 @@ def test_agent_composer_routes_resolve_app_from_agent_id( def test_agent_chat_generate_and_stop_routes_resolve_app_from_agent_id( - app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str + app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str, unbound_session: Session ) -> None: agent_id = "00000000-0000-0000-0000-000000000001" app_model = SimpleNamespace(id="app-1", tenant_id="tenant-1", mode="agent") @@ -1358,22 +1438,21 @@ def test_agent_chat_generate_and_stop_routes_resolve_app_from_agent_id( ) monkeypatch.setattr(completion_controller, "_create_chat_message", create_chat_message) monkeypatch.setattr(completion_controller, "_stop_chat_message", stop_chat_message) - session = Mock() with app.test_request_context(json={"inputs": {}, "query": "hello"}): assert unwrap(AgentChatMessageApi.post)( - AgentChatMessageApi(), session, "tenant-1", SimpleNamespace(id=account_id), agent_id + AgentChatMessageApi(), unbound_session, "tenant-1", SimpleNamespace(id=account_id), agent_id ) == {"result": "generated"} assert cast(dict[str, object], captured["resolve"]) == {"tenant_id": "tenant-1", "agent_id": agent_id} - assert captured["resolve_session"] is session + assert captured["resolve_session"] is unbound_session create_call = cast(dict[str, object], captured["create"]) - assert create_call["session"] is session + assert create_call["session"] is unbound_session assert create_call["app_model"] is app_model assert cast(SimpleNamespace, create_call["current_user"]).id == account_id assert unwrap(AgentChatMessageStopApi.post)( - AgentChatMessageStopApi(), session, "tenant-1", account_id, agent_id, "task-1" + AgentChatMessageStopApi(), unbound_session, "tenant-1", account_id, agent_id, "task-1" ) == ({"result": "success"}, 200) assert captured["stop_resolve"] == { - "session": session, + "session": unbound_session, "tenant_id": "tenant-1", "agent_id": agent_id, } @@ -1424,7 +1503,7 @@ def test_agent_chat_stream_preflight_preserves_first_normal_event() -> None: def test_agent_build_chat_finalize_route_resolves_app_from_agent_id( - app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str + app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str, unbound_session: Session ) -> None: agent_id = "00000000-0000-0000-0000-000000000001" app_model = SimpleNamespace(id="app-1", tenant_id="tenant-1", mode="agent") @@ -1444,14 +1523,13 @@ def test_agent_build_chat_finalize_route_resolves_app_from_agent_id( lambda _self, **kwargs: resolve_agent_app_model(**kwargs), ) monkeypatch.setattr(completion_controller, "_create_build_chat_finalization_message", create_finalization_message) - session = Mock() with app.test_request_context(): assert unwrap(AgentBuildChatFinalizeApi.post)( - AgentBuildChatFinalizeApi(), session, "tenant-1", SimpleNamespace(id=account_id), agent_id + AgentBuildChatFinalizeApi(), unbound_session, "tenant-1", SimpleNamespace(id=account_id), agent_id ) == {"result": "generated"} assert cast(dict[str, object], captured["resolve"]) == {"tenant_id": "tenant-1", "agent_id": agent_id} finalize_call = cast(dict[str, object], captured["finalize"]) - assert finalize_call["session"] is session + assert finalize_call["session"] is unbound_session assert finalize_call["app_model"] is app_model assert finalize_call["current_tenant_id"] == "tenant-1" assert finalize_call["agent_id"] == agent_id @@ -1459,7 +1537,7 @@ def test_agent_build_chat_finalize_route_resolves_app_from_agent_id( def test_build_chat_finalization_helper_forces_debug_build_and_push_prompt( - app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str + app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str, unbound_session: Session ) -> None: app_model = SimpleNamespace(id="app-1", tenant_id="tenant-1", mode="agent") captured: dict[str, object] = {} @@ -1478,18 +1556,17 @@ def test_build_chat_finalization_helper_forces_debug_build_and_push_prompt( completion_controller, "_resolve_current_user_agent_debug_conversation_id", resolve_debug_conversation ) monkeypatch.setattr(completion_controller.AppGenerateService, "generate", generate) - session = Mock() with app.test_request_context(headers={"X-Trace-Id": "trace-1"}): result = completion_controller._create_build_chat_finalization_message( current_tenant_id="tenant-1", current_user=SimpleNamespace(id=account_id), app_model=app_model, agent_id="agent-1", - session=session, + session=unbound_session, ) assert result == ({"result": "success"}, 200) assert captured["resolve_debug_conversation"] == { - "session": session, + "session": unbound_session, "current_tenant_id": "tenant-1", "current_user": SimpleNamespace(id=account_id), "app_model": app_model, @@ -1576,6 +1653,7 @@ def test_agent_chat_helper_resolves_scoped_conversation_and_forces_streaming( payload_extra: dict[str, str | None], expected_draft_type: AgentConfigDraftType, expected_start_new: bool, + unbound_session: Session, ) -> None: app_model = SimpleNamespace(id="app-1", tenant_id="tenant-1", mode="agent") current_user = SimpleNamespace(id=account_id) @@ -1603,7 +1681,7 @@ def test_agent_chat_helper_resolves_scoped_conversation_and_forces_streaming( headers={"X-Trace-Id": "trace-1"}, ): result = completion_controller._create_chat_message( - current_user=current_user, app_model=app_model, session=Mock() + current_user=current_user, app_model=app_model, session=unbound_session ) assert result == {"response": {"answer": "ok"}} assert captured["app_model"] is app_model @@ -1620,7 +1698,7 @@ def test_agent_chat_helper_resolves_scoped_conversation_and_forces_streaming( def test_agent_chat_helper_ignores_private_exit_intent_payload_key( - app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str + app: Flask, monkeypatch: pytest.MonkeyPatch, account_id: str, unbound_session: Session ) -> None: app_model = SimpleNamespace(id="app-1", tenant_id="tenant-1", mode="agent") current_user = SimpleNamespace(id=account_id) @@ -1653,7 +1731,7 @@ def test_agent_chat_helper_ignores_private_exit_intent_payload_key( result = completion_controller._create_chat_message( current_user=current_user, app_model=app_model, - session=Mock(), + session=unbound_session, ) assert result == {"response": {"answer": "ok"}} @@ -1677,6 +1755,7 @@ def test_agent_chat_helper_rejects_foreign_debug_conversation_before_generation( account_id: str, payload_extra: dict[str, str], expected_draft_type: AgentConfigDraftType, + unbound_session: Session, ) -> None: app_model = SimpleNamespace(id="app-1", tenant_id="tenant-1", mode="agent") generate = MagicMock() @@ -1702,7 +1781,7 @@ def test_agent_chat_helper_rejects_foreign_debug_conversation_before_generation( current_user=SimpleNamespace(id=account_id), app_model=app_model, agent_id="agent-1", - session=Mock(), + session=unbound_session, ) resolve_debug_conversation.assert_called_once() @@ -1835,25 +1914,30 @@ def test_resolve_current_user_agent_debug_conversation_uses_agent_or_backing_app ], ) def test_agent_chat_helper_maps_generation_errors( - app: Flask, monkeypatch: pytest.MonkeyPatch, error: Exception, expected: type[Exception] + app: Flask, + monkeypatch: pytest.MonkeyPatch, + error: Exception, + expected: type[Exception], + unbound_session: Session, ) -> None: app_model = SimpleNamespace(id="app-1", mode="chat") monkeypatch.setattr(completion_controller.AppGenerateService, "generate", lambda **_: (_ for _ in ()).throw(error)) with app.test_request_context(json={"inputs": {}, "query": "hello"}): with pytest.raises(expected): completion_controller._create_chat_message( - current_user=SimpleNamespace(id="account-1"), app_model=app_model, session=Mock() + current_user=SimpleNamespace(id="account-1"), app_model=app_model, session=unbound_session ) -def test_agent_chat_message_routes_resolve_app_from_agent_id(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def test_agent_chat_message_routes_resolve_app_from_agent_id( + app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: agent_id = "00000000-0000-0000-0000-000000000001" message_id = "00000000-0000-0000-0000-000000000002" app_model = SimpleNamespace(id="app-1", mode="agent") current_user = SimpleNamespace(id="account-1") captured: dict[str, object] = {} resolver_calls: list[dict[str, object]] = [] - session = Mock() def resolve_agent_app_model(**kwargs: object) -> object: resolver_calls.append(kwargs) @@ -1881,50 +1965,68 @@ def test_agent_chat_message_routes_resolve_app_from_agent_id(app: Flask, monkeyp monkeypatch.setattr(message_controller, "_get_message_suggested_questions", get_message_suggested_questions) monkeypatch.setattr(message_controller, "_get_message_detail", get_message_detail) assert unwrap(AgentChatMessageListApi.get)( - AgentChatMessageListApi(), session, "tenant-1", current_user, agent_id + AgentChatMessageListApi(), unbound_session, "tenant-1", current_user, agent_id ) == {"data": []} list_call = cast(dict[str, object], captured["list"]) - assert list_call["session"] is session + assert list_call["session"] is unbound_session assert list_call["app_model"] is app_model with app.test_request_context(json={"message_id": message_id, "rating": "like"}): assert unwrap(AgentMessageFeedbackApi.post)( - AgentMessageFeedbackApi(), session, "tenant-1", current_user, agent_id + AgentMessageFeedbackApi(), unbound_session, "tenant-1", current_user, agent_id ) == {"result": "success"} feedback_call = cast(dict[str, object], captured["feedback"]) - assert feedback_call["session"] is session + assert feedback_call["session"] is unbound_session assert feedback_call["app_model"] is app_model assert feedback_call["current_user"] is current_user assert unwrap(AgentMessageSuggestedQuestionApi.get)( - AgentMessageSuggestedQuestionApi(), session, "tenant-1", current_user, agent_id, message_id + AgentMessageSuggestedQuestionApi(), unbound_session, "tenant-1", current_user, agent_id, message_id ) == {"data": ["next"]} suggested_call = cast(dict[str, object], captured["suggested"]) - assert suggested_call["session"] is session + assert suggested_call["session"] is unbound_session assert suggested_call["app_model"] is app_model assert suggested_call["current_user"] is current_user assert suggested_call["message_id"] == message_id - assert unwrap(AgentMessageApi.get)(AgentMessageApi(), session, "tenant-1", agent_id, message_id) == { + assert unwrap(AgentMessageApi.get)(AgentMessageApi(), unbound_session, "tenant-1", agent_id, message_id) == { "id": message_id } detail_call = cast(dict[str, object], captured["detail"]) - assert detail_call == {"session": session, "app_model": app_model, "message_id": message_id} + assert detail_call == {"session": unbound_session, "app_model": app_model, "message_id": message_id} assert resolver_calls == [ - {"session": session, "tenant_id": "tenant-1", "agent_id": agent_id}, - {"session": session, "tenant_id": "tenant-1", "agent_id": agent_id}, - {"session": session, "tenant_id": "tenant-1", "agent_id": agent_id}, - {"session": session, "tenant_id": "tenant-1", "agent_id": agent_id}, + {"session": unbound_session, "tenant_id": "tenant-1", "agent_id": agent_id}, + {"session": unbound_session, "tenant_id": "tenant-1", "agent_id": agent_id}, + {"session": unbound_session, "tenant_id": "tenant-1", "agent_id": agent_id}, + {"session": unbound_session, "tenant_id": "tenant-1", "agent_id": agent_id}, ] -def test_list_chat_messages_supports_first_id_pagination(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def test_list_chat_messages_supports_first_id_pagination( + app: Flask, monkeypatch: pytest.MonkeyPatch, sqlite_session: Session +) -> None: + app_id = "00000000-0000-0000-0000-000000000001" conversation_id = "00000000-0000-0000-0000-000000000010" first_message_id = "00000000-0000-0000-0000-000000000011" older_message_id = "00000000-0000-0000-0000-000000000012" - conversation = SimpleNamespace(id=conversation_id) - first_message = SimpleNamespace(id=first_message_id, created_at=2) - older_message = SimpleNamespace(id=older_message_id, created_at=1) - scalar_values = iter([conversation, first_message, True]) - scalars_result = SimpleNamespace(all=lambda: [older_message]) - session = SimpleNamespace(scalar=lambda _stmt: next(scalar_values), scalars=lambda _stmt: scalars_result) + _persist_conversation_message( + sqlite_session, + app_id=app_id, + conversation_id=conversation_id, + message_id="00000000-0000-0000-0000-000000000013", + created_at=datetime(2025, 1, 1), + ) + _persist_conversation_message( + sqlite_session, + app_id=app_id, + conversation_id=conversation_id, + message_id=older_message_id, + created_at=datetime(2025, 1, 2), + ) + _persist_conversation_message( + sqlite_session, + app_id=app_id, + conversation_id=conversation_id, + message_id=first_message_id, + created_at=datetime(2025, 1, 3), + ) class FakeMessagePaginationResponse: @classmethod @@ -1943,20 +2045,27 @@ def test_list_chat_messages_supports_first_id_pagination(app: Flask, monkeypatch f"/console/api/agent/agent-1/chat-messages?conversation_id={conversation_id}&first_id={first_message_id}&limit=1" ): result = message_controller._list_chat_messages( - session=session, app_model=SimpleNamespace(id="app-1", mode="chat") + session=sqlite_session, app_model=SimpleNamespace(id=app_id, mode="chat") ) assert result == {"data": [older_message_id], "limit": 1, "has_more": True} -def test_list_agent_chat_messages_uses_current_user_conversation(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def test_list_agent_chat_messages_uses_current_user_conversation( + app: Flask, monkeypatch: pytest.MonkeyPatch, sqlite_session: Session +) -> None: + app_id = "00000000-0000-0000-0000-000000000001" conversation_id = "00000000-0000-0000-0000-000000000010" message_id = "00000000-0000-0000-0000-000000000011" - conversation = SimpleNamespace(id=conversation_id) - message = SimpleNamespace(id=message_id, created_at=1) + conversation, _ = _persist_conversation_message( + sqlite_session, + app_id=app_id, + conversation_id=conversation_id, + message_id=message_id, + created_at=datetime(2025, 1, 1), + ) current_user = SimpleNamespace(id="account-1") - app_model = SimpleNamespace(id="app-1", mode="agent") + app_model = SimpleNamespace(id=app_id, mode="agent") captured: dict[str, object] = {} - session = SimpleNamespace(scalar=lambda _stmt: False, scalars=lambda _stmt: SimpleNamespace(all=lambda: [message])) class FakeMessagePaginationResponse: @classmethod @@ -1977,13 +2086,17 @@ def test_list_agent_chat_messages_uses_current_user_conversation(app: Flask, mon monkeypatch.setattr(message_controller, "attach_message_extra_contents", lambda messages: None) monkeypatch.setattr(message_controller, "MessageInfiniteScrollPaginationResponse", FakeMessagePaginationResponse) with app.test_request_context(f"/console/api/agent/agent-1/chat-messages?conversation_id={conversation_id}"): - result = message_controller._list_chat_messages(session=session, app_model=app_model, current_user=current_user) + result = message_controller._list_chat_messages( + session=sqlite_session, app_model=app_model, current_user=current_user + ) assert result == {"data": [message_id], "limit": 20, "has_more": False} - assert captured.pop("session") is session + assert captured.pop("session") is sqlite_session assert captured == {"app_model": app_model, "conversation_id": conversation_id, "user": current_user} -def test_list_agent_chat_messages_rejects_foreign_conversation(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def test_list_agent_chat_messages_rejects_foreign_conversation( + app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: conversation_id = "00000000-0000-0000-0000-000000000010" monkeypatch.setattr( message_controller.ConversationService, @@ -1993,32 +2106,33 @@ def test_list_agent_chat_messages_rejects_foreign_conversation(app: Flask, monke with app.test_request_context(f"/console/api/agent/agent-1/chat-messages?conversation_id={conversation_id}"): with pytest.raises(NotFound): message_controller._list_chat_messages( - session=Mock(), + session=unbound_session, app_model=SimpleNamespace(id="app-1", mode="agent"), current_user=SimpleNamespace(id="account-1"), ) def test_update_message_feedback_rejects_empty_rating_without_existing_feedback( - app: Flask, + app: Flask, sqlite_session: Session ) -> None: + app_id = "00000000-0000-0000-0000-000000000001" message_id = "00000000-0000-0000-0000-000000000002" - message = SimpleNamespace( - id=message_id, - app_id="app-1", - admin_feedback_with_session=MagicMock(return_value=None), + _, message = _persist_conversation_message( + sqlite_session, + app_id=app_id, + conversation_id="00000000-0000-0000-0000-000000000010", + message_id=message_id, + created_at=datetime(2025, 1, 1), ) - session = MagicMock() - session.scalar.return_value = message with app.test_request_context(json={"message_id": message_id, "rating": None}): with pytest.raises(ValueError, match="rating cannot be None"): message_controller._update_message_feedback( - session=session, + session=sqlite_session, current_user=SimpleNamespace(id="account-1"), - app_model=SimpleNamespace(id="app-1"), + app_model=SimpleNamespace(id=app_id), ) - message.admin_feedback_with_session.assert_called_once_with(session=session) + assert message.admin_feedback_with_session(session=sqlite_session) is None @pytest.mark.parametrize( @@ -2041,12 +2155,10 @@ def test_update_message_feedback_rejects_empty_rating_without_existing_feedback( ], ) def test_get_message_suggested_questions_maps_service_errors( - monkeypatch: pytest.MonkeyPatch, error: Exception, expected: type[Exception] + monkeypatch: pytest.MonkeyPatch, error: Exception, expected: type[Exception], unbound_session: Session ) -> None: - session = Mock() - def raise_error(**kwargs: object) -> None: - assert kwargs["session"] is session + assert kwargs["session"] is unbound_session raise error monkeypatch.setattr( @@ -2056,7 +2168,7 @@ def test_get_message_suggested_questions_maps_service_errors( ) with pytest.raises(expected): message_controller._get_message_suggested_questions( - session=session, + session=unbound_session, current_user=SimpleNamespace(id="account-1"), app_model=SimpleNamespace(id="app-1"), message_id="00000000-0000-0000-0000-000000000002", diff --git a/api/tests/unit_tests/controllers/console/agent/test_app_helpers.py b/api/tests/unit_tests/controllers/console/agent/test_app_helpers.py index e05d57bd73a..b396c9bfd1b 100644 --- a/api/tests/unit_tests/controllers/console/agent/test_app_helpers.py +++ b/api/tests/unit_tests/controllers/console/agent/test_app_helpers.py @@ -2,12 +2,15 @@ from unittest.mock import MagicMock from uuid import UUID import pytest +from sqlalchemy.orm import Session from controllers.console.agent import app_helpers -def test_resolve_agent_app_model_reuses_caller_session(monkeypatch: pytest.MonkeyPatch) -> None: - session = MagicMock() +def test_resolve_agent_app_model_reuses_caller_session( + monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: + session = unbound_session app = MagicMock() service = MagicMock() service.get_agent_app_model.return_value = app @@ -28,8 +31,10 @@ def test_resolve_agent_app_model_reuses_caller_session(monkeypatch: pytest.Monke ) -def test_resolve_agent_runtime_app_model_reuses_caller_session(monkeypatch: pytest.MonkeyPatch) -> None: - session = MagicMock() +def test_resolve_agent_runtime_app_model_reuses_caller_session( + monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: + session = unbound_session app = MagicMock() service = MagicMock() service.get_agent_runtime_app_model.return_value = app diff --git a/api/tests/unit_tests/controllers/console/app/test_agent_app_sandbox.py b/api/tests/unit_tests/controllers/console/app/test_agent_app_sandbox.py index 1a1b6f87916..bd7f70a8884 100644 --- a/api/tests/unit_tests/controllers/console/app/test_agent_app_sandbox.py +++ b/api/tests/unit_tests/controllers/console/app/test_agent_app_sandbox.py @@ -7,6 +7,7 @@ from unittest.mock import MagicMock import pytest from dify_agent.client import DifyAgentClientError, DifyAgentHTTPError, DifyAgentTimeoutError from dify_agent.protocol import WorkspaceListResponse, WorkspaceReadResponse +from sqlalchemy.orm import Session from controllers.console import agent_app_sandbox as module from models.model import App, AppMode, IconType @@ -159,9 +160,9 @@ def test_handle_maps_sandbox_and_agent_backend_errors() -> None: module._handle(RuntimeError("boom")) -def test_agent_app_sandbox_resources_proxy_service(monkeypatch: pytest.MonkeyPatch) -> None: +def test_agent_app_sandbox_resources_proxy_service(monkeypatch: pytest.MonkeyPatch, unbound_session: Session) -> None: service = _AgentAppService() - session = MagicMock() + session = unbound_session account = SimpleNamespace(id="account-1") resolver = MagicMock(return_value=_app_model()) monkeypatch.setattr(module, "AgentAppSandboxService", lambda: service) @@ -201,7 +202,9 @@ def test_agent_app_sandbox_resources_proxy_service(monkeypatch: pytest.MonkeyPat assert all(call.kwargs["session"] is session for call in resolver.call_args_list) -def test_agent_app_sandbox_resource_returns_normalized_errors(monkeypatch: pytest.MonkeyPatch) -> None: +def test_agent_app_sandbox_resource_returns_normalized_errors( + monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: class FailingService: def get_info(self, **kwargs): raise AgentSandboxInspectorError("no_active_binding", "no active binding", status_code=404) @@ -210,7 +213,7 @@ def test_agent_app_sandbox_resource_returns_normalized_errors(monkeypatch: pytes raise AgentSandboxInspectorError("no_active_binding", "no active binding", status_code=404) monkeypatch.setattr(module, "AgentAppSandboxService", FailingService) - session = MagicMock() + session = unbound_session account = SimpleNamespace(id="account-1") monkeypatch.setattr(module, "resolve_agent_runtime_app_model", MagicMock(return_value=_app_model())) monkeypatch.setattr( diff --git a/api/tests/unit_tests/controllers/console/app/test_agent_config_inspector.py b/api/tests/unit_tests/controllers/console/app/test_agent_config_inspector.py index 1d857e7284c..f7ed936ee74 100644 --- a/api/tests/unit_tests/controllers/console/app/test_agent_config_inspector.py +++ b/api/tests/unit_tests/controllers/console/app/test_agent_config_inspector.py @@ -11,6 +11,8 @@ from types import SimpleNamespace from unittest.mock import MagicMock, PropertyMock, patch from flask import Flask +from sqlalchemy import event +from sqlalchemy.orm import Session from controllers.console.app import agent_config_inspector as inspector from controllers.console.app.agent_config_inspector import ( @@ -46,8 +48,8 @@ _APP = SimpleNamespace( _USER = SimpleNamespace(id="acct-1") -def test_resolve_bound_agent_uses_injected_session(): - session = MagicMock() +def test_resolve_bound_agent_uses_injected_session(unbound_session: Session): + session = unbound_session resolver = MagicMock(return_value="agent-1") app_model = SimpleNamespace(bound_agent_id_with_session=resolver) result = inspector._resolve_agent_id(session, app_model, None) @@ -100,8 +102,10 @@ def test_manifest_resolves_workflow_node_agent_and_normal_draft(): assert config_service.return_value.manifest.call_args.kwargs["config_version_kind"].value == "draft" -def test_normal_draft_resolution_commits_created_draft_before_service_session() -> None: - session = MagicMock() +def test_normal_draft_resolution_commits_created_draft_before_service_session(sqlite_session: Session) -> None: + session = sqlite_session + commits: list[str] = [] + event.listen(session, "after_commit", lambda _session: commits.append("commit")) with patch(f"{_MOD}.AgentComposerService") as composer: composer.load_agent_composer.return_value = {"draft": {"id": "draft-1"}} version_id, version_kind = inspector._resolve_console_version( @@ -114,7 +118,7 @@ def test_normal_draft_resolution_commits_created_draft_before_service_session() ) assert version_id == "draft-1" assert version_kind.value == "draft" - session.commit.assert_called_once() + assert commits == ["commit"] def test_skill_inspect_by_agent_returns_strict_json_response(): diff --git a/api/tests/unit_tests/controllers/console/app/test_agent_drive_inspector.py b/api/tests/unit_tests/controllers/console/app/test_agent_drive_inspector.py index 453e2f29ff0..ad38a927fcd 100644 --- a/api/tests/unit_tests/controllers/console/app/test_agent_drive_inspector.py +++ b/api/tests/unit_tests/controllers/console/app/test_agent_drive_inspector.py @@ -12,6 +12,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch from flask import Flask +from sqlalchemy.orm import Session from controllers.console.app import agent_drive_inspector as inspector from controllers.console.app.agent_drive_inspector import ( @@ -43,18 +44,17 @@ _APP = SimpleNamespace( ) -def test_resolve_bound_agent_uses_injected_session(): - session = MagicMock() +def test_resolve_bound_agent_uses_injected_session(unbound_session: Session): resolver = MagicMock(return_value="agent-1") app_model = SimpleNamespace(bound_agent_id_with_session=resolver) - result = inspector._resolve_agent_id(session, app_model, None) + result = inspector._resolve_agent_id(unbound_session, app_model, None) assert result == "agent-1" - resolver.assert_called_once_with(session=session) - assert resolver.call_args.kwargs["session"] is session + resolver.assert_called_once_with(session=unbound_session) + assert resolver.call_args.kwargs["session"] is unbound_session -def test_list_filters_value_pointers_out_of_console_payload(): +def test_list_filters_value_pointers_out_of_console_payload(unbound_session: Session): raw = _raw(AgentDriveListApi.get) with app.test_request_context("/?prefix=pdf-toolkit/"): with patch(f"{_MOD}.AgentDriveService") as drive: @@ -69,15 +69,14 @@ def test_list_filters_value_pointers_out_of_console_payload(): "created_at": 1718000000, } ] - body = raw(AgentDriveListApi(), MagicMock(), _APP) + body = raw(AgentDriveListApi(), unbound_session, _APP) assert body["items"][0]["key"] == "pdf-toolkit/SKILL.md" assert "file_id" not in body["items"][0] assert drive.return_value.manifest.call_args.kwargs["prefix"] == "pdf-toolkit/" -def test_list_by_agent_filters_value_pointers_out_of_console_payload(): +def test_list_by_agent_filters_value_pointers_out_of_console_payload(unbound_session: Session): raw = _raw(AgentDriveListByAgentApi.get) - session = MagicMock() with app.test_request_context("/?prefix=pdf-toolkit/"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, @@ -94,28 +93,27 @@ def test_list_by_agent_filters_value_pointers_out_of_console_payload(): "created_at": 1718000000, } ] - body = raw(AgentDriveListByAgentApi(), session, "tenant-1", "agent-1") + body = raw(AgentDriveListByAgentApi(), unbound_session, "tenant-1", "agent-1") assert body["items"][0]["key"] == "pdf-toolkit/SKILL.md" assert "file_id" not in body["items"][0] - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") assert drive.return_value.manifest.call_args.kwargs["agent_id"] == "agent-1" - assert drive.return_value.manifest.call_args.kwargs["session"] is session + assert drive.return_value.manifest.call_args.kwargs["session"] is unbound_session -def test_list_resolves_workflow_node_binding_agent(): +def test_list_resolves_workflow_node_binding_agent(unbound_session: Session): raw = _raw(AgentDriveListApi.get) with app.test_request_context("/?node_id=agent-node-1"): with patch(f"{_MOD}.AgentComposerService") as composer, patch(f"{_MOD}.AgentDriveService") as drive: composer.resolve_workflow_node_agent_id.return_value = "wf-agent-9" drive.return_value.manifest.return_value = [] - raw(AgentDriveListApi(), MagicMock(), _APP) + raw(AgentDriveListApi(), unbound_session, _APP) assert drive.return_value.manifest.call_args.kwargs["agent_id"] == "wf-agent-9" assert composer.resolve_workflow_node_agent_id.call_args.kwargs["node_id"] == "agent-node-1" -def test_skill_list_by_agent_calls_service(): +def test_skill_list_by_agent_calls_service(unbound_session: Session): raw = _raw(AgentDriveSkillListByAgentApi.get) - session = MagicMock() with app.test_request_context("/"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, @@ -134,27 +132,26 @@ def test_skill_list_by_agent_calls_service(): "created_at": 1718000000, } ] - body = raw(AgentDriveSkillListByAgentApi(), session, "tenant-1", "agent-1") + body = raw(AgentDriveSkillListByAgentApi(), unbound_session, "tenant-1", "agent-1") assert body["items"][0]["path"] == "pdf-toolkit" - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") assert drive.return_value.list_skills.call_args.kwargs["agent_id"] == "agent-1" - assert drive.return_value.list_skills.call_args.kwargs["session"] is session + assert drive.return_value.list_skills.call_args.kwargs["session"] is unbound_session -def test_skill_list_resolves_workflow_node_binding_agent(): +def test_skill_list_resolves_workflow_node_binding_agent(unbound_session: Session): raw = _raw(AgentDriveSkillListApi.get) with app.test_request_context("/?node_id=agent-node-1"): with patch(f"{_MOD}.AgentComposerService") as composer, patch(f"{_MOD}.AgentDriveService") as drive: composer.resolve_workflow_node_agent_id.return_value = "wf-agent-9" drive.return_value.list_skills.return_value = [] - body = raw(AgentDriveSkillListApi(), MagicMock(), _APP) + body = raw(AgentDriveSkillListApi(), unbound_session, _APP) assert body == {"items": []} assert drive.return_value.list_skills.call_args.kwargs["agent_id"] == "wf-agent-9" -def test_skill_inspect_by_agent_returns_strict_json_response(): +def test_skill_inspect_by_agent_returns_strict_json_response(unbound_session: Session): raw = _raw(AgentDriveSkillInspectByAgentApi.get) - session = MagicMock() payload = { "path": "pdf-toolkit", "skill_md_key": "pdf-toolkit/SKILL.md", @@ -191,14 +188,14 @@ def test_skill_inspect_by_agent_returns_strict_json_response(): patch(f"{_MOD}.AgentDriveService") as drive, ): drive.return_value.inspect_skill.return_value = payload - response = raw(AgentDriveSkillInspectByAgentApi(), session, "tenant-1", "agent-1", "pdf-toolkit") + response = raw(AgentDriveSkillInspectByAgentApi(), unbound_session, "tenant-1", "agent-1", "pdf-toolkit") assert response.status_code == 200 assert response.get_json()["skill_md"]["text"] == "# PDF Toolkit\nUse it.\n" assert b"# PDF Toolkit\\nUse it.\\n" in response.get_data() - assert drive.return_value.inspect_skill.call_args.kwargs["session"] is session + assert drive.return_value.inspect_skill.call_args.kwargs["session"] is unbound_session -def test_skill_inspect_resolves_workflow_node_binding_agent(): +def test_skill_inspect_resolves_workflow_node_binding_agent(unbound_session: Session): raw = _raw(AgentDriveSkillInspectApi.get) payload = { "path": "pdf-toolkit", @@ -220,24 +217,23 @@ def test_skill_inspect_resolves_workflow_node_binding_agent(): with patch(f"{_MOD}.AgentComposerService") as composer, patch(f"{_MOD}.AgentDriveService") as drive: composer.resolve_workflow_node_agent_id.return_value = "wf-agent-9" drive.return_value.inspect_skill.return_value = payload - response = raw(AgentDriveSkillInspectApi(), MagicMock(), _APP, "pdf-toolkit") + response = raw(AgentDriveSkillInspectApi(), unbound_session, _APP, "pdf-toolkit") assert response.get_json()["path"] == "pdf-toolkit" assert drive.return_value.inspect_skill.call_args.kwargs["agent_id"] == "wf-agent-9" -def test_list_400_when_no_agent_bound(): +def test_list_400_when_no_agent_bound(unbound_session: Session): raw = _raw(AgentDriveListApi.get) resolver = MagicMock(return_value=None) app_without_agent = SimpleNamespace(bound_agent_id_with_session=resolver) - session = MagicMock() with app.test_request_context("/"): - body, status = raw(AgentDriveListApi(), session, app_without_agent) + body, status = raw(AgentDriveListApi(), unbound_session, app_without_agent) assert status == 400 assert body["code"] == "agent_not_bound" - resolver.assert_called_once_with(session=session) + resolver.assert_called_once_with(session=unbound_session) -def test_preview_passes_through_and_maps_errors(): +def test_preview_passes_through_and_maps_errors(unbound_session: Session): raw = _raw(AgentDrivePreviewApi.get) with app.test_request_context("/?key=pdf-toolkit/SKILL.md"): with patch(f"{_MOD}.AgentDriveService") as drive: @@ -248,21 +244,20 @@ def test_preview_passes_through_and_maps_errors(): "binary": False, "text": "# hi", } - body = raw(AgentDrivePreviewApi(), MagicMock(), _APP) + body = raw(AgentDrivePreviewApi(), unbound_session, _APP) assert body["text"] == "# hi" with app.test_request_context("/?key=ghost/SKILL.md"): with patch(f"{_MOD}.AgentDriveService") as drive: drive.return_value.preview.side_effect = AgentDriveError( "drive_key_not_found", "no drive entry", status_code=404 ) - body, status = raw(AgentDrivePreviewApi(), MagicMock(), _APP) + body, status = raw(AgentDrivePreviewApi(), unbound_session, _APP) assert status == 404 assert body["code"] == "drive_key_not_found" -def test_preview_by_agent_passes_through_and_maps_errors(): +def test_preview_by_agent_passes_through_and_maps_errors(unbound_session: Session): raw = _raw(AgentDrivePreviewByAgentApi.get) - session = MagicMock() with app.test_request_context("/?key=pdf-toolkit/SKILL.md"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, @@ -275,10 +270,10 @@ def test_preview_by_agent_passes_through_and_maps_errors(): "binary": False, "text": "# hi", } - body = raw(AgentDrivePreviewByAgentApi(), session, "tenant-1", "agent-1") + body = raw(AgentDrivePreviewByAgentApi(), unbound_session, "tenant-1", "agent-1") assert body["text"] == "# hi" - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") - assert drive.return_value.preview.call_args.kwargs["session"] is session + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") + assert drive.return_value.preview.call_args.kwargs["session"] is unbound_session with app.test_request_context("/?key=ghost/SKILL.md"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP), @@ -287,30 +282,29 @@ def test_preview_by_agent_passes_through_and_maps_errors(): drive.return_value.preview.side_effect = AgentDriveError( "drive_key_not_found", "no drive entry", status_code=404 ) - body, status = raw(AgentDrivePreviewByAgentApi(), session, "tenant-1", "agent-1") + body, status = raw(AgentDrivePreviewByAgentApi(), unbound_session, "tenant-1", "agent-1") assert status == 404 assert body["code"] == "drive_key_not_found" -def test_download_returns_signed_url_json(): +def test_download_returns_signed_url_json(unbound_session: Session): raw = _raw(AgentDriveDownloadApi.get) with app.test_request_context("/?key=pdf-toolkit/.DIFY-SKILL-FULL.zip"): with patch(f"{_MOD}.AgentDriveService") as drive: drive.return_value.download_url.return_value = "https://signed.example/zip" - body = raw(AgentDriveDownloadApi(), MagicMock(), _APP) + body = raw(AgentDriveDownloadApi(), unbound_session, _APP) assert body == {"url": "https://signed.example/zip"} -def test_download_by_agent_returns_signed_url_json(): +def test_download_by_agent_returns_signed_url_json(unbound_session: Session): raw = _raw(AgentDriveDownloadByAgentApi.get) - session = MagicMock() with app.test_request_context("/?key=pdf-toolkit/.DIFY-SKILL-FULL.zip"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, patch(f"{_MOD}.AgentDriveService") as drive, ): drive.return_value.download_url.return_value = "https://signed.example/zip" - body = raw(AgentDriveDownloadByAgentApi(), session, "tenant-1", "agent-1") + body = raw(AgentDriveDownloadByAgentApi(), unbound_session, "tenant-1", "agent-1") assert body == {"url": "https://signed.example/zip"} - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") - assert drive.return_value.download_url.call_args.kwargs["session"] is session + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") + assert drive.return_value.download_url.call_args.kwargs["session"] is unbound_session diff --git a/api/tests/unit_tests/controllers/console/app/test_agent_skills.py b/api/tests/unit_tests/controllers/console/app/test_agent_skills.py index e555e9c5094..f0496fda088 100644 --- a/api/tests/unit_tests/controllers/console/app/test_agent_skills.py +++ b/api/tests/unit_tests/controllers/console/app/test_agent_skills.py @@ -8,12 +8,15 @@ bare Flask request context with the services mocked — covering request handlin from __future__ import annotations import io +from datetime import UTC, datetime from inspect import unwrap from types import SimpleNamespace from unittest.mock import MagicMock, patch +from uuid import uuid4 import pytest from flask import Flask +from sqlalchemy.orm import Session from controllers.console.app import agent as agent_controller from controllers.console.app.agent import ( @@ -23,12 +26,16 @@ from controllers.console.app.agent import ( AgentSkillUploadApi, AgentSkillUploadByAgentApi, ) -from models.model import AppMode +from extensions.storage.storage_type import StorageType +from models.enums import CreatorUserRole +from models.model import AppMode, UploadFile from services.agent.skill_package_service import SkillPackageError from services.agent_drive_service import AgentDriveError _MOD = "controllers.console.app.agent" app = Flask(__name__) +_TENANT_ID = "00000000-0000-0000-0000-000000000010" +_UPLOAD_FILE_ID = "0fa6f9bc-3416-4476-8857-a13129704dd9" def _raw(method): @@ -43,30 +50,49 @@ def _file_ctx(*, files: dict[str, bytes] | None = None): _USER = SimpleNamespace(id="user-1") _APP = SimpleNamespace( id="app-1", - tenant_id="tenant-1", + tenant_id=_TENANT_ID, mode=AppMode.AGENT, bound_agent_id_with_session=lambda *, session: "agent-1", ) _WORKFLOW_APP = SimpleNamespace( id="app-1", - tenant_id="tenant-1", + tenant_id=_TENANT_ID, mode=AppMode.WORKFLOW, bound_agent_id_with_session=lambda *, session: None, ) -def test_resolve_bound_agent_uses_injected_session(): - session = MagicMock() +def _persist_upload(session: Session, *, name: str = "sample.pdf") -> UploadFile: + upload = UploadFile( + tenant_id=_TENANT_ID, + storage_type=StorageType.LOCAL, + key=f"uploads/{name}", + name=name, + size=5, + extension="pdf", + mime_type="application/pdf", + created_by_role=CreatorUserRole.ACCOUNT, + created_by=str(uuid4()), + created_at=datetime.now(UTC), + used=False, + ) + upload.id = _UPLOAD_FILE_ID + session.add(upload) + session.commit() + return upload + + +def test_resolve_bound_agent_uses_injected_session(unbound_session: Session): resolver = MagicMock(return_value="agent-1") app_model = SimpleNamespace(bound_agent_id_with_session=resolver) - result = agent_controller._resolve_agent_id(session, app_model, None) + result = agent_controller._resolve_agent_id(unbound_session, app_model, None) assert result == "agent-1" - resolver.assert_called_once_with(session=session) - assert resolver.call_args.kwargs["session"] is session + resolver.assert_called_once_with(session=unbound_session) + assert resolver.call_args.kwargs["session"] is unbound_session -def test_upload_standardizes_into_drive_and_returns_skill_ref(): +def test_upload_standardizes_into_drive_and_returns_skill_ref(unbound_session: Session): raw = _raw(AgentSkillUploadApi.post) with _file_ctx(files={"file": b"zip-bytes"}): with patch(f"{_MOD}.SkillStandardizeService") as svc: @@ -74,61 +100,59 @@ def test_upload_standardizes_into_drive_and_returns_skill_ref(): "skill": {"path": "skill-a", "skill_md_key": "skill-a/SKILL.md"}, "manifest": {"name": "Skill A"}, } - body, status = raw(AgentSkillUploadApi(), MagicMock(), _USER, _APP) + body, status = raw(AgentSkillUploadApi(), unbound_session, _USER, _APP) assert status == 201 assert body["skill"] == {"path": "skill-a", "skill_md_key": "skill-a/SKILL.md"} assert svc.return_value.standardize.call_args.kwargs["agent_id"] == "agent-1" -def test_upload_by_agent_resolves_app_and_standardizes_into_drive(): +def test_upload_by_agent_resolves_app_and_standardizes_into_drive(unbound_session: Session): raw = _raw(AgentSkillUploadByAgentApi.post) with _file_ctx(files={"file": b"zip-bytes"}): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, patch(f"{_MOD}.SkillStandardizeService") as svc, ): - session = MagicMock() svc.return_value.standardize.return_value = {"skill": {"path": "skill-a"}, "manifest": {}} - body, status = raw(AgentSkillUploadByAgentApi(), session, "tenant-1", _USER, "agent-1") + body, status = raw(AgentSkillUploadByAgentApi(), unbound_session, "tenant-1", _USER, "agent-1") assert status == 201 assert body["skill"] == {"path": "skill-a"} - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") assert svc.return_value.standardize.call_args.kwargs["agent_id"] == "agent-1" -def test_upload_no_file_is_400(): +def test_upload_no_file_is_400(unbound_session: Session): raw = _raw(AgentSkillUploadApi.post) with _file_ctx(files={}): - body, status = raw(AgentSkillUploadApi(), MagicMock(), _USER, _APP) + body, status = raw(AgentSkillUploadApi(), unbound_session, _USER, _APP) assert status == 400 assert body["code"] == "no_file" -def test_upload_maps_package_error(): +def test_upload_maps_package_error(unbound_session: Session): raw = _raw(AgentSkillUploadApi.post) with _file_ctx(files={"file": b"bad"}): with patch(f"{_MOD}.SkillStandardizeService") as svc: svc.return_value.standardize.side_effect = SkillPackageError( "missing_skill_md", "no SKILL.md", status_code=400 ) - body, status = raw(AgentSkillUploadApi(), MagicMock(), _USER, _APP) + body, status = raw(AgentSkillUploadApi(), unbound_session, _USER, _APP) assert status == 400 assert body["code"] == "missing_skill_md" -def test_upload_no_bound_agent_is_400(): +def test_upload_no_bound_agent_is_400(unbound_session: Session): raw = _raw(AgentSkillUploadApi.post) resolver = MagicMock(return_value=None) app_without_agent = SimpleNamespace(bound_agent_id_with_session=resolver) - session = MagicMock() with _file_ctx(files={"file": b"zip"}): - body, status = raw(AgentSkillUploadApi(), session, _USER, app_without_agent) + body, status = raw(AgentSkillUploadApi(), unbound_session, _USER, app_without_agent) assert status == 400 assert body["code"] == "agent_not_bound" - resolver.assert_called_once_with(session=session) + resolver.assert_called_once_with(session=unbound_session) -def test_upload_resolves_workflow_node_agent(): +def test_upload_resolves_workflow_node_agent(unbound_session: Session): raw = _raw(AgentSkillUploadApi.post) with app.test_request_context( "/?node_id=agent-node-1", method="POST", data={"file": (io.BytesIO(b"zip"), "skill.zip")} @@ -136,18 +160,18 @@ def test_upload_resolves_workflow_node_agent(): with patch(f"{_MOD}.AgentComposerService") as composer, patch(f"{_MOD}.SkillStandardizeService") as svc: composer.resolve_workflow_node_agent_id.return_value = "wf-agent-1" svc.return_value.standardize.return_value = {"skill": {"path": "s"}, "manifest": {}} - body, status = raw(AgentSkillUploadApi(), MagicMock(), _USER, _WORKFLOW_APP) + body, status = raw(AgentSkillUploadApi(), unbound_session, _USER, _WORKFLOW_APP) assert status == 201 assert body["skill"] == {"path": "s"} assert svc.return_value.standardize.call_args.kwargs["agent_id"] == "wf-agent-1" -def test_upload_maps_drive_error(): +def test_upload_maps_drive_error(unbound_session: Session): raw = _raw(AgentSkillUploadApi.post) with _file_ctx(files={"file": b"zip"}): with patch(f"{_MOD}.SkillStandardizeService") as svc: svc.return_value.standardize.side_effect = AgentDriveError("source_not_found", "nope", status_code=404) - body, status = raw(AgentSkillUploadApi(), MagicMock(), _USER, _APP) + body, status = raw(AgentSkillUploadApi(), unbound_session, _USER, _APP) assert status == 404 assert body["code"] == "source_not_found" @@ -156,86 +180,81 @@ def _json_ctx(payload: dict | None = None, *, method: str = "POST", query_string return app.test_request_context(f"/?{query_string}", method=method, json=payload or {}) -def test_files_commit_validates_upload_and_returns_drive_ref(): +def test_files_commit_validates_upload_and_returns_drive_ref(sqlite_session: Session): from controllers.console.app.agent import AgentDriveFilesApi raw = _raw(AgentDriveFilesApi.post) - upload = SimpleNamespace(id="uf-1", name="sample qna.pdf") - with _json_ctx({"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"}): + upload = _persist_upload(sqlite_session, name="sample qna.pdf") + with _json_ctx({"upload_file_id": _UPLOAD_FILE_ID}): with patch(f"{_MOD}.console_ns") as ns, patch(f"{_MOD}.AgentDriveService") as drive: - session = MagicMock() - ns.payload = {"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"} - session.scalar.return_value = upload + ns.payload = {"upload_file_id": _UPLOAD_FILE_ID} drive.return_value.commit.return_value = [ {"key": "files/sample qna.pdf", "size": 5, "mime_type": "application/pdf"} ] - body, status = raw(AgentDriveFilesApi(), session, _USER, _APP) + body, status = raw(AgentDriveFilesApi(), sqlite_session, _USER, _APP) assert status == 201 assert body["file"]["drive_key"] == "files/sample qna.pdf" - assert body["file"]["file_id"] == "uf-1" + assert body["file"]["file_id"] == upload.id item = drive.return_value.commit.call_args.kwargs["items"][0] assert item.value_owned_by_drive is True assert item.file_ref.kind == "upload_file" -def test_files_by_agent_commit_uses_agent_route_and_ignores_node_id(): +def test_files_by_agent_commit_uses_agent_route_and_ignores_node_id(sqlite_session: Session): raw = _raw(AgentDriveFilesByAgentApi.post) - upload = SimpleNamespace(id="uf-1", name="sample.pdf") - with _json_ctx({"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"}, query_string="node_id=ignored"): + _persist_upload(sqlite_session) + with _json_ctx({"upload_file_id": _UPLOAD_FILE_ID}, query_string="node_id=ignored"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, patch(f"{_MOD}.console_ns") as ns, patch(f"{_MOD}.AgentDriveService") as drive, ): - session = MagicMock() - ns.payload = {"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"} - session.scalar.return_value = upload + ns.payload = {"upload_file_id": _UPLOAD_FILE_ID} drive.return_value.commit.return_value = [ {"key": "files/sample.pdf", "size": 5, "mime_type": "application/pdf"} ] - body, status = raw(AgentDriveFilesByAgentApi(), session, "tenant-1", _USER, "agent-1") + body, status = raw(AgentDriveFilesByAgentApi(), sqlite_session, "tenant-1", _USER, "agent-1") assert status == 201 - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") + resolve_app.assert_called_once_with(session=sqlite_session, tenant_id="tenant-1", agent_id="agent-1") -def test_files_commit_404_when_upload_not_in_tenant(): +def test_files_commit_404_when_upload_not_in_tenant(sqlite_session: Session): from controllers.console.app.agent import AgentDriveFilesApi raw = _raw(AgentDriveFilesApi.post) - with _json_ctx({"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"}): + other_upload = _persist_upload(sqlite_session) + other_upload.tenant_id = str(uuid4()) + sqlite_session.commit() + with _json_ctx({"upload_file_id": _UPLOAD_FILE_ID}): with patch(f"{_MOD}.console_ns") as ns: - session = MagicMock() - ns.payload = {"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"} - session.scalar.return_value = None - body, status = raw(AgentDriveFilesApi(), session, _USER, _APP) + ns.payload = {"upload_file_id": _UPLOAD_FILE_ID} + body, status = raw(AgentDriveFilesApi(), sqlite_session, _USER, _APP) assert status == 404 assert body["code"] == "upload_file_not_found" -def test_files_commit_resolves_workflow_node_agent(): +def test_files_commit_resolves_workflow_node_agent(sqlite_session: Session): from controllers.console.app.agent import AgentDriveFilesApi raw = _raw(AgentDriveFilesApi.post) - upload = SimpleNamespace(id="uf-1", name="sample.pdf") - with _json_ctx({"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"}, query_string="node_id=agent-node-1"): + _persist_upload(sqlite_session) + with _json_ctx({"upload_file_id": _UPLOAD_FILE_ID}, query_string="node_id=agent-node-1"): with ( patch(f"{_MOD}.console_ns") as ns, patch(f"{_MOD}.AgentDriveService") as drive, patch(f"{_MOD}.AgentComposerService") as composer, ): - session = MagicMock() - ns.payload = {"upload_file_id": "0fa6f9bc-3416-4476-8857-a13129704dd9"} - session.scalar.return_value = upload + ns.payload = {"upload_file_id": _UPLOAD_FILE_ID} composer.resolve_workflow_node_agent_id.return_value = "wf-agent-1" drive.return_value.commit.return_value = [ {"key": "files/sample.pdf", "size": 5, "mime_type": "application/pdf"} ] - body, status = raw(AgentDriveFilesApi(), session, _USER, _WORKFLOW_APP) + body, status = raw(AgentDriveFilesApi(), sqlite_session, _USER, _WORKFLOW_APP) assert status == 201 assert drive.return_value.commit.call_args.kwargs["agent_id"] == "wf-agent-1" -def test_files_delete_updates_soul_then_drive(): +def test_files_delete_updates_soul_then_drive(unbound_session: Session): from controllers.console.app.agent import AgentDriveFilesApi raw = _raw(AgentDriveFilesApi.delete) @@ -245,26 +264,25 @@ def test_files_delete_updates_soul_then_drive(): drive.return_value.commit.side_effect = lambda **kw: ( calls.append("drive") or [{"key": "files/sample.pdf", "removed": True}] ) - body = raw(AgentDriveFilesApi(), MagicMock(), _USER, _APP) + body = raw(AgentDriveFilesApi(), unbound_session, _USER, _APP) assert calls == ["drive"] assert body == {"result": "success", "removed_keys": ["files/sample.pdf"]} -def test_files_by_agent_delete_uses_agent_route_and_ignores_node_id(): +def test_files_by_agent_delete_uses_agent_route_and_ignores_node_id(unbound_session: Session): raw = _raw(AgentDriveFilesByAgentApi.delete) with _json_ctx(method="DELETE", query_string="key=files/sample.pdf&node_id=ignored"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, patch(f"{_MOD}.AgentDriveService") as drive, ): - session = MagicMock() drive.return_value.commit.return_value = [{"key": "files/sample.pdf", "removed": True}] - body = raw(AgentDriveFilesByAgentApi(), session, "tenant-1", _USER, "agent-1") + body = raw(AgentDriveFilesByAgentApi(), unbound_session, "tenant-1", _USER, "agent-1") assert body == {"result": "success", "removed_keys": ["files/sample.pdf"]} - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") -def test_files_delete_resolves_workflow_node_agent(): +def test_files_delete_resolves_workflow_node_agent(unbound_session: Session): from controllers.console.app.agent import AgentDriveFilesApi raw = _raw(AgentDriveFilesApi.delete) @@ -272,12 +290,12 @@ def test_files_delete_resolves_workflow_node_agent(): with patch(f"{_MOD}.AgentComposerService") as composer, patch(f"{_MOD}.AgentDriveService") as drive: composer.resolve_workflow_node_agent_id.return_value = "wf-agent-1" drive.return_value.commit.return_value = [{"key": "files/sample.pdf", "removed": True}] - body = raw(AgentDriveFilesApi(), MagicMock(), _USER, _WORKFLOW_APP) + body = raw(AgentDriveFilesApi(), unbound_session, _USER, _WORKFLOW_APP) assert body == {"result": "success", "removed_keys": ["files/sample.pdf"]} assert drive.return_value.commit.call_args.kwargs["agent_id"] == "wf-agent-1" -def test_files_delete_survives_drive_failure(): +def test_files_delete_survives_drive_failure(unbound_session: Session): from controllers.console.app.agent import AgentDriveFilesApi raw = _raw(AgentDriveFilesApi.delete) @@ -285,10 +303,10 @@ def test_files_delete_survives_drive_failure(): with patch(f"{_MOD}.AgentDriveService") as drive: drive.return_value.commit.side_effect = RuntimeError("storage down") with pytest.raises(RuntimeError, match="storage down"): - raw(AgentDriveFilesApi(), MagicMock(), _USER, _APP) + raw(AgentDriveFilesApi(), unbound_session, _USER, _APP) -def test_skill_delete_uses_slug_prefix_and_is_idempotent(): +def test_skill_delete_uses_slug_prefix_and_is_idempotent(unbound_session: Session): from controllers.console.app.agent import AgentSkillApi raw = _raw(AgentSkillApi.delete) @@ -298,38 +316,37 @@ def test_skill_delete_uses_slug_prefix_and_is_idempotent(): {"key": "tender-analyzer/SKILL.md", "removed": True}, {"key": "tender-analyzer/.DIFY-SKILL-FULL.zip", "removed": True}, ] - body = raw(AgentSkillApi(), MagicMock(), _USER, _APP, "tender-analyzer") + body = raw(AgentSkillApi(), unbound_session, _USER, _APP, "tender-analyzer") assert body == { "result": "success", "removed_keys": ["tender-analyzer/SKILL.md", "tender-analyzer/.DIFY-SKILL-FULL.zip"], } -def test_skill_delete_by_agent_uses_agent_route(): +def test_skill_delete_by_agent_uses_agent_route(unbound_session: Session): raw = _raw(AgentSkillByAgentApi.delete) with _json_ctx(method="DELETE", query_string="node_id=ignored"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, patch(f"{_MOD}.AgentDriveService") as drive, ): - session = MagicMock() drive.return_value.commit.return_value = [{"key": "tender-analyzer/SKILL.md", "removed": True}] - body = raw(AgentSkillByAgentApi(), session, "tenant-1", _USER, "agent-1", "tender-analyzer") + body = raw(AgentSkillByAgentApi(), unbound_session, "tenant-1", _USER, "agent-1", "tender-analyzer") assert body == {"result": "success", "removed_keys": ["tender-analyzer/SKILL.md"]} - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") -def test_skill_delete_rejects_path_like_slug(): +def test_skill_delete_rejects_path_like_slug(unbound_session: Session): from controllers.console.app.agent import AgentSkillApi raw = _raw(AgentSkillApi.delete) with _json_ctx(method="DELETE"): - body, status = raw(AgentSkillApi(), MagicMock(), _USER, _APP, "a/b") + body, status = raw(AgentSkillApi(), unbound_session, _USER, _APP, "a/b") assert status == 400 assert body["code"] == "drive_key_invalid" -def test_infer_tools_returns_draft_suggestions(): +def test_infer_tools_returns_draft_suggestions(unbound_session: Session): from controllers.console.app.agent import AgentSkillInferToolsApi raw = _raw(AgentSkillInferToolsApi.post) @@ -340,27 +357,32 @@ def test_infer_tools_returns_draft_suggestions(): "cli_tools": [{"name": "ffmpeg", "inferred_from": "audio-transcribe"}], "reason": None, } - body = raw(AgentSkillInferToolsApi(), MagicMock(), _APP, "audio-transcribe") + body = raw(AgentSkillInferToolsApi(), unbound_session, _APP, "audio-transcribe") assert body["inferable"] is True assert svc.return_value.infer.call_args.kwargs["slug"] == "audio-transcribe" -def test_infer_tools_by_agent_uses_agent_route(): +def test_infer_tools_by_agent_uses_agent_route(unbound_session: Session): raw = _raw(AgentSkillInferToolsByAgentApi.post) with _json_ctx(query_string="node_id=ignored"): with ( patch(f"{_MOD}.resolve_agent_runtime_app_model", return_value=_APP) as resolve_app, patch(f"{_MOD}.SkillToolInferenceService") as svc, ): - session = MagicMock() svc.return_value.infer.return_value = {"inferable": True, "cli_tools": [], "reason": None} - body = raw(AgentSkillInferToolsByAgentApi(), session, "tenant-1", "agent-1", "audio-transcribe") + body = raw( + AgentSkillInferToolsByAgentApi(), + unbound_session, + "tenant-1", + "agent-1", + "audio-transcribe", + ) assert body["inferable"] is True - resolve_app.assert_called_once_with(session=session, tenant_id="tenant-1", agent_id="agent-1") + resolve_app.assert_called_once_with(session=unbound_session, tenant_id="tenant-1", agent_id="agent-1") assert svc.return_value.infer.call_args.kwargs["agent_id"] == "agent-1" -def test_infer_tools_resolves_workflow_node_agent(): +def test_infer_tools_resolves_workflow_node_agent(unbound_session: Session): from controllers.console.app.agent import AgentSkillInferToolsApi raw = _raw(AgentSkillInferToolsApi.post) @@ -368,12 +390,12 @@ def test_infer_tools_resolves_workflow_node_agent(): with patch(f"{_MOD}.AgentComposerService") as composer, patch(f"{_MOD}.SkillToolInferenceService") as svc: composer.resolve_workflow_node_agent_id.return_value = "wf-agent-1" svc.return_value.infer.return_value = {"inferable": False, "cli_tools": [], "reason": "none"} - body = raw(AgentSkillInferToolsApi(), MagicMock(), _WORKFLOW_APP, "audio-transcribe") + body = raw(AgentSkillInferToolsApi(), unbound_session, _WORKFLOW_APP, "audio-transcribe") assert body["inferable"] is False assert svc.return_value.infer.call_args.kwargs["agent_id"] == "wf-agent-1" -def test_infer_tools_maps_inference_errors(): +def test_infer_tools_maps_inference_errors(unbound_session: Session): from controllers.console.app.agent import AgentSkillInferToolsApi from services.agent.skill_tool_inference_service import SkillToolInferenceError @@ -383,19 +405,19 @@ def test_infer_tools_maps_inference_errors(): svc.return_value.infer.side_effect = SkillToolInferenceError( "default_model_not_configured", "no model", status_code=400 ) - body, status = raw(AgentSkillInferToolsApi(), MagicMock(), _APP, "audio-transcribe") + body, status = raw(AgentSkillInferToolsApi(), unbound_session, _APP, "audio-transcribe") assert status == 400 assert body["code"] == "default_model_not_configured" -def test_infer_tools_rejects_path_like_slug_and_unbound_app(): +def test_infer_tools_rejects_path_like_slug_and_unbound_app(unbound_session: Session): from controllers.console.app.agent import AgentSkillInferToolsApi raw = _raw(AgentSkillInferToolsApi.post) with _json_ctx(): - body, status = raw(AgentSkillInferToolsApi(), MagicMock(), _APP, "a/b") + body, status = raw(AgentSkillInferToolsApi(), unbound_session, _APP, "a/b") assert (status, body["code"]) == (400, "drive_key_invalid") app_without_agent = SimpleNamespace(bound_agent_id_with_session=MagicMock(return_value=None)) with _json_ctx(): - body, status = raw(AgentSkillInferToolsApi(), MagicMock(), app_without_agent, "x") + body, status = raw(AgentSkillInferToolsApi(), unbound_session, app_without_agent, "x") assert (status, body["code"]) == (400, "agent_not_bound") diff --git a/api/tests/unit_tests/controllers/console/app/test_annotation_api.py b/api/tests/unit_tests/controllers/console/app/test_annotation_api.py index 33f19ae3550..f225ef73477 100644 --- a/api/tests/unit_tests/controllers/console/app/test_annotation_api.py +++ b/api/tests/unit_tests/controllers/console/app/test_annotation_api.py @@ -2,18 +2,33 @@ from __future__ import annotations from inspect import unwrap from types import SimpleNamespace -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch import pytest from flask import Flask +from sqlalchemy.orm import Session from werkzeug.exceptions import NotFound from controllers.console.app import annotation as annotation_module +from models.model import App, AppMode, IconType from services.app_ref_service import AnnotationRef, AppRef -def _app_model() -> SimpleNamespace: - return SimpleNamespace(id="app-1", tenant_id="tenant-1", status="normal") +def _persist_app(session: Session) -> App: + app = App( + id="app-1", + tenant_id="tenant-1", + name="Annotation app", + mode=AppMode.CHAT, + icon_type=IconType.EMOJI, + icon="chat", + icon_background="#ffffff", + enable_site=False, + enable_api=True, + ) + session.add(app) + session.commit() + return app def _annotation_model(annotation_id: str = "ann-1") -> SimpleNamespace: @@ -109,27 +124,25 @@ def test_annotation_file_payload_valid(): assert payload.message_id == "550e8400-e29b-41d4-a716-446655440000" -def test_get_app_ref_raises_not_found_when_app_is_not_in_current_tenant(): - session = MagicMock() - session.scalar.return_value = None +def test_get_app_ref_raises_not_found_when_app_is_not_in_current_tenant(sqlite_session: Session): + _persist_app(sqlite_session) with ( patch.object( annotation_module, "current_account_with_tenant", - return_value=(SimpleNamespace(id="account-1"), "tenant-1"), + return_value=(SimpleNamespace(id="account-1"), "tenant-2"), ), ): with pytest.raises(NotFound): - annotation_module._get_app_ref(session, "app-1") + annotation_module._get_app_ref(sqlite_session, "app-1") class TestConsoleAnnotationRefBoundaries: - def test_batch_delete_uses_app_ref(self, app: Flask): + def test_batch_delete_uses_app_ref(self, app: Flask, sqlite_session: Session): api = annotation_module.AnnotationApi() handler = unwrap(api.delete) delete_mock = Mock() - session = MagicMock() - session.scalar.return_value = _app_model() + _persist_app(sqlite_session) with ( app.test_request_context("/?annotation_id=ann-1&annotation_id=ann-2", method="DELETE"), @@ -140,19 +153,18 @@ class TestConsoleAnnotationRefBoundaries: ), patch.object(annotation_module.AppAnnotationService, "delete_app_annotations_in_batch", delete_mock), ): - response, status = handler(api, session, "app-1") + response, status = handler(api, sqlite_session, "app-1") assert response == "" assert status == 204 - delete_mock.assert_called_once_with(AppRef("tenant-1", "app-1"), ["ann-1", "ann-2"], session) + delete_mock.assert_called_once_with(AppRef("tenant-1", "app-1"), ["ann-1", "ann-2"], sqlite_session) - def test_update_uses_annotation_ref(self, app: Flask): + def test_update_uses_annotation_ref(self, app: Flask, sqlite_session: Session): api = annotation_module.AnnotationUpdateDeleteApi() handler = unwrap(api.post) update_mock = Mock(return_value=_annotation_model()) payload = {"question": "updated"} - session = MagicMock() - session.scalar.return_value = _app_model() + _persist_app(sqlite_session) with ( app.test_request_context("/annotations/ann-1", method="POST", json=payload), @@ -164,19 +176,18 @@ class TestConsoleAnnotationRefBoundaries: ), patch.object(annotation_module.AppAnnotationService, "update_app_annotation_directly", update_mock), ): - response = handler(api, session, "app-1", "ann-1") + response = handler(api, sqlite_session, "app-1", "ann-1") assert response["question"] == "q" update_mock.assert_called_once() assert update_mock.call_args.args[1] == AnnotationRef(AppRef("tenant-1", "app-1"), "ann-1") - assert update_mock.call_args.args[2] is session + assert update_mock.call_args.args[2] is sqlite_session - def test_delete_uses_annotation_ref(self, app: Flask): + def test_delete_uses_annotation_ref(self, app: Flask, sqlite_session: Session): api = annotation_module.AnnotationUpdateDeleteApi() handler = unwrap(api.delete) delete_mock = Mock() - session = MagicMock() - session.scalar.return_value = _app_model() + _persist_app(sqlite_session) with ( app.test_request_context("/annotations/ann-1", method="DELETE"), @@ -187,15 +198,15 @@ class TestConsoleAnnotationRefBoundaries: ), patch.object(annotation_module.AppAnnotationService, "delete_app_annotation", delete_mock), ): - response, status = handler(api, session, "app-1", "ann-1") + response, status = handler(api, sqlite_session, "app-1", "ann-1") assert response == "" assert status == 204 delete_mock.assert_called_once() assert delete_mock.call_args.args[0] == AnnotationRef(AppRef("tenant-1", "app-1"), "ann-1") - assert delete_mock.call_args.args[1] is session + assert delete_mock.call_args.args[1] is sqlite_session - def test_hit_history_uses_annotation_ref(self, app: Flask): + def test_hit_history_uses_annotation_ref(self, app: Flask, sqlite_session: Session): api = annotation_module.AnnotationHitHistoryListApi() handler = unwrap(api.get) history = SimpleNamespace( @@ -208,8 +219,7 @@ class TestConsoleAnnotationRefBoundaries: created_at=None, ) hit_history_mock = Mock(return_value=([history], 1)) - session = MagicMock() - session.scalar.return_value = _app_model() + _persist_app(sqlite_session) with ( app.test_request_context("/hit-histories?page=2&limit=5", method="GET"), @@ -220,7 +230,9 @@ class TestConsoleAnnotationRefBoundaries: ), patch.object(annotation_module.AppAnnotationService, "get_annotation_hit_histories", hit_history_mock), ): - response = handler(api, session, "app-1", "ann-1") + response = handler(api, sqlite_session, "app-1", "ann-1") assert response["total"] == 1 - hit_history_mock.assert_called_once_with(AnnotationRef(AppRef("tenant-1", "app-1"), "ann-1"), 2, 5, session) + hit_history_mock.assert_called_once_with( + AnnotationRef(AppRef("tenant-1", "app-1"), "ann-1"), 2, 5, sqlite_session + ) diff --git a/api/tests/unit_tests/controllers/console/app/test_app_apis.py b/api/tests/unit_tests/controllers/console/app/test_app_apis.py index 523e15b9411..2b428ad1f12 100644 --- a/api/tests/unit_tests/controllers/console/app/test_app_apis.py +++ b/api/tests/unit_tests/controllers/console/app/test_app_apis.py @@ -209,7 +209,9 @@ class TestCompletionEndpoints: class TestAppEndpoints: - def test_app_put_should_preserve_icon_type_when_payload_omits_it(self, app: Flask, monkeypatch: pytest.MonkeyPatch): + def test_app_put_should_preserve_icon_type_when_payload_omits_it( + self, app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session + ): api = app_module.AppApi() method = unwrap(api.put) payload = { @@ -230,7 +232,7 @@ class TestAppEndpoints: app.test_request_context("/console/api/apps/app-1", method="PUT", json=payload), patch.object(type(console_ns), "payload", payload), ): - response = method(api, MagicMock(spec=Session), app_model=_make_app(icon_type=app_module.IconType.EMOJI)) + response = method(api, unbound_session, app_model=_make_app(icon_type=app_module.IconType.EMOJI)) assert response == {"id": "app-1"} assert app_service.update_app.call_args.args[1]["icon_type"] is None @@ -247,7 +249,9 @@ class TestAppEndpoints: } ) - def test_app_icon_post_should_forward_icon_type(self, app: Flask, monkeypatch: pytest.MonkeyPatch): + def test_app_icon_post_should_forward_icon_type( + self, app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session + ): api = app_module.AppIconApi() method = unwrap(api.post) payload = { @@ -267,7 +271,7 @@ class TestAppEndpoints: app.test_request_context("/console/api/apps/app-1/icon", method="POST", json=payload), patch.object(type(console_ns), "payload", payload), ): - response = method(api, MagicMock(spec=Session), app_model=_make_app()) + response = method(api, unbound_session, app_model=_make_app()) assert response == {"id": "app-1"} assert app_service.update_app_icon.call_args.args[1:] == ( diff --git a/api/tests/unit_tests/controllers/console/app/test_app_response_models.py b/api/tests/unit_tests/controllers/console/app/test_app_response_models.py index b5631b4c1d1..2d4a0555f29 100644 --- a/api/tests/unit_tests/controllers/console/app/test_app_response_models.py +++ b/api/tests/unit_tests/controllers/console/app/test_app_response_models.py @@ -1,6 +1,7 @@ from __future__ import annotations import builtins +import json import sys from datetime import datetime from importlib import util @@ -12,9 +13,13 @@ import pytest from flask import Flask from flask.views import MethodView from pydantic import ValidationError +from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session from werkzeug.datastructures import MultiDict from configs import dify_config +from models.model import App, AppMode, IconType +from models.workflow import Workflow, WorkflowType # kombu references MethodView as a global when importing celery/kombu pools. if not hasattr(builtins, "MethodView"): @@ -326,7 +331,7 @@ def test_app_list_query_accepts_single_repeated_tag_id(app_module): assert query.tag_ids == [tag_id] -def test_create_app_endpoint_rejects_agent_mode(app_module, monkeypatch: pytest.MonkeyPatch): +def test_create_app_endpoint_rejects_agent_mode(app_module, monkeypatch: pytest.MonkeyPatch, unbound_session: Session): payload = {"name": "Iris", "mode": "agent", "description": "Agent app"} app_service = MagicMock() monkeypatch.setattr(app_module, "AppService", lambda: app_service) @@ -334,7 +339,7 @@ def test_create_app_endpoint_rejects_agent_mode(app_module, monkeypatch: pytest. app_module.console_ns.payload = payload try: with pytest.raises(ValidationError): - _unwrap(app_module.AppListApi().post)(MagicMock(), "tenant-1", SimpleNamespace(id="account-1")) + _unwrap(app_module.AppListApi().post)(unbound_session, "tenant-1", SimpleNamespace(id="account-1")) finally: app_module.console_ns.payload = None @@ -439,8 +444,9 @@ def test_app_detail_with_site_includes_nested_serialization(app_models): assert "role" not in serialized -def test_app_response_view_uses_the_caller_session_for_query_backed_fields(app_module, monkeypatch): - session = MagicMock() +def test_app_response_view_uses_the_caller_session_for_query_backed_fields( + app_module, monkeypatch, unbound_session: Session +): app_obj = MagicMock() app_model_config = SimpleNamespace(app_id="app-1") app_obj.desc_or_prompt_with_session.return_value = "Description" @@ -455,7 +461,7 @@ def test_app_response_view_uses_the_caller_session_for_query_backed_fields(app_m load_annotation_reply = MagicMock(return_value={"enabled": False}) monkeypatch.setattr("services.app_service.load_annotation_reply_config", load_annotation_reply) - view = app_module.AppResponseView(app_obj, session=session) + view = app_module.AppResponseView(app_obj, session=unbound_session) site = view.site workflow = view.workflow model_config = view.app_model_config @@ -483,8 +489,8 @@ def test_app_response_view_uses_the_caller_session_for_query_backed_fields(app_m app_obj.tags_with_session, app_obj.author_name_with_session, ): - method.assert_called_once_with(session=session) - load_annotation_reply.assert_called_once_with(session, "app-1") + method.assert_called_once_with(session=unbound_session) + load_annotation_reply.assert_called_once_with(unbound_session, "app-1") def test_app_pagination_aliases_per_page_and_has_next(app_models): @@ -529,7 +535,11 @@ def test_app_pagination_aliases_per_page_and_has_next(app_models): def test_app_list_uses_injected_session_for_draft_workflows( - app: Flask, app_module: ModuleType, monkeypatch: pytest.MonkeyPatch + app: Flask, + app_module: ModuleType, + monkeypatch: pytest.MonkeyPatch, + sqlite_session: Session, + unbound_session: Session, ) -> None: api = app_module.AppListApi() method = _unwrap(api.get) @@ -541,15 +551,20 @@ def test_app_list_uses_injected_session_for_draft_workflows( mode_compatible_with_agent="workflow", ) app_pagination = SimpleNamespace(page=1, per_page=20, total=1, has_next=False, items=[app_item]) - workflow = SimpleNamespace( + workflow = Workflow( id="workflow-1", + tenant_id="tenant-1", app_id="app-1", - walk_nodes=lambda: iter([("trigger-1", {"type": "trigger-webhook"})]), + type=WorkflowType.WORKFLOW, + version=Workflow.VERSION_DRAFT, + graph=json.dumps({"nodes": [{"id": "trigger-1", "data": {"type": "trigger-webhook"}}], "edges": []}), + features=json.dumps({}), + created_by="user-1", + environment_variables=[], + conversation_variables=[], ) - session = MagicMock() - session.execute.return_value.scalars.return_value.all.return_value = [workflow] - scoped_session = MagicMock() - scoped_session.execute.side_effect = AssertionError("db.session should not be used") + sqlite_session.add(workflow) + sqlite_session.commit() monkeypatch.setattr( app_module, @@ -578,20 +593,18 @@ def test_app_list_uses_injected_session_for_draft_workflows( "get", get_permissions, ) - monkeypatch.setattr(app_module, "db", SimpleNamespace(session=scoped_session)) + monkeypatch.setattr(app_module, "db", SimpleNamespace(session=unbound_session)) with app.test_request_context("/console/api/apps?page=1&limit=20", method="GET"): - response, status = method("tenant-1", "user-1", session) + response, status = method("tenant-1", "user-1", sqlite_session) assert status == 200 assert response["data"][0]["has_draft_trigger"] is True - session.execute.assert_called_once() - scoped_session.execute.assert_not_called() - get_permissions.assert_called_once_with("tenant-1", "user-1", session=session) + get_permissions.assert_called_once_with("tenant-1", "user-1", session=sqlite_session) assert response["data"][0]["permission_keys"] == ["app.acl.edit"] -def test_app_create_api_attaches_permission_keys(app, app_module): +def test_app_create_api_attaches_permission_keys(app, app_module, unbound_session: Session): method = app_module.AppListApi.post while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -637,7 +650,7 @@ def test_app_create_api_attaches_permission_keys(app, app_module): replace_whitelist, ) - resp, status = method(app_module.AppListApi(), MagicMock(), "tenant-1", SimpleNamespace(id="acct-1")) + resp, status = method(app_module.AppListApi(), unbound_session, "tenant-1", SimpleNamespace(id="acct-1")) assert status == 201 assert resp["permission_keys"] == ["app.acl.view_layout", "app.acl.edit"] @@ -645,7 +658,7 @@ def test_app_create_api_attaches_permission_keys(app, app_module): initialize_rbac_task.delay.assert_called_once_with("tenant-1", "acct-1", app_id="app-new") -def test_app_list_api_attaches_permission_keys(app, app_module): +def test_app_list_api_attaches_permission_keys(app, app_module, sqlite_session: Session): method = app_module.AppListApi.get while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -697,9 +710,7 @@ def test_app_list_api_attaches_permission_keys(app, app_module): lambda tenant_id, account_id: SimpleNamespace(unrestricted=True, resource_ids=[]), ) - session = MagicMock() - session.execute.return_value.scalars.return_value.all.return_value = [] - resp, status = method(app_module.AppListApi(), "tenant-1", "acct-1", session) + resp, status = method(app_module.AppListApi(), "tenant-1", "acct-1", sqlite_session) assert status == 200 params = get_paginate_apps.call_args.args[2] @@ -708,7 +719,7 @@ def test_app_list_api_attaches_permission_keys(app, app_module): assert resp["data"][0]["permission_keys"] == ["app.acl.view_layout", "app.acl.edit"] -def test_recent_app_list_api_returns_only_home_card_fields(app, app_module): +def test_recent_app_list_api_returns_only_home_card_fields(app, app_module, unbound_session: Session): method = app_module.RecentAppListApi.get while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -745,7 +756,7 @@ def test_recent_app_list_api_returns_only_home_card_fields(app, app_module): ), ) - resp, status = method(app_module.RecentAppListApi(), "tenant-1", "acct-1", MagicMock()) + resp, status = method(app_module.RecentAppListApi(), "tenant-1", "acct-1", unbound_session) assert status == 200 assert resp == { @@ -786,7 +797,7 @@ def test_recent_app_response_rejects_non_home_app_modes(app_module, mode: str) - ) -def test_recent_app_list_api_applies_rbac_visibility_filter(app, app_module): +def test_recent_app_list_api_applies_rbac_visibility_filter(app, app_module, unbound_session: Session): method = app_module.RecentAppListApi.get while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -814,7 +825,7 @@ def test_recent_app_list_api_applies_rbac_visibility_filter(app, app_module): ), ) - resp, status = method(app_module.RecentAppListApi(), "tenant-1", "acct-1", MagicMock()) + resp, status = method(app_module.RecentAppListApi(), "tenant-1", "acct-1", unbound_session) assert status == 200 assert resp == {"data": []} @@ -823,7 +834,9 @@ def test_recent_app_list_api_applies_rbac_visibility_filter(app, app_module): assert params.include_own_apps is True -def test_app_list_api_limits_to_apps_created_by_current_user_without_view_permission(app, app_module): +def test_app_list_api_limits_to_apps_created_by_current_user_without_view_permission( + app, app_module, unbound_session: Session +): method = app_module.AppListApi.get while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -855,8 +868,7 @@ def test_app_list_api_limits_to_apps_created_by_current_user_without_view_permis lambda: SimpleNamespace(webapp_auth=SimpleNamespace(enabled=False)), ) - session = MagicMock() - resp, status = method(app_module.AppListApi(), "tenant-1", "acct-1", session) + resp, status = method(app_module.AppListApi(), "tenant-1", "acct-1", unbound_session) assert status == 200 assert resp["data"] == [] @@ -866,7 +878,9 @@ def test_app_list_api_limits_to_apps_created_by_current_user_without_view_permis assert params.is_created_by_me is None -def test_app_list_api_limits_to_preview_overrides_without_manage_own_permission(app, app_module): +def test_app_list_api_limits_to_preview_overrides_without_manage_own_permission( + app, app_module, unbound_session: Session +): method = app_module.AppListApi.get while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -913,8 +927,7 @@ def test_app_list_api_limits_to_preview_overrides_without_manage_own_permission( lambda: SimpleNamespace(webapp_auth=SimpleNamespace(enabled=False)), ) - session = MagicMock() - method(app_module.AppListApi(), "tenant-1", "acct-1", session) + method(app_module.AppListApi(), "tenant-1", "acct-1", unbound_session) params = get_paginate_apps.call_args.args[2] assert params.accessible_app_ids == ["app-acl-shared", "app-full", "app-shared", "app-whitelist-only"] @@ -922,7 +935,9 @@ def test_app_list_api_limits_to_preview_overrides_without_manage_own_permission( assert params.is_created_by_me is None -def test_app_list_api_returns_no_apps_without_workspace_or_resource_view_permission(app, app_module): +def test_app_list_api_returns_no_apps_without_workspace_or_resource_view_permission( + app, app_module, unbound_session: Session +): method = app_module.AppListApi.get while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -950,8 +965,7 @@ def test_app_list_api_returns_no_apps_without_workspace_or_resource_view_permiss lambda: SimpleNamespace(webapp_auth=SimpleNamespace(enabled=False)), ) - session = MagicMock() - method(app_module.AppListApi(), "tenant-1", "acct-1", session) + method(app_module.AppListApi(), "tenant-1", "acct-1", unbound_session) params = get_paginate_apps.call_args.args[2] assert params.accessible_app_ids == ["app-not-permitted"] @@ -959,7 +973,7 @@ def test_app_list_api_returns_no_apps_without_workspace_or_resource_view_permiss assert params.is_created_by_me is None -def test_app_detail_api_attaches_current_user_permission_keys(app, app_module): +def test_app_detail_api_attaches_current_user_permission_keys(app, app_module, unbound_session: Session): method = app_module.AppApi.get while hasattr(method, "__wrapped__"): method = method.__wrapped__ @@ -1002,40 +1016,40 @@ def test_app_detail_api_attaches_current_user_permission_keys(app, app_module): get_permissions, ) - session = MagicMock() resp = method( app_module.AppApi(), - session, + unbound_session, "tenant-1", SimpleNamespace(id="acct-1"), app_model=app_obj, ) - get_app.assert_called_once_with(app_obj, session=session) - get_permissions.assert_called_once_with("tenant-1", "acct-1", app_id="app-1", session=session) + get_app.assert_called_once_with(app_obj, session=unbound_session) + get_permissions.assert_called_once_with("tenant-1", "acct-1", app_id="app-1", session=unbound_session) assert resp["permission_keys"] == ["app.acl.view_layout", "app.acl.edit", "app.acl.monitor"] -def test_app_copy_api_attaches_permission_keys(app, app_module): +def test_app_copy_api_attaches_permission_keys(app, app_module, sqlite_session: Session, sqlite_engine: Engine): method = app_module.AppCopyApi.post while hasattr(method, "__wrapped__"): method = method.__wrapped__ - app_obj = SimpleNamespace( - id="app-new", + app_obj = App( + id="00000000-0000-0000-0000-000000000101", + tenant_id="00000000-0000-0000-0000-000000000102", name="Copied App", description="Summary", - mode_compatible_with_agent="workflow", + mode=AppMode.WORKFLOW, + icon_type=IconType.EMOJI, + icon="copy", + icon_background="#ffffff", enable_site=True, enable_api=True, - permission_keys=[], ) + sqlite_session.add(app_obj) + sqlite_session.commit() - import_result = SimpleNamespace(status=app_module.ImportStatus.COMPLETED, app_id="app-new") - fake_session = MagicMock() - fake_session.__enter__.return_value = fake_session - fake_session.__exit__.return_value = None - fake_session.scalar.return_value = app_obj + import_result = SimpleNamespace(status=app_module.ImportStatus.COMPLETED, app_id=app_obj.id) with app.test_request_context("/apps/app-original/copy", method="POST", json={}): with pytest.MonkeyPatch.context() as monkeypatch: @@ -1053,16 +1067,11 @@ def test_app_copy_api_attaches_permission_keys(app, app_module): "get_system_features", lambda: SimpleNamespace(webapp_auth=SimpleNamespace(enabled=False)), ) - monkeypatch.setattr(app_module, "db", SimpleNamespace(engine=object(), session=lambda: MagicMock())) - monkeypatch.setattr( - app_module, - "Session", - lambda *_args, **_kwargs: fake_session, - ) + monkeypatch.setattr(app_module, "db", SimpleNamespace(engine=sqlite_engine)) monkeypatch.setattr( app_module.enterprise_rbac_service.RBACService.AppPermissions, "batch_get", - lambda tenant_id, account_id, app_ids, session: {"app-new": ["app.acl.view_layout", "app.acl.edit"]}, + lambda tenant_id, account_id, app_ids, session: {app_obj.id: ["app.acl.view_layout", "app.acl.edit"]}, ) resp, status = method( @@ -1073,5 +1082,5 @@ def test_app_copy_api_attaches_permission_keys(app, app_module): ) assert status == 201 - assert fake_session.scalar.called + assert sqlite_session.get(App, app_obj.id) is not None assert resp["permission_keys"] == ["app.acl.view_layout", "app.acl.edit"] diff --git a/api/tests/unit_tests/controllers/console/app/test_conversation_api.py b/api/tests/unit_tests/controllers/console/app/test_conversation_api.py index 1f2819e6d39..28cff1ba942 100644 --- a/api/tests/unit_tests/controllers/console/app/test_conversation_api.py +++ b/api/tests/unit_tests/controllers/console/app/test_conversation_api.py @@ -6,10 +6,13 @@ from unittest.mock import MagicMock import pytest from flask import Flask +from sqlalchemy.orm import Session from werkzeug.exceptions import BadRequest, NotFound from controllers.console.app import conversation as conversation_module -from models.model import AppMode +from core.app.entities.app_invoke_entities import InvokeFrom +from models.enums import ConversationFromSource +from models.model import AppMode, Conversation from services.errors.conversation import ConversationNotExistsError @@ -17,7 +20,32 @@ def _make_account(): return SimpleNamespace(timezone="UTC", id="u1") -def test_completion_conversation_list_returns_paginated_result(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def _conversation(*, conversation_id: str = "c1", app_id: str = "app-1") -> Conversation: + conversation = Conversation( + app_id=app_id, + app_model_config_id=None, + model_provider=None, + override_model_configs=None, + model_id=None, + mode=AppMode.CHAT, + name="Conversation", + inputs={}, + introduction="", + system_instruction="", + system_instruction_tokens=0, + status="normal", + invoke_from=InvokeFrom.EXPLORE, + from_source=ConversationFromSource.CONSOLE, + from_end_user_id=None, + from_account_id="u1", + ) + conversation.id = conversation_id + return conversation + + +def test_completion_conversation_list_returns_paginated_result( + app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: api = conversation_module.CompletionConversationApi() method = unwrap(api.get) account = _make_account() @@ -30,11 +58,13 @@ def test_completion_conversation_list_returns_paginated_result(app: Flask, monke paginate_result.items = [] monkeypatch.setattr(conversation_module, "paginate_query", lambda *_args, **_kwargs: paginate_result) with app.test_request_context("/console/api/apps/app-1/completion-conversations", method="GET"): - response = method(api, MagicMock(), account, app_model=SimpleNamespace(id="app-1")) + response = method(api, unbound_session, account, app_model=SimpleNamespace(id="app-1")) assert response == {"page": 1, "limit": 20, "total": 0, "has_more": False, "data": []} -def test_completion_conversation_list_invalid_time_range(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def test_completion_conversation_list_invalid_time_range( + app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: api = conversation_module.CompletionConversationApi() method = unwrap(api.get) account = _make_account() @@ -47,10 +77,12 @@ def test_completion_conversation_list_invalid_time_range(app: Flask, monkeypatch "/console/api/apps/app-1/completion-conversations", method="GET", query_string={"start": "bad"} ): with pytest.raises(BadRequest): - method(api, MagicMock(), account, app_model=SimpleNamespace(id="app-1")) + method(api, unbound_session, account, app_model=SimpleNamespace(id="app-1")) -def test_chat_conversation_list_advanced_chat_calls_paginate(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: +def test_chat_conversation_list_advanced_chat_calls_paginate( + app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: api = conversation_module.ChatConversationApi() method = unwrap(api.get) account = _make_account() @@ -63,30 +95,31 @@ def test_chat_conversation_list_advanced_chat_calls_paginate(app: Flask, monkeyp paginate_result.items = [] monkeypatch.setattr(conversation_module, "paginate_query", lambda *_args, **_kwargs: paginate_result) with app.test_request_context("/console/api/apps/app-1/chat-conversations", method="GET"): - response = method(api, MagicMock(), account, app_model=SimpleNamespace(id="app-1", mode=AppMode.ADVANCED_CHAT)) + response = method( + api, unbound_session, account, app_model=SimpleNamespace(id="app-1", mode=AppMode.ADVANCED_CHAT) + ) assert response == {"page": 1, "limit": 20, "total": 0, "has_more": False, "data": []} -def test_get_conversation_updates_read_at(monkeypatch: pytest.MonkeyPatch) -> None: - conversation = SimpleNamespace(id="c1", app_id="app-1") - session = MagicMock() - session.scalar.return_value = conversation +def test_get_conversation_updates_read_at(sqlite_session: Session) -> None: + conversation = _conversation() + sqlite_session.add(conversation) + sqlite_session.flush() + session = sqlite_session result = conversation_module._get_conversation(session, _make_account(), SimpleNamespace(id="app-1"), "c1") assert result is conversation - session.execute.assert_called_once() - session.flush.assert_called_once() - session.refresh.assert_called_once_with(conversation) + assert conversation.read_at is not None + assert conversation.read_account_id == "u1" -def test_get_conversation_missing_raises_not_found(monkeypatch: pytest.MonkeyPatch) -> None: - session = MagicMock() - session.scalar.return_value = None +def test_get_conversation_missing_raises_not_found(sqlite_session: Session) -> None: + session = sqlite_session with pytest.raises(NotFound): conversation_module._get_conversation(session, _make_account(), SimpleNamespace(id="app-1"), "missing") -def test_conversation_response_source_uses_caller_session() -> None: - session = MagicMock() +def test_conversation_response_source_uses_caller_session(unbound_session: Session) -> None: + session = unbound_session account = object() annotation = MagicMock() annotation.account_with_session.return_value = account @@ -136,7 +169,9 @@ def test_conversation_response_source_uses_caller_session() -> None: annotation.account_with_session.assert_called_once_with(session=session) -def test_completion_conversation_delete_maps_not_found(monkeypatch: pytest.MonkeyPatch) -> None: +def test_completion_conversation_delete_maps_not_found( + monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: api = conversation_module.CompletionConversationDetailApi() method = unwrap(api.delete) monkeypatch.setattr( @@ -144,6 +179,6 @@ def test_completion_conversation_delete_maps_not_found(monkeypatch: pytest.Monke "delete", lambda *_args, **_kwargs: (_ for _ in ()).throw(ConversationNotExistsError()), ) - session = MagicMock() + session = unbound_session with pytest.raises(NotFound): method(api, session, _make_account(), app_model=SimpleNamespace(id="app-1"), conversation_id="c1") diff --git a/api/tests/unit_tests/controllers/console/app/test_conversation_variables_api.py b/api/tests/unit_tests/controllers/console/app/test_conversation_variables_api.py index c24fff392dc..688309b0281 100644 --- a/api/tests/unit_tests/controllers/console/app/test_conversation_variables_api.py +++ b/api/tests/unit_tests/controllers/console/app/test_conversation_variables_api.py @@ -16,7 +16,6 @@ from graphon.variables.types import SegmentType from models import ConversationVariable -@pytest.mark.parametrize("sqlite_session", [(ConversationVariable,)], indirect=True) def test_get_conversation_variables_returns_paginated_response( app: Flask, monkeypatch: pytest.MonkeyPatch, diff --git a/api/tests/unit_tests/controllers/console/app/test_mcp_server_response.py b/api/tests/unit_tests/controllers/console/app/test_mcp_server_response.py index e9b01c37872..a6568d5932a 100644 --- a/api/tests/unit_tests/controllers/console/app/test_mcp_server_response.py +++ b/api/tests/unit_tests/controllers/console/app/test_mcp_server_response.py @@ -129,7 +129,6 @@ class TestAppMCPServerResponse: class TestAppMCPServerController: - @pytest.mark.parametrize("sqlite_session", [(AppMCPServer,)], indirect=True) def test_get_returns_empty_dict_when_server_missing(self, sqlite_session: Session) -> None: api = AppMCPServerController() method = unwrap(api.get) @@ -139,7 +138,6 @@ class TestAppMCPServerController: assert response == {} - @pytest.mark.parametrize("sqlite_session", [(AppMCPServer,)], indirect=True) def test_post_returns_201(self, sqlite_session: Session) -> None: api = AppMCPServerController() method = unwrap(api.post) @@ -163,7 +161,6 @@ class TestAppMCPServerController: assert response["parameters"] == {"timeout": 30} assert status_code == 201 - @pytest.mark.parametrize("sqlite_session", [(AppMCPServer,)], indirect=True) def test_put_updates_server_for_app(self, sqlite_session: Session) -> None: api = AppMCPServerController() method = unwrap(api.put) @@ -193,7 +190,6 @@ class TestAppMCPServerController: assert response["id"] == "server-1" assert updated_server.description == "Updated" - @pytest.mark.parametrize("sqlite_session", [(AppMCPServer,)], indirect=True) @pytest.mark.parametrize( ("foreign_tenant_id", "foreign_app_id"), [ diff --git a/api/tests/unit_tests/controllers/console/app/test_message_api.py b/api/tests/unit_tests/controllers/console/app/test_message_api.py index 0c2efc12198..ae6e994b6d5 100644 --- a/api/tests/unit_tests/controllers/console/app/test_message_api.py +++ b/api/tests/unit_tests/controllers/console/app/test_message_api.py @@ -7,12 +7,67 @@ from unittest.mock import MagicMock import pytest from flask import Flask +from sqlalchemy import event +from sqlalchemy.orm import Session from controllers.console.app import message as message_module +from core.app.entities.app_invoke_entities import InvokeFrom +from models.enums import ConversationFromSource +from models.model import AppMode, Conversation, Message -def test_app_message_routes_pass_injected_session(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: - session = MagicMock() +def _persist_message(session: Session, *, message_id: str, app_id: str = "app-1") -> Message: + conversation = Conversation( + app_id=app_id, + app_model_config_id=None, + model_provider=None, + override_model_configs=None, + model_id=None, + mode=AppMode.CHAT, + name="Conversation", + inputs={}, + introduction="", + system_instruction="", + system_instruction_tokens=0, + status="normal", + invoke_from=InvokeFrom.DEBUGGER, + from_source=ConversationFromSource.CONSOLE, + from_end_user_id=None, + from_account_id="account-1", + ) + conversation.id = "conversation-1" + message = Message( + app_id=app_id, + conversation_id=conversation.id, + inputs={}, + query="query", + message="", + message_tokens=0, + message_unit_price=0, + message_price_unit=0, + answer="answer", + answer_tokens=0, + answer_unit_price=0, + answer_price_unit=0, + provider_response_latency=0, + total_price=0, + currency="USD", + invoke_from=InvokeFrom.DEBUGGER, + from_source=ConversationFromSource.CONSOLE, + from_end_user_id=None, + from_account_id="account-1", + app_mode=AppMode.CHAT, + ) + message.id = message_id + session.add_all([conversation, message]) + session.flush() + return message + + +def test_app_message_routes_pass_injected_session( + app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: + session = unbound_session current_user = SimpleNamespace(id="account-1") app_model = SimpleNamespace(id="app-1", mode="chat") message_id = "550e8400-e29b-41d4-a716-446655440000" @@ -44,17 +99,15 @@ def test_app_message_routes_pass_injected_session(app: Flask, monkeypatch: pytes assert get_message_detail.call_args.kwargs["session"] is session -def test_update_message_feedback_commits_injected_session(app: Flask) -> None: +def test_update_message_feedback_commits_injected_session(app: Flask, sqlite_session: Session) -> None: message_id = "550e8400-e29b-41d4-a716-446655440000" feedback = SimpleNamespace(rating="dislike", content=None) get_admin_feedback = MagicMock(return_value=feedback) - message = SimpleNamespace( - id=message_id, - conversation_id="conversation-1", - admin_feedback_with_session=get_admin_feedback, - ) - session = MagicMock() - session.scalar.return_value = message + message = _persist_message(sqlite_session, message_id=message_id) + message.admin_feedback_with_session = get_admin_feedback + session = sqlite_session + commits: list[str] = [] + event.listen(session, "after_commit", lambda _session: commits.append("commit")) with app.test_request_context(json={"message_id": message_id, "rating": "like", "content": "helpful"}): result = message_module._update_message_feedback( @@ -67,16 +120,15 @@ def test_update_message_feedback_commits_injected_session(app: Flask) -> None: assert feedback.rating == "like" assert feedback.content == "helpful" get_admin_feedback.assert_called_once_with(session=session) - session.commit.assert_called_once_with() + assert commits == ["commit"] -def test_get_message_detail_uses_injected_session(monkeypatch: pytest.MonkeyPatch) -> None: +def test_get_message_detail_uses_injected_session(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session) -> None: message_id = "550e8400-e29b-41d4-a716-446655440000" - message = SimpleNamespace(id=message_id) + message = _persist_message(sqlite_session, message_id=message_id) response_source = object() response_source_factory = MagicMock(return_value=response_source) - session = MagicMock() - session.scalar.return_value = message + session = sqlite_session monkeypatch.setattr(message_module, "attach_message_extra_contents", MagicMock()) monkeypatch.setattr(message_module, "MessageResponseSource", response_source_factory) monkeypatch.setattr(message_module, "dump_response", lambda _model, value: value) @@ -89,11 +141,10 @@ def test_get_message_detail_uses_injected_session(monkeypatch: pytest.MonkeyPatc assert result is response_source response_source_factory.assert_called_once_with(message, session=session) - session.scalar.assert_called_once() -def test_message_response_source_uses_caller_session_for_nested_fields() -> None: - session = MagicMock() +def test_message_response_source_uses_caller_session_for_nested_fields(unbound_session: Session) -> None: + session = unbound_session account = object() feedback = MagicMock() feedback.from_account_with_session.return_value = account diff --git a/api/tests/unit_tests/controllers/console/app/test_workflow.py b/api/tests/unit_tests/controllers/console/app/test_workflow.py index 02e9ffefdf5..82d6add152b 100644 --- a/api/tests/unit_tests/controllers/console/app/test_workflow.py +++ b/api/tests/unit_tests/controllers/console/app/test_workflow.py @@ -724,6 +724,7 @@ def test_advanced_chat_run_conversation_not_exists(app: Flask, monkeypatch: pyte def test_trigger_run_loads_draft_with_request_session( app: Flask, monkeypatch: pytest.MonkeyPatch, + unbound_session: Session, resource: type, payload: dict[str, object], ) -> None: @@ -733,7 +734,7 @@ def test_trigger_run_loads_draft_with_request_session( "WorkflowService", lambda: SimpleNamespace(get_draft_workflow=get_draft_workflow), ) - session = Mock() + session = unbound_session app_model = SimpleNamespace(id="app-1") handler = inspect.unwrap(resource.post) diff --git a/api/tests/unit_tests/controllers/console/app/test_wraps.py b/api/tests/unit_tests/controllers/console/app/test_wraps.py index a92a9dee433..2f94aedaf52 100644 --- a/api/tests/unit_tests/controllers/console/app/test_wraps.py +++ b/api/tests/unit_tests/controllers/console/app/test_wraps.py @@ -7,7 +7,6 @@ from unittest.mock import MagicMock from uuid import uuid4 import pytest -from sqlalchemy import Select from sqlalchemy.orm import Session from controllers.common import session as session_module @@ -33,7 +32,6 @@ def _persist_app(sqlite_session: Session, *, mode: AppMode = AppMode.CHAT) -> Ap return app_model -@pytest.mark.parametrize("sqlite_session", [(App,)], indirect=True) def test_get_app_model_injects_model(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session) -> None: app_model = _persist_app(sqlite_session) monkeypatch.setattr(wraps_module, "current_account_with_tenant", lambda: (None, app_model.tenant_id)) @@ -46,7 +44,6 @@ def test_get_app_model_injects_model(monkeypatch: pytest.MonkeyPatch, sqlite_ses assert handler(app_id=app_model.id) == app_model.id -@pytest.mark.parametrize("sqlite_session", [(App,)], indirect=True) def test_get_app_model_rejects_wrong_mode(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session) -> None: app_model = _persist_app(sqlite_session) monkeypatch.setattr(wraps_module, "current_account_with_tenant", lambda: (None, app_model.tenant_id)) @@ -60,17 +57,10 @@ def test_get_app_model_rejects_wrong_mode(monkeypatch: pytest.MonkeyPatch, sqlit handler(app_id=app_model.id) -def test_get_app_model_with_trial_requires_trial_app_registration(monkeypatch: pytest.MonkeyPatch) -> None: - app_model = SimpleNamespace(id="app-1", mode=AppMode.CHAT.value, status="normal", tenant_id="t1") - session = MagicMock(spec=Session) - - def scalar(statement: Select[tuple[App]]) -> object | None: - has_trial_app_join = any( - from_clause.is_derived_from(TrialApp.__table__) for from_clause in statement.get_final_froms() - ) - return None if has_trial_app_join else app_model - - monkeypatch.setattr(session, "scalar", scalar) +def test_get_app_model_with_trial_requires_trial_app_registration( + monkeypatch: pytest.MonkeyPatch, sqlite_session: Session +) -> None: + app_model = _persist_app(sqlite_session) recommended_get_app = MagicMock(return_value=None) monkeypatch.setattr(wraps_module.RecommendedAppService, "get_app", recommended_get_app) @@ -80,14 +70,15 @@ def test_get_app_model_with_trial_requires_trial_app_registration(monkeypatch: p return app_model.id with pytest.raises(AppNotFoundError): - Handler().get(session, app_id="app-1") + Handler().get(sqlite_session, app_id=app_model.id) - recommended_get_app.assert_called_once_with("app-1", session=session) + recommended_get_app.assert_called_once_with(app_model.id, session=sqlite_session) -def test_get_app_model_with_trial_falls_back_to_recommended_app(monkeypatch: pytest.MonkeyPatch) -> None: +def test_get_app_model_with_trial_falls_back_to_recommended_app( + monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: app_model = SimpleNamespace(id="app-1", mode=AppMode.CHAT.value, status="normal", tenant_id="t1") - session = MagicMock(spec=Session) trial_app_loader = MagicMock(return_value=None) recommended_get_app = MagicMock(return_value=app_model) monkeypatch.setattr(wraps_module, "_load_app_model_with_trial", trial_app_loader) @@ -98,14 +89,15 @@ def test_get_app_model_with_trial_falls_back_to_recommended_app(monkeypatch: pyt def get(self, _injected_session, app_model): return app_model.id - assert Handler().get(session, app_id="app-1") == "app-1" - trial_app_loader.assert_called_once_with(session, "app-1") - recommended_get_app.assert_called_once_with("app-1", session=session) + assert Handler().get(unbound_session, app_id="app-1") == "app-1" + trial_app_loader.assert_called_once_with(unbound_session, "app-1") + recommended_get_app.assert_called_once_with("app-1", session=unbound_session) -def test_get_app_model_with_trial_prefers_trial_registration(monkeypatch: pytest.MonkeyPatch) -> None: +def test_get_app_model_with_trial_prefers_trial_registration( + monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: app_model = SimpleNamespace(id="app-1", mode=AppMode.CHAT.value, status="normal", tenant_id="t1") - session = MagicMock(spec=Session) trial_app_loader = MagicMock(return_value=app_model) recommended_get_app = MagicMock() monkeypatch.setattr(wraps_module, "_load_app_model_with_trial", trial_app_loader) @@ -116,8 +108,8 @@ def test_get_app_model_with_trial_prefers_trial_registration(monkeypatch: pytest def get(self, _injected_session, app_model): return app_model.id - assert Handler().get(session, app_id="app-1") == "app-1" - trial_app_loader.assert_called_once_with(session, "app-1") + assert Handler().get(unbound_session, app_id="app-1") == "app-1" + trial_app_loader.assert_called_once_with(unbound_session, "app-1") recommended_get_app.assert_not_called() @@ -134,7 +126,6 @@ def test_wraps_with_session_reexports_common_session_decorator() -> None: assert wraps_module.with_session is with_session -@pytest.mark.parametrize("sqlite_session", [(App,)], indirect=True) def test_get_app_model_prefers_injected_session( monkeypatch: pytest.MonkeyPatch, sqlite_session: Session, @@ -154,26 +145,27 @@ def test_get_app_model_prefers_injected_session( assert Handler().get(sqlite_session, app_id=app_model.id) == app_model.id -def test_get_app_model_with_trial_prefers_injected_session(monkeypatch: pytest.MonkeyPatch) -> None: - app_model = SimpleNamespace(id="app-1", mode=AppMode.CHAT.value, status="normal") - session = MagicMock(spec=Session) - session.scalar.return_value = app_model +def test_get_app_model_with_trial_prefers_injected_session( + monkeypatch: pytest.MonkeyPatch, sqlite_session: Session +) -> None: + app_model = _persist_app(sqlite_session) + sqlite_session.add(TrialApp(app_id=app_model.id, tenant_id=app_model.tenant_id)) + sqlite_session.commit() monkeypatch.setattr( wraps_module.db, "session", SimpleNamespace(scalar=lambda *_args, **_kwargs: pytest.fail("db.session should not be used")), ) - monkeypatch.setattr(session_module.session_factory, "create_session", lambda: nullcontext(session)) + monkeypatch.setattr(session_module.session_factory, "create_session", lambda: nullcontext(sqlite_session)) class Handler: @with_session(write=False) @wraps_module.get_app_model_with_trial(None) def get(self, injected_session, app_model): - assert injected_session is session + assert injected_session is sqlite_session return app_model.id - assert Handler().get(app_id="app-1") == "app-1" - session.scalar.assert_called_once() + assert Handler().get(app_id=app_model.id) == app_model.id def test_get_app_model_with_trial_requires_injected_session() -> None: