feat: Add response-cache plugin #3061
Conversation
…update Go module version, and enhance README documentation. Add comprehensive tests for cache key extraction from headers and bodies, ensuring proper handling of various configurations. Change-Id: I3c6ee39de0211533931afec3f559603aa7407ecf Co-developed-by: Cursor <noreply@cursor.com>
新增响应缓存插件功能及完善测试覆盖变更概述
变更文件
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3061 +/- ##
==========================================
+ Coverage 43.42% 44.16% +0.73%
==========================================
Files 82 87 +5
Lines 10922 11199 +277
==========================================
+ Hits 4743 4946 +203
- Misses 5850 5897 +47
- Partials 329 356 +27
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题,需立即修复。例如:系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 1 | 严重问题,高优先级修复。例如:核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 3 | 主要问题,建议修复。例如:非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 1 | 次要问题,酬情优化。例如:代码格式不规范或注释缺失。 |
总计: 5 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议,聚焦于代码的可读性、可维护性和潜在问题。
🔹 plugins/wasm-go/extensions/response-cache/cache/provider.go (2 💬)
- 默认端口逻辑应与文档保持一致,避免因服务类型判断错误导致连接失败。 (L65-L72)
- 应移除冗余的字段初始化代码以提高可读性和维护性。 (L74-L81)
🔹 plugins/wasm-go/extensions/response-cache/cache/redis.go (1 💬)
- Redis 客户端初始化失败时未向上传播错误,影响故障排查效率。 (L42-L48)
🔹 plugins/wasm-go/extensions/response-cache/config/config.go (1 💬)
- 响应码验证逻辑存在提前返回的风险,可能导致部分有效配置被忽略。 (L39-L46)
🔹 plugins/wasm-go/extensions/response-cache/core.go (1 💬)
🚀 架构设计建议
以下是对代码架构和设计的综合分析,聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 架构一致性问题:缓存提供者初始化逻辑分散
当前缓存提供者的初始化逻辑分散在多个文件中,例如在 provider.go 中定义了接口和初始化逻辑,而在 redis.go 中实现了具体的初始化。这种分散可能导致未来新增缓存类型时维护成本增加。建议将初始化逻辑集中到一个统一的工厂类或初始化管理器中,以提高代码的可维护性和扩展性。
未来新增缓存类型时,可能需要修改多个文件,增加出错风险和维护成本。
🔍2. 跨文件问题:配置解析逻辑重复
在 provider.go 和 config.go 中都存在对配置项的解析逻辑,尤其是对默认值的处理。这种重复可能导致配置解析不一致,增加调试和维护难度。建议将配置解析逻辑统一到一个地方,避免重复代码。
📌 关键代码
func (c *ProviderConfig) FromJson(json gjson.Result) {
c.typ = json.Get("type").String()
c.serviceName = json.Get("serviceName").String()
c.servicePort = int(json.Get("servicePort").Int())
if !json.Get("servicePort").Exists() {
if strings.HasSuffix(c.serviceName, ".static") {
// use default logic port which is 80 for static service
c.servicePort = 80
} else {
c.servicePort = 6379
}
}
c.serviceHost = json.Get("serviceHost").String()
c.username = json.Get("username").String()
if !json.Get("username").Exists() {
c.username = ""
}
c.password = json.Get("password").String()
if !json.Get("password").Exists() {
c.password = ""
}
c.timeout = uint32(json.Get("timeout").Int())
if !json.Get("timeout").Exists() {
c.timeout = 10000
}
c.cacheTTL = int(json.Get("cacheTTL").Int())
if !json.Get("cacheTTL").Exists() {
c.cacheTTL = 0
// c.cacheTTL = 3600000
}
if json.Get("cacheKeyPrefix").Exists() {
c.cacheKeyPrefix = json.Get("cacheKeyPrefix").String()
} else {
c.cacheKeyPrefix = DEFAULT_CACHE_PREFIX
}
}func (c *PluginConfig) FromJson(json gjson.Result) {
c.cacheProviderConfig.FromJson(json.Get("cache"))
c.CacheKeyFromHeader = json.Get("cacheKeyFromHeader").String()
c.CacheKeyFromBody = json.Get("cacheKeyFromBody").String()
c.CacheValueFromBodyType = json.Get("cacheValueFromBodyType").String()
if c.CacheValueFromBodyType == "" {
c.CacheValueFromBodyType = "application/json"
}
c.CacheValueFromBody = json.Get("cacheValueFromBody").String()
cacheResponseCode := json.Get("cacheResponseCode").Array()
c.CacheResponseCode = make([]int32, 0, len(cacheResponseCode))
for _, v := range cacheResponseCode {
responseCode, err := strconv.Atoi(v.String())
if err != nil || responseCode < 100 || responseCode > 999 {
log.Errorf("Skip invalid response_code value: %s", v.String())
return
}
c.CacheResponseCode = append(c.CacheResponseCode, int32(responseCode))
}
if len(c.CacheResponseCode) == 0 {
c.CacheResponseCode = []int32{200}
}
}配置解析不一致可能导致缓存功能异常,增加调试和维护难度。
🔍3. 技术债务:错误处理不一致
在多个文件中,错误处理的方式不一致。例如,在 redis.go 中,Redis客户端初始化失败时未向上传播错误,而在其他地方则有错误传播。这种不一致可能导致错误处理不完整,影响系统的健壮性。建议统一错误处理机制,确保所有错误都能被正确处理和传播。
📌 关键代码
func (rp *redisProvider) Init(username string, password string, timeout uint32) error {
err := rp.client.Init(rp.config.username, rp.config.password, int64(rp.config.timeout))
if rp.client.Ready() {
log.Info("redis init successfully")
} else {
log.Error("redis init failed, will try later")
}
return err
}错误处理不一致可能导致系统在遇到错误时行为不可预测,影响系统的稳定性和可靠性。
🔍4. 性能影响:缓存键生成逻辑可能成为瓶颈
在 core.go 中,缓存键的生成逻辑涉及字符串拼接和多次调用。如果请求量大,这种操作可能成为性能瓶颈。建议优化缓存键生成逻辑,减少不必要的字符串操作,提高性能。
📌 关键代码
queryKey := activeCacheProvider.GetCacheKeyPrefix() + key在高并发场景下,缓存键生成逻辑可能成为性能瓶颈,影响系统响应速度。
🔍5. 安全风险:缓存键和值的处理缺乏安全校验
在 core.go 和 main.go 中,缓存键和值的处理缺乏安全校验,可能存在注入攻击的风险。建议对缓存键和值进行严格的校验和过滤,防止恶意输入导致的安全问题。
📌 关键代码
queryKey := activeCacheProvider.GetCacheKeyPrefix() + keykey = bodyJson.Get(c.CacheKeyFromBody).String()缺乏安全校验可能导致缓存键和值被恶意利用,引发安全问题。
审查详情
📒 文件清单 (11 个文件)
✅ 新增: 11 个文件
✅ 新增文件:
plugins/wasm-go/extensions/response-cache/README.mdplugins/wasm-go/extensions/response-cache/README_EN.mdplugins/wasm-go/extensions/response-cache/cache/provider.goplugins/wasm-go/extensions/response-cache/cache/redis.goplugins/wasm-go/extensions/response-cache/config/config.goplugins/wasm-go/extensions/response-cache/core.goplugins/wasm-go/extensions/response-cache/go.modplugins/wasm-go/extensions/response-cache/go.sumplugins/wasm-go/extensions/response-cache/main.goplugins/wasm-go/extensions/response-cache/main_test.goplugins/wasm-go/extensions/response-cache/option.yaml
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论,lingma-agents 将自动处理您的请求。例如:
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案,并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如:
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如:
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
… key construction, streamline cache key usage in CheckCacheForKey and cacheResponse functions, and clean up unused username and password checks in ProviderConfig. Change-Id: I784f4811ee3f38a4f58db37b07075c545366763c Co-developed-by: Cursor <noreply@cursor.com>
Co-authored-by: mirror58229 <674958229@qq.com>
Co-authored-by: mirror58229 <674958229@qq.com>
Co-authored-by: mirror58229 <674958229@qq.com>
Co-authored-by: mirror58229 <674958229@qq.com>
Co-authored-by: mirror58229 <674958229@qq.com>
Ⅰ. Describe what this PR did
This PR fixes issues in the response-cache plugin implementation #2025 and adds comprehensive unit tests:
Bug Fixes:
cacheKeyFromHeaderorcacheKeyFromBodyis empty - now properly skips response processing instead of unnecessarily buffering response bodieslogparameter from cache provider functions to fix interface mismatchwrapper.ParseConfigBy→wrapper.ParseConfig, etc.)Improvements:
Testing:
main_test.go) covering:Ⅱ. Does this pull request fix one issue?
No specific issue, but fixes critical bugs in PR #2025.
Ⅲ. Why don't you add test cases (unit test/integration test)?
✅ Comprehensive unit tests have been added.
The test suite includes:
TestParseConfig: Tests configuration validation for various scenarios (header key, body key, conflicts, missing cache provider)TestOnHttpRequestHeaders: Tests request header processing for different content typesTestOnHttpRequestBody: Tests body parsing and key extractionTestOnHttpResponseHeaders: Tests response header processingTestOnHttpResponseBody: Tests response body extractionTestCacheHitFlow: Tests complete cache hit/miss flows with Redis mockingAll tests use proper Redis response mocking via the test framework utilities.
Ⅳ. Describe how to verify it
Run unit tests:
Expected test results:
Ⅴ. Special notes for reviews
This PR addresses several critical issues from the original PR #2025:
The changes maintain backward compatibility while fixing these issues and adding robust test coverage.