From bbbc4d6400ef5a3e5b72342d49fe7b1ace7be804 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Jun 2025 16:13:34 +0000 Subject: [PATCH 1/6] Initial plan for issue From 71c3945e3a452f702865783aabe5deef557ee8bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Jun 2025 16:19:29 +0000 Subject: [PATCH 2/6] Implement Redis cache functionality with tests Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- modules/cache/go.mod | 3 + modules/cache/go.sum | 6 ++ modules/cache/module_test.go | 126 ++++++++++++++++++++++++++++++ modules/cache/redis.go | 144 ++++++++++++++++++++++++++++------- 4 files changed, 250 insertions(+), 29 deletions(-) diff --git a/modules/cache/go.mod b/modules/cache/go.mod index b5a9e24c..2db3ef87 100644 --- a/modules/cache/go.mod +++ b/modules/cache/go.mod @@ -11,11 +11,14 @@ require ( require ( github.com/BurntSushi/toml v1.5.0 // indirect + github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect + github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/golobby/cast v1.3.3 // indirect github.com/golobby/config/v3 v3.4.2 // indirect github.com/golobby/dotenv v1.3.2 // indirect github.com/golobby/env/v2 v2.2.4 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect + github.com/redis/go-redis/v9 v9.10.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/modules/cache/go.sum b/modules/cache/go.sum index 60d11ed9..0c7657be 100644 --- a/modules/cache/go.sum +++ b/modules/cache/go.sum @@ -3,11 +3,15 @@ github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg github.com/BurntSushi/toml v1.5.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/GoCodeAlone/modular v1.2.6 h1:3zjPzER5HMIOdQIU8eIO3KUP+0OGu/aQOBLzJbo5xH8= github.com/GoCodeAlone/modular v1.2.6/go.mod h1:oE3e/FGZJeQq9+QXkN4wbKRaOLxaHJYmH2K4aV/kb+8= +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= +github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/golobby/cast v1.3.3 h1:s2Lawb9RMz7YyYf8IrfMQY4IFmA1R/lgfmj97Vc6fig= github.com/golobby/cast v1.3.3/go.mod h1:0oDO5IT84HTXcbLDf1YXuk0xtg/cRDrxhbpWKxwtJCY= github.com/golobby/config/v3 v3.4.2 h1:oIOSo24mC0A8f93ZTL24NDNw0hZ3Tbb34wc1ckn2CsA= @@ -27,6 +31,8 @@ github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsK github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/redis/go-redis/v9 v9.10.0 h1:FxwK3eV8p/CQa0Ch276C7u2d0eNC9kCmAYQ7mCXCzVs= +github.com/redis/go-redis/v9 v9.10.0/go.mod h1:huWgSWd8mW6+m0VPhJjSSQ+d6Nh1VICQ6Q5lHuCH/Iw= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= diff --git a/modules/cache/module_test.go b/modules/cache/module_test.go index 0b465210..24311820 100644 --- a/modules/cache/module_test.go +++ b/modules/cache/module_test.go @@ -214,3 +214,129 @@ func TestExpiration(t *testing.T) { err = module.Stop(ctx) require.NoError(t, err) } + +// TestRedisConfiguration tests Redis configuration handling without actual Redis connection +func TestRedisConfiguration(t *testing.T) { + // Create the module + module := NewModule().(*CacheModule) + + // Initialize with Redis config + app := newMockApp() + err := module.RegisterConfig(app) + require.NoError(t, err) + + // Override config for Redis + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://localhost:6379", + RedisPassword: "", + RedisDB: 0, + ConnectionMaxAge: 60, + } + app.RegisterConfigSection(ModuleName, modular.NewStdConfigProvider(config)) + + err = module.Init(app) + require.NoError(t, err) + + // Ensure we have a Redis cache + assert.IsType(t, &RedisCache{}, module.cacheEngine) + + // Note: We don't start the module here as it would require an actual Redis connection +} + +// TestRedisOperationsWithMockBehavior tests Redis cache operations that don't require a real connection +func TestRedisOperationsWithMockBehavior(t *testing.T) { + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://localhost:6379", + RedisPassword: "", + RedisDB: 0, + ConnectionMaxAge: 60, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Test operations without connection (should return appropriate errors) + _, found := cache.Get(ctx, "test-key") + assert.False(t, found) + + err := cache.Set(ctx, "test-key", "test-value", time.Minute) + assert.Equal(t, ErrNotConnected, err) + + err = cache.Delete(ctx, "test-key") + assert.Equal(t, ErrNotConnected, err) + + err = cache.Flush(ctx) + assert.Equal(t, ErrNotConnected, err) + + _, err = cache.GetMulti(ctx, []string{"key1", "key2"}) + assert.Equal(t, ErrNotConnected, err) + + err = cache.SetMulti(ctx, map[string]interface{}{"key1": "value1"}, time.Minute) + assert.Equal(t, ErrNotConnected, err) + + err = cache.DeleteMulti(ctx, []string{"key1", "key2"}) + assert.Equal(t, ErrNotConnected, err) + + // Test close without connection + err = cache.Close(ctx) + assert.NoError(t, err) +} + +// TestRedisConfigurationEdgeCases tests edge cases in Redis configuration +func TestRedisConfigurationEdgeCases(t *testing.T) { + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "invalid-url", + RedisPassword: "test-password", + RedisDB: 1, + ConnectionMaxAge: 120, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Test connection with invalid URL + err := cache.Connect(ctx) + assert.Error(t, err) +} + +// TestRedisMultiOperationsEmptyInputs tests multi operations with empty inputs +func TestRedisMultiOperationsEmptyInputs(t *testing.T) { + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://localhost:6379", + RedisPassword: "", + RedisDB: 0, + ConnectionMaxAge: 60, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Test GetMulti with empty keys - should return empty map (no connection needed) + results, err := cache.GetMulti(ctx, []string{}) + assert.NoError(t, err) + assert.Equal(t, map[string]interface{}{}, results) + + // Test SetMulti with empty items - should succeed (no connection needed) + err = cache.SetMulti(ctx, map[string]interface{}{}, time.Minute) + assert.NoError(t, err) + + // Test DeleteMulti with empty keys - should succeed (no connection needed) + err = cache.DeleteMulti(ctx, []string{}) + assert.NoError(t, err) +} diff --git a/modules/cache/redis.go b/modules/cache/redis.go index 8454a044..2e2d004c 100644 --- a/modules/cache/redis.go +++ b/modules/cache/redis.go @@ -2,13 +2,16 @@ package cache import ( "context" + "encoding/json" "time" + + "github.com/redis/go-redis/v9" ) // RedisCache implements CacheEngine using Redis type RedisCache struct { config *CacheConfig - client interface{} `json:"-"` // Placeholder for a Redis client; will be initialized in Connect + client *redis.Client } // NewRedisCache creates a new Redis cache engine @@ -20,65 +23,148 @@ func NewRedisCache(config *CacheConfig) *RedisCache { // Connect establishes connection to Redis func (c *RedisCache) Connect(ctx context.Context) error { - // Note: Actual implementation would initialize a Redis client - // This is a placeholder implementation that would be replaced - // when implementing a real Redis client - return nil + opts, err := redis.ParseURL(c.config.RedisURL) + if err != nil { + return err + } + + if c.config.RedisPassword != "" { + opts.Password = c.config.RedisPassword + } + + opts.DB = c.config.RedisDB + opts.ConnMaxLifetime = time.Duration(c.config.ConnectionMaxAge) * time.Second + + c.client = redis.NewClient(opts) + + // Test the connection + return c.client.Ping(ctx).Err() } // Close closes the connection to Redis func (c *RedisCache) Close(ctx context.Context) error { - // Note: Actual implementation would close the Redis client + if c.client != nil { + return c.client.Close() + } return nil } // Get retrieves an item from the Redis cache func (c *RedisCache) Get(ctx context.Context, key string) (interface{}, bool) { - // Note: This is a placeholder implementation - // In a real implementation, we would: - // 1. Get the item from Redis - // 2. Deserialize the JSON data - // 3. Return the value - return nil, false + if c.client == nil { + return nil, false + } + + val, err := c.client.Get(ctx, key).Result() + if err != nil { + if err == redis.Nil { + return nil, false + } + return nil, false + } + + var result interface{} + if err := json.Unmarshal([]byte(val), &result); err != nil { + return nil, false + } + + return result, true } // Set stores an item in the Redis cache with a TTL func (c *RedisCache) Set(ctx context.Context, key string, value interface{}, ttl time.Duration) error { - // Note: This is a placeholder implementation - // In a real implementation, we would: - // 1. Serialize the value to JSON - // 2. Store in Redis with the TTL - return nil + if c.client == nil { + return ErrNotConnected + } + + data, err := json.Marshal(value) + if err != nil { + return ErrInvalidValue + } + + return c.client.Set(ctx, key, data, ttl).Err() } // Delete removes an item from the Redis cache func (c *RedisCache) Delete(ctx context.Context, key string) error { - // Note: This is a placeholder implementation - return nil + if c.client == nil { + return ErrNotConnected + } + + return c.client.Del(ctx, key).Err() } // Flush removes all items from the Redis cache func (c *RedisCache) Flush(ctx context.Context) error { - // Note: This is a placeholder implementation - return nil + if c.client == nil { + return ErrNotConnected + } + + return c.client.FlushDB(ctx).Err() } // GetMulti retrieves multiple items from the Redis cache func (c *RedisCache) GetMulti(ctx context.Context, keys []string) (map[string]interface{}, error) { - // Note: This is a placeholder implementation - return make(map[string]interface{}), nil + if len(keys) == 0 { + return make(map[string]interface{}), nil + } + + if c.client == nil { + return nil, ErrNotConnected + } + + vals, err := c.client.MGet(ctx, keys...).Result() + if err != nil { + return nil, err + } + + result := make(map[string]interface{}, len(keys)) + for i, val := range vals { + if val != nil { + var value interface{} + if str, ok := val.(string); ok { + if err := json.Unmarshal([]byte(str), &value); err == nil { + result[keys[i]] = value + } + } + } + } + + return result, nil } // SetMulti stores multiple items in the Redis cache with a TTL func (c *RedisCache) SetMulti(ctx context.Context, items map[string]interface{}, ttl time.Duration) error { - // Note: This is a placeholder implementation - return nil + if len(items) == 0 { + return nil + } + + if c.client == nil { + return ErrNotConnected + } + + pipe := c.client.Pipeline() + for key, value := range items { + data, err := json.Marshal(value) + if err != nil { + return ErrInvalidValue + } + pipe.Set(ctx, key, data, ttl) + } + + _, err := pipe.Exec(ctx) + return err } // DeleteMulti removes multiple items from the Redis cache func (c *RedisCache) DeleteMulti(ctx context.Context, keys []string) error { - // Note: This is a placeholder implementation - return nil + if len(keys) == 0 { + return nil + } + + if c.client == nil { + return ErrNotConnected + } + + return c.client.Del(ctx, keys...).Err() } - -// Note: The actual Redis implementation would be completed when adding Redis dependency From 975f0010b90eaf9b0366acf23b2a356e399dc768 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Jun 2025 20:36:23 +0000 Subject: [PATCH 3/6] Add comprehensive Redis cache tests to improve coverage Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- modules/cache/coverage.html | 615 +++++++++++++++++++++++++++++++++++ modules/cache/go.mod | 4 +- modules/cache/go.sum | 8 + modules/cache/module_test.go | 221 +++++++++++++ modules/cache/redis.go | 40 +-- 5 files changed, 867 insertions(+), 21 deletions(-) create mode 100644 modules/cache/coverage.html diff --git a/modules/cache/coverage.html b/modules/cache/coverage.html new file mode 100644 index 00000000..b92effa0 --- /dev/null +++ b/modules/cache/coverage.html @@ -0,0 +1,615 @@ + + + + + + cache: Go Coverage Report + + + +
+ +
+ not tracked + + not covered + covered + +
+
+
+ + + + + + + +
+ + + diff --git a/modules/cache/go.mod b/modules/cache/go.mod index 2db3ef87..4646aee7 100644 --- a/modules/cache/go.mod +++ b/modules/cache/go.mod @@ -6,11 +6,13 @@ toolchain go1.24.3 require ( github.com/GoCodeAlone/modular v1.2.6 + github.com/redis/go-redis/v9 v9.10.0 github.com/stretchr/testify v1.10.0 ) require ( github.com/BurntSushi/toml v1.5.0 // indirect + github.com/alicebob/miniredis/v2 v2.35.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect @@ -19,6 +21,6 @@ require ( github.com/golobby/dotenv v1.3.2 // indirect github.com/golobby/env/v2 v2.2.4 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/redis/go-redis/v9 v9.10.0 // indirect + github.com/yuin/gopher-lua v1.1.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/modules/cache/go.sum b/modules/cache/go.sum index 0c7657be..406c15e3 100644 --- a/modules/cache/go.sum +++ b/modules/cache/go.sum @@ -3,6 +3,12 @@ github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg github.com/BurntSushi/toml v1.5.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/GoCodeAlone/modular v1.2.6 h1:3zjPzER5HMIOdQIU8eIO3KUP+0OGu/aQOBLzJbo5xH8= github.com/GoCodeAlone/modular v1.2.6/go.mod h1:oE3e/FGZJeQq9+QXkN4wbKRaOLxaHJYmH2K4aV/kb+8= +github.com/alicebob/miniredis/v2 v2.35.0 h1:QwLphYqCEAo1eu1TqPRN2jgVMPBweeQcR21jeqDCONI= +github.com/alicebob/miniredis/v2 v2.35.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= +github.com/bsm/ginkgo/v2 v2.12.0 h1:Ny8MWAHyOepLGlLKYmXG4IEkioBysk6GpaRTLC8zwWs= +github.com/bsm/ginkgo/v2 v2.12.0/go.mod h1:SwYbGRRDovPVboqFv0tPTcG1sN61LM1Z4ARdbAV9g4c= +github.com/bsm/gomega v1.27.10 h1:yeMWxP2pV2fG3FgAODIY8EiRE3dy0aeFYt4l7wh6yKA= +github.com/bsm/gomega v1.27.10/go.mod h1:JyEr/xRbxbtgWNi8tIEVPUYZ5Dzef52k01W3YH0H+O0= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= @@ -46,6 +52,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/yuin/gopher-lua v1.1.1 h1:kYKnWBjvbNP4XLT3+bPEwAXJx262OhaHDWDVOPjL46M= +github.com/yuin/gopher-lua v1.1.1/go.mod h1:GBR0iDaNXjAgGg9zfCvksxSRnQx76gclCIb7kdAd1Pw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/modules/cache/module_test.go b/modules/cache/module_test.go index 24311820..db6e1a10 100644 --- a/modules/cache/module_test.go +++ b/modules/cache/module_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/GoCodeAlone/modular" + "github.com/alicebob/miniredis/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -340,3 +341,223 @@ func TestRedisMultiOperationsEmptyInputs(t *testing.T) { err = cache.DeleteMulti(ctx, []string{}) assert.NoError(t, err) } + +// TestRedisConnectWithPassword tests connection configuration with password +func TestRedisConnectWithPassword(t *testing.T) { + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://localhost:6379", + RedisPassword: "test-password", + RedisDB: 1, + ConnectionMaxAge: 120, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Test connection with password and different DB - this will fail since no Redis server + // but will exercise the connection configuration code paths + err := cache.Connect(ctx) + assert.Error(t, err) // Expected to fail without Redis server + + // Test Close when client is nil initially + err = cache.Close(ctx) + assert.NoError(t, err) +} + +// TestRedisJSONMarshaling tests JSON marshaling error scenarios +func TestRedisJSONMarshaling(t *testing.T) { + // Start a test Redis server + s := miniredis.RunT(t) + defer s.Close() + + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://" + s.Addr(), + RedisPassword: "", + RedisDB: 0, + ConnectionMaxAge: 60, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Connect to the test Redis server + err := cache.Connect(ctx) + require.NoError(t, err) + defer cache.Close(ctx) + + // Test Set with invalid JSON value (function cannot be marshaled) + err = cache.Set(ctx, "test-key", func() {}, time.Minute) + assert.Equal(t, ErrInvalidValue, err) + + // Test SetMulti with values that cause JSON marshaling errors + invalidItems := map[string]interface{}{ + "valid-key": "valid-value", + "invalid-key": func() {}, // Functions cannot be marshaled to JSON + } + + err = cache.SetMulti(ctx, invalidItems, time.Minute) + assert.Equal(t, ErrInvalidValue, err) +} + +// TestRedisFullOperations tests Redis operations with a test server +func TestRedisFullOperations(t *testing.T) { + // Start a test Redis server + s := miniredis.RunT(t) + defer s.Close() + + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://" + s.Addr(), + RedisPassword: "", + RedisDB: 0, + ConnectionMaxAge: 60, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Test Connect + err := cache.Connect(ctx) + require.NoError(t, err) + + // Test Set and Get + err = cache.Set(ctx, "test-key", "test-value", time.Minute) + assert.NoError(t, err) + + value, found := cache.Get(ctx, "test-key") + assert.True(t, found) + assert.Equal(t, "test-value", value) + + // Test Delete + err = cache.Delete(ctx, "test-key") + assert.NoError(t, err) + + _, found = cache.Get(ctx, "test-key") + assert.False(t, found) + + // Test SetMulti and GetMulti + items := map[string]interface{}{ + "key1": "value1", + "key2": 42, + "key3": map[string]string{"nested": "value"}, + } + + err = cache.SetMulti(ctx, items, time.Minute) + assert.NoError(t, err) + + results, err := cache.GetMulti(ctx, []string{"key1", "key2", "key3", "nonexistent"}) + assert.NoError(t, err) + assert.Equal(t, "value1", results["key1"]) + assert.Equal(t, float64(42), results["key2"]) // JSON unmarshaling returns numbers as float64 + assert.Equal(t, map[string]interface{}{"nested": "value"}, results["key3"]) + assert.NotContains(t, results, "nonexistent") + + // Test DeleteMulti + err = cache.DeleteMulti(ctx, []string{"key1", "key2"}) + assert.NoError(t, err) + + // Verify deletions + _, found = cache.Get(ctx, "key1") + assert.False(t, found) + _, found = cache.Get(ctx, "key2") + assert.False(t, found) + value, found = cache.Get(ctx, "key3") + assert.True(t, found) + assert.Equal(t, map[string]interface{}{"nested": "value"}, value) + + // Test Flush + err = cache.Flush(ctx) + assert.NoError(t, err) + + _, found = cache.Get(ctx, "key3") + assert.False(t, found) + + // Test Close + err = cache.Close(ctx) + assert.NoError(t, err) +} + +// TestRedisGetJSONUnmarshalError tests JSON unmarshaling errors in Get +func TestRedisGetJSONUnmarshalError(t *testing.T) { + // Start a test Redis server + s := miniredis.RunT(t) + defer s.Close() + + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://" + s.Addr(), + RedisPassword: "", + RedisDB: 0, + ConnectionMaxAge: 60, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Connect to the test Redis server + err := cache.Connect(ctx) + require.NoError(t, err) + defer cache.Close(ctx) + + // Manually insert invalid JSON into Redis + s.Set("invalid-json", "this is not valid JSON {") + + // Try to get the invalid JSON value + value, found := cache.Get(ctx, "invalid-json") + assert.False(t, found) + assert.Nil(t, value) +} + +// TestRedisGetWithServerError tests Get with server errors +func TestRedisGetWithServerError(t *testing.T) { + // Start a test Redis server + s := miniredis.RunT(t) + + config := &CacheConfig{ + Engine: "redis", + DefaultTTL: 300, + CleanupInterval: 60, + MaxItems: 10000, + RedisURL: "redis://" + s.Addr(), + RedisPassword: "", + RedisDB: 0, + ConnectionMaxAge: 60, + } + + cache := NewRedisCache(config) + ctx := context.Background() + + // Connect to the test Redis server + err := cache.Connect(ctx) + require.NoError(t, err) + + // Close the server to simulate connection error + s.Close() + + // Try to get a value when server is down + value, found := cache.Get(ctx, "test-key") + assert.False(t, found) + assert.Nil(t, value) + + // Try GetMulti when server is down + results, err := cache.GetMulti(ctx, []string{"key1", "key2"}) + assert.Error(t, err) + assert.Nil(t, results) + + // Close cache + cache.Close(ctx) +} diff --git a/modules/cache/redis.go b/modules/cache/redis.go index 2e2d004c..c7a8f3ff 100644 --- a/modules/cache/redis.go +++ b/modules/cache/redis.go @@ -27,16 +27,16 @@ func (c *RedisCache) Connect(ctx context.Context) error { if err != nil { return err } - + if c.config.RedisPassword != "" { opts.Password = c.config.RedisPassword } - + opts.DB = c.config.RedisDB opts.ConnMaxLifetime = time.Duration(c.config.ConnectionMaxAge) * time.Second - + c.client = redis.NewClient(opts) - + // Test the connection return c.client.Ping(ctx).Err() } @@ -54,7 +54,7 @@ func (c *RedisCache) Get(ctx context.Context, key string) (interface{}, bool) { if c.client == nil { return nil, false } - + val, err := c.client.Get(ctx, key).Result() if err != nil { if err == redis.Nil { @@ -62,12 +62,12 @@ func (c *RedisCache) Get(ctx context.Context, key string) (interface{}, bool) { } return nil, false } - + var result interface{} if err := json.Unmarshal([]byte(val), &result); err != nil { return nil, false } - + return result, true } @@ -76,12 +76,12 @@ func (c *RedisCache) Set(ctx context.Context, key string, value interface{}, ttl if c.client == nil { return ErrNotConnected } - + data, err := json.Marshal(value) if err != nil { return ErrInvalidValue } - + return c.client.Set(ctx, key, data, ttl).Err() } @@ -90,7 +90,7 @@ func (c *RedisCache) Delete(ctx context.Context, key string) error { if c.client == nil { return ErrNotConnected } - + return c.client.Del(ctx, key).Err() } @@ -99,7 +99,7 @@ func (c *RedisCache) Flush(ctx context.Context) error { if c.client == nil { return ErrNotConnected } - + return c.client.FlushDB(ctx).Err() } @@ -108,16 +108,16 @@ func (c *RedisCache) GetMulti(ctx context.Context, keys []string) (map[string]in if len(keys) == 0 { return make(map[string]interface{}), nil } - + if c.client == nil { return nil, ErrNotConnected } - + vals, err := c.client.MGet(ctx, keys...).Result() if err != nil { return nil, err } - + result := make(map[string]interface{}, len(keys)) for i, val := range vals { if val != nil { @@ -129,7 +129,7 @@ func (c *RedisCache) GetMulti(ctx context.Context, keys []string) (map[string]in } } } - + return result, nil } @@ -138,11 +138,11 @@ func (c *RedisCache) SetMulti(ctx context.Context, items map[string]interface{}, if len(items) == 0 { return nil } - + if c.client == nil { return ErrNotConnected } - + pipe := c.client.Pipeline() for key, value := range items { data, err := json.Marshal(value) @@ -151,7 +151,7 @@ func (c *RedisCache) SetMulti(ctx context.Context, items map[string]interface{}, } pipe.Set(ctx, key, data, ttl) } - + _, err := pipe.Exec(ctx) return err } @@ -161,10 +161,10 @@ func (c *RedisCache) DeleteMulti(ctx context.Context, keys []string) error { if len(keys) == 0 { return nil } - + if c.client == nil { return ErrNotConnected } - + return c.client.Del(ctx, keys...).Err() } From 32a4d08fa5c8073d480e390f45100ef11075718c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Jun 2025 15:05:12 +0000 Subject: [PATCH 4/6] Delete coverage.html file and run go mod tidy Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- modules/cache/coverage.html | 615 ------------------------------------ modules/cache/go.mod | 2 +- 2 files changed, 1 insertion(+), 616 deletions(-) delete mode 100644 modules/cache/coverage.html diff --git a/modules/cache/coverage.html b/modules/cache/coverage.html deleted file mode 100644 index b92effa0..00000000 --- a/modules/cache/coverage.html +++ /dev/null @@ -1,615 +0,0 @@ - - - - - - cache: Go Coverage Report - - - -
- -
- not tracked - - not covered - covered - -
-
-
- - - - - - - -
- - - diff --git a/modules/cache/go.mod b/modules/cache/go.mod index 4646aee7..9338324e 100644 --- a/modules/cache/go.mod +++ b/modules/cache/go.mod @@ -6,13 +6,13 @@ toolchain go1.24.3 require ( github.com/GoCodeAlone/modular v1.2.6 + github.com/alicebob/miniredis/v2 v2.35.0 github.com/redis/go-redis/v9 v9.10.0 github.com/stretchr/testify v1.10.0 ) require ( github.com/BurntSushi/toml v1.5.0 // indirect - github.com/alicebob/miniredis/v2 v2.35.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect From 314e64ccf2ab06f0e31e87449c99efe1ac4fcfd6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Jun 2025 15:07:16 +0000 Subject: [PATCH 5/6] Improve error handling in Redis Get function Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- modules/cache/redis.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/cache/redis.go b/modules/cache/redis.go index c7a8f3ff..034a5ce7 100644 --- a/modules/cache/redis.go +++ b/modules/cache/redis.go @@ -3,6 +3,7 @@ package cache import ( "context" "encoding/json" + "errors" "time" "github.com/redis/go-redis/v9" @@ -57,7 +58,7 @@ func (c *RedisCache) Get(ctx context.Context, key string) (interface{}, bool) { val, err := c.client.Get(ctx, key).Result() if err != nil { - if err == redis.Nil { + if errors.Is(err, redis.Nil) { return nil, false } return nil, false From 2e9ebc7e5105f0019529f6598b9f130b6d5ea584 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Jun 2025 14:56:11 +0000 Subject: [PATCH 6/6] Fix linter errors in cache module: wrap external errors and fix context usage Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- modules/cache/memory.go | 6 ++++-- modules/cache/module.go | 41 ++++++++++++++++++++++++++++++++--------- modules/cache/redis.go | 39 ++++++++++++++++++++++++++++++--------- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/modules/cache/memory.go b/modules/cache/memory.go index 54af6cc5..b81bf5fd 100644 --- a/modules/cache/memory.go +++ b/modules/cache/memory.go @@ -30,9 +30,11 @@ func NewMemoryCache(config *CacheConfig) *MemoryCache { // Connect initializes the memory cache func (c *MemoryCache) Connect(ctx context.Context) error { - // Start cleanup goroutine + // Start cleanup goroutine with derived context c.cleanupCtx, c.cancelFunc = context.WithCancel(ctx) - go c.startCleanupTimer(c.cleanupCtx) + go func() { + c.startCleanupTimer(c.cleanupCtx) + }() return nil } diff --git a/modules/cache/module.go b/modules/cache/module.go index e51a794a..609c38f9 100644 --- a/modules/cache/module.go +++ b/modules/cache/module.go @@ -2,6 +2,7 @@ package cache import ( "context" + "fmt" "time" "github.com/GoCodeAlone/modular" @@ -56,7 +57,7 @@ func (m *CacheModule) Init(app modular.Application) error { // Retrieve the registered config section for access cfg, err := app.GetConfigSection(m.name) if err != nil { - return err + return fmt.Errorf("failed to get config section for cache module: %w", err) } m.config = cfg.GetConfig().(*CacheConfig) @@ -84,7 +85,7 @@ func (m *CacheModule) Start(ctx context.Context) error { m.logger.Info("Starting cache module") err := m.cacheEngine.Connect(ctx) if err != nil { - return err + return fmt.Errorf("failed to connect cache engine: %w", err) } return nil } @@ -92,7 +93,10 @@ func (m *CacheModule) Start(ctx context.Context) error { // Stop performs shutdown logic for the module func (m *CacheModule) Stop(ctx context.Context) error { m.logger.Info("Stopping cache module") - return m.cacheEngine.Close(ctx) + if err := m.cacheEngine.Close(ctx); err != nil { + return fmt.Errorf("failed to close cache engine: %w", err) + } + return nil } // Dependencies returns the names of modules this module depends on @@ -133,22 +137,35 @@ func (m *CacheModule) Set(ctx context.Context, key string, value interface{}, tt if ttl == 0 { ttl = time.Duration(m.config.DefaultTTL) * time.Second } - return m.cacheEngine.Set(ctx, key, value, ttl) + if err := m.cacheEngine.Set(ctx, key, value, ttl); err != nil { + return fmt.Errorf("failed to set cache item: %w", err) + } + return nil } // Delete removes an item from the cache func (m *CacheModule) Delete(ctx context.Context, key string) error { - return m.cacheEngine.Delete(ctx, key) + if err := m.cacheEngine.Delete(ctx, key); err != nil { + return fmt.Errorf("failed to delete cache item: %w", err) + } + return nil } // Flush removes all items from the cache func (m *CacheModule) Flush(ctx context.Context) error { - return m.cacheEngine.Flush(ctx) + if err := m.cacheEngine.Flush(ctx); err != nil { + return fmt.Errorf("failed to flush cache: %w", err) + } + return nil } // GetMulti retrieves multiple items from the cache func (m *CacheModule) GetMulti(ctx context.Context, keys []string) (map[string]interface{}, error) { - return m.cacheEngine.GetMulti(ctx, keys) + result, err := m.cacheEngine.GetMulti(ctx, keys) + if err != nil { + return nil, fmt.Errorf("failed to get multiple cache items: %w", err) + } + return result, nil } // SetMulti stores multiple items in the cache @@ -156,10 +173,16 @@ func (m *CacheModule) SetMulti(ctx context.Context, items map[string]interface{} if ttl == 0 { ttl = time.Duration(m.config.DefaultTTL) * time.Second } - return m.cacheEngine.SetMulti(ctx, items, ttl) + if err := m.cacheEngine.SetMulti(ctx, items, ttl); err != nil { + return fmt.Errorf("failed to set multiple cache items: %w", err) + } + return nil } // DeleteMulti removes multiple items from the cache func (m *CacheModule) DeleteMulti(ctx context.Context, keys []string) error { - return m.cacheEngine.DeleteMulti(ctx, keys) + if err := m.cacheEngine.DeleteMulti(ctx, keys); err != nil { + return fmt.Errorf("failed to delete multiple cache items: %w", err) + } + return nil } diff --git a/modules/cache/redis.go b/modules/cache/redis.go index 034a5ce7..8c856abc 100644 --- a/modules/cache/redis.go +++ b/modules/cache/redis.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "time" "github.com/redis/go-redis/v9" @@ -26,7 +27,7 @@ func NewRedisCache(config *CacheConfig) *RedisCache { func (c *RedisCache) Connect(ctx context.Context) error { opts, err := redis.ParseURL(c.config.RedisURL) if err != nil { - return err + return fmt.Errorf("failed to parse Redis URL: %w", err) } if c.config.RedisPassword != "" { @@ -39,13 +40,18 @@ func (c *RedisCache) Connect(ctx context.Context) error { c.client = redis.NewClient(opts) // Test the connection - return c.client.Ping(ctx).Err() + if err := c.client.Ping(ctx).Err(); err != nil { + return fmt.Errorf("failed to ping Redis server: %w", err) + } + return nil } // Close closes the connection to Redis func (c *RedisCache) Close(ctx context.Context) error { if c.client != nil { - return c.client.Close() + if err := c.client.Close(); err != nil { + return fmt.Errorf("failed to close Redis connection: %w", err) + } } return nil } @@ -83,7 +89,10 @@ func (c *RedisCache) Set(ctx context.Context, key string, value interface{}, ttl return ErrInvalidValue } - return c.client.Set(ctx, key, data, ttl).Err() + if err := c.client.Set(ctx, key, data, ttl).Err(); err != nil { + return fmt.Errorf("failed to set Redis key %s: %w", key, err) + } + return nil } // Delete removes an item from the Redis cache @@ -92,7 +101,10 @@ func (c *RedisCache) Delete(ctx context.Context, key string) error { return ErrNotConnected } - return c.client.Del(ctx, key).Err() + if err := c.client.Del(ctx, key).Err(); err != nil { + return fmt.Errorf("failed to delete Redis key %s: %w", key, err) + } + return nil } // Flush removes all items from the Redis cache @@ -101,7 +113,10 @@ func (c *RedisCache) Flush(ctx context.Context) error { return ErrNotConnected } - return c.client.FlushDB(ctx).Err() + if err := c.client.FlushDB(ctx).Err(); err != nil { + return fmt.Errorf("failed to flush Redis database: %w", err) + } + return nil } // GetMulti retrieves multiple items from the Redis cache @@ -116,7 +131,7 @@ func (c *RedisCache) GetMulti(ctx context.Context, keys []string) (map[string]in vals, err := c.client.MGet(ctx, keys...).Result() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get multiple Redis keys: %w", err) } result := make(map[string]interface{}, len(keys)) @@ -154,7 +169,10 @@ func (c *RedisCache) SetMulti(ctx context.Context, items map[string]interface{}, } _, err := pipe.Exec(ctx) - return err + if err != nil { + return fmt.Errorf("failed to execute Redis pipeline for SetMulti: %w", err) + } + return nil } // DeleteMulti removes multiple items from the Redis cache @@ -167,5 +185,8 @@ func (c *RedisCache) DeleteMulti(ctx context.Context, keys []string) error { return ErrNotConnected } - return c.client.Del(ctx, keys...).Err() + if err := c.client.Del(ctx, keys...).Err(); err != nil { + return fmt.Errorf("failed to delete multiple Redis keys: %w", err) + } + return nil }