mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 14:55:43 +00:00
fix(library): All Albums compilation filter matches VA album artist (#1026)
* fix(library): All Albums compilation filter matches VA album artist Local index SQL and track-grouped browse missed compilations tagged via Various Artists on album_artist; genre-only browse also ignored combined filters. Extend predicates on both sides and route genre+filter to advanced search. * docs(changelog): All Albums compilation filter fix (PR #1026)
This commit is contained in:
@@ -161,6 +161,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
### All Albums — Only compilations filter returns results
|
||||||
|
|
||||||
|
**By [@cucadmuh](https://github.com/cucadmuh), reported by zunoz on Discord, PR [#1026](https://github.com/Psychotoxical/psysonic/pull/1026)**
|
||||||
|
|
||||||
|
* The **Only compilations** toggle on **All Albums** no longer returns an empty list when compilations are tagged via **Various Artists** as album artist or when genre is combined with other browse filters — local index SQL, track-grouped browse, and client-side detection now agree on the same compilation signals.
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
## [1.47.0]
|
## [1.47.0]
|
||||||
|
|
||||||
> **🙏 Thank you to our amazing Discord community.** This release would not have been possible without your tireless support, quality checks, bug reports and all-round collaboration. Every report, every repro and every bit of feedback shaped what shipped here — thank you. Come join us: [discord.gg/AMnDRErm4u](https://discord.gg/AMnDRErm4u)
|
> **🙏 Thank you to our amazing Discord community.** This release would not have been possible without your tireless support, quality checks, bug reports and all-round collaboration. Every report, every repro and every bit of feedback shaped what shipped here — thank you. Come join us: [discord.gg/AMnDRErm4u](https://discord.gg/AMnDRErm4u)
|
||||||
|
|||||||
@@ -460,8 +460,8 @@ fn build_album_from_tracks(
|
|||||||
);
|
);
|
||||||
|
|
||||||
let select = "t.server_id, t.album_id, MAX(t.album), MAX(t.artist), MAX(t.artist_id), \
|
let select = "t.server_id, t.album_id, MAX(t.album), MAX(t.artist), MAX(t.artist_id), \
|
||||||
COUNT(*), SUM(t.duration_sec), MAX(t.year), MAX(t.genre), MAX(t.cover_art_id), \
|
MAX(t.album_artist), COUNT(*), SUM(t.duration_sec), MAX(t.year), MAX(t.genre), \
|
||||||
MAX(t.starred_at), MAX(t.synced_at)";
|
MAX(t.cover_art_id), MAX(t.starred_at), MAX(t.synced_at)";
|
||||||
let order = album_order_from_track_groups(&req.sort).unwrap_or_else(|| {
|
let order = album_order_from_track_groups(&req.sort).unwrap_or_else(|| {
|
||||||
"ORDER BY MAX(t.album) COLLATE NOCASE ASC, t.album_id ASC".to_string()
|
"ORDER BY MAX(t.album) COLLATE NOCASE ASC, t.album_id ASC".to_string()
|
||||||
});
|
});
|
||||||
@@ -868,10 +868,10 @@ fn resolve_clause(
|
|||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
("compilation", EntityKind::Album) => {
|
("compilation", EntityKind::Album) => {
|
||||||
return compilation_filter_fragment(&c.field, c.op, c.value.as_ref(), "a");
|
return compilation_filter_fragment(&c.field, c.op, c.value.as_ref(), EntityKind::Album);
|
||||||
}
|
}
|
||||||
("compilation", EntityKind::Track) => {
|
("compilation", EntityKind::Track) => {
|
||||||
return compilation_filter_fragment(&c.field, c.op, c.value.as_ref(), "t");
|
return compilation_filter_fragment(&c.field, c.op, c.value.as_ref(), EntityKind::Track);
|
||||||
}
|
}
|
||||||
("compilation", _) => return Ok(None),
|
("compilation", _) => return Ok(None),
|
||||||
// `text` is handled by the entity builder (FTS / LIKE), never here.
|
// `text` is handled by the entity builder (FTS / LIKE), never here.
|
||||||
@@ -1156,19 +1156,21 @@ fn map_artist(r: &rusqlite::Row<'_>) -> rusqlite::Result<LibraryArtistDto> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn map_album_from_tracks(r: &rusqlite::Row<'_>) -> rusqlite::Result<LibraryAlbumDto> {
|
fn map_album_from_tracks(r: &rusqlite::Row<'_>) -> rusqlite::Result<LibraryAlbumDto> {
|
||||||
|
let track_artist: Option<String> = r.get(3)?;
|
||||||
|
let album_artist: Option<String> = r.get(5)?;
|
||||||
Ok(LibraryAlbumDto {
|
Ok(LibraryAlbumDto {
|
||||||
server_id: r.get(0)?,
|
server_id: r.get(0)?,
|
||||||
id: r.get(1)?,
|
id: r.get(1)?,
|
||||||
name: r.get(2)?,
|
name: r.get(2)?,
|
||||||
artist: r.get(3)?,
|
artist: crate::album_compilation_filter::pick_album_group_artist(track_artist, album_artist),
|
||||||
artist_id: r.get(4)?,
|
artist_id: r.get(4)?,
|
||||||
song_count: Some(r.get(5)?),
|
song_count: Some(r.get(6)?),
|
||||||
duration_sec: Some(r.get(6)?),
|
duration_sec: Some(r.get(7)?),
|
||||||
year: r.get(7)?,
|
year: r.get(8)?,
|
||||||
genre: r.get(8)?,
|
genre: r.get(9)?,
|
||||||
cover_art_id: r.get(9)?,
|
cover_art_id: r.get(10)?,
|
||||||
starred_at: r.get(10)?,
|
starred_at: r.get(11)?,
|
||||||
synced_at: r.get(11)?,
|
synced_at: r.get(12)?,
|
||||||
raw_json: Value::Null,
|
raw_json: Value::Null,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -1264,9 +1266,21 @@ fn compilation_filter_fragment(
|
|||||||
field: &str,
|
field: &str,
|
||||||
op: FilterOp,
|
op: FilterOp,
|
||||||
value: Option<&Value>,
|
value: Option<&Value>,
|
||||||
table_alias: &str,
|
kind: EntityKind,
|
||||||
) -> Result<Option<SqlFragment>, String> {
|
) -> Result<Option<SqlFragment>, String> {
|
||||||
let comp_sql = crate::album_compilation_filter::compilation_raw_json_sql(table_alias);
|
let comp_sql = match kind {
|
||||||
|
EntityKind::Album => crate::album_compilation_filter::compilation_predicate_sql(
|
||||||
|
"a",
|
||||||
|
Some("a.artist"),
|
||||||
|
None,
|
||||||
|
),
|
||||||
|
EntityKind::Track => crate::album_compilation_filter::compilation_predicate_sql(
|
||||||
|
"t",
|
||||||
|
Some("t.artist"),
|
||||||
|
Some("t.album_artist"),
|
||||||
|
),
|
||||||
|
_ => crate::album_compilation_filter::compilation_raw_json_sql("t"),
|
||||||
|
};
|
||||||
match op {
|
match op {
|
||||||
FilterOp::IsTrue => Ok(Some(SqlFragment {
|
FilterOp::IsTrue => Ok(Some(SqlFragment {
|
||||||
sql: comp_sql,
|
sql: comp_sql,
|
||||||
@@ -2032,6 +2046,27 @@ mod tests {
|
|||||||
assert_eq!(resp.albums[0].id, "al_comp");
|
assert_eq!(resp.albums[0].id, "al_comp");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn compilation_filter_matches_va_album_artist_on_track_groups() {
|
||||||
|
let store = LibraryStore::open_in_memory();
|
||||||
|
let mut comp = track("s1", "t_comp", "Hit", "Alice", "Comp Album");
|
||||||
|
comp.album_id = Some("al_comp".into());
|
||||||
|
comp.album_artist = Some("Various Artists".into());
|
||||||
|
comp.raw_json = "{}".into();
|
||||||
|
let mut reg = track("s1", "t_reg", "Song", "Band", "Studio");
|
||||||
|
reg.album_id = Some("al_reg".into());
|
||||||
|
reg.raw_json = "{}".into();
|
||||||
|
TrackRepository::new(&store)
|
||||||
|
.upsert_batch(&[comp, reg])
|
||||||
|
.unwrap();
|
||||||
|
let mut r = req("s1", &[EntityKind::Album]);
|
||||||
|
r.filters = vec![clause("compilation", FilterOp::IsTrue, None, None)];
|
||||||
|
let resp = run_advanced_search(&store, &r).unwrap();
|
||||||
|
assert_eq!(resp.albums.len(), 1);
|
||||||
|
assert_eq!(resp.albums[0].id, "al_comp");
|
||||||
|
assert_eq!(resp.albums[0].artist.as_deref(), Some("Various Artists"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn compilation_filter_on_track_grouped_album_browse() {
|
fn compilation_filter_on_track_grouped_album_browse() {
|
||||||
let store = LibraryStore::open_in_memory();
|
let store = LibraryStore::open_in_memory();
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
//! OpenSubsonic compilation flag in entity `raw_json` (Navidrome: `compilation`,
|
//! OpenSubsonic compilation flag in entity `raw_json` (Navidrome: `compilation`,
|
||||||
//! `isCompilation`, or `releaseTypes` containing `Compilation`).
|
//! `isCompilation`, or `releaseTypes` containing `Compilation`), plus the same
|
||||||
|
//! "Various Artists" heuristics the web UI uses when structured flags are absent.
|
||||||
|
|
||||||
/// SQL predicate on any row with a `raw_json` column (album or track).
|
/// SQL predicate on any row with a `raw_json` column (album or track).
|
||||||
pub fn compilation_raw_json_sql(table_alias: &str) -> String {
|
pub fn compilation_raw_json_sql(table_alias: &str) -> String {
|
||||||
@@ -17,6 +18,49 @@ pub fn compilation_raw_json_sql(table_alias: &str) -> String {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn various_artists_like_sql(column: &str) -> String {
|
||||||
|
format!(
|
||||||
|
"lower(trim(coalesce({column}, ''))) LIKE '%various artists%'",
|
||||||
|
column = column
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Full compilation predicate for browse filters — JSON flags plus VA artist labels.
|
||||||
|
pub fn compilation_predicate_sql(
|
||||||
|
table_alias: &str,
|
||||||
|
artist_column: Option<&str>,
|
||||||
|
album_artist_column: Option<&str>,
|
||||||
|
) -> String {
|
||||||
|
let mut parts = vec![compilation_raw_json_sql(table_alias)];
|
||||||
|
parts.push(format!(
|
||||||
|
"lower(trim(coalesce(json_extract({a}.raw_json, '$.displayArtist'), ''))) LIKE '%various artists%'",
|
||||||
|
a = table_alias
|
||||||
|
));
|
||||||
|
if let Some(col) = artist_column {
|
||||||
|
parts.push(various_artists_like_sql(col));
|
||||||
|
}
|
||||||
|
if let Some(col) = album_artist_column {
|
||||||
|
parts.push(various_artists_like_sql(col));
|
||||||
|
}
|
||||||
|
format!("({})", parts.join(" OR "))
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn various_artists_label(s: &str) -> bool {
|
||||||
|
s.trim().to_ascii_lowercase().contains("various artists")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Track-grouped album rows: prefer album artist when it marks a VA compilation.
|
||||||
|
pub fn pick_album_group_artist(
|
||||||
|
track_artist: Option<String>,
|
||||||
|
album_artist: Option<String>,
|
||||||
|
) -> Option<String> {
|
||||||
|
let aa = album_artist.as_deref().unwrap_or("").trim();
|
||||||
|
if various_artists_label(aa) {
|
||||||
|
return Some(aa.to_string());
|
||||||
|
}
|
||||||
|
track_artist
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -27,4 +71,24 @@ mod tests {
|
|||||||
assert!(sql.contains("$.compilation"));
|
assert!(sql.contains("$.compilation"));
|
||||||
assert!(sql.contains("$.releaseTypes"));
|
assert!(sql.contains("$.releaseTypes"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn predicate_includes_artist_columns() {
|
||||||
|
let sql = compilation_predicate_sql("t", Some("t.artist"), Some("t.album_artist"));
|
||||||
|
assert!(sql.contains("t.artist"));
|
||||||
|
assert!(sql.contains("t.album_artist"));
|
||||||
|
assert!(sql.contains("$.displayArtist"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn pick_album_group_artist_prefers_va_album_artist() {
|
||||||
|
assert_eq!(
|
||||||
|
pick_album_group_artist(Some("Alice".into()), Some("Various Artists".into())),
|
||||||
|
Some("Various Artists".to_string())
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
pick_album_group_artist(Some("Alice".into()), Some("Bob".into())),
|
||||||
|
Some("Alice".to_string())
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -30,19 +30,42 @@ export async function runLocalAlbumBrowse(
|
|||||||
|
|
||||||
if (query.genres.length > 0) {
|
if (query.genres.length > 0) {
|
||||||
if (query.genres.length === 1) {
|
if (query.genres.length === 1) {
|
||||||
|
// Genre-only fast path; combined filters (year / lossless / compilation) need advanced search.
|
||||||
|
if (shared.length === 0) {
|
||||||
|
try {
|
||||||
|
const resp = await libraryListAlbumsByGenre({
|
||||||
|
serverId,
|
||||||
|
genre: query.genres[0],
|
||||||
|
libraryScope: scope,
|
||||||
|
sort: albumSortClauses(query.sort),
|
||||||
|
limit: pageSize,
|
||||||
|
offset,
|
||||||
|
});
|
||||||
|
if (resp.source !== 'local') return null;
|
||||||
|
let albums = resp.albums.map(albumToAlbum);
|
||||||
|
if (useServerStarredIds) albums = markServerStarredAlbums(albums);
|
||||||
|
return { albums, hasMore: resp.hasMore };
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
const resp = await libraryListAlbumsByGenre({
|
const resp = await libraryAdvancedSearch({
|
||||||
serverId,
|
serverId,
|
||||||
genre: query.genres[0],
|
|
||||||
libraryScope: scope,
|
libraryScope: scope,
|
||||||
|
entityTypes: ['album'],
|
||||||
|
filters: [{ field: 'genre', op: 'eq', value: query.genres[0] }, ...shared],
|
||||||
|
starredOnly,
|
||||||
|
restrictAlbumIds: useServerStarredIds ? restrictAlbumIds : undefined,
|
||||||
sort: albumSortClauses(query.sort),
|
sort: albumSortClauses(query.sort),
|
||||||
limit: pageSize,
|
limit: pageSize,
|
||||||
offset,
|
offset,
|
||||||
|
skipTotals: true,
|
||||||
});
|
});
|
||||||
if (resp.source !== 'local') return null;
|
if (resp.source !== 'local') return null;
|
||||||
let albums = resp.albums.map(albumToAlbum);
|
let albums = resp.albums.map(albumToAlbum);
|
||||||
if (useServerStarredIds) albums = markServerStarredAlbums(albums);
|
if (useServerStarredIds) albums = markServerStarredAlbums(albums);
|
||||||
return { albums, hasMore: resp.hasMore };
|
return { albums, hasMore: albums.length === pageSize };
|
||||||
} catch {
|
} catch {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ import {
|
|||||||
import { filterAlbumsByCompilation } from './albumBrowseFilters';
|
import { filterAlbumsByCompilation } from './albumBrowseFilters';
|
||||||
|
|
||||||
const album = (
|
const album = (
|
||||||
overrides: Partial<SubsonicAlbum> & { compilation?: boolean } = {},
|
overrides: Partial<SubsonicAlbum> & { compilation?: boolean; albumArtist?: string } = {},
|
||||||
): SubsonicAlbum => ({
|
): SubsonicAlbum => ({
|
||||||
id: '1',
|
id: '1',
|
||||||
name: 'A',
|
name: 'A',
|
||||||
@@ -25,6 +25,7 @@ describe('albumIsCompilation', () => {
|
|||||||
expect(albumIsCompilation(album({ compilation: true }))).toBe(true);
|
expect(albumIsCompilation(album({ compilation: true }))).toBe(true);
|
||||||
expect(albumIsCompilation(album({ releaseTypes: ['Live', 'Compilation'] }))).toBe(true);
|
expect(albumIsCompilation(album({ releaseTypes: ['Live', 'Compilation'] }))).toBe(true);
|
||||||
expect(albumIsCompilation(album({ artist: 'Various Artists' }))).toBe(true);
|
expect(albumIsCompilation(album({ artist: 'Various Artists' }))).toBe(true);
|
||||||
|
expect(albumIsCompilation(album({ albumArtist: 'Various Artists' }))).toBe(true);
|
||||||
expect(albumIsCompilation(album())).toBe(false);
|
expect(albumIsCompilation(album())).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -10,12 +10,15 @@ const VARIOUS_ARTISTS = /\bvarious artists\b/i;
|
|||||||
/** OpenSubsonic / Navidrome: `compilation`, `isCompilation`, `releaseTypes`, or VA artist. */
|
/** OpenSubsonic / Navidrome: `compilation`, `isCompilation`, `releaseTypes`, or VA artist. */
|
||||||
export function albumIsCompilation(a: SubsonicAlbum): boolean {
|
export function albumIsCompilation(a: SubsonicAlbum): boolean {
|
||||||
if (a.isCompilation === true) return true;
|
if (a.isCompilation === true) return true;
|
||||||
const loose = a as SubsonicAlbum & { compilation?: boolean };
|
const loose = a as SubsonicAlbum & { compilation?: boolean; albumArtist?: string };
|
||||||
if (loose.compilation === true) return true;
|
if (loose.compilation === true) return true;
|
||||||
if (a.releaseTypes?.some(t => /^compilation$/i.test(t.trim()))) return true;
|
if (a.releaseTypes?.some(t => /^compilation$/i.test(t.trim()))) return true;
|
||||||
const artist = (a.artist ?? '').trim();
|
const artist = (a.artist ?? '').trim();
|
||||||
const displayArtist = (a.displayArtist ?? '').trim();
|
const displayArtist = (a.displayArtist ?? '').trim();
|
||||||
return VARIOUS_ARTISTS.test(artist) || VARIOUS_ARTISTS.test(displayArtist);
|
const albumArtist = (loose.albumArtist ?? '').trim();
|
||||||
|
return VARIOUS_ARTISTS.test(artist)
|
||||||
|
|| VARIOUS_ARTISTS.test(displayArtist)
|
||||||
|
|| VARIOUS_ARTISTS.test(albumArtist);
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Stop paginating when the catalog tail is reached or the scan budget is spent. */
|
/** Stop paginating when the catalog tail is reached or the scan budget is spent. */
|
||||||
|
|||||||
Reference in New Issue
Block a user