Compare commits

...
4 Commits
Author SHA1 Message Date
Matysh b7ae3e7adf Release v1.46.2
Validate / hacs (push) Failing after 7s
Validate / hassfest (push) Failing after 7s
Validate / frontend (push) Successful in 1m51s
Validate / backend (push) Failing after 7m11s
Validate / smoke (push) Failing after 6m35s
Re-check of v1.46.1: the scheduled sweep now collects unreferenced attachments
and plans against the stored configuration, and a drag in flight survives a
concurrent remote position change.
2026-07-28 17:47:19 +03:00
Matysh 379fb68db2 v1.46.2: re-check of v1.46.1 — HP-1461-01, -02
Validate / hacs (push) Failing after 23s
Validate / hassfest (push) Failing after 22s
Validate / frontend (push) Successful in 1m40s
Validate / backend (push) Failing after 7m16s
Validate / smoke (push) Failing after 7m13s
HP-1461-01: collection was tied to config/set, which is the right scope for
what a commit supersedes but leaves a file nobody references with no future
write to notice it — cancel a dialog after the upload finished, drop the
connection just after, or call the upload API directly. The daily sweep added
in v1.46.1 only removed streaming temporaries, so the documented 'a cancelled
attachment is collected an hour later' did not hold on an instance nobody
edits. The scheduled pass now loads the stored config under the same write_lock
a commit uses and runs collect_attachments/collect_plans with it as BOTH sides:
nothing counts as superseded, referenced files are preserved, aged unreferenced
ones go. Doing it under the lock keeps it from deciding on a snapshot a commit
is about to replace.

HP-1461-02: _reloadLayoutOnly captured the dirty set AFTER flushing the pending
write, and the flush empties it first — so during a real drag (where a write is
already scheduled) the snapshot was empty and the server's older position was
merged over the user's move. The snapshot is taken before the flush, by value,
and a _sentPos map now holds positions that are sent but unacknowledged, which
closes the same window for a write that was already in flight.

Tests: the upload test now cancels the request task for real (the previous one
claimed to and only walked error paths); smoke_layout_sync schedules a genuine
debounced write and delays it — verified failing on a v1.46.1 build with
exactly the reported symptom; a new backend test reloads the entry and asserts
the scheduled sweep takes an aged cancelled attachment and an orphan plan while
keeping everything the config still references.
Docs: CHANGELOG.md + CHANGELOG.ru.md + ARCHITECTURE.md + TESTING.md + STATUS.md.
2026-07-28 17:44:03 +03:00
Matysh 96a70495d3 Release v1.46.1
Re-check of v1.46.0: atomic filename reservation with a bounded collision name,
unconditional cleanup of streaming temporaries plus a scheduled sweep, and the
full card following layout events with a dirty-position merge.
2026-07-28 16:51:19 +03:00
Matysh d3db9e30e6 v1.46.1: re-check of v1.46.0 — HP-1460-01, -02, -03
HP-1460-01: v1.46.0 stopped overwriting attachments, but picking a free name
and taking it were two steps. Two uploads racing between them agreed on the
same name, both answered 200, and one set of bytes replaced the other;
files/migrate had the same check-then-copy gap. reserve_filename now claims the
name with O_CREAT|O_EXCL as it picks it, and both paths use it. It also splits
the extension off the RAW name and budgets the stem against MAX_FILENAME
including the collision tag — a maximal name lost its '.pdf' and then grew past
the limit, so the view sanitised the request back to a different name and the
attachment 404'd for good.

HP-1460-02: cleanup lived in an 'except Exception', which CancelledError walks
past, only one tmp_path was tracked, promotion had no finally, and the
collector only walks marker folders — an aborted transfer stranded a .upload-*
that nothing would ever remove. An outer finally owns every temporary, a second
'file' part is refused, promotion failure cleans up, and sweep_upload_temps
runs at setup, daily, and inside the commit-scoped collector. Chunks are
batched to 1 MB per disk task instead of one per 64 KB.

HP-1460-03: the layout event reached the static card and not the full one, so
two full cards diverged until a reload. The full card subscribes now and
re-reads ONLY the layout, keyed on its revision. Two hazards handled: it
records revisions it produced itself, and the reaction is deferred ~200 ms
because the event can beat the reply to our own write over the same socket;
positions dragged but not yet sent are flushed and merged on top, so a fix for
a stale UI cannot become a lost drag.

Tests: smoke_layout_sync (fails on a v1.46.0 build), four pure tests for atomic
reservation incl. 20-thread concurrency and the length boundary, a backend test
walking every failing exit path of an upload, and — as the report asked — an
HA-harness test that a repair issue disappears with its space.
Docs: CHANGELOG.md + CHANGELOG.ru.md + ARCHITECTURE.md + TESTING.md + STATUS.md.
2026-07-28 16:48:32 +03:00
21 changed files with 899 additions and 156 deletions
+46 -1
View File
@@ -2,11 +2,13 @@
from __future__ import annotations from __future__ import annotations
import logging import logging
from datetime import timedelta
from pathlib import Path from pathlib import Path
from homeassistant.components.frontend import add_extra_js_url from homeassistant.components.frontend import add_extra_js_url
from homeassistant.core import HomeAssistant from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers.event import async_track_time_interval
from . import websocket_api as hp_ws from . import websocket_api as hp_ws
from .const import ( from .const import (
@@ -18,8 +20,9 @@ from .const import (
PLANS_URL, PLANS_URL,
VERSION, VERSION,
) )
from .plans import collect_attachments, collect_plans, sweep_upload_temps
from .repairs import async_check_plan_files from .repairs import async_check_plan_files
from .store import HouseplanConfigEntry, create_data from .store import HouseplanConfigEntry, create_data, get_data
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
@@ -97,6 +100,48 @@ async def async_setup_entry(hass: HomeAssistant, entry: HouseplanConfigEntry) ->
) )
await async_check_plan_files(hass, entry) await async_check_plan_files(hass, entry)
# Scheduled collection of everything nobody ended up referencing.
#
# A commit collects what that commit superseded, which is the right rule for
# a commit — but it only ever runs when somebody saves. Cancel a dialog
# after the file has already uploaded, lose the connection after the upload
# succeeded, or call the API directly, and the file is unreferenced with no
# future write to notice it (HP-1461-01). The earlier version of this sweep
# only removed streaming temporaries, which are a different, narrower case.
#
# Passing the CURRENT configuration as both sides means "nothing was
# superseded": every referenced file is preserved and only unreferenced ones
# past PLAN_ORPHAN_TTL_S go. It runs under the same lock as a config write,
# so it cannot decide from a snapshot that a commit is about to replace.
async def _sweep(_now=None) -> None:
files_dir = Path(hass.config.path(FILES_DIR))
plans_dir = Path(hass.config.path(PLANS_DIR))
try:
data = get_data(hass)
if data is None: # entry unloaded — nothing authoritative to compare against
await hass.async_add_executor_job(sweep_upload_temps, files_dir)
return
async with data.write_lock:
stored = await data.config_store.async_load() or {}
cfg = stored.get("config") or {}
def _collect() -> int:
n = sweep_upload_temps(files_dir)
n += collect_attachments(files_dir, cfg, cfg)
n += collect_plans(plans_dir, cfg, cfg)
return n
n = await hass.async_add_executor_job(_collect)
if n:
_LOGGER.info("House Plan: removed %s unreferenced file(s)", n)
except Exception: # noqa: BLE001 — housekeeping must never fail a setup
_LOGGER.exception("House Plan: sweeping unreferenced files failed")
await _sweep()
entry.async_on_unload(
async_track_time_interval(hass, _sweep, timedelta(hours=24))
)
return True return True
+1 -1
View File
@@ -24,7 +24,7 @@ MAX_SIGN_PATHS = 200
PLAN_ORPHAN_TTL_S = 3600 PLAN_ORPHAN_TTL_S = 3600
FILES_DIR = "houseplan/files" FILES_DIR = "houseplan/files"
CONF_ADMIN_ONLY = "admin_only" CONF_ADMIN_ONLY = "admin_only"
VERSION = "1.46.0" VERSION = "1.46.2"
DEFAULT_CONFIG: dict = { DEFAULT_CONFIG: dict = {
"spaces": [], "spaces": [],
File diff suppressed because one or more lines are too long
+107 -75
View File
@@ -22,7 +22,7 @@ from homeassistant.core import HomeAssistant
from .const import CONF_ADMIN_ONLY, CONTENT_URL, FILES_DIR, FILES_URL, PLANS_DIR from .const import CONF_ADMIN_ONLY, CONTENT_URL, FILES_DIR, FILES_URL, PLANS_DIR
from .auth import may_write from .auth import may_write
from .plans import unique_filename from .plans import TMP_PREFIX, reserve_filename
from .validation import ( from .validation import (
FILE_EXTENSIONS, FILE_EXTENSIONS,
MAX_FILE_BYTES, MAX_FILE_BYTES,
@@ -34,6 +34,8 @@ from .validation import (
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_CHUNK = 64 * 1024 _CHUNK = 64 * 1024
# batch disk writes: one executor job per megabyte instead of per chunk
_FLUSH_AT = 1024 * 1024
_MIME = { _MIME = {
".pdf": "application/pdf", ".pdf": "application/pdf",
@@ -119,92 +121,122 @@ class HouseplanUploadView(HomeAssistantView):
if not may_write(hass, request.get("hass_user")): if not may_write(hass, request.get("hass_user")):
return web.json_response({"error": "unauthorized"}, status=403) return web.json_response({"error": "unauthorized"}, status=403)
files_root = Path(hass.config.path(FILES_DIR))
marker_id = "misc" marker_id = "misc"
filename: str | None = None filename: str | None = None
tmp_path: Path | None = None # Every temporary file this request creates, promoted or not. The outer
too_large = False # `finally` removes whatever is left: a dropped connection, a second
bad_ext = False # `file` part or a failure while promoting used to leave a `.upload-*`
files_root = Path(hass.config.path(FILES_DIR)) # behind for good, and the collector only ever walks marker folders, so
# nothing would have picked it up (HP-1460-02).
temps: list[Path] = []
error: tuple[dict, int] | None = None
def _open_tmp() -> Path: def _new_tmp() -> Path:
files_root.mkdir(parents=True, exist_ok=True) files_root.mkdir(parents=True, exist_ok=True)
fd, name = tempfile.mkstemp(prefix=".upload-", dir=str(files_root)) fd, name = tempfile.mkstemp(prefix=TMP_PREFIX, dir=str(files_root))
os.close(fd) os.close(fd)
return Path(name) return Path(name)
def _append(target: Path, data: bytes) -> None: def _flush(target: Path, blocks: list[bytes]) -> None:
with open(target, "ab") as fh: with open(target, "ab") as fh:
fh.write(data) for block in blocks:
fh.write(block)
def _discard(target: Path) -> None: def _cleanup(paths: list[Path]) -> None:
try: for path in paths:
target.unlink() try:
except OSError: path.unlink()
pass except OSError:
pass
try: try:
reader = await request.multipart() try:
async for part in reader: reader = await request.multipart()
if part.name == "marker_id": async for part in reader:
marker_id = sanitize_marker_id(await part.text()) if part.name == "marker_id":
elif part.name == "file": marker_id = sanitize_marker_id(await part.text())
filename = part.filename or "file" elif part.name == "file":
if file_ext(filename) not in FILE_EXTENSIONS: if filename is not None:
bad_ext = True # one upload per request: a second part would strand
break # the first temporary file and make the response
# Stream to a temporary file instead of collecting the whole # ambiguous about which url was returned
# upload in memory and copying it again into one buffer: a error = ({"error": "one_file_only"}, 400)
# 50 MB manual used to cost ~100 MB of RSS mid-request, and
# a few of those at once is real pressure on a small Home
# Assistant host (HP-1454-06).
tmp_path = await hass.async_add_executor_job(_open_tmp)
size = 0
while chunk := await part.read_chunk(_CHUNK):
size += len(chunk)
if size > MAX_FILE_BYTES:
too_large = True
break break
await hass.async_add_executor_job(_append, tmp_path, chunk) filename = part.filename or "file"
if too_large: if file_ext(filename) not in FILE_EXTENSIONS:
break error = ({"error": "bad_ext", "allowed": sorted(FILE_EXTENSIONS)}, 400)
except Exception as err: # noqa: BLE001 break
_LOGGER.warning("House Plan upload: multipart read error: %s", err) # Stream to a temporary file instead of collecting the
if tmp_path is not None: # whole upload in memory and copying it again into one
await hass.async_add_executor_job(_discard, tmp_path) # buffer: a 50 MB manual used to cost ~100 MB of RSS
return web.json_response({"error": "bad_request"}, status=400) # mid-request (HP-1454-06). Blocks are batched so this
# is one executor job per megabyte, not per 64 KB.
tmp = await hass.async_add_executor_job(_new_tmp)
temps.append(tmp)
size = 0
pending: list[bytes] = []
buffered = 0
while chunk := await part.read_chunk(_CHUNK):
size += len(chunk)
if size > MAX_FILE_BYTES:
error = (
{"error": "too_large", "max_mb": MAX_FILE_BYTES // 1024 // 1024},
413,
)
break
pending.append(chunk)
buffered += len(chunk)
if buffered >= _FLUSH_AT:
await hass.async_add_executor_job(_flush, tmp, pending)
pending, buffered = [], 0
if error:
break
if pending:
await hass.async_add_executor_job(_flush, tmp, pending)
except Exception as err: # noqa: BLE001
_LOGGER.warning("House Plan upload: multipart read error: %s", err)
error = ({"error": "bad_request"}, 400)
if bad_ext: if error:
if tmp_path is not None: return web.json_response(error[0], status=error[1])
await hass.async_add_executor_job(_discard, tmp_path) if not temps or not filename:
return web.json_response({"error": "no_file"}, status=400)
tmp_path = temps[0]
target_dir = files_root / marker_id
safe_name = filename
def _promote() -> str:
"""Claim a free name, then move the finished upload onto it.
Never overwrite an existing attachment: its bytes may be
referenced by the stored configuration, and this upload is not
part of that transaction — a cancelled dialog or a rejected save
would leave the old url serving the new content (HP-1454-02).
The name is reserved atomically, so two uploads racing on the
same filename cannot agree on it (HP-1460-01).
"""
name = reserve_filename(target_dir, safe_name)
try:
os.replace(tmp_path, target_dir / name)
except OSError:
(target_dir / name).unlink(missing_ok=True)
raise
return name
try:
name = await hass.async_add_executor_job(_promote)
except OSError as err:
_LOGGER.warning("House Plan upload: could not store the file: %s", err)
return web.json_response({"error": "io_error"}, status=500)
temps.remove(tmp_path) # it is the attachment now, not a temporary
return web.json_response( return web.json_response(
{"error": "bad_ext", "allowed": sorted(FILE_EXTENSIONS)}, status=400 {"ok": True, "url": f"{CONTENT_URL}/files/{marker_id}/{name}", "name": filename}
) )
if too_large: finally:
await hass.async_add_executor_job(_discard, tmp_path) # BaseException too: cancelling the request task raises
return web.json_response( # asyncio.CancelledError, which an `except Exception` never saw —
{"error": "too_large", "max_mb": MAX_FILE_BYTES // 1024 // 1024}, status=413 # an aborted large upload leaked its temporary file every time
) if temps:
if tmp_path is None or not filename: await hass.async_add_executor_job(_cleanup, list(temps))
return web.json_response({"error": "no_file"}, status=400)
safe_name = sanitize_filename(filename)
target_dir = files_root / marker_id
def _promote() -> str:
"""Move the finished upload in under a name that is free.
Never overwrite: the previous bytes may be referenced by the stored
configuration, and this upload is not part of that transaction — a
cancelled dialog or a rejected save would leave the old url pointing
at the new content (HP-1454-02). An unreferenced upload is collected
later by config/set, once it is old enough.
"""
target_dir.mkdir(parents=True, exist_ok=True)
name = unique_filename(target_dir, safe_name)
os.replace(tmp_path, target_dir / name)
return name
name = await hass.async_add_executor_job(_promote)
return web.json_response(
{"ok": True, "url": f"{CONTENT_URL}/files/{marker_id}/{name}", "name": filename}
)
+1 -1
View File
@@ -16,5 +16,5 @@
"issue_tracker": "https://github.com/Matysh/houseplan-card/issues", "issue_tracker": "https://github.com/Matysh/houseplan-card/issues",
"requirements": [], "requirements": [],
"single_config_entry": true, "single_config_entry": true,
"version": "1.46.0" "version": "1.46.2"
} }
+76 -21
View File
@@ -9,40 +9,66 @@ and tested.
from __future__ import annotations from __future__ import annotations
import logging import logging
import os
import time import time
from pathlib import Path from pathlib import Path
from typing import Any from typing import Any
from .const import PLAN_ORPHAN_TTL_S from .const import PLAN_ORPHAN_TTL_S
from .validation import PLAN_EXTENSIONS, sanitize_filename from .validation import MAX_FILENAME, PLAN_EXTENSIONS, sanitize_filename
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
# Streaming uploads land here first. The prefix is a dot so the name can never
# collide with an attachment (sanitize_filename strips leading dots) and is easy
# to sweep.
TMP_PREFIX = ".upload-"
def unique_filename(directory: Path, name: str) -> str:
"""A name inside `directory` that is not taken, deriving from `name`.
Uploads never overwrite. The bytes already under a name may be referenced by def reserve_filename(directory: Path, name: str) -> str:
the stored configuration, and an upload is not part of that transaction: a """Atomically claim a free name inside `directory` and return it.
cancelled dialog or a rejected save would otherwise leave a live url serving
someone else's file (HP-1454-02). Creates the file, empty, with `O_CREAT | O_EXCL`, so the name is *taken* the
moment it is chosen. The previous version asked `exists()` and returned a
string; two uploads racing between the check and the write agreed on the
same name and one silently overwrote the other, both reporting success
(HP-1460-01). The caller writes the real bytes over the placeholder — it
owns the name by then — and must remove it if it never gets that far.
The result is guaranteed to satisfy `sanitize_filename(result) == result`:
the content view sanitises the name in the request too, so a name it would
shorten or rewrite is a file that is written and then never served.
""" """
safe = sanitize_filename(name) directory.mkdir(parents=True, exist_ok=True)
if not (directory / safe).exists(): # Split the extension off the RAW name: sanitize_filename() truncates to
return safe # MAX_FILENAME, so sanitising first would cut ".pdf" off a long name and the
stem, dot, suffix = safe.rpartition(".") # attachment would be stored — and served — without its type.
base = name.rsplit("/", 1)[-1].rsplit("\\", 1)[-1]
stem, dot, suffix = base.rpartition(".")
if not dot: if not dot:
stem, suffix = safe, "" stem, suffix = base, ""
i = 2 stem = sanitize_filename(stem)
ext = f".{sanitize_filename(suffix)[:16]}" if suffix else ""
i = 1
while True: while True:
# "-2", not " (2)": the content view sanitizes the name in the REQUEST tag = "" if i == 1 else f"-{i}"
# too, and a space or a bracket there turns into "_", so a file called # budget the stem so the WHOLE name fits, including the collision tag —
# "manual (2).pdf" was written and then never served. Only characters # appending "-2" to an already maximal name produced a url the view
# that survive sanitize_filename may be used to build a name. # truncated back to something else, i.e. a permanent 404
candidate = f"{stem}-{i}{'.' + suffix if suffix else ''}" room = MAX_FILENAME - len(ext) - len(tag)
if not (directory / candidate).exists(): candidate = (stem[:room] if room > 0 else "f") + tag + ext
return candidate candidate = sanitize_filename(candidate)
i += 1 if candidate.startswith("."): # a name that is only an extension
candidate = "file" + candidate
try:
fd = os.open(directory / candidate, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o644)
except FileExistsError:
i += 1
if i > 10000: # pathological directory; do not spin forever
raise
continue
os.close(fd)
return candidate
def attachment_refs(cfg: dict[str, Any] | None) -> set[str]: def attachment_refs(cfg: dict[str, Any] | None) -> set[str]:
@@ -59,6 +85,34 @@ def attachment_refs(cfg: dict[str, Any] | None) -> set[str]:
return out return out
def sweep_upload_temps(files_dir: Path, now: float | None = None) -> int:
"""Remove abandoned streaming temporaries (HP-1460-02).
The request itself deletes its own, but a hard kill — a restart mid-upload,
an OOM — leaves one behind, and the attachment collector only walks marker
folders, so it would never be seen. Age-gated for the same reason as the
rest: a fresh one belongs to a request still in flight.
"""
cutoff = (time.time() if now is None else now) - PLAN_ORPHAN_TTL_S
removed = 0
try:
items = [p for p in files_dir.iterdir() if p.is_file()] if files_dir.is_dir() else []
except OSError as err:
_LOGGER.warning("House Plan: could not list %s: %s", files_dir, err)
return 0
for item in items:
if not item.name.startswith(TMP_PREFIX):
continue
try:
if item.stat().st_mtime >= cutoff:
continue
item.unlink()
removed += 1
except OSError:
continue
return removed
def collect_attachments( def collect_attachments(
files_dir: Path, files_dir: Path,
old_cfg: dict[str, Any] | None, old_cfg: dict[str, Any] | None,
@@ -82,6 +136,7 @@ def collect_attachments(
except OSError as err: except OSError as err:
_LOGGER.warning("House Plan: could not list %s: %s", files_dir, err) _LOGGER.warning("House Plan: could not list %s: %s", files_dir, err)
return 0 return 0
removed += sweep_upload_temps(files_dir, now)
for folder in folders: for folder in folders:
try: try:
items = sorted(p for p in folder.iterdir() if p.is_file()) items = sorted(p for p in folder.iterdir() if p.is_file())
+4 -1
View File
@@ -16,6 +16,9 @@ MAX_FILE_BYTES = 50 * 1024 * 1024
SPACE_ID_RE = re.compile(r"^[a-z0-9_-]{1,64}$") SPACE_ID_RE = re.compile(r"^[a-z0-9_-]{1,64}$")
_SAFE_NAME_RE = re.compile(r"[^A-Za-z0-9._-]+") _SAFE_NAME_RE = re.compile(r"[^A-Za-z0-9._-]+")
# The name length the content view will accept back in a request. Anything a
# generated name must fit inside, collision tag included (HP-1460-01).
MAX_FILENAME = 120
# ---------- sanitizers ---------- # ---------- sanitizers ----------
@@ -33,7 +36,7 @@ def sanitize_marker_id(value: str) -> str:
def sanitize_filename(value: str) -> str: def sanitize_filename(value: str) -> str:
"""Drop the path and leading dots, keep a safe file name.""" """Drop the path and leading dots, keep a safe file name."""
raw = value.rsplit("/", 1)[-1].rsplit("\\", 1)[-1] raw = value.rsplit("/", 1)[-1].rsplit("\\", 1)[-1]
return _SAFE_NAME_RE.sub("_", raw).lstrip(".")[:120] or "file" return _SAFE_NAME_RE.sub("_", raw).lstrip(".")[:MAX_FILENAME] or "file"
def file_ext(filename: str) -> str: def file_ext(filename: str) -> str:
+12 -6
View File
@@ -20,7 +20,7 @@ from .const import (
CONTENT_URL, FILES_DIR, MAX_SIGN_PATHS, PLANS_DIR, PLANS_URL, CONTENT_URL, FILES_DIR, MAX_SIGN_PATHS, PLANS_DIR, PLANS_URL,
) )
from .auth import may_write from .auth import may_write
from .plans import collect_attachments, collect_plans, unique_filename from .plans import collect_attachments, collect_plans, reserve_filename
from .store import HouseplanData, get_data, get_entry from .store import HouseplanData, get_data, get_entry
from .validation import ( from .validation import (
CONFIG_SCHEMA, LAYOUT_SCHEMA, MAX_CONFIG_BYTES, MAX_PLAN_BYTES, CONFIG_SCHEMA, LAYOUT_SCHEMA, MAX_CONFIG_BYTES, MAX_PLAN_BYTES,
@@ -194,11 +194,17 @@ async def ws_files_migrate(hass: HomeAssistant, connection, msg: dict[str, Any])
if not f.is_file(): if not f.is_file():
continue continue
# a different file may already own this name — do NOT silently point # a different file may already own this name — do NOT silently point
# the url at it; the shared helper picks a free one, using only # the url at it. The shared helper CLAIMS a free one atomically, so
# characters the content view will accept back in a request # a concurrent migrate or upload cannot pick the same one, and the
target = dst / unique_filename(dst, f.name) # name it returns is one the content view accepts back in a request.
shutil.copy2(str(f), str(target)) name = reserve_filename(dst, f.name)
mapping[f.name] = target.name target = dst / name
try:
shutil.copy2(str(f), str(target))
except OSError:
target.unlink(missing_ok=True) # never leave an empty placeholder
raise
mapping[f.name] = name
return mapping return mapping
try: try:
+103
View File
@@ -0,0 +1,103 @@
// HP-1460-03: позиции — отдельное состояние. В v1.46.0 событие layout_updated
// научилась слушать статическая карточка, а полная — нет, поэтому две полные
// карточки рядом расходились до перезагрузки. Проверяем и обратное: приход
// чужой ревизии не должен затирать перетаскивание, которое ещё не улетело.
import { launch, checkAll, finish } from './serve.mjs';
const { page, browser } = await launch();
const res = await page.evaluate(async () => {
const out = {};
const c = window.__card;
const base = c.hass.callWS;
// общий «сервер»: layout с ревизией и подписчики на событие
let rev = 5;
let layout = { dev_a: { x: 10, y: 10 }, dev_b: { x: 20, y: 20 } };
const subs = [];
let gets = 0;
const hass = { ...c.hass,
callWS: async (m) => {
if (m.type === 'houseplan/layout/get') { gets++; return { layout: JSON.parse(JSON.stringify(layout)), rev }; }
if (m.type === 'houseplan/layout/update') {
layout = { ...layout, [m.device_id]: m.pos }; rev += 1;
subs.forEach((f) => f({ data: { rev } }));
return { ok: true, rev };
}
return base(m);
},
connection: { subscribeEvents: async (cb, ev) => {
if (ev === 'houseplan_layout_updated') { subs.push(cb); return () => {}; }
return () => {};
} },
};
c.hass = hass;
c._serverStorage = true;
c._layout = JSON.parse(JSON.stringify(layout));
c._layoutRev = rev;
c._unsubLayout = await hass.connection.subscribeEvents(
(e) => c._onLayoutEvent(Number(e?.data?.rev ?? -1)),
'houseplan_layout_updated',
);
out.subscribed = subs.length === 1;
// 1) чужая карточка подвинула иконку — наша обязана подхватить без перезагрузки
layout = { ...layout, dev_a: { x: 77, y: 88 } }; rev += 1;
subs.forEach((f) => f({ data: { rev } }));
await new Promise((r) => setTimeout(r, 350));
out.adoptedRemoteMove = JSON.stringify(c._layout.dev_a) === JSON.stringify({ x: 77, y: 88 });
out.revFollowed = c._layoutRev === rev;
// 2) собственная запись не вызывает лишнего перечитывания
const before = gets;
c._layout = { ...c._layout, dev_b: { x: 31, y: 32 } };
c._dirtyPos.add('dev_b');
c._persistLayout();
c._persistLayout.flush();
await new Promise((r) => setTimeout(r, 350));
out.ownWriteNoReload = gets === before;
out.ownWriteKept = JSON.stringify(c._layout.dev_b) === JSON.stringify({ x: 31, y: 32 });
// 3) НАСТОЯЩЕЕ перетаскивание: debounce запланирован, запись задержана, а
// layout/get отвечает мгновенно. Именно этот порядок и терял позицию:
// flush() внутри перечитывания опустошал _dirtyPos ДО снятия снимка.
let releaseUpdate;
const updateGate = new Promise((r) => { releaseUpdate = r; });
let delayUpdate = true;
const plain = hass.callWS;
c.hass = { ...hass, callWS: async (m) => {
if (m.type === 'houseplan/layout/update' && delayUpdate) {
delayUpdate = false;
await updateGate;
}
return plain(m);
} };
c._layout = { ...c._layout, dev_a: { x: 5, y: 6 } };
c._dirtyPos.add('dev_a');
c._persistLayout(); // debounce запланирован, не сброшен вручную
layout = { ...layout, dev_b: { x: 99, y: 99 } }; rev += 1;
subs.forEach((f) => f({ data: { rev } }));
await new Promise((r) => setTimeout(r, 400));
out.dragKeptWhileWriteInFlight = JSON.stringify(c._layout.dev_a) === JSON.stringify({ x: 5, y: 6 });
out.remoteChangeApplied = JSON.stringify(c._layout.dev_b) === JSON.stringify({ x: 99, y: 99 });
releaseUpdate();
await new Promise((r) => setTimeout(r, 350));
out.localDragSurvived = JSON.stringify(c._layout.dev_a) === JSON.stringify({ x: 5, y: 6 });
out.serverAgrees = JSON.stringify(layout.dev_a) === JSON.stringify({ x: 5, y: 6 });
out.sentPosDrained = c._sentPos.size === 0;
return out;
});
// зафиксировано прогоном на v1.46.1 и сверено с кодом
checkAll(res, {
subscribed: true,
adoptedRemoteMove: true,
revFollowed: true,
ownWriteNoReload: true,
ownWriteKept: true,
dragKeptWhileWriteInFlight: true,
remoteChangeApplied: true,
localDragSurvived: true,
serverAgrees: true,
sentPosDrained: true,
});
await finish(browser);
File diff suppressed because one or more lines are too long
+7 -7
View File
File diff suppressed because one or more lines are too long
+15 -3
View File
@@ -190,9 +190,21 @@ and only with SVG: a CSP on a PDF response can break the browser's built-in
viewer, and a raster image has no execution model to disable. viewer, and a raster image has no execution model to disable.
**Attachments follow the same commit-scoped lifecycle as plans** (HP-1454-02). **Attachments follow the same commit-scoped lifecycle as plans** (HP-1454-02).
An upload takes a free name (`unique_filename`) and never overwrites, because An upload takes a free name and never overwrites, because the bytes under an
the bytes under an existing name may be referenced by the stored configuration existing name may be referenced by the stored configuration and an upload is
and an upload is not part of that transaction. A new icon has no id yet, so its not part of that transaction. `reserve_filename` *claims* the name as it picks
it (`O_CREAT | O_EXCL`) — asking `exists()` and returning a string let two
uploads agree on one name and quietly overwrite each other. It also budgets the
length so the result survives the sanitiser the content view applies to the
request, since a name the view rewrites is a file written and never served.
Streaming temporaries live in the files root under `.upload-`, are removed on
every exit path of the request (including cancellation, which is a
BaseException and slips past `except Exception`), and are swept at startup and
daily. That scheduled pass also runs the two collectors with the stored
configuration as *both* sides — nothing superseded, so every referenced file is
kept and only aged unreferenced ones go. Without it, collection would only ever
happen when somebody saves, and a file uploaded into a dialog that was then
cancelled would wait for a write that may never come. A new icon has no id yet, so its
files go to a per-dialog staging folder and move to the real id once the config files go to a per-dialog staging folder and move to the real id once the config
write is accepted — the same copy → save → cleanup order as a rebind. write is accepted — the same copy → save → cleanup order as a rebind.
`config/set` collects what its commit superseded, and anything unreferenced and `config/set` collects what its commit superseded, and anything unreferenced and
+51
View File
@@ -1,5 +1,56 @@
# Changelog # Changelog
## v1.46.2 — 2026-07-28 (re-check of v1.46.1: HP-1461-01, -02)
- **A file nobody ended up using is now cleaned up even if nothing is ever
saved again (HP-1461-01).** Collection is tied to a configuration write,
which is right for what a write supersedes but leaves a gap: cancel a dialog
after the file has already uploaded, lose the connection just after, or call
the upload API directly, and nothing references the file and no future write
notices it. The daily sweep added in v1.46.1 only removed half-finished
transfers, so the promise that a cancelled attachment disappears after an hour
did not hold on an instance nobody edits. The sweep now compares against the
stored configuration — under the same lock a write uses — and collects aged
unreferenced attachments and plans as well.
- **A drag is no longer undone by someone else's move (HP-1461-02).** When the
full card learned to follow position changes in v1.46.1, it protected the
positions you had moved but not yet sent — except it read that list *after*
flushing the pending write, and flushing empties it first. In a real drag,
where a write is already scheduled, the list was therefore empty and the
server's older position was painted over your move. The card now takes the
snapshot before flushing and also holds on to positions that are sent but not
yet acknowledged: until the server confirms a position, the card that moved it
is the authority on it.
- Two tests grew up to their docstrings: the upload test now actually cancels
the request task instead of only exercising error paths, and the position-sync
smoke schedules a real debounced write and delays it, which is the ordering
that lost the drag.
## v1.46.1 — 2026-07-28 (re-check of v1.46.0: HP-1460-01 … -03)
- **Two uploads of the same file name can no longer collide (HP-1460-01).**
v1.46.0 stopped overwriting attachments, but choosing a free name and taking
it were two steps: two uploads racing between them agreed on the same name,
both reported success, and one set of bytes replaced the other. The name is
now claimed atomically as it is chosen — twenty simultaneous uploads of
`manual.pdf` produce twenty files. The same helper is used when rebinding
moves files, which had the same gap.
Also fixed there: a name at the length limit lost its extension, and the
collision suffix pushed it past the limit, so the attachment was stored under
a name the server would not serve back — a permanent 404 on a file the UI
reported as attached.
- **An interrupted upload no longer leaves a temporary file forever
(HP-1460-02).** Cleanup ran in an `except Exception`, which a cancelled
request walks straight past, and the collector only ever looks inside marker
folders — so an aborted transfer left a `.upload-*` in place with nothing able
to remove it. Every exit path now cleans up, a request carrying two files is
refused outright, and abandoned temporaries are swept at startup and daily.
Uploads also write in 1 MB batches instead of one disk task per 64 KB.
- **Two full cards side by side keep the same positions (HP-1460-03).** v1.46.0
taught the static card to follow position changes and left the full one
behind, so dragging an icon in one window did not move it in another until a
reload. It follows now, without disturbing a drag of its own: a revision
arriving mid-drag is merged rather than applied over the top, and a card does
not re-read what it just wrote itself.
## v1.46.0 — 2026-07-28 (full external audit of v1.45.4: HP-1454-01 … -10) ## v1.46.0 — 2026-07-28 (full external audit of v1.45.4: HP-1454-01 … -10)
**Security** **Security**
+52
View File
@@ -6,6 +6,58 @@
> **Правило проекта:** оба файла пополняются в одном коммите с самим > **Правило проекта:** оба файла пополняются в одном коммите с самим
> изменением — как и остальная документация (см. docs/STATUS.md). > изменением — как и остальная документация (см. docs/STATUS.md).
## v1.46.2 — 2026-07-28 (перепроверка v1.46.1: HP-1461-01, -02)
- **Файл, который в итоге никому не понадобился, убирается, даже если больше
ничего не сохраняют (HP-1461-01).** Сборка привязана к записи конфигурации —
это верно для того, что запись вытесняет, но оставляет зазор: отмените диалог
после того, как файл уже загрузился, потеряйте связь сразу после, или
вызовите API загрузки напрямую — и на файл никто не ссылается, а будущей
записи, которая бы это заметила, нет. Добавленное в v1.46.1 ежедневное
подметание убирало только незавершённые передачи, поэтому обещание «отменённое
вложение исчезнет через час» не выполнялось там, где никто ничего не правит.
Теперь подметание сверяется с сохранённой конфигурацией — под той же
блокировкой, что и запись, — и собирает устаревшие непривязанные вложения и
планы тоже.
- **Перетаскивание больше не отменяется чужим перемещением (HP-1461-02).** Когда
в v1.46.1 полная карточка научилась следить за позициями, она защищала те,
что вы подвинули, но ещё не отправили, — вот только читала этот список *после*
сброса отложенной записи, а сброс его первым делом опустошает. При настоящем
перетаскивании, когда запись уже запланирована, список оказывался пустым, и
старая серверная позиция закрашивала ваше движение. Теперь снимок снимается до
сброса, и вдобавок удерживаются позиции, отправленные, но ещё не
подтверждённые: пока сервер не подтвердил позицию, авторитет по ней — та
карточка, которая её подвинула.
- Два теста доросли до своих же описаний: тест загрузки теперь действительно
отменяет задачу запроса, а не только проходит по путям ошибок, а смоук
синхронизации позиций планирует настоящую отложенную запись и задерживает её —
именно этот порядок и терял перетаскивание.
## v1.46.1 — 2026-07-28 (перепроверка v1.46.0: HP-1460-01 … -03)
- **Две загрузки с одинаковым именем больше не сталкиваются (HP-1460-01).**
v1.46.0 перестала затирать вложения, но выбор свободного имени и его занятие
были двумя шагами: две загрузки, попавшие между ними, сходились на одном
имени, обе рапортовали успех, и одни байты заменяли другие. Теперь имя
занимается атомарно в момент выбора — двадцать одновременных загрузок
`manual.pdf` дают двадцать файлов. Тот же механизм используется при переносе
файлов на перепривязке, где был ровно такой же зазор.
Заодно там же: имя предельной длины теряло расширение, а суффикс коллизии
выталкивал его за предел, и вложение сохранялось под именем, которое сервер
обратно не отдаёт — вечный 404 на файл, который интерфейс считал
прикреплённым.
- **Прерванная загрузка больше не оставляет временный файл навсегда
(HP-1460-02).** Уборка стояла в `except Exception`, мимо которого отменённый
запрос проходит насквозь, а сборщик заглядывает только в папки маркеров —
поэтому оборванная передача оставляла `.upload-*`, и убрать его было некому.
Теперь убирает любой путь выхода, запрос с двумя файлами отклоняется сразу, а
брошенные временные подметаются при старте и раз в сутки. Запись идёт
порциями по мегабайту, а не отдельной задачей на каждые 64 КБ.
- **Две полные карточки рядом держат одинаковые позиции (HP-1460-03).** v1.46.0
научила следить за перемещениями статическую карточку и оставила позади
полную, поэтому перетаскивание иконки в одном окне не двигало её в другом до
перезагрузки. Теперь следит — и не мешает собственному перетаскиванию: чужая
ревизия, пришедшая в его разгар, сливается, а не накатывается сверху, и
карточка не перечитывает то, что записала сама.
## v1.46.0 — 2026-07-28 (полный внешний аудит v1.45.4: HP-1454-01 … -10) ## v1.46.0 — 2026-07-28 (полный внешний аудит v1.45.4: HP-1454-01 … -10)
**Безопасность** **Безопасность**
+2 -2
View File
@@ -15,12 +15,12 @@
| Item | State | | Item | State |
|---|---| |---|---|
| Version | **v1.46.0** everywhere (manifest, const.py, package.json, CARD_VERSION); deployed to the home instance | | Version | **v1.46.2** everywhere (manifest, const.py, package.json, CARD_VERSION); deployed to the home instance |
| Workflow | Since 2026-07-22: minor changes go to branch **`dev`** (build + smokes → deploy home → commit → push, NO release); releases are batched on the owner's command (merge dev→main, one tag, one release with a summary changelog, CI checked on dev beforehand) | | Workflow | Since 2026-07-22: minor changes go to branch **`dev`** (build + smokes → deploy home → commit → push, NO release); releases are batched on the owner's command (merge dev→main, one tag, one release with a summary changelog, CI checked on dev beforehand) |
| GitHub | https://github.com/Matysh/houseplan-card — **`main` carries every published release, the latest tag is the current version above**; `dev` is where work lands and is merged into `main` at release time (so `dev` is normally equal to or ahead of `main`, never behind). Push via SSH key `ha_jb` (remote git@github.com:…); API releases via the fine-grained PAT in `~/.git-credentials` (Contents R/W, issued 2026-07-23) | | GitHub | https://github.com/Matysh/houseplan-card — **`main` carries every published release, the latest tag is the current version above**; `dev` is where work lands and is merged into `main` at release time (so `dev` is normally equal to or ahead of `main`, never behind). Push via SSH key `ha_jb` (remote git@github.com:…); API releases via the fine-grained PAT in `~/.git-credentials` (Contents R/W, issued 2026-07-23) |
| CI | validate.yml (hacs + hassfest + frontend + backend) green; release.yml attaches the bundle on release publish | | CI | validate.yml (hacs + hassfest + frontend + backend) green; release.yml attaches the bundle on release publish |
| HACS | Custom repository works. **Inclusion PR: hacs/default#9004** — open, valid, labeled; ~864 older open PRs but merge rate ≈180/mo; realistic ETA 1–3 months (checked 2026-07-24) | | HACS | Custom repository works. **Inclusion PR: hacs/default#9004** — open, valid, labeled; ~864 older open PRs but merge rate ≈180/mo; realistic ETA 1–3 months (checked 2026-07-24) |
| Home instance | ha.jbstudio.pro (SSH port 323, key `ha_jb`), deployed **v1.46.0** via direct copy (HACS custom repo also installed) | | Home instance | ha.jbstudio.pro (SSH port 323, key `ha_jb`), deployed **v1.46.2** via direct copy (HACS custom repo also installed) |
| Localization | UI en/ru (src/i18n/*.json), everything user-visible localized incl. kiosk popover | | Localization | UI en/ru (src/i18n/*.json), everything user-visible localized incl. kiosk popover |
| Tests | Four layers: frontend unit (`npm test`, node:test over `test-build/`), pure backend (`pytest tests_backend`, runs anywhere), HA-harness backend (same folder, CI only — needs py3.13 + pytest-homeassistant-custom-component), and browser smokes (`demo/smoke_*.mjs`, headless chromium). **Counts are not written down here** — they went stale within two releases while the version line beside them was kept current, which reads as less coverage than exists (review R5-2). Run `npm run inventory` for the current numbers, or read them off the last CI run | | Tests | Four layers: frontend unit (`npm test`, node:test over `test-build/`), pure backend (`pytest tests_backend`, runs anywhere), HA-harness backend (same folder, CI only — needs py3.13 + pytest-homeassistant-custom-component), and browser smokes (`demo/smoke_*.mjs`, headless chromium). **Counts are not written down here** — they went stale within two releases while the version line beside them was kept current, which reads as less coverage than exists (review R5-2). Run `npm run inventory` for the current numbers, or read them off the last CI run |
| Community | **Telegram chat: https://t.me/ha_houseplan** (created 2026-07-27) — the primary user-facing support channel; GitHub issues stay for bugs/features. Link it from any new release notes and posts | | Community | **Telegram chat: https://t.me/ha_houseplan** (created 2026-07-27) — the primary user-facing support channel; GitHub issues stay for bugs/features. Link it from any new release notes and posts |
+21
View File
@@ -239,6 +239,27 @@ Run the *core flows* (marked ★ below) in each environment at least once per mi
on the plan after a reload. Same for each tap action and each fill mode on the plan after a reload. Same for each tap action and each fill mode
[auto: backend test_every_display_mode_the_editor_offers_is_accepted and [auto: backend test_every_display_mode_the_editor_offers_is_accepted and
neighbours, test_a_marker_showing_its_value_can_be_saved] neighbours, test_a_marker_showing_its_value_can_be_saved]
- [ ] Nothing accumulates on an idle instance (v1.46.2, HP-1461-01): attach a
file, cancel the dialog, and do not save anything else — an hour later (or
after a restart) the file is gone, while every file the configuration
still references is untouched
[auto: backend test_scheduled_sweep_collects_what_no_commit_will]
- [ ] A drag wins over a concurrent remote move (v1.46.2, HP-1461-02): drag an
icon and, while the save is still in flight, have another window move a
different icon — your icon stays where you put it and the other one
updates [auto: smoke_layout_sync]
- [ ] Concurrent uploads of one name (v1.46.1, HP-1460-01): attach the same
file from two browser tabs at once — two attachments, two sets of bytes,
neither lost. A file whose name is at the length limit still downloads
[auto: unit: test_reserve_filename_is_safe_under_concurrency and neighbours]
- [ ] No temporary files survive (v1.46.1, HP-1460-02): abort a large upload
mid-transfer, send two files in one request, make promotion fail — in each
case the files folder holds no `.upload-*`. An old one is swept at startup
[auto: backend test_upload_leaves_no_temporary_behind + unit: sweep_upload_temps]
- [ ] Two full cards agree on positions (v1.46.1, HP-1460-03): open the plan in
two windows, drag an icon in one — it moves in the other without a reload;
a drag in progress in the second window is not thrown away
[auto: smoke_layout_sync]
- [ ] Uploaded SVG is inert as a document (v1.46.0, HP-1454-01): open a plan's - [ ] Uploaded SVG is inert as a document (v1.46.0, HP-1454-01): open a plan's
signed url directly in a tab — a `<script>` inside it must not run and must signed url directly in a tab — a `<script>` inside it must not run and must
not reach the HA session's localStorage; the same plan still renders in the not reach the HA session's localStorage; the same plan still renders in the
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "houseplan-card", "name": "houseplan-card",
"version": "1.46.0", "version": "1.46.2",
"description": "Interactive house plan Lovelace card for Home Assistant", "description": "Interactive house plan Lovelace card for Home Assistant",
"license": "MIT", "license": "MIT",
"type": "module", "type": "module",
+95 -5
View File
@@ -35,7 +35,7 @@ import './space-card';
import { cardStyles } from './styles'; import { cardStyles } from './styles';
import { langOf, t, type I18nKey } from './i18n'; import { langOf, t, type I18nKey } from './i18n';
const CARD_VERSION = '1.46.0'; const CARD_VERSION = '1.46.2';
const LS_KEY = 'houseplan_card_layout_v1'; const LS_KEY = 'houseplan_card_layout_v1';
const LS_CFG = 'houseplan_card_cfg_v1'; // cache of the server config+layout for instant rendering const LS_CFG = 'houseplan_card_cfg_v1'; // cache of the server config+layout for instant rendering
const LS_ZOOM = 'houseplan_card_zoom_v1'; const LS_ZOOM = 'houseplan_card_zoom_v1';
@@ -122,6 +122,8 @@ class HouseplanCard extends LitElement {
private _serverCfg: ServerConfig | null = null; private _serverCfg: ServerConfig | null = null;
private _cfgRev = 0; private _cfgRev = 0;
private _unsubCfg: (() => void) | null = null; private _unsubCfg: (() => void) | null = null;
private _unsubLayout: (() => void) | null = null;
private _layoutRev = 0;
private _devices: DevItem[] = []; private _devices: DevItem[] = [];
private _regSignature = ''; private _regSignature = '';
private _defPos: Record<string, { x: number; y: number }> = {}; private _defPos: Record<string, { x: number; y: number }> = {};
@@ -402,6 +404,11 @@ class HouseplanCard extends LitElement {
this._unsubCfg(); this._unsubCfg();
this._unsubCfg = null; this._unsubCfg = null;
} }
if (this._unsubLayout) {
this._unsubLayout();
this._unsubLayout = null;
}
clearTimeout(this._layoutSyncTimer);
super.disconnectedCallback(); super.disconnectedCallback();
} }
@@ -723,6 +730,7 @@ class HouseplanCard extends LitElement {
this._cfgEpoch++; this._cfgEpoch++;
this._cfgRev = cfgResp?.rev || 0; this._cfgRev = cfgResp?.rev || 0;
this._layout = layResp?.layout || {}; this._layout = layResp?.layout || {};
this._layoutRev = layResp?.rev ?? 0;
// live sync: the config was changed in another window → re-read it // live sync: the config was changed in another window → re-read it
if (!this._unsubCfg) { if (!this._unsubCfg) {
this._unsubCfg = await this.hass.connection.subscribeEvents((ev: any) => { this._unsubCfg = await this.hass.connection.subscribeEvents((ev: any) => {
@@ -732,6 +740,15 @@ class HouseplanCard extends LitElement {
if ((ev?.data?.rev ?? -1) !== this._cfgRev) this._reloadConfigOnly(); if ((ev?.data?.rev ?? -1) !== this._cfgRev) this._reloadConfigOnly();
}, 'houseplan_config_updated'); }, 'houseplan_config_updated');
} }
if (!this._unsubLayout) {
// Positions are separate state. The static card learned to follow them
// in v1.46.0 and the full one did not, so two full cards side by side
// stayed out of sync until a reload (HP-1460-03).
this._unsubLayout = await this.hass.connection.subscribeEvents(
(ev: any) => this._onLayoutEvent(Number(ev?.data?.rev ?? -1)),
'houseplan_layout_updated',
);
}
const hs = this._hashSpace(); const hs = this._hashSpace();
const nav = this._savedNav(); const nav = this._savedNav();
if (!this._hashApplied && hs && this._model.find((s) => s.id === hs)) { if (!this._hashApplied && hs && this._model.find((s) => s.id === hs)) {
@@ -818,7 +835,73 @@ class HouseplanCard extends LitElement {
this._signer.resign(this.hass, referencedContentUrls(this._serverCfg)); this._signer.resign(this.hass, referencedContentUrls(this._serverCfg));
} }
private _layoutSyncTimer?: number;
/**
* A layout revision appeared. It may well be ours: the event travels over the
* same socket as the reply to our own write and can arrive first, so
* reacting immediately means re-reading what we just sent — and, worse,
* racing a drag that has not been flushed yet. Wait a beat; if our own reply
* lands in the meantime, `_layoutRev` catches up and there is nothing to do.
*/
private _onLayoutEvent(rev: number): void {
if (rev <= this._layoutRev) return;
clearTimeout(this._layoutSyncTimer);
this._layoutSyncTimer = window.setTimeout(() => {
if (rev <= this._layoutRev) return; // it was ours after all
this._reloadLayoutOnly();
}, 200);
}
/**
* Remember a revision this card produced, so its own `layout_updated` event
* is not mistaken for someone else's and does not trigger a pointless
* re-read of what we just wrote.
*/
private _noteLayoutRev(r: any): void {
const rev = r?.rev;
if (typeof rev === 'number' && rev > this._layoutRev) this._layoutRev = rev;
}
/**
* Adopt positions written elsewhere, without dropping our own (HP-1460-03).
*
* Only the layout is re-read — the config is untouched, so this cannot
* disturb an edit in progress. Positions this card has moved but not yet
* sent are flushed first and then kept on top of the server's answer: a fix
* for a stale UI must not turn into a lost drag.
*/
private async _reloadLayoutOnly(): Promise<void> {
if (!this._serverStorage || !this.hass?.callWS) return;
// Snapshot BEFORE flushing. `flush()` runs the debounced writer
// synchronously, and the first thing that does is empty `_dirtyPos` — so
// reading the dirty set afterwards found nothing to protect and the server's
// older position was painted over the drag the user had just made
// (HP-1461-02). Positions are captured by value for the same reason.
const mine = new Map<string, any>();
for (const id of this._dirtyPos) if (this._layout[id]) mine.set(id, this._layout[id]);
if (this._persistLayout.pending()) this._persistLayout.flush();
// …and again after the flush: what was dirty is now in flight, and a write
// sent before this reload was even scheduled is in there too. Until the
// server acknowledges a position, this card is the authority on it.
for (const [id, pos] of this._sentPos) mine.set(id, pos);
try {
const resp = await this.hass.callWS({ type: 'houseplan/layout/get' });
const remote = resp?.layout || {};
const merged: Record<string, any> = { ...remote };
for (const [id, pos] of mine) merged[id] = pos;
this._layout = merged;
this._layoutRev = resp?.rev ?? this._layoutRev;
this._cacheSnapshot();
this.requestUpdate();
} catch {
/* a failed refresh just leaves the positions we already had */
}
}
private _dirtyPos = new Set<string>(); private _dirtyPos = new Set<string>();
/** Positions sent to the server and not acknowledged yet (HP-1461-02). */
private _sentPos = new Map<string, { s?: string; x: number; y: number }>();
private _persistLayout = debounce(() => { private _persistLayout = debounce(() => {
if (this._serverStorage) { if (this._serverStorage) {
@@ -828,9 +911,14 @@ class HouseplanCard extends LitElement {
for (const id of ids) { for (const id of ids) {
const pos = this._layout[id]; const pos = this._layout[id];
if (!pos) continue; if (!pos) continue;
// in flight until the server answers: a layout reload triggered in the
// meantime must keep this position, not the one the server still has
this._sentPos.set(id, pos);
this.hass this.hass
.callWS({ type: 'houseplan/layout/update', device_id: id, pos }) .callWS({ type: 'houseplan/layout/update', device_id: id, pos })
.catch((e: any) => this._showToast(this._t('toast.pos_save_failed', { err: this._errText(e) }))); .then((r: any) => this._noteLayoutRev(r))
.catch((e: any) => this._showToast(this._t('toast.pos_save_failed', { err: this._errText(e) })))
.finally(() => { if (this._sentPos.get(id) === pos) this._sentPos.delete(id); });
} }
this._cacheSnapshot(); this._cacheSnapshot();
} else { } else {
@@ -2831,11 +2919,12 @@ class HouseplanCard extends LitElement {
this._layout = { ...this._layout, [id]: newPos }; this._layout = { ...this._layout, [id]: newPos };
} }
await this._saveConfigNow(); await this._saveConfigNow();
if (newPos) await this.hass.callWS({ type: 'houseplan/layout/update', device_id: id, pos: newPos }); if (newPos) this._noteLayoutRev(await this.hass.callWS({ type: 'houseplan/layout/update', device_id: id, pos: newPos }));
if (oldId && oldId !== id) { if (oldId && oldId !== id) {
// rebinding changed the icon id — clean up the old position // rebinding changed the icon id — clean up the old position
delete this._layout[oldId]; delete this._layout[oldId];
await this.hass.callWS({ type: 'houseplan/layout/delete', device_id: oldId }).catch(() => undefined); await this.hass.callWS({ type: 'houseplan/layout/delete', device_id: oldId })
.then((r: any) => this._noteLayoutRev(r)).catch(() => undefined);
} }
// the config is committed — now it is safe to drop the old folder // the config is committed — now it is safe to drop the old folder
if (cleanupOldFiles && fileSrc) { if (cleanupOldFiles && fileSrc) {
@@ -2885,7 +2974,8 @@ class HouseplanCard extends LitElement {
if (d && d.bindingKind === 'virtual' && this._layout[d.id]) { if (d && d.bindingKind === 'virtual' && this._layout[d.id]) {
// the virtual one is deleted for good → its position is no longer needed // the virtual one is deleted for good → its position is no longer needed
delete this._layout[d.id]; delete this._layout[d.id];
await this.hass.callWS({ type: 'houseplan/layout/delete', device_id: d.id }).catch(() => undefined); await this.hass.callWS({ type: 'houseplan/layout/delete', device_id: d.id })
.then((r: any) => this._noteLayoutRev(r)).catch(() => undefined);
} }
this._markerDialog = null; this._markerDialog = null;
this._regSignature = ''; this._regSignature = '';
+217
View File
@@ -696,3 +696,220 @@ async def test_upload_never_overwrites_an_existing_attachment(
assert await got.read() == b"ONE", "the first file is untouched" assert await got.read() == b"ONE", "the first file is untouched"
got2 = await http.get(second) got2 = await http.get(second)
assert await got2.read() == b"TWO" assert await got2.read() == b"TWO"
async def test_upload_leaves_no_temporary_behind(
hass: HomeAssistant, hass_ws_client: WebSocketGenerator, hass_client, monkeypatch
) -> None:
"""HP-1460-02: every exit path must take its temporary file with it.
The streaming rewrite kept one `tmp_path` and cleaned it in an
`except Exception`, which cancellation (a BaseException) walks straight
past — and the attachment collector only ever looks inside marker folders,
so a stranded `.upload-*` was never seen again.
"""
import os
from custom_components.houseplan import http_api
from custom_components.houseplan.const import FILES_DIR
from custom_components.houseplan.plans import TMP_PREFIX
await _setup(hass)
http = await hass_client()
root = hass.config.path(FILES_DIR)
def temps() -> list[str]:
return [n for n in os.listdir(root) if n.startswith(TMP_PREFIX)]
import aiohttp
def form(*files, marker="m8"):
w = aiohttp.FormData()
w.add_field("marker_id", marker)
for name, data in files:
w.add_field("file", data, filename=name)
return w
# two file parts: refused, and nothing left over
resp = await http.post("/api/houseplan/upload", data=form(("a.pdf", b"A"), ("b.pdf", b"B")))
assert resp.status == 400 and (await resp.json())["error"] == "one_file_only"
assert temps() == []
# a rejected extension after the temporary already exists
resp = await http.post("/api/houseplan/upload", data=form(("evil.exe", b"X")))
assert resp.status == 400
assert temps() == []
# promotion itself blows up
real = http_api.reserve_filename
def _boom(*_a, **_k):
raise OSError("disk on fire")
monkeypatch.setattr(http_api, "reserve_filename", _boom)
resp = await http.post("/api/houseplan/upload", data=form(("c.pdf", b"C")))
assert resp.status == 500
assert temps() == [], "a failed promotion must not strand the upload"
monkeypatch.setattr(http_api, "reserve_filename", real)
# and the happy path leaves nothing either
resp = await http.post("/api/houseplan/upload", data=form(("d.pdf", b"D")))
assert resp.status == 200
assert temps() == []
async def test_repair_issue_goes_when_its_space_does(
hass: HomeAssistant, hass_ws_client: WebSocketGenerator
) -> None:
"""HP-1454-09: the cleanup used to walk only spaces that still exist.
So deleting or renaming a space with a missing plan left its warning in
Repairs with nothing able to clear it.
"""
from homeassistant.helpers import issue_registry as ir
from custom_components.houseplan.const import DOMAIN as HP_DOMAIN
await _setup(hass)
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"]
await hass.async_block_till_done()
assert registry.async_get_issue(HP_DOMAIN, "broken_plan_r7") is not None
# the space is deleted entirely — the warning must not outlive it
await _save(client, await _cfg([{"id": "other", "plan_url": None}]), rev)
await hass.async_block_till_done()
assert registry.async_get_issue(HP_DOMAIN, "broken_plan_r7") is None
async def test_cancelling_an_upload_takes_its_temporary_with_it(
hass: HomeAssistant, hass_ws_client: WebSocketGenerator
) -> None:
"""HP-1460-02, properly this time: cancellation, not an ordinary error.
`asyncio.CancelledError` is a BaseException, so the old `except Exception`
never saw it and an aborted transfer stranded its `.upload-*`. The previous
test claimed to cover this and did not — it only exercised error paths.
"""
import asyncio
import os
from custom_components.houseplan.const import FILES_DIR
from custom_components.houseplan.http_api import HouseplanUploadView
from custom_components.houseplan.plans import TMP_PREFIX
entry = await _setup(hass)
root = hass.config.path(FILES_DIR)
os.makedirs(root, exist_ok=True)
started = asyncio.Event()
class _Part:
name = "file"
filename = "big.pdf"
async def read_chunk(self, _size):
started.set()
await asyncio.sleep(3600) # the client stopped sending; we wait
class _Reader:
def __aiter__(self):
return self
async def __anext__(self):
if getattr(self, "_done", False):
raise StopAsyncIteration
self._done = True
return _Part()
from custom_components.houseplan import http_api as hp_http
class _User:
is_admin = True
class _Request:
app = {hp_http.KEY_HASS: hass}
def get(self, _key, default=None):
return _User()
async def multipart(self):
return _Reader()
task = hass.async_create_task(HouseplanUploadView().post(_Request()))
await started.wait()
await asyncio.sleep(0)
assert [n for n in os.listdir(root) if n.startswith(TMP_PREFIX)], "temp exists mid-upload"
task.cancel()
with pytest.raises(asyncio.CancelledError):
await task
await hass.async_block_till_done()
assert [n for n in os.listdir(root) if n.startswith(TMP_PREFIX)] == [], (
"a cancelled upload must not leave its temporary behind"
)
assert entry
async def test_scheduled_sweep_collects_what_no_commit_will(
hass: HomeAssistant, hass_ws_client: WebSocketGenerator
) -> None:
"""HP-1461-01: a commit collects what it superseded — but only when it runs.
Cancel a dialog after the file uploaded, or lose the connection right after,
and nothing references the file and no future write notices it. The daily
sweep used to remove only streaming temporaries, so the promise that a
cancelled attachment goes after an hour did not hold on an instance nobody
edits.
"""
import os
import time
from custom_components.houseplan.const import FILES_DIR, PLANS_DIR, PLAN_ORPHAN_TTL_S
await _setup(hass)
client = await hass_ws_client(hass)
files = hass.config.path(FILES_DIR)
plans = hass.config.path(PLANS_DIR)
old = time.time() - PLAN_ORPHAN_TTL_S - 60
def _seed() -> None:
os.makedirs(os.path.join(files, "m5"), exist_ok=True)
os.makedirs(os.path.join(files, "up_cancelled"), exist_ok=True)
os.makedirs(plans, exist_ok=True)
for path in (
os.path.join(files, "m5", "kept.pdf"),
os.path.join(files, "up_cancelled", "manual.pdf"),
os.path.join(plans, "s5.tok.png"),
os.path.join(plans, "s5.orphan.png"),
):
with open(path, "wb") as fh:
fh.write(b"x")
os.utime(path, (old, old))
await hass.async_add_executor_job(_seed)
cfg = await _cfg([{"id": "s5", "plan_url": "/api/houseplan/content/plans/_/s5.tok.png"}])
cfg["markers"] = [
{"id": "m5", "binding": "virtual",
"pdfs": [{"name": "k", "url": "/api/houseplan/content/files/m5/kept.pdf"}]}
]
assert (await _save(client, cfg, 0))["success"]
# reload the entry: that is what runs the sweep at startup
entry = hass.config_entries.async_entries(DOMAIN)[0]
assert await hass.config_entries.async_reload(entry.entry_id)
await hass.async_block_till_done()
exists = lambda p: os.path.isfile(p) # noqa: E731
assert await hass.async_add_executor_job(exists, os.path.join(files, "m5", "kept.pdf"))
assert await hass.async_add_executor_job(exists, os.path.join(plans, "s5.tok.png"))
assert not await hass.async_add_executor_job(
exists, os.path.join(files, "up_cancelled", "manual.pdf")
), "an aged cancelled attachment is collected without any further config write"
assert not await hass.async_add_executor_job(exists, os.path.join(plans, "s5.orphan.png"))
assert not await hass.async_add_executor_job(os.path.isdir, os.path.join(files, "up_cancelled"))
+74 -18
View File
@@ -386,25 +386,81 @@ def test_every_room_fill_mode_the_editor_offers_is_accepted():
# ---------- attachments & inner limits (HP-1454-02, -05) ---------- # ---------- attachments & inner limits (HP-1454-02, -05) ----------
def test_unique_filename_never_returns_a_taken_name(tmp_path): def test_reserve_filename_claims_the_name_atomically(tmp_path):
unique_filename = plans.unique_filename """HP-1460-01: choosing a name and taking it must be one operation.
d = tmp_path / "m1"
d.mkdir()
assert unique_filename(d, "manual.pdf") == "manual.pdf"
(d / "manual.pdf").write_bytes(b"x")
assert unique_filename(d, "manual.pdf") == "manual-2.pdf"
(d / "manual-2.pdf").write_bytes(b"x")
assert unique_filename(d, "manual.pdf") == "manual-3.pdf"
# no extension, and a name that needs sanitising
(d / "readme").write_bytes(b"x")
assert unique_filename(d, "readme") == "readme-2"
assert unique_filename(d, "../../etc/passwd") == "passwd"
# A generated name has to survive the sanitiser the CONTENT VIEW applies to The old helper asked `exists()` and returned a string; two uploads racing
# the request, or the file is written and then never served: " (2)" became between the check and the write agreed on the same name and one overwrote
# "_2_" and every collision-renamed attachment 404'd. the other, both reporting success.
for candidate in ("manual-2.pdf", "manual-3.pdf", "readme-2"): """
assert v.sanitize_filename(candidate) == candidate reserve = plans.reserve_filename
d = tmp_path / "m1"
first = reserve(d, "manual.pdf")
assert first == "manual.pdf"
assert (d / first).is_file(), "the name is taken, not merely picked"
second = reserve(d, "manual.pdf")
assert second == "manual-2.pdf" and (d / second).is_file()
assert reserve(d, "manual.pdf") == "manual-3.pdf"
assert reserve(d, "readme") == "readme"
assert reserve(d, "readme") == "readme-2"
assert reserve(d, "../../etc/passwd") == "passwd"
def test_reserve_filename_result_survives_the_content_sanitizer(tmp_path):
"""A name the view would rewrite is a file written and then never served."""
reserve = plans.reserve_filename
d = tmp_path / "m2"
long_stem = "x" * 200 # far past the limit
names = [reserve(d, f"{long_stem}.pdf") for _ in range(12)]
assert len(set(names)) == 12, "each call takes its own name"
for n in names:
assert len(n) <= v.MAX_FILENAME
assert v.sanitize_filename(n) == n, n
assert n.endswith(".pdf")
# a name already exactly at the limit still leaves room for the tag
exact = "y" * (v.MAX_FILENAME - 4) + ".pdf"
assert len(exact) == v.MAX_FILENAME
a = reserve(d, exact)
b = reserve(d, exact)
assert a != b and len(b) <= v.MAX_FILENAME and v.sanitize_filename(b) == b
def test_reserve_filename_is_safe_under_concurrency(tmp_path):
"""Twenty threads, one filename: twenty distinct files, nothing overwritten."""
from concurrent.futures import ThreadPoolExecutor
reserve = plans.reserve_filename
d = tmp_path / "m3"
d.mkdir(parents=True)
with ThreadPoolExecutor(max_workers=20) as pool:
names = list(pool.map(lambda _: reserve(d, "manual.pdf"), range(20)))
assert len(set(names)) == 20
assert sorted(p.name for p in d.iterdir()) == sorted(names)
def test_sweep_upload_temps(tmp_path):
"""HP-1460-02: a crashed transfer leaves a temp no other collector sees."""
import os
import time
files = tmp_path / "files"
files.mkdir()
fresh = files / f"{plans.TMP_PREFIX}fresh"
old = files / f"{plans.TMP_PREFIX}old"
keep = files / "notes.txt"
for f in (fresh, old, keep):
f.write_bytes(b"x")
t = time.time() - const.PLAN_ORPHAN_TTL_S - 60
os.utime(old, (t, t))
assert plans.sweep_upload_temps(files) == 1
assert not old.exists(), "an aged temporary goes"
assert fresh.is_file(), "a fresh one may belong to a request in flight"
assert keep.is_file(), "nothing else is touched"
assert plans.sweep_upload_temps(tmp_path / "nope") == 0
def _acfg(*pairs): def _acfg(*pairs):