From e882d47cded0d2fde27c9547b993ab09e0743df5 Mon Sep 17 00:00:00 2001 From: Erwin Douna Date: Mon, 18 Mar 2024 16:16:24 +0100 Subject: [PATCH] Add Downloader config flow, including tests (#98722) * Adding base line, including tests * Adding validatge input and expanding tests * Updating manifest * Minor patch * Revert minor patch, wrong nesting * Adding proper translations * Including abort message * Update homeassistant/components/downloader/config_flow.py Co-authored-by: Erik Montnemery * Rename exception class * Refactor import * Update strings * Apply suggestions from code review * Update homeassistant/components/downloader/__init__.py Co-authored-by: Erik Montnemery * Update homeassistant/components/downloader/__init__.py Co-authored-by: Erik Montnemery * Update homeassistant/components/downloader/__init__.py Co-authored-by: Erik Montnemery * Update homeassistant/components/downloader/__init__.py Co-authored-by: Erik Montnemery * Update homeassistant/components/downloader/__init__.py Co-authored-by: Erik Montnemery * Update homeassistant/components/downloader/__init__.py Co-authored-by: Erik Montnemery * Update homeassistant/components/downloader/__init__.py Co-authored-by: Erik Montnemery * Reverting back filename and fix typing * Reverting back mutex/lock * Upgrade version * Adding typing * Removing coroutine * Removing unload entry (for now) * Removing comment * Change type * Putting download back in setup_entry * Revert back code --------- Co-authored-by: Erik Montnemery --- .coveragerc | 2 +- CODEOWNERS | 2 + .../components/downloader/__init__.py | 92 ++++++++++++------ .../components/downloader/config_flow.py | 69 ++++++++++++++ homeassistant/components/downloader/const.py | 19 ++++ .../components/downloader/manifest.json | 3 +- .../components/downloader/strings.json | 23 +++++ homeassistant/generated/config_flows.py | 1 + homeassistant/generated/integrations.json | 2 +- tests/components/downloader/__init__.py | 1 + .../components/downloader/test_config_flow.py | 94 +++++++++++++++++++ 11 files changed, 277 insertions(+), 31 deletions(-) create mode 100644 homeassistant/components/downloader/config_flow.py create mode 100644 homeassistant/components/downloader/const.py create mode 100644 tests/components/downloader/__init__.py create mode 100644 tests/components/downloader/test_config_flow.py diff --git a/.coveragerc b/.coveragerc index adf3c7b6eeb..80412c14a65 100644 --- a/.coveragerc +++ b/.coveragerc @@ -250,7 +250,7 @@ omit = homeassistant/components/dormakaba_dkey/lock.py homeassistant/components/dormakaba_dkey/sensor.py homeassistant/components/dovado/* - homeassistant/components/downloader/* + homeassistant/components/downloader/__init__.py homeassistant/components/dsmr_reader/__init__.py homeassistant/components/dsmr_reader/definitions.py homeassistant/components/dsmr_reader/sensor.py diff --git a/CODEOWNERS b/CODEOWNERS index abeddd439ab..440ddd45cfa 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -309,6 +309,8 @@ build.json @home-assistant/supervisor /tests/components/doorbird/ @oblogic7 @bdraco @flacjacket /homeassistant/components/dormakaba_dkey/ @emontnemery /tests/components/dormakaba_dkey/ @emontnemery +/homeassistant/components/downloader/ @erwindouna +/tests/components/downloader/ @erwindouna /homeassistant/components/dremel_3d_printer/ @tkdrob /tests/components/dremel_3d_printer/ @tkdrob /homeassistant/components/drop_connect/ @ChandlerSystems @pfrazer diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index 5d7f1594be0..94d243e2cf2 100644 --- a/homeassistant/components/downloader/__init__.py +++ b/homeassistant/components/downloader/__init__.py @@ -3,7 +3,6 @@ from __future__ import annotations from http import HTTPStatus -import logging import os import re import threading @@ -11,33 +10,26 @@ import threading import requests import voluptuous as vol +from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.core import HomeAssistant, ServiceCall +from homeassistant.data_entry_flow import FlowResultType import homeassistant.helpers.config_validation as cv +from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue +from homeassistant.helpers.service import async_register_admin_service from homeassistant.helpers.typing import ConfigType from homeassistant.util import raise_if_invalid_filename, raise_if_invalid_path -_LOGGER = logging.getLogger(__name__) - -ATTR_FILENAME = "filename" -ATTR_SUBDIR = "subdir" -ATTR_URL = "url" -ATTR_OVERWRITE = "overwrite" - -CONF_DOWNLOAD_DIR = "download_dir" - -DOMAIN = "downloader" -DOWNLOAD_FAILED_EVENT = "download_failed" -DOWNLOAD_COMPLETED_EVENT = "download_completed" - -SERVICE_DOWNLOAD_FILE = "download_file" - -SERVICE_DOWNLOAD_FILE_SCHEMA = vol.Schema( - { - vol.Required(ATTR_URL): cv.url, - vol.Optional(ATTR_SUBDIR): cv.string, - vol.Optional(ATTR_FILENAME): cv.string, - vol.Optional(ATTR_OVERWRITE, default=False): cv.boolean, - } +from .const import ( + _LOGGER, + ATTR_FILENAME, + ATTR_OVERWRITE, + ATTR_SUBDIR, + ATTR_URL, + CONF_DOWNLOAD_DIR, + DOMAIN, + DOWNLOAD_COMPLETED_EVENT, + DOWNLOAD_FAILED_EVENT, + SERVICE_DOWNLOAD_FILE, ) CONFIG_SCHEMA = vol.Schema( @@ -46,9 +38,46 @@ CONFIG_SCHEMA = vol.Schema( ) -def setup(hass: HomeAssistant, config: ConfigType) -> bool: +async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: + """Set up the Downloader component, via the YAML file.""" + if DOMAIN not in config: + return True + + import_result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_IMPORT}, + data={ + CONF_DOWNLOAD_DIR: config[DOMAIN][CONF_DOWNLOAD_DIR], + }, + ) + + translation_key = "deprecated_yaml" + if ( + import_result["type"] == FlowResultType.ABORT + and import_result["reason"] == "import_failed" + ): + translation_key = "import_failed" + + async_create_issue( + hass, + DOMAIN, + f"deprecated_yaml_{DOMAIN}", + breaks_in_ha_version="2024.9.0", + is_fixable=False, + issue_domain=DOMAIN, + severity=IssueSeverity.WARNING, + translation_key=translation_key, + translation_placeholders={ + "domain": DOMAIN, + "integration_title": "Downloader", + }, + ) + return True + + +async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Listen for download events to download files.""" - download_path = config[DOMAIN][CONF_DOWNLOAD_DIR] + download_path = entry.data[CONF_DOWNLOAD_DIR] # If path is relative, we assume relative to Home Assistant config dir if not os.path.isabs(download_path): @@ -58,7 +87,6 @@ def setup(hass: HomeAssistant, config: ConfigType) -> bool: _LOGGER.error( "Download path %s does not exist. File Downloader not active", download_path ) - return False def download_file(service: ServiceCall) -> None: @@ -169,11 +197,19 @@ def setup(hass: HomeAssistant, config: ConfigType) -> bool: threading.Thread(target=do_download).start() - hass.services.register( + async_register_admin_service( + hass, DOMAIN, SERVICE_DOWNLOAD_FILE, download_file, - schema=SERVICE_DOWNLOAD_FILE_SCHEMA, + schema=vol.Schema( + { + vol.Optional(ATTR_FILENAME): cv.string, + vol.Optional(ATTR_SUBDIR): cv.string, + vol.Required(ATTR_URL): cv.url, + vol.Optional(ATTR_OVERWRITE, default=False): cv.boolean, + } + ), ) return True diff --git a/homeassistant/components/downloader/config_flow.py b/homeassistant/components/downloader/config_flow.py new file mode 100644 index 00000000000..70557d30e19 --- /dev/null +++ b/homeassistant/components/downloader/config_flow.py @@ -0,0 +1,69 @@ +"""Config flow for Downloader integration.""" +from __future__ import annotations + +import os +from typing import Any + +import voluptuous as vol + +from homeassistant import exceptions +from homeassistant.config_entries import ConfigFlow, ConfigFlowResult +from homeassistant.helpers import config_validation as cv + +from .const import _LOGGER, CONF_DOWNLOAD_DIR, DEFAULT_NAME, DOMAIN + + +class DownloaderConfigFlow(ConfigFlow, domain=DOMAIN): + """Handle a config flow for Downloader.""" + + VERSION = 1 + + async def async_step_user( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle the initial step.""" + errors: dict[str, str] = {} + + if self._async_current_entries(): + return self.async_abort(reason="single_instance_allowed") + + if user_input is not None: + try: + await self._validate_input(user_input) + except DirectoryDoesNotExist: + errors["base"] = "cannot_connect" + else: + return self.async_create_entry(title=DEFAULT_NAME, data=user_input) + + return self.async_show_form( + step_id="user", + data_schema=vol.Schema( + { + vol.Required(CONF_DOWNLOAD_DIR): cv.string, + } + ), + errors=errors, + ) + + async def async_step_import( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle a flow initiated by configuration file.""" + + return await self.async_step_user(user_input) + + async def _validate_input(self, user_input: dict[str, Any]) -> None: + """Validate the user input if the directory exists.""" + if not os.path.isabs(user_input[CONF_DOWNLOAD_DIR]): + download_path = self.hass.config.path(user_input[CONF_DOWNLOAD_DIR]) + + if not os.path.isdir(download_path): + _LOGGER.error( + "Download path %s does not exist. File Downloader not active", + download_path, + ) + raise DirectoryDoesNotExist + + +class DirectoryDoesNotExist(exceptions.HomeAssistantError): + """Error to indicate the specified download directory does not exist.""" diff --git a/homeassistant/components/downloader/const.py b/homeassistant/components/downloader/const.py new file mode 100644 index 00000000000..047dff2b0cc --- /dev/null +++ b/homeassistant/components/downloader/const.py @@ -0,0 +1,19 @@ +"""Constants for the Downloader component.""" +import logging + +_LOGGER = logging.getLogger(__package__) + +DOMAIN = "downloader" +DEFAULT_NAME = "Downloader" +CONF_DOWNLOAD_DIR = "download_dir" +ATTR_FILENAME = "filename" +ATTR_SUBDIR = "subdir" +ATTR_URL = "url" +ATTR_OVERWRITE = "overwrite" + +CONF_DOWNLOAD_DIR = "download_dir" + +DOWNLOAD_FAILED_EVENT = "download_failed" +DOWNLOAD_COMPLETED_EVENT = "download_completed" + +SERVICE_DOWNLOAD_FILE = "download_file" diff --git a/homeassistant/components/downloader/manifest.json b/homeassistant/components/downloader/manifest.json index 5e4f0f5fde9..876404be889 100644 --- a/homeassistant/components/downloader/manifest.json +++ b/homeassistant/components/downloader/manifest.json @@ -1,7 +1,8 @@ { "domain": "downloader", "name": "Downloader", - "codeowners": [], + "codeowners": ["@erwindouna"], + "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/downloader", "quality_scale": "internal" } diff --git a/homeassistant/components/downloader/strings.json b/homeassistant/components/downloader/strings.json index c81b9f0ea39..77dd0abd9d3 100644 --- a/homeassistant/components/downloader/strings.json +++ b/homeassistant/components/downloader/strings.json @@ -1,4 +1,17 @@ { + "config": { + "step": { + "user": { + "description": "Select a location to get to store downloads. The setup will check if the directory exists." + } + }, + "error": { + "cannot_connect": "The directory could not be reached. Please check your settings." + }, + "abort": { + "single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]" + } + }, "services": { "download_file": { "name": "Download file", @@ -22,5 +35,15 @@ } } } + }, + "issues": { + "deprecated_yaml": { + "title": "The {integration_title} YAML configuration is being removed", + "description": "Configuring {integration_title} using YAML is being removed.\n\nYour configuration is already imported.\n\nRemove the `{domain}` configuration from your configuration.yaml file and restart Home Assistant to fix this issue." + }, + "import_failed": { + "title": "The {integration_title} failed to import", + "description": "The {integration_title} integration failed to import.\n\nPlease check the logs for more details." + } } } diff --git a/homeassistant/generated/config_flows.py b/homeassistant/generated/config_flows.py index 957a7696055..638f9497db9 100644 --- a/homeassistant/generated/config_flows.py +++ b/homeassistant/generated/config_flows.py @@ -118,6 +118,7 @@ FLOWS = { "dnsip", "doorbird", "dormakaba_dkey", + "downloader", "dremel_3d_printer", "drop_connect", "dsmr", diff --git a/homeassistant/generated/integrations.json b/homeassistant/generated/integrations.json index 4ce6ce6283c..424861e0f58 100644 --- a/homeassistant/generated/integrations.json +++ b/homeassistant/generated/integrations.json @@ -1298,7 +1298,7 @@ "downloader": { "name": "Downloader", "integration_type": "hub", - "config_flow": false + "config_flow": true }, "dremel_3d_printer": { "name": "Dremel 3D Printer", diff --git a/tests/components/downloader/__init__.py b/tests/components/downloader/__init__.py new file mode 100644 index 00000000000..abf11631bb9 --- /dev/null +++ b/tests/components/downloader/__init__.py @@ -0,0 +1 @@ +"""Tests for the downloader component.""" diff --git a/tests/components/downloader/test_config_flow.py b/tests/components/downloader/test_config_flow.py new file mode 100644 index 00000000000..597ecb333bf --- /dev/null +++ b/tests/components/downloader/test_config_flow.py @@ -0,0 +1,94 @@ +"""Test the Downloader config flow.""" +from unittest.mock import patch + +import pytest + +from homeassistant import config_entries +from homeassistant.components.downloader.config_flow import DirectoryDoesNotExist +from homeassistant.components.downloader.const import CONF_DOWNLOAD_DIR, DOMAIN +from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER +from homeassistant.core import HomeAssistant +from homeassistant.data_entry_flow import FlowResultType + +from tests.common import MockConfigEntry + +CONFIG = {CONF_DOWNLOAD_DIR: "download_dir"} + + +async def test_user_form(hass: HomeAssistant) -> None: + """Test the full user configuration flow.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER} + ) + + with patch( + "homeassistant.components.downloader.async_setup_entry", + return_value=True, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input=CONFIG, + ) + assert result["type"] == FlowResultType.FORM + + with patch( + "homeassistant.components.downloader.config_flow.DownloaderConfigFlow._validate_input", + side_effect=DirectoryDoesNotExist, + ): + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" + assert result["errors"] == {"base": "cannot_connect"} + + with patch( + "homeassistant.components.downloader.async_setup_entry", return_value=True + ), patch( + "homeassistant.components.downloader.config_flow.DownloaderConfigFlow._validate_input", + return_value=None, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input=CONFIG, + ) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["title"] == "Downloader" + assert result["data"] == {"download_dir": "download_dir"} + + +@pytest.mark.parametrize("source", [SOURCE_USER, SOURCE_IMPORT]) +async def test_single_instance_allowed( + hass: HomeAssistant, + source: str, +) -> None: + """Test we abort if already setup.""" + mock_config_entry = MockConfigEntry(domain=DOMAIN) + + mock_config_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": source} + ) + + assert result["type"] == FlowResultType.ABORT + assert result["reason"] == "single_instance_allowed" + + +async def test_import_flow_success(hass: HomeAssistant) -> None: + """Test import flow.""" + with patch( + "homeassistant.components.downloader.async_setup_entry", return_value=True + ), patch( + "homeassistant.components.downloader.config_flow.DownloaderConfigFlow._validate_input", + return_value=None, + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_IMPORT}, + data={}, + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["title"] == "Downloader" + assert result["data"] == {} + assert result["options"] == {}