diff --git a/custom_components/inpost_air/__init__.py b/custom_components/inpost_air/__init__.py index fbb9652..19b8bab 100644 --- a/custom_components/inpost_air/__init__.py +++ b/custom_components/inpost_air/__init__.py @@ -1,7 +1,7 @@ """The InPost Air integration.""" from __future__ import annotations -from dataclasses import dataclass +from dataclasses import asdict, dataclass import logging from dacite import from_dict @@ -15,7 +15,7 @@ from custom_components.inpost_air.models import ParcelLocker from custom_components.inpost_air.utils import get_device_info, get_parcel_locker_url -from .api import InPostAirPoint, InPostApi +from .api import InPostAirApiClientError, InPostAirPoint, InPostApi _LOGGER = logging.getLogger(__name__) @@ -35,34 +35,80 @@ class InPostAirData: PLATFORMS: list[Platform] = [Platform.SENSOR] +def get_configured_point(entry: InPostAirConfiEntry) -> InPostAirPoint: + """Read parcel locker data stored in the config entry.""" + entry_data = entry.data.get("parcel_locker") + + if entry_data is None: + raise ConfigEntryError( + "Config entry does not contain any parcel locker data. " + "Please remove the integration and set it up again." + ) + + if isinstance(entry_data, InPostAirPoint): + return entry_data + + try: + return from_dict(InPostAirPoint, entry_data) + except Exception as ex: + raise ConfigEntryError( + f"Parcel locker data stored in the config entry is invalid: {ex}" + ) from ex + + +async def resolve_parcel_locker( + hass: HomeAssistant, + entry: InPostAirConfiEntry, + api_client: InPostApi, + point: InPostAirPoint, +) -> tuple[InPostAirPoint, str]: + """Find the parcel locker ID, refreshing outdated config entry data if needed.""" + refreshed_point = None + + try: + if (locker_id := await api_client.find_parcel_locker_id(point)) is not None: + return point, locker_id + + _LOGGER.info( + "Refetching data of parcel locker %s - the one stored in the config entry might be outdated", + point.n, + ) + refreshed_point = await api_client.refresh_parcel_locker(point.n) + + if refreshed_point is not None and refreshed_point != point: + locker_id = await api_client.find_parcel_locker_id(refreshed_point) + except InPostAirApiClientError as err: + # Transient problem on InPost side - let Home Assistant retry the setup. + raise ConfigEntryNotReady(f"Error communicating with InPost: {err}") from err + + if locker_id is None: + raise ConfigEntryNotReady( + f"Could not find air quality data of parcel locker {point.n} on {get_parcel_locker_url(point)}" + ) + + hass.config_entries.async_update_entry( + entry, + data={**entry.data, "parcel_locker": asdict(refreshed_point)}, + ) + + return refreshed_point, locker_id + + async def async_setup_entry(hass: HomeAssistant, entry: InPostAirConfiEntry) -> bool: """Set up InPost Air from a config entry.""" api_client = InPostApi(hass) - entry_data = entry.data.get("parcel_locker") + point = get_configured_point(entry) - if ( - point := None - if entry_data is None - else entry_data - if isinstance(entry_data, InPostAirPoint) - else from_dict(InPostAirPoint, entry_data) - ) is None: - return False - - if (parcel_locker_id := await api_client.find_parcel_locker_id(point)) is None: - return False + point, parcel_locker_id = await resolve_parcel_locker( + hass, entry, api_client, point + ) parcel_locker = ParcelLocker(point.n, parcel_locker_id) coordinator = InPostAirDataCoordinator(hass, api_client, parcel_locker) entry.runtime_data = InPostAirData(parcel_locker, coordinator) - try: - await coordinator.async_config_entry_first_refresh() - except ConfigEntryNotReady as ex: - if "Air sensors are not available" in str(ex): - raise ConfigEntryError(ex) - raise ex + await coordinator.async_config_entry_first_refresh() device_registry = dr.async_get(hass) device_registry.async_get_or_create( @@ -92,12 +138,19 @@ async def async_migrate_entry(hass: HomeAssistant, config_entry: InPostAirConfiE if config_entry.version > 2: # This means the user has downgraded from a future version + _LOGGER.error( + "Cannot migrate %s from version %s - it was created by a newer version of the integration", + config_entry.title, + config_entry.version, + ) return False if config_entry.version == 1: hass.config_entries.async_update_entry( config_entry, - data={"parcel_locker": from_dict(InPostAirPoint, config_entry.data)}, + data={ + "parcel_locker": asdict(from_dict(InPostAirPoint, config_entry.data)) + }, version=2, ) diff --git a/custom_components/inpost_air/api.py b/custom_components/inpost_air/api.py index 2e58e5b..b086eea 100644 --- a/custom_components/inpost_air/api.py +++ b/custom_components/inpost_air/api.py @@ -2,9 +2,11 @@ import asyncio from dataclasses import dataclass +import json import logging import re -from aiohttp import ClientResponse, ClientResponseError +from typing import Any +from aiohttp import ClientResponseError from dacite import from_dict from homeassistant.core import HomeAssistant from homeassistant.helpers.aiohttp_client import async_create_clientsession @@ -13,6 +15,24 @@ _LOGGER = logging.getLogger(__name__) +REQUEST_TIMEOUT = 30 + +AIR_DATA_PATTERN = re.compile( + r"data-shipx-url=\"/shipx-point-data/(.*?)/(.*?)/air_index_level\"" +) + + +@dataclass +class ApiResponse: + """API response with an already read body.""" + + url: str + body: str + + def json(self) -> Any: + """Parse the body as JSON.""" + return json.loads(self.body) + @dataclass class ParcelLockerListResponse: @@ -43,10 +63,15 @@ async def _request( url: str, headers: dict | None = None, raise_client_response_error: bool = False, - ) -> ClientResponse: - """Get information from the API.""" + ) -> ApiResponse: + """Get information from the API. + + The body is read inside the timeout on purpose - `session.request` + returns as soon as the response headers arrive, so reading the body + outside of it would leave that part of the request without any limit. + """ try: - async with asyncio.timeout(30): + async with asyncio.timeout(REQUEST_TIMEOUT): response = await self.session.request( method=method, url=url, @@ -54,23 +79,25 @@ async def _request( ) response.raise_for_status() - return response + return ApiResponse(url=str(response.url), body=await response.text()) except TimeoutError as e: - _LOGGER.warning("Request timed out") + _LOGGER.warning("Request to %s timed out", url) raise InPostAirApiClientError("Request timed out") from e except ClientResponseError as e: if raise_client_response_error: raise - raise InPostAirApiClientError("Something really wrong happened!") from e + _LOGGER.warning("Request to %s failed with status %s", url, e.status) + raise InPostAirApiClientError( + f"Request to {url} failed with status {e.status}" + ) from e except Exception as exception: # pylint: disable=broad-except + _LOGGER.warning("Request to %s failed: %s", url, exception) raise InPostAirApiClientError( "Something really wrong happened!" ) from exception - async def _search_easypack24_locker( - self, locker_code: str - ) -> InPostAirPoint | None: + async def _search_easypack24_locker(self, locker_code: str) -> dict | None: """Find info about given parcel locker.""" if not locker_code or locker_code == "": return None @@ -79,7 +106,7 @@ async def _search_easypack24_locker( method="get", url="https://api-shipx-pl.easypack24.net/v1/points/" + locker_code, ) - resp = await response.json() + resp = response.json() error = resp.get("error") if error: @@ -120,11 +147,7 @@ async def search_parcel_locker(self, locker_code: str) -> InPostAirPoint | None: method="get", url="https://inpost.pl/sites/default/files/points.json" ) parcel_locker = next( - ( - x - for x in (await response.json()).get("items") - if x.get("n") == locker_code - ), + (x for x in response.json().get("items") if x.get("n") == locker_code), None, ) @@ -133,27 +156,54 @@ async def search_parcel_locker(self, locker_code: str) -> InPostAirPoint | None: return from_dict(InPostAirPoint, parcel_locker) if parcel_locker else None + async def refresh_parcel_locker(self, locker_code: str) -> InPostAirPoint | None: + """Get current data of an already known parcel locker. + + Data stored in a config entry gets outdated when InPost changes details + of a parcel locker, so it has to be refetched. The single point endpoint + is tried first - the full list is a few megabytes of JSON. + """ + parcel_locker = await self._search_easypack24_locker(locker_code) + + if parcel_locker is None: + return await self.search_parcel_locker(locker_code) + + return from_dict(InPostAirPoint, parcel_locker) + async def get_parcel_lockers_list(self) -> list[InPostAirPoint]: """Get parcel lockers list.""" response = await self._request( method="get", url="https://inpost.pl/sites/default/files/points.json" ) - response_data = from_dict(ParcelLockerListResponse, await response.json()) + response_data = from_dict(ParcelLockerListResponse, response.json()) return response_data.items async def find_parcel_locker_id(self, point: InPostAirPoint) -> str | None: """Find parcel locker ID by its code.""" - response = await self._request( - method="get", - url=get_parcel_locker_url(point), - ) - match = re.search( - r"data-shipx-url=\"/shipx-point-data/(.*?)/(.*?)/air_index_level\"", - await response.text(), - ) + url = get_parcel_locker_url(point) + response = await self._request(method="get", url=url) + + if response.url.rstrip("/") != url.rstrip("/"): + _LOGGER.warning( + "Page of parcel locker %s (%s) redirected to %s - data stored for that parcel locker is outdated", + point.n, + url, + response.url, + ) + return None + + match = AIR_DATA_PATTERN.search(response.body) + + if match is None: + _LOGGER.warning( + "Could not find air quality data of parcel locker %s on %s", + point.n, + url, + ) + return None - return None if match is None else match.group(1) + return match.group(1) async def get_parcel_locker_air_data( self, locker_code: str, locker_id: str @@ -171,11 +221,11 @@ async def get_parcel_locker_air_data( raise InPostAirApiClientSensorsMissingError( "Air sensors are not available" ) from e - raise InPostAirApiClientError("Something really wrong happened!") from e - except: - raise + raise InPostAirApiClientError( + f"Request for air data of parcel locker {locker_code} failed with status {e.status}" + ) from e - return from_dict(ParcelLockerAirDataResponse, await response.json()) + return from_dict(ParcelLockerAirDataResponse, response.json()) class InPostAirApiClientError(Exception): diff --git a/custom_components/inpost_air/config_flow.py b/custom_components/inpost_air/config_flow.py index 0836856..99c12cc 100644 --- a/custom_components/inpost_air/config_flow.py +++ b/custom_components/inpost_air/config_flow.py @@ -1,7 +1,7 @@ """Config flow for InPost Air integration.""" from __future__ import annotations -from dataclasses import dataclass +from dataclasses import asdict, dataclass import logging from typing import Any @@ -48,7 +48,13 @@ async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> InPostAir try: parcel_locker_id = await api_client.find_parcel_locker_id(parcel_locker) + + if parcel_locker_id is None: + raise ParcelLockerWithoutAirData + await api_client.get_parcel_locker_air_data(parcel_locker.n, parcel_locker_id) + except ParcelLockerWithoutAirData: + raise except Exception as exc: raise ParcelLockerWithoutAirData from exc @@ -79,7 +85,7 @@ async def async_step_user( else: return self.async_create_entry( title=f"Parcel locker {parcel_locker.n}", - data={"parcel_locker": parcel_locker}, + data={"parcel_locker": asdict(parcel_locker)}, ) parcel_lockers = [ diff --git a/custom_components/inpost_air/coordinator.py b/custom_components/inpost_air/coordinator.py index eddf4aa..3f9df01 100644 --- a/custom_components/inpost_air/coordinator.py +++ b/custom_components/inpost_air/coordinator.py @@ -7,10 +7,15 @@ import re from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryError from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from .models import ParcelLocker -from .api import InPostAirApiClientError, InPostApi +from .api import ( + InPostAirApiClientError, + InPostAirApiClientSensorsMissingError, + InPostApi, +) from .const import Entities _LOGGER = logging.getLogger(__name__) @@ -90,7 +95,11 @@ async def _async_update_data(self): return { x.name: x for line in data.air_sensors if (x := create_value(line)) } + except InPostAirApiClientSensorsMissingError as err: + # Nothing to poll for - retrying will not help, so let the config + # entry fail permanently instead of ending up in a retry loop. + raise ConfigEntryError(err) from err except InPostAirApiClientError as err: raise UpdateFailed(err) from err except Exception as err: - raise UpdateFailed("Error communicating with API") from err + raise UpdateFailed(f"Error communicating with API: {err}") from err diff --git a/tests/test_api.py b/tests/test_api.py index b6315e5..2829882 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,7 +1,15 @@ +import asyncio +import os +from unittest.mock import patch + import pytest import pytest_socket -import os -from custom_components.inpost_air.api import InPostApi + +from custom_components.inpost_air.api import ( + ApiResponse, + InPostAirApiClientError, + InPostApi, +) from custom_components.inpost_air.models import ( InPostAirPoint, InPostAirPointCoordinates, @@ -66,3 +74,106 @@ async def test_find_parcel_locker_id(hass, _allow_inpost_requests): async def test_air_data(hass, _allow_inpost_requests): response = await InPostApi(hass).get_parcel_locker_air_data("AJE01BAPP", "56311") assert response is not None + + +class _StallingResponse: + """Response that never finishes sending its body.""" + + def __init__(self, url: str) -> None: + self.url = url + self.status = 200 + + def raise_for_status(self) -> None: + pass + + async def text(self) -> str: + await asyncio.sleep(10) + return "" + + +class _StallingSession: + """Session returning responses with a stalled body.""" + + async def request(self, method: str, url: str, headers=None) -> _StallingResponse: + return _StallingResponse(url) + + +PAGE_WITH_AIR_DATA = ( + '
' +) + +mocked_point = InPostAirPoint( + "AJE01BAPP", + 1, + "Market Dino", + "", + "", + "006", + "Andrzejewo", + "andrzejewo", + "Warszawska", + "mazowieckie", + "07-305", + "62A", + "24/7", + "[]", + InPostAirPointCoordinates(52.83679, 22.20968), + 0, + 1, +) + +mocked_point_url = ( + "https://inpost.pl/paczkomat-andrzejewo-aje01bapp-warszawska-paczkomaty-mazowieckie" +) + + +async def test_request_times_out_while_reading_body(hass): + """Reading the body is covered by the request timeout.""" + api = InPostApi(hass) + api.session = _StallingSession() + + with patch("custom_components.inpost_air.api.REQUEST_TIMEOUT", 0.05): + with pytest.raises(InPostAirApiClientError, match="Request timed out"): + await api._request(method="get", url="https://inpost.pl/whatever") + + +async def test_find_parcel_locker_id_from_page(hass): + """Parcel locker ID is scraped from its page.""" + api = InPostApi(hass) + + with patch.object( + InPostApi, + "_request", + return_value=ApiResponse(url=mocked_point_url, body=PAGE_WITH_AIR_DATA), + ): + assert await api.find_parcel_locker_id(mocked_point) == "56311" + + +async def test_find_parcel_locker_id_without_air_data(hass, caplog): + """Missing air data on the page is reported instead of failing silently.""" + api = InPostApi(hass) + + with patch.object( + InPostApi, + "_request", + return_value=ApiResponse(url=mocked_point_url, body=""), + ): + assert await api.find_parcel_locker_id(mocked_point) is None + + assert "Could not find air quality data" in caplog.text + + +async def test_find_parcel_locker_id_when_page_redirects(hass, caplog): + """Redirect away from the parcel locker page means outdated entry data.""" + api = InPostApi(hass) + + with patch.object( + InPostApi, + "_request", + return_value=ApiResponse( + url="https://inpost.pl/znajdz-paczkomat", body="" + ), + ): + assert await api.find_parcel_locker_id(mocked_point) is None + + assert "redirected to" in caplog.text diff --git a/tests/test_init.py b/tests/test_init.py new file mode 100644 index 0000000..02387fa --- /dev/null +++ b/tests/test_init.py @@ -0,0 +1,221 @@ +"""Define tests for setting up a config entry.""" + +from dataclasses import asdict +from unittest.mock import patch + +import pytest +from homeassistant.config_entries import ConfigEntryState +from pytest_homeassistant_custom_component.common import MockConfigEntry + +from custom_components.inpost_air.api import ( + InPostAirApiClientError, + InPostAirApiClientSensorsMissingError, + InPostApi, + ParcelLockerAirDataResponse, +) +from custom_components.inpost_air.const import DOMAIN +from custom_components.inpost_air.models import ( + InPostAirPoint, + InPostAirPointCoordinates, +) + +mocked_point = InPostAirPoint( + "AJE01BAPP", + 1, + "Market Dino", + "", + "", + "006", + "Andrzejewo", + "andrzejewo", + "Warszawska", + "mazowieckie", + "07-305", + "62A", + "24/7", + "[]", + InPostAirPointCoordinates(52.83679, 22.20968), + 0, + 1, +) + +mocked_air_data = ParcelLockerAirDataResponse( + message="", + air_index_level="good", + air_sensors=["PM25:10.0:20.0", "TEMPERATURE:21.5:", "HUMIDITY:50.0:"], +) + + +def create_entry(hass, data=None) -> MockConfigEntry: + """Add a config entry to hass.""" + entry = MockConfigEntry( + domain=DOMAIN, + version=2, + minor_version=1, + unique_id=mocked_point.n, + title=f"Parcel locker {mocked_point.n}", + data={"parcel_locker": asdict(mocked_point)} if data is None else data, + ) + entry.add_to_hass(hass) + return entry + + +async def setup_entry(hass, entry: MockConfigEntry) -> None: + """Run setup of the given config entry.""" + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + +async def test_setup_entry(hass): + """Entry is loaded when parcel locker data can be fetched.""" + entry = create_entry(hass) + + with ( + patch.object(InPostApi, "find_parcel_locker_id", return_value="56311"), + patch.object( + InPostApi, "get_parcel_locker_air_data", return_value=mocked_air_data + ), + ): + await setup_entry(hass, entry) + + assert entry.state is ConfigEntryState.LOADED + assert entry.runtime_data.parcel_locker.locker_id == "56311" + assert hass.states.get("sensor.parcel_locker_aje01bapp_temperature") is not None + + +async def test_setup_entry_without_parcel_locker_data(hass, caplog): + """Entry with no parcel locker data fails with a logged error, not silently.""" + entry = create_entry(hass, data={}) + + await setup_entry(hass, entry) + + assert entry.state is ConfigEntryState.SETUP_ERROR + assert entry.reason is not None and entry.reason != "Unknown error" + assert "does not contain any parcel locker data" in caplog.text + + +async def test_setup_entry_with_invalid_parcel_locker_data(hass, caplog): + """Entry with broken parcel locker data fails with a logged error.""" + entry = create_entry(hass, data={"parcel_locker": {"n": "AJE01BAPP"}}) + + await setup_entry(hass, entry) + + assert entry.state is ConfigEntryState.SETUP_ERROR + assert "is invalid" in caplog.text + + +async def test_setup_entry_retries_when_parcel_locker_id_not_found(hass): + """Missing parcel locker ID schedules a retry instead of killing the entry.""" + entry = create_entry(hass) + + with ( + patch.object(InPostApi, "find_parcel_locker_id", return_value=None), + patch.object(InPostApi, "refresh_parcel_locker", return_value=mocked_point), + ): + await setup_entry(hass, entry) + + assert entry.state is ConfigEntryState.SETUP_RETRY + assert entry.reason is not None and "AJE01BAPP" in entry.reason + + +async def test_setup_entry_retries_on_api_error(hass): + """API errors schedule a retry instead of killing the entry.""" + entry = create_entry(hass) + + with patch.object( + InPostApi, + "find_parcel_locker_id", + side_effect=InPostAirApiClientError("Request timed out"), + ): + await setup_entry(hass, entry) + + assert entry.state is ConfigEntryState.SETUP_RETRY + + +async def test_setup_entry_refreshes_outdated_parcel_locker_data(hass): + """Outdated data stored in the entry is refetched and saved.""" + entry = create_entry(hass) + refreshed_point = InPostAirPoint(**{**asdict(mocked_point), "e": "Nowa Warszawska"}) + refreshed_point.l = InPostAirPointCoordinates(**asdict(mocked_point).get("l")) + + def find_parcel_locker_id(_self, point: InPostAirPoint): + return "56311" if point.e == "Nowa Warszawska" else None + + with ( + patch.object( + InPostApi, + "find_parcel_locker_id", + autospec=True, + side_effect=find_parcel_locker_id, + ), + patch.object(InPostApi, "refresh_parcel_locker", return_value=refreshed_point), + patch.object( + InPostApi, "get_parcel_locker_air_data", return_value=mocked_air_data + ), + ): + await setup_entry(hass, entry) + + assert entry.state is ConfigEntryState.LOADED + assert entry.data["parcel_locker"] == asdict(refreshed_point) + + +async def test_setup_entry_without_air_sensors(hass): + """Parcel locker without air sensors fails permanently, without retrying.""" + entry = create_entry(hass) + + with ( + patch.object(InPostApi, "find_parcel_locker_id", return_value="56311"), + patch.object( + InPostApi, + "get_parcel_locker_air_data", + side_effect=InPostAirApiClientSensorsMissingError( + "Air sensors are not available" + ), + ), + ): + await setup_entry(hass, entry) + + assert entry.state is ConfigEntryState.SETUP_ERROR + assert entry.reason == "Air sensors are not available" + + +@pytest.mark.parametrize("expected_lingering_timers", [True]) +async def test_unload_entry(hass): + """Entry can be unloaded.""" + entry = create_entry(hass) + + with ( + patch.object(InPostApi, "find_parcel_locker_id", return_value="56311"), + patch.object( + InPostApi, "get_parcel_locker_air_data", return_value=mocked_air_data + ), + ): + await setup_entry(hass, entry) + + assert await hass.config_entries.async_unload(entry.entry_id) + await hass.async_block_till_done() + + assert entry.state is ConfigEntryState.NOT_LOADED + + +async def test_migrate_entry_from_version_1(hass): + """Version 1 entry data is migrated into a serializable dict.""" + entry = MockConfigEntry( + domain=DOMAIN, + version=1, + unique_id=mocked_point.n, + data=asdict(mocked_point), + ) + entry.add_to_hass(hass) + + with ( + patch.object(InPostApi, "find_parcel_locker_id", return_value="56311"), + patch.object( + InPostApi, "get_parcel_locker_air_data", return_value=mocked_air_data + ), + ): + await setup_entry(hass, entry) + + assert entry.version == 2 + assert entry.data == {"parcel_locker": asdict(mocked_point)} + assert entry.state is ConfigEntryState.LOADED