refactor(storage,processing): kind registries + versioned data migrations
Two CRITICAL data-safety bugs from the architecture audit and the two
worst parallel-change problems are fixed in one coherent pass.
Audit findings addressed:
- C2 silent CSS response fallback. The previous _RESPONSE_MAP fell
through to a fabricated PictureCSSResponse whenever a source
class lacked an entry; in particular game_event sources were
silently mis-shaped. Now: GameEventCSSResponse/Create/Update
schemas exist, _RESPONSE_MAP is re-keyed by source_type string,
an import-time _assert_response_map_coverage() requires symmetric
agreement with storage._SOURCE_TYPE_MAP, and the runtime path
raises instead of fabricating a response.
- C11 string-replace JSON migration. ColorStripStore used
blob.replace('"source_type": "static"', '"source_type":
"single_color"') which can corrupt unrelated substrings (e.g.
an animation type named "static_wave") and provides no audit,
no transaction, no idempotency. Replaced with
storage.data_migrations.MigrationRunner backed by a
data_migrations audit table. Each migration runs inside one
db.transaction() that covers the applied-check, the apply(),
and the audit-INSERT — partial failures roll back atomically.
StaticToSingleColorMigration parses each row with json.loads
and mutates only the source_type field. Frozen-write databases
skip with a warning.
- C3+C4 color-strip stream dispatch. The 7-branch elif in
ColorStripStreamManager.acquire() and the duplicate one in
ws_stream._create_stream() now share a single STREAM_BUILDERS
registry in core.processing.color_strip_kinds, keyed by
source.source_type. Both call sites populate a StreamDeps bag
and delegate to build_stream(). _assert_stream_kind_coverage()
asserts at import that STREAM_BUILDERS plus SHARABLE_KINDS
partitions storage._SOURCE_TYPE_MAP. ws_stream's preview path
wraps each FastAPI-DI getter in _safe() so non-audio previews
no longer crash when audio/CSPT stores are not wired.
- C6+C7 value stream dispatch. The 14-branch isinstance ladder in
ValueStreamManager._create_stream and its silent
StaticValueStream(value=1.0) fallback are replaced by
core.processing.value_kinds.STREAM_BUILDERS, keyed by
source_type string (so AdaptiveValueSource's adaptive_time and
adaptive_scene route to different builders correctly). The
manager retains only the SyncClockRuntime pre-acquisition step
for animated_color (kinds needing this are listed explicitly
in NEEDS_CLOCK_RUNTIME). Symmetric coverage assertion plus a
separate assertion that NEEDS_CLOCK_RUNTIME is a subset of the
registry.
Bundled in: the static->single_color rename plus the HTTPValueStream
/ http_endpoint introduction that were already in flight on this
branch share these files; the registry refactor naturally absorbs
both via the new "single_color" / "static" alias entries and the
_build_http builder.
Tests: 26 new tests cover response-map coverage drift, migration
runner audit-table mechanics + transactional rollback +
frozen-write skip, and the two stream-builder registries. 343
existing storage / API / e2e tests stay green. Ruff clean.
This commit is contained in:
@@ -0,0 +1,97 @@
|
||||
"""Tests for the unified color-strip stream-builder registry."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from ledgrab.core.processing import color_strip_kinds
|
||||
from ledgrab.core.processing.color_strip_kinds import (
|
||||
SHARABLE_KINDS,
|
||||
STREAM_BUILDERS,
|
||||
StreamDeps,
|
||||
build_stream,
|
||||
)
|
||||
from ledgrab.storage.color_strip_source import _SOURCE_TYPE_MAP
|
||||
|
||||
|
||||
def test_stream_builders_partition_storage_kinds_with_sharable():
|
||||
"""STREAM_BUILDERS ∪ SHARABLE_KINDS == storage source_types."""
|
||||
storage_kinds = set(_SOURCE_TYPE_MAP.keys())
|
||||
covered = set(STREAM_BUILDERS.keys()) | SHARABLE_KINDS
|
||||
assert (
|
||||
storage_kinds == covered
|
||||
), f"missing={storage_kinds - covered}, extra={covered - storage_kinds}"
|
||||
|
||||
|
||||
def test_sharable_kinds_list_matches_actual_sharable_property():
|
||||
"""The hand-coded SHARABLE_KINDS list mirrors the `sharable` @property values.
|
||||
|
||||
Every ColorStripSource subclass currently implements ``sharable`` as a
|
||||
literal ``return True``/``return False`` that does not touch ``self``,
|
||||
so ``fget(None)`` is a safe introspection. If a future subclass changes
|
||||
that contract — e.g. by computing sharability from instance state — this
|
||||
test will raise a clear error rather than silently misclassify the kind,
|
||||
forcing the maintainer to update both ``SHARABLE_KINDS`` and this test.
|
||||
"""
|
||||
|
||||
def _is_sharable(name: str, cls) -> bool:
|
||||
prop = getattr(cls, "sharable", None)
|
||||
if not isinstance(prop, property) or prop.fget is None:
|
||||
raise AssertionError(
|
||||
f"{cls.__name__} (source_type={name!r}) does not expose `sharable` "
|
||||
f"as a @property; SHARABLE_KINDS introspection no longer works."
|
||||
)
|
||||
try:
|
||||
return bool(prop.fget(None))
|
||||
except (AttributeError, TypeError) as e:
|
||||
raise AssertionError(
|
||||
f"{cls.__name__}.sharable now depends on `self` ({e}); update "
|
||||
"this test and SHARABLE_KINDS together."
|
||||
) from e
|
||||
|
||||
actually_sharable = {name for name, cls in _SOURCE_TYPE_MAP.items() if _is_sharable(name, cls)}
|
||||
assert actually_sharable == set(SHARABLE_KINDS), (
|
||||
"SHARABLE_KINDS drifted from the @property values: "
|
||||
f"actually_sharable={sorted(actually_sharable)}, "
|
||||
f"SHARABLE_KINDS={sorted(SHARABLE_KINDS)}"
|
||||
)
|
||||
|
||||
|
||||
def test_build_stream_raises_on_unknown_kind():
|
||||
class _FakeSource:
|
||||
source_type = "totally_unregistered_kind"
|
||||
id = "fake"
|
||||
|
||||
deps = StreamDeps(css_manager=None)
|
||||
with pytest.raises(ValueError, match="totally_unregistered_kind"):
|
||||
build_stream(_FakeSource(), deps)
|
||||
|
||||
|
||||
def test_coverage_assertion_raises_on_drift(monkeypatch):
|
||||
"""Adding a kind to storage's _SOURCE_TYPE_MAP without a builder fails fast."""
|
||||
|
||||
monkeypatch.setitem(_SOURCE_TYPE_MAP, "phantom", object)
|
||||
with pytest.raises(RuntimeError, match="phantom"):
|
||||
color_strip_kinds._assert_stream_kind_coverage()
|
||||
|
||||
|
||||
def test_legacy_static_alias_resolves_to_single_color_builder():
|
||||
"""The legacy 'static' source_type still maps to the single_color builder."""
|
||||
assert STREAM_BUILDERS["static"] is not None
|
||||
assert STREAM_BUILDERS["single_color"] is not None
|
||||
# The two route to the same loader, but builders are distinct closures —
|
||||
# call them with a tiny source stub and confirm same class.
|
||||
from ledgrab.core.processing.color_strip_stream import SingleColorStripStream
|
||||
from ledgrab.storage.color_strip_source import SingleColorStripSource
|
||||
from datetime import datetime, timezone
|
||||
|
||||
src = SingleColorStripSource(
|
||||
id="t",
|
||||
name="t",
|
||||
source_type="static",
|
||||
created_at=datetime.now(timezone.utc),
|
||||
updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
deps = StreamDeps(css_manager=None)
|
||||
s = STREAM_BUILDERS["static"](src, deps)
|
||||
assert isinstance(s, SingleColorStripStream)
|
||||
@@ -0,0 +1,64 @@
|
||||
"""Tests for the unified value-stream-builder registry."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from ledgrab.core.processing import value_kinds
|
||||
from ledgrab.core.processing.value_kinds import (
|
||||
NEEDS_CLOCK_RUNTIME,
|
||||
STREAM_BUILDERS,
|
||||
ValueStreamDeps,
|
||||
build_stream,
|
||||
)
|
||||
from ledgrab.storage.value_source import _VALUE_SOURCE_MAP
|
||||
|
||||
|
||||
def test_builders_cover_every_storage_kind():
|
||||
"""STREAM_BUILDERS keys == storage._VALUE_SOURCE_MAP keys."""
|
||||
storage_kinds = set(_VALUE_SOURCE_MAP.keys())
|
||||
builder_kinds = set(STREAM_BUILDERS.keys())
|
||||
assert storage_kinds == builder_kinds, (
|
||||
f"missing={storage_kinds - builder_kinds}, " f"extra={builder_kinds - storage_kinds}"
|
||||
)
|
||||
|
||||
|
||||
def test_build_stream_raises_on_unknown_kind():
|
||||
class _Fake:
|
||||
source_type = "totally_unregistered"
|
||||
id = "fake"
|
||||
|
||||
deps = ValueStreamDeps(value_stream_manager=None)
|
||||
with pytest.raises(ValueError, match="totally_unregistered"):
|
||||
build_stream(_Fake(), deps)
|
||||
|
||||
|
||||
def test_coverage_assertion_raises_on_drift(monkeypatch):
|
||||
"""A kind added to storage without a builder fails the import-time check."""
|
||||
monkeypatch.setitem(_VALUE_SOURCE_MAP, "phantom_kind", object)
|
||||
with pytest.raises(RuntimeError, match="phantom_kind"):
|
||||
value_kinds._assert_value_kind_coverage()
|
||||
|
||||
|
||||
def test_needs_clock_runtime_is_subset_of_registered_kinds():
|
||||
assert NEEDS_CLOCK_RUNTIME.issubset(set(STREAM_BUILDERS.keys()))
|
||||
|
||||
|
||||
def test_static_builder_returns_static_value_stream():
|
||||
from ledgrab.core.processing.value_stream import StaticValueStream
|
||||
from ledgrab.storage.value_source import StaticValueSource
|
||||
from datetime import datetime, timezone
|
||||
|
||||
src = StaticValueSource(
|
||||
id="vs_t",
|
||||
name="t",
|
||||
source_type="static",
|
||||
created_at=datetime.now(timezone.utc),
|
||||
updated_at=datetime.now(timezone.utc),
|
||||
value=0.42,
|
||||
)
|
||||
deps = ValueStreamDeps(value_stream_manager=None)
|
||||
stream = build_stream(src, deps)
|
||||
assert isinstance(stream, StaticValueStream)
|
||||
# Sanity: the value flowed through
|
||||
assert stream.get_value() == 0.42
|
||||
Reference in New Issue
Block a user