pkg/store/redis_store.go (2)

90-121: Handle missing keys explicitly.
When a key is missing in Redis, Get returns an error. Consider distinguishing the redis.Nil error to return nil or a more explicit "not found" message rather than a generic failure.

Here is a sample snippet:

 jsonData, err := s.redisClient.Get(ctx, paramName).Result()
 if err == redis.Nil {
-    return nil, fmt.Errorf("failed to get key: %v", err)
+    return nil, nil // or a custom 'not found' error
 } else if err != nil {
     return nil, fmt.Errorf("failed to get key: %v", err)
 }

---

123-151: Assess context handling strategy.
Using context.Background() for each call is acceptable for small-scale usage. However, you might consider passing a context from the caller to handle cancellation and timeouts more granularly.

pkg/store/store.go (1)

15-15: Fix the typo in the comment.
"containts" should be "contains."

-// then it splits the component if it containts a "/"
+// then it splits the component if it contains a "/"

pkg/store/redis_store_test.go (2)

35-72: Consider enhancing test assertions.

The test is well-structured, but consider adding assertions for:

• The mock call count to verify the exact number of Redis operations
• The actual key format to ensure proper key construction

 // Assert
 assert.NoError(t, err)
 assert.Equal(t, expectedValue, result)
+mockClient.AssertNumberOfCalls(t, "Get", 1)
+assert.Equal(t, "repo/testprefix/mystack/mycomponent/mykey", fullKey)
 mockClient.AssertExpectations(t)

---

107-142: Consider testing with non-zero expiration.

The test currently only validates with zero expiration. Consider adding test cases with:

• Positive expiration duration
• Different expiration values

 func TestRedisStore_Set_Success(t *testing.T) {
+	testCases := []struct {
+		name       string
+		expiration time.Duration
+	}{
+		{"NoExpiration", 0},
+		{"WithExpiration", time.Hour},
+		{"ShortExpiration", time.Minute},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
			// Existing test code but use tc.expiration
+		})
+	}
 }

website/docs/core-concepts/projects/configuration/stores.mdx (1)

131-182: Consider enhancing Redis documentation.

While the documentation is clear, consider adding:

• Example of using Redis with authentication
• Example of using custom stacks_delimiter
• Best practices for key naming conventions

Add these examples:

# Example with authentication
stores:
  prod/redis:
    type: redis
    options:
      url: redis://:password@redis.example.com:6379

# Example with custom delimiter
stores:
  stage/redis:
    type: redis
    options:
      stacks_delimiter: "/"
      url: redis://localhost:6379

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

pkg/store/store.go (1)

18-30: Consider adding input validation.

The function could benefit from validating input parameters to prevent potential issues with empty or malformed inputs.

 func getKey(prefix string, stackDelimiter string, stack string, component string, key string, finalDelimiter string) (string, error) {
+    if stackDelimiter == "" {
+        return "", fmt.Errorf("stack delimiter cannot be empty")
+    }
+    if finalDelimiter == "" {
+        return "", fmt.Errorf("final delimiter cannot be empty")
+    }
     stackParts := strings.Split(stack, stackDelimiter)

pkg/store/redis_store.go (2)

36-51: Consider adding connection timeout.

The Redis connection options could benefit from a configurable timeout to prevent hanging in case of network issues.

 func getRedisOptions(options *RedisStoreOptions) (*redis.Options, error) {
     if options.URL != nil {
         opts, err := redis.ParseURL(*options.URL)
         if err != nil {
             return &redis.Options{}, fmt.Errorf("failed to parse redis url: %v", err)
         }
+        // Add default timeouts
+        opts.DialTimeout = 5 * time.Second
+        opts.ReadTimeout = 3 * time.Second
+        opts.WriteTimeout = 3 * time.Second
         return opts, nil
     }

---

89-120: Consider adding Redis key existence check.

The Get method could benefit from explicitly checking if the key exists before attempting to retrieve it.

     ctx := context.Background()
+    exists, err := s.redisClient.Exists(ctx, paramName).Result()
+    if err != nil {
+        return nil, fmt.Errorf("failed to check key existence: %v", err)
+    }
+    if exists == 0 {
+        return nil, fmt.Errorf("key not found: %s", paramName)
+    }
     jsonData, err := s.redisClient.Get(ctx, paramName).Result()

pkg/store/redis_store_test.go (1)

35-71: Consider adding timeout test cases.

The success test could benefit from additional cases testing timeout scenarios.

+func TestRedisStore_Get_Timeout(t *testing.T) {
+    mockClient := new(MockRedisClient)
+    store, err := NewRedisStore(RedisStoreOptions{
+        Prefix:         ptr("testprefix"),
+        StackDelimiter: ptr("/"),
+        URL:            ptr("redis://localhost:6379"),
+    })
+    assert.NoError(t, err)
+
+    redisStore, ok := store.(*RedisStore)
+    assert.True(t, ok)
+    redisStore.redisClient = mockClient
+
+    // Simulate timeout
+    mockClient.On("Get", context.Background(), mock.Anything).Return("", redis.ErrConnDone)
+
+    result, err := redisStore.Get("stack", "component", "key")
+    assert.Error(t, err)
+    assert.Nil(t, result)
+    assert.Contains(t, err.Error(), "failed to get key")
+}

website/docs/core-concepts/projects/configuration/stores.mdx (1)

131-182: Consider adding Redis configuration best practices.

The Redis documentation would benefit from a section on recommended configurations and best practices.

Add a section on Redis best practices:

#### Best Practices

- Use a prefix to namespace your keys and prevent conflicts
- Consider using SSL/TLS for production environments
- Set appropriate timeouts for your environment
- Use strong passwords in production
- Consider implementing key expiration policies

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

website/docs/core-concepts/projects/configuration/stores.mdx (2)

137-139: Add secure Redis URL example.

The example shows an insecure Redis URL. Consider adding a secure example.

Add secure URL example:

     options:
-      url: redis://localhost:6379
+      url: rediss://username:password@host:6379/0

---

180-181: Enhance Redis authentication documentation.

The authentication section could be more detailed about security best practices.

Add security recommendations:

• Use TLS (rediss://) in production
• Avoid embedding credentials in URLs, use environment variables instead
• Set appropriate Redis ACLs

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro