Files
shopdb-flask/docs/adr/ADR-001-asset-as-platform-contract.md
cproudlock 5fa5160420 Apply skill-driven review fixes: security, hook isolation, tests, docs
Addresses findings from a 6-lens review against the project skills
(defining-asset-contract, enforcing-plugin-contract, hardening-flask-config,
integrating-plugin-hooks, pinning-flask-behavior, simplifying-python).

Security (hardening-flask-config):
- Load per-plugin COLLECTOR_API_KEY_<PLUGIN> from env in create_app. from_object
  only copies class attributes, so per-plugin keys (ADR-006) were dead in real
  deploys and silently fell back to the shared key.
- EMPLOYEE_DB_USER/PASSWORD no longer default to root/rootpassword (no safe
  default for a secret; unset fails loud). Documented in .env.example + DEPLOY.md.
- COLLECTOR_API_KEY + per-plugin + EMPLOYEE_DB_* added to .env.example/DEPLOY.md.

Hook isolation (integrating-plugin-hooks):
- collector _collector_plugins and dashboard get_navigation now re-raise in
  dev/test and log+isolate in prod, instead of silently swallowing a broken
  plugin hook.

Plugin loader (enforcing-plugin-contract):
- enable_plugin/install_plugin read dependencies+version from the manifest
  instead of instantiating the plugin class.
- _register_plugin_components rejects a second plugin claiming an already-used
  api_prefix (reset per app in init_app).

Tests (pinning-flask-behavior):
- test_identifiers.py: gauge/maintenance round-trip on computer/printer/network
  create+update; per-type seed yields the 12 identifier keys.
- contract tests for apply_collector_payload presence + schema-declarers-implement.
- security tests for per-plugin key env loading + no employee-db password default.

Docs/contract sync (defining-asset-contract):
- PLUGIN-HOOKS.md documents apply_collector_payload; stale 0.2.0 -> 0.3.0.
- ADR-006 documents apply_collector_payload + single-dispatch rationale.
- ADR-001 enumerates the expanded shopdb.api import surface.

Simplify (simplifying-python):
- De-duplicate the 21-entry settings defaults: shared build_default_settings()
  used by both the /settings/seed route and the CLI (were drifting copies).
- Remove dead AssetStatus import + redundant AssetType local import in computers
  plugin; comment the statusid=1 collector default.

153 tests pass (was 145), naming/style green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-26 19:25:52 -04:00

193 lines
9.4 KiB
Markdown

# ADR-001: Asset model is the platform contract
- **Status:** ACCEPTED
- **Date:** 2026-05-08
- **Deciders:** cproudlock
- **Supersedes:** none
## Context
shopdb-flask is being shaped as a framework that sister GE Aerospace facilities can adopt. The framework defines a stable core; sites install plugins for the asset classes they care about (equipment, computers, printers, measuring tools, network gear, etc.).
The codebase ran two parallel object models:
1. **Legacy `Machine` model** in `shopdb/core/models/machine.py`. Original schema inherited from the classic-ASP shopdb. Tables: `machines`, `pctypes`, `machinetypes`. Plugins like printers stored extension data via `PrinterData` keyed by `machineid`.
2. **New `Asset` model** in `shopdb/core/models/asset.py`. Generic asset abstraction with `AssetType`, `AssetStatus`, `AssetRelationship`. Plugins also exposed asset-based tables keyed by `assetid`.
For the framework to be adoptable, the platform contract has to be one model, documented, versioned, and stable.
## Decision
**`Asset` is the platform contract.** Plugin authors target the asset-based API. The `Machine` model and its dependents (`PCType`, `MachineType`, `PrinterData` keyed by `machineid`) are legacy and will be retired through a tracked migration.
### Platform contract surface
The following are the public, versioned surface. Plugin authors may depend on them. Breaking changes require a major version bump per ADR-002.
#### Models
- `Asset` (core entity)
- `AssetType` (asset classification, registered by plugins)
- `AssetStatus` (active / inactive / decommissioned / retired)
- `AssetRelationship` + `RelationshipType` (cross-plugin links)
- `Vendor`, `Location`, `LocationType`, `BusinessUnit`, `Model`, `OperatingSystem` (shared reference data)
#### Helpers
- `AuditLog` API: `audit_log(action, entitytype, entityid, ...)` for plugins to record audit entries with consistent schema
- `Setting` API: `plugin.get_setting(key)` and `plugin.set_setting(key, value)` for plugin-scoped config persisted via the core `Setting` model
- `resolve_asset_position(asset)` for the documented position-resolution algorithm
#### Import surface (`shopdb.api`) - expanded in __contract_version__ 0.3.0
`shopdb.api` is the ONLY core module plugins may import (plus `shopdb.plugins.base`
for the ABC). Deep imports of `shopdb.core.*`, `shopdb.extensions`, or
`shopdb.utils.*` are contract violations enforced by the
`test_plugins_only_import_contract_surface` test. The surface re-exports:
- Infrastructure: `db`, `cache`
- Model bases: `BaseModel`, `AuditMixin`
- Core models: `Asset`, `AssetType`, `AssetStatus`, `Vendor`, `Model`,
`Communication`, `CommunicationType`, `Location`, `Setting`, `AuditLog`,
`Application`, `AppVersion`, `OperatingSystem`
- Responses: `success_response`, `error_response`, `paginated_response`, `ErrorCodes`
- Pagination: `get_pagination_params`, `paginate_query`
- Helpers: `audit_log`, `resolve_asset_position`; legacy `employee_connection`
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`)
#### Excluded from the contract for v1
- Event bus (`get_event_handlers` removed from `BasePlugin`). Add later if a real use case appears.
### Relationship types (seeded core values)
`RelationshipType` is seeded with three rows. Plugin authors do not add new types in v1.
| Type | Meaning | Boundary rule |
|------|---------|---------------|
| `partof` | Composition, siblings, sub-assemblies | Parent dies without it |
| `controls` | One asset has operational authority over another | PC commands a machine, measuring tool, etc. |
| `connectedto` | Network or data link without operational authority | Cable, switch port, NAS mount |
### AssetRelationship columns
```
AssetRelationship
- relationshipid PK
- sourceassetid FK to assets
- targetassetid FK to assets
- relationshiptypeid FK to relationshiptypes (one of partof, controls, connectedto)
- label text, free description (e.g., "ethernet PoE", "DNC feed")
- inheritsposition bool, true means resolved-position walk follows this edge
- propagatesthroughid FK to relationshiptypes, nullable, see propagation below
- notes text
- isactive bool
- createddate, modifieddate
```
Free-text `label` carries the domain nuance (`controls` with label "DNC feed" vs `controls` with label "operator workstation"). Avoids inflating the type list.
### Sibling propagation
When a relationship is created or deleted, the framework checks `RelationshipType.propagatesthroughid`. If set, it finds all assets related to the source via the propagation type and applies the same change.
Default seeding:
| RelationshipType | propagatesthroughid |
|------------------|---------------------|
| `partof` | null (the propagation rail itself) |
| `controls` | `partof` (controls relationships propagate across siblings) |
| `connectedto` | null (network paths don't propagate) |
Cycle protection: max walk depth 3, visited-set during traversal.
### Position resolution
Resolved map position for any asset follows this priority chain:
1. Asset-specific override (`assets.mapx`, `assets.mapy` if non-null)
2. Walk relationships where `inheritsposition=true`, ordered by relationship type priority (`partof` first, then `controls`), recursively resolve the related asset's position with cycle detection
3. Fall back to the asset's location coords (`locations.mapx`, `locations.mapy` via `assets.locationid`)
4. If none of the above, asset is "unplaced" and rendered in a tray, not on the map
API exposes resolved position with a source indicator: `{"mapx": 234, "mapy": 567, "positionsource": "self|related|location|none"}`.
### Hierarchical locations
`locations` is extended with `parentlocationid` (self-FK, nullable). Sites define their own location tree (cells, sub-cells, network closets, meeting rooms, labs).
`locationtypes` lookup is added with seeded values: `section`, `cell`, `subcell`, `meetingroom`, `lab`, `office`, `storage`, `hallway`, `networkcloset`, `building`. Sites can extend.
Asset-to-location is the existing `assets.locationid` FK only (single primary location for v1). Multi-location and transient placement are deferred.
## Migration scope (narrowed)
Only one class of legacy data migrates: physical manufacturing equipment with a `machinenumber` (5-axis mills, lathes, broachers, heat treatment, CMMs, etc.).
Migration filter:
```sql
SELECT * FROM legacy.machines
WHERE category = 'Equipment'
AND machinenumber IS NOT NULL
```
Rows without `machinenumber`: **skipped.** Add manually post-migration if any matter.
Migrated rows become `assets` with `assettype='Equipment'` plus a row in the equipment plugin's `equipment` table. Legacy `machinetypeid` is preserved on the equipment row to enable later reclassification (some "Equipment" rows are actually measuring tools, see ADR-005).
Skipped from migration:
- PCs (rebuilt via PXE collector pipeline, see ADR-006)
- Printers, USB devices, network gear, notifications, KB articles (operational or re-collectable, decision per class deferred until needed)
## Consequences
### Positive
- Plugin authors at sister sites have a single, documented contract to target
- Cross-plugin features (relationships, search, map) work uniformly
- Three relationship types are easy to learn; free-text label captures nuance
- Sibling propagation handles dual-path machines without code changes per use case
- Position resolution gives users one map without per-plugin map code
### Negative
- Migration of equipment data must run cleanly; `migrating-asset-schema` skill owns the procedure
- Frontend `views/machines/` will be repointed or replaced; effort tracked in Phase 5
- Legacy `core/api/machines.py` (641 LOC) becomes deprecated; deletion or shim deferred until equipment is migrated
### Neutral
- `Machine`, `PCType`, `MachineType`, `PrinterData` remain in the schema until the migration completes, behind a deprecation notice. New code does not touch them.
## Alternatives considered
1. **Keep both models indefinitely.** Doubles test surface, confuses contract docs, breaks cross-plugin features. Rejected.
2. **Make `Machine` the contract; retire `Asset`.** `Machine` has shop-floor-specific assumptions that don't generalize. Rejected.
3. **Define a higher abstraction above both.** Yet another layer. `Asset` is already the abstraction. Rejected.
4. **More relationship types** (`controls`, `operates`, `monitors`, `dncfeeds`, `mountedon`, `connectedvia`, `inspects`, `siblingbay`). Eight types proved unwieldy. Collapsed to three with free-text labels and per-row inheritance/propagation flags.
## Open questions deferred
- Lifecycle relationship type (`replaces`, `replacedby`). Add when first needed.
- Multi-location asset placement, transient placement (calibration trip, off-site repair). Add when first needed.
- Map editing UI (drag locations onto floor plan). Phase 6 polish.
- Plugin-extensible relationship types beyond the three seeded. Add when a real cross-plugin case can't be expressed via labels.
## References
- `shopdb/core/models/asset.py`
- `shopdb/core/models/machine.py` (legacy, deprecated)
- `shopdb/plugins/base.py`
- ADR-002 (versioning of the surface)
- ADR-003 (plugin distribution)
- ADR-004 (deployment topology)
- ADR-005 (equipment vs measuring tools split)
- ADR-006 (collector contract pattern)