From 572d5a09cd27bcabd8b5bcf0c012cebce9fa994e Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Wed, 8 Jul 2020 15:31:41 -0400 Subject: [PATCH] Vizio: when checking new host against existing config entry hosts, make check hostname aware (#37397) * make ip check hostname aware * add executor job for sync function doing IO and remove errant comma * revert change to update new_data explicitly for options keys since it is already done later * empty commit to retrigger CI --- homeassistant/components/vizio/config_flow.py | 25 +++++++-- .../components/vizio/media_player.py | 2 +- tests/components/vizio/conftest.py | 11 ++++ tests/components/vizio/test_config_flow.py | 52 +++++++++++++++++++ 4 files changed, 84 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/vizio/config_flow.py b/homeassistant/components/vizio/config_flow.py index a0fe5a6fa31..335f138ee7e 100644 --- a/homeassistant/components/vizio/config_flow.py +++ b/homeassistant/components/vizio/config_flow.py @@ -1,6 +1,7 @@ """Config flow for Vizio.""" import copy import logging +import socket from typing import Any, Dict, Optional from pyvizio import VizioAsync, async_guess_device_type @@ -29,6 +30,7 @@ from homeassistant.core import callback from homeassistant.helpers import config_validation as cv from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.typing import DiscoveryInfoType +from homeassistant.util.network import is_ip_address from .const import ( CONF_APPS, @@ -90,7 +92,11 @@ def _get_pairing_schema(input_dict: Dict[str, Any] = None) -> vol.Schema: def _host_is_same(host1: str, host2: str) -> bool: """Check if host1 and host2 are the same.""" - return host1.split(":")[0] == host2.split(":")[0] + host1 = host1.split(":")[0] + host1 = host1 if is_ip_address(host1) else socket.gethostbyname(host1) + host2 = host2.split(":")[0] + host2 = host2 if is_ip_address(host2) else socket.gethostbyname(host2) + return host1 == host2 class VizioOptionsConfigFlow(config_entries.OptionsFlow): @@ -206,8 +212,9 @@ class VizioConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): # If source is ignore bypass host and name check and continue through loop if entry.source == SOURCE_IGNORE: continue - - if _host_is_same(entry.data[CONF_HOST], user_input[CONF_HOST]): + if await self.hass.async_add_executor_job( + _host_is_same, entry.data[CONF_HOST], user_input[CONF_HOST] + ): errors[CONF_HOST] = "host_exists" if entry.data[CONF_NAME] == user_input[CONF_NAME]: @@ -284,11 +291,16 @@ class VizioConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if entry.source == SOURCE_IGNORE: continue - if _host_is_same(entry.data[CONF_HOST], import_config[CONF_HOST]): + if await self.hass.async_add_executor_job( + _host_is_same, entry.data[CONF_HOST], import_config[CONF_HOST] + ): updated_options = {} updated_data = {} remove_apps = False + if entry.data[CONF_HOST] != import_config[CONF_HOST]: + updated_data[CONF_HOST] = import_config[CONF_HOST] + if entry.data[CONF_NAME] != import_config[CONF_NAME]: updated_data[CONF_NAME] = import_config[CONF_NAME] @@ -314,6 +326,7 @@ class VizioConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if updated_data: new_data.update(updated_data) + # options are stored in entry options and data so update both if updated_options: new_data.update(updated_options) new_options.update(updated_options) @@ -353,7 +366,9 @@ class VizioConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if entry.source == SOURCE_IGNORE: continue - if _host_is_same(entry.data[CONF_HOST], discovery_info[CONF_HOST]): + if await self.hass.async_add_executor_job( + _host_is_same, entry.data[CONF_HOST], discovery_info[CONF_HOST] + ): return self.async_abort(reason="already_configured_device") # Set default name to discovered device name by stripping zeroconf service diff --git a/homeassistant/components/vizio/media_player.py b/homeassistant/components/vizio/media_player.py index 942411b0536..4a93e7886ef 100644 --- a/homeassistant/components/vizio/media_player.py +++ b/homeassistant/components/vizio/media_player.py @@ -110,7 +110,7 @@ async def async_setup_entry( _LOGGER.warning("Failed to connect to %s", host) raise PlatformNotReady - entity = VizioDevice(config_entry, device, name, device_class,) + entity = VizioDevice(config_entry, device, name, device_class) async_add_entities([entity], update_before_add=True) diff --git a/tests/components/vizio/conftest.py b/tests/components/vizio/conftest.py index f7448a71c31..fd5ff7cd468 100644 --- a/tests/components/vizio/conftest.py +++ b/tests/components/vizio/conftest.py @@ -16,6 +16,7 @@ from .const import ( RESPONSE_TOKEN, UNIQUE_ID, VERSION, + ZEROCONF_HOST, MockCompletePairingResponse, MockStartPairingResponse, ) @@ -183,3 +184,13 @@ def vizio_update_with_apps_fixture(vizio_update: pytest.fixture): return_value=CURRENT_APP_CONFIG, ): yield + + +@pytest.fixture(name="vizio_hostname_check") +def vizio_hostname_check(): + """Mock vizio hostname resolution.""" + with patch( + "homeassistant.components.vizio.config_flow.socket.gethostbyname", + return_value=ZEROCONF_HOST, + ): + yield diff --git a/tests/components/vizio/test_config_flow.py b/tests/components/vizio/test_config_flow.py index 2b1908dc770..74d4d6a2c62 100644 --- a/tests/components/vizio/test_config_flow.py +++ b/tests/components/vizio/test_config_flow.py @@ -853,3 +853,55 @@ async def test_zeroconf_abort_when_ignored( assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "already_configured" + + +async def test_zeroconf_flow_already_configured_hostname( + hass: HomeAssistantType, + vizio_connect: pytest.fixture, + vizio_bypass_setup: pytest.fixture, + vizio_hostname_check: pytest.fixture, +) -> None: + """Test entity is already configured during zeroconf setup when existing entry uses hostname.""" + config = MOCK_SPEAKER_CONFIG.copy() + config[CONF_HOST] = "hostname" + entry = MockConfigEntry( + domain=DOMAIN, data=config, options={CONF_VOLUME_STEP: VOLUME_STEP} + ) + entry.add_to_hass(hass) + + # Try rediscovering same device + discovery_info = MOCK_ZEROCONF_SERVICE_INFO.copy() + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_ZEROCONF}, data=discovery_info + ) + + # Flow should abort because device is already setup + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_configured_device" + + +async def test_import_flow_already_configured_hostname( + hass: HomeAssistantType, + vizio_connect: pytest.fixture, + vizio_bypass_setup: pytest.fixture, + vizio_hostname_check: pytest.fixture, +) -> None: + """Test entity is already configured during import setup when existing entry uses hostname.""" + config = MOCK_SPEAKER_CONFIG.copy() + config[CONF_HOST] = "hostname" + entry = MockConfigEntry( + domain=DOMAIN, data=config, options={CONF_VOLUME_STEP: VOLUME_STEP} + ) + entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_IMPORT}, + data=vol.Schema(VIZIO_SCHEMA)(MOCK_SPEAKER_CONFIG), + ) + + # Flow should abort because device was updated + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "updated_entry" + + assert entry.data[CONF_HOST] == HOST