From c7eab49c70373a45e1036e7e4c25991b4b6ff1bd Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Wed, 27 Dec 2023 09:45:55 +0100 Subject: [PATCH] Raise ServiceValidationError on invalid select option (#106350) * Raise ServiceValidationError on invalid select option * Fix tests * Correct place holders * More test fixes --- homeassistant/components/select/__init__.py | 37 ++++++++++++++----- homeassistant/components/select/strings.json | 5 +++ tests/components/airzone/test_select.py | 3 +- .../bmw_connected_drive/test_select.py | 6 +-- tests/components/demo/test_select.py | 3 +- tests/components/flux_led/test_select.py | 11 +++--- tests/components/knx/test_select.py | 3 +- tests/components/litterrobot/test_select.py | 3 +- .../rituals_perfume_genie/test_select.py | 3 +- tests/components/select/test_init.py | 10 +++-- tests/components/xiaomi_miio/test_select.py | 3 +- 11 files changed, 60 insertions(+), 27 deletions(-) diff --git a/homeassistant/components/select/__init__.py b/homeassistant/components/select/__init__.py index 459083cedd4..8ec08f4606f 100644 --- a/homeassistant/components/select/__init__.py +++ b/homeassistant/components/select/__init__.py @@ -8,7 +8,8 @@ from typing import TYPE_CHECKING, Any, final import voluptuous as vol from homeassistant.config_entries import ConfigEntry -from homeassistant.core import HomeAssistant, ServiceCall +from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers import config_validation as cv from homeassistant.helpers.config_validation import ( PLATFORM_SCHEMA, @@ -90,7 +91,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: component.async_register_entity_service( SERVICE_SELECT_OPTION, {vol.Required(ATTR_OPTION): cv.string}, - async_select_option, + SelectEntity.async_handle_select_option.__name__, ) component.async_register_entity_service( @@ -102,14 +103,6 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: return True -async def async_select_option(entity: SelectEntity, service_call: ServiceCall) -> None: - """Service call wrapper to set a new value.""" - option = service_call.data[ATTR_OPTION] - if option not in entity.options: - raise ValueError(f"Option {option} not valid for {entity.entity_id}") - await entity.async_select_option(option) - - async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up a config entry.""" component: EntityComponent[SelectEntity] = hass.data[DOMAIN] @@ -177,6 +170,30 @@ class SelectEntity(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): """Return the selected entity option to represent the entity state.""" return self._attr_current_option + @final + @callback + def _valid_option_or_raise(self, option: str) -> None: + """Raise ServiceValidationError on invalid option.""" + options = self.options + if not options or option not in options: + friendly_options: str = ", ".join(options or []) + raise ServiceValidationError( + f"Option {option} is not valid for {self.entity_id}", + translation_domain=DOMAIN, + translation_key="not_valid_option", + translation_placeholders={ + "entity_id": self.entity_id, + "option": option, + "options": friendly_options, + }, + ) + + @final + async def async_handle_select_option(self, option: str) -> None: + """Service call wrapper to set a new value.""" + self._valid_option_or_raise(option) + await self.async_select_option(option) + def select_option(self, option: str) -> None: """Change the selected option.""" raise NotImplementedError() diff --git a/homeassistant/components/select/strings.json b/homeassistant/components/select/strings.json index d058ff6e6f2..9c9d1136b99 100644 --- a/homeassistant/components/select/strings.json +++ b/homeassistant/components/select/strings.json @@ -64,5 +64,10 @@ } } } + }, + "exceptions": { + "not_valid_option": { + "message": "Option {option} is not valid for entity {entity_id}, valid options are: {options}." + } } } diff --git a/tests/components/airzone/test_select.py b/tests/components/airzone/test_select.py index c7c32022123..01617eab175 100644 --- a/tests/components/airzone/test_select.py +++ b/tests/components/airzone/test_select.py @@ -15,6 +15,7 @@ import pytest from homeassistant.components.select import DOMAIN as SELECT_DOMAIN from homeassistant.const import ATTR_ENTITY_ID, ATTR_OPTION, SERVICE_SELECT_OPTION from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from .util import async_init_integration @@ -85,7 +86,7 @@ async def test_airzone_select_sleep(hass: HomeAssistant) -> None: ] } - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( SELECT_DOMAIN, SERVICE_SELECT_OPTION, diff --git a/tests/components/bmw_connected_drive/test_select.py b/tests/components/bmw_connected_drive/test_select.py index 2dbe66139b2..1860ed19720 100644 --- a/tests/components/bmw_connected_drive/test_select.py +++ b/tests/components/bmw_connected_drive/test_select.py @@ -8,7 +8,7 @@ import respx from syrupy.assertion import SnapshotAssertion from homeassistant.core import HomeAssistant -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from . import check_remote_service_call, setup_mocked_integration @@ -92,7 +92,7 @@ async def test_service_call_invalid_input( old_value = hass.states.get(entity_id).state # Test - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( "select", "select_option", @@ -108,7 +108,7 @@ async def test_service_call_invalid_input( [ (MyBMWRemoteServiceError, HomeAssistantError), (MyBMWAPIError, HomeAssistantError), - (ValueError, ValueError), + (ServiceValidationError, ServiceValidationError), ], ) async def test_service_call_fail( diff --git a/tests/components/demo/test_select.py b/tests/components/demo/test_select.py index a4fff2a231e..013a9900a83 100644 --- a/tests/components/demo/test_select.py +++ b/tests/components/demo/test_select.py @@ -11,6 +11,7 @@ from homeassistant.components.select import ( ) from homeassistant.const import ATTR_ENTITY_ID, Platform from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from homeassistant.setup import async_setup_component ENTITY_SPEED = "select.speed" @@ -51,7 +52,7 @@ async def test_select_option_bad_attr(hass: HomeAssistant) -> None: assert state assert state.state == "ridiculous_speed" - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( DOMAIN, SERVICE_SELECT_OPTION, diff --git a/tests/components/flux_led/test_select.py b/tests/components/flux_led/test_select.py index c8fd64c6811..1cdbb9369ab 100644 --- a/tests/components/flux_led/test_select.py +++ b/tests/components/flux_led/test_select.py @@ -14,6 +14,7 @@ from homeassistant.components.flux_led.const import CONF_WHITE_CHANNEL_TYPE, DOM from homeassistant.components.select import DOMAIN as SELECT_DOMAIN from homeassistant.const import ATTR_ENTITY_ID, ATTR_OPTION, CONF_HOST, CONF_NAME from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers import entity_registry as er from homeassistant.setup import async_setup_component @@ -133,7 +134,7 @@ async def test_select_addressable_strip_config(hass: HomeAssistant) -> None: state = hass.states.get(ic_type_entity_id) assert state.state == "WS2812B" - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( SELECT_DOMAIN, "select_option", @@ -149,7 +150,7 @@ async def test_select_addressable_strip_config(hass: HomeAssistant) -> None: bulb.async_set_device_config.assert_called_once_with(wiring="GRBW") bulb.async_set_device_config.reset_mock() - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( SELECT_DOMAIN, "select_option", @@ -191,7 +192,7 @@ async def test_select_mutable_0x25_strip_config(hass: HomeAssistant) -> None: state = hass.states.get(operating_mode_entity_id) assert state.state == "RGBWW" - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( SELECT_DOMAIN, "select_option", @@ -226,7 +227,7 @@ async def test_select_24ghz_remote_config(hass: HomeAssistant) -> None: state = hass.states.get(remote_config_entity_id) assert state.state == "Open" - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( SELECT_DOMAIN, "select_option", @@ -275,7 +276,7 @@ async def test_select_white_channel_type(hass: HomeAssistant) -> None: state = hass.states.get(operating_mode_entity_id) assert state.state == WhiteChannelType.WARM.name.title() - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( SELECT_DOMAIN, "select_option", diff --git a/tests/components/knx/test_select.py b/tests/components/knx/test_select.py index f113a83f7a0..1b408a298a2 100644 --- a/tests/components/knx/test_select.py +++ b/tests/components/knx/test_select.py @@ -11,6 +11,7 @@ from homeassistant.components.knx.const import ( from homeassistant.components.knx.schema import SelectSchema from homeassistant.const import CONF_NAME, CONF_PAYLOAD, STATE_UNKNOWN from homeassistant.core import HomeAssistant, State +from homeassistant.exceptions import ServiceValidationError from .conftest import KNXTestKit @@ -76,7 +77,7 @@ async def test_select_dpt_2_simple(hass: HomeAssistant, knx: KNXTestKit) -> None assert state.state is STATE_UNKNOWN # select invalid option - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( "select", "select_option", diff --git a/tests/components/litterrobot/test_select.py b/tests/components/litterrobot/test_select.py index f6a32a6ef35..b35fdf5c917 100644 --- a/tests/components/litterrobot/test_select.py +++ b/tests/components/litterrobot/test_select.py @@ -12,6 +12,7 @@ from homeassistant.components.select import ( ) from homeassistant.const import ATTR_ENTITY_ID, EntityCategory from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers import entity_registry as er from .conftest import setup_integration @@ -59,7 +60,7 @@ async def test_invalid_wait_time_select(hass: HomeAssistant, mock_account) -> No data = {ATTR_ENTITY_ID: SELECT_ENTITY_ID, ATTR_OPTION: "10"} - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( PLATFORM_DOMAIN, SERVICE_SELECT_OPTION, diff --git a/tests/components/rituals_perfume_genie/test_select.py b/tests/components/rituals_perfume_genie/test_select.py index 3153005d094..a055e8fed05 100644 --- a/tests/components/rituals_perfume_genie/test_select.py +++ b/tests/components/rituals_perfume_genie/test_select.py @@ -15,6 +15,7 @@ from homeassistant.const import ( EntityCategory, ) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers import entity_registry as er from homeassistant.setup import async_setup_component @@ -84,7 +85,7 @@ async def test_select_invalid_option(hass: HomeAssistant) -> None: assert state assert state.state == "60" - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( SELECT_DOMAIN, SERVICE_SELECT_OPTION, diff --git a/tests/components/select/test_init.py b/tests/components/select/test_init.py index 585972a0953..604bf3f0fb9 100644 --- a/tests/components/select/test_init.py +++ b/tests/components/select/test_init.py @@ -17,6 +17,7 @@ from homeassistant.components.select import ( ) from homeassistant.const import ATTR_ENTITY_ID, CONF_PLATFORM, STATE_UNKNOWN from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from homeassistant.setup import async_setup_component @@ -111,8 +112,8 @@ async def test_custom_integration_and_validation( await hass.async_block_till_done() assert hass.states.get("select.select_1").state == "option 2" - # test ValueError trigger - with pytest.raises(ValueError): + # test ServiceValidationError trigger + with pytest.raises(ServiceValidationError) as exc: await hass.services.async_call( DOMAIN, SERVICE_SELECT_OPTION, @@ -120,11 +121,14 @@ async def test_custom_integration_and_validation( blocking=True, ) await hass.async_block_till_done() + assert exc.value.translation_domain == DOMAIN + assert exc.value.translation_key == "not_valid_option" + assert hass.states.get("select.select_1").state == "option 2" assert hass.states.get("select.select_2").state == STATE_UNKNOWN - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( DOMAIN, SERVICE_SELECT_OPTION, diff --git a/tests/components/xiaomi_miio/test_select.py b/tests/components/xiaomi_miio/test_select.py index a999f0e7c9a..794fbb090e0 100644 --- a/tests/components/xiaomi_miio/test_select.py +++ b/tests/components/xiaomi_miio/test_select.py @@ -31,6 +31,7 @@ from homeassistant.const import ( Platform, ) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from . import TEST_MAC @@ -77,7 +78,7 @@ async def test_select_bad_attr(hass: HomeAssistant) -> None: assert state assert state.state == "forward" - with pytest.raises(ValueError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( "select", SERVICE_SELECT_OPTION,