Skip to content

Commit 25a6778

Browse files
authored
Fix user store not loaded on restart (home-assistant#157616)
1 parent f564b8c commit 25a6778

File tree

2 files changed

+109
-6
lines changed

2 files changed

+109
-6
lines changed

homeassistant/components/frontend/storage.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import asyncio
56
from collections.abc import Callable, Coroutine
67
from functools import wraps
78
from typing import Any
@@ -15,7 +16,9 @@
1516
from homeassistant.helpers.storage import Store
1617
from homeassistant.util.hass_dict import HassKey
1718

18-
DATA_STORAGE: HassKey[dict[str, UserStore]] = HassKey("frontend_storage")
19+
DATA_STORAGE: HassKey[dict[str, asyncio.Future[UserStore]]] = HassKey(
20+
"frontend_storage"
21+
)
1922
DATA_SYSTEM_STORAGE: HassKey[SystemStore] = HassKey("frontend_system_storage")
2023
STORAGE_VERSION_USER_DATA = 1
2124
STORAGE_VERSION_SYSTEM_DATA = 1
@@ -34,11 +37,18 @@ async def async_setup_frontend_storage(hass: HomeAssistant) -> None:
3437
async def async_user_store(hass: HomeAssistant, user_id: str) -> UserStore:
3538
"""Access a user store."""
3639
stores = hass.data.setdefault(DATA_STORAGE, {})
37-
if (store := stores.get(user_id)) is None:
38-
store = stores[user_id] = UserStore(hass, user_id)
39-
await store.async_load()
40-
41-
return store
40+
if (future := stores.get(user_id)) is None:
41+
future = stores[user_id] = hass.loop.create_future()
42+
store = UserStore(hass, user_id)
43+
try:
44+
await store.async_load()
45+
except BaseException as ex:
46+
del stores[user_id]
47+
future.set_exception(ex)
48+
raise
49+
future.set_result(store)
50+
51+
return await future
4252

4353

4454
class UserStore:

tests/components/frontend/test_storage.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
"""The tests for frontend storage."""
22

3+
import asyncio
34
from typing import Any
5+
from unittest.mock import patch
46

57
import pytest
68

79
from homeassistant.components.frontend import DOMAIN
10+
from homeassistant.components.frontend.storage import async_user_store
811
from homeassistant.core import HomeAssistant
12+
from homeassistant.helpers.storage import Store
913
from homeassistant.setup import async_setup_component
1014

1115
from tests.common import MockUser
@@ -572,3 +576,92 @@ async def test_set_system_data_requires_admin(
572576
assert not res["success"], res
573577
assert res["error"]["code"] == "unauthorized"
574578
assert res["error"]["message"] == "Unauthorized"
579+
580+
581+
async def test_user_store_concurrent_access(
582+
hass: HomeAssistant,
583+
hass_admin_user: MockUser,
584+
hass_storage: dict[str, Any],
585+
) -> None:
586+
"""Test that concurrent access to user store returns loaded data."""
587+
storage_key = f"{DOMAIN}.user_data_{hass_admin_user.id}"
588+
hass_storage[storage_key] = {
589+
"version": 1,
590+
"data": {"test-key": "test-value"},
591+
}
592+
593+
load_count = 0
594+
original_async_load = Store.async_load
595+
596+
async def slow_async_load(self: Store) -> Any:
597+
"""Simulate slow loading to trigger race condition."""
598+
nonlocal load_count
599+
load_count += 1
600+
await asyncio.sleep(0) # Yield to allow other coroutines to run
601+
return await original_async_load(self)
602+
603+
with patch.object(Store, "async_load", slow_async_load):
604+
# Request the same user store concurrently
605+
results = await asyncio.gather(
606+
async_user_store(hass, hass_admin_user.id),
607+
async_user_store(hass, hass_admin_user.id),
608+
async_user_store(hass, hass_admin_user.id),
609+
)
610+
611+
# All results should be the same store instance with loaded data
612+
assert results[0] is results[1] is results[2]
613+
assert results[0].data == {"test-key": "test-value"}
614+
# Store should only be loaded once due to Future synchronization
615+
assert load_count == 1
616+
617+
618+
async def test_user_store_load_error(
619+
hass: HomeAssistant,
620+
hass_admin_user: MockUser,
621+
) -> None:
622+
"""Test that load errors are propagated and allow retry."""
623+
624+
async def failing_async_load(self: Store) -> Any:
625+
"""Simulate a load failure."""
626+
raise OSError("Storage read error")
627+
628+
with (
629+
patch.object(Store, "async_load", failing_async_load),
630+
pytest.raises(OSError, match="Storage read error"),
631+
):
632+
await async_user_store(hass, hass_admin_user.id)
633+
634+
# After error, the future should be removed, allowing retry
635+
# This time without the patch, it should work (empty store)
636+
store = await async_user_store(hass, hass_admin_user.id)
637+
assert store.data == {}
638+
639+
640+
async def test_user_store_concurrent_load_error(
641+
hass: HomeAssistant,
642+
hass_admin_user: MockUser,
643+
) -> None:
644+
"""Test that concurrent callers all receive the same error."""
645+
646+
async def failing_async_load(self: Store) -> Any:
647+
"""Simulate a slow load failure."""
648+
await asyncio.sleep(0) # Yield to allow other coroutines to run
649+
raise OSError("Storage read error")
650+
651+
with patch.object(Store, "async_load", failing_async_load):
652+
results = await asyncio.gather(
653+
async_user_store(hass, hass_admin_user.id),
654+
async_user_store(hass, hass_admin_user.id),
655+
async_user_store(hass, hass_admin_user.id),
656+
return_exceptions=True,
657+
)
658+
659+
# All callers should receive the same OSError
660+
assert len(results) == 3
661+
for result in results:
662+
assert isinstance(result, OSError)
663+
assert str(result) == "Storage read error"
664+
665+
# After error, retry should work
666+
store = await async_user_store(hass, hass_admin_user.id)
667+
assert store.data == {}

0 commit comments

Comments
 (0)