diff --git a/PROGRESS.md b/PROGRESS.md index 02f2e7a..b24862f 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -12,6 +12,15 @@ **Gates:** ruff exit=0 · tsc --noEmit exit=0 · pytest 11 passed · Vitest 2 passed +## Welle 1 — Plugin-Lifecycle-Fix (2026-08-27) + +| Finding | Issue | Fix | Verifikation (Live-Messung) | +|---|---|---|---| +| P1: `was_already_active`/`was_already_inactive` wurden in `plugin_service.activate/deactivate_plugin()` aus dem Record NACH dem Registry-Aufruf berechnet → konstant falsch → Runtime-Deregistrierung beim Deactivate war toter Code; Activate-Zweig lief nie | [#355](https://forgejo.media-on.de/Leopoldadmin/leocrm/issues/355) | Vorher-Status wird VOR dem Registry-Aufruf gelesen (`_get_plugin_record`) und beide Zweige laufen jetzt wirklich | TDD: neuer echter Integrationstest `tests/test_plugin_lifecycle_service.py` (install→activate×2→deactivate×2→re-activate über PluginService, beweist Permissions×1, Gate-Eintrag, ENTITY_MODELS on/off) rot→grün; Regression 93 passed (nur bekannter #354-Vorbestand) | +| P3: `registry.activate()` synced Notification Types VOR dem Statusupdate → Types des frisch aktivierten Plugins fehlten | [#355](https://forgejo.media-on.de/Leopoldadmin/leocrm/issues/355) | `sync_notification_types()` hinter DB-Statusupdate+Flush verschoben | Beweis im selben Integrationstest: NotificationType existiert nach activate, entfernt nach deactivate | + +**Gates:** ruff modified-files grün · pytest Lifecycle 2/2 + Regression 93 passed · Deploy folgt + ## Legacy-Cleanup (2026-08-27) | Aktion | Commit | Verifikation (Live-Messung) | diff --git a/app/plugins/registry.py b/app/plugins/registry.py index 17364a4..0b76930 100644 --- a/app/plugins/registry.py +++ b/app/plugins/registry.py @@ -635,15 +635,17 @@ class PluginRegistry: # Activation status is enforced per-request via require_active_plugin(). # No dynamic route registration here — prevents duplicate routes. - # Sync notification types from this plugin - await self.sync_notification_types(db) - - # Update DB record + # Update DB record FIRST — sync_notification_types() only picks up types + # from plugins whose record is already active; running it before the + # status update silently skipped the freshly activated plugin's types. record.status = "active" record.active = True await db.flush() self._db_status[name] = record + # Sync notification types from this plugin (AFTER status update) + await self.sync_notification_types(db) + # Invalidate Redis cache for this plugin across all tenants try: from app.core.redis import get_redis diff --git a/app/services/plugin_service.py b/app/services/plugin_service.py index 9bec3d7..32031e3 100644 --- a/app/services/plugin_service.py +++ b/app/services/plugin_service.py @@ -91,8 +91,15 @@ class PluginService: Checks that all declared dependencies are active first. """ try: + # Pre-state MUST be captured BEFORE the mutation: the record returned + # by registry.activate() always reflects the NEW state, so computing + # was_already_active afterwards yielded constant True and silently + # skipped all runtime registrations (dead code — proven by + # tests/test_plugin_lifecycle_service.py, Welle 1 / Kritikpunkt 1). + pre = await self._registry._get_plugin_record(db, name) + was_already_active = bool(pre and pre.active and pre.status == "active") + record = await self._registry.activate(db, name) - was_already_active = record.active and record.status == "active" # Update permission registry at runtime so newly activated plugins # are immediately usable without app restart (P0-10 fix). @@ -155,8 +162,14 @@ class PluginService: Unregisters event listeners and routes. Idempotent. """ try: + # Pre-state MUST be captured BEFORE the mutation (same reasoning as + # activate_plugin — the returned record always reflects the NEW + # state; computing was_already_inactive afterwards yielded constant + # True and skipped permission/entity/gate deregistration entirely). + pre = await self._registry._get_plugin_record(db, name) + was_already_inactive = bool(pre and not pre.active and pre.status == "inactive") + record = await self._registry.deactivate(db, name) - was_already_inactive = not record.active and record.status == "inactive" # Update permission registry at runtime so deactivated plugins # immediately stop being usable (P0-10 fix). diff --git a/tests/test_plugin_lifecycle_service.py b/tests/test_plugin_lifecycle_service.py new file mode 100644 index 0000000..dacfa87 --- /dev/null +++ b/tests/test_plugin_lifecycle_service.py @@ -0,0 +1,199 @@ +"""Echter Plugin-Lifecycle-Integrationstest ueber PluginService. + +Regression fuer die Welle-1-Findings: +- P1: Runtime-Registrierungen (Permissions, active-set, Entity-Models) liefen nie, + weil der Vorher-Status erst NACH dem registry-Aufruf berechnet wurde. +- P3: sync_notification_types lief vor dem DB-Statusupdate. +Realer Produktionsweg: PluginService.install/activate/deactivate - keine manuellen +Registry-Manipulationen. +""" + +from __future__ import annotations + +import uuid +from typing import Any + +import pytest +import pytest_asyncio +from sqlalchemy import Column, String, select +from sqlalchemy.dialects.postgresql import UUID as PGUUID +from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession + +from app.core.db import Base, close_engine, reset_engine_for_testing +from app.core.hooks import get_hook_registry +from app.core.permission_registry import ( + get_permission_registry, + init_permission_registry, +) +from app.models.notification import NotificationType +from app.plugins.base import BasePlugin +from app.plugins.manifest import PluginManifest +from app.plugins.registry import PluginRegistry, reset_registry_for_testing +from app.services.entity_permission_service import ENTITY_MODELS +from app.services.plugin_service import reset_plugin_service_for_testing + +PLUGIN_NAME = "lifecycle_svc_plugin" +PERM = "lifecycletest:read" +ENTITY = "lifecycle_svc_item" +NT_KEY = f"{PLUGIN_NAME}.updated" + + +class LifecycleSvcItem(Base): + """Minimal tenant-scoped model owned by the test plugin.""" + + __tablename__ = "lifecycle_svc_items" + + id: Any = Column(PGUUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + tenant_id: Any = Column(PGUUID(as_uuid=True), nullable=False, index=True) + name: Any = Column(String(120), nullable=False) + + +class LifecycleSvcPlugin(BasePlugin): + """Test plugin contributing permissions, entity model and notification type.""" + + manifest = PluginManifest( + name=PLUGIN_NAME, + version="1.0.0", + display_name="Lifecycle Svc Plugin", + description="Integration fixture for the full service lifecycle.", + dependencies=[], + routes=[], + events=[], + migrations=[], + permissions=[PERM], + ) + + def __init__(self) -> None: + super().__init__() + self.activate_calls = 0 + self.deactivate_calls = 0 + + def get_entity_models(self) -> dict[str, type]: + return {ENTITY: LifecycleSvcItem} + + async def on_activate(self, db, service_container, event_bus) -> None: + self.activate_calls += 1 + await super().on_activate(db, service_container, event_bus) + + async def on_deactivate(self, db, service_container, event_bus) -> None: + self.deactivate_calls += 1 + await super().on_deactivate(db, service_container, event_bus) + + def get_notification_types(self) -> list[dict[str, Any]]: + return [ + { + "type_key": NT_KEY, + "category": "general", + "label": "Lifecycle update", + "description": "Type contributed by the lifecycle svc plugin", + "is_enabled_by_default": True, + } + ] + + +@pytest_asyncio.fixture +async def svc_env(engine: AsyncEngine): + """Fresh registries plus registered plugin instance and its table.""" + reset_engine_for_testing(engine) + reg: PluginRegistry = reset_registry_for_testing() + init_permission_registry(set()) + get_hook_registry()._reset_for_testing() + service = reset_plugin_service_for_testing(reg) + + plugin = LifecycleSvcPlugin() + reg.register_plugin(plugin) + + async with engine.begin() as conn: + await conn.run_sync( + lambda sync_conn: Base.metadata.create_all( + sync_conn, tables=[LifecycleSvcItem.__table__] + ) + ) + + yield {"service": service, "registry": reg, "plugin": plugin} + + await close_engine() + async with engine.begin() as conn: + await conn.run_sync( + lambda sync_conn: Base.metadata.drop_all( + sync_conn, tables=[LifecycleSvcItem.__table__] + ) + ) + + +def _plugin_perms() -> int: + return sum( + 1 for p in get_permission_registry().get_all() + if p.get("plugin_name") == PLUGIN_NAME + ) + + +@pytest.mark.asyncio +async def test_full_lifecycle_registers_and_deregisters_runtime( + db_session: AsyncSession, svc_env, +): + service = svc_env["service"] + perm_reg = get_permission_registry() + + # Install (real path) + install_resp = await service.install_plugin(db_session, PLUGIN_NAME) + assert install_resp["installed"] is True + + # Activate #1 - runtime registrations MUST run + resp1 = await service.activate_plugin(db_session, PLUGIN_NAME) + assert resp1["message"] == "Plugin activated successfully", ( + "Vorher-Status falsch berechnet: Runtime-Registrierungen wurden uebersprungen" + ) + assert perm_reg.is_valid(PERM) + assert _plugin_perms() == 1 + # Gate source _active_plugins must contain the plugin (today's prod-403 root cause) + assert perm_reg.is_plugin_active(PLUGIN_NAME) + assert ENTITY_MODELS.get(ENTITY) is LifecycleSvcItem + assert svc_env["plugin"].activate_calls == 1 + + # P3 regression: notification type synced AFTER status became active + nt_q = select(NotificationType).where(NotificationType.type_key == NT_KEY) + assert (await db_session.execute(nt_q)).scalar_one_or_none() is not None, ( + "Notification-Type fehlt: sync lief vor dem Statusupdate" + ) + + # Activate #2 - idempotent, no double registration + resp2 = await service.activate_plugin(db_session, PLUGIN_NAME) + assert resp2["message"] == "Plugin is already active" + assert _plugin_perms() == 1 + assert svc_env["plugin"].activate_calls == 1 + + # Deactivate #1 - runtime deregistrations MUST run + resp3 = await service.deactivate_plugin(db_session, PLUGIN_NAME) + assert resp3["message"] == "Plugin deactivated successfully" + assert not perm_reg.is_valid(PERM), "Permission blieb registriert (P1 dead-cleanup)" + assert not perm_reg.is_plugin_active(PLUGIN_NAME), "stale gate entry blieb stehen" + assert ENTITY not in ENTITY_MODELS, "Entity-Model blieb registriert" + assert svc_env["plugin"].deactivate_calls == 1 + + # Notification type removed by post-deactivation re-sync + assert (await db_session.execute(nt_q)).scalar_one_or_none() is None + + # Deactivate #2 - safe no-op + resp4 = await service.deactivate_plugin(db_session, PLUGIN_NAME) + assert resp4["message"] == "Plugin is already inactive" + assert svc_env["plugin"].deactivate_calls == 1 + + +@pytest.mark.asyncio +async def test_reactivate_restores_contributions_exactly_once( + db_session: AsyncSession, svc_env, +): + """Activate-deactivate-activate restores every contribution exactly once.""" + service = svc_env["service"] + perm_reg = get_permission_registry() + + await service.install_plugin(db_session, PLUGIN_NAME) + await service.activate_plugin(db_session, PLUGIN_NAME) + await service.deactivate_plugin(db_session, PLUGIN_NAME) + + resp = await service.activate_plugin(db_session, PLUGIN_NAME) + assert resp["message"] == "Plugin activated successfully" + assert _plugin_perms() == 1 + assert perm_reg.is_plugin_active(PLUGIN_NAME) + assert ENTITY_MODELS.get(ENTITY) is LifecycleSvcItem