mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 23:35:44 +00:00
fix(cluster): dedup merged album lists by album_key at album grain
Roll up track candidates to one row per (server_id, album_id) before partitioning by album_key so the same album on multiple servers (or mixed keyed/unkeyed tracks) appears once with priority winner per spec §4.
This commit is contained in:
@@ -368,6 +368,41 @@ mod tests {
|
||||
assert_eq!(resp.totals.artists, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merges_albums_by_album_key_with_priority() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
TrackRepository::new(&store)
|
||||
.upsert_batch(&[
|
||||
track("s1", "t1", "Band", "art-1", "LP", "alb-1"),
|
||||
track("s2", "t2", "Band", "art-2", "LP", "alb-2"),
|
||||
])
|
||||
.unwrap();
|
||||
rebuild_all_cluster_keys(&store).unwrap();
|
||||
|
||||
let resp = run_cluster_advanced_search(
|
||||
&store,
|
||||
LibraryClusterAdvancedSearchRequest {
|
||||
servers_ordered: vec!["s1".into(), "s2".into()],
|
||||
query: None,
|
||||
entity_types: vec![EntityKind::Album],
|
||||
filters: Vec::new(),
|
||||
starred_only: None,
|
||||
restrict_album_ids: None,
|
||||
restrict_album_scopes: HashMap::new(),
|
||||
query_album_title_only: None,
|
||||
sort: Vec::new(),
|
||||
limit: 50,
|
||||
offset: 0,
|
||||
skip_totals: false,
|
||||
library_scopes: HashMap::new(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(resp.albums.len(), 1);
|
||||
assert_eq!(resp.albums[0].server_id, "s1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn applies_offset_after_merge() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
|
||||
@@ -15,6 +15,7 @@ use crate::store::LibraryStore;
|
||||
use super::db::ATTACH_ALIAS;
|
||||
use super::keys::artist_key_from_display_name;
|
||||
use super::list_albums::list_merged_albums;
|
||||
use super::merge::ALBUM_ROLLUP_AND_PARTITION_CTE;
|
||||
use super::merge::{solo_partition_key, DURATION_TOLERANCE_SEC};
|
||||
use super::priority::{in_list_sql, priority_case_sql};
|
||||
|
||||
@@ -548,15 +549,7 @@ fn merged_albums_for_artist_key(
|
||||
AND k.artist_key = ?
|
||||
AND t.album_id IS NOT NULL AND t.album_id != ''
|
||||
),
|
||||
partitioned AS (
|
||||
SELECT c.tid,
|
||||
CASE
|
||||
WHEN c.album_key IS NULL THEN 'solo:' || c.server_id || ':' || c.album_id
|
||||
ELSE c.album_key
|
||||
END AS merge_key,
|
||||
c.priority_rank
|
||||
FROM candidates c
|
||||
),
|
||||
{ALBUM_ROLLUP_AND_PARTITION_CTE}
|
||||
winners AS (
|
||||
SELECT tid,
|
||||
ROW_NUMBER() OVER (PARTITION BY merge_key ORDER BY priority_rank) AS rn
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::store::LibraryStore;
|
||||
|
||||
use super::db::ATTACH_ALIAS;
|
||||
use super::library_scope::scope_filter_sql_and_params;
|
||||
use super::merge::ALBUM_ROLLUP_AND_PARTITION_CTE;
|
||||
use super::priority::{in_list_sql, priority_case_sql};
|
||||
|
||||
pub fn list_merged_albums(
|
||||
@@ -45,15 +46,7 @@ pub fn list_merged_albums(
|
||||
AND t.server_id IN ({in_placeholders})
|
||||
AND t.album_id IS NOT NULL AND t.album_id != ''{scope_sql}
|
||||
),
|
||||
partitioned AS (
|
||||
SELECT c.tid,
|
||||
CASE
|
||||
WHEN c.album_key IS NULL THEN 'solo:' || c.server_id || ':' || c.album_id
|
||||
ELSE c.album_key
|
||||
END AS merge_key,
|
||||
c.priority_rank
|
||||
FROM candidates c
|
||||
),
|
||||
{ALBUM_ROLLUP_AND_PARTITION_CTE}
|
||||
winners AS (
|
||||
SELECT tid,
|
||||
ROW_NUMBER() OVER (PARTITION BY merge_key ORDER BY priority_rank) AS rn
|
||||
@@ -200,4 +193,93 @@ mod tests {
|
||||
assert_eq!(resp.albums.len(), 1);
|
||||
assert_eq!(resp.albums[0].server_id, "s1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rollup_collapses_multiple_tracks_per_server_album() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
TrackRepository::new(&store)
|
||||
.upsert_batch(&[
|
||||
track("s1", "t1", "A", "Band", "LP", "alb1"),
|
||||
track("s1", "t2", "B", "Band", "LP", "alb1"),
|
||||
track("s1", "t3", "C", "Band", "LP", "alb1"),
|
||||
])
|
||||
.unwrap();
|
||||
rebuild_all_cluster_keys(&store).unwrap();
|
||||
|
||||
let resp = list_merged_albums(
|
||||
&store,
|
||||
&["s1".into()],
|
||||
50,
|
||||
0,
|
||||
&std::collections::HashMap::new(),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(resp.albums.len(), 1);
|
||||
assert_eq!(resp.albums[0].id, "alb1");
|
||||
}
|
||||
|
||||
fn track_no_key(server: &str, id: &str, album_id: &str) -> TrackRow {
|
||||
TrackRow {
|
||||
server_id: server.into(),
|
||||
id: id.into(),
|
||||
title: "".into(),
|
||||
title_sort: None,
|
||||
artist: Some("Band".into()),
|
||||
artist_id: Some(format!("art-{server}")),
|
||||
album: "LP".into(),
|
||||
album_id: Some(album_id.into()),
|
||||
album_artist: Some("Band".into()),
|
||||
duration_sec: 200,
|
||||
track_number: Some(1),
|
||||
disc_number: Some(1),
|
||||
year: Some(2020),
|
||||
genre: None,
|
||||
suffix: None,
|
||||
bit_rate: None,
|
||||
size_bytes: None,
|
||||
cover_art_id: None,
|
||||
starred_at: None,
|
||||
user_rating: None,
|
||||
play_count: None,
|
||||
played_at: None,
|
||||
server_path: None,
|
||||
library_id: None,
|
||||
isrc: None,
|
||||
mbid_recording: None,
|
||||
bpm: None,
|
||||
replay_gain_track_db: None,
|
||||
replay_gain_album_db: None,
|
||||
content_hash: None,
|
||||
server_updated_at: None,
|
||||
server_created_at: None,
|
||||
deleted: false,
|
||||
synced_at: 1,
|
||||
raw_json: "{}".into(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rollup_uses_album_key_when_some_tracks_lack_keys() {
|
||||
let store = LibraryStore::open_in_memory();
|
||||
TrackRepository::new(&store)
|
||||
.upsert_batch(&[
|
||||
track("s1", "t1", "A", "Band", "LP", "alb1"),
|
||||
track_no_key("s1", "t2", "alb1"),
|
||||
track("s2", "t3", "B", "Band", "LP", "alb2"),
|
||||
track_no_key("s2", "t4", "alb2"),
|
||||
])
|
||||
.unwrap();
|
||||
rebuild_all_cluster_keys(&store).unwrap();
|
||||
|
||||
let resp = list_merged_albums(
|
||||
&store,
|
||||
&["s1".into(), "s2".into()],
|
||||
50,
|
||||
0,
|
||||
&std::collections::HashMap::new(),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(resp.albums.len(), 1);
|
||||
assert_eq!(resp.albums[0].server_id, "s1");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -148,15 +148,27 @@ pub fn list_merged_favorite_albums(
|
||||
AND t.server_id IN ({in_placeholders})
|
||||
AND t.album_id IS NOT NULL AND t.album_id != ''
|
||||
),
|
||||
partitioned AS (
|
||||
SELECT c.tid,
|
||||
CASE
|
||||
WHEN c.album_key IS NULL THEN 'solo:' || c.server_id || ':' || c.album_id
|
||||
ELSE c.album_key
|
||||
END AS merge_key,
|
||||
c.priority_rank,
|
||||
c.starred_at
|
||||
album_rollup AS (
|
||||
SELECT
|
||||
c.server_id,
|
||||
c.album_id,
|
||||
MIN(c.tid) AS tid,
|
||||
MIN(c.priority_rank) AS priority_rank,
|
||||
MAX(c.album_key) AS album_key,
|
||||
MAX(c.starred_at) AS starred_at
|
||||
FROM candidates c
|
||||
GROUP BY c.server_id, c.album_id
|
||||
),
|
||||
partitioned AS (
|
||||
SELECT
|
||||
r.tid,
|
||||
CASE
|
||||
WHEN r.album_key IS NOT NULL THEN r.album_key
|
||||
ELSE 'solo:' || r.server_id || ':' || r.album_id
|
||||
END AS merge_key,
|
||||
r.priority_rank,
|
||||
r.starred_at
|
||||
FROM album_rollup r
|
||||
),
|
||||
starred_merge AS (
|
||||
SELECT DISTINCT merge_key
|
||||
|
||||
@@ -2,6 +2,31 @@
|
||||
|
||||
pub const DURATION_TOLERANCE_SEC: i64 = 5;
|
||||
|
||||
/// Roll up per-track candidates to one row per `(server_id, album_id)` before
|
||||
/// partitioning by `album_key` (spec §4 — album lists dedup by `album_key`).
|
||||
pub const ALBUM_ROLLUP_AND_PARTITION_CTE: &str = "
|
||||
album_rollup AS (
|
||||
SELECT
|
||||
c.server_id,
|
||||
c.album_id,
|
||||
MIN(c.tid) AS tid,
|
||||
MIN(c.priority_rank) AS priority_rank,
|
||||
MAX(c.album_key) AS album_key
|
||||
FROM candidates c
|
||||
GROUP BY c.server_id, c.album_id
|
||||
),
|
||||
partitioned AS (
|
||||
SELECT
|
||||
r.tid,
|
||||
CASE
|
||||
WHEN r.album_key IS NOT NULL THEN r.album_key
|
||||
ELSE 'solo:' || r.server_id || ':' || r.album_id
|
||||
END AS merge_key,
|
||||
r.priority_rank
|
||||
FROM album_rollup r
|
||||
),
|
||||
";
|
||||
|
||||
/// Synthetic partition for tracks without a `cluster_key` row (never merged).
|
||||
pub fn solo_partition_key(server_id: &str, track_id: &str) -> String {
|
||||
format!("solo:{server_id}:{track_id}")
|
||||
|
||||
Reference in New Issue
Block a user