More precise checks for when tool state save is needed

lib/storage/home.rs

@@ -1,6 +1,5 @@
 use std::env::var;
 use std::path::{Path, PathBuf};
-use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 
 use crate::result::{AftmanError, AftmanResult};
@@ -18,7 +17,6 @@ use super::{ToolCache, ToolStorage};
 #[derive(Debug, Clone)]
 pub struct Home {
     path: Arc<Path>,
-    did_save: Arc<AtomicBool>,
     tool_storage: ToolStorage,
     tool_cache: ToolCache,
 }
@@ -29,14 +27,12 @@ impl Home {
     */
     async fn load_from_path(path: impl Into<PathBuf>) -> AftmanResult<Self> {
         let path: Arc<Path> = path.into().into();
-        let did_save = Arc::new(AtomicBool::new(false));
 
         let (tool_storage, tool_cache) =
             tokio::try_join!(ToolStorage::load(&path), ToolCache::load(&path))?;
 
         Ok(Self {
             path,
-            did_save,
             tool_storage,
             tool_cache,
         })
@@ -90,7 +86,6 @@ impl Home {
     */
     pub async fn save(&self) -> AftmanResult<()> {
         self.tool_cache.save(&self.path).await?;
-        self.did_save.store(true, Ordering::SeqCst);
         Ok(())
     }
 }
@@ -107,11 +102,14 @@ impl Home {
 impl Drop for Home {
     fn drop(&mut self) {
         let is_last = Arc::strong_count(&self.path) <= 1;
-        if is_last && !self.did_save.load(Ordering::SeqCst) {
+        if !is_last {
+            return;
+        }
+        if self.tool_cache.needs_saving() || self.tool_storage.needs_saving() {
             tracing::error!(
-                "Aftman home was dropped without being saved!\
+                "Aftman home was dropped without saving!\
                 \nChanges to trust, tools, and more may have been lost."
-            )
+            );
         }
     }
 }

lib/storage/tool_cache.rs

@@ -4,12 +4,15 @@
 use std::{
     collections::BTreeSet,
     path::{Path, PathBuf},
-    sync::Arc,
+    sync::{
+        atomic::{AtomicBool, Ordering},
+        Arc,
+    },
 };
 
 use dashmap::DashSet;
 use semver::Version;
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 use tokio::{task::spawn_blocking, time::Instant};
 use tracing::{instrument, trace};
 
@@ -23,10 +26,12 @@ use crate::{
 
     Can be cheaply cloned while still referring to the same underlying data.
 */
-#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+#[derive(Debug, Default, Clone, Deserialize)]
 pub struct ToolCache {
     trusted: Arc<DashSet<ToolId>>,
     installed: Arc<DashSet<ToolSpec>>,
+    #[serde(default, skip)]
+    needs_saving: Arc<AtomicBool>,
 }
 
 impl ToolCache {
@@ -43,6 +48,7 @@ impl ToolCache {
         Returns `true` if the tool was added and not already trusted.
     */
     pub fn add_trust(&self, tool: ToolId) -> bool {
+        self.needs_saving.store(true, Ordering::SeqCst);
         self.trusted.insert(tool)
     }
 
@@ -52,6 +58,7 @@ impl ToolCache {
         Returns `true` if the tool was previously trusted and has now been removed.
     */
     pub fn remove_trust(&self, tool: &ToolId) -> bool {
+        self.needs_saving.store(true, Ordering::SeqCst);
         self.trusted.remove(tool).is_some()
     }
 
@@ -77,6 +84,7 @@ impl ToolCache {
         Returns `true` if the tool was added and not already cached.
     */
     pub fn add_installed(&self, tool: ToolSpec) -> bool {
+        self.needs_saving.store(true, Ordering::SeqCst);
         self.installed.insert(tool)
     }
 
@@ -86,6 +94,7 @@ impl ToolCache {
         Returns `true` if the tool was previously cached and has now been removed.
     */
     pub fn remove_installed(&self, tool: &ToolSpec) -> bool {
+        self.needs_saving.store(true, Ordering::SeqCst);
         self.installed.remove(tool).is_some()
     }
 
@@ -158,12 +167,17 @@ impl ToolCache {
 
     #[instrument(skip(self, home_path), level = "trace")]
     pub(crate) async fn save(&self, home_path: impl AsRef<Path>) -> AftmanResult<()> {
+        self.needs_saving.store(false, Ordering::SeqCst);
         let start = Instant::now();
         let path = Self::path(home_path);
         save_impl(path.clone(), self).await?;
         trace!(?path, elapsed = ?start.elapsed(), "Saved tool cache");
         Ok(())
     }
+
+    pub(crate) fn needs_saving(&self) -> bool {
+        self.needs_saving.load(Ordering::SeqCst)
+    }
 }
 
 async fn load_impl(path: PathBuf) -> AftmanResult<ToolCache> {

lib/storage/tool_storage.rs

@@ -191,4 +191,10 @@ impl ToolStorage {
             aliases_dir,
         })
     }
+
+    pub(crate) fn needs_saving(&self) -> bool {
+        // Tool storage always writes all state directly
+        // to the disk, but this may change in the future
+        false
+    }
 }