diff --git a/custom_components/houseplan/websocket_api.py b/custom_components/houseplan/websocket_api.py index 5242b08..28eaa56 100755 --- a/custom_components/houseplan/websocket_api.py +++ b/custom_components/houseplan/websocket_api.py @@ -477,12 +477,11 @@ async def ws_config_get(hass: HomeAssistant, connection, msg: dict[str, Any]) -> -def _missing_internal_plans(plans_dir: Path, config: dict[str, Any]) -> set[str]: - """Plan files a configuration names that are not on disk. +def _internal_plan_names(config: dict[str, Any]) -> set[str]: + """Plan file names a configuration names through OUR urls. - Only OUR urls are checked — `/api/houseplan/content/plans/_/` and the - legacy static path. Anything else belongs to the user and may point wherever - they like. + Only `/api/houseplan/content/plans/_/` and the legacy static path + count. Anything else belongs to the user and may point wherever they like. """ out: set[str] = set() for space in (config or {}).get("spaces") or []: @@ -492,11 +491,32 @@ def _missing_internal_plans(plans_dir: Path, config: dict[str, Any]) -> set[str] if not (url.startswith(CONTENT_URL + "/plans/") or url.startswith(PLANS_URL + "/")): continue name = plan_basename(url) - if name and not (plans_dir / name).is_file(): + if name: out.add(name) return out +def _missing_internal_plans( + plans_dir: Path, config: dict[str, Any], previous: dict[str, Any] | None = None +) -> set[str]: + """Newly named plan files that are not on disk. + + Guards the pick-then-save window: another client may delete a plan between + the moment this one chose it and the moment it saves, which would otherwise + store a url with nothing behind it (HP-1470-02). + + A name the stored configuration already carries is deliberately let through. + It is already broken — repairs says so — and refusing the write would lock + the owner out of every other edit, including the one that detaches it. + """ + known = _internal_plan_names(previous or {}) + return { + name + for name in _internal_plan_names(config) + if name not in known and not (plans_dir / name).is_file() + } + + @websocket_api.websocket_command( { vol.Required("type"): "houseplan/config/set", @@ -552,7 +572,10 @@ async def ws_config_set(hass: HomeAssistant, connection, msg: dict[str, Any]) -> # nothing about whether the file survived (HP-1470-02). External and # legacy urls are not ours to check and are left alone. missing = await hass.async_add_executor_job( - _missing_internal_plans, Path(hass.config.path(PLANS_DIR)), msg["config"] + _missing_internal_plans, + Path(hass.config.path(PLANS_DIR)), + msg["config"], + data.get("config"), ) if missing: connection.send_error( diff --git a/tests_backend/test_ha_websocket.py b/tests_backend/test_ha_websocket.py index 4a63b7c..967aa99 100644 --- a/tests_backend/test_ha_websocket.py +++ b/tests_backend/test_ha_websocket.py @@ -784,8 +784,22 @@ async def test_repair_issue_goes_when_its_space_does( client = await hass_ws_client(hass) registry = ir.async_get(hass) - gone = "/api/houseplan/content/plans/_/nosuchfile.png" - rev = (await _save(client, await _cfg([{"id": "r7", "plan_url": gone}]), 0))["result"]["rev"] + # A reference can only go bad AFTER it is stored — config/set refuses a new + # one that is already broken (HP-1470-02). So: attach a real plan, then let + # the file disappear the way it does in life, from outside Home Assistant. + import os + + from custom_components.houseplan.const import PLANS_DIR + + url, name = await _upload(client, "r7", b"PLAN") + rev = (await _save(client, await _cfg([{"id": "r7", "plan_url": url}]), 0))["result"]["rev"] + await hass.async_block_till_done() + assert registry.async_get_issue(HP_DOMAIN, "broken_plan_r7") is None, "the file is there" + + await hass.async_add_executor_job( + os.remove, os.path.join(hass.config.path(PLANS_DIR), name) + ) + rev = (await _save(client, await _cfg([{"id": "r7", "plan_url": url}]), rev))["result"]["rev"] await hass.async_block_till_done() assert registry.async_get_issue(HP_DOMAIN, "broken_plan_r7") is not None @@ -940,6 +954,9 @@ async def test_startup_sweep_collects_what_no_commit_will( await _setup(hass) client = await hass_ws_client(hass) files, _plans, p = _paths(hass) + # the plan it names has to exist: config/set refuses a NEW broken + # reference (HP-1470-02). The orphans still arrive after the save. + await _seed_aged(hass, [p["kept_plan"]]) assert (await _save(client, await _referenced_config(), 0))["success"] await _seed_aged(hass, list(p.values())) @@ -967,6 +984,9 @@ async def test_periodic_sweep_collects_too( await _setup(hass) client = await hass_ws_client(hass) _files, _plans, p = _paths(hass) + # the plan it names has to exist: config/set refuses a NEW broken + # reference (HP-1470-02). The orphans still arrive after the save. + await _seed_aged(hass, [p["kept_plan"]]) assert (await _save(client, await _referenced_config(), 0))["success"] await _seed_aged(hass, list(p.values())) @@ -994,6 +1014,9 @@ async def test_sweep_and_a_config_write_do_not_race( await _setup(hass) client = await hass_ws_client(hass) _files, plans, p = _paths(hass) + # the plan it names has to exist: config/set refuses a NEW broken + # reference (HP-1470-02). The orphans still arrive after the save. + await _seed_aged(hass, [p["kept_plan"]]) rev = (await _save(client, await _referenced_config(), 0))["result"]["rev"] # an aged, currently unreferenced plan that the commit below adopts @@ -1192,9 +1215,19 @@ async def test_uploads_are_bounded_by_a_store_quota( """HP-1470-01: nothing is deleted for being old, so growth stops at the door.""" from custom_components.houseplan import websocket_api as wsapi + from pathlib import Path + + from custom_components.houseplan.const import PLANS_DIR + from custom_components.houseplan.plans import dir_usage + await _setup(hass) client = await hass_ws_client(hass) - monkeypatch.setattr(wsapi, "MAX_PLANS_FILES", 2) + # every test in this module shares one config directory, so the plans folder + # is not empty here — budget two more from whatever is already stored + stored, _b = await hass.async_add_executor_job( + lambda: dir_usage(Path(hass.config.path(PLANS_DIR)))[::-1] + ) + monkeypatch.setattr(wsapi, "MAX_PLANS_FILES", stored + 2) await _upload(client, "q1", b"one") await _upload(client, "q1", b"two")