EBML: Add generic tag conversion test

Serial-ATA · Serial-ATA · commit e37fe441f6ca · 2024-10-18T22:29:21.000-04:00
diff --git a/lofty/src/ebml/tag/generic.rs b/lofty/src/ebml/tag/generic.rs
@@ -4,7 +4,7 @@
 
 use super::{Language, MatroskaTag, SimpleTag, TargetType, TOMBSTONE_SIMPLE_TAG};
 use crate::tag::items::Lang;
-use crate::tag::{ItemKey, Tag, TagItem, TagType};
+use crate::tag::{ItemKey, ItemValue, Tag, TagItem, TagType};
 
 use std::collections::HashMap;
 use std::sync::LazyLock;
@@ -140,7 +140,9 @@ pub(super) fn split_tag(mut matroska_tag: MatroskaTag) -> (MatroskaTag, Tag) {
 	let mut tag = Tag::new(TagType::Matroska);
 
 	// TODO: Pictures, can they be handled in a generic way?
-	//       What about the uid and referral?
+	//       - What about the uid and referral?
+	//       - It seems like the "standard" way of adding cover art is to name it "cover.{ext}"
+	//       - Maybe only support front covers? who knows.
 
 	matroska_tag.tags.retain_mut(|t| {
 		let target_type = match &t.target {
@@ -168,15 +170,17 @@ fn split_simple_tags(
 	tag: &mut Tag,
 ) -> bool {
 	let lang: Lang;
-	match &simple_tag.language {
-		Some(Language::Iso639_2(l)) if l.len() == 3 => {
-			lang = l.as_bytes().try_into().unwrap(); // Infallible
-		},
-		None => lang = *b"und",
-		// `Lang` doesn't support anything outside of a 3 character ISO-639-2 code.
-		_ => return TAG_RETAINED,
+	let Language::Iso639_2(l) = &simple_tag.language else {
+		return TAG_RETAINED;
+	};
+
+	// `Lang` doesn't support anything outside of a 3 character ISO-639-2 code.
+	if l.len() != 3 {
+		return TAG_CONSUMED;
 	}
 
+	lang = l.as_bytes().try_into().unwrap(); // Infallible
+
 	let Some(item_key) = MAPPINGS.get(&(target_type, &*simple_tag.name)).cloned() else {
 		return TAG_RETAINED;
 	};
@@ -197,6 +201,44 @@ fn split_simple_tags(
 	return TAG_CONSUMED;
 }
 
-pub(super) fn merge_tag(tag: Tag, matroska_tag: MatroskaTag) -> MatroskaTag {
-	todo!()
+pub(super) fn merge_tag(tag: Tag, mut matroska_tag: MatroskaTag) -> MatroskaTag {
+	for item in tag.items {
+		let Some((simple_tag, target_type)) = simple_tag_for_item(item) else {
+			continue;
+		};
+
+		let tag = matroska_tag.get_or_insert_tag_for_type(target_type);
+
+		tag.simple_tags.push(simple_tag);
+	}
+
+	matroska_tag
+}
+
+fn simple_tag_for_item(item: TagItem) -> Option<(SimpleTag<'static>, TargetType)> {
+	let TagItem {
+		mut lang,
+		item_key,
+		item_value: ItemValue::Text(text) | ItemValue::Locator(text),
+		..
+	} = item
+	else {
+		return None;
+	};
+
+	let Some((target_type, simple_tag_name)) = REVERSE_MAPPINGS.get(&item_key) else {
+		return None;
+	};
+
+	// Matroska uses "und" for unknown languages
+	if lang == *b"XXX" {
+		lang = *b"und";
+	}
+
+	let lang_str = std::str::from_utf8(lang.as_slice()).unwrap_or("und");
+
+	let mut simple_tag = SimpleTag::new(simple_tag_name.to_string(), text);
+	simple_tag.language = Language::Iso639_2(lang_str.to_string());
+
+	Some((simple_tag, *target_type))
 }
diff --git a/lofty/src/ebml/tag/mod.rs b/lofty/src/ebml/tag/mod.rs
@@ -4,6 +4,8 @@ mod simple_tag;
 mod tag;
 mod tag_name;
 mod target;
+#[cfg(test)]
+mod tests;
 mod write;
 
 pub use attached_file::*;
@@ -70,26 +72,12 @@ pub struct MatroskaTagKey<'a>(TargetType, Cow<'a, str>);
 
 impl MatroskaTag {
 	fn get(&self, key: MatroskaTagKey<'_>) -> Option<&SimpleTag<'_>> {
-		fn tag_matches_target(tag: &Tag<'_>, target_type: TargetType) -> bool {
-			let Some(target) = &tag.target else {
-				// An empty target is implicitly `Album`
-				return target_type == TargetType::Album;
-			};
-
-			target.is_candidate_for_type(target_type)
-		}
-
 		let MatroskaTagKey(target, key) = key;
 
-		let applicable_tags = self
-			.tags
-			.iter()
-			.filter(|tag| tag_matches_target(tag, target));
+		let applicable_tags = self.tags.iter().filter(|tag| tag.matches_target(target));
 		for applicable_tag in applicable_tags {
 			for item in applicable_tag.simple_tags.iter() {
-				if item.name == key
-					&& (item.language.is_none()
-						|| matches!(&item.language, Some(Language::Iso639_2(l)) if l == "und"))
+				if item.name == key && matches!(&item.language, Language::Iso639_2(l) if l == "und")
 				{
 					return Some(item);
 				}
@@ -99,6 +87,33 @@ impl MatroskaTag {
 		None
 	}
 
+	fn get_or_insert_tag_for_type<'a>(
+		&'a mut self,
+		target_type: TargetType,
+	) -> &'a mut Tag<'static> {
+		let mut pos = None;
+		if let Some(applicable_tag_pos) = self
+			.tags
+			.iter()
+			.position(|tag| tag.matches_target(target_type))
+		{
+			pos = Some(applicable_tag_pos);
+		}
+
+		if pos.is_none() {
+			pos = Some(self.tags.len());
+
+			let mut new_tag = Tag::default();
+			if target_type != TargetType::Album {
+				new_tag.target = Some(Target::from(target_type));
+			}
+
+			self.tags.push(new_tag);
+		}
+
+		self.tags.get_mut(pos.unwrap()).unwrap()
+	}
+
 	fn get_str(&self, key: MatroskaTagKey<'_>) -> Option<Cow<'_, str>> {
 		let simple_tag = self.get(key)?;
 		simple_tag.get_str().map(Cow::from)
@@ -229,8 +244,12 @@ impl Accessor for MatroskaTag {
 	);
 
 	fn track(&self) -> Option<u32> {
-		// `PART_NUMBER` at level Track
-		todo!()
+		self.get(MatroskaTagKey(
+			TargetType::Track,
+			Cow::Borrowed("PART_NUMBER"),
+		))
+		.and_then(SimpleTag::get_str)
+		.and_then(|val| val.parse::<u32>().ok())
 	}
 
 	fn set_track(&mut self, _value: u32) {
@@ -242,8 +261,12 @@ impl Accessor for MatroskaTag {
 	}
 
 	fn track_total(&self) -> Option<u32> {
-		// `TOTAL_PARTS` at level album
-		todo!()
+		self.get(MatroskaTagKey(
+			TargetType::Album,
+			Cow::Borrowed("TOTAL_PARTS"),
+		))
+		.and_then(SimpleTag::get_str)
+		.and_then(|val| val.parse::<u32>().ok())
 	}
 
 	fn set_track_total(&mut self, _value: u32) {
@@ -318,19 +341,6 @@ impl TagExt for MatroskaTag {
 		todo!()
 	}
 
-	fn remove_from_path<P: AsRef<Path>>(&self, _path: P) -> std::result::Result<(), Self::Err> {
-		todo!()
-	}
-
-	fn remove_from<F>(&self, _file: &mut F) -> std::result::Result<(), Self::Err>
-	where
-		F: FileLike,
-		LoftyError: From<<F as Truncate>::Error>,
-		LoftyError: From<<F as Length>::Error>,
-	{
-		todo!()
-	}
-
 	fn clear(&mut self) {
 		self.tags.clear();
 		self.attached_files.clear();
diff --git a/lofty/src/ebml/tag/simple_tag.rs b/lofty/src/ebml/tag/simple_tag.rs
@@ -1,7 +1,7 @@
-use std::borrow::Cow;
-
 use crate::tag::ItemValue;
 
+use std::borrow::Cow;
+
 /// The language of a [`SimpleTag`] or chapter
 ///
 /// Notes:
@@ -157,7 +157,7 @@ pub struct SimpleTag<'a> {
 	/// The language of the tag
 	///
 	/// See [`Language`] for more information.
-	pub language: Option<Language>,
+	pub language: Language,
 	/// Whether [`language`] is the default/original language to use
 	///
 	/// This is used when multiple languages are present in a file. Otherwise, this
@@ -188,7 +188,7 @@ impl<'a> SimpleTag<'a> {
 	{
 		Self {
 			name: name.into(),
-			language: None,
+			language: Language::default(),
 			default: false,
 			value: Some(value.into()),
 		}
@@ -241,7 +241,7 @@ impl<'a> SimpleTag<'a> {
 // Used in conversions
 pub(super) const TOMBSTONE_SIMPLE_TAG: SimpleTag<'static> = SimpleTag {
 	name: Cow::Borrowed(""),
-	language: None,
+	language: Language::Iso639_2(String::new()),
 	default: false,
 	value: None,
 };
diff --git a/lofty/src/ebml/tag/tag.rs b/lofty/src/ebml/tag/tag.rs
@@ -1,4 +1,5 @@
-use super::{Language, SimpleTag, Target};
+use super::simple_tag::SimpleTag;
+use super::target::{Target, TargetDescriptor, TargetType};
 
 /// A single metadata descriptor.
 ///
@@ -24,7 +25,7 @@ pub struct Tag<'a> {
 }
 
 impl<'a> Tag<'a> {
-	/// Get all [`SimpleTag`]s with `name` and `language`
+	/// Get all [`SimpleTag`]s with `name`
 	///
 	/// # Example
 	///
@@ -40,18 +41,12 @@ impl<'a> Tag<'a> {
 	/// 	],
 	/// };
 	///
-	/// assert_eq!(tag.get("TITLE", None).count(), 1);
-	/// assert_eq!(tag.get("ARTIST", None).count(), 1);
-	/// assert_eq!(tag.get("SOMETHING_ELSE", None).count(), 0);
+	/// assert_eq!(tag.get("TITLE").count(), 1);
+	/// assert_eq!(tag.get("ARTIST").count(), 1);
+	/// assert_eq!(tag.get("SOMETHING_ELSE").count(), 0);
 	/// ```
-	pub fn get(
-		&'a self,
-		name: &'a str,
-		language: Option<Language>,
-	) -> impl Iterator<Item = &'a SimpleTag<'a>> {
-		self.simple_tags
-			.iter()
-			.filter(move |tag| tag.name == name && tag.language == language)
+	pub fn get(&'a self, name: &'a str) -> impl Iterator<Item = &'a SimpleTag<'a>> {
+		self.simple_tags.iter().filter(move |tag| tag.name == name)
 	}
 
 	/// Get the number of simple tags in this tag.
@@ -92,6 +87,18 @@ impl<'a> Tag<'a> {
 		self.simple_tags.is_empty()
 	}
 
+	/// Whether the tag can be used solely by the TargetType (its target is not bound to any uids)
+	///
+	/// This is used by `MatroskaTag::get` to find applicable tags for `Accessor` methods
+	pub(crate) fn matches_target(&self, target_type: TargetType) -> bool {
+		let Some(target) = &self.target else {
+			// An empty target is implicitly `Album`
+			return target_type == TargetType::Album;
+		};
+
+		target.is_candidate_for_type(target_type)
+	}
+
 	pub(crate) fn into_owned(self) -> Tag<'static> {
 		Tag {
 			target: self.target,
diff --git a/lofty/src/ebml/tag/target.rs b/lofty/src/ebml/tag/target.rs
@@ -131,3 +131,37 @@ impl Target {
 			|| self.attachment_uids.is_some()
 	}
 }
+
+/// Used to simplify conversions when writing a generic `Tag`, where extra Target information
+/// will, of course, not be available.
+pub(crate) enum TargetDescriptor<'a> {
+	Basic(TargetType),
+	Full(&'a Target),
+}
+
+impl TargetDescriptor<'_> {
+	pub(crate) fn target_type(&self) -> TargetType {
+		match self {
+			Self::Basic(ty) => *ty,
+			Self::Full(target) => target.target_type,
+		}
+	}
+
+	pub(crate) fn is_empty_candidate(&self) -> bool {
+		match self {
+			Self::Basic(ty) if *ty == TargetType::Album => true,
+			Self::Full(target) => target.is_empty_candidate(),
+			_ => false,
+		}
+	}
+}
+
+impl<'a> From<&'a Target> for TargetDescriptor<'a> {
+	fn from(target: &'a Target) -> Self {
+		if !target.has_uids() {
+			return TargetDescriptor::Basic(target.target_type);
+		}
+
+		TargetDescriptor::Full(target)
+	}
+}
diff --git a/lofty/src/ebml/tag/tests.rs b/lofty/src/ebml/tag/tests.rs
@@ -0,0 +1,22 @@
+use crate::ebml::MatroskaTag;
+use crate::prelude::ItemKey;
+use crate::tag::{Accessor, Tag, TagType};
+
+#[test_log::test]
+fn tag_to_matroska_tag() {
+	let mut tag = Tag::new(TagType::Matroska);
+
+	tag.insert_text(ItemKey::TrackArtist, String::from("Foo artist"));
+	tag.insert_text(ItemKey::TrackTitle, String::from("Bar title"));
+	tag.insert_text(ItemKey::AlbumTitle, String::from("Baz album"));
+	tag.insert_text(ItemKey::TrackNumber, String::from("1"));
+	tag.insert_text(ItemKey::TrackTotal, String::from("2"));
+
+	let matroska_tag: MatroskaTag = tag.into();
+
+	assert_eq!(matroska_tag.artist().as_deref(), Some("Foo artist"));
+	assert_eq!(matroska_tag.title().as_deref(), Some("Bar title"));
+	assert_eq!(matroska_tag.album().as_deref(), Some("Baz album"));
+	assert_eq!(matroska_tag.track(), Some(1));
+	assert_eq!(matroska_tag.track_total(), Some(2));
+}
diff --git a/lofty/src/ebml/tag/write/elements/simple_tag.rs b/lofty/src/ebml/tag/write/elements/simple_tag.rs
diff --git a/lofty/src/ebml/tag/write/elements/target.rs b/lofty/src/ebml/tag/write/elements/target.rs
