-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
upgrade to springboot 3.4 #1535
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request includes updates to several configuration files within the Changes
Possibly related PRs
Suggested labels
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
batch-boot-jpa-sample/docker/docker-compose.yml (1)
5-5
: Consider adding health check for PostgreSQL containerTo ensure proper startup order and application stability, consider adding a healthcheck configuration:
image: postgres:17.2-alpine hostname: postgresqldb extra_hosts: [ 'host.docker.internal:host-gateway' ] + healthcheck: + test: ["CMD-SHELL", "pg_isready -U appuser -d appdb"] + interval: 10s + timeout: 5s + retries: 5batch-boot-jpa-sample/src/main/java/com/example/bootbatchjpa/config/BatchConfig.java (3)
Line range hint
52-57
: Critical: Remove duplicate start() call in job configurationThere's a duplicate
start()
call in the job configuration which could lead to unexpected behavior:return new JobBuilder("all-customers-job", jobRepository) .start(step) .incrementer(new RunIdIncrementer()) .listener(this) - .start(step) .build();
Line range hint
44-57
: Consider using step builder pattern for better readabilityThe job configuration can be made more maintainable by extracting the step configuration to a separate method.
+ private Step buildCustomerStep( + JpaPagingItemReader<Customer> jpaPagingItemReader, + JobRepository jobRepository, + PlatformTransactionManager transactionManager) { + return new StepBuilder("all-customers-step", jobRepository) + .allowStartIfComplete(true) + .<Customer, CustomerDTO>chunk(10, transactionManager) + .reader(jpaPagingItemReader) + .processor(getCustomerCustomerDTOItemProcessor()) + .writer(getCustomerDTOItemWriter()) + .faultTolerant() + .skip(Exception.class) + .noSkip(ConstraintViolationException.class) + .skipLimit(20) + .build(); + } + @Bean Job allCustomersJob( JpaPagingItemReader<Customer> jpaPagingItemReader, JobRepository jobRepository, PlatformTransactionManager transactionManager) { - Step step = - new StepBuilder("all-customers-step", jobRepository) - .allowStartIfComplete(true) - .<Customer, CustomerDTO>chunk(10, transactionManager) - .reader(jpaPagingItemReader) - .processor(getCustomerCustomerDTOItemProcessor()) - .writer(getCustomerDTOItemWriter()) - .faultTolerant() - .skip(Exception.class) - .noSkip(ConstraintViolationException.class) - .skipLimit(20) - .build(); + Step step = buildCustomerStep(jpaPagingItemReader, jobRepository, transactionManager); return new JobBuilder("all-customers-job", jobRepository) .start(step) .incrementer(new RunIdIncrementer()) .listener(this) .build(); }
Spring Batch configuration is compatible with Spring Boot 3.4
The batch configuration is correctly using the modern Spring Batch patterns compatible with Spring Boot 3.4:
- Uses the new builder pattern with
JobBuilder
andStepBuilder
instead of deprecated factories- Correctly configures chunks with transaction manager
- Uses the standard
@EnableBatchProcessing
annotation- No deprecated APIs are found in the codebase
However, there is one issue in the job configuration:
- The job definition has duplicate
start(step)
calls which could cause unexpected behavior:return new JobBuilder("all-customers-job", jobRepository) .start(step) .incrementer(new RunIdIncrementer()) .listener(this) .start(step) // <-- Duplicate start .build();🔗 Analysis chain
Line range hint
1-103
: Verify Spring Batch configuration with Spring Boot 3.4Since this PR involves upgrading to Spring Boot 3.4, let's verify that all batch configurations are compatible with the new version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any Spring Boot 3.4 specific batch configuration changes in the codebase echo "Checking for Spring Boot batch configuration patterns..." # Look for EnableBatchProcessing annotation usage echo "Checking @EnableBatchProcessing usage:" rg --type java "EnableBatchProcessing" # Look for batch configuration classes echo "Checking for batch configuration classes:" rg --type java "class.*Config.*implements.*JobExecutionListener" # Look for potential deprecated batch APIs echo "Checking for potential deprecated batch APIs:" rg --type java "JobBuilderFactory|StepBuilderFactory"Length of output: 981
Script:
#!/bin/bash # Let's check the Spring Boot version and its dependencies echo "Checking Spring Boot version:" fd pom.xml --exec grep -A 5 "<parent>" {} \; # Check for Spring Batch dependencies echo -e "\nChecking Spring Batch dependencies:" fd pom.xml --exec grep -A 3 "spring-batch" {} \; # Look for JobBuilder and StepBuilder usage (new style in Spring Boot 3.x) echo -e "\nChecking for new style builders usage:" rg --type java "new (Job|Step)Builder\(" # Check for chunk configuration style echo -e "\nChecking chunk configuration style:" rg --type java "chunk\([0-9]+,"Length of output: 10071
batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/web/controllers/CustomerControllerIT.java (1)
42-42
: Consider performance impact for larger test datasets.While
saveAllAndFlush
is excellent for test determinism, be mindful that forcing an immediate flush can impact performance when testing with larger datasets. Consider using@Transactional
on the test class if you need to scale up the test data significantly in the future.batch-boot-jpa-sample/pom.xml (1)
Line range hint
1-400
: Consider adding Spring Boot BOM version propertiesFor better dependency management, consider extracting the versions of Spring Boot managed dependencies (like
spring-batch-test
,testcontainers
) into properties. This makes version management more explicit and easier to update.Add these properties to the
<properties>
section:<properties> + <testcontainers.version>${spring-boot.version}</testcontainers.version> + <spring-batch.version>${spring-boot.version}</spring-batch.version> ... </properties>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
batch-boot-jpa-sample/docker/docker-compose.yml
(1 hunks)batch-boot-jpa-sample/pom.xml
(3 hunks)batch-boot-jpa-sample/src/main/java/com/example/bootbatchjpa/config/BatchConfig.java
(2 hunks)batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/common/TestContainersConfig.java
(1 hunks)batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/repository/SchemaValidationTest.java
(1 hunks)batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/web/controllers/CustomerControllerIT.java
(1 hunks)batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/web/controllers/CustomerControllerTest.java
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/common/TestContainersConfig.java
🔇 Additional comments (11)
batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/repository/SchemaValidationTest.java (2)
14-16
: LGTM! Clean test configuration setup.
The simplified test configuration is better aligned with Spring Boot 3.4's best practices:
- Using @AutoConfigureTestDatabase for test database configuration
- Keeping only essential properties in @DataJpaTest
- Proper integration with TestContainers
15-15
: Verify PostgreSQL version in TestContainersConfig
Since this test depends on TestContainersConfig and there's a PostgreSQL version upgrade mentioned in the summary, let's verify the version consistency.
✅ Verification successful
PostgreSQL version is consistent in TestContainersConfig
The PostgreSQL version in TestContainersConfig (17.2-alpine) matches the version in docker-compose.yml for the batch-boot-jpa-sample project. Both files use PostgreSQL 17.2-alpine:
- TestContainersConfig.java:
new PostgreSQLContainer<>(DockerImageName.parse("postgres").withTag("17.2-alpine"));
- docker/docker-compose.yml:
image: postgres:17.2-alpine
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PostgreSQL version consistency across test configurations
# Expected: Should find PostgreSQL version 17.2-alpine
# Check TestContainersConfig for PostgreSQL version
echo "Checking TestContainersConfig for PostgreSQL version:"
rg -A 2 "PostgreSQLContainer|postgres:" --type java --type yaml
Length of output: 33882
batch-boot-jpa-sample/docker/docker-compose.yml (2)
5-5
: Verify volume persistence configuration
Currently, there's no volume configuration for PostgreSQL data persistence. This means data will be lost when the container is removed.
Consider adding a named volume:
image: postgres:17.2-alpine
+ volumes:
+ - postgres_data:/var/lib/postgresql/data
hostname: postgresqldb
# At the bottom of the file
volumes:
postgres_data:
5-5
: Major PostgreSQL version upgrade (16 -> 17) requires careful consideration
This is a major version upgrade that requires careful planning:
- PostgreSQL 17 introduces breaking changes and new features that need validation
- Ensure Spring Boot 3.4 is fully compatible with PostgreSQL 17
- Consider implementing a data backup and migration strategy before upgrading
Let's verify the compatibility:
✅ Verification successful
Let me analyze the Spring Boot and PostgreSQL configuration to verify compatibility and potential issues.
PostgreSQL 17.2 upgrade is compatible with Spring Boot 3.4
The codebase analysis shows:
- The application uses standard JDBC configuration without version-specific PostgreSQL features
- Database schema changes are managed through Liquibase (db.changelog-master.yaml)
- No PostgreSQL dialect-specific configurations that could be affected by the version upgrade
The upgrade from PostgreSQL 16.6 to 17.2 is safe because:
- Spring Boot 3.4 uses the latest PostgreSQL JDBC driver which supports PostgreSQL 17
- The application uses version-agnostic configuration
- Schema management is handled through Liquibase, providing rollback capability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Spring Boot and PostgreSQL compatibility documentation
# Check if there are any Spring Boot configuration files that might need updates
rg -g '*.properties' -g '*.yml' 'spring.jpa|spring.datasource'
# Look for any PostgreSQL-specific configuration or SQL scripts that might need review
fd -e sql -e properties -e yml -e yaml | grep -i 'postgres'
Length of output: 64219
Script:
#!/bin/bash
# Check for PostgreSQL-specific configuration and migration scripts in the batch-boot-jpa-sample
fd -t f . batch-boot-jpa-sample/src/main/resources/db/
# Check for any PostgreSQL dialect or version-specific configuration
rg -g '*.properties' -g '*.yml' 'postgresql|postgres.*.dialect' batch-boot-jpa-sample/
Length of output: 1273
batch-boot-jpa-sample/src/main/java/com/example/bootbatchjpa/config/BatchConfig.java (1)
26-26
: LGTM: NonNull annotation addition enhances parameter validation
The addition of @NonNull
annotation is a good practice for explicitly documenting and enforcing that the jobExecution
parameter cannot be null. This aligns well with Spring's push towards better null-safety in Spring Boot 3.x.
Also applies to: 98-98
batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/web/controllers/CustomerControllerIT.java (1)
42-42
: LGTM! Good improvement for test determinism.
The change from saveAll
to saveAllAndFlush
ensures that the test data is immediately persisted to the database, making the test behavior more deterministic and reliable.
batch-boot-jpa-sample/src/test/java/com/example/bootbatchjpa/web/controllers/CustomerControllerTest.java (2)
33-33
: LGTM! Import aligned with Spring Boot 3.4
The import change from @MockBean
to @MockitoBean
aligns with the Spring Boot 3.4 upgrade.
42-42
: LGTM! Annotation upgrade improves specificity
The change from @MockBean
to @MockitoBean
is appropriate as it's more specific to Mockito and is the preferred choice in newer Spring versions. The test functionality remains unchanged.
Let's verify there are no remaining @MockBean annotations that should be upgraded:
batch-boot-jpa-sample/pom.xml (3)
245-245
: LGTM: Google Java Format version update
The update to Google Java Format 1.24.0 is a safe change as it's only used during the build process and won't affect runtime behavior.
25-25
: Verify springdoc-openapi compatibility
The springdoc-openapi upgrade to 2.7.0 needs verification for compatibility with Spring Boot 3.4.0.
Let's check the compatibility:
✅ Verification successful
Let me check the Spring Boot version in the project to ensure compatibility.
Springdoc-openapi 2.7.0 is compatible with Spring Boot 3.4.0
Based on the verification results:
- The project uses Spring Boot 3.4.0
- Springdoc-openapi v2.x series is specifically designed for Spring Boot 3.x as confirmed in their documentation
- The upgrade from 2.6.0 (seen in another module) to 2.7.0 is a minor version bump within the same major version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check springdoc-openapi's documented compatibility with Spring Boot versions
curl -s "https://raw.githubusercontent.com/springdoc/springdoc-openapi/v2.7.0/README.md" | grep -i "spring boot"
# Look for any springdoc-openapi usage in the codebase that might need updates
rg "springdoc|openapi" --type java
Length of output: 3176
Script:
#!/bin/bash
# Check Spring Boot version in the project
cat batch-boot-jpa-sample/pom.xml | grep -A 2 "<parent>"
# Check if there are any springdoc-openapi specific configurations that might need updates
rg "springdoc" --type properties --type yaml
Length of output: 266
Script:
#!/bin/bash
# Check Spring Boot version in parent pom
cat batch-boot-jpa-sample/pom.xml | grep -A 1 "<version>" | head -2
# Check for springdoc configurations with correct file extensions
rg "springdoc" -g "*.properties" -g "*.yml" -g "*.yaml"
Length of output: 494
10-10
: Verify Spring Boot 3.4.0 milestone compatibility
The upgrade to Spring Boot 3.4.0 is using a milestone release (as indicated by the milestone repository configuration). While milestone releases are generally stable, they may contain breaking changes or bugs.
Let's verify the compatibility:
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
batch-boot-jpa-sample/src/main/java/com/example/bootbatchjpa/config/Initializer.java (1)
Line range hint
24-33
: Enhance initialization robustness and inclusivityConsider the following improvements:
- Add a configuration flag to control initialization in different environments
- Expand gender options beyond binary choices
- Add error handling for database operations
Here's a suggested implementation:
+ @Value("${app.initialization.enabled:false}") + private boolean initializationEnabled; + @Override public void run(String... args) { + if (!initializationEnabled) { + log.info("Initialization disabled"); + return; + } log.info("Running Initializer....."); - List<Customer> customerList = - Instancio.ofList(Customer.class) - .size(1000) - .generate( - field(Customer.class, "gender"), gen -> gen.oneOf("male", "female")) - .create(); + try { + List<Customer> customerList = + Instancio.ofList(Customer.class) + .size(1000) + .generate( + field(Customer.class, "gender"), + gen -> gen.oneOf("male", "female", "non-binary", "prefer_not_to_say")) + .create(); + log.info("Saving Customers of size :{}", customerList.size()); + // ... save logic ... + } catch (Exception e) { + log.error("Failed to initialize customers", e); + throw new RuntimeException("Initialization failed", e); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
batch-boot-jpa-sample/src/main/java/com/example/bootbatchjpa/config/Initializer.java
(1 hunks)
🔇 Additional comments (1)
batch-boot-jpa-sample/src/main/java/com/example/bootbatchjpa/config/Initializer.java (1)
34-34
: Consider performance implications of using saveAllAndFlush
While saveAllAndFlush
ensures immediate persistence, it may impact performance when initializing large datasets (currently 1000 records) as it forces a database flush. Consider these alternatives:
- Batch the saves into smaller chunks
- Use
saveAll
if immediate persistence isn't required - Use native batch insert capabilities
Here's a suggested implementation using batching:
- customerList = customerRepository.saveAllAndFlush(customerList);
+ int batchSize = 100;
+ for (int i = 0; i < customerList.size(); i += batchSize) {
+ int end = Math.min(i + batchSize, customerList.size());
+ List<Customer> batch = customerList.subList(i, end);
+ customerRepository.saveAll(batch);
+ if (i % 500 == 0) { // Flush periodically
+ customerRepository.flush();
+ }
+ }
+ customerRepository.flush(); // Final flush
Let's verify if this initialization is causing any performance issues:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
These changes aim to enhance the overall performance, reliability, and maintainability of the application.