test: use sqlite3 session in test_conversation_service (#38774)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

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.
+        Create an App 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.
 
         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 = App(
-        app.id = app_id
+            id=app_id,
-        app.tenant_id = tenant_id
+            tenant_id=tenant_id,
-        app.name = kwargs.get("name", "Test App")
+            name=kwargs.get("name", "Test App"),
-        app.mode = kwargs.get("mode", "chat")
+            mode=kwargs.get("mode", AppMode.CHAT),
-        app.status = kwargs.get("status", "normal")
+            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(
+    def create_conversation(
-        conversation_id: str = "conv-123",
+        conversation_id: str = CONVERSATION_ID,
-        app_id: str = "app-123",
+        app_id: str = APP_ID,
-        from_source: str = "console",
+        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 = Conversation(
-        conversation.id = conversation_id
+            id=conversation_id,
-        conversation.app_id = app_id
+            app_id=app_id,
-        conversation.from_source = from_source
+            mode=AppMode.CHAT,
-        conversation.from_end_user_id = kwargs.get("from_end_user_id")
+            name=kwargs.get("name", "Test Conversation"),
-        conversation.from_account_id = kwargs.get("from_account_id")
+            status=kwargs.get("status", ConversationStatus.NORMAL),
-        conversation.is_deleted = kwargs.get("is_deleted", False)
+            from_source=from_source,
-        conversation.name = kwargs.get("name", "Test Conversation")
+            from_end_user_id=kwargs.get("from_end_user_id"),
-        conversation.status = kwargs.get("status", "normal")
+            from_account_id=kwargs.get("from_account_id", ACCOUNT_ID),
-        conversation.created_at = kwargs.get("created_at", naive_utc_now())
+            is_deleted=kwargs.get("is_deleted", False),
-        conversation.updated_at = kwargs.get("updated_at", naive_utc_now())
+            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
+        app_model = ConversationServiceTestDataFactory.create_app()
-        mock_app_model = ConversationServiceTestDataFactory.create_app_mock()
+        user = ConversationServiceTestDataFactory.create_account()
-        mock_user = ConversationServiceTestDataFactory.create_account_mock()
 
         # Act - Call the service method with empty include_ids
         result = ConversationService.pagination_by_last_id(
-            session=mock_session,
+            session=sqlite_session,
-            app_model=mock_app_model,
+            app_model=app_model,
-            user=mock_user,
+            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()
+        app_model = ConversationServiceTestDataFactory.create_app()
-        mock_app_model = ConversationServiceTestDataFactory.create_app_mock()
 
         # Act
         result = ConversationService.pagination_by_last_id(
-            session=mock_session,
+            session=sqlite_session,
-            app_model=mock_app_model,
+            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()
+        conversation = ConversationServiceTestDataFactory.create_conversation()
-        mock_conversation.updated_at = naive_utc_now()
+        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()
+        conversation = ConversationServiceTestDataFactory.create_conversation()
-        mock_conversation.created_at = naive_utc_now()
+        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()
+        app_model = ConversationServiceTestDataFactory.create_app()
-        user = ConversationServiceTestDataFactory.create_account_mock()
+        user = ConversationServiceTestDataFactory.create_account()
-        conversation = ConversationServiceTestDataFactory.create_conversation_mock()
+        conversation = ConversationServiceTestDataFactory.create_conversation()
-
+        matching_variable = _conversation_variable(
-        mock_get_conversation.return_value = conversation
+            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 - SQLite executes the MySQL-compatible JSON extraction boundary.
-        assert mock_session.scalars.called
+        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"