From 4bfdc6104566cb12237b19d172e52d3a3644db11 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jun 2022 03:02:02 -0500 Subject: [PATCH] Fix rachio webhook not being unregistered on unload (#73795) --- homeassistant/components/rachio/__init__.py | 17 ++++--- .../components/rachio/binary_sensor.py | 11 +++-- homeassistant/components/rachio/const.py | 10 +++++ homeassistant/components/rachio/device.py | 45 +++++++++++-------- homeassistant/components/rachio/entity.py | 16 +++---- homeassistant/components/rachio/switch.py | 10 +++-- homeassistant/components/rachio/webhooks.py | 35 +++++++++++---- 7 files changed, 89 insertions(+), 55 deletions(-) diff --git a/homeassistant/components/rachio/__init__.py b/homeassistant/components/rachio/__init__.py index e75d7117d73..e0ac98b7546 100644 --- a/homeassistant/components/rachio/__init__.py +++ b/homeassistant/components/rachio/__init__.py @@ -17,6 +17,7 @@ from .device import RachioPerson from .webhooks import ( async_get_or_create_registered_webhook_id_and_url, async_register_webhook, + async_unregister_webhook, ) _LOGGER = logging.getLogger(__name__) @@ -28,8 +29,8 @@ CONFIG_SCHEMA = cv.removed(DOMAIN, raise_if_present=False) async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" - unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) - if unload_ok: + if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS): + async_unregister_webhook(hass, entry) hass.data[DOMAIN].pop(entry.entry_id) return unload_ok @@ -59,10 +60,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: # Get the URL of this server rachio.webhook_auth = secrets.token_hex() try: - ( - webhook_id, - webhook_url, - ) = await async_get_or_create_registered_webhook_id_and_url(hass, entry) + webhook_url = await async_get_or_create_registered_webhook_id_and_url( + hass, entry + ) except cloud.CloudNotConnected as exc: # User has an active cloud subscription, but the connection to the cloud is down raise ConfigEntryNotReady from exc @@ -92,9 +92,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ) # Enable platform - hass.data.setdefault(DOMAIN, {}) - hass.data[DOMAIN][entry.entry_id] = person - async_register_webhook(hass, webhook_id, entry.entry_id) + hass.data.setdefault(DOMAIN, {})[entry.entry_id] = person + async_register_webhook(hass, entry) hass.config_entries.async_setup_platforms(entry, PLATFORMS) diff --git a/homeassistant/components/rachio/binary_sensor.py b/homeassistant/components/rachio/binary_sensor.py index 1bfc3cc03ee..2fe99fb442e 100644 --- a/homeassistant/components/rachio/binary_sensor.py +++ b/homeassistant/components/rachio/binary_sensor.py @@ -9,6 +9,7 @@ from homeassistant.components.binary_sensor import ( from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.dispatcher import async_dispatcher_connect +from homeassistant.helpers.entity import Entity from homeassistant.helpers.entity_platform import AddEntitiesCallback from .const import ( @@ -21,6 +22,7 @@ from .const import ( SIGNAL_RACHIO_RAIN_SENSOR_UPDATE, STATUS_ONLINE, ) +from .device import RachioPerson from .entity import RachioDevice from .webhooks import ( SUBTYPE_COLD_REBOOT, @@ -41,12 +43,13 @@ async def async_setup_entry( """Set up the Rachio binary sensors.""" entities = await hass.async_add_executor_job(_create_entities, hass, config_entry) async_add_entities(entities) - _LOGGER.info("%d Rachio binary sensor(s) added", len(entities)) + _LOGGER.debug("%d Rachio binary sensor(s) added", len(entities)) -def _create_entities(hass, config_entry): - entities = [] - for controller in hass.data[DOMAIN_RACHIO][config_entry.entry_id].controllers: +def _create_entities(hass: HomeAssistant, config_entry: ConfigEntry) -> list[Entity]: + entities: list[Entity] = [] + person: RachioPerson = hass.data[DOMAIN_RACHIO][config_entry.entry_id] + for controller in person.controllers: entities.append(RachioControllerOnlineBinarySensor(controller)) entities.append(RachioRainSensor(controller)) return entities diff --git a/homeassistant/components/rachio/const.py b/homeassistant/components/rachio/const.py index 9dbf14e3907..92a57505a7c 100644 --- a/homeassistant/components/rachio/const.py +++ b/homeassistant/components/rachio/const.py @@ -67,3 +67,13 @@ SIGNAL_RACHIO_SCHEDULE_UPDATE = f"{SIGNAL_RACHIO_UPDATE}_schedule" CONF_WEBHOOK_ID = "webhook_id" CONF_CLOUDHOOK_URL = "cloudhook_url" + +# Webhook callbacks +LISTEN_EVENT_TYPES = [ + "DEVICE_STATUS_EVENT", + "ZONE_STATUS_EVENT", + "RAIN_DELAY_EVENT", + "RAIN_SENSOR_DETECTION_EVENT", + "SCHEDULE_STATUS_EVENT", +] +WEBHOOK_CONST_ID = "homeassistant.rachio:" diff --git a/homeassistant/components/rachio/device.py b/homeassistant/components/rachio/device.py index 911049883d9..5053fa01495 100644 --- a/homeassistant/components/rachio/device.py +++ b/homeassistant/components/rachio/device.py @@ -3,11 +3,14 @@ from __future__ import annotations from http import HTTPStatus import logging +from typing import Any +from rachiopy import Rachio import voluptuous as vol +from homeassistant.config_entries import ConfigEntry from homeassistant.const import EVENT_HOMEASSISTANT_STOP -from homeassistant.core import ServiceCall +from homeassistant.core import HomeAssistant, ServiceCall from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady from homeassistant.helpers import config_validation as cv @@ -26,12 +29,13 @@ from .const import ( KEY_STATUS, KEY_USERNAME, KEY_ZONES, + LISTEN_EVENT_TYPES, MODEL_GENERATION_1, SERVICE_PAUSE_WATERING, SERVICE_RESUME_WATERING, SERVICE_STOP_WATERING, + WEBHOOK_CONST_ID, ) -from .webhooks import LISTEN_EVENT_TYPES, WEBHOOK_CONST_ID _LOGGER = logging.getLogger(__name__) @@ -54,16 +58,16 @@ STOP_SERVICE_SCHEMA = vol.Schema({vol.Optional(ATTR_DEVICES): cv.string}) class RachioPerson: """Represent a Rachio user.""" - def __init__(self, rachio, config_entry): + def __init__(self, rachio: Rachio, config_entry: ConfigEntry) -> None: """Create an object from the provided API instance.""" # Use API token to get user ID self.rachio = rachio self.config_entry = config_entry self.username = None - self._id = None - self._controllers = [] + self._id: str | None = None + self._controllers: list[RachioIro] = [] - async def async_setup(self, hass): + async def async_setup(self, hass: HomeAssistant) -> None: """Create rachio devices and services.""" await hass.async_add_executor_job(self._setup, hass) can_pause = False @@ -121,7 +125,7 @@ class RachioPerson: schema=RESUME_SERVICE_SCHEMA, ) - def _setup(self, hass): + def _setup(self, hass: HomeAssistant) -> None: """Rachio device setup.""" rachio = self.rachio @@ -139,7 +143,7 @@ class RachioPerson: if int(data[0][KEY_STATUS]) != HTTPStatus.OK: raise ConfigEntryNotReady(f"API Error: {data}") self.username = data[1][KEY_USERNAME] - devices = data[1][KEY_DEVICES] + devices: list[dict[str, Any]] = data[1][KEY_DEVICES] for controller in devices: webhooks = rachio.notification.get_device_webhook(controller[KEY_ID])[1] # The API does not provide a way to tell if a controller is shared @@ -169,12 +173,12 @@ class RachioPerson: _LOGGER.info('Using Rachio API as user "%s"', self.username) @property - def user_id(self) -> str: + def user_id(self) -> str | None: """Get the user ID as defined by the Rachio API.""" return self._id @property - def controllers(self) -> list: + def controllers(self) -> list[RachioIro]: """Get a list of controllers managed by this account.""" return self._controllers @@ -186,7 +190,13 @@ class RachioPerson: class RachioIro: """Represent a Rachio Iro.""" - def __init__(self, hass, rachio, data, webhooks): + def __init__( + self, + hass: HomeAssistant, + rachio: Rachio, + data: dict[str, Any], + webhooks: list[dict[str, Any]], + ) -> None: """Initialize a Rachio device.""" self.hass = hass self.rachio = rachio @@ -199,10 +209,10 @@ class RachioIro: self._schedules = data[KEY_SCHEDULES] self._flex_schedules = data[KEY_FLEX_SCHEDULES] self._init_data = data - self._webhooks = webhooks + self._webhooks: list[dict[str, Any]] = webhooks _LOGGER.debug('%s has ID "%s"', self, self.controller_id) - def setup(self): + def setup(self) -> None: """Rachio Iro setup for webhooks.""" # Listen for all updates self._init_webhooks() @@ -226,7 +236,7 @@ class RachioIro: or webhook[KEY_ID] == current_webhook_id ): self.rachio.notification.delete(webhook[KEY_ID]) - self._webhooks = None + self._webhooks = [] _deinit_webhooks(None) @@ -306,9 +316,6 @@ class RachioIro: _LOGGER.debug("Resuming watering on %s", self) -def is_invalid_auth_code(http_status_code): +def is_invalid_auth_code(http_status_code: int) -> bool: """HTTP status codes that mean invalid auth.""" - if http_status_code in (HTTPStatus.UNAUTHORIZED, HTTPStatus.FORBIDDEN): - return True - - return False + return http_status_code in (HTTPStatus.UNAUTHORIZED, HTTPStatus.FORBIDDEN) diff --git a/homeassistant/components/rachio/entity.py b/homeassistant/components/rachio/entity.py index a95b6ffb557..1bb971e3e01 100644 --- a/homeassistant/components/rachio/entity.py +++ b/homeassistant/components/rachio/entity.py @@ -4,25 +4,19 @@ from homeassistant.helpers import device_registry from homeassistant.helpers.entity import DeviceInfo, Entity from .const import DEFAULT_NAME, DOMAIN +from .device import RachioIro class RachioDevice(Entity): """Base class for rachio devices.""" - def __init__(self, controller): + _attr_should_poll = False + + def __init__(self, controller: RachioIro) -> None: """Initialize a Rachio device.""" super().__init__() self._controller = controller - - @property - def should_poll(self) -> bool: - """Declare that this entity pushes its state to HA.""" - return False - - @property - def device_info(self) -> DeviceInfo: - """Return the device_info of the device.""" - return DeviceInfo( + self._attr_device_info = DeviceInfo( identifiers={ ( DOMAIN, diff --git a/homeassistant/components/rachio/switch.py b/homeassistant/components/rachio/switch.py index 55427741bf0..227e8beaec3 100644 --- a/homeassistant/components/rachio/switch.py +++ b/homeassistant/components/rachio/switch.py @@ -13,6 +13,7 @@ from homeassistant.core import HomeAssistant, ServiceCall, callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import config_validation as cv, entity_platform from homeassistant.helpers.dispatcher import async_dispatcher_connect +from homeassistant.helpers.entity import Entity from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.event import async_track_point_in_utc_time from homeassistant.util.dt import as_timestamp, now, parse_datetime, utc_from_timestamp @@ -52,6 +53,7 @@ from .const import ( SLOPE_SLIGHT, SLOPE_STEEP, ) +from .device import RachioPerson from .entity import RachioDevice from .webhooks import ( SUBTYPE_RAIN_DELAY_OFF, @@ -106,7 +108,7 @@ async def async_setup_entry( has_flex_sched = True async_add_entities(entities) - _LOGGER.info("%d Rachio switch(es) added", len(entities)) + _LOGGER.debug("%d Rachio switch(es) added", len(entities)) def start_multiple(service: ServiceCall) -> None: """Service to start multiple zones in sequence.""" @@ -154,9 +156,9 @@ async def async_setup_entry( ) -def _create_entities(hass, config_entry): - entities = [] - person = hass.data[DOMAIN_RACHIO][config_entry.entry_id] +def _create_entities(hass: HomeAssistant, config_entry: ConfigEntry) -> list[Entity]: + entities: list[Entity] = [] + person: RachioPerson = hass.data[DOMAIN_RACHIO][config_entry.entry_id] # Fetch the schedule once at startup # in order to avoid every zone doing it for controller in person.controllers: diff --git a/homeassistant/components/rachio/webhooks.py b/homeassistant/components/rachio/webhooks.py index 6ad396b76a1..5c2fbe5965f 100644 --- a/homeassistant/components/rachio/webhooks.py +++ b/homeassistant/components/rachio/webhooks.py @@ -1,9 +1,12 @@ """Webhooks used by rachio.""" +from __future__ import annotations + from aiohttp import web from homeassistant.components import cloud, webhook +from homeassistant.config_entries import ConfigEntry from homeassistant.const import URL_API -from homeassistant.core import callback +from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.dispatcher import async_dispatcher_send from .const import ( @@ -18,6 +21,7 @@ from .const import ( SIGNAL_RACHIO_SCHEDULE_UPDATE, SIGNAL_RACHIO_ZONE_UPDATE, ) +from .device import RachioPerson # Device webhook values TYPE_CONTROLLER_STATUS = "DEVICE_STATUS" @@ -79,16 +83,22 @@ SIGNAL_MAP = { @callback -def async_register_webhook(hass, webhook_id, entry_id): +def async_register_webhook(hass: HomeAssistant, entry: ConfigEntry) -> None: """Register a webhook.""" + webhook_id: str = entry.data[CONF_WEBHOOK_ID] - async def _async_handle_rachio_webhook(hass, webhook_id, request): + async def _async_handle_rachio_webhook( + hass: HomeAssistant, webhook_id: str, request: web.Request + ) -> web.Response: """Handle webhook calls from the server.""" + person: RachioPerson = hass.data[DOMAIN][entry.entry_id] data = await request.json() try: - auth = data.get(KEY_EXTERNAL_ID, "").split(":")[1] - assert auth == hass.data[DOMAIN][entry_id].rachio.webhook_auth + assert ( + data.get(KEY_EXTERNAL_ID, "").split(":")[1] + == person.rachio.webhook_auth + ) except (AssertionError, IndexError): return web.Response(status=web.HTTPForbidden.status_code) @@ -103,8 +113,17 @@ def async_register_webhook(hass, webhook_id, entry_id): ) -async def async_get_or_create_registered_webhook_id_and_url(hass, entry): - """Generate webhook ID.""" +@callback +def async_unregister_webhook(hass: HomeAssistant, entry: ConfigEntry) -> None: + """Unregister a webhook.""" + webhook_id: str = entry.data[CONF_WEBHOOK_ID] + webhook.async_unregister(hass, webhook_id) + + +async def async_get_or_create_registered_webhook_id_and_url( + hass: HomeAssistant, entry: ConfigEntry +) -> str: + """Generate webhook url.""" config = entry.data.copy() updated_config = False @@ -128,4 +147,4 @@ async def async_get_or_create_registered_webhook_id_and_url(hass, entry): if updated_config: hass.config_entries.async_update_entry(entry, data=config) - return webhook_id, webhook_url + return webhook_url