diff --git a/api/tests/unit_tests/services/test_conversation_service.py b/api/tests/unit_tests/services/test_conversation_service.py index e6f7b48f651..90727705061 100644 --- a/api/tests/unit_tests/services/test_conversation_service.py +++ b/api/tests/unit_tests/services/test_conversation_service.py @@ -3,204 +3,156 @@ Comprehensive unit tests for ConversationService. This file provides complete test coverage for all ConversationService methods. Tests are organized by functionality and include edge cases, error handling, -and both positive and negative test scenarios. +and both positive and negative test scenarios. Database paths use isolated +in-memory SQLite sessions with persisted ORM rows. """ -from unittest.mock import MagicMock, Mock, create_autospec, patch +import json +from unittest.mock import patch +import pytest from sqlalchemy import asc, desc +from sqlalchemy.orm import Session from core.app.entities.app_invoke_entities import InvokeFrom from libs.datetime_utils import naive_utc_now from models import Account, ConversationVariable -from models.model import App, Conversation, EndUser, Message +from models.enums import AppStatus, ConversationFromSource, ConversationStatus +from models.model import App, AppMode, Conversation from services.conversation_service import ConversationService +TENANT_ID = "11111111-1111-1111-1111-111111111111" +APP_ID = "22222222-2222-2222-2222-222222222222" +ACCOUNT_ID = "33333333-3333-3333-3333-333333333333" +CONVERSATION_ID = "44444444-4444-4444-4444-444444444444" +VARIABLE_ID = "55555555-5555-5555-5555-555555555555" +OTHER_VARIABLE_ID = "66666666-6666-6666-6666-666666666666" +OTHER_APP_ID = "77777777-7777-7777-7777-777777777777" +OTHER_CONVERSATION_ID = "88888888-8888-8888-8888-888888888888" +OTHER_APP_VARIABLE_ID = "99999999-9999-9999-9999-999999999999" +OTHER_CONVERSATION_VARIABLE_ID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + + +def _conversation_variable( + *, + variable_id: str, + name: str, + value: str, + conversation_id: str = CONVERSATION_ID, + app_id: str = APP_ID, +) -> ConversationVariable: + return ConversationVariable( + id=variable_id, + conversation_id=conversation_id, + app_id=app_id, + data=json.dumps( + { + "id": variable_id, + "name": name, + "value_type": "string", + "value": value, + } + ), + ) + class ConversationServiceTestDataFactory: """ - Factory for creating test data and mock objects. + Factory for creating test ORM objects. - Provides reusable methods to create consistent mock objects for testing + Provides reusable methods to create consistent model objects for testing conversation-related operations. """ @staticmethod - def create_account_mock(account_id: str = "account-123", **kwargs) -> Mock: + def create_account(account_id: str = ACCOUNT_ID, **kwargs) -> Account: """ - Create a mock Account object. + Create an Account object. Args: account_id: Unique identifier for the account - **kwargs: Additional attributes to set on the mock + **kwargs: Additional attributes to set on the model Returns: - Mock Account object with specified attributes + Account object with specified attributes """ - account = create_autospec(Account, instance=True) + account = Account(name="Test User", email="test@example.com") account.id = account_id for key, value in kwargs.items(): setattr(account, key, value) return account @staticmethod - def create_end_user_mock(user_id: str = "user-123", **kwargs) -> Mock: + def create_app(app_id: str = APP_ID, tenant_id: str = TENANT_ID, **kwargs) -> App: """ - Create a mock EndUser object. - - Args: - user_id: Unique identifier for the end user - **kwargs: Additional attributes to set on the mock - - Returns: - Mock EndUser object with specified attributes - """ - user = create_autospec(EndUser, instance=True) - user.id = user_id - for key, value in kwargs.items(): - setattr(user, key, value) - return user - - @staticmethod - def create_app_mock(app_id: str = "app-123", tenant_id: str = "tenant-123", **kwargs) -> Mock: - """ - Create a mock App object. + Create an App object. Args: app_id: Unique identifier for the app tenant_id: Tenant/workspace identifier - **kwargs: Additional attributes to set on the mock + **kwargs: Additional attributes to set on the model Returns: - Mock App object with specified attributes + App object with specified attributes """ - app = create_autospec(App, instance=True) - app.id = app_id - app.tenant_id = tenant_id - app.name = kwargs.get("name", "Test App") - app.mode = kwargs.get("mode", "chat") - app.status = kwargs.get("status", "normal") + app = App( + id=app_id, + tenant_id=tenant_id, + name=kwargs.get("name", "Test App"), + mode=kwargs.get("mode", AppMode.CHAT), + status=kwargs.get("status", AppStatus.NORMAL), + description="", + enable_site=False, + enable_api=False, + max_active_requests=None, + ) for key, value in kwargs.items(): setattr(app, key, value) return app @staticmethod - def create_conversation_mock( - conversation_id: str = "conv-123", - app_id: str = "app-123", - from_source: str = "console", + def create_conversation( + conversation_id: str = CONVERSATION_ID, + app_id: str = APP_ID, + from_source: ConversationFromSource = ConversationFromSource.CONSOLE, **kwargs, - ) -> Mock: + ) -> Conversation: """ - Create a mock Conversation object. + Create a Conversation object. Args: conversation_id: Unique identifier for the conversation app_id: Associated app identifier from_source: Source of conversation ('console' or 'api') - **kwargs: Additional attributes to set on the mock + **kwargs: Additional attributes to set on the model Returns: - Mock Conversation object with specified attributes + Conversation object with specified attributes """ - conversation = create_autospec(Conversation, instance=True) - conversation.id = conversation_id - conversation.app_id = app_id - conversation.from_source = from_source - conversation.from_end_user_id = kwargs.get("from_end_user_id") - conversation.from_account_id = kwargs.get("from_account_id") - conversation.is_deleted = kwargs.get("is_deleted", False) - conversation.name = kwargs.get("name", "Test Conversation") - conversation.status = kwargs.get("status", "normal") - conversation.created_at = kwargs.get("created_at", naive_utc_now()) - conversation.updated_at = kwargs.get("updated_at", naive_utc_now()) + conversation = Conversation( + id=conversation_id, + app_id=app_id, + mode=AppMode.CHAT, + name=kwargs.get("name", "Test Conversation"), + status=kwargs.get("status", ConversationStatus.NORMAL), + from_source=from_source, + from_end_user_id=kwargs.get("from_end_user_id"), + from_account_id=kwargs.get("from_account_id", ACCOUNT_ID), + is_deleted=kwargs.get("is_deleted", False), + created_at=kwargs.get("created_at", naive_utc_now()), + updated_at=kwargs.get("updated_at", naive_utc_now()), + ) + conversation._inputs = {} for key, value in kwargs.items(): setattr(conversation, key, value) return conversation - @staticmethod - def create_message_mock( - message_id: str = "msg-123", - conversation_id: str = "conv-123", - app_id: str = "app-123", - **kwargs, - ) -> Mock: - """ - Create a mock Message object. - - Args: - message_id: Unique identifier for the message - conversation_id: Associated conversation identifier - app_id: Associated app identifier - **kwargs: Additional attributes to set on the mock - - Returns: - Mock Message object with specified attributes - """ - message = create_autospec(Message, instance=True) - message.id = message_id - message.conversation_id = conversation_id - message.app_id = app_id - message.query = kwargs.get("query", "Test message content") - message.created_at = kwargs.get("created_at", naive_utc_now()) - for key, value in kwargs.items(): - setattr(message, key, value) - return message - - @staticmethod - def create_conversation_variable_mock( - variable_id: str = "var-123", - conversation_id: str = "conv-123", - app_id: str = "app-123", - **kwargs, - ) -> Mock: - """ - Create a mock ConversationVariable object. - - Args: - variable_id: Unique identifier for the variable - conversation_id: Associated conversation identifier - app_id: Associated app identifier - **kwargs: Additional attributes to set on the mock - - Returns: - Mock ConversationVariable object with specified attributes - """ - variable = create_autospec(ConversationVariable, instance=True) - variable.id = variable_id - variable.conversation_id = conversation_id - variable.app_id = app_id - variable.data = {"name": kwargs.get("name", "test_var"), "value": kwargs.get("value", "test_value")} - variable.created_at = kwargs.get("created_at", naive_utc_now()) - variable.updated_at = kwargs.get("updated_at", naive_utc_now()) - - # Mock to_variable method - mock_variable = Mock() - mock_variable.id = variable_id - mock_variable.name = kwargs.get("name", "test_var") - mock_variable.value_type = kwargs.get("value_type", "string") - mock_variable.value = kwargs.get("value", "test_value") - mock_variable.description = kwargs.get("description", "") - mock_variable.selector = kwargs.get("selector", {}) - mock_variable.model_dump.return_value = { - "id": variable_id, - "name": kwargs.get("name", "test_var"), - "value_type": kwargs.get("value_type", "string"), - "value": kwargs.get("value", "test_value"), - "description": kwargs.get("description", ""), - "selector": kwargs.get("selector", {}), - } - variable.to_variable.return_value = mock_variable - - for key, value in kwargs.items(): - setattr(variable, key, value) - return variable - +@pytest.mark.parametrize("sqlite_session", [(Conversation,)], indirect=True) class TestConversationServicePagination: """Test conversation pagination operations.""" - def test_pagination_with_empty_include_ids(self): + def test_pagination_with_empty_include_ids(self, sqlite_session: Session): """ Test that empty include_ids returns empty result. @@ -208,15 +160,14 @@ class TestConversationServicePagination: and return empty results without querying the database. """ # Arrange - Set up test data - mock_session = MagicMock() # Mock database session - mock_app_model = ConversationServiceTestDataFactory.create_app_mock() - mock_user = ConversationServiceTestDataFactory.create_account_mock() + app_model = ConversationServiceTestDataFactory.create_app() + user = ConversationServiceTestDataFactory.create_account() # Act - Call the service method with empty include_ids result = ConversationService.pagination_by_last_id( - session=mock_session, - app_model=mock_app_model, - user=mock_user, + session=sqlite_session, + app_model=app_model, + user=user, last_id=None, limit=20, invoke_from=InvokeFrom.WEB_APP, @@ -228,21 +179,21 @@ class TestConversationServicePagination: assert result.data == [] # No conversations returned assert result.has_more is False # No more pages available assert result.limit == 20 # Limit preserved in response + assert not sqlite_session.in_transaction() - def test_pagination_returns_empty_when_user_is_none(self): + def test_pagination_returns_empty_when_user_is_none(self, sqlite_session: Session): """ Test that pagination returns empty result when user is None. This ensures proper handling of unauthenticated requests. """ # Arrange - mock_session = MagicMock() - mock_app_model = ConversationServiceTestDataFactory.create_app_mock() + app_model = ConversationServiceTestDataFactory.create_app() # Act result = ConversationService.pagination_by_last_id( - session=mock_session, - app_model=mock_app_model, + session=sqlite_session, + app_model=app_model, user=None, # No user provided last_id=None, limit=20, @@ -253,6 +204,7 @@ class TestConversationServicePagination: assert result.data == [] assert result.has_more is False assert result.limit == 20 + assert not sqlite_session.in_transaction() class TestConversationServiceHelpers: @@ -291,14 +243,14 @@ class TestConversationServiceHelpers: Should create a less-than filter condition. """ # Arrange - mock_conversation = ConversationServiceTestDataFactory.create_conversation_mock() - mock_conversation.updated_at = naive_utc_now() + conversation = ConversationServiceTestDataFactory.create_conversation() + conversation.updated_at = naive_utc_now() # Act condition = ConversationService._build_filter_condition( sort_field="updated_at", sort_direction=desc, - reference_conversation=mock_conversation, + reference_conversation=conversation, ) # Assert @@ -312,14 +264,14 @@ class TestConversationServiceHelpers: Should create a greater-than filter condition. """ # Arrange - mock_conversation = ConversationServiceTestDataFactory.create_conversation_mock() - mock_conversation.created_at = naive_utc_now() + conversation = ConversationServiceTestDataFactory.create_conversation() + conversation.created_at = naive_utc_now() # Act condition = ConversationService._build_filter_condition( sort_field="created_at", sort_direction=asc, - reference_conversation=mock_conversation, + reference_conversation=conversation, ) # Assert @@ -330,36 +282,71 @@ class TestConversationServiceHelpers: class TestConversationServiceConversationalVariable: """Test conversational variable operations.""" - @patch("services.conversation_service.ConversationService.get_conversation") + @pytest.mark.parametrize("sqlite_session", [(Conversation, ConversationVariable)], indirect=True) @patch("services.conversation_service.dify_config") - def test_get_conversational_variable_with_name_filter_mysql(self, mock_config, mock_get_conversation): + def test_get_conversational_variable_with_name_filter_mysql( + self, + mock_config, + sqlite_session: Session, + ): """ Test variable filtering by name for MySQL databases. Should apply JSON extraction filter for variable names. """ # Arrange - app_model = ConversationServiceTestDataFactory.create_app_mock() - user = ConversationServiceTestDataFactory.create_account_mock() - conversation = ConversationServiceTestDataFactory.create_conversation_mock() - - mock_get_conversation.return_value = conversation + app_model = ConversationServiceTestDataFactory.create_app() + user = ConversationServiceTestDataFactory.create_account() + conversation = ConversationServiceTestDataFactory.create_conversation() + matching_variable = _conversation_variable( + variable_id=VARIABLE_ID, + name="test_var", + value="matching", + ) + other_variable = _conversation_variable( + variable_id=OTHER_VARIABLE_ID, + name="unrelated", + value="excluded", + ) + other_app_variable = _conversation_variable( + variable_id=OTHER_APP_VARIABLE_ID, + name="test_var", + value="other-app", + app_id=OTHER_APP_ID, + ) + other_conversation_variable = _conversation_variable( + variable_id=OTHER_CONVERSATION_VARIABLE_ID, + name="test_var", + value="other-conversation", + conversation_id=OTHER_CONVERSATION_ID, + ) + sqlite_session.add_all( + [ + conversation, + matching_variable, + other_variable, + other_app_variable, + other_conversation_variable, + ] + ) + sqlite_session.commit() mock_config.DB_TYPE = "mysql" - # Mock session - mock_session = MagicMock() - mock_session.scalars.return_value.all.return_value = [] - # Act - ConversationService.get_conversational_variable( + result = ConversationService.get_conversational_variable( app_model=app_model, - conversation_id="conv-123", + conversation_id=CONVERSATION_ID, user=user, limit=10, last_id=None, variable_name="test_var", - session=mock_session, + session=sqlite_session, ) - # Assert - JSON filter should be applied - assert mock_session.scalars.called + # Assert - SQLite executes the MySQL-compatible JSON extraction boundary. + assert result.has_more is False + assert result.limit == 10 + assert len(result.data) == 1 + assert result.data[0]["id"] == VARIABLE_ID + assert result.data[0]["name"] == "test_var" + assert result.data[0]["value"] == "matching"