refactor(#11-Kritik): Contacts-Entities aus statischem Core-Registry entfernt — ContactsPlugin.get_entity_models() ist Single Source (contact/contacts/company); conftest spiegelt Produktions-Bootstrap idempotent (autouse-Fixture); Regressionstests beweisen Plugin-Registrierung; 12 ACL-Batch-Failures per Stash-Test als Vorbestand bewiesen (Suite-Isolation, identisch auf clean HEAD)

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
+    # Core models only — plugin models (incl. contacts: contact/contacts/company)
-    # via plugin.get_entity_models() at activation time (P0-3 fix).
+    # are registered dynamically via plugin.get_entity_models() at activation
-    "contact": Contact,
+    # time (P0-3 fix). The contacts entries were duplicated here historically
-    "contacts": Contact,
+    # — ContactsPlugin.get_entity_models() is the single source of truth.
-    "company": Contact,
     "address": Address,
     "attachment": Attachment,
     "bank_account": BankAccount,

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

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