mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 15:25:46 +00:00
fix(artists): deduplicate albums across servers
This commit is contained in:
@@ -3683,6 +3683,23 @@ mod tests {
|
||||
|
||||
fn seed_and_rebuild(store: &LibraryStore, rows: &[TrackRow]) {
|
||||
TrackRepository::new(store).upsert_batch(rows).unwrap();
|
||||
store
|
||||
.with_conn_mut("test.seed_scoped_artists", |conn| {
|
||||
for row in rows {
|
||||
let (Some(artist_id), Some(artist)) =
|
||||
(row.artist_id.as_deref(), row.artist.as_deref())
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
conn.execute(
|
||||
"INSERT INTO artist (server_id, id, name, synced_at) VALUES (?1, ?2, ?3, 1) \
|
||||
ON CONFLICT(server_id, id) DO NOTHING",
|
||||
rusqlite::params![&row.server_id, artist_id, artist],
|
||||
)?;
|
||||
}
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
crate::identity::rebuild_cluster_keys(store, None).unwrap();
|
||||
}
|
||||
|
||||
|
||||
@@ -17,6 +17,10 @@ fn album_identity_source<'a>(album_artist: Option<&'a str>, artist: Option<&'a s
|
||||
.or_else(|| artist.map(str::trim).filter(|s| !s.is_empty()))
|
||||
}
|
||||
|
||||
pub(super) fn build_album_key(artist: Option<&str>, album: &str) -> Option<String> {
|
||||
join_norm_parts([norm_part(artist.unwrap_or("")), norm_part(album)])
|
||||
}
|
||||
|
||||
pub fn build_track_cluster_keys(
|
||||
artist: Option<&str>,
|
||||
title: &str,
|
||||
@@ -30,10 +34,7 @@ pub fn build_track_cluster_keys(
|
||||
let cluster_key = join_norm_parts([artist_norm.clone(), title_norm, album_norm.clone()]);
|
||||
|
||||
let album_source = album_identity_source(album_artist, artist);
|
||||
let album_key = join_norm_parts([
|
||||
norm_part(album_source.unwrap_or("")),
|
||||
album_norm,
|
||||
]);
|
||||
let album_key = build_album_key(album_source, album);
|
||||
|
||||
TrackClusterKeys {
|
||||
cluster_key,
|
||||
|
||||
@@ -22,7 +22,8 @@ pub(crate) const KEY_SEP: char = '\u{001f}';
|
||||
/// Bump when cluster-key derivation changes; stored in `cluster.cluster_meta.norm_version`.
|
||||
/// v2: locale-aware folding (ß→ss, æ→ae, œ→oe, Romanian ș/ț, Cyrillic ё/й).
|
||||
/// v3: artist keys use the canonical artist entity name when the track has an artist id.
|
||||
pub const NORM_VERSION: &str = "3";
|
||||
/// v4: physical albums use one canonical or server-qualified album key.
|
||||
pub const NORM_VERSION: &str = "4";
|
||||
|
||||
/// Normalize one identity field. Returns `None` when input is empty/whitespace-only
|
||||
/// or when normalization strips everything (punctuation-only, etc.).
|
||||
|
||||
@@ -7,7 +7,7 @@ use rusqlite::{params, Connection, OptionalExtension};
|
||||
use crate::store::LibraryStore;
|
||||
|
||||
use super::attach::CLUSTER_SCHEMA;
|
||||
use super::keys::build_track_cluster_keys;
|
||||
use super::keys::{build_album_key, build_track_cluster_keys};
|
||||
use super::norm::{norm_part, NORM_VERSION};
|
||||
|
||||
const UPSERT_CLUSTER_KEY_SQL: &str = "
|
||||
@@ -71,26 +71,64 @@ type SourceTrackRow = (
|
||||
String,
|
||||
Option<String>,
|
||||
String,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
i64,
|
||||
);
|
||||
|
||||
fn concrete_physical_album_key(server_id: &str, album_id: &str) -> String {
|
||||
format!("physical:{}:{server_id}:{album_id}", server_id.len())
|
||||
}
|
||||
|
||||
/// Rebuild identity keys for one server or all servers. Returns rows upserted.
|
||||
pub fn rebuild_cluster_keys(
|
||||
store: &LibraryStore,
|
||||
server_id: Option<&str>,
|
||||
) -> Result<u64, String> {
|
||||
store.with_conn_mut("identity.rebuild_cluster_keys", |conn| {
|
||||
// `norm_version` is global. A stale per-server request must rebuild every
|
||||
// server before stamping the new version, otherwise untouched keys would
|
||||
// be stranded under the new global marker.
|
||||
let server_id = if server_id.is_some() && cluster_rebuild_needed(conn)? {
|
||||
None
|
||||
} else {
|
||||
server_id
|
||||
};
|
||||
let tx = conn.transaction()?;
|
||||
let mut select = String::from(
|
||||
"SELECT t.server_id, COALESCE(t.library_id, ''), t.id, t.artist, ar.name, t.title, \
|
||||
t.album_artist, t.album, t.duration_sec \
|
||||
let album_server_filter = if server_id.is_some() {
|
||||
" AND source.server_id = ?1"
|
||||
} else {
|
||||
""
|
||||
};
|
||||
let track_server_filter = if server_id.is_some() {
|
||||
" AND t.server_id = ?1"
|
||||
} else {
|
||||
""
|
||||
};
|
||||
let select = format!(
|
||||
"WITH physical_album AS MATERIALIZED ( \
|
||||
SELECT source.server_id, source.album_id, \
|
||||
CASE WHEN COUNT(*) = COUNT(ar_source.id) \
|
||||
AND COUNT(DISTINCT source.artist_id) = 1 \
|
||||
THEN MAX(ar_source.name) END AS canonical_album_artist, \
|
||||
MAX(source.album) AS canonical_album \
|
||||
FROM track source \
|
||||
LEFT JOIN artist ar_source \
|
||||
ON ar_source.server_id = source.server_id AND ar_source.id = source.artist_id \
|
||||
WHERE source.deleted = 0 \
|
||||
AND source.album_id IS NOT NULL AND source.album_id != ''{album_server_filter} \
|
||||
GROUP BY source.server_id, source.album_id \
|
||||
) \
|
||||
SELECT t.server_id, COALESCE(t.library_id, ''), t.id, t.artist, ar.name, t.title, \
|
||||
t.album_artist, t.album, t.album_id, physical_album.canonical_album_artist, \
|
||||
physical_album.canonical_album, t.duration_sec \
|
||||
FROM track t \
|
||||
LEFT JOIN artist ar ON ar.server_id = t.server_id AND ar.id = t.artist_id \
|
||||
WHERE t.deleted = 0",
|
||||
LEFT JOIN physical_album \
|
||||
ON physical_album.server_id = t.server_id AND physical_album.album_id = t.album_id \
|
||||
WHERE t.deleted = 0{track_server_filter}"
|
||||
);
|
||||
if server_id.is_some() {
|
||||
select.push_str(" AND t.server_id = ?1");
|
||||
}
|
||||
// Stream rows straight from the `track` SELECT into the sidecar UPSERT
|
||||
// (both statements borrow the same tx; the SELECT reads `track`, the
|
||||
// UPSERT writes the attached `cluster` table, so they don't contend).
|
||||
@@ -111,6 +149,9 @@ pub fn rebuild_cluster_keys(
|
||||
title,
|
||||
album_artist,
|
||||
album,
|
||||
album_id,
|
||||
canonical_album_artist,
|
||||
canonical_album,
|
||||
duration_sec,
|
||||
) = map_source_track_row(row)?;
|
||||
let mut keys = build_track_cluster_keys(
|
||||
@@ -124,6 +165,13 @@ pub fn rebuild_cluster_keys(
|
||||
.filter(|name| !name.trim().is_empty())
|
||||
.or(artist.as_deref())
|
||||
.and_then(norm_part);
|
||||
if let Some(album_id) = album_id.as_deref().filter(|id| !id.trim().is_empty()) {
|
||||
keys.album_key = canonical_album_artist
|
||||
.as_deref()
|
||||
.filter(|name| !name.trim().is_empty())
|
||||
.and_then(|name| build_album_key(Some(name), canonical_album.as_deref().unwrap_or(&album)))
|
||||
.or_else(|| Some(concrete_physical_album_key(&server_id, album_id)));
|
||||
}
|
||||
upsert.execute(params![
|
||||
server_id,
|
||||
library_id,
|
||||
@@ -218,6 +266,9 @@ fn map_source_track_row(row: &rusqlite::Row<'_>) -> rusqlite::Result<SourceTrack
|
||||
row.get(6)?,
|
||||
row.get(7)?,
|
||||
row.get(8)?,
|
||||
row.get(9)?,
|
||||
row.get(10)?,
|
||||
row.get(11)?,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -296,6 +347,33 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn physical_album_track_row(
|
||||
server: &str,
|
||||
id: &str,
|
||||
title: &str,
|
||||
artist: &str,
|
||||
artist_id: &str,
|
||||
album: &str,
|
||||
album_id: &str,
|
||||
album_artist: &str,
|
||||
library_id: &str,
|
||||
) -> TrackRow {
|
||||
let mut row = track_row(
|
||||
server,
|
||||
id,
|
||||
title,
|
||||
Some(artist),
|
||||
album,
|
||||
Some(album_artist),
|
||||
200,
|
||||
library_id,
|
||||
);
|
||||
row.artist_id = Some(artist_id.into());
|
||||
row.album_id = Some(album_id.into());
|
||||
row
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rebuild_populates_keys_and_duration() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
@@ -385,6 +463,87 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rebuild_canonicalizes_unambiguous_physical_album_artist() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
TrackRepository::new(&store)
|
||||
.upsert_batch(&[physical_album_track_row(
|
||||
"s1",
|
||||
"t1",
|
||||
"The Ecstasy of Gold",
|
||||
"Metallica",
|
||||
"artist-1",
|
||||
"S&M2",
|
||||
"album-1",
|
||||
"Metallica & San Francisco Symphony",
|
||||
"lib-a",
|
||||
)])
|
||||
.unwrap();
|
||||
store
|
||||
.with_conn_mut("test.canonical_album_artist", |conn| {
|
||||
conn.execute(
|
||||
"INSERT INTO artist (server_id, id, name, synced_at) \
|
||||
VALUES ('s1', 'artist-1', 'Metallica', 1)",
|
||||
[],
|
||||
)?;
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
rebuild_cluster_keys(&store, None).unwrap();
|
||||
|
||||
let row = store
|
||||
.with_read_conn(|conn| read_cluster_row(conn, "s1", "t1"))
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(row.1, build_album_key(Some("Metallica"), "S&M2"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rebuild_keeps_ambiguous_physical_album_concrete() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
TrackRepository::new(&store)
|
||||
.upsert_batch(&[
|
||||
physical_album_track_row(
|
||||
"s1", "t1", "One", "Artist A", "artist-a", "Split", "album-1",
|
||||
"Various Artists", "lib-a",
|
||||
),
|
||||
physical_album_track_row(
|
||||
"s1", "t2", "Two", "Artist B", "artist-b", "Split", "album-1",
|
||||
"Various Artists", "lib-a",
|
||||
),
|
||||
])
|
||||
.unwrap();
|
||||
store
|
||||
.with_conn_mut("test.ambiguous_album_artist", |conn| {
|
||||
conn.execute(
|
||||
"INSERT INTO artist (server_id, id, name, synced_at) VALUES \
|
||||
('s1', 'artist-a', 'Artist A', 1), \
|
||||
('s1', 'artist-b', 'Artist B', 1)",
|
||||
[],
|
||||
)?;
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
rebuild_cluster_keys(&store, None).unwrap();
|
||||
|
||||
let first = store
|
||||
.with_read_conn(|conn| read_cluster_row(conn, "s1", "t1"))
|
||||
.unwrap()
|
||||
.unwrap()
|
||||
.1
|
||||
.unwrap();
|
||||
let second = store
|
||||
.with_read_conn(|conn| read_cluster_row(conn, "s1", "t2"))
|
||||
.unwrap()
|
||||
.unwrap()
|
||||
.1
|
||||
.unwrap();
|
||||
assert_eq!(first, second);
|
||||
assert!(first.starts_with("physical:2:s1:album-1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rebuild_is_idempotent() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
@@ -637,6 +796,43 @@ mod tests {
|
||||
assert_eq!(s2_keys, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stale_per_server_rebuild_refreshes_all_servers_before_stamping_version() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
TrackRepository::new(&store)
|
||||
.upsert_batch(&[
|
||||
track_row("s1", "t1", "One", Some("A"), "Al", None, 1, "lib"),
|
||||
track_row("s2", "t2", "Two", Some("B"), "Al", None, 2, "lib"),
|
||||
])
|
||||
.unwrap();
|
||||
rebuild_cluster_keys(&store, None).unwrap();
|
||||
store
|
||||
.with_conn_mut("test.stale_per_server", |conn| {
|
||||
conn.execute(
|
||||
"UPDATE track SET title = 'Updated' WHERE server_id = 's2' AND id = 't2'",
|
||||
[],
|
||||
)?;
|
||||
conn.execute(
|
||||
"UPDATE cluster.cluster_meta SET value = 'stale' WHERE key = 'norm_version'",
|
||||
[],
|
||||
)?;
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
rebuild_cluster_keys(&store, Some("s1")).unwrap();
|
||||
|
||||
let rebuilt = store
|
||||
.with_read_conn(|conn| read_cluster_row(conn, "s2", "t2"))
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
rebuilt.0,
|
||||
build_track_cluster_keys(Some("B"), "Updated", "Al", None).cluster_key
|
||||
);
|
||||
assert!(!store.with_read_conn(cluster_rebuild_needed).unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cluster_attach_visible_on_read_connection() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
|
||||
@@ -864,6 +864,17 @@ mod tests {
|
||||
},
|
||||
])
|
||||
.unwrap();
|
||||
store
|
||||
.with_conn_mut("test.multi_scope_artists", |conn| {
|
||||
conn.execute(
|
||||
"INSERT INTO artist (server_id, id, name, synced_at) VALUES \
|
||||
('s1', 'ar-a', 'Shared Artist', 1), \
|
||||
('s1', 'ar-b', 'Shared Artist', 1)",
|
||||
[],
|
||||
)?;
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
rebuild_cluster_keys(&store, None).unwrap();
|
||||
|
||||
let scopes = vec![
|
||||
|
||||
@@ -396,6 +396,19 @@ mod tests {
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
fn insert_artist(store: &LibraryStore, server_id: &str) {
|
||||
store
|
||||
.with_conn_mut("test.mainstage_artist", |conn| {
|
||||
conn.execute(
|
||||
"INSERT INTO artist (server_id, id, name, synced_at) VALUES (?1, ?2, 'Artist', 1) \
|
||||
ON CONFLICT(server_id, id) DO NOTHING",
|
||||
params![server_id, format!("artist-{server_id}")],
|
||||
)?;
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_releases_are_globally_ordered_and_exclude_null_created_at() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
@@ -563,6 +576,8 @@ mod tests {
|
||||
track("s2", "t-later", "Shared", "later-id", "l2", Some(500)),
|
||||
])
|
||||
.unwrap();
|
||||
insert_artist(&store, "s1");
|
||||
insert_artist(&store, "s2");
|
||||
ensure_cluster_keys_built(&store, "s1").unwrap();
|
||||
ensure_cluster_keys_built(&store, "s2").unwrap();
|
||||
|
||||
@@ -700,6 +715,8 @@ mod tests {
|
||||
track("s2", "t-later", "Shared", "later-id", "l2", Some(500)),
|
||||
])
|
||||
.unwrap();
|
||||
insert_artist(&store, "s1");
|
||||
insert_artist(&store, "s2");
|
||||
ensure_cluster_keys_built(&store, "s1").unwrap();
|
||||
ensure_cluster_keys_built(&store, "s2").unwrap();
|
||||
store
|
||||
|
||||
@@ -295,9 +295,9 @@ pub(crate) fn ensure_cluster_keys_for_scopes(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Artist reads use `artist_key` even for a single library, so they must apply
|
||||
/// identity-key version upgrades without relying on multi-library dedup being enabled.
|
||||
fn ensure_artist_cluster_keys_for_scopes(
|
||||
/// Detail and artist reads use identity keys even for a single library, so they
|
||||
/// must apply version upgrades without relying on multi-library dedup being enabled.
|
||||
fn ensure_cluster_keys_for_all_scopes(
|
||||
store: &LibraryStore,
|
||||
scopes: &[LibraryScopePair],
|
||||
) -> Result<(), String> {
|
||||
@@ -401,7 +401,7 @@ pub fn list_artists(
|
||||
request: &LibraryScopeListRequest,
|
||||
) -> Result<Vec<LibraryArtistDto>, String> {
|
||||
let scopes = non_empty_scopes(&request.scopes)?;
|
||||
ensure_artist_cluster_keys_for_scopes(store, scopes)?;
|
||||
ensure_cluster_keys_for_all_scopes(store, scopes)?;
|
||||
let limit = clamp_limit(request.limit);
|
||||
let offset = clamp_offset(request.offset);
|
||||
let order = artist_order_sql(request.sort.as_deref());
|
||||
@@ -653,7 +653,7 @@ pub(crate) fn list_artists_layer1_filtered(
|
||||
skip_totals: bool,
|
||||
) -> Result<(Vec<LibraryArtistDto>, u32), String> {
|
||||
let scopes = non_empty_scopes(scopes)?;
|
||||
ensure_artist_cluster_keys_for_scopes(store, scopes)?;
|
||||
ensure_cluster_keys_for_all_scopes(store, scopes)?;
|
||||
let (cte, scope_binds) = scope_cte_sql(scopes);
|
||||
let scoped = if scopes.len() == 1 {
|
||||
scoped_track_join_layer1()
|
||||
@@ -753,7 +753,7 @@ pub(crate) fn list_index_artists_layer1_filtered(
|
||||
skip_totals: bool,
|
||||
) -> Result<(Vec<LibraryArtistDto>, u32), String> {
|
||||
let scopes = non_empty_scopes(scopes)?;
|
||||
ensure_artist_cluster_keys_for_scopes(store, scopes)?;
|
||||
ensure_cluster_keys_for_all_scopes(store, scopes)?;
|
||||
let (cte, scope_binds) = scope_cte_sql(scopes);
|
||||
let scoped_from = "FROM scope s \
|
||||
CROSS JOIN track t ON t.server_id = s.server_id AND t.library_id = s.library_id";
|
||||
@@ -1053,7 +1053,7 @@ pub(crate) fn list_artists_filtered(
|
||||
skip_totals: bool,
|
||||
) -> Result<(Vec<LibraryArtistDto>, u32), String> {
|
||||
let scopes = non_empty_scopes(scopes)?;
|
||||
ensure_artist_cluster_keys_for_scopes(store, scopes)?;
|
||||
ensure_cluster_keys_for_all_scopes(store, scopes)?;
|
||||
let (cte, scope_binds) = scope_cte_sql(scopes);
|
||||
let base_where = append_extra_where(
|
||||
&format!(
|
||||
@@ -1435,7 +1435,7 @@ pub(crate) fn live_search_artists(
|
||||
limit: u32,
|
||||
) -> Result<Vec<LibraryArtistDto>, String> {
|
||||
let scopes = non_empty_scopes(scopes)?;
|
||||
ensure_artist_cluster_keys_for_scopes(store, scopes)?;
|
||||
ensure_cluster_keys_for_all_scopes(store, scopes)?;
|
||||
let (cte, mut binds) = scope_cte_sql(scopes);
|
||||
let sql = format!(
|
||||
"{cte}, \
|
||||
@@ -1524,17 +1524,15 @@ fn lookup_album_key(
|
||||
album_id: &str,
|
||||
) -> rusqlite::Result<Option<String>> {
|
||||
conn.query_row(
|
||||
"SELECT ck.album_key FROM track t \
|
||||
"SELECT CASE WHEN COUNT(*) = COUNT(ck.album_key) \
|
||||
AND COUNT(DISTINCT ck.album_key) = 1 \
|
||||
THEN MIN(ck.album_key) END \
|
||||
FROM track t \
|
||||
INNER JOIN cluster.track_cluster_key ck ON ck.server_id = t.server_id AND ck.track_id = t.id \
|
||||
WHERE t.server_id = ? AND t.album_id = ? AND t.deleted = 0 LIMIT 1",
|
||||
WHERE t.server_id = ? AND t.album_id = ? AND t.deleted = 0",
|
||||
rusqlite::params![server_id, album_id],
|
||||
// The row exists but `album_key` is SQL NULL by design (any empty name
|
||||
// part → NULL key). Read it as `Option` so a NULL key yields `None`
|
||||
// instead of an `InvalidColumnType` error that would fail detail open.
|
||||
|r| r.get::<_, Option<String>>(0),
|
||||
)
|
||||
.optional()
|
||||
.map(Option::flatten)
|
||||
}
|
||||
|
||||
fn lookup_artist_key(
|
||||
@@ -1642,7 +1640,7 @@ fn fetch_album_candidates(
|
||||
let (cte, scoped, key_filter, priority) = keyed_detail_track_source(
|
||||
scope_cte,
|
||||
album_key.map(|_| "album_key"),
|
||||
"AND t.server_id = ? AND t.album_id = ? AND ck.album_key IS NULL",
|
||||
"AND t.server_id = ? AND t.album_id = ?",
|
||||
);
|
||||
let sql = format!(
|
||||
"{cte}, \
|
||||
@@ -1707,7 +1705,7 @@ fn fetch_scope_deduped_tracks_for_album_key(
|
||||
let (cte, scoped, key_filter, priority) = keyed_detail_track_source(
|
||||
scope_cte,
|
||||
album_key.map(|_| "album_key"),
|
||||
"AND t.server_id = ? AND t.album_id = ? AND ck.album_key IS NULL",
|
||||
"AND t.server_id = ? AND t.album_id = ?",
|
||||
);
|
||||
let cols = aliased_track_columns("t");
|
||||
let plain_cols = plain_track_columns_sql();
|
||||
@@ -1744,6 +1742,7 @@ pub fn album_detail(
|
||||
request: &LibraryScopeAlbumDetailRequest,
|
||||
) -> Result<LibraryScopeAlbumDetailResponse, String> {
|
||||
let scopes = non_empty_scopes(&request.scopes)?;
|
||||
ensure_cluster_keys_for_all_scopes(store, scopes)?;
|
||||
let server_id = request.server_id.trim();
|
||||
let album_id = request.album_id.trim();
|
||||
if server_id.is_empty() || album_id.is_empty() {
|
||||
@@ -1753,13 +1752,10 @@ pub fn album_detail(
|
||||
store.with_read_conn(|conn| {
|
||||
let album_key = lookup_album_key(conn, server_id, album_id)?;
|
||||
let candidates = fetch_album_candidates(conn, scopes, album_key.as_deref(), server_id, album_id)?;
|
||||
let mut albums: Vec<LibraryAlbumDto> = candidates.into_iter().map(|(_, a)| a).collect();
|
||||
albums.sort_by_key(|a| {
|
||||
scopes
|
||||
.iter()
|
||||
.position(|p| p.server_id == a.server_id)
|
||||
.unwrap_or(usize::MAX) as i64
|
||||
});
|
||||
let albums: Vec<LibraryAlbumDto> = candidates
|
||||
.into_iter()
|
||||
.map(|(_, album)| album)
|
||||
.collect();
|
||||
let mut album = merge_album_by_priority(&albums);
|
||||
album.starred_at =
|
||||
read_album_starred_at(conn, server_id, album_id).unwrap_or(None);
|
||||
@@ -1841,14 +1837,27 @@ fn fetch_albums_for_artist_key(
|
||||
let sql = format!(
|
||||
"{cte}, \
|
||||
base AS ( \
|
||||
SELECT t.server_id, t.album_id, t.album, t.artist, t.artist_id, t.album_artist, \
|
||||
t.year, t.genre, t.cover_art_id, t.starred_at, t.synced_at, t.duration_sec, t.id, \
|
||||
{priority} AS pr, {ALBUM_DEDUP_KEY} AS album_dedup, {TRACK_DEDUP_KEY} AS track_dedup \
|
||||
{scoped} AND t.album_id IS NOT NULL AND t.album_id != '' {key_filter} \
|
||||
), \
|
||||
deduped_tracks AS ( \
|
||||
SELECT *, ROW_NUMBER() OVER (PARTITION BY track_dedup ORDER BY pr ASC, id ASC) AS trn \
|
||||
FROM base \
|
||||
SELECT t.server_id, t.album_id, t.album, t.artist, t.artist_id, t.album_artist, \
|
||||
t.year, t.genre, t.cover_art_id, t.starred_at, t.synced_at, t.duration_sec, t.id, \
|
||||
ck.album_key, {priority} AS pr, {TRACK_DEDUP_KEY} AS track_dedup \
|
||||
{scoped} AND t.album_id IS NOT NULL AND t.album_id != '' {key_filter} \
|
||||
), \
|
||||
physical_albums AS ( \
|
||||
SELECT server_id, album_id, \
|
||||
CASE WHEN COUNT(*) = COUNT(album_key) AND COUNT(DISTINCT album_key) = 1 \
|
||||
THEN MIN(album_key) \
|
||||
ELSE ('physical:' || LENGTH(server_id) || ':' || server_id || ':' || album_id) END AS album_dedup \
|
||||
FROM base GROUP BY server_id, album_id \
|
||||
), \
|
||||
physical_tracks AS ( \
|
||||
SELECT b.*, physical_albums.album_dedup \
|
||||
FROM base b \
|
||||
INNER JOIN physical_albums \
|
||||
ON physical_albums.server_id = b.server_id AND physical_albums.album_id = b.album_id \
|
||||
), \
|
||||
deduped_tracks AS ( \
|
||||
SELECT *, ROW_NUMBER() OVER (PARTITION BY album_dedup, track_dedup ORDER BY pr ASC, id ASC) AS trn \
|
||||
FROM physical_tracks \
|
||||
), \
|
||||
album_stats AS ( \
|
||||
SELECT album_dedup, COUNT(*) AS song_count, SUM(duration_sec) AS duration_total \
|
||||
@@ -1858,7 +1867,7 @@ fn fetch_albums_for_artist_key(
|
||||
SELECT b.server_id, b.album_id, b.album, b.artist, b.artist_id, b.album_artist, \
|
||||
b.year, b.genre, b.cover_art_id, b.starred_at, b.synced_at, b.album_dedup, \
|
||||
ROW_NUMBER() OVER (PARTITION BY b.album_dedup ORDER BY b.pr ASC, b.album_id ASC, b.id ASC) AS rn \
|
||||
FROM base b \
|
||||
FROM physical_tracks b \
|
||||
) \
|
||||
SELECT p.server_id, p.album_id, p.album, p.artist, p.artist_id, p.album_artist, \
|
||||
st.song_count, st.duration_total, p.year, p.genre, p.cover_art_id, p.starred_at, p.synced_at \
|
||||
@@ -2019,7 +2028,7 @@ pub fn artist_detail(
|
||||
request: &LibraryScopeArtistDetailRequest,
|
||||
) -> Result<LibraryScopeArtistDetailResponse, String> {
|
||||
let scopes = non_empty_scopes(&request.scopes)?;
|
||||
ensure_artist_cluster_keys_for_scopes(store, scopes)?;
|
||||
ensure_cluster_keys_for_all_scopes(store, scopes)?;
|
||||
let server_id = request.server_id.trim();
|
||||
let artist_id = request.artist_id.trim();
|
||||
if server_id.is_empty() || artist_id.is_empty() {
|
||||
@@ -2164,6 +2173,24 @@ mod tests {
|
||||
|
||||
fn seed_and_rebuild(store: &LibraryStore, rows: &[TrackRow]) {
|
||||
TrackRepository::new(store).upsert_batch(rows).unwrap();
|
||||
store
|
||||
.with_conn_mut("test.seed_artists", |conn| {
|
||||
for row in rows {
|
||||
let Some(artist_id) = row.artist_id.as_deref() else {
|
||||
continue;
|
||||
};
|
||||
let Some(artist) = row.artist.as_deref() else {
|
||||
continue;
|
||||
};
|
||||
conn.execute(
|
||||
"INSERT INTO artist (server_id, id, name, synced_at) VALUES (?1, ?2, ?3, 1) \
|
||||
ON CONFLICT(server_id, id) DO NOTHING",
|
||||
rusqlite::params![&row.server_id, artist_id, artist],
|
||||
)?;
|
||||
}
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
rebuild_cluster_keys(store, None).unwrap();
|
||||
}
|
||||
|
||||
@@ -2819,6 +2846,150 @@ mod tests {
|
||||
assert_eq!(detail.album.starred_at, Some(1111));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn canonical_artist_album_key_merges_discography_and_preserves_track_owners() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
let mut s1_shared = track(
|
||||
"s1", "s1-shared", "Shared", Some("Metallica"), "S&M2", "s1-album",
|
||||
Some("s1-artist"), 200, "lib-a", Some(2020), None, None,
|
||||
);
|
||||
s1_shared.album_artist = Some("Metallica & San Francisco Symphony".into());
|
||||
let s2_shared = track(
|
||||
"s2", "s2-shared", "Shared", Some("Metallica"), "S&M2", "s2-album",
|
||||
Some("s2-artist"), 200, "lib-b", Some(2020), None, None,
|
||||
);
|
||||
let s2_unique = track(
|
||||
"s2", "s2-unique", "Unique", Some("Metallica"), "S&M2", "s2-album",
|
||||
Some("s2-artist"), 240, "lib-b", Some(2020), None, None,
|
||||
);
|
||||
seed_and_rebuild(&store, &[s1_shared, s2_shared, s2_unique]);
|
||||
store
|
||||
.with_conn_mut("test.stale_album_identity", |conn| {
|
||||
conn.execute(
|
||||
"UPDATE cluster.track_cluster_key \
|
||||
SET album_key = CASE server_id \
|
||||
WHEN 's1' THEN 'metallicasymphony-old' ELSE 'metallica-old' END",
|
||||
[],
|
||||
)?;
|
||||
conn.execute(
|
||||
"UPDATE cluster.cluster_meta SET value = 'stale' WHERE key = 'norm_version'",
|
||||
[],
|
||||
)?;
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
let scopes = vec![scope_pair("s1", "lib-a"), scope_pair("s2", "lib-b")];
|
||||
let detail = album_detail(
|
||||
&store,
|
||||
&LibraryScopeAlbumDetailRequest {
|
||||
scopes: scopes.clone(),
|
||||
album_id: "s1-album".into(),
|
||||
server_id: "s1".into(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(detail.album.server_id, "s1");
|
||||
assert_eq!(detail.album.id, "s1-album");
|
||||
assert_eq!(
|
||||
detail
|
||||
.tracks
|
||||
.iter()
|
||||
.map(|track| (track.server_id.as_str(), track.id.as_str()))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![("s1", "s1-shared"), ("s2", "s2-unique")]
|
||||
);
|
||||
|
||||
let artist = artist_detail(
|
||||
&store,
|
||||
&LibraryScopeArtistDetailRequest {
|
||||
scopes,
|
||||
artist_id: "s1-artist".into(),
|
||||
server_id: "s1".into(),
|
||||
include_tracks: false,
|
||||
top_tracks_limit: None,
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(artist.albums.len(), 1);
|
||||
assert_eq!(artist.albums[0].server_id, "s1");
|
||||
assert_eq!(artist.albums[0].id, "s1-album");
|
||||
assert_eq!(artist.albums[0].song_count, Some(2));
|
||||
|
||||
let reverse = album_detail(
|
||||
&store,
|
||||
&LibraryScopeAlbumDetailRequest {
|
||||
scopes: vec![scope_pair("s2", "lib-b"), scope_pair("s1", "lib-a")],
|
||||
album_id: "s2-album".into(),
|
||||
server_id: "s2".into(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(reverse.album.server_id, "s2");
|
||||
assert_eq!(reverse.album.id, "s2-album");
|
||||
assert_eq!(
|
||||
reverse
|
||||
.tracks
|
||||
.iter()
|
||||
.map(|track| (track.server_id.as_str(), track.id.as_str()))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![("s2", "s2-shared"), ("s2", "s2-unique")]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ambiguous_physical_albums_stay_separate_but_open_all_tracks() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
let mut rows = vec![
|
||||
track(
|
||||
"s1", "s1-a", "One", Some("Artist A"), "Split", "s1-album",
|
||||
Some("s1-artist-a"), 200, "lib-a", None, None, None,
|
||||
),
|
||||
track(
|
||||
"s1", "s1-b", "Two", Some("Artist B"), "Split", "s1-album",
|
||||
Some("s1-artist-b"), 210, "lib-a", None, None, None,
|
||||
),
|
||||
track(
|
||||
"s2", "s2-a", "One", Some("Artist A"), "Split", "s2-album",
|
||||
Some("s2-artist-a"), 200, "lib-b", None, None, None,
|
||||
),
|
||||
track(
|
||||
"s2", "s2-c", "Three", Some("Artist C"), "Split", "s2-album",
|
||||
Some("s2-artist-c"), 220, "lib-b", None, None, None,
|
||||
),
|
||||
];
|
||||
for row in &mut rows {
|
||||
row.album_artist = Some("Various Artists".into());
|
||||
}
|
||||
seed_and_rebuild(&store, &rows);
|
||||
|
||||
let scopes = vec![scope_pair("s1", "lib-a"), scope_pair("s2", "lib-b")];
|
||||
let artist = artist_detail(
|
||||
&store,
|
||||
&LibraryScopeArtistDetailRequest {
|
||||
scopes: scopes.clone(),
|
||||
artist_id: "s1-artist-a".into(),
|
||||
server_id: "s1".into(),
|
||||
include_tracks: false,
|
||||
top_tracks_limit: None,
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(artist.albums.len(), 2);
|
||||
|
||||
let detail = album_detail(
|
||||
&store,
|
||||
&LibraryScopeAlbumDetailRequest {
|
||||
scopes,
|
||||
album_id: "s1-album".into(),
|
||||
server_id: "s1".into(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(detail.tracks.len(), 2);
|
||||
assert!(detail.tracks.iter().all(|track| track.server_id == "s1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn artist_dedup_collapses_across_libraries() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
|
||||
Reference in New Issue
Block a user