Repository: gorse-io/gorse

Issue:
Return zero values instead of error if value not exists in cache
Return zero values instead of error if value not exists in cache:

https://github.com/gorse-io/gorse/blob/d8956cf154c14cd567fc4d109d5052b18e74155b/storage/cache/database.go#L230

Meanwhile, add a method to check if value exists for ReturnValue struct.

https://github.com/gorse-io/gorse/blob/d8956cf154c14cd567fc4d109d5052b18e74155b/storage/cache/database.go#L147

Discussion:

Solution patch:
diff --git a/storage/cache/database.go b/storage/cache/database.go
index e3ee318..ee52977 100644
--- a/storage/cache/database.go
+++ b/storage/cache/database.go
@@ -145,18 +145,28 @@ func Time(name string, value time.Time) Value {
 }
 
 type ReturnValue struct {
-	value string
-	err   error
+	value  string
+	err    error
+	exists bool
 }
 
 func (r *ReturnValue) String() (string, error) {
-	return r.value, r.err
+	if r.err != nil {
+		return "", r.err
+	}
+	if !r.exists {
+		return "", nil
+	}
+	return r.value, nil
 }
 
 func (r *ReturnValue) Integer() (int, error) {
 	if r.err != nil {
 		return 0, r.err
 	}
+	if !r.exists {
+		return 0, nil
+	}
 	return strconv.Atoi(r.value)
 }
 
@@ -164,6 +174,9 @@ func (r *ReturnValue) Time() (time.Time, error) {
 	if r.err != nil {
 		return time.Time{}, r.err
 	}
+	if !r.exists {
+		return time.Time{}, nil
+	}
 	t, err := dateparse.ParseAny(r.value)
 	if err != nil {
 		return time.Time{}, errors.Trace(err)
@@ -171,6 +184,10 @@ func (r *ReturnValue) Time() (time.Time, error) {
 	return t.In(time.UTC), nil
 }
 
+func (r *ReturnValue) Exists() bool {
+	return r.exists
+}
+
 type Score struct {
 	Id         string
 	Score      float64
diff --git a/storage/cache/mongodb.go b/storage/cache/mongodb.go
index 0178c62..1032e32 100644
--- a/storage/cache/mongodb.go
+++ b/storage/cache/mongodb.go
@@ -171,14 +171,14 @@ func (m MongoDB) Get(ctx context.Context, name string) *ReturnValue {
 	c := m.client.Database(m.dbName).Collection(m.ValuesTable())
 	r := c.FindOne(ctx, bson.M{"_id": bson.M{"$eq": name}})
 	if err := r.Err(); err == mongo.ErrNoDocuments {
-		return &ReturnValue{err: errors.Annotate(ErrObjectNotExist, name)}
+		return &ReturnValue{value: "", exists: false}
 	} else if err != nil {
-		return &ReturnValue{err: errors.Trace(err)}
+		return &ReturnValue{err: errors.Trace(err), exists: false}
 	}
 	if raw, err := r.DecodeBytes(); err != nil {
-		return &ReturnValue{err: errors.Trace(err)}
+		return &ReturnValue{err: errors.Trace(err), exists: false}
 	} else {
-		return &ReturnValue{value: raw.Lookup("value").StringValue()}
+		return &ReturnValue{value: raw.Lookup("value").StringValue(), exists: true}
 	}
 }
 
diff --git a/storage/cache/no_database.go b/storage/cache/no_database.go
index f75ae0c..d52df78 100644
--- a/storage/cache/no_database.go
+++ b/storage/cache/no_database.go
@@ -50,7 +50,7 @@ func (NoDatabase) Set(_ context.Context, _ ...Value) error {
 
 // Get method of NoDatabase returns ErrNoDatabase.
 func (NoDatabase) Get(_ context.Context, _ string) *ReturnValue {
-	return &ReturnValue{err: ErrNoDatabase}
+	return &ReturnValue{err: ErrNoDatabase, exists: false}
 }
 
 // Delete method of NoDatabase returns ErrNoDatabase.
diff --git a/storage/cache/proxy.go b/storage/cache/proxy.go
index f57696d..445c6f6 100644
--- a/storage/cache/proxy.go
+++ b/storage/cache/proxy.go
@@ -56,7 +56,7 @@ func (p *ProxyServer) Ping(context.Context, *protocol.PingRequest) (*protocol.Pi
 
 func (p *ProxyServer) Get(ctx context.Context, request *protocol.GetRequest) (*protocol.GetResponse, error) {
 	value := p.database.Get(ctx, request.GetName())
-	if errors.Is(value.err, errors.NotFound) {
+	if !value.Exists() {
 		return &protocol.GetResponse{}, nil
 	}
 	return &protocol.GetResponse{Value: proto.String(value.value)}, value.err
@@ -241,12 +241,12 @@ func (p ProxyClient) Get(ctx context.Context, name string) *ReturnValue {
 		Name: name,
 	})
 	if err != nil {
-		return &ReturnValue{err: err}
+		return &ReturnValue{err: err, exists: false}
 	}
 	if resp.Value == nil {
-		return &ReturnValue{err: errors.NotFound}
+		return &ReturnValue{value: "", exists: false}
 	}
-	return &ReturnValue{value: resp.GetValue(), err: err}
+	return &ReturnValue{value: resp.GetValue(), err: err, exists: true}
 }
 
 func (p ProxyClient) Delete(ctx context.Context, name string) error {
diff --git a/storage/cache/redis.go b/storage/cache/redis.go
index d769aa9..1adb81b 100644
--- a/storage/cache/redis.go
+++ b/storage/cache/redis.go
@@ -168,11 +168,11 @@ func (r *Redis) Get(ctx context.Context, key string) *ReturnValue {
 	val, err := r.client.Get(ctx, r.Key(key)).Result()
 	if err != nil {
 		if err == redis.Nil {
-			return &ReturnValue{err: errors.Annotate(ErrObjectNotExist, key)}
+			return &ReturnValue{value: "", exists: false}
 		}
-		return &ReturnValue{err: err}
+		return &ReturnValue{err: err, exists: false}
 	}
-	return &ReturnValue{value: val}
+	return &ReturnValue{value: val, exists: true}
 }
 
 // Delete object from Redis.
diff --git a/storage/cache/sql.go b/storage/cache/sql.go
index f3a69ea..c6e5b97 100644
--- a/storage/cache/sql.go
+++ b/storage/cache/sql.go
@@ -204,18 +204,18 @@ func (db *SQLDatabase) Set(ctx context.Context, values ...Value) error {
 func (db *SQLDatabase) Get(ctx context.Context, name string) *ReturnValue {
 	rs, err := db.gormDB.WithContext(ctx).Table(db.ValuesTable()).Where("name = ?", name).Select("value").Rows()
 	if err != nil {
-		return &ReturnValue{err: errors.Trace(err)}
+		return &ReturnValue{err: errors.Trace(err), exists: false}
 	}
 	defer rs.Close()
 	if rs.Next() {
 		var value string
 		err := rs.Scan(&value)
 		if err != nil {
-			return &ReturnValue{err: errors.Trace(err)}
+			return &ReturnValue{err: errors.Trace(err), exists: false}
 		}
-		return &ReturnValue{value: value}
+		return &ReturnValue{value: value, exists: true}
 	}
-	return &ReturnValue{err: errors.Annotate(ErrObjectNotExist, name)}
+	return &ReturnValue{value: "", exists: false}
 }
 
 func (db *SQLDatabase) Delete(ctx context.Context, name string) error {
diff --git a/worker/worker.go b/worker/worker.go
index 539ee1d..3141553 100644
--- a/worker/worker.go
+++ b/worker/worker.go
@@ -738,9 +738,10 @@ func (w *Worker) checkUserActiveTime(ctx context.Context, userId string) bool {
 	// read active time
 	activeTime, err := w.CacheClient.Get(ctx, cache.Key(cache.LastModifyUserTime, userId)).Time()
 	if err != nil {
-		if !errors.Is(err, errors.NotFound) {
-			log.Logger().Error("failed to read last modify user time", zap.Error(err))
-		}
+		log.Logger().Error("failed to read last modify user time", zap.String("user_id", userId), zap.Error(err))
+		return true
+	}
+	if activeTime.IsZero() {
 		return true
 	}
 	// check active time
@@ -773,19 +774,22 @@ func (w *Worker) checkRecommendCacheOutOfDate(ctx context.Context, userId string
 	}
 
 	// 2. If digest is empty or not match, stale.
-	_, err = w.CacheClient.Get(ctx, cache.Key(cache.OfflineRecommendDigest, userId)).String()
+	digest, err := w.CacheClient.Get(ctx, cache.Key(cache.OfflineRecommendDigest, userId)).String()
 	if err != nil {
-		if !errors.Is(err, errors.NotFound) {
-			log.Logger().Error("failed to load offline recommendation digest", zap.String("user_id", userId), zap.Error(err))
-		}
+		log.Logger().Error("failed to read offline recommendation digest", zap.String("user_id", userId), zap.Error(err))
+		return true
+	}
+	if digest == "" {
 		return true
 	}
 	// read active time
 	activeTime, err = w.CacheClient.Get(ctx, cache.Key(cache.LastModifyUserTime, userId)).Time()
 	if err != nil {
-		if !errors.Is(err, errors.NotFound) {
-			log.Logger().Error("failed to read last modify user time", zap.Error(err))
-		}
+		log.Logger().Error("failed to read last modify user time", zap.String("user_id", userId), zap.Error(err))
+		return true
+	}
+	if activeTime.IsZero() {
+		return true
 	}
 
 	// 3. If update time is empty, stale.



Test patch:
diff --git a/master/rest_test.go b/master/rest_test.go
index 61bf47b..2c4ec74 100644
--- a/master/rest_test.go
+++ b/master/rest_test.go
@@ -37,7 +37,6 @@ import (
 	"github.com/gorse-io/gorse/storage/cache"
 	"github.com/gorse-io/gorse/storage/data"
 	"github.com/gorse-io/gorse/storage/meta"
-	"github.com/juju/errors"
 	"github.com/samber/lo"
 	"github.com/sashabaranov/go-openai"
 	"github.com/steinfletcher/apitest"
@@ -753,8 +752,8 @@ func (suite *MasterAPITestSuite) TestPurge() {
 	suite.purge(w, req)
 	suite.Equal(http.StatusOK, w.Code)
 
-	_, err = suite.CacheClient.Get(ctx, "key").String()
-	suite.ErrorIs(err, errors.NotFound)
+	v := suite.CacheClient.Get(ctx, "key")
+	suite.False(v.Exists())
 	z, err = suite.CacheClient.SearchScores(ctx, "sorted", "", []string{""}, 0, -1)
 	suite.NoError(err)
 	suite.Empty(z)
diff --git a/storage/cache/database_test.go b/storage/cache/database_test.go
index fcb1d0b..5d0fbff 100644
--- a/storage/cache/database_test.go
+++ b/storage/cache/database_test.go
@@ -76,8 +76,10 @@ func (suite *baseTestSuite) TestMeta() {
 	err = suite.Database.Delete(ctx, Key("meta", "1"))
 	suite.NoError(err)
 	// Get meta not existed
-	value, err = suite.Database.Get(ctx, Key("meta", "1")).String()
-	suite.True(errors.Is(err, errors.NotFound), err)
+	ret := suite.Database.Get(ctx, Key("meta", "1"))
+	suite.False(ret.Exists())
+	value, err = ret.String()
+	suite.NoError(err)
 	suite.Equal("", value)
 	// Set meta int
 	err = suite.Database.Set(ctx, Integer(Key("meta", "1"), 2))
@@ -104,6 +106,52 @@ func (suite *baseTestSuite) TestMeta() {
 	suite.NoError(err)
 }
 
+func (suite *baseTestSuite) TestExists() {
+	ctx := context.Background()
+	
+	// Test non-existent key
+	ret := suite.Database.Get(ctx, Key("test", "nonexistent"))
+	suite.False(ret.Exists())
+	value, err := ret.String()
+	suite.NoError(err)
+	suite.Equal("", value)
+	
+	// Set a value
+	err = suite.Database.Set(ctx, String(Key("test", "exists"), "somevalue"))
+	suite.NoError(err)
+	
+	// Test existing key
+	ret = suite.Database.Get(ctx, Key("test", "exists"))
+	suite.True(ret.Exists())
+	value, err = ret.String()
+	suite.NoError(err)
+	suite.Equal("somevalue", value)
+	
+	// Delete the key
+	err = suite.Database.Delete(ctx, Key("test", "exists"))
+	suite.NoError(err)
+	
+	// Test deleted key no longer exists
+	ret = suite.Database.Get(ctx, Key("test", "exists"))
+	suite.False(ret.Exists())
+	
+	// Test with integer values
+	err = suite.Database.Set(ctx, Integer(Key("test", "int"), 42))
+	suite.NoError(err)
+	ret = suite.Database.Get(ctx, Key("test", "int"))
+	suite.True(ret.Exists())
+	intVal, err := ret.Integer()
+	suite.NoError(err)
+	suite.Equal(42, intVal)
+	
+	// Test non-existent integer - should return 0 with no error
+	ret = suite.Database.Get(ctx, Key("test", "noint"))
+	suite.False(ret.Exists())
+	intVal, err = ret.Integer()
+	suite.NoError(err)
+	suite.Equal(0, intVal)
+}
+
 func (suite *baseTestSuite) TestScan() {
 	ctx := context.Background()
 	err := suite.Database.Set(ctx, String("1", "1"))
@@ -126,12 +174,13 @@ func (suite *baseTestSuite) TestPurge() {
 	ret := suite.Database.Get(ctx, "key")
 	suite.NoError(ret.err)
 	suite.Equal("value", ret.value)
+	suite.True(ret.Exists())
 
 	// purge data
 	err = suite.Database.Purge()
 	suite.NoError(err)
 	ret = suite.Database.Get(ctx, "key")
-	suite.ErrorIs(ret.err, errors.NotFound)
+	suite.False(ret.Exists())
 
 	// purge empty dataset
 	err = suite.Database.Purge()