diff --git a/CHANGELOG.md b/CHANGELOG.md index f233aca9..075172c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ - Implement `IntoIterator` to the all caches (including experimental `dash::Cache`) ([#114][gh-pull-0114]) +### Fixed + +- Fix the `dash::Cache` iterator not to return expired entries. + ([#116][gh-pull-0116]) + ## Version 0.8.1 @@ -300,6 +305,7 @@ The minimum supported Rust version (MSRV) is now 1.51.0 (2021-03-25). [gh-issue-0038]: https://github.com/moka-rs/moka/issues/38/ [gh-issue-0031]: https://github.com/moka-rs/moka/issues/31/ +[gh-pull-0116]: https://github.com/moka-rs/moka/pull/116/ [gh-pull-0114]: https://github.com/moka-rs/moka/pull/114/ [gh-pull-0105]: https://github.com/moka-rs/moka/pull/105/ [gh-pull-0104]: https://github.com/moka-rs/moka/pull/104/ diff --git a/src/dash/base_cache.rs b/src/dash/base_cache.rs index 648a7108..de375020 100644 --- a/src/dash/base_cache.rs +++ b/src/dash/base_cache.rs @@ -1,4 +1,4 @@ -use super::Iter; +use super::{iter::DashMapIter, Iter}; use crate::{ common::{ self, @@ -227,7 +227,18 @@ where S: BuildHasher + Clone, { pub(crate) fn iter(&self) -> Iter<'_, K, V, S> { - self.inner.iter() + Iter::new(self, self.inner.iter()) + } +} + +impl BaseCache { + pub(crate) fn is_expired_entry(&self, entry: &TrioArc>) -> bool { + let i = &self.inner; + let (ttl, tti, va) = (&i.time_to_live(), &i.time_to_idle(), &i.valid_after()); + let now = i.current_time_from_expiration_clock(); + let entry = &*entry; + + is_expired_entry_wo(ttl, va, entry, now) || is_expired_entry_ao(tti, va, entry, now) } } @@ -527,7 +538,10 @@ where .remove(key) .map(|(key, entry)| KvEntry::new(key, entry)) } +} +// functions/methods used by BaseCache +impl Inner { fn policy(&self) -> Policy { Policy::new(self.max_capacity, 1, self.time_to_live, self.time_to_idle) } @@ -606,9 +620,8 @@ where V: 'a, S: BuildHasher + Clone, { - fn iter(&self) -> Iter<'_, K, V, S> { - let map_iter = self.cache.iter(); - Iter::new(map_iter) + fn iter(&self) -> DashMapIter<'_, K, V, S> { + self.cache.iter() } } diff --git a/src/dash/cache.rs b/src/dash/cache.rs index cd2515a9..29d85a28 100644 --- a/src/dash/cache.rs +++ b/src/dash/cache.rs @@ -439,10 +439,6 @@ where /// Unlike the `get` method, visiting entries via an iterator do not update the /// historic popularity estimator or reset idle timers for keys. /// - /// # Guarantees - /// - /// **TODO** - /// /// # Locking behavior /// /// This iterator relies on the iterator of [`dashmap::DashMap`][dashmap-iter], @@ -808,10 +804,12 @@ mod tests { assert!(cache.contains_key(&"a")); mock.increment(Duration::from_secs(5)); // 10 secs. - cache.sync(); - assert_eq!(cache.get(&"a"), None); assert!(!cache.contains_key(&"a")); + + assert_eq!(cache.iter().count(), 0); + + cache.sync(); assert!(cache.is_table_empty()); cache.insert("b", "bob"); @@ -837,12 +835,14 @@ mod tests { assert_eq!(cache.estimated_entry_count(), 1); mock.increment(Duration::from_secs(5)); // 25 secs - cache.sync(); - assert_eq!(cache.get(&"a"), None); assert_eq!(cache.get(&"b"), None); assert!(!cache.contains_key(&"a")); assert!(!cache.contains_key(&"b")); + + assert_eq!(cache.iter().count(), 0); + + cache.sync(); assert!(cache.is_table_empty()); } @@ -888,21 +888,25 @@ mod tests { assert_eq!(cache.estimated_entry_count(), 2); mock.increment(Duration::from_secs(3)); // 15 secs. - cache.sync(); - assert_eq!(cache.get(&"a"), None); assert_eq!(cache.get(&"b"), Some("bob")); assert!(!cache.contains_key(&"a")); assert!(cache.contains_key(&"b")); - assert_eq!(cache.estimated_entry_count(), 1); - mock.increment(Duration::from_secs(10)); // 25 secs + assert_eq!(cache.iter().count(), 1); + cache.sync(); + assert_eq!(cache.estimated_entry_count(), 1); + mock.increment(Duration::from_secs(10)); // 25 secs assert_eq!(cache.get(&"a"), None); assert_eq!(cache.get(&"b"), None); assert!(!cache.contains_key(&"a")); assert!(!cache.contains_key(&"b")); + + assert_eq!(cache.iter().count(), 0); + + cache.sync(); assert!(cache.is_table_empty()); } diff --git a/src/dash/iter.rs b/src/dash/iter.rs index f625736e..ca3632ad 100644 --- a/src/dash/iter.rs +++ b/src/dash/iter.rs @@ -1,4 +1,4 @@ -use super::mapref::EntryRef; +use super::{base_cache::BaseCache, mapref::EntryRef}; use crate::sync::ValueEntry; use std::{ @@ -7,13 +7,17 @@ use std::{ }; use triomphe::Arc as TrioArc; -type DashMapIter<'a, K, V, S> = dashmap::iter::Iter<'a, Arc, TrioArc>, S>; +pub(crate) type DashMapIter<'a, K, V, S> = + dashmap::iter::Iter<'a, Arc, TrioArc>, S>; -pub struct Iter<'a, K, V, S>(DashMapIter<'a, K, V, S>); +pub struct Iter<'a, K, V, S> { + cache: &'a BaseCache, + map_iter: DashMapIter<'a, K, V, S>, +} impl<'a, K, V, S> Iter<'a, K, V, S> { - pub(crate) fn new(map_iter: DashMapIter<'a, K, V, S>) -> Self { - Self(map_iter) + pub(crate) fn new(cache: &'a BaseCache, map_iter: DashMapIter<'a, K, V, S>) -> Self { + Self { cache, map_iter } } } @@ -25,7 +29,13 @@ where type Item = EntryRef<'a, K, V, S>; fn next(&mut self) -> Option { - self.0.next().map(|map_ref| EntryRef::new(map_ref)) + for map_ref in &mut self.map_iter { + if !self.cache.is_expired_entry(map_ref.value()) { + return Some(EntryRef::new(map_ref)); + } + } + + None } } diff --git a/src/lib.rs b/src/lib.rs index fa86421b..b03b5f6d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,7 @@ #![warn(clippy::all)] #![warn(rust_2018_idioms)] +// Temporary disable this lint as the MSRV (1.51) require an older lint name: +// #![deny(rustdoc::broken_intra_doc_links)] //! Moka is a fast, concurrent cache library for Rust. Moka is inspired by //! the [Caffeine][caffeine-git] library for Java.