From 0c288bcabb7e1b2c371d406cc69b485ff83b93ad Mon Sep 17 00:00:00 2001 From: Martin Hjelmare Date: Fri, 7 May 2021 13:37:38 +0200 Subject: [PATCH] Fix mysensors default persistence file on import (#48410) --- .../components/mysensors/__init__.py | 32 ++-- .../components/mysensors/config_flow.py | 4 +- .../components/mysensors/test_config_flow.py | 3 + tests/components/mysensors/test_init.py | 169 ++++++++++++++---- 4 files changed, 157 insertions(+), 51 deletions(-) diff --git a/homeassistant/components/mysensors/__init__.py b/homeassistant/components/mysensors/__init__.py index 812e6bf1670..9d23cfd24b6 100644 --- a/homeassistant/components/mysensors/__init__.py +++ b/homeassistant/components/mysensors/__init__.py @@ -58,18 +58,23 @@ DEFAULT_TCP_PORT = 5003 DEFAULT_VERSION = "1.4" +def set_default_persistence_file(value: dict) -> dict: + """Set default persistence file.""" + for idx, gateway in enumerate(value): + fil = gateway.get(CONF_PERSISTENCE_FILE) + if fil is not None: + continue + new_name = f"mysensors{idx + 1}.pickle" + gateway[CONF_PERSISTENCE_FILE] = new_name + + return value + + def has_all_unique_files(value): """Validate that all persistence files are unique and set if any is set.""" - persistence_files = [gateway.get(CONF_PERSISTENCE_FILE) for gateway in value] - if None in persistence_files and any( - name is not None for name in persistence_files - ): - raise vol.Invalid( - "persistence file name of all devices must be set if any is set" - ) - if not all(name is None for name in persistence_files): - schema = vol.Schema(vol.Unique()) - schema(persistence_files) + persistence_files = [gateway[CONF_PERSISTENCE_FILE] for gateway in value] + schema = vol.Schema(vol.Unique()) + schema(persistence_files) return value @@ -128,7 +133,10 @@ CONFIG_SCHEMA = vol.Schema( deprecated(CONF_PERSISTENCE), { vol.Required(CONF_GATEWAYS): vol.All( - cv.ensure_list, has_all_unique_files, [GATEWAY_SCHEMA] + cv.ensure_list, + set_default_persistence_file, + has_all_unique_files, + [GATEWAY_SCHEMA], ), vol.Optional(CONF_RETAIN, default=True): cv.boolean, vol.Optional(CONF_VERSION, default=DEFAULT_VERSION): cv.string, @@ -159,7 +167,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: CONF_TOPIC_IN_PREFIX: gw.get(CONF_TOPIC_IN_PREFIX, ""), CONF_RETAIN: config[CONF_RETAIN], CONF_VERSION: config[CONF_VERSION], - CONF_PERSISTENCE_FILE: gw.get(CONF_PERSISTENCE_FILE) + CONF_PERSISTENCE_FILE: gw[CONF_PERSISTENCE_FILE] # nodes config ignored at this time. renaming nodes can now be done from the frontend. } for gw in config[CONF_GATEWAYS] diff --git a/homeassistant/components/mysensors/config_flow.py b/homeassistant/components/mysensors/config_flow.py index 59dff4829de..847408abcc5 100644 --- a/homeassistant/components/mysensors/config_flow.py +++ b/homeassistant/components/mysensors/config_flow.py @@ -324,7 +324,9 @@ class MySensorsConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): except vol.Invalid: errors[CONF_PERSISTENCE_FILE] = "invalid_persistence_file" else: - real_persistence_path = self._normalize_persistence_file( + real_persistence_path = user_input[ + CONF_PERSISTENCE_FILE + ] = self._normalize_persistence_file( user_input[CONF_PERSISTENCE_FILE] ) for other_entry in self.hass.config_entries.async_entries(DOMAIN): diff --git a/tests/components/mysensors/test_config_flow.py b/tests/components/mysensors/test_config_flow.py index 66900066cd1..161d00e44b3 100644 --- a/tests/components/mysensors/test_config_flow.py +++ b/tests/components/mysensors/test_config_flow.py @@ -380,6 +380,9 @@ async def test_config_invalid( with patch( "homeassistant.components.mysensors.config_flow.try_connect", return_value=True + ), patch( + "homeassistant.components.mysensors.gateway.socket.getaddrinfo", + side_effect=OSError, ), patch( "homeassistant.components.mysensors.async_setup", return_value=True ) as mock_setup, patch( diff --git a/tests/components/mysensors/test_init.py b/tests/components/mysensors/test_init.py index 4fb51d6c17a..05e3df76285 100644 --- a/tests/components/mysensors/test_init.py +++ b/tests/components/mysensors/test_init.py @@ -32,7 +32,7 @@ from homeassistant.setup import async_setup_component @pytest.mark.parametrize( - "config, expected_calls, expected_to_succeed, expected_config_flow_user_input", + "config, expected_calls, expected_to_succeed, expected_config_entry_data", [ ( { @@ -52,13 +52,19 @@ from homeassistant.setup import async_setup_component }, 1, True, - { - CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_SERIAL, - CONF_DEVICE: "COM5", - CONF_PERSISTENCE_FILE: "bla.json", - CONF_BAUD_RATE: 57600, - CONF_VERSION: "2.3", - }, + [ + { + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_SERIAL, + CONF_DEVICE: "COM5", + CONF_PERSISTENCE_FILE: "bla.json", + CONF_BAUD_RATE: 57600, + CONF_VERSION: "2.3", + CONF_TCP_PORT: 5003, + CONF_TOPIC_IN_PREFIX: "", + CONF_TOPIC_OUT_PREFIX: "", + CONF_RETAIN: True, + } + ], ), ( { @@ -78,13 +84,19 @@ from homeassistant.setup import async_setup_component }, 1, True, - { - CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_TCP, - CONF_DEVICE: "127.0.0.1", - CONF_PERSISTENCE_FILE: "blub.pickle", - CONF_TCP_PORT: 343, - CONF_VERSION: "2.4", - }, + [ + { + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_TCP, + CONF_DEVICE: "127.0.0.1", + CONF_PERSISTENCE_FILE: "blub.pickle", + CONF_TCP_PORT: 343, + CONF_VERSION: "2.4", + CONF_BAUD_RATE: 115200, + CONF_TOPIC_IN_PREFIX: "", + CONF_TOPIC_OUT_PREFIX: "", + CONF_RETAIN: False, + } + ], ), ( { @@ -100,12 +112,19 @@ from homeassistant.setup import async_setup_component }, 1, True, - { - CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_TCP, - CONF_DEVICE: "127.0.0.1", - CONF_TCP_PORT: 5003, - CONF_VERSION: DEFAULT_VERSION, - }, + [ + { + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_TCP, + CONF_DEVICE: "127.0.0.1", + CONF_TCP_PORT: 5003, + CONF_VERSION: DEFAULT_VERSION, + CONF_BAUD_RATE: 115200, + CONF_TOPIC_IN_PREFIX: "", + CONF_TOPIC_OUT_PREFIX: "", + CONF_RETAIN: False, + CONF_PERSISTENCE_FILE: "mysensors1.pickle", + } + ], ), ( { @@ -125,13 +144,19 @@ from homeassistant.setup import async_setup_component }, 1, True, - { - CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_MQTT, - CONF_DEVICE: "mqtt", - CONF_VERSION: DEFAULT_VERSION, - CONF_TOPIC_OUT_PREFIX: "outtopic", - CONF_TOPIC_IN_PREFIX: "intopic", - }, + [ + { + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_MQTT, + CONF_DEVICE: "mqtt", + CONF_VERSION: DEFAULT_VERSION, + CONF_BAUD_RATE: 115200, + CONF_TCP_PORT: 5003, + CONF_TOPIC_OUT_PREFIX: "outtopic", + CONF_TOPIC_IN_PREFIX: "intopic", + CONF_RETAIN: False, + CONF_PERSISTENCE_FILE: "mysensors1.pickle", + } + ], ), ( { @@ -149,7 +174,7 @@ from homeassistant.setup import async_setup_component }, 0, True, - {}, + [{}], ), ( { @@ -177,7 +202,30 @@ from homeassistant.setup import async_setup_component }, 2, True, - {}, + [ + { + CONF_DEVICE: "mqtt", + CONF_PERSISTENCE_FILE: "bla.json", + CONF_TOPIC_OUT_PREFIX: "out", + CONF_TOPIC_IN_PREFIX: "in", + CONF_BAUD_RATE: 115200, + CONF_TCP_PORT: 5003, + CONF_VERSION: "2.4", + CONF_RETAIN: False, + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_MQTT, + }, + { + CONF_DEVICE: "COM6", + CONF_PERSISTENCE_FILE: "bla2.json", + CONF_TOPIC_OUT_PREFIX: "", + CONF_TOPIC_IN_PREFIX: "", + CONF_BAUD_RATE: 115200, + CONF_TCP_PORT: 5003, + CONF_VERSION: "2.4", + CONF_RETAIN: False, + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_SERIAL, + }, + ], ), ( { @@ -203,7 +251,7 @@ from homeassistant.setup import async_setup_component }, 0, False, - {}, + [{}], ), ( { @@ -223,7 +271,47 @@ from homeassistant.setup import async_setup_component }, 0, True, - {}, + [{}], + ), + ( + { + DOMAIN: { + CONF_GATEWAYS: [ + { + CONF_DEVICE: "COM1", + }, + { + CONF_DEVICE: "COM2", + }, + ], + } + }, + 2, + True, + [ + { + CONF_DEVICE: "COM1", + CONF_PERSISTENCE_FILE: "mysensors1.pickle", + CONF_TOPIC_OUT_PREFIX: "", + CONF_TOPIC_IN_PREFIX: "", + CONF_BAUD_RATE: 115200, + CONF_TCP_PORT: 5003, + CONF_VERSION: "1.4", + CONF_RETAIN: True, + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_SERIAL, + }, + { + CONF_DEVICE: "COM2", + CONF_PERSISTENCE_FILE: "mysensors2.pickle", + CONF_TOPIC_OUT_PREFIX: "", + CONF_TOPIC_IN_PREFIX: "", + CONF_BAUD_RATE: 115200, + CONF_TCP_PORT: 5003, + CONF_VERSION: "1.4", + CONF_RETAIN: True, + CONF_GATEWAY_TYPE: CONF_GATEWAY_TYPE_SERIAL, + }, + ], ), ], ) @@ -233,7 +321,7 @@ async def test_import( config: ConfigType, expected_calls: int, expected_to_succeed: bool, - expected_config_flow_user_input: dict[str, Any], + expected_config_entry_data: list[dict[str, Any]], ) -> None: """Test importing a gateway.""" await async_setup_component(hass, "persistent_notification", {}) @@ -249,8 +337,13 @@ async def test_import( assert len(mock_setup_entry.mock_calls) == expected_calls - if expected_calls > 0: - config_flow_user_input = mock_setup_entry.mock_calls[0][1][1].data - for key, value in expected_config_flow_user_input.items(): - assert key in config_flow_user_input - assert config_flow_user_input[key] == value + for idx in range(expected_calls): + config_entry = mock_setup_entry.mock_calls[idx][1][1] + expected_persistence_file = expected_config_entry_data[idx].pop( + CONF_PERSISTENCE_FILE + ) + expected_persistence_path = hass.config.path(expected_persistence_file) + config_entry_data = dict(config_entry.data) + persistence_path = config_entry_data.pop(CONF_PERSISTENCE_FILE) + assert persistence_path == expected_persistence_path + assert config_entry_data == expected_config_entry_data[idx]