From fc4b18b907ebccba3b2eaa9e7b4b80165f559e4f Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Tue, 27 Feb 2024 18:28:19 +0100 Subject: [PATCH] Return FlowResultType.ABORT when violating single_config_entry (#111637) * Return FlowResultType.ABORT when violating single_config_entry * Fix translations * Fix tests --- .../components/homeassistant/strings.json | 6 ++ homeassistant/config_entries.py | 40 +++++---- homeassistant/data_entry_flow.py | 1 + tests/test_config_entries.py | 88 ++++++++++--------- 4 files changed, 77 insertions(+), 58 deletions(-) diff --git a/homeassistant/components/homeassistant/strings.json b/homeassistant/components/homeassistant/strings.json index e2a6fc1c9e7..0b20f8698c2 100644 --- a/homeassistant/components/homeassistant/strings.json +++ b/homeassistant/components/homeassistant/strings.json @@ -1,4 +1,10 @@ { + "config": { + "abort": { + "single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]" + }, + "step": {} + }, "issues": { "country_not_configured": { "title": "The country has not been configured", diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index ede8c4139e6..bd83a922df4 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -1008,6 +1008,8 @@ class ConfigEntriesFlowManager(data_entry_flow.FlowManager): if not context or "source" not in context: raise KeyError("Context not set or doesn't have a source set") + flow_id = uuid_util.random_uuid_hex() + # Avoid starting a config flow on an integration that only supports # a single config entry, but which already has an entry if ( @@ -1015,13 +1017,14 @@ class ConfigEntriesFlowManager(data_entry_flow.FlowManager): and await _support_single_config_entry_only(self.hass, handler) and self.config_entries.async_entries(handler, include_ignore=False) ): - raise HomeAssistantError( - "Cannot start a config flow, the integration" - " supports only a single config entry" - " but already has one" + return FlowResult( + type=data_entry_flow.FlowResultType.ABORT, + flow_id=flow_id, + handler=handler, + reason="single_instance_allowed", + translation_domain=HA_DOMAIN, ) - flow_id = uuid_util.random_uuid_hex() loop = self.hass.loop if context["source"] == SOURCE_IMPORT: @@ -1111,6 +1114,21 @@ class ConfigEntriesFlowManager(data_entry_flow.FlowManager): if result["type"] != data_entry_flow.FlowResultType.CREATE_ENTRY: return result + # Avoid adding a config entry for a integration + # that only supports a single config entry, but already has an entry + if ( + await _support_single_config_entry_only(self.hass, flow.handler) + and flow.context["source"] != SOURCE_IGNORE + and self.config_entries.async_entries(flow.handler, include_ignore=False) + ): + return FlowResult( + type=data_entry_flow.FlowResultType.ABORT, + flow_id=flow.flow_id, + handler=flow.handler, + reason="single_instance_allowed", + translation_domain=HA_DOMAIN, + ) + # Check if config entry exists with unique ID. Unload it. existing_entry = None @@ -1398,18 +1416,6 @@ class ConfigEntries: f"An entry with the id {entry.entry_id} already exists." ) - # Avoid adding a config entry for a integration - # that only supports a single config entry, but already has an entry - if ( - await _support_single_config_entry_only(self.hass, entry.domain) - and entry.source != SOURCE_IGNORE - and self.async_entries(entry.domain, include_ignore=False) - ): - raise HomeAssistantError( - f"An entry for {entry.domain} already exists," - f" but integration supports only one config entry" - ) - self._entries[entry.entry_id] = entry self._async_dispatch(ConfigEntryChange.ADDED, entry) await self.async_setup(entry.entry_id) diff --git a/homeassistant/data_entry_flow.py b/homeassistant/data_entry_flow.py index d08e76edbd2..bbb6621cfcc 100644 --- a/homeassistant/data_entry_flow.py +++ b/homeassistant/data_entry_flow.py @@ -157,6 +157,7 @@ class FlowResult(TypedDict, total=False): result: Any step_id: str title: str + translation_domain: str type: FlowResultType url: str version: int diff --git a/tests/test_config_entries.py b/tests/test_config_entries.py index fb22aa99830..ab1942b5c43 100644 --- a/tests/test_config_entries.py +++ b/tests/test_config_entries.py @@ -3,7 +3,6 @@ from __future__ import annotations import asyncio from collections.abc import Generator -from contextlib import AbstractContextManager, nullcontext as does_not_raise from datetime import timedelta import logging from typing import Any @@ -4435,41 +4434,32 @@ async def test_hashable_non_string_unique_id( assert entries.get_entry_by_domain_and_unique_id("test", unique_id) is None -RAISES_SINGLE_ENTRY_ERROR = pytest.raises( - HomeAssistantError, - match=( - r"Cannot start a config flow, " - r"the integration supports only a single config entry but already has one" - ), -) - - @pytest.mark.parametrize( - ("source", "user_input", "expectation", "expected_result"), + ("source", "user_input", "expected_result"), [ ( config_entries.SOURCE_IGNORE, {"unique_id": "blah", "title": "blah"}, - does_not_raise(), {"type": data_entry_flow.FlowResultType.CREATE_ENTRY}, ), ( config_entries.SOURCE_REAUTH, None, - does_not_raise(), {"type": data_entry_flow.FlowResultType.FORM, "step_id": "reauth_confirm"}, ), ( config_entries.SOURCE_UNIGNORE, None, - does_not_raise(), {"type": data_entry_flow.FlowResultType.ABORT, "reason": "not_implemented"}, ), ( config_entries.SOURCE_USER, None, - RAISES_SINGLE_ENTRY_ERROR, - {}, + { + "type": data_entry_flow.FlowResultType.ABORT, + "reason": "single_instance_allowed", + "translation_domain": HA_DOMAIN, + }, ), ], ) @@ -4478,7 +4468,6 @@ async def test_starting_config_flow_on_single_config_entry( manager: config_entries.ConfigEntries, source: str, user_input: dict, - expectation: AbstractContextManager, expected_result: dict, ) -> None: """Test starting a config flow for a single config entry integration. @@ -4520,7 +4509,7 @@ async def test_starting_config_flow_on_single_config_entry( with patch( "homeassistant.loader.async_get_integration", return_value=integration, - ), expectation: + ): result = await hass.config_entries.flow.async_init( "comp", context={"source": source}, data=user_input ) @@ -4530,30 +4519,26 @@ async def test_starting_config_flow_on_single_config_entry( @pytest.mark.parametrize( - ("source", "user_input", "expectation", "expected_result"), + ("source", "user_input", "expected_result"), [ ( config_entries.SOURCE_IGNORE, {"unique_id": "blah", "title": "blah"}, - does_not_raise(), {"type": data_entry_flow.FlowResultType.CREATE_ENTRY}, ), ( config_entries.SOURCE_REAUTH, None, - does_not_raise(), {"type": data_entry_flow.FlowResultType.FORM, "step_id": "reauth_confirm"}, ), ( config_entries.SOURCE_UNIGNORE, None, - does_not_raise(), {"type": data_entry_flow.FlowResultType.ABORT, "reason": "not_implemented"}, ), ( config_entries.SOURCE_USER, None, - does_not_raise(), {"type": data_entry_flow.FlowResultType.ABORT, "reason": "not_implemented"}, ), ], @@ -4563,7 +4548,6 @@ async def test_starting_config_flow_on_single_config_entry_2( manager: config_entries.ConfigEntries, source: str, user_input: dict, - expectation: AbstractContextManager, expected_result: dict, ) -> None: """Test starting a config flow for a single config entry integration. @@ -4597,7 +4581,7 @@ async def test_starting_config_flow_on_single_config_entry_2( with patch( "homeassistant.loader.async_get_integration", return_value=integration, - ), expectation: + ): result = await hass.config_entries.flow.async_init( "comp", context={"source": source}, data=user_input ) @@ -4610,6 +4594,19 @@ async def test_avoid_adding_second_config_entry_on_single_config_entry( hass: HomeAssistant, manager: config_entries.ConfigEntries ) -> None: """Test that we cannot add a second entry for a single config entry integration.""" + + class TestFlow(config_entries.ConfigFlow): + """Test flow.""" + + VERSION = 1 + + async def async_step_user(self, user_input=None): + """Test user step.""" + if user_input is None: + return self.async_show_form(step_id="user") + + return self.async_create_entry(title="yo", data={}) + integration = loader.Integration( hass, "components.comp", @@ -4622,27 +4619,36 @@ async def test_avoid_adding_second_config_entry_on_single_config_entry( "single_config_entry": True, }, ) - entry = MockConfigEntry( - domain="comp", - unique_id="1234", - title="Test", - data={"vendor": "data"}, - options={"vendor": "options"}, - ) - entry.add_to_hass(hass) + mock_integration(hass, MockModule("comp")) + mock_platform(hass, "comp.config_flow", None) with patch( "homeassistant.loader.async_get_integration", return_value=integration, - ), pytest.raises( - HomeAssistantError, - match=r"An entry for comp already exists, but integration supports only one config entry", - ): - await hass.config_entries.async_add( - MockConfigEntry( - title="Second comp entry", domain="comp", data={"vendor": "data2"} - ) + ), patch.dict(config_entries.HANDLERS, {"comp": TestFlow}): + # Start a flow + result = await manager.flow.async_init( + "comp", context={"source": config_entries.SOURCE_USER} ) + assert result["type"] == data_entry_flow.FlowResultType.FORM + + # Add a config entry + entry = MockConfigEntry( + domain="comp", + unique_id="1234", + title="Test", + data={"vendor": "data"}, + options={"vendor": "options"}, + ) + entry.add_to_hass(hass) + + # Finish the in progress flow + result = await manager.flow.async_configure( + result["flow_id"], user_input={"host": "127.0.0.1"} + ) + assert result["type"] == data_entry_flow.FlowResultType.ABORT + assert result["reason"] == "single_instance_allowed" + assert result["translation_domain"] == HA_DOMAIN async def test_in_progress_get_canceled_when_entry_is_created(