#48: Atomic files

build.rs

@@ -11,7 +11,7 @@ fn main() {
     let t = env::var("TARGET").unwrap();
     let target = Triple::from_str(t.as_str()).unwrap();
     let out_dir = env::var_os("OUT_DIR").unwrap();
-    
+
     println!("{}", target.operating_system);
     // Avoid duplicate download
     if !fs_extra::dir::ls(&out_dir, &HashSet::new())
@@ -86,7 +86,7 @@ fn main() {
             major: _,
             minor: _,
             patch: _,
-        } 
+        }
         | OperatingSystem::Darwin => fs_extra::file::move_file(
             PathBuf::from(&out_dir)
                 .join("bin")

src/atomic/file.rs

@@ -276,40 +276,40 @@ mod tests {
     fn delete_old_files() {
         let dir = TempDir::new("max_files").unwrap();
         let root = dir.path();
-        let file = AtomicFile::new(&root).unwrap();
+        let file = AtomicFile::new(root).unwrap();
         let number_of_version = 20;
         assert!(number_of_version > MAX_VERSION_FILES);
         for i in 0..number_of_version {
             let temp = file.make_temp().unwrap();
             let current = file.load().unwrap();
             let content = format!("Version {}", i + 1);
-            (&temp).write_all(&content.as_bytes()).unwrap();
+            (&temp).write_all(content.as_bytes()).unwrap();
             file.compare_and_swap(&current, temp).unwrap();
         }
 
         // Check the number of files
-        let version_files = fs::read_dir(&root).unwrap().count();
+        let version_files = fs::read_dir(root).unwrap().count();
         assert_eq!(version_files, MAX_VERSION_FILES);
     }
 
     #[test]
     fn mutliples_version_files() {
         let dir = TempDir::new("multiples_version").unwrap();
         let root = dir.path();
-        let file = AtomicFile::new(&root).unwrap();
+        let file = AtomicFile::new(root).unwrap();
         let temp = file.make_temp().unwrap();
         let current = file.load().unwrap();
-        let current_machine = format!("Content from current machine");
+        let current_machine = "Content from current machine".to_string();
         (&temp)
-            .write_all(&current_machine.as_bytes())
+            .write_all(current_machine.as_bytes())
             .unwrap();
         file.compare_and_swap(&current, temp).unwrap();
         // Other machine file (renamed on purpose to validate test)
         let current = file.load().unwrap();
-        let other_machine = format!("Content from Cellphone");
+        let other_machine = "Content from Cellphone".to_string();
         let temp = file.make_temp().unwrap();
         (&temp)
-            .write_all(&other_machine.as_bytes())
+            .write_all(other_machine.as_bytes())
             .unwrap();
         file.compare_and_swap(&current, temp).unwrap();
         let version_2_path = file.path(2);
@@ -336,7 +336,7 @@ mod tests {
         let dir = TempDir::new(temp_name).unwrap();
         let root = dir.path();
         let current_machine = machine_uid::get().unwrap();
-        let file = AtomicFile::new(&root).unwrap();
+        let file = AtomicFile::new(root).unwrap();
         let prefix = &file.prefix;
         for version in 0..versions {
             let file_path = root.join(format!("{}{}", prefix, version + 1));

src/atomic/mod.rs

@@ -66,15 +66,15 @@ mod tests {
     fn failed_to_write_simultaneously() {
         let dir = TempDir::new("writing_test").unwrap();
         let root = dir.path();
-        let shared_file = std::sync::Arc::new(AtomicFile::new(&root).unwrap());
+        let shared_file = std::sync::Arc::new(AtomicFile::new(root).unwrap());
         let mut handles = Vec::with_capacity(5);
         for i in 0..5 {
             let file = shared_file.clone();
             let handle = std::thread::spawn(move || {
                 let temp = file.make_temp().unwrap();
                 let current = file.load().unwrap();
                 let content = format!("Content from thread {i}!");
-                (&temp).write_all(&content.as_bytes()).unwrap();
+                (&temp).write_all(content.as_bytes()).unwrap();
                 // In case slow computer ensure each thread are running in the same time
                 std::thread::sleep(std::time::Duration::from_millis(300));
                 file.compare_and_swap(&current, temp)
@@ -99,7 +99,7 @@ mod tests {
     fn multiple_writes_detected() {
         let dir = TempDir::new("simultaneous_writes").unwrap();
         let root = dir.path();
-        let shared_file = std::sync::Arc::new(AtomicFile::new(&root).unwrap());
+        let shared_file = std::sync::Arc::new(AtomicFile::new(root).unwrap());
         let thread_number = 10;
         assert!(thread_number > 3);
         // Need to have less than 255 thread to store thread number as byte directly

src/id.rs

@@ -120,14 +120,15 @@ mod tests {
     fn compute_id_test() {
         let file_path = Path::new("./tests/lena.jpg");
         let data_size = fs::metadata(file_path)
-            .expect(&format!(
-                "Could not open image test file_path.{}",
-                file_path.display()
-            ))
+            .unwrap_or_else(|_| {
+                panic!(
+                    "Could not open image test file_path.{}",
+                    file_path.display()
+                )
+            })
             .len();
 
-        let id1 = ResourceId::compute(data_size.try_into().unwrap(), file_path)
-            .unwrap();
+        let id1 = ResourceId::compute(data_size, file_path).unwrap();
         assert_eq!(id1.crc32, 0x342a3d4a);
         assert_eq!(id1.data_size, 128760);
 

src/index.rs

@@ -661,8 +661,8 @@ mod tests {
     }
 
     fn run_test_and_clean_up(
-        test: impl FnOnce(PathBuf) -> () + std::panic::UnwindSafe,
-    ) -> () {
+        test: impl FnOnce(PathBuf) + std::panic::UnwindSafe,
+    ) {
         let path = get_temp_dir();
         let result = std::panic::catch_unwind(|| test(path.clone()));
         std::fs::remove_dir_all(path.clone())
@@ -777,7 +777,7 @@ mod tests {
             assert_eq!(update.added.len(), 1);
 
             let added_key =
-                CanonicalPathBuf::canonicalize(&expected_path.clone())
+                CanonicalPathBuf::canonicalize(expected_path.clone())
                     .expect("CanonicalPathBuf should be fine");
             assert_eq!(
                 update

src/link.rs

@@ -25,6 +25,20 @@ pub struct Properties {
     pub title: String,
     pub desc: Option<String>,
 }
+/// Write data to a tempory file and move that written file to destination
+///
+/// May failed if writing or moving failed
+fn temp_and_move(
+    data: &[u8],
+    dest_dir: impl AsRef<Path>,
+    filename: &str,
+) -> Result<()> {
+    let mut path = std::env::temp_dir();
+    path.push(filename);
+    std::fs::write(&path, data)?;
+    std::fs::rename(path, dest_dir.as_ref().join(filename))?;
+    Ok(())
+}
 
 impl Link {
     pub fn new(url: Url, title: String, desc: Option<String>) -> Self {
@@ -55,8 +69,8 @@ impl Link {
     }
 
     /// Load a link with its properties from file
-    pub fn load<P: AsRef<Path>>(root: P, path: P) -> Result<Self> {
-        let p = path.as_ref().to_path_buf();
+    pub fn load<P: AsRef<Path>>(root: P, filename: P) -> Result<Self> {
+        let p = root.as_ref().join(filename);
         let url = Self::load_url(p)?;
         let id = ResourceId::compute_bytes(url.as_str().as_bytes())?;
         // Load user properties first
@@ -88,13 +102,8 @@ impl Link {
         let id_string = id.to_string();
 
         // Resources are stored in the folder chosen by user
-        let folder = root.as_ref().join(&id_string);
-        let link_file = AtomicFile::new(&folder)?;
-        let tmp = link_file.make_temp()?;
-        (&tmp).write_all(self.url.as_str().as_bytes())?;
-        let current_link = link_file.load()?;
-        link_file.compare_and_swap(&current_link, tmp)?;
-
+        let bytes = self.url.as_str().as_bytes();
+        temp_and_move(bytes, root.as_ref(), &id_string)?;
         //User defined properties
         store_properties(&root, id, &self.prop)?;
 
@@ -167,11 +176,8 @@ impl Link {
     }
 
     fn load_url(path: PathBuf) -> Result<Url> {
-        let file = AtomicFile::new(path)?;
-        let read_file = file.load()?;
-        let content = read_file.read_to_string()?;
-        let url_str = str::from_utf8(content.as_bytes())?;
-        Ok(Url::from_str(url_str)?)
+        let content = std::fs::read_to_string(path)?;
+        Ok(Url::from_str(&content)?)
     }
 }
 
@@ -310,29 +316,28 @@ async fn test_create_link_file() {
 
     // Resources are stored in the folder chosen by user
     let path = root.join(link.id().unwrap().to_string());
-    let file = AtomicFile::new(&path).unwrap();
+
     for save_preview in [false, true] {
-        link.save(root, save_preview).await.unwrap();
-        let current = file.load().unwrap();
-        let current_bytes = current.read_to_string().unwrap();
+        link.save(&root, save_preview).await.unwrap();
+        let current_bytes = std::fs::read_to_string(&path).unwrap();
         let url: Url =
             Url::from_str(str::from_utf8(current_bytes.as_bytes()).unwrap())
                 .unwrap();
         assert_eq!(url.as_str(), "https://kaydee.net/blog/open-graph-image/");
-        let link = Link::load(root, path.as_path()).unwrap();
+        let link = Link::load(root, &path).unwrap();
         assert_eq!(link.url.as_str(), url.as_str());
         assert_eq!(link.prop.desc.unwrap(), "test_desc");
         assert_eq!(link.prop.title, "test_title");
 
         let id = ResourceId::compute_bytes(current_bytes.as_bytes()).unwrap();
-        let path = Path::new(root)
+        let path = Path::new(&root)
             .join(ARK_FOLDER)
             .join(PREVIEWS_STORAGE_FOLDER)
             .join(id.to_string());
         if path.exists() {
-            assert_eq!(save_preview, true)
+            assert!(save_preview)
         } else {
-            assert_eq!(save_preview, false)
+            assert!(!save_preview)
         }
     }
 }