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.