fix(cache): remove old group data if new data is empty on refresh

ThinkChaos · Nov 12, 2023 · 435615b · 435615b
1 parent 3e0df5f
commit 435615b
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 3 deletions.
diff --git a/cache/stringcache/chained_grouped_cache_test.go b/cache/stringcache/chained_grouped_cache_test.go
@@ -87,7 +87,7 @@ var _ = Describe("Chained grouped cache", func() {
 				Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group1", "group2"))
 			})
 
-			It("Should replace group content on refresh", func() {
+			It("should replace group content on refresh", func() {
 				factory = cache.Refresh("group1")
 				factory.AddEntry("newString")
 				factory.Finish()
@@ -99,6 +99,19 @@ var _ = Describe("Chained grouped cache", func() {
 				Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2"))
 				Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group2"))
 			})
+
+			It("should replace empty groups on refresh", func() {
+				factory = cache.Refresh("group")
+				factory.AddEntry("begone")
+				factory.Finish()
+
+				Expect(cache.ElementCount("group")).Should(BeNumerically("==", 2))
+
+				factory = cache.Refresh("group")
+				factory.Finish()
+
+				Expect(cache.ElementCount("group")).Should(BeNumerically("==", 0))
+			})
 		})
 	})
 })
diff --git a/cache/stringcache/in_memory_grouped_cache.go b/cache/stringcache/in_memory_grouped_cache.go
@@ -57,8 +57,13 @@ func (c *InMemoryGroupedCache) Refresh(group string) GroupFactory {
 		factory: c.factoryFn(),
 		finishFn: func(sc stringCache) {
 			c.lock.Lock()
-			c.caches[group] = sc
-			c.lock.Unlock()
+			defer c.lock.Unlock()
+
+			if sc != nil {
+				c.caches[group] = sc
+			} else {
+				delete(c.caches, group)
+			}
 		},
 	}
 }

diff --git a/cache/stringcache/string_caches.go b/cache/stringcache/string_caches.go
@@ -107,6 +107,10 @@ func (s *stringCacheFactory) addEntry(entry string) {
 }
 
 func (s *stringCacheFactory) create() stringCache {
+	if len(s.tmp) == 0 {
+		return nil
+	}
+
 	cache := make(stringMap, len(s.tmp))
 	for k, v := range s.tmp {
 		cache[k] = strings.Join(v, "")
@@ -161,6 +165,10 @@ func (r *regexCacheFactory) count() int {
 }
 
 func (r *regexCacheFactory) create() stringCache {
+	if len(r.cache) == 0 {
+		return nil
+	}
+
 	return r.cache
 }
 

diff --git a/cache/stringcache/string_caches_test.go b/cache/stringcache/string_caches_test.go
@@ -11,6 +11,19 @@ var _ = Describe("Caches", func() {
 		factory cacheFactory
 	)
 	Describe("String StringCache", func() {
+		It("should not return a cache when empty", func() {
+			Expect(newStringCacheFactory().create()).Should(BeNil())
+		})
+
+		It("should recognise the empty string", func() {
+			factory := newStringCacheFactory()
+
+			factory.addEntry("")
+
+			Expect(factory.count()).Should(BeNumerically("==", 0))
+			Expect(factory.create()).Should(BeNil())
+		})
+
 		When("string StringCache was created", func() {
 			BeforeEach(func() {
 				factory = newStringCacheFactory()
@@ -42,6 +55,22 @@ var _ = Describe("Caches", func() {
 	})
 
 	Describe("Regex StringCache", func() {
+		It("should not return a cache when empty", func() {
+			Expect(newRegexCacheFactory().create()).Should(BeNil())
+		})
+
+		It("should recognise invalid regexes", func() {
+			factory := newRegexCacheFactory()
+
+			factory.addEntry("/*/")
+			factory.addEntry("/?/")
+			factory.addEntry("/+/")
+			factory.addEntry("/[/")
+
+			Expect(factory.count()).Should(BeNumerically("==", 0))
+			Expect(factory.create()).Should(BeNil())
+		})
+
 		When("regex StringCache was created", func() {
 			BeforeEach(func() {
 				factory = newRegexCacheFactory()