From 9a23afbacb39e4a60f0e3cd74837aca1bf24c9f9 Mon Sep 17 00:00:00 2001
From: maypok86 <alex.mayshev.86@gmail.com>
Date: Tue, 6 Feb 2024 18:12:49 +0300
Subject: [PATCH] [#44] Fix issues with concurrent updates in DeleteByFunc

---
 internal/core/cache.go    | 28 +++++++++++++++++-----------
 internal/hashtable/map.go |  4 ++--
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/internal/core/cache.go b/internal/core/cache.go
index 85c4e73..ece04a2 100644
--- a/internal/core/cache.go
+++ b/internal/core/cache.go
@@ -232,20 +232,26 @@ func (c *Cache[K, V]) Delete(key K) {
 	}
 }
 
+func (c *Cache[K, V]) deleteNode(n *node.Node[K, V]) {
+	deleted := c.hashmap.DeleteNode(n)
+	if deleted != nil {
+		c.writeBuffer.Insert(node.NewDeleteTask(deleted))
+	}
+}
+
 // DeleteByFunc removes the association for this key from the cache when the given function returns true.
 func (c *Cache[K, V]) DeleteByFunc(f func(key K, value V) bool) {
-	// TODO(maypok86): This function can be implemented more efficiently, if the performance of this implementation is not enough for you, then come with an issue :)
-	var keysToDelete []K
-	c.Range(func(key K, value V) bool {
-		if f(key, value) {
-			keysToDelete = append(keysToDelete, key)
+	c.hashmap.Range(func(n *node.Node[K, V]) bool {
+		if n.IsExpired() {
+			return true
+		}
+
+		if f(n.Key(), n.Value()) {
+			c.deleteNode(n)
 		}
+
 		return true
 	})
-
-	for _, key := range keysToDelete {
-		c.Delete(key)
-	}
 }
 
 func (c *Cache[K, V]) cleanup() {
@@ -264,7 +270,7 @@ func (c *Cache[K, V]) cleanup() {
 		c.evictionMutex.Unlock()
 
 		for _, n := range e {
-			c.hashmap.EvictNode(n)
+			c.hashmap.DeleteNode(n)
 		}
 
 		expired = clearBuffer(expired)
@@ -325,7 +331,7 @@ func (c *Cache[K, V]) process() {
 			c.evictionMutex.Unlock()
 
 			for _, n := range d {
-				c.hashmap.EvictNode(n)
+				c.hashmap.DeleteNode(n)
 			}
 
 			buffer = clearBuffer(buffer)
diff --git a/internal/hashtable/map.go b/internal/hashtable/map.go
index 2c5c901..5e9df24 100644
--- a/internal/hashtable/map.go
+++ b/internal/hashtable/map.go
@@ -296,10 +296,10 @@ func (m *Map[K, V]) Delete(key K) *node.Node[K, V] {
 	})
 }
 
-// EvictNode evicts the node for a key.
+// DeleteNode evicts the node for a key.
 //
 // Returns the evicted node or nil if the node wasn't evicted.
-func (m *Map[K, V]) EvictNode(n *node.Node[K, V]) *node.Node[K, V] {
+func (m *Map[K, V]) DeleteNode(n *node.Node[K, V]) *node.Node[K, V] {
 	return m.delete(n.Key(), func(current *node.Node[K, V]) bool {
 		return n == current
 	})