Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/sep/api/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def build_apps_router(registry: AppRegistry) -> APIRouter:
plugin_deps = (
[]
if app.state_key in PROTECTED_APP_KEYS
else [Depends(require_app_enabled(app.state_key))]
else [Depends(require_app_enabled(app.key))]
)
apps_router.include_router(
app.api_router,
Expand Down
21 changes: 11 additions & 10 deletions app/sep/api/routes/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@
from app.sep.apps.framework.registry import get_app_registry
from app.sep.apps.nav_icons import NavIcon
from app.sep.crud import AppStateManager
from app.sep.deps import PROTECTED_APP_KEYS, SessionDep
from app.sep.models import AppLifecycleEnum
from app.sep.deps import SessionDep

router = APIRouter(tags=["apps"])
APPS_ROUTE_PREFIX = "/apps"
Expand Down Expand Up @@ -88,21 +87,23 @@ async def list_apps_for_navigation(session: SessionDep) -> list[AppKeyResponse]:
"""Return per-app state for the current user's navigation.

Protected apps are always reported ``enabled=True``. Every other app reflects
the DB state of the row governing it (a missing row -> ``enabled=True``: a
configured plugin is active until explicitly disabled). A child app owns no
row, so it resolves through its parent via
:attr:`~app.sep.apps.framework.base.BaseApp.state_key`.
its *effective* state via :meth:`AppRegistry.resolve_effective_enabled`: its
own row must be ``ENABLED`` (a missing row -> ``enabled=True``: a configured
plugin is active until explicitly disabled) **and** every app it declares in
``requires_apps`` must be effectively enabled, so the shell hides an app when
a dependency is off. A child app owns no row, so it resolves through its
parent via :attr:`~app.sep.apps.framework.base.BaseApp.state_key`.

:param session: The database session.
:return: The per-app navigation list.
"""
states = await AppStateManager.all_lifecycle_states(session)
registry = get_app_registry()
memo: dict[str, bool] = {}
return [
AppKeyResponse(
app_key=app.key,
enabled=app.state_key in PROTECTED_APP_KEYS
or states.get(app.state_key, AppLifecycleEnum.ENABLED)
== AppLifecycleEnum.ENABLED,
enabled=registry.resolve_effective_enabled(app.key, states, memo),
sidebar=app.sidebar,
uri_path=app.uri_path,
display_name=app.display_name,
Expand All @@ -112,5 +113,5 @@ async def list_apps_for_navigation(session: SessionDep) -> list[AppKeyResponse]:
react_route=build_navigation_react_route(app.key, app.react_route),
nav_icon=app.nav_icon,
)
for app in get_app_registry()
for app in registry
]
1 change: 1 addition & 0 deletions app/sep/apps/atw/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,5 @@
nav_icon=NavIcon.SUPPORT_AGENT,
api_router=api_router,
schema=atw_schema,
requires_apps=("snippets",),
)
8 changes: 8 additions & 0 deletions app/sep/apps/framework/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ class BaseApp(BaseModel):
registry appends each right after this app and stamps its ``enabled`` from
this app's, so a child is mounted and snapshotted exactly when its parent
is. Each child must set ``parent_key`` to this app's key.
:param requires_apps: App **keys** (not display names) this app functionally
depends on. An app's *effective* enabled state ANDs its own ``AppState``
with the effective state of every app named here, so a disabled dependency
hides and gates this app too. Depending on a protected app (one that can
never be disabled) is a harmless no-op. Keys are validated at
registry-build time: an unknown key, a self-dependency, or a cycle fails
fast. See :meth:`AppRegistry.resolve_effective_enabled`.
"""

model_config = ConfigDict(arbitrary_types_allowed=True, populate_by_name=True)
Expand All @@ -88,6 +95,7 @@ class BaseApp(BaseModel):
app_schema: AppSchema | None = Field(default=None, alias="schema")
parent_key: str | None = None
child_apps: tuple["BaseApp", ...] = ()
requires_apps: tuple[str, ...] = ()

@model_validator(mode="before")
@classmethod
Expand Down
164 changes: 162 additions & 2 deletions app/sep/apps/framework/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
imports through plugin ``__init__`` modules.
"""

from collections.abc import Iterable, Iterator
from collections.abc import Iterable, Iterator, Mapping
from functools import lru_cache
from importlib import import_module

Expand All @@ -47,6 +47,7 @@
from app.sep.config import App, sep_settings
from app.sep.crud import AppStateManager
from app.sep.deps import PROTECTED_APP_KEYS
from app.sep.models import AppLifecycleEnum


class AppRegistry:
Expand All @@ -57,11 +58,170 @@ class AppRegistry:

:param apps: The mounted apps, in activation order.
:type apps: list[BaseApp]
:raises ValueError: When two apps share a key, or a ``requires_apps`` entry
names an unknown app, itself, or forms a dependency cycle.
"""

def __init__(self, apps: list[BaseApp]) -> None:
self._apps = apps
self._by_key = {app.key: app for app in apps}
# Explicit loop, not a comprehension: a duplicate key must raise, not
# silently overwrite, or a ``requires_apps`` reference turns ambiguous.
self._by_key: dict[str, BaseApp] = {}
for app in apps:
if app.key in self._by_key:
raise ValueError(
f"Duplicate app key {app.key!r}: two apps cannot share a"
" key, or a requires_apps reference would be ambiguous.",
)
self._by_key[app.key] = app
self._validate_dependencies()

def _validate_dependencies(self) -> None:
"""Reject self-dependencies, dangling deps, and cycles at build time.

:raises ValueError: When a ``requires_apps`` entry names the app itself,
names an unregistered key, participates in a dependency cycle, or is
declared alongside ``child_apps`` (unsupported -- children resolve
their own state through the parent's ``state_key`` but do not inherit
the parent's ``requires_apps``, so gating the parent would leave its
children reachable).
"""
for app in self._apps:
if app.child_apps and app.requires_apps:
raise ValueError(
f"App {app.key!r} combines child_apps with requires_apps, "
"which is not supported.",
)
for dep_key in app.requires_apps:
if dep_key == app.key:
raise ValueError(
f"App {app.key!r} cannot depend on itself.",
)
if dep_key not in self._by_key:
raise ValueError(
f"App {app.key!r} requires unknown app key {dep_key!r}.",
)
self._assert_acyclic()

def _assert_acyclic(self) -> None:
"""Raise when the ``requires_apps`` graph contains a cycle.

:raises ValueError: When a dependency cycle is detected.
"""
white, gray, black = 0, 1, 2
color = {app.key: white for app in self._apps}

def visit(key: str, path: list[str]) -> None:
color[key] = gray
for dep_key in self._by_key[key].requires_apps:
if color[dep_key] == gray:
chain = " -> ".join([*path, key, dep_key])
raise ValueError(f"App dependency cycle detected: {chain}.")
if color[dep_key] == white:
visit(dep_key, [*path, key])
color[key] = black

for app in self._apps:
if color[app.key] == white:
visit(app.key, [])

def resolve_effective_enabled(
self,
key: str,
states: Mapping[str, AppLifecycleEnum],
memo: dict[str, bool] | None = None,
) -> bool:
"""Return whether ``key``'s app is effectively enabled.

An app is effective-enabled when its own ``AppState`` is ``ENABLED``
**and** every app in its ``requires_apps`` is itself effective-enabled
(resolved transitively). This is the single resolver the mount gate, the
sidebar filter, and the ``GET /api/apps`` projection all share, so they
cannot drift. A protected app (or dependency) is always treated as
enabled; a missing ``AppState`` row defaults to ``ENABLED``.

:param key: The registry key of the app to resolve.
:param states: A ``{state_key: lifecycle}`` map (e.g. from
:meth:`AppStateManager.all_lifecycle_states`).
:param memo: An optional ``{key: bool}`` cache shared across a
full-registry projection so shared dependency subtrees are walked
once rather than re-walked per app. Only reuse a memo within a
single ``states`` snapshot; a fresh ``states`` needs a fresh memo.
:return: ``True`` when the app and every dependency are enabled.
"""
app = self._by_key.get(key)
if app is None:
return False
return self._effective_enabled(app, states, frozenset(), memo)

def _effective_enabled(
self,
app: BaseApp,
states: Mapping[str, AppLifecycleEnum],
stack: frozenset[str],
memo: dict[str, bool] | None = None,
) -> bool:
"""Resolve ``app``'s effective-enabled state along a DFS path.

:param app: The app being resolved.
:param states: The ``{state_key: lifecycle}`` map.
:param stack: The keys on the current recursion path (cycle guard).
:param memo: An optional ``{key: bool}`` cache; a resolved result is
stored under ``app.key`` and reused on the next visit.
:return: ``True`` when the app and every dependency are enabled.
"""
if memo is not None and app.key in memo:
return memo[app.key]
result = self._resolve_effective(app, states, stack, memo)
if memo is not None:
memo[app.key] = result
return result

def _resolve_effective(
self,
app: BaseApp,
states: Mapping[str, AppLifecycleEnum],
stack: frozenset[str],
memo: dict[str, bool] | None,
) -> bool:
"""Compute ``app``'s effective-enabled state (the un-memoized body).

:param app: The app being resolved.
:param states: The ``{state_key: lifecycle}`` map.
:param stack: The keys on the current recursion path (cycle guard).
:param memo: The projection cache threaded into dependency resolution.
:return: ``True`` when the app and every dependency are enabled.
"""
if app.state_key in PROTECTED_APP_KEYS:
# Can never be disabled, so return True without recursing into its
# own ``requires_apps``.
return True
if not self._own_enabled(app, states):
return False
if app.key in stack:
# Defensive: the build rejects cycles, so this is unreachable. Fail
# closed -- an unexpected cycle gates the app off, never on.
return False
stack = stack | {app.key}
for dep_key in app.requires_apps:
Comment thread
marcuscruz-percona marked this conversation as resolved.
dep = self._by_key.get(dep_key)
if dep is None or not self._effective_enabled(dep, states, stack, memo):
return False
return True

@staticmethod
def _own_enabled(app: BaseApp, states: Mapping[str, AppLifecycleEnum]) -> bool:
"""Return whether ``app``'s own ``AppState`` is enabled (ignoring deps).

:param app: The app whose own state is inspected.
:param states: The ``{state_key: lifecycle}`` map.
:return: ``True`` when protected or the row is ``ENABLED`` (or absent).
"""
return (
app.state_key in PROTECTED_APP_KEYS
or states.get(app.state_key, AppLifecycleEnum.ENABLED)
== AppLifecycleEnum.ENABLED
)

def __iter__(self) -> Iterator[BaseApp]:
return iter(self._apps)
Expand Down
40 changes: 25 additions & 15 deletions app/sep/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
CSRF_FORM_FIELD,
request_has_bearer_authorization,
)
from app.sep.models import AppLifecycleEnum, SyncInventoryEntityTypeEnum
from app.sep.models import SyncInventoryEntityTypeEnum
from app.tasks.config import tasks_settings
from app.tasks.models import (
Task,
Expand Down Expand Up @@ -483,7 +483,12 @@ def require_app_enabled(app_key: str) -> Callable[[AsyncSession], Awaitable[None
Used as ``dependencies=[Depends(require_app_enabled(<key>))]`` on each
non-protected app's router at mount time. Raises
:class:`app.core.exceptions.HTTPServiceUnavailableException` (HTTP 503) when
the app is disabled in :class:`app.sep.models.AppState`.
the app is not *effectively* enabled -- that is, when its own
:class:`app.sep.models.AppState` is disabled **or** any app it declares in
``requires_apps`` is disabled. Resolution is delegated to
:meth:`AppRegistry.resolve_effective_enabled`, the single source of truth
shared with the sidebar filter and the ``GET /api/apps`` projection, so the
gate is passed the app's ``key`` (not ``state_key``) to resolve dependencies.

The factory closure-captures ``app_key`` at router-mount time; the returned
coroutine is invoked per request and queries the DB via the standard
Expand All @@ -503,16 +508,20 @@ def require_app_enabled(app_key: str) -> Callable[[AsyncSession], Awaitable[None
"""

async def _gate(session: SessionDep) -> None:
# Deferred: the framework package __init__ imports back into this module,
# so a top-level import here would cycle.
from app.sep.apps.framework.registry import get_app_registry

try:
enabled = await AppStateManager.is_enabled(session, app_key)
states = await AppStateManager.all_lifecycle_states(session)
except SQLAlchemyError:
logger.warning(
"Could not read app state for '%s'; allowing the request.",
app_key,
exc_info=True,
)
return
if not enabled:
if not get_app_registry().resolve_effective_enabled(app_key, states):
raise HTTPServiceUnavailableException(
detail=f"App '{app_key}' is currently disabled.",
)
Expand Down Expand Up @@ -575,13 +584,14 @@ async def get_default_context(
) -> dict[str, Any]:
"""Return the default context for templates.

The sidebar ``plugins`` list is filtered by runtime app state: protected
apps always pass through; every other app is shown unless the
:class:`app.sep.models.AppState` row governing it has
``lifecycle_state != ENABLED`` (a missing row is treated as enabled). A child
app owns no row, so it resolves through its parent via
:attr:`~app.sep.apps.framework.base.BaseApp.state_key`. This is the single
source of truth that drives sidebar visibility.
The sidebar ``plugins`` list is filtered by *effective* app state via
:meth:`AppRegistry.resolve_effective_enabled`: protected apps always pass
through; every other app is shown only when its own
:class:`app.sep.models.AppState` row is ``ENABLED`` (a missing row is treated
as enabled) **and** every app it declares in ``requires_apps`` is itself
effectively enabled. A child app owns no row, so it resolves through its
parent via :attr:`~app.sep.apps.framework.base.BaseApp.state_key`. This
shares the one resolver used by the mount gate and the JSON app listing.

:param request: The HTTP request object.
:param user: The authenticated user.
Expand All @@ -603,12 +613,12 @@ async def get_default_context(
# so a top-level import here would cycle.
from app.sep.apps.framework.registry import get_app_registry

registry = get_app_registry()
memo: dict[str, bool] = {}
plugins = [
app
for app in get_app_registry()
if app.state_key in PROTECTED_APP_KEYS
or states.get(app.state_key, AppLifecycleEnum.ENABLED)
== AppLifecycleEnum.ENABLED
for app in registry
if registry.resolve_effective_enabled(app.key, states, memo)
]
return {
"user": user,
Expand Down
2 changes: 1 addition & 1 deletion app/sep/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ async def sep_lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
plugin_deps = (
[]
if app.state_key in PROTECTED_APP_KEYS
else [Depends(require_app_enabled(app.state_key))]
else [Depends(require_app_enabled(app.key))]
)
sep_app.include_router(
app.jinja_router, prefix=app.uri_path, dependencies=plugin_deps
Expand Down
1 change: 1 addition & 0 deletions changelog.d/SEP-1432.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
atw is hidden and returns 503 when the snippets app it depends on is disabled, via a new requires_apps cross-app dependency primitive
2 changes: 1 addition & 1 deletion frontend/packages/api/specs/sep.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions frontend/packages/api/src/generated/sep.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading