#1283

fix: Make `HybridCache::is_hybrid()` correctly return `false` when storage is not configured

Openuckelman-sfwants to merge intofrom

What's changed and what's your intention?

HybridCache::is_hybrid() presently returns true in all circumstances, even when storage is not configured. It is supposed to return false when storage is not configured.

This test, which I added in bd866eec, fails in main currently:

    #[test_log::test(tokio::test)]
    async fn test_is_hybrid_without_disk_cache() {
        let cache = HybridCacheBuilder::<u8, u8>::new()
            .memory(1)
            .storage()
            .build()
            .await
            .unwrap();

        assert!(!cache.is_hybrid());
    }

HybridCache::is_hybrid() is this:

    pub fn is_hybrid(&self) -> bool {
        self.inner.storage.is_enabled()
    }

This is Store::is_enabled():

     pub fn is_enabled(&self) -> bool {
        self.inner.engine.type_id() != TypeId::of::<Arc<NoopEngine<K, V, P>>>()
     }

The problem with is_enabled() is that self.inner.engine is Arc<dyn Engine<K, V, P>> but Any::type_id() does not downcast---which means there are no circumstances where self.inner.engine.type_id() could return something equal to TypeId::of::<Arc<NoopEngine<K, V, P>>>().

The correct thing to do here is upcast the Arc's referent to a &dyn Any and then check if the concrete type is a NoopEngine<K, V, P>:

    pub fn is_enabled(&self) -> bool {
        !(self.inner.engine.as_ref() as &dyn Any).is::<NoopEngine<K, V, P>>()
     }

I've also provided a test for the case where a disk cache is in use, to ensure is_hybrid() returns true there.

Checklist

  • I have written the necessary rustdoc comments
  • I have added the necessary unit tests and integration tests
  • I have passed cargo x (or cargo x --fast instead if the old tests are not modified) in my local environment.

Activity

3
codecov[bot]7mo ago

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests.

Files with missing linesCoverage Δ
foyer-storage/src/store.rs86.43% <100.00%> (ø)
foyer/src/hybrid/cache.rs85.23% <ø> (+0.48%):arrow_up:

... and 1 file with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.