Skip to content

Commit 202a02d

Browse files
authored
Skip stale artist paths during filesystem track parsing (#4191)
# What does this implement/fix? Fixes library sync aborting on every track whose artist has a stale folder path stored in the library. When an artist folder on disk is renamed - typically from display-name style (`Nina Simone`) to sort-name style (`Simone, Nina`) - the artist's provider mapping in the library database keeps the old path in its `url` field. `_parse_artist` trusted that stored path without verifying it exists, so the artwork scan (`_get_local_images` -> `sorted_scandir`) raised `FileNotFoundError`, which aborted sync processing for **every track by that artist**, on **every sync**: ``` ERROR [music_assistant.Filesystem (local disk)] Error processing Simone, Nina/1987 Live At Ronnie Scott's [album; live] []/12. My Baby Just Cares for Me.mp3 - [Errno 2] No such file or directory: '/my-music/Nina Simone' ``` The stale path can never self-heal: `set_provider_mappings` only overwrites `url` when the incoming value is non-None, so once a bad path is stored it persists until the row is deleted. This surfaces as user reports of "tracks with commas in the path fail to sync/analyze" because sort-name folder layouts are exactly the rename pattern that triggers it. The fix verifies the resolved artist path exists before hunting for `artist.nfo`/artwork in it, degrading to "no folder metadata" instead of failing the track. This covers all sources of a stale path (stored mapping url, the `artist_path` parameter from `get_artist`, and folder matching). Includes a regression test that reproduces the exact failure (stale `url="Nina Simone"` with the folder on disk renamed to `Simone, Nina`) plus a test confirming valid stored paths still resolve. Note: the bug is present in `stable` (2.9.0) as well - the relevant code is identical, so this may be a candidate for `backport-to-stable`. **Related issue (if applicable):** - n/a (reported via user logs) ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) — `bugfix` - [ ] New feature (non-breaking change which adds functionality) — `new-feature` - [ ] Enhancement to an existing feature — `enhancement` - [ ] New music/player/metadata/plugin provider — `new-provider` - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) — `breaking-change` - [ ] Refactor (no behaviour change) — `refactor` - [ ] Documentation only — `documentation` - [ ] Maintenance / chore — `maintenance` - [ ] CI / workflow change — `ci` - [ ] Dependencies bump — `dependencies` ## Checklist - [x] The code change is tested and works locally. - [x] `pre-commit run --all-files` passes. - [x] `pytest` passes, and tests have been added/updated under `tests/` where applicable. - [ ] For changes to shared models, the companion PR in `music-assistant/models` is linked. - [ ] For changes affecting the UI, the companion PR in `music-assistant/frontend` is linked. - [x] I have read and complied with the project's [AI Policy](https://github.com/music-assistant/.github/blob/main/AI_POLICY.md) for any AI-assisted contributions. - [ ] I have raised a PR against the documentation repository targeting the main or beta branch as appropriate.
1 parent 49f57ad commit 202a02d

2 files changed

Lines changed: 121 additions & 1 deletion

File tree

music_assistant/providers/filesystem_local/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1302,7 +1302,7 @@ async def _parse_artist(
13021302
)
13031303
if mbid:
13041304
artist.mbid = mbid
1305-
if not artist_path:
1305+
if not artist_path or not await self.exists(artist_path):
13061306
return artist
13071307

13081308
# grab additional metadata within the Artist's folder
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
"""Tests for filesystem provider artist path resolution with stale library paths."""
2+
3+
import os
4+
from collections.abc import AsyncGenerator
5+
from pathlib import Path
6+
from unittest.mock import AsyncMock, MagicMock
7+
8+
import pytest
9+
from music_assistant_models.media_items import Artist, ProviderMapping
10+
11+
from music_assistant.helpers.tags import AudioTags
12+
from music_assistant.providers.filesystem_local import LocalFileSystemProvider
13+
14+
INSTANCE_ID = "filesystem_local--test"
15+
16+
ARTIST_FOLDER = "Simone, Nina"
17+
ALBUM_FOLDER = "1987 Live At Ronnie Scott's"
18+
TRACK_FILE = "12. My Baby Just Cares for Me.mp3"
19+
20+
21+
def _make_tags(artist: str, album: str) -> AudioTags:
22+
return AudioTags(
23+
raw={},
24+
sample_rate=44100,
25+
channels=2,
26+
bits_per_sample=16,
27+
format="mp3",
28+
bit_rate=320,
29+
duration=240.0,
30+
tags={
31+
"artist": artist,
32+
"albumartist": artist,
33+
"album": album,
34+
"title": "My Baby Just Cares for Me",
35+
"track": "12",
36+
},
37+
has_cover_image=False,
38+
filename=os.path.join(ARTIST_FOLDER, ALBUM_FOLDER, TRACK_FILE),
39+
)
40+
41+
42+
def _make_provider(base_path: str, lib_artists: list[Artist]) -> LocalFileSystemProvider:
43+
provider = LocalFileSystemProvider.__new__(LocalFileSystemProvider)
44+
provider.base_path = base_path
45+
provider.logger = MagicMock()
46+
provider.write_access = False
47+
provider.media_content_type = "music"
48+
provider.config = MagicMock()
49+
provider.config.instance_id = INSTANCE_ID
50+
provider.config.get_value = MagicMock(return_value="various_artists")
51+
provider.manifest = MagicMock()
52+
provider.manifest.domain = "filesystem_local"
53+
provider.cache = MagicMock()
54+
provider.cache.get = AsyncMock(return_value=None)
55+
provider.cache.set = AsyncMock(return_value=None)
56+
57+
async def iter_library_items(
58+
search: str | None = None, # noqa: ARG001
59+
provider: str | None = None, # noqa: ARG001
60+
) -> AsyncGenerator[Artist]:
61+
for lib_artist in lib_artists:
62+
yield lib_artist
63+
64+
provider.mass = MagicMock()
65+
provider.mass.music.artists.iter_library_items = iter_library_items
66+
provider.mass.get_provider = MagicMock(return_value=None)
67+
provider.mass.create_task = MagicMock()
68+
return provider
69+
70+
71+
def _lib_artist(name: str, url: str | None) -> Artist:
72+
return Artist(
73+
item_id="1",
74+
provider="library",
75+
name=name,
76+
provider_mappings={
77+
ProviderMapping(
78+
item_id=url or name,
79+
provider_domain="filesystem_local",
80+
provider_instance=INSTANCE_ID,
81+
url=url,
82+
in_library=True,
83+
)
84+
},
85+
)
86+
87+
88+
@pytest.fixture
89+
def music_tree(tmp_path: Path) -> str:
90+
"""Create a sort-name style artist folder with one track file."""
91+
track_dir = tmp_path / ARTIST_FOLDER / ALBUM_FOLDER
92+
track_dir.mkdir(parents=True)
93+
(track_dir / TRACK_FILE).write_bytes(b"\x00" * 128)
94+
return str(tmp_path)
95+
96+
97+
async def test_stale_artist_path_does_not_fail_track_parse(music_tree: str) -> None:
98+
"""A stale artist path stored in the library must not fail parsing the track.
99+
100+
Regression test: the artist folder was renamed from display-name style
101+
("Nina Simone") to sort-name style ("Simone, Nina"), but the library still
102+
holds the old path in the provider mapping url. Parsing any track by that
103+
artist raised FileNotFoundError and aborted the sync for that track.
104+
"""
105+
provider = _make_provider(music_tree, [_lib_artist("Nina Simone", "Nina Simone")])
106+
file_item = await provider.resolve(os.path.join(ARTIST_FOLDER, ALBUM_FOLDER, TRACK_FILE))
107+
108+
track = await provider._parse_track(file_item, _make_tags("Nina Simone", ALBUM_FOLDER))
109+
110+
assert [a.name for a in track.artists] == ["Nina Simone"]
111+
112+
113+
async def test_valid_artist_path_still_resolved(music_tree: str) -> None:
114+
"""A valid stored artist path keeps being used as the artist's item_id."""
115+
provider = _make_provider(music_tree, [_lib_artist("Nina Simone", ARTIST_FOLDER)])
116+
file_item = await provider.resolve(os.path.join(ARTIST_FOLDER, ALBUM_FOLDER, TRACK_FILE))
117+
118+
track = await provider._parse_track(file_item, _make_tags("Nina Simone", ALBUM_FOLDER))
119+
120+
assert [a.item_id for a in track.artists] == [ARTIST_FOLDER]

0 commit comments

Comments
 (0)