Decouple core from plugins; wire widgets hook; drop dead search hook
Architectural pass from the skill review ("plugin is the product" boundary).
Core no longer imports plugin models at module load (was a hard import-time
dependency that broke core if the computers plugin was absent/disabled):
- collector.py, applications.py, reports.py: lazy + guarded imports of the
computers plugin models. Endpoints that need install-tracking now return 503
when the plugin is absent instead of failing at import.
Search honors runtime enable/disable:
- search.py: _require_enabled(name) raises ImportError for a disabled plugin,
so each plugin-scoped block skips it (a disabled plugin's rows leave search).
- Replace hardcoded root/rootpassword employee-DB connection in _search_employees
with the shared env-backed employee_connection helper.
Plugin hooks (integrating-plugin-hooks: every hook needs a consumer):
- get_dashboard_widgets: add the consumer GET /api/dashboard/widgets (5 plugins
already implemented the hook; it had none). Skips disabled, isolates in prod.
- get_searchable_fields: REMOVED. Zero plugins implemented it and there was no
consumer; global search is a core concern over the asset model. Contract
reduction, __contract_version__ 0.3.0 -> 0.4.0.
Docs/contract: PLUGIN-HOOKS.md (widgets consumer note, searchable-fields removal,
0.4.0), PLUGIN-QUICKSTART.md, ADR-001 hook list. Tests: widgets endpoint
aggregate + disabled-skip; contract tests for the removed/added hooks.
151 tests pass, naming/style green, app boots all 6 plugins.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -9,7 +9,7 @@ The contract is locked in [ADR-001](../docs/adr/ADR-001-asset-as-platform-contra
|
||||
The framework declares its contract version in `shopdb/__init__.py`:
|
||||
|
||||
```python
|
||||
__contract_version__ = '0.3.0'
|
||||
__contract_version__ = '0.4.0'
|
||||
```
|
||||
|
||||
Each plugin's `manifest.json` declares the range of contract versions it supports:
|
||||
@@ -166,6 +166,10 @@ class NotificationsPlugin(BasePlugin):
|
||||
}]
|
||||
```
|
||||
|
||||
Consumed by `GET /api/dashboard/widgets`, which merges widgets from all enabled
|
||||
plugins sorted by `position` (disabled plugins are skipped; a broken plugin is
|
||||
isolated in prod, re-raised in dev/test).
|
||||
|
||||
### `get_navigation_items() -> List[Dict]`
|
||||
|
||||
Returns navigation menu items.
|
||||
@@ -181,24 +185,10 @@ class ComputersPlugin(BasePlugin):
|
||||
}]
|
||||
```
|
||||
|
||||
### `get_searchable_fields() -> List[Dict]`
|
||||
|
||||
Declares fields the plugin contributes to global search.
|
||||
|
||||
```python
|
||||
from .models import Computer
|
||||
|
||||
class ComputersPlugin(BasePlugin):
|
||||
def get_searchable_fields(self):
|
||||
return [{
|
||||
'model': Computer,
|
||||
'search_fields': ['hostname', 'serialnumber', 'currentuser'],
|
||||
'result_type': 'computer',
|
||||
'url_template': '/computers/{id}',
|
||||
'title_field': 'hostname',
|
||||
'subtitle_field': 'currentuser',
|
||||
}]
|
||||
```
|
||||
> Removed in contract 0.4.0: `get_searchable_fields`. Global search
|
||||
> (`/api/search`) is a core concern that queries the asset model directly and
|
||||
> already covers every bundled asset type; no plugin ever implemented the hook.
|
||||
> Search honors runtime plugin enable/disable.
|
||||
|
||||
### `get_collector_schema() -> Optional[Dict]`
|
||||
|
||||
|
||||
@@ -119,10 +119,9 @@ Override hooks on the plugin class as needed. See [PLUGIN-HOOKS.md](PLUGIN-HOOKS
|
||||
|
||||
| Hook | Adds |
|
||||
|------|------|
|
||||
| `get_searchable_fields` | Plugin contributes to the global search endpoint |
|
||||
| `get_navigation_items` | Plugin shows up in the sidebar nav |
|
||||
| `get_dashboard_widgets` | Plugin's dashboard widget appears on the home page |
|
||||
| `get_collector_schema` | Plugin accepts external pushes at `/api/collector/<name>` |
|
||||
| `get_collector_schema` + `apply_collector_payload` | Plugin accepts external pushes at `/api/collector/<name>` |
|
||||
|
||||
Each hook has a default that does nothing. Override only what your plugin needs.
|
||||
|
||||
|
||||
@@ -58,7 +58,7 @@ Adding a name here is a minor (additive) contract change; removing one is major.
|
||||
|
||||
#### Plugin contract
|
||||
|
||||
- `BasePlugin` ABC and its hooks (search, navigation, dashboard, relationships, collector schema + `apply_collector_payload`)
|
||||
- `BasePlugin` ABC and its hooks (navigation, dashboard widgets, collector schema + `apply_collector_payload`). Note: `get_searchable_fields` was removed in contract 0.4.0 - global search is a core concern over the asset model, not a per-plugin hook.
|
||||
|
||||
#### Excluded from the contract for v1
|
||||
|
||||
|
||||
@@ -15,7 +15,10 @@ from .plugins import plugin_manager
|
||||
# 0.3.0: shopdb.api expanded to the full plugin import surface (db, cache,
|
||||
# model bases, core models, response + pagination helpers, employee_connection)
|
||||
# so plugins no longer import internal core paths. Additive, hence minor bump.
|
||||
__contract_version__ = '0.3.0'
|
||||
# 0.4.0: removed the never-implemented get_searchable_fields hook (search is a
|
||||
# core concern over the asset model) and wired the get_dashboard_widgets hook to
|
||||
# a real consumer (/api/dashboard/widgets). Pre-1.0 contract reduction.
|
||||
__contract_version__ = '0.4.0'
|
||||
|
||||
|
||||
def create_app(config_name: str = None) -> Flask:
|
||||
|
||||
@@ -7,13 +7,49 @@ from shopdb.extensions import db
|
||||
from shopdb.core.models import (
|
||||
Application, AppVersion, AppOwner, SupportTeam, AuditLog
|
||||
)
|
||||
from plugins.computers.models import Computer, ComputerInstalledApp
|
||||
from shopdb.utils.responses import (
|
||||
success_response,
|
||||
error_response,
|
||||
paginated_response,
|
||||
ErrorCodes
|
||||
)
|
||||
|
||||
|
||||
def _computer_models():
|
||||
"""Lazily import the computers plugin models, or None if unavailable.
|
||||
|
||||
Application install-tracking is a join over the computers plugin's tables.
|
||||
Importing lazily keeps the applications API importable when the computers
|
||||
plugin is absent or disabled.
|
||||
"""
|
||||
try:
|
||||
from plugins.computers.models import Computer, ComputerInstalledApp
|
||||
return Computer, ComputerInstalledApp
|
||||
except ImportError:
|
||||
return None
|
||||
|
||||
|
||||
def _installed_count(appid):
|
||||
"""Count active installs of an app, 0 when the computers plugin is absent."""
|
||||
models = _computer_models()
|
||||
if not models:
|
||||
return 0
|
||||
_, ComputerInstalledApp = models
|
||||
return ComputerInstalledApp.query.filter_by(appid=appid, isactive=True).count()
|
||||
|
||||
|
||||
def _require_computer_models():
|
||||
"""Resolve (Computer, ComputerInstalledApp) or a 503 response tuple.
|
||||
|
||||
Usage: `models, err = _require_computer_models(); if err: return err`.
|
||||
"""
|
||||
models = _computer_models()
|
||||
if not models:
|
||||
return None, error_response(
|
||||
ErrorCodes.INTERNAL_ERROR,
|
||||
'Install tracking requires the computers plugin',
|
||||
http_code=503)
|
||||
return models, None
|
||||
from shopdb.utils.pagination import get_pagination_params, paginate_query
|
||||
|
||||
applications_bp = Blueprint('applications', __name__)
|
||||
@@ -66,8 +102,7 @@ def list_applications():
|
||||
}
|
||||
else:
|
||||
app_dict['supportteam'] = None
|
||||
app_dict['installedcount'] = ComputerInstalledApp.query.filter_by(
|
||||
appid=app.appid, isactive=True).count()
|
||||
app_dict['installedcount'] = _installed_count(app.appid)
|
||||
data.append(app_dict)
|
||||
|
||||
return paginated_response(data, page, per_page, total)
|
||||
@@ -97,8 +132,7 @@ def get_application(app_id: int):
|
||||
else:
|
||||
data['supportteam'] = None
|
||||
data['versions'] = [v.to_dict() for v in app.versions.filter_by(isactive=True).order_by(AppVersion.version.desc()).all()]
|
||||
data['installedcount'] = ComputerInstalledApp.query.filter_by(
|
||||
appid=app.appid, isactive=True).count()
|
||||
data['installedcount'] = _installed_count(app.appid)
|
||||
|
||||
return success_response(data)
|
||||
|
||||
@@ -263,6 +297,11 @@ def list_installed_machines(app_id: int):
|
||||
if not app:
|
||||
return error_response(ErrorCodes.NOT_FOUND, 'Application not found', http_code=404)
|
||||
|
||||
models, err = _require_computer_models()
|
||||
if err:
|
||||
return err
|
||||
Computer, ComputerInstalledApp = models
|
||||
|
||||
installed = ComputerInstalledApp.query.filter_by(
|
||||
appid=app_id, isactive=True).all()
|
||||
data = []
|
||||
@@ -293,6 +332,11 @@ def list_installed_machines(app_id: int):
|
||||
@jwt_required(optional=True)
|
||||
def list_machine_applications(machine_id: int):
|
||||
"""List all applications installed on a computer."""
|
||||
models, err = _require_computer_models()
|
||||
if err:
|
||||
return err
|
||||
Computer, ComputerInstalledApp = models
|
||||
|
||||
comp = Computer.query.get(machine_id)
|
||||
if not comp:
|
||||
return error_response(ErrorCodes.NOT_FOUND, 'Computer not found', http_code=404)
|
||||
@@ -305,6 +349,11 @@ def list_machine_applications(machine_id: int):
|
||||
@jwt_required()
|
||||
def install_application(machine_id: int):
|
||||
"""Install an application on a computer."""
|
||||
models, err = _require_computer_models()
|
||||
if err:
|
||||
return err
|
||||
Computer, ComputerInstalledApp = models
|
||||
|
||||
comp = Computer.query.get(machine_id)
|
||||
if not comp:
|
||||
return error_response(ErrorCodes.NOT_FOUND, 'Computer not found', http_code=404)
|
||||
@@ -351,6 +400,11 @@ def install_application(machine_id: int):
|
||||
@jwt_required()
|
||||
def uninstall_application(machine_id: int, app_id: int):
|
||||
"""Uninstall an application from a computer."""
|
||||
models, err = _require_computer_models()
|
||||
if err:
|
||||
return err
|
||||
Computer, ComputerInstalledApp = models
|
||||
|
||||
installed = ComputerInstalledApp.query.filter_by(
|
||||
computerid=machine_id,
|
||||
appid=app_id,
|
||||
@@ -370,6 +424,11 @@ def uninstall_application(machine_id: int, app_id: int):
|
||||
@jwt_required()
|
||||
def update_installed_app(machine_id: int, app_id: int):
|
||||
"""Update installed application (e.g., change version)."""
|
||||
models, err = _require_computer_models()
|
||||
if err:
|
||||
return err
|
||||
Computer, ComputerInstalledApp = models
|
||||
|
||||
installed = ComputerInstalledApp.query.filter_by(
|
||||
computerid=machine_id,
|
||||
appid=app_id,
|
||||
|
||||
@@ -12,13 +12,27 @@ from flask import Blueprint, request, current_app
|
||||
|
||||
from shopdb.extensions import db
|
||||
from shopdb.core.models import Asset, Application
|
||||
from plugins.computers.models import Computer, ComputerInstalledApp
|
||||
|
||||
from shopdb.utils.responses import success_response, error_response, ErrorCodes
|
||||
|
||||
collector_bp = Blueprint('collector', __name__)
|
||||
|
||||
|
||||
def _computer_models():
|
||||
"""Lazily import the computers plugin models.
|
||||
|
||||
The legacy /pc, /apps, /heartbeat, /bulk endpoints predate the generic
|
||||
collector contract and are computers-specific. Importing the plugin lazily
|
||||
(instead of at module load) keeps core importable when the computers plugin
|
||||
is absent or disabled. Returns (Computer, ComputerInstalledApp) or None.
|
||||
"""
|
||||
try:
|
||||
from plugins.computers.models import Computer, ComputerInstalledApp
|
||||
return Computer, ComputerInstalledApp
|
||||
except ImportError:
|
||||
return None
|
||||
|
||||
|
||||
def require_api_key(f):
|
||||
"""Require API key authentication."""
|
||||
@wraps(f)
|
||||
@@ -40,7 +54,15 @@ def require_api_key(f):
|
||||
|
||||
|
||||
def _find_pc(hostname):
|
||||
"""Find a computer by hostname, falling back to its asset number."""
|
||||
"""Find a computer by hostname, falling back to its asset number.
|
||||
|
||||
Returns None if not found or if the computers plugin is unavailable;
|
||||
callers already treat None as a 404.
|
||||
"""
|
||||
models = _computer_models()
|
||||
if not models:
|
||||
return None
|
||||
Computer, _ = models
|
||||
comp = Computer.query.filter(Computer.hostname.ilike(hostname)).first()
|
||||
if comp:
|
||||
return comp
|
||||
@@ -237,6 +259,9 @@ def update_installed_apps():
|
||||
f'PC with hostname {hostname} not found',
|
||||
http_code=404)
|
||||
|
||||
# comp existing implies the computers plugin is loaded.
|
||||
_, ComputerInstalledApp = _computer_models()
|
||||
|
||||
updated_count = created_count = skipped_count = 0
|
||||
|
||||
for app_data in apps:
|
||||
|
||||
@@ -152,6 +152,37 @@ def get_navigation():
|
||||
return success_response(all_items)
|
||||
|
||||
|
||||
@dashboard_bp.route('/widgets', methods=['GET'])
|
||||
@jwt_required(optional=True)
|
||||
def get_widgets():
|
||||
"""Aggregate dashboard widget definitions from all enabled plugins.
|
||||
|
||||
Consumer for the BasePlugin.get_dashboard_widgets hook. Skips disabled
|
||||
plugins, isolates a broken plugin in prod (re-raises in dev/test), and
|
||||
returns the merged list sorted by position.
|
||||
"""
|
||||
pm = current_app.extensions.get('plugin_manager')
|
||||
if not pm:
|
||||
return success_response([])
|
||||
|
||||
widgets = []
|
||||
for name, plugin in pm.get_all_plugins().items():
|
||||
if not pm.registry.is_enabled(name):
|
||||
continue
|
||||
try:
|
||||
for widget in plugin.get_dashboard_widgets() or []:
|
||||
widget['plugin'] = name
|
||||
widgets.append(widget)
|
||||
except Exception:
|
||||
if current_app.config.get('DEBUG') or current_app.config.get('TESTING'):
|
||||
raise
|
||||
current_app.logger.exception(
|
||||
'Plugin %s get_dashboard_widgets failed', name)
|
||||
|
||||
widgets.sort(key=lambda w: w.get('position', 99))
|
||||
return success_response(widgets)
|
||||
|
||||
|
||||
@dashboard_bp.route('/health', methods=['GET'])
|
||||
def health_check():
|
||||
"""Health check endpoint (no auth required)."""
|
||||
|
||||
@@ -11,7 +11,6 @@ from shopdb.core.models import (
|
||||
Asset, AssetType, AssetStatus,
|
||||
Application, KnowledgeBase
|
||||
)
|
||||
from plugins.computers.models import Computer, ComputerInstalledApp
|
||||
from shopdb.utils.responses import success_response, error_response, ErrorCodes
|
||||
|
||||
reports_bp = Blueprint('reports', __name__)
|
||||
@@ -317,6 +316,16 @@ def software_compliance():
|
||||
- appid: Filter to specific application
|
||||
- format: 'json' (default) or 'csv'
|
||||
"""
|
||||
# Install tracking lives in the computers plugin; degrade gracefully if
|
||||
# it is not installed.
|
||||
try:
|
||||
from plugins.computers.models import Computer, ComputerInstalledApp
|
||||
except ImportError:
|
||||
return error_response(
|
||||
ErrorCodes.INTERNAL_ERROR,
|
||||
'Software compliance requires the computers plugin',
|
||||
http_code=503)
|
||||
|
||||
# Get required applications
|
||||
required_apps = Application.query.filter(
|
||||
Application.isactive == True,
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -178,31 +178,3 @@ class BasePlugin(ABC):
|
||||
}
|
||||
"""
|
||||
return []
|
||||
|
||||
def get_searchable_fields(self) -> List[Dict]:
|
||||
"""
|
||||
Return fields this plugin contributes to global search.
|
||||
|
||||
Each field: {
|
||||
'model': Type, # SQLAlchemy model class
|
||||
'field': str, # Column name to search
|
||||
'result_type': str, # Type identifier for search results
|
||||
'url_template': str, # URL template with {id} placeholder
|
||||
'title_field': str, # Field to use for result title
|
||||
'subtitle_field': str, # Optional field for subtitle
|
||||
'relevance_boost': int # Optional relevance score multiplier
|
||||
}
|
||||
|
||||
Example for equipment plugin:
|
||||
return [{
|
||||
'model': Equipment,
|
||||
'join_model': Asset,
|
||||
'join_condition': Equipment.assetid == Asset.assetid,
|
||||
'search_fields': ['assetnumber', 'name', 'serialnumber'],
|
||||
'result_type': 'equipment',
|
||||
'url_template': '/equipment/{id}',
|
||||
'title_field': 'assetnumber',
|
||||
'subtitle_field': 'name',
|
||||
}]
|
||||
"""
|
||||
return []
|
||||
|
||||
38
tests/test_core/test_dashboard_widgets.py
Normal file
38
tests/test_core/test_dashboard_widgets.py
Normal file
@@ -0,0 +1,38 @@
|
||||
"""Tests for the dashboard-widgets hook consumer (/api/dashboard/widgets).
|
||||
|
||||
Pins the wiring added for the BasePlugin.get_dashboard_widgets hook: the
|
||||
endpoint aggregates enabled plugins' widgets and skips disabled ones.
|
||||
|
||||
Plugin enabled-state is monkeypatched (not persisted) so these tests do not
|
||||
mutate the shared instance/plugins.json registry file.
|
||||
"""
|
||||
|
||||
|
||||
def _widget_plugins(client, headers):
|
||||
response = client.get('/api/dashboard/widgets', headers=headers)
|
||||
assert response.status_code == 200, response.get_json()
|
||||
widgets = response.get_json()['data']
|
||||
assert isinstance(widgets, list)
|
||||
return widgets, {w.get('plugin') for w in widgets}
|
||||
|
||||
|
||||
def test_widgets_endpoint_aggregates_enabled_plugins(app, client, auth_headers,
|
||||
monkeypatch):
|
||||
"""An enabled plugin that implements the hook contributes a widget."""
|
||||
pm = app.extensions['plugin_manager']
|
||||
monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True)
|
||||
|
||||
widgets, plugins = _widget_plugins(client, auth_headers)
|
||||
assert 'computers' in plugins # computers implements get_dashboard_widgets
|
||||
positions = [w.get('position', 99) for w in widgets]
|
||||
assert positions == sorted(positions)
|
||||
|
||||
|
||||
def test_widgets_endpoint_skips_disabled_plugin(app, client, auth_headers,
|
||||
monkeypatch):
|
||||
"""A disabled plugin's widgets drop out of the aggregate."""
|
||||
pm = app.extensions['plugin_manager']
|
||||
monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: name != 'computers')
|
||||
|
||||
_, plugins = _widget_plugins(client, auth_headers)
|
||||
assert 'computers' not in plugins
|
||||
@@ -126,12 +126,15 @@ def test_plugin_get_navigation_items_is_iterable(plugin_instances, name):
|
||||
assert isinstance(items, list)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('name', BUNDLED_PLUGINS)
|
||||
def test_plugin_get_searchable_fields_is_iterable(plugin_instances, name):
|
||||
"""get_searchable_fields returns a list (default empty)."""
|
||||
plugin = plugin_instances[name]
|
||||
fields = plugin.get_searchable_fields()
|
||||
assert isinstance(fields, list)
|
||||
def test_baseplugin_has_no_searchable_fields_hook():
|
||||
"""get_searchable_fields was removed: global search is a core concern over
|
||||
the asset model, and no plugin ever implemented the hook (contract 0.4.0)."""
|
||||
assert not hasattr(BasePlugin, 'get_searchable_fields')
|
||||
|
||||
|
||||
def test_baseplugin_has_dashboard_widgets_hook():
|
||||
"""The dashboard widgets hook is on the contract surface and consumed."""
|
||||
assert hasattr(BasePlugin, 'get_dashboard_widgets')
|
||||
|
||||
|
||||
def test_baseplugin_does_not_have_event_handlers_hook():
|
||||
|
||||
Reference in New Issue
Block a user