Catch malformed coordinates in here_travel_time (#69023)

* Catch malformed coordinates in here_travel_time

* Add testcase for malformed entity_id state

* Replace type ignore with None check

* Directly raise InvalidCoordinatesException

homeassistant/components/here_travel_time/__init__.py

@@ -6,9 +6,11 @@ import logging
 
 import async_timeout
 from herepy import NoRouteFoundError, RouteMode, RoutingApi, RoutingResponse
+import voluptuous as vol
 
 from homeassistant.const import ATTR_ATTRIBUTION, CONF_UNIT_SYSTEM_IMPERIAL, Platform
 from homeassistant.core import HomeAssistant
+import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.location import find_coordinates
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 from homeassistant.util import dt
@@ -64,32 +66,13 @@ class HereTravelTimeDataUpdateCoordinator(DataUpdateCoordinator):
 
     def _update(self) -> HERERoutingData | None:
         """Get the latest data from the HERE Routing API."""
-        if self.config.origin_entity_id is not None:
+        try:
-            origin = find_coordinates(self.hass, self.config.origin_entity_id)
+            origin, destination, arrival, departure = self._prepare_parameters()
-        else:
-            origin = self.config.origin
-
-        if self.config.destination_entity_id is not None:
-            destination = find_coordinates(self.hass, self.config.destination_entity_id)
-        else:
-            destination = self.config.destination
-        if destination is not None and origin is not None:
-            here_formatted_destination = destination.split(",")
-            here_formatted_origin = origin.split(",")
-            arrival: str | None = None
-            departure: str | None = None
-            if self.config.arrival is not None:
-                arrival = convert_time_to_isodate(self.config.arrival)
-            if self.config.departure is not None:
-                departure = convert_time_to_isodate(self.config.departure)
-
-            if arrival is None and departure is None:
-                departure = "now"
 
             _LOGGER.debug(
                 "Requesting route for origin: %s, destination: %s, route_mode: %s, mode: %s, traffic_mode: %s, arrival: %s, departure: %s",
-                here_formatted_origin,
+                origin,
-                here_formatted_destination,
+                destination,
                 RouteMode[self.config.route_mode],
                 RouteMode[self.config.travel_mode],
                 RouteMode[TRAFFIC_MODE_ENABLED],
@@ -98,8 +81,8 @@ class HereTravelTimeDataUpdateCoordinator(DataUpdateCoordinator):
             )
 
             response: RoutingResponse = self._api.public_transport_timetable(
-                here_formatted_origin,
+                origin,
-                here_formatted_destination,
+                destination,
                 True,
                 [
                     RouteMode[self.config.route_mode],
@@ -137,14 +120,60 @@ class HereTravelTimeDataUpdateCoordinator(DataUpdateCoordinator):
                     ATTR_DURATION_IN_TRAFFIC: traffic_time / 60,
                     ATTR_DISTANCE: distance,
                     ATTR_ROUTE: response.route_short,
-                    ATTR_ORIGIN: ",".join(here_formatted_origin),
+                    ATTR_ORIGIN: ",".join(origin),
-                    ATTR_DESTINATION: ",".join(here_formatted_destination),
+                    ATTR_DESTINATION: ",".join(destination),
                     ATTR_ORIGIN_NAME: waypoint[0]["mappedRoadName"],
                     ATTR_DESTINATION_NAME: waypoint[1]["mappedRoadName"],
                 }
             )
+        except InvalidCoordinatesException as ex:
+            _LOGGER.error("Could not call HERE api: %s", ex)
         return None
 
+    def _prepare_parameters(
+        self,
+    ) -> tuple[list[str], list[str], str | None, str | None]:
+        """Prepare parameters for the HERE api."""
+
+        if self.config.origin_entity_id is not None:
+            origin = find_coordinates(self.hass, self.config.origin_entity_id)
+        else:
+            origin = self.config.origin
+
+        if self.config.destination_entity_id is not None:
+            destination = find_coordinates(self.hass, self.config.destination_entity_id)
+        else:
+            destination = self.config.destination
+        if destination is None:
+            raise InvalidCoordinatesException("Destination must be configured")
+        try:
+            here_formatted_destination = destination.split(",")
+            vol.Schema(cv.gps(here_formatted_destination))
+        except (vol.Invalid) as ex:
+            raise InvalidCoordinatesException(
+                f"{destination} are not valid coordinates"
+            ) from ex
+        if origin is None:
+            raise InvalidCoordinatesException("Origin must be configured")
+        try:
+            here_formatted_origin = origin.split(",")
+            vol.Schema(cv.gps(here_formatted_origin))
+        except (AttributeError, vol.Invalid) as ex:
+            raise InvalidCoordinatesException(
+                f"{origin} are not valid coordinates"
+            ) from ex
+        arrival: str | None = None
+        departure: str | None = None
+        if self.config.arrival is not None:
+            arrival = convert_time_to_isodate(self.config.arrival)
+        if self.config.departure is not None:
+            departure = convert_time_to_isodate(self.config.departure)
+
+        if arrival is None and departure is None:
+            departure = "now"
+
+        return (here_formatted_origin, here_formatted_destination, arrival, departure)
+
 
 def build_hass_attribution(source_attribution: dict) -> str | None:
     """Build a hass frontend ready string out of the sourceAttribution."""
@@ -164,3 +193,7 @@ def convert_time_to_isodate(simple_time: time) -> str:
     if combined < datetime.now():
         combined = combined + timedelta(days=1)
     return combined.isoformat()
+
+
+class InvalidCoordinatesException(Exception):
+    """Coordinates for origin or destination are malformed."""

tests/components/here_travel_time/test_sensor.py

@@ -214,6 +214,104 @@ async def test_entity_ids(hass, valid_response: MagicMock):
         )
 
 
+async def test_destination_entity_not_found(hass, caplog, valid_response: MagicMock):
+    """Test that a not existing destination_entity_id is caught."""
+    config = {
+        DOMAIN: {
+            "platform": PLATFORM,
+            "name": "test",
+            "origin_latitude": CAR_ORIGIN_LATITUDE,
+            "origin_longitude": CAR_ORIGIN_LONGITUDE,
+            "destination_entity_id": "device_tracker.test",
+            "api_key": API_KEY,
+            "mode": TRAVEL_MODE_TRUCK,
+        }
+    }
+    assert await async_setup_component(hass, DOMAIN, config)
+    await hass.async_block_till_done()
+
+    hass.bus.async_fire(EVENT_HOMEASSISTANT_START)
+    await hass.async_block_till_done()
+
+    assert "device_tracker.test are not valid coordinates" in caplog.text
+
+
+async def test_origin_entity_not_found(hass, caplog, valid_response: MagicMock):
+    """Test that a not existing origin_entity_id is caught."""
+    config = {
+        DOMAIN: {
+            "platform": PLATFORM,
+            "name": "test",
+            "origin_entity_id": "device_tracker.test",
+            "destination_latitude": CAR_ORIGIN_LATITUDE,
+            "destination_longitude": CAR_ORIGIN_LONGITUDE,
+            "api_key": API_KEY,
+            "mode": TRAVEL_MODE_TRUCK,
+        }
+    }
+    assert await async_setup_component(hass, DOMAIN, config)
+    await hass.async_block_till_done()
+
+    hass.bus.async_fire(EVENT_HOMEASSISTANT_START)
+    await hass.async_block_till_done()
+
+    assert "device_tracker.test are not valid coordinates" in caplog.text
+
+
+async def test_invalid_destination_entity_state(
+    hass, caplog, valid_response: MagicMock
+):
+    """Test that an invalid state of the destination_entity_id is caught."""
+    hass.states.async_set(
+        "device_tracker.test",
+        "test_state",
+    )
+    config = {
+        DOMAIN: {
+            "platform": PLATFORM,
+            "name": "test",
+            "origin_latitude": CAR_ORIGIN_LATITUDE,
+            "origin_longitude": CAR_ORIGIN_LONGITUDE,
+            "destination_entity_id": "device_tracker.test",
+            "api_key": API_KEY,
+            "mode": TRAVEL_MODE_TRUCK,
+        }
+    }
+    assert await async_setup_component(hass, DOMAIN, config)
+    await hass.async_block_till_done()
+
+    hass.bus.async_fire(EVENT_HOMEASSISTANT_START)
+    await hass.async_block_till_done()
+
+    assert "test_state are not valid coordinates" in caplog.text
+
+
+async def test_invalid_origin_entity_state(hass, caplog, valid_response: MagicMock):
+    """Test that an invalid state of the origin_entity_id is caught."""
+    hass.states.async_set(
+        "device_tracker.test",
+        "test_state",
+    )
+    config = {
+        DOMAIN: {
+            "platform": PLATFORM,
+            "name": "test",
+            "origin_entity_id": "device_tracker.test",
+            "destination_latitude": CAR_ORIGIN_LATITUDE,
+            "destination_longitude": CAR_ORIGIN_LONGITUDE,
+            "api_key": API_KEY,
+            "mode": TRAVEL_MODE_TRUCK,
+        }
+    }
+    assert await async_setup_component(hass, DOMAIN, config)
+    await hass.async_block_till_done()
+
+    hass.bus.async_fire(EVENT_HOMEASSISTANT_START)
+    await hass.async_block_till_done()
+
+    assert "test_state are not valid coordinates" in caplog.text
+
+
 async def test_route_not_found(hass, caplog):
     """Test that route not found error is correctly handled."""
     config = {