mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-04 02:08:06 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f31b458bfd |
@@ -79,5 +79,4 @@ jobs:
|
||||
test/test_sports_scroll.py \
|
||||
test/test_version_consistency.py \
|
||||
test/test_plugin_compatibility_gate.py \
|
||||
test/test_install_preserves_existing.py \
|
||||
test/test_core_owned_config_keys.py
|
||||
test/test_install_preserves_existing.py
|
||||
|
||||
@@ -27,6 +27,7 @@ fixes their version string. See `docs/SPORTS_UNIFICATION.md`, phase B4.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from typing import Any, Dict, Optional, Tuple
|
||||
|
||||
# Below this, the core's self-reported version is not evidence of anything.
|
||||
@@ -49,6 +50,92 @@ def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]:
|
||||
return tuple(nums) # type: ignore[return-value]
|
||||
|
||||
|
||||
# `parse_semver` is deliberately lenient — it strips non-digits and yields
|
||||
# (0, 0, 0) for a string with no numbers at all, which is fine for a floor
|
||||
# (a floor of 0.0.0 never blocks anything) but wrong for a range, where the
|
||||
# same leniency would turn an unreadable spec into a *refusal*. Range specs
|
||||
# are therefore validated against this first, so garbage reads as "no
|
||||
# evidence" rather than "incompatible".
|
||||
_VERSION_TOKEN = re.compile(r"^v?\d+(\.\d+){0,2}(-[\w.-]+)?(\+[\w.-]+)?$")
|
||||
|
||||
|
||||
def _parse_strict(value: str) -> Optional[Tuple[int, int, int]]:
|
||||
"""`parse_semver`, but ``None`` unless the string really looks like one."""
|
||||
if not isinstance(value, str) or not _VERSION_TOKEN.match(value.strip()):
|
||||
return None
|
||||
return parse_semver(value)
|
||||
|
||||
|
||||
def _satisfies_range(core: Tuple[int, int, int], spec: str) -> Optional[bool]:
|
||||
"""Does ``core`` satisfy one `compatible_versions` entry?
|
||||
|
||||
Returns ``None`` when the spec cannot be parsed — the caller treats that as
|
||||
"no evidence" rather than as a refusal, so an unrecognised spelling never
|
||||
costs a user a working install.
|
||||
|
||||
Supports the forms `schema/manifest_schema.json` permits: `>=`, `<=`, `>`,
|
||||
`<`, `~`, `^`, a bare exact version, and an inclusive `A - B` range.
|
||||
Prerelease/build suffixes are tolerated and ignored, matching `parse_semver`.
|
||||
"""
|
||||
spec = spec.strip()
|
||||
if not spec:
|
||||
return None
|
||||
|
||||
if " - " in spec: # inclusive range, e.g. "2.0.0 - 3.1.0"
|
||||
low_raw, _, high_raw = spec.partition(" - ")
|
||||
low, high = _parse_strict(low_raw), _parse_strict(high_raw)
|
||||
if low is None or high is None:
|
||||
return None
|
||||
return low <= core <= high
|
||||
|
||||
for op in (">=", "<=", ">", "<", "~", "^"):
|
||||
if spec.startswith(op):
|
||||
target = _parse_strict(spec[len(op):])
|
||||
if target is None:
|
||||
return None
|
||||
if op == ">=":
|
||||
return core >= target
|
||||
if op == "<=":
|
||||
return core <= target
|
||||
if op == ">":
|
||||
return core > target
|
||||
if op == "<":
|
||||
return core < target
|
||||
if op == "~":
|
||||
# Patch-level changes only: >=X.Y.Z, <X.(Y+1).0
|
||||
return target <= core < (target[0], target[1] + 1, 0)
|
||||
# "^": minor and patch changes: >=X.Y.Z, <(X+1).0.0
|
||||
return target <= core < (target[0] + 1, 0, 0)
|
||||
|
||||
exact = _parse_strict(spec)
|
||||
return None if exact is None else core == exact
|
||||
|
||||
|
||||
def satisfies_compatible_versions(
|
||||
manifest: Dict[str, Any], core: Tuple[int, int, int]
|
||||
) -> Optional[bool]:
|
||||
"""Evaluate the manifest's `compatible_versions` array against ``core``.
|
||||
|
||||
The array is a set of *alternatives*: satisfying any one entry means the
|
||||
plugin declares itself compatible. Returns ``None`` when the field is
|
||||
absent or no entry could be parsed, so callers can distinguish "declared
|
||||
incompatible" from "did not say".
|
||||
|
||||
This is the field `schema/manifest_schema.json` marks **required**, and it
|
||||
is the only one that can express an upper bound — `ledmatrix_min_version`
|
||||
is a floor and cannot say "not compatible with 4.x".
|
||||
"""
|
||||
specs = manifest.get('compatible_versions')
|
||||
if not isinstance(specs, list) or not specs:
|
||||
return None
|
||||
|
||||
verdicts = [_satisfies_range(core, s) for s in specs if isinstance(s, str)]
|
||||
parsed = [v for v in verdicts if v is not None]
|
||||
if not parsed:
|
||||
return None
|
||||
return any(parsed)
|
||||
|
||||
|
||||
def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]:
|
||||
"""The core version this plugin says it needs, or ``None`` if it doesn't say.
|
||||
|
||||
@@ -74,26 +161,46 @@ def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]:
|
||||
def check(manifest: Dict[str, Any], core_version: str) -> Tuple[bool, Optional[str]]:
|
||||
"""Return ``(compatible, reason)``.
|
||||
|
||||
``compatible`` is False **only** when the plugin declares a parseable floor,
|
||||
the core reports a parseable and trustworthy version, and the floor is
|
||||
genuinely above it. Every uncertain case resolves to compatible: an
|
||||
undeclared floor, an unparseable version on either side, or a core whose
|
||||
version is below `TRUSTWORTHY_FLOOR`. Refusing on a guess would break
|
||||
working installs, which is the more expensive mistake here.
|
||||
Two fields can say a plugin is incompatible and **the more restrictive
|
||||
wins**:
|
||||
|
||||
- `compatible_versions` — the schema-required array of semver ranges, and
|
||||
the only one that can express an upper bound.
|
||||
- `ledmatrix_min_version` (or the deprecated `ledmatrix_min`) — the
|
||||
per-release floor inside `versions[]`.
|
||||
|
||||
They agree across every published manifest today except `7-segment-clock`,
|
||||
but they *can* disagree, and a plugin that says `["2.0.0 - 2.9.9"]` means
|
||||
"not compatible with 3.x" no matter what its floor says.
|
||||
|
||||
``compatible`` is False **only** on evidence: the core reports a parseable,
|
||||
trustworthy version and a field genuinely excludes it. Every uncertain case
|
||||
resolves to compatible — nothing declared, an unparseable version on either
|
||||
side, or a core below `TRUSTWORTHY_FLOOR`. Refusing on a guess breaks a
|
||||
working install, which is the more expensive mistake here.
|
||||
|
||||
``reason`` is user-facing text, present only when incompatible.
|
||||
"""
|
||||
declared = declared_min_version(manifest)
|
||||
needed = parse_semver(declared)
|
||||
if needed is None:
|
||||
return True, None
|
||||
|
||||
current = parse_semver(core_version)
|
||||
if current is None or current < TRUSTWORTHY_FLOOR:
|
||||
return True, None
|
||||
|
||||
if needed > current:
|
||||
name = manifest.get('name') or manifest.get('id') or 'This plugin'
|
||||
name = manifest.get('name') or manifest.get('id') or 'This plugin'
|
||||
|
||||
# Ranges first: they are the canonical field and can rule out a core that
|
||||
# clears the floor.
|
||||
if satisfies_compatible_versions(manifest, current) is False:
|
||||
specs = ", ".join(
|
||||
s for s in manifest.get('compatible_versions', []) if isinstance(s, str))
|
||||
return False, (
|
||||
f"{name} supports LEDMatrix {specs}, but this system is running "
|
||||
f"{core_version}. Install a build in that range, or a plugin "
|
||||
f"version that supports {core_version}."
|
||||
)
|
||||
|
||||
declared = declared_min_version(manifest)
|
||||
needed = parse_semver(declared)
|
||||
if needed is not None and needed > current:
|
||||
return False, (
|
||||
f"{name} requires LEDMatrix {declared} or newer, but this system is "
|
||||
f"running {core_version}. Update LEDMatrix first, then install it."
|
||||
|
||||
@@ -395,37 +395,6 @@ class PluginManager:
|
||||
self.state_manager.set_state(plugin_id, PluginState.ERROR, error=e)
|
||||
return False
|
||||
|
||||
#: Config keys the **core** reads out of a plugin's own config block. The
|
||||
#: plugin never declares them, so a schema with
|
||||
#: ``"additionalProperties": false`` — 37 of the 42 published ones — reports
|
||||
#: them as violations and the plugin gets flagged degraded in the web UI for
|
||||
#: using a documented core feature.
|
||||
#:
|
||||
#: Listed explicitly rather than matched on a ``vegas_`` prefix, because
|
||||
#: ``vegas_mode`` is the opposite case: plugins *do* declare that one, and a
|
||||
#: prefix rule would silently stop validating it.
|
||||
#:
|
||||
#: Read by: ``vegas_mode/plugin_adapter.py`` (``vegas_width_pct``,
|
||||
#: ``vegas_overflow``) and ``base_plugin.py`` (``vegas_max_width_screens``).
|
||||
CORE_OWNED_CONFIG_KEYS = frozenset({
|
||||
'vegas_width_pct',
|
||||
'vegas_overflow',
|
||||
'vegas_max_width_screens',
|
||||
})
|
||||
|
||||
def _strip_core_owned_keys(self, config: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""A shallow copy of ``config`` without the core's own tuning keys.
|
||||
|
||||
Only the top level is touched, and only when such a key is present, so
|
||||
the common case allocates nothing extra.
|
||||
"""
|
||||
if not isinstance(config, dict):
|
||||
return config
|
||||
if not self.CORE_OWNED_CONFIG_KEYS.intersection(config):
|
||||
return config
|
||||
return {k: v for k, v in config.items()
|
||||
if k not in self.CORE_OWNED_CONFIG_KEYS}
|
||||
|
||||
def _validate_config_schema_soft(self, plugin_id: str, config: Dict[str, Any]) -> None:
|
||||
"""Validate a plugin's config against its JSON schema — warn/degrade only.
|
||||
|
||||
@@ -450,7 +419,7 @@ class PluginManager:
|
||||
|
||||
try:
|
||||
is_valid, errors = self.schema_manager.validate_config_against_schema(
|
||||
self._strip_core_owned_keys(config), schema, plugin_id
|
||||
config, schema, plugin_id
|
||||
)
|
||||
except Exception as e: # pragma: no cover - defensive
|
||||
# Validation machinery itself failed — do not penalise the plugin.
|
||||
|
||||
@@ -1,98 +0,0 @@
|
||||
"""The core's own tuning keys must not make a plugin look broken.
|
||||
|
||||
`vegas_width_pct`, `vegas_overflow` and `vegas_max_width_screens` are read by
|
||||
the *core* out of each plugin's config block — `vegas_mode/plugin_adapter.py`
|
||||
and `base_plugin.py`. No plugin declares them, and 37 of the 42 published
|
||||
config schemas set `"additionalProperties": false`, so schema validation
|
||||
reported them as violations.
|
||||
|
||||
That is not just log noise: `_validate_config_schema_soft` sets `degraded` in
|
||||
the health tracker, which the web UI surfaces. Measured on a real device, **9
|
||||
of 27 installed plugins** were flagged degraded purely for using a documented
|
||||
core feature — including `baseball-scoreboard` and `f1-scoreboard`.
|
||||
|
||||
The fix strips those keys before validating. It deliberately does *not* match
|
||||
on a `vegas_` prefix: `vegas_mode` is plugin-owned and declared in schemas, and
|
||||
a prefix rule would silently stop validating it.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from src.plugin_system.plugin_manager import PluginManager
|
||||
|
||||
|
||||
STRICT_SCHEMA = {
|
||||
"type": "object",
|
||||
"additionalProperties": False,
|
||||
"properties": {
|
||||
"enabled": {"type": "boolean"},
|
||||
"vegas_mode": {"type": "string"}, # plugin-owned, must stay validated
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def manager():
|
||||
mgr = PluginManager.__new__(PluginManager) # skip the heavy constructor
|
||||
mgr.logger = MagicMock()
|
||||
mgr.schema_manager = MagicMock()
|
||||
mgr._set_degraded_safe = MagicMock()
|
||||
return mgr
|
||||
|
||||
|
||||
class TestStripCoreOwnedKeys:
|
||||
def test_removes_every_core_owned_key(self, manager):
|
||||
cfg = {"enabled": True, "vegas_width_pct": 50,
|
||||
"vegas_overflow": "wrap", "vegas_max_width_screens": 2}
|
||||
assert manager._strip_core_owned_keys(cfg) == {"enabled": True}
|
||||
|
||||
def test_leaves_plugin_owned_vegas_mode_alone(self, manager):
|
||||
"""A prefix rule would have eaten this one."""
|
||||
cfg = {"enabled": True, "vegas_mode": "scroll"}
|
||||
assert manager._strip_core_owned_keys(cfg) == cfg
|
||||
|
||||
def test_returns_the_same_object_when_nothing_to_strip(self, manager):
|
||||
cfg = {"enabled": True}
|
||||
assert manager._strip_core_owned_keys(cfg) is cfg
|
||||
|
||||
def test_does_not_mutate_the_caller_config(self, manager):
|
||||
cfg = {"enabled": True, "vegas_width_pct": 50}
|
||||
manager._strip_core_owned_keys(cfg)
|
||||
assert "vegas_width_pct" in cfg, "the live plugin config was mutated"
|
||||
|
||||
def test_tolerates_a_non_dict(self, manager):
|
||||
assert manager._strip_core_owned_keys(None) is None
|
||||
|
||||
|
||||
class TestSoftValidation:
|
||||
def _validate_with(self, manager, config, valid=True, errors=()):
|
||||
manager.schema_manager.load_schema.return_value = STRICT_SCHEMA
|
||||
manager.schema_manager.validate_config_against_schema.return_value = (
|
||||
valid, list(errors))
|
||||
manager._validate_config_schema_soft("baseball-scoreboard", config)
|
||||
return manager.schema_manager.validate_config_against_schema.call_args
|
||||
|
||||
def test_core_keys_never_reach_the_validator(self, manager):
|
||||
"""The regression: these keys reaching a strict schema is what flagged
|
||||
9 of 27 plugins degraded."""
|
||||
args = self._validate_with(
|
||||
manager, {"enabled": True, "vegas_width_pct": 50})
|
||||
validated = args[0][0]
|
||||
assert "vegas_width_pct" not in validated
|
||||
assert validated == {"enabled": True}
|
||||
|
||||
def test_plugin_owned_keys_still_reach_the_validator(self, manager):
|
||||
args = self._validate_with(
|
||||
manager, {"enabled": True, "vegas_mode": "scroll"})
|
||||
assert args[0][0]["vegas_mode"] == "scroll"
|
||||
|
||||
def test_a_genuine_violation_is_still_reported(self, manager):
|
||||
"""Stripping core keys must not turn the check into a no-op."""
|
||||
self._validate_with(
|
||||
manager, {"enabled": True, "typo_key": 1},
|
||||
valid=False, errors=["Field root: 'typo_key' was unexpected"])
|
||||
manager._set_degraded_safe.assert_called()
|
||||
reason = manager._set_degraded_safe.call_args[0][1]
|
||||
assert reason and "typo_key" in reason
|
||||
@@ -230,3 +230,85 @@ class TestLoaderAndStoreAgree:
|
||||
)
|
||||
assert loader_would_warn is (not expected)
|
||||
assert hasattr(PluginLoader, "_warn_if_incompatible")
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# compatible_versions — the schema-required field, and the only one that can
|
||||
# express an upper bound
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
class TestCompatibleVersions:
|
||||
@pytest.mark.parametrize("spec,core,expected", [
|
||||
(">=2.0.0", "3.2.0", True),
|
||||
(">=2.0.0", "1.9.9", False),
|
||||
("<=3.0.0", "3.2.0", False),
|
||||
("<=3.0.0", "2.9.0", True),
|
||||
(">3.2.0", "3.2.0", False),
|
||||
("<4.0.0", "3.2.0", True),
|
||||
("3.2.0", "3.2.0", True), # bare == exact match
|
||||
("3.2.0", "3.2.1", False),
|
||||
("~3.2.0", "3.2.9", True), # patch-level only
|
||||
("~3.2.0", "3.3.0", False),
|
||||
("^3.2.0", "3.9.9", True), # minor + patch
|
||||
("^3.2.0", "4.0.0", False),
|
||||
("2.0.0 - 3.2.0", "3.2.0", True), # inclusive both ends
|
||||
("2.0.0 - 3.2.0", "2.0.0", True),
|
||||
("2.0.0 - 3.2.0", "3.2.1", False),
|
||||
("v3.2.0", "3.2.0", True), # leading v tolerated
|
||||
("3.2.0-beta.1", "3.2.0", True), # prerelease suffix ignored
|
||||
])
|
||||
def test_range_forms(self, spec, core, expected):
|
||||
got = compatibility.satisfies_compatible_versions(
|
||||
{"compatible_versions": [spec]}, compatibility.parse_semver(core))
|
||||
assert got is expected, f"{spec!r} vs {core}"
|
||||
|
||||
def test_array_is_alternatives_not_conjunction(self):
|
||||
"""Satisfying any one entry is enough — otherwise ['<2.0.0','>=3.0.0']
|
||||
could never be satisfied by anything."""
|
||||
m = {"compatible_versions": ["<2.0.0", ">=3.0.0"]}
|
||||
assert compatibility.satisfies_compatible_versions(
|
||||
m, compatibility.parse_semver("3.2.0")) is True
|
||||
|
||||
def test_absent_or_unparseable_is_no_evidence(self):
|
||||
core = compatibility.parse_semver("3.2.0")
|
||||
assert compatibility.satisfies_compatible_versions({}, core) is None
|
||||
assert compatibility.satisfies_compatible_versions(
|
||||
{"compatible_versions": []}, core) is None
|
||||
assert compatibility.satisfies_compatible_versions(
|
||||
{"compatible_versions": ["not a version"]}, core) is None
|
||||
# One unparseable entry alongside a good one must not poison the result.
|
||||
assert compatibility.satisfies_compatible_versions(
|
||||
{"compatible_versions": ["garbage", ">=2.0.0"]}, core) is True
|
||||
|
||||
|
||||
class TestMoreRestrictiveWins:
|
||||
def test_upper_bound_blocks_a_core_that_clears_the_floor(self):
|
||||
"""The gap this closes: the floor says 2.0.0 and the core is 3.2.0, so
|
||||
the floor alone would allow it — but the plugin said it stops at 2.x."""
|
||||
m = {"name": "Legacy Plugin",
|
||||
"compatible_versions": ["2.0.0 - 2.9.9"],
|
||||
"versions": [{"ledmatrix_min_version": "2.0.0"}]}
|
||||
ok, reason = compatibility.check(m, "3.2.0")
|
||||
assert ok is False
|
||||
assert "2.0.0 - 2.9.9" in reason and "3.2.0" in reason
|
||||
|
||||
def test_floor_blocks_when_ranges_would_allow(self):
|
||||
m = {"name": "Needs Newer",
|
||||
"compatible_versions": [">=1.0.0"],
|
||||
"versions": [{"ledmatrix_min_version": "9.9.9"}]}
|
||||
ok, reason = compatibility.check(m, "3.2.0")
|
||||
assert ok is False
|
||||
assert "9.9.9" in reason
|
||||
|
||||
def test_both_satisfied_allows(self):
|
||||
m = {"compatible_versions": [">=2.0.0"],
|
||||
"versions": [{"ledmatrix_min_version": "2.0.0"}]}
|
||||
assert compatibility.check(m, "3.2.0") == (True, None)
|
||||
|
||||
def test_untrustworthy_core_still_bypasses_both_checks(self):
|
||||
"""A core reporting 1.0.0 fails `>=2.0.0`, which 41 of 42 published
|
||||
manifests declare. Blocking there would empty the plugin store for
|
||||
exactly the users who cannot be helped by it."""
|
||||
m = {"compatible_versions": [">=2.0.0"],
|
||||
"versions": [{"ledmatrix_min_version": "2.0.0"}]}
|
||||
assert compatibility.check(m, "1.0.0") == (True, None)
|
||||
|
||||
Reference in New Issue
Block a user