fix(#355): Plugin-Lifecycle Runtime-Registrierungen repariert — Vorher-Status wird jetzt VOR dem registry-Aufruf gelesen (war konstant falsch → Deactivate-Cleanup toter Code); sync_notification_types hinter DB-Statusupdate verschoben; echter Integrationstest tests/test_plugin_lifecycle_service.py (install→activate×2→deactivate×2→re-activate über PluginService) rot→grün
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
fixes #355
This commit is contained in:
@@ -12,6 +12,15 @@
|
|||||||
|
|
||||||
**Gates:** ruff exit=0 · tsc --noEmit exit=0 · pytest 11 passed · Vitest 2 passed
|
**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)
|
## Legacy-Cleanup (2026-08-27)
|
||||||
|
|
||||||
| Aktion | Commit | Verifikation (Live-Messung) |
|
| Aktion | Commit | Verifikation (Live-Messung) |
|
||||||
|
|||||||
@@ -635,15 +635,17 @@ class PluginRegistry:
|
|||||||
# Activation status is enforced per-request via require_active_plugin().
|
# Activation status is enforced per-request via require_active_plugin().
|
||||||
# No dynamic route registration here — prevents duplicate routes.
|
# No dynamic route registration here — prevents duplicate routes.
|
||||||
|
|
||||||
# Sync notification types from this plugin
|
# Update DB record FIRST — sync_notification_types() only picks up types
|
||||||
await self.sync_notification_types(db)
|
# from plugins whose record is already active; running it before the
|
||||||
|
# status update silently skipped the freshly activated plugin's types.
|
||||||
# Update DB record
|
|
||||||
record.status = "active"
|
record.status = "active"
|
||||||
record.active = True
|
record.active = True
|
||||||
await db.flush()
|
await db.flush()
|
||||||
self._db_status[name] = record
|
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
|
# Invalidate Redis cache for this plugin across all tenants
|
||||||
try:
|
try:
|
||||||
from app.core.redis import get_redis
|
from app.core.redis import get_redis
|
||||||
|
|||||||
@@ -91,8 +91,15 @@ class PluginService:
|
|||||||
Checks that all declared dependencies are active first.
|
Checks that all declared dependencies are active first.
|
||||||
"""
|
"""
|
||||||
try:
|
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)
|
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
|
# Update permission registry at runtime so newly activated plugins
|
||||||
# are immediately usable without app restart (P0-10 fix).
|
# are immediately usable without app restart (P0-10 fix).
|
||||||
@@ -155,8 +162,14 @@ class PluginService:
|
|||||||
Unregisters event listeners and routes. Idempotent.
|
Unregisters event listeners and routes. Idempotent.
|
||||||
"""
|
"""
|
||||||
try:
|
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)
|
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
|
# Update permission registry at runtime so deactivated plugins
|
||||||
# immediately stop being usable (P0-10 fix).
|
# immediately stop being usable (P0-10 fix).
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user