增强 Cache 体验 #286
增强 Cache 体验 #286
Conversation
增加 对非 GET 请求进行缓存时需显式二次确认
📝 WalkthroughWalkthroughThe CacheAttribute class now supports caching for non-GET requests and error responses. A new EnableNonGet property controls non-GET caching behavior, while ErrorExpiration defines the cache duration for non-200 status codes. Dynamic expiration is managed via a private field and applied through an overridden Expiration property. GetWritePolicy and GetReadPolicy methods implement conditional caching logic based on request type and response status. Changes
Sequence DiagramsequenceDiagram
participant Client
participant CacheAttr as CacheAttribute
participant CacheStore as Cache Store
participant Handler as Response Handler
Client->>CacheAttr: Incoming Request (GET/non-GET)
CacheAttr->>CacheAttr: Check GetReadPolicy()
alt EnableNonGet = false && non-GET
CacheAttr-->>Client: Bypass cache (warn log)
else EnableNonGet = true OR GET
CacheAttr->>CacheStore: Check cache
CacheStore-->>CacheAttr: Return cached data (if exists)
end
Client->>Handler: Send actual request
Handler-->>CacheAttr: Response (status code)
CacheAttr->>CacheAttr: Check GetWritePolicy()
alt Status = 200 OK
CacheAttr->>CacheStore: Cache response (normal Expiration)
CacheAttr->>CacheAttr: Reset _dynamicExpiration
else Status ≠ 200
alt ErrorExpiration > 0
CacheAttr->>CacheStore: Cache error response (ErrorExpiration)
CacheAttr->>CacheAttr: Set _dynamicExpiration
else ErrorExpiration ≤ 0
CacheAttr-->>CacheStore: Skip caching error
end
end
CacheAttr-->>Client: Return response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@WebApiClientCore/Attributes/CacheAttributes/CacheAttribute.cs`:
- Around line 29-38: The Expiration property on CacheAttribute is hidden with
new so calls through the ApiCacheAttribute/IApiCacheAttribute path never hit the
dynamic TTL; change the base ApiCacheAttribute.Expiration to virtual and change
CacheAttribute.Expiration to override so polymorphism works (referencing
ApiCacheAttribute.Expiration and CacheAttribute.Expiration), and replace the
mutable per-attribute field _dynamicExpiration with a request-scoped/functional
approach (e.g., a Func<HttpResponseMessage,TimeSpan?> or a
ComputeExpiration(HttpResponseMessage) method) to avoid thread-safety issues;
also remove Console.WriteLine calls and instead accept/use an ILogger (inject or
pass ILogger to the cache logic) in place of Console.WriteLine occurrences, and
adjust the StatusCode check (currently using StatusCode != HttpStatusCode.OK) to
treat 2xx responses as successful or make the success status range configurable.
🧹 Nitpick comments (1)
WebApiClientCore/Attributes/CacheAttributes/CacheAttribute.cs (1)
98-100: Consider success-range check instead of only 200
StatusCode != HttpStatusCode.OKtreats 201/204 (and other 2xx) as errors, which can be surprising—especially when EnableNonGet is true. ConsiderIsSuccessStatusCodeor an explicit success range.Suggested change (if you want all 2xx treated as success)
- if (response != null && response.StatusCode != HttpStatusCode.OK) + if (response != null && !response.IsSuccessStatusCode)
| /// <summary> | ||
| /// 用于在运行时存储动态计算的过期时间 | ||
| /// </summary> | ||
| private TimeSpan? _dynamicExpiration; | ||
|
|
||
| /// <summary> | ||
| /// 重写 Expiration 属性，使其能返回动态调整后的值 | ||
| /// </summary> | ||
| public new TimeSpan Expiration => _dynamicExpiration ?? base.Expiration; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the ApiCacheAttribute base class definition and check Expiration property signature
find . -type f -name "*.cs" | head -20Repository: dotnetcore/WebApiClient
Length of output: 770
🏁 Script executed:
# Locate ApiCacheAttribute class
rg -l "class ApiCacheAttribute" --type=csRepository: dotnetcore/WebApiClient
Length of output: 131
🏁 Script executed:
# Inspect the base ApiCacheAttribute class
rg -n "class ApiCacheAttribute|Expiration" --type=cs -A 3Repository: dotnetcore/WebApiClient
Length of output: 5583
🏁 Script executed:
# Check CacheAttribute.cs for full context around lines 29-38 and 106-113
wc -l WebApiClientCore/Attributes/CacheAttributes/CacheAttribute.csRepository: dotnetcore/WebApiClient
Length of output: 132
🏁 Script executed:
# Read the full CacheAttribute.cs to see lines 90-120 context
sed -n '90,120p' WebApiClientCore/Attributes/CacheAttributes/CacheAttribute.csRepository: dotnetcore/WebApiClient
Length of output: 1009
🏁 Script executed:
# Check ApiRequestSender to see how attribute is passed
rg -n "SetAsync.*attribute\|provider.SetAsync" --type=cs -B 5 -A 2 WebApiClientCore/Implementations/ApiRequestSender.csRepository: dotnetcore/WebApiClient
Length of output: 49
🏁 Script executed:
# Find GetWritePolicy and GetReadPolicy definitions
rg -n "GetWritePolicy|GetReadPolicy" --type=cs -A 15 WebApiClientCore/Attributes/CacheAttributes/CacheAttribute.csRepository: dotnetcore/WebApiClient
Length of output: 1458
🏁 Script executed:
# Check IApiCacheAttribute interface definition
cat WebApiClientCore.Abstractions/Attributes/IApiCacheAttribute.csRepository: dotnetcore/WebApiClient
Length of output: 1263
🏁 Script executed:
# Check how attribute is passed/used in ApiRequestSender and related code
rg -n "attribute.*Expiration|IApiCacheAttribute.*Expiration|GetWritePolicy|GetReadPolicy" --type=cs -B 3 -A 3 WebApiClientCore/Implementations/ApiRequestSender.csRepository: dotnetcore/WebApiClient
Length of output: 741
🏁 Script executed:
# Find where SetAsync is called and what type attribute is
rg -n "SetAsync" --type=cs -B 10 WebApiClientCore/Implementations/ApiRequestSender.cs | head -40Repository: dotnetcore/WebApiClient
Length of output: 515
🏁 Script executed:
# Check if ApiCacheAttribute.Expiration is read anywhere and from what type
rg -n "\.Expiration" --type=cs WebApiClientCore/Implementations/ | grep -v "ErrorExpiration"Repository: dotnetcore/WebApiClient
Length of output: 226
🏁 Script executed:
# Check the type of 'attribute' in ApiRequestSender - look at method signature
rg -n "DoCachingAsync\|DoResponseCachingAsync" --type=cs -A 20 WebApiClientCore/Implementations/ApiRequestSender.cs | head -60Repository: dotnetcore/WebApiClient
Length of output: 49
🏁 Script executed:
# Find where the attribute variable is declared or retrieved
rg -n "attribute.*=" --type=cs -B 5 WebApiClientCore/Implementations/ApiRequestSender.cs | grep -A 5 "attribute.*GetCustomAttribute\|attribute.*As\|IApiCacheAttribute"Repository: dotnetcore/WebApiClient
Length of output: 49
🏁 Script executed:
# Look at full method context to see how attribute parameter is typed
rg -n "void.*Cach\|async.*Cach" --type=cs -B 2 -A 15 WebApiClientCore/Implementations/ApiRequestSender.cs | head -80Repository: dotnetcore/WebApiClient
Length of output: 49
🏁 Script executed:
# Get the method containing SetAsync call
sed -n '140,180p' WebApiClientCore/Implementations/ApiRequestSender.csRepository: dotnetcore/WebApiClient
Length of output: 1158
🏁 Script executed:
# Look for method signatures around caching
sed -n '70,150p' WebApiClientCore/Implementations/ApiRequestSender.csRepository: dotnetcore/WebApiClient
Length of output: 2810
🏁 Script executed:
# Check the entire file size and structure
wc -l WebApiClientCore/Implementations/ApiRequestSender.cs
head -100 WebApiClientCore/Implementations/ApiRequestSender.csRepository: dotnetcore/WebApiClient
Length of output: 3730
🏁 Script executed:
# Find ActionDescriptor class to see what type CacheAttribute property is
rg -n "class ActionDescriptor\|CacheAttribute" --type=cs WebApiClientCore.Abstractions/ApiActionDescriptor.csRepository: dotnetcore/WebApiClient
Length of output: 49
🏁 Script executed:
# Get the full CacheAttribute property definition in ActionDescriptor
cat WebApiClientCore.Abstractions/ApiActionDescriptor.cs | head -100Repository: dotnetcore/WebApiClient
Length of output: 1826
🏁 Script executed:
# Confirm CacheAttribute is the concrete type being stored as IApiCacheAttribute
rg -n "new CacheAttribute\|typeof.*CacheAttribute" --type=cs -B 2 -A 2Repository: dotnetcore/WebApiClient
Length of output: 49
🏁 Script executed:
# Double-check the Expiration property access path through interface
sed -n '1,50p' WebApiClientCore.Abstractions/Attributes/IApiCacheAttribute.csRepository: dotnetcore/WebApiClient
Length of output: 1263
Expiration property won't be called due to interface access—dynamic TTL feature is broken
CacheAttribute.Expiration is stored and accessed as IApiCacheAttribute (the interface) in ApiRequestSender.SetCacheAsync (line 174). The interface property isn't virtual in the base class, so using new on the derived property doesn't enable polymorphism. The pipeline always calls ApiCacheAttribute.Expiration directly, never the overridden CacheAttribute.Expiration, making _dynamicExpiration unreachable dead code.
To fix this, make Expiration virtual in ApiCacheAttribute, then use override in CacheAttribute. Additionally:
_dynamicExpirationis mutable instance state on a reused attribute, introducing thread safety concerns.Console.WriteLineat lines 88, 101, and 128 logsRequestUri(may contain sensitive data) and is noisy for library consumers. UseILoggerinstead.- Line 100's
StatusCode != HttpStatusCode.OKtreats all non-2xx responses as errors. Confirm this is intended, as 201/204/3xx responses may not warrant short caching.
🤖 Prompt for AI Agents
In `@WebApiClientCore/Attributes/CacheAttributes/CacheAttribute.cs` around lines
29 - 38, The Expiration property on CacheAttribute is hidden with new so calls
through the ApiCacheAttribute/IApiCacheAttribute path never hit the dynamic TTL;
change the base ApiCacheAttribute.Expiration to virtual and change
CacheAttribute.Expiration to override so polymorphism works (referencing
ApiCacheAttribute.Expiration and CacheAttribute.Expiration), and replace the
mutable per-attribute field _dynamicExpiration with a request-scoped/functional
approach (e.g., a Func<HttpResponseMessage,TimeSpan?> or a
ComputeExpiration(HttpResponseMessage) method) to avoid thread-safety issues;
also remove Console.WriteLine calls and instead accept/use an ILogger (inject or
pass ILogger to the cache logic) in place of Console.WriteLine occurrences, and
adjust the StatusCode check (currently using StatusCode != HttpStatusCode.OK) to
treat 2xx responses as successful or make the success status range configurable.
增加 接口异常时的缓存毫秒数（修复第三方接口升级瞬间异常导致了错误缓存）
增加 对非 GET 请求进行缓存时需显式二次确认（提醒避免非GET接口缓存错误使用，坚持使用的需要二次确认）
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.