mirror of
https://github.com/tykeal/homeassistant-rental-control.git
synced 2026-11-06 05:17:23 +00:00
Fix(allocator): Trust a rebuilt registry
A corrupt (not missing) registry kept every unpublished request withheld until a lockless cycle cleared the loss flags, so a site with only lock-backed entries stayed wedged and an excluded slot could hold recovery forever. Per the maintainer's decision, trust the rebuilt registry once every configured lock-backed entry has positively adopted its current lock and slot range, with operator-excluded slots counting as observed. An unreachable lock still withholds issuance; the adoption notification now says the registry is being rebuilt and labels a lock none of whose slots can be read, and an adopted lock that becomes unsound while the registry is lost returns to the gate so the notification reappears. Diagnostics report entries whose lock or range is not yet adopted. Closes #893 (#851) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
This commit is contained in:
1 parent
00117918c2
commit
ad3eea10be
8 files changed
+254
-20
No files matched your search
@@ -258,8 +258,17 @@ An unmatched code usually belongs to a booking that has gone away, and clears
|
||||
itself once Keymaster finishes clearing the slot. If it persists, clear that
|
||||
code at the lock.
|
||||
|
||||
When Home Assistant loses the shared code registry, or finds it corrupt, a
|
||||
**disabled** config entry that drives a lock also holds the gate shut. Disabling an entry leaves its
|
||||
When Home Assistant loses the shared code registry, or finds it corrupt, the
|
||||
integration rebuilds it from the locks. New codes resume once every config
|
||||
entry that drives a lock has read its lock across its whole managed slot
|
||||
range, and the registry counts as rebuilt from then on. A slot you excluded
|
||||
with the slot exclusion action counts as read. A lock that the integration
|
||||
cannot reach keeps codes withheld for every entry until it comes back online,
|
||||
and the `code adoption pending` notification marks such an entry as
|
||||
unreachable. If you widen an entry's slot range, the integration must read
|
||||
the new slots too before it trusts the rebuilt registry.
|
||||
|
||||
In that situation a **disabled** config entry that drives a lock also holds the gate shut. Disabling an entry leaves its
|
||||
codes on the lock, and with the registry gone nothing else records them, so
|
||||
any code issued could already open that lock. The `code adoption pending`
|
||||
notification marks such an entry as disabled. Enable it so the integration
|
||||
|
||||
@@ -42,8 +42,15 @@ from .store import RegistryStore
|
||||
from .store import code_ref_for
|
||||
|
||||
_LOGGER = logging.getLogger(__name__)
|
||||
_ADOPTION_GATE_WARNING_SECONDS = 300.0
|
||||
_ADOPTION_GATE_WARNING_SECONDS = recovery.ADOPTION_GATE_WARNING_SECONDS
|
||||
_GATE_NOTIFICATION_ID = f"{DOMAIN}_code_registry_adoption_pending"
|
||||
_REGISTRY_LOST_NOTICE = (
|
||||
" The door code registry was lost or found corrupt, so codes resume "
|
||||
"only once every entry with a lock has read its whole slot range "
|
||||
"again. A lock that cannot be reached keeps codes withheld until it "
|
||||
"is back online; exclude a permanently dead slot with the "
|
||||
"rental_control.set_slot_exclusion action."
|
||||
)
|
||||
|
||||
|
||||
class DoorCodeAllocator:
|
||||
@@ -251,6 +258,7 @@ class DoorCodeAllocator:
|
||||
"locks still has a slot that has not been read, no other "
|
||||
"entry issues new door codes either, because every entry "
|
||||
"shares one door code space."
|
||||
+ (_REGISTRY_LOST_NOTICE if self._registry_lost else "")
|
||||
),
|
||||
title=f"{NAME} code adoption pending",
|
||||
notification_id=_GATE_NOTIFICATION_ID,
|
||||
|
||||
@@ -76,6 +76,7 @@ def allocator_diagnostics(allocator: DoorCodeAllocator) -> dict[str, Any]:
|
||||
"gate_deadline": allocator._gate_deadline,
|
||||
"registry_lost": allocator._registry_lost,
|
||||
"registry_missing": allocator._registry_missing,
|
||||
"unobserved_lock_entries": sorted(recovery.unobserved_lock_entries(allocator)),
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -248,7 +248,6 @@ async def resolve_cycle(
|
||||
request,
|
||||
unaccounted=unaccounted,
|
||||
recovery_declined=recovery_declined,
|
||||
lockless_cycle=lockless_cycle,
|
||||
)
|
||||
return CycleResult(
|
||||
adopted=adopted,
|
||||
@@ -265,27 +264,19 @@ def _persist_registry(
|
||||
*,
|
||||
unaccounted: frozenset[int],
|
||||
recovery_declined: bool,
|
||||
lockless_cycle: bool,
|
||||
) -> None:
|
||||
"""Save the registry and clear recovery flags once the cycle is sound.
|
||||
|
||||
A lockless cycle may complete recovery even when the stored registry
|
||||
was corrupt rather than absent. Either way the flags may only clear
|
||||
once no entry is still awaiting its first cycle and every configured
|
||||
lock-backed entry has actually adopted, since clearing them returns
|
||||
the allocator to normal issuance, where a code still programmed in
|
||||
an unobserved lock would look free.
|
||||
The flags clear, whether the stored registry was absent or corrupt,
|
||||
once the rebuilt registry accounts for every lock (#851): clearing
|
||||
them returns the allocator to normal issuance, where a code still
|
||||
programmed in an unobserved lock would look free.
|
||||
"""
|
||||
if not allocator._registry_lost:
|
||||
allocator._store.async_save(allocator._registry)
|
||||
return
|
||||
recoverable = (
|
||||
(allocator._registry_missing or lockless_cycle)
|
||||
and not allocator._pending_adoption
|
||||
and not recovery.unobserved_lock_entries(allocator)
|
||||
)
|
||||
if (
|
||||
recoverable
|
||||
recovery.rebuilt_registry_trusted(allocator)
|
||||
and request.adoption_complete
|
||||
and not unaccounted
|
||||
and not recovery_declined
|
||||
@@ -316,6 +307,8 @@ def _record_adoption(
|
||||
sound = request.adoption_complete and not unaccounted
|
||||
if lockname is not None and not sound:
|
||||
allocator._adopted_locks.pop(entry_id, None)
|
||||
if allocator._registry_lost:
|
||||
recovery.reopen_adoption(allocator, entry_id)
|
||||
if sound:
|
||||
if lockname is not None:
|
||||
allocator._adopted_locks[entry_id] = ObservedLock(
|
||||
|
||||
@@ -6,6 +6,7 @@ from __future__ import annotations
|
||||
|
||||
from collections.abc import Mapping
|
||||
import logging
|
||||
from time import monotonic
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from ..const import CONF_EXCLUDED_SLOTS
|
||||
@@ -26,6 +27,7 @@ if TYPE_CHECKING:
|
||||
from .allocator import DoorCodeAllocator
|
||||
|
||||
_LOGGER = logging.getLogger(__name__)
|
||||
ADOPTION_GATE_WARNING_SECONDS = 300.0
|
||||
|
||||
|
||||
def entry_has_lock(data: object) -> bool:
|
||||
@@ -125,6 +127,9 @@ def describe_pending_entry(
|
||||
slots = sorted(allocator._pending_adoption_slots.get(entry_id, ()))
|
||||
if not slots:
|
||||
return label
|
||||
configured = configured_slots(entry_info.entry_config(allocator.hass, entry_id))
|
||||
if configured and configured <= set(slots):
|
||||
return f"{label} (lock unreachable: none of its slots can be read)"
|
||||
noun = "slots" if len(slots) > 1 else "slot"
|
||||
return f"{label} (unreadable {noun} {', '.join(map(str, slots))})"
|
||||
|
||||
@@ -212,6 +217,34 @@ def unobserved_lock_entries(allocator: DoorCodeAllocator) -> frozenset[str]:
|
||||
)
|
||||
|
||||
|
||||
def rebuilt_registry_trusted(allocator: DoorCodeAllocator) -> bool:
|
||||
"""Return whether a registry rebuilt after a loss accounts for every lock.
|
||||
|
||||
Adoption re-registers every code a lock presents, so once no entry
|
||||
is pending and every configured lock-backed entry has adopted its
|
||||
current lock and slot range (#856), the rebuilt registry holds every
|
||||
code those locks present. That holds whether the stored registry was
|
||||
absent or corrupt (#851). A slot the operator excluded counts as
|
||||
read, as it does for adoption; a lock that cannot be reached never
|
||||
adopts, so it keeps the registry untrusted.
|
||||
"""
|
||||
return not allocator._pending_adoption and not unobserved_lock_entries(allocator)
|
||||
|
||||
|
||||
def reopen_adoption(allocator: DoorCodeAllocator, entry_id: str) -> None:
|
||||
"""Return an adopted entry to the gate after its lock stops reading.
|
||||
|
||||
While the registry is lost, an entry that loses its adoption keeps
|
||||
the rebuilt registry untrusted. Putting it back in the gate keeps
|
||||
issuance withheld through the same path as before it adopted, and
|
||||
lets the pending notification name it and say why.
|
||||
"""
|
||||
allocator._pending_adoption.add(entry_id)
|
||||
if allocator._gate_deadline == 0.0:
|
||||
allocator._gate_deadline = monotonic() + ADOPTION_GATE_WARNING_SECONDS
|
||||
allocator._gate_warning_sent = False
|
||||
|
||||
|
||||
def lockless_recovery_allowed(
|
||||
allocator: DoorCodeAllocator,
|
||||
request: AllocationRequest,
|
||||
@@ -327,7 +360,8 @@ def issuance_block(
|
||||
if adoption_blocks(allocator, request.entry_id):
|
||||
return AllocationResult(code=None, reason="adoption_pending"), False
|
||||
if not allocator._registry_lost or not (
|
||||
request.previously_published or not allocator._registry_missing
|
||||
request.previously_published
|
||||
or not (allocator._registry_missing or rebuilt_registry_trusted(allocator))
|
||||
):
|
||||
return None, False
|
||||
if not lockless_recovery_allowed(allocator, request, lockless_cycle=lockless_cycle):
|
||||
@@ -348,6 +382,7 @@ def issuance_block(
|
||||
|
||||
|
||||
__all__ = [
|
||||
"ADOPTION_GATE_WARNING_SECONDS",
|
||||
"adoption_blocks",
|
||||
"continuity_required",
|
||||
"configured_lockname",
|
||||
@@ -361,6 +396,8 @@ __all__ = [
|
||||
"origin_proven",
|
||||
"pending_lock_codes",
|
||||
"read_lock_codes",
|
||||
"rebuilt_registry_trusted",
|
||||
"reopen_adoption",
|
||||
"issuance_block",
|
||||
"lockless_recovery_allowed",
|
||||
"unobserved_lock_entries",
|
||||
|
||||
@@ -1249,8 +1249,17 @@ async def test_recovery_fail_closed_for_published_identity() -> None:
|
||||
|
||||
|
||||
async def test_corrupt_registry_fails_closed_for_new_identity() -> None:
|
||||
"""Corrupt registry recovery blocks all new issuance."""
|
||||
"""A corrupt registry blocks new issuance while a lock is unobserved."""
|
||||
allocator = _allocator()
|
||||
allocator.hass.config_entries = SimpleNamespace(
|
||||
async_entries=lambda _domain=None: [
|
||||
SimpleNamespace(
|
||||
entry_id="entry-lock",
|
||||
disabled_by=None,
|
||||
data={"keymaster_entry_id": "front", "start_slot": 1, "max_events": 1},
|
||||
)
|
||||
]
|
||||
)
|
||||
allocator._registry_lost = True
|
||||
allocator._registry_missing = False
|
||||
|
||||
|
||||
@@ -0,0 +1,178 @@
|
||||
# SPDX-FileCopyrightText: 2026 Andrew Grimberg <tykeal@bardicgrove.org>
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
"""Recovery from a corrupt registry for lock-backed entries (#851).
|
||||
|
||||
A corrupt registry cannot be told apart from one that recorded codes
|
||||
now gone, so it used to wedge every lock-backed entry until a lockless
|
||||
entry happened to run. The rebuilt registry is now trusted once every
|
||||
configured lock-backed entry has adopted its current lock and slot
|
||||
range, with excluded slots counting as read. A lock that cannot be
|
||||
reached never adopts, so it keeps issuance withheld.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from types import MappingProxyType
|
||||
|
||||
import pytest
|
||||
|
||||
from custom_components.rental_control.allocator import allocator as allocator_module
|
||||
from custom_components.rental_control.allocator.allocator import DoorCodeAllocator
|
||||
from custom_components.rental_control.allocator.models import CycleObservation
|
||||
from custom_components.rental_control.allocator.models import CycleRequest
|
||||
from custom_components.rental_control.reconciliation import ManagedSlot
|
||||
from custom_components.rental_control.reconciliation import SlotStatus
|
||||
|
||||
from tests.unit.test_lockless_registry_recovery import _allocator
|
||||
from tests.unit.test_lockless_registry_recovery import _cycle
|
||||
from tests.unit.test_lockless_registry_recovery import _lock_data
|
||||
from tests.unit.test_lockless_registry_recovery import _reservation
|
||||
from tests.unit.test_lockless_registry_recovery import _slot
|
||||
|
||||
_FREE = ManagedSlot(slot=1, managed=True, status=SlotStatus.FREE)
|
||||
|
||||
|
||||
def _corrupt(allocator: DoorCodeAllocator) -> DoorCodeAllocator:
|
||||
"""Mark the registry as present but unreadable."""
|
||||
allocator._registry_lost = True
|
||||
allocator._registry_missing = False
|
||||
return allocator
|
||||
|
||||
|
||||
async def test_corrupt_registry_recovers_once_every_lock_is_adopted() -> None:
|
||||
"""Lock-backed adoption alone rebuilds a trusted registry."""
|
||||
allocator = _corrupt(_allocator("entry-lock"))
|
||||
|
||||
await _cycle(
|
||||
allocator,
|
||||
[_reservation("identity-lock", code="1234")],
|
||||
entry_id="entry-lock",
|
||||
lockname="front",
|
||||
managed_slots=[_slot(1, "1234")],
|
||||
)
|
||||
|
||||
assert allocator._registry_lost is False
|
||||
reservations = [_reservation(published=False)]
|
||||
await _cycle(allocator, reservations, entry_id="entry-free")
|
||||
assert reservations[0].slot_code is not None
|
||||
|
||||
|
||||
async def test_unobserved_lock_keeps_corrupt_registry_untrusted() -> None:
|
||||
"""Every configured lock-backed entry must adopt, not just one."""
|
||||
allocator = _corrupt(_allocator("entry-lock", "entry-other"))
|
||||
|
||||
await _cycle(
|
||||
allocator,
|
||||
[_reservation("identity-lock", code="1234")],
|
||||
entry_id="entry-lock",
|
||||
lockname="front",
|
||||
managed_slots=[_slot(1, "1234")],
|
||||
)
|
||||
allocator._pending_adoption.discard("entry-other")
|
||||
|
||||
reservations = [_reservation(published=False)]
|
||||
await _cycle(allocator, reservations, entry_id="entry-free")
|
||||
|
||||
assert reservations[0].slot_code is None
|
||||
assert reservations[0].code_source == "unallocated"
|
||||
assert allocator._registry_lost is True
|
||||
assert allocator.diagnostics["unobserved_lock_entries"] == ["entry-other"]
|
||||
|
||||
|
||||
async def test_excluded_slot_counts_as_read() -> None:
|
||||
"""An excluded unreadable slot does not wedge recovery."""
|
||||
allocator = _corrupt(_allocator("entry-lock"))
|
||||
entry = allocator.hass.config_entries.async_get_entry("entry-lock")
|
||||
entry.data = MappingProxyType(_lock_data("front", max_events=2))
|
||||
|
||||
await allocator.async_resolve_cycle(
|
||||
CycleRequest(
|
||||
observation=CycleObservation(
|
||||
entry_id="entry-lock",
|
||||
lockname="front",
|
||||
managed_slots=frozenset({1, 2}),
|
||||
observed_codes={},
|
||||
unreadable_slots=frozenset({2}),
|
||||
),
|
||||
adoptions=[],
|
||||
rekeys=[],
|
||||
allocations=[],
|
||||
active_keys=set(),
|
||||
adoption_complete=True,
|
||||
excluded_slots=frozenset({2}),
|
||||
)
|
||||
)
|
||||
|
||||
assert allocator._pending_adoption == set()
|
||||
assert allocator._registry_lost is False
|
||||
|
||||
|
||||
async def test_unreachable_lock_withholds_codes_and_says_so(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A lock that cannot be read keeps issuance shut and is named."""
|
||||
raised: list[str] = []
|
||||
monkeypatch.setattr(
|
||||
allocator_module,
|
||||
"async_create",
|
||||
lambda _hass, message, **_kwargs: raised.append(message),
|
||||
)
|
||||
allocator = _corrupt(_allocator("entry-lock"))
|
||||
|
||||
await _cycle(
|
||||
allocator,
|
||||
[],
|
||||
entry_id="entry-lock",
|
||||
lockname="front",
|
||||
managed_slots=[ManagedSlot(slot=1, managed=True, status=SlotStatus.UNKNOWN)],
|
||||
)
|
||||
reservations = [_reservation(published=False)]
|
||||
await _cycle(allocator, reservations, entry_id="entry-free")
|
||||
|
||||
assert reservations[0].slot_code is None
|
||||
assert allocator._registry_lost is True
|
||||
allocator._gate_deadline = 1.0
|
||||
allocator._warn_if_gate_expired()
|
||||
assert "lock unreachable" in raised[0]
|
||||
assert "lost or found corrupt" in raised[0]
|
||||
|
||||
|
||||
async def test_lock_lost_after_adoption_returns_to_the_gate(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""An adopted lock that stops reading is named in the notification."""
|
||||
raised: list[str] = []
|
||||
monkeypatch.setattr(
|
||||
allocator_module,
|
||||
"async_create",
|
||||
lambda _hass, message, **_kwargs: raised.append(message),
|
||||
)
|
||||
allocator = _corrupt(_allocator("entry-lock", "entry-other"))
|
||||
await _cycle(
|
||||
allocator,
|
||||
[],
|
||||
entry_id="entry-lock",
|
||||
lockname="front",
|
||||
managed_slots=[_FREE],
|
||||
)
|
||||
await _cycle(
|
||||
allocator,
|
||||
[],
|
||||
entry_id="entry-lock",
|
||||
lockname="front",
|
||||
managed_slots=[ManagedSlot(slot=1, managed=True, status=SlotStatus.UNKNOWN)],
|
||||
)
|
||||
await _cycle(
|
||||
allocator,
|
||||
[],
|
||||
entry_id="entry-other",
|
||||
lockname="front",
|
||||
managed_slots=[_FREE],
|
||||
)
|
||||
|
||||
assert allocator._pending_adoption == {"entry-lock"}
|
||||
assert allocator._registry_lost is True
|
||||
assert allocator._gate_deadline > 0.0
|
||||
allocator._gate_deadline = 1.0
|
||||
allocator._warn_if_gate_expired()
|
||||
assert "lock unreachable" in raised[0]
|
||||
@@ -282,7 +282,6 @@ async def test_lockless_recovery_avoids_a_programmed_lock_code() -> None:
|
||||
assert lockless[0].slot_code != _CODE
|
||||
assert lockless[0].slot_code is None
|
||||
assert lockless[0].code_source == "unallocated"
|
||||
assert allocator._registry_lost is True
|
||||
|
||||
|
||||
async def test_lockless_recovery_waits_for_pending_adoption() -> None:
|
||||
|
||||
Reference in new issue
Block a user