From c05eca1c82a054a8d286a5fbc0200ac5d90e256f Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Wed, 8 Dec 2021 21:28:26 +0100 Subject: [PATCH] Remove log flooding prevention logic from discovery info (#61243) Co-authored-by: epenet --- homeassistant/components/dhcp/__init__.py | 27 ++++----- homeassistant/components/mqtt/__init__.py | 15 ++--- homeassistant/components/ssdp/__init__.py | 41 +++++-------- homeassistant/components/usb/__init__.py | 15 ++--- homeassistant/components/zeroconf/__init__.py | 27 ++++----- tests/components/dhcp/test_init.py | 28 ++++----- tests/components/mqtt/test_init.py | 12 ++-- tests/components/ssdp/test_init.py | 58 +++++++++++++++++++ tests/components/usb/test_init.py | 11 ++-- tests/components/zeroconf/test_init.py | 28 ++++----- tests/conftest.py | 29 +++++++++- tests/helpers/conftest.py | 31 ---------- 12 files changed, 163 insertions(+), 159 deletions(-) delete mode 100644 tests/helpers/conftest.py diff --git a/homeassistant/components/dhcp/__init__.py b/homeassistant/components/dhcp/__init__.py index 7da74fb66b0..aa3fb2118cb 100644 --- a/homeassistant/components/dhcp/__init__.py +++ b/homeassistant/components/dhcp/__init__.py @@ -68,22 +68,17 @@ class DhcpServiceInfo(BaseServiceInfo): hostname: str macaddress: str - # Used to prevent log flooding. To be removed in 2022.6 - _warning_logged: bool = False - def __getitem__(self, name: str) -> Any: """ Enable method for compatibility reason. Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={DOMAIN}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) return getattr(self, name) def get(self, name: str, default: Any = None) -> Any: @@ -92,13 +87,11 @@ class DhcpServiceInfo(BaseServiceInfo): Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={DOMAIN}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) if hasattr(self, name): return getattr(self, name) return default diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index cdf37cd6381..ae7b60c4454 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -261,22 +261,17 @@ class MqttServiceInfo(BaseServiceInfo): subscribed_topic: str timestamp: dt.datetime - # Used to prevent log flooding. To be removed in 2022.6 - _warning_logged: bool = False - def __getitem__(self, name: str) -> Any: """ Allow property access by name for compatibility reason. Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={"mqtt"}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) return getattr(self, name) diff --git a/homeassistant/components/ssdp/__init__.py b/homeassistant/components/ssdp/__init__.py index 57948000701..f7ccff153ac 100644 --- a/homeassistant/components/ssdp/__init__.py +++ b/homeassistant/components/ssdp/__init__.py @@ -103,22 +103,17 @@ class SsdpServiceInfo( ): """Prepared info from ssdp/upnp entries.""" - # Used to prevent log flooding. To be removed in 2022.6 - _warning_logged: bool = False - def __getitem__(self, name: str) -> Any: """ Allow property access by name for compatibility reason. Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={DOMAIN}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) # Use a property if it is available, fallback to upnp data if hasattr(self, name): return getattr(self, name) @@ -132,13 +127,11 @@ class SsdpServiceInfo( Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={DOMAIN}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) if hasattr(self, name): return getattr(self, name) return self.upnp.get(name, self.ssdp_headers.get(name, default)) @@ -149,14 +142,12 @@ class SsdpServiceInfo( Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - "accessed discovery_info.__contains__() instead of discovery_info.upnp.__contains__() " - "or discovery_info.ssdp_headers.__contains__(); this will fail in version 2022.6", - exclude_integrations={DOMAIN}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info.__contains__('{name}') instead of discovery_info.upnp.__contains__('{name}') " + f"or discovery_info.ssdp_headers.__contains__('{name}'); this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) if hasattr(self, name): return getattr(self, name) is not None return name in self.upnp or name in self.ssdp_headers diff --git a/homeassistant/components/usb/__init__.py b/homeassistant/components/usb/__init__.py index 87216c65e9e..ec3ffa0405a 100644 --- a/homeassistant/components/usb/__init__.py +++ b/homeassistant/components/usb/__init__.py @@ -44,22 +44,17 @@ class UsbServiceInfo(BaseServiceInfo): manufacturer: str | None description: str | None - # Used to prevent log flooding. To be removed in 2022.6 - _warning_logged: bool = False - def __getitem__(self, name: str) -> Any: """ Allow property access by name for compatibility reason. Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={"usb"}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) return getattr(self, name) diff --git a/homeassistant/components/zeroconf/__init__.py b/homeassistant/components/zeroconf/__init__.py index 50db346451f..221a4d6b834 100644 --- a/homeassistant/components/zeroconf/__init__.py +++ b/homeassistant/components/zeroconf/__init__.py @@ -107,22 +107,17 @@ class ZeroconfServiceInfo(BaseServiceInfo): name: str properties: dict[str, Any] - # Used to prevent log flooding. To be removed in 2022.6 - _warning_logged: bool = False - def __getitem__(self, name: str) -> Any: """ Enable method for compatibility reason. Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={DOMAIN}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) return getattr(self, name) def get(self, name: str, default: Any = None) -> Any: @@ -131,13 +126,11 @@ class ZeroconfServiceInfo(BaseServiceInfo): Deprecated, and will be removed in version 2022.6. """ - if not self._warning_logged: - report( - f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6", - exclude_integrations={DOMAIN}, - error_if_core=False, - ) - self._warning_logged = True + report( + f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6", + exclude_integrations={DOMAIN}, + error_if_core=False, + ) if hasattr(self, name): return getattr(self, name) return default diff --git a/tests/components/dhcp/test_init.py b/tests/components/dhcp/test_init.py index 41059722113..fb3387aeab6 100644 --- a/tests/components/dhcp/test_init.py +++ b/tests/components/dhcp/test_init.py @@ -3,7 +3,8 @@ import datetime import threading from unittest.mock import MagicMock, patch -from scapy import arch # pylint: unused-import # noqa: F401 +import pytest +from scapy import arch # pylint: disable=unused-import # noqa: F401 from scapy.error import Scapy_Exception from scapy.layers.dhcp import DHCP from scapy.layers.l2 import Ether @@ -845,6 +846,7 @@ async def test_aiodiscover_finds_new_hosts_after_interval(hass): ) +@pytest.mark.usefixtures("mock_integration_frame") async def test_service_info_compatibility(hass, caplog): """Test compatibility with old-style dict. @@ -856,21 +858,13 @@ async def test_service_info_compatibility(hass, caplog): macaddress="b8b7f16db533", ) - # Ensure first call get logged - assert discovery_info["ip"] == "192.168.210.56" - assert discovery_info.get("ip") == "192.168.210.56" + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info["ip"] == "192.168.210.56" + assert "Detected integration that accessed discovery_info['ip']" in caplog.text + + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info.get("ip") == "192.168.210.56" + assert "Detected integration that accessed discovery_info.get('ip')" in caplog.text + assert discovery_info.get("ip", "fallback_host") == "192.168.210.56" assert discovery_info.get("invalid_key", "fallback_host") == "fallback_host" - assert "Detected code that accessed discovery_info['ip']" in caplog.text - assert "Detected code that accessed discovery_info.get('ip')" not in caplog.text - - # Ensure second call doesn't get logged - caplog.clear() - assert discovery_info["ip"] == "192.168.210.56" - assert discovery_info.get("ip") == "192.168.210.56" - assert "Detected code that accessed discovery_info['ip']" not in caplog.text - assert "Detected code that accessed discovery_info.get('ip')" not in caplog.text - - discovery_info._warning_logged = False # pylint: disable=[protected-access] - assert discovery_info.get("ip") == "192.168.210.56" - assert "Detected code that accessed discovery_info.get('ip')" in caplog.text diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index acc25b59442..a4a3c1d6909 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -1814,6 +1814,7 @@ async def test_publish_json_from_template(hass, mqtt_mock): assert mqtt_mock.async_publish.call_args[0][1] == test_str +@pytest.mark.usefixtures("mock_integration_frame") async def test_service_info_compatibility(hass, caplog): """Test compatibility with old-style dict. @@ -1828,11 +1829,6 @@ async def test_service_info_compatibility(hass, caplog): timestamp=None, ) - # Ensure first call get logged - assert discovery_info["topic"] == "tasmota/discovery/DC4F220848A2/config" - assert "Detected code that accessed discovery_info['topic']" in caplog.text - - # Ensure second call doesn't get logged - caplog.clear() - assert discovery_info["topic"] == "tasmota/discovery/DC4F220848A2/config" - assert "Detected code that accessed discovery_info['topic']" not in caplog.text + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info["topic"] == "tasmota/discovery/DC4F220848A2/config" + assert "Detected integration that accessed discovery_info['topic']" in caplog.text diff --git a/tests/components/ssdp/test_init.py b/tests/components/ssdp/test_init.py index 9e4d7424eb9..758f8fb79ba 100644 --- a/tests/components/ssdp/test_init.py +++ b/tests/components/ssdp/test_init.py @@ -793,3 +793,61 @@ async def test_ipv4_does_additional_search_for_sonos( ), ) assert ssdp_listener.async_search.call_args[1] == {} + + +@pytest.mark.usefixtures("mock_integration_frame") +async def test_service_info_compatibility(hass, caplog): + """Test compatibility with old-style dict. + + To be removed in 2022.6 + """ + discovery_info = ssdp.SsdpServiceInfo( + ssdp_st="mock-st", + ssdp_location="http://1.1.1.1", + ssdp_usn="uuid:mock-udn::mock-st", + ssdp_server="mock-server", + ssdp_ext="", + ssdp_headers=_ssdp_headers( + { + "st": "mock-st", + "location": "http://1.1.1.1", + "usn": "uuid:mock-udn::mock-st", + "server": "mock-server", + "ext": "", + } + ), + upnp={ssdp.ATTR_UPNP_DEVICE_TYPE: "ABC"}, + ) + + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info["ssdp_st"] == "mock-st" + assert "Detected integration that accessed discovery_info['ssdp_st']" in caplog.text + + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info.get("ssdp_location") == "http://1.1.1.1" + assert ( + "Detected integration that accessed discovery_info.get('ssdp_location')" + in caplog.text + ) + + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert "ssdp_usn" in discovery_info + assert ( + "Detected integration that accessed discovery_info.__contains__('ssdp_usn')" + in caplog.text + ) + + # Root item + assert discovery_info["ssdp_usn"] == "uuid:mock-udn::mock-st" + assert discovery_info.get("ssdp_usn") == "uuid:mock-udn::mock-st" + assert "ssdp_usn" in discovery_info + + # SSDP header + assert discovery_info["st"] == "mock-st" + assert discovery_info.get("st") == "mock-st" + assert "st" in discovery_info + + # UPnP item + assert discovery_info[ssdp.ATTR_UPNP_DEVICE_TYPE] == "ABC" + assert discovery_info.get(ssdp.ATTR_UPNP_DEVICE_TYPE) == "ABC" + assert ssdp.ATTR_UPNP_DEVICE_TYPE in discovery_info diff --git a/tests/components/usb/test_init.py b/tests/components/usb/test_init.py index cc34add6726..ce86965f093 100644 --- a/tests/components/usb/test_init.py +++ b/tests/components/usb/test_init.py @@ -779,6 +779,7 @@ def test_human_readable_device_name(): assert "8A2A" in name +@pytest.mark.usefixtures("mock_integration_frame") async def test_service_info_compatibility(hass, caplog): """Test compatibility with old-style dict. @@ -794,10 +795,6 @@ async def test_service_info_compatibility(hass, caplog): ) # Ensure first call get logged - assert discovery_info["vid"] == 12345 - assert "Detected code that accessed discovery_info['vid']" in caplog.text - - # Ensure second call doesn't get logged - caplog.clear() - assert discovery_info["vid"] == 12345 - assert "Detected code that accessed discovery_info['vid']" not in caplog.text + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info["vid"] == 12345 + assert "Detected integration that accessed discovery_info['vid']" in caplog.text diff --git a/tests/components/zeroconf/test_init.py b/tests/components/zeroconf/test_init.py index e306edba357..16aca70d7fc 100644 --- a/tests/components/zeroconf/test_init.py +++ b/tests/components/zeroconf/test_init.py @@ -3,6 +3,7 @@ from ipaddress import ip_address from typing import Any from unittest.mock import call, patch +import pytest from zeroconf import InterfaceChoice, IPVersion, ServiceStateChange from zeroconf.asyncio import AsyncServiceInfo @@ -1032,6 +1033,7 @@ async def test_no_name(hass, mock_async_zeroconf): assert info.name == "Home._home-assistant._tcp.local." +@pytest.mark.usefixtures("mock_integration_frame") async def test_service_info_compatibility(hass, caplog): """Test compatibility with old-style dict. @@ -1046,21 +1048,15 @@ async def test_service_info_compatibility(hass, caplog): properties={}, ) - # Ensure first call get logged - assert discovery_info["host"] == "mock_host" - assert discovery_info.get("host") == "mock_host" + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info["host"] == "mock_host" + assert "Detected integration that accessed discovery_info['host']" in caplog.text + + with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()): + assert discovery_info.get("host") == "mock_host" + assert ( + "Detected integration that accessed discovery_info.get('host')" in caplog.text + ) + assert discovery_info.get("host", "fallback_host") == "mock_host" assert discovery_info.get("invalid_key", "fallback_host") == "fallback_host" - assert "Detected code that accessed discovery_info['host']" in caplog.text - assert "Detected code that accessed discovery_info.get('host')" not in caplog.text - - # Ensure second call doesn't get logged - caplog.clear() - assert discovery_info["host"] == "mock_host" - assert discovery_info.get("host") == "mock_host" - assert "Detected code that accessed discovery_info['host']" not in caplog.text - assert "Detected code that accessed discovery_info.get('host')" not in caplog.text - - discovery_info._warning_logged = False - assert discovery_info.get("host") == "mock_host" - assert "Detected code that accessed discovery_info.get('host')" in caplog.text diff --git a/tests/conftest.py b/tests/conftest.py index 10a9dd1627b..88651a0ec3f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,7 +6,7 @@ import logging import socket import ssl import threading -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, Mock, patch from aiohttp.test_utils import make_mocked_request import freezegun @@ -778,3 +778,30 @@ def hass_recorder(enable_statistics, hass_storage): yield setup_recorder hass.stop() + + +@pytest.fixture +def mock_integration_frame(): + """Mock as if we're calling code from inside an integration.""" + correct_frame = Mock( + filename="/home/paulus/homeassistant/components/hue/light.py", + lineno="23", + line="self.light.is_on", + ) + with patch( + "homeassistant.helpers.frame.extract_stack", + return_value=[ + Mock( + filename="/home/paulus/homeassistant/core.py", + lineno="23", + line="do_something()", + ), + correct_frame, + Mock( + filename="/home/paulus/aiohue/lights.py", + lineno="2", + line="something()", + ), + ], + ): + yield correct_frame diff --git a/tests/helpers/conftest.py b/tests/helpers/conftest.py deleted file mode 100644 index 4b3b9bf465d..00000000000 --- a/tests/helpers/conftest.py +++ /dev/null @@ -1,31 +0,0 @@ -"""Fixtures for helpers.""" -from unittest.mock import Mock, patch - -import pytest - - -@pytest.fixture -def mock_integration_frame(): - """Mock as if we're calling code from inside an integration.""" - correct_frame = Mock( - filename="/home/paulus/homeassistant/components/hue/light.py", - lineno="23", - line="self.light.is_on", - ) - with patch( - "homeassistant.helpers.frame.extract_stack", - return_value=[ - Mock( - filename="/home/paulus/homeassistant/core.py", - lineno="23", - line="do_something()", - ), - correct_frame, - Mock( - filename="/home/paulus/aiohue/lights.py", - lineno="2", - line="something()", - ), - ], - ): - yield correct_frame