From f05a9c5dec3acab34f9601dcbb9cfe6706ec3129 Mon Sep 17 00:00:00 2001
From: Thomas LE ROUX <thomas@leroux.io>
Date: Fri, 3 Apr 2020 16:31:56 +0200
Subject: [PATCH 1/2] fix(redis): PTTL return ns with redis 5.0 and go-redis/v7

---
 drivers/store/redis/store.go      | 19 ++++++++++---
 drivers/store/redis/store_test.go | 46 +++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/store/redis/store.go b/drivers/store/redis/store.go
index cbf981c..d4a311b 100644
--- a/drivers/store/redis/store.go
+++ b/drivers/store/redis/store.go
@@ -260,10 +260,9 @@ func updateValue(rtx *libredis.Tx, key string, expiration time.Duration) (int64,
 	}
 
 	// If ttl is -1ms, we have to define key expiration.
-	// PTTL return values changed as of Redis 2.8
-	// Now the command returns -2ms if the key does not exist, and -1ms if the key exists, but there is no expiry set
-	// We shouldn't try to set an expiry on a key that doesn't exist
-	if ttl == (-1 * time.Millisecond) {
+	// The PTTL command returns -2 if the key does not exist, and -1 if the key exists, but there is no expiry set.
+	// We shouldn't try to set an expiry on a key that doesn't exist.
+	if isKeyExpired(ttl) {
 		expire := rtx.Expire(key, expiration)
 
 		ok, err := expire.Result()
@@ -313,3 +312,15 @@ func (store *Store) ping() (bool, error) {
 
 	return (pong == "PONG"), nil
 }
+
+// isKeyExpired returns from PTTL command -2 if the key does not exist, and -1 if the key exists.
+// Usually, it should be returned in nanosecond, but some users have reported that it could be in millisecond as well.
+// Better safe than sorry: we handle both.
+func isKeyExpired(ttl time.Duration) bool {
+	switch ttl {
+	case -1 * time.Nanosecond, -1 * time.Millisecond:
+		return true
+	default:
+		return false
+	}
+}
diff --git a/drivers/store/redis/store_test.go b/drivers/store/redis/store_test.go
index be58c31..7b0c103 100644
--- a/drivers/store/redis/store_test.go
+++ b/drivers/store/redis/store_test.go
@@ -3,6 +3,7 @@ package redis_test
 import (
 	"os"
 	"testing"
+	"time"
 
 	libredis "github.com/go-redis/redis/v7"
 	"github.com/stretchr/testify/require"
@@ -46,6 +47,51 @@ func TestRedisStoreConcurrentAccess(t *testing.T) {
 	tests.TestStoreConcurrentAccess(t, store)
 }
 
+func TestRedisClientExpiration(t *testing.T) {
+	is := require.New(t)
+
+	client, err := newRedisClient()
+	is.NoError(err)
+	is.NotNil(client)
+
+	key := "foobar"
+	value := 642
+	keyNoExpiration := -1 * time.Nanosecond
+	keyNotExist := -2 * time.Nanosecond
+
+	delCmd := client.Del(key)
+	_, err = delCmd.Result()
+	is.NoError(err)
+
+	expCmd := client.PTTL(key)
+	ttl, err := expCmd.Result()
+	is.NoError(err)
+	is.Equal(keyNotExist, ttl)
+
+	setCmd := client.Set(key, value, 0)
+	_, err = setCmd.Result()
+	is.NoError(err)
+
+	expCmd = client.PTTL(key)
+	ttl, err = expCmd.Result()
+	is.NoError(err)
+	is.Equal(keyNoExpiration, ttl)
+
+	setCmd = client.Set(key, value, time.Second)
+	_, err = setCmd.Result()
+	is.NoError(err)
+
+	time.Sleep(100 * time.Millisecond)
+
+	expCmd = client.PTTL(key)
+	ttl, err = expCmd.Result()
+	is.NoError(err)
+
+	expected := int64(0)
+	actual := int64(ttl)
+	is.Greater(actual, expected)
+}
+
 func newRedisClient() (*libredis.Client, error) {
 	uri := "redis://localhost:6379/0"
 	if os.Getenv("REDIS_URI") != "" {

From 3604078a686fb9df41aa66b1a740094165e232b1 Mon Sep 17 00:00:00 2001
From: Thomas LE ROUX <thomas@leroux.io>
Date: Fri, 3 Apr 2020 16:57:25 +0200
Subject: [PATCH 2/2] refactor(redis): Better naming

---
 drivers/store/redis/store.go | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/store/redis/store.go b/drivers/store/redis/store.go
index d4a311b..af97931 100644
--- a/drivers/store/redis/store.go
+++ b/drivers/store/redis/store.go
@@ -259,10 +259,10 @@ func updateValue(rtx *libredis.Tx, key string, expiration time.Duration) (int64,
 		return 0, 0, err
 	}
 
-	// If ttl is -1ms, we have to define key expiration.
+	// If ttl is less than zero, we have to define key expiration.
 	// The PTTL command returns -2 if the key does not exist, and -1 if the key exists, but there is no expiry set.
 	// We shouldn't try to set an expiry on a key that doesn't exist.
-	if isKeyExpired(ttl) {
+	if isExpirationRequired(ttl) {
 		expire := rtx.Expire(key, expiration)
 
 		ok, err := expire.Result()
@@ -313,10 +313,11 @@ func (store *Store) ping() (bool, error) {
 	return (pong == "PONG"), nil
 }
 
-// isKeyExpired returns from PTTL command -2 if the key does not exist, and -1 if the key exists.
+// isExpirationRequired returns if we should set an expiration on a key, using (error) result from PTTL command.
+// The error code is -2 if the key does not exist, and -1 if the key exists.
 // Usually, it should be returned in nanosecond, but some users have reported that it could be in millisecond as well.
 // Better safe than sorry: we handle both.
-func isKeyExpired(ttl time.Duration) bool {
+func isExpirationRequired(ttl time.Duration) bool {
 	switch ttl {
 	case -1 * time.Nanosecond, -1 * time.Millisecond:
 		return true