fix(plugins): FastAPI-Route-Matching — /plugins/{name} (d9aed51) verschlang literale Routen /active-manifests, /manifest, /updates (404 'Plugin not found'); Reihenfolge korrigiert: statische Routen jetzt vor /{name}. Folge war: Sidebar ohne Plugin-Menüeinträge in Produktion (nur Kontakte/Dashboard/System). +2 Regressionstests
This commit is contained in:
+50
-50
@@ -44,46 +44,24 @@ async def list_plugins(
|
|||||||
return {"plugins": plugins, "total": len(plugins)}
|
return {"plugins": plugins, "total": len(plugins)}
|
||||||
|
|
||||||
|
|
||||||
@router.get("/{name}")
|
@router.get("/active-manifests")
|
||||||
async def get_plugin_detail(
|
async def get_active_manifests(
|
||||||
name: str,
|
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
current_user: dict = Depends(require_permission("plugins:read")),
|
# ARCH-003 fix: every authenticated user needs the UI manifests for the
|
||||||
|
# dynamic sidebar/routes — the data is pure UI metadata; actual data
|
||||||
|
# access stays protected by each endpoint's own permission.
|
||||||
|
current_user: dict = Depends(get_current_user),
|
||||||
):
|
):
|
||||||
"""Get detail for a single plugin: manifest metadata + DB status.
|
"""Get UI manifests for all active plugins.
|
||||||
|
|
||||||
BUG-024 fix: this endpoint was missing entirely (404).
|
Returns menu_items, page_routes, detail_tabs, settings_pages, and
|
||||||
|
dashboard_widgets contributed by each active plugin. Used by the
|
||||||
|
frontend PluginRegistry to dynamically register routes, sidebar items,
|
||||||
|
settings pages, and detail tabs.
|
||||||
"""
|
"""
|
||||||
registry = get_registry()
|
service = get_plugin_service()
|
||||||
plugin = registry.get_plugin(name)
|
manifests = await service.get_active_manifests(db)
|
||||||
if plugin is None:
|
return {"plugins": manifests, "total": len(manifests)}
|
||||||
raise HTTPException(
|
|
||||||
status_code=status.HTTP_404_NOT_FOUND,
|
|
||||||
detail={"detail": f"Plugin '{name}' not found", "code": "not_found"},
|
|
||||||
)
|
|
||||||
|
|
||||||
m = plugin.manifest
|
|
||||||
record = registry.get_db_status(name)
|
|
||||||
return {
|
|
||||||
"name": m.name,
|
|
||||||
"version": m.version,
|
|
||||||
"display_name": getattr(m, "display_name", None) or m.name,
|
|
||||||
"description": getattr(m, "description", None),
|
|
||||||
"author": getattr(m, "author", None),
|
|
||||||
"is_core": bool(getattr(m, "is_core", False)),
|
|
||||||
"permissions": list(m.permissions),
|
|
||||||
"depends_on": list(getattr(m, "depends_on", []) or []),
|
|
||||||
"status": {
|
|
||||||
"installed": record is not None,
|
|
||||||
"active": bool(record.active) if record is not None else False,
|
|
||||||
"version_installed": getattr(record, "version", None),
|
|
||||||
},
|
|
||||||
"menu_items": len(getattr(m, "menu_items", []) or []),
|
|
||||||
"page_routes": len(getattr(m, "page_routes", []) or []),
|
|
||||||
"detail_tabs": len(getattr(m, "detail_tabs", []) or []),
|
|
||||||
"settings_pages": len(getattr(m, "settings_pages", []) or []),
|
|
||||||
"dashboard_widgets": len(getattr(m, "dashboard_widgets", []) or []),
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
@router.get("/manifest")
|
@router.get("/manifest")
|
||||||
@@ -134,24 +112,46 @@ async def check_plugin_updates(
|
|||||||
return {"updates": updates, "total": len(updates)}
|
return {"updates": updates, "total": len(updates)}
|
||||||
|
|
||||||
|
|
||||||
@router.get("/active-manifests")
|
@router.get("/{name}")
|
||||||
async def get_active_manifests(
|
async def get_plugin_detail(
|
||||||
|
name: str,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
# ARCH-003 fix: every authenticated user needs the UI manifests for the
|
current_user: dict = Depends(require_permission("plugins:read")),
|
||||||
# dynamic sidebar/routes — the data is pure UI metadata; actual data
|
|
||||||
# access stays protected by each endpoint's own permission.
|
|
||||||
current_user: dict = Depends(get_current_user),
|
|
||||||
):
|
):
|
||||||
"""Get UI manifests for all active plugins.
|
"""Get detail for a single plugin: manifest metadata + DB status.
|
||||||
|
|
||||||
Returns menu_items, page_routes, detail_tabs, settings_pages, and
|
BUG-024 fix: this endpoint was missing entirely (404).
|
||||||
dashboard_widgets contributed by each active plugin. Used by the
|
|
||||||
frontend PluginRegistry to dynamically register routes, sidebar items,
|
|
||||||
settings pages, and detail tabs.
|
|
||||||
"""
|
"""
|
||||||
service = get_plugin_service()
|
registry = get_registry()
|
||||||
manifests = await service.get_active_manifests(db)
|
plugin = registry.get_plugin(name)
|
||||||
return {"plugins": manifests, "total": len(manifests)}
|
if plugin is None:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_404_NOT_FOUND,
|
||||||
|
detail={"detail": f"Plugin '{name}' not found", "code": "not_found"},
|
||||||
|
)
|
||||||
|
|
||||||
|
m = plugin.manifest
|
||||||
|
record = registry.get_db_status(name)
|
||||||
|
return {
|
||||||
|
"name": m.name,
|
||||||
|
"version": m.version,
|
||||||
|
"display_name": getattr(m, "display_name", None) or m.name,
|
||||||
|
"description": getattr(m, "description", None),
|
||||||
|
"author": getattr(m, "author", None),
|
||||||
|
"is_core": bool(getattr(m, "is_core", False)),
|
||||||
|
"permissions": list(m.permissions),
|
||||||
|
"depends_on": list(getattr(m, "depends_on", []) or []),
|
||||||
|
"status": {
|
||||||
|
"installed": record is not None,
|
||||||
|
"active": bool(record.active) if record is not None else False,
|
||||||
|
"version_installed": getattr(record, "version", None),
|
||||||
|
},
|
||||||
|
"menu_items": len(getattr(m, "menu_items", []) or []),
|
||||||
|
"page_routes": len(getattr(m, "page_routes", []) or []),
|
||||||
|
"detail_tabs": len(getattr(m, "detail_tabs", []) or []),
|
||||||
|
"settings_pages": len(getattr(m, "settings_pages", []) or []),
|
||||||
|
"dashboard_widgets": len(getattr(m, "dashboard_widgets", []) or []),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@router.get("/{name}/config")
|
@router.get("/{name}/config")
|
||||||
|
|||||||
@@ -0,0 +1,40 @@
|
|||||||
|
"""Regressionstest: FastAPI-Route-Matching im Plugin-Router.
|
||||||
|
|
||||||
|
GET /{name} (d9aed51) wurde vor /active-manifests registriert und
|
||||||
|
verschluckte literale Routen: /plugins/active-manifests wurde als
|
||||||
|
Plugin-Name 'active-manifests' interpretiert → 404 → Frontend-Sidebar
|
||||||
|
ohne Plugin-Menüeinträge (nur statische Dashboard/Kontakte/System).
|
||||||
|
|
||||||
|
Fix: statische Routen werden vor dynamischen /{name} registriert.
|
||||||
|
"""
|
||||||
|
from fastapi.routing import APIRoute
|
||||||
|
|
||||||
|
from app.routes.plugins import router
|
||||||
|
|
||||||
|
|
||||||
|
def test_active_manifests_registered_before_dynamic_name_route():
|
||||||
|
"""active-manifests muss vor /{name} registriert sein (FastAPI matcht in Reihenfolge)."""
|
||||||
|
paths = [r.path for r in router.routes if isinstance(r, APIRoute)]
|
||||||
|
assert '/api/v1/plugins/active-manifests' in paths, 'active-manifests Route fehlt'
|
||||||
|
assert '/api/v1/plugins/{name}' in paths, '/{name} Route fehlt'
|
||||||
|
assert paths.index('/api/v1/plugins/active-manifests') < paths.index(
|
||||||
|
'/api/v1/plugins/{name}'
|
||||||
|
), (
|
||||||
|
'/active-manifests muss VOR /{name} registriert werden — '
|
||||||
|
'sonst matcht /{name} das Literal und liefert 404 '
|
||||||
|
'(Sidebar ohne Plugin-Menüeinträge)'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_other_literal_routes_also_before_dynamic_name_route():
|
||||||
|
"""Alle literalen Unterrouten müssen vor /{name} registriert sein."""
|
||||||
|
literals = [
|
||||||
|
'/api/v1/plugins/manifest',
|
||||||
|
'/api/v1/plugins/updates',
|
||||||
|
'/api/v1/plugins/active-manifests',
|
||||||
|
]
|
||||||
|
paths = [r.path for r in router.routes if isinstance(r, APIRoute)]
|
||||||
|
name_idx = paths.index('/api/v1/plugins/{name}')
|
||||||
|
for lit in literals:
|
||||||
|
assert lit in paths, f'{lit} fehlt'
|
||||||
|
assert paths.index(lit) < name_idx, f'{lit} muss vor /{{name}} registriert sein'
|
||||||
Reference in New Issue
Block a user