fix(bookmarks): don't write to the bookmarks file on every bookmark

src/play.rs

@@ -65,6 +65,9 @@ pub async fn play(args: Args) -> eyre::Result<(), player::Error> {
         .await
         .map_err(player::Error::PersistentVolumeSave)?;
 
+    // Save the bookmarks for the next session.
+    player.bookmarks.save().await?;
+
     drop(stream);
     player.sink.stop();
     ui.map(|x| x.abort());

src/player.rs

@@ -2,14 +2,7 @@
 //! This also has the code for the underlying
 //! audio server which adds new tracks.
 
-use std::{
+use std::{collections::VecDeque, sync::Arc, time::Duration};
-    collections::VecDeque,
-    sync::{
-        atomic::{AtomicBool, Ordering},
-        Arc,
-    },
-    time::Duration,
-};
 
 use arc_swap::ArcSwapOption;
 use downloader::Downloader;
@@ -29,7 +22,7 @@ use mpris_server::{PlaybackStatus, PlayerInterface, Property};
 
 use crate::{
     messages::Message,
-    player::{self, persistent_volume::PersistentVolume},
+    player::{self, bookmark::Bookmarks, persistent_volume::PersistentVolume},
     tracks::{self, list::List},
     Args,
 };
@@ -64,9 +57,6 @@ pub struct Player {
     /// The internal buffer size.
     pub buffer_size: usize,
 
-    /// Whether the current track has been bookmarked.
-    bookmarked: AtomicBool,
-
     /// The [`TrackInfo`] of the current track.
     /// This is [`None`] when lowfi is buffering/loading.
     current: ArcSwapOption<tracks::Info>,
@@ -77,6 +67,9 @@ pub struct Player {
     /// This is populated specifically by the [Downloader].
     tracks: RwLock<VecDeque<tracks::QueuedTrack>>,
 
+    /// The bookmarks, which are saved on quit.
+    pub bookmarks: Bookmarks,
+
     /// The actual list of tracks to be played.
     list: List,
 
@@ -108,6 +101,9 @@ impl Player {
     ///
     /// This also will load the track list & persistent volume.
     pub async fn new(args: &Args) -> eyre::Result<(Self, OutputStream), player::Error> {
+        // Load the bookmarks.
+        let bookmarks = Bookmarks::load().await?;
+
         // Load the volume file.
         let volume = PersistentVolume::load()
             .await
@@ -146,11 +142,11 @@ impl Player {
             tracks: RwLock::new(VecDeque::with_capacity(args.buffer_size)),
             buffer_size: args.buffer_size,
             current: ArcSwapOption::new(None),
+            bookmarks,
             client,
             sink,
             volume,
             list,
-            bookmarked: AtomicBool::new(false),
         };
 
         Ok((player, stream))
@@ -223,7 +219,7 @@ impl Player {
 
             match msg {
                 Message::Next | Message::Init | Message::TryAgain => {
-                    player.bookmarked.swap(false, Ordering::Relaxed);
+                    player.bookmarks.set_bookmarked(false);
 
                     // We manually skipped, so we shouldn't actually wait for the song
                     // to be over until we recieve the `NewSong` signal.
@@ -297,18 +293,17 @@ impl Player {
                     let current = player.current.load();
                     let current = current.as_ref().unwrap();
 
-                    let bookmarked = bookmark::bookmark(
+                    player
-                        current.full_path.clone(),
+                        .bookmarks
-                        if current.custom_name {
+                        .bookmark(
-                            Some(current.display_name.clone())
+                            current.full_path.clone(),
-                        } else {
+                            if current.custom_name {
-                            None
+                                Some(current.display_name.clone())
-                        },
+                            } else {
-                    )
+                                None
-                    .await
+                            },
-                    .map_err(player::Error::Bookmark)?;
+                        )
-
+                        .await?;
-                    player.bookmarked.swap(bookmarked, Ordering::Relaxed);
                 }
                 Message::Quit => break,
             }

src/player/bookmark.rs

@@ -1,50 +1,108 @@
 use std::io::SeekFrom;
+use std::sync::atomic::AtomicBool;
 
-use tokio::fs::{create_dir_all, OpenOptions};
+use tokio::fs::{create_dir_all, File, OpenOptions};
-use tokio::io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt};
+use tokio::io::{self, AsyncReadExt, AsyncSeekExt, AsyncWriteExt};
+use tokio::sync::RwLock;
 
 use crate::data_dir;
 
-/// Bookmarks a given track with a full path and optional custom name.
+#[derive(Debug, thiserror::Error)]
-///
+pub enum BookmarkError {
-/// Returns whether the track is now bookmarked, or not.
+    #[error("data directory not found")]
-pub async fn bookmark(path: String, custom: Option<String>) -> eyre::Result<bool> {
+    DataDir,
-    let mut entry = path.to_string();
-    if let Some(custom) = custom {
-        entry.push('!');
-        entry.push_str(&custom);
-    }
 
-    let data_dir = data_dir()?;
+    #[error("io failure")]
-    create_dir_all(data_dir.clone()).await?;
+    Io(#[from] io::Error),
-
+}
-    // TODO: Only open and close the file at startup and shutdown, not every single bookmark.
+
-    // TODO: Sort of like PersistentVolume, but for bookmarks.
+/// Manages the bookmarks in the current player.
-    let mut file = OpenOptions::new()
+pub struct Bookmarks {
-        .create(true)
+    entries: RwLock<Vec<String>>,
-        .write(true)
+    file: RwLock<File>,
-        .read(true)
+    bookmarked: AtomicBool,
-        .append(false)
+}
-        .truncate(true)
+
-        .open(data_dir.join("bookmarks.txt"))
+impl Bookmarks {
-        .await?;
+    pub async fn load() -> eyre::Result<Self, BookmarkError> {
-
+        let data_dir = data_dir().map_err(|_| BookmarkError::DataDir)?;
-    let mut text = String::new();
+        create_dir_all(data_dir.clone()).await?;
-    file.read_to_string(&mut text).await?;
+
-
+        let mut file = OpenOptions::new()
-    let mut lines: Vec<&str> = text.trim().lines().filter(|x| !x.is_empty()).collect();
+            .create(true)
-    let idx = lines.iter().position(|x| **x == entry);
+            .write(true)
-
+            .read(true)
-    if let Some(idx) = idx {
+            .append(false)
-        lines.remove(idx);
+            .truncate(false)
-    } else {
+            .open(data_dir.join("bookmarks.txt"))
-        lines.push(&entry);
+            .await?;
-    }
+
-
+        let mut text = String::new();
-    let text = format!("\n{}", lines.join("\n"));
+        file.read_to_string(&mut text).await?;
-    file.seek(SeekFrom::Start(0)).await?;
+
-    file.set_len(0).await?;
+        let lines: Vec<String> = text
-    file.write_all(text.as_bytes()).await?;
+            .trim()
-
+            .lines()
-    Ok(idx.is_none())
+            .filter_map(|x| {
+                if !x.is_empty() {
+                    Some(x.to_string())
+                } else {
+                    None
+                }
+            })
+            .collect();
+
+        Ok(Self {
+            entries: RwLock::new(lines),
+            file: RwLock::new(file),
+            bookmarked: AtomicBool::new(false),
+        })
+    }
+
+    pub async fn save(&self) -> eyre::Result<(), BookmarkError> {
+        let text = format!("\n{}", self.entries.read().await.join("\n"));
+
+        let mut lock = self.file.write().await;
+        lock.seek(SeekFrom::Start(0)).await?;
+        lock.set_len(0).await?;
+        lock.write_all(text.as_bytes()).await?;
+        lock.flush().await?;
+
+        Ok(())
+    }
+
+    /// Bookmarks a given track with a full path and optional custom name.
+    ///
+    /// Returns whether the track is now bookmarked, or not.
+    pub async fn bookmark(
+        &self,
+        mut entry: String,
+        custom: Option<String>,
+    ) -> eyre::Result<(), BookmarkError> {
+        if let Some(custom) = custom {
+            entry.push('!');
+            entry.push_str(&custom);
+        }
+
+        let idx = self.entries.read().await.iter().position(|x| **x == entry);
+
+        if let Some(idx) = idx {
+            self.entries.write().await.remove(idx);
+        } else {
+            self.entries.write().await.push(entry);
+        };
+
+        self.set_bookmarked(idx.is_none());
+
+        Ok(())
+    }
+
+    pub fn bookmarked(&self) -> bool {
+        self.bookmarked.load(std::sync::atomic::Ordering::Relaxed)
+    }
+
+    pub fn set_bookmarked(&self, val: bool) {
+        self.bookmarked
+            .swap(val, std::sync::atomic::Ordering::Relaxed);
+    }
 }

src/player/error.rs

@@ -1,6 +1,6 @@
 use std::ffi::NulError;
 
-use crate::messages::Message;
+use crate::{messages::Message, player::bookmark::BookmarkError};
 use tokio::sync::mpsc::error::SendError;
 
 #[cfg(feature = "mpris")]
@@ -43,9 +43,9 @@ pub enum Error {
     #[error("unable to notify downloader")]
     DownloaderNotify(#[from] SendError<()>),
 
-    #[error("unable to bookmark track")]
-    Bookmark(eyre::Error),
-
     #[error("unable to find data directory")]
     DataDir,
+
+    #[error("bookmarking load/unload failed")]
+    Bookmark(#[from] BookmarkError),
 }

src/player/ui/components.rs

@@ -1,11 +1,7 @@
 //! Various different individual components that
 //! appear in lowfi's UI, like the progress bar.
 
-use std::{
+use std::{ops::Deref as _, sync::Arc, time::Duration};
-    ops::Deref as _,
-    sync::{atomic::Ordering, Arc},
-    time::Duration,
-};
 
 use crossterm::style::Stylize as _;
 use unicode_segmentation::UnicodeSegmentation as _;
@@ -108,7 +104,7 @@ pub fn action(player: &Player, current: Option<&Arc<Info>>, width: usize) -> Str
                 ActionBar::Playing(info)
             }
         })
-        .format(player.bookmarked.load(Ordering::Relaxed));
+        .format(player.bookmarks.bookmarked());
 
     if len > width {
         let chopped: String = main.graphemes(true).take(width + 1).collect();