fix: sqlalchemy.orm.exc.DetachedInstanceError (#34910)

api/services/plugin/plugin_permission_service.py

@@ -1,14 +1,13 @@
 from sqlalchemy import select
-from sqlalchemy.orm import sessionmaker
 
-from extensions.ext_database import db
+from core.db.session_factory import session_factory
 from models.account import TenantPluginPermission
 
 
 class PluginPermissionService:
     @staticmethod
     def get_permission(tenant_id: str) -> TenantPluginPermission | None:
-        with sessionmaker(bind=db.engine).begin() as session:
+        with session_factory.create_session() as session:
             return session.scalar(
                 select(TenantPluginPermission).where(TenantPluginPermission.tenant_id == tenant_id).limit(1)
             )
@@ -19,7 +18,7 @@ class PluginPermissionService:
         install_permission: TenantPluginPermission.InstallPermission,
         debug_permission: TenantPluginPermission.DebugPermission,
     ):
-        with sessionmaker(bind=db.engine).begin() as session:
+        with session_factory.create_session() as session, session.begin():
             permission = session.scalar(
                 select(TenantPluginPermission).where(TenantPluginPermission.tenant_id == tenant_id).limit(1)
             )

api/tests/unit_tests/services/plugin/test_plugin_permission_service.py

@@ -6,23 +6,25 @@ MODULE = "services.plugin.plugin_permission_service"
 
 
 def _patched_session():
-    """Patch sessionmaker(bind=db.engine).begin() to return a mock session as context manager."""
+    """Patch session_factory.create_session() to return a mock session as context manager."""
     session = MagicMock()
-    mock_sessionmaker = MagicMock()
-    mock_sessionmaker.return_value.begin.return_value.__enter__ = MagicMock(return_value=session)
-    mock_sessionmaker.return_value.begin.return_value.__exit__ = MagicMock(return_value=False)
-    patcher = patch(f"{MODULE}.sessionmaker", mock_sessionmaker)
-    db_patcher = patch(f"{MODULE}.db")
-    return patcher, db_patcher, session
+    session.__enter__ = MagicMock(return_value=session)
+    session.__exit__ = MagicMock(return_value=False)
+    session.begin.return_value.__enter__ = MagicMock(return_value=session)
+    session.begin.return_value.__exit__ = MagicMock(return_value=False)
+    mock_factory = MagicMock()
+    mock_factory.create_session.return_value = session
+    patcher = patch(f"{MODULE}.session_factory", mock_factory)
+    return patcher, session
 
 
 class TestGetPermission:
     def test_returns_permission_when_found(self):
-        p1, p2, session = _patched_session()
+        p1, session = _patched_session()
         permission = MagicMock()
         session.scalar.return_value = permission
 
-        with p1, p2:
+        with p1:
             from services.plugin.plugin_permission_service import PluginPermissionService
 
             result = PluginPermissionService.get_permission("t1")
@@ -30,10 +32,10 @@ class TestGetPermission:
         assert result is permission
 
     def test_returns_none_when_not_found(self):
-        p1, p2, session = _patched_session()
+        p1, session = _patched_session()
         session.scalar.return_value = None
 
-        with p1, p2:
+        with p1:
             from services.plugin.plugin_permission_service import PluginPermissionService
 
             result = PluginPermissionService.get_permission("t1")
@@ -43,10 +45,10 @@ class TestGetPermission:
 
 class TestChangePermission:
     def test_creates_new_permission_when_not_exists(self):
-        p1, p2, session = _patched_session()
+        p1, session = _patched_session()
         session.scalar.return_value = None
 
-        with p1, p2, patch(f"{MODULE}.select"), patch(f"{MODULE}.TenantPluginPermission") as perm_cls:
+        with p1, patch(f"{MODULE}.select"), patch(f"{MODULE}.TenantPluginPermission") as perm_cls:
             perm_cls.return_value = MagicMock()
             from services.plugin.plugin_permission_service import PluginPermissionService
 
@@ -54,20 +56,24 @@ class TestChangePermission:
                 "t1", TenantPluginPermission.InstallPermission.EVERYONE, TenantPluginPermission.DebugPermission.EVERYONE
             )
 
+        assert result is True
+        session.begin.assert_called_once()
         session.add.assert_called_once()
 
     def test_updates_existing_permission(self):
-        p1, p2, session = _patched_session()
+        p1, session = _patched_session()
         existing = MagicMock()
         session.scalar.return_value = existing
 
-        with p1, p2:
+        with p1:
             from services.plugin.plugin_permission_service import PluginPermissionService
 
             result = PluginPermissionService.change_permission(
                 "t1", TenantPluginPermission.InstallPermission.ADMINS, TenantPluginPermission.DebugPermission.ADMINS
             )
 
+        assert result is True
+        session.begin.assert_called_once()
         assert existing.install_permission == TenantPluginPermission.InstallPermission.ADMINS
         assert existing.debug_permission == TenantPluginPermission.DebugPermission.ADMINS
         session.add.assert_not_called()