Merge pull request #116 from moka-rs/fix-dash-cache-iter

Fix the `dash::Cache` iterator not to return expired entries

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/

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<K, V, S> BaseCache<K, V, S> {
+ pub(crate) fn is_expired_entry(&self, entry: &TrioArc<ValueEntry<K, V>>) -> 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<K, V, S> Inner<K, V, S> {
  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()
  }
 }
 

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());
  }
 

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<K>, TrioArc<ValueEntry<K, V>>, S>;
+pub(crate) type DashMapIter<'a, K, V, S> =
+ dashmap::iter::Iter<'a, Arc<K>, TrioArc<ValueEntry<K, V>>, S>;
 
-pub struct Iter<'a, K, V, S>(DashMapIter<'a, K, V, S>);
+pub struct Iter<'a, K, V, S> {
+ cache: &'a BaseCache<K, V, S>,
+ 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<K, V, S>, 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::Item> {
- 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
  }
 }
 

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.