From 649efe26599aa4d2f449be1cf835998dbbdaea70 Mon Sep 17 00:00:00 2001 From: fdebrus <33791533+fdebrus@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:26:00 +0200 Subject: [PATCH] Close the Firestore clients when the Vistapool auth is done (#180866) --- .../components/vistapool/__init__.py | 4 ++ .../components/vistapool/config_flow.py | 6 +++ tests/components/vistapool/conftest.py | 3 ++ .../components/vistapool/test_config_flow.py | 46 ++++++++++++++++++ tests/components/vistapool/test_init.py | 48 +++++++++++++++++++ 5 files changed, 107 insertions(+) diff --git a/homeassistant/components/vistapool/__init__.py b/homeassistant/components/vistapool/__init__.py index f4210ea111fc..1e083996b7d9 100644 --- a/homeassistant/components/vistapool/__init__.py +++ b/homeassistant/components/vistapool/__init__.py @@ -66,6 +66,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: VistapoolConfigEntry) -> session = async_get_clientsession(hass) auth = AquariteAuth(session, user_config[CONF_USERNAME], user_config[CONF_PASSWORD]) + # Home Assistant runs these callbacks on a failed setup as well, so + # registering before authenticating releases the Firestore gRPC channels + # on every path out of this function. + entry.async_on_unload(auth.close) try: await auth.authenticate() except AuthenticationError as exc: diff --git a/homeassistant/components/vistapool/config_flow.py b/homeassistant/components/vistapool/config_flow.py index c10232bf3165..82c6f419ccf9 100644 --- a/homeassistant/components/vistapool/config_flow.py +++ b/homeassistant/components/vistapool/config_flow.py @@ -74,6 +74,10 @@ class VistapoolConfigFlow(ConfigFlow, domain=DOMAIN): CONF_PASSWORD: password, }, ) + finally: + # The entry is set up from the stored credentials with its own + # auth, so this one only lives for the length of the flow. + auth.close() return self.async_show_form( step_id="user", data_schema=AUTH_SCHEMA, errors=errors @@ -130,6 +134,8 @@ class VistapoolConfigFlow(ConfigFlow, domain=DOMAIN): return self.async_update_reload_and_abort( entry, data_updates={CONF_PASSWORD: password} ) + finally: + auth.close() return self.async_show_form( step_id=step_id, diff --git a/tests/components/vistapool/conftest.py b/tests/components/vistapool/conftest.py index 9d35cc16c46d..cc5ad5dd8e12 100644 --- a/tests/components/vistapool/conftest.py +++ b/tests/components/vistapool/conftest.py @@ -46,6 +46,9 @@ def mock_vistapool_auth() -> Generator[MagicMock]: """Mock `AquariteAuth` across the config flow and the integration setup.""" auth = MagicMock() auth.authenticate = AsyncMock() + # Home Assistant turns a truthy on-unload return value into a task, so this + # has to mirror the real method and return None. + auth.close = MagicMock(return_value=None) auth.user_id = MOCK_USER_ID auth.is_token_expiring = MagicMock(return_value=False) auth.calculate_sleep_duration = MagicMock(return_value=3600) diff --git a/tests/components/vistapool/test_config_flow.py b/tests/components/vistapool/test_config_flow.py index 4807d9cff2e5..5b38d7b77d3a 100644 --- a/tests/components/vistapool/test_config_flow.py +++ b/tests/components/vistapool/test_config_flow.py @@ -451,3 +451,49 @@ async def test_credential_update_error_paths( assert result["type"] is FlowResultType.ABORT assert result["reason"] == success_reason assert mock_setup_entry.call_count == 1 + + +@pytest.mark.parametrize( + "authenticate_error", + [ + pytest.param(None, id="success"), + pytest.param(AuthenticationError, id="invalid_auth"), + pytest.param(AquariteError, id="cannot_connect"), + ], +) +@pytest.mark.usefixtures("mock_setup_entry", "mock_vistapool_client") +async def test_user_step_closes_firestore_clients( + hass: HomeAssistant, + mock_vistapool_auth: MagicMock, + authenticate_error: type[Exception] | None, +) -> None: + """Test the user step releases the Firestore channels on every outcome.""" + mock_vistapool_auth.authenticate.side_effect = authenticate_error + + await _configure(hass) + + mock_vistapool_auth.close.assert_called_once() + + +@pytest.mark.parametrize(("flow_starter", "step_id", "success_reason"), _FLOW_PARAMS) +@pytest.mark.usefixtures("mock_setup_entry", "mock_vistapool_client") +async def test_credential_update_closes_firestore_clients( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_vistapool_auth: MagicMock, + flow_starter: Any, + step_id: str, + success_reason: str, +) -> None: + """Test reauth and reconfigure release the Firestore channels.""" + mock_config_entry.add_to_hass(hass) + + result = await flow_starter(mock_config_entry, hass) + assert result["step_id"] == step_id + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_PASSWORD: _NEW_PASSWORD} + ) + + assert result["reason"] == success_reason + mock_vistapool_auth.close.assert_called_once() diff --git a/tests/components/vistapool/test_init.py b/tests/components/vistapool/test_init.py index 31bd531a833d..24a64c5a689d 100644 --- a/tests/components/vistapool/test_init.py +++ b/tests/components/vistapool/test_init.py @@ -5,6 +5,7 @@ from __future__ import annotations from unittest.mock import AsyncMock, MagicMock from aioaquarite import AquariteError, AuthenticationError +import pytest from homeassistant.components.light import DOMAIN as LIGHT_DOMAIN from homeassistant.components.vistapool.const import DOMAIN @@ -409,3 +410,50 @@ async def test_unload_entry( await hass.async_block_till_done() assert mock_config_entry.state is ConfigEntryState.NOT_LOADED + + +async def test_unload_closes_firestore_clients( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_vistapool_auth: MagicMock, + mock_vistapool_client: AsyncMock, +) -> None: + """Test unloading releases the Firestore gRPC channels.""" + mock_config_entry.add_to_hass(hass) + + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + mock_vistapool_auth.close.assert_not_called() + + assert await hass.config_entries.async_unload(mock_config_entry.entry_id) + await hass.async_block_till_done() + + mock_vistapool_auth.close.assert_called_once() + + +@pytest.mark.parametrize( + ("owner", "method", "exception"), + [ + pytest.param("auth", "authenticate", AuthenticationError, id="auth_rejected"), + pytest.param("auth", "authenticate", AquariteError, id="auth_unreachable"), + pytest.param("client", "get_pools", AquariteError, id="pools_unreachable"), + ], +) +async def test_failed_setup_closes_firestore_clients( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_vistapool_auth: MagicMock, + mock_vistapool_client: AsyncMock, + owner: str, + method: str, + exception: type[Exception], +) -> None: + """Test a setup that never completes still releases the Firestore channels.""" + mocks = {"auth": mock_vistapool_auth, "client": mock_vistapool_client} + getattr(mocks[owner], method).side_effect = exception + mock_config_entry.add_to_hass(hass) + + assert not await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + mock_vistapool_auth.close.assert_called_once()