german/houseplan-card
1
0
Fork 0
mirror of https://github.com/Matysh/houseplan-card synced 2026-07-31 08:28:31 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

v1.46.6: the detach promise, actually kept this time

Browse Source 
v1.46.4 and v1.46.5 documented that detaching a plan leaves the image on disk,
added guards for it, and shipped tests. The guards were never reached: they sit
behind 'not superseded', and a file that left the configuration was called
superseded. From old_refs - new_refs alone, replacing a plan, detaching one and
deleting its space are indistinguishable — so all three deleted the file, at the
moment of the save, before any scheduled pass ever ran.

Every test I wrote for this called collect_plans(d, cfg, cfg): old config equal
to new, i.e. only the scheduled pass. The transition that mattered was never
exercised. Codex reproduced it in four lines.

Classification is by owner now:
  space in both, plan A -> plan B  : the user picked another image -> removed
  space in both, plan -> none      : detached -> kept
  space gone                       : kept (the image was imported; a thirty-day
                                     grace measured from file age is meaningless
                                     anyway, it was uploaded months ago)
  space has a plan, other file     : rejected upload -> 1 h
Attachments follow the same shape: dropped from a device that still exists ->
removed (a trash button promises nothing); device gone -> kept; staging folder
-> 1 h.

Tests: a matrix per rule in the pure module, and — the part that was missing —
test_detaching_a_plan_keeps_the_file, which goes through real config/set calls:
attach, detach, assert the file is there, restart, assert again, re-attach,
replace, assert the replaced one is gone, delete the space, assert the plan
survives. Also strengthened the sweep/save race test to assert the save actually
succeeded and the config points at the specific expected file, per the report.
This commit is contained in:
Matysh
2026-07-28 21:11:11 +03:00
parent 2c7a2f849d
commit 9868f1035f
15 changed files with 288 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests_backend/test_ha_websocket.py
+54 -5
View File
@@ -889,7 +889,7 @@ def _paths(hass):
"kept_file": os.path.join(files, "m5", "kept.pdf"),
"kept_plan": os.path.join(plans, "s5.tok.png"),
"orphan_file": os.path.join(files, "up_cancelled", "manual.pdf"),
"orphan_plan": os.path.join(plans, "deleted_space.orphan.png"),
"orphan_plan": os.path.join(plans, "s5.reject.png"),
}
@@ -992,18 +992,21 @@ async def test_sweep_and_a_config_write_do_not_race(
cfg2["spaces"][0]["plan_url"] = "/api/houseplan/content/plans/_/s5.newcomer.png"
entry = hass.config_entries.async_entries(DOMAIN)[0]
await asyncio.gather(
_reload, saved = await asyncio.gather(
hass.config_entries.async_reload(entry.entry_id), # runs the sweep
_save(client, cfg2, rev),
)
await hass.async_block_till_done()
# assert the CONCRETE outcome: a save that came back `not_ready` would leave
# the old config pointing at the old file and satisfy a vaguer check
assert saved["success"], saved.get("error")
await client.send_json_auto_id({"type": "houseplan/config/get"})
stored = (await client.receive_json())["result"]["config"]
referenced = stored["spaces"][0]["plan_url"].rsplit("/", 1)[-1]
assert stored["spaces"][0]["plan_url"].endswith("s5.newcomer.png")
assert await hass.async_add_executor_job(
os.path.isfile, os.path.join(plans, referenced)
), f"the accepted config points at {referenced}, which must exist"
os.path.isfile, os.path.join(plans, "s5.newcomer.png")
), "the file the accepted config points at must exist"
async def test_files_cleanup_keeps_referenced_files(
@@ -1048,3 +1051,49 @@ async def test_files_cleanup_keeps_referenced_files(
"a file the configuration still references survives a cleanup of its folder"
)
assert not await hass.async_add_executor_job(os.path.isfile, os.path.join(folder, "orphan.pdf"))
async def test_detaching_a_plan_keeps_the_file(
hass: HomeAssistant, hass_ws_client: WebSocketGenerator
) -> None:
"""HP-1465-01, through the real save — where the earlier tests never looked.
Every check for this lived in the pure collector with old and new config
equal, i.e. the scheduled pass. The transition that matters is a save, and
there the file was deleted the moment the reference was cleared.
"""
import os
from custom_components.houseplan.const import PLANS_DIR
await _setup(hass)
client = await hass_ws_client(hass)
plans = hass.config.path(PLANS_DIR)
url, name = await _upload(client, "d1", b"PLAN", ext="png")
rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": url}]), 0))["result"]["rev"]
assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name))
# detach: the space stays, its plan does not
rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": None}]), rev))["result"]["rev"]
assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name)), (
"the editor says the image stays on disk — it has to actually stay"
)
# a restart does not change its mind either
entry = hass.config_entries.async_entries(DOMAIN)[0]
assert await hass.config_entries.async_reload(entry.entry_id)
await hass.async_block_till_done()
assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name))
# re-attach, then replace: THAT removes the one it replaced
rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": url}]), rev))["result"]["rev"]
url2, name2 = await _upload(client, "d1", b"NEWPLAN", ext="png")
rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": url2}]), rev))["result"]["rev"]
assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name2))
assert not await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name))
# and deleting the space keeps its plan
await _save(client, await _cfg([]), rev)
await hass.async_block_till_done()
assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name2))
tests_backend/test_validation.py
+115 -74
View File
@@ -265,17 +265,6 @@ def test_collect_plans_keeps_a_fresh_unreferenced_upload(tmp_path):
assert (d / "f1.inflight.png").is_file()
def test_collect_plans_takes_an_aged_orphan(tmp_path):
collect_plans = plans.collect_plans
d = _plans(tmp_path, ["f1.keep.png"])
# past the long grace, and belonging to no space in the config
_plans(tmp_path, ["f1.abandoned.png"], age=const.SCHEDULED_GRACE_S + 60)
removed = collect_plans(d, _cfg("/p/f1.keep.png"), _cfg("/p/f1.keep.png"))
assert removed == 1
assert (d / "f1.keep.png").is_file() and not (d / "f1.abandoned.png").exists()
def test_collect_plans_never_touches_a_referenced_or_foreign_file(tmp_path):
PLAN_ORPHAN_TTL_S = const.PLAN_ORPHAN_TTL_S
collect_plans = plans.collect_plans
@@ -477,31 +466,6 @@ def test_attachment_refs_reads_marker_urls():
assert plans.attachment_refs(cfg) == set()
def test_collect_attachments_supersedes_and_ages(tmp_path):
import os
import time
collect_attachments = plans.collect_attachments
files = tmp_path / "files"
(files / "m1").mkdir(parents=True)
for n in ("old.pdf", "new.pdf", "cancelled.pdf"):
(files / "m1" / n).write_bytes(b"x")
# the commit swapped old.pdf for new.pdf; cancelled.pdf is a fresh upload
# nobody saved — it may belong to a dialog that is still open
removed = collect_attachments(files, _acfg("m1/old.pdf"), _acfg("m1/new.pdf"))
assert removed == 1
assert not (files / "m1" / "old.pdf").exists()
assert (files / "m1" / "new.pdf").is_file()
assert (files / "m1" / "cancelled.pdf").is_file()
old = time.time() - const.SCHEDULED_GRACE_S - 60
os.utime(files / "m1" / "cancelled.pdf", (old, old))
assert collect_attachments(files, _acfg("m1/new.pdf"), _acfg("m1/new.pdf")) == 1
assert not (files / "m1" / "cancelled.pdf").exists()
assert (files / "m1" / "new.pdf").is_file()
def test_collect_attachments_removes_the_empty_folder_and_never_raises(tmp_path):
import os
import time
@@ -554,53 +518,130 @@ def test_legacy_segments_are_dropped_by_the_server():
assert "segments" not in out
def test_scheduled_collection_never_takes_a_detached_plan(tmp_path):
"""2026-07-28, on the author's own instance: two plans were deleted.
Detaching a plan (switching a space to "draw") is reversible and the editor
says the file stays on disk. The timer only knows "nothing points at it",
applied the one-hour orphan rule, and removed images that had been detached
weeks earlier. A commit may still collect what it superseded — it knows it
replaced something. The timer may not.
"""
def _aged(path, seconds):
import os
import time
t = time.time() - seconds
os.utime(path, (t, t))
def _sp(sid, url):
return {"id": sid, "plan_url": f"/api/houseplan/content/plans/_/{url}" if url else None}
def test_plan_collection_matrix(tmp_path):
"""Which config transition means "the user asked for this file to go"?
`old_refs - new_refs` cannot tell replace, detach and delete-space apart —
they look identical. v1.46.4/v1.46.5 added guards for detach and only ever
reached them on the scheduled pass, so the commit itself still deleted a
detached plan the moment it was detached (HP-1465-01). One case is a
deletion the user asked for; the rest are kept.
"""
collect = plans.collect_plans
d = tmp_path / "plans"
d.mkdir()
for n in ("f1.svg", "f2.tok.png", "gone.old.png"):
(d / n).write_bytes(b"x")
t = time.time() - const.SCHEDULED_GRACE_S - 60
os.utime(d / n, (t, t))
cfg = {"spaces": [{"id": "f1", "plan_url": None}, # detached, space alive
{"id": "f2", "plan_url": None}]} # same
assert plans.collect_plans(d, cfg, cfg) == 1
assert (d / "f1.svg").is_file(), "a detached plan of a live space is kept, at any age"
assert (d / "f2.tok.png").is_file()
assert not (d / "gone.old.png").exists(), "a plan of a deleted space ages out after a month"
def seed(*names):
for n in names:
(d / n).write_bytes(b"x")
_aged(d / n, const.SCHEDULED_GRACE_S * 2) # old enough for any rule
# a space that HAS a plan: its other files can only be its own rejects
import os
import time
# 1. replace: the user picked a different image for the same space
seed("f1.old.png", "f1.new.png")
assert collect(d, {"spaces": [_sp("f1", "f1.old.png")]},
{"spaces": [_sp("f1", "f1.new.png")]}) == 1
assert not (d / "f1.old.png").exists() and (d / "f1.new.png").is_file()
(d / "f9.current.png").write_bytes(b"x")
(d / "f9.reject.png").write_bytes(b"x")
hour = time.time() - const.PLAN_ORPHAN_TTL_S - 60
os.utime(d / "f9.reject.png", (hour, hour))
live = {"spaces": [{"id": "f1", "plan_url": None}, {"id": "f2", "plan_url": None},
{"id": "f9", "plan_url": "/p/f9.current.png"}]}
assert plans.collect_plans(d, live, live) == 1
assert (d / "f9.current.png").is_file()
assert not (d / "f9.reject.png").exists()
# 2. detach: same space, switched to "draw"
seed("f2.png")
assert collect(d, {"spaces": [_sp("f2", "f2.png")]},
{"spaces": [_sp("f2", None)]}) == 0
assert (d / "f2.png").is_file(), "the editor says the file stays — it stays"
# a commit still removes what it SUPERSEDED — that it knows for certain
old = {"spaces": [{"id": "f1", "plan_url": "/p/f1.svg"}, {"id": "f2", "plan_url": None}]}
new = {"spaces": [{"id": "f1", "plan_url": "/p/f1.new.png"}, {"id": "f2", "plan_url": None}]}
(d / "f1.new.png").write_bytes(b"x")
assert plans.collect_plans(d, old, new) == 1
assert not (d / "f1.svg").exists()
assert (d / "f2.tok.png").is_file(), "and still touches nothing else of a live space"
# 3. the space is deleted outright
seed("f3.png")
assert collect(d, {"spaces": [_sp("f3", "f3.png")]}, {"spaces": []}) == 0
assert (d / "f3.png").is_file()
# 4. the scheduled pass, later, still keeps both
cfg = {"spaces": [_sp("f2", None)]}
assert collect(d, cfg, cfg) == 0
assert (d / "f2.png").is_file() and (d / "f3.png").is_file()
# 5. a rejected upload: the space HAS a plan, this file never was one
seed("f4.current.png", "f4.reject.png")
live = {"spaces": [_sp("f4", "f4.current.png")]}
assert collect(d, live, live) == 1
assert (d / "f4.current.png").is_file() and not (d / "f4.reject.png").exists()
# …but only once it is old; a fresh one may be a transaction in flight
(d / "f4.fresh.png").write_bytes(b"x")
assert collect(d, live, live) == 0
assert (d / "f4.fresh.png").is_file()
# 6. the same file still referenced by another space is never touched
seed("shared.png")
assert collect(d, {"spaces": [_sp("a", "shared.png"), _sp("b", "shared.png")]},
{"spaces": [_sp("a", None), _sp("b", "shared.png")]}) == 0
assert (d / "shared.png").is_file()
def test_attachment_collection_matrix(tmp_path):
"""Removing an attachment is a trash button; deleting the device is not."""
collect = plans.collect_attachments
def case(name):
d = tmp_path / name
d.mkdir()
return d
def seed(root, folder, fname, age=None):
(root / folder).mkdir(parents=True, exist_ok=True)
p = root / folder / fname
p.write_bytes(b"x")
if age:
_aged(p, age)
return p
def cfg(*markers):
return {"markers": [
{"id": mid, "pdfs": [{"url": f"/api/houseplan/content/files/{mid}/{n}"} for n in names]}
for mid, names in markers
]}
# 1. the user removed one attachment from a device that still exists
d = case("dropped")
seed(d, "m1", "dropped.pdf")
seed(d, "m1", "kept.pdf")
assert collect(d, cfg(("m1", ["dropped.pdf", "kept.pdf"])), cfg(("m1", ["kept.pdf"]))) == 1
assert not (d / "m1" / "dropped.pdf").exists()
assert (d / "m1" / "kept.pdf").is_file()
# 2. the device itself is gone: its manuals are not ours to throw away
d = case("device_gone")
seed(d, "m2", "manual.pdf", age=const.SCHEDULED_GRACE_S * 2)
assert collect(d, cfg(("m2", ["manual.pdf"])), cfg()) == 0
assert (d / "m2" / "manual.pdf").is_file()
# and the scheduled pass, later, agrees
assert collect(d, cfg(), cfg()) == 0
assert (d / "m2" / "manual.pdf").is_file()
# 3. a dialog that was never saved, in its own staging folder
d = case("staging")
seed(d, "up_x", "manual.pdf", age=const.PLAN_ORPHAN_TTL_S + 60)
assert collect(d, cfg(), cfg()) == 1
assert not (d / "up_x").exists()
# 4. an upload into a live device's folder whose save was rejected
d = case("reject")
seed(d, "m3", "current.pdf")
seed(d, "m3", "rejected.pdf", age=const.SCHEDULED_GRACE_S + 60)
live = cfg(("m3", ["current.pdf"]))
assert collect(d, live, live) == 1
assert (d / "m3" / "current.pdf").is_file()
assert not (d / "m3" / "rejected.pdf").exists()
def test_attachment_grace_is_a_month_outside_a_staging_folder(tmp_path):