diff --git a/core/plex_client.py b/core/plex_client.py index 890e6f46..0cd31a35 100644 --- a/core/plex_client.py +++ b/core/plex_client.py @@ -371,6 +371,74 @@ class PlexClient(MediaServerClient): if self.music_library: return self.music_library.searchArtists(title=title, limit=limit) return [] + + # ------------------------------------------------------------------ + # Cross-section dedup. Applied ONLY in all-libraries mode and ONLY + # at the listing/stats layer. Never apply to ratingKey enumeration + # (``get_all_artist_ids`` / ``get_all_album_ids``) — removal + # detection compares those sets against DB-linked ratingKeys and + # deduping would falsely prune library tracks pointing at the + # non-canonical ratingKey. Single-library mode is a no-op fast + # path so the dedup code path never touches it. + # ------------------------------------------------------------------ + + def _dedupe_artists(self, artists: List[PlexArtist]) -> List[PlexArtist]: + """Collapse same-name artists across sections to a single + canonical entry (the one with the higher track count). Returns + the input unchanged in single-library mode and when there's + nothing to dedup.""" + if not self._all_libraries_mode or not artists: + return artists + by_name: Dict[str, PlexArtist] = {} + for a in artists: + name_key = (getattr(a, 'title', '') or '').strip().lower() + if not name_key: + # Untitled artist — keep as-is, can't dedup blindly. + by_name[f'__nokey_{getattr(a, "ratingKey", id(a))}'] = a + continue + existing = by_name.get(name_key) + if existing is None: + by_name[name_key] = a + continue + try: + existing_count = getattr(existing, 'leafCount', 0) or 0 + new_count = getattr(a, 'leafCount', 0) or 0 + if new_count > existing_count: + by_name[name_key] = a + except Exception: + pass + return list(by_name.values()) + + def _dedupe_albums(self, albums: List[PlexAlbum]) -> List[PlexAlbum]: + """Collapse same-album-by-same-artist across sections to a + single canonical entry. Group key is (artist_lower, title_lower); + canonical = higher track count. No-op in single-library mode.""" + if not self._all_libraries_mode or not albums: + return albums + by_key: Dict[tuple, PlexAlbum] = {} + for alb in albums: + title = (getattr(alb, 'title', '') or '').strip().lower() + if not title: + by_key[('__nokey__', getattr(alb, 'ratingKey', id(alb)))] = alb + continue + artist = '' + try: + artist = (getattr(alb, 'parentTitle', '') or '').strip().lower() + except Exception: + pass + key = (artist, title) + existing = by_key.get(key) + if existing is None: + by_key[key] = alb + continue + try: + existing_count = getattr(existing, 'leafCount', 0) or 0 + new_count = getattr(alb, 'leafCount', 0) or 0 + if new_count > existing_count: + by_key[key] = alb + except Exception: + pass + return list(by_key.values()) def get_all_playlists(self) -> List[PlaylistInfo]: if not self.ensure_connection(): @@ -720,9 +788,13 @@ class PlexClient(MediaServerClient): return {} try: + # Apply dedup to artist + album counts so stats match what + # the user actually sees in the library list (deduped). Tracks + # stay raw — same track in two sections means two distinct + # files / Plex entries, not a logical duplicate. return { - 'artists': len(self._all_artists()), - 'albums': len(self._all_albums()), + 'artists': len(self._dedupe_artists(self._all_artists())), + 'albums': len(self._dedupe_albums(self._all_albums())), 'tracks': len(self._all_tracks()) } except Exception as e: @@ -868,14 +940,27 @@ class PlexClient(MediaServerClient): return {} def get_all_artists(self) -> List[PlexArtist]: - """Get all artists from the music library""" + """Get all artists from the music library. + + In all-libraries mode, dedupes same-name artists across + sections (canonical = higher track count) so the library list + doesn't show "Drake" twice when Drake is in two sections. + Single-library mode is unaffected — dedup helper is a no-op. + """ if not self.ensure_connection() or not self._can_query(): logger.error("Not connected to Plex server or no music library") return [] try: - artists = self._all_artists() - logger.info(f"Found {len(artists)} artists in Plex library") + raw = self._all_artists() + artists = self._dedupe_artists(raw) + if len(raw) != len(artists): + logger.info( + f"Found {len(raw)} artists across all music sections " + f"({len(artists)} unique after cross-section dedup)" + ) + else: + logger.info(f"Found {len(artists)} artists in Plex library") return artists except Exception as e: logger.error(f"Error getting all artists: {e}") diff --git a/tests/media_server/test_plex_all_libraries.py b/tests/media_server/test_plex_all_libraries.py index fc84883e..ee4cc823 100644 --- a/tests/media_server/test_plex_all_libraries.py +++ b/tests/media_server/test_plex_all_libraries.py @@ -155,12 +155,16 @@ def test_get_all_album_ids_uses_server_search_in_all_libraries_mode(): def test_get_library_stats_unions_across_sections_in_all_libraries_mode(): """All-libraries stats sum totals across every music section via - server-wide search calls.""" + server-wide search calls. Artists / albums use distinct names so + dedup doesn't collapse them — separate test pins the dedup path.""" server = MagicMock() + distinct_artists = [_fake_artist(f'Artist {i}', rating_key=str(i), leaf_count=i + 1) for i in range(5)] + distinct_albums = [_fake_album(f'Album {i}', parent=f'Artist {i}', rating_key=str(100 + i), leaf_count=i + 1) for i in range(12)] + distinct_tracks = [MagicMock(ratingKey=str(1000 + i)) for i in range(87)] server.library.search.side_effect = [ - [MagicMock()] * 5, # artists - [MagicMock()] * 12, # albums - [MagicMock()] * 87, # tracks + distinct_artists, + distinct_albums, + distinct_tracks, ] client = _make_client(server=server, all_libraries_mode=True, music_library=None) @@ -377,3 +381,164 @@ def test_get_music_library_locations_dedupes_overlapping_paths(): assert locations.count('/data/shared') == 1 assert '/data/userTwo' in locations + + +# --------------------------------------------------------------------------- +# Cross-section dedup (only active in all-libraries mode) +# --------------------------------------------------------------------------- + + +def _fake_artist(name, rating_key, leaf_count): + a = MagicMock() + a.title = name + a.ratingKey = rating_key + a.leafCount = leaf_count + return a + + +def _fake_album(title, parent, rating_key, leaf_count): + a = MagicMock() + a.title = title + a.parentTitle = parent + a.ratingKey = rating_key + a.leafCount = leaf_count + return a + + +def test_dedupe_artists_keeps_canonical_with_higher_track_count(): + """Pin: same-name artists across sections collapse to one — the + one with the higher leafCount wins. Plex Home users with overlapping + music tastes (both have Drake) shouldn't see "Drake" twice in + SoulSync's library.""" + server = MagicMock() + client = _make_client(server=server, all_libraries_mode=True, music_library=None) + + drake_a = _fake_artist('Drake', rating_key='1', leaf_count=12) + drake_b = _fake_artist('Drake', rating_key='2', leaf_count=87) # canonical + kendrick = _fake_artist('Kendrick Lamar', rating_key='3', leaf_count=40) + + deduped = client._dedupe_artists([drake_a, drake_b, kendrick]) + + names = sorted(a.title for a in deduped) + assert names == ['Drake', 'Kendrick Lamar'] + drake_picked = next(a for a in deduped if a.title == 'Drake') + assert drake_picked.ratingKey == '2' # higher leafCount wins + + +def test_dedupe_artists_case_insensitive_match(): + """Pin: dedup matches lowercased name so "Drake" + "drake" + "DRAKE" + collapse together.""" + server = MagicMock() + client = _make_client(server=server, all_libraries_mode=True, music_library=None) + a1 = _fake_artist('Drake', rating_key='1', leaf_count=10) + a2 = _fake_artist('drake', rating_key='2', leaf_count=20) + a3 = _fake_artist('DRAKE', rating_key='3', leaf_count=5) + + deduped = client._dedupe_artists([a1, a2, a3]) + + assert len(deduped) == 1 + assert deduped[0].ratingKey == '2' # canonical = highest count + + +def test_dedupe_artists_noop_in_single_library_mode(): + """Pin: dedup is bypassed entirely in single-library mode — the + input list comes back unchanged. Single-section users get zero + behavior change from the dedup logic.""" + server = MagicMock() + section = MagicMock() + client = _make_client(server=server, music_library=section, all_libraries_mode=False) + + drake_a = _fake_artist('Drake', rating_key='1', leaf_count=12) + drake_b = _fake_artist('Drake', rating_key='2', leaf_count=87) + + result = client._dedupe_artists([drake_a, drake_b]) + + assert len(result) == 2 + assert result == [drake_a, drake_b] + + +def test_dedupe_albums_groups_by_artist_and_title(): + """Pin: album dedup keys on (artist, title) so two artists with + same album title (e.g. self-titled) stay separate.""" + server = MagicMock() + client = _make_client(server=server, all_libraries_mode=True, music_library=None) + + drake_self_a = _fake_album('Drake', parent='Drake', rating_key='1', leaf_count=15) + drake_self_b = _fake_album('Drake', parent='Drake', rating_key='2', leaf_count=15) + weeknd_drake = _fake_album('Drake', parent='The Weeknd', rating_key='3', leaf_count=8) # different artist + + deduped = client._dedupe_albums([drake_self_a, drake_self_b, weeknd_drake]) + + assert len(deduped) == 2 + artists = sorted(a.parentTitle for a in deduped) + assert artists == ['Drake', 'The Weeknd'] + + +def test_get_all_artists_dedupes_in_all_libraries_mode(): + """Pin: ``get_all_artists`` (the public listing) returns deduped + list in all-libraries mode.""" + server = MagicMock() + drake_a = _fake_artist('Drake', rating_key='1', leaf_count=12) + drake_b = _fake_artist('Drake', rating_key='2', leaf_count=87) + server.library.search.return_value = [drake_a, drake_b] + client = _make_client(server=server, all_libraries_mode=True, music_library=None) + + result = client.get_all_artists() + + assert len(result) == 1 + assert result[0].ratingKey == '2' + + +def test_get_all_artist_ids_does_NOT_dedupe_critical_for_removal_detection(): + """CRITICAL pin: ``get_all_artist_ids`` returns the RAW ratingKey + set, even in all-libraries mode. Removal detection compares this + set against DB-linked ratingKeys to decide what's been removed — + deduping here would falsely report non-canonical ratingKeys as + "removed" and prune library tracks pointing at them.""" + server = MagicMock() + drake_a = _fake_artist('Drake', rating_key='1', leaf_count=12) + drake_b = _fake_artist('Drake', rating_key='2', leaf_count=87) + server.library.search.return_value = [drake_a, drake_b] + client = _make_client(server=server, all_libraries_mode=True, music_library=None) + + ids = client.get_all_artist_ids() + + # Both ratingKeys returned, NOT deduped. + assert ids == {'1', '2'} + + +def test_get_all_album_ids_does_NOT_dedupe_critical_for_removal_detection(): + """Same critical pin for albums.""" + server = MagicMock() + alb_a = _fake_album('Take Care', parent='Drake', rating_key='10', leaf_count=15) + alb_b = _fake_album('Take Care', parent='Drake', rating_key='20', leaf_count=15) + server.library.search.return_value = [alb_a, alb_b] + client = _make_client(server=server, all_libraries_mode=True, music_library=None) + + ids = client.get_all_album_ids() + + assert ids == {'10', '20'} + + +def test_get_library_stats_uses_deduped_counts_for_artists_albums(): + """Pin: stats reflect the deduped counts users see in the library + list. Tracks stay raw — same track in two sections is two files.""" + server = MagicMock() + drake_a = _fake_artist('Drake', rating_key='1', leaf_count=12) + drake_b = _fake_artist('Drake', rating_key='2', leaf_count=87) + kendrick = _fake_artist('Kendrick Lamar', rating_key='3', leaf_count=40) + alb_a = _fake_album('Take Care', parent='Drake', rating_key='10', leaf_count=15) + alb_b = _fake_album('Take Care', parent='Drake', rating_key='20', leaf_count=15) + track1 = MagicMock(ratingKey=100) + track2 = MagicMock(ratingKey=101) + track3 = MagicMock(ratingKey=102) + server.library.search.side_effect = [ + [drake_a, drake_b, kendrick], # artist call → 3 raw → 2 deduped + [alb_a, alb_b], # album call → 2 raw → 1 deduped + [track1, track2, track3], # track call → 3 raw, no dedup + ] + client = _make_client(server=server, all_libraries_mode=True, music_library=None) + + stats = client.get_library_stats() + + assert stats == {'artists': 2, 'albums': 1, 'tracks': 3} diff --git a/webui/static/helper.js b/webui/static/helper.js index ad83c591..fe7fe366 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3432,7 +3432,7 @@ const WHATS_NEW = { '2.4.2': [ // --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- { date: 'Unreleased — 2.4.2 dev cycle' }, - { title: 'Plex: "All Libraries (Combined)" Mode', desc: 'github issue #505 (popebruhlxix): users with multiple plex music libraries (e.g. one per plex home user) only saw one library inside soulsync because the connection settings forced you to pick a single library section. now there\'s a new "all libraries (combined)" option in settings → connections → plex → music library dropdown. picking it flips the plex client into a server-wide read mode where every read method (`get_all_artists` / `get_all_album_ids` / `search_tracks` / `get_library_stats` / etc) dispatches through `server.library.search(libtype=...)` instead of querying a single section. one api call, plex handles the aggregation. write methods (genre / poster / metadata updates) are unaffected — they operate on plex objects via ratingKey, section-agnostic. trigger_library_scan + is_library_scanning fan out across every music section in the new mode. backward compatible — existing users with a real library name saved see no behavior change. the "all libraries" option only appears in the dropdown when more than one music library exists on the server. honest tradeoffs: same artist appearing in multiple sections shows as separate entries (no dedup), and write-back updates only one section\'s copy when an artist exists in multiple. document and revisit if it bites users. 15 new tests pin both modes (single-section preserved, all-libraries dispatches through server-wide search).', page: 'settings' }, + { title: 'Plex: "All Libraries (Combined)" Mode', desc: 'github issue #505 (popebruhlxix): users with multiple plex music libraries (e.g. one per plex home user) only saw one library inside soulsync because the connection settings forced you to pick a single library section. now there\'s a new "all libraries (combined)" option in settings → connections → plex → music library dropdown. picking it flips the plex client into a server-wide read mode where every read method (`get_all_artists` / `get_all_album_ids` / `search_tracks` / `get_library_stats` / etc) dispatches through `server.library.search(libtype=...)` instead of querying a single section. one api call, plex handles the aggregation. cross-section dedup applied at the listing layer — same-name artists across sections collapse to a canonical entry (the one with more tracks), so plex home families with overlapping music tastes don\'t see "drake" twice. removal-detection id enumeration stays raw on purpose — deduping there would falsely prune tracks linked to non-canonical ratingKeys. write methods (genre / poster / metadata updates) are unaffected and operate on plex objects via ratingKey directly — write-back targets one section\'s copy of an artist if it exists in multiple, document and revisit if it matters. trigger_library_scan + is_library_scanning fan out across every music section in the new mode. backward compatible — existing users with a real library name saved see no behavior change. the "all libraries" option only appears in the dropdown when more than one music library exists on the server. 29 new tests pin both modes (single-section preserved, all-libraries dispatches through server-wide search, dedup keeps canonical, id enumeration stays raw).', page: 'settings' }, { title: 'Fix: Download Discography Showed Wrong Artist\'s Albums', desc: 'clicked download discography on 50 cent → modal showed young hot rod\'s albums. clicked weird al → modal showed the beatles. cause: the endpoint received whichever single artist id the frontend happened to pick (spotify or itunes or deezer or library db id) and dispatched it as-is to whichever source it queried. when the picked id didn\'t match the queried source\'s id format, lookup either returned wrong-artist results (numeric collisions — db id 194687 was a real deezer artist for someone else) or fell back to a fuzzy name search that picked a wrong artist. the per-source id dispatch mechanism (`MetadataLookupOptions.artist_source_ids`) already existed and the watchlist scanner already used it; the on-demand discography endpoint just wasn\'t wired to it. fixed: when the url artist_id matches a library row by ANY stored id (db id, spotify_artist_id, itunes_artist_id, deezer_id, musicbrainz_id), backend pulls every stored provider id and dispatches the right id to each source. each source gets its OWN stored id regardless of what the url carries. when the url id is a non-library source-native id and the row lookup misses entirely, behavior is identical to before (single-id fallback). also fixed two log-namespace bugs: enhance quality and multi-source search were writing through `getLogger(__name__)` which resolves to `core.artists.quality` / `core.metadata.multi_source_search` — neither under the soulsync handler — so every diagnostic line was silently dropped. switched both to `get_logger()` from utils.logging_config so they actually land in app.log.', page: 'library' }, { title: 'Enhance Quality: Direct ID Lookup Like Download Discography', desc: 'two related fixes. (1) discord report: enhance quality on an artist with neither spotify nor deezer connected added tracks as "unknown artist - unknown album - unknown track". enhance was running a single-source itunes fallback chain that returned junk matches with empty fields, while track redownload had been doing parallel multi-source search the whole time. extracted that search into `core/metadata/multi_source_search.py` and pointed both flows at it. (2) followup: enhance was still using fuzzy text search even when the library track had a stored source ID (spotify_track_id / deezer_id / itunes_track_id / soul_id) on the row, which meant tracks with messy tags ("Title (Live)", featured artists in the artist field, etc.) failed to match even though a perfect ID was sitting right there. download discography never had this problem because it resolves albums by stable ID, not by name. enhance now does the same: for every source you have configured, if the track has a stored ID for that source, it calls `get_track_details(id)` directly — no fuzzy matching. preferred source (your configured primary) is tried first so a deezer-primary user gets deezer payloads on the wishlist entry. text search is only the fallback now (kicks in for tracks with no stored IDs). also fixed the modal toast that lied "matching tracks to spotify..." regardless of which sources were actually being queried.', page: 'library' }, { title: 'Internal: Media Server Engine Cin/JohnBaumb Pass', desc: 'internal — applied the same architectural cleanups the download engine PR went through to the media server engine PR before review. (1) every server client (Plex / Jellyfin / Navidrome / SoulSync) now explicitly inherits `MediaServerClient` instead of relying on structural typing — drift in any class fails at the conformance test boundary. (2) generic accessors on the engine: `configured_clients()` (replaces per-server `if X and X.is_connected(): clients[name] = X` chains in web_server.py) and `reload_config(name=None)` (generic dispatch instead of per-client reload calls). (3) singleton factory: `get_media_server_engine()` / `set_media_server_engine()` matching the metadata + download engine shape. web_server.py boots via `set_media_server_engine(...)` so factory + global handle share state. (4) ~70 direct `plex_client.X` / `jellyfin_client.X` / `navidrome_client.X` / `soulsync_library_client.X` attribute reaches in web_server.py migrated to `media_server_engine.client(\'\').X`. ~60 standalone refs (truthy checks, media_client assignments, source-name tuples) also routed through the engine. (5) the per-server `plex_client` / `jellyfin_client` / `navidrome_client` / `soulsync_library_client` globals in web_server.py are gone entirely — engine owns the client instances now, every caller reaches via `media_server_engine.client(\'\')`. four multi-client consumers (`PlaylistSyncService`, `ListeningStatsWorker`, `WebScanManager`, discovery `SyncDeps`) refactored to take the engine instead of separate per-server kwargs. (6) `TrackInfo` and `PlaylistInfo` lifted out of `core/plex_client.py` / `jellyfin_client.py` / `navidrome_client.py` (each was defining a near-identical copy) into the neutral `core/media_server/types.py` module — same lift Cin caught on the download `TrackResult`/`AlbumResult`/`DownloadStatus` situation. consumers (matching engine, sync service) get one import. zero behavior change.' }, @@ -3784,10 +3784,11 @@ const VERSION_MODAL_SECTIONS = [ "• new \"all libraries (combined)\" option in settings → connections → plex → music library dropdown — only shows up when your server has more than one music library", "• picking it flips the plex client into server-wide read mode — every scan / search / library-stat call dispatches through `server.library.search()` instead of querying a single section", "• one api call, plex handles the aggregation — no per-section iteration code on our side", - "• write methods (genre / poster / metadata updates) and playlists are unaffected — section-agnostic", + "• cross-section dedup at the listing layer — same-name artists across sections (e.g. plex home families that both have drake) collapse to one canonical entry in your library list, so no more visual duplicates", + "• removal detection stays on raw ratingKeys — deduping there would falsely prune tracks linked to non-canonical entries", + "• write methods (genre / poster / metadata updates) and playlists are unaffected — section-agnostic, operate on plex objects via ratingKey", "• trigger_library_scan + is_library_scanning fan out across every music section in the new mode", "• backward compatible — existing users with a single library saved see no behavior change", - "• honest tradeoffs: same artist appearing in multiple sections shows as separate entries (no dedup yet), and write-back updates only one section\'s copy if an artist exists in multiple — revisit if it bites users", ], usage_note: "settings → connections → plex → music library → pick \"all libraries (combined)\"", },