mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-04 13:48:57 +00:00
fix(config): an ordinary save without led_strips keeps the stored shapes (#780)
r1 H1: a previous frontend sends every space without the unknown field and the ordinary write path replaced the document with it, erasing every LED strip. The shared ordinary-writer helper now copies the stored shapes of a space the payload omits (config/set and Optimize); an explicit list from a new client still deletes, a removed space takes its strips, and a link to a marker the same write deleted unbinds by the usual rule. config/set reports the normalisation counters after the preservation. Issue: #780 User-Visible: no
This commit is contained in:
@@ -28,6 +28,7 @@ Write-path order (ТЗ #780 §9):
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import copy
|
||||
import math
|
||||
from collections.abc import Iterator
|
||||
from typing import Any
|
||||
@@ -161,6 +162,33 @@ def _strips(config: dict[str, Any]) -> Iterator[tuple[dict[str, Any], dict[str,
|
||||
yield space, strip
|
||||
|
||||
|
||||
def preserve_led_strips(config: dict[str, Any], previous: dict[str, Any] | None) -> int:
|
||||
"""An ordinary writer that omits ``led_strips`` keeps the stored shapes (#780 r1 H1).
|
||||
|
||||
A previous frontend does not know the field and sends every space without
|
||||
it; the omission is not a deletion. A new client deletes shapes by sending
|
||||
an explicit list (``[]`` removes all of them). Applies to spaces that keep
|
||||
their id; a space the writer removed takes its strips with it. Runs on the
|
||||
ordinary write path before normalisation, so a preserved link to a marker
|
||||
the same write deleted becomes an unbound strip by the usual rule.
|
||||
Returns how many spaces received their stored shapes back.
|
||||
"""
|
||||
stored = {
|
||||
space.get("id"): space[LED_KEY]
|
||||
for space in (previous or {}).get("spaces") or []
|
||||
if isinstance(space, dict) and isinstance(space.get(LED_KEY), list) and space[LED_KEY]
|
||||
}
|
||||
restored = 0
|
||||
for space in config.get("spaces") or []:
|
||||
if not isinstance(space, dict) or LED_KEY in space:
|
||||
continue
|
||||
shapes = stored.get(space.get("id"))
|
||||
if shapes:
|
||||
space[LED_KEY] = copy.deepcopy(shapes)
|
||||
restored += 1
|
||||
return restored
|
||||
|
||||
|
||||
def led_strip_link_report(config: dict[str, Any]) -> dict[str, int]:
|
||||
"""What the normaliser would change, without changing anything.
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ from custom_components.houseplan.coordinate_canonicalization import (
|
||||
from custom_components.houseplan.led_strips import (
|
||||
LED_STRIPS_SCHEMA,
|
||||
config_led_strip_links,
|
||||
preserve_led_strips,
|
||||
)
|
||||
from custom_components.houseplan.vacuum_routes import validate_marker_routes
|
||||
|
||||
@@ -2274,13 +2275,16 @@ def prepare_ordinary_summary_candidate(
|
||||
readable_entity_ids: set[str],
|
||||
normalize,
|
||||
) -> dict:
|
||||
"""Apply the one summary-panel contract shared by ordinary config writers.
|
||||
"""Apply the one preservation contract shared by ordinary config writers.
|
||||
|
||||
`normalize` owns the writer-specific schema/migration sequence. Preservation
|
||||
must happen before it; change-aware reference validation must happen after
|
||||
it. Authoritative full import/restore intentionally does not use this helper.
|
||||
Preserved: the summary panel namespace (#437) and the LED strip shapes of a
|
||||
space an older writer sent without the field (#780).
|
||||
"""
|
||||
preserve_summary_panel_namespace(candidate, previous)
|
||||
preserve_led_strips(candidate, previous)
|
||||
checked = normalize(candidate)
|
||||
validate_summary_panel_references(checked, previous, readable_entity_ids)
|
||||
return checked
|
||||
|
||||
@@ -70,7 +70,7 @@ from .import_export import (
|
||||
revalidate_candidate,
|
||||
)
|
||||
from .junction_limits import JunctionLimitError, validate_junction_limits
|
||||
from .led_strips import led_strip_link_report
|
||||
from .led_strips import led_strip_link_report, preserve_led_strips
|
||||
from .plans import (
|
||||
QuotaError,
|
||||
collect_attachments,
|
||||
@@ -1685,6 +1685,9 @@ async def ws_config_set(hass: HomeAssistant, connection, msg: dict[str, Any]) ->
|
||||
# #780: what the write path will normalise in LED strip links (an
|
||||
# orphan becomes unbound, an empty marker.space adopts the strip's
|
||||
# space). Reported back so a new client re-reads; old ones ignore it.
|
||||
# An older writer omits the field: its stored shapes are kept first
|
||||
# (r1 H1), so the report also counts their links.
|
||||
preserve_led_strips(msg["config"], data.get("config"))
|
||||
led_report = led_strip_link_report(msg["config"])
|
||||
|
||||
def _validate_config_cpu():
|
||||
|
||||
@@ -1139,8 +1139,14 @@ led_strips?: Array<{ id: string; points: [number, number][]; marker: string | nu
|
||||
adopts the strip's space. A non-empty foreign `space` or a second link
|
||||
still rejects the whole write. `config/set` reports the counts
|
||||
`{led_strips: {unbound, space_adopted}}`; older clients may ignore them.
|
||||
- **Older clients** keep the unknown array on an ordinary save; deleting a
|
||||
bound marker succeeds and leaves an unbound strip.
|
||||
- **Older clients.** An ordinary writer (`config/set`, Optimize) that omits
|
||||
the field keeps the stored shapes of every space it still sends: the backend
|
||||
copies them from the stored revision before normalisation
|
||||
(`preserve_led_strips`, r1 H1). Omission is never a deletion; a new client
|
||||
deletes shapes only with an explicit list (`[]` removes all). Deleting a
|
||||
bound marker in the same write succeeds and leaves an unbound strip; a
|
||||
removed space takes its strips with it. Full import/restore is authoritative
|
||||
and does not preserve.
|
||||
- **Transfer.** Full export/import keeps geometry, links and `active`. A
|
||||
space import remaps links through the same marker-id map as the devices;
|
||||
a strip whose marker did not travel (skipped duplicate, absent) arrives
|
||||
|
||||
@@ -14374,6 +14374,16 @@ const MUTANT_DEFINITIONS = [
|
||||
replace: ' unbound_led_strips = unbind_strips(space, remap={\n old_id: old_id for old_id, new_id in marker_map.items()',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'led-old-writer-drops-strips',
|
||||
guard: 'node scripts/backend-test-guard.mjs old_writer tests_backend/test_led_strips.py',
|
||||
because: '#780 r1 H1: an ordinary save without the unknown field keeps the stored shapes; only an explicit list deletes',
|
||||
patches: [{
|
||||
file: 'custom_components/houseplan/validation.py',
|
||||
find: ' preserve_summary_panel_namespace(candidate, previous)\n preserve_led_strips(candidate, previous)',
|
||||
replace: ' preserve_summary_panel_namespace(candidate, previous)',
|
||||
}],
|
||||
},
|
||||
];
|
||||
|
||||
const mutationCardSource = readFileSync(join(repoRoot, 'src/houseplan-card.ts'), 'utf8');
|
||||
|
||||
@@ -4485,3 +4485,69 @@ async def test_issue_780_old_client_deleting_a_bound_marker_still_saves(
|
||||
await client.send_json_auto_id({"type": "houseplan/config/get"})
|
||||
unchanged = (await client.receive_json())["result"]
|
||||
assert unchanged["rev"] == second["result"]["rev"], "a rejected write leaves the revision"
|
||||
|
||||
|
||||
async def test_issue_780_old_writer_omitting_led_strips_keeps_the_shapes(
|
||||
hass: HomeAssistant, hass_ws_client: WebSocketGenerator,
|
||||
) -> None:
|
||||
"""r1 H1: a previous frontend does not know the field and sends every space
|
||||
without it. The omission is not a deletion: the stored shapes survive, a
|
||||
link to a marker the same write deleted becomes unbound, and only an
|
||||
explicit list from a new client removes them. Judged by a fresh connection."""
|
||||
await _setup(hass)
|
||||
client = await hass_ws_client(hass)
|
||||
space = _space("f1", "r1")
|
||||
shapes = [
|
||||
{"id": "led-1", "points": [[0.1, 0.1], [0.6, 0.1], [0.6, 0.5]], "marker": "lamp", "active": False},
|
||||
{"id": "led-2", "points": [[0.2, 0.2], [0.5, 0.2]], "marker": None, "active": True},
|
||||
]
|
||||
config = {
|
||||
"spaces": [{**space, "led_strips": copy.deepcopy(shapes)}, _space("f2", "r2")],
|
||||
"markers": [{"id": "lamp", "binding": "entity:light.kitchen", "space": "f1"}],
|
||||
"settings": {},
|
||||
}
|
||||
await client.send_json_auto_id({"type": "houseplan/config/set", "config": config, "expected_rev": 0})
|
||||
first = await client.receive_json()
|
||||
assert first["success"], first
|
||||
|
||||
# The old writer: the same plan, no `led_strips` key anywhere.
|
||||
old_writer = copy.deepcopy(config)
|
||||
for item in old_writer["spaces"]:
|
||||
item.pop("led_strips", None)
|
||||
old_writer["markers"][0]["name"] = "Kitchen strip"
|
||||
await client.send_json_auto_id({
|
||||
"type": "houseplan/config/set", "config": old_writer, "expected_rev": first["result"]["rev"],
|
||||
})
|
||||
kept = await client.receive_json()
|
||||
assert kept["success"], kept
|
||||
assert "led_strips" not in kept["result"], "nothing to normalise: the shapes came back as stored"
|
||||
|
||||
# The old writer deletes the bound marker: the save stands, the kept shape unbinds.
|
||||
deleting = copy.deepcopy(old_writer)
|
||||
deleting["markers"] = []
|
||||
await client.send_json_auto_id({
|
||||
"type": "houseplan/config/set", "config": deleting, "expected_rev": kept["result"]["rev"],
|
||||
})
|
||||
unbound = await client.receive_json()
|
||||
assert unbound["success"], unbound
|
||||
assert unbound["result"]["led_strips"] == {"unbound": 1, "space_adopted": 0}
|
||||
|
||||
fresh = await hass_ws_client(hass)
|
||||
await fresh.send_json_auto_id({"type": "houseplan/config/get"})
|
||||
stored = (await fresh.receive_json())["result"]
|
||||
assert stored["config"]["spaces"][0]["led_strips"] == [
|
||||
{**shapes[0], "marker": None, "active": True}, shapes[1],
|
||||
]
|
||||
assert "led_strips" not in stored["config"]["spaces"][1]
|
||||
|
||||
# A new client deletes every shape with an explicit empty list.
|
||||
explicit = copy.deepcopy(stored["config"])
|
||||
explicit["spaces"][0]["led_strips"] = []
|
||||
await client.send_json_auto_id({
|
||||
"type": "houseplan/config/set", "config": explicit, "expected_rev": stored["rev"],
|
||||
})
|
||||
assert (await client.receive_json())["success"]
|
||||
reader = await hass_ws_client(hass)
|
||||
await reader.send_json_auto_id({"type": "houseplan/config/get"})
|
||||
emptied = (await reader.receive_json())["result"]
|
||||
assert emptied["config"]["spaces"][0].get("led_strips") == []
|
||||
|
||||
@@ -185,6 +185,48 @@ def test_deleted_marker_leaves_an_unbound_strip_with_its_geometry(active) -> Non
|
||||
assert report == {"unbound": 1, "space_adopted": 0}
|
||||
|
||||
|
||||
def _old_writer(config: dict) -> dict:
|
||||
"""The payload of a frontend that does not know the field (r1 H1)."""
|
||||
payload = copy.deepcopy(config)
|
||||
for space in payload["spaces"]:
|
||||
space.pop("led_strips", None)
|
||||
return payload
|
||||
|
||||
|
||||
def _ordinary(candidate: dict, previous: dict) -> dict:
|
||||
return v.prepare_ordinary_summary_candidate(copy.deepcopy(candidate), previous, set(), v.CONFIG_SCHEMA)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("active", [True, False])
|
||||
def test_old_writer_omitting_the_field_keeps_the_stored_shapes(active) -> None:
|
||||
"""r1 H1: an omitted ``led_strips`` is not a deletion on the ordinary write path."""
|
||||
previous = _check(_config([_strip(active=active)]))
|
||||
checked = _ordinary(_old_writer(previous), previous)
|
||||
assert _strips_of(checked) == _strips_of(previous)
|
||||
|
||||
|
||||
def test_old_writer_deleting_the_marker_unbinds_the_kept_shape() -> None:
|
||||
previous = _check(_config([_strip(active=False)]))
|
||||
payload = _old_writer(previous)
|
||||
payload["markers"] = []
|
||||
strip = _strips_of(_ordinary(payload, previous))[0]
|
||||
assert strip == {**_strip(), "marker": None, "active": True}
|
||||
|
||||
|
||||
def test_explicit_empty_list_deletes_and_a_removed_space_takes_its_shapes() -> None:
|
||||
previous = _check(_config([_strip()], extra_spaces=("upper",)))
|
||||
explicit = copy.deepcopy(previous)
|
||||
explicit["spaces"][0]["led_strips"] = []
|
||||
assert _strips_of(_ordinary(explicit, previous)) == []
|
||||
removed = _old_writer(previous)
|
||||
removed["spaces"] = [space for space in removed["spaces"] if space["id"] != "ground"]
|
||||
removed["markers"] = []
|
||||
checked = _ordinary(removed, previous)
|
||||
assert all("led_strips" not in space for space in checked["spaces"])
|
||||
# The preservation never invents a key for a space that had none.
|
||||
assert "led_strips" not in _ordinary(_old_writer(_check(_config())), _check(_config()))["spaces"][0]
|
||||
|
||||
|
||||
def test_tombstoned_marker_is_not_live() -> None:
|
||||
markers = [{"id": "lamp", "binding": "entity:light.kitchen", "space": "ground", "removed": True}]
|
||||
strip = _strips_of(_check(_config([_strip()], markers)))[0]
|
||||
|
||||
Reference in New Issue
Block a user