From c24b811180f322e8010f6ae3abbde9f07d7bb0ff Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Wed, 21 Sep 2022 14:52:06 -0400 Subject: [PATCH] Use dbus-fast unpack_variants option (#3885) * Use dbus-fast unpack_variants option * More readable log on signals --- requirements.txt | 2 +- supervisor/dbus/network/setting/__init__.py | 6 ++-- supervisor/host/network.py | 2 -- supervisor/utils/dbus.py | 40 ++++++++++----------- tests/conftest.py | 2 +- tests/dbus/network/setting/test_init.py | 4 +-- tests/dbus/test_interface.py | 4 +-- tests/host/test_network.py | 4 +-- tests/utils/test_dbus.py | 19 ---------- 9 files changed, 30 insertions(+), 53 deletions(-) delete mode 100644 tests/utils/test_dbus.py diff --git a/requirements.txt b/requirements.txt index d74be75a0..acd09feda 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,4 +22,4 @@ ruamel.yaml==0.17.21 securetar==2022.2.0 sentry-sdk==1.9.8 voluptuous==0.13.1 -dbus-fast==1.6.0 +dbus-fast==1.7.0 diff --git a/supervisor/dbus/network/setting/__init__.py b/supervisor/dbus/network/setting/__init__.py index 240d7e44f..6d71f5d13 100644 --- a/supervisor/dbus/network/setting/__init__.py +++ b/supervisor/dbus/network/setting/__init__.py @@ -130,7 +130,7 @@ class NetworkSetting(DBusInterface): async def update(self, settings: Any) -> None: """Update connection settings.""" new_settings = await self.dbus.Settings.Connection.call_get_settings( - remove_signature=False + unpack_variants=False ) _merge_settings_attribute(new_settings, settings, CONF_ATTR_CONNECTION) @@ -165,9 +165,7 @@ class NetworkSetting(DBusInterface): await super().connect(bus) await self.reload() - # pylint: disable=unnecessary-lambda - # wrapper created by annotation fails the signature test, varargs not supported - self.dbus.Settings.Connection.on_updated(lambda: self.reload()) + self.dbus.Settings.Connection.on_updated(self.reload) def disconnect(self) -> None: """Disconnect from D-Bus.""" diff --git a/supervisor/host/network.py b/supervisor/host/network.py index e19261d54..80cd199a1 100644 --- a/supervisor/host/network.py +++ b/supervisor/host/network.py @@ -36,7 +36,6 @@ from ..exceptions import ( from ..jobs.const import JobCondition from ..jobs.decorator import Job from ..resolution.checks.network_interface_ipv4 import CheckNetworkInterfaceIPV4 -from ..utils.dbus import DBus from .const import AuthMethod, InterfaceMethod, InterfaceType, WifiMode _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -148,7 +147,6 @@ class NetworkManager(CoreSysAttributes): if interface != DBUS_IFACE_NM: return - changed = DBus.remove_dbus_signature(changed) connectivity_check: bool | None = changed.get(DBUS_ATTR_CONNECTION_ENABLED) connectivity: bool | None = changed.get(DBUS_ATTR_CONNECTIVITY) diff --git a/supervisor/utils/dbus.py b/supervisor/utils/dbus.py index fdfff36e7..a0e63512c 100644 --- a/supervisor/utils/dbus.py +++ b/supervisor/utils/dbus.py @@ -54,17 +54,6 @@ class DBus: _LOGGER.debug("Connect to D-Bus: %s - %s", bus_name, object_path) return self - @staticmethod - def remove_dbus_signature(data: Any) -> Any: - """Remove signature info.""" - if isinstance(data, Variant): - return DBus.remove_dbus_signature(data.value) - if isinstance(data, dict): - return {k: DBus.remove_dbus_signature(v) for k, v in data.items()} - if isinstance(data, list): - return [DBus.remove_dbus_signature(item) for item in data] - return data - @staticmethod def from_dbus_error(err: DBusError) -> HassioNotSupportedError | DBusError: """Return correct dbus error based on type.""" @@ -91,7 +80,7 @@ class DBus: proxy_interface: ProxyInterface, method: str, *args, - remove_signature: bool = True, + unpack_variants: bool = True, ) -> Any: """Call a dbus method and handle the signature and errors.""" _LOGGER.debug( @@ -101,8 +90,9 @@ class DBus: proxy_interface.path, ) try: - body = await getattr(proxy_interface, method)(*args) - return DBus.remove_dbus_signature(body) if remove_signature else body + return await getattr(proxy_interface, method)( + *args, unpack_variants=unpack_variants + ) except DBusError as err: raise DBus.from_dbus_error(err) @@ -178,10 +168,18 @@ class DBus: if interface != prop_interface: return + _LOGGER.debug( + "Property change for %s-%s: %s changed & %s invalidated", + self.bus_name, + self.object_path, + changed.keys(), + invalidated, + ) + if invalidated: await update() else: - await update(DBus.remove_dbus_signature(changed)) + await update(changed) self.properties.on_properties_changed(sync_property_change) return sync_property_change @@ -195,7 +193,9 @@ class DBus: for intr, signals in self._signal_monitors.items(): for name, callbacks in signals.items(): for callback in callbacks: - getattr(self._proxies[intr], f"off_{name}")(callback) + getattr(self._proxies[intr], f"off_{name}")( + callback, unpack_variants=True + ) self._signal_monitors = {} @@ -254,7 +254,7 @@ class DBusCallWrapper: if dbus_type == "on": def _on_signal(callback: Callable): - getattr(self._proxy, name)(callback) + getattr(self._proxy, name)(callback, unpack_variants=True) # pylint: disable=protected-access if self.interface not in self.dbus._signal_monitors: @@ -272,7 +272,7 @@ class DBusCallWrapper: return _on_signal def _off_signal(callback: Callable): - getattr(self._proxy, name)(callback) + getattr(self._proxy, name)(callback, unpack_variants=True) # pylint: disable=protected-access if ( @@ -298,9 +298,9 @@ class DBusCallWrapper: if dbus_type in ["call", "get", "set"]: - def _method_wrapper(*args, remove_signature: bool = True) -> Awaitable: + def _method_wrapper(*args, unpack_variants: bool = True) -> Awaitable: return DBus.call_dbus( - self._proxy, name, *args, remove_signature=remove_signature + self._proxy, name, *args, unpack_variants=unpack_variants ) return _method_wrapper diff --git a/tests/conftest.py b/tests/conftest.py index 823b436d5..2a926392d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -171,7 +171,7 @@ def dbus(dbus_bus: MessageBus) -> DBus: proxy_interface: ProxyInterface, method: str, *args, - remove_signature: bool = True, + unpack_variants: bool = True, ): if ( proxy_interface.introspection.name == DBUS_INTERFACE_PROPERTIES diff --git a/tests/dbus/network/setting/test_init.py b/tests/dbus/network/setting/test_init.py index 2744f5ac5..c4532f799 100644 --- a/tests/dbus/network/setting/test_init.py +++ b/tests/dbus/network/setting/test_init.py @@ -72,10 +72,10 @@ SETTINGS_WITH_SIGNATURE = { async def mock_call_dbus_get_settings_signature( - _: ProxyInterface, method: str, *args, remove_signature: bool = True + _: ProxyInterface, method: str, *args, unpack_variants: bool = True ) -> list[dict[str, Any]]: """Call dbus method mock for get settings that keeps signature.""" - if method == "call_get_settings" and not remove_signature: + if method == "call_get_settings" and not unpack_variants: return SETTINGS_WITH_SIGNATURE else: assert method == "call_update" diff --git a/tests/dbus/test_interface.py b/tests/dbus/test_interface.py index 612ce1c3d..6aa54a950 100644 --- a/tests/dbus/test_interface.py +++ b/tests/dbus/test_interface.py @@ -80,7 +80,7 @@ async def test_signal_listener_disconnect( device = dev proxy.obj.dbus.on_device_added(callback) - proxy.on_device_added.assert_called_once_with(callback) + proxy.on_device_added.assert_called_once_with(callback, unpack_variants=True) fire_watched_signal( proxy.obj, "org.freedesktop.NetworkManager.DeviceAdded", ["/test/obj/1"] @@ -89,4 +89,4 @@ async def test_signal_listener_disconnect( assert device == "/test/obj/1" proxy.obj.disconnect() - proxy.off_device_added.assert_called_once_with(callback) + proxy.off_device_added.assert_called_once_with(callback, unpack_variants=True) diff --git a/tests/host/test_network.py b/tests/host/test_network.py index 1c00b1ba6..8be0f55ab 100644 --- a/tests/host/test_network.py +++ b/tests/host/test_network.py @@ -140,7 +140,7 @@ async def test_scan_wifi_with_failures(coresys: CoreSys, caplog): proxy_interface: ProxyInterface, method: str, *args, - remove_signature: bool = True, + unpack_variants: bool = True, ): if method == "call_get_all_access_points": return [ @@ -150,7 +150,7 @@ async def test_scan_wifi_with_failures(coresys: CoreSys, caplog): ] return await call_dbus( - proxy_interface, method, *args, remove_signature=remove_signature + proxy_interface, method, *args, unpack_variants=unpack_variants ) with patch("supervisor.host.network.asyncio.sleep"), patch( diff --git a/tests/utils/test_dbus.py b/tests/utils/test_dbus.py deleted file mode 100644 index 3cb3806c0..000000000 --- a/tests/utils/test_dbus.py +++ /dev/null @@ -1,19 +0,0 @@ -"""Check dbus-next implementation.""" -from dbus_fast.signature import Variant - -from supervisor.utils.dbus import DBus - - -def test_remove_dbus_signature(): - """Check D-Bus signature clean-up.""" - test = DBus.remove_dbus_signature(Variant("s", "Value")) - assert isinstance(test, str) - assert test == "Value" - - test_dict = DBus.remove_dbus_signature({"Key": Variant("s", "Value")}) - assert isinstance(test_dict["Key"], str) - assert test_dict["Key"] == "Value" - - test_dict = DBus.remove_dbus_signature([Variant("s", "Value")]) - assert isinstance(test_dict[0], str) - assert test_dict[0] == "Value"