ADR-013 Phase 0: plugin lifecycle groundwork
Additive, zero-risk-to-running-sites prep for the plugin catalog. No distribution or lean-build behavior yet; fixes latent bugs and adds the declarative + validate tooling later phases build on. Fixes: - upgrade_all_plugins iterates registry.get_all(); only adopted plugins are migrated. Removes the phantom hasattr(registry, 'list_installed') probe that always fell through to migrating every folder on disk (unadopted DDL ran with full DB rights on every deploy). - Reverse-dependency checks on uninstall/disable read dependencies from the manifest on disk via _installed_dependents, so an installed-but-unloaded or disabled dependent is counted. Uninstall blocks on any installed dependent; disable blocks on an enabled dependent. - _sort_by_dependencies detects a dependency cycle (back edge in the DFS) and raises PluginDependencyError instead of looping or dropping a plugin. New: - flask plugin validate <name>: manifest loads + name match, manifest-schema check, core_version admits the framework contract, declared dependencies exist on disk. No new dependency (lightweight checker); schema ships in the package at shopdb/plugins/manifest_schema.json (docs/ is stripped on publish). The check caught that provides is an object, not an array. - flask plugin apply-profile <file>: declarative install AND enable of a chosen plugin set plus its hard-dependency closure, in dependency order, idempotent. Replaces the hand-ordered runbook sequences that could enable a plugin that was never installed. deploy/site-profile.example.json template. - Dockerfile header corrected (all 13 catalog plugins, not "eleven core"). 10 new lifecycle tests (reverse-deps from disk, cycle detection, upgrade-all scope, profile closure, schema, all 13 manifests match schema). 1018 pass, naming green.
This commit is contained in:
@@ -9,6 +9,7 @@ from .base import BasePlugin, PluginMeta
|
||||
from .registry import PluginRegistry, PluginState
|
||||
from .loader import PluginLoader
|
||||
from .migrations import PluginMigrationManager
|
||||
from ..exceptions import PluginError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -91,11 +92,13 @@ class PluginManager:
|
||||
results: Dict[str, str] = {}
|
||||
if not self.migration_manager:
|
||||
return results
|
||||
plugin_names = list(self.registry.list_installed().keys()) \
|
||||
if hasattr(self.registry, 'list_installed') else []
|
||||
if not plugin_names:
|
||||
# Fall back to whatever the loader found on disk.
|
||||
plugin_names = self.loader.discover_plugins()
|
||||
# Only ADOPTED plugins (those in the registry) get migrated. A plugin
|
||||
# folder that merely sits on disk unadopted must not have its DDL run as
|
||||
# a side effect of a deploy - migrations execute with full DB rights, so
|
||||
# "on disk" is not "trusted to run". (The old code probed a
|
||||
# list_installed() method the registry never had, so it always fell
|
||||
# through to discover_plugins() and migrated every folder.)
|
||||
plugin_names = list(self.registry.get_all().keys())
|
||||
for name in plugin_names:
|
||||
if not self.migration_manager.has_pending_migrations(name):
|
||||
results[name] = 'no-migrations'
|
||||
@@ -254,6 +257,30 @@ class PluginManager:
|
||||
"Seeded %d permission(s) for plugin %s",
|
||||
created, plugin.meta.name)
|
||||
|
||||
def _installed_dependents(self, name: str,
|
||||
enabled_only: bool = False) -> List[str]:
|
||||
"""Installed plugins that declare `name` as a hard dependency.
|
||||
|
||||
Reads each dependency list from the manifest on DISK, not from a loaded
|
||||
plugin instance, so a dependent that is installed-but-not-loaded (e.g.
|
||||
disabled, or failed to load) is still counted. enabled_only restricts to
|
||||
currently-enabled dependents (what matters when disabling, since a
|
||||
disabled dependent is not running).
|
||||
"""
|
||||
dependents = []
|
||||
for other_name, state in self.registry.get_all().items():
|
||||
if other_name == name:
|
||||
continue
|
||||
if enabled_only and not state.enabled:
|
||||
continue
|
||||
try:
|
||||
manifest = self.loader.load_manifest(other_name)
|
||||
except PluginError:
|
||||
continue
|
||||
if name in manifest.get('dependencies', []):
|
||||
dependents.append(other_name)
|
||||
return dependents
|
||||
|
||||
def uninstall_plugin(self, name: str, remove_data: bool = False) -> bool:
|
||||
"""
|
||||
Uninstall a plugin.
|
||||
@@ -266,16 +293,13 @@ class PluginManager:
|
||||
logger.warning(f"Plugin {name} is not installed")
|
||||
return False
|
||||
|
||||
# Check if other plugins depend on this one
|
||||
for other_name in self.registry.get_enabled_plugins():
|
||||
if other_name == name:
|
||||
continue
|
||||
other_plugin = self.loader.get_loaded_plugin(other_name)
|
||||
if other_plugin and name in other_plugin.meta.dependencies:
|
||||
logger.error(
|
||||
f"Cannot uninstall {name}: {other_name} depends on it"
|
||||
)
|
||||
return False
|
||||
# Refuse if any INSTALLED plugin (enabled or not) depends on this one.
|
||||
dependents = self._installed_dependents(name)
|
||||
if dependents:
|
||||
logger.error(
|
||||
f"Cannot uninstall {name}: {', '.join(dependents)} depend(s) on it"
|
||||
)
|
||||
return False
|
||||
|
||||
# Get plugin instance
|
||||
plugin = self.loader.get_loaded_plugin(name)
|
||||
@@ -348,16 +372,14 @@ class PluginManager:
|
||||
logger.info(f"Plugin {name} is already disabled")
|
||||
return True
|
||||
|
||||
# Check if other plugins depend on this one
|
||||
for other_name in self.registry.get_enabled_plugins():
|
||||
if other_name == name:
|
||||
continue
|
||||
other_plugin = self.loader.get_loaded_plugin(other_name)
|
||||
if other_plugin and name in other_plugin.meta.dependencies:
|
||||
logger.error(
|
||||
f"Cannot disable {name}: {other_name} depends on it"
|
||||
)
|
||||
return False
|
||||
# Refuse if any ENABLED plugin depends on this one (a disabled dependent
|
||||
# is not running, so it does not block). Manifests read from disk.
|
||||
dependents = self._installed_dependents(name, enabled_only=True)
|
||||
if dependents:
|
||||
logger.error(
|
||||
f"Cannot disable {name}: {', '.join(dependents)} depend(s) on it"
|
||||
)
|
||||
return False
|
||||
|
||||
plugin = self.loader.get_loaded_plugin(name)
|
||||
if plugin:
|
||||
@@ -367,6 +389,77 @@ class PluginManager:
|
||||
logger.info(f"Disabled plugin: {name}")
|
||||
return True
|
||||
|
||||
def _dependency_closure(self, names: List[str]) -> List[str]:
|
||||
"""Chosen plugins plus their hard-dependency closure, dependencies first.
|
||||
|
||||
Post-order DFS over manifest `dependencies`, so the returned order is
|
||||
safe to install/enable top to bottom (a dependency always precedes the
|
||||
plugin that needs it). Deduplicated.
|
||||
"""
|
||||
closure: List[str] = []
|
||||
seen = set()
|
||||
|
||||
def add(name):
|
||||
if name in seen:
|
||||
return
|
||||
seen.add(name)
|
||||
try:
|
||||
deps = self.loader.load_manifest(name).get('dependencies', [])
|
||||
except PluginError:
|
||||
deps = []
|
||||
for dep in deps:
|
||||
add(dep)
|
||||
closure.append(name)
|
||||
|
||||
for name in names:
|
||||
add(name)
|
||||
return closure
|
||||
|
||||
def apply_profile(self, plugins: List[str],
|
||||
locked: Optional[List[str]] = None) -> Dict[str, List[str]]:
|
||||
"""Install AND enable exactly the chosen plugins plus their dependency
|
||||
closure, in dependency order.
|
||||
|
||||
Declarative replacement for the hand-ordered install/enable sequences in
|
||||
the deploy runbooks (which could enable a plugin that was never
|
||||
installed). Idempotent: already-installed/enabled plugins are left as-is.
|
||||
This does NOT remove anything absent from the list - removal stays an
|
||||
explicit, separate operation. `locked` is accepted for forward
|
||||
compatibility (a future guard against removing a site-mandated plugin)
|
||||
and is currently informational.
|
||||
|
||||
Raises PluginError if the profile names a plugin (or pulls in a
|
||||
dependency) that does not exist on disk. Returns
|
||||
{installed: [...], enabled: [...], already: [...]}.
|
||||
"""
|
||||
available = set(self.loader.discover_plugins())
|
||||
unknown = [p for p in plugins if p not in available]
|
||||
if unknown:
|
||||
raise PluginError(
|
||||
f"Profile names unknown plugins: {', '.join(sorted(unknown))}")
|
||||
|
||||
order = self._dependency_closure(plugins)
|
||||
missing_deps = [p for p in order if p not in available]
|
||||
if missing_deps:
|
||||
raise PluginError(
|
||||
f"Missing dependency plugins: {', '.join(sorted(missing_deps))}")
|
||||
|
||||
result: Dict[str, List[str]] = {
|
||||
'installed': [], 'enabled': [], 'already': []}
|
||||
for name in order:
|
||||
changed = False
|
||||
if not self.registry.is_installed(name):
|
||||
if self.install_plugin(name):
|
||||
result['installed'].append(name)
|
||||
changed = True
|
||||
if not self.registry.is_enabled(name):
|
||||
if self.enable_plugin(name):
|
||||
result['enabled'].append(name)
|
||||
changed = True
|
||||
if not changed:
|
||||
result['already'].append(name)
|
||||
return result
|
||||
|
||||
def get_plugin(self, name: str) -> Optional[BasePlugin]:
|
||||
"""Get a loaded plugin instance."""
|
||||
return self.loader.get_loaded_plugin(name)
|
||||
|
||||
Reference in New Issue
Block a user