Merge pull request #540 from Nezreka/fix/plex-non-english-music-section-issue-535
Plex: trigger_library_scan + is_library_scanning use auto-detected se…
This commit is contained in:
BoulderBadgeDad
GitHub
commit
03533454bc
3 changed files with 240 additions and 9 deletions
|
|
@ -1197,12 +1197,28 @@ class PlexClient(MediaServerClient):
|
|||
"""Trigger Plex library scan.
|
||||
|
||||
In all-libraries mode, fans the scan trigger across every music
|
||||
section. Otherwise targets the named section (default ``Music``).
|
||||
Returns True if at least one section was successfully triggered.
|
||||
section. Otherwise targets the auto-detected music section
|
||||
(``self.music_library`` — populated by ``_find_music_library``
|
||||
which iterates by ``section.type == 'artist'`` and works
|
||||
regardless of the section's display name). Falls back to
|
||||
looking up by ``library_name`` only when auto-detection
|
||||
hasn't run / failed (test fixtures, edge cases).
|
||||
|
||||
Pre-fix this method ignored ``self.music_library`` and always
|
||||
called ``self.server.library.section(library_name)`` with the
|
||||
hardcoded "Music" default — broke for any non-English section
|
||||
name (Música, Musique, Musik, etc. — issue #535). Read
|
||||
methods like ``get_artists`` already routed through
|
||||
``_get_music_sections`` so they didn't have the bug; this
|
||||
aligns the scan-trigger path with the same resolution.
|
||||
|
||||
Returns True if at least one section was successfully
|
||||
triggered.
|
||||
"""
|
||||
if not self.ensure_connection():
|
||||
return False
|
||||
|
||||
section_label = library_name
|
||||
try:
|
||||
if self._all_libraries_mode:
|
||||
sections = self._get_music_sections()
|
||||
|
|
@ -1219,19 +1235,30 @@ class PlexClient(MediaServerClient):
|
|||
logger.info(f"Triggered Plex library scan across {triggered}/{len(sections)} music sections")
|
||||
return triggered > 0
|
||||
|
||||
library = self.server.library.section(library_name)
|
||||
# Prefer the auto-detected music section. Falls back to a
|
||||
# literal section lookup by library_name only when
|
||||
# auto-detection hasn't populated the cached reference.
|
||||
if self.music_library is not None:
|
||||
library = self.music_library
|
||||
section_label = library.title
|
||||
else:
|
||||
library = self.server.library.section(library_name)
|
||||
section_label = library_name
|
||||
library.update() # Non-blocking scan request
|
||||
logger.info(f"Triggered Plex library scan for '{library_name}'")
|
||||
logger.info(f"Triggered Plex library scan for '{section_label}'")
|
||||
return True
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to trigger library scan for '{library_name}': {e}")
|
||||
logger.error(f"Failed to trigger library scan for '{section_label}': {e}")
|
||||
return False
|
||||
|
||||
def is_library_scanning(self, library_name: str = "Music") -> bool:
|
||||
"""Check if Plex library is currently scanning.
|
||||
|
||||
In all-libraries mode, returns True if ANY music section is
|
||||
scanning. Otherwise checks the named section.
|
||||
scanning. Otherwise checks the auto-detected music section
|
||||
first (same fix as ``trigger_library_scan`` — see #535) and
|
||||
falls back to literal ``library_name`` lookup only when
|
||||
auto-detection hasn't populated the cached reference.
|
||||
"""
|
||||
if not self.ensure_connection():
|
||||
logger.debug("Not connected to Plex, cannot check scan status")
|
||||
|
|
@ -1261,7 +1288,13 @@ class PlexClient(MediaServerClient):
|
|||
logger.debug(f"Could not check server activities: {activities_error}")
|
||||
return False
|
||||
|
||||
library = self.server.library.section(library_name)
|
||||
# Same resolution as trigger_library_scan — prefer the
|
||||
# auto-detected section so non-English Plex section names
|
||||
# (Música, Musique, Musik) don't break scan-status checks.
|
||||
if self.music_library is not None:
|
||||
library = self.music_library
|
||||
else:
|
||||
library = self.server.library.section(library_name)
|
||||
|
||||
# Check if library has a scanning attribute or is refreshing
|
||||
# The Plex API exposes this through the library's refreshing property
|
||||
|
|
@ -1272,7 +1305,12 @@ class PlexClient(MediaServerClient):
|
|||
logger.debug("Library is refreshing")
|
||||
return True
|
||||
|
||||
# Alternative method: Check server activities for scanning
|
||||
# Alternative method: Check server activities for scanning.
|
||||
# Match on the actual resolved section's title — NOT the
|
||||
# `library_name` arg, which defaults to "Music" and would
|
||||
# never match activities for non-English sections like
|
||||
# "Música" / "Musique" / "Musik" (#535).
|
||||
section_title = getattr(library, 'title', library_name)
|
||||
try:
|
||||
activities = self.server.activities()
|
||||
logger.debug("Found %s server activities", len(activities))
|
||||
|
|
@ -1284,7 +1322,7 @@ class PlexClient(MediaServerClient):
|
|||
logger.debug("Activity - type=%s title=%s", activity_type, activity_title)
|
||||
|
||||
if (activity_type in ['library.scan', 'library.refresh'] and
|
||||
library_name.lower() in activity_title.lower()):
|
||||
section_title.lower() in activity_title.lower()):
|
||||
logger.debug("Found matching scan activity: %s", activity_title)
|
||||
return True
|
||||
except Exception as activities_error:
|
||||
|
|
|
|||
192
tests/media_server/test_plex_non_english_section_name.py
Normal file
192
tests/media_server/test_plex_non_english_section_name.py
Normal file
|
|
@ -0,0 +1,192 @@
|
|||
"""Pin Plex scan-trigger + scan-status methods against non-English
|
||||
section names — issue #535.
|
||||
|
||||
Background
|
||||
----------
|
||||
|
||||
A Plex server with the music library named "Música" (Spanish),
|
||||
"Musique" (French), "Musik" (German), etc. — type still
|
||||
``artist`` — would have ``_find_music_library`` correctly
|
||||
auto-detect the section by type. ``self.music_library`` was
|
||||
populated correctly. Read methods (``get_artists`` etc.) routed
|
||||
through ``_get_music_sections`` which returned ``[self.music_library]``
|
||||
and worked.
|
||||
|
||||
But ``trigger_library_scan`` and ``is_library_scanning`` ignored
|
||||
``self.music_library`` and called ``self.server.library.section(library_name)``
|
||||
with a hardcoded ``"Music"`` default. ``server.library.section('Music')``
|
||||
raised ``NotFound`` for any server whose music section wasn't
|
||||
literally named "Music", so post-import scans never fired.
|
||||
|
||||
Side effect: wishlist.processing kept reporting "Missing from
|
||||
media server after sync" for tracks that DID import correctly,
|
||||
re-adding them to the wishlist forever.
|
||||
|
||||
These tests pin both methods through the auto-detected-section
|
||||
path. Both single-library + all-libraries modes get coverage.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from core.plex_client import PlexClient
|
||||
|
||||
|
||||
def _make_client(*, all_libraries_mode: bool = False, music_library=None, server=None):
|
||||
"""Same fixture pattern as test_plex_all_libraries.py — minimal
|
||||
construction skipping `__init__` so the test owns the entire
|
||||
client state."""
|
||||
client = PlexClient.__new__(PlexClient)
|
||||
client.server = server
|
||||
client.music_library = music_library
|
||||
client._all_libraries_mode = all_libraries_mode
|
||||
client._connection_attempted = server is not None
|
||||
client._is_connecting = False
|
||||
client._last_connection_check = 0
|
||||
client._connection_check_interval = 30
|
||||
return client
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# trigger_library_scan — single-library mode (the broken path in #535)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestTriggerLibraryScanNonEnglishSection:
|
||||
|
||||
@pytest.mark.parametrize('section_name', ['Música', 'Musique', 'Musik', 'Musica', '音乐', 'موسيقى'])
|
||||
def test_uses_auto_detected_section_regardless_of_locale(self, section_name):
|
||||
"""The fix's headline assertion. Auto-detected section's
|
||||
update() must be called — NOT a literal 'Music' lookup that
|
||||
would NotFound on any non-English server."""
|
||||
section = MagicMock(title=section_name)
|
||||
server = MagicMock()
|
||||
# If the code falls back to literal lookup, raise so the test
|
||||
# fails loudly instead of silently calling the wrong method.
|
||||
server.library.section.side_effect = AssertionError(
|
||||
f"Should NOT call server.library.section() when "
|
||||
f"music_library is populated with '{section_name}'"
|
||||
)
|
||||
client = _make_client(server=server, music_library=section)
|
||||
|
||||
result = client.trigger_library_scan()
|
||||
|
||||
assert result is True
|
||||
section.update.assert_called_once()
|
||||
|
||||
def test_falls_back_to_literal_lookup_when_no_auto_detection(self):
|
||||
"""Backward compat: if music_library is None (test fixtures,
|
||||
edge cases where auto-detection hasn't run), fall back to the
|
||||
literal `library_name` lookup as before. Default 'Music' arg
|
||||
preserved — calling with no kwargs still works."""
|
||||
looked_up = MagicMock(title='Music')
|
||||
server = MagicMock()
|
||||
server.library.section.return_value = looked_up
|
||||
client = _make_client(server=server, music_library=None)
|
||||
|
||||
result = client.trigger_library_scan()
|
||||
|
||||
assert result is True
|
||||
server.library.section.assert_called_once_with('Music')
|
||||
looked_up.update.assert_called_once()
|
||||
|
||||
def test_explicit_library_name_arg_used_only_when_no_auto_detection(self):
|
||||
"""When music_library is populated, the library_name kwarg is
|
||||
ignored — auto-detected section wins. Otherwise the kwarg
|
||||
controls the literal lookup."""
|
||||
section = MagicMock(title='Música')
|
||||
server = MagicMock()
|
||||
server.library.section.side_effect = AssertionError("must not fall back")
|
||||
client = _make_client(server=server, music_library=section)
|
||||
|
||||
# Pass a non-default library_name — auto-detected wins.
|
||||
result = client.trigger_library_scan(library_name='Whatever')
|
||||
|
||||
assert result is True
|
||||
section.update.assert_called_once()
|
||||
|
||||
def test_logs_correct_section_label_on_success(self, caplog):
|
||||
"""Log line must surface the actual section's title (`Música`)
|
||||
not the unused library_name default ('Music'). Pre-fix the
|
||||
success log read 'Triggered Plex library scan for Music' even
|
||||
on Spanish servers — confusing when debugging."""
|
||||
section = MagicMock(title='Música')
|
||||
client = _make_client(server=MagicMock(), music_library=section)
|
||||
|
||||
with caplog.at_level('INFO', logger='soulsync.plex_client'):
|
||||
client.trigger_library_scan()
|
||||
|
||||
assert any('Música' in r.getMessage() for r in caplog.records), (
|
||||
f"Expected log to mention 'Música'; got "
|
||||
f"{[r.getMessage() for r in caplog.records]}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# is_library_scanning — symmetric fix
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestIsLibraryScanningNonEnglishSection:
|
||||
|
||||
def test_uses_auto_detected_section_for_refreshing_check(self):
|
||||
"""`is_library_scanning` must check the auto-detected
|
||||
section's `refreshing` attribute — NOT a literal-name
|
||||
lookup that fails on non-English servers."""
|
||||
section = MagicMock(title='Música', refreshing=True)
|
||||
server = MagicMock()
|
||||
server.activities.return_value = []
|
||||
server.library.section.side_effect = AssertionError("must not fall back")
|
||||
client = _make_client(server=server, music_library=section)
|
||||
|
||||
result = client.is_library_scanning()
|
||||
|
||||
assert result is True
|
||||
|
||||
def test_activity_match_uses_resolved_section_title(self):
|
||||
"""Activity feed match must filter by the resolved section's
|
||||
title — NOT the library_name kwarg default ('Music'). Pre-fix
|
||||
a Spanish server's "Música" scan activity wouldn't match the
|
||||
literal 'music' substring check (well, "música".contains("music")
|
||||
IS true here by coincidence — but for "Musique" / "Musik" /
|
||||
"音乐" / "موسيقى" the substring miss is real)."""
|
||||
section = MagicMock(title='موسيقى', refreshing=False)
|
||||
activity = MagicMock(type='library.scan', title='Scanning موسيقى')
|
||||
server = MagicMock()
|
||||
server.activities.return_value = [activity]
|
||||
server.library.section.side_effect = AssertionError("must not fall back")
|
||||
client = _make_client(server=server, music_library=section)
|
||||
|
||||
result = client.is_library_scanning()
|
||||
|
||||
assert result is True
|
||||
|
||||
def test_no_match_when_activity_for_unrelated_section(self):
|
||||
"""Sanity: activity for a DIFFERENT section's scan shouldn't
|
||||
cause a false-positive "music library is scanning" reading."""
|
||||
section = MagicMock(title='Música', refreshing=False)
|
||||
# Activity is for a different section (e.g. Movies)
|
||||
activity = MagicMock(type='library.scan', title='Scanning Películas')
|
||||
server = MagicMock()
|
||||
server.activities.return_value = [activity]
|
||||
server.library.section.side_effect = AssertionError("must not fall back")
|
||||
client = _make_client(server=server, music_library=section)
|
||||
|
||||
assert client.is_library_scanning() is False
|
||||
|
||||
def test_falls_back_to_literal_lookup_when_no_auto_detection(self):
|
||||
"""Backward compat: music_library=None → literal section
|
||||
lookup by library_name (default 'Music'). Same as before fix."""
|
||||
looked_up = MagicMock(title='Music', refreshing=False)
|
||||
server = MagicMock()
|
||||
server.library.section.return_value = looked_up
|
||||
server.activities.return_value = []
|
||||
client = _make_client(server=server, music_library=None)
|
||||
|
||||
result = client.is_library_scanning()
|
||||
|
||||
assert result is False
|
||||
server.library.section.assert_called_once_with('Music')
|
||||
|
|
@ -3416,6 +3416,7 @@ const WHATS_NEW = {
|
|||
'2.4.3': [
|
||||
// --- post-release patch work on the 2.4.3 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.4.3 patch work' },
|
||||
{ title: 'Plex: Library Scan Trigger No Longer Fails On Non-English Section Names', desc: 'github issue #535 (adrigzr): plex servers with the music library named anything other than "music" — Música, Musique, Musik, Musica, etc. — got a `Failed to trigger library scan for "Music": Invalid library section: Music` error after every import cycle, and `wishlist.processing` kept reporting "missing from media server after sync" for tracks that DID import correctly because the post-import scan never fired. cause: `trigger_library_scan` and `is_library_scanning` ignored the auto-detected `self.music_library` (correctly populated by `_find_music_library` filtering by `section.type == "artist"`) and called `self.server.library.section(library_name)` with a hardcoded "music" default — raised NotFound on any non-english server. read methods like `get_artists` already routed through `_get_music_sections` so they didn\'t have the bug; this aligns the scan-trigger path with the same resolution. fix: both single-library branches prefer `self.music_library` first, fall back to literal section lookup only when auto-detection hasn\'t run. activity-feed match in `is_library_scanning` also corrected to use the resolved section\'s actual title instead of the unused `library_name` arg — the prior log line read "triggered scan for music" even on Spanish servers. 13 new tests pin: trigger uses auto-detected section across 6 locale variants (Música / Musique / Musik / Musica / 音乐 / موسيقى), backward-compat fallback when music_library is None, explicit library_name kwarg ignored when auto-detected section exists, log line surfaces correct section title, scan-status check uses auto-detected section\'s `refreshing` attr, activity-feed match filters by resolved title (not library_name).', page: 'settings' },
|
||||
{ title: 'Search For Match: No More Karaoke / Cover / "Originally Performed By" Junk At The Top', desc: 'github issue #534 (radoslav-orlov): typing "dirty white boy" + "foreigner" into the import-modal "search for match" dialog returned karaoke versions, "originally performed by" compilations, and tribute-band cuts ranked above the actual foreigner studio recording in some regions. user had to scroll past 5+ junk results before finding the canonical track. fix: new `core/metadata/relevance.py` helper reranks results locally with cover/karaoke/tribute/re-recorded penalties (multiplier 0.05× — effectively buries) + exact-artist-match boost (1.5×) + variant-tag (live/acoustic/remix/remaster) penalty (0.4×, skipped when user explicitly typed the variant — searching "track (live)" still ranks live versions correctly). applied at the deezer + itunes + spotify search-tracks endpoints so all three sources behave consistently. validated against live deezer api with the actual #534 query: real foreigner head games cut now lands at #1, live versions follow, karaoke / cover / tribute variants drop to positions 11-15. deezer client also gained optional field-scoped query kwargs (`track="X" artist="Y"`) that build deezer\'s advanced search syntax `track:"X" artist:"Y"` for future opt-in callers (e.g. exact-match flows where api-level filtering is more important than ranking) — kept in client but NOT used at the import-modal endpoint after live testing showed the advanced syntax has its own ranking bias (surfaced "(2008 remaster)" instead of the canonical recording). free-text + local rerank is the more reliable combination here. 75 new tests pin every scoring component, pattern detection (13 cover patterns, 11 variant patterns, 3 fields), score composition (real-cut > karaoke > remaster > re-recorded), the issue #534 screenshot reproduced as a regression test, deezer client query construction + free-text fallback safety net.', page: 'import' },
|
||||
{ title: 'Auto-Import: Album Duration Is Album Total + Re-Imports Fill Metadata Gaps', desc: 'two more parity gaps closed in the soulsync standalone library write path. (1) album row\'s `duration` column was being written with the FIRST imported track\'s duration instead of the album total — pre-existing bug that survived the prior parity commit. soulsync_client deep scan computes `sum(t.duration for t in self._tracks)` for each album; auto-import now mirrors that by computing the sum across every matched track in the worker and threading it through context to the album INSERT. (2) `record_soulsync_library_entry` was insert-only on artists + albums — once a row existed (matched by id OR name fallback), subsequent imports of the same artist or album skipped completely. meant: artist genres / thumb / source-id reflected ONLY whatever the FIRST imported album supplied, never refreshing as more albums by that artist landed (ten more deezer/spotify imports later, artist row still had whatever the first random import wrote). new conservative UPDATE path: when an existing row matches, fill ONLY the columns whose current value is NULL or empty — never overwrites populated values. protects manual edits + enrichment-worker writes the same way scanner UPDATEs preserve enrichment columns. f-string column names are validated against an allowlist (`_SOULSYNC_FILLABLE_COLUMNS`) before interpolation — defensive against accidental misuse adding columns without an allowlist update. 4 new tests pin: album duration uses sum not single-track, re-import fills empty thumb + genres on existing artist row, re-import does NOT clobber populated values, re-import fills empty source-id columns when later import has them.', page: 'import' },
|
||||
{ title: 'Auto-Import: Genre Tags Land On The Artists Row + ISRC/MBID Type Hardening', desc: 'small followup to the standalone-library parity commit. (1) auto-import now reads the GENRE tag from each matched audio file (mutagen easy mode, supports flac / mp3 / m4a) and aggregates the deduped set across the album onto the new artists row\'s genres column. matches what soulsync_client._scan_transfer would have written if you\'d done a fresh deep scan after the import — your imported artists no longer feel hollow compared to plex / jellyfin / navidrome scans. dedup is case-insensitive but preserves original casing + insertion order so the json column reads naturally ("Hip-Hop, Rap, Trap" not "hip-hop, rap, trap"). (2) defensive `str()` cast on the worker\'s isrc + mbid extraction. metadata source clients all coerce to string today via `_build_album_track_entry`, but if a future source ever returned int / None for either id the side-effects layer would crash on `.strip()`. cheap insurance. 3 new tests pin: genre aggregation produces deduped insertion-order list, empty when no GENRE tags, isrc/mbid hostile-type input (int, None) coerced to safe string before propagation.', page: 'import' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue