🔧 rm default Expires response header filter #303
Conversation
WalkthroughThe changes involve the modification of the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
Component: API-Gateway
simonhir
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range comments (1)
refarch-gateway/src/test/java/de/muenchen/refarch/gateway/route/BackendRouteTest.java (1)
Line range hint
32-45: Add test coverage for Expires header behavior.Since the PR removes the
RemoveResponseHeader=Expiresfilter, we should add test coverage to verify that Expires headers now pass through the gateway unchanged. This ensures the new behavior is working as expected.Add the Expires header to the stub response and verify it in the test:
stubFor(get(urlEqualTo("/remote/endpoint")) .willReturn(aResponse() .withStatus(HttpStatus.OK.value()) .withHeaders(new HttpHeaders( new HttpHeader(org.springframework.http.HttpHeaders.CONTENT_TYPE, "application/json"), + new HttpHeader(org.springframework.http.HttpHeaders.EXPIRES, "Wed, 21 Oct 2025 07:28:00 GMT"), new HttpHeader(org.springframework.http.HttpHeaders.WWW_AUTHENTICATE, "Bearer realm=\"Access to the staging site\", charset=\"UTF-8\"") // removed by route filter )) .withBody("{ \"testkey\" : \"testvalue\" }")));And in the test method:
.expectStatus().isEqualTo(HttpStatus.OK) .expectHeader().valueMatches(org.springframework.http.HttpHeaders.CONTENT_TYPE, "application/json") + .expectHeader().valueMatches(org.springframework.http.HttpHeaders.EXPIRES, "Wed, 21 Oct 2025 07:28:00 GMT") .expectHeader().doesNotExist(org.springframework.http.HttpHeaders.WWW_AUTHENTICATE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (4)
refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalAuthenticationErrorFilterTest.java(1 hunks)refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalBackend5xxTo400MapperTest.java(2 hunks)refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalBackendErrorFilterTest.java(1 hunks)refarch-gateway/src/test/java/de/muenchen/refarch/gateway/route/BackendRouteTest.java(1 hunks)
🔇 Additional comments (8)
refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalBackend5xxTo400MapperTest.java (3)
48-48: LGTM! Clean formatting of WWW_AUTHENTICATE header value
The formatting change improves code readability while maintaining the same functionality.
70-70: LGTM! Consistent formatting with previous change
The formatting change maintains consistency with the earlier change in backendError500.
Line range hint 1-83: Verify complete removal of Expires header handling
The changes align with the PR objective of removing the Expires header filter. Let's verify that all Expires header related code has been properly removed from the codebase.
✅ Verification successful
All Expires header related code has been properly removed
The search results show that all occurrences of "expires" in the Java codebase are related to S3 presigned URL functionality, which is a different feature from HTTP Expires header handling. There are no remaining references to HTTP Expires header filtering in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to Expires header in the codebase
rg -i "expires" --type java
Length of output: 21751
refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalBackendErrorFilterTest.java (2)
46-46: LGTM! Header setup correctly simplified
The removal of the Expires header from the test stub aligns with the PR objective of removing the default Expires header filter. The test remains focused on core error handling functionality.
Line range hint 52-60: Verify complete error handling test coverage
The test verifies essential aspects of error handling (status code, content type, WWW-Authenticate removal, error response format). However, let's verify that no other tests were responsible for Expires header behavior.
✅ Verification successful
Let me check the specific usage of "Expires" in the found test file to ensure we have complete context.
Error handling test coverage is complete - no Expires header concerns
The found references to "Expires" are unrelated to HTTP headers - they are part of S3 presigned URL functionality where expiresInMinutes is a parameter for URL expiration. There are no remaining test assertions for HTTP Expires headers in the codebase. The current error handling test coverage in GlobalBackendErrorFilterTest is sufficient.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining Expires header tests that might need updating
# Search for any test files that might still reference Expires headers
echo "Searching for any remaining test assertions for Expires header..."
rg -l "Expires" --type java --glob "*Test.java"
echo "Searching for any remaining header setup in test files..."
ast-grep --pattern 'new HttpHeader("Expires"'
Length of output: 584
Script:
#!/bin/bash
# Check the specific usage of Expires header in the found test file
rg -A 5 -B 5 "Expires" refarch-integrations/refarch-s3-integration/refarch-s3-integration-rest/refarch-s3-integration-rest-client/src/test/java/de/muenchen/refarch/integration/s3/client/repository/presignedurl/PresignedUrlRestRepositoryTest.java
Length of output: 2376
refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalAuthenticationErrorFilterTest.java (2)
40-40: LGTM! Header configuration aligns with PR objectives
The removal of the Expires header from the test stub is consistent with the PR's goal of no longer managing the Expires header at the gateway level.
Line range hint 44-54: Verify complete test coverage for header handling
The test appropriately verifies authentication error scenarios and header management. However, let's ensure we have comprehensive test coverage for all header-related scenarios.
✅ Verification successful
Header handling test coverage is comprehensive and well-implemented
The test coverage for header handling is complete across the codebase. The current test and related test files demonstrate thorough verification of:
- Content-Type header assertions
- WWW-Authenticate header removal
- CSRF token cookie handling
- Error response headers
- Authentication-related headers
Each scenario properly validates both the presence and absence of specific headers as needed, with appropriate assertions for header values and patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other test files that might need similar header-related test updates
# Search for similar test patterns in other files
rg -l "expectHeader|withHeader" --type java src/test/
# Search for any remaining Expires header assertions
rg -i "expires" --type java src/test/
Length of output: 193
Script:
#!/bin/bash
# Description: Check for other test files that might need similar header-related test updates
# Search for similar test patterns in other files
rg -l "expectHeader|withHeader" --type java refarch-gateway/src/test/
# Search for any remaining Expires header assertions
rg -i "expires" --type java refarch-gateway/src/test/
# Look for other test files with HTTP status assertions to ensure complete coverage
rg -l "expectStatus|isEqualTo\(HttpStatus" --type java refarch-gateway/src/test/
# Search for authentication-related test files
rg -l "WithMockUser|Authentication" --type java refarch-gateway/src/test/
Length of output: 2099
Script:
#!/bin/bash
# Let's examine the header assertions in these test files to ensure complete coverage
# Check GlobalAuthenticationErrorFilterTest for all header-related tests
rg "expectHeader|withHeader" -A 3 -B 3 refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalAuthenticationErrorFilterTest.java
# Check other relevant test files for header assertions
rg "expectHeader|withHeader" -A 3 -B 3 refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/CsrfTokenAppendingHelperFilterTest.java
rg "expectHeader|withHeader" -A 3 -B 3 refarch-gateway/src/test/java/de/muenchen/refarch/gateway/filter/GlobalBackendErrorFilterTest.java
Length of output: 2689
refarch-gateway/src/test/java/de/muenchen/refarch/gateway/route/BackendRouteTest.java (1)
Line range hint 1-1: Verify consistent test updates across all gateway test files.
The AI summary indicates changes in other test files. Let's verify that all gateway test files are consistently updated to reflect the new Expires header behavior.
refarch-gateway/src/test/java/de/muenchen/refarch/gateway/route/BackendRouteTest.java
Show resolved
Hide resolved
Description
Remove default-filter
RemoveResponseHeader=Expiresas it can lead to problems with applications.ExpiresandCache-ControlSummary by CodeRabbit
New Features
Expiresheader in responses, improving caching behavior for users.Bug Fixes
Expiresheader was being stripped from responses.