diff --git a/custom_components/nestquest/children.py b/custom_components/nestquest/children.py index e3439aa..4e1d56e 100644 --- a/custom_components/nestquest/children.py +++ b/custom_components/nestquest/children.py @@ -116,23 +116,27 @@ async def create_child( The display name is required, trimmed, and rejected when empty. A duplicate display name (case-insensitive) is allowed but logged - as a warning. The duplicate scan and the INSERT are deliberately - not one transaction: the warning is advisory, so serialising - creates to protect it would buy nothing. + as a warning. The scan and the INSERT run inside the + connection-scoped lock shared with the DAOs, so a concurrent + create of the same normalised name queues behind the first insert + and sees it — the duplicate still warns; it can never slip through + unlogged. """ name = _validate_text(display_name, "display_name", required=True) assert name is not None colour_value = _validate_text(colour, "colour") avatar_value = _validate_text(avatar_ref, "avatar_ref") order = _validate_sort_order(sort_order) - await _warn_on_duplicate_name(database, name) - return await ChildrenDao(database).create( - name, - _now_stamp(), - colour=colour_value, - avatar_ref=avatar_value, - sort_order=order, - ) + async with _connection_lock(database): + await _warn_on_duplicate_name(database, name) + created = await ChildrenDao(database).create( + name, + _now_stamp(), + colour=colour_value, + avatar_ref=avatar_value, + sort_order=order, + ) + return created async def edit_child( diff --git a/tests/test_children_registry.py b/tests/test_children_registry.py index 9cd6779..63f6d73 100644 --- a/tests/test_children_registry.py +++ b/tests/test_children_registry.py @@ -320,6 +320,68 @@ def test_edit_child_rename_to_own_name_casing_alone_warns_not( _with_db(tmp_path, "edit-rename-self.db")(_body) +def test_create_child_concurrent_same_name_still_warns(tmp_path) -> None: + """Racing creates of the same normalised name cannot slip through + unlogged: the scan and INSERT are serialized on the connection + lock, so the second create sees the first's committed row and + warns. Deterministic gate on task A's INSERT while task B queues + behind the lock. Without the lock, B's scan would run before A's + INSERT commits and no warning would fire.""" + async def _main(): + armed = {"active": False, "gated": False} + gate_open = asyncio.Event() + a_started = asyncio.Event() + hass = _gated_hass(armed, gate_open, a_started) + database = NestQuestDatabase(hass) + await database.open(tmp_path / "create-race.db") + try: + await apply_migrations(database) + logger = logging.getLogger( + "custom_components.nestquest.children" + ) + seen: list[logging.LogRecord] = [] + + class _Capture(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + seen.append(record) + + handler = _Capture() + logger.addHandler(handler) + try: + armed["active"] = True + task_a = asyncio.ensure_future( + create_child(database, "Ada") + ) + await a_started.wait() + armed["active"] = False + task_b = asyncio.ensure_future( + create_child(database, " ada ") + ) + await asyncio.sleep(0) + gate_open.set() + first, second = await asyncio.gather(task_a, task_b) + finally: + logger.removeHandler(handler) + + # Both creates land; the duplicate is allowed. + assert first.display_name == "Ada" + assert second.display_name == "ada" + names = [c.display_name for c in await list_children(database)] + assert names == ["Ada", "ada"] + # Exactly one warning: B's scan saw A's committed row. + warnings = [ + r for r in seen if "duplicates existing child" in r.getMessage() + ] + assert len(warnings) == 1, ( + "the queued duplicate create must warn exactly once" + ) + assert "Ada" in warnings[0].getMessage() + finally: + await database.close() + + _run(_main()) + + def test_edit_child_case_only_edit_colliding_with_other_child_warns( tmp_path, caplog ) -> None: