From 32754f4864e1f57f0e673b2c0ee3c9dd7b048e1f Mon Sep 17 00:00:00 2001 From: Will Beeching Date: Mon, 20 Apr 2026 13:44:24 +0100 Subject: [PATCH] Isolate aiohttp session and storage per config entry Multiple BoilerJuice accounts were overwriting each other because the coordinator used Home Assistant's shared aiohttp session (and therefore a shared cookie jar), and the consumption store fell back to a single "default" bucket when no tank id was configured. Each coordinator now owns a private ClientSession with its own CookieJar, and consumption data is keyed by config entry id with a migration path from legacy tank-id keys. Also: - Close the private session on entry unload and after config-flow validation so temporary coordinators don't leak connections. - Add optional device_id/entry_id target selectors to the reset_consumption and set_consumption services so they no longer fan out to every configured account. - Remove a stray reset_consumption registration in sensor.py that shadowed the properly-schemaed service with a single-coordinator one, and make async_unload_services drop set_consumption too. --- custom_components/boilerjuice/__init__.py | 83 ++++++- custom_components/boilerjuice/config_flow.py | 39 ++-- custom_components/boilerjuice/coordinator.py | 218 ++++++++++--------- custom_components/boilerjuice/manifest.json | 2 +- custom_components/boilerjuice/sensor.py | 14 +- custom_components/boilerjuice/services.yaml | 30 ++- 6 files changed, 240 insertions(+), 146 deletions(-) diff --git a/custom_components/boilerjuice/__init__.py b/custom_components/boilerjuice/__init__.py index 7686678..4403526 100644 --- a/custom_components/boilerjuice/__init__.py +++ b/custom_components/boilerjuice/__init__.py @@ -9,12 +9,14 @@ from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, Platform from homeassistant.core import HomeAssistant, ServiceCall, callback -from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady +from homeassistant.exceptions import ( + ConfigEntryAuthFailed, + ConfigEntryNotReady, + HomeAssistantError, +) from homeassistant.helpers import config_validation as cv -from homeassistant.helpers import service -from homeassistant.helpers.device_registry import DeviceEntryType, DeviceRegistry +from homeassistant.helpers.device_registry import DeviceEntryType from homeassistant.helpers.device_registry import async_get as async_get_device_registry -from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.typing import ConfigType from .const import CONF_KWH_PER_LITRE, CONF_TANK_ID, DEFAULT_KWH_PER_LITRE, DOMAIN @@ -40,16 +42,76 @@ # Service schemas SERVICE_RESET_CONSUMPTION = "reset_consumption" SERVICE_SET_CONSUMPTION = "set_consumption" -RESET_CONSUMPTION_SCHEMA = vol.Schema({}) + +# Optional target selectors so users with multiple BoilerJuice accounts can +# address a single tank instead of fanning the service call out to every +# configured entry. HA injects device_id/area_id/entity_id/label_id when the +# user picks a target from the UI, so allow extras through the voluptuous +# schema rather than trying to enumerate every key. +RESET_CONSUMPTION_SCHEMA = vol.Schema( + { + vol.Optional("entry_id"): vol.Any(cv.string, [cv.string]), + }, + extra=vol.ALLOW_EXTRA, +) SET_CONSUMPTION_SCHEMA = vol.Schema( { vol.Required("liters"): cv.positive_float, vol.Optional("daily"): cv.positive_float, - } + vol.Optional("entry_id"): vol.Any(cv.string, [cv.string]), + }, + extra=vol.ALLOW_EXTRA, ) +def _resolve_target_coordinators(hass: HomeAssistant, call: ServiceCall) -> list: + """Return the coordinators a service call should operate on. + + Honours the optional ``device_id`` / ``entry_id`` fields plus any target + selector the user picks in the UI. Falls back to all configured entries + for backwards compatibility. + """ + entry_ids: set[str] = set() + + def _collect(value): + if value is None: + return + if isinstance(value, str): + entry_ids.add(value) + else: + entry_ids.update(value) + + _collect(call.data.get("entry_id")) + + device_registry = async_get_device_registry(hass) + device_ids = call.data.get("device_id") + if isinstance(device_ids, str): + device_ids = [device_ids] + for device_id in device_ids or []: + device = device_registry.async_get(device_id) + if device is None: + raise HomeAssistantError(f"Unknown device_id {device_id}") + for entry_id in device.config_entries: + if entry_id in hass.data.get(DOMAIN, {}): + entry_ids.add(entry_id) + + coordinators_by_entry = hass.data.get(DOMAIN, {}) + + if not entry_ids: + return list(coordinators_by_entry.values()) + + resolved = [] + for entry_id in entry_ids: + coordinator = coordinators_by_entry.get(entry_id) + if coordinator is None: + raise HomeAssistantError( + f"No BoilerJuice integration loaded for entry_id {entry_id}" + ) + resolved.append(coordinator) + return resolved + + @callback def async_setup_services(hass: HomeAssistant) -> None: """Set up the BoilerJuice services.""" @@ -58,7 +120,7 @@ def async_setup_services(hass: HomeAssistant) -> None: async def async_handle_reset_consumption(call: ServiceCall) -> None: """Handle the service call to reset consumption.""" - for entry_id, coordinator in hass.data[DOMAIN].items(): + for coordinator in _resolve_target_coordinators(hass, call): coordinator.reset_consumption() await coordinator.async_request_refresh() @@ -75,7 +137,7 @@ async def async_handle_set_consumption(call: ServiceCall) -> None: total_consumption = data["liters"] daily_consumption = data.get("daily") - for entry_id, coordinator in hass.data[DOMAIN].items(): + for coordinator in _resolve_target_coordinators(hass, call): if coordinator.data: # Set the consumption values coordinator._total_consumption_usable_liters = total_consumption @@ -125,6 +187,7 @@ def async_unload_services(hass: HomeAssistant) -> None: return hass.services.async_remove(DOMAIN, SERVICE_RESET_CONSUMPTION) + hass.services.async_remove(DOMAIN, SERVICE_SET_CONSUMPTION) async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: @@ -187,7 +250,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS): - hass.data[DOMAIN].pop(entry.entry_id) + coordinator = hass.data[DOMAIN].pop(entry.entry_id, None) + if coordinator is not None: + await coordinator.async_close() if not hass.data[DOMAIN]: async_unload_services(hass) hass.data.pop(DOMAIN) diff --git a/custom_components/boilerjuice/config_flow.py b/custom_components/boilerjuice/config_flow.py index 5740e67..1409752 100644 --- a/custom_components/boilerjuice/config_flow.py +++ b/custom_components/boilerjuice/config_flow.py @@ -35,23 +35,28 @@ async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> dict[str, coordinator = BoilerJuiceDataUpdateCoordinator(hass, data) try: - await coordinator.async_refresh() - except Exception as err: - if "Invalid credentials" in str(err): - raise InvalidAuth from err - if "Failed to login" in str(err): - raise CannotConnect from err - raise err - - # Get the model name if available, fallback to tank name, then default - title = "BoilerJuice Tank" - if coordinator.data: - if coordinator.data.get("model"): - title = coordinator.data["model"] - elif coordinator.data.get("name"): - title = coordinator.data["name"] - - return {"title": title} + try: + await coordinator.async_refresh() + except Exception as err: + if "Invalid credentials" in str(err): + raise InvalidAuth from err + if "Failed to login" in str(err): + raise CannotConnect from err + raise err + + # Get the model name if available, fallback to tank name, then default + title = "BoilerJuice Tank" + if coordinator.data: + if coordinator.data.get("model"): + title = coordinator.data["model"] + elif coordinator.data.get("name"): + title = coordinator.data["name"] + + return {"title": title} + finally: + # Each coordinator owns a private aiohttp session; always close the + # throwaway one used for config-flow validation. + await coordinator.async_close() class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): diff --git a/custom_components/boilerjuice/coordinator.py b/custom_components/boilerjuice/coordinator.py index 80bfdae..968d22d 100644 --- a/custom_components/boilerjuice/coordinator.py +++ b/custom_components/boilerjuice/coordinator.py @@ -11,10 +11,11 @@ from datetime import datetime, timedelta from typing import Any, Dict, List, Tuple, Union +import aiohttp from bs4 import BeautifulSoup from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant -from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.aiohttp_client import async_create_clientsession from homeassistant.helpers.storage import Store from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed @@ -67,7 +68,10 @@ def __init__( update_interval=SCAN_INTERVAL, ) self._config = config - self._session = None + # Each coordinator owns its own aiohttp session with a dedicated cookie + # jar. Using the shared HA session caused two BoilerJuice accounts to + # overwrite each other's login cookies (see GitHub issue #3). + self._session: aiohttp.ClientSession | None = None self._previous_usable_volume = None self._previous_total_level = None self._total_consumption_usable_liters = 0.0 @@ -82,7 +86,11 @@ def __init__( # Add seasonal tracking self._consumption_history_with_dates: List[Tuple[datetime, float]] = [] - # Set up storage + # Set up storage. Keyed per config entry so multiple accounts don't + # collide on the legacy "default" bucket when no tank id is provided. + self._entry_id: str | None = ( + config.entry_id if isinstance(config, ConfigEntry) else None + ) self._store = Store(hass, STORAGE_VERSION, STORAGE_KEY) self._tank_id = self._get_config_value_optional(CONF_TANK_ID) @@ -144,103 +152,84 @@ def days_until_empty(self) -> float | None: return None - async def _load_consumption_data(self) -> None: - """Load consumption data from storage.""" - if self._consumption_data_loaded: - return + def _apply_stored_data(self, source: str, data: dict) -> None: + """Hydrate coordinator state from a stored-data blob.""" + self._total_consumption_usable_liters = data.get( + "total_consumption_liters", 0.0 + ) + self._total_consumption_usable_kwh = data.get("total_consumption_kwh", 0.0) + self._daily_consumption_usable_liters = data.get( + "daily_consumption_liters", 0.0 + ) + self._daily_consumption_history = data.get("consumption_history", []) - stored_data = await self._store.async_load() + history_with_dates = data.get("consumption_history_with_dates", []) + self._consumption_history_with_dates = [ + (datetime.fromisoformat(dt), cons) for dt, cons in history_with_dates + ] - if stored_data: - _LOGGER.debug("Loading stored consumption data: %s", stored_data) + last_update_str = data.get("last_update") + if last_update_str: + try: + self._last_update = datetime.fromisoformat(last_update_str) + except (ValueError, TypeError): + self._last_update = None - # If we have a tank ID, try to get data specific to this tank - if self._tank_id and self._tank_id in stored_data: - tank_data = stored_data[self._tank_id] + self._previous_usable_volume = data.get("reference_volume") + self._previous_total_level = data.get("reference_level") - self._total_consumption_usable_liters = tank_data.get( - "total_consumption_liters", 0.0 - ) - self._total_consumption_usable_kwh = tank_data.get( - "total_consumption_kwh", 0.0 - ) - self._daily_consumption_usable_liters = tank_data.get( - "daily_consumption_liters", 0.0 - ) - self._daily_consumption_history = tank_data.get( - "consumption_history", [] - ) + _LOGGER.info( + "Loaded stored consumption data from %s: total=%s L, daily=%s L/day", + source, + self._total_consumption_usable_liters, + self._daily_consumption_usable_liters, + ) - # Load consumption history with dates - history_with_dates = tank_data.get("consumption_history_with_dates", []) - self._consumption_history_with_dates = [ - (datetime.fromisoformat(dt), cons) - for dt, cons in history_with_dates - ] - - # Convert stored string timestamp to datetime if exists - last_update_str = tank_data.get("last_update") - if last_update_str: - try: - self._last_update = datetime.fromisoformat(last_update_str) - except (ValueError, TypeError): - self._last_update = None - - # Get reference values if available - self._previous_usable_volume = tank_data.get("reference_volume") - self._previous_total_level = tank_data.get("reference_level") + async def _load_consumption_data(self) -> None: + """Load consumption data from storage. + + Data is keyed by config entry id so that multiple BoilerJuice accounts + never share state. For backwards compatibility we fall back to the + legacy tank-id key (and, only when a tank id is configured, the + "default" key) so existing users don't lose consumption history on + upgrade. + """ + if self._consumption_data_loaded: + return - _LOGGER.info( - "Loaded stored consumption data for tank %s: total=%s L, daily=%s L/day", - self._tank_id, - self._total_consumption_usable_liters, - self._daily_consumption_usable_liters, - ) - elif not self._tank_id and stored_data.get("default"): - # Fallback to default if no tank ID - default_data = stored_data["default"] + stored_data = await self._store.async_load() or {} - self._total_consumption_usable_liters = default_data.get( - "total_consumption_liters", 0.0 - ) - self._total_consumption_usable_kwh = default_data.get( - "total_consumption_kwh", 0.0 - ) - self._daily_consumption_usable_liters = default_data.get( - "daily_consumption_liters", 0.0 - ) - self._daily_consumption_history = default_data.get( - "consumption_history", [] - ) + if stored_data: + _LOGGER.debug("Loading stored consumption data: %s", stored_data) - # Load consumption history with dates - history_with_dates = default_data.get( - "consumption_history_with_dates", [] - ) - self._consumption_history_with_dates = [ - (datetime.fromisoformat(dt), cons) - for dt, cons in history_with_dates - ] - - # Convert stored string timestamp to datetime if exists - last_update_str = default_data.get("last_update") - if last_update_str: - try: - self._last_update = datetime.fromisoformat(last_update_str) - except (ValueError, TypeError): - self._last_update = None - - # Get reference values if available - self._previous_usable_volume = default_data.get("reference_volume") - self._previous_total_level = default_data.get("reference_level") + loaded = False - _LOGGER.info( - "Loaded default stored consumption data: total=%s L, daily=%s L/day", - self._total_consumption_usable_liters, - self._daily_consumption_usable_liters, - ) + if self._entry_id and self._entry_id in stored_data: + self._apply_stored_data( + f"entry {self._entry_id}", stored_data[self._entry_id] + ) + loaded = True + elif self._tank_id and self._tank_id in stored_data: + # Legacy per-tank key – migrate into the entry-keyed slot. + self._apply_stored_data( + f"legacy tank {self._tank_id}", stored_data[self._tank_id] + ) + loaded = True + elif self._tank_id and stored_data.get("default"): + # Only migrate the legacy "default" bucket when we can be sure it + # belongs to this entry (i.e. a tank id is explicitly configured). + # With multiple untagged accounts the default bucket is ambiguous, + # so we leave it untouched rather than risk cross-contamination. + self._apply_stored_data("legacy default", stored_data["default"]) + loaded = True + + if not loaded and stored_data: + _LOGGER.debug( + "No stored consumption data for entry %s / tank %s; starting fresh", + self._entry_id, + self._tank_id, + ) - # Mark data as loaded self._consumption_data_loaded = True _LOGGER.debug("Consumption data loading completed") @@ -335,16 +324,22 @@ def _calculate_seasonal_stats(self) -> Dict[str, Any]: return seasonal_data async def _save_consumption_data(self) -> None: - """Save consumption data to storage.""" - tank_id = self.data.get("id") if self.data else self._tank_id + """Save consumption data to storage. + + Saved under this entry's id so multiple accounts never collide. As a + transitional step we also drop any legacy key that refers to the same + tank, keeping storage tidy after migration. + """ + # Prefer the config entry id (stable, unique per instance). Fall back + # to the scraped tank id, then the configured tank id, then "default" + # for coordinators created outside a config entry (the config flow's + # validation path does not persist state anyway). + storage_key = self._entry_id + if not storage_key: + storage_key = (self.data or {}).get("id") or self._tank_id or "default" - if not tank_id: - tank_id = "default" - - # Load existing data first stored_data = await self._store.async_load() or {} - # Update with current values tank_data = { "total_consumption_liters": self._total_consumption_usable_liters, "total_consumption_kwh": self._total_consumption_usable_kwh, @@ -352,22 +347,35 @@ async def _save_consumption_data(self) -> None: "reference_volume": self._previous_usable_volume, "reference_level": self._previous_total_level, "consumption_history": self._daily_consumption_history, - # Store consumption history with dates as list of [timestamp, consumption] pairs "consumption_history_with_dates": [ [dt.isoformat(), cons] for dt, cons in self._consumption_history_with_dates ], } - # Store last update time as ISO format string if self._last_update: tank_data["last_update"] = self._last_update.isoformat() - stored_data[tank_id] = tank_data + stored_data[storage_key] = tank_data + + # Clean up legacy tank-id keyed entries that are now owned by this + # config entry. The shared "default" bucket is left alone because we + # can't safely tell whether it still belongs to another entry that + # hasn't yet migrated. + if self._entry_id: + scraped_tank_id = (self.data or {}).get("id") + for legacy_key in {self._tank_id, scraped_tank_id}: + if legacy_key and legacy_key != self._entry_id: + stored_data.pop(legacy_key, None) - # Save to storage await self._store.async_save(stored_data) - _LOGGER.debug("Saved consumption data for tank %s: %s", tank_id, tank_data) + _LOGGER.debug("Saved consumption data under %s: %s", storage_key, tank_data) + + async def async_close(self) -> None: + """Close the private aiohttp session (call on unload).""" + if self._session is not None: + await self._session.close() + self._session = None def reset_consumption(self) -> None: """Reset the consumption counter.""" @@ -480,7 +488,11 @@ async def _async_update_data(self): await self._load_consumption_data() if self._session is None: - self._session = async_get_clientsession(self.hass) + # Dedicated cookie jar so concurrent BoilerJuice accounts never + # share login state with each other or with other HA integrations. + self._session = async_create_clientsession( + self.hass, cookie_jar=aiohttp.CookieJar() + ) try: # First, get the login page to get the CSRF token diff --git a/custom_components/boilerjuice/manifest.json b/custom_components/boilerjuice/manifest.json index 773fdef..0851104 100644 --- a/custom_components/boilerjuice/manifest.json +++ b/custom_components/boilerjuice/manifest.json @@ -13,6 +13,6 @@ "quality_scale": "silver", "requirements": ["aiohttp>=3.8.0", "beautifulsoup4>=4.12.0"], "ssdp": [], - "version": "1.1.11", + "version": "1.2.0", "zeroconf": [] } diff --git a/custom_components/boilerjuice/sensor.py b/custom_components/boilerjuice/sensor.py index 228c878..065582b 100644 --- a/custom_components/boilerjuice/sensor.py +++ b/custom_components/boilerjuice/sensor.py @@ -15,7 +15,7 @@ ) from homeassistant.config_entries import ConfigEntry from homeassistant.const import PERCENTAGE, UnitOfEnergy, UnitOfVolume -from homeassistant.core import HomeAssistant, ServiceCall +from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import DeviceEntryType from homeassistant.helpers.entity import DeviceInfo, EntityCategory from homeassistant.helpers.entity_platform import AddEntitiesCallback @@ -50,18 +50,6 @@ async def async_setup_entry( """Set up BoilerJuice sensors from a config entry.""" coordinator = hass.data[DOMAIN][entry.entry_id] - # Add the consumption reset service - async def handle_reset_consumption(call: ServiceCall) -> None: - """Handle the service call to reset consumption.""" - coordinator.reset_consumption() - await coordinator.async_request_refresh() - - hass.services.async_register( - DOMAIN, - "reset_consumption", - handle_reset_consumption, - ) - async_add_entities( [ # Simplified sensors - BoilerJuice now only provides one oil level (not separate total/usable) diff --git a/custom_components/boilerjuice/services.yaml b/custom_components/boilerjuice/services.yaml index 804c02c..faa40b7 100644 --- a/custom_components/boilerjuice/services.yaml +++ b/custom_components/boilerjuice/services.yaml @@ -1,10 +1,28 @@ reset_consumption: name: Reset Consumption - description: Reset the consumption counters to zero. + description: >- + Reset the consumption counters to zero. If no target is provided the + service is applied to every configured BoilerJuice account. + target: + device: + integration: boilerjuice + fields: + entry_id: + name: Config entry ID + description: Optional config entry ID to target a specific account. + required: false + example: 01HZ...abcd + selector: + text: set_consumption: name: Set Consumption - description: Manually set the consumption values for the oil tank. + description: >- + Manually set the consumption values for the oil tank. If no target is + provided the service is applied to every configured BoilerJuice account. + target: + device: + integration: boilerjuice fields: liters: name: Total Consumption @@ -28,4 +46,10 @@ set_consumption: step: 0.1 unit_of_measurement: "L/day" mode: box - + entry_id: + name: Config entry ID + description: Optional config entry ID to target a specific account. + required: false + example: 01HZ...abcd + selector: + text: