From 0eacac0c796301e101ddcba4546aa4f3ce7663e6 Mon Sep 17 00:00:00 2001
From: Carsten Dietrich <3203968+carstendietrich@users.noreply.github.com>
Date: Wed, 4 Oct 2023 15:26:05 +0200
Subject: [PATCH] fix(redis): Correctly set expiry in seconds, drop usage of
 deprecated `SETEX` (#48)

Previously the expiration was not properly set in seconds, therefore keys basically never expired.
Replaced the SETEX with SET and PX argument since SETEX is deprecated. Therefore we can now have expiries even in milliseconds.

Covered expiration with tests for all backends.
---
 backendTestCase_test.go | 11 +++++++++++
 inMemoryBackend.go      | 19 +++++++++++++++++--
 inMemoryBackend_test.go |  3 ++-
 redisBackend.go         |  7 ++++---
 twoLevelBackend_test.go |  3 ++-
 5 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/backendTestCase_test.go b/backendTestCase_test.go
index c9b172f..9cfe98f 100644
--- a/backendTestCase_test.go
+++ b/backendTestCase_test.go
@@ -42,6 +42,8 @@ func NewBackendTestCase(t *testing.T, backend httpcache.Backend, tagsInResult bo
 func (tc *BackendTestCase) RunTests() {
 	tc.testSetGetPurge()
 
+	tc.testSetTTL()
+
 	tc.testFlush()
 
 	if _, ok := tc.backend.(httpcache.TagSupporting); ok {
@@ -178,3 +180,12 @@ func (tc *BackendTestCase) buildEntry(content string, tags []string) httpcache.E
 		Body:       []byte(content),
 	}
 }
+
+func (tc *BackendTestCase) testSetTTL() {
+	entry := tc.buildEntry("expires quickly", nil)
+	entry.Meta.GraceTime = time.Now().Add(200 * time.Millisecond)
+	entry.Meta.LifeTime = entry.Meta.GraceTime
+	tc.setEntry("EXPIRED_ENTRY", entry)
+	time.Sleep(300 * time.Millisecond)
+	tc.shouldNotExist("EXPIRED_ENTRY")
+}
diff --git a/inMemoryBackend.go b/inMemoryBackend.go
index c1b890f..979a39c 100644
--- a/inMemoryBackend.go
+++ b/inMemoryBackend.go
@@ -7,13 +7,14 @@ import (
 	lru "github.com/hashicorp/golang-lru/v2"
 )
 
-const lurkerPeriod = 1 * time.Minute
+const defaultLurkerPeriod = 1 * time.Minute
 
 type (
 	// MemoryBackend implements the cache backend interface with an "in memory" solution
 	MemoryBackend struct {
 		cacheMetrics Metrics
 		pool         *lru.TwoQueueCache[string, inMemoryCacheEntry]
+		lurkerPeriod time.Duration
 	}
 
 	// MemoryBackendConfig config
@@ -25,6 +26,7 @@ type (
 	InMemoryBackendFactory struct {
 		config       MemoryBackendConfig
 		frontendName string
+		lurkerPeriod time.Duration
 	}
 
 	inMemoryCacheEntry struct {
@@ -41,6 +43,12 @@ func (f *InMemoryBackendFactory) SetConfig(config MemoryBackendConfig) *InMemory
 	return f
 }
 
+// SetLurkerPeriod sets the timeframe how often expired cache entries should be checked/cleaned up, if 0 is provided the default period of 1 minute is taken
+func (f *InMemoryBackendFactory) SetLurkerPeriod(period time.Duration) *InMemoryBackendFactory {
+	f.lurkerPeriod = period
+	return f
+}
+
 // SetFrontendName used in Metrics
 func (f *InMemoryBackendFactory) SetFrontendName(frontendName string) *InMemoryBackendFactory {
 	f.frontendName = frontendName
@@ -51,10 +59,17 @@ func (f *InMemoryBackendFactory) SetFrontendName(frontendName string) *InMemoryB
 func (f *InMemoryBackendFactory) Build() (Backend, error) {
 	cache, _ := lru.New2Q[string, inMemoryCacheEntry](f.config.Size)
 
+	lurkerPeriod := defaultLurkerPeriod
+	if f.lurkerPeriod > 0 {
+		lurkerPeriod = f.lurkerPeriod
+	}
+
 	memoryBackend := &MemoryBackend{
 		pool:         cache,
 		cacheMetrics: NewCacheMetrics("memory", f.frontendName),
+		lurkerPeriod: lurkerPeriod,
 	}
+
 	go memoryBackend.lurker()
 
 	return memoryBackend, nil
@@ -115,7 +130,7 @@ func (m *MemoryBackend) Flush() error {
 }
 
 func (m *MemoryBackend) lurker() {
-	for range time.Tick(lurkerPeriod) {
+	for range time.Tick(m.lurkerPeriod) {
 		m.cacheMetrics.recordEntries(int64(m.pool.Len()))
 
 		for _, key := range m.pool.Keys() {
diff --git a/inMemoryBackend_test.go b/inMemoryBackend_test.go
index 27e81ee..2d96e20 100644
--- a/inMemoryBackend_test.go
+++ b/inMemoryBackend_test.go
@@ -2,6 +2,7 @@ package httpcache_test
 
 import (
 	"testing"
+	"time"
 
 	"flamingo.me/httpcache"
 )
@@ -10,7 +11,7 @@ func Test_RunDefaultBackendTestCase_InMemoryBackend(t *testing.T) {
 	t.Parallel()
 
 	f := httpcache.InMemoryBackendFactory{}
-	backend, _ := f.SetConfig(httpcache.MemoryBackendConfig{Size: 100}).SetFrontendName("default").Build()
+	backend, _ := f.SetConfig(httpcache.MemoryBackendConfig{Size: 100}).SetFrontendName("default").SetLurkerPeriod(100 * time.Millisecond).Build()
 
 	testCase := NewBackendTestCase(t, backend, true)
 	testCase.RunTests()
diff --git a/redisBackend.go b/redisBackend.go
index ef5abad..10fa37f 100644
--- a/redisBackend.go
+++ b/redisBackend.go
@@ -217,16 +217,17 @@ func (b *RedisBackend) Set(key string, entry Entry) error {
 	}
 
 	err = conn.Send(
-		"SETEX",
+		"SET",
 		b.createPrefixedKey(key, valuePrefix),
-		int(entry.Meta.GraceTime.Sub(time.Now().Round(time.Second))),
 		buffer,
+		"PX",
+		time.Until(entry.Meta.GraceTime).Round(time.Millisecond).Milliseconds(),
 	)
 	if err != nil {
 		b.cacheMetrics.countError("SetFailed")
 		b.logger.Error(fmt.Sprintf("Error setting key %q with timeout %v and buffer %v", key, entry.Meta.GraceTime, buffer))
 
-		return fmt.Errorf("redis SETEX failed: %w", err)
+		return fmt.Errorf("redis SET PX failed: %w", err)
 	}
 
 	for _, tag := range entry.Meta.Tags {
diff --git a/twoLevelBackend_test.go b/twoLevelBackend_test.go
index f1d042b..ceaa80c 100644
--- a/twoLevelBackend_test.go
+++ b/twoLevelBackend_test.go
@@ -2,6 +2,7 @@ package httpcache_test
 
 import (
 	"testing"
+	"time"
 
 	"flamingo.me/flamingo/v3/framework/flamingo"
 	"github.com/stretchr/testify/assert"
@@ -12,7 +13,7 @@ import (
 func createInMemoryBackend() httpcache.Backend {
 	return func() httpcache.Backend {
 		f := httpcache.InMemoryBackendFactory{}
-		backend, _ := f.SetConfig(httpcache.MemoryBackendConfig{Size: 100}).SetFrontendName("default").Build()
+		backend, _ := f.SetConfig(httpcache.MemoryBackendConfig{Size: 100}).SetFrontendName("default").SetLurkerPeriod(100 * time.Millisecond).Build()
 
 		return backend
 	}()