refactor(models): remove implicit-session Account tenant setters (#43064)

This commit is contained in:
Denis Ermakov authored and GitHub committed 2026-09-29 02:03:34 +00:00
1 parent 1ff3d3db68
commit 9bb9935f44
37 files changed
+63 -88

No files matched your search

-17
View File
@@ -14,7 +14,6 @@ from typing_extensions import deprecated
from configs import dify_config
from .base import TypeBase
from .engine import db
from .types import EnumText, LongText, StringUUID
@@ -131,18 +130,6 @@ class Account(UserMixin, TypeBase):
def current_tenant(self):
return self._current_tenant
@current_tenant.setter
def current_tenant(self, tenant: "Tenant") -> None:
with Session(db.engine, expire_on_commit=False) as session:
# TODO: A workaround to reload the tenant with `expire_on_commit=False`, allowing
# access to it after the session has been closed.
# This prevents `DetachedInstanceError` when accessing the tenant outside
# the session's lifecycle.
# (The `tenant` argument is typically loaded by `db.session` without the
# `expire_on_commit=False` flag, meaning its lifetime is tied to the web
# request's lifecycle.)
self.set_current_tenant_with_session(tenant, session=session)
def set_current_tenant_with_session(self, tenant: "Tenant", *, session: Session) -> None:
"""Set the current tenant and role using the caller-owned session."""
tenant_join_query = select(TenantAccountJoin).where(
@@ -162,10 +149,6 @@ class Account(UserMixin, TypeBase):
def current_tenant_id(self) -> str | None:
return self._current_tenant.id if self._current_tenant else None
def set_tenant_id(self, tenant_id: str) -> None:
with Session(db.engine, expire_on_commit=False) as session:
self.set_tenant_id_with_session(tenant_id, session=session)
def set_tenant_id_with_session(self, tenant_id: str, *, session: Session) -> None:
"""Set the current tenant by id using the caller-owned session."""
query = select(Tenant, TenantAccountJoin).where(
@@ -461,7 +461,6 @@ class TestTencentDataTrace:
trace_info.workflow_run_id = "run-1"
database = SimpleNamespace(engine=sqlite3_session.get_bind())
monkeypatch.setattr("dify_trace_tencent.tencent_trace.db", database)
monkeypatch.setattr("models.account.db", database)
with patch("dify_trace_tencent.tencent_trace.SQLAlchemyWorkflowNodeExecutionRepository") as mock_repo:
mock_repo.return_value.get_by_workflow_execution.return_value = []
@@ -57,11 +57,7 @@ class TestChatMessageApiPermissions:
mock_scalars_result.one.return_value = tenant
monkeypatch.setattr(mock_session_instance, "scalars", mock.Mock(return_value=mock_scalars_result))
mock_session_context = mock.Mock()
mock_session_context.__enter__.return_value = mock_session_instance
monkeypatch.setattr("models.account.Session", lambda _, expire_on_commit: mock_session_context)
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=mock_session_instance)
return account
@pytest.mark.parametrize(
@@ -58,11 +58,7 @@ class TestFeedbackExportApi:
mock_scalars_result.one.return_value = tenant
monkeypatch.setattr(mock_session_instance, "scalars", mock.Mock(return_value=mock_scalars_result))
mock_session_context = mock.Mock()
mock_session_context.__enter__.return_value = mock_session_instance
monkeypatch.setattr("models.account.Session", lambda _, expire_on_commit: mock_session_context)
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=mock_session_instance)
return account
@pytest.fixture
@@ -53,11 +53,7 @@ class TestModelConfigResourcePermissions:
mock_scalars_result.one.return_value = tenant
monkeypatch.setattr(mock_session_instance, "scalars", mock.Mock(return_value=mock_scalars_result))
mock_session_context = mock.Mock()
mock_session_context.__enter__.return_value = mock_session_instance
monkeypatch.setattr("models.account.Session", lambda _, expire_on_commit: mock_session_context)
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=mock_session_instance)
return account
@pytest.mark.parametrize(
@@ -43,7 +43,7 @@ def _create_account_and_tenant(db_session: Session) -> tuple[Account, Tenant]:
db_session.add(join)
db_session.commit()
account.set_tenant_id(tenant.id)
account.set_tenant_id_with_session(tenant.id, session=db_session)
account.timezone = "UTC"
db_session.commit()
@@ -53,7 +53,7 @@ def create_console_account_and_tenant(db_session: Session) -> tuple[Account, Ten
)
db_session.commit()
account.set_tenant_id(tenant.id)
account.set_tenant_id_with_session(tenant.id, session=db_session)
account.timezone = "UTC"
db_session.commit()
@@ -78,7 +78,7 @@ class WorkspaceMembersIntegrationFactory:
)
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account
@staticmethod
@@ -203,7 +203,7 @@ class TestHumanInputResumeNodeExecutionIntegration:
db_session_with_containers.add(tenant_join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
app = App(
tenant_id=tenant.id,
@@ -6,8 +6,8 @@ Migrated from unit_tests/models/test_account_models.py, replacing
Also absorbs unit_tests/models/test_account.py role helper coverage.
Covers:
- Account.current_tenant setter (sets _current_tenant and role from TenantAccountJoin)
- Account.set_tenant_id (resolves tenant + role from real join row)
- Account.set_current_tenant_with_session (sets _current_tenant and role from TenantAccountJoin)
- Account.set_tenant_id_with_session (resolves tenant + role from real join row)
- Tenant.get_accounts (returns accounts linked via TenantAccountJoin)
"""
@@ -103,8 +103,8 @@ class TestTenantAccountRole:
assert not TenantAccountRole.is_privileged_role(cast(TenantAccountRole, ""))
class TestAccountCurrentTenantSetter(_DBTrackingTestBase):
"""Integration tests for Account.current_tenant property setter."""
class TestAccountSetCurrentTenant(_DBTrackingTestBase):
"""Integration tests for Account.current_tenant and set_current_tenant_with_session."""
def test_current_tenant_property_returns_cached_tenant(self, db_session_with_containers: Session) -> None:
"""current_tenant getter returns the in-memory _current_tenant without DB access."""
@@ -114,59 +114,61 @@ class TestAccountCurrentTenantSetter(_DBTrackingTestBase):
assert account.current_tenant is tenant
def test_current_tenant_setter_sets_tenant_and_role_when_join_exists(
def test_set_current_tenant_with_session_sets_tenant_and_role_when_join_exists(
self, db_session_with_containers: Session
) -> None:
"""Setting current_tenant loads the join row and assigns role when relationship exists."""
"""set_current_tenant_with_session loads the join row and assigns role when relationship exists."""
tenant = self._create_tenant(db_session_with_containers)
account = self._create_account(db_session_with_containers)
self._create_join(db_session_with_containers, tenant.id, account.id, TenantAccountRole.OWNER)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
assert account._current_tenant is not None
assert account._current_tenant.id == tenant.id
assert account.role == TenantAccountRole.OWNER
def test_current_tenant_setter_sets_none_when_no_join_exists(self, db_session_with_containers: Session) -> None:
"""Setting current_tenant results in _current_tenant=None when no join row exists."""
def test_set_current_tenant_with_session_sets_none_when_no_join_exists(
self, db_session_with_containers: Session
) -> None:
"""set_current_tenant_with_session results in _current_tenant=None when no join row exists."""
tenant = self._create_tenant(db_session_with_containers)
account = self._create_account(db_session_with_containers)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
assert account._current_tenant is None
class TestAccountSetTenantId(_DBTrackingTestBase):
"""Integration tests for Account.set_tenant_id method."""
"""Integration tests for Account.set_tenant_id_with_session method."""
def test_set_tenant_id_sets_tenant_and_role_when_relationship_exists(
def test_set_tenant_id_with_session_sets_tenant_and_role_when_relationship_exists(
self, db_session_with_containers: Session
) -> None:
"""set_tenant_id loads the tenant and assigns role when a join row exists."""
"""set_tenant_id_with_session loads the tenant and assigns role when a join row exists."""
tenant = self._create_tenant(db_session_with_containers)
account = self._create_account(db_session_with_containers)
self._create_join(db_session_with_containers, tenant.id, account.id, TenantAccountRole.ADMIN)
db_session_with_containers.commit()
account.set_tenant_id(tenant.id)
account.set_tenant_id_with_session(tenant.id, session=db_session_with_containers)
assert account._current_tenant is not None
assert account._current_tenant.id == tenant.id
assert account.role == TenantAccountRole.ADMIN
def test_set_tenant_id_does_not_set_tenant_when_no_relationship_exists(
def test_set_tenant_id_with_session_does_not_set_tenant_when_no_relationship_exists(
self, db_session_with_containers: Session
) -> None:
"""set_tenant_id does nothing when no join row matches the tenant."""
"""set_tenant_id_with_session does nothing when no join row matches the tenant."""
tenant = self._create_tenant(db_session_with_containers)
account = self._create_account(db_session_with_containers)
db_session_with_containers.commit()
account.set_tenant_id(tenant.id)
account.set_tenant_id_with_session(tenant.id, session=db_session_with_containers)
assert account._current_tenant is None
@@ -57,7 +57,7 @@ class DatasetUpdateDeleteTestDataFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@staticmethod
@@ -882,7 +882,7 @@ class TestTenantService:
TenantService.create_tenant_member(tenant2, account, db_session_with_containers, role="admin")
# Set initial current tenant
account.current_tenant = tenant1
account.set_current_tenant_with_session(tenant1, session=db_session_with_containers)
db_session_with_containers.commit()
@@ -1535,7 +1535,7 @@ class TestTenantService:
)
existing_tenant = TenantService.create_tenant(name=existing_tenant_name, session=db_session_with_containers)
TenantService.create_tenant_member(existing_tenant, account, db_session_with_containers, role="owner")
account.current_tenant = existing_tenant
account.set_current_tenant_with_session(existing_tenant, session=db_session_with_containers)
db_session_with_containers.commit()
@@ -55,7 +55,7 @@ class DatasetPermissionTestDataFactory:
db.session.add(join)
db.session.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db.session)
return account, tenant
@staticmethod
@@ -43,7 +43,7 @@ class DatasetDeleteIntegrationDataFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@staticmethod
@@ -54,7 +54,7 @@ class SegmentServiceTestDataFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@staticmethod
@@ -62,7 +62,7 @@ class DatasetRetrievalTestDataFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@staticmethod
@@ -88,7 +88,7 @@ class DatasetRetrievalTestDataFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account
@staticmethod
@@ -51,7 +51,7 @@ class DatasetUpdateTestDataFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@staticmethod
@@ -91,7 +91,7 @@ class TestFileService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account
@@ -93,7 +93,7 @@ class TestMetadataService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -106,7 +106,7 @@ class TestModelLoadBalancingService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -77,7 +77,7 @@ class TestModelProviderService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -42,7 +42,7 @@ class ScheduleServiceIntegrationFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@staticmethod
@@ -83,7 +83,7 @@ class TestWebAppAuthService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -146,7 +146,7 @@ class TestWebAppAuthService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant, password
@@ -77,7 +77,7 @@ class WebhookServiceRelationshipFactory:
db_session_with_containers.add(join)
db_session_with_containers.commit()
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@staticmethod
@@ -76,7 +76,7 @@ class TestWorkflowService:
db_session_with_containers.commit()
# Set the current tenant for the account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account
@@ -83,7 +83,7 @@ class TestWorkspaceService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -89,7 +89,7 @@ class TestApiToolManageService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -87,7 +87,7 @@ class TestMCPToolManageService:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -131,7 +131,7 @@ class TestWorkflowConverter:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -95,7 +95,7 @@ class TestBatchCleanDocumentTask:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account
@@ -123,7 +123,7 @@ class TestBatchCreateSegmentToIndexTask:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -99,7 +99,7 @@ class TestCreateSegmentToIndexTask:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -78,7 +78,7 @@ class TestDisableSegmentFromIndexTask:
db_session_with_containers.commit()
# Set current tenant for account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account, tenant
@@ -81,7 +81,7 @@ class TestDisableSegmentsFromIndexTask:
db_session_with_containers.commit()
# Set the current tenant for the account
account.current_tenant = tenant
account.set_current_tenant_with_session(tenant, session=db_session_with_containers)
return account
@@ -25,10 +25,9 @@ class ConcreteTraceInstance(BaseTraceInstance):
@pytest.fixture(autouse=True)
def _bind_production_sessions(monkeypatch: pytest.MonkeyPatch, sqlite_engine: Engine) -> None:
"""Bind both service-owned ORM sessions to the test's SQLite engine."""
"""Bind the service-owned ORM session to the test's SQLite engine."""
database = SimpleNamespace(engine=sqlite_engine)
monkeypatch.setattr("core.ops.base_trace_instance.db", database)
monkeypatch.setattr("models.account.db", database)
def _persist_app(session: Session, *, created_by: str | None) -> App:
@@ -52,15 +52,11 @@ def sqlite_tool_db(
monkeypatch: pytest.MonkeyPatch,
sqlite_engine: Engine,
) -> Iterator[SqliteToolDb]:
"""Bind service-owned sessions and Account tenant reloads to SQLite."""
"""Bind service-owned sessions to SQLite."""
models = (App, Workflow, EndUser, Account, Tenant, TenantAccountJoin)
TypeBase.metadata.create_all(sqlite_engine, tables=[model.__table__ for model in models])
session_maker = sessionmaker(bind=sqlite_engine, expire_on_commit=False)
monkeypatch.setattr(workflow_tool_module.session_factory, "create_session", session_maker)
from models import account as account_module
monkeypatch.setattr(account_module, "db", SimpleNamespace(engine=sqlite_engine))
with session_maker() as caller_session:
yield SqliteToolDb(engine=sqlite_engine, session_maker=session_maker, caller_session=caller_session)
@@ -335,6 +335,14 @@ class TestTenantRelationshipIntegrity:
# Assert
assert tenant_id_none is None
def test_current_tenant_is_read_only(self):
"""current_tenant has no setter, so tenant switching cannot open a hidden session."""
account = Account(name="Test User", email="test@example.com")
tenant = Tenant(name="Test Tenant")
with pytest.raises(AttributeError):
account.current_tenant = tenant # type: ignore[misc]
@pytest.mark.parametrize("sqlite_session", [(Account, Tenant, TenantAccountJoin)], indirect=True)
def test_set_current_tenant_with_session_uses_caller_session(self, sqlite_session: Session):
account = Account(name="Test User", email="test@example.com")