From 22d0a0e3f2d75bdd0720680504dc7d954a989fab Mon Sep 17 00:00:00 2001 From: raman325 <7243222+raman325@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:37:58 -0400 Subject: [PATCH 1/2] fix: settle staged options on every exit, and stop double-counting a repaired user Two follow-ups to #1515. An options submission that changes nothing returned before the settle, so it sat in `options` for good -- read in preference to `data` for as long as the entry existed, then cleared by the next write that touched the entry, taking anything that lived only there. Settling is now one helper called from both exits. The unnumbered-user repair asked allocation for room for one user more than exists: the person being edited is already in the entry's users, so they were counted twice. On a lock with exactly enough room the repair dialog refused, naming a user count that does not exist -- the very failure the path was added to prevent. A repair that also renamed issued the new name a second number and burned the first. Both come from the same place, so both now read from one `others` dict, which is what the name check already used. Entire-Checkpoint: 0c8adf394d8d --- .../lock_code_manager/__init__.py | 61 ++++++++------ .../lock_code_manager/config_flow.py | 16 ++-- tests/test_config_flow.py | 79 +++++++++++++++++++ tests/test_init.py | 27 +++++++ 4 files changed, 153 insertions(+), 30 deletions(-) diff --git a/custom_components/lock_code_manager/__init__.py b/custom_components/lock_code_manager/__init__.py index f70ccd104..a88cf7acd 100644 --- a/custom_components/lock_code_manager/__init__.py +++ b/custom_components/lock_code_manager/__init__.py @@ -1828,6 +1828,36 @@ async def async_update_listener( config_entry.runtime_data.settled.set() +@callback +def _async_settle_options( + hass: HomeAssistant, + config_entry: LockCodeManagerConfigEntry, + config: EntryConfig, +) -> None: + """ + Fold a staged options submission into the entry's own data. + + `data` is where the configuration lives; `options` is a staging area, + because an options flow cannot write `data` itself. Settling is what keeps + that true, so it happens on every path out of the update listener. + + Only when something is actually staged. Settling unconditionally would + corrupt the direct writes: `async_write_entry_config` reconciles the + subentries and the entry in separate calls, each of which wakes the + listener, and a pass that woke between them would write back the half of + the configuration it had read -- undoing the half that had already landed. + Those callers clear `options` themselves, so this is a no-op for them. + + ``to_dict()`` is what makes the stored data plain dicts rather than the + read-only ``MappingProxyType`` wrappers ``EntryConfig`` uses internally, + which Home Assistant's storage layer cannot serialize. + """ + if config_entry.options: + hass.config_entries.async_update_entry( + config_entry, data=config.to_dict(), options={} + ) + + async def _async_apply_entry_update( hass: HomeAssistant, config_entry: LockCodeManagerConfigEntry, @@ -1867,6 +1897,11 @@ async def _async_apply_entry_update( # produces no diff when it re-enters. diff = EntryConfigDiff(old=old_config, new=new_config) if not diff.has_changes: + # Settled here too. An options submission that changes nothing reaches + # this return, and leaving it staged would strand it in `options` for + # good -- read in preference to `data` for as long as the entry + # exists, then cleared by the next write that touches the entry. + _async_settle_options(hass, config_entry, new_config) return ent_reg = er.async_get(hass) @@ -2021,31 +2056,7 @@ async def _async_apply_entry_update( _LOGGER.info( "%s (%s): Done creating and/or updating entities", entry_id, entry_title ) - # Only when there is something staged to settle. The options flow leaves - # its submission in `options` for this listener to fold into `data`; - # everything else writes `data` directly and clears `options` itself. - # - # An options submission that changes nothing does not reach here at all -- - # the diff above is empty and returns first -- so `options` is left - # standing. Harmless while it holds what the options flow writes, which is - # `data`'s own contents; a field that lived in `options` WITHOUT also being - # in `data` would go stale there, so do not add one. - # - # Settling unconditionally corrupts those direct writes. - # `async_write_entry_config` reconciles the subentries and the entry in - # separate calls, each of which wakes this listener -- and a pass that - # woke between them would write back the half of the configuration it - # read, undoing the half that had already landed. - # - # to_dict() is what makes the stored data plain dicts rather than the - # read-only MappingProxyType wrappers EntryConfig uses internally, which - # Home Assistant's storage layer cannot serialize. - if config_entry.options: - hass.config_entries.async_update_entry( - config_entry, data=new_config.to_dict(), options={} - ) - # The async_update_entry above re-triggers this listener, which - # refreshes runtime_data.config at the top before the early-return. + _async_settle_options(hass, config_entry, new_config) # Notify Lovelace dashboards to re-render when structure changes # (slots or locks added/removed), so strategy-generated cards update diff --git a/custom_components/lock_code_manager/config_flow.py b/custom_components/lock_code_manager/config_flow.py index 827c8547a..fc8dd227b 100644 --- a/custom_components/lock_code_manager/config_flow.py +++ b/custom_components/lock_code_manager/config_flow.py @@ -795,11 +795,17 @@ async def async_step_reconfigure( description_placeholders: dict[str, Any] = {} held = normalize_name(subentry.title) + # Everybody except the person being edited. Both the name check and, + # below, the count allocation is asked for are about the OTHERS: the + # edited user is already in `config.users`, so counting them again + # refuses on a lock with exactly enough room, and reconciling against + # a dict that still holds their old name issues their new one a + # second number. + others = {other: user for other, user in config.users.items() if other != held} + if user_input is not None: name, fields, errors, placeholders = _validate_user_form( - self.hass, - user_input, - [other for other in config.users if other != held], + self.hass, user_input, others ) description_placeholders.update(placeholders) @@ -822,7 +828,7 @@ async def async_step_reconfigure( allocation_errors, allocation_placeholders, ) = await _allocate_for( - self.hass, entry, config.locks, len(config.users) + 1 + self.hass, entry, config.locks, len(others) + 1 ) if unavailable is None: errors.update(allocation_errors) @@ -831,7 +837,7 @@ async def async_step_reconfigure( subentry, user_input, errors, description_placeholders ) held_slot = config.assignment.reconcile( - {**config.users, name: fields}, + {**others, name: fields}, start=1, unavailable=unavailable, ).slot(name) diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 5f713b003..0fe5566ff 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -806,6 +806,85 @@ async def test_editing_a_user_who_has_no_number_issues_one( assert entry.subentries[subentry.subentry_id].data[CONF_SLOT] == 1 +async def test_repairing_an_unnumbered_user_fits_on_a_lock_with_exactly_enough_room( + hass: HomeAssistant, mock_lock_config_entry +): + """ + The repair asks for room for the OTHERS plus this one, not for one extra. + + The user being edited is already in the entry's users, so counting them + again refuses on a lock with exactly enough room -- naming a user count + that does not exist -- and the dialog that exists to repair the record + becomes one that can only fail. + """ + entry = MockConfigEntry( + domain=DOMAIN, + title="test", + data={CONF_LOCKS: [LOCK_1_ENTITY_ID]}, + subentries_data=[ + *user_subentries({1: {CONF_NAME: "Alice", CONF_PIN: "1111"}}), + unnumbered_user_subentry("Nomad", **{CONF_ENABLED: False}), + ], + unique_id="unnumbered-exact-fit", + ) + entry.add_to_hass(hass) + subentry = next(s for s in entry.subentries.values() if s.title == "Nomad") + + result = await hass.config_entries.subentries.async_init( + (entry.entry_id, SUBENTRY_TYPE_USER), + context={"source": "reconfigure", "subentry_id": subentry.subentry_id}, + ) + + # Two slots on the lock, Alice holds one, and two users all told. + with ( + patch.object(MockLCMLock, "async_get_max_slot", AsyncMock(return_value=2)), + _holding(), + ): + result = await hass.config_entries.subentries.async_configure( + result["flow_id"], + {CONF_NAME: "Nomad", CONF_ENABLED: True, CONF_PIN: "4242"}, + ) + + assert result["type"] == "abort" + assert entry.subentries[subentry.subentry_id].data[CONF_SLOT] == 2 + + +async def test_repairing_an_unnumbered_user_while_renaming_them_wastes_no_slot( + hass: HomeAssistant, mock_lock_config_entry +): + """ + A repair that also renames issues one number, from the bottom. + + Reconciling against a dict that still holds the old name puts the same + person in twice under two identities, so the new one is numbered around + the old one and the lowest free position is burned for good. + """ + entry = MockConfigEntry( + domain=DOMAIN, + title="test", + data={CONF_LOCKS: [LOCK_1_ENTITY_ID]}, + subentries_data=[unnumbered_user_subentry("Nomad", **{CONF_ENABLED: False})], + unique_id="unnumbered-renamed", + ) + entry.add_to_hass(hass) + subentry = next(iter(entry.subentries.values())) + + result = await hass.config_entries.subentries.async_init( + (entry.entry_id, SUBENTRY_TYPE_USER), + context={"source": "reconfigure", "subentry_id": subentry.subentry_id}, + ) + + with _holding(): + result = await hass.config_entries.subentries.async_configure( + result["flow_id"], + {CONF_NAME: "Zed", CONF_ENABLED: True, CONF_PIN: "4242"}, + ) + + assert result["type"] == "abort" + assert entry.subentries[subentry.subentry_id].title == "Zed" + assert entry.subentries[subentry.subentry_id].data[CONF_SLOT] == 1 + + async def test_editing_an_unnumbered_user_refuses_when_the_lock_cannot_be_read( hass: HomeAssistant, mock_lock_config_entry ): diff --git a/tests/test_init.py b/tests/test_init.py index 059532660..8b4dd41c5 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -2113,6 +2113,33 @@ async def _fail_lock_2(self: MockLCMLock, config_entry) -> None: await hass.config_entries.async_unload(entry.entry_id) +async def test_an_options_submission_that_changes_nothing_still_settles( + hass: HomeAssistant, mock_lock_config_entry, lock_code_manager_config_entry +) -> None: + """ + `options` is a staging area, and it is emptied even when nothing changed. + + The update listener returns early when the submission produces no diff, + and that was the one path out of it that skipped settling. What was left + behind read fine -- `options` holds what `data` holds -- right up until + the next write cleared it, which would take anything that lived only + there with it. + """ + entry = lock_code_manager_config_entry + locks = list(get_entry_config(entry).locks) + assert entry.options == {} + + result = await hass.config_entries.options.async_init(entry.entry_id) + result = await hass.config_entries.options.async_configure( + result["flow_id"], user_input={CONF_LOCKS: locks} + ) + await hass.async_block_till_done() + + assert result["type"] == "create_entry" + assert entry.options == {} + assert entry.data[CONF_LOCKS] == locks + + async def test_migration_v5_moves_an_existing_slot_device_to_its_user( hass: HomeAssistant, mock_lock_config_entry ) -> None: From 0e26b9adf0efe280a7d87a919642a51b58715f12 Mon Sep 17 00:00:00 2001 From: raman325 <7243222+raman325@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:50:46 -0400 Subject: [PATCH 2/2] fix: a batch hand-off applies to everybody in it One delete_user call is several entry writes -- a subentry removal per departing user -- and each wakes the update listener. The listener took the whole hand-off set on its first pass, so from the second user onward the credential the caller explicitly asked to leave programmed was wiped off every lock instead. Consumed per pair now. Also bind the reclaimed slot device to its user's subentry, so the pre-2026.8 upgrade path stops taking the deprecated implicit move, and drop the ZHA operation-source names the event removal left behind. Entire-Checkpoint: 7121ca62e8d5 --- .../lock_code_manager/__init__.py | 21 ++++++++--- .../lock_code_manager/providers/zha.py | 8 ---- tests/test_services.py | 37 +++++++++++++++++++ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/custom_components/lock_code_manager/__init__.py b/custom_components/lock_code_manager/__init__.py index a88cf7acd..46aeb36f6 100644 --- a/custom_components/lock_code_manager/__init__.py +++ b/custom_components/lock_code_manager/__init__.py @@ -128,7 +128,7 @@ MIN_PIN_LENGTH, generate_pin, ) -from .domain.queries import get_entry_config +from .domain.queries import get_entry_config, subentry_id_for_slot from .domain.references import async_notify_moved from .domain.services import ( async_add_users, @@ -1359,8 +1359,14 @@ def _is_ours(device: dr.DeviceEntry) -> bool: slot_num = parse_slot_unique_id(entry_id, entity.unique_id) if slot_num is None: continue + # Created under the user who holds the slot, like every other + # device this integration makes. Created bare, the first + # `async_add_entities` would pull it into that subentry implicitly -- + # which Home Assistant deprecates, and on the very upgrade path this + # function exists to serve. slot_device = dev_reg.async_get_or_create( config_entry_id=entry_id, + config_subentry_id=subentry_id_for_slot(config_entry, slot_num), **build_slot_device_info(config_entry, slot_num), ) _LOGGER.debug( @@ -1975,11 +1981,16 @@ async def _async_apply_entry_update( # anchored the slot; slot-only providers leave the default no-op in # place. This runs before ``locks_to_remove`` processing so providers # in ``runtime_data.locks`` are still usable. - # Drained, not read: a hand-off applies to the write that requested it, - # and leaving the pair behind would spare the next occupant of that slot - # number the cleanup it does need. + # Consumed pair by pair, not drained wholesale. A hand-off applies to the + # write that requested it and must not outlive it -- leaving a pair behind + # would spare the next occupant of that number the cleanup it does need -- + # but one `delete_user` call is several entry writes, one per departing + # user, and each wakes this listener. Taking the whole set on the first + # pass left every departure after the first one unprotected, and their + # credential was wiped off every lock despite the caller asking for it to + # be left programmed. retained_pairs = runtime_data.retained_pairs - runtime_data.retained_pairs = set() + runtime_data.retained_pairs = retained_pairs - diff.pairs_removed for lock_entity_id, slot_num in diff.pairs_removed: release_lock = runtime_data.locks.get(lock_entity_id) if release_lock is None: diff --git a/custom_components/lock_code_manager/providers/zha.py b/custom_components/lock_code_manager/providers/zha.py index dfe223d4b..f92b44e97 100644 --- a/custom_components/lock_code_manager/providers/zha.py +++ b/custom_components/lock_code_manager/providers/zha.py @@ -54,14 +54,6 @@ DoorLock.OperationEvent.ScheduleUnlock: False, } -OPERATION_SOURCE_NAMES: dict[int, str] = { - DoorLock.OperationEventSource.Keypad: "Keypad", - DoorLock.OperationEventSource.RF: "RF", - DoorLock.OperationEventSource.Manual: "Manual", - DoorLock.OperationEventSource.RFID: "RFID", - DoorLock.OperationEventSource.Indeterminate: "Unknown", -} - @dataclass(repr=False, eq=False) class ZHALock(BaseLock): diff --git a/tests/test_services.py b/tests/test_services.py index ef34c5532..24de33470 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -596,6 +596,43 @@ async def test_delete_user_service_can_hand_the_credential_over( assert not entry.runtime_data.retained_pairs +async def test_delete_user_hands_over_every_credential_not_just_the_first( + hass: HomeAssistant, + mock_lock_config_entry, + lock_code_manager_config_entry, +) -> None: + """ + A batch hand-off applies to everybody in it. + + One `delete_user` call is several entry writes -- a subentry removal per + departing user -- and each one wakes the update listener. The listener + drained the whole hand-off set on its first pass, so from the second user + onward the credential the caller explicitly asked to leave programmed was + wiped off every lock instead. + """ + entry = lock_code_manager_config_entry + for lock in entry.runtime_data.locks.values(): + lock.async_release_managed_slot = AsyncMock() + + await hass.services.async_call( + DOMAIN, + SERVICE_DELETE_USER, + { + "config_entry_id": entry.entry_id, + CONF_NAME: ["test1", "test2"], + ATTR_CLEAR_CREDENTIALS: False, + }, + blocking=True, + ) + await hass.async_block_till_done() + + config = get_entry_config(hass.config_entries.async_get_entry(entry.entry_id)) + assert not config.users + for lock in entry.runtime_data.locks.values(): + lock.async_release_managed_slot.assert_not_called() + assert not entry.runtime_data.retained_pairs + + async def test_delete_user_service_unknown_name( hass: HomeAssistant, mock_lock_config_entry,