From e2a1e0db554a8f6bf21927e31681d1f43796672b Mon Sep 17 00:00:00 2001 From: Yevhenii Vaskivskyi Date: Sat, 7 Sep 2024 09:05:49 +0200 Subject: [PATCH] 1.13.0 (#537) * Bump actions/setup-python from 5.1.1 to 5.2.0 (#531) * Close aiohttp session when deleting a `Connection` (#532) * Add AsusData.DSL for dsllog_dataratedown/up (#530) * Define DSL models by `Endpoint.DSL` (#533) * Bump actions/upload-artifact from 4.3.6 to 4.4.0 (#534) * Improve `Connection` initialization (#535) * Bump python-dateutil to `2.9.0.post0` (#536) * Bump version to `1.13.0` --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GaryHuang-ASUS <164834485+GaryHuang-ASUS@users.noreply.github.com> --- .github/workflows/ci.yml | 10 +- .github/workflows/release.yml | 2 +- asusrouter/asusrouter.py | 4 + asusrouter/connection.py | 97 +++++++--- asusrouter/modules/data.py | 1 + asusrouter/modules/data_finder.py | 8 + asusrouter/modules/endpoint/__init__.py | 1 + asusrouter/modules/endpoint/hook.py | 30 ++++ asusrouter/modules/identity.py | 6 + pyproject.toml | 4 +- requirements.txt | 2 +- tests/test_connection.py | 230 ++++++++++++++++++++++++ 12 files changed, 362 insertions(+), 33 deletions(-) create mode 100644 tests/test_connection.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c651198..d1d3ae8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: uses: actions/checkout@v4.1.7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5.1.1 + uses: actions/setup-python@v5.2.0 with: python-version: ${{ matrix.python-version }} @@ -63,7 +63,7 @@ jobs: uses: actions/checkout@v4.1.7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5.1.1 + uses: actions/setup-python@v5.2.0 with: python-version: ${{ matrix.python-version }} @@ -92,7 +92,7 @@ jobs: pytest --cov=asusrouter --cov-report=xml:unit-tests-cov-${{ matrix.python-version }}.xml -k 'not test_devices' - name: Upload coverage to artifacts - uses: actions/upload-artifact@v4.3.6 + uses: actions/upload-artifact@v4.4.0 with: name: unit-tests-cov-${{ matrix.python-version }} path: unit-tests-cov-${{ matrix.python-version }}.xml @@ -109,7 +109,7 @@ jobs: uses: actions/checkout@v4.1.7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5.1.1 + uses: actions/setup-python@v5.2.0 with: python-version: ${{ matrix.python-version }} @@ -138,7 +138,7 @@ jobs: pytest --cov=asusrouter --cov-report=xml:real-data-tests-cov-${{ matrix.python-version }}.xml tests/test_devices.py --log-cli-level=INFO - name: Upload coverage to artifacts - uses: actions/upload-artifact@v4.3.6 + uses: actions/upload-artifact@v4.4.0 with: name: real-data-tests-cov-${{ matrix.python-version }} path: real-data-tests-cov-${{ matrix.python-version }}.xml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2320005..21e7d9b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -19,7 +19,7 @@ jobs: ref: ${{ github.event.release.target_commitish }} - name: Setup Python ${{ env.DEFAULT_PYTHON }} - uses: actions/setup-python@v5.1.1 + uses: actions/setup-python@v5.2.0 with: python-version: ${{ env.DEFAULT_PYTHON }} diff --git a/asusrouter/asusrouter.py b/asusrouter/asusrouter.py index d469b37..0a78fb4 100644 --- a/asusrouter/asusrouter.py +++ b/asusrouter/asusrouter.py @@ -277,6 +277,10 @@ async def async_get_identity(self, force: bool = False) -> AsusDevice: remove_data_rule(AsusData.WIREGUARD_CLIENT) remove_data_rule(AsusData.WIREGUARD_SERVER) + # DSL connection + if self._identity.dsl is False: + remove_data_rule(AsusData.DSL) + # Ookla Speedtest if self._identity.ookla is False: remove_data_rule(AsusData.SPEEDTEST) diff --git a/asusrouter/connection.py b/asusrouter/connection.py index 76c1484..dc5b58b 100644 --- a/asusrouter/connection.py +++ b/asusrouter/connection.py @@ -1,6 +1,6 @@ """Connection module. -This module handles the connection between the library and the router +This module handles the connection between the library and the router as well as all the data transfer.""" from __future__ import annotations @@ -29,7 +29,9 @@ _LOGGER = logging.getLogger(__name__) -def generate_credentials(username: str, password: str) -> Tuple[str, dict[str, str]]: +def generate_credentials( + username: str, password: str +) -> Tuple[str, dict[str, str]]: """Generate credentials for connection""" auth = f"{username}:{password}".encode("ascii") @@ -43,18 +45,6 @@ def generate_credentials(username: str, password: str) -> Tuple[str, dict[str, s class Connection: # pylint: disable=too-many-instance-attributes """A connection between the library and the device.""" - # A token for the current session - _token: Optional[str] = None - # A header to use in the current session - _header: Optional[dict[str, str]] = None - - # SSL - _ssl = False - - # Connection status - _connected: bool = False - _connection_lock: asyncio.Lock = asyncio.Lock() - def __init__( # pylint: disable=too-many-arguments self, hostname: str, @@ -69,26 +59,71 @@ def __init__( # pylint: disable=too-many-arguments _LOGGER.debug("Initializing a new connection to `%s`", hostname) + # Initialize parameters for connection + self._token: Optional[str] = None + self._header: Optional[dict[str, str]] = None + self._connected: bool = False + self._connection_lock: asyncio.Lock = asyncio.Lock() + # Hostname and credentials self._hostname = hostname self._username = username self._password = password # Client session - self._session = session if session is not None else self._new_session() + self._manage_session: bool = False + if session is not None: + _LOGGER.debug("Using provided session") + self._session = session + else: + _LOGGER.debug("No session provided. Will create a new one") + self._session = self._new_session() _LOGGER.debug("Using session `%s`", session) # Set the port and protocol based on the input self._http = "https" if use_ssl else "http" self._port = port or (8443 if use_ssl else 80) - _LOGGER.debug("Using `%s` and port `%s`", self._http, self._port) + self._ssl = use_ssl + self._verify_ssl = False + _LOGGER.debug( + "Using `%s` and port `%s` with ssl flag `%s`", + self._http, + self._port, + self._ssl, + ) # Callback for dumping data self._dumpback = dumpback + def __del__(self) -> None: + """Proper cleanup when the object is deleted.""" + + if self._manage_session and self._session: + if not self._session.closed: + _LOGGER.debug( + "Connection object is managing a session. Trying to close it" + ) + try: + loop = asyncio.get_running_loop() + loop.create_task(self._session.close()) + _LOGGER.debug("Session close task created") + except RuntimeError: + try: + asyncio.run(self._session.close()) + _LOGGER.debug("Trying to close the session") + except Exception as ex: # pylint: disable=broad-except + _LOGGER.warning( + "Error while closing the session: %s", ex + ) + + else: + _LOGGER.debug("Session closed") + def _new_session(self) -> aiohttp.ClientSession: """Create a new session.""" + # If we create a new session, we will manage it + self._manage_session = True return aiohttp.ClientSession() async def async_connect( @@ -112,7 +147,9 @@ async def async_connect( self._connected = False # Generate payload and header for login request - payload, headers = generate_credentials(self._username, self._password) + payload, headers = generate_credentials( + self._username, self._password + ) # Request authotization _LOGGER.debug("Requesting authorization") @@ -139,7 +176,9 @@ async def async_connect( The last error: {ex}" ) from ex except Exception as ex: # pylint: disable=broad-except - _LOGGER.debug("Error while connecting to %s: %s", self._hostname, ex) + _LOGGER.debug( + "Error while connecting to %s: %s", self._hostname, ex + ) raise ex # Convert the response to JSON @@ -181,7 +220,9 @@ async def async_disconnect(self) -> bool: _LOGGER.debug("Disconnected from %s", self._hostname) return True except AsusRouterError as ex: - _LOGGER.debug("Error while disconnecting from %s: %s", self._hostname, ex) + _LOGGER.debug( + "Error while disconnecting from %s: %s", self._hostname, ex + ) return False # Anything else would mean error when disconnecting @@ -252,11 +293,17 @@ async def async_query( # If still not connected, raise an error if not self._connected: - raise AsusRouterTimeoutError("Data cannot be retrieved. Connection failed") + raise AsusRouterTimeoutError( + "Data cannot be retrieved. Connection failed" + ) # Send the request - _LOGGER.debug("Sending `%s` request to `%s`", request_type, self._hostname) - return await self._send_request(endpoint, payload, headers, request_type) + _LOGGER.debug( + "Sending `%s` request to `%s`", request_type, self._hostname + ) + return await self._send_request( + endpoint, payload, headers, request_type + ) async def _make_request( self, @@ -277,7 +324,9 @@ async def _make_request( # Reconnect await self.async_connect() # Retry the request - return await self._make_request(endpoint, payload, headers, request_type) + return await self._make_request( + endpoint, payload, headers, request_type + ) # Check headers if not headers: @@ -300,7 +349,7 @@ async def _make_request( url, data=payload_to_send if request_type == RequestType.POST else None, headers=headers, - ssl=self._ssl, + verify_ssl=self._verify_ssl, ) as response: # Read the status code resp_status = response.status diff --git a/asusrouter/modules/data.py b/asusrouter/modules/data.py index 14e18ca..fb08b96 100644 --- a/asusrouter/modules/data.py +++ b/asusrouter/modules/data.py @@ -20,6 +20,7 @@ class AsusData(str, Enum): CLIENTS = "clients" CPU = "cpu" DEVICEMAP = "devicemap" + DSL = "dsl" FIRMWARE = "firmware" FIRMWARE_NOTE = "firmware_note" FLAGS = "flags" diff --git a/asusrouter/modules/data_finder.py b/asusrouter/modules/data_finder.py index 4a2f7b4..f854a30 100644 --- a/asusrouter/modules/data_finder.py +++ b/asusrouter/modules/data_finder.py @@ -130,6 +130,10 @@ def __init__( for key, _, _ in [converters.safe_unpack_keys(element)] if key != "get_wgsc_status" ], + "dsl": [ + "dsllog_dataratedown", + "dsllog_datarateup", + ] } ASUSDATA_NVRAM["aura"].extend([f"ledg_rgb{num}" for num in range(0, 8)]) ASUSDATA_NVRAM["vpnc"].extend( @@ -256,6 +260,10 @@ def __init__( method=wlan_nvram_request, arguments=AsusRouterAttribute.WLAN_LIST, ), + AsusData.DSL: AsusDataFinder( + Endpoint.HOOK, + nvram=ASUSDATA_NVRAM["dsl"], + ), } diff --git a/asusrouter/modules/endpoint/__init__.py b/asusrouter/modules/endpoint/__init__.py index 65a2f3f..6da43a7 100644 --- a/asusrouter/modules/endpoint/__init__.py +++ b/asusrouter/modules/endpoint/__init__.py @@ -22,6 +22,7 @@ class Endpoint(str, Enum): """Endpoint enum. These endpoints are used to receive data from the device.""" DEVICEMAP = "ajax_status.xml" + DSL = "ajax_AdslStatus.asp" ETHERNET_PORTS = "ajax_ethernet_ports.asp" FIRMWARE = "detect_firmware.asp" FIRMWARE_NOTE = "release_note0.asp" diff --git a/asusrouter/modules/endpoint/hook.py b/asusrouter/modules/endpoint/hook.py index d16cf3d..f7d97c8 100644 --- a/asusrouter/modules/endpoint/hook.py +++ b/asusrouter/modules/endpoint/hook.py @@ -163,6 +163,13 @@ def process(data: dict[str, Any]) -> dict[AsusData, Any]: or "wl3_wpa_psk" in data ): state[AsusData.WLAN] = process_wlan(data, wlan) + + # DSL + if ( + "dsllog_dataratedown" in data + or "dsllog_datarateup" in data + ): + state[AsusData.DSL] = process_dsl(data) return state @@ -733,3 +740,26 @@ def process_wlan( wlan[interface.value] = info return wlan + +def process_dsl(dsl_info: dict[str, Any]) -> dict[str, Any]: + """Process DSL data""" + + def remove_units(value: Optional[str]) -> Optional[str]: + """Remove units from the value.""" + + if value is None: + return None + return value.split(" ")[0] + + dsl: dict[str, Any] = {} + + # Data preprocessing + _dataratedown = remove_units(dsl_info.get("dsllog_dataratedown")) + _datarateup = remove_units(dsl_info.get("dsllog_datarateup")) + + dsl["datarate"] = { + "down": safe_int(_dataratedown, 0), + "up": safe_int(_datarateup, 0), + } + + return dsl diff --git a/asusrouter/modules/identity.py b/asusrouter/modules/identity.py index e5d9144..38b8037 100644 --- a/asusrouter/modules/identity.py +++ b/asusrouter/modules/identity.py @@ -81,6 +81,7 @@ class AsusDevice: # pylint: disable=too-many-instance-attributes # Flags for device features aura: bool = False aura_zone: int = 0 + dsl: bool = False led: bool = False ookla: bool = False vpn_status: bool = False @@ -129,6 +130,11 @@ async def collect_identity( if isinstance(this_device, AiMeshDevice): identity["model"] = this_device.model + # Check if DSL + if endpoints[Endpoint.DSL] is True: + identity["dsl"] = True + _LOGGER.debug("DSL-compatible device detected") + # Check if Merlin if endpoints[Endpoint.SYSINFO] is True: identity["merlin"] = True diff --git a/pyproject.toml b/pyproject.toml index c370629..ad227f0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "asusrouter" -version = "1.12.2" +version = "1.13.0" license = {text = "Apache-2.0"} requires-python = ">=3.11.0" readme = "README.md" @@ -22,7 +22,7 @@ classifiers = [ ] dependencies = [ "aiohttp >=3.8.1", - "python-dateutil ==2.8.2", + "python-dateutil ==2.9.0.post0", "urllib3 >=1.26.14", "xmltodict >=0.12.0", ] diff --git a/requirements.txt b/requirements.txt index 98ff28f..0880077 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ # Requirements for AsusRouter aiohttp>=3.8.1 -python-dateutil==2.8.2 +python-dateutil==2.9.0.post0 urllib3>=1.26.14 xmltodict>=0.12.0 diff --git a/tests/test_connection.py b/tests/test_connection.py new file mode 100644 index 0000000..6ca52e6 --- /dev/null +++ b/tests/test_connection.py @@ -0,0 +1,230 @@ +"""Tests for the connection module.""" + +import asyncio +import gc +from unittest.mock import Mock, patch + +import pytest +from asusrouter.connection import Connection, generate_credentials + + +@pytest.mark.asyncio +async def test_generate_credentials(): + username = "user" + password = "pass" + payload, headers = generate_credentials(username, password) + assert payload == "login_authorization=dXNlcjpwYXNz" + assert headers == {"user-agent": "asusrouter--DUTUtil-"} + + +class TestConnection: + def _assert_connection( + self, + conn, + hostname, + username, + password, + port, + use_ssl, + session, + dumpback, + mock_new_session, + ): + assert conn._hostname == hostname + assert conn._username == username + assert conn._password == password + assert conn._port == (port or (8443 if use_ssl else 80)) + assert conn._ssl == use_ssl + assert conn._session == (session or mock_new_session.return_value) + assert conn._dumpback == dumpback + assert conn._token is None + assert conn._header is None + assert conn._connected is False + assert isinstance(conn._connection_lock, asyncio.Lock) + + @pytest.mark.parametrize( + "hostname, username, password, port, use_ssl, session, dumpback", + [ + ("localhost", "user", "pass", None, False, None, None), + ("localhost", "user", "pass", 8080, True, None, None), + ("localhost", "user", "pass", 8080, False, Mock(), None), + ("localhost", "user", "pass", 8080, True, Mock(), Mock()), + ("", "user", "pass", 8080, True, Mock(), Mock()), + ("localhost", "", "pass", 8080, True, None, None), + ("localhost", "user", "pass", -1, True, None, None), + ], + ) + @patch.object(Connection, "_new_session", return_value=Mock()) + def test_init( + self, + mock_new_session, + hostname, + username, + password, + port, + use_ssl, + session, + dumpback, + ): + conn = Connection( + hostname=hostname, + username=username, + password=password, + port=port, + use_ssl=use_ssl, + session=session, + dumpback=dumpback, + ) + + self._assert_connection( + conn, + hostname, + username, + password, + port, + use_ssl, + session, + dumpback, + mock_new_session, + ) + + @patch.object(Connection, "_new_session", return_value=Mock()) + def test_init_multiple_instances(self, mock_new_session): + conn1 = Connection( + hostname="localhost1", + username="user1", + password="pass1", + port=8080, + use_ssl=True, + ) + conn2 = Connection( + hostname="localhost2", + username="user2", + password="pass2", + port=9090, + use_ssl=False, + ) + + self._assert_connection( + conn1, + "localhost1", + "user1", + "pass1", + 8080, + True, + None, + None, + mock_new_session, + ) + self._assert_connection( + conn2, + "localhost2", + "user2", + "pass2", + 9090, + False, + None, + None, + mock_new_session, + ) + + # Ensure that instances do not interfere with each other + assert conn1._hostname != conn2._hostname + assert conn1._username != conn2._username + assert conn1._password != conn2._password + assert conn1._port != conn2._port + assert conn1._ssl != conn2._ssl + + @pytest.mark.parametrize( + "manage_session, session_closed, running_loop, expected_close_call, expected_create_task, expected_run", + [ + ( + True, + False, + True, + True, + True, + False, + ), # Managed session, not closed, running loop + ( + True, + False, + False, + True, + False, + True, + ), # Managed session, not closed, no running loop + ( + True, + True, + True, + False, + False, + False, + ), # Managed session, already closed + (False, False, True, False, False, False), # Not managed session + ], + ) + @patch.object(Connection, "_new_session", return_value=Mock()) + def test_del( + self, + mock_new_session, + manage_session, + session_closed, + running_loop, + expected_close_call, + expected_create_task, + expected_run, + ): + # Create a mock session + mock_session = Mock() + mock_session.closed = session_closed + + # Create an instance of Connection with the mock session + conn = Connection( + hostname="localhost", + username="user", + password="pass", + session=mock_session, + ) + conn._manage_session = manage_session + + # Mock the asyncio.get_running_loop to return a mock loop or raise RuntimeError + if running_loop: + mock_loop = Mock() + get_running_loop_patch = patch( + "asyncio.get_running_loop", return_value=mock_loop + ) + else: + get_running_loop_patch = patch( + "asyncio.get_running_loop", side_effect=RuntimeError + ) + + with get_running_loop_patch as mock_get_running_loop: + with patch("asyncio.run") as mock_run: + # Delete the connection object to trigger __del__ + del conn + + # Force garbage collection + gc.collect() + + # Verify that the session's close method was called if expected + if expected_close_call: + mock_session.close.assert_called_once() + else: + mock_session.close.assert_not_called() + + # Verify that create_task was called if expected + if expected_create_task: + mock_get_running_loop.return_value.create_task.assert_called_once_with( + mock_session.close() + ) + else: + if running_loop: + mock_get_running_loop.return_value.create_task.assert_not_called() + + # Verify that asyncio.run was called if expected + if expected_run: + mock_run.assert_called_once_with(mock_session.close()) + else: + mock_run.assert_not_called()