mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
fix: avoid double disk reserve for staged uploads (#554)
Issue: #554 User-Visible: yes
This commit is contained in:
committed by
claude[bot]
parent
49130d4ffd
commit
51ead265f0
@@ -451,6 +451,7 @@ class HouseplanUploadView(HomeAssistantView):
|
||||
partial(
|
||||
check_quota, files_root, tmp_path.stat().st_size,
|
||||
MAX_FILES_BYTES, MAX_FILES_COUNT, exclude=tmp_path,
|
||||
additional_disk_bytes=0,
|
||||
),
|
||||
)
|
||||
except QuotaError as err:
|
||||
|
||||
@@ -222,9 +222,20 @@ def dir_usage(path: Path, *, exclude: Path | None = None) -> tuple[int, int]:
|
||||
|
||||
|
||||
def check_quota(
|
||||
path: Path, incoming: int, max_bytes: int, max_files: int, *, exclude: Path | None = None,
|
||||
path: Path,
|
||||
incoming: int,
|
||||
max_bytes: int,
|
||||
max_files: int,
|
||||
*,
|
||||
exclude: Path | None = None,
|
||||
additional_disk_bytes: int | None = None,
|
||||
) -> None:
|
||||
"""Raise QuotaError unless `incoming` more bytes fit (`exclude`: see dir_usage).
|
||||
"""Raise QuotaError unless `incoming` fits the store and disk.
|
||||
|
||||
`exclude` is omitted from current store usage but still charged as
|
||||
`incoming`. `additional_disk_bytes` is the part not physically written yet;
|
||||
by default all incoming bytes still need disk space. A staged file already
|
||||
under `path` passes zero because promotion only renames it (#554).
|
||||
|
||||
Deliberately not an age rule. Files are never removed for getting old — that
|
||||
cost real plans twice — so the limit sits where a decision is being made
|
||||
@@ -245,7 +256,8 @@ def check_quota(
|
||||
free = shutil.disk_usage(str(path if path.is_dir() else path.parent)).free
|
||||
except OSError:
|
||||
return
|
||||
if free - incoming < MIN_FREE_BYTES:
|
||||
disk_incoming = incoming if additional_disk_bytes is None else additional_disk_bytes
|
||||
if free - disk_incoming < MIN_FREE_BYTES:
|
||||
raise QuotaError("low_disk_space", f"only {free // 1024 // 1024} MB free on the disk")
|
||||
|
||||
|
||||
|
||||
@@ -2,6 +2,11 @@
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Uploading an attachment no longer reports low disk space merely because its
|
||||
completed staging file was counted a second time. The 512 MiB safety reserve
|
||||
itself is unchanged
|
||||
([#554](https://github.com/Matysh/houseplan-card/issues/554)).
|
||||
|
||||
- Range and zone radar sources now stay live in the setup preview and use the
|
||||
same complete source list for Home Assistant read-permission checks as for
|
||||
frame calculation
|
||||
|
||||
@@ -8,6 +8,11 @@
|
||||
|
||||
## Не выпущено
|
||||
|
||||
- Загрузка вложения больше не сообщает о нехватке места только потому, что уже
|
||||
записанный staging-файл посчитали повторно. Сам резерв безопасности 512 МиБ
|
||||
не изменился
|
||||
([#554](https://github.com/Matysh/houseplan-card/issues/554)).
|
||||
|
||||
- Источники диапазонов и зон радара теперь обновляются в предпросмотре
|
||||
настройки, а проверка прав Home Assistant использует тот же полный список
|
||||
источников, что и расчёт данных радара
|
||||
|
||||
@@ -1652,6 +1652,13 @@ separately promised workflows:
|
||||
returns the newest 60 with a total
|
||||
[auto: unit: test_check_quota_counts_the_whole_store_not_one_request,
|
||||
backend test_uploads_are_bounded_by_a_store_quota]
|
||||
- [ ] An attachment already written to staging is not reserved twice: with the
|
||||
real 512 MiB disk reserve intact its same-filesystem promotion succeeds,
|
||||
while one byte below the reserve still fails. Uploads checked before any
|
||||
bytes are written continue to reserve their full incoming size
|
||||
[auto: backend test_issue_554_low_disk_reserve_distinguishes_staged_and_unwritten_bytes
|
||||
+ test_issue_554_upload_uses_actual_free_space_after_staging; mutation:
|
||||
quota-reserves-staged-bytes-twice-on-disk]
|
||||
- [ ] Square canvas migration (v1.48.0): after the upgrade every existing plan
|
||||
looks exactly as before, just with margins where the canvas was extended.
|
||||
Measure a wall in the plan editor — the length in cm is unchanged. Marker
|
||||
|
||||
@@ -8873,6 +8873,19 @@ const MUTANT_DEFINITIONS = [
|
||||
replace: ' if item.name.startswith(TMP_PREFIX): # mutant: every staged file is invisible\n',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'quota-reserves-staged-bytes-twice-on-disk',
|
||||
guard: 'node scripts/backend-test-guard.mjs '
|
||||
+ 'issue_554_upload_uses_actual_free_space_after_staging '
|
||||
+ 'tests_backend/test_ha_upload.py',
|
||||
because: 'the attachment body already occupies disk space inside files_root; subtracting its '
|
||||
+ 'size again rejects a rename even when the real reserve is intact (#554 AC5)',
|
||||
patches: [{
|
||||
file: 'custom_components/houseplan/http_api.py',
|
||||
find: ' additional_disk_bytes=0,\n',
|
||||
replace: ' # mutant: staged bytes are reserved a second time\n',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'support-palette-copies-any-key',
|
||||
guard: 'node scripts/backend-test-guard.mjs '
|
||||
|
||||
@@ -150,6 +150,38 @@ async def test_issue_498_concurrent_uploads_still_count_each_other(
|
||||
assert stored <= 1000
|
||||
|
||||
|
||||
async def test_issue_554_upload_uses_actual_free_space_after_staging(
|
||||
hass: HomeAssistant, hass_client: ClientSessionGenerator, monkeypatch,
|
||||
) -> None:
|
||||
"""A completed staging file needs no second disk reserve before its rename."""
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
from custom_components.houseplan.const import FILES_DIR, MIN_FREE_BYTES
|
||||
from custom_components.houseplan.plans import TMP_PREFIX
|
||||
|
||||
await _setup(hass)
|
||||
client = await hass_client()
|
||||
usage = type("Usage", (), {"free": MIN_FREE_BYTES})()
|
||||
monkeypatch.setattr(shutil, "disk_usage", lambda _path: usage)
|
||||
|
||||
accepted = await client.post(
|
||||
"/api/houseplan/upload", data=_pdf_form("at-reserve.pdf", 50),
|
||||
)
|
||||
assert accepted.status == 200, await accepted.text()
|
||||
|
||||
usage.free = MIN_FREE_BYTES - 1
|
||||
refused = await client.post(
|
||||
"/api/houseplan/upload", data=_pdf_form("below-reserve.pdf", 50),
|
||||
)
|
||||
assert refused.status == 507
|
||||
assert (await refused.json())["error"] == "low_disk_space"
|
||||
|
||||
root = Path(hass.config.path(FILES_DIR))
|
||||
assert not list(root.glob(TMP_PREFIX + "*"))
|
||||
assert sorted(path.name for path in (root / "m1").iterdir()) == ["at-reserve.pdf"]
|
||||
|
||||
|
||||
def _svg_chain(length: int) -> bytes:
|
||||
defs = "".join(
|
||||
f'<linearGradient id="g{index}" href="#g{index + 1}"/>' for index in range(length - 1)
|
||||
|
||||
@@ -1574,6 +1574,48 @@ def test_check_quota_refuses_when_the_disk_is_nearly_full(tmp_path, monkeypatch)
|
||||
assert e.value.reason == "low_disk_space"
|
||||
|
||||
|
||||
def test_issue_554_low_disk_reserve_distinguishes_staged_and_unwritten_bytes(
|
||||
tmp_path, monkeypatch,
|
||||
):
|
||||
"""Already-written staging needs no second reserve; pending bytes still do."""
|
||||
import shutil
|
||||
|
||||
d = tmp_path / "files"
|
||||
d.mkdir()
|
||||
staged = d / (plans.TMP_PREFIX + "own")
|
||||
staged.write_bytes(b"x" * 50)
|
||||
usage = type("Usage", (), {"free": const.MIN_FREE_BYTES})()
|
||||
monkeypatch.setattr(shutil, "disk_usage", lambda _path: usage)
|
||||
|
||||
plans.check_quota(
|
||||
d,
|
||||
50,
|
||||
max_bytes=1000,
|
||||
max_files=10,
|
||||
exclude=staged,
|
||||
additional_disk_bytes=0,
|
||||
)
|
||||
|
||||
usage.free = const.MIN_FREE_BYTES - 1
|
||||
with pytest.raises(plans.QuotaError) as staged_low:
|
||||
plans.check_quota(
|
||||
d,
|
||||
50,
|
||||
max_bytes=1000,
|
||||
max_files=10,
|
||||
exclude=staged,
|
||||
additional_disk_bytes=0,
|
||||
)
|
||||
assert staged_low.value.reason == "low_disk_space"
|
||||
|
||||
usage.free = const.MIN_FREE_BYTES + 50
|
||||
plans.check_quota(d, 50, max_bytes=1000, max_files=10)
|
||||
usage.free -= 1
|
||||
with pytest.raises(plans.QuotaError) as unwritten_low:
|
||||
plans.check_quota(d, 50, max_bytes=1000, max_files=10)
|
||||
assert unwritten_low.value.reason == "low_disk_space"
|
||||
|
||||
|
||||
class TestVacuum:
|
||||
"""marker.vacuum (docs/VACUUM.md): optional everywhere, matrices strict."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user