From 9c86adf644506bdc80344c394d27995d760b6c69 Mon Sep 17 00:00:00 2001 From: jflefebvre06 Date: Sun, 19 Nov 2023 00:02:00 +0100 Subject: [PATCH] Fix integration failed when freebox is configured in bridge mode (#103221) --- .coveragerc | 2 - homeassistant/components/freebox/router.py | 44 ++++++++++++++++- tests/components/freebox/conftest.py | 21 +++++++- tests/components/freebox/const.py | 4 +- .../fixtures/lan_get_hosts_list_bridge.json | 5 ++ .../components/freebox/test_device_tracker.py | 49 +++++++++++++++++++ tests/components/freebox/test_router.py | 22 +++++++++ 7 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 tests/components/freebox/fixtures/lan_get_hosts_list_bridge.json create mode 100644 tests/components/freebox/test_device_tracker.py create mode 100644 tests/components/freebox/test_router.py diff --git a/.coveragerc b/.coveragerc index 97ee86f1d44..a05cc48785e 100644 --- a/.coveragerc +++ b/.coveragerc @@ -425,9 +425,7 @@ omit = homeassistant/components/foursquare/* homeassistant/components/free_mobile/notify.py homeassistant/components/freebox/camera.py - homeassistant/components/freebox/device_tracker.py homeassistant/components/freebox/home_base.py - homeassistant/components/freebox/router.py homeassistant/components/freebox/switch.py homeassistant/components/fritz/common.py homeassistant/components/fritz/device_tracker.py diff --git a/homeassistant/components/freebox/router.py b/homeassistant/components/freebox/router.py index 6a73624a776..765761c43f2 100644 --- a/homeassistant/components/freebox/router.py +++ b/homeassistant/components/freebox/router.py @@ -4,9 +4,11 @@ from __future__ import annotations from collections.abc import Mapping from contextlib import suppress from datetime import datetime +import json import logging import os from pathlib import Path +import re from typing import Any from freebox_api import Freepybox @@ -36,6 +38,20 @@ from .const import ( _LOGGER = logging.getLogger(__name__) +def is_json(json_str): + """Validate if a String is a JSON value or not.""" + try: + json.loads(json_str) + return True + except (ValueError, TypeError) as err: + _LOGGER.error( + "Failed to parse JSON '%s', error '%s'", + json_str, + err, + ) + return False + + async def get_api(hass: HomeAssistant, host: str) -> Freepybox: """Get the Freebox API.""" freebox_path = Store(hass, STORAGE_VERSION, STORAGE_KEY).path @@ -69,6 +85,7 @@ class FreeboxRouter: self._sw_v: str = freebox_config["firmware_version"] self._attrs: dict[str, Any] = {} + self.supports_hosts = True self.devices: dict[str, dict[str, Any]] = {} self.disks: dict[int, dict[str, Any]] = {} self.supports_raid = True @@ -89,7 +106,32 @@ class FreeboxRouter: async def update_device_trackers(self) -> None: """Update Freebox devices.""" new_device = False - fbx_devices: list[dict[str, Any]] = await self._api.lan.get_hosts_list() + + fbx_devices: list[dict[str, Any]] = [] + + # Access to Host list not available in bridge mode, API return error_code 'nodev' + if self.supports_hosts: + try: + fbx_devices = await self._api.lan.get_hosts_list() + except HttpRequestError as err: + if ( + ( + matcher := re.search( + r"Request failed \(APIResponse: (.+)\)", str(err) + ) + ) + and is_json(json_str := matcher.group(1)) + and (json_resp := json.loads(json_str)).get("error_code") == "nodev" + ): + # No need to retry, Host list not available + self.supports_hosts = False + _LOGGER.debug( + "Host list is not available using bridge mode (%s)", + json_resp.get("msg"), + ) + + else: + raise err # Adds the Freebox itself fbx_devices.append( diff --git a/tests/components/freebox/conftest.py b/tests/components/freebox/conftest.py index 8a6590d1105..39ed596e6db 100644 --- a/tests/components/freebox/conftest.py +++ b/tests/components/freebox/conftest.py @@ -1,6 +1,8 @@ """Test helpers for Freebox.""" +import json from unittest.mock import AsyncMock, PropertyMock, patch +from freebox_api.exceptions import HttpRequestError import pytest from homeassistant.core import HomeAssistant @@ -12,6 +14,7 @@ from .const import ( DATA_HOME_GET_NODES, DATA_HOME_PIR_GET_VALUES, DATA_LAN_GET_HOSTS_LIST, + DATA_LAN_GET_HOSTS_LIST_MODE_BRIDGE, DATA_STORAGE_GET_DISKS, DATA_STORAGE_GET_RAIDS, DATA_SYSTEM_GET_CONFIG, @@ -41,7 +44,9 @@ def enable_all_entities(): @pytest.fixture -def mock_device_registry_devices(hass: HomeAssistant, device_registry): +def mock_device_registry_devices( + hass: HomeAssistant, device_registry: dr.DeviceRegistry +): """Create device registry devices so the device tracker entities are enabled.""" config_entry = MockConfigEntry(domain="something_else") config_entry.add_to_hass(hass) @@ -87,3 +92,17 @@ def mock_router(mock_device_registry_devices): ) instance.close = AsyncMock() yield service_mock + + +@pytest.fixture(name="router_bridge_mode") +def mock_router_bridge_mode(mock_device_registry_devices, router): + """Mock a successful connection to Freebox Bridge mode.""" + + router().lan.get_hosts_list = AsyncMock( + side_effect=HttpRequestError( + "Request failed (APIResponse: %s)" + % json.dumps(DATA_LAN_GET_HOSTS_LIST_MODE_BRIDGE) + ) + ) + + return router diff --git a/tests/components/freebox/const.py b/tests/components/freebox/const.py index a7dd3132719..84667bf9d70 100644 --- a/tests/components/freebox/const.py +++ b/tests/components/freebox/const.py @@ -25,7 +25,9 @@ WIFI_GET_GLOBAL_CONFIG = load_json_object_fixture("freebox/wifi_get_global_confi # device_tracker DATA_LAN_GET_HOSTS_LIST = load_json_array_fixture("freebox/lan_get_hosts_list.json") - +DATA_LAN_GET_HOSTS_LIST_MODE_BRIDGE = load_json_object_fixture( + "freebox/lan_get_hosts_list_bridge.json" +) # Home # ALL diff --git a/tests/components/freebox/fixtures/lan_get_hosts_list_bridge.json b/tests/components/freebox/fixtures/lan_get_hosts_list_bridge.json new file mode 100644 index 00000000000..4afda465712 --- /dev/null +++ b/tests/components/freebox/fixtures/lan_get_hosts_list_bridge.json @@ -0,0 +1,5 @@ +{ + "msg": "Erreur lors de la récupération de la liste des hôtes : Interface invalide", + "success": false, + "error_code": "nodev" +} diff --git a/tests/components/freebox/test_device_tracker.py b/tests/components/freebox/test_device_tracker.py new file mode 100644 index 00000000000..6d4ca5fb7ee --- /dev/null +++ b/tests/components/freebox/test_device_tracker.py @@ -0,0 +1,49 @@ +"""Tests for the Freebox device trackers.""" +from unittest.mock import Mock + +from freezegun.api import FrozenDateTimeFactory + +from homeassistant.components.device_tracker import DOMAIN as DEVICE_TRACKER_DOMAIN +from homeassistant.components.freebox import SCAN_INTERVAL +from homeassistant.core import HomeAssistant + +from .common import setup_platform + +from tests.common import async_fire_time_changed + + +async def test_router_mode( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + router: Mock, +) -> None: + """Test get_hosts_list invoqued multiple times if freebox into router mode.""" + await setup_platform(hass, DEVICE_TRACKER_DOMAIN) + + assert router().lan.get_hosts_list.call_count == 1 + + # Simulate an update + freezer.tick(SCAN_INTERVAL) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert router().lan.get_hosts_list.call_count == 2 + + +async def test_bridge_mode( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + router_bridge_mode: Mock, +) -> None: + """Test get_hosts_list invoqued once if freebox into bridge mode.""" + await setup_platform(hass, DEVICE_TRACKER_DOMAIN) + + assert router_bridge_mode().lan.get_hosts_list.call_count == 1 + + # Simulate an update + freezer.tick(SCAN_INTERVAL) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + # If get_hosts_list failed, not called again + assert router_bridge_mode().lan.get_hosts_list.call_count == 1 diff --git a/tests/components/freebox/test_router.py b/tests/components/freebox/test_router.py new file mode 100644 index 00000000000..595aab24fc9 --- /dev/null +++ b/tests/components/freebox/test_router.py @@ -0,0 +1,22 @@ +"""Tests for the Freebox utility methods.""" +import json + +from homeassistant.components.freebox.router import is_json + +from .const import DATA_LAN_GET_HOSTS_LIST_MODE_BRIDGE, WIFI_GET_GLOBAL_CONFIG + + +async def test_is_json() -> None: + """Test is_json method.""" + + # Valid JSON values + assert is_json("{}") + assert is_json('{ "simple":"json" }') + assert is_json(json.dumps(WIFI_GET_GLOBAL_CONFIG)) + assert is_json(json.dumps(DATA_LAN_GET_HOSTS_LIST_MODE_BRIDGE)) + + # Not valid JSON values + assert not is_json(None) + assert not is_json("") + assert not is_json("XXX") + assert not is_json("{XXX}")