From caacdabd3d3c11563e5fd7908cdfa3cf478c6b11 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sat, 23 Sep 2023 14:14:57 -0700 Subject: [PATCH] Fix rainbird unique id (#99704) * Don't set a unique id for devices with no serial * Add additional check for the same config entry host/port when there is no serial * Update homeassistant/components/rainbird/config_flow.py Co-authored-by: Robert Resch * Update tests/components/rainbird/test_config_flow.py Co-authored-by: Robert Resch * Update tests/components/rainbird/test_config_flow.py Co-authored-by: Robert Resch --------- Co-authored-by: Robert Resch --- .../components/rainbird/config_flow.py | 9 +- tests/components/rainbird/conftest.py | 12 +- tests/components/rainbird/test_config_flow.py | 121 +++++++++++++++++- 3 files changed, 136 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/rainbird/config_flow.py b/homeassistant/components/rainbird/config_flow.py index a784e4623d6..bf6682e7a6f 100644 --- a/homeassistant/components/rainbird/config_flow.py +++ b/homeassistant/components/rainbird/config_flow.py @@ -125,8 +125,13 @@ class RainbirdConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): options: dict[str, Any], ) -> FlowResult: """Create the config entry.""" - await self.async_set_unique_id(serial_number) - self._abort_if_unique_id_configured() + # Prevent devices with the same serial number. If the device does not have a serial number + # then we can at least prevent configuring the same host twice. + if serial_number: + await self.async_set_unique_id(serial_number) + self._abort_if_unique_id_configured() + else: + self._async_abort_entries_match(data) return self.async_create_entry( title=data[CONF_HOST], data=data, diff --git a/tests/components/rainbird/conftest.py b/tests/components/rainbird/conftest.py index 9e4e4e546cb..40b400210aa 100644 --- a/tests/components/rainbird/conftest.py +++ b/tests/components/rainbird/conftest.py @@ -35,6 +35,7 @@ SERIAL_NUMBER = 0x12635436566 # Get serial number Command 0x85. Serial is 0x12635436566 SERIAL_RESPONSE = "850000012635436566" +ZERO_SERIAL_RESPONSE = "850000000000000000" # Model and version command 0x82 MODEL_AND_VERSION_RESPONSE = "820006090C" # Get available stations command 0x83 @@ -84,6 +85,12 @@ def yaml_config() -> dict[str, Any]: return {} +@pytest.fixture +async def unique_id() -> str: + """Fixture for serial number used in the config entry.""" + return SERIAL_NUMBER + + @pytest.fixture async def config_entry_data() -> dict[str, Any]: """Fixture for MockConfigEntry data.""" @@ -92,13 +99,14 @@ async def config_entry_data() -> dict[str, Any]: @pytest.fixture async def config_entry( - config_entry_data: dict[str, Any] | None + config_entry_data: dict[str, Any] | None, + unique_id: str, ) -> MockConfigEntry | None: """Fixture for MockConfigEntry.""" if config_entry_data is None: return None return MockConfigEntry( - unique_id=SERIAL_NUMBER, + unique_id=unique_id, domain=DOMAIN, data=config_entry_data, options={ATTR_DURATION: DEFAULT_TRIGGER_TIME_MINUTES}, diff --git a/tests/components/rainbird/test_config_flow.py b/tests/components/rainbird/test_config_flow.py index f11eba4fed7..e7337ad6508 100644 --- a/tests/components/rainbird/test_config_flow.py +++ b/tests/components/rainbird/test_config_flow.py @@ -3,6 +3,7 @@ import asyncio from collections.abc import Generator from http import HTTPStatus +from typing import Any from unittest.mock import Mock, patch import pytest @@ -19,8 +20,11 @@ from .conftest import ( CONFIG_ENTRY_DATA, HOST, PASSWORD, + SERIAL_NUMBER, SERIAL_RESPONSE, URL, + ZERO_SERIAL_RESPONSE, + ComponentSetup, mock_response, ) @@ -66,19 +70,132 @@ async def complete_flow(hass: HomeAssistant) -> FlowResult: ) -async def test_controller_flow(hass: HomeAssistant, mock_setup: Mock) -> None: +@pytest.mark.parametrize( + ("responses", "expected_config_entry", "expected_unique_id"), + [ + ( + [mock_response(SERIAL_RESPONSE)], + CONFIG_ENTRY_DATA, + SERIAL_NUMBER, + ), + ( + [mock_response(ZERO_SERIAL_RESPONSE)], + {**CONFIG_ENTRY_DATA, "serial_number": 0}, + None, + ), + ], +) +async def test_controller_flow( + hass: HomeAssistant, + mock_setup: Mock, + expected_config_entry: dict[str, str], + expected_unique_id: int | None, +) -> None: """Test the controller is setup correctly.""" result = await complete_flow(hass) assert result.get("type") == "create_entry" assert result.get("title") == HOST assert "result" in result - assert result["result"].data == CONFIG_ENTRY_DATA + assert dict(result["result"].data) == expected_config_entry assert result["result"].options == {ATTR_DURATION: 6} + assert result["result"].unique_id == expected_unique_id assert len(mock_setup.mock_calls) == 1 +@pytest.mark.parametrize( + ( + "unique_id", + "config_entry_data", + "config_flow_responses", + "expected_config_entry", + ), + [ + ( + "other-serial-number", + {**CONFIG_ENTRY_DATA, "host": "other-host"}, + [mock_response(SERIAL_RESPONSE)], + CONFIG_ENTRY_DATA, + ), + ( + None, + {**CONFIG_ENTRY_DATA, "serial_number": 0, "host": "other-host"}, + [mock_response(ZERO_SERIAL_RESPONSE)], + {**CONFIG_ENTRY_DATA, "serial_number": 0}, + ), + ], + ids=["with-serial", "zero-serial"], +) +async def test_multiple_config_entries( + hass: HomeAssistant, + setup_integration: ComponentSetup, + responses: list[AiohttpClientMockResponse], + config_flow_responses: list[AiohttpClientMockResponse], + expected_config_entry: dict[str, Any] | None, +) -> None: + """Test setting up multiple config entries that refer to different devices.""" + assert await setup_integration() + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ConfigEntryState.LOADED + + responses.clear() + responses.extend(config_flow_responses) + + result = await complete_flow(hass) + assert result.get("type") == FlowResultType.CREATE_ENTRY + assert dict(result.get("result").data) == expected_config_entry + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 2 + + +@pytest.mark.parametrize( + ( + "unique_id", + "config_entry_data", + "config_flow_responses", + ), + [ + ( + SERIAL_NUMBER, + CONFIG_ENTRY_DATA, + [mock_response(SERIAL_RESPONSE)], + ), + ( + None, + {**CONFIG_ENTRY_DATA, "serial_number": 0}, + [mock_response(ZERO_SERIAL_RESPONSE)], + ), + ], + ids=[ + "duplicate-serial-number", + "duplicate-host-port-no-serial", + ], +) +async def test_duplicate_config_entries( + hass: HomeAssistant, + setup_integration: ComponentSetup, + responses: list[AiohttpClientMockResponse], + config_flow_responses: list[AiohttpClientMockResponse], +) -> None: + """Test that a device can not be registered twice.""" + assert await setup_integration() + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ConfigEntryState.LOADED + + responses.clear() + responses.extend(config_flow_responses) + + result = await complete_flow(hass) + assert result.get("type") == FlowResultType.ABORT + assert result.get("reason") == "already_configured" + + async def test_controller_cannot_connect( hass: HomeAssistant, mock_setup: Mock,