Validate Number value before calling entity method (#52343)

Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

homeassistant/components/demo/number.py

@@ -1,8 +1,6 @@
 """Demo platform that offers a fake Number entity."""
 from __future__ import annotations
 
-import voluptuous as vol
-
 from homeassistant.components.number import NumberEntity
 from homeassistant.const import DEVICE_DEFAULT_NAME
 
@@ -82,12 +80,5 @@ class DemoNumber(NumberEntity):
 
     async def async_set_value(self, value):
         """Update the current value."""
-        num_value = float(value)
+        self._attr_value = value
-
-        if num_value < self.min_value or num_value > self.max_value:
-            raise vol.Invalid(
-                f"Invalid value for {self.entity_id}: {value} (range {self.min_value} - {self.max_value})"
-            )
-
-        self._attr_value = num_value
         self.async_write_ha_state()

homeassistant/components/number/__init__.py

@@ -9,7 +9,7 @@ from typing import Any, final
 import voluptuous as vol
 
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, ServiceCall
 from homeassistant.helpers.config_validation import (  # noqa: F401
     PLATFORM_SCHEMA,
     PLATFORM_SCHEMA_BASE,
@@ -49,12 +49,22 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
     component.async_register_entity_service(
         SERVICE_SET_VALUE,
         {vol.Required(ATTR_VALUE): vol.Coerce(float)},
-        "async_set_value",
+        async_set_value,
     )
 
     return True
 
 
+async def async_set_value(entity: NumberEntity, service_call: ServiceCall) -> None:
+    """Service call wrapper to set a new value."""
+    value = service_call.data["value"]
+    if value < entity.min_value or value > entity.max_value:
+        raise ValueError(
+            f"Value {value} for {entity.name} is outside valid range {entity.min_value} - {entity.max_value}"
+        )
+    await entity.async_set_value(value)
+
+
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up a config entry."""
     component: EntityComponent = hass.data[DOMAIN]

tests/components/demo/test_number.py

@@ -67,7 +67,7 @@ async def test_set_value_bad_range(hass):
     state = hass.states.get(ENTITY_VOLUME)
     assert state.state == "42.0"
 
-    with pytest.raises(vol.Invalid):
+    with pytest.raises(ValueError):
         await hass.services.async_call(
             DOMAIN,
             SERVICE_SET_VALUE,

tests/components/number/test_init.py

@@ -1,7 +1,18 @@
 """The tests for the Number component."""
 from unittest.mock import MagicMock
 
-from homeassistant.components.number import NumberEntity
+import pytest
+
+from homeassistant.components.number import (
+    ATTR_STEP,
+    ATTR_VALUE,
+    DOMAIN,
+    SERVICE_SET_VALUE,
+    NumberEntity,
+)
+from homeassistant.const import ATTR_ENTITY_ID, CONF_PLATFORM
+from homeassistant.core import HomeAssistant
+from homeassistant.setup import async_setup_component
 
 
 class MockDefaultNumberEntity(NumberEntity):
@@ -27,7 +38,7 @@ class MockNumberEntity(NumberEntity):
         return 0.5
 
 
-async def test_step(hass):
+async def test_step(hass: HomeAssistant) -> None:
     """Test the step calculation."""
     number = MockDefaultNumberEntity()
     assert number.step == 1.0
@@ -36,7 +47,7 @@ async def test_step(hass):
     assert number_2.step == 0.1
 
 
-async def test_sync_set_value(hass):
+async def test_sync_set_value(hass: HomeAssistant) -> None:
     """Test if async set_value calls sync set_value."""
     number = MockDefaultNumberEntity()
     number.hass = hass
@@ -46,3 +57,43 @@ async def test_sync_set_value(hass):
 
     assert number.set_value.called
     assert number.set_value.call_args[0][0] == 42
+
+
+async def test_custom_integration_and_validation(
+    hass: HomeAssistant, enable_custom_integrations: None
+) -> None:
+    """Test we can only set valid values."""
+    platform = getattr(hass.components, f"test.{DOMAIN}")
+    platform.init()
+
+    assert await async_setup_component(hass, DOMAIN, {DOMAIN: {CONF_PLATFORM: "test"}})
+    await hass.async_block_till_done()
+
+    state = hass.states.get("number.test")
+    assert state.state == "50.0"
+    assert state.attributes.get(ATTR_STEP) == 1.0
+
+    await hass.services.async_call(
+        DOMAIN,
+        SERVICE_SET_VALUE,
+        {ATTR_VALUE: 60.0, ATTR_ENTITY_ID: "number.test"},
+        blocking=True,
+    )
+
+    hass.states.async_set("number.test", 60.0)
+    await hass.async_block_till_done()
+    state = hass.states.get("number.test")
+    assert state.state == "60.0"
+
+    # test ValueError trigger
+    with pytest.raises(ValueError):
+        await hass.services.async_call(
+            DOMAIN,
+            SERVICE_SET_VALUE,
+            {ATTR_VALUE: 110.0, ATTR_ENTITY_ID: "number.test"},
+            blocking=True,
+        )
+
+    await hass.async_block_till_done()
+    state = hass.states.get("number.test")
+    assert state.state == "60.0"

tests/testing_config/custom_components/test/number.py

@@ -0,0 +1,51 @@
+"""
+Provide a mock number platform.
+
+Call init before using it in your tests to ensure clean test data.
+"""
+from homeassistant.components.number import NumberEntity
+
+from tests.common import MockEntity
+
+UNIQUE_NUMBER = "unique_number"
+
+ENTITIES = []
+
+
+class MockNumberEntity(MockEntity, NumberEntity):
+    """Mock Select class."""
+
+    _attr_value = 50.0
+    _attr_step = 1.0
+
+    @property
+    def value(self):
+        """Return the current value."""
+        return self._handle("value")
+
+    def set_value(self, value: float) -> None:
+        """Change the selected option."""
+        self._attr_value = value
+
+
+def init(empty=False):
+    """Initialize the platform with entities."""
+    global ENTITIES
+
+    ENTITIES = (
+        []
+        if empty
+        else [
+            MockNumberEntity(
+                name="test",
+                unique_id=UNIQUE_NUMBER,
+            ),
+        ]
+    )
+
+
+async def async_setup_platform(
+    hass, config, async_add_entities_callback, discovery_info=None
+):
+    """Return mock entities."""
+    async_add_entities_callback(ENTITIES)