From 4465fd093af6fc220211735ddfb97d0835563bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathieu=20R=C3=A9quillart?= Date: Wed, 20 May 2026 15:59:35 +0200 Subject: [PATCH] fix: correct python-mpd version identification --- mpdris2/mpd_client.py | 31 +++++++++++----- pyproject.toml | 6 ++-- tests/test_mpd_client.py | 78 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 tests/test_mpd_client.py diff --git a/mpdris2/mpd_client.py b/mpdris2/mpd_client.py index a18177c..3692523 100644 --- a/mpdris2/mpd_client.py +++ b/mpdris2/mpd_client.py @@ -75,16 +75,29 @@ async def connect( async def fetch_config(client: MPDClient) -> dict[str, str]: """Send MPD's ``config`` command and parse the response as a dict. - Works around python-mpd2 3.1.x mapping ``config`` to - ``_parse_item`` (which only handles single-pair responses); - ``config`` actually returns multiple pairs (``music_directory``, - ``playlist_directory``, ``pcre``), so the upstream parser returns - ``None`` and we never see the data. - - We reuse python-mpd2's internal command queue + writer with a - correct dict-parsing callback. Only allowed on local socket - connections (MPD answers "Access denied" on TCP). + ``config`` returns multiple pairs (``music_directory``, + ``playlist_directory``, ``pcre``). python-mpd2 >= 3.1.2 parses this + correctly, so ``client.config()`` returns the dict directly. Older + releases (e.g. 3.1.1 from Debian stable) map ``config`` to + ``_parse_item``, which only handles single-pair responses and returns + ``None``; we detect that and re-issue the command through + python-mpd2's internal queue with a correct dict-parsing callback. + + Only allowed on local socket connections (MPD answers + "Access denied" on TCP). """ + try: + async with asyncio.timeout(CONFIG_PROBE_TIMEOUT): + raw = await client.config() + except (TimeoutError, mpd.ConnectionError, OSError) as e: + logger.debug("config probe gave up: %s", e) + return {} + if raw: + return dict(raw) + + # python-mpd2 < 3.1.2 mis-parsed the multi-pair response as a single + # item and gave us ``None``; re-issue ``config`` with a parser that + # keeps every pair. def _parse_as_dict(client_: MPDClient, lines: list) -> dict[str, str]: return dict(client_._parse_pairs(lines)) diff --git a/pyproject.toml b/pyproject.toml index 47be686..246b107 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,8 +14,10 @@ classifiers = [ "Operating System :: POSIX :: Linux", ] dependencies = [ - # <3.2: mpd_client.fetch_config leans on private attrs. - "python-mpd2>=3.1,<3.2", + # fetch_config uses the public client.config() and only falls back to + # private attrs when an older lib mis-parses it (e.g. Debian stable's + # 3.1.1); the public path keeps newer majors working, so no cap. + "python-mpd2>=3.1", "dbus-fast>=2.0", ] dynamic = ["version"] diff --git a/tests/test_mpd_client.py b/tests/test_mpd_client.py new file mode 100644 index 0000000..34e7b47 --- /dev/null +++ b/tests/test_mpd_client.py @@ -0,0 +1,78 @@ +"""Unit tests for mpd_client.fetch_config — no MPD, no socket. + +``fetch_config`` has two paths: on python-mpd2 >= 3.1.2 ``client.config()`` +returns a parsed dict directly; on older releases (e.g. Debian stable's +3.1.1) it mis-parses the multi-pair response and returns ``None``, so we +re-issue ``config`` through python-mpd2's internal command queue with a +correct dict parser. Both branches are exercised here by controlling the +mocked ``client.config()`` return value, independent of the installed +python-mpd2 version. A real ``MPDClient`` instance is used (never +connected) so the fallback's ``client._parse_pairs`` does real parsing. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock + +import mpd +import pytest +from mpd.asyncio import MPDClient + +from mpdris2.mpd_client import fetch_config + + +@pytest.mark.asyncio +async def test_fetch_config_native_returns_dict() -> None: + # python-mpd2 >= 3.1.2: config() already yields a dict. + client = MagicMock() + client.config = AsyncMock( + return_value={"music_directory": "/srv/music", "pcre": "1"} + ) + + assert await fetch_config(client) == { + "music_directory": "/srv/music", + "pcre": "1", + } + client.config.assert_awaited_once() + # The native path must not touch the private-API fallback. + client._write_command.assert_not_called() + + +@pytest.mark.asyncio +async def test_fetch_config_fallback_parses_multipair() -> None: + # python-mpd2 < 3.1.2: config() mis-parses the multi-pair answer as a + # single item and returns None, triggering the private-queue fallback. + client = MPDClient() + client.config = AsyncMock(return_value=None) + client._end_idle = MagicMock() + client._write_command = MagicMock() + + # Capture the CommandResult the fallback enqueues and drive it as the + # real read loop would: feed the raw config lines, then a None sentinel + # that flushes them through the dict-parsing callback. + async def _put(cmd_result: object) -> None: + for line in ("music_directory: /srv/music", + "playlist_directory: /srv/pl", "pcre: 1"): + cmd_result._feed_line(line) + cmd_result._feed_line(None) + + queue = MagicMock() + queue.put = AsyncMock(side_effect=_put) + client._MPDClient__command_queue = queue + + assert await fetch_config(client) == { + "music_directory": "/srv/music", + "playlist_directory": "/srv/pl", + "pcre": "1", + } + client._write_command.assert_called_once_with("config") + + +@pytest.mark.asyncio +async def test_fetch_config_returns_empty_on_connection_error() -> None: + # A dropped connection during the probe is swallowed: the caller just + # ends up without an auto-detected music_directory. + client = MagicMock() + client.config = AsyncMock(side_effect=mpd.ConnectionError("gone")) + + assert await fetch_config(client) == {}