Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3772 +/- ##
==========================================
+ Coverage 94.90% 94.93% +0.02%
==========================================
Files 111 111
Lines 3870 3886 +16
Branches 780 783 +3
==========================================
+ Hits 3673 3689 +16
Misses 197 197
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request fixes browser caching behavior for Swagger UI and ReDoc by properly implementing HTTP caching with ETags. The changes address issues where ETags were not being used due to non-zero max-age, incorrect weak ETag types for hashed files, and missing 304 Not Modified responses.
Changes:
- Changed default cache lifetime from 7 days to 0 days to enable ETag-based caching
- Fixed ETag generation to use strong ETags instead of weak ETags for content-hashed files
- Implemented HTTP 304 Not Modified responses when If-None-Match header matches the ETag
- Fixed document URL endpoint to properly serialize Urls property when custom JSON serializer options are used
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Swashbuckle.AspNetCore.SwaggerUI/SwaggerUIOptions.cs | Changed default CacheLifetime from 7 days to TimeSpan.Zero to enable ETag-based validation |
| src/Swashbuckle.AspNetCore.SwaggerUI/SwaggerUIMiddleware.cs | Implemented 304 response handling, fixed ETag generation to use strong ETags, refactored to check If-None-Match header, fixed document URL serialization bug |
| src/Swashbuckle.AspNetCore.ReDoc/ReDocOptions.cs | Changed default CacheLifetime from 7 days to TimeSpan.Zero to enable ETag-based validation |
| src/Swashbuckle.AspNetCore.ReDoc/ReDocMiddleware.cs | Implemented 304 response handling, fixed ETag generation to use strong ETags, refactored to check If-None-Match header |
| test/Swashbuckle.AspNetCore.IntegrationTests/SwaggerUIIntegrationTests.cs | Updated test assertions for strong ETags and zero max-age, added tests for 304 Not Modified responses |
| test/Swashbuckle.AspNetCore.IntegrationTests/ReDocIntegrationTests.cs | Updated test assertions for strong ETags and zero max-age, added tests for 304 Not Modified responses |
Comments suppressed due to low confidence (1)
src/Swashbuckle.AspNetCore.ReDoc/ReDocMiddleware.cs:136
- The status code and ContentType are being set before checking If-None-Match header. When the response is 304 Not Modified, these headers should not be set. The ContentType and status code should only be set after verifying that the content has changed (i.e., when ifNoneMatch != etag). This is inconsistent with how SwaggerUIMiddleware handles the same scenario (lines 165-166 in SwaggerUIMiddleware.cs), where ContentType and StatusCode are only set in the else block after confirming the content has changed.
response.StatusCode = StatusCodes.Status200OK;
Stream stream;
switch (fileName)
{
case "index.css":
response.ContentType = "text/css";
stream = ResourceHelper.GetEmbeddedResource(fileName);
break;
case "index.js":
response.ContentType = "application/javascript;charset=utf-8";
stream = ResourceHelper.GetEmbeddedResource(fileName);
break;
default:
response.ContentType = "text/html;charset=utf-8";
stream = _options.IndexStream();
break;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pass the current HttpContext's CancellationToken to the write for the document URLs.
| } | ||
|
|
||
| json ??= JsonSerializer.Serialize(_options.ConfigObject, _jsonSerializerOptions); | ||
| json ??= JsonSerializer.Serialize(_options.ConfigObject.Urls, _jsonSerializerOptions); |
There was a problem hiding this comment.
Doesn't this still suffer the issue? The json variable has a non-null default value, so if you pass json options it won't null coalesce
Or am I missing something?
Fix URLs not being serialized correctly if a custom `JsonSerializerOptions` is provided. See #3772 (comment).
Fix URLs not being serialized correctly if a custom `JsonSerializerOptions` is provided. See #3772 (comment).
Pull Request
The issue or feature being addressed
#3709 and #3768.
Details on the issue fix or feature implementation
max-age(SwaggerUI combo for api definitions is cached / not refreshed #3709).