From 757482ee853fc22b39e05696167ecdead98aba99 Mon Sep 17 00:00:00 2001
From: John Luetke <johnl1479@gmail.com>
Date: Tue, 3 Sep 2019 16:18:06 -0700
Subject: [PATCH] Refactor pihole integration (#25837)

* Adds tests for pi_hole integration

* Refactor pi_hole component to an integration supporting multiple platforms

* Adds mock of Hole dependency

* Aborts platform setup when discovery_info is none

* Removes use of monitored_conditions

* Adds integration setup test

* Removes PlatformNotReady check

* Adds sensor test

* Code review updates

* Refactor tests to assert state through hass

* Reorder imports
---
 homeassistant/components/pi_hole/__init__.py |  95 ++++++++++++++
 homeassistant/components/pi_hole/const.py    |  43 ++++++
 homeassistant/components/pi_hole/sensor.py   | 130 +++----------------
 requirements_test_all.txt                    |   3 +
 script/gen_requirements_all.py               |   1 +
 tests/components/pi_hole/__init__.py         |   1 +
 tests/components/pi_hole/test_init.py        |  99 ++++++++++++++
 7 files changed, 260 insertions(+), 112 deletions(-)
 create mode 100644 homeassistant/components/pi_hole/const.py
 create mode 100644 tests/components/pi_hole/__init__.py
 create mode 100644 tests/components/pi_hole/test_init.py

diff --git a/homeassistant/components/pi_hole/__init__.py b/homeassistant/components/pi_hole/__init__.py
index 432e0f3fa11..ffc9827eed4 100644
--- a/homeassistant/components/pi_hole/__init__.py
+++ b/homeassistant/components/pi_hole/__init__.py
@@ -1 +1,96 @@
 """The pi_hole component."""
+import logging
+
+import voluptuous as vol
+from hole import Hole
+from hole.exceptions import HoleError
+
+from homeassistant.const import CONF_HOST, CONF_NAME, CONF_SSL, CONF_VERIFY_SSL
+from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
+from homeassistant.helpers import config_validation as cv
+from homeassistant.helpers.aiohttp_client import async_get_clientsession
+from homeassistant.helpers.discovery import async_load_platform
+from homeassistant.util import Throttle
+
+from .const import (
+    DOMAIN,
+    CONF_LOCATION,
+    DEFAULT_HOST,
+    DEFAULT_LOCATION,
+    DEFAULT_NAME,
+    DEFAULT_SSL,
+    DEFAULT_VERIFY_SSL,
+    MIN_TIME_BETWEEN_UPDATES,
+)
+
+LOGGER = logging.getLogger(__name__)
+
+CONFIG_SCHEMA = vol.Schema(
+    {
+        DOMAIN: vol.Schema(
+            {
+                vol.Optional(CONF_HOST, default=DEFAULT_HOST): cv.string,
+                vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
+                vol.Optional(CONF_SSL, default=DEFAULT_SSL): cv.boolean,
+                vol.Optional(CONF_LOCATION, default=DEFAULT_LOCATION): cv.string,
+                vol.Optional(CONF_VERIFY_SSL, default=DEFAULT_VERIFY_SSL): cv.boolean,
+            }
+        )
+    },
+    extra=vol.ALLOW_EXTRA,
+)
+
+
+async def async_setup(hass, config):
+    """Set up the pi_hole integration."""
+
+    conf = config[DOMAIN]
+    name = conf[CONF_NAME]
+    host = conf[CONF_HOST]
+    use_tls = conf[CONF_SSL]
+    verify_tls = conf[CONF_VERIFY_SSL]
+    location = conf[CONF_LOCATION]
+
+    LOGGER.debug("Setting up %s integration with host %s", DOMAIN, host)
+
+    session = async_get_clientsession(hass, True)
+    pi_hole = PiHoleData(
+        Hole(
+            host,
+            hass.loop,
+            session,
+            location=location,
+            tls=use_tls,
+            verify_tls=verify_tls,
+        ),
+        name,
+    )
+
+    await pi_hole.async_update()
+
+    hass.data[DOMAIN] = pi_hole
+
+    hass.async_create_task(async_load_platform(hass, SENSOR_DOMAIN, DOMAIN, {}, config))
+
+    return True
+
+
+class PiHoleData:
+    """Get the latest data and update the states."""
+
+    def __init__(self, api, name):
+        """Initialize the data object."""
+        self.api = api
+        self.name = name
+        self.available = True
+
+    @Throttle(MIN_TIME_BETWEEN_UPDATES)
+    async def async_update(self):
+        """Get the latest data from the Pi-hole."""
+
+        try:
+            await self.api.get_data()
+            self.available = True
+        except HoleError:
+            LOGGER.error("Unable to fetch data from Pi-hole")
+            self.available = False
diff --git a/homeassistant/components/pi_hole/const.py b/homeassistant/components/pi_hole/const.py
new file mode 100644
index 00000000000..ba83bf1d805
--- /dev/null
+++ b/homeassistant/components/pi_hole/const.py
@@ -0,0 +1,43 @@
+"""Constants for the pi_hole intergration."""
+from datetime import timedelta
+
+DOMAIN = "pi_hole"
+
+CONF_LOCATION = "location"
+
+DEFAULT_HOST = "pi.hole"
+DEFAULT_LOCATION = "admin"
+DEFAULT_METHOD = "GET"
+DEFAULT_NAME = "Pi-Hole"
+DEFAULT_SSL = False
+DEFAULT_VERIFY_SSL = True
+
+ATTR_BLOCKED_DOMAINS = "domains_blocked"
+
+MIN_TIME_BETWEEN_UPDATES = timedelta(minutes=5)
+
+SENSOR_DICT = {
+    "ads_blocked_today": ["Ads Blocked Today", "ads", "mdi:close-octagon-outline"],
+    "ads_percentage_today": [
+        "Ads Percentage Blocked Today",
+        "%",
+        "mdi:close-octagon-outline",
+    ],
+    "clients_ever_seen": ["Seen Clients", "clients", "mdi:account-outline"],
+    "dns_queries_today": [
+        "DNS Queries Today",
+        "queries",
+        "mdi:comment-question-outline",
+    ],
+    "domains_being_blocked": ["Domains Blocked", "domains", "mdi:block-helper"],
+    "queries_cached": ["DNS Queries Cached", "queries", "mdi:comment-question-outline"],
+    "queries_forwarded": [
+        "DNS Queries Forwarded",
+        "queries",
+        "mdi:comment-question-outline",
+    ],
+    "unique_clients": ["DNS Unique Clients", "clients", "mdi:account-outline"],
+    "unique_domains": ["DNS Unique Domains", "domains", "mdi:domain"],
+}
+
+SENSOR_LIST = list(SENSOR_DICT)
diff --git a/homeassistant/components/pi_hole/sensor.py b/homeassistant/components/pi_hole/sensor.py
index 9c41c20fd63..4e80e9767a6 100644
--- a/homeassistant/components/pi_hole/sensor.py
+++ b/homeassistant/components/pi_hole/sensor.py
@@ -1,100 +1,27 @@
 """Support for getting statistical data from a Pi-hole system."""
-from datetime import timedelta
 import logging
 
-import voluptuous as vol
-
-from homeassistant.components.sensor import PLATFORM_SCHEMA
-from homeassistant.const import (
-    CONF_HOST,
-    CONF_MONITORED_CONDITIONS,
-    CONF_NAME,
-    CONF_SSL,
-    CONF_VERIFY_SSL,
-)
-from homeassistant.exceptions import PlatformNotReady
-from homeassistant.helpers.aiohttp_client import async_get_clientsession
-import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity import Entity
-from homeassistant.util import Throttle
-
-_LOGGER = logging.getLogger(__name__)
-
-ATTR_BLOCKED_DOMAINS = "domains_blocked"
-ATTR_PERCENTAGE_TODAY = "percentage_today"
-ATTR_QUERIES_TODAY = "queries_today"
-
-CONF_LOCATION = "location"
-DEFAULT_HOST = "localhost"
-
-DEFAULT_LOCATION = "admin"
-DEFAULT_METHOD = "GET"
-DEFAULT_NAME = "Pi-Hole"
-DEFAULT_SSL = False
-DEFAULT_VERIFY_SSL = True
-
-MIN_TIME_BETWEEN_UPDATES = timedelta(minutes=5)
-
-MONITORED_CONDITIONS = {
-    "ads_blocked_today": ["Ads Blocked Today", "ads", "mdi:close-octagon-outline"],
-    "ads_percentage_today": [
-        "Ads Percentage Blocked Today",
-        "%",
-        "mdi:close-octagon-outline",
-    ],
-    "clients_ever_seen": ["Seen Clients", "clients", "mdi:account-outline"],
-    "dns_queries_today": [
-        "DNS Queries Today",
-        "queries",
-        "mdi:comment-question-outline",
-    ],
-    "domains_being_blocked": ["Domains Blocked", "domains", "mdi:block-helper"],
-    "queries_cached": ["DNS Queries Cached", "queries", "mdi:comment-question-outline"],
-    "queries_forwarded": [
-        "DNS Queries Forwarded",
-        "queries",
-        "mdi:comment-question-outline",
-    ],
-    "unique_clients": ["DNS Unique Clients", "clients", "mdi:account-outline"],
-    "unique_domains": ["DNS Unique Domains", "domains", "mdi:domain"],
-}
-
-PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
-    {
-        vol.Optional(CONF_HOST, default=DEFAULT_HOST): cv.string,
-        vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
-        vol.Optional(CONF_SSL, default=DEFAULT_SSL): cv.boolean,
-        vol.Optional(CONF_LOCATION, default=DEFAULT_LOCATION): cv.string,
-        vol.Optional(CONF_VERIFY_SSL, default=DEFAULT_VERIFY_SSL): cv.boolean,
-        vol.Optional(CONF_MONITORED_CONDITIONS, default=["ads_blocked_today"]): vol.All(
-            cv.ensure_list, [vol.In(MONITORED_CONDITIONS)]
-        ),
-    }
-)
-
 
-async def async_setup_platform(hass, config, async_add_entities, discovery_info=None):
-    """Set up the Pi-hole sensor."""
-    from hole import Hole
+from .const import (
+    DOMAIN as PIHOLE_DOMAIN,
+    ATTR_BLOCKED_DOMAINS,
+    SENSOR_LIST,
+    SENSOR_DICT,
+)
 
-    name = config.get(CONF_NAME)
-    host = config.get(CONF_HOST)
-    use_tls = config.get(CONF_SSL)
-    location = config.get(CONF_LOCATION)
-    verify_tls = config.get(CONF_VERIFY_SSL)
+LOGGER = logging.getLogger(__name__)
 
-    session = async_get_clientsession(hass, verify_tls)
-    pi_hole = PiHoleData(Hole(host, hass.loop, session, location=location, tls=use_tls))
 
-    await pi_hole.async_update()
+async def async_setup_platform(hass, config, async_add_entities, discovery_info=None):
+    """Set up the pi-hole sensor."""
+    if discovery_info is None:
+        return
 
-    if pi_hole.api.data is None:
-        raise PlatformNotReady
+    pi_hole = hass.data[PIHOLE_DOMAIN]
 
-    sensors = [
-        PiHoleSensor(pi_hole, name, condition)
-        for condition in config[CONF_MONITORED_CONDITIONS]
-    ]
+    sensors = []
+    sensors = [PiHoleSensor(pi_hole, sensor_name) for sensor_name in SENSOR_LIST]
 
     async_add_entities(sensors, True)
 
@@ -102,13 +29,13 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info=
 class PiHoleSensor(Entity):
     """Representation of a Pi-hole sensor."""
 
-    def __init__(self, pi_hole, name, condition):
+    def __init__(self, pi_hole, sensor_name):
         """Initialize a Pi-hole sensor."""
         self.pi_hole = pi_hole
-        self._name = name
-        self._condition = condition
+        self._name = pi_hole.name
+        self._condition = sensor_name
 
-        variable_info = MONITORED_CONDITIONS[condition]
+        variable_info = SENSOR_DICT[sensor_name]
         self._condition_name = variable_info[0]
         self._unit_of_measurement = variable_info[1]
         self._icon = variable_info[2]
@@ -151,24 +78,3 @@ class PiHoleSensor(Entity):
         """Get the latest data from the Pi-hole API."""
         await self.pi_hole.async_update()
         self.data = self.pi_hole.api.data
-
-
-class PiHoleData:
-    """Get the latest data and update the states."""
-
-    def __init__(self, api):
-        """Initialize the data object."""
-        self.api = api
-        self.available = True
-
-    @Throttle(MIN_TIME_BETWEEN_UPDATES)
-    async def async_update(self):
-        """Get the latest data from the Pi-hole."""
-        from hole.exceptions import HoleError
-
-        try:
-            await self.api.get_data()
-            self.available = True
-        except HoleError:
-            _LOGGER.error("Unable to fetch data from Pi-hole")
-            self.available = False
diff --git a/requirements_test_all.txt b/requirements_test_all.txt
index 2ef8f0ca672..f6013efee06 100644
--- a/requirements_test_all.txt
+++ b/requirements_test_all.txt
@@ -172,6 +172,9 @@ hbmqtt==0.9.4
 # homeassistant.components.jewish_calendar
 hdate==0.9.0
 
+# homeassistant.components.pi_hole
+hole==0.5.0
+
 # homeassistant.components.workday
 holidays==0.9.11
 
diff --git a/script/gen_requirements_all.py b/script/gen_requirements_all.py
index e99fd0a6c46..39e5de3e2b0 100755
--- a/script/gen_requirements_all.py
+++ b/script/gen_requirements_all.py
@@ -86,6 +86,7 @@ TEST_REQUIREMENTS = (
     "haversine",
     "hbmqtt",
     "hdate",
+    "hole",
     "holidays",
     "home-assistant-frontend",
     "homekit[IP]",
diff --git a/tests/components/pi_hole/__init__.py b/tests/components/pi_hole/__init__.py
new file mode 100644
index 00000000000..7eea15b79c8
--- /dev/null
+++ b/tests/components/pi_hole/__init__.py
@@ -0,0 +1 @@
+"""Tests for the pi_hole component."""
diff --git a/tests/components/pi_hole/test_init.py b/tests/components/pi_hole/test_init.py
new file mode 100644
index 00000000000..f30422bfea9
--- /dev/null
+++ b/tests/components/pi_hole/test_init.py
@@ -0,0 +1,99 @@
+"""Test pi_hole component."""
+
+from asynctest import CoroutineMock
+from hole import Hole
+
+from homeassistant.components import pi_hole
+from tests.common import async_setup_component
+from unittest.mock import patch
+
+
+def mock_pihole_data_call(Hole):
+    """Need to override so as to allow mocked data."""
+    Hole.__init__ = (
+        lambda self, host, loop, session, location, tls, verify_tls=True, api_token=None: None
+    )
+    Hole.data = {
+        "ads_blocked_today": 0,
+        "ads_percentage_today": 0,
+        "clients_ever_seen": 0,
+        "dns_queries_today": 0,
+        "domains_being_blocked": 0,
+        "queries_cached": 0,
+        "queries_forwarded": 0,
+        "status": 0,
+        "unique_clients": 0,
+        "unique_domains": 0,
+    }
+    pass
+
+
+async def test_setup_no_config(hass):
+    """Tests component setup with no config."""
+    with patch.object(
+        Hole, "get_data", new=CoroutineMock(side_effect=mock_pihole_data_call(Hole))
+    ):
+        assert await async_setup_component(hass, pi_hole.DOMAIN, {pi_hole.DOMAIN: {}})
+
+    await hass.async_block_till_done()
+
+    assert (
+        hass.states.get("sensor.pi_hole_ads_blocked_today").name
+        == "Pi-Hole Ads Blocked Today"
+    )
+    assert (
+        hass.states.get("sensor.pi_hole_ads_percentage_blocked_today").name
+        == "Pi-Hole Ads Percentage Blocked Today"
+    )
+    assert (
+        hass.states.get("sensor.pi_hole_dns_queries_cached").name
+        == "Pi-Hole DNS Queries Cached"
+    )
+    assert (
+        hass.states.get("sensor.pi_hole_dns_queries_forwarded").name
+        == "Pi-Hole DNS Queries Forwarded"
+    )
+    assert (
+        hass.states.get("sensor.pi_hole_dns_queries_today").name
+        == "Pi-Hole DNS Queries Today"
+    )
+    assert (
+        hass.states.get("sensor.pi_hole_dns_unique_clients").name
+        == "Pi-Hole DNS Unique Clients"
+    )
+    assert (
+        hass.states.get("sensor.pi_hole_dns_unique_domains").name
+        == "Pi-Hole DNS Unique Domains"
+    )
+    assert (
+        hass.states.get("sensor.pi_hole_domains_blocked").name
+        == "Pi-Hole Domains Blocked"
+    )
+    assert hass.states.get("sensor.pi_hole_seen_clients").name == "Pi-Hole Seen Clients"
+
+    assert hass.states.get("sensor.pi_hole_ads_blocked_today").state == "0"
+    assert hass.states.get("sensor.pi_hole_ads_percentage_blocked_today").state == "0"
+    assert hass.states.get("sensor.pi_hole_dns_queries_cached").state == "0"
+    assert hass.states.get("sensor.pi_hole_dns_queries_forwarded").state == "0"
+    assert hass.states.get("sensor.pi_hole_dns_queries_today").state == "0"
+    assert hass.states.get("sensor.pi_hole_dns_unique_clients").state == "0"
+    assert hass.states.get("sensor.pi_hole_dns_unique_domains").state == "0"
+    assert hass.states.get("sensor.pi_hole_domains_blocked").state == "0"
+    assert hass.states.get("sensor.pi_hole_seen_clients").state == "0"
+
+
+async def test_setup_custom_config(hass):
+    """Tests component setup with custom config."""
+    with patch.object(
+        Hole, "get_data", new=CoroutineMock(side_effect=mock_pihole_data_call(Hole))
+    ):
+        assert await async_setup_component(
+            hass, pi_hole.DOMAIN, {pi_hole.DOMAIN: {"name": "Custom"}}
+        )
+
+    await hass.async_block_till_done()
+
+    assert (
+        hass.states.get("sensor.custom_ads_blocked_today").name
+        == "Custom Ads Blocked Today"
+    )
-- 
GitLab