From 7b15f9bffbc50b73dcd4f4e2e8eefb1834358ff3 Mon Sep 17 00:00:00 2001
From: Nghia <git@vnghia.com>
Date: Fri, 3 Jan 2025 17:07:31 +0100
Subject: [PATCH] backend: add support for unsync external lyrics (#633)

* add missing required field for lyrics

* add support for unsync external lyrics

* add test for get lyrics by song id
---
 .../media_retrieval/get_lyrics_by_song_id.rs  |   6 +-
 nghe-backend/src/file/lyric/mod.rs            | 191 ++++++++++--------
 .../media_retrieval/get_lyrics_by_song_id.rs  |  56 ++++-
 .../src/test/mock_impl/information.rs         |  10 +-
 4 files changed, 165 insertions(+), 98 deletions(-)

diff --git a/nghe-api/src/media_retrieval/get_lyrics_by_song_id.rs b/nghe-api/src/media_retrieval/get_lyrics_by_song_id.rs
index 0089e6fd..26337713 100644
--- a/nghe-api/src/media_retrieval/get_lyrics_by_song_id.rs
+++ b/nghe-api/src/media_retrieval/get_lyrics_by_song_id.rs
@@ -14,15 +14,17 @@ pub struct Line {
 }
 
 #[api_derive]
-pub struct Lyrics {
+pub struct Lyric {
+    pub display_title: Option<String>,
     pub lang: String,
     pub synced: bool,
+    pub offset: u32,
     pub line: Vec<Line>,
 }
 
 #[api_derive]
 pub struct LyricsList {
-    pub structured_lyrics: Vec<Lyrics>,
+    pub structured_lyrics: Vec<Lyric>,
 }
 
 #[api_derive]
diff --git a/nghe-backend/src/file/lyric/mod.rs b/nghe-backend/src/file/lyric/mod.rs
index 75c14492..36a517f9 100644
--- a/nghe-backend/src/file/lyric/mod.rs
+++ b/nghe-backend/src/file/lyric/mod.rs
@@ -7,7 +7,7 @@ use diesel_async::RunQueryDsl;
 use futures_lite::{StreamExt as _, stream};
 use isolang::Language;
 use lofty::id3::v2::{BinaryFrame, SynchronizedTextFrame, UnsynchronizedTextFrame};
-use typed_path::Utf8TypedPath;
+use typed_path::{Utf8TypedPath, Utf8TypedPathBuf};
 use uuid::Uuid;
 
 use crate::database::Database;
@@ -37,6 +37,12 @@ impl<'a> FromIterator<&'a str> for Lines<'a> {
     }
 }
 
+impl FromIterator<String> for Lines<'_> {
+    fn from_iter<T: IntoIterator<Item = String>>(iter: T) -> Self {
+        Self::Unsync(iter.into_iter().map(Cow::Owned).collect())
+    }
+}
+
 impl FromIterator<(u32, String)> for Lines<'_> {
     fn from_iter<T: IntoIterator<Item = (u32, String)>>(iter: T) -> Self {
         Self::Sync(iter.into_iter().map(|(duration, text)| (duration, text.into())).collect())
@@ -71,7 +77,7 @@ impl<'a> TryFrom<&'a BinaryFrame<'_>> for Lyric<'a> {
 
 impl<'a> Lyric<'a> {
     pub fn from_unsync_text(content: &'a str) -> Self {
-        let lines = content.lines().filter(|text| !text.is_empty());
+        let lines = content.lines();
         let (description, lines) = if cfg!(test) && content.starts_with('#') {
             let mut lines = lines;
             let description = lines.next().unwrap().strip_prefix('#').unwrap();
@@ -103,11 +109,23 @@ impl<'a> Lyric<'a> {
                 .collect(),
         })
     }
+
+    pub fn into_owned(self) -> Lyric<'static> {
+        Lyric {
+            description: self.description.map(Cow::into_owned).map(Cow::Owned),
+            language: self.language,
+            lines: match self.lines {
+                Lines::Unsync(lines) => lines.into_iter().map(Cow::into_owned).collect(),
+                Lines::Sync(lines) => lines
+                    .into_iter()
+                    .map(|(duration, text)| (duration, text.into_owned()))
+                    .collect(),
+            },
+        }
+    }
 }
 
 impl Lyric<'_> {
-    pub const EXTERNAL_EXTENSION: &'static str = "lrc";
-
     pub async fn upsert(
         &self,
         database: &Database,
@@ -128,32 +146,85 @@ impl Lyric<'_> {
             .await
     }
 
-    async fn set_external_scanned_at(
+    pub async fn cleanup_one_external(
         database: &Database,
+        started_at: time::OffsetDateTime,
         song_id: Uuid,
-    ) -> Result<Option<Uuid>, Error> {
-        diesel::update(lyrics::table)
+    ) -> Result<(), Error> {
+        // Delete all lyrics of a song which haven't been refreshed since timestamp.
+        diesel::delete(lyrics::table)
             .filter(lyrics::song_id.eq(song_id))
+            .filter(lyrics::scanned_at.lt(started_at))
             .filter(lyrics::external)
-            .set(lyrics::scanned_at.eq(crate::time::now().await))
-            .returning(lyrics::id)
-            .get_result(&mut database.get().await?)
-            .await
-            .optional()
-            .map_err(Error::from)
+            .execute(&mut database.get().await?)
+            .await?;
+        Ok(())
+    }
+
+    pub async fn cleanup_one(
+        database: &Database,
+        started_at: time::OffsetDateTime,
+        song_id: Uuid,
+    ) -> Result<(), Error> {
+        // Delete all lyrics of a song which haven't been refreshed since timestamp.
+        diesel::delete(lyrics::table)
+            .filter(lyrics::song_id.eq(song_id))
+            .filter(lyrics::scanned_at.lt(started_at))
+            .execute(&mut database.get().await?)
+            .await?;
+        Ok(())
+    }
+}
+
+impl Lyric<'static> {
+    const EXTERNAL_SYNC_EXTENSION: &'static str = "lrc";
+    const EXTERNAL_UNSYNC_EXTENSION: &'static str = "txt";
+
+    fn path(path: Utf8TypedPath<'_>, sync: bool) -> Utf8TypedPathBuf {
+        path.with_extension(if sync {
+            Self::EXTERNAL_SYNC_EXTENSION
+        } else {
+            Self::EXTERNAL_UNSYNC_EXTENSION
+        })
+    }
+
+    fn from_text(content: &str, sync: bool) -> Result<Self, Error> {
+        if sync {
+            Self::from_sync_text(content)
+        } else {
+            Ok(Lyric::from_unsync_text(content).into_owned())
+        }
     }
 
     pub async fn load(
         filesystem: &filesystem::Impl<'_>,
         path: Utf8TypedPath<'_>,
+        sync: bool,
     ) -> Result<Option<Self>, Error> {
+        let path = Self::path(path, sync);
+        let path = path.to_path();
         if filesystem.exists(path).await? {
             let content = filesystem.read_to_string(path).await?;
-            return Ok(Some(Self::from_sync_text(&content)?));
+            return Ok(Some(Self::from_text(&content, sync)?));
         }
         Ok(None)
     }
 
+    async fn set_external_scanned_at(
+        database: &Database,
+        song_id: Uuid,
+    ) -> Result<Option<Uuid>, Error> {
+        diesel::update(lyrics::table)
+            .filter(lyrics::song_id.eq(song_id))
+            .filter(lyrics::external)
+            .set(lyrics::scanned_at.eq(crate::time::now().await))
+            .returning(lyrics::id)
+            .get_result(&mut database.get().await?)
+            .await
+            .optional()
+            .map_err(Error::from)
+    }
+
     pub async fn scan(
         database: &Database,
         filesystem: &filesystem::Impl<'_>,
@@ -166,45 +237,15 @@ impl Lyric<'_> {
                 && let Some(lyrics_id) = Self::set_external_scanned_at(database, song_id).await?
             {
                 Some(lyrics_id)
-            } else if let Some(lyrics) =
-                Self::load(filesystem, song_path.with_extension(Self::EXTERNAL_EXTENSION).to_path())
-                    .await?
-            {
+            } else if let Some(lyrics) = Self::load(filesystem, song_path, true).await? {
+                Some(lyrics.upsert(database, lyrics::Foreign { song_id }, true).await?)
+            } else if let Some(lyrics) = Self::load(filesystem, song_path, false).await? {
                 Some(lyrics.upsert(database, lyrics::Foreign { song_id }, true).await?)
             } else {
                 None
             },
         )
     }
-
-    pub async fn cleanup_one_external(
-        database: &Database,
-        started_at: time::OffsetDateTime,
-        song_id: Uuid,
-    ) -> Result<(), Error> {
-        // Delete all lyrics of a song which haven't been refreshed since timestamp.
-        diesel::delete(lyrics::table)
-            .filter(lyrics::song_id.eq(song_id))
-            .filter(lyrics::scanned_at.lt(started_at))
-            .filter(lyrics::external)
-            .execute(&mut database.get().await?)
-            .await?;
-        Ok(())
-    }
-
-    pub async fn cleanup_one(
-        database: &Database,
-        started_at: time::OffsetDateTime,
-        song_id: Uuid,
-    ) -> Result<(), Error> {
-        // Delete all lyrics of a song which haven't been refreshed since timestamp.
-        diesel::delete(lyrics::table)
-            .filter(lyrics::song_id.eq(song_id))
-            .filter(lyrics::scanned_at.lt(started_at))
-            .execute(&mut database.get().await?)
-            .await?;
-        Ok(())
-    }
 }
 
 #[cfg(test)]
@@ -219,20 +260,15 @@ mod test {
     use lofty::id3::v2::Frame;
 
     use super::*;
-    use crate::test::Mock;
-
-    impl FromIterator<String> for Lines<'_> {
-        fn from_iter<T: IntoIterator<Item = String>>(iter: T) -> Self {
-            Self::Unsync(iter.into_iter().map(Cow::Owned).collect())
-        }
-    }
+    use crate::test::filesystem::Trait as _;
+    use crate::test::{Mock, filesystem};
 
     impl Lyric<'_> {
         pub fn is_sync(&self) -> bool {
             matches!(self.lines, Lines::Sync(_))
         }
 
-        pub fn fake_sync() -> Self {
+        fn fake_sync() -> Self {
             // Force description as Some to avoid clash with unsync.
             Self {
                 description: Some(Faker.fake::<String>().into()),
@@ -253,7 +289,7 @@ mod test {
             }
         }
 
-        pub fn fake_unsync() -> Self {
+        fn fake_unsync() -> Self {
             Self {
                 description: Faker.fake::<Option<String>>().map(Cow::Owned),
                 language: Language::Und,
@@ -261,11 +297,21 @@ mod test {
             }
         }
 
+        pub fn fake(sync: bool) -> Self {
+            if sync { Self::fake_sync() } else { Self::fake_unsync() }
+        }
+
         pub fn fake_vec() -> Vec<Self> {
-            let unsync = if Faker.fake() { Some(Self::fake_unsync()) } else { None };
-            let sync = if Faker.fake() { Some(Self::fake_sync()) } else { None };
+            let unsync = if Faker.fake() { Some(Self::fake(false)) } else { None };
+            let sync = if Faker.fake() { Some(Self::fake(true)) } else { None };
             unsync.into_iter().chain(sync).collect()
         }
+
+        pub async fn dump(&self, filesystem: &filesystem::Impl<'_>, path: Utf8TypedPath<'_>) {
+            let path = Lyric::path(path, self.is_sync());
+            let content = self.to_string();
+            filesystem.write(path.to_path(), content.as_bytes()).await;
+        }
     }
 
     impl Lyric<'static> {
@@ -308,7 +354,7 @@ mod test {
 
     impl Dummy<Faker> for Lyric<'_> {
         fn dummy_with_rng<R: fake::rand::Rng + ?Sized>(config: &Faker, rng: &mut R) -> Self {
-            if config.fake_with_rng(rng) { Self::fake_sync() } else { Self::fake_unsync() }
+            Self::fake(config.fake_with_rng(rng))
         }
     }
 
@@ -386,13 +432,8 @@ mod tests {
 
     #[rstest]
     fn test_lyrics_roundtrip(#[values(true, false)] sync: bool) {
-        if sync {
-            let lyrics = Lyric::fake_sync();
-            assert_eq!(lyrics, Lyric::from_sync_text(&lyrics.to_string()).unwrap());
-        } else {
-            let lyrics = Lyric::fake_unsync();
-            assert_eq!(lyrics, Lyric::from_unsync_text(&lyrics.to_string()));
-        }
+        let lyrics = Lyric::fake(sync);
+        assert_eq!(lyrics, Lyric::from_text(&lyrics.to_string(), sync).unwrap());
     }
 
     #[rstest]
@@ -413,6 +454,7 @@ mod tests {
         lines: vec![
             "Hello hi",
             "Bonjour salut",
+            "",
             "おはよう こんにちは",
         ]
         .into_iter()
@@ -420,11 +462,7 @@ mod tests {
     })]
     fn test_from_text(#[case] filename: &str, #[case] lyrics: Lyric<'_>) {
         let content = std::fs::read_to_string(assets::dir().join("lyrics").join(filename)).unwrap();
-        let parsed = if lyrics.is_sync() {
-            Lyric::from_sync_text(&content).unwrap()
-        } else {
-            Lyric::from_unsync_text(&content)
-        };
+        let parsed = Lyric::from_text(&content, lyrics.is_sync()).unwrap();
         assert_eq!(parsed, lyrics);
     }
 
@@ -487,17 +525,12 @@ mod tests {
         let lyric: Lyric = Faker.fake();
         let id = lyric.upsert(mock.database(), lyrics::Foreign { song_id }, true).await.unwrap();
 
-        let new_lyric = Lyric::fake_sync();
+        let new_lyric: Lyric = Faker.fake();
 
-        let filesystem = music_folder.to_impl();
+        let filesystem = &music_folder.to_impl();
         let path = filesystem.prefix().join("test");
         let path = path.to_path();
-        filesystem
-            .write(
-                path.with_extension(Lyric::EXTERNAL_EXTENSION).to_path(),
-                new_lyric.to_string().as_bytes(),
-            )
-            .await;
+        new_lyric.dump(filesystem, path).await;
         let scanned_id = Lyric::scan(mock.database(), &filesystem.main(), full, song_id, path)
             .await
             .unwrap()
diff --git a/nghe-backend/src/route/media_retrieval/get_lyrics_by_song_id.rs b/nghe-backend/src/route/media_retrieval/get_lyrics_by_song_id.rs
index 5f3de0b4..8607703b 100644
--- a/nghe-backend/src/route/media_retrieval/get_lyrics_by_song_id.rs
+++ b/nghe-backend/src/route/media_retrieval/get_lyrics_by_song_id.rs
@@ -1,7 +1,9 @@
+use std::borrow::Cow;
+
 use diesel::{ExpressionMethods, QueryDsl, SelectableHelper};
 use diesel_async::RunQueryDsl;
 use itertools::{EitherOrBoth, Itertools};
-use nghe_api::media_retrieval::get_lyrics_by_song_id::{Line, Lyrics, LyricsList};
+use nghe_api::media_retrieval::get_lyrics_by_song_id::{Line, Lyric, LyricsList};
 pub use nghe_api::media_retrieval::get_lyrics_by_song_id::{Request, Response};
 use nghe_proc_macro::handler;
 
@@ -21,14 +23,19 @@ pub async fn handler(database: &Database, request: Request) -> Result<Response,
         lyrics_list: LyricsList {
             structured_lyrics: lyrics
                 .into_iter()
-                .map(|lyrics| -> Result<_, Error> {
-                    if let Some(durations) = lyrics.durations {
-                        Ok(Lyrics {
-                            lang: lyrics.language.into_owned(),
+                .map(|lyric| -> Result<_, Error> {
+                    let display_title = lyric.description.map(Cow::into_owned);
+                    let lang = lyric.language.into_owned();
+
+                    if let Some(durations) = lyric.durations {
+                        Ok(Lyric {
+                            display_title,
+                            lang,
                             synced: true,
+                            offset: 0,
                             line: durations
                                 .into_iter()
-                                .zip_longest(lyrics.texts.into_iter())
+                                .zip_longest(lyric.texts.into_iter())
                                 .map(|iter| {
                                     if let EitherOrBoth::Both(duration, text) = iter {
                                         Ok(Line {
@@ -42,10 +49,12 @@ pub async fn handler(database: &Database, request: Request) -> Result<Response,
                                 .try_collect()?,
                         })
                     } else {
-                        Ok(Lyrics {
-                            lang: lyrics.language.into_owned(),
+                        Ok(Lyric {
+                            display_title,
+                            lang,
                             synced: false,
-                            line: lyrics
+                            offset: 0,
+                            line: lyric
                                 .texts
                                 .into_iter()
                                 .map(|text| Line { start: None, value: text.into_owned() })
@@ -57,3 +66,32 @@ pub async fn handler(database: &Database, request: Request) -> Result<Response,
         },
     })
 }
+
+#[cfg(test)]
+#[coverage(off)]
+mod tests {
+    use rstest::rstest;
+
+    use super::*;
+    use crate::test::{Mock, mock};
+
+    #[rstest]
+    #[tokio::test]
+    async fn test_handler(#[future(awt)] mock: Mock) {
+        let mut music_folder = mock.music_folder(0).await;
+        let (id, audio) = music_folder.add_audio().call().await.database.get_index(0).unwrap();
+
+        let n_lyrics =
+            usize::from(audio.external_lyric.is_some()) + audio.information.metadata.lyrics.len();
+
+        assert_eq!(
+            handler(mock.database(), Request { id: *id })
+                .await
+                .unwrap()
+                .lyrics_list
+                .structured_lyrics
+                .len(),
+            n_lyrics
+        );
+    }
+}
diff --git a/nghe-backend/src/test/mock_impl/information.rs b/nghe-backend/src/test/mock_impl/information.rs
index 7c94b2d3..0d0703fa 100644
--- a/nghe-backend/src/test/mock_impl/information.rs
+++ b/nghe-backend/src/test/mock_impl/information.rs
@@ -101,8 +101,7 @@ impl Mock<'static, 'static, 'static, 'static> {
         });
         let property = property.unwrap_or_else(|| audio::Property::default(file.format));
 
-        let external_lyric = external_lyric
-            .unwrap_or_else(|| if Faker.fake() { Some(lyric::Lyric::fake_sync()) } else { None });
+        let external_lyric = external_lyric.unwrap_or_else(|| Faker.fake());
         let dir_picture = dir_picture.unwrap_or_else(|| Faker.fake());
         let relative_path =
             relative_path.map_or_else(|| Faker.fake::<String>().into(), std::convert::Into::into);
@@ -187,12 +186,7 @@ impl Mock<'_, '_, '_, '_> {
         filesystem.write(path, &data).await;
 
         if let Some(external_lyric) = self.external_lyric.as_ref() {
-            filesystem
-                .write(
-                    path.with_extension(lyric::Lyric::EXTERNAL_EXTENSION).to_path(),
-                    external_lyric.to_string().as_bytes(),
-                )
-                .await;
+            external_lyric.dump(filesystem, path).await;
         }
 
         let cover_art_config = &music_folder.config.cover_art;