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 <erik@montnemery.com>

* Rename exception class

* Refactor import

* Update strings

* Apply suggestions from code review

* Update homeassistant/components/downloader/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

* Update homeassistant/components/downloader/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

* Update homeassistant/components/downloader/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

* Update homeassistant/components/downloader/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

* Update homeassistant/components/downloader/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

* Update homeassistant/components/downloader/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

* Update homeassistant/components/downloader/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

* 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 <erik@montnemery.com>

.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

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

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__)
+from .const import (
-
+    _LOGGER,
-ATTR_FILENAME = "filename"
+    ATTR_FILENAME,
-ATTR_SUBDIR = "subdir"
+    ATTR_OVERWRITE,
-ATTR_URL = "url"
+    ATTR_SUBDIR,
-ATTR_OVERWRITE = "overwrite"
+    ATTR_URL,
-
+    CONF_DOWNLOAD_DIR,
-CONF_DOWNLOAD_DIR = "download_dir"
+    DOMAIN,
-
+    DOWNLOAD_COMPLETED_EVENT,
-DOMAIN = "downloader"
+    DOWNLOAD_FAILED_EVENT,
-DOWNLOAD_FAILED_EVENT = "download_failed"
+    SERVICE_DOWNLOAD_FILE,
-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,
-    }
 )
 
 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

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."""

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"

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"
 }

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."
+    }
   }
 }

homeassistant/generated/config_flows.py

@@ -118,6 +118,7 @@ FLOWS = {
         "dnsip",
         "doorbird",
         "dormakaba_dkey",
+        "downloader",
         "dremel_3d_printer",
         "drop_connect",
         "dsmr",

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",

tests/components/downloader/__init__.py

@@ -0,0 +1 @@
+"""Tests for the downloader component."""

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"] == {}