Use Cache::open() terminology, not Cache::new() (#1317)

Addressing some feedback from
#1298, no review required

I have left `OakSourceHandler::new()` as is. Even though all it does is
open the underlying caches, this name still felt more correct.

Author: DavisVaughan

crates/ark/src/lsp/sources.rs

@@ -38,8 +38,8 @@ impl OakSourceHandler {
     /// Build the handler, opening both caches against the shared on disk cache
     pub(crate) fn new(r: PathBuf) -> anyhow::Result<Self> {
         Ok(Self {
-            srcref: SrcrefCache::new(r)?,
-            source: SourceCache::new()?,
+            srcref: SrcrefCache::open(r)?,
+            source: SourceCache::open()?,
         })
     }
 
@@ -48,8 +48,8 @@ impl OakSourceHandler {
     #[cfg(test)]
     pub(crate) fn new_in(root: &Path, r: PathBuf) -> anyhow::Result<Self> {
         Ok(Self {
-            srcref: SrcrefCache::new_in(root.join("srcref"), r)?,
-            source: SourceCache::new_in(root.join("source"))?,
+            srcref: SrcrefCache::open_in(root.join("srcref"), r)?,
+            source: SourceCache::open_in(root.join("source"))?,
         })
     }
 }

crates/oak_cache/src/lib.rs

@@ -54,7 +54,7 @@ const COMPLETE_FILENAME: &str = ".complete";
 ///   simultaneously. It keeps any [`PathBuf`] handed out by [`Cache::get`] valid for the
 ///   life of the cache.
 ///
-/// - **Exclusive** is only attempted once, at [`Cache::new`], to run [`Cache::clean`].
+/// - **Exclusive** is only attempted once, at [`Cache::open`], to run [`Cache::clean`].
 ///   It is skipped if another session already holds the shared lock, in which case we
 ///   just try again next time.
 ///
@@ -81,13 +81,13 @@ impl Cache {
     /// Runs a best-effort [`Cache::clean`] under the exclusive root lock (skipped if
     /// another session holds the shared lock), then holds the shared root lock for the
     /// life of the returned `Cache` so handed-out paths stay valid.
-    pub fn new(root: &str, capacity: usize) -> anyhow::Result<Self> {
-        Self::new_in(cache_dir()?.join(root), capacity)
+    pub fn open(root: &str, capacity: usize) -> anyhow::Result<Self> {
+        Self::open_in(cache_dir()?.join(root), capacity)
     }
 
-    /// Like [`Cache::new`], but rooted at an explicit `root` rather than a subfolder of
+    /// Like [`Cache::open`], but rooted at an explicit `root` rather than a subfolder of
     /// the shared cache directory. Only useful for testing against a temp directory.
-    pub fn new_in(root: PathBuf, capacity: usize) -> anyhow::Result<Self> {
+    pub fn open_in(root: PathBuf, capacity: usize) -> anyhow::Result<Self> {
         let root = Filesystem::new(root);
         root.create_dir()?;
 
@@ -293,7 +293,7 @@ mod tests {
     /// alive for the test.
     fn new(capacity: usize) -> (TempDir, Cache) {
         let dir = TempDir::new().unwrap();
-        let cache = Cache::new_in(dir.path().join("subfolder"), capacity).unwrap();
+        let cache = Cache::open_in(dir.path().join("subfolder"), capacity).unwrap();
         (dir, cache)
     }
 
@@ -357,15 +357,15 @@ mod tests {
 
         // Populate one good entry, then forge a crashed partial (no `.complete`).
         {
-            let cache = Cache::new_in(root.clone(), 10).unwrap();
+            let cache = Cache::open_in(root.clone(), 10).unwrap();
             cache.insert("good", write_file("ok")).unwrap();
         }
         let partial = root.join("partial");
         std::fs::create_dir(&partial).unwrap();
         std::fs::write(partial.join("content.txt"), "junk").unwrap();
 
         // Reopening runs `clean`, which removes the partial but keeps the good entry.
-        let cache = Cache::new_in(root, 10).unwrap();
+        let cache = Cache::open_in(root, 10).unwrap();
         assert!(!partial.exists());
         assert!(cache.get("good").is_some());
     }
@@ -376,7 +376,7 @@ mod tests {
         let root = dir.path().join("subfolder");
 
         {
-            let cache = Cache::new_in(root.clone(), 10).unwrap();
+            let cache = Cache::open_in(root.clone(), 10).unwrap();
             cache.insert("good", write_file("ok")).unwrap();
         }
         // A stray file in the cache root that doesn't belong to any entry.
@@ -385,7 +385,7 @@ mod tests {
 
         // Reopening runs `clean`, which removes the stray file but keeps our root
         // `.lock` and the good entry.
-        let cache = Cache::new_in(root.clone(), 10).unwrap();
+        let cache = Cache::open_in(root.clone(), 10).unwrap();
         assert!(!stray.exists());
         assert!(root.join(".lock").exists());
         assert!(cache.get("good").is_some());
@@ -400,15 +400,15 @@ mod tests {
         // Insert four entries under a capacity that won't evict, then stamp their
         // access times so the ordering is deterministic.
         {
-            let cache = Cache::new_in(root.clone(), 10).unwrap();
+            let cache = Cache::open_in(root.clone(), 10).unwrap();
             for (index, key) in ["oldest", "older", "newer", "newest"].iter().enumerate() {
                 let entry = cache.insert(key, write_file(key)).unwrap().unwrap();
                 set_accessed(&entry, now + Duration::from_secs(index as u64));
             }
         }
 
         // Reopening with capacity 2 evicts the two least-recently-accessed.
-        let cache = Cache::new_in(root, 2).unwrap();
+        let cache = Cache::open_in(root, 2).unwrap();
         assert_eq!(cache.get("oldest"), None);
         assert_eq!(cache.get("older"), None);
         assert!(cache.get("newer").is_some());

crates/oak_source/src/lib.rs

@@ -39,19 +39,19 @@ pub struct SourceCache {
 }
 
 impl SourceCache {
-    pub fn new() -> anyhow::Result<Self> {
+    pub fn open() -> anyhow::Result<Self> {
         Ok(Self {
-            cran: Cache::new(&format!("source/{CACHE_VERSION}/cran"), CRAN_CAPACITY)?,
-            r: Cache::new(&format!("source/{CACHE_VERSION}/r"), R_CAPACITY)?,
+            cran: Cache::open(&format!("source/{CACHE_VERSION}/cran"), CRAN_CAPACITY)?,
+            r: Cache::open(&format!("source/{CACHE_VERSION}/r"), R_CAPACITY)?,
         })
     }
 
-    /// Like [`SourceCache::new`], but rooted at an explicit `root` rather than the shared
-    /// cache directory. Only useful for testing against a temp directory.
-    pub fn new_in(root: PathBuf) -> anyhow::Result<Self> {
+    /// Like [`SourceCache::open`], but rooted at an explicit `root` rather than the
+    /// shared cache directory. Only useful for testing against a temp directory.
+    pub fn open_in(root: PathBuf) -> anyhow::Result<Self> {
         Ok(Self {
-            cran: Cache::new_in(root.join("cran"), CRAN_CAPACITY)?,
-            r: Cache::new_in(root.join("r"), R_CAPACITY)?,
+            cran: Cache::open_in(root.join("cran"), CRAN_CAPACITY)?,
+            r: Cache::open_in(root.join("r"), R_CAPACITY)?,
         })
     }
 
@@ -102,7 +102,7 @@ mod tests {
     #[test]
     fn test_cran_round_trip() {
         let dir = TempDir::new().unwrap();
-        let source = SourceCache::new_in(dir.path().to_path_buf()).unwrap();
+        let source = SourceCache::open_in(dir.path().to_path_buf()).unwrap();
 
         // Miss before insert
         assert_eq!(source.get_cran("vctrs", "0.7.2"), None);
@@ -127,7 +127,7 @@ mod tests {
     #[test]
     fn test_cran_not_found() {
         let dir = TempDir::new().unwrap();
-        let source = SourceCache::new_in(dir.path().to_path_buf()).unwrap();
+        let source = SourceCache::open_in(dir.path().to_path_buf()).unwrap();
         assert_eq!(
             source.insert_cran("definitely_not_a_package", "0.0.0"),
             None
@@ -139,7 +139,7 @@ mod tests {
     #[test]
     fn test_r_round_trip() {
         let dir = TempDir::new().unwrap();
-        let source = SourceCache::new_in(dir.path().to_path_buf()).unwrap();
+        let source = SourceCache::open_in(dir.path().to_path_buf()).unwrap();
 
         let root = source.insert_r("4.5.0").unwrap();
 
@@ -156,7 +156,7 @@ mod tests {
     #[test]
     fn test_r_unknown_version() {
         let dir = TempDir::new().unwrap();
-        let source = SourceCache::new_in(dir.path().to_path_buf()).unwrap();
+        let source = SourceCache::open_in(dir.path().to_path_buf()).unwrap();
         assert_eq!(source.insert_r("0.0.0"), None);
     }
 }

crates/oak_srcref/src/lib.rs

@@ -28,20 +28,20 @@ pub struct SrcrefCache {
 }
 
 impl SrcrefCache {
-    pub fn new(r: PathBuf) -> anyhow::Result<Self> {
+    pub fn open(r: PathBuf) -> anyhow::Result<Self> {
         Ok(Self {
             r,
-            cache: Cache::new(&format!("srcref/{CACHE_VERSION}"), CACHE_CAPACITY)?,
+            cache: Cache::open(&format!("srcref/{CACHE_VERSION}"), CACHE_CAPACITY)?,
         })
     }
 
     /// Open a `SrcrefCache` rooted at an explicit directory instead of the shared cache
     ///
     /// Useful for integration tests that don't want to touch the real on disk cache.
-    pub fn new_in(root: PathBuf, r: PathBuf) -> anyhow::Result<Self> {
+    pub fn open_in(root: PathBuf, r: PathBuf) -> anyhow::Result<Self> {
         Ok(Self {
             r,
-            cache: Cache::new_in(root, CACHE_CAPACITY)?,
+            cache: Cache::open_in(root, CACHE_CAPACITY)?,
         })
     }