fix(artists): stabilize scoped top tracks

This commit is contained in:
cucadmuh
2026-07-18 03:20:11 +03:00
parent acaa9c1c50
commit 46b9ede010
16 changed files with 890 additions and 30 deletions
@@ -962,6 +962,10 @@ pub struct LibraryScopeArtistDetailRequest {
/// the extra scoped track query.
#[serde(default = "default_true")]
pub include_tracks: bool,
/// When set, return only the highest personal-play-count tracks. The
/// artist page uses this as a bounded fallback while server Top Songs load.
#[serde(default)]
pub top_tracks_limit: Option<u32>,
}
/// `library_scope_album_detail` response.
@@ -979,6 +983,12 @@ pub struct LibraryScopeArtistDetailResponse {
pub artist: LibraryArtistDto,
pub albums: Vec<LibraryAlbumDto>,
pub tracks: Vec<LibraryTrackDto>,
/// Server with the broadest scoped catalog for this artist. Present only
/// for bounded Top Tracks requests.
pub top_tracks_server_id: Option<String>,
/// Stable hash of the scoped artist tracks used to invalidate Top Songs
/// rankings only when that catalog slice changes.
pub top_tracks_fingerprint: Option<String>,
}
/// `library_search_cross_server` response (§5.5B / §5.9).
@@ -5,6 +5,8 @@
use rusqlite::types::Value as SqlValue;
use rusqlite::{params_from_iter, OptionalExtension};
use serde_json::Value;
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};
use crate::album_compilation_filter::pick_album_group_artist;
use crate::artist_sort::{sort_key_for_display_name, DEFAULT_IGNORED_ARTICLES};
@@ -1886,6 +1888,7 @@ fn fetch_scope_deduped_tracks_for_artist_key(
artist_key: Option<&str>,
anchor_server: &str,
anchor_artist_id: &str,
top_tracks_limit: Option<u32>,
) -> rusqlite::Result<Vec<LibraryTrackDto>> {
let (scope_cte, scope_binds) = scope_cte_sql(scopes);
let (cte, scoped, key_filter, priority) = keyed_detail_track_source(
@@ -1895,6 +1898,11 @@ fn fetch_scope_deduped_tracks_for_artist_key(
);
let cols = aliased_track_columns("t");
let plain_cols = plain_track_columns_sql();
let order_and_limit = if top_tracks_limit.is_some() {
"ORDER BY play_count DESC NULLS LAST, played_at DESC NULLS LAST, title COLLATE NOCASE ASC LIMIT ?"
} else {
"ORDER BY album COLLATE NOCASE ASC, track_number ASC NULLS LAST, title COLLATE NOCASE ASC"
};
let sql = format!(
"{cte}, \
ranked AS ( \
@@ -1902,8 +1910,82 @@ fn fetch_scope_deduped_tracks_for_artist_key(
ROW_NUMBER() OVER (PARTITION BY {TRACK_DEDUP_KEY} ORDER BY {priority} ASC, t.id ASC) AS rn \
{scoped} AND t.artist_id IS NOT NULL {key_filter} \
) \
SELECT {plain_cols} FROM ranked WHERE rn = 1 \
ORDER BY album COLLATE NOCASE ASC, track_number ASC NULLS LAST, title COLLATE NOCASE ASC",
SELECT {plain_cols} FROM ranked WHERE rn = 1 {order_and_limit}",
scoped = scoped,
);
let mut binds = scope_binds;
if let Some(key) = artist_key {
binds.push(SqlValue::Text(key.to_string()));
} else {
binds.push(SqlValue::Text(anchor_server.to_string()));
binds.push(SqlValue::Text(anchor_artist_id.to_string()));
}
if let Some(limit) = top_tracks_limit {
binds.push(SqlValue::Integer(i64::from(limit.clamp(1, 50))));
}
let mut stmt = conn.prepare(&sql)?;
let rows = stmt
.query_map(params_from_iter(binds.iter()), |r| {
Ok(LibraryTrackDto::from_row(&row_to_track_row(r)?))
})?
.collect::<rusqlite::Result<Vec<_>>>()?;
Ok(rows)
}
fn fetch_top_tracks_server_id(
conn: &rusqlite::Connection,
scopes: &[LibraryScopePair],
artist_key: Option<&str>,
anchor_server: &str,
anchor_artist_id: &str,
) -> rusqlite::Result<Option<String>> {
let (scope_cte, scope_binds) = scope_cte_sql(scopes);
let (cte, scoped, key_filter, priority) = keyed_detail_track_source(
scope_cte,
artist_key.map(|_| "artist_key"),
"AND t.server_id = ? AND t.artist_id = ? AND ck.artist_key IS NULL",
);
let sql = format!(
"{cte}, \
server_counts AS ( \
SELECT t.server_id, COUNT(DISTINCT {TRACK_DEDUP_KEY}) AS track_count, \
MIN({priority}) AS best_pr \
{scoped} AND t.artist_id IS NOT NULL {key_filter} \
GROUP BY t.server_id \
) \
SELECT server_id FROM server_counts \
ORDER BY track_count DESC, best_pr ASC, server_id ASC LIMIT 1",
scoped = scoped,
);
let mut binds = scope_binds;
if let Some(key) = artist_key {
binds.push(SqlValue::Text(key.to_string()));
} else {
binds.push(SqlValue::Text(anchor_server.to_string()));
binds.push(SqlValue::Text(anchor_artist_id.to_string()));
}
conn.query_row(&sql, params_from_iter(binds.iter()), |row| row.get(0))
.optional()
}
fn fetch_top_tracks_fingerprint(
conn: &rusqlite::Connection,
scopes: &[LibraryScopePair],
artist_key: Option<&str>,
anchor_server: &str,
anchor_artist_id: &str,
) -> rusqlite::Result<String> {
let (scope_cte, scope_binds) = scope_cte_sql(scopes);
let (cte, scoped, key_filter, _) = keyed_detail_track_source(
scope_cte,
artist_key.map(|_| "artist_key"),
"AND t.server_id = ? AND t.artist_id = ? AND ck.artist_key IS NULL",
);
let sql = format!(
"{cte} \
SELECT t.server_id, t.id, t.library_id, t.title, t.album_id, t.duration_sec \
{scoped} AND t.artist_id IS NOT NULL {key_filter} \
ORDER BY t.server_id ASC, t.id ASC, t.library_id ASC",
scoped = scoped,
);
let mut binds = scope_binds;
@@ -1914,12 +1996,21 @@ fn fetch_scope_deduped_tracks_for_artist_key(
binds.push(SqlValue::Text(anchor_artist_id.to_string()));
}
let mut stmt = conn.prepare(&sql)?;
let rows = stmt
.query_map(params_from_iter(binds.iter()), |r| {
Ok(LibraryTrackDto::from_row(&row_to_track_row(r)?))
})?
.collect::<rusqlite::Result<Vec<_>>>()?;
Ok(rows)
let rows = stmt.query_map(params_from_iter(binds.iter()), |row| {
Ok((
row.get::<_, String>(0)?,
row.get::<_, String>(1)?,
row.get::<_, Option<String>>(2)?,
row.get::<_, String>(3)?,
row.get::<_, Option<String>>(4)?,
row.get::<_, i64>(5)?,
))
})?;
let mut hasher = DefaultHasher::new();
for row in rows {
row?.hash(&mut hasher);
}
Ok(format!("{:016x}", hasher.finish()))
}
/// `library_scope_artist_detail` — resolve anchor → `artist_key`, aggregate albums + tracks.
@@ -1965,14 +2056,40 @@ pub fn artist_detail(
artist_key.as_deref(),
server_id,
artist_id,
request.top_tracks_limit,
)?
} else {
Vec::new()
};
let (top_tracks_server_id, top_tracks_fingerprint) = if request.top_tracks_limit.is_some() {
let source_server_id = fetch_top_tracks_server_id(
conn,
scopes,
artist_key.as_deref(),
server_id,
artist_id,
)?;
let fingerprint = if source_server_id.is_some() {
Some(fetch_top_tracks_fingerprint(
conn,
scopes,
artist_key.as_deref(),
server_id,
artist_id,
)?)
} else {
None
};
(source_server_id, fingerprint)
} else {
(None, None)
};
Ok(LibraryScopeArtistDetailResponse {
artist,
albums,
tracks,
top_tracks_server_id,
top_tracks_fingerprint,
})
})
}
@@ -2078,6 +2195,7 @@ mod tests {
artist_id: "art1".into(),
server_id: "s1".into(),
include_tracks: false,
top_tracks_limit: None,
},
)
.unwrap();
@@ -2093,6 +2211,7 @@ mod tests {
artist_id: "art1".into(),
server_id: "s1".into(),
include_tracks: true,
top_tracks_limit: None,
},
)
.unwrap();
@@ -2100,6 +2219,138 @@ mod tests {
assert_eq!(with_tracks.tracks[0].id, "t1");
}
#[test]
fn artist_detail_bounds_top_tracks_and_selects_broadest_server() {
let store = LibraryStore::open_in_memory();
let mut rows = vec![
track(
"s1",
"s1-low",
"Local Low",
Some("Artist"),
"One",
"s1-alb",
Some("s1-art"),
180,
"lib-a",
None,
None,
None,
),
track(
"s1",
"s1-mid",
"Local Mid",
Some("Artist"),
"One",
"s1-alb",
Some("s1-art"),
181,
"lib-a",
None,
None,
None,
),
track(
"s2",
"s2-top",
"Global Top",
Some("Artist"),
"Two",
"s2-alb",
Some("s2-art"),
182,
"lib-b",
None,
None,
None,
),
track(
"s2",
"s2-second",
"Global Second",
Some("Artist"),
"Two",
"s2-alb",
Some("s2-art"),
183,
"lib-b",
None,
None,
None,
),
track(
"s2",
"s2-low",
"Global Low",
Some("Artist"),
"Two",
"s2-alb",
Some("s2-art"),
184,
"lib-b",
None,
None,
None,
),
];
for (row, play_count) in rows.iter_mut().zip([5, 10, 100, 50, 1]) {
row.play_count = Some(play_count);
}
seed_and_rebuild(&store, &rows);
let response = artist_detail(
&store,
&LibraryScopeArtistDetailRequest {
scopes: vec![scope_pair("s1", "lib-a"), scope_pair("s2", "lib-b")],
artist_id: "s1-art".into(),
server_id: "s1".into(),
include_tracks: true,
top_tracks_limit: Some(2),
},
)
.unwrap();
assert_eq!(response.top_tracks_server_id.as_deref(), Some("s2"));
let fingerprint = response.top_tracks_fingerprint.clone().unwrap();
assert_eq!(response.tracks.len(), 2);
assert_eq!(response.tracks[0].id, "s2-top");
assert_eq!(response.tracks[1].id, "s2-second");
seed_and_rebuild(
&store,
&[track(
"s2",
"s2-new",
"New Track",
Some("Artist"),
"Two",
"s2-alb",
Some("s2-art"),
185,
"lib-b",
None,
None,
None,
)],
);
let updated = artist_detail(
&store,
&LibraryScopeArtistDetailRequest {
scopes: vec![scope_pair("s1", "lib-a"), scope_pair("s2", "lib-b")],
artist_id: "s1-art".into(),
server_id: "s1".into(),
include_tracks: true,
top_tracks_limit: Some(2),
},
)
.unwrap();
assert_ne!(
updated.top_tracks_fingerprint.as_deref(),
Some(fingerprint.as_str())
);
}
#[test]
fn list_artists_collapses_collaboration_track_names_for_one_artist_id() {
let store = LibraryStore::open_in_memory();
@@ -12,6 +12,7 @@ import { topSongAlbumForCover } from '@/features/artist/components/topSongAlbumF
interface Props {
topSongs: SubsonicSong[];
loading?: boolean;
albums: SubsonicAlbum[];
marginTop: string;
playTopSongWithContinuation: (startIndex: number) => Promise<void>;
@@ -19,7 +20,7 @@ interface Props {
}
export default function ArtistDetailTopTracks({
topSongs, albums, marginTop, playTopSongWithContinuation, losslessOnly = false,
topSongs, loading = false, albums, marginTop, playTopSongWithContinuation, losslessOnly = false,
}: Props) {
const { t } = useTranslation();
const currentTrack = usePlayerStore(s => s.currentTrack);
@@ -34,14 +35,34 @@ export default function ArtistDetailTopTracks({
<h2 className="section-title" style={{ marginTop, marginBottom: '1rem' }}>
{t(losslessOnly ? 'artistDetail.topTracksLossless' : 'artistDetail.topTracks')}
</h2>
<div className="tracklist" data-preview-loc="artist" style={{ padding: 0, marginBottom: '2rem' }}>
<div
className="tracklist"
data-preview-loc="artist"
aria-busy={loading}
style={{ padding: 0, marginBottom: '2rem' }}
>
<div className="tracklist-header" style={{ gridTemplateColumns: '60px minmax(150px, 1fr) minmax(100px, 1fr) 65px' }}>
<div style={{ textAlign: 'center' }}>#</div>
<div>{t('artistDetail.trackTitle')}</div>
<div>{t('artistDetail.trackAlbum')}</div>
<div style={{ textAlign: 'right' }}>{t('artistDetail.trackDuration')}</div>
</div>
{topSongs.map((song, idx) => {
{loading && topSongs.length === 0 ? Array.from({ length: 5 }, (_, idx) => (
<div
key={idx}
className="track-row artist-top-track-skeleton"
style={{ gridTemplateColumns: '60px minmax(150px, 1fr) minmax(100px, 1fr) 65px' }}
aria-hidden="true"
>
<div className="artist-top-track-skeleton-rank" />
<div className="artist-top-track-skeleton-title">
<div className="artist-top-track-skeleton-cover" />
<div className="artist-top-track-skeleton-line artist-top-track-skeleton-line--title" />
</div>
<div className="artist-top-track-skeleton-line artist-top-track-skeleton-line--album" />
<div className="artist-top-track-skeleton-line artist-top-track-skeleton-line--duration" />
</div>
)) : topSongs.map((song, idx) => {
const track = songToTrack(song);
return (
<div
@@ -110,7 +131,7 @@ export default function ArtistDetailTopTracks({
</div>
</div>
);
})}
})}
</div>
</Fragment>
);
@@ -5,11 +5,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { useAuthStore } from '@/store/authStore';
const tryLoadArtistDetailMultiScopeMock = vi.fn();
const loadScopedArtistTopSongsMock = vi.fn();
vi.mock('@/lib/library/loadArtistDetailMultiScope', () => ({
tryLoadArtistDetailMultiScope: (...args: unknown[]) => tryLoadArtistDetailMultiScopeMock(...args),
}));
vi.mock('@/lib/library/loadScopedArtistTopSongs', () => ({
loadScopedArtistTopSongs: (...args: unknown[]) => loadScopedArtistTopSongsMock(...args),
}));
vi.mock('@/lib/network/subsonicNetworkGuard', () => ({
shouldAttemptSubsonicForServer: () => true,
}));
vi.mock('@/lib/api/subsonicArtists');
vi.mock('@/lib/api/subsonicSearch');
@@ -23,7 +32,9 @@ vi.mock('@/lib/hooks/useConnectionStatus', () => ({
useConnectionStatus: () => ({ status: 'connected' }),
}));
import { getArtist, getArtistForServer, getArtistInfo, getTopSongs } from '@/lib/api/subsonicArtists';
import {
getArtist, getArtistForServer, getArtistInfo, getTopSongs, getTopSongsForServer,
} from '@/lib/api/subsonicArtists';
import { loadArtistFromLibraryIndex } from '@/features/offline';
import { search } from '@/lib/api/subsonicSearch';
import { useArtistDetailData } from './useArtistDetailData';
@@ -35,7 +46,9 @@ function routerWrapper({ children }: { children: React.ReactNode }) {
describe('useArtistDetailData — multi-library selection', () => {
beforeEach(() => {
tryLoadArtistDetailMultiScopeMock.mockReset();
loadScopedArtistTopSongsMock.mockReset();
vi.mocked(getTopSongs).mockResolvedValue([]);
vi.mocked(getTopSongsForServer).mockResolvedValue([]);
vi.mocked(getArtistInfo).mockResolvedValue({} as Awaited<ReturnType<typeof getArtistInfo>>);
vi.mocked(search).mockResolvedValue({ songs: [], albums: [], artists: [] });
useAuthStore.setState({
@@ -81,6 +94,59 @@ describe('useArtistDetailData — multi-library selection', () => {
expect(result.current.topSongs.map(s => s.id)).toEqual(['trk-high', 'trk-low']);
});
it('renders local detail while one server Top Songs request remains pending', async () => {
let resolveTopSongs!: (songs: Array<{ id: string; title: string }>) => void;
loadScopedArtistTopSongsMock.mockImplementation(() => new Promise(resolve => {
resolveTopSongs = resolve;
}));
tryLoadArtistDetailMultiScopeMock.mockResolvedValue({
artist: { id: 'art-1', name: 'Merged' },
albums: [{ id: 'alb-1', name: 'Album' }],
topSongs: [{ id: 'fallback', title: 'Fallback' }],
topTracksServerId: 'srv-2',
topTracksFingerprint: 'tracks-v1',
});
const { result } = renderHook(() => useArtistDetailData('art-1'), { wrapper: routerWrapper });
await waitFor(() => expect(result.current.loading).toBe(false));
expect(result.current.artist?.name).toBe('Merged');
expect(result.current.topSongsLoading).toBe(true);
expect(result.current.topSongs.map(song => song.id)).toEqual(['fallback']);
expect(loadScopedArtistTopSongsMock).toHaveBeenCalledWith({
artistName: 'Merged',
sourceServerId: 'srv-2',
scopes: [
{ serverId: 'srv-1', libraryId: 'lib-a' },
{ serverId: 'srv-2', libraryId: 'lib-b' },
],
localFallback: [{ id: 'fallback', title: 'Fallback' }],
tracksFingerprint: 'tracks-v1',
});
resolveTopSongs([{ id: 'global', title: 'Global' }]);
await waitFor(() => expect(result.current.topSongsLoading).toBe(false));
expect(result.current.topSongs.map(song => song.id)).toEqual(['global']);
});
it('keeps local Top Tracks when the optional ranking request fails', async () => {
loadScopedArtistTopSongsMock.mockRejectedValue(new TypeError('invalid song metadata'));
tryLoadArtistDetailMultiScopeMock.mockResolvedValue({
artist: { id: 'art-1', name: 'Merged' },
albums: [{ id: 'alb-1', name: 'Album' }],
topSongs: [{ id: 'fallback', title: 'Fallback' }],
topTracksServerId: 'srv-2',
topTracksFingerprint: 'tracks-v1',
});
const { result } = renderHook(() => useArtistDetailData('art-1'), { wrapper: routerWrapper });
await waitFor(() => expect(loadScopedArtistTopSongsMock).toHaveBeenCalledOnce());
await waitFor(() => expect(result.current.topSongsLoading).toBe(false));
expect(result.current.loading).toBe(false);
expect(result.current.topSongs.map(song => song.id)).toEqual(['fallback']);
});
it('loads via the authoritative scope when one folder is selected', async () => {
useAuthStore.setState({
libraryBrowseServerIds: ['srv-1'],
@@ -1,7 +1,9 @@
import { useEffect, useState } from 'react';
import { useSearchParams } from 'react-router-dom';
import { search } from '@/lib/api/subsonicSearch';
import { getArtist, getArtistForServer, getArtistInfo, getTopSongs } from '@/lib/api/subsonicArtists';
import {
getArtist, getArtistForServer, getArtistInfo, getTopSongs, getTopSongsForServer,
} from '@/lib/api/subsonicArtists';
import type {
SubsonicAlbum, SubsonicArtist, SubsonicArtistInfo, SubsonicSong,
} from '@/lib/api/subsonicTypes';
@@ -15,6 +17,8 @@ import { runLocalArtistLosslessBrowse } from '@/lib/library/browseTextSearch';
import { isLosslessSuffix } from '@/lib/library/losslessFormats';
import { tryLoadArtistDetailMultiScope } from '@/lib/library/loadArtistDetailMultiScope';
import { getLibraryBrowseScope } from '@/lib/library/libraryBrowseScope';
import { loadScopedArtistTopSongs } from '@/lib/library/loadScopedArtistTopSongs';
import { shouldAttemptSubsonicForServer } from '@/lib/network/subsonicNetworkGuard';
export interface UseArtistDetailDataOptions {
/** When true, albums and top tracks are limited to lossless containers (local index preferred). */
@@ -29,6 +33,7 @@ export interface ArtistDetailDataResult {
info: SubsonicArtistInfo | null;
featuredAlbums: SubsonicAlbum[];
loading: boolean;
topSongsLoading: boolean;
artistInfoLoading: boolean;
featuredLoading: boolean;
isStarred: boolean;
@@ -74,6 +79,7 @@ export function useArtistDetailData(
const [topSongs, setTopSongs] = useState<SubsonicSong[]>([]);
const [infoEntry, setInfoEntry] = useState<{ id: string; value: SubsonicArtistInfo | null } | null>(null);
const [loading, setLoading] = useState(true);
const [topSongsLoading, setTopSongsLoading] = useState(false);
const [isStarred, setIsStarred] = useState(false);
const [artistInfoLoading, setArtistInfoLoading] = useState(false);
const [featuredLoading, setFeaturedLoading] = useState(false);
@@ -86,6 +92,7 @@ export function useArtistDetailData(
setLoading(true);
setInfoEntry(null);
setTopSongs([]);
setTopSongsLoading(false);
setFeaturedAlbums([]);
(async () => {
@@ -96,7 +103,9 @@ export function useArtistDetailData(
return;
}
if (serverId && currentBrowseScope.pairs.length > 0) {
const multi = await tryLoadArtistDetailMultiScope(currentBrowseScope.pairs, serverId, id);
const multi = losslessOnly
? await tryLoadArtistDetailMultiScope(currentBrowseScope.pairs, serverId, id, null)
: await tryLoadArtistDetailMultiScope(currentBrowseScope.pairs, serverId, id);
if (cancelled) return;
if (multi) {
setArtist(multi.artist);
@@ -104,6 +113,27 @@ export function useArtistDetailData(
setAlbums(multi.albums);
setTopSongs(multi.topSongs);
setLoading(false);
if (
!losslessOnly
&& multi.topTracksServerId
&& multi.topTracksFingerprint
&& shouldAttemptSubsonicForServer(multi.topTracksServerId)
) {
setTopSongsLoading(true);
try {
const ranked = await loadScopedArtistTopSongs({
artistName: multi.artist.name,
sourceServerId: multi.topTracksServerId,
scopes: currentBrowseScope.pairs,
localFallback: multi.topSongs,
tracksFingerprint: multi.topTracksFingerprint,
}).catch(() => multi.topSongs);
if (cancelled) return;
setTopSongs(ranked);
} finally {
if (!cancelled) setTopSongsLoading(false);
}
}
return;
}
setLoading(false);
@@ -156,7 +186,13 @@ export function useArtistDetailData(
setIsStarred(!!artistData.artist.starred);
setLoading(false);
const songsData = await getTopSongs(artistData.artist.name).catch(() => [] as SubsonicSong[]);
const canLoadTopSongs = !serverId || shouldAttemptSubsonicForServer(serverId);
if (!canLoadTopSongs) return;
setTopSongsLoading(true);
const songsData = await (serverId
? getTopSongsForServer(serverId, artistData.artist.name)
: getTopSongs(artistData.artist.name)
).catch(() => [] as SubsonicSong[]);
if (cancelled) return;
let nextSongs = songsData ?? [];
if (losslessOnly) {
@@ -164,6 +200,7 @@ export function useArtistDetailData(
}
setAlbums(nextAlbums);
setTopSongs(nextSongs);
setTopSongsLoading(false);
} catch (err) {
if (cancelled) return;
// Network `getArtist` can fail for an id that is a valid card link but
@@ -189,6 +226,7 @@ export function useArtistDetailData(
} catch { /* ignore */ }
}
console.error(err);
setTopSongsLoading(false);
setLoading(false);
}
})();
@@ -273,7 +311,7 @@ export function useArtistDetailData(
return {
artist, setArtist, albums, topSongs, info, featuredAlbums,
loading, artistInfoLoading, featuredLoading,
loading, topSongsLoading, artistInfoLoading, featuredLoading,
isStarred, setIsStarred,
losslessOnly,
};
+3 -2
View File
@@ -50,7 +50,7 @@ export default function ArtistDetail() {
const losslessOnly = searchParams.get('lossless') === '1';
const {
artist, setArtist, albums, topSongs, info, featuredAlbums,
loading, artistInfoLoading, featuredLoading,
loading, topSongsLoading, artistInfoLoading, featuredLoading,
isStarred, setIsStarred,
} = useArtistDetailData(id, { losslessOnly });
const [radioLoading, setRadioLoading] = useState(false);
@@ -247,7 +247,7 @@ export default function ArtistDetail() {
const sectionHasData = (id: ArtistSectionId): boolean => {
switch (id) {
case 'bio': return !!info?.biography;
case 'topTracks': return topSongs.length > 0;
case 'topTracks': return topSongsLoading || topSongs.length > 0;
case 'similar': return showSimilarSection;
case 'albums': return true; // always renders (empty state included)
case 'featured': return featuredLoading || featuredAlbums.length > 0;
@@ -315,6 +315,7 @@ export default function ArtistDetail() {
<ArtistDetailTopTracks
key="topTracks"
topSongs={topSongs}
loading={topSongsLoading}
albums={albums}
marginTop={sectionMt('topTracks')}
playTopSongWithContinuation={playTopSongWithContinuation}
+7 -1
View File
@@ -252,12 +252,15 @@ describe('libraryScopeArtistDetail', () => {
},
albums: [],
tracks: [],
topTracksServerId: 's2.example',
topTracksFingerprint: 'tracks-v1',
};
});
await libraryScopeArtistDetail('profile-s1', {
const response = await libraryScopeArtistDetail('profile-s1', {
scopes,
artistId: 'ar-1',
serverId: 'profile-s1',
topTracksLimit: 5,
});
expect(captured).toEqual({
request: {
@@ -267,7 +270,10 @@ describe('libraryScopeArtistDetail', () => {
],
artistId: 'ar-1',
serverId: 's1.example',
topTracksLimit: 5,
},
});
expect(response.topTracksServerId).toBe('profile-s2');
expect(response.topTracksFingerprint).toBe('tracks-v1');
});
});
+8
View File
@@ -94,6 +94,8 @@ export interface LibraryScopeArtistDetailRequest {
serverId: string;
/** Skip tracks when the caller needs only artist metadata and discography. */
includeTracks?: boolean;
/** Return a bounded personal-play-count fallback for the Top Tracks section. */
topTracksLimit?: number;
}
export interface LibraryScopeAlbumDetailResponse {
@@ -105,6 +107,8 @@ export interface LibraryScopeArtistDetailResponse {
artist: LibraryArtistDto;
albums: LibraryAlbumDto[];
tracks: LibraryTrackDto[];
topTracksServerId?: string | null;
topTracksFingerprint?: string | null;
}
function mapScopePairServerId(pair: LibraryScopePair, profileServerId: string): LibraryScopePair {
@@ -298,5 +302,9 @@ export function libraryScopeArtistDetail(
},
albums: mapAlbumsServerId(response.albums, serverId),
tracks: mapTracksServerId(response.tracks, serverId),
topTracksServerId: response.topTracksServerId
? mapServerIdFromIndexKey(response.topTracksServerId, serverId)
: null,
topTracksFingerprint: response.topTracksFingerprint ?? null,
}));
}
@@ -23,6 +23,7 @@ import {
getArtistForServer,
getArtistInfoForServer,
getArtistsForServer,
getTopSongsForServer,
} from '@/lib/api/subsonicArtists';
const artist = { id: 'artist-1', name: 'Artist' };
@@ -81,4 +82,34 @@ describe('explicit-server artist wrappers', () => {
5678,
);
});
it('loads a bounded Top Songs candidate set for one explicit server', async () => {
apiForServerMock.mockResolvedValue({
topSongs: {
song: [
{ id: 'top-1', title: 'First' },
{ id: 'top-2', title: 'Second' },
],
},
});
const songs = await getTopSongsForServer('srv-top', 'Artist', {
requestCount: 20,
limit: 20,
timeout: 4321,
libraryIds: ['lib-a'],
filterToLibrary: false,
});
expect(apiForServerMock).toHaveBeenCalledWith(
'srv-top',
'getTopSongs.view',
{ artist: 'Artist', count: 20, musicFolderId: ['lib-a'] },
4321,
);
expect(songs).toEqual([
{ id: 'top-1', title: 'First', serverId: 'srv-top' },
{ id: 'top-2', title: 'Second', serverId: 'srv-top' },
]);
});
});
+30 -7
View File
@@ -159,19 +159,42 @@ export async function getTopSongs(artist: string): Promise<SubsonicSong[]> {
return getTopSongsForServer(activeServerId, artist);
}
export async function getTopSongsForServer(serverId: string, artist: string): Promise<SubsonicSong[]> {
export interface GetTopSongsForServerOptions {
/** Number requested from the server before local scope validation. */
requestCount?: number;
/** Maximum returned to the caller. */
limit?: number;
timeout?: number;
/** Explicit browse folders; useful when the Library page scope differs from the legacy filter. */
libraryIds?: string[];
/** Disable the legacy single-folder filter when the caller validates against the local index. */
filterToLibrary?: boolean;
}
export async function getTopSongsForServer(
serverId: string,
artist: string,
options: GetTopSongsForServerOptions = {},
): Promise<SubsonicSong[]> {
try {
const { musicLibraryFilterByServer } = useAuthStore.getState();
const scoped = musicLibraryFilterByServer[serverId] && musicLibraryFilterByServer[serverId] !== 'all';
const topCount = scoped ? 20 : 5;
const libraryIds = options.libraryIds ?? librarySelectionForServer(serverId);
const scoped = libraryIds.length > 0;
const limit = options.limit ?? 5;
const requestCount = Math.max(limit, options.requestCount ?? (scoped ? 20 : 5));
const libraryParams = options.libraryIds
? (libraryIds.length > 0 ? { musicFolderId: libraryIds } : {})
: libraryFilterParamsForServer(serverId);
const data = await apiForServer<{ topSongs: { song: SubsonicSong[] } }>(
serverId,
'getTopSongs.view',
{ artist, count: topCount, ...libraryFilterParamsForServer(serverId) },
{ artist, count: requestCount, ...libraryParams },
options.timeout,
);
const raw = data.topSongs?.song ?? [];
const filtered = await filterSongsToServerLibrary(raw, serverId);
return filtered.slice(0, 5);
const filtered = options.filterToLibrary === false
? raw
: await filterSongsToServerLibrary(raw, serverId);
return filtered.slice(0, limit).map(song => ({ ...song, serverId }));
} catch {
return [];
}
+2 -2
View File
@@ -85,8 +85,8 @@ export interface SubsonicSong {
albumArtist?: string;
/** OpenSubsonic: single-string album-artist for display (mirrors `albumArtists` joined). */
displayAlbumArtist?: string;
/** ISRC code when available (e.g., Navidrome) */
isrc?: string;
/** ISRC code; OpenSubsonic/Navidrome may return a string array. */
isrc?: string | string[];
/** Times the track has been played, surfaced by Navidrome's Subsonic API. */
playCount?: number;
/** ISO datetime of the last play, surfaced by Navidrome (OpenSubsonic). */
@@ -65,6 +65,8 @@ describe('tryLoadArtistDetailMultiScope', () => {
trackDto({ id: 'low', playCount: 1 }),
trackDto({ id: 'high', playCount: 99 }),
],
topTracksServerId: 'srv-2',
topTracksFingerprint: 'tracks-v1',
});
const scopes = [
@@ -80,10 +82,13 @@ describe('tryLoadArtistDetailMultiScope', () => {
],
artistId: 'art-1',
serverId: 'srv-1',
topTracksLimit: 5,
});
expect(result?.artist).toMatchObject({ id: 'art-1', name: 'Merged Artist' });
expect(result?.albums).toHaveLength(1);
expect(result?.topSongs.map(s => s.id)).toEqual(['high', 'low']);
expect(result?.topTracksServerId).toBe('srv-2');
expect(result?.topTracksFingerprint).toBe('tracks-v1');
});
it('returns null when the merged artist anchor is missing', async () => {
@@ -9,6 +9,8 @@ export interface ArtistDetailMultiScopePayload {
artist: SubsonicArtist;
albums: SubsonicAlbum[];
topSongs: SubsonicSong[];
topTracksServerId: string | null;
topTracksFingerprint: string | null;
}
/**
@@ -19,12 +21,14 @@ export async function tryLoadArtistDetailMultiScope(
scopes: LibraryScopePair[],
serverId: string,
artistId: string,
topTracksLimit: number | null = 5,
): Promise<ArtistDetailMultiScopePayload | null> {
try {
const response = await libraryScopeArtistDetail(serverId, {
scopes,
artistId,
serverId,
...(topTracksLimit == null ? {} : { topTracksLimit }),
});
if (!response.artist?.id) return null;
return {
@@ -33,6 +37,8 @@ export async function tryLoadArtistDetailMultiScope(
topSongs: [...response.tracks.map(trackToSong)].sort(
(a, b) => (b.playCount ?? 0) - (a.playCount ?? 0),
),
topTracksServerId: response.topTracksServerId ?? null,
topTracksFingerprint: response.topTracksFingerprint ?? null,
};
} catch {
return null;
@@ -0,0 +1,200 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type { LibraryTrackDto } from '@/lib/api/library/dto';
import type { SubsonicSong } from '@/lib/api/subsonicTypes';
const { getTopSongsForServerMock, libraryGetTracksBatchMock } = vi.hoisted(() => ({
getTopSongsForServerMock: vi.fn(),
libraryGetTracksBatchMock: vi.fn(),
}));
vi.mock('@/lib/api/subsonicArtists', () => ({
getTopSongsForServer: (...args: unknown[]) => getTopSongsForServerMock(...args),
}));
vi.mock('@/lib/api/library/reads', () => ({
libraryGetTracksBatch: (...args: unknown[]) => libraryGetTracksBatchMock(...args),
}));
import { loadScopedArtistTopSongs } from './loadScopedArtistTopSongs';
function song(id: string, title: string, album = 'Album'): SubsonicSong {
return { id, title, album, albumId: `${album}-id`, artist: 'Artist', duration: 180 };
}
function indexedTrack(id: string, libraryId: string): LibraryTrackDto {
return {
serverId: 'srv-2', id, title: id, album: 'Album', durationSec: 180,
libraryId, syncedAt: 0, rawJson: {},
};
}
describe('loadScopedArtistTopSongs', () => {
beforeEach(() => {
getTopSongsForServerMock.mockReset();
libraryGetTracksBatchMock.mockReset();
});
afterEach(() => {
vi.useRealTimers();
});
it('queries only the broadest server, preserves global rank, and filters to scope', async () => {
getTopSongsForServerMock.mockResolvedValue([
song('global-2', 'Second'),
song('outside', 'Outside'),
song('global-1', 'First'),
]);
libraryGetTracksBatchMock.mockResolvedValue([
indexedTrack('global-2', 'lib-b'),
indexedTrack('outside', 'lib-x'),
indexedTrack('global-1', 'lib-b'),
]);
const result = await loadScopedArtistTopSongs({
artistName: 'Unique Artist One',
sourceServerId: 'srv-2',
scopes: [
{ serverId: 'srv-1', libraryId: 'lib-a' },
{ serverId: 'srv-2', libraryId: 'lib-b' },
],
localFallback: [
song('fallback-duplicate', 'First'),
song('fallback-local', 'Local Favourite'),
],
tracksFingerprint: 'tracks-1',
});
expect(getTopSongsForServerMock).toHaveBeenCalledOnce();
expect(getTopSongsForServerMock).toHaveBeenCalledWith('srv-2', 'Unique Artist One', {
requestCount: 20,
limit: 20,
timeout: 5000,
libraryIds: ['lib-b'],
filterToLibrary: false,
});
expect(result.map(track => track.id)).toEqual(['global-2', 'global-1', 'fallback-local']);
expect(result[0]?.serverId).toBe('srv-2');
});
it('uses the bounded local fallback when the chosen server fails', async () => {
getTopSongsForServerMock.mockRejectedValue(new Error('offline'));
const fallback = Array.from({ length: 7 }, (_, index) => song(`local-${index}`, `Local ${index}`));
const result = await loadScopedArtistTopSongs({
artistName: 'Unique Artist Two',
sourceServerId: 'srv-2',
scopes: [{ serverId: 'srv-2', libraryId: 'lib-b' }],
localFallback: fallback,
tracksFingerprint: 'tracks-2',
});
expect(result.map(track => track.id)).toEqual([
'local-0', 'local-1', 'local-2', 'local-3', 'local-4',
]);
expect(libraryGetTracksBatchMock).not.toHaveBeenCalled();
});
it('coalesces concurrent requests for the same artist scope', async () => {
let resolveNetwork!: (songs: SubsonicSong[]) => void;
getTopSongsForServerMock.mockReturnValue(new Promise(resolve => {
resolveNetwork = resolve;
}));
libraryGetTracksBatchMock.mockResolvedValue([indexedTrack('global', 'lib-b')]);
const options = {
artistName: 'Unique Artist Three',
sourceServerId: 'srv-2',
scopes: [{ serverId: 'srv-2', libraryId: 'lib-b' }],
localFallback: [song('local', 'Local')],
tracksFingerprint: 'tracks-3',
};
const first = loadScopedArtistTopSongs(options);
const second = loadScopedArtistTopSongs(options);
expect(getTopSongsForServerMock).toHaveBeenCalledOnce();
resolveNetwork([song('global', 'Global')]);
await expect(first).resolves.toEqual([
expect.objectContaining({ id: 'global' }),
expect.objectContaining({ id: 'local' }),
]);
await expect(second).resolves.toEqual([
expect.objectContaining({ id: 'global' }),
expect.objectContaining({ id: 'local' }),
]);
});
it('falls back when scoped batch validation remains pending', async () => {
vi.useFakeTimers();
getTopSongsForServerMock.mockResolvedValue([song('global-pending', 'Global Pending')]);
libraryGetTracksBatchMock.mockReturnValue(new Promise(() => {}));
const request = loadScopedArtistTopSongs({
artistName: 'Unique Artist Pending Batch',
sourceServerId: 'srv-2',
scopes: [{ serverId: 'srv-2', libraryId: 'lib-b' }],
localFallback: [song('local-pending', 'Local Pending')],
tracksFingerprint: 'tracks-pending',
});
await vi.advanceTimersByTimeAsync(2000);
await expect(request).resolves.toEqual([
expect.objectContaining({ id: 'local-pending' }),
]);
});
it('caches successful rankings until the scoped track fingerprint changes', async () => {
getTopSongsForServerMock
.mockResolvedValueOnce([song('global-v1', 'Global V1')])
.mockResolvedValueOnce([song('global-v2', 'Global V2')]);
libraryGetTracksBatchMock
.mockResolvedValueOnce([indexedTrack('global-v1', 'lib-b')])
.mockResolvedValueOnce([indexedTrack('global-v2', 'lib-b')]);
const options = {
artistName: 'Unique Artist Fingerprint Cache',
sourceServerId: 'srv-2',
scopes: [{ serverId: 'srv-2', libraryId: 'lib-b' }],
localFallback: [song('local', 'Local')],
tracksFingerprint: 'tracks-v1',
};
await expect(loadScopedArtistTopSongs(options)).resolves.toEqual([
expect.objectContaining({ id: 'global-v1' }),
expect.objectContaining({ id: 'local' }),
]);
await expect(loadScopedArtistTopSongs({
...options,
localFallback: [song('local-new', 'Local New')],
})).resolves.toEqual([
expect.objectContaining({ id: 'global-v1' }),
expect.objectContaining({ id: 'local-new' }),
]);
expect(getTopSongsForServerMock).toHaveBeenCalledTimes(1);
await expect(loadScopedArtistTopSongs({
...options,
tracksFingerprint: 'tracks-v2',
})).resolves.toEqual([
expect.objectContaining({ id: 'global-v2' }),
expect.objectContaining({ id: 'local' }),
]);
expect(getTopSongsForServerMock).toHaveBeenCalledTimes(2);
});
it('accepts OpenSubsonic isrc arrays when merging Top Songs', async () => {
getTopSongsForServerMock.mockResolvedValue([
{ ...song('global-isrc', 'Global ISRC'), isrc: ['USRC17607839'] },
]);
libraryGetTracksBatchMock.mockResolvedValue([indexedTrack('global-isrc', 'lib-b')]);
await expect(loadScopedArtistTopSongs({
artistName: 'Unique Artist ISRC Array',
sourceServerId: 'srv-2',
scopes: [{ serverId: 'srv-2', libraryId: 'lib-b' }],
localFallback: [],
tracksFingerprint: 'tracks-isrc-array',
})).resolves.toEqual([
expect.objectContaining({ id: 'global-isrc' }),
]);
});
});
+135
View File
@@ -0,0 +1,135 @@
import { getTopSongsForServer } from '@/lib/api/subsonicArtists';
import { libraryGetTracksBatch } from '@/lib/api/library/reads';
import type { LibraryScopePair } from '@/lib/api/library/scopeReads';
import type { SubsonicSong } from '@/lib/api/subsonicTypes';
const DISPLAY_LIMIT = 5;
const REQUEST_COUNT = 20;
const NETWORK_TIMEOUT_MS = 5000;
const BATCH_VALIDATION_TIMEOUT_MS = 2000;
interface CachedTopSongs {
tracksFingerprint: string;
songs: SubsonicSong[];
}
export interface LoadScopedArtistTopSongsOptions {
artistName: string;
sourceServerId: string;
scopes: LibraryScopePair[];
localFallback: SubsonicSong[];
tracksFingerprint: string;
}
const topSongsCache = new Map<string, CachedTopSongs>();
const topSongsInFlight = new Map<string, Promise<SubsonicSong[]>>();
function scopeKey(options: LoadScopedArtistTopSongsOptions): string {
const libraries = options.scopes
.filter(scope => scope.serverId === options.sourceServerId)
.map(scope => scope.libraryId)
.join(',');
return `${options.sourceServerId}\u0000${libraries}\u0000${options.artistName.trim().toLocaleLowerCase()}`;
}
function requestKey(options: LoadScopedArtistTopSongsOptions): string {
return `${scopeKey(options)}\u0000${options.tracksFingerprint}`;
}
function songIdentity(song: SubsonicSong): string {
const rawIsrc: unknown = song.isrc;
const isrcValue = Array.isArray(rawIsrc)
? rawIsrc.find((value): value is string => typeof value === 'string' && value.trim().length > 0)
: typeof rawIsrc === 'string' ? rawIsrc : undefined;
const isrc = isrcValue?.trim().toLocaleLowerCase();
if (isrc) return `isrc:${isrc}`;
const normalize = (value: string | undefined) => value?.trim().toLocaleLowerCase() ?? '';
return [normalize(song.title), normalize(song.album), Math.round((song.duration ?? 0) / 5)].join('\u0000');
}
function mergeWithFallback(networkSongs: SubsonicSong[], localFallback: SubsonicSong[]): SubsonicSong[] {
const merged: SubsonicSong[] = [];
const seen = new Set<string>();
for (const song of [...networkSongs, ...localFallback]) {
const identity = songIdentity(song);
if (seen.has(identity)) continue;
seen.add(identity);
merged.push(song);
if (merged.length === DISPLAY_LIMIT) break;
}
return merged;
}
async function fetchScopedTopSongs(
options: LoadScopedArtistTopSongsOptions,
): Promise<SubsonicSong[]> {
const libraryIds = options.scopes
.filter(scope => scope.serverId === options.sourceServerId)
.map(scope => scope.libraryId);
if (libraryIds.length === 0) return [];
const candidates = await getTopSongsForServer(options.sourceServerId, options.artistName, {
requestCount: REQUEST_COUNT,
limit: REQUEST_COUNT,
timeout: NETWORK_TIMEOUT_MS,
libraryIds,
filterToLibrary: false,
});
if (candidates.length === 0) return [];
let timeoutId: ReturnType<typeof setTimeout> | undefined;
let indexed: Awaited<ReturnType<typeof libraryGetTracksBatch>>;
try {
indexed = await Promise.race([
libraryGetTracksBatch(candidates.map(song => ({
serverId: options.sourceServerId,
trackId: song.id,
}))),
new Promise<Awaited<ReturnType<typeof libraryGetTracksBatch>>>(resolve => {
timeoutId = setTimeout(() => resolve([]), BATCH_VALIDATION_TIMEOUT_MS);
}),
]);
} finally {
if (timeoutId !== undefined) clearTimeout(timeoutId);
}
const allowedLibraries = new Set(libraryIds);
const allowedTrackIds = new Set(
indexed
.filter(track => !!track.libraryId && allowedLibraries.has(track.libraryId))
.map(track => track.id),
);
return candidates
.filter(song => allowedTrackIds.has(song.id))
.map(song => ({ ...song, serverId: options.sourceServerId }));
}
/**
* Load one server's global Top Songs ranking without blocking artist metadata.
* The local index validates browse scope and supplies a deterministic fallback.
*/
export async function loadScopedArtistTopSongs(
options: LoadScopedArtistTopSongsOptions,
): Promise<SubsonicSong[]> {
const key = scopeKey(options);
const cached = topSongsCache.get(key);
if (cached?.tracksFingerprint === options.tracksFingerprint) {
return mergeWithFallback(cached.songs, options.localFallback);
}
topSongsCache.delete(key);
const inFlightKey = requestKey(options);
let request = topSongsInFlight.get(inFlightKey);
if (!request) {
request = fetchScopedTopSongs(options)
.catch(() => [])
.then(songs => {
if (songs.length > 0) {
topSongsCache.set(key, { tracksFingerprint: options.tracksFingerprint, songs });
}
return songs;
})
.finally(() => topSongsInFlight.delete(inFlightKey));
topSongsInFlight.set(inFlightKey, request);
}
return mergeWithFallback(await request, options.localFallback);
}
+60 -1
View File
@@ -42,6 +42,66 @@
overflow: hidden;
}
.artist-top-track-skeleton {
pointer-events: none;
}
.artist-top-track-skeleton-rank,
.artist-top-track-skeleton-cover,
.artist-top-track-skeleton-line {
background: linear-gradient(
90deg,
var(--card-placeholder-bg) 0%,
color-mix(in srgb, var(--card-placeholder-bg) 70%, var(--text-muted)) 50%,
var(--card-placeholder-bg) 100%
);
background-size: 200% 100%;
animation: artist-top-track-skeleton-shimmer 1.4s ease-in-out infinite;
}
.artist-top-track-skeleton-rank {
width: 0.75rem;
height: 0.65rem;
margin: auto;
border-radius: var(--radius-sm);
}
.artist-top-track-skeleton-title {
display: flex;
align-items: center;
gap: 0.75rem;
min-width: 0;
}
.artist-top-track-skeleton-cover {
width: 32px;
height: 32px;
flex: 0 0 32px;
border-radius: var(--radius-sm);
}
.artist-top-track-skeleton-line {
height: 0.65rem;
border-radius: var(--radius-sm);
}
.artist-top-track-skeleton-line--title { width: min(70%, 20rem); }
.artist-top-track-skeleton-line--album { width: min(65%, 14rem); }
.artist-top-track-skeleton-line--duration { width: 2.5rem; margin-left: auto; }
@keyframes artist-top-track-skeleton-shimmer {
0% { background-position: 100% 0; }
100% { background-position: -100% 0; }
}
@media (prefers-reduced-motion: reduce) {
.artist-top-track-skeleton-rank,
.artist-top-track-skeleton-cover,
.artist-top-track-skeleton-line {
animation: none;
}
}
.tracklist-total {
display: grid;
align-items: center;
@@ -757,4 +817,3 @@ html[data-track-previews-randommix="off"] [data-preview-loc="randomMix"] .pl
background: color-mix(in srgb, var(--accent) 20%, transparent);
color: var(--accent);
}