Skip to content

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

Description

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

debtCode quality issues

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions