From fa31213e455f123f09396d720a801784073944d2 Mon Sep 17 00:00:00 2001
From: Wojciech Kozlowski <wk@wojciechkozlowski.eu>
Date: Mon, 10 Apr 2023 20:45:36 +0200
Subject: [PATCH] Make the SystemExecutor unsafe

---
 src/database/json.rs   |  4 +++-
 src/library/beets.rs   | 37 +++++++++++++++++++++++++++++--------
 src/main.rs            |  2 +-
 tests/library/beets.rs | 13 ++++++++-----
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/src/database/json.rs b/src/database/json.rs
index 03e1830..d6ad01b 100644
--- a/src/database/json.rs
+++ b/src/database/json.rs
@@ -17,12 +17,13 @@ pub trait DatabaseJsonBackend {
     fn write(&mut self, json: &str) -> Result<(), std::io::Error>;
 }
 
-/// A JSON file database.
+/// JSON database.
 pub struct DatabaseJson {
     backend: Box<dyn DatabaseJsonBackend + Send>,
 }
 
 impl DatabaseJson {
+    /// Create a new JSON database with the provided executor, e.g. [DatabaseJsonFile].
     pub fn new(backend: Box<dyn DatabaseJsonBackend + Send>) -> Self {
         DatabaseJson { backend }
     }
@@ -50,6 +51,7 @@ impl DatabaseWrite for DatabaseJson {
     }
 }
 
+/// JSON database that uses a local file for persistent storage.
 pub struct DatabaseJsonFile {
     path: PathBuf,
 }
diff --git a/src/library/beets.rs b/src/library/beets.rs
index f5b1531..f74a098 100644
--- a/src/library/beets.rs
+++ b/src/library/beets.rs
@@ -106,6 +106,7 @@ trait LibraryPrivate {
 }
 
 impl Beets {
+    /// Create a new beets library instance with the provided executor, e.g. [SystemExecutor].
     pub fn new(executor: Box<dyn BeetsExecutor + Send>) -> Beets {
         Beets { executor }
     }
@@ -236,31 +237,51 @@ impl LibraryPrivate for Beets {
 }
 
 /// Executor for executing beets commands on the local system.
+///
+/// # Safety
+///
+/// The beets executable is not safe to call concurrently for operations on the same
+/// database/library. Therefore, all functions that create a [SystemExecutor] or modify which
+/// library it works with are marked unsafe. It is the caller's responsibility to make sure that
+/// only one instance can access one particular library at a time.
 pub struct SystemExecutor {
     bin: String,
     config: Option<PathBuf>,
 }
 
 impl SystemExecutor {
-    pub fn new(bin: &str) -> Self {
+    /// Create a new [SystemExecutor] that uses the provided beets executable.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure the library is not being concurrently accessed from anywhere else.
+    pub unsafe fn new(bin: &str) -> Self {
         SystemExecutor {
             bin: bin.to_string(),
             config: None,
         }
     }
 
-    pub fn config(mut self, path: Option<&Path>) -> Self {
+    /// Create a new [SystemExecutor] that uses the system's default beets executable.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure the library is not being concurrently accessed from anywhere else.
+    pub unsafe fn default() -> Self {
+        SystemExecutor::new("beet")
+    }
+
+    /// Update the configuration file for the beets executable.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure the library is not being concurrently accessed from anywhere else.
+    pub unsafe fn config(mut self, path: Option<&Path>) -> Self {
         self.config = path.map(|p| p.to_path_buf());
         self
     }
 }
 
-impl Default for SystemExecutor {
-    fn default() -> Self {
-        SystemExecutor::new("beet")
-    }
-}
-
 impl BeetsExecutor for SystemExecutor {
     fn exec(&mut self, arguments: &[String]) -> Result<Vec<String>, Error> {
         let mut cmd = Command::new(&self.bin);
diff --git a/src/main.rs b/src/main.rs
index 0cfcfc8..ee8abe5 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -37,7 +37,7 @@ fn main() {
     let opt = Opt::from_args();
 
     let mut beets = Beets::new(Box::new(
-        SystemExecutor::default().config(opt.beets_config_file_path.as_deref()),
+        unsafe { SystemExecutor::default().config(opt.beets_config_file_path.as_deref()) },
     ));
 
     let collection = beets
diff --git a/tests/library/beets.rs b/tests/library/beets.rs
index be06b02..a290500 100644
--- a/tests/library/beets.rs
+++ b/tests/library/beets.rs
@@ -15,15 +15,18 @@ use musichoard::{
 
 use crate::COLLECTION;
 
-static BEETS_EMPTY_CONFIG: Lazy<Arc<Mutex<Beets>>> =
-    Lazy::new(|| Arc::new(Mutex::new(Beets::new(Box::new(SystemExecutor::default())))));
+static BEETS_EMPTY_CONFIG: Lazy<Arc<Mutex<Beets>>> = Lazy::new(|| {
+    Arc::new(Mutex::new(Beets::new(Box::new(unsafe {
+        SystemExecutor::default()
+    }))))
+});
 
 static BEETS_TEST_CONFIG: Lazy<Arc<Mutex<Beets>>> = Lazy::new(|| {
-    Arc::new(Mutex::new(Beets::new(Box::new(
+    Arc::new(Mutex::new(Beets::new(Box::new(unsafe {
         SystemExecutor::default().config(Some(
             &fs::canonicalize("./tests/files/library/config.yml").unwrap(),
-        )),
-    ))))
+        ))
+    }))))
 });
 
 #[test]