Phase 3 (part 1): manifest-first loader, shopdb.api namespace, auto-register
Hardens the plugin framework so sister-site adoption is safe. Loader rewrite (shopdb/plugins/loader.py): - Reads manifest.json directly. Dependency sort and version checks no longer instantiate plugin classes (avoids __init__ side effects). - Fail-loud policy: in dev/test (DEBUG or TESTING true), plugin errors re-raise. In production, errors log with full context and the plugin is excluded from registration. Framework keeps booting. - Contract-version range check via packaging.SpecifierSet. Plugin's manifest.core_version must include the framework's __contract_version__ or load fails per the policy above. - Manifest validation: required fields (name, version, description), name matches directory, JSON parseable. Exceptions (shopdb/exceptions.py): - PluginNotFoundError, PluginContractError, PluginVersionError, PluginDependencyError. Specific types replace generic Exception swallowing. Auto-register core blueprints (shopdb/__init__.py): - CORE_BLUEPRINT_NAMES tuple drives registration. Adding a core resource is one entry, not three lines (import + register call). - Replaces 27 hand-coded register_blueprint calls. - Asserts each blueprint is exported by shopdb.core.api at boot. Public API namespace (shopdb/api/__init__.py): - audit_log: thin wrapper over AuditLog.log() with stable signature. - resolve_asset_position: implements ADR-001 position resolution (asset > related > location). Asset.mapx/mapy and AssetRelationship.inheritsposition columns are part of the locked contract surface but not yet in models; helper degrades gracefully to location-only fallback until the migration lands. BasePlugin helpers (shopdb/plugins/base.py): - get_setting(key, default), set_setting(key, value, ...). Settings namespaced as plugin.<pluginname>.<key> so two plugins can use the same key without colliding. Manifest version compatibility (plugins/*/manifest.json): - Bumped core_version from ">=1.0.0" to ">=0.1.0,<1.0.0" so all bundled plugins satisfy the new range check. Contract version bump (shopdb/__init__.py): - 0.1.0 -> 0.2.0. Additive surface change (Setting helpers, shopdb.api namespace) per ADR-002 minor-bump rules. Tests (tests/test_plugin_loader.py, tests/test_api_namespace.py): - 13 loader tests: manifest validation failures, version range checks, plugin.py import errors, strict-vs-isolate behavior under TESTING vs production-like config, manifest-first dependency sort. - 8 api-namespace tests: audit_log roundtrip, resolve position fallback chain, plugin.get_setting/set_setting roundtrip with per-plugin namespacing. Test count: 66 -> 87 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
cproudlock
127
tests/test_api_namespace.py
Normal file
127
tests/test_api_namespace.py
Normal file
@@ -0,0 +1,127 @@
|
||||
"""Tests for the public shopdb.api namespace exposed to plugins.
|
||||
|
||||
Pins audit_log, resolve_asset_position, and the BasePlugin
|
||||
get_setting/set_setting helpers as part of the contract surface.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from shopdb.api import audit_log, resolve_asset_position
|
||||
from shopdb.core.models import AuditLog, Setting
|
||||
|
||||
|
||||
def test_audit_log_creates_row(db, client):
|
||||
"""audit_log writes an AuditLog row with the standard fields."""
|
||||
with client.application.test_request_context('/'):
|
||||
entry = audit_log(
|
||||
action='created',
|
||||
entitytype='Computer',
|
||||
entityid=42,
|
||||
entityname='PC-1234',
|
||||
changes={'before': {}, 'after': {'hostname': 'PC-1234'}},
|
||||
)
|
||||
|
||||
assert entry is not None
|
||||
assert entry.action == 'created'
|
||||
assert entry.entitytype == 'Computer'
|
||||
assert entry.entityid == 42
|
||||
assert entry.entityname == 'PC-1234'
|
||||
|
||||
saved = AuditLog.query.filter_by(entityid=42, entitytype='Computer').first()
|
||||
assert saved is not None
|
||||
|
||||
|
||||
def test_resolve_asset_position_returns_none_when_no_data():
|
||||
"""An asset with no coords and no location returns None."""
|
||||
class FakeAsset:
|
||||
mapx = None
|
||||
mapy = None
|
||||
location = None
|
||||
|
||||
assert resolve_asset_position(FakeAsset()) is None
|
||||
|
||||
|
||||
def test_resolve_asset_position_uses_self_when_set():
|
||||
"""Asset-specific coords win over everything else."""
|
||||
class FakeLocation:
|
||||
mapx = 10
|
||||
mapy = 20
|
||||
|
||||
class FakeAsset:
|
||||
mapx = 100
|
||||
mapy = 200
|
||||
location = FakeLocation()
|
||||
|
||||
result = resolve_asset_position(FakeAsset())
|
||||
assert result == {'mapx': 100, 'mapy': 200, 'positionsource': 'self'}
|
||||
|
||||
|
||||
def test_resolve_asset_position_falls_back_to_location():
|
||||
"""When asset has no coords, falls back to location coords."""
|
||||
class FakeLocation:
|
||||
mapx = 50
|
||||
mapy = 75
|
||||
|
||||
class FakeAsset:
|
||||
mapx = None
|
||||
mapy = None
|
||||
location = FakeLocation()
|
||||
|
||||
result = resolve_asset_position(FakeAsset())
|
||||
assert result == {'mapx': 50, 'mapy': 75, 'positionsource': 'location'}
|
||||
|
||||
|
||||
def test_resolve_asset_position_handles_asset_without_mapx_attr():
|
||||
"""Assets that don't yet have mapx/mapy columns degrade gracefully."""
|
||||
class FakeLocation:
|
||||
mapx = 1
|
||||
mapy = 2
|
||||
|
||||
class FakeAsset:
|
||||
location = FakeLocation()
|
||||
|
||||
result = resolve_asset_position(FakeAsset())
|
||||
assert result == {'mapx': 1, 'mapy': 2, 'positionsource': 'location'}
|
||||
|
||||
|
||||
def test_plugin_get_setting_returns_default_when_unset(db, app):
|
||||
"""A plugin reading an unset setting gets the default."""
|
||||
from shopdb.plugins import plugin_manager
|
||||
with app.app_context():
|
||||
printers = plugin_manager.get_plugin('printers')
|
||||
if printers is None:
|
||||
pytest.skip('printers plugin not loaded')
|
||||
assert printers.get_setting('nonexistentkey', default='fallback') == 'fallback'
|
||||
|
||||
|
||||
def test_plugin_set_and_get_setting_roundtrip(db, app):
|
||||
"""A plugin can set and read its own setting; key is namespaced."""
|
||||
from shopdb.plugins import plugin_manager
|
||||
with app.app_context():
|
||||
printers = plugin_manager.get_plugin('printers')
|
||||
if printers is None:
|
||||
pytest.skip('printers plugin not loaded')
|
||||
|
||||
printers.set_setting('zabbix_url', 'http://zabbix.example.com')
|
||||
|
||||
assert printers.get_setting('zabbix_url') == 'http://zabbix.example.com'
|
||||
|
||||
raw = Setting.query.filter_by(key='plugin.printers.zabbix_url').first()
|
||||
assert raw is not None
|
||||
assert raw.value == 'http://zabbix.example.com'
|
||||
|
||||
|
||||
def test_plugin_setting_is_namespaced_per_plugin(db, app):
|
||||
"""Two plugins using the same key do not collide."""
|
||||
from shopdb.plugins import plugin_manager
|
||||
with app.app_context():
|
||||
printers = plugin_manager.get_plugin('printers')
|
||||
computers = plugin_manager.get_plugin('computers')
|
||||
if printers is None or computers is None:
|
||||
pytest.skip('required plugins not loaded')
|
||||
|
||||
printers.set_setting('shared_key', 'printers_value')
|
||||
computers.set_setting('shared_key', 'computers_value')
|
||||
|
||||
assert printers.get_setting('shared_key') == 'printers_value'
|
||||
assert computers.get_setting('shared_key') == 'computers_value'
|
||||
261
tests/test_plugin_loader.py
Normal file
261
tests/test_plugin_loader.py
Normal file
@@ -0,0 +1,261 @@
|
||||
"""Tests for the plugin loader behavior.
|
||||
|
||||
Pins the manifest-first design and the strict-vs-isolate failure policy
|
||||
described in the enforcing-plugin-contract skill.
|
||||
"""
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from shopdb.plugins.loader import PluginLoader
|
||||
from shopdb.plugins.registry import PluginRegistry
|
||||
from shopdb.exceptions import (
|
||||
PluginNotFoundError,
|
||||
PluginContractError,
|
||||
PluginVersionError,
|
||||
PluginDependencyError,
|
||||
PluginError,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_plugins_dir(tmp_path):
|
||||
"""An empty temporary plugins directory."""
|
||||
plugins = tmp_path / 'plugins'
|
||||
plugins.mkdir()
|
||||
return plugins
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_registry(tmp_path):
|
||||
"""An empty temporary registry."""
|
||||
return PluginRegistry(tmp_path / 'plugins.json')
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def loader(temp_plugins_dir, temp_registry):
|
||||
"""A loader pointed at the temporary plugins dir."""
|
||||
return PluginLoader(temp_plugins_dir, temp_registry)
|
||||
|
||||
|
||||
def write_plugin(plugins_dir, name, manifest=None, plugin_py=None):
|
||||
"""Helper: write a plugin directory with manifest.json and plugin.py."""
|
||||
plugin_dir = plugins_dir / name
|
||||
plugin_dir.mkdir()
|
||||
|
||||
if manifest is not None:
|
||||
(plugin_dir / 'manifest.json').write_text(json.dumps(manifest))
|
||||
|
||||
if plugin_py is not None:
|
||||
(plugin_dir / 'plugin.py').write_text(plugin_py)
|
||||
|
||||
return plugin_dir
|
||||
|
||||
|
||||
VALID_PLUGIN_PY = '''
|
||||
from shopdb.plugins.base import BasePlugin, PluginMeta
|
||||
|
||||
|
||||
class FakePlugin(BasePlugin):
|
||||
@property
|
||||
def meta(self):
|
||||
return PluginMeta(
|
||||
name='fake',
|
||||
version='1.0.0',
|
||||
description='Fake test plugin',
|
||||
)
|
||||
|
||||
def get_blueprint(self):
|
||||
return None
|
||||
|
||||
def get_models(self):
|
||||
return []
|
||||
'''
|
||||
|
||||
|
||||
def test_load_manifest_raises_on_missing_manifest(loader, temp_plugins_dir):
|
||||
"""A plugin without manifest.json raises PluginNotFoundError."""
|
||||
plugin_dir = temp_plugins_dir / 'noplugin'
|
||||
plugin_dir.mkdir()
|
||||
(plugin_dir / 'plugin.py').write_text('# empty')
|
||||
|
||||
with pytest.raises(PluginNotFoundError, match='manifest.json'):
|
||||
loader.load_manifest('noplugin')
|
||||
|
||||
|
||||
def test_load_manifest_raises_on_invalid_json(loader, temp_plugins_dir):
|
||||
"""An unparseable manifest.json raises PluginContractError."""
|
||||
plugin_dir = temp_plugins_dir / 'bad'
|
||||
plugin_dir.mkdir()
|
||||
(plugin_dir / 'manifest.json').write_text('{not json}')
|
||||
|
||||
with pytest.raises(PluginContractError, match='invalid manifest.json'):
|
||||
loader.load_manifest('bad')
|
||||
|
||||
|
||||
def test_load_manifest_raises_on_missing_required_fields(loader, temp_plugins_dir):
|
||||
"""Missing name/version/description raises PluginContractError."""
|
||||
write_plugin(temp_plugins_dir, 'incomplete', manifest={'name': 'incomplete'})
|
||||
|
||||
with pytest.raises(PluginContractError, match='missing required field'):
|
||||
loader.load_manifest('incomplete')
|
||||
|
||||
|
||||
def test_load_manifest_raises_on_name_mismatch(loader, temp_plugins_dir):
|
||||
"""manifest.name must match the directory name."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'mydir',
|
||||
manifest={'name': 'different', 'version': '1.0.0', 'description': 'x'},
|
||||
)
|
||||
|
||||
with pytest.raises(PluginContractError, match='does not match manifest name'):
|
||||
loader.load_manifest('mydir')
|
||||
|
||||
|
||||
def test_check_contract_version_passes_when_in_range(loader, temp_plugins_dir):
|
||||
"""A core_version range that covers the framework version passes."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'compat',
|
||||
manifest={
|
||||
'name': 'compat',
|
||||
'version': '1.0.0',
|
||||
'description': 'x',
|
||||
'core_version': '>=0.1.0,<1.0.0',
|
||||
},
|
||||
)
|
||||
loader.check_contract_version('compat', '0.1.0')
|
||||
|
||||
|
||||
def test_check_contract_version_raises_when_out_of_range(loader, temp_plugins_dir):
|
||||
"""A core_version range that excludes the framework version raises."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'incompat',
|
||||
manifest={
|
||||
'name': 'incompat',
|
||||
'version': '1.0.0',
|
||||
'description': 'x',
|
||||
'core_version': '>=2.0.0',
|
||||
},
|
||||
)
|
||||
|
||||
with pytest.raises(PluginVersionError, match='requires core_version'):
|
||||
loader.check_contract_version('incompat', '0.1.0')
|
||||
|
||||
|
||||
def test_check_contract_version_skipped_when_unspecified(loader, temp_plugins_dir):
|
||||
"""A manifest without core_version does not raise."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'unspec',
|
||||
manifest={
|
||||
'name': 'unspec',
|
||||
'version': '1.0.0',
|
||||
'description': 'x',
|
||||
},
|
||||
)
|
||||
loader.check_contract_version('unspec', '0.1.0')
|
||||
|
||||
|
||||
def test_load_plugin_class_raises_on_missing_plugin_py(loader, temp_plugins_dir):
|
||||
"""A directory with no plugin.py raises PluginNotFoundError."""
|
||||
plugin_dir = temp_plugins_dir / 'nofile'
|
||||
plugin_dir.mkdir()
|
||||
|
||||
with pytest.raises(PluginNotFoundError, match='plugin.py not found'):
|
||||
loader.load_plugin_class('nofile')
|
||||
|
||||
|
||||
def test_load_plugin_class_raises_when_no_subclass_found(loader, temp_plugins_dir):
|
||||
"""A plugin.py without a BasePlugin subclass raises PluginContractError."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'empty',
|
||||
manifest={'name': 'empty', 'version': '1.0.0', 'description': 'x'},
|
||||
plugin_py='# no plugin defined',
|
||||
)
|
||||
|
||||
with pytest.raises(PluginContractError, match='no BasePlugin subclass'):
|
||||
loader.load_plugin_class('empty')
|
||||
|
||||
|
||||
def test_load_plugin_class_raises_on_import_error(loader, temp_plugins_dir):
|
||||
"""A plugin.py with a syntax/import error raises PluginContractError."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'broken',
|
||||
manifest={'name': 'broken', 'version': '1.0.0', 'description': 'x'},
|
||||
plugin_py='import nonexistent_module_xyz',
|
||||
)
|
||||
|
||||
with pytest.raises(PluginContractError, match='import failed'):
|
||||
loader.load_plugin_class('broken')
|
||||
|
||||
|
||||
def test_sort_by_dependencies_uses_manifest_not_instantiation(loader, temp_plugins_dir):
|
||||
"""Topological sort reads manifest.json directly, never instantiates."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'a',
|
||||
manifest={
|
||||
'name': 'a',
|
||||
'version': '1.0.0',
|
||||
'description': 'x',
|
||||
'dependencies': ['b'],
|
||||
},
|
||||
)
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'b',
|
||||
manifest={'name': 'b', 'version': '1.0.0', 'description': 'x'},
|
||||
)
|
||||
|
||||
sorted_names = loader._sort_by_dependencies(['a', 'b'])
|
||||
assert sorted_names.index('b') < sorted_names.index('a')
|
||||
|
||||
|
||||
def test_load_plugin_strict_mode_reraises(loader, app, temp_plugins_dir):
|
||||
"""In TESTING/DEBUG mode, load_plugin re-raises plugin errors."""
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'incompat',
|
||||
manifest={
|
||||
'name': 'incompat',
|
||||
'version': '1.0.0',
|
||||
'description': 'x',
|
||||
'core_version': '>=99.0.0',
|
||||
},
|
||||
plugin_py=VALID_PLUGIN_PY,
|
||||
)
|
||||
|
||||
assert app.config.get('TESTING') is True
|
||||
with pytest.raises(PluginVersionError):
|
||||
loader.load_plugin('incompat', app, None)
|
||||
|
||||
|
||||
def test_load_plugin_production_mode_isolates(loader, temp_plugins_dir):
|
||||
"""In production-like config, load_plugin returns None on failure."""
|
||||
from flask import Flask
|
||||
|
||||
fake_app = Flask(__name__)
|
||||
fake_app.config['TESTING'] = False
|
||||
fake_app.config['DEBUG'] = False
|
||||
|
||||
write_plugin(
|
||||
temp_plugins_dir,
|
||||
'incompat',
|
||||
manifest={
|
||||
'name': 'incompat',
|
||||
'version': '1.0.0',
|
||||
'description': 'x',
|
||||
'core_version': '>=99.0.0',
|
||||
},
|
||||
plugin_py=VALID_PLUGIN_PY,
|
||||
)
|
||||
|
||||
result = loader.load_plugin('incompat', fake_app, None)
|
||||
assert result is None
|
||||
Reference in New Issue
Block a user