refactor(cache): Changing constructor design to be less chatty

Signed-off-by: Vincent Boutour <[email protected]>
ViBiOh · Aug 11, 2023 · d6c5719 · d6c5719
1 parent 39e9853
commit d6c5719
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 28 deletions.
diff --git a/cmd/http/port.go b/cmd/http/port.go
@@ -26,7 +26,7 @@ func newPort(config configuration, client client, adapter adapter) port {
 		defer end(nil)
 
 		return id, nil
-	}, time.Hour, 4, portTracer)
+	}, portTracer).WithTTL(time.Hour)
 
 	output.template = func(w http.ResponseWriter, r *http.Request) (renderer.Page, error) {
 		var err error

diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go
@@ -43,21 +43,18 @@ type App[K comparable, V any] struct {
 	onMissMany  fetchMany[K, V]
 	ttl         time.Duration
 	concurrency int
-	extendOnHit bool
 }
 
-func New[K comparable, V any](client RedisClient, toKey func(K) string, onMiss fetch[K, V], ttl time.Duration, concurrency int, tracer trace.Tracer) *App[K, V] {
+func New[K comparable, V any](client RedisClient, toKey func(K) string, onMiss fetch[K, V], tracer trace.Tracer) *App[K, V] {
 	client = getClient(client)
 
 	return &App[K, V]{
-		read:        client,
-		write:       client,
-		toKey:       toKey,
-		serializer:  JSONSerializer[V]{},
-		onMiss:      onMiss,
-		ttl:         ttl,
-		concurrency: concurrency,
-		tracer:      tracer,
+		read:       client,
+		write:      client,
+		toKey:      toKey,
+		serializer: JSONSerializer[V]{},
+		onMiss:     onMiss,
+		tracer:     tracer,
 	}
 }
 
@@ -79,8 +76,14 @@ func (a *App[K, V]) WithRead(client RedisClient) *App[K, V] {
 	return a
 }
 
-func (a *App[K, V]) WithExtendOnHit() *App[K, V] {
-	a.extendOnHit = true
+func (a *App[K, V]) WithTTL(ttl time.Duration) *App[K, V] {
+	a.ttl = ttl
+
+	return a
+}
+
+func (a *App[K, V]) WithMaxConcurrency(concurrency int) *App[K, V] {
+	a.concurrency = concurrency
 
 	return a
 }
@@ -117,9 +120,7 @@ func (a *App[K, V]) Get(ctx context.Context, id K) (V, error) {
 	} else if value, ok, err := a.decode([]byte(content)); err != nil {
 		logUnmarshallError(ctx, key, err)
 	} else if ok {
-		if a.extendOnHit {
-			a.extendTTL(ctx, key)
-		}
+		a.extendTTL(ctx, key)
 
 		return value, nil
 	}
@@ -175,7 +176,7 @@ func (a *App[K, V]) decode(content []byte) (value V, ok bool, err error) {
 }
 
 func (a *App[K, V]) extendTTL(ctx context.Context, keys ...string) {
-	if a.write == nil || len(keys) == 0 {
+	if a.write == nil || len(keys) == 0 || a.ttl == 0 {
 		return
 	}
 

diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go
@@ -44,7 +44,6 @@ func TestGet(t *testing.T) {
 						ID: 8000,
 					}, nil
 				},
-				duration: time.Minute,
 			},
 			cacheableItem{
 				ID: 8000,
@@ -59,14 +58,26 @@ func TestGet(t *testing.T) {
 						ID: 8000,
 					}, nil
 				},
-				duration: time.Minute,
 			},
 			cacheableItem{
 				ID: 8000,
 			},
 			nil,
 		},
 		"cached": {
+			args{
+				key: 8000,
+				onMiss: func(_ context.Context, id int) (cacheableItem, error) {
+					return cacheableItem{}, nil
+				},
+				duration: time.Minute,
+			},
+			cacheableItem{
+				ID: 8000,
+			},
+			nil,
+		},
+		"cached no extend": {
 			args{
 				key: 8000,
 				onMiss: func(_ context.Context, id int) (cacheableItem, error) {
@@ -86,7 +97,6 @@ func TestGet(t *testing.T) {
 						ID: 8000,
 					}, nil
 				},
-				duration: time.Minute,
 			},
 			cacheableItem{
 				ID: 8000,
@@ -148,10 +158,7 @@ func TestGet(t *testing.T) {
 				mockRedisClient.EXPECT().Store(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
 			}
 
-			instance := New(mockRedisClient, strconv.Itoa, testCase.args.onMiss, testCase.args.duration, -1, nil)
-			if intention == "cached" {
-				instance = instance.WithExtendOnHit()
-			}
+			instance := New(mockRedisClient, strconv.Itoa, testCase.args.onMiss, nil).WithTTL(testCase.args.duration)
 
 			got, gotErr := instance.Get(context.Background(), testCase.args.key)
 
@@ -229,7 +236,7 @@ func TestGetError(t *testing.T) {
 				mockRedisClient.EXPECT().Load(gomock.Any(), gomock.Any()).Return(nil, nil)
 			}
 
-			instance := New(mockRedisClient, strconv.Itoa, testCase.args.onMiss, testCase.args.duration, -1, nil)
+			instance := New(mockRedisClient, strconv.Itoa, testCase.args.onMiss, nil).WithTTL(testCase.args.duration)
 
 			got, gotErr := instance.Get(context.Background(), testCase.args.key)
 

diff --git a/pkg/cache/list.go b/pkg/cache/list.go
@@ -96,7 +96,7 @@ func (a App[K, V]) handleListSingle(ctx context.Context, onMissError func(K, err
 			if ok {
 				output[index] = value
 
-				if a.extendOnHit {
+				if a.ttl != 0 {
 					extendKeys = append(extendKeys, keys[index])
 				}
 
@@ -131,7 +131,7 @@ func (a App[K, V]) handleListMany(ctx context.Context, items []K, keys, values [
 		if value, ok, err := a.decode([]byte(values[index])); ok {
 			output[index] = value
 
-			if a.extendOnHit {
+			if a.ttl != 0 {
 				extendKeys = append(extendKeys, keys[index])
 			}
 

diff --git a/pkg/redis/redis.go b/pkg/redis/redis.go
@@ -96,7 +96,7 @@ func (a *App) Ping(ctx context.Context) error {
 }
 
 func (a *App) Store(ctx context.Context, key string, value any, duration time.Duration) error {
-	return a.client.SetEx(ctx, key, value, duration).Err()
+	return a.client.Set(ctx, key, value, duration).Err()
 }
 
 func (a *App) Load(ctx context.Context, key string) ([]byte, error) {
@@ -162,6 +162,10 @@ func (a *App) pipelinedGet(ctx context.Context, keys ...string) ([]string, error
 }
 
 func (a *App) Expire(ctx context.Context, ttl time.Duration, keys ...string) error {
+	if len(keys) == 0 {
+		return nil
+	}
+
 	pipeline := a.client.Pipeline()
 
 	for _, key := range keys {
@@ -172,6 +176,10 @@ func (a *App) Expire(ctx context.Context, ttl time.Duration, keys ...string) err
 }
 
 func (a *App) Delete(ctx context.Context, keys ...string) (err error) {
+	if len(keys) == 0 {
+		return nil
+	}
+
 	pipeline := a.client.Pipeline()
 
 	for _, key := range keys {