From e1a59e759fa3c11cf357f9377ae4f9a46f774de3 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Thu, 27 Aug 2026 18:53:12 +0200 Subject: [PATCH] =?UTF-8?q?refactor(#11-Kritik):=20Contacts-Entities=20aus?= =?UTF-8?q?=20statischem=20Core-Registry=20entfernt=20=E2=80=94=20Contacts?= =?UTF-8?q?Plugin.get=5Fentity=5Fmodels()=20ist=20Single=20Source=20(conta?= =?UTF-8?q?ct/contacts/company);=20conftest=20spiegelt=20Produktions-Boots?= =?UTF-8?q?trap=20idempotent=20(autouse-Fixture);=20Regressionstests=20bew?= =?UTF-8?q?eisen=20Plugin-Registrierung;=2012=20ACL-Batch-Failures=20per?= =?UTF-8?q?=20Stash-Test=20als=20Vorbestand=20bewiesen=20(Suite-Isolation,?= =?UTF-8?q?=20identisch=20auf=20clean=20HEAD)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/services/entity_permission_service.py | 10 ++--- tests/conftest.py | 27 ++++++++++++- tests/test_contacts_entity_registry.py | 47 +++++++++++++++++++++++ 3 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 tests/test_contacts_entity_registry.py diff --git a/app/services/entity_permission_service.py b/app/services/entity_permission_service.py index 4098fd0..6d183f6 100644 --- a/app/services/entity_permission_service.py +++ b/app/services/entity_permission_service.py @@ -29,7 +29,6 @@ from app.core.notifications import post_system_message from app.models.address import Address from app.models.attachment import Attachment from app.models.bank_account import BankAccount -from app.models.contact import Contact from app.models.contact_folder import ContactFolder from app.models.custom_field_definition import CustomFieldDefinition from app.models.entity_permission import EntityPermission @@ -53,11 +52,10 @@ logger = logging.getLogger(__name__) # This replaces insecure text(f"SELECT ... FROM {entity_type}s") queries # with safe SQLAlchemy model-based queries (prevents SQL injection). ENTITY_MODELS: dict[str, type] = { - # Core models only — plugin models are registered dynamically - # via plugin.get_entity_models() at activation time (P0-3 fix). - "contact": Contact, - "contacts": Contact, - "company": Contact, + # Core models only — plugin models (incl. contacts: contact/contacts/company) + # are registered dynamically via plugin.get_entity_models() at activation + # time (P0-3 fix). The contacts entries were duplicated here historically + # — ContactsPlugin.get_entity_models() is the single source of truth. "address": Address, "attachment": Attachment, "bank_account": BankAccount, diff --git a/tests/conftest.py b/tests/conftest.py index a81d083..572da45 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -65,12 +65,19 @@ from app.plugins.registry import get_registry _registry = get_registry() _registry.discover_builtins() +from app.services.entity_permission_service import register_entity_model # noqa: E402 + for _plugin_name in _registry.list_discovered(): _plugin = _registry.get_plugin(_plugin_name) if _plugin is not None: # Importing get_entity_models() triggers model class imports # which registers them with Base.metadata - _plugin.get_entity_models() + _entity_models = _plugin.get_entity_models() + # Mirror the production bootstrap (main.py lifespan / activation): + # every plugin's entity models are registered in ENTITY_MODELS — + # the core registry itself carries core entities only. + for _entity_type, _model_class in _entity_models.items(): + register_entity_model(_entity_type, _model_class) # Also import the plugin's __init__ to ensure all models are loaded import importlib try: @@ -83,6 +90,24 @@ for _plugin_name in _registry.list_discovered(): except Exception: pass + +@pytest.fixture(autouse=True) +def _ensure_plugin_entity_models(): + """Mirror the production bootstrap before EVERY test (idempotent). + + Lifecycle tests may deactivate plugins (deregistering their entity + models). In production main.py lifespan re-activates them at startup; + in tests we re-run the same registration so isolation is guaranteed + without a static duplicate in the core registry. + """ + for _plugin_name in _registry.list_discovered(): + _plugin = _registry.get_plugin(_plugin_name) + if _plugin is None: + continue + for _entity_type, _model_class in _plugin.get_entity_models().items(): + register_entity_model(_entity_type, _model_class) + yield + # Also import core models that may be missing # Wiki plugin models — not loaded by get_entity_models() from app.core.permission_registry import init_permission_registry # noqa: F401 diff --git a/tests/test_contacts_entity_registry.py b/tests/test_contacts_entity_registry.py new file mode 100644 index 0000000..0777644 --- /dev/null +++ b/tests/test_contacts_entity_registry.py @@ -0,0 +1,47 @@ +"""Contacts-Entity-Registry: single source of truth (Kritikpunkt 11). + +Beweist, dass die Contacts-Entities (contact/contacts/company) NICHT mehr +statisch im Core-Registry stehen, sondern ausschliesslich vom ContactsPlugin +ueber get_entity_models() + register_entity_model() kommen — wie jedes +andere Plugin auch. is_core=True heisst Pflichtplugin, nicht Core-Verdrahtung. +""" + +from __future__ import annotations + +from app.models.contact import Contact +from app.plugins.builtins.contacts.plugin import ContactsPlugin +from app.services.entity_permission_service import ENTITY_MODELS + + +def test_core_registry_source_has_no_static_contacts_entries(): + """The core registry SOURCE must not hardcode contacts entities anymore. + + (After bootstrap ENTITY_MODELS legitimately contains them — registered + via the plugin. This test proves the duplicate static source is gone.) + """ + from pathlib import Path + + module_path = Path(__file__).parent.parent / "app" / "services" / "entity_permission_service.py" + text = module_path.read_text(encoding="utf-8") + assert '"contact": Contact' not in text, "statischer contact-Eintrag noch im Core!" + assert '"contacts": Contact' not in text, "statischer contacts-Eintrag noch im Core!" + assert '"company": Contact' not in text, "statischer company-Eintrag noch im Core!" + + +def test_contacts_plugin_is_single_source_for_its_entities(): + """ContactsPlugin.get_entity_models() defines contact/contacts/company.""" + plugin_models = ContactsPlugin().get_entity_models() + assert plugin_models == { + "contact": Contact, + "contacts": Contact, + "company": Contact, + } + + +def test_bootstrap_registers_contacts_entities_via_plugin(): + """After the real bootstrap path (conftest mirrors main.py lifespan: + every plugin's get_entity_models() is registered), contacts entities + are present — via the plugin, not via the core registry.""" + assert ENTITY_MODELS["contact"] is Contact + assert ENTITY_MODELS["contacts"] is Contact + assert ENTITY_MODELS["company"] is Contact