Improve local calendar based on local todo review feedback (#103483)

* Improve local calendar based on local todo review feedback

* Revert fakestore change to diagnose timeout

* Revert init changes

* Revert and add assert

homeassistant/components/local_calendar/__init__.py

@@ -7,9 +7,10 @@ from pathlib import Path
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.util import slugify
 
-from .const import CONF_CALENDAR_NAME, DOMAIN
+from .const import CONF_CALENDAR_NAME, CONF_STORAGE_KEY, DOMAIN
 from .store import LocalCalendarStore
 
 _LOGGER = logging.getLogger(__name__)
@@ -24,9 +25,23 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up Local Calendar from a config entry."""
     hass.data.setdefault(DOMAIN, {})
 
-    key = slugify(entry.data[CONF_CALENDAR_NAME])
-    path = Path(hass.config.path(STORAGE_PATH.format(key=key)))
-    hass.data[DOMAIN][entry.entry_id] = LocalCalendarStore(hass, path)
+    if CONF_STORAGE_KEY not in entry.data:
+        hass.config_entries.async_update_entry(
+            entry,
+            data={
+                **entry.data,
+                CONF_STORAGE_KEY: slugify(entry.data[CONF_CALENDAR_NAME]),
+            },
+        )
+
+    path = Path(hass.config.path(STORAGE_PATH.format(key=entry.data[CONF_STORAGE_KEY])))
+    store = LocalCalendarStore(hass, path)
+    try:
+        await store.async_load()
+    except OSError as err:
+        raise ConfigEntryNotReady("Failed to load file {path}: {err}") from err
+
+    hass.data[DOMAIN][entry.entry_id] = store
 
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 

homeassistant/components/local_calendar/config_flow.py

@@ -7,8 +7,9 @@ import voluptuous as vol
 
 from homeassistant import config_entries
 from homeassistant.data_entry_flow import FlowResult
+from homeassistant.util import slugify
 
-from .const import CONF_CALENDAR_NAME, DOMAIN
+from .const import CONF_CALENDAR_NAME, CONF_STORAGE_KEY, DOMAIN
 
 STEP_USER_DATA_SCHEMA = vol.Schema(
     {
@@ -31,6 +32,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
                 step_id="user", data_schema=STEP_USER_DATA_SCHEMA
             )
 
+        key = slugify(user_input[CONF_CALENDAR_NAME])
+        self._async_abort_entries_match({CONF_STORAGE_KEY: key})
+        user_input[CONF_STORAGE_KEY] = key
         return self.async_create_entry(
             title=user_input[CONF_CALENDAR_NAME], data=user_input
         )

homeassistant/components/local_calendar/const.py

@@ -3,3 +3,4 @@
 DOMAIN = "local_calendar"
 
 CONF_CALENDAR_NAME = "calendar_name"
+CONF_STORAGE_KEY = "storage_key"

tests/components/local_calendar/conftest.py

@@ -4,7 +4,7 @@ from collections.abc import Awaitable, Callable, Generator
 from http import HTTPStatus
 from pathlib import Path
 from typing import Any
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 import urllib
 
 from aiohttp import ClientWebSocketResponse
@@ -20,24 +20,31 @@ from tests.typing import ClientSessionGenerator, WebSocketGenerator
 
 CALENDAR_NAME = "Light Schedule"
 FRIENDLY_NAME = "Light schedule"
+STORAGE_KEY = "light_schedule"
 TEST_ENTITY = "calendar.light_schedule"
 
 
 class FakeStore(LocalCalendarStore):
     """Mock storage implementation."""
 
-    def __init__(self, hass: HomeAssistant, path: Path, ics_content: str) -> None:
+    def __init__(
+        self, hass: HomeAssistant, path: Path, ics_content: str, read_side_effect: Any
+    ) -> None:
         """Initialize FakeStore."""
         super().__init__(hass, path)
-        self._content = ics_content
+        mock_path = self._mock_path = Mock()
+        mock_path.exists = self._mock_exists
+        mock_path.read_text = Mock()
+        mock_path.read_text.return_value = ics_content
+        mock_path.read_text.side_effect = read_side_effect
+        mock_path.write_text = self._mock_write_text
+        super().__init__(hass, mock_path)
 
-    def _load(self) -> str:
-        """Read from calendar storage."""
-        return self._content
+    def _mock_exists(self) -> bool:
+        return self._mock_path.read_text.return_value is not None
 
-    def _store(self, ics_content: str) -> None:
-        """Persist the calendar storage."""
-        self._content = ics_content
+    def _mock_write_text(self, content: str) -> None:
+        self._mock_path.read_text.return_value = content
 
 
 @pytest.fixture(name="ics_content", autouse=True)
@@ -46,15 +53,23 @@ def mock_ics_content() -> str:
     return ""
 
 
+@pytest.fixture(name="store_read_side_effect")
+def mock_store_read_side_effect() -> Any | None:
+    """Fixture to raise errors from the FakeStore."""
+    return None
+
+
 @pytest.fixture(name="store", autouse=True)
-def mock_store(ics_content: str) -> Generator[None, None, None]:
+def mock_store(
+    ics_content: str, store_read_side_effect: Any | None
+) -> Generator[None, None, None]:
     """Test cleanup, remove any media storage persisted during the test."""
 
     stores: dict[Path, FakeStore] = {}
 
     def new_store(hass: HomeAssistant, path: Path) -> FakeStore:
         if path not in stores:
-            stores[path] = FakeStore(hass, path, ics_content)
+            stores[path] = FakeStore(hass, path, ics_content, store_read_side_effect)
         return stores[path]
 
     with patch(

tests/components/local_calendar/test_config_flow.py

@@ -2,10 +2,16 @@
 from unittest.mock import patch
 
 from homeassistant import config_entries
-from homeassistant.components.local_calendar.const import CONF_CALENDAR_NAME, DOMAIN
+from homeassistant.components.local_calendar.const import (
+    CONF_CALENDAR_NAME,
+    CONF_STORAGE_KEY,
+    DOMAIN,
+)
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 
+from tests.common import MockConfigEntry
+
 
 async def test_form(hass: HomeAssistant) -> None:
     """Test we get the form."""
@@ -31,5 +37,30 @@ async def test_form(hass: HomeAssistant) -> None:
     assert result2["title"] == "My Calendar"
     assert result2["data"] == {
         CONF_CALENDAR_NAME: "My Calendar",
+        CONF_STORAGE_KEY: "my_calendar",
     }
     assert len(mock_setup_entry.mock_calls) == 1
+
+
+async def test_duplicate_name(
+    hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry
+) -> None:
+    """Test two calendars cannot be added with the same name."""
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    assert result["type"] == FlowResultType.FORM
+    assert not result.get("errors")
+
+    result2 = await hass.config_entries.flow.async_configure(
+        result["flow_id"],
+        {
+            # Pick a name that has the same slugify value as an existing config entry
+            CONF_CALENDAR_NAME: "light schedule",
+        },
+    )
+    await hass.async_block_till_done()
+
+    assert result2["type"] == FlowResultType.ABORT
+    assert result2["reason"] == "already_configured"

tests/components/local_calendar/test_init.py

@@ -2,11 +2,36 @@
 
 from unittest.mock import patch
 
+import pytest
+
+from homeassistant.config_entries import ConfigEntryState
 from homeassistant.core import HomeAssistant
 
+from .conftest import TEST_ENTITY
+
 from tests.common import MockConfigEntry
 
 
+async def test_load_unload(
+    hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry
+) -> None:
+    """Test loading and unloading a config entry."""
+
+    assert config_entry.state == ConfigEntryState.LOADED
+
+    state = hass.states.get(TEST_ENTITY)
+    assert state
+    assert state.state == "off"
+
+    await hass.config_entries.async_unload(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert config_entry.state == ConfigEntryState.NOT_LOADED
+    state = hass.states.get(TEST_ENTITY)
+    assert state
+    assert state.state == "unavailable"
+
+
 async def test_remove_config_entry(
     hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry
 ) -> None:
@@ -16,3 +41,20 @@ async def test_remove_config_entry(
         assert await hass.config_entries.async_remove(config_entry.entry_id)
         await hass.async_block_till_done()
         unlink_mock.assert_called_once()
+
+
+@pytest.mark.parametrize(
+    ("store_read_side_effect"),
+    [
+        (OSError("read error")),
+    ],
+)
+async def test_load_failure(
+    hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry
+) -> None:
+    """Test failures loading the store."""
+
+    assert config_entry.state == ConfigEntryState.SETUP_RETRY
+
+    state = hass.states.get(TEST_ENTITY)
+    assert not state