feat : upgrade to spring boot 3.4.0-RC1 #886
Conversation
WalkthroughThe pull request introduces significant changes to the testing configuration within the Changes
Possibly related PRs
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 (
|
![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 and nitpick comments (2)
catalog-service/src/test/java/com/example/catalogservice/common/SQLContainerConfig.java (1)
20-24: Consider enhancing container configuration for robustness.While the current configuration is functional, consider adding these improvements:
PostgreSQLContainer<?> postgreSQLContainer() { return new PostgreSQLContainer<>(DockerImageName.parse("postgres").withTag("17.1-alpine")) .withDatabaseName("catalog-service") - .withReuse(true); + .withReuse(true) + .withInitScript("init.sql") // Add if you need initial schema/data + .withUrlParam("TC_REUSABLE", "true") // Explicit reuse flag + .withTmpFs(Collections.singletonMap("/var/lib/postgresql/data", "rw")); // Improved performance }Also, consider adding a comment explaining the container reuse behavior for other developers.
catalog-service/src/test/java/com/example/catalogservice/common/ContainersConfig.java (1)
18-19: Add class-level documentationConsider adding Javadoc to explain the purpose of this configuration class and its role in the test infrastructure.
Add documentation like this:
+/** + * Test configuration for managing test containers (Kafka and Zipkin). + * This configuration is used across integration tests to provide consistent + * container initialization and configuration. + */ @TestConfiguration(proxyBeanMethods = false) public class ContainersConfig {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
catalog-service/pom.xmlis excluded by!**/*.xml
📒 Files selected for processing (5)
catalog-service/src/test/java/com/example/catalogservice/TestCatalogServiceApplication.java(1 hunks)catalog-service/src/test/java/com/example/catalogservice/common/AbstractIntegrationTest.java(2 hunks)catalog-service/src/test/java/com/example/catalogservice/common/ContainersConfig.java(1 hunks)catalog-service/src/test/java/com/example/catalogservice/common/SQLContainerConfig.java(1 hunks)catalog-service/src/test/java/com/example/catalogservice/web/controllers/ProductControllerTest.java(3 hunks)
🔇 Additional comments (14)
catalog-service/src/test/java/com/example/catalogservice/TestCatalogServiceApplication.java (2)
9-10: LGTM! Clean import additions
The new imports for ContainersConfig and SQLContainerConfig align well with the architectural change to centralize container configurations.
18-19: LGTM! Improved test configuration structure
The changes reflect a better separation of concerns by:
- Moving container configurations to dedicated config classes
- Using explicit profile setting through constants
Let's verify the new configuration classes are properly set up:
✅ Verification successful
Let me gather more information about the configuration classes since they weren't found in the initial search.
Let's check the content of these config classes to verify their proper setup.
Configuration classes are properly structured and well-implemented
The verification confirms that:
- ContainersConfig properly manages Kafka and Zipkin containers with appropriate service connections
- SQLContainerConfig correctly sets up PostgreSQL container for tests
- Test profile constant is properly defined as "test"
- Both config classes use proper Spring annotations (@TestConfiguration) and container configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and structure of new config classes
# Check for ContainersConfig
echo "Checking ContainersConfig..."
ast-grep --pattern 'class ContainersConfig {
$$$
@Bean
$$$
}'
# Check for SQLContainerConfig
echo "Checking SQLContainerConfig..."
ast-grep --pattern 'class SQLContainerConfig {
$$$
@Bean
$$$
}'
# Verify the profile constant
echo "Checking profile constant..."
rg -l "PROFILE_TEST" --type java
Length of output: 1640
Script:
#!/bin/bash
# Let's search for these config classes in the catalog-service directory specifically
echo "Searching for config classes in catalog-service..."
fd -e java . catalog-service -x rg -l "class (Containers|SQL).*Config"
# Let's also check the content of these files if found
echo -e "\nChecking the content of AbstractIntegrationTest for container config..."
rg -A 10 "class AbstractIntegrationTest" catalog-service/src/test/java/com/example/catalogservice/common/AbstractIntegrationTest.java
# Check TestCatalogServiceApplication content
echo -e "\nChecking TestCatalogServiceApplication content..."
cat catalog-service/src/test/java/com/example/catalogservice/TestCatalogServiceApplication.java
Length of output: 1776
Script:
#!/bin/bash
# Check ContainersConfig content
echo "ContainersConfig content:"
cat catalog-service/src/test/java/com/example/catalogservice/common/ContainersConfig.java
echo -e "\nSQLContainerConfig content:"
cat catalog-service/src/test/java/com/example/catalogservice/common/SQLContainerConfig.java
# Verify the PROFILE_TEST constant value
echo -e "\nChecking PROFILE_TEST constant value:"
rg "PROFILE_TEST\s*=\s*" catalog-service/src/main/java/com/example/catalogservice/utils/AppConstants.java
Length of output: 2833
catalog-service/src/test/java/com/example/catalogservice/common/SQLContainerConfig.java (4)
1-8: LGTM: License header and package structure are well-defined.
9-14: LGTM: Imports are clean and appropriate.
15-16: LGTM: Optimal test configuration setup.
The use of proxyBeanMethods = false is a good practice as it improves performance by avoiding CGLIB proxies in test context.
18-24: Verify PostgreSQL version compatibility.
The PostgreSQL 17.1 is a very recent version. Ensure it's compatible with your production environment and other dependencies.
✅ Verification successful
Let me gather more information about the PostgreSQL version being used in the project.
PostgreSQL 17.1 version aligns with project configuration
The PostgreSQL version 17.1-alpine used in the test container configuration aligns with the project's infrastructure, as evidenced by multiple services using postgres:17-alpine in their Docker Compose configurations. The minor version difference (17.1 vs 17) is not a concern as they maintain compatibility within the same major version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any dependencies specify PostgreSQL version constraints
rg -l "postgresql" . | xargs rg "postgresql.*version|version.*postgresql"
# Check for Spring Boot's default PostgreSQL driver version
find . -name "pom.xml" -exec rg "postgresql" {} \;
Length of output: 1463
Script:
#!/bin/bash
# Check for PostgreSQL version property in parent pom or properties
find . -name "pom.xml" -exec rg "<postgresql\.version>.*</postgresql\.version>" {} \;
# Check Spring Boot parent version which might influence PostgreSQL driver
find . -name "pom.xml" -exec rg "<spring-boot.*version>.*</.*version>" {} \;
# Check for any docker-compose files that might specify PostgreSQL version
fd "docker-compose" --exec rg "postgres:" {} \;
Length of output: 397
catalog-service/src/test/java/com/example/catalogservice/common/AbstractIntegrationTest.java (2)
3-3: LGTM: License year updated
Copyright year has been correctly updated to include 2024.
26-26: LGTM: Improved test configuration architecture
The shift from TestCatalogServiceApplication to dedicated configuration classes (SQLContainerConfig, ContainersConfig) represents a better separation of concerns and more maintainable test setup.
Let's verify the test execution with the new configuration:
✅ Verification successful
Configuration classes are well-structured and properly integrated
The verification confirms that the test configuration architecture is indeed improved:
SQLContainerConfigproperly manages PostgreSQL test container with reuse capabilityContainersConfighandles Kafka container setup with appropriate scopingTestKafkaListenerConfigimplements the necessary test listener configuration with proper event counting- All configurations are correctly integrated in
AbstractIntegrationTestand used throughout the test suite
The separation of concerns is clear and each configuration class has a single responsibility, making the test setup more maintainable and modular.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test configuration and execution
# Check if all necessary container configurations are present
echo "Checking container configurations..."
rg -l "class \w+Config.*extends TestContainersConfiguration" catalog-service/src/test/java/
# Verify test execution time impact
echo "Checking test execution patterns..."
rg "@Test" catalog-service/src/test/java/ --stats
# Look for any TestContainers lifecycle hooks
echo "Checking container lifecycle management..."
rg "implements BeforeAllCallback|extends TestContainersConfiguration" catalog-service/src/test/java/
Length of output: 3995
Script:
#!/bin/bash
# Let's examine the configuration classes more closely
# Check the content of SQLContainerConfig
echo "Examining SQLContainerConfig..."
rg -A 10 "class SQLContainerConfig" catalog-service/src/test/java/
# Check the content of ContainersConfig
echo "Examining ContainersConfig..."
rg -A 10 "class ContainersConfig" catalog-service/src/test/java/
# Check the content of TestKafkaListenerConfig
echo "Examining TestKafkaListenerConfig..."
rg -A 10 "class TestKafkaListenerConfig" catalog-service/src/test/java/
# Check how these configs are used in tests
echo "Checking test usage..."
rg "SQLContainerConfig|ContainersConfig|TestKafkaListenerConfig" catalog-service/src/test/java/ -A 2
Length of output: 8603
catalog-service/src/test/java/com/example/catalogservice/common/ContainersConfig.java (3)
37-43: LGTM! Clean implementation of dynamic property registration
The DynamicPropertyRegistrar is well implemented, using method reference for getting bootstrap servers and correctly configuring the Spring Cloud Stream Kafka binder.
7-17: Verify complete migration to centralized container configuration
This centralized approach is good for maintainability. However, we should ensure all test classes have been updated to use this configuration.
Run the following script to check for any remaining container definitions that should be migrated:
#!/bin/bash
# Description: Search for test classes that might still have container definitions
# Look for container definitions in test classes
ast-grep --pattern 'class $_ {
$$$
@Bean
$$$Container $_() {
$$$
}
$$$
}'
# Look for direct container instantiations
rg "new (KafkaContainer|GenericContainer|PostgreSQLContainer)"24-27: Verify compatibility with apache/kafka-native image
While using the native Kafka image can improve container startup time, it's relatively new and might have different behavior compared to the traditional Kafka image. Please ensure all Kafka-dependent tests work as expected.
Run the following script to check for any potential issues or discussions about the native Kafka image:
✅ Verification successful
Using apache/kafka-native image is appropriate and consistent across services
The verification shows that:
- The apache/kafka-native:3.8.1 image is consistently used across multiple services (catalog-service, order-service, payment-service)
- All services using this image have similar container configurations with reuse enabled
- Basic Kafka serialization configurations are properly set in test properties
- Only inventory-service uses a different image (confluentinc/cp-kafka:7.6.0)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for Kafka-related test files and configurations
# to ensure compatibility with native Kafka image
# Check for Kafka consumer/producer configurations
rg -l "KafkaTemplate|@KafkaListener"
# Check for Kafka-related test files
fd -e java -e properties -e yml | rg -i "kafka"
Length of output: 2483
Script:
#!/bin/bash
# Let's check for Kafka-related configurations and tests across services
# Search for Kafka configuration properties
rg -l "spring\.kafka\." --type yaml --type properties
# Search for Kafka test classes and configurations
rg -l "KafkaTemplate|@KafkaListener|@TestConfiguration.*Kafka|KafkaConsumer|KafkaProducer"
# Search for specific Kafka test methods
ast-grep --pattern 'class $_ {
$$$
@Test
$_ $_($$$) {
$$$kafka$$$
}
$$$
}'
Length of output: 1820
Script:
#!/bin/bash
# Let's examine the specific Kafka configurations and test implementations
# Check Kafka configuration in catalog-service
rg -A 10 "KafkaContainer" catalog-service/src/test/java/com/example/catalogservice/config/TestKafkaListenerConfig.java
rg -A 10 "KafkaContainer" catalog-service/src/test/java/com/example/catalogservice/services/ProductServiceTest.java
# Check if other services using native Kafka have specific configurations
rg -A 10 "apache/kafka-native" order-service/src/test/java/com/example/orderservice/common/ContainersConfig.java
rg -A 10 "apache/kafka-native" payment-service/src/test/java/com/example/paymentservice/common/NonSQLContainerConfig.java
# Look for any Kafka-specific test properties or configurations
rg "kafka\." catalog-service/src/test/resources/
Length of output: 1395
catalog-service/src/test/java/com/example/catalogservice/web/controllers/ProductControllerTest.java (3)
3-3: LGTM: License year correctly updated
The copyright year has been appropriately updated to include 2024.
Line range hint 46-270: LGTM: Test implementation remains robust
The test suite is comprehensive and well-structured, covering all essential scenarios:
- Fetching all products with pagination
- Finding products by ID
- Creating new products
- Updating existing products
- Deleting products
- Error handling for non-existing products
The reactive WebFlux testing patterns are correctly implemented using WebTestClient.
32-32: Verify the necessity of @MockitoBean migration
The change from @MockBean to @MockitoBean might be related to the Spring Boot 3.4.0-RC1 upgrade. Let's verify if this change is necessary and consistent across the codebase.
Also applies to: 42-42
| return new GenericContainer<>(DockerImageName.parse("openzipkin/zipkin:latest")) | ||
| .withExposedPorts(9411) | ||
| .withReuse(true); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin the Zipkin image version for reproducible builds
Using the latest tag can lead to inconsistent behavior across different environments as the image version may change unexpectedly.
Apply this diff to pin the Zipkin image version:
- .withExposedPorts(9411)
- .withReuse(true);
+ .withExposedPorts(9411)
+ .withReuse(true)
+ .withImagePullPolicy(PullPolicy.alwaysLocal());Also, update the image tag to a specific version:
-DockerImageName.parse("openzipkin/zipkin:latest")
+DockerImageName.parse("openzipkin/zipkin:2.24") // Use appropriate versionCommittable suggestion skipped: line range outside the PR's diff.
No description provided.