From 2087479e16c35f0598f188d9db7594a1f534d526 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Fri, 3 May 2024 13:12:34 +0000 Subject: [PATCH 01/16] File integration entry setup --- homeassistant/components/file/__init__.py | 71 ++++++++++++++++++++ homeassistant/components/file/config_flow.py | 1 + homeassistant/components/file/const.py | 5 ++ homeassistant/components/file/manifest.json | 1 + homeassistant/generated/config_flows.py | 1 + homeassistant/generated/integrations.json | 2 +- 6 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 homeassistant/components/file/config_flow.py create mode 100644 homeassistant/components/file/const.py diff --git a/homeassistant/components/file/__init__.py b/homeassistant/components/file/__init__.py index ed31fa957dd00c..eb9f79f8edaabd 100644 --- a/homeassistant/components/file/__init__.py +++ b/homeassistant/components/file/__init__.py @@ -1 +1,72 @@ """The file component.""" + +from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry +from homeassistant.const import CONF_PLATFORM, CONF_VALUE_TEMPLATE, Platform +from homeassistant.core import HomeAssistant +from homeassistant.helpers import config_validation as cv, discovery +from homeassistant.helpers.template import Template +from homeassistant.helpers.typing import ConfigType + +from .const import DOMAIN + +CONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN) + +PLATFORMS = [Platform.NOTIFY, Platform.SENSOR] + + +async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: + """Set up the file integration. + + The file integration uses config flow for configuration. + But, a `file:` entry in configuration.yaml will trigger an import flow + if a config entry doesn't already exist. + """ + + config_data: dict[str, list[ConfigType]] = { + domain: [item for item in config[domain] if item.pop(CONF_PLATFORM) == DOMAIN] + for domain in config + if domain in PLATFORMS + } + # Ensure we pass the string value for a value_template + if Platform.SENSOR in config_data and ( + items := [ + item for item in config_data[Platform.SENSOR] if CONF_VALUE_TEMPLATE in item + ] + ): + for item in items: + value_template: Template = item[CONF_VALUE_TEMPLATE] + item[CONF_VALUE_TEMPLATE] = value_template.template + if not hass.config_entries.async_entries(DOMAIN): + # No config entry exists and configuration.yaml + # config exists, then trigger the import flow. + hass.async_create_task( + hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_IMPORT}, + data=config_data, + ) + ) + + # The legacy File notify.notify service is deprecated with HA Core 2024.5.0 + # with HA Core 2024.5.0 and will be removed with HA core 2024.12.0 + hass.async_create_task( + discovery.async_load_platform( + hass, + Platform.NOTIFY, + DOMAIN, + None, + config, + ) + ) + return True + + +async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Set up a file component entry.""" + await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + return True + + +async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Unload a config entry.""" + return await hass.config_entries.async_unload_platforms(entry, PLATFORMS) diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py new file mode 100644 index 00000000000000..a7754652652dec --- /dev/null +++ b/homeassistant/components/file/config_flow.py @@ -0,0 +1 @@ +"""Config flow for file integration.""" diff --git a/homeassistant/components/file/const.py b/homeassistant/components/file/const.py new file mode 100644 index 00000000000000..797a17badee29e --- /dev/null +++ b/homeassistant/components/file/const.py @@ -0,0 +1,5 @@ +"""Constants for the file integration.""" + +from typing import Final + +DOMAIN: Final = "file" diff --git a/homeassistant/components/file/manifest.json b/homeassistant/components/file/manifest.json index fb09e5151f2034..37bb108e1d5fe9 100644 --- a/homeassistant/components/file/manifest.json +++ b/homeassistant/components/file/manifest.json @@ -2,6 +2,7 @@ "domain": "file", "name": "File", "codeowners": ["@fabaff"], + "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/file", "iot_class": "local_polling", "requirements": ["file-read-backwards==2.0.0"] diff --git a/homeassistant/generated/config_flows.py b/homeassistant/generated/config_flows.py index 134b1e80d98688..5657b171701fdb 100644 --- a/homeassistant/generated/config_flows.py +++ b/homeassistant/generated/config_flows.py @@ -164,6 +164,7 @@ "faa_delays", "fastdotcom", "fibaro", + "file", "filesize", "fireservicerota", "fitbit", diff --git a/homeassistant/generated/integrations.json b/homeassistant/generated/integrations.json index e16f29a14e288b..97fd6d30ecaad0 100644 --- a/homeassistant/generated/integrations.json +++ b/homeassistant/generated/integrations.json @@ -1815,7 +1815,7 @@ "file": { "name": "File", "integration_type": "hub", - "config_flow": false, + "config_flow": true, "iot_class": "local_polling" }, "filesize": { From 3944b2ed4108f3e673de77e81c10fd17cd843f9c Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sat, 4 May 2024 17:09:57 +0000 Subject: [PATCH 02/16] Import to entry and tests --- homeassistant/components/file/__init__.py | 73 +++---- homeassistant/components/file/config_flow.py | 48 ++++ homeassistant/components/file/const.py | 4 + homeassistant/components/file/notify.py | 58 ++++- homeassistant/components/file/sensor.py | 43 ++-- tests/components/file/conftest.py | 25 +++ tests/components/file/test_notify.py | 217 +++++++++++++++++-- tests/components/file/test_sensor.py | 107 +++++---- 8 files changed, 461 insertions(+), 114 deletions(-) create mode 100644 tests/components/file/conftest.py diff --git a/homeassistant/components/file/__init__.py b/homeassistant/components/file/__init__.py index eb9f79f8edaabd..efa0c2b00c2f65 100644 --- a/homeassistant/components/file/__init__.py +++ b/homeassistant/components/file/__init__.py @@ -1,10 +1,9 @@ """The file component.""" from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry -from homeassistant.const import CONF_PLATFORM, CONF_VALUE_TEMPLATE, Platform +from homeassistant.const import CONF_PLATFORM, Platform from homeassistant.core import HomeAssistant -from homeassistant.helpers import config_validation as cv, discovery -from homeassistant.helpers.template import Template +from homeassistant.helpers import config_validation as cv from homeassistant.helpers.typing import ConfigType from .const import DOMAIN @@ -15,58 +14,56 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: - """Set up the file integration. + """Set up the file integration.""" - The file integration uses config flow for configuration. - But, a `file:` entry in configuration.yaml will trigger an import flow - if a config entry doesn't already exist. - """ + # The legacy File notify.notify service is deprecated with HA Core 2024.5.0 + # with HA Core 2024.5.0 and will be removed with HA core 2024.12.0 + # The legacy service will coexist together with the new entity platform service + # hass.async_create_task( + # discovery.async_load_platform( + # hass, + # Platform.NOTIFY, + # DOMAIN, + # None, + # config, + # ) + # ) + + if hass.config_entries.async_entries(DOMAIN): + # We skip import in case we already have config entries + return True + # Prepare to import the YAML config into separate config entries config_data: dict[str, list[ConfigType]] = { domain: [item for item in config[domain] if item.pop(CONF_PLATFORM) == DOMAIN] for domain in config if domain in PLATFORMS } - # Ensure we pass the string value for a value_template - if Platform.SENSOR in config_data and ( - items := [ - item for item in config_data[Platform.SENSOR] if CONF_VALUE_TEMPLATE in item - ] - ): + + for domain, items in config_data.items(): for item in items: - value_template: Template = item[CONF_VALUE_TEMPLATE] - item[CONF_VALUE_TEMPLATE] = value_template.template - if not hass.config_entries.async_entries(DOMAIN): - # No config entry exists and configuration.yaml - # config exists, then trigger the import flow. - hass.async_create_task( - hass.config_entries.flow.async_init( - DOMAIN, - context={"source": SOURCE_IMPORT}, - data=config_data, + item[CONF_PLATFORM] = domain + hass.async_create_task( + hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_IMPORT}, + data=item, + ) ) - ) - # The legacy File notify.notify service is deprecated with HA Core 2024.5.0 - # with HA Core 2024.5.0 and will be removed with HA core 2024.12.0 - hass.async_create_task( - discovery.async_load_platform( - hass, - Platform.NOTIFY, - DOMAIN, - None, - config, - ) - ) return True async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up a file component entry.""" - await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + await hass.config_entries.async_forward_entry_setups( + entry, [Platform(entry.data[CONF_PLATFORM])] + ) return True async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" - return await hass.config_entries.async_unload_platforms(entry, PLATFORMS) + return await hass.config_entries.async_unload_platforms( + entry, [entry.data[CONF_PLATFORM]] + ) diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index a7754652652dec..52f4aae742cb9d 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -1 +1,49 @@ """Config flow for file integration.""" + +from typing import Any + +import voluptuous as vol + +from homeassistant.config_entries import ConfigFlow, ConfigFlowResult +from homeassistant.const import ( + CONF_FILE_PATH, + CONF_FILENAME, + CONF_NAME, + CONF_PLATFORM, + CONF_UNIT_OF_MEASUREMENT, + CONF_VALUE_TEMPLATE, + Platform, +) +import homeassistant.helpers.config_validation as cv + +from .const import DEFAULT_NAME, DOMAIN + +FILE_SENSOR_SCHEMA = vol.Schema( + { + vol.Required(CONF_FILE_PATH): cv.isfile, + vol.Optional(CONF_NAME, default="File"): cv.string, + vol.Optional(CONF_VALUE_TEMPLATE): cv.template, + vol.Optional(CONF_UNIT_OF_MEASUREMENT): cv.string, + } +) + + +class FileConfigFlowHandler(ConfigFlow, domain=DOMAIN): + """Handle a file config flow.""" + + VERSION = 1 + + async def async_step_import( + self, import_data: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Import `file`` config from configuration.yaml.""" + assert import_data is not None + platform = import_data[CONF_PLATFORM] + name: str = import_data.get(CONF_NAME, DEFAULT_NAME) + file_name: str + if platform == Platform.NOTIFY: + file_name = import_data[CONF_FILENAME] + else: + file_name = import_data[CONF_FILE_PATH] + title = f"{name} [{file_name}]" + return self.async_create_entry(title=title, data=import_data) diff --git a/homeassistant/components/file/const.py b/homeassistant/components/file/const.py index 797a17badee29e..c9a53eeead1ed1 100644 --- a/homeassistant/components/file/const.py +++ b/homeassistant/components/file/const.py @@ -3,3 +3,7 @@ from typing import Final DOMAIN: Final = "file" + +DEFAULT_NAME = "File" + +FILE_ICON = "mdi:file" diff --git a/homeassistant/components/file/notify.py b/homeassistant/components/file/notify.py index 50e6cec09a8f6b..4871b703980f34 100644 --- a/homeassistant/components/file/notify.py +++ b/homeassistant/components/file/notify.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import os from typing import Any, TextIO @@ -12,13 +13,21 @@ ATTR_TITLE_DEFAULT, PLATFORM_SCHEMA, BaseNotificationService, + NotifyEntity, + NotifyEntityFeature, ) -from homeassistant.const import CONF_FILENAME +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_FILENAME, CONF_NAME from homeassistant.core import HomeAssistant import homeassistant.helpers.config_validation as cv +from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType import homeassistant.util.dt as dt_util +from .const import FILE_ICON + +_LOGGER = logging.getLogger(__name__) + CONF_TIMESTAMP = "timestamp" PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( @@ -29,6 +38,53 @@ ) +async def async_setup_entry( + hass: HomeAssistant, + entry: ConfigEntry, + async_add_entities: AddEntitiesCallback, +) -> None: + """Set up the file sensor.""" + config: dict[str, Any] = dict(entry.data) + if not await hass.async_add_executor_job( + hass.config.is_allowed_path, config[CONF_FILENAME] + ): + _LOGGER.error("'%s' is not an allowed directory", config[CONF_FILENAME]) + return + async_add_entities([FileNotifyEntity(config)]) + + +class FileNotifyEntity(NotifyEntity): + """Implement the notification entity platform for the File service.""" + + _attr_icon = FILE_ICON + _attr_supported_features = NotifyEntityFeature.TITLE + + def __init__(self, config: dict[str, Any]) -> None: + """Initialize the service.""" + self.filename: str = config[CONF_FILENAME] + self.add_timestamp: bool = config.get(CONF_TIMESTAMP, False) + self._attr_name = config.get(CONF_NAME, config[CONF_FILENAME]) + self._attr_unique_id = f"notify_{config[CONF_FILENAME]}" + + def send_message(self, message: str, title: str | None = None) -> None: + """Send a message to a file.""" + file: TextIO + filepath: str = os.path.join(self.hass.config.config_dir, self.filename) + with open(filepath, "a", encoding="utf8") as file: + if os.stat(filepath).st_size == 0: + title = ( + f"{title or ATTR_TITLE_DEFAULT} notifications (Log" + f" started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" + ) + file.write(title) + + if self.add_timestamp: + text = f"{dt_util.utcnow().isoformat()} {message}\n" + else: + text = f"{message}\n" + file.write(text) + + def get_service( hass: HomeAssistant, config: ConfigType, diff --git a/homeassistant/components/file/sensor.py b/homeassistant/components/file/sensor.py index f70b0bce701edb..0a1208c3fbd6d4 100644 --- a/homeassistant/components/file/sensor.py +++ b/homeassistant/components/file/sensor.py @@ -9,6 +9,7 @@ import voluptuous as vol from homeassistant.components.sensor import PLATFORM_SCHEMA, SensorEntity +from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( CONF_FILE_PATH, CONF_NAME, @@ -16,22 +17,20 @@ CONF_VALUE_TEMPLATE, ) from homeassistant.core import HomeAssistant -import homeassistant.helpers.config_validation as cv +from homeassistant.helpers import config_validation as cv from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.template import Template from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType -_LOGGER = logging.getLogger(__name__) - -DEFAULT_NAME = "File" +from .const import DEFAULT_NAME, FILE_ICON -ICON = "mdi:file" +_LOGGER = logging.getLogger(__name__) PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( { vol.Required(CONF_FILE_PATH): cv.isfile, vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, - vol.Optional(CONF_VALUE_TEMPLATE): cv.template, + vol.Optional(CONF_VALUE_TEMPLATE): cv.string, vol.Optional(CONF_UNIT_OF_MEASUREMENT): cv.string, } ) @@ -42,26 +41,41 @@ async def async_setup_platform( config: ConfigType, async_add_entities: AddEntitiesCallback, discovery_info: DiscoveryInfoType | None = None, +) -> None: + """Set up the file sensor from YAML. + + The YAML platform config is automatically + imported to a config entry, this method can be removed + when YAML support is removed. + """ + + +async def async_setup_entry( + hass: HomeAssistant, + entry: ConfigEntry, + async_add_entities: AddEntitiesCallback, ) -> None: """Set up the file sensor.""" + config = dict(entry.data) file_path: str = config[CONF_FILE_PATH] - name: str = config[CONF_NAME] + name: str = config.get(CONF_NAME, DEFAULT_NAME) unit: str | None = config.get(CONF_UNIT_OF_MEASUREMENT) - value_template: Template | None = config.get(CONF_VALUE_TEMPLATE) + value_template: Template | None = None - if value_template is not None: - value_template.hass = hass + if CONF_VALUE_TEMPLATE in config: + value_template = Template(config[CONF_VALUE_TEMPLATE], hass) - if hass.config.is_allowed_path(file_path): - async_add_entities([FileSensor(name, file_path, unit, value_template)], True) - else: + if not await hass.async_add_executor_job(hass.config.is_allowed_path, file_path): _LOGGER.error("'%s' is not an allowed directory", file_path) + return + + async_add_entities([FileSensor(name, file_path, unit, value_template)], True) class FileSensor(SensorEntity): """Implementation of a file sensor.""" - _attr_icon = ICON + _attr_icon = FILE_ICON def __init__( self, @@ -75,6 +89,7 @@ def __init__( self._file_path = file_path self._attr_native_unit_of_measurement = unit_of_measurement self._val_tpl = value_template + self._attr_unique_id = f"sensor_{file_path}" def update(self) -> None: """Get the latest entry from a file and updates the state.""" diff --git a/tests/components/file/conftest.py b/tests/components/file/conftest.py new file mode 100644 index 00000000000000..bcd5fb37842fe4 --- /dev/null +++ b/tests/components/file/conftest.py @@ -0,0 +1,25 @@ +"""Test fixtures for file platform.""" + +from collections.abc import Generator +from unittest.mock import MagicMock, patch + +import pytest + +from homeassistant.core import HomeAssistant + + +@pytest.fixture +def is_allowed() -> bool: + """Parameterize mock_is_allowed_path, default True.""" + return True + + +@pytest.fixture +def mock_is_allowed_path( + hass: HomeAssistant, is_allowed: bool +) -> Generator[None, MagicMock]: + """Mock is_allowed_path method.""" + with patch.object( + hass.config, "is_allowed_path", return_value=is_allowed + ) as allowed_path_mock: + yield allowed_path_mock diff --git a/tests/components/file/test_notify.py b/tests/components/file/test_notify.py index 3077d71bdde6ab..cc06c47563cede 100644 --- a/tests/components/file/test_notify.py +++ b/tests/components/file/test_notify.py @@ -1,18 +1,21 @@ """The tests for the notify file platform.""" import os -from unittest.mock import call, mock_open, patch +from typing import Any +from unittest.mock import MagicMock, call, mock_open, patch from freezegun.api import FrozenDateTimeFactory import pytest from homeassistant.components import notify +from homeassistant.components.file import DOMAIN from homeassistant.components.notify import ATTR_TITLE_DEFAULT from homeassistant.core import HomeAssistant +from homeassistant.helpers.typing import ConfigType from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util -from tests.common import assert_setup_component +from tests.common import MockConfigEntry, assert_setup_component async def test_bad_config(hass: HomeAssistant) -> None: @@ -25,33 +28,143 @@ async def test_bad_config(hass: HomeAssistant) -> None: @pytest.mark.parametrize( - "timestamp", + ("domain", "service", "params"), [ - False, - True, + (notify.DOMAIN, "test", {"message": "one, two, testing, testing"}), + ( + notify.DOMAIN, + "send_message", + {"entity_id": "notify.test", "message": "one, two, testing, testing"}, + ), + ], + ids=["legacy", "entity"], +) +@pytest.mark.parametrize( + ("timestamp", "config"), + [ + ( + False, + { + "notify": [ + { + "name": "test", + "platform": "file", + "filename": "mock_file", + "timestamp": False, + } + ] + }, + ), + ( + True, + { + "notify": [ + { + "name": "test", + "platform": "file", + "filename": "mock_file", + "timestamp": True, + } + ] + }, + ), ], + ids=["no_timestamp", "timestamp"], ) async def test_notify_file( - hass: HomeAssistant, freezer: FrozenDateTimeFactory, timestamp: bool + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + timestamp: bool, + mock_is_allowed_path: MagicMock, + config: ConfigType, + domain: str, + service: str, + params: dict[str, str], ) -> None: """Test the notify file output.""" filename = "mock_file" - message = "one, two, testing, testing" - with assert_setup_component(1) as handle_config: - assert await async_setup_component( - hass, - notify.DOMAIN, + message = params["message"] + assert await async_setup_component(hass, notify.DOMAIN, config) + await hass.async_block_till_done() + assert await async_setup_component(hass, DOMAIN, config) + await hass.async_block_till_done(wait_background_tasks=True) + + freezer.move_to(dt_util.utcnow()) + + m_open = mock_open() + with ( + patch("homeassistant.components.file.notify.open", m_open, create=True), + patch("homeassistant.components.file.notify.os.stat") as mock_st, + ): + mock_st.return_value.st_size = 0 + title = ( + f"{ATTR_TITLE_DEFAULT} notifications " + f"(Log started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" + ) + + await hass.services.async_call(domain, service, params, blocking=True) + + full_filename = os.path.join(hass.config.path(), filename) + assert m_open.call_count == 1 + assert m_open.call_args == call(full_filename, "a", encoding="utf8") + + assert m_open.return_value.write.call_count == 2 + if not timestamp: + assert m_open.return_value.write.call_args_list == [ + call(title), + call(f"{message}\n"), + ] + else: + assert m_open.return_value.write.call_args_list == [ + call(title), + call(f"{dt_util.utcnow().isoformat()} {message}\n"), + ] + + +@pytest.mark.parametrize( + ("timestamp", "data"), + [ + ( + False, { - "notify": { - "name": "test", - "platform": "file", - "filename": filename, - "timestamp": timestamp, - } + "name": "test", + "platform": "notify", + "filename": "mock_file", + "timestamp": False, }, - ) - await hass.async_block_till_done() - assert handle_config[notify.DOMAIN] + ), + ( + True, + { + "name": "test", + "platform": "notify", + "filename": "mock_file", + "timestamp": True, + }, + ), + ], + ids=["no_timestamp", "timestamp"], +) +async def test_notify_file_entry_only_setup( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + timestamp: bool, + mock_is_allowed_path: MagicMock, + data: dict[str, Any], +) -> None: + """Test the notify file output.""" + filename = "mock_file" + + domain = notify.DOMAIN + service = "send_message" + params = {"entity_id": "notify.test", "message": "one, two, testing, testing"} + message = params["message"] + + entry = MockConfigEntry( + domain=DOMAIN, data=data, title=f"test [{data['filename']}]" + ) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) freezer.move_to(dt_util.utcnow()) @@ -66,9 +179,7 @@ async def test_notify_file( f"(Log started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" ) - await hass.services.async_call( - "notify", "test", {"message": message}, blocking=True - ) + await hass.services.async_call(domain, service, params, blocking=True) full_filename = os.path.join(hass.config.path(), filename) assert m_open.call_count == 1 @@ -85,3 +196,63 @@ async def test_notify_file( call(title), call(f"{dt_util.utcnow().isoformat()} {message}\n"), ] + + +@pytest.mark.parametrize( + ("data", "is_allowed"), + [ + ( + { + "name": "test", + "platform": "notify", + "filename": "mock_file", + "timestamp": False, + }, + False, + ), + ( + { + "name": "test", + "platform": "notify", + "filename": "mock_file", + "timestamp": True, + }, + True, + ), + ], + ids=["not_allowed", "allowed"], +) +async def test_notify_file_not_allowed_path( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_is_allowed_path: MagicMock, + is_allowed: bool, + data: dict[str, Any], +) -> None: + """Test the notify file output.""" + filename = "mock_file" + + domain = notify.DOMAIN + service = "send_message" + params = {"entity_id": "notify.test", "message": "one, two, testing, testing"} + + entry = MockConfigEntry( + domain=DOMAIN, data=data, title=f"test [{data['filename']}]" + ) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + await hass.async_block_till_done(wait_background_tasks=True) + + freezer.move_to(dt_util.utcnow()) + + m_open = mock_open() + with ( + patch("homeassistant.components.file.notify.open", m_open, create=True), + patch("homeassistant.components.file.notify.os.stat") as mock_st, + ): + mock_st.return_value.st_size = 0 + await hass.services.async_call(domain, service, params, blocking=True) + os.path.join(hass.config.path(), filename) + call_count = {False: 0, True: 1} + assert m_open.call_count == call_count[is_allowed] diff --git a/tests/components/file/test_sensor.py b/tests/components/file/test_sensor.py index 8acdc3242095c9..d2059f4d56405a 100644 --- a/tests/components/file/test_sensor.py +++ b/tests/components/file/test_sensor.py @@ -1,18 +1,23 @@ """The tests for local file sensor platform.""" -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch +import pytest + +from homeassistant.components.file import DOMAIN from homeassistant.const import STATE_UNKNOWN from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component -from tests.common import get_fixture_path +from tests.common import MockConfigEntry, get_fixture_path @patch("os.path.isfile", Mock(return_value=True)) @patch("os.access", Mock(return_value=True)) -async def test_file_value(hass: HomeAssistant) -> None: - """Test the File sensor.""" +async def test_file_value_yaml_setup( + hass: HomeAssistant, mock_is_allowed_path: MagicMock +) -> None: + """Test the File sensor from YAML setup.""" config = { "sensor": { "platform": "file", @@ -21,9 +26,30 @@ async def test_file_value(hass: HomeAssistant) -> None: } } - with patch.object(hass.config, "is_allowed_path", return_value=True): - assert await async_setup_component(hass, "sensor", config) - await hass.async_block_till_done() + assert await async_setup_component(hass, "sensor", config) + await hass.async_block_till_done() + + state = hass.states.get("sensor.file1") + assert state.state == "21" + + +@patch("os.path.isfile", Mock(return_value=True)) +@patch("os.access", Mock(return_value=True)) +async def test_file_value_entry_setup( + hass: HomeAssistant, mock_is_allowed_path: MagicMock +) -> None: + """Test the File sensor from an entry setup.""" + data = { + "platform": "sensor", + "name": "file1", + "file_path": get_fixture_path("file_value.txt", "file"), + } + + entry = MockConfigEntry( + domain=DOMAIN, data=data, title=f"test [{data['file_path']}]" + ) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) state = hass.states.get("sensor.file1") assert state.state == "21" @@ -31,20 +57,22 @@ async def test_file_value(hass: HomeAssistant) -> None: @patch("os.path.isfile", Mock(return_value=True)) @patch("os.access", Mock(return_value=True)) -async def test_file_value_template(hass: HomeAssistant) -> None: +async def test_file_value_template( + hass: HomeAssistant, mock_is_allowed_path: MagicMock +) -> None: """Test the File sensor with JSON entries.""" - config = { - "sensor": { - "platform": "file", - "name": "file2", - "file_path": get_fixture_path("file_value_template.txt", "file"), - "value_template": "{{ value_json.temperature }}", - } + data = { + "platform": "sensor", + "name": "file2", + "file_path": get_fixture_path("file_value_template.txt", "file"), + "value_template": "{{ value_json.temperature }}", } - with patch.object(hass.config, "is_allowed_path", return_value=True): - assert await async_setup_component(hass, "sensor", config) - await hass.async_block_till_done() + entry = MockConfigEntry( + domain=DOMAIN, data=data, title=f"test [{data['file_path']}]" + ) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) state = hass.states.get("sensor.file2") assert state.state == "26" @@ -52,19 +80,19 @@ async def test_file_value_template(hass: HomeAssistant) -> None: @patch("os.path.isfile", Mock(return_value=True)) @patch("os.access", Mock(return_value=True)) -async def test_file_empty(hass: HomeAssistant) -> None: +async def test_file_empty(hass: HomeAssistant, mock_is_allowed_path: MagicMock) -> None: """Test the File sensor with an empty file.""" - config = { - "sensor": { - "platform": "file", - "name": "file3", - "file_path": get_fixture_path("file_empty.txt", "file"), - } + data = { + "platform": "sensor", + "name": "file3", + "file_path": get_fixture_path("file_empty.txt", "file"), } - with patch.object(hass.config, "is_allowed_path", return_value=True): - assert await async_setup_component(hass, "sensor", config) - await hass.async_block_till_done() + entry = MockConfigEntry( + domain=DOMAIN, data=data, title=f"test [{data['file_path']}]" + ) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) state = hass.states.get("sensor.file3") assert state.state == STATE_UNKNOWN @@ -72,18 +100,21 @@ async def test_file_empty(hass: HomeAssistant) -> None: @patch("os.path.isfile", Mock(return_value=True)) @patch("os.access", Mock(return_value=True)) -async def test_file_path_invalid(hass: HomeAssistant) -> None: +@pytest.mark.parametrize("is_allowed", [False]) +async def test_file_path_invalid( + hass: HomeAssistant, mock_is_allowed_path: MagicMock +) -> None: """Test the File sensor with invalid path.""" - config = { - "sensor": { - "platform": "file", - "name": "file4", - "file_path": get_fixture_path("file_value.txt", "file"), - } + data = { + "platform": "sensor", + "name": "file4", + "file_path": get_fixture_path("file_value.txt", "file"), } - with patch.object(hass.config, "is_allowed_path", return_value=False): - assert await async_setup_component(hass, "sensor", config) - await hass.async_block_till_done() + entry = MockConfigEntry( + domain=DOMAIN, data=data, title=f"test [{data['file_path']}]" + ) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) assert len(hass.states.async_entity_ids("sensor")) == 0 From 7b9c624915fdcfc3460cbaed23d168553b840429 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 5 May 2024 00:09:24 +0000 Subject: [PATCH 03/16] Add config flow --- homeassistant/components/file/config_flow.py | 103 +++++++++++++++++-- homeassistant/components/file/const.py | 5 +- homeassistant/components/file/notify.py | 15 +-- homeassistant/components/file/strings.json | 49 +++++++++ 4 files changed, 157 insertions(+), 15 deletions(-) create mode 100644 homeassistant/components/file/strings.json diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index 52f4aae742cb9d..bf84bb4f28aea4 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -1,5 +1,6 @@ """Config flow for file integration.""" +import os from typing import Any import voluptuous as vol @@ -14,16 +15,45 @@ CONF_VALUE_TEMPLATE, Platform, ) -import homeassistant.helpers.config_validation as cv +from homeassistant.helpers.selector import ( + BooleanSelector, + BooleanSelectorConfig, + SelectSelector, + SelectSelectorConfig, + SelectSelectorMode, + TemplateSelector, + TemplateSelectorConfig, + TextSelector, + TextSelectorConfig, + TextSelectorType, +) + +from .const import CONF_TIMESTAMP, DEFAULT_NAME, DOMAIN -from .const import DEFAULT_NAME, DOMAIN +BOOLEAN_SELECTOR = BooleanSelector(BooleanSelectorConfig()) +PLATFORM_SELECTOR = SelectSelector( + SelectSelectorConfig( + options=["notify", "sensor"], + mode=SelectSelectorMode.DROPDOWN, + ) +) +TEMPLATE_SELECTOR = TemplateSelector(TemplateSelectorConfig()) +TEXT_SELECTOR = TextSelector(TextSelectorConfig(type=TextSelectorType.TEXT)) FILE_SENSOR_SCHEMA = vol.Schema( { - vol.Required(CONF_FILE_PATH): cv.isfile, - vol.Optional(CONF_NAME, default="File"): cv.string, - vol.Optional(CONF_VALUE_TEMPLATE): cv.template, - vol.Optional(CONF_UNIT_OF_MEASUREMENT): cv.string, + vol.Optional(CONF_NAME): TEXT_SELECTOR, + vol.Required(CONF_FILE_PATH): TEXT_SELECTOR, + vol.Optional(CONF_VALUE_TEMPLATE): TEMPLATE_SELECTOR, + vol.Optional(CONF_UNIT_OF_MEASUREMENT): TEXT_SELECTOR, + } +) + +FILE_NOTIFY_SCHEMA = vol.Schema( + { + vol.Optional(CONF_NAME): TEXT_SELECTOR, + vol.Required(CONF_FILENAME): TEXT_SELECTOR, + vol.Optional(CONF_TIMESTAMP, default=False): BOOLEAN_SELECTOR, } ) @@ -33,11 +63,72 @@ class FileConfigFlowHandler(ConfigFlow, domain=DOMAIN): VERSION = 1 + async def validate_file_path(self, file_path: str) -> bool: + """Ensure the file path is valid.""" + return await self.hass.async_add_executor_job( + self.hass.config.is_allowed_path, file_path + ) + + async def async_step_user( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle a flow initiated by the user.""" + return self.async_show_menu( + step_id="user", + menu_options=["notify", "sensor"], + ) + + async def async_step_notify( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle file notifier config flow.""" + errors: dict[str, str] = {} + if user_input: + user_input[CONF_PLATFORM] = "notify" + self._async_abort_entries_match( + {key: user_input[key] for key in (CONF_PLATFORM, CONF_FILENAME)} + ) + filepath: str = os.path.join( + self.hass.config.config_dir, user_input[CONF_FILENAME] + ) + if not await self.validate_file_path(filepath): + errors[CONF_FILENAME] = "not_allowed" + else: + name: str = user_input.get(CONF_NAME, DEFAULT_NAME) + title = f"{name} [{user_input[CONF_FILENAME]}]" + return self.async_create_entry(data=user_input, title=title) + + return self.async_show_form( + step_id="notify", data_schema=FILE_NOTIFY_SCHEMA, errors=errors + ) + + async def async_step_sensor( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle file sensor config flow.""" + errors: dict[str, str] = {} + if user_input: + user_input[CONF_PLATFORM] = "sensor" + self._async_abort_entries_match( + {key: user_input[key] for key in (CONF_PLATFORM, CONF_FILE_PATH)} + ) + if not await self.validate_file_path(user_input[CONF_FILENAME]): + errors[CONF_FILE_PATH] = "not_allowed" + else: + name: str = user_input.get(CONF_NAME, DEFAULT_NAME) + title = f"{name} [{user_input[CONF_FILE_PATH]}]" + return self.async_create_entry(data=user_input, title=title) + + return self.async_show_form( + step_id="sensor", data_schema=FILE_SENSOR_SCHEMA, errors=errors + ) + async def async_step_import( self, import_data: dict[str, Any] | None = None ) -> ConfigFlowResult: """Import `file`` config from configuration.yaml.""" assert import_data is not None + self._async_abort_entries_match(import_data) platform = import_data[CONF_PLATFORM] name: str = import_data.get(CONF_NAME, DEFAULT_NAME) file_name: str diff --git a/homeassistant/components/file/const.py b/homeassistant/components/file/const.py index c9a53eeead1ed1..0fa9f8a421bd22 100644 --- a/homeassistant/components/file/const.py +++ b/homeassistant/components/file/const.py @@ -1,9 +1,8 @@ """Constants for the file integration.""" -from typing import Final +DOMAIN = "file" -DOMAIN: Final = "file" +CONF_TIMESTAMP = "timestamp" DEFAULT_NAME = "File" - FILE_ICON = "mdi:file" diff --git a/homeassistant/components/file/notify.py b/homeassistant/components/file/notify.py index 4871b703980f34..476a01abe86dae 100644 --- a/homeassistant/components/file/notify.py +++ b/homeassistant/components/file/notify.py @@ -24,12 +24,10 @@ from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType import homeassistant.util.dt as dt_util -from .const import FILE_ICON +from .const import CONF_TIMESTAMP, FILE_ICON _LOGGER = logging.getLogger(__name__) -CONF_TIMESTAMP = "timestamp" - PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( { vol.Required(CONF_FILENAME): cv.string, @@ -45,9 +43,8 @@ async def async_setup_entry( ) -> None: """Set up the file sensor.""" config: dict[str, Any] = dict(entry.data) - if not await hass.async_add_executor_job( - hass.config.is_allowed_path, config[CONF_FILENAME] - ): + filepath: str = os.path.join(hass.config.config_dir, config[CONF_FILENAME]) + if not await hass.async_add_executor_job(hass.config.is_allowed_path, filepath): _LOGGER.error("'%s' is not an allowed directory", config[CONF_FILENAME]) return async_add_entities([FileNotifyEntity(config)]) @@ -70,6 +67,9 @@ def send_message(self, message: str, title: str | None = None) -> None: """Send a message to a file.""" file: TextIO filepath: str = os.path.join(self.hass.config.config_dir, self.filename) + if not self.hass.config.is_allowed_path(filepath): + _LOGGER.error("'%s' is not an allowed directory", filepath) + return with open(filepath, "a", encoding="utf8") as file: if os.stat(filepath).st_size == 0: title = ( @@ -109,6 +109,9 @@ def send_message(self, message: str = "", **kwargs: Any) -> None: """Send a message to a file.""" file: TextIO filepath: str = os.path.join(self.hass.config.config_dir, self.filename) + if self.hass.config.is_allowed_path(filepath): + _LOGGER.error("'%s' is not an allowed directory", filepath) + return with open(filepath, "a", encoding="utf8") as file: if os.stat(filepath).st_size == 0: title = ( diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json new file mode 100644 index 00000000000000..afd95f995840bc --- /dev/null +++ b/homeassistant/components/file/strings.json @@ -0,0 +1,49 @@ +{ + "config": { + "step": { + "user": { + "description": "Make a choice", + "menu_options": { + "sensor": "Set up a file based sensor", + "notify": "Set up a notification service" + } + }, + "sensor": { + "title": "File sensor", + "description": "Set up a file based sensor", + "data": { + "file_path": "File path", + "name": "Name", + "value_template": "Value template", + "unit_of_measurement": "Unit of measurement" + }, + "data_description": { + "file_path": "The local file path to rereive the sensor value from", + "name": "Name of the file based sensor", + "value_template": "A template to render the the sensors value based on the file content", + "unit_of_measurement": "Unit of measurement for the sensor" + } + }, + "notify": { + "title": "Notification to file service", + "description": "Set up a service that allows to write notification to a file.", + "data": { + "filename": "Filename", + "name": "Name", + "timestamp": "Timestamp" + }, + "data_description": { + "filename": "A relative file path in the hass config root to write the notification to", + "name": "Name of the notify service", + "timestamp": "Add a timestamp to the notification" + } + } + }, + "error": { + "not_allowed": "Access to the selected file path is not allowed" + }, + "abort": { + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" + } + } +} From ce82bb10508ca6e34200a21e74ce97a05edff467 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 5 May 2024 11:59:46 +0000 Subject: [PATCH 04/16] Exception handling and tests --- homeassistant/components/file/const.py | 4 + homeassistant/components/file/notify.py | 95 ++++++++----- homeassistant/components/file/strings.json | 8 ++ tests/components/file/test_notify.py | 148 +++++++++++++++++++-- 4 files changed, 206 insertions(+), 49 deletions(-) diff --git a/homeassistant/components/file/const.py b/homeassistant/components/file/const.py index 0fa9f8a421bd22..e0e6ff2ca6991e 100644 --- a/homeassistant/components/file/const.py +++ b/homeassistant/components/file/const.py @@ -6,3 +6,7 @@ DEFAULT_NAME = "File" FILE_ICON = "mdi:file" + +URL_ALLOWLIST_EXT_DIRS = ( + "https://www.home-assistant.io/integrations/homeassistant/#allowlist_external_dirs" +) diff --git a/homeassistant/components/file/notify.py b/homeassistant/components/file/notify.py index 476a01abe86dae..81c2f0507bb011 100644 --- a/homeassistant/components/file/notify.py +++ b/homeassistant/components/file/notify.py @@ -19,12 +19,13 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_FILENAME, CONF_NAME from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType import homeassistant.util.dt as dt_util -from .const import CONF_TIMESTAMP, FILE_ICON +from .const import CONF_TIMESTAMP, DOMAIN, FILE_ICON, URL_ALLOWLIST_EXT_DIRS _LOGGER = logging.getLogger(__name__) @@ -43,10 +44,6 @@ async def async_setup_entry( ) -> None: """Set up the file sensor.""" config: dict[str, Any] = dict(entry.data) - filepath: str = os.path.join(hass.config.config_dir, config[CONF_FILENAME]) - if not await hass.async_add_executor_job(hass.config.is_allowed_path, filepath): - _LOGGER.error("'%s' is not an allowed directory", config[CONF_FILENAME]) - return async_add_entities([FileNotifyEntity(config)]) @@ -68,21 +65,34 @@ def send_message(self, message: str, title: str | None = None) -> None: file: TextIO filepath: str = os.path.join(self.hass.config.config_dir, self.filename) if not self.hass.config.is_allowed_path(filepath): - _LOGGER.error("'%s' is not an allowed directory", filepath) - return - with open(filepath, "a", encoding="utf8") as file: - if os.stat(filepath).st_size == 0: - title = ( - f"{title or ATTR_TITLE_DEFAULT} notifications (Log" - f" started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" - ) - file.write(title) - - if self.add_timestamp: - text = f"{dt_util.utcnow().isoformat()} {message}\n" - else: - text = f"{message}\n" - file.write(text) + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="dir_not_allowed", + translation_placeholders={ + "filename": filepath, + "url": URL_ALLOWLIST_EXT_DIRS, + }, + ) + try: + with open(filepath, "a", encoding="utf8") as file: + if os.stat(filepath).st_size == 0: + title = ( + f"{title or ATTR_TITLE_DEFAULT} notifications (Log" + f" started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" + ) + file.write(title) + + if self.add_timestamp: + text = f"{dt_util.utcnow().isoformat()} {message}\n" + else: + text = f"{message}\n" + file.write(text) + except Exception as exc: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="write_access_failed", + translation_placeholders={"filename": filepath, "exc": f"{exc!r}"}, + ) from exc def get_service( @@ -109,19 +119,32 @@ def send_message(self, message: str = "", **kwargs: Any) -> None: """Send a message to a file.""" file: TextIO filepath: str = os.path.join(self.hass.config.config_dir, self.filename) - if self.hass.config.is_allowed_path(filepath): - _LOGGER.error("'%s' is not an allowed directory", filepath) - return - with open(filepath, "a", encoding="utf8") as file: - if os.stat(filepath).st_size == 0: - title = ( - f"{kwargs.get(ATTR_TITLE, ATTR_TITLE_DEFAULT)} notifications (Log" - f" started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" - ) - file.write(title) - - if self.add_timestamp: - text = f"{dt_util.utcnow().isoformat()} {message}\n" - else: - text = f"{message}\n" - file.write(text) + if not self.hass.config.is_allowed_path(filepath): + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="dir_not_allowed", + translation_placeholders={ + "filename": filepath, + "url": URL_ALLOWLIST_EXT_DIRS, + }, + ) + try: + with open(filepath, "a", encoding="utf8") as file: + if os.stat(filepath).st_size == 0: + title = ( + f"{kwargs.get(ATTR_TITLE, ATTR_TITLE_DEFAULT)} notifications (Log" + f" started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" + ) + file.write(title) + + if self.add_timestamp: + text = f"{dt_util.utcnow().isoformat()} {message}\n" + else: + text = f"{message}\n" + file.write(text) + except Exception as exc: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="write_access_failed", + translation_placeholders={"filename": filepath, "exc": f"{exc!r}"}, + ) from exc diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json index afd95f995840bc..04108896809576 100644 --- a/homeassistant/components/file/strings.json +++ b/homeassistant/components/file/strings.json @@ -45,5 +45,13 @@ "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" } + }, + "exceptions": { + "dir_not_allowed": { + "message": "Access to {filename} is not allowed, make sure the [directory is allowed]({url})." + }, + "write_access_failed": { + "message": "Write access to {filename} failed: {exc}." + } } } diff --git a/tests/components/file/test_notify.py b/tests/components/file/test_notify.py index cc06c47563cede..a36f23ab92e416 100644 --- a/tests/components/file/test_notify.py +++ b/tests/components/file/test_notify.py @@ -11,6 +11,7 @@ from homeassistant.components.file import DOMAIN from homeassistant.components.notify import ATTR_TITLE_DEFAULT from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers.typing import ConfigType from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util @@ -121,6 +122,102 @@ async def test_notify_file( ] +@pytest.mark.parametrize( + ("domain", "service", "params"), + [(notify.DOMAIN, "test", {"message": "one, two, testing, testing"})], + ids=["legacy"], +) +@pytest.mark.parametrize( + ("is_allowed", "config"), + [ + ( + False, + { + "notify": [ + { + "name": "test", + "platform": "file", + "filename": "mock_file", + } + ] + }, + ), + ], + ids=["not_allowed"], +) +async def test_legacy_notify_file_not_allowed( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_is_allowed_path: MagicMock, + config: ConfigType, + domain: str, + service: str, + params: dict[str, str], +) -> None: + """Test legacy notify file output not allowed.""" + assert await async_setup_component(hass, notify.DOMAIN, config) + await hass.async_block_till_done() + assert await async_setup_component(hass, DOMAIN, config) + await hass.async_block_till_done(wait_background_tasks=True) + + freezer.move_to(dt_util.utcnow()) + + with pytest.raises(ServiceValidationError) as exc: + await hass.services.async_call(domain, service, params, blocking=True) + assert f"{exc.value!r}" == "ServiceValidationError('dir_not_allowed')" + + +@pytest.mark.parametrize( + ("domain", "service", "params"), + [(notify.DOMAIN, "test", {"message": "one, two, testing, testing"})], + ids=["legacy"], +) +@pytest.mark.parametrize( + ("is_allowed", "config"), + [ + ( + True, + { + "notify": [ + { + "name": "test", + "platform": "file", + "filename": "mock_file", + } + ] + }, + ), + ], + ids=["allowed_but_access_failed"], +) +async def test_legacy_notify_file_exception( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_is_allowed_path: MagicMock, + config: ConfigType, + domain: str, + service: str, + params: dict[str, str], +) -> None: + """Test legacy notify file output has exception.""" + assert await async_setup_component(hass, notify.DOMAIN, config) + await hass.async_block_till_done() + assert await async_setup_component(hass, DOMAIN, config) + await hass.async_block_till_done(wait_background_tasks=True) + + freezer.move_to(dt_util.utcnow()) + + m_open = mock_open() + with ( + patch("homeassistant.components.file.notify.open", m_open, create=True), + patch("homeassistant.components.file.notify.os.stat") as mock_st, + ): + mock_st.side_effect = OSError("Access Failed") + with pytest.raises(ServiceValidationError) as exc: + await hass.services.async_call(domain, service, params, blocking=True) + assert f"{exc.value!r}" == "ServiceValidationError('write_access_failed')" + + @pytest.mark.parametrize( ("timestamp", "data"), [ @@ -206,32 +303,58 @@ async def test_notify_file_entry_only_setup( "name": "test", "platform": "notify", "filename": "mock_file", - "timestamp": False, }, False, ), + ], + ids=["not_allowed"], +) +async def test_notify_file_not_allowed_path( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_is_allowed_path: MagicMock, + data: dict[str, Any], +) -> None: + """Test the notify file output is not allowed.""" + domain = notify.DOMAIN + service = "send_message" + params = {"entity_id": "notify.test", "message": "one, two, testing, testing"} + + entry = MockConfigEntry( + domain=DOMAIN, data=data, title=f"test [{data['filename']}]" + ) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + await hass.async_block_till_done(wait_background_tasks=True) + + freezer.move_to(dt_util.utcnow()) + with pytest.raises(ServiceValidationError) as exc: + await hass.services.async_call(domain, service, params, blocking=True) + assert f"{exc.value!r}" == "ServiceValidationError('dir_not_allowed')" + + +@pytest.mark.parametrize( + ("data", "is_allowed"), + [ ( { "name": "test", "platform": "notify", "filename": "mock_file", - "timestamp": True, }, True, ), ], - ids=["not_allowed", "allowed"], + ids=["not_allowed"], ) -async def test_notify_file_not_allowed_path( +async def test_notify_file_write_access_failed( hass: HomeAssistant, freezer: FrozenDateTimeFactory, mock_is_allowed_path: MagicMock, - is_allowed: bool, data: dict[str, Any], ) -> None: - """Test the notify file output.""" - filename = "mock_file" - + """Test the notify file fails.""" domain = notify.DOMAIN service = "send_message" params = {"entity_id": "notify.test", "message": "one, two, testing, testing"} @@ -251,8 +374,7 @@ async def test_notify_file_not_allowed_path( patch("homeassistant.components.file.notify.open", m_open, create=True), patch("homeassistant.components.file.notify.os.stat") as mock_st, ): - mock_st.return_value.st_size = 0 - await hass.services.async_call(domain, service, params, blocking=True) - os.path.join(hass.config.path(), filename) - call_count = {False: 0, True: 1} - assert m_open.call_count == call_count[is_allowed] + mock_st.side_effect = OSError("Access Failed") + with pytest.raises(ServiceValidationError) as exc: + await hass.services.async_call(domain, service, params, blocking=True) + assert f"{exc.value!r}" == "ServiceValidationError('write_access_failed')" From 6e2950288dce808207072cfe2328d3415974055c Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 5 May 2024 12:48:47 +0000 Subject: [PATCH 05/16] Add config flow tests --- homeassistant/components/file/config_flow.py | 2 +- tests/components/file/conftest.py | 11 +- tests/components/file/test_config_flow.py | 147 +++++++++++++++++++ 3 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 tests/components/file/test_config_flow.py diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index bf84bb4f28aea4..234d5e679a3373 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -112,7 +112,7 @@ async def async_step_sensor( self._async_abort_entries_match( {key: user_input[key] for key in (CONF_PLATFORM, CONF_FILE_PATH)} ) - if not await self.validate_file_path(user_input[CONF_FILENAME]): + if not await self.validate_file_path(user_input[CONF_FILE_PATH]): errors[CONF_FILE_PATH] = "not_allowed" else: name: str = user_input.get(CONF_NAME, DEFAULT_NAME) diff --git a/tests/components/file/conftest.py b/tests/components/file/conftest.py index bcd5fb37842fe4..082483266a2c5e 100644 --- a/tests/components/file/conftest.py +++ b/tests/components/file/conftest.py @@ -1,13 +1,22 @@ """Test fixtures for file platform.""" from collections.abc import Generator -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from homeassistant.core import HomeAssistant +@pytest.fixture +def mock_setup_entry() -> Generator[AsyncMock, None, None]: + """Override async_setup_entry.""" + with patch( + "homeassistant.components.file.async_setup_entry", return_value=True + ) as mock_setup_entry: + yield mock_setup_entry + + @pytest.fixture def is_allowed() -> bool: """Parameterize mock_is_allowed_path, default True.""" diff --git a/tests/components/file/test_config_flow.py b/tests/components/file/test_config_flow.py new file mode 100644 index 00000000000000..bf420a9e89e043 --- /dev/null +++ b/tests/components/file/test_config_flow.py @@ -0,0 +1,147 @@ +"""Tests for the file config flow.""" + +from typing import Any +from unittest.mock import AsyncMock + +import pytest + +from homeassistant import config_entries +from homeassistant.components.file import DOMAIN +from homeassistant.core import HomeAssistant +from homeassistant.data_entry_flow import FlowResultType + +from tests.common import MockConfigEntry + +MOCK_CONFIG_NOTIFY = { + "platform": "notify", + "filename": "some_file", + "timestamp": True, +} +MOCK_CONFIG_SENSOR = { + "platform": "sensor", + "file_path": "some/path", + "value_template": "{{ value | round(1) }}", +} + +pytestmark = pytest.mark.usefixtures("mock_setup_entry") + + +@pytest.mark.parametrize( + ("platform", "data"), + [("sensor", MOCK_CONFIG_SENSOR), ("notify", MOCK_CONFIG_NOTIFY)], +) +async def test_form( + hass: HomeAssistant, + mock_setup_entry: AsyncMock, + mock_is_allowed_path: bool, + platform: str, + data: dict[str, Any], +) -> None: + """Test we get the form.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] is FlowResultType.MENU + assert result["step_id"] == "user" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {"next_step_id": platform}, + ) + await hass.async_block_till_done() + + user_input = dict(data) + user_input.pop("platform") + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=user_input + ) + await hass.async_block_till_done() + + assert result2["type"] is FlowResultType.CREATE_ENTRY + assert result2["data"] == data + assert len(mock_setup_entry.mock_calls) == 1 + + +@pytest.mark.parametrize( + ("platform", "data"), + [("sensor", MOCK_CONFIG_SENSOR), ("notify", MOCK_CONFIG_NOTIFY)], +) +async def test_already_configured( + hass: HomeAssistant, + mock_setup_entry: AsyncMock, + mock_is_allowed_path: bool, + platform: str, + data: dict[str, Any], +) -> None: + """Test aborting if the entry is already configured.""" + entry = MockConfigEntry(domain=DOMAIN, data=data) + entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] is FlowResultType.MENU + assert result["step_id"] == "user" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {"next_step_id": platform}, + ) + await hass.async_block_till_done() + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == platform + + user_input = dict(data) + user_input.pop("platform") + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input=user_input, + ) + await hass.async_block_till_done() + + assert result2["type"] is FlowResultType.ABORT + assert result2["reason"] == "already_configured" + + +@pytest.mark.parametrize("is_allowed", [False], ids=["not_allowed"]) +@pytest.mark.parametrize( + ("platform", "data"), + [("sensor", MOCK_CONFIG_SENSOR), ("notify", MOCK_CONFIG_NOTIFY)], +) +async def test_not_allowed( + hass: HomeAssistant, + mock_setup_entry: AsyncMock, + mock_is_allowed_path: bool, + platform: str, + data: dict[str, Any], +) -> None: + """Test aborting if the file path is not allowed.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] is FlowResultType.MENU + assert result["step_id"] == "user" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {"next_step_id": platform}, + ) + await hass.async_block_till_done() + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == platform + + user_input = dict(data) + user_input.pop("platform") + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input=user_input, + ) + await hass.async_block_till_done() + + assert result2["type"] is FlowResultType.FORM + + expected_error: dict[str, Any] = { + "notify": {"filename": "not_allowed"}, + "sensor": {"file_path": "not_allowed"}, + } + assert result2["errors"] == expected_error[platform] From c4f2ea0fee76acfead81ec3d607a0a998cf0759d Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 5 May 2024 13:37:40 +0000 Subject: [PATCH 06/16] Add issue for micration and deprecation --- homeassistant/components/file/__init__.py | 31 +++++++++++++--------- homeassistant/components/file/strings.json | 6 +++++ 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/file/__init__.py b/homeassistant/components/file/__init__.py index efa0c2b00c2f65..109baf9b6c0ead 100644 --- a/homeassistant/components/file/__init__.py +++ b/homeassistant/components/file/__init__.py @@ -3,7 +3,7 @@ from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import CONF_PLATFORM, Platform from homeassistant.core import HomeAssistant -from homeassistant.helpers import config_validation as cv +from homeassistant.helpers import config_validation as cv, issue_registry as ir from homeassistant.helpers.typing import ConfigType from .const import DOMAIN @@ -16,19 +16,24 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up the file integration.""" - # The legacy File notify.notify service is deprecated with HA Core 2024.5.0 - # with HA Core 2024.5.0 and will be removed with HA core 2024.12.0 + # The legacy File notify.notify service is deprecated with HA Core 2024.6.0 + # with HA Core 2024.6.0 and will be removed with HA core 2024.12.0 # The legacy service will coexist together with the new entity platform service - # hass.async_create_task( - # discovery.async_load_platform( - # hass, - # Platform.NOTIFY, - # DOMAIN, - # None, - # config, - # ) - # ) - + ir.async_create_issue( + hass, + DOMAIN, + "deprecated_yaml_notify_migration", + breaks_in_ha_version="2024.12", + is_fixable=False, + issue_domain=DOMAIN, + learn_more_url="https://www.home-assistant.io/integrations/file/", + severity=ir.IssueSeverity.WARNING, + translation_key="deprecated_yaml_notify_migration", + translation_placeholders={ + "domain": DOMAIN, + "integration_title": "File", + }, + ) if hass.config_entries.async_entries(DOMAIN): # We skip import in case we already have config entries return True diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json index 04108896809576..10499da57880a4 100644 --- a/homeassistant/components/file/strings.json +++ b/homeassistant/components/file/strings.json @@ -53,5 +53,11 @@ "write_access_failed": { "message": "Write access to {filename} failed: {exc}." } + }, + "issues": { + "deprecated_yaml_notify_migration": { + "title": "The {integration_title} YAML configuration is being removed", + "description": "Configuring {integration_title} using YAML is being removed.\n\nYour existing YAML configuration has been imported into the UI automatically.\n\nNotification services are migrated to be used with the `notify.send_message` entity service. Please adjust your automations to use the new service, as the leagcy services will stop working as soon as the `{domain}` configuration is removed from your configuration.yaml file.\n\nRemove the `{domain}` configuration from your configuration.yaml file and restart Home Assistant to fix this issue." + } } } From 78305559265b888ee0989bbfadab69fa67af7eb2 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 5 May 2024 16:10:30 +0000 Subject: [PATCH 07/16] Check whole entry data for uniqueness --- homeassistant/components/file/config_flow.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index 234d5e679a3373..0d24389462a4f6 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -85,9 +85,7 @@ async def async_step_notify( errors: dict[str, str] = {} if user_input: user_input[CONF_PLATFORM] = "notify" - self._async_abort_entries_match( - {key: user_input[key] for key in (CONF_PLATFORM, CONF_FILENAME)} - ) + self._async_abort_entries_match(user_input) filepath: str = os.path.join( self.hass.config.config_dir, user_input[CONF_FILENAME] ) @@ -109,9 +107,7 @@ async def async_step_sensor( errors: dict[str, str] = {} if user_input: user_input[CONF_PLATFORM] = "sensor" - self._async_abort_entries_match( - {key: user_input[key] for key in (CONF_PLATFORM, CONF_FILE_PATH)} - ) + self._async_abort_entries_match(user_input) if not await self.validate_file_path(user_input[CONF_FILE_PATH]): errors[CONF_FILE_PATH] = "not_allowed" else: From f8b475c94e3e75081c6f8fd428f146c52777a5c7 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 8 May 2024 10:08:38 +0000 Subject: [PATCH 08/16] Revert changes change new notify entity --- homeassistant/components/file/__init__.py | 46 +++++++++---- homeassistant/components/file/notify.py | 77 +++------------------- homeassistant/components/file/strings.json | 6 -- tests/components/file/test_notify.py | 29 ++++---- 4 files changed, 55 insertions(+), 103 deletions(-) diff --git a/homeassistant/components/file/__init__.py b/homeassistant/components/file/__init__.py index 109baf9b6c0ead..9f8908b46fa34d 100644 --- a/homeassistant/components/file/__init__.py +++ b/homeassistant/components/file/__init__.py @@ -2,33 +2,38 @@ from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import CONF_PLATFORM, Platform -from homeassistant.core import HomeAssistant -from homeassistant.helpers import config_validation as cv, issue_registry as ir +from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant +from homeassistant.helpers import ( + config_validation as cv, + discovery, + issue_registry as ir, +) from homeassistant.helpers.typing import ConfigType from .const import DOMAIN CONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN) -PLATFORMS = [Platform.NOTIFY, Platform.SENSOR] +PLATFORMS = [Platform.SENSOR] + +YAML_PLATFORMS = [Platform.NOTIFY, Platform.SENSOR] async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up the file integration.""" - # The legacy File notify.notify service is deprecated with HA Core 2024.6.0 - # with HA Core 2024.6.0 and will be removed with HA core 2024.12.0 - # The legacy service will coexist together with the new entity platform service + # The YAML config was imported with HA Core 2024.6.0 and will be removed with + # HA Core 2024.12 ir.async_create_issue( hass, - DOMAIN, - "deprecated_yaml_notify_migration", + HOMEASSISTANT_DOMAIN, + f"deprecated_yaml_{DOMAIN}", breaks_in_ha_version="2024.12", is_fixable=False, issue_domain=DOMAIN, learn_more_url="https://www.home-assistant.io/integrations/file/", severity=ir.IssueSeverity.WARNING, - translation_key="deprecated_yaml_notify_migration", + translation_key="deprecated_yaml", translation_placeholders={ "domain": DOMAIN, "integration_title": "File", @@ -42,7 +47,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: config_data: dict[str, list[ConfigType]] = { domain: [item for item in config[domain] if item.pop(CONF_PLATFORM) == DOMAIN] for domain in config - if domain in PLATFORMS + if domain in YAML_PLATFORMS } for domain, items in config_data.items(): @@ -61,9 +66,24 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up a file component entry.""" - await hass.config_entries.async_forward_entry_setups( - entry, [Platform(entry.data[CONF_PLATFORM])] - ) + if entry.data[CONF_PLATFORM] in PLATFORMS: + await hass.config_entries.async_forward_entry_setups( + entry, [Platform(entry.data[CONF_PLATFORM])] + ) + else: + # The notify platform is not yet set up as entry, so + # forward setup config through discovery to ensure setup notify service. + # This is needed as long as the legacy service is not migrated + hass.async_create_task( + discovery.async_load_platform( + hass, + Platform.NOTIFY, + DOMAIN, + dict(entry.data), + {}, + ) + ) + return True diff --git a/homeassistant/components/file/notify.py b/homeassistant/components/file/notify.py index 81c2f0507bb011..bbab8dc094dc84 100644 --- a/homeassistant/components/file/notify.py +++ b/homeassistant/components/file/notify.py @@ -13,19 +13,15 @@ ATTR_TITLE_DEFAULT, PLATFORM_SCHEMA, BaseNotificationService, - NotifyEntity, - NotifyEntityFeature, ) -from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_FILENAME, CONF_NAME +from homeassistant.const import CONF_FILENAME from homeassistant.core import HomeAssistant from homeassistant.exceptions import ServiceValidationError import homeassistant.helpers.config_validation as cv -from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType import homeassistant.util.dt as dt_util -from .const import CONF_TIMESTAMP, DOMAIN, FILE_ICON, URL_ALLOWLIST_EXT_DIRS +from .const import CONF_TIMESTAMP, DOMAIN, URL_ALLOWLIST_EXT_DIRS _LOGGER = logging.getLogger(__name__) @@ -37,72 +33,17 @@ ) -async def async_setup_entry( - hass: HomeAssistant, - entry: ConfigEntry, - async_add_entities: AddEntitiesCallback, -) -> None: - """Set up the file sensor.""" - config: dict[str, Any] = dict(entry.data) - async_add_entities([FileNotifyEntity(config)]) - - -class FileNotifyEntity(NotifyEntity): - """Implement the notification entity platform for the File service.""" - - _attr_icon = FILE_ICON - _attr_supported_features = NotifyEntityFeature.TITLE - - def __init__(self, config: dict[str, Any]) -> None: - """Initialize the service.""" - self.filename: str = config[CONF_FILENAME] - self.add_timestamp: bool = config.get(CONF_TIMESTAMP, False) - self._attr_name = config.get(CONF_NAME, config[CONF_FILENAME]) - self._attr_unique_id = f"notify_{config[CONF_FILENAME]}" - - def send_message(self, message: str, title: str | None = None) -> None: - """Send a message to a file.""" - file: TextIO - filepath: str = os.path.join(self.hass.config.config_dir, self.filename) - if not self.hass.config.is_allowed_path(filepath): - raise ServiceValidationError( - translation_domain=DOMAIN, - translation_key="dir_not_allowed", - translation_placeholders={ - "filename": filepath, - "url": URL_ALLOWLIST_EXT_DIRS, - }, - ) - try: - with open(filepath, "a", encoding="utf8") as file: - if os.stat(filepath).st_size == 0: - title = ( - f"{title or ATTR_TITLE_DEFAULT} notifications (Log" - f" started: {dt_util.utcnow().isoformat()})\n{'-' * 80}\n" - ) - file.write(title) - - if self.add_timestamp: - text = f"{dt_util.utcnow().isoformat()} {message}\n" - else: - text = f"{message}\n" - file.write(text) - except Exception as exc: - raise ServiceValidationError( - translation_domain=DOMAIN, - translation_key="write_access_failed", - translation_placeholders={"filename": filepath, "exc": f"{exc!r}"}, - ) from exc - - -def get_service( +async def async_get_service( hass: HomeAssistant, config: ConfigType, discovery_info: DiscoveryInfoType | None = None, -) -> FileNotificationService: +) -> FileNotificationService | None: """Get the file notification service.""" - filename: str = config[CONF_FILENAME] - timestamp: bool = config[CONF_TIMESTAMP] + if discovery_info is None: + # We only set up through discovery + return None + filename: str = discovery_info[CONF_FILENAME] + timestamp: bool = discovery_info.get(CONF_TIMESTAMP, False) return FileNotificationService(filename, timestamp) diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json index 10499da57880a4..04108896809576 100644 --- a/homeassistant/components/file/strings.json +++ b/homeassistant/components/file/strings.json @@ -53,11 +53,5 @@ "write_access_failed": { "message": "Write access to {filename} failed: {exc}." } - }, - "issues": { - "deprecated_yaml_notify_migration": { - "title": "The {integration_title} YAML configuration is being removed", - "description": "Configuring {integration_title} using YAML is being removed.\n\nYour existing YAML configuration has been imported into the UI automatically.\n\nNotification services are migrated to be used with the `notify.send_message` entity service. Please adjust your automations to use the new service, as the leagcy services will stop working as soon as the `{domain}` configuration is removed from your configuration.yaml file.\n\nRemove the `{domain}` configuration from your configuration.yaml file and restart Home Assistant to fix this issue." - } } } diff --git a/tests/components/file/test_notify.py b/tests/components/file/test_notify.py index a36f23ab92e416..968708e6c6c940 100644 --- a/tests/components/file/test_notify.py +++ b/tests/components/file/test_notify.py @@ -32,13 +32,8 @@ async def test_bad_config(hass: HomeAssistant) -> None: ("domain", "service", "params"), [ (notify.DOMAIN, "test", {"message": "one, two, testing, testing"}), - ( - notify.DOMAIN, - "send_message", - {"entity_id": "notify.test", "message": "one, two, testing, testing"}, - ), ], - ids=["legacy", "entity"], + ids=["legacy"], ) @pytest.mark.parametrize( ("timestamp", "config"), @@ -242,19 +237,19 @@ async def test_legacy_notify_file_exception( ], ids=["no_timestamp", "timestamp"], ) -async def test_notify_file_entry_only_setup( +async def test_legacy_notify_file_entry_only_setup( hass: HomeAssistant, freezer: FrozenDateTimeFactory, timestamp: bool, mock_is_allowed_path: MagicMock, data: dict[str, Any], ) -> None: - """Test the notify file output.""" + """Test the legacy notify file output in entry only setup.""" filename = "mock_file" domain = notify.DOMAIN - service = "send_message" - params = {"entity_id": "notify.test", "message": "one, two, testing, testing"} + service = "test" + params = {"message": "one, two, testing, testing"} message = params["message"] entry = MockConfigEntry( @@ -262,6 +257,8 @@ async def test_notify_file_entry_only_setup( ) entry.add_to_hass(hass) await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + await hass.async_block_till_done(wait_background_tasks=True) freezer.move_to(dt_util.utcnow()) @@ -309,16 +306,16 @@ async def test_notify_file_entry_only_setup( ], ids=["not_allowed"], ) -async def test_notify_file_not_allowed_path( +async def test_legacy_notify_file_not_allowed_path( hass: HomeAssistant, freezer: FrozenDateTimeFactory, mock_is_allowed_path: MagicMock, data: dict[str, Any], ) -> None: - """Test the notify file output is not allowed.""" + """Test the legacy notify file output is not allowed.""" domain = notify.DOMAIN - service = "send_message" - params = {"entity_id": "notify.test", "message": "one, two, testing, testing"} + service = "test" + params = {"message": "one, two, testing, testing"} entry = MockConfigEntry( domain=DOMAIN, data=data, title=f"test [{data['filename']}]" @@ -356,8 +353,8 @@ async def test_notify_file_write_access_failed( ) -> None: """Test the notify file fails.""" domain = notify.DOMAIN - service = "send_message" - params = {"entity_id": "notify.test", "message": "one, two, testing, testing"} + service = "test" + params = {"message": "one, two, testing, testing"} entry = MockConfigEntry( domain=DOMAIN, data=data, title=f"test [{data['filename']}]" From 05c9d903c9f07c3178b3c4fb65e10a423945a4c9 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 8 May 2024 22:58:08 +0000 Subject: [PATCH 09/16] Follow up on code review --- homeassistant/components/file/__init__.py | 22 +++++- homeassistant/components/file/config_flow.py | 14 +--- homeassistant/components/file/const.py | 4 -- homeassistant/components/file/notify.py | 11 +-- homeassistant/components/file/sensor.py | 4 -- homeassistant/components/file/strings.json | 6 +- tests/components/file/test_notify.py | 72 +++----------------- 7 files changed, 33 insertions(+), 100 deletions(-) diff --git a/homeassistant/components/file/__init__.py b/homeassistant/components/file/__init__.py index 9f8908b46fa34d..f49b38f59b2e52 100644 --- a/homeassistant/components/file/__init__.py +++ b/homeassistant/components/file/__init__.py @@ -1,8 +1,11 @@ """The file component.""" +import os + from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry -from homeassistant.const import CONF_PLATFORM, Platform +from homeassistant.const import CONF_FILE_PATH, CONF_FILENAME, CONF_PLATFORM, Platform from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import ( config_validation as cv, discovery, @@ -66,6 +69,21 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up a file component entry.""" + config = dict(entry.data) + filepath: str = "" + if (platform := config[CONF_PLATFORM]) == Platform.NOTIFY: + filepath = os.path.join(hass.config.config_dir, config[CONF_FILENAME]) + elif platform == Platform.SENSOR: + filepath = config[CONF_FILE_PATH] + if filepath and not await hass.async_add_executor_job( + hass.config.is_allowed_path, filepath + ): + raise ConfigEntryNotReady( + translation_domain=DOMAIN, + translation_key="dir_not_allowed", + translation_placeholders={"filename": filepath}, + ) + if entry.data[CONF_PLATFORM] in PLATFORMS: await hass.config_entries.async_forward_entry_setups( entry, [Platform(entry.data[CONF_PLATFORM])] @@ -79,7 +97,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: hass, Platform.NOTIFY, DOMAIN, - dict(entry.data), + config, {}, ) ) diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index 0d24389462a4f6..44ced6a6654d3f 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -18,9 +18,6 @@ from homeassistant.helpers.selector import ( BooleanSelector, BooleanSelectorConfig, - SelectSelector, - SelectSelectorConfig, - SelectSelectorMode, TemplateSelector, TemplateSelectorConfig, TextSelector, @@ -31,18 +28,11 @@ from .const import CONF_TIMESTAMP, DEFAULT_NAME, DOMAIN BOOLEAN_SELECTOR = BooleanSelector(BooleanSelectorConfig()) -PLATFORM_SELECTOR = SelectSelector( - SelectSelectorConfig( - options=["notify", "sensor"], - mode=SelectSelectorMode.DROPDOWN, - ) -) TEMPLATE_SELECTOR = TemplateSelector(TemplateSelectorConfig()) TEXT_SELECTOR = TextSelector(TextSelectorConfig(type=TextSelectorType.TEXT)) FILE_SENSOR_SCHEMA = vol.Schema( { - vol.Optional(CONF_NAME): TEXT_SELECTOR, vol.Required(CONF_FILE_PATH): TEXT_SELECTOR, vol.Optional(CONF_VALUE_TEMPLATE): TEMPLATE_SELECTOR, vol.Optional(CONF_UNIT_OF_MEASUREMENT): TEXT_SELECTOR, @@ -51,7 +41,6 @@ FILE_NOTIFY_SCHEMA = vol.Schema( { - vol.Optional(CONF_NAME): TEXT_SELECTOR, vol.Required(CONF_FILENAME): TEXT_SELECTOR, vol.Optional(CONF_TIMESTAMP, default=False): BOOLEAN_SELECTOR, } @@ -111,8 +100,7 @@ async def async_step_sensor( if not await self.validate_file_path(user_input[CONF_FILE_PATH]): errors[CONF_FILE_PATH] = "not_allowed" else: - name: str = user_input.get(CONF_NAME, DEFAULT_NAME) - title = f"{name} [{user_input[CONF_FILE_PATH]}]" + title = f"{DEFAULT_NAME} [{user_input[CONF_FILE_PATH]}]" return self.async_create_entry(data=user_input, title=title) return self.async_show_form( diff --git a/homeassistant/components/file/const.py b/homeassistant/components/file/const.py index e0e6ff2ca6991e..0fa9f8a421bd22 100644 --- a/homeassistant/components/file/const.py +++ b/homeassistant/components/file/const.py @@ -6,7 +6,3 @@ DEFAULT_NAME = "File" FILE_ICON = "mdi:file" - -URL_ALLOWLIST_EXT_DIRS = ( - "https://www.home-assistant.io/integrations/homeassistant/#allowlist_external_dirs" -) diff --git a/homeassistant/components/file/notify.py b/homeassistant/components/file/notify.py index bbab8dc094dc84..e34e33e9919b60 100644 --- a/homeassistant/components/file/notify.py +++ b/homeassistant/components/file/notify.py @@ -21,7 +21,7 @@ from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType import homeassistant.util.dt as dt_util -from .const import CONF_TIMESTAMP, DOMAIN, URL_ALLOWLIST_EXT_DIRS +from .const import CONF_TIMESTAMP, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -60,15 +60,6 @@ def send_message(self, message: str = "", **kwargs: Any) -> None: """Send a message to a file.""" file: TextIO filepath: str = os.path.join(self.hass.config.config_dir, self.filename) - if not self.hass.config.is_allowed_path(filepath): - raise ServiceValidationError( - translation_domain=DOMAIN, - translation_key="dir_not_allowed", - translation_placeholders={ - "filename": filepath, - "url": URL_ALLOWLIST_EXT_DIRS, - }, - ) try: with open(filepath, "a", encoding="utf8") as file: if os.stat(filepath).st_size == 0: diff --git a/homeassistant/components/file/sensor.py b/homeassistant/components/file/sensor.py index 0a1208c3fbd6d4..95599d5c541428 100644 --- a/homeassistant/components/file/sensor.py +++ b/homeassistant/components/file/sensor.py @@ -65,10 +65,6 @@ async def async_setup_entry( if CONF_VALUE_TEMPLATE in config: value_template = Template(config[CONF_VALUE_TEMPLATE], hass) - if not await hass.async_add_executor_job(hass.config.is_allowed_path, file_path): - _LOGGER.error("'%s' is not an allowed directory", file_path) - return - async_add_entities([FileSensor(name, file_path, unit, value_template)], True) diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json index 04108896809576..2cc54c63b040e3 100644 --- a/homeassistant/components/file/strings.json +++ b/homeassistant/components/file/strings.json @@ -13,13 +13,11 @@ "description": "Set up a file based sensor", "data": { "file_path": "File path", - "name": "Name", "value_template": "Value template", "unit_of_measurement": "Unit of measurement" }, "data_description": { - "file_path": "The local file path to rereive the sensor value from", - "name": "Name of the file based sensor", + "file_path": "The local file path to retrieve the sensor value from", "value_template": "A template to render the the sensors value based on the file content", "unit_of_measurement": "Unit of measurement for the sensor" } @@ -48,7 +46,7 @@ }, "exceptions": { "dir_not_allowed": { - "message": "Access to {filename} is not allowed, make sure the [directory is allowed]({url})." + "message": "Access to {filename} is not allowed." }, "write_access_failed": { "message": "Write access to {filename} failed: {exc}." diff --git a/tests/components/file/test_notify.py b/tests/components/file/test_notify.py index 968708e6c6c940..e5697f9edaf181 100644 --- a/tests/components/file/test_notify.py +++ b/tests/components/file/test_notify.py @@ -117,51 +117,6 @@ async def test_notify_file( ] -@pytest.mark.parametrize( - ("domain", "service", "params"), - [(notify.DOMAIN, "test", {"message": "one, two, testing, testing"})], - ids=["legacy"], -) -@pytest.mark.parametrize( - ("is_allowed", "config"), - [ - ( - False, - { - "notify": [ - { - "name": "test", - "platform": "file", - "filename": "mock_file", - } - ] - }, - ), - ], - ids=["not_allowed"], -) -async def test_legacy_notify_file_not_allowed( - hass: HomeAssistant, - freezer: FrozenDateTimeFactory, - mock_is_allowed_path: MagicMock, - config: ConfigType, - domain: str, - service: str, - params: dict[str, str], -) -> None: - """Test legacy notify file output not allowed.""" - assert await async_setup_component(hass, notify.DOMAIN, config) - await hass.async_block_till_done() - assert await async_setup_component(hass, DOMAIN, config) - await hass.async_block_till_done(wait_background_tasks=True) - - freezer.move_to(dt_util.utcnow()) - - with pytest.raises(ServiceValidationError) as exc: - await hass.services.async_call(domain, service, params, blocking=True) - assert f"{exc.value!r}" == "ServiceValidationError('dir_not_allowed')" - - @pytest.mark.parametrize( ("domain", "service", "params"), [(notify.DOMAIN, "test", {"message": "one, two, testing, testing"})], @@ -293,42 +248,33 @@ async def test_legacy_notify_file_entry_only_setup( @pytest.mark.parametrize( - ("data", "is_allowed"), + ("is_allowed", "config"), [ ( + False, { "name": "test", "platform": "notify", "filename": "mock_file", }, - False, ), ], ids=["not_allowed"], ) -async def test_legacy_notify_file_not_allowed_path( +async def test_legacy_notify_file_not_allowed( hass: HomeAssistant, - freezer: FrozenDateTimeFactory, + caplog: pytest.LogCaptureFixture, mock_is_allowed_path: MagicMock, - data: dict[str, Any], + config: dict[str, Any], ) -> None: - """Test the legacy notify file output is not allowed.""" - domain = notify.DOMAIN - service = "test" - params = {"message": "one, two, testing, testing"} - + """Test legacy notify file output not allowed.""" entry = MockConfigEntry( - domain=DOMAIN, data=data, title=f"test [{data['filename']}]" + domain=DOMAIN, data=config, title=f"test [{config['filename']}]" ) entry.add_to_hass(hass) - await hass.config_entries.async_setup(entry.entry_id) - await hass.async_block_till_done() + assert not await hass.config_entries.async_setup(entry.entry_id) await hass.async_block_till_done(wait_background_tasks=True) - - freezer.move_to(dt_util.utcnow()) - with pytest.raises(ServiceValidationError) as exc: - await hass.services.async_call(domain, service, params, blocking=True) - assert f"{exc.value!r}" == "ServiceValidationError('dir_not_allowed')" + assert "is not allowed" in caplog.text @pytest.mark.parametrize( From 4bb7c079ae7fb751a7822a4fe363a3e6f9536800 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 8 May 2024 23:02:50 +0000 Subject: [PATCH 10/16] Keep name service option --- homeassistant/components/file/config_flow.py | 1 + 1 file changed, 1 insertion(+) diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index 44ced6a6654d3f..edf62143c29637 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -41,6 +41,7 @@ FILE_NOTIFY_SCHEMA = vol.Schema( { + vol.Optional(CONF_NAME): TEXT_SELECTOR, vol.Required(CONF_FILENAME): TEXT_SELECTOR, vol.Optional(CONF_TIMESTAMP, default=False): BOOLEAN_SELECTOR, } From 99c047a0c875e63dcc43b8c4158a6e39392d6e64 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 8 May 2024 23:53:30 +0000 Subject: [PATCH 11/16] Also keep sensor name --- homeassistant/components/file/config_flow.py | 1 + homeassistant/components/file/sensor.py | 3 ++- homeassistant/components/file/strings.json | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index edf62143c29637..15fc6ac7c3e1e6 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -33,6 +33,7 @@ FILE_SENSOR_SCHEMA = vol.Schema( { + vol.Optional(CONF_NAME): TEXT_SELECTOR, vol.Required(CONF_FILE_PATH): TEXT_SELECTOR, vol.Optional(CONF_VALUE_TEMPLATE): TEMPLATE_SELECTOR, vol.Optional(CONF_UNIT_OF_MEASUREMENT): TEXT_SELECTOR, diff --git a/homeassistant/components/file/sensor.py b/homeassistant/components/file/sensor.py index 95599d5c541428..0a31cbf0391c2b 100644 --- a/homeassistant/components/file/sensor.py +++ b/homeassistant/components/file/sensor.py @@ -21,6 +21,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.template import Template from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.util import slugify from .const import DEFAULT_NAME, FILE_ICON @@ -85,7 +86,7 @@ def __init__( self._file_path = file_path self._attr_native_unit_of_measurement = unit_of_measurement self._val_tpl = value_template - self._attr_unique_id = f"sensor_{file_path}" + self._attr_unique_id = slugify(f"sensor_{file_path}") def update(self) -> None: """Get the latest entry from a file and updates the state.""" diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json index 2cc54c63b040e3..117d4660826524 100644 --- a/homeassistant/components/file/strings.json +++ b/homeassistant/components/file/strings.json @@ -12,11 +12,13 @@ "title": "File sensor", "description": "Set up a file based sensor", "data": { + "name": "Name", "file_path": "File path", "value_template": "Value template", "unit_of_measurement": "Unit of measurement" }, "data_description": { + "name": "Name of the file based sensor", "file_path": "The local file path to retrieve the sensor value from", "value_template": "A template to render the the sensors value based on the file content", "unit_of_measurement": "Unit of measurement for the sensor" From 9f60e9a0288eb4ac6c5651eab2e7f05039cd79f5 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 8 May 2024 23:56:18 +0000 Subject: [PATCH 12/16] Make name unique --- homeassistant/components/file/sensor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/file/sensor.py b/homeassistant/components/file/sensor.py index 0a31cbf0391c2b..f81b30e03cd0c6 100644 --- a/homeassistant/components/file/sensor.py +++ b/homeassistant/components/file/sensor.py @@ -86,7 +86,7 @@ def __init__( self._file_path = file_path self._attr_native_unit_of_measurement = unit_of_measurement self._val_tpl = value_template - self._attr_unique_id = slugify(f"sensor_{file_path}") + self._attr_unique_id = slugify(f"{name}_{file_path}") def update(self) -> None: """Get the latest entry from a file and updates the state.""" From 1864d9b99579cf53bf3276dd14fd03fd25862c9b Mon Sep 17 00:00:00 2001 From: jbouwh Date: Thu, 9 May 2024 20:41:14 +0000 Subject: [PATCH 13/16] Follow up comment --- homeassistant/components/file/__init__.py | 43 ++++++++------------ homeassistant/components/file/config_flow.py | 26 ++++++------ homeassistant/components/file/notify.py | 14 ++++--- homeassistant/components/file/strings.json | 4 +- tests/components/file/test_config_flow.py | 11 ++--- tests/components/file/test_notify.py | 17 ++++---- 6 files changed, 51 insertions(+), 64 deletions(-) diff --git a/homeassistant/components/file/__init__.py b/homeassistant/components/file/__init__.py index f49b38f59b2e52..82e12ee5d165c3 100644 --- a/homeassistant/components/file/__init__.py +++ b/homeassistant/components/file/__init__.py @@ -1,9 +1,7 @@ """The file component.""" -import os - from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry -from homeassistant.const import CONF_FILE_PATH, CONF_FILENAME, CONF_PLATFORM, Platform +from homeassistant.const import CONF_FILE_PATH, CONF_PLATFORM, Platform from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import ( @@ -25,13 +23,16 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up the file integration.""" + if hass.config_entries.async_entries(DOMAIN): + # We skip import in case we already have config entries + return True # The YAML config was imported with HA Core 2024.6.0 and will be removed with # HA Core 2024.12 ir.async_create_issue( hass, HOMEASSISTANT_DOMAIN, f"deprecated_yaml_{DOMAIN}", - breaks_in_ha_version="2024.12", + breaks_in_ha_version="2024.12.0", is_fixable=False, issue_domain=DOMAIN, learn_more_url="https://www.home-assistant.io/integrations/file/", @@ -42,27 +43,19 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: "integration_title": "File", }, ) - if hass.config_entries.async_entries(DOMAIN): - # We skip import in case we already have config entries - return True - # Prepare to import the YAML config into separate config entries - config_data: dict[str, list[ConfigType]] = { - domain: [item for item in config[domain] if item.pop(CONF_PLATFORM) == DOMAIN] - for domain in config - if domain in YAML_PLATFORMS - } - - for domain, items in config_data.items(): + # Import the YAML config into separate config entries + for domain, items in config.items(): for item in items: - item[CONF_PLATFORM] = domain - hass.async_create_task( - hass.config_entries.flow.async_init( - DOMAIN, - context={"source": SOURCE_IMPORT}, - data=item, + if item[CONF_PLATFORM] == DOMAIN: + item[CONF_PLATFORM] = domain + hass.async_create_task( + hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_IMPORT}, + data=item, + ) ) - ) return True @@ -70,11 +63,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up a file component entry.""" config = dict(entry.data) - filepath: str = "" - if (platform := config[CONF_PLATFORM]) == Platform.NOTIFY: - filepath = os.path.join(hass.config.config_dir, config[CONF_FILENAME]) - elif platform == Platform.SENSOR: - filepath = config[CONF_FILE_PATH] + filepath: str = config[CONF_FILE_PATH] if filepath and not await hass.async_add_executor_job( hass.config.is_allowed_path, filepath ): diff --git a/homeassistant/components/file/config_flow.py b/homeassistant/components/file/config_flow.py index 15fc6ac7c3e1e6..9c6bcb4df003eb 100644 --- a/homeassistant/components/file/config_flow.py +++ b/homeassistant/components/file/config_flow.py @@ -33,7 +33,7 @@ FILE_SENSOR_SCHEMA = vol.Schema( { - vol.Optional(CONF_NAME): TEXT_SELECTOR, + vol.Optional(CONF_NAME, default=DEFAULT_NAME): TEXT_SELECTOR, vol.Required(CONF_FILE_PATH): TEXT_SELECTOR, vol.Optional(CONF_VALUE_TEMPLATE): TEMPLATE_SELECTOR, vol.Optional(CONF_UNIT_OF_MEASUREMENT): TEXT_SELECTOR, @@ -42,8 +42,8 @@ FILE_NOTIFY_SCHEMA = vol.Schema( { - vol.Optional(CONF_NAME): TEXT_SELECTOR, - vol.Required(CONF_FILENAME): TEXT_SELECTOR, + vol.Optional(CONF_NAME, default=DEFAULT_NAME): TEXT_SELECTOR, + vol.Required(CONF_FILE_PATH): TEXT_SELECTOR, vol.Optional(CONF_TIMESTAMP, default=False): BOOLEAN_SELECTOR, } ) @@ -77,14 +77,11 @@ async def async_step_notify( if user_input: user_input[CONF_PLATFORM] = "notify" self._async_abort_entries_match(user_input) - filepath: str = os.path.join( - self.hass.config.config_dir, user_input[CONF_FILENAME] - ) - if not await self.validate_file_path(filepath): - errors[CONF_FILENAME] = "not_allowed" + if not await self.validate_file_path(user_input[CONF_FILE_PATH]): + errors[CONF_FILE_PATH] = "not_allowed" else: name: str = user_input.get(CONF_NAME, DEFAULT_NAME) - title = f"{name} [{user_input[CONF_FILENAME]}]" + title = f"{name} [{user_input[CONF_FILE_PATH]}]" return self.async_create_entry(data=user_input, title=title) return self.async_show_form( @@ -102,7 +99,8 @@ async def async_step_sensor( if not await self.validate_file_path(user_input[CONF_FILE_PATH]): errors[CONF_FILE_PATH] = "not_allowed" else: - title = f"{DEFAULT_NAME} [{user_input[CONF_FILE_PATH]}]" + name: str = user_input.get(CONF_NAME, DEFAULT_NAME) + title = f"{name} [{user_input[CONF_FILE_PATH]}]" return self.async_create_entry(data=user_input, title=title) return self.async_show_form( @@ -119,8 +117,10 @@ async def async_step_import( name: str = import_data.get(CONF_NAME, DEFAULT_NAME) file_name: str if platform == Platform.NOTIFY: - file_name = import_data[CONF_FILENAME] + file_name = import_data.pop(CONF_FILENAME) + file_path: str = os.path.join(self.hass.config.config_dir, file_name) + import_data[CONF_FILE_PATH] = file_path else: - file_name = import_data[CONF_FILE_PATH] - title = f"{name} [{file_name}]" + file_path = import_data[CONF_FILE_PATH] + title = f"{name} [{file_path}]" return self.async_create_entry(title=title, data=import_data) diff --git a/homeassistant/components/file/notify.py b/homeassistant/components/file/notify.py index e34e33e9919b60..34d8e1e978d6cc 100644 --- a/homeassistant/components/file/notify.py +++ b/homeassistant/components/file/notify.py @@ -14,7 +14,7 @@ PLATFORM_SCHEMA, BaseNotificationService, ) -from homeassistant.const import CONF_FILENAME +from homeassistant.const import CONF_FILE_PATH, CONF_FILENAME from homeassistant.core import HomeAssistant from homeassistant.exceptions import ServiceValidationError import homeassistant.helpers.config_validation as cv @@ -25,6 +25,8 @@ _LOGGER = logging.getLogger(__name__) +# The legacy platform schema uses a filename, after import +# The full file path is stored in the config entry PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( { vol.Required(CONF_FILENAME): cv.string, @@ -42,24 +44,24 @@ async def async_get_service( if discovery_info is None: # We only set up through discovery return None - filename: str = discovery_info[CONF_FILENAME] + file_path: str = discovery_info[CONF_FILE_PATH] timestamp: bool = discovery_info.get(CONF_TIMESTAMP, False) - return FileNotificationService(filename, timestamp) + return FileNotificationService(file_path, timestamp) class FileNotificationService(BaseNotificationService): """Implement the notification service for the File service.""" - def __init__(self, filename: str, add_timestamp: bool) -> None: + def __init__(self, file_path: str, add_timestamp: bool) -> None: """Initialize the service.""" - self.filename = filename + self._file_path = file_path self.add_timestamp = add_timestamp def send_message(self, message: str = "", **kwargs: Any) -> None: """Send a message to a file.""" file: TextIO - filepath: str = os.path.join(self.hass.config.config_dir, self.filename) + filepath = self._file_path try: with open(filepath, "a", encoding="utf8") as file: if os.stat(filepath).st_size == 0: diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json index 117d4660826524..03963dbac5c77d 100644 --- a/homeassistant/components/file/strings.json +++ b/homeassistant/components/file/strings.json @@ -28,12 +28,12 @@ "title": "Notification to file service", "description": "Set up a service that allows to write notification to a file.", "data": { - "filename": "Filename", + "file_path": "File path", "name": "Name", "timestamp": "Timestamp" }, "data_description": { - "filename": "A relative file path in the hass config root to write the notification to", + "file_path": "A local file path to write the notification to", "name": "Name of the notify service", "timestamp": "Add a timestamp to the notification" } diff --git a/tests/components/file/test_config_flow.py b/tests/components/file/test_config_flow.py index bf420a9e89e043..1378793f9bdb59 100644 --- a/tests/components/file/test_config_flow.py +++ b/tests/components/file/test_config_flow.py @@ -14,13 +14,15 @@ MOCK_CONFIG_NOTIFY = { "platform": "notify", - "filename": "some_file", + "file_path": "some_file", "timestamp": True, + "name": "File", } MOCK_CONFIG_SENSOR = { "platform": "sensor", "file_path": "some/path", "value_template": "{{ value | round(1) }}", + "name": "File", } pytestmark = pytest.mark.usefixtures("mock_setup_entry") @@ -139,9 +141,4 @@ async def test_not_allowed( await hass.async_block_till_done() assert result2["type"] is FlowResultType.FORM - - expected_error: dict[str, Any] = { - "notify": {"filename": "not_allowed"}, - "sensor": {"file_path": "not_allowed"}, - } - assert result2["errors"] == expected_error[platform] + assert result2["errors"] == {"file_path": "not_allowed"} diff --git a/tests/components/file/test_notify.py b/tests/components/file/test_notify.py index e5697f9edaf181..b080439bdec716 100644 --- a/tests/components/file/test_notify.py +++ b/tests/components/file/test_notify.py @@ -176,7 +176,7 @@ async def test_legacy_notify_file_exception( { "name": "test", "platform": "notify", - "filename": "mock_file", + "file_path": "mock_file", "timestamp": False, }, ), @@ -185,7 +185,7 @@ async def test_legacy_notify_file_exception( { "name": "test", "platform": "notify", - "filename": "mock_file", + "file_path": "mock_file", "timestamp": True, }, ), @@ -208,7 +208,7 @@ async def test_legacy_notify_file_entry_only_setup( message = params["message"] entry = MockConfigEntry( - domain=DOMAIN, data=data, title=f"test [{data['filename']}]" + domain=DOMAIN, data=data, title=f"test [{data['file_path']}]" ) entry.add_to_hass(hass) await hass.config_entries.async_setup(entry.entry_id) @@ -230,9 +230,8 @@ async def test_legacy_notify_file_entry_only_setup( await hass.services.async_call(domain, service, params, blocking=True) - full_filename = os.path.join(hass.config.path(), filename) assert m_open.call_count == 1 - assert m_open.call_args == call(full_filename, "a", encoding="utf8") + assert m_open.call_args == call(filename, "a", encoding="utf8") assert m_open.return_value.write.call_count == 2 if not timestamp: @@ -255,7 +254,7 @@ async def test_legacy_notify_file_entry_only_setup( { "name": "test", "platform": "notify", - "filename": "mock_file", + "file_path": "mock_file", }, ), ], @@ -269,7 +268,7 @@ async def test_legacy_notify_file_not_allowed( ) -> None: """Test legacy notify file output not allowed.""" entry = MockConfigEntry( - domain=DOMAIN, data=config, title=f"test [{config['filename']}]" + domain=DOMAIN, data=config, title=f"test [{config['file_path']}]" ) entry.add_to_hass(hass) assert not await hass.config_entries.async_setup(entry.entry_id) @@ -284,7 +283,7 @@ async def test_legacy_notify_file_not_allowed( { "name": "test", "platform": "notify", - "filename": "mock_file", + "file_path": "mock_file", }, True, ), @@ -303,7 +302,7 @@ async def test_notify_file_write_access_failed( params = {"message": "one, two, testing, testing"} entry = MockConfigEntry( - domain=DOMAIN, data=data, title=f"test [{data['filename']}]" + domain=DOMAIN, data=data, title=f"test [{data['file_path']}]" ) entry.add_to_hass(hass) await hass.config_entries.async_setup(entry.entry_id) From fd2cce55fe1915cc17edeb1d030332b8b7d6dd67 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Thu, 9 May 2024 20:56:08 +0000 Subject: [PATCH 14/16] No default timestamp needed --- homeassistant/components/file/notify.py | 2 +- tests/components/file/test_notify.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/file/notify.py b/homeassistant/components/file/notify.py index 34d8e1e978d6cc..69ebda46e572c3 100644 --- a/homeassistant/components/file/notify.py +++ b/homeassistant/components/file/notify.py @@ -45,7 +45,7 @@ async def async_get_service( # We only set up through discovery return None file_path: str = discovery_info[CONF_FILE_PATH] - timestamp: bool = discovery_info.get(CONF_TIMESTAMP, False) + timestamp: bool = discovery_info[CONF_TIMESTAMP] return FileNotificationService(file_path, timestamp) diff --git a/tests/components/file/test_notify.py b/tests/components/file/test_notify.py index b080439bdec716..f6d30c2f166196 100644 --- a/tests/components/file/test_notify.py +++ b/tests/components/file/test_notify.py @@ -46,7 +46,6 @@ async def test_bad_config(hass: HomeAssistant) -> None: "name": "test", "platform": "file", "filename": "mock_file", - "timestamp": False, } ] }, @@ -255,6 +254,7 @@ async def test_legacy_notify_file_entry_only_setup( "name": "test", "platform": "notify", "file_path": "mock_file", + "timestamp": False, }, ), ], @@ -284,6 +284,7 @@ async def test_legacy_notify_file_not_allowed( "name": "test", "platform": "notify", "file_path": "mock_file", + "timestamp": False, }, True, ), From 48a620dc0329f0b1aa32dcb0f47e7a82357b3c20 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Thu, 9 May 2024 21:00:31 +0000 Subject: [PATCH 15/16] Remove default name as it is already set --- homeassistant/components/file/sensor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/file/sensor.py b/homeassistant/components/file/sensor.py index f81b30e03cd0c6..55ccc0965bc297 100644 --- a/homeassistant/components/file/sensor.py +++ b/homeassistant/components/file/sensor.py @@ -59,7 +59,7 @@ async def async_setup_entry( """Set up the file sensor.""" config = dict(entry.data) file_path: str = config[CONF_FILE_PATH] - name: str = config.get(CONF_NAME, DEFAULT_NAME) + name: str = config[CONF_NAME] unit: str | None = config.get(CONF_UNIT_OF_MEASUREMENT) value_template: Template | None = None From bb0fa37886dafcf578cfbf1bc2875618a5458c6c Mon Sep 17 00:00:00 2001 From: jbouwh Date: Fri, 10 May 2024 08:43:43 +0000 Subject: [PATCH 16/16] Use links --- homeassistant/components/file/strings.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/file/strings.json b/homeassistant/components/file/strings.json index 03963dbac5c77d..243695b79cb7ba 100644 --- a/homeassistant/components/file/strings.json +++ b/homeassistant/components/file/strings.json @@ -28,8 +28,8 @@ "title": "Notification to file service", "description": "Set up a service that allows to write notification to a file.", "data": { - "file_path": "File path", - "name": "Name", + "file_path": "[%key:component::file::config::step::sensor::data::file_path%]", + "name": "[%key:component::file::config::step::sensor::data::name%]", "timestamp": "Timestamp" }, "data_description": {