Cover art: stop crying "(read-only?)" when files are simply already arted (Sokhi/Boulder)
The real reason Sokhi's cover-art fix "didn't work" and Boulder saw the same message on a WRITABLE Windows box: _fix_missing_cover_art reported "Updated database thumbnail, but could not write art to files (read-only?)" whenever embedded==0 AND cover_written==False — but the overwhelmingly common cause of that is every file ALREADY having embedded art (skipped, not failed). The message blamed read-only on a perfectly writable library, which sent us chasing a read-only ghost across three commits. Sokhi's /fix returns 200 (success), so he was never hitting the genuine EROFS path at all. Now the message is derived from the art_result breakdown: - embedded/cover written → "Applied cover art: …" - failed>0 (non-EROFS) → "… check file/folder permissions" - skipped>0 (already arted) → "Cover art already present on all N file(s)" - otherwise → "no file artwork was applied" Genuine read-only (EROFS) still hard-fails with the clear mount message. Tests: already-arted→present (not read-only), failed→permissions, EROFS→hard fail, embedded→success. 453 cover/repair/art tests pass.
This commit is contained in:
parent
41f4eeb91e
commit
c83d4862e8
2 changed files with 88 additions and 6 deletions
|
|
@ -1407,12 +1407,34 @@ class RepairWorker:
|
|||
'mount (NFS/SMB/mergerfs) is read-write, then recreate the '
|
||||
'container. (Database thumbnail was still updated.)'),
|
||||
'art_result': art_result}
|
||||
msg = f'Applied cover art: embedded into {embedded}/{len(resolved)} file(s)'
|
||||
if art_result.get('cover_written'):
|
||||
msg += ' + wrote cover.jpg'
|
||||
if embedded == 0 and not art_result.get('cover_written'):
|
||||
# DB updated but nothing reached disk (e.g. permissions).
|
||||
msg = 'Updated database thumbnail, but could not write art to files (read-only?)'
|
||||
skipped = art_result.get('skipped', 0)
|
||||
failed = art_result.get('failed', 0)
|
||||
cover_written = art_result.get('cover_written')
|
||||
|
||||
wrote_parts = []
|
||||
if embedded:
|
||||
wrote_parts.append(f'embedded into {embedded}/{len(resolved)} file(s)')
|
||||
if cover_written:
|
||||
wrote_parts.append('wrote cover.jpg')
|
||||
|
||||
if wrote_parts:
|
||||
msg = 'Applied cover art: ' + ' + '.join(wrote_parts)
|
||||
elif failed:
|
||||
# Real per-file write failures that were NOT a read-only mount
|
||||
# (genuine EROFS is handled above) — almost always file/folder
|
||||
# permissions or a locked file.
|
||||
msg = (f'Updated database thumbnail, but could not write art to '
|
||||
f'{failed} file(s) — check file/folder permissions')
|
||||
elif skipped:
|
||||
# Every file already had embedded art and no new cover.jpg was
|
||||
# needed — nothing to do, NOT a failure. This is the case that made
|
||||
# the old "(read-only?)" message fire on perfectly writable
|
||||
# libraries (Boulder on Windows, Sokhi): the files were simply
|
||||
# already arted, so embedded==0 and cover_written==False.
|
||||
msg = f'Cover art already present on all {skipped} file(s) — database thumbnail updated'
|
||||
else:
|
||||
# No file art applied and nothing found to write.
|
||||
msg = 'Updated database thumbnail (no file artwork was applied)'
|
||||
if artist_result is not None and artist_result.get('success'):
|
||||
msg += ' + applied artist image'
|
||||
return {'success': True, 'action': 'applied_cover_art', 'message': msg, 'art_result': art_result}
|
||||
|
|
|
|||
|
|
@ -128,3 +128,63 @@ def test_artist_action_without_found_artist_url_fails_cleanly(tmp_path):
|
|||
assert res['success'] is False
|
||||
album_thumb, artist_thumb = _thumbs(w)
|
||||
assert artist_thumb == 'http://old/artist.jpg' # nothing changed
|
||||
|
||||
|
||||
# ── apply-result message accuracy (Sokhi/Boulder: "read-only?" on writable fs) ──
|
||||
|
||||
def _add_track(w, path):
|
||||
conn = w.db._get_connection()
|
||||
c = conn.cursor()
|
||||
c.execute("INSERT INTO tracks VALUES ('t1', 'al1', ?)", (str(path),))
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
|
||||
def _apply_returns(monkeypatch, **art_result):
|
||||
import core.metadata.art_apply as aa
|
||||
base = {'embedded': 0, 'failed': 0, 'skipped': 0, 'cover_written': False, 'read_only_fs': False}
|
||||
base.update(art_result)
|
||||
monkeypatch.setattr(aa, 'apply_art_to_album_files', lambda *a, **k: base)
|
||||
|
||||
|
||||
def test_already_arted_reports_present_not_readonly(tmp_path, monkeypatch):
|
||||
# The bug: all files already had art (skipped) → embedded 0, cover 0 → the
|
||||
# old message cried "(read-only?)" on a perfectly writable library.
|
||||
w = _worker(tmp_path)
|
||||
f = tmp_path / 'song.mp3'; f.write_bytes(b'x')
|
||||
_add_track(w, f)
|
||||
_apply_returns(monkeypatch, skipped=1)
|
||||
res = w._fix_missing_cover_art('album', 'al1', None, {**DETAILS, '_fix_action': 'album'})
|
||||
assert res['success'] is True
|
||||
assert 'already present' in res['message'].lower()
|
||||
assert 'read-only' not in res['message'].lower()
|
||||
|
||||
|
||||
def test_failed_writes_blame_permissions_not_readonly(tmp_path, monkeypatch):
|
||||
w = _worker(tmp_path)
|
||||
f = tmp_path / 'song.mp3'; f.write_bytes(b'x')
|
||||
_add_track(w, f)
|
||||
_apply_returns(monkeypatch, failed=1)
|
||||
res = w._fix_missing_cover_art('album', 'al1', None, {**DETAILS, '_fix_action': 'album'})
|
||||
assert res['success'] is True
|
||||
assert 'permission' in res['message'].lower()
|
||||
assert 'read-only' not in res['message'].lower()
|
||||
|
||||
|
||||
def test_genuine_read_only_still_hard_fails(tmp_path, monkeypatch):
|
||||
w = _worker(tmp_path)
|
||||
f = tmp_path / 'song.mp3'; f.write_bytes(b'x')
|
||||
_add_track(w, f)
|
||||
_apply_returns(monkeypatch, read_only_fs=True)
|
||||
res = w._fix_missing_cover_art('album', 'al1', None, {**DETAILS, '_fix_action': 'album'})
|
||||
assert res['success'] is False
|
||||
assert 'read-only' in res['error'].lower()
|
||||
|
||||
|
||||
def test_embedded_success_message(tmp_path, monkeypatch):
|
||||
w = _worker(tmp_path)
|
||||
f = tmp_path / 'song.mp3'; f.write_bytes(b'x')
|
||||
_add_track(w, f)
|
||||
_apply_returns(monkeypatch, embedded=1)
|
||||
res = w._fix_missing_cover_art('album', 'al1', None, {**DETAILS, '_fix_action': 'album'})
|
||||
assert res['success'] is True and 'embedded into 1' in res['message']
|
||||
|
|
|
|||
Loading…
Reference in a new issue