Fix #789: Navidrome library selection ignored (all libraries imported)
The saved music-folder selection was silently dropped on every reconnect. _setup_client's restore step called the public get_music_folders(), which starts with ensure_connection() — but we're already inside ensure_connection() at that point (_is_connecting=True, _connection_attempted not yet set), so the re-entrant call bailed and returned []. The restore matched nothing, music_folder_id stayed None, and the per-call musicFolderId filters all no-op'd → scans imported every library regardless of the user's choice. Surfaces after any restart or settings save (reload_config resets the state). Split get_music_folders() into the public method (does the connection check) and a non-reentrant _fetch_music_folders() seam; the restore now calls the seam directly (connection is already established + ping succeeded by then). Regression + seam tests added.
This commit is contained in:
parent
adbdda7b0e
commit
cf655c5009
2 changed files with 60 additions and 1 deletions
|
|
@ -192,6 +192,14 @@ class NavidromeClient(MediaServerClient):
|
|||
"""Get available music folders from Navidrome."""
|
||||
if not self.ensure_connection():
|
||||
return []
|
||||
return self._fetch_music_folders()
|
||||
|
||||
def _fetch_music_folders(self) -> list:
|
||||
"""Fetch + parse the Navidrome music folders. Assumes the connection is
|
||||
already established (base_url/username set) and does NOT call
|
||||
ensure_connection — so it is safe to call from inside _setup_client's
|
||||
restore step without re-entering the connection guard (which would
|
||||
otherwise bail with _is_connecting=True and return an empty list)."""
|
||||
try:
|
||||
response = self._make_request('getMusicFolders')
|
||||
if not response:
|
||||
|
|
@ -278,7 +286,11 @@ class NavidromeClient(MediaServerClient):
|
|||
db = MusicDatabase()
|
||||
saved_folder = db.get_preference('navidrome_music_folder')
|
||||
if saved_folder:
|
||||
folders = self.get_music_folders()
|
||||
# Use the non-reentrant fetch: we're still inside
|
||||
# ensure_connection() here (_is_connecting=True), so the
|
||||
# public get_music_folders() would re-enter the guard and
|
||||
# return [], silently dropping the saved selection.
|
||||
folders = self._fetch_music_folders()
|
||||
for folder in folders:
|
||||
if folder['title'] == saved_folder:
|
||||
self.music_folder_id = folder['key']
|
||||
|
|
|
|||
|
|
@ -79,6 +79,53 @@ def test_get_all_album_ids_returns_set(nav_client):
|
|||
assert result == {'nav-1', 'nav-2'}
|
||||
|
||||
|
||||
def test_fetch_music_folders_parses_without_ensure_connection(nav_client):
|
||||
"""The seam: _fetch_music_folders does its own _make_request + parse and
|
||||
does NOT gate on ensure_connection, so it is safe to call mid-connect."""
|
||||
nav_client.base_url = 'http://nav'
|
||||
nav_client.username = 'u'
|
||||
nav_client.password = 'p'
|
||||
folders_envelope = {'musicFolders': {'musicFolder': [
|
||||
{'id': 1, 'name': 'Music'},
|
||||
{'id': 2, 'name': 'Audiobooks'},
|
||||
]}}
|
||||
with patch.object(nav_client, '_make_request', return_value=folders_envelope):
|
||||
folders = nav_client._fetch_music_folders()
|
||||
assert folders == [
|
||||
{'title': 'Music', 'key': '1'},
|
||||
{'title': 'Audiobooks', 'key': '2'},
|
||||
]
|
||||
|
||||
|
||||
def test_setup_client_restores_saved_music_folder(nav_client):
|
||||
"""Regression for #789: a saved music-folder selection must survive
|
||||
_setup_client. The restore runs while still inside ensure_connection()
|
||||
(_is_connecting=True); the old code called the public get_music_folders(),
|
||||
which re-entered the guard, got [], and left music_folder_id=None — so
|
||||
every scan imported all libraries regardless of the user's selection."""
|
||||
def fake_request(endpoint, params=None):
|
||||
if endpoint == 'ping':
|
||||
return {'status': 'ok', 'version': '1.16.1'}
|
||||
if endpoint == 'getMusicFolders':
|
||||
return {'musicFolders': {'musicFolder': [
|
||||
{'id': 1, 'name': 'Music'},
|
||||
{'id': 2, 'name': 'Audiobooks'},
|
||||
]}}
|
||||
return None
|
||||
|
||||
fake_db = MagicMock()
|
||||
fake_db.get_preference.return_value = 'Audiobooks'
|
||||
|
||||
with patch('core.navidrome_client.config_manager.get_navidrome_config',
|
||||
return_value={'base_url': 'http://nav', 'username': 'u', 'password': 'p'}), \
|
||||
patch('database.music_database.MusicDatabase', return_value=fake_db), \
|
||||
patch.object(nav_client, '_make_request', side_effect=fake_request):
|
||||
assert nav_client.ensure_connection() is True
|
||||
|
||||
# Selection ('Audiobooks') was restored to its folder id, not left None.
|
||||
assert nav_client.music_folder_id == '2'
|
||||
|
||||
|
||||
def test_navidrome_album_exposes_cover_art_url(nav_client):
|
||||
album = NavidromeAlbum({
|
||||
'id': 'album-1',
|
||||
|
|
|
|||
Loading…
Reference in a new issue