Simplify store interface

Use a sinf `Set` method instead of `Add` and `Update`.

Signed-off-by: Soule BA <[email protected]>

cache/cache.go

@@ -129,44 +129,9 @@ func (c *Cache[T]) Close() error {
 	return nil
 }
 
-// Add an item to the cache, existing index will not be overwritten.
-// To overwrite existing index, use Update.
+// Set an item in the cache, existing index will be overwritten.
 // If the cache is full, Add will return an error.
-func (c *Cache[T]) Add(object T) error {
-	key, err := c.keyFunc(object)
-	if err != nil {
-		recordRequest(c.metrics, StatusFailure)
-		return KeyError{object, err}
-	}
-
-	c.mu.Lock()
-	if c.closed {
-		c.mu.Unlock()
-		recordRequest(c.metrics, StatusFailure)
-		return KeyError{object, ErrClosed}
-	}
-	_, found := c.index[key]
-	if found {
-		c.mu.Unlock()
-		recordRequest(c.metrics, StatusFailure)
-		return KeyError{object, ErrAlreadyExists}
-	}
-
-	if c.capacity > 0 && len(c.index) < c.capacity {
-		c.set(key, object)
-		c.mu.Unlock()
-		recordRequest(c.metrics, StatusSuccess)
-		recordItemIncrement(c.metrics)
-		return nil
-	}
-	c.mu.Unlock()
-	recordRequest(c.metrics, StatusFailure)
-	return KeyError{object, ErrFull}
-}
-
-// Update adds an item to the cache, replacing any existing item.
-// If the cache is full, it will return an error.
-func (c *Cache[T]) Update(object T) error {
+func (c *Cache[T]) Set(object T) error {
 	key, err := c.keyFunc(object)
 	if err != nil {
 		recordRequest(c.metrics, StatusFailure)

cache/cache_test.go

@@ -57,7 +57,7 @@ func TestCache(t *testing.T) {
 		g.Expect(found).To(BeFalse())
 
 		// Add an item to the cache
-		err = cache.Add(obj)
+		err = cache.Set(obj)
 		g.Expect(err).ToNot(HaveOccurred())
 
 		// Get the item from the cache
@@ -77,7 +77,7 @@ func TestCache(t *testing.T) {
 			},
 		}
 		// Add another item to the cache
-		err = cache.Add(obj2)
+		err = cache.Set(obj2)
 		g.Expect(err).ToNot(HaveOccurred())
 		g.Expect(cache.ListKeys()).To(ConsistOf("test-ns/test", "test-ns/test2"))
 
@@ -98,12 +98,12 @@ func TestCache(t *testing.T) {
 				Name:      "test3",
 			},
 		}
-		err = cache.Add(obj3)
+		err = cache.Set(obj3)
 		g.Expect(err).ToNot(HaveOccurred())
 
 		// Replace an item in the cache
 		obj3.Labels = map[string]string{"pp.kubernetes.io/created-by: ": "flux"}
-		err = cache.Update(obj3)
+		err = cache.Set(obj3)
 		g.Expect(err).ToNot(HaveOccurred())
 
 		// Get the item from the cache
@@ -146,7 +146,7 @@ func TestCache(t *testing.T) {
 			},
 		}
 
-		err = cache.Add(obj)
+		err = cache.Set(obj)
 		g.Expect(err).ToNot(HaveOccurred())
 
 		// set expiration time to 2 seconds
@@ -190,16 +190,15 @@ func Test_Cache_Add(t *testing.T) {
 		},
 		Object: "test-token",
 	}
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	err = cache.SetExpiration(obj, 10*time.Millisecond)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test_test-group_TestObject"))
 
-	// try adding the same object again
-	err = cache.Add(obj)
-	g.Expect(err).To(HaveOccurred())
-	g.Expect(err.Error()).To(ContainSubstring("already exists"))
+	// try adding the same object again, it should overwrite the existing one
+	err = cache.Set(obj)
+	g.Expect(err).ToNot(HaveOccurred())
 
 	// wait for the item to expire
 	time.Sleep(20 * time.Millisecond)
@@ -209,7 +208,7 @@ func Test_Cache_Add(t *testing.T) {
 
 	// add another object
 	obj.Name = "test2"
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test2_test-group_TestObject"))
 
@@ -220,8 +219,7 @@ func Test_Cache_Add(t *testing.T) {
 	gotk_cache_evictions_total 1
 	# HELP gotk_cache_requests_total Total number of cache requests partioned by success or failure.
 	# TYPE gotk_cache_requests_total counter
-	gotk_cache_requests_total{status="failure"} 1
-	gotk_cache_requests_total{status="success"} 6
+	gotk_cache_requests_total{status="success"} 7
 	# HELP gotk_cached_items Total number of items in the cache.
 	# TYPE gotk_cached_items gauge
 	gotk_cached_items 1
@@ -247,12 +245,12 @@ func Test_Cache_Update(t *testing.T) {
 		},
 		Object: "test-token",
 	}
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test_test-group_TestObject"))
 
 	obj.Object = "test-token2"
-	err = cache.Update(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test_test-group_TestObject"))
 	g.Expect(cache.index["test-ns_test_test-group_TestObject"].object.Object).To(Equal("test-token2"))
@@ -296,7 +294,7 @@ func Test_Cache_Get(t *testing.T) {
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(found).To(BeFalse())
 
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 
 	item, found, err := cache.Get(obj)
@@ -342,7 +340,7 @@ func Test_Cache_Delete(t *testing.T) {
 		Object: "test-token",
 	}
 
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test_test-group_TestObject"))
 
@@ -459,7 +457,7 @@ func Test_Cache_deleteExpired(t *testing.T) {
 			g.Expect(err).ToNot(HaveOccurred())
 
 			for _, item := range tt.items {
-				err := cache.Add(item.object)
+				err := cache.Set(item.object)
 				g.Expect(err).ToNot(HaveOccurred())
 				if item.expire {
 					err = cache.SetExpiration(item.object, item.expiration)
@@ -495,7 +493,7 @@ func Test_Cache_Resize(t *testing.T) {
 			},
 			Object: "test-token",
 		}
-		err = cache.Add(obj)
+		err = cache.Set(obj)
 		g.Expect(err).ToNot(HaveOccurred())
 		err = cache.SetExpiration(obj, 10*time.Minute)
 		g.Expect(err).ToNot(HaveOccurred())
@@ -529,7 +527,7 @@ func TestCache_Concurrent(t *testing.T) {
 		wg.Add(2)
 		go func() {
 			defer wg.Done()
-			_ = cache.Add(objmap[key])
+			_ = cache.Set(objmap[key])
 		}()
 		go func() {
 			defer wg.Done()

cache/lru.go

@@ -85,25 +85,25 @@ func NewLRU[T any](capacity int, keyFunc KeyFunc[T], opts ...Options[T]) (*LRU[T
 	return lru, nil
 }
 
-// Add adds a node to the end of the list
-// if the node is already in the cache, an error is returned
-func (c *LRU[T]) Add(object T) error {
+// Set an item in the cache, existing index will be overwritten.
+func (c *LRU[T]) Set(object T) error {
 	key, err := c.keyFunc(object)
 	if err != nil {
 		recordRequest(c.metrics, StatusFailure)
 		return KeyError{object, err}
 	}
 
 	// if node is already in cache, return error
-	c.mu.RLock()
-	_, ok := c.cache[key]
-	c.mu.RUnlock()
+	c.mu.Lock()
+	node, ok := c.cache[key]
 	if ok {
-		recordRequest(c.metrics, StatusFailure)
-		return KeyError{object, ErrAlreadyExists}
+		c.delete(node)
+		_ = c.add(&Node[T]{key: key, object: object})
+		c.mu.Unlock()
+		recordRequest(c.metrics, StatusSuccess)
+		return nil
 	}
 
-	c.mu.Lock()
 	evicted := c.add(&Node[T]{key: key, object: object})
 	c.mu.Unlock()
 	recordRequest(c.metrics, StatusSuccess)
@@ -228,28 +228,6 @@ func (c *LRU[T]) get(key string) (item T, exists bool, err error) {
 	return node.object, true, nil
 }
 
-// Update updates an object in the cache.
-// If the object is not in the cache, it is added.
-func (c *LRU[T]) Update(object T) error {
-	key, err := c.keyFunc(object)
-	if err != nil {
-		recordRequest(c.metrics, StatusFailure)
-		return KeyError{object, err}
-	}
-
-	c.mu.Lock()
-	node, ok := c.cache[key]
-	if ok {
-		c.delete(node)
-		_ = c.add(&Node[T]{key: key, object: object})
-		c.mu.Unlock()
-		recordRequest(c.metrics, StatusSuccess)
-		return nil
-	}
-	c.mu.Unlock()
-	return c.Add(object)
-}
-
 // ListKeys returns a list of keys in the cache.
 func (c *LRU[T]) ListKeys() []string {
 	keys := make([]string, 0, len(c.cache))

cache/lru_test.go

@@ -222,7 +222,7 @@ func Test_LRU(t *testing.T) {
 				WithMetricsRegisterer[any](prometheus.NewPedanticRegistry()))
 			g.Expect(err).ToNot(HaveOccurred())
 			for _, input := range v.inputs {
-				err := cache.Add(input)
+				err := cache.Set(input)
 				g.Expect(err).ToNot(HaveOccurred())
 			}
 
@@ -256,18 +256,17 @@ func Test_LRU_Add(t *testing.T) {
 		},
 		Object: "test-token",
 	}
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test_test-group_TestObject"))
 
-	// try adding the same object again
-	err = cache.Add(obj)
-	g.Expect(err).To(HaveOccurred())
-	g.Expect(err.Error()).To(ContainSubstring("already exists"))
+	// try adding the same object again, it should overwrite the existing one
+	err = cache.Set(obj)
+	g.Expect(err).ToNot(HaveOccurred())
 
 	// add another object
 	obj.Name = "test2"
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test2_test-group_TestObject"))
 
@@ -278,8 +277,7 @@ func Test_LRU_Add(t *testing.T) {
 	gotk_cache_evictions_total 1
 	# HELP gotk_cache_requests_total Total number of cache requests partioned by success or failure.
 	# TYPE gotk_cache_requests_total counter
-	gotk_cache_requests_total{status="failure"}1
-	gotk_cache_requests_total{status="success"} 4
+	gotk_cache_requests_total{status="success"} 5
 	# HELP gotk_cached_items Total number of items in the cache.
 	# TYPE gotk_cached_items gauge
 	gotk_cached_items 1
@@ -305,12 +303,12 @@ func Test_LRU_Update(t *testing.T) {
 		},
 		Object: "test-token",
 	}
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test_test-group_TestObject"))
 
 	obj.Object = "test-token2"
-	err = cache.Update(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(cache.ListKeys()).To(ConsistOf("test-ns_test_test-group_TestObject"))
 	g.Expect(cache.cache["test-ns_test_test-group_TestObject"].object.Object).To(Equal("test-token2"))
@@ -354,7 +352,7 @@ func Test_LRU_Get(t *testing.T) {
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(found).To(BeFalse())
 
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 
 	item, found, err := cache.Get(obj)
@@ -399,7 +397,7 @@ func Test_LRU_Delete(t *testing.T) {
 		Object: "test-token",
 	}
 
-	err = cache.Add(obj)
+	err = cache.Set(obj)
 	g.Expect(err).ToNot(HaveOccurred())
 
 	err = cache.Delete(obj)
@@ -439,7 +437,7 @@ func Test_LRU_Resize(t *testing.T) {
 			},
 			Object: "test-token",
 		}
-		err = cache.Add(obj)
+		err = cache.Set(obj)
 		g.Expect(err).ToNot(HaveOccurred())
 	}
 
@@ -470,7 +468,7 @@ func TestLRU_Concurrent(t *testing.T) {
 		wg.Add(2)
 		go func() {
 			defer wg.Done()
-			_ = cache.Add(objmap[key])
+			_ = cache.Set(objmap[key])
 		}()
 		go func() {
 			defer wg.Done()

cache/store.go

@@ -28,10 +28,9 @@ import (
 // It is a generic version of the Kubernetes client-go cache.Store interface.
 // See https://pkg.go.dev/k8s.io/client-go/tools/cache#Store
 type Store[T any] interface {
-	// Add adds an object to the store.
-	Add(object T) error
-	// Update updates an object in the store.
-	Update(object T) error
+	// Set adds an object to the store.
+	// It will overwrite the item if it already exists.
+	Set(object T) error
 	// Delete deletes an object from the store.
 	Delete(object T) error
 	// ListKeys returns a list of keys in the store.