Upgrade to spring boot 3.4.1 #1590
Conversation
WalkthroughThis pull request updates 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: 1
🧹 Nitpick comments (5)
httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/RestTemplateConfiguration.java (4)
54-59: Use @nonnull annotation for environment injection
It's helpful to validate that the environment is not null, ensuring safety in production. You can annotate the constructor parameter or field with @nonnull to enforce this contract.
62-73: Ensure permissive SSL usage is tightly controlled
The use of a permissive TrustStrategy is fine for non-production, but be mindful that it should never leak into production. The conditional checks are a good safeguard, but consider adding further assertion or fallback logic to confirm that the production environment never uses bypassed certificate validation.
102-104: Consider using Spring’s built-in profile checks
Using List.of(environment.getActiveProfiles()).contains("prod") is valid. However, you might simplify this check by using environment.acceptsProfiles(Profiles.of("prod")), which can be more idiomatic for Spring-based configurations.
147-147: Unify connection timeout with existing constants
Currently, you use a hard-coded 60s timeout. To maintain consistency and possible future customizations, consider leveraging the existing constants (e.g., REQUEST_TIMEOUT) or retrieving the timeout from properties:Example diff:
- .connectTimeout(Duration.ofSeconds(60)) + .connectTimeout(Duration.ofSeconds(REQUEST_TIMEOUT))httpClients/boot-rest-template/pom.xml (1)
Line range hint
293-307: Consider timeline for moving to GA releasesThe project is configured to use Spring Milestones repository for both dependencies and plugins. While this is necessary for Spring Boot 3.4.1, consider:
- Documenting the timeline for moving to GA releases
- Adding a comment in the repository configuration explaining the temporary nature of using milestone releases
Also applies to: 308-321
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
httpClients/boot-rest-template/pom.xml(2 hunks)httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/RestTemplateConfiguration.java(4 hunks)httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/WebMvcConfig.java(2 hunks)
🔇 Additional comments (4)
httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/WebMvcConfig.java (1)
5-5: LGTM! Good addition of Spring's null safety annotations.
The addition of Spring's @nonnull import aligns well with Spring Boot's emphasis on null safety.
httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/RestTemplateConfiguration.java (1)
75-94: Pooling manager configuration looks solid
Your builder-based approach for pooling connections and establishing timeouts is cleanly structured. This is an effective way to manage HTTP connections while keeping the code organized and readable.
httpClients/boot-rest-template/pom.xml (2)
22-22: Verify springdoc-openapi compatibility
The springdoc-openapi upgrade to 2.7.0 should be verified for compatibility with Spring Boot 3.4.1.
✅ Verification successful
The previous output shows that springdoc-openapi 2.7.0 was specifically tested with Spring Boot 3.4.0, but we need to verify our project's Spring Boot version and any specific springdoc configurations that might need updates. Let's gather this additional context to make a proper assessment.
Let's check the specific module's pom.xml to verify the Spring Boot version and springdoc configuration, as the previous search was too broad.
springdoc-openapi 2.7.0 is compatible with Spring Boot 3.4.1
The release notes show that springdoc-openapi 2.7.0 was tested with Spring Boot 3.4.0, and our project uses Spring Boot 3.4.1. This is a compatible combination as they are in the same minor version series (3.4.x). The upgrade includes important fixes and improvements that are relevant for Spring Boot 3.4.x.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check springdoc-openapi 2.7.0 release notes for compatibility
gh api \
-H "Accept: application/vnd.github+json" \
repos/springdoc/springdoc-openapi/releases | \
jq '.[] | select(.tag_name | contains("v2.7.0")) | .body'
# Look for any springdoc-specific configurations that might need updates
rg -A 5 "springdoc" --type yaml --type properties
Length of output: 12242
Script:
#!/bin/bash
# Check Spring Boot version in pom.xml
rg -A 2 "<parent>" pom.xml
# Check for springdoc configuration files
fd -e yml -e yaml -e properties | xargs rg "springdoc"
# Check for any springdoc dependencies in pom.xml
rg -A 3 "springdoc" pom.xml
Length of output: 311
Script:
#!/bin/bash
# Check Spring Boot version in the specific pom.xml file
cat httpClients/boot-rest-template/pom.xml | grep -A 5 "<parent>"
# Check for any springdoc dependencies and configurations in this specific module
cat httpClients/boot-rest-template/pom.xml | grep -A 10 "<properties>"
Length of output: 983
8-8: Verify Spring Boot 3.4.1 milestone release compatibility
The upgrade to Spring Boot 3.4.1 requires careful consideration:
- This is a milestone release (not GA) as indicated by the milestone repository configuration
- Ensure all dependencies are compatible with this version
| @@ -11,7 +12,7 @@ public class WebMvcConfig implements WebMvcConfigurer { | |||
| private final ApplicationProperties properties; | |||
|
|
|||
| @Override | |||
| public void addCorsMappings(CorsRegistry registry) { | |||
| public void addCorsMappings(@NonNull CorsRegistry registry) { | |||
There was a problem hiding this comment.
💡 Codebase verification
Null safety annotations are inconsistently applied across configuration classes
Based on the search results, while some configuration classes use null safety annotations, the usage is not consistent across the codebase:
- Several
WebMvcConfigclasses have@NonNullonly onaddCorsMappingsmethod but not on other methods - Some packages have
@NonNullApiat package level (e.g., ingraphql,multitenancymodules) - Bean post processors consistently use
@NonNullon their parameters - Only a few classes properly annotate nullable parameters with
@Nullable
🔗 Analysis chain
Consider extending null safety annotations across the configuration.
The addition of @nonnull to the registry parameter is a good practice. Consider applying similar null safety annotations (@nonnull, @nullable) consistently across other methods and fields in your configuration classes for better null safety coverage.
Let's check if similar annotations are used consistently in other configuration classes:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of null safety annotations in config classes
# Look for configuration classes that might benefit from similar null safety annotations
# Find all configuration classes
echo "Searching for Spring configuration classes..."
rg -l "class.*implements.*Configurer" --type java
# Check current usage of Spring's null safety annotations
echo "Checking current usage of null safety annotations..."
rg "@(NonNull|Nullable)" --type java
Length of output: 7418
Qodana Community for JVM103 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 (4)
httpClients/boot-rest-template/src/main/java/com/example/rest/template/services/PostService.java (2)
41-41: Clarify the intention of always nullifying the ID when saving a Post.
Settingpost.setId(null);will cause each call tosavePostto create a new entity instead of updating an existing one, even if thepostobject has anid. If your intent is to enforce that the method only creates new records, this is fine. Otherwise, you may wish to remove this assignment (or rename the method) to avoid confusion.
50-53: Consider adding an existence check before saving during update.
This newupdatePostmethod doesn’t validate that thePostbeing updated actually exists. If you want to ensure that an exception is thrown when no such entity is found, you could do something like checkingpostRepository.existsById(post.getId())before callingsave(post). Otherwise, a call could unintentionally create a newPostrecord.@Transactional public Post updatePost(Post post) { + if (!postRepository.existsById(post.getId())) { + throw new IllegalArgumentException("Post not found for update: " + post.getId()); + } return postRepository.save(post); }httpClients/boot-rest-template/src/main/resources/db/changelog/migration/02-create_posts_table.xml (1)
Line range hint
7-12: Document the rationale for sequence start value.Consider adding a comment in the changelog to explain why 1001 was chosen as the start value. This helps future maintainers understand the reasoning behind this specific number.
<changeSet author="app" id="createTable-posts"> + <!-- Starting from 1001 to create a clear separation between existing and new records --> <createSequence sequenceName="posts_seq" incrementBy="50" startValue="1001" />httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/Initializer.java (1)
47-48: Avoid usingpeekfor side effects.
Usingpeekto mutate elements can be confusing, aspeekis conventionally used for debugging. Consider replacing with:- List<Post> modifiedPostList = response.stream().peek(post -> post.setId(null)).toList(); + List<Post> modifiedPostList = response.stream() + .map(post -> { + post.setId(null); + return post; + }) + .toList();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/Initializer.java(1 hunks)httpClients/boot-rest-template/src/main/java/com/example/rest/template/services/PostService.java(1 hunks)httpClients/boot-rest-template/src/main/java/com/example/rest/template/web/controllers/PostController.java(1 hunks)httpClients/boot-rest-template/src/main/resources/db/changelog/migration/02-create_posts_table.xml(1 hunks)httpClients/boot-rest-template/src/test/java/com/example/rest/template/services/PostServiceTest.java(2 hunks)httpClients/boot-rest-template/src/test/java/com/example/rest/template/web/controllers/PostControllerTest.java(1 hunks)
🔇 Additional comments (7)
httpClients/boot-rest-template/src/main/java/com/example/rest/template/web/controllers/PostController.java (1)
65-65: Ensure concurrency handling and partial updates are consideredWhile this change from
savePosttoupdatePostis appropriate for updating an existing entity, consider if concurrency handling (e.g., optimistic locking) or partial update logic is needed to prevent overwriting updated fields. If partial updates are required, a PATCH endpoint or dedicated approach may be more suitable.httpClients/boot-rest-template/src/test/java/com/example/rest/template/web/controllers/PostControllerTest.java (1)
140-140: Good job aligning the test with the newupdatePostmethod.This updated mocking setup accurately reflects the switched logic from
savePosttoupdatePost. The test verifies that the correct service method is invoked. As an extra step, you might also validate partial update scenarios or concurrency controls if your application allows multiple concurrent updates to the same post.httpClients/boot-rest-template/src/main/java/com/example/rest/template/services/PostService.java (1)
49-49: No concerns here.
No code changes on this blank line, so there's nothing to address.httpClients/boot-rest-template/src/main/resources/db/changelog/migration/02-create_posts_table.xml (1)
11-11: Verify impact on existing data.The sequence start value change to 1001 looks good for creating a clear separation between existing and new records. However, let's verify there are no existing records with IDs that could conflict with this new sequence range.
✅ Verification successful
Let's first identify any data files or post-related files that might contain information about existing records.
Let's examine the table creation file and any previous migrations to understand the context better.
Sequence start value change is safe to proceed
The change to start the
posts_seqsequence at 1001 is safe because:
- This is a new table creation (
02-create_posts_table.xml), not a modification of an existing table- No pre-existing data or migrations were found that insert records into the
poststable- The sequence is being created for the first time along with the table
The sequence configuration with
startValue="1001"andincrementBy="50"is a good practice for:
- Providing a clear separation between system-generated IDs and any potential manually inserted data
- Optimizing performance through batch allocation of sequence values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for existing post IDs that might conflict with the new sequence # Look for SQL files or other changelogs that might contain post data rg -g '*.sql' -g '*.xml' -A 5 'INSERT INTO posts|id.*1001'Length of output: 60
Script:
#!/bin/bash # Let's check if there are any existing data files or test data fd -e sql -e xml | grep -i "data\|test\|sample" # Also check for any INSERT statements in SQL files rg "INSERT INTO" -g "*.sql" # Look for any post-related files that might contain ID information fd -e sql -e xml -e java | grep -i "post"Length of output: 23019
Script:
#!/bin/bash # Let's check the content of the posts table creation file to understand the initial setup cat "httpClients/boot-rest-template/src/main/resources/db/changelog/migration/02-create_posts_table.xml" # Also check if there are any existing data files or migrations that came before this one fd -e xml -e sql | grep "01-" | xargs catLength of output: 16729
httpClients/boot-rest-template/src/test/java/com/example/rest/template/services/PostServiceTest.java (1)
69-69: Re-evaluate disabling this test
It's unclear why this test is disabled. If it's no longer valid, consider removing it to maintain clarity and avoid confusion. Otherwise, if there's a temporary reason to skip it—such as pending changes—document that reason or re-enable the test when ready to ensure full coverage.As a follow-up, remember to confirm whether
savePostis being tested or exercised elsewhere.- @Disabled void savePost() {httpClients/boot-rest-template/src/main/java/com/example/rest/template/config/Initializer.java (2)
37-39: Consider verifying the response before using its body.
IfpostApplicationRestResponse.body()happens to be null or malformed, this code may result in a NullPointerException. A quick null check (or an optional check) would help avoid runtime failures.
38-38: Clarify intention of setting the ID to null.
By nullifying the ID, the local database will treat it as a new entity. Verify whether this is the desired behavior, as it may cause duplicate entries if a record already exists or if the remote data maps to an existing entity in the DB.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor