Correct artist id-info split

This commit is contained in:
Wojciech Kozlowski 2025-01-18 09:45:28 +01:00
parent 8a9030bf61
commit 7934d7eccf
12 changed files with 29 additions and 85 deletions

View File

@ -22,13 +22,13 @@ pub struct Artist {
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
pub struct ArtistMeta { pub struct ArtistMeta {
pub id: ArtistId, pub id: ArtistId,
pub sort: Option<String>,
pub info: ArtistInfo, pub info: ArtistInfo,
} }
/// Artist non-identifier metadata. /// Artist non-identifier metadata.
#[derive(Clone, Debug, Default, PartialEq, Eq)] #[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct ArtistInfo { pub struct ArtistInfo {
pub sort: Option<String>,
pub properties: HashMap<String, Vec<String>>, pub properties: HashMap<String, Vec<String>>,
} }
@ -152,7 +152,6 @@ impl ArtistMeta {
pub fn new<Id: Into<ArtistId>>(id: Id) -> Self { pub fn new<Id: Into<ArtistId>>(id: Id) -> Self {
ArtistMeta { ArtistMeta {
id: id.into(), id: id.into(),
sort: None,
info: ArtistInfo::default(), info: ArtistInfo::default(),
} }
} }
@ -166,15 +165,7 @@ impl ArtistMeta {
} }
pub fn get_sort_key(&self) -> (&str,) { pub fn get_sort_key(&self) -> (&str,) {
(self.sort.as_ref().unwrap_or(&self.id.name),) (self.info.sort.as_ref().unwrap_or(&self.id.name),)
}
pub fn set_sort_key<S: Into<String>>(&mut self, sort: S) {
self.sort = Some(sort.into());
}
pub fn clear_sort_key(&mut self) {
self.sort.take();
} }
} }
@ -240,13 +231,13 @@ impl Merge for ArtistMeta {
fn merge_in_place(&mut self, other: Self) { fn merge_in_place(&mut self, other: Self) {
assert!(self.id.compatible(&other.id)); assert!(self.id.compatible(&other.id));
self.id.mb_ref = self.id.mb_ref.take().or(other.id.mb_ref); self.id.mb_ref = self.id.mb_ref.take().or(other.id.mb_ref);
self.sort = self.sort.take().or(other.sort);
self.info.merge_in_place(other.info); self.info.merge_in_place(other.info);
} }
} }
impl Merge for ArtistInfo { impl Merge for ArtistInfo {
fn merge_in_place(&mut self, other: Self) { fn merge_in_place(&mut self, other: Self) {
self.sort = self.sort.take().or(other.sort);
self.properties.merge_in_place(other.properties); self.properties.merge_in_place(other.properties);
} }
} }
@ -318,53 +309,6 @@ mod tests {
static MUSICBUTLER: &str = "https://www.musicbutler.io/artist-page/483340948"; static MUSICBUTLER: &str = "https://www.musicbutler.io/artist-page/483340948";
static MUSICBUTLER_2: &str = "https://www.musicbutler.io/artist-page/658903042/"; static MUSICBUTLER_2: &str = "https://www.musicbutler.io/artist-page/658903042/";
#[test]
fn artist_sort_set_clear() {
let artist_id = ArtistId::new("an artist");
let sort_id_1 = String::from("sort id 1");
let sort_id_2 = String::from("sort id 2");
let mut artist = Artist::new(&artist_id.name);
assert_eq!(artist.meta.id, artist_id);
assert_eq!(artist.meta.sort, None);
assert_eq!(artist.meta.get_sort_key(), (artist_id.name.as_str(),));
assert!(artist.meta < ArtistMeta::new(sort_id_1.clone()));
assert!(artist.meta < ArtistMeta::new(sort_id_2.clone()));
assert!(artist < Artist::new(sort_id_1.clone()));
assert!(artist < Artist::new(sort_id_2.clone()));
artist.meta.set_sort_key(sort_id_1.clone());
assert_eq!(artist.meta.id, artist_id);
assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_1));
assert_eq!(artist.meta.get_sort_key(), (sort_id_1.as_str(),));
assert!(artist.meta > ArtistMeta::new(artist_id.clone()));
assert!(artist.meta < ArtistMeta::new(sort_id_2.clone()));
assert!(artist > Artist::new(artist_id.clone()));
assert!(artist < Artist::new(sort_id_2.clone()));
artist.meta.set_sort_key(sort_id_2.clone());
assert_eq!(artist.meta.id, artist_id);
assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_2));
assert_eq!(artist.meta.get_sort_key(), (sort_id_2.as_str(),));
assert!(artist.meta > ArtistMeta::new(artist_id.clone()));
assert!(artist.meta > ArtistMeta::new(sort_id_1.clone()));
assert!(artist > Artist::new(artist_id.clone()));
assert!(artist > Artist::new(sort_id_1.clone()));
artist.meta.clear_sort_key();
assert_eq!(artist.meta.id, artist_id);
assert_eq!(artist.meta.sort, None);
assert_eq!(artist.meta.get_sort_key(), (artist_id.name.as_str(),));
assert!(artist.meta < ArtistMeta::new(sort_id_1.clone()));
assert!(artist.meta < ArtistMeta::new(sort_id_2.clone()));
assert!(artist < Artist::new(sort_id_1.clone()));
assert!(artist < Artist::new(sort_id_2.clone()));
}
#[test] #[test]
fn set_clear_musicbrainz_url() { fn set_clear_musicbrainz_url() {
let mut artist = Artist::new(ArtistId::new("an artist")); let mut artist = Artist::new(ArtistId::new("an artist"));

View File

@ -216,13 +216,13 @@ mod tests {
assert!(right.first().unwrap() > left.first().unwrap()); assert!(right.first().unwrap() > left.first().unwrap());
let artist_sort = Some(String::from("Album_Artist 0")); let artist_sort = Some(String::from("Album_Artist 0"));
right[0].meta.sort = artist_sort.clone(); right[0].meta.info.sort = artist_sort.clone();
assert!(right.first().unwrap() < left.first().unwrap()); assert!(right.first().unwrap() < left.first().unwrap());
// The result of the merge should be the same list of artists, but with the last artist now // The result of the merge should be the same list of artists, but with the last artist now
// in first place. // in first place.
let mut expected = left.to_owned(); let mut expected = left.to_owned();
expected.last_mut().as_mut().unwrap().meta.sort = artist_sort.clone(); expected.last_mut().as_mut().unwrap().meta.info.sort = artist_sort.clone();
expected.rotate_right(1); expected.rotate_right(1);
let mut mh = MusicHoard { let mut mh = MusicHoard {

View File

@ -92,17 +92,17 @@ impl<Database, Library: ILibrary> MusicHoard<Database, Library> {
.or_insert_with(|| Artist::new(artist_id)), .or_insert_with(|| Artist::new(artist_id)),
}; };
if artist.meta.sort.is_some() { if artist.meta.info.sort.is_some() {
if artist_sort.is_some() && (artist.meta.sort != artist_sort) { if artist_sort.is_some() && (artist.meta.info.sort != artist_sort) {
return Err(Error::CollectionError(format!( return Err(Error::CollectionError(format!(
"multiple album_artist_sort found for artist '{}': '{}' != '{}'", "multiple album_artist_sort found for artist '{}': '{}' != '{}'",
artist.meta.id, artist.meta.id,
artist.meta.sort.as_ref().unwrap(), artist.meta.info.sort.as_ref().unwrap(),
artist_sort.as_ref().unwrap() artist_sort.as_ref().unwrap()
))); )));
} }
} else if artist_sort.is_some() { } else if artist_sort.is_some() {
artist.meta.sort = artist_sort; artist.meta.info.sort = artist_sort;
} }
// Do a linear search as few artists have more than a handful of albums. Search from the // Do a linear search as few artists have more than a handful of albums. Search from the

View File

@ -122,8 +122,8 @@ impl From<DeserializeArtist> for Artist {
name: artist.name, name: artist.name,
mb_ref: artist.mb_ref.into(), mb_ref: artist.mb_ref.into(),
}, },
sort: artist.sort,
info: ArtistInfo { info: ArtistInfo {
sort: artist.sort,
properties: artist.properties, properties: artist.properties,
}, },
}, },

View File

@ -76,7 +76,7 @@ impl<'a> From<&'a Artist> for SerializeArtist<'a> {
SerializeArtist { SerializeArtist {
name: &artist.meta.id.name, name: &artist.meta.id.name,
mb_ref: (&artist.meta.id.mb_ref).into(), mb_ref: (&artist.meta.id.mb_ref).into(),
sort: &artist.meta.sort, sort: &artist.meta.info.sort,
properties: &artist.meta.info.properties, properties: &artist.meta.info.properties,
albums: artist.albums.iter().map(Into::into).collect(), albums: artist.albums.iter().map(Into::into).collect(),
} }

View File

@ -9,8 +9,8 @@ macro_rules! full_collection {
"https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000" "https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000"
).unwrap()), ).unwrap()),
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::from([ properties: HashMap::from([
(String::from("MusicButler"), vec![ (String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/000000000"), String::from("https://www.musicbutler.io/artist-page/000000000"),
@ -139,8 +139,8 @@ macro_rules! full_collection {
"https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111" "https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111"
).unwrap()), ).unwrap()),
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::from([ properties: HashMap::from([
(String::from("MusicButler"), vec![ (String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/111111111"), String::from("https://www.musicbutler.io/artist-page/111111111"),
@ -338,8 +338,8 @@ macro_rules! full_collection {
name: "The Album_Artist C".to_string(), name: "The Album_Artist C".to_string(),
mb_ref: ArtistMbRef::CannotHaveMbid, mb_ref: ArtistMbRef::CannotHaveMbid,
}, },
sort: Some("Album_Artist C, The".to_string()),
info: ArtistInfo { info: ArtistInfo {
sort: Some("Album_Artist C, The".to_string()),
properties: HashMap::new(), properties: HashMap::new(),
}, },
}, },
@ -436,8 +436,8 @@ macro_rules! full_collection {
name: "Album_Artist D".to_string(), name: "Album_Artist D".to_string(),
mb_ref: ArtistMbRef::None, mb_ref: ArtistMbRef::None,
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::new(), properties: HashMap::new(),
}, },
}, },

View File

@ -8,8 +8,8 @@ macro_rules! library_collection {
name: "Album_Artist A".to_string(), name: "Album_Artist A".to_string(),
mb_ref: ArtistMbRef::None, mb_ref: ArtistMbRef::None,
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::new(), properties: HashMap::new(),
}, },
}, },
@ -119,8 +119,8 @@ macro_rules! library_collection {
name: "Album_Artist B".to_string(), name: "Album_Artist B".to_string(),
mb_ref: ArtistMbRef::None, mb_ref: ArtistMbRef::None,
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::new(), properties: HashMap::new(),
}, },
}, },
@ -289,8 +289,8 @@ macro_rules! library_collection {
name: "The Album_Artist C".to_string(), name: "The Album_Artist C".to_string(),
mb_ref: ArtistMbRef::None, mb_ref: ArtistMbRef::None,
}, },
sort: Some("Album_Artist C, The".to_string()),
info: ArtistInfo { info: ArtistInfo {
sort: Some("Album_Artist C, The".to_string()),
properties: HashMap::new(), properties: HashMap::new(),
}, },
}, },
@ -381,8 +381,8 @@ macro_rules! library_collection {
name: "Album_Artist D".to_string(), name: "Album_Artist D".to_string(),
mb_ref: ArtistMbRef::None, mb_ref: ArtistMbRef::None,
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::new(), properties: HashMap::new(),
}, },
}, },

View File

@ -162,7 +162,7 @@ impl IAppInteractSearchPrivate for AppMachine<SearchState> {
let name = string::normalize_string_with(&probe.meta.id.name, !case_sens, !char_sens); let name = string::normalize_string_with(&probe.meta.id.name, !case_sens, !char_sens);
let mut result = name.string.starts_with(search); let mut result = name.string.starts_with(search);
if let Some(ref probe_sort) = probe.meta.sort { if let Some(ref probe_sort) = probe.meta.info.sort {
if !result { if !result {
let name = string::normalize_string_with(probe_sort, !case_sens, !char_sens); let name = string::normalize_string_with(probe_sort, !case_sens, !char_sens);
result = name.string.starts_with(search); result = name.string.starts_with(search);

View File

@ -119,8 +119,8 @@ fn from_mb_artist_meta(meta: MbArtistMeta) -> (ArtistMeta, Option<String>) {
name: meta.name, name: meta.name,
mb_ref: ArtistMbRef::Some(meta.id.into()), mb_ref: ArtistMbRef::Some(meta.id.into()),
}, },
sort,
info: ArtistInfo { info: ArtistInfo {
sort,
properties: HashMap::new(), properties: HashMap::new(),
}, },
}, },

View File

@ -65,7 +65,7 @@ fn merge_library_then_database() {
assert_eq!(music_hoard.get_collection(), &*COLLECTION); assert_eq!(music_hoard.get_collection(), &*COLLECTION);
sqldiff(&*COMPLETE_DB_TEST_FILE.as_os_str(), file.path().as_os_str()); sqldiff(COMPLETE_DB_TEST_FILE.as_os_str(), file.path().as_os_str());
} }
#[test] #[test]
@ -91,5 +91,5 @@ fn merge_database_then_library() {
assert_eq!(music_hoard.get_collection(), &*COLLECTION); assert_eq!(music_hoard.get_collection(), &*COLLECTION);
sqldiff(&*COMPLETE_DB_TEST_FILE.as_os_str(), file.path().as_os_str()); sqldiff(COMPLETE_DB_TEST_FILE.as_os_str(), file.path().as_os_str());
} }

View File

@ -22,8 +22,8 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
"https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212" "https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212"
).unwrap()), ).unwrap()),
}, },
sort: Some(String::from("Arkona")),
info: ArtistInfo { info: ArtistInfo {
sort: Some(String::from("Arkona")),
properties: HashMap::from([ properties: HashMap::from([
(String::from("MusicButler"), vec![ (String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/283448581"), String::from("https://www.musicbutler.io/artist-page/283448581"),
@ -217,8 +217,8 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
"https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38" "https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38"
).unwrap()), ).unwrap()),
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::from([ properties: HashMap::from([
(String::from("MusicButler"), vec![ (String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/269358403"), String::from("https://www.musicbutler.io/artist-page/269358403"),
@ -472,8 +472,8 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
"https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490" "https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490"
).unwrap()), ).unwrap()),
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::from([ properties: HashMap::from([
(String::from("MusicButler"), vec![ (String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/826588800"), String::from("https://www.musicbutler.io/artist-page/826588800"),
@ -631,8 +631,8 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
"https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc" "https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc"
).unwrap()), ).unwrap()),
}, },
sort: Some(String::from("Heavens Basement")),
info: ArtistInfo { info: ArtistInfo {
sort: Some(String::from("Heavens Basement")),
properties: HashMap::from([ properties: HashMap::from([
(String::from("MusicButler"), vec![ (String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/291158685"), String::from("https://www.musicbutler.io/artist-page/291158685"),
@ -770,8 +770,8 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
"https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab" "https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab"
).unwrap()), ).unwrap()),
}, },
sort: None,
info: ArtistInfo { info: ArtistInfo {
sort: None,
properties: HashMap::from([ properties: HashMap::from([
(String::from("MusicButler"), vec![ (String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/3996865"), String::from("https://www.musicbutler.io/artist-page/3996865"),