From 3077363f449744e690091ea7761d0018f1e4ee78 Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Sun, 25 Apr 2021 06:27:46 -0700 Subject: [PATCH] Supplementary fixes to new motionEye integration (#49626) --- .../components/motioneye/__init__.py | 28 +--- homeassistant/components/motioneye/camera.py | 9 +- .../components/motioneye/config_flow.py | 158 ++++++++++-------- homeassistant/components/motioneye/const.py | 1 - .../components/motioneye/test_config_flow.py | 78 +++++++-- 5 files changed, 162 insertions(+), 112 deletions(-) diff --git a/homeassistant/components/motioneye/__init__.py b/homeassistant/components/motioneye/__init__.py index 61e7a7d12f3..5387de8225c 100644 --- a/homeassistant/components/motioneye/__init__.py +++ b/homeassistant/components/motioneye/__init__.py @@ -13,10 +13,10 @@ from motioneye_client.client import ( from motioneye_client.const import KEY_CAMERAS, KEY_ID, KEY_NAME from homeassistant.components.camera.const import DOMAIN as CAMERA_DOMAIN -from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntry -from homeassistant.const import CONF_SOURCE, CONF_URL +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_URL from homeassistant.core import HomeAssistant, callback -from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady from homeassistant.helpers import device_registry as dr from homeassistant.helpers.dispatcher import ( async_dispatcher_connect, @@ -28,7 +28,6 @@ from .const import ( CONF_ADMIN_PASSWORD, CONF_ADMIN_USERNAME, CONF_CLIENT, - CONF_CONFIG_ENTRY, CONF_COORDINATOR, CONF_SURVEILLANCE_PASSWORD, CONF_SURVEILLANCE_USERNAME, @@ -98,22 +97,6 @@ def listen_for_new_cameras( ) -async def _create_reauth_flow( - hass: HomeAssistant, - config_entry: ConfigEntry, -) -> None: - hass.async_create_task( - hass.config_entries.flow.async_init( - DOMAIN, - context={ - CONF_SOURCE: SOURCE_REAUTH, - CONF_CONFIG_ENTRY: config_entry, - }, - data=config_entry.data, - ) - ) - - @callback def _add_camera( hass: HomeAssistant, @@ -155,10 +138,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: try: await client.async_client_login() - except MotionEyeClientInvalidAuthError: + except MotionEyeClientInvalidAuthError as exc: await client.async_client_close() - await _create_reauth_flow(hass, entry) - return False + raise ConfigEntryAuthFailed from exc except MotionEyeClientError as exc: await client.async_client_close() raise ConfigEntryNotReady from exc diff --git a/homeassistant/components/motioneye/camera.py b/homeassistant/components/motioneye/camera.py index 58df22198bf..5f64616e1a4 100644 --- a/homeassistant/components/motioneye/camera.py +++ b/homeassistant/components/motioneye/camera.py @@ -59,7 +59,7 @@ PLATFORMS = ["camera"] async def async_setup_entry( hass: HomeAssistant, entry: ConfigEntry, async_add_entities: Callable -) -> bool: +) -> None: """Set up motionEye from a config entry.""" entry_data = hass.data[DOMAIN][entry.entry_id] @@ -82,7 +82,6 @@ async def async_setup_entry( ) listen_for_new_cameras(hass, entry, camera_add) - return True class MotionEyeMjpegCamera(MjpegCamera, CoordinatorEntity): @@ -96,7 +95,7 @@ class MotionEyeMjpegCamera(MjpegCamera, CoordinatorEntity): camera: dict[str, Any], client: MotionEyeClient, coordinator: DataUpdateCoordinator, - ): + ) -> None: """Initialize a MJPEG camera.""" self._surveillance_username = username self._surveillance_password = password @@ -109,7 +108,7 @@ class MotionEyeMjpegCamera(MjpegCamera, CoordinatorEntity): config_entry_id, self._camera_id, TYPE_MOTIONEYE_MJPEG_CAMERA ) self._motion_detection_enabled: bool = camera.get(KEY_MOTION_DETECTION, False) - self._available = MotionEyeMjpegCamera._is_acceptable_streaming_camera(camera) + self._available = self._is_acceptable_streaming_camera(camera) # motionEye cameras are always streaming or unavailable. self.is_streaming = True @@ -184,7 +183,7 @@ class MotionEyeMjpegCamera(MjpegCamera, CoordinatorEntity): available = False if self.coordinator.last_update_success: camera = get_camera_from_cameras(self._camera_id, self.coordinator.data) - if MotionEyeMjpegCamera._is_acceptable_streaming_camera(camera): + if self._is_acceptable_streaming_camera(camera): assert camera self._set_mjpeg_camera_state_for_camera(camera) self._motion_detection_enabled = camera.get(KEY_MOTION_DETECTION, False) diff --git a/homeassistant/components/motioneye/config_flow.py b/homeassistant/components/motioneye/config_flow.py index f0ff0a38836..5e37ae7bf6b 100644 --- a/homeassistant/components/motioneye/config_flow.py +++ b/homeassistant/components/motioneye/config_flow.py @@ -24,7 +24,6 @@ from . import create_motioneye_client from .const import ( CONF_ADMIN_PASSWORD, CONF_ADMIN_USERNAME, - CONF_CONFIG_ENTRY, CONF_SURVEILLANCE_PASSWORD, CONF_SURVEILLANCE_USERNAME, DOMAIN, @@ -43,81 +42,96 @@ class MotionEyeConfigFlow(ConfigFlow, domain=DOMAIN): self, user_input: ConfigType | None = None ) -> dict[str, Any]: """Handle the initial step.""" - out: dict[str, Any] = {} - errors = {} + + def _get_form( + user_input: ConfigType, errors: dict[str, str] | None = None + ) -> dict[str, Any]: + """Show the form to the user.""" + return self.async_show_form( + step_id="user", + data_schema=vol.Schema( + { + vol.Required( + CONF_URL, default=user_input.get(CONF_URL, "") + ): str, + vol.Optional( + CONF_ADMIN_USERNAME, + default=user_input.get(CONF_ADMIN_USERNAME), + ): str, + vol.Optional( + CONF_ADMIN_PASSWORD, + default=user_input.get(CONF_ADMIN_PASSWORD), + ): str, + vol.Optional( + CONF_SURVEILLANCE_USERNAME, + default=user_input.get(CONF_SURVEILLANCE_USERNAME), + ): str, + vol.Optional( + CONF_SURVEILLANCE_PASSWORD, + default=user_input.get(CONF_SURVEILLANCE_PASSWORD), + ): str, + } + ), + errors=errors, + ) + + reauth_entry = None + if self.context.get("entry_id"): + reauth_entry = self.hass.config_entries.async_get_entry( + self.context["entry_id"] + ) + if user_input is None: - entry = self.context.get(CONF_CONFIG_ENTRY) - user_input = entry.data if entry else {} - else: - try: - # Cannot use cv.url validation in the schema itself, so - # apply extra validation here. - cv.url(user_input[CONF_URL]) - except vol.Invalid: - errors["base"] = "invalid_url" - else: - client = create_motioneye_client( - user_input[CONF_URL], - admin_username=user_input.get(CONF_ADMIN_USERNAME), - admin_password=user_input.get(CONF_ADMIN_PASSWORD), - surveillance_username=user_input.get(CONF_SURVEILLANCE_USERNAME), - surveillance_password=user_input.get(CONF_SURVEILLANCE_PASSWORD), - ) + return _get_form(reauth_entry.data if reauth_entry else {}) - try: - await client.async_client_login() - except MotionEyeClientConnectionError: - errors["base"] = "cannot_connect" - except MotionEyeClientInvalidAuthError: - errors["base"] = "invalid_auth" - except MotionEyeClientRequestError: - errors["base"] = "unknown" - else: - entry = self.context.get(CONF_CONFIG_ENTRY) - if ( - self.context.get(CONF_SOURCE) == SOURCE_REAUTH - and entry is not None - ): - self.hass.config_entries.async_update_entry( - entry, data=user_input - ) - # Need to manually reload, as the listener won't have been - # installed because the initial load did not succeed (the reauth - # flow will not be initiated if the load succeeds). - await self.hass.config_entries.async_reload(entry.entry_id) - out = self.async_abort(reason="reauth_successful") - return out + try: + # Cannot use cv.url validation in the schema itself, so + # apply extra validation here. + cv.url(user_input[CONF_URL]) + except vol.Invalid: + return _get_form(user_input, {"base": "invalid_url"}) - out = self.async_create_entry( - title=f"{user_input[CONF_URL]}", - data=user_input, - ) - return out - - out = self.async_show_form( - step_id="user", - data_schema=vol.Schema( - { - vol.Required(CONF_URL, default=user_input.get(CONF_URL, "")): str, - vol.Optional( - CONF_ADMIN_USERNAME, default=user_input.get(CONF_ADMIN_USERNAME) - ): str, - vol.Optional( - CONF_ADMIN_PASSWORD, default=user_input.get(CONF_ADMIN_PASSWORD) - ): str, - vol.Optional( - CONF_SURVEILLANCE_USERNAME, - default=user_input.get(CONF_SURVEILLANCE_USERNAME), - ): str, - vol.Optional( - CONF_SURVEILLANCE_PASSWORD, - default=user_input.get(CONF_SURVEILLANCE_PASSWORD), - ): str, - } - ), - errors=errors, + client = create_motioneye_client( + user_input[CONF_URL], + admin_username=user_input.get(CONF_ADMIN_USERNAME), + admin_password=user_input.get(CONF_ADMIN_PASSWORD), + surveillance_username=user_input.get(CONF_SURVEILLANCE_USERNAME), + surveillance_password=user_input.get(CONF_SURVEILLANCE_PASSWORD), + ) + + errors = {} + try: + await client.async_client_login() + except MotionEyeClientConnectionError: + errors["base"] = "cannot_connect" + except MotionEyeClientInvalidAuthError: + errors["base"] = "invalid_auth" + except MotionEyeClientRequestError: + errors["base"] = "unknown" + finally: + await client.async_client_close() + + if errors: + return _get_form(user_input, errors) + + if self.context.get(CONF_SOURCE) == SOURCE_REAUTH and reauth_entry is not None: + self.hass.config_entries.async_update_entry(reauth_entry, data=user_input) + # Need to manually reload, as the listener won't have been + # installed because the initial load did not succeed (the reauth + # flow will not be initiated if the load succeeds). + await self.hass.config_entries.async_reload(reauth_entry.entry_id) + return self.async_abort(reason="reauth_successful") + + # Search for duplicates: there isn't a useful unique_id, but + # at least prevent entries with the same motionEye URL. + for existing_entry in self._async_current_entries(include_ignore=False): + if existing_entry.data.get(CONF_URL) == user_input[CONF_URL]: + return self.async_abort(reason="already_configured") + + return self.async_create_entry( + title=f"{user_input[CONF_URL]}", + data=user_input, ) - return out async def async_step_reauth( self, diff --git a/homeassistant/components/motioneye/const.py b/homeassistant/components/motioneye/const.py index a76053b2854..fbd0d9b4d2e 100644 --- a/homeassistant/components/motioneye/const.py +++ b/homeassistant/components/motioneye/const.py @@ -3,7 +3,6 @@ from datetime import timedelta DOMAIN = "motioneye" -CONF_CONFIG_ENTRY = "config_entry" CONF_CLIENT = "client" CONF_COORDINATOR = "coordinator" CONF_ADMIN_PASSWORD = "admin_password" diff --git a/tests/components/motioneye/test_config_flow.py b/tests/components/motioneye/test_config_flow.py index 2c16aea14be..d8700e162c4 100644 --- a/tests/components/motioneye/test_config_flow.py +++ b/tests/components/motioneye/test_config_flow.py @@ -12,7 +12,6 @@ from homeassistant import config_entries, data_entry_flow, setup from homeassistant.components.motioneye.const import ( CONF_ADMIN_PASSWORD, CONF_ADMIN_USERNAME, - CONF_CONFIG_ENTRY, CONF_SURVEILLANCE_PASSWORD, CONF_SURVEILLANCE_USERNAME, DOMAIN, @@ -22,6 +21,8 @@ from homeassistant.core import HomeAssistant from . import TEST_URL, create_mock_motioneye_client, create_mock_motioneye_config_entry +from tests.common import MockConfigEntry + _LOGGER = logging.getLogger(__name__) @@ -32,7 +33,7 @@ async def test_user_success(hass: HomeAssistant) -> None: DOMAIN, context={"source": config_entries.SOURCE_USER} ) assert result["type"] == "form" - assert result["errors"] == {} + assert not result["errors"] mock_client = create_mock_motioneye_client() @@ -65,6 +66,7 @@ async def test_user_success(hass: HomeAssistant) -> None: CONF_SURVEILLANCE_PASSWORD: "surveillance-password", } assert len(mock_setup_entry.mock_calls) == 1 + assert mock_client.async_client_close.called async def test_user_invalid_auth(hass: HomeAssistant) -> None: @@ -92,10 +94,11 @@ async def test_user_invalid_auth(hass: HomeAssistant) -> None: CONF_SURVEILLANCE_PASSWORD: "surveillance-password", }, ) - await mock_client.async_client_close() + await hass.async_block_till_done() assert result["type"] == "form" assert result["errors"] == {"base": "invalid_auth"} + assert mock_client.async_client_close.called async def test_user_invalid_url(hass: HomeAssistant) -> None: @@ -105,9 +108,10 @@ async def test_user_invalid_url(hass: HomeAssistant) -> None: DOMAIN, context={"source": config_entries.SOURCE_USER} ) + mock_client = create_mock_motioneye_client() with patch( "homeassistant.components.motioneye.MotionEyeClient", - return_value=create_mock_motioneye_client(), + return_value=mock_client, ): result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -119,6 +123,7 @@ async def test_user_invalid_url(hass: HomeAssistant) -> None: CONF_SURVEILLANCE_PASSWORD: "surveillance-password", }, ) + await hass.async_block_till_done() assert result["type"] == "form" assert result["errors"] == {"base": "invalid_url"} @@ -149,10 +154,11 @@ async def test_user_cannot_connect(hass: HomeAssistant) -> None: CONF_SURVEILLANCE_PASSWORD: "surveillance-password", }, ) - await mock_client.async_client_close() + await hass.async_block_till_done() assert result["type"] == "form" assert result["errors"] == {"base": "cannot_connect"} + assert mock_client.async_client_close.called async def test_user_request_error(hass: HomeAssistant) -> None: @@ -178,10 +184,11 @@ async def test_user_request_error(hass: HomeAssistant) -> None: CONF_SURVEILLANCE_PASSWORD: "surveillance-password", }, ) - await mock_client.async_client_close() + await hass.async_block_till_done() assert result["type"] == "form" assert result["errors"] == {"base": "unknown"} + assert mock_client.async_client_close.called async def test_reauth(hass: HomeAssistant) -> None: @@ -197,11 +204,11 @@ async def test_reauth(hass: HomeAssistant) -> None: DOMAIN, context={ "source": config_entries.SOURCE_REAUTH, - CONF_CONFIG_ENTRY: config_entry, + "entry_id": config_entry.entry_id, }, ) assert result["type"] == "form" - assert result["errors"] == {} + assert not result["errors"] mock_client = create_mock_motioneye_client() @@ -226,8 +233,57 @@ async def test_reauth(hass: HomeAssistant) -> None: ) await hass.async_block_till_done() - assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "reauth_successful" - assert config_entry.data == new_data + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "reauth_successful" + assert config_entry.data == new_data assert len(mock_setup_entry.mock_calls) == 1 + assert mock_client.async_client_close.called + + +async def test_duplicate(hass: HomeAssistant) -> None: + """Test that a duplicate entry (same URL) is rejected.""" + config_data = { + CONF_URL: TEST_URL, + } + + # Add an existing entry with the same URL. + existing_entry: MockConfigEntry = MockConfigEntry( # type: ignore[no-untyped-call] + domain=DOMAIN, + data=config_data, + ) + existing_entry.add_to_hass(hass) # type: ignore[no-untyped-call] + + # Now do the usual config entry process, and verify it is rejected. + create_mock_motioneye_config_entry(hass, data=config_data) + + await setup.async_setup_component(hass, "persistent_notification", {}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + + assert result["type"] == "form" + assert not result["errors"] + mock_client = create_mock_motioneye_client() + + new_data = { + CONF_URL: TEST_URL, + CONF_ADMIN_USERNAME: "admin-username", + CONF_ADMIN_PASSWORD: "admin-password", + CONF_SURVEILLANCE_USERNAME: "surveillance-username", + CONF_SURVEILLANCE_PASSWORD: "surveillance-password", + } + + with patch( + "homeassistant.components.motioneye.MotionEyeClient", + return_value=mock_client, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + new_data, + ) + await hass.async_block_till_done() + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_configured" + assert mock_client.async_client_close.called