Skip to content

Refactor: Consolidate caching patterns across locators into a shared abstraction #291

@karthiknadig

Description

@karthiknadig

Summary

Multiple locators implement nearly identical caching patterns for environments and managers. Consolidating these into a shared abstraction would reduce code duplication and ensure consistent behavior.

Current Duplicate Patterns

Pattern: Cache with mutex + contains_key check

Found in:

  • crates/pet-conda/src/lib.rs
  • crates/pet-poetry/src/lib.rs
  • crates/pet-windows-store/src/lib.rs
  • crates/pet-linux-global-python/src/lib.rs
// This pattern appears in multiple files:
pub struct SomeLocator {
    environments: Arc<Mutex<HashMap<PathBuf, PythonEnvironment>>>,
}

impl SomeLocator {
    fn find_with_cache(&self) -> Option<...> {
        let environments = self.environments.lock().unwrap();
        if environments.contains_key(&key) {
            return Some(environments.get(&key).unwrap().clone());
        }
        // ... compute and cache
    }
    
    fn clear(&self) {
        self.environments.lock().unwrap().clear();
    }
}

Proposed Solution

Create a generic caching wrapper

// In pet-core or a new pet-cache crate
pub struct LocatorCache<K, V> {
    cache: RwLock<HashMap<K, V>>,
}

impl<K: Eq + Hash, V: Clone> LocatorCache<K, V> {
    pub fn new() -> Self {
        Self { cache: RwLock::new(HashMap::new()) }
    }
    
    pub fn get(&self, key: &K) -> Option<V> {
        self.cache.read().unwrap().get(key).cloned()
    }
    
    pub fn get_or_insert_with<F>(&self, key: K, f: F) -> V
    where
        F: FnOnce() -> Option<V>,
    {
        // Check read lock first
        if let Some(v) = self.cache.read().unwrap().get(&key) {
            return v.clone();
        }
        // Upgrade to write lock and compute
        if let Some(value) = f() {
            self.cache.write().unwrap().insert(key, value.clone());
            value
        }
    }
    
    pub fn clear(&self) {
        self.cache.write().unwrap().clear();
    }
}

// Type aliases for common uses
pub type EnvironmentCache = LocatorCache<PathBuf, PythonEnvironment>;
pub type ManagerCache = LocatorCache<PathBuf, EnvManager>;

Usage in locators

pub struct Conda {
    pub environments: EnvironmentCache,
    pub managers: ManagerCache,
    // ...
}

impl Conda {
    fn try_from(&self, env: &PythonEnv) -> Option<PythonEnvironment> {
        self.environments.get_or_insert_with(path.clone(), || {
            // compute environment
        })
    }
}

Benefits

  1. Consistency: All locators behave the same way
  2. Less code: Remove duplicate boilerplate
  3. Centralized improvements: RwLock upgrade, better error handling, etc. benefit all locators
  4. Testability: Cache behavior can be tested once

Priority

Low - Requires significant refactoring but improves maintainability long-term.

Metadata

Metadata

Labels

debtCode quality issues

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions