fix v1.44.2: external review CR-1..CR-3

CR-1: the lock invariant is restated precisely (never by an accidental
tap; the door card's labeled button is the ONE sanctioned surface),
unlocking now confirms, and smoke_lock_invariant exercises all five
actuation paths (icon tap, controls[], card entities, _cardToggle,
opening card).

CR-2: attachment migration is transactional — the server COPIES files,
the config is committed with its revision check, and only then the old
folder is removed via the new houseplan/files/cleanup. A rejected save
no longer leaves the stored urls pointing at an emptied folder.

CR-3: migrate returns an exact {source: written} mapping; only confirmed
copies are rewritten, destination name collisions get a unique name
instead of silently linking a pre-existing file, and a failed migration
raises a toast instead of being swallowed.

+1 unit test (119), +1 backend test, +1 smoke (51 total); docs
same-commit
This commit is contained in:
Matysh
2026-07-27 12:58:27 +03:00
parent 45c863138a
commit ae9168f6ec
18 changed files with 289 additions and 48 deletions
+1 -1
View File
@@ -13,7 +13,7 @@ FILES_URL = "/houseplan_files/files"
CONTENT_URL = "/api/houseplan/content" CONTENT_URL = "/api/houseplan/content"
FILES_DIR = "houseplan/files" FILES_DIR = "houseplan/files"
CONF_ADMIN_ONLY = "admin_only" CONF_ADMIN_ONLY = "admin_only"
VERSION = "1.44.1" VERSION = "1.44.2"
DEFAULT_CONFIG: dict = { DEFAULT_CONFIG: dict = {
"spaces": [], "spaces": [],
File diff suppressed because one or more lines are too long
+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.44.1" "version": "1.44.2"
} }
+73 -20
View File
@@ -38,6 +38,7 @@ def async_register(hass: HomeAssistant) -> None:
websocket_api.async_register_command(hass, ws_config_set) websocket_api.async_register_command(hass, ws_config_set)
websocket_api.async_register_command(hass, ws_plan_set) websocket_api.async_register_command(hass, ws_plan_set)
websocket_api.async_register_command(hass, ws_files_migrate) websocket_api.async_register_command(hass, ws_files_migrate)
websocket_api.async_register_command(hass, ws_files_cleanup)
def _runtime(hass: HomeAssistant, connection, msg_id: int) -> HouseplanData | None: def _runtime(hass: HomeAssistant, connection, msg_id: int) -> HouseplanData | None:
@@ -149,17 +150,24 @@ async def ws_layout_update(hass: HomeAssistant, connection, msg: dict[str, Any])
) )
@websocket_api.async_response @websocket_api.async_response
async def ws_files_migrate(hass: HomeAssistant, connection, msg: dict[str, Any]) -> None: async def ws_files_migrate(hass: HomeAssistant, connection, msg: dict[str, Any]) -> None:
"""Move a marker's uploaded files to its new id (rebinding changes the id). """COPY a marker's uploaded files to its new id and report the exact mapping.
Without this the PDF urls keep pointing at the OLD id's folder, which then Rebinding changes the marker id, so the files must follow (that is how the
looks orphaned and is one cleanup away from deletion — the exact way the owner lost a set of manuals, 2026-07-26). This used to MOVE them before the
owner lost the sauna heater manuals (field incident, 2026-07-26). revision-checked config save: when that save was rejected, the server kept
the old urls while the files had already left the old folder — a permanent
broken link (review CR-2, 2026-07-27).
Now it copies, never overwrites, and returns {src: dst} for every file so
the client can rewrite EXACTLY the urls that made it (review CR-3). The old
folder is removed later by houseplan/files/cleanup, once the config is
safely committed.
""" """
if not _check_write(hass, connection): if not _check_write(hass, connection):
connection.send_error(msg["id"], "unauthorized", "Only administrators may edit files") connection.send_error(msg["id"], "unauthorized", "Only administrators may edit files")
return return
from pathlib import Path
import shutil import shutil
from pathlib import Path
from .const import FILES_DIR from .const import FILES_DIR
from .validation import sanitize_marker_id from .validation import sanitize_marker_id
@@ -167,32 +175,77 @@ async def ws_files_migrate(hass: HomeAssistant, connection, msg: dict[str, Any])
src_id = sanitize_marker_id(msg["from_id"]) src_id = sanitize_marker_id(msg["from_id"])
dst_id = sanitize_marker_id(msg["to_id"]) dst_id = sanitize_marker_id(msg["to_id"])
if not src_id or not dst_id or src_id == dst_id: if not src_id or not dst_id or src_id == dst_id:
connection.send_result(msg["id"], {"ok": True, "moved": 0}) connection.send_result(msg["id"], {"ok": True, "mapping": {}, "copied": 0})
return return
base = Path(hass.config.path(FILES_DIR)) base = Path(hass.config.path(FILES_DIR))
src = base / src_id src = base / src_id
dst = base / dst_id dst = base / dst_id
def _move() -> int: def _copy() -> dict[str, str]:
if not src.is_dir(): if not src.is_dir():
return 0 return {}
dst.mkdir(parents=True, exist_ok=True) dst.mkdir(parents=True, exist_ok=True)
n = 0 mapping: dict[str, str] = {}
for f in src.iterdir(): for f in sorted(src.iterdir()):
if not f.is_file(): if not f.is_file():
continue continue
target = dst / f.name target = dst / f.name
if not target.exists(): if target.exists():
shutil.move(str(f), str(target)) # a different file already owns this name — do NOT silently
n += 1 # point the url at it; give the copy a unique name instead
try: stem, suffix = f.stem, f.suffix
src.rmdir() # only when empty i = 2
except OSError: while (dst / f"{stem} ({i}){suffix}").exists():
pass i += 1
return n target = dst / f"{stem} ({i}){suffix}"
shutil.copy2(str(f), str(target))
mapping[f.name] = target.name
return mapping
moved = await hass.async_add_executor_job(_move) try:
connection.send_result(msg["id"], {"ok": True, "moved": moved}) mapping = await hass.async_add_executor_job(_copy)
except OSError as err:
connection.send_error(msg["id"], "io_error", f"Could not copy marker files: {err}")
return
connection.send_result(msg["id"], {"ok": True, "mapping": mapping, "copied": len(mapping)})
@websocket_api.websocket_command(
{
vol.Required("type"): "houseplan/files/cleanup",
vol.Required("marker_id"): str,
}
)
@websocket_api.async_response
async def ws_files_cleanup(hass: HomeAssistant, connection, msg: dict[str, Any]) -> None:
"""Delete a marker's file folder — called only AFTER the config is committed."""
if not _check_write(hass, connection):
connection.send_error(msg["id"], "unauthorized", "Only administrators may edit files")
return
import shutil
from pathlib import Path
from .const import FILES_DIR
from .validation import sanitize_marker_id
mid = sanitize_marker_id(msg["marker_id"])
if not mid:
connection.send_result(msg["id"], {"ok": True, "removed": False})
return
base = Path(hass.config.path(FILES_DIR)).resolve()
target = (base / mid).resolve()
if not str(target).startswith(str(base)) or target == base:
connection.send_result(msg["id"], {"ok": True, "removed": False})
return
def _rm() -> bool:
if not target.is_dir():
return False
shutil.rmtree(target, ignore_errors=True)
return True
removed = await hass.async_add_executor_job(_rm)
connection.send_result(msg["id"], {"ok": True, "removed": removed})
@websocket_api.websocket_command( @websocket_api.websocket_command(
+1
View File
@@ -5,6 +5,7 @@ const res = await page.evaluate(async () => {
const c = window.__card; const c = window.__card;
const sr = () => c.shadowRoot || c.renderRoot; const sr = () => c.shadowRoot || c.renderRoot;
const calls = []; const calls = [];
window.confirm = () => true; // review CR-1: unlocking now confirms
c.hass = { ...c.hass, callService: (d, s, data) => calls.push([d, s, data.entity_id]) }; c.hass = { ...c.hass, callService: (d, s, data) => calls.push([d, s, data.entity_id]) };
await c.updateComplete; await c.updateComplete;
// добавить дверь с замком на f1 // добавить дверь с замком на f1
+48
View File
@@ -0,0 +1,48 @@
// review CR-1: exercise EVERY actuation path and prove locks/alarms are safe
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 calls = [];
c.hass = { ...c.hass, callService: (d, s, data) => { calls.push(`${d}.${s}:${data.entity_id}`); return Promise.resolve(); },
states: { ...c.hass.states,
'lock.front_door': { state: 'locked', attributes: { friendly_name: 'Front door' } },
'alarm_control_panel.home': { state: 'armed_away', attributes: {} } } };
await c.updateComplete;
c._setMode('view'); await c.updateComplete;
const lockCalls = () => calls.filter((x) => x.includes('lock.') || x.includes('alarm_control_panel.'));
// 1) тап по значку устройства с замком
const lockDev = c._devices.find((d) => d.entities?.some((e) => e.startsWith('lock.'))) || c._devices[0];
const fake = { ...lockDev, primary: 'lock.front_door', tapAction: 'toggle',
marker: { ...(lockDev.marker || {}), tap_action: 'toggle' } };
c._clickDevice(new MouseEvent('click'), fake);
out.iconTapSafe = lockCalls().length === 0;
// 2) controls[] с замком внутри
const withControls = { ...fake, tapAction: 'toggle',
marker: { controls: ['lock.front_door', 'alarm_control_panel.home'], tap_action: 'toggle' } };
c._clickDevice(new MouseEvent('click'), withControls);
out.controlsSafe = lockCalls().length === 0;
// 3) карточка устройства: замок отдаётся в more-info, а не тумблером
const kinds = c._cardEntities({ ...fake, entities: ['lock.front_door', 'alarm_control_panel.home'] });
out.cardNoToggleForLocks = kinds.every((k) => k.kind !== 'toggle');
c._cardToggle('lock.front_door');
c._cardToggle('alarm_control_panel.home');
out.cardToggleRefuses = lockCalls().length === 0;
// 4) кнопка в карточке двери — единственная разрешённая поверхность, и спрашивает подтверждение
let asked = null;
window.confirm = (msg) => { asked = msg; return false; };
c._lockAction('lock.front_door', 'unlock');
out.unlockAsksConfirm = asked !== null && lockCalls().length === 0;
window.confirm = () => true;
c._lockAction('lock.front_door', 'unlock');
out.unlockAfterConfirm = calls.at(-1) === 'lock.unlock:lock.front_door';
// запирание не спрашивает
asked = null;
window.confirm = (m) => { asked = m; return true; };
c._lockAction('lock.front_door', 'lock');
out.lockNoConfirm = asked === null && calls.at(-1) === 'lock.lock:lock.front_door';
return out;
});
checkAll(res);
await finish(browser, res);
File diff suppressed because one or more lines are too long
+4 -4
View File
File diff suppressed because one or more lines are too long
+23
View File
@@ -1,5 +1,28 @@
# Changelog # Changelog
## v1.44.2 — 2026-07-27 (external code review: CR-1…CR-3)
A second, adversarial review (of v1.44.0) produced three findings; all are
addressed.
- **The lock invariant is now precise and enforced (CR-1).** The reviewer was
right that "locks can never be actuated from the plan" was too absolute a
claim: the door card's Unlock button does call the service. That button is a
deliberate product decision, so the invariant is restated where it belongs
("never by an accidental tap; exactly one labeled surface"), unlocking now
**asks for confirmation**, and a new smoke exercises all five actuation paths
to prove icons, `controls[]` and the device card still refuse locks outright.
- **Attachment migration became transactional (CR-2).** Rebinding a marker used
to MOVE its files before the revision-checked config save — if that save was
rejected, the stored config kept the old urls while the files had already
left. Now the server **copies**, the config is committed, and only then the
old folder is removed (`houseplan/files/cleanup`).
- **Failed or partial migrations no longer rewrite urls (CR-3).** The copy
reports an exact `{source: written}` mapping; only confirmed copies are
rewritten, name collisions get a unique name instead of silently linking a
pre-existing file, and a failed migration surfaces as a toast with the links
left pointing at the still-existing originals.
## v1.44.1 — 2026-07-27 ## v1.44.1 — 2026-07-27
- Added the community chat everywhere users look: **https://t.me/ha_houseplan** - Added the community chat everywhere users look: **https://t.me/ha_houseplan**
(badge and header line in both READMEs, a "Getting help" section, the issue (badge and header line in both READMEs, a "Getting help" section, the issue
+12 -1
View File
@@ -33,7 +33,7 @@ Editors are admin-only tools and must never leak interactions into View
|---|---|---| |---|---|---|
| J1 | "Show the whole home and what's happening right now" — live spatial overview: device states, room fills (light/temp/LQI), values, multi-floor tabs | **Closed** | | J1 | "Show the whole home and what's happening right now" — live spatial overview: device states, room fills (light/temp/LQI), values, multi-floor tabs | **Closed** |
| J2 | "Something is wrong — show me *where*" — leak/smoke/gas pulse, open doors/windows, unlocked locks, red dot on devices HA added silently | **Closed** | | J2 | "Something is wrong — show me *where*" — leak/smoke/gas pulse, open doors/windows, unlocked locks, red dot on devices HA added silently | **Closed** |
| J3 | "Let me act on the obvious right from the plan" — tap-to-toggle for safe domains, info cards, guarded lock action (explicit button only, never a plan tap) | **Closed** | | J3 | "Let me act on the obvious right from the plan" — tap-to-toggle for safe domains, info cards, guarded lock action | **Closed** |
| J4 | "From zero to a working plan in one evening, no Inkscape/YAML" — image/PDF/draw, floors-import wizard, room polygons bound to areas, curated auto-placement, editable icon rules | **Closed**; onboarding polish is *partial* (no registry-driven room suggestions) | | J4 | "From zero to a working plan in one evening, no Inkscape/YAML" — image/PDF/draw, floors-import wizard, room polygons bound to areas, curated auto-placement, editable icon rules | **Closed**; onboarding polish is *partial* (no registry-driven room suggestions) |
| J5 | "Room climate at a glance" — per-room temperature/humidity, comfort-range fills, room-card metrics | **Closed** | | J5 | "Room climate at a glance" — per-room temperature/humidity, comfort-range fills, room-card metrics | **Closed** |
| J6 | "Keep the plan true as the home evolves" — new-device flag, two editors, drag/resize, merge/split, multi-client live sync, optimistic locking | **Closed** | | J6 | "Keep the plan true as the home evolves" — new-device flag, two editors, drag/resize, merge/split, multi-client live sync, optimistic locking | **Closed** |
@@ -52,6 +52,17 @@ Editors are admin-only tools and must never leak interactions into View
## Known gaps that fit the mission (build only on owner's request) ## Known gaps that fit the mission (build only on owner's request)
- Person/presence shown in rooms (classic floorplan ask; pure J1). - Person/presence shown in rooms (classic floorplan ask; pure J1).
### The lock invariant, stated precisely (review CR-1)
No lock or alarm panel is ever actuated **by a tap on the plan**: icons, lock
badges, `marker.controls[]` and the device card all refuse (`resolveTapAction`
+ `TOGGLE_FORBIDDEN_DOMAINS`, `isControllable`, `_cardToggle`). There is exactly
**one** sanctioned actuation surface: the labeled Unlock/Lock button inside an
opened door card, which additionally confirms before unlocking. That is a
product decision (2026-07-22), not an oversight — but it means the invariant is
"never by accident", not "never at all". Any new actuation path must either
refuse locks or be added to this paragraph.
- Plan-level "security glance": one badge for "all locked / N open" (J2). - Plan-level "security glance": one badge for "all locked / N open" (J2).
- Threshold colouring for room-card metrics (J5). - Threshold colouring for room-card metrics (J5).
+10
View File
@@ -40,6 +40,16 @@
below; config/diagnostic entities are not listed; locks never toggle from below; config/diagnostic entities are not listed; locks never toggle from
the card [auto: smoke_card_controls] the card [auto: smoke_card_controls]
- [ ] Lock invariant, all paths (v1.44.2, review CR-1): icon tap, controls[],
device card and _cardToggle refuse locks/alarm panels entirely; the door
card's Unlock asks for confirmation, Lock does not [auto: smoke_lock_invariant]
- [ ] Attachment migration is transactional (v1.44.2, review CR-2/CR-3):
rebinding COPIES files, saves the config, and only then deletes the old
folder; a rejected save leaves the old files and urls intact; a name
collision in the destination gets a unique name (the pre-existing file is
never silently linked); urls are rewritten only for confirmed copies
[auto: unit logic.test + tests_backend]
## Environments matrix ## Environments matrix
Run the *core flows* (marked ★ below) in each environment at least once per minor release: Run the *core flows* (marked ★ below) in each environment at least once per minor release:
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "houseplan-card", "name": "houseplan-card",
"version": "1.44.1", "version": "1.44.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",
+33 -8
View File
@@ -32,7 +32,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.44.1'; const CARD_VERSION = '1.44.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';
@@ -2667,14 +2667,23 @@ class HouseplanCard extends LitElement {
const prevRoomId = prevDev?.marker?.room_id ?? null; const prevRoomId = prevDev?.marker?.room_id ?? null;
const roomChanged = !!dlg.room && prevDev != null const roomChanged = !!dlg.room && prevDev != null
&& (prevDev.space !== space || prevDev.area !== area || prevRoomId !== roomId); && (prevDev.space !== space || prevDev.area !== area || prevRoomId !== roomId);
// rebinding changed the id → move the uploaded files along (server-side) // Rebinding changes the marker id, so the uploaded files must follow.
// and rewrite the attached urls; otherwise the old-id folder goes orphan // Order matters (review CR-2): COPY first, save the config, and only then
// (that is how the sauna manuals were lost — incident 2026-07-26) // delete the old folder. If the save is rejected, the old urls in the
// stored config still resolve — the files never left. A failed copy
// leaves the urls untouched and tells the user (review CR-3).
let cleanupOldFiles = false;
if (oldId && oldId !== id && marker.pdfs?.length) { if (oldId && oldId !== id && marker.pdfs?.length) {
await this.hass try {
.callWS({ type: 'houseplan/files/migrate', from_id: oldId, to_id: id }) const res: any = await this.hass.callWS({
.catch(() => undefined); type: 'houseplan/files/migrate', from_id: oldId, to_id: id,
marker.pdfs = migratePdfUrls(marker.pdfs, oldId, id); });
const mapping = res?.mapping || {};
marker.pdfs = migratePdfUrls(marker.pdfs, oldId, id, mapping);
cleanupOldFiles = Object.keys(mapping).length > 0;
} catch (e: any) {
this._showToast(this._t('toast.files_migrate_failed', { err: this._errText(e) }));
}
} }
// remove the previous marker (by the old id and by the new id) // remove the previous marker (by the old id and by the new id)
cfg.markers = cfg.markers.filter((m) => m.id !== id && m.id !== oldId); cfg.markers = cfg.markers.filter((m) => m.id !== id && m.id !== oldId);
@@ -2721,6 +2730,12 @@ class HouseplanCard extends LitElement {
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 }).catch(() => undefined);
} }
// the config is committed — now it is safe to drop the old folder
if (cleanupOldFiles && oldId) {
await this.hass
.callWS({ type: 'houseplan/files/cleanup', marker_id: oldId })
.catch(() => undefined); // leftovers are harmless; broken links are not
}
this._markerDialog = null; this._markerDialog = null;
this._regSignature = ''; this._regSignature = '';
this._maybeRebuildDevices(); this._maybeRebuildDevices();
@@ -4258,6 +4273,16 @@ class HouseplanCard extends LitElement {
* clearly labeled action button same interaction contract as HA's more-info. * clearly labeled action button same interaction contract as HA's more-info.
*/ */
private _lockAction(entityId: string, action: 'lock' | 'unlock'): void { private _lockAction(entityId: string, action: 'lock' | 'unlock'): void {
// THE ONLY sanctioned lock actuation surface (review CR-1, 2026-07-27).
// The invariant is "no lock or alarm panel is ever actuated by a TAP on the
// plan" — icons, badges, controls[] and the device card all refuse. This
// button is a deliberate, labeled control inside an opened card, the same
// contract as Home Assistant's own more-info dialog. Unlocking additionally
// asks for confirmation; locking does not (locking is never destructive).
if (action === 'unlock') {
const name = this.hass?.states?.[entityId]?.attributes?.friendly_name || entityId;
if (!confirm(this._t('confirm.unlock', { name }))) return;
}
this.hass?.callService?.('lock', action, { entity_id: entityId }); this.hass?.callService?.('lock', action, { entity_id: entityId });
} }
+3 -1
View File
@@ -327,5 +327,7 @@
"room.settings_short": "Room", "room.settings_short": "Room",
"room.unnamed": "Unnamed room", "room.unnamed": "Unnamed room",
"marker.is_light": "This device is a light source", "marker.is_light": "This device is a light source",
"marker.is_light_tip": "Makes the icon glow in the “Light sources” fill even without a light entity — for a smart switch driving ordinary fixtures. The glow follows the switch (or the lights bound above)." "marker.is_light_tip": "Makes the icon glow in the “Light sources” fill even without a light entity — for a smart switch driving ordinary fixtures. The glow follows the switch (or the lights bound above).",
"confirm.unlock": "Unlock “{name}”?",
"toast.files_migrate_failed": "Attachments could not be moved to the new binding, links keep pointing at the old files: {err}"
} }
+3 -1
View File
@@ -327,5 +327,7 @@
"room.settings_short": "Комната", "room.settings_short": "Комната",
"room.unnamed": "Комната без имени", "room.unnamed": "Комната без имени",
"marker.is_light": "Это устройство — источник света", "marker.is_light": "Это устройство — источник света",
"marker.is_light_tip": "Даёт ореол в заливке «Свет по источникам» даже без light-сущности — для умного выключателя с обычными светильниками. Ореол следует за выключателем (или за привязанными выше лампами)." "marker.is_light_tip": "Даёт ореол в заливке «Свет по источникам» даже без light-сущности — для умного выключателя с обычными светильниками. Ореол следует за выключателем (или за привязанными выше лампами).",
"confirm.unlock": "Открыть замок «{name}»?",
"toast.files_migrate_failed": "Не удалось перенести вложения к новой привязке, ссылки остались на старые файлы: {err}"
} }
+15 -2
View File
@@ -1070,12 +1070,25 @@ export function roomFillModeOf(
* server moves /files/<oldId>/ to /files/<newId>/, the urls must follow. * server moves /files/<oldId>/ to /files/<newId>/, the urls must follow.
*/ */
export function migratePdfUrls<T extends { url: string }>( export function migratePdfUrls<T extends { url: string }>(
pdfs: T[], oldId: string, newId: string, pdfs: T[], oldId: string, newId: string, mapping?: Record<string, string>,
): T[] { ): T[] {
if (!oldId || !newId || oldId === newId) return pdfs; if (!oldId || !newId || oldId === newId) return pdfs;
const from = '/files/' + oldId + '/'; const from = '/files/' + oldId + '/';
const to = '/files/' + newId + '/'; const to = '/files/' + newId + '/';
return pdfs.map((p) => (p.url.includes(from) ? { ...p, url: p.url.split(from).join(to) } : p)); return pdfs.map((p) => {
if (!p.url.includes(from)) return p;
const tail = p.url.split(from)[1] || '';
const [name, query] = [tail.split('?')[0], tail.includes('?') ? '?' + tail.split('?')[1] : ''];
if (mapping) {
// review CR-3: rewrite ONLY files the server confirmed it copied, and use
// the name it actually wrote (collisions get a unique name). A url that
// was not copied keeps pointing at the still-existing old folder.
const dst = mapping[decodeURIComponent(name)] ?? mapping[name];
if (!dst) return p;
return { ...p, url: p.url.split(from + name)[0] + to + encodeURIComponent(dst) + query };
}
return { ...p, url: p.url.split(from).join(to) };
});
} }
// ---------------- kiosk gestures ---------------- // ---------------- kiosk gestures ----------------
+14
View File
@@ -792,6 +792,20 @@ test('clampScale', () => {
assert.equal(clampScale(undefined, 1.5), 1.5); assert.equal(clampScale(undefined, 1.5), 1.5);
}); });
test('migratePdfUrls: only confirmed copies are rewritten (review CR-3)', () => {
const pdfs = [
{ name: 'a.pdf', url: '/houseplan_files/files/v_old1/a.pdf?v=1' },
{ name: 'b.pdf', url: '/houseplan_files/files/v_old1/b.pdf?v=2' },
];
// сервер скопировал только a.pdf, причём переименовал из-за коллизии
const out = migratePdfUrls(pdfs, 'v_old1', 'dev99', { 'a.pdf': 'a (2).pdf' });
assert.equal(out[0].url, '/houseplan_files/files/dev99/a%20(2).pdf?v=1');
assert.equal(out[1].url, pdfs[1].url, 'нескопированный файл ссылается на старую папку');
// пустой маппинг = ничего не переносим
assert.deepEqual(migratePdfUrls(pdfs, 'v_old1', 'dev99', {}).map((p) => p.url),
pdfs.map((p) => p.url));
});
test('migratePdfUrls: rebinding rewrites file urls', () => { test('migratePdfUrls: rebinding rewrites file urls', () => {
const pdfs = [ const pdfs = [
{ name: 'a.pdf', url: '/houseplan_files/files/v_old1/a.pdf?v=1' }, { name: 'a.pdf', url: '/houseplan_files/files/v_old1/a.pdf?v=1' },
+39
View File
@@ -125,3 +125,42 @@ async def test_admin_check_fails_closed(hass, hass_ws_client):
user = _Admin() user = _Admin()
assert wsapi._check_write(hass, _AdminConn()) is True assert wsapi._check_write(hass, _AdminConn()) is True
async def test_files_migrate_copies_and_reports_mapping(hass, hass_ws_client, tmp_path):
"""review CR-2/CR-3: migrate COPIES, never overwrites, and reports the mapping."""
import os
from custom_components.houseplan.const import FILES_DIR
base = os.path.join(hass.config.path(FILES_DIR))
src = os.path.join(base, "old1")
dst = os.path.join(base, "new1")
await hass.async_add_executor_job(lambda: os.makedirs(src, exist_ok=True))
await hass.async_add_executor_job(lambda: os.makedirs(dst, exist_ok=True))
await hass.async_add_executor_job(
lambda: open(os.path.join(src, "m.pdf"), "wb").write(b"SOURCE")
)
# коллизия: в целевой папке уже есть ДРУГОЙ файл с тем же именем
await hass.async_add_executor_job(
lambda: open(os.path.join(dst, "m.pdf"), "wb").write(b"OTHER")
)
client = await hass_ws_client(hass)
await client.send_json({"id": 1, "type": "houseplan/files/migrate",
"from_id": "old1", "to_id": "new1"})
resp = await client.receive_json()
assert resp["success"]
mapping = resp["result"]["mapping"]
assert mapping["m.pdf"] != "m.pdf" # переименован, а не перезаписан
# источник ещё на месте (копия, не перенос) и чужой файл не тронут
assert await hass.async_add_executor_job(lambda: os.path.isfile(os.path.join(src, "m.pdf")))
assert await hass.async_add_executor_job(
lambda: open(os.path.join(dst, "m.pdf"), "rb").read()) == b"OTHER"
assert await hass.async_add_executor_job(
lambda: open(os.path.join(dst, mapping["m.pdf"]), "rb").read()) == b"SOURCE"
# cleanup удаляет исходную папку — вызывается только после успешного сохранения
await client.send_json({"id": 2, "type": "houseplan/files/cleanup", "marker_id": "old1"})
resp2 = await client.receive_json()
assert resp2["success"] and resp2["result"]["removed"] is True
assert not await hass.async_add_executor_job(lambda: os.path.isdir(src))