feat : upgrade to spring boot 3.4.1 #1589
Conversation
|
Warning Rate limit exceeded@rajadilipkolli has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request focuses on updating dependencies and reconfiguring the HTTP client in the Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
github-actions
bot
added
type: dependency-upgrade
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
httpClients/boot-http-proxy/pom.xml (1)
Line range hint
351-370: Consider using GA releases instead of milestones in productionThe project is configured to use Spring milestone repositories. For production environments, it's recommended to:
- Use GA releases instead of milestones for better stability
- Remove milestone repositories unless specifically needed for testing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
httpClients/boot-http-proxy/pom.xml(3 hunks)httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java(2 hunks)
🔇 Additional comments (5)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java (2)
Line range hint 75-86: LGTM: Clean RestClientCustomizer implementation
The RestClientCustomizer implementation follows Spring Boot 3.4 best practices with proper header configuration and interceptor setup. The removal of CloseableHttpClient dependency is appropriate as it's now managed by the factory builder.
Line range hint 1-100: Verify Spring Boot 3.4 compatibility
Let's ensure all dependencies and APIs are compatible with Spring Boot 3.4.
✅ Verification successful
Spring Boot 3.4 compatibility verified
The code is fully compatible with Spring Boot 3.4:
- Using Spring Boot 3.4.1 as parent version
- Using modern
RestClientandHttpServiceProxyFactoryAPIs instead of deprecated ones - Properly configured HTTP Components 5.x client with current best practices
- No deprecated API usage found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for deprecated API usage and compatibility issues
# Search for deprecated API usage
rg -l "import.*\.Deprecated" .
# Check parent version in pom.xml
echo "Checking Spring Boot parent version:"
cat pom.xml | grep -A 1 "<parent>"
# Check for any remaining references to old HTTP client APIs
rg "CloseableHttpClient|HttpClients\.custom|PoolingHttpClientConnectionManager"
Length of output: 1984
Script:
#!/bin/bash
# Let's check the Spring Boot version and dependencies in the module's pom.xml
# Check the specific module's pom.xml
echo "Checking module's pom.xml:"
cat httpClients/boot-http-proxy/pom.xml
# Check for Spring Boot and HTTP Components versions
echo -e "\nChecking dependency versions:"
rg "<spring-boot.version>|<httpclient5.version>" . -g "pom.xml"
# Check for any usage of deprecated Spring Boot HTTP client APIs
echo -e "\nChecking for deprecated API usage:"
rg "RestTemplateBuilder|RestTemplate|WebClient\.Builder|WebClient" httpClients/boot-http-proxy/src/
Length of output: 15853
httpClients/boot-http-proxy/pom.xml (3)
224-224: LGTM: Minor version update for Google Java Format
The update to Google Java Format 1.25.2 is a minor version change and should not introduce any breaking changes.
23-23: Verify SpringDoc OpenAPI compatibility
The upgrade to springdoc-openapi 2.7.0 should be verified for compatibility with Spring Boot 3.4.1.
✅ Verification successful
Let me gather more specific information about the Spring Boot version and springdoc-openapi usage.
Let me check the specific OpenAPI configuration and Spring Boot version details.
SpringDoc OpenAPI 2.7.0 is compatible with Spring Boot 3.4.1
The verification shows that:
- The project uses Spring Boot 3.4.1
- SpringDoc OpenAPI 2.7.0 is correctly configured with
springdoc-openapi-starter-webmvc-ui - The OpenAPI configuration is minimal and follows the standard approach using annotations
- No compatibility issues are apparent in the implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for OpenAPI usage and potential breaking changes
# Search for OpenAPI annotations and configurations
echo "Checking OpenAPI usage..."
rg -l "@Operation|@ApiResponse|@Tag" --type java
# Search for OpenAPI configuration files
echo "Checking OpenAPI configurations..."
fd -e yml -e yaml -e properties | xargs rg -l "springdoc|openapi"
Length of output: 1013
Script:
#!/bin/bash
# Check Spring Boot and springdoc-openapi versions and configurations
# Check parent pom for Spring Boot version
echo "Checking Spring Boot version..."
cat httpClients/boot-http-proxy/pom.xml | grep -A 2 "<parent>"
# Check springdoc-openapi configuration and usage
echo -e "\nChecking springdoc-openapi configuration..."
cat httpClients/boot-http-proxy/src/main/resources/application.properties 2>/dev/null || echo "No application.properties found"
cat httpClients/boot-http-proxy/src/main/resources/application.yml 2>/dev/null || echo "No application.yml found"
# Check for springdoc-openapi specific configurations in Java code
echo -e "\nChecking OpenAPI configuration classes..."
rg -l "OpenAPI|GroupedOpenApi|SpringDocConfiguration" --type java httpClients/boot-http-proxy/
Length of output: 2372
Script:
#!/bin/bash
# Check specific version and configuration details
# Get Spring Boot version
echo "Spring Boot version:"
cat httpClients/boot-http-proxy/pom.xml | grep -A 1 "<artifactId>spring-boot-starter-parent</artifactId>"
# Check SwaggerConfig implementation
echo -e "\nSwaggerConfig implementation:"
cat httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/config/SwaggerConfig.java
# Check springdoc-openapi dependencies
echo -e "\nSpringdoc OpenAPI dependencies:"
cat httpClients/boot-http-proxy/pom.xml | grep -A 3 "springdoc-openapi"
Length of output: 1473
9-9: Verify Spring Boot 3.4.1 milestone release compatibility
The upgrade from Spring Boot 3.3.7 to 3.4.1 is a significant change as it's a milestone release. Please ensure:
- All dependencies are compatible with Spring Boot 3.4.1
- Review the migration guide for any breaking changes
| return ClientHttpRequestFactoryBuilder.httpComponents() | ||
| .withHttpClientCustomizer( | ||
| httpClientBuilder -> | ||
| httpClientBuilder.setKeepAliveStrategy( | ||
| DefaultConnectionKeepAliveStrategy.INSTANCE)) | ||
| .withConnectionManagerCustomizer( | ||
| poolingHttpClientConnectionManagerBuilder -> { | ||
| poolingHttpClientConnectionManagerBuilder.setMaxConnTotal(200); | ||
| poolingHttpClientConnectionManagerBuilder.setMaxConnPerRoute(100); | ||
| poolingHttpClientConnectionManagerBuilder.setTlsSocketStrategy( | ||
| new DefaultClientTlsStrategy( | ||
| sslContext, | ||
| HostnameVerificationPolicy.CLIENT, | ||
| NoopHostnameVerifier.INSTANCE)); | ||
| }) | ||
| .withDefaultRequestConfigCustomizer( | ||
| (builder) -> { | ||
| builder.setConnectionKeepAlive(TimeValue.ofSeconds(10)); | ||
| builder.setConnectionRequestTimeout(Timeout.ofSeconds(30)); | ||
| builder.setResponseTimeout(Timeout.ofSeconds(60)); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make connection settings configurable and remove NoopHostnameVerifier
The HTTP client configuration has several hardcoded values that should be externalized to configuration properties. Additionally, the NoopHostnameVerifier poses a security risk.
Consider these improvements:
- Move connection pool settings to configuration:
-poolingHttpClientConnectionManagerBuilder.setMaxConnTotal(200);
-poolingHttpClientConnectionManagerBuilder.setMaxConnPerRoute(100);
+poolingHttpClientConnectionManagerBuilder.setMaxConnTotal(applicationProperties.getHttp().getMaxConnTotal());
+poolingHttpClientConnectionManagerBuilder.setMaxConnPerRoute(applicationProperties.getHttp().getMaxConnPerRoute());- Make timeout settings configurable:
-builder.setConnectionKeepAlive(TimeValue.ofSeconds(10));
-builder.setConnectionRequestTimeout(Timeout.ofSeconds(30));
-builder.setResponseTimeout(Timeout.ofSeconds(60));
+builder.setConnectionKeepAlive(TimeValue.ofSeconds(applicationProperties.getHttp().getKeepAliveSeconds()));
+builder.setConnectionRequestTimeout(Timeout.ofSeconds(applicationProperties.getHttp().getRequestTimeoutSeconds()));
+builder.setResponseTimeout(Timeout.ofSeconds(applicationProperties.getHttp().getResponseTimeoutSeconds()));- Remove NoopHostnameVerifier when not in development:
- NoopHostnameVerifier.INSTANCE));
+ isProdEnvironment() ? null : NoopHostnameVerifier.INSTANCE));Committable suggestion skipped: line range outside the PR's diff.
...nts/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java
Outdated
Show resolved
Hide resolved
Qodana Community for JVM106 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at [email protected]
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/services/PostService.java (1)
103-104: Consider a more robust approach using explicit persistence operationsInstead of nullifying the ID, consider using a more explicit approach that better represents the intention of creating a new local entity from an external one.
Here's a suggested refactoring:
@Transactional public PostResponse savePost(Post post) { Post fetchedPost = jsonPlaceholderService.createPost(post); - // To fix optimisticLock Exception - fetchedPost.setId(null); + // Create a new local entity from the external post + Post localPost = new Post(); + localPost.setUserId(fetchedPost.getUserId()); + localPost.setTitle(fetchedPost.getTitle()); + localPost.setBody(fetchedPost.getBody()); - Post savedPost = postRepository.save(fetchedPost); + Post savedPost = postRepository.save(localPost); return postMapper.mapToPostResponse(savedPost); }This approach:
- Explicitly shows the intention of creating a new local entity
- Avoids manipulation of entity state
- Makes it clearer which fields are being copied
- Reduces the risk of unintended side effects
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/services/PostService.java(1 hunks)
🔇 Additional comments (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/services/PostService.java (1)
103-104: Verify duplicate prevention logic
While setting ID to null fixes the optimistic locking exception, ensure that there's logic in place to prevent duplicate posts when the same external post is fetched multiple times.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/services/PostService.java (1)
115-120: Consider extracting common entity creation logic.While the implementation is correct, there's duplicate logic between this method and
savePost(). Consider extracting the entity creation logic into a private helper method.+ private Post createLocalPost(Post sourcePost) { + Post localPost = new Post(); + localPost.setUserId(sourcePost.getUserId()); + localPost.setTitle(sourcePost.getTitle()); + localPost.setBody(sourcePost.getBody()); + return localPost; + } @Transactional public Post save(Post postEntity) { - Post localPost = new Post(); - localPost.setUserId(postEntity.getUserId()); - localPost.setTitle(postEntity.getTitle()); - localPost.setBody(postEntity.getBody()); + Post localPost = createLocalPost(postEntity); return postRepository.save(localPost); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/services/PostService.java(1 hunks)
🔇 Additional comments (2)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/services/PostService.java (2)
103-109: LGTM! Good fix for optimistic locking.
The approach of creating a new local entity instead of directly saving the fetched post is the correct way to prevent optimistic locking exceptions. The implementation cleanly copies only the required fields.
123-126: Verify if this method needs optimistic locking protection.
While the @transactional annotation is correct, this method directly saves the post without creating a new local entity, unlike the other save methods. This could potentially lead to optimistic locking issues if the input post comes from an external source.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java (1)
69-90: 🛠️ Refactor suggestionExternalize configuration values and improve security
The current implementation has hardcoded values and uses unsafe verification settings.
Consider these improvements:
- Move connection pool settings to configuration:
-poolingHttpClientConnectionManagerBuilder.setMaxConnTotal(200); -poolingHttpClientConnectionManagerBuilder.setMaxConnPerRoute(100); +poolingHttpClientConnectionManagerBuilder.setMaxConnTotal(applicationProperties.getHttp().getMaxConnTotal()); +poolingHttpClientConnectionManagerBuilder.setMaxConnPerRoute(applicationProperties.getHttp().getMaxConnPerRoute());
- Make timeout settings configurable:
-builder.setConnectionKeepAlive(TimeValue.ofSeconds(10)); -builder.setConnectionRequestTimeout(Timeout.ofSeconds(30)); -builder.setResponseTimeout(Timeout.ofSeconds(60)); +builder.setConnectionKeepAlive(TimeValue.ofSeconds(applicationProperties.getHttp().getKeepAliveSeconds())); +builder.setConnectionRequestTimeout(Timeout.ofSeconds(applicationProperties.getHttp().getRequestTimeoutSeconds())); +builder.setResponseTimeout(Timeout.ofSeconds(applicationProperties.getHttp().getResponseTimeoutSeconds()));
- Only use NoopHostnameVerifier in non-production:
- NoopHostnameVerifier.INSTANCE)); + isProdEnvironment() ? null : NoopHostnameVerifier.INSTANCE));
🧹 Nitpick comments (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java (1)
93-94: Minor optimization possible for environment checkThe current implementation creates an unnecessary intermediate List.
Consider this more efficient implementation:
- return List.of(environment.getActiveProfiles()).contains("prod"); + return Arrays.asList(environment.getActiveProfiles()).contains("prod");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java(2 hunks)
🔇 Additional comments (2)
httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java (2)
34-42: LGTM: Clean dependency injection setup
The addition of the logger and environment dependencies follows Spring best practices with proper constructor injection.
45-91: Verify the impact of HTTP client changes
The significant changes to HTTP client configuration might affect other parts of the application.
Let's verify the impact:
✅ Verification successful
Let me gather more specific information about the related configurations.
Based on the gathered information, I can now provide the final response:
HTTP client changes are isolated and safe
The changes to RestClientConfiguration are self-contained within the boot-http-proxy module. Other HTTP client implementations in the codebase:
- WebClient configurations (both MVC and WebFlux) use standard Spring Boot configuration
- RestTemplate configuration uses default SSL socket factory
- Services using RestClient are properly injecting the configured beans
The SSL/TLS changes only affect the boot-http-proxy module and include appropriate environment-based validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other HTTP client configurations that might need similar updates
rg -l "HttpClient|SSLContext|TrustStrategy" --type java
# Check for potential SSL configuration in application properties
rg -l "ssl\.|trust" --type properties --type yaml
# Look for other classes using RestClient or similar HTTP clients
ast-grep --pattern 'class $_ {
$$$
RestClient $_ = $$$
$$$
}'
Length of output: 1063
Script:
#!/bin/bash
# Check WebClientConfiguration implementations
rg -A 10 "class WebClientConfiguration" httpClients/boot-web-client-webflux/src/main/java/com/example/rest/webclient/config/WebClientConfiguration.java httpClients/boot-web-client-mvc/src/main/java/com/example/rest/webclient/config/WebClientConfiguration.java
# Check RestTemplateConfiguration
rg -A 10 "class RestTemplateConfiguration" httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/RestTemplateConfiguration.java
# Check for SSL/TLS related configurations in these files
rg "SSLContext|TrustStrategy|TlsStrategy" httpClients/boot-web-client-webflux/src/main/java/com/example/rest/webclient/config/WebClientConfiguration.java httpClients/boot-web-client-mvc/src/main/java/com/example/rest/webclient/config/WebClientConfiguration.java httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/RestTemplateConfiguration.java
# Check how PostService and HttpClientService use these configurations
rg -A 5 "RestClient|WebClient|RestTemplate" httpClients/boot-http-proxy/src/main/java/com/example/rest/proxy/services/PostService.java httpClients/boot-restclient/src/main/java/com/example/restclient/bootrestclient/services/PostService.java httpClients/boot-restclient/src/main/java/com/example/restclient/bootrestclient/services/HttpClientService.java
Length of output: 8842
...nts/boot-http-proxy/src/main/java/com/example/rest/proxy/config/RestClientConfiguration.java
Outdated
Show resolved
Hide resolved
|
rajadilipkolli
changed the title
Summary by CodeRabbit
New Features
Bug Fixes