feat : Added R2DBC proxy support for enhanced database query logging #1630
Conversation
WalkthroughThis pull request introduces enhancements to an R2DBC-based application, focusing on query logging, auditing configuration, and repository modifications. The changes include adding the R2DBC proxy dependency, creating a new query execution listener for logging, restructuring the auditing configuration, and updating repository methods to simplify query retrieval. The modifications aim to improve observability and streamline the data access layer of the application. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Repository
participant QueryListener
participant Database
Client->>Repository: retrievePostsWithCommentsAndTags()
Repository->>QueryListener: Log query details
Repository->>Database: Execute query
Database-->>Repository: Return results
QueryListener->>QueryListener: Format and log query info
Repository-->>Client: Return post data
Possibly related PRs
Suggested labels
Poem
Finishing Touches
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: 3
🧹 Nitpick comments (5)
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/Initializer.java (1)
Line range hint
93-97: LGTM! Consider enhancing error logging.The removal of the
nullparameter correctly aligns with the updated method signature. However, the error logging could be more descriptive.Consider this improvement for better error observability:
- error -> log.debug("error: ", error), + error -> log.error("Failed to retrieve posts with comments and tags", error),r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java (1)
82-104: Add method documentation for better maintainability.The method implementation is well-structured and type-safe. Consider adding JavaDoc to document:
- The purpose of the method
- The fields included in the Record7
- The relationship between posts, comments, and tags in the query
Example documentation:
/** * Creates a base select query for posts with their associated comments and tags. * The query includes: * - Basic post fields (id, title, content, created_by, status) * - Nested comments as a multiset * - Associated tags as a multiset * * @return SelectJoinStep configured with the base query */r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/CustomPostRepository.java (1)
12-12: Consider pagination for retrievePostsWithCommentsAndTagsThe removal of the Condition parameter simplifies the API, but retrieving all posts with their comments and tags in a single call might lead to performance issues with large datasets.
Consider:
- Adding pagination support similar to the
findByKeywordmethod- Implementing lazy loading for comments and tags
- Adding filtering capabilities at the service layer if needed
Example signature:
-Flux<PostResponse> retrievePostsWithCommentsAndTags(); +Mono<Page<PostResponse>> retrievePostsWithCommentsAndTags(Pageable pageable);r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/QueryProxyExecutionListener.java (1)
17-19: Consider adding log level check for performance.Add a log level check before string formatting to avoid unnecessary processing when debug logging is disabled.
- QueryExecutionInfoFormatter formatter = QueryExecutionInfoFormatter.showAll(); - String str = formatter.format(queryExecutionInfo); - logger.info("Query: {}", str); + if (logger.isDebugEnabled()) { + QueryExecutionInfoFormatter formatter = QueryExecutionInfoFormatter.showAll(); + String str = formatter.format(queryExecutionInfo); + logger.debug("Query: {}", str); + }r2dbc/boot-jooq-r2dbc-sample/src/test/java/com/example/jooq/r2dbc/repository/PostRepositoryTest.java (1)
135-199: LGTM! Comprehensive test coverage for database constraints and concurrency.The new test methods effectively cover:
- Unique constraint violations
- Foreign key constraints
- Optimistic locking for concurrent modifications
Good use of StepVerifier and clear error validations.
Consider adding timeout durations to the StepVerifier verifications for better test reliability:
StepVerifier.create(secondTag) .expectErrorMatches( throwable -> throwable instanceof DataIntegrityViolationException && throwable.getMessage().contains("unique constraint")) - .verify(); + .verify(Duration.ofSeconds(5));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
r2dbc/boot-jooq-r2dbc-sample/pom.xml(3 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/Initializer.java(1 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/JooqConfiguration.java(0 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/QueryProxyExecutionListener.java(1 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/R2dbcConfiguration.java(1 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/CustomPostRepository.java(1 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java(3 hunks)r2dbc/boot-jooq-r2dbc-sample/src/test/java/com/example/jooq/r2dbc/repository/PostRepositoryTest.java(7 hunks)
💤 Files with no reviewable changes (1)
- r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/JooqConfiguration.java
🧰 Additional context used
📓 Learnings (1)
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java (1)
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1627
File: r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java:86-96
Timestamp: 2025-01-08T16:38:06.730Z
Learning: In jOOQ, use generic type parameters with `record.get()` method for type-safe record mapping (e.g., `record.<List<String>>get("field")`). For multiset conversions, use `records.into(Class)` pattern (e.g., `records.into(PostCommentResponse.class)`).
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Unit & Integration Tests
🔇 Additional comments (8)
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java (2)
52-58: LGTM! Good refactoring to reduce code duplication.The use of
commonSelectJoinStep()improves code maintainability while preserving the existing pagination and sorting functionality.
75-75: LGTM! Method signature simplified appropriately.The removal of the unused Condition parameter and use of commonSelectJoinStep makes the code cleaner and more maintainable.
Also applies to: 78-78
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/CustomPostRepository.java (1)
12-12: Verify all callers have been updatedThe method signature change from accepting a Condition parameter to no parameters requires verification of all calling code.
Let's verify all callers have been updated:
✅ Verification successful
✓ All callers are correctly using the updated method signature
All usages of
retrievePostsWithCommentsAndTags()have been verified and are correctly using the parameter-less version of the method. The implementation and all calling code are in sync with the interface definition.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining calls to retrievePostsWithCommentsAndTags with parameters rg -l "retrievePostsWithCommentsAndTags\s*\([^)]+\)" --type java # Search for the implementation class to verify the changes rg -l "class.*CustomPostRepositoryImpl.*implements.*CustomPostRepository" --type javaLength of output: 273
Script:
#!/bin/bash # Find all calls to the method regardless of parameters rg "retrievePostsWithCommentsAndTags" -A 2 --type java # Check the implementation class cat r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.javaLength of output: 8074
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/R2dbcConfiguration.java (1)
24-29: LGTM! Well-structured connection factory configuration.The connection factory configuration is properly set up with:
- Proxy driver for query interception
- Connection pooling for PostgreSQL
- Integration with the query execution listener
r2dbc/boot-jooq-r2dbc-sample/pom.xml (2)
107-111: LGTM! Proper dependency management.The r2dbc-proxy dependency is correctly added and version management is delegated to spring-boot-starter-parent.
Line range hint
254-271: LGTM! Proper configuration for generated sources.The build-helper-maven-plugin is correctly configured to add the JOOQ generated sources to the build path.
r2dbc/boot-jooq-r2dbc-sample/src/test/java/com/example/jooq/r2dbc/repository/PostRepositoryTest.java (2)
7-8: LGTM! Configuration changes align with R2DBC proxy integration.The addition of R2dbcConfiguration and QueryProxyExecutionListener properly sets up the infrastructure for R2DBC proxy and query logging in the test environment.
Also applies to: 32-32
89-89: LGTM! Method calls updated correctly.The calls to
retrievePostsWithCommentsAndTagshave been properly updated to match the simplified method signature, maintaining the original test scenarios while removing the unnecessary parameter.Also applies to: 119-119
| package com.example.jooq.r2dbc.config; | ||
|
|
||
| import io.r2dbc.proxy.core.QueryExecutionInfo; | ||
| import io.r2dbc.proxy.listener.ProxyExecutionListener; | ||
| import io.r2dbc.proxy.support.QueryExecutionInfoFormatter; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class QueryProxyExecutionListener implements ProxyExecutionListener { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(QueryProxyExecutionListener.class); | ||
|
|
||
| @Override | ||
| public void afterQuery(QueryExecutionInfo queryExecutionInfo) { | ||
| QueryExecutionInfoFormatter formatter = QueryExecutionInfoFormatter.showAll(); | ||
| String str = formatter.format(queryExecutionInfo); | ||
| logger.info("Query: {}", str); | ||
| logger.info("Execution Time: {} ms", queryExecutionInfo.getExecuteDuration().toMillis()); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Consider implementing query logging controls for security.
- Add configuration property to control logging detail level:
@ConfigurationProperties("application.query-logging")
public class QueryLoggingProperties {
private boolean enabled = false;
private boolean maskSensitiveData = true;
}- Modify QueryProxyExecutionListener to use custom formatter:
@Component
public class QueryProxyExecutionListener implements ProxyExecutionListener {
private final QueryLoggingProperties properties;
@Override
public void afterQuery(QueryExecutionInfo info) {
if (!properties.isEnabled()) {
return;
}
QueryExecutionInfoFormatter formatter = properties.isMaskSensitiveData()
? createMaskingFormatter()
: QueryExecutionInfoFormatter.showAll();
// ... rest of the implementation
}
}🔗 Analysis chain
Consider security implications of logging full query details.
The current implementation logs complete query details which might expose sensitive data. Consider:
- Using a custom formatter to mask sensitive information
- Adding a configuration flag to enable/disable detailed logging in different environments
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential sensitive data in SQL queries
rg -i "password|secret|key|token|credit|ssn" --type sql
Length of output: 4054
Script:
#!/bin/bash
# Check for INSERT/UPDATE statements and configuration properties
echo "=== Checking for INSERT/UPDATE statements ==="
rg -i "insert\s+into|update\s+.*\s+set" --type sql
echo "=== Checking for configuration properties ==="
fd "application.*properties|application.*yml" --type f --exec cat {}
echo "=== Looking for similar logging implementations ==="
rg -i "QueryExecutionInfoFormatter|ProxyExecutionListener" -A 5 -B 5
Length of output: 66314
| ReactiveAuditorAware<String> myAuditorProvider() { | ||
| return () -> Mono.justOrEmpty("appUser"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hard-coded auditor with SecurityContext integration.
The current implementation uses a hard-coded value "appUser" for auditing. Consider integrating with Spring Security's ReactiveSecurityContextHolder for proper user auditing.
- ReactiveAuditorAware<String> myAuditorProvider() {
- return () -> Mono.justOrEmpty("appUser");
+ ReactiveAuditorAware<String> myAuditorProvider() {
+ return () -> ReactiveSecurityContextHolder.getContext()
+ .map(SecurityContext::getAuthentication)
+ .filter(Authentication::isAuthenticated)
+ .map(Authentication::getName);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ReactiveAuditorAware<String> myAuditorProvider() { | |
| return () -> Mono.justOrEmpty("appUser"); | |
| } | |
| ReactiveAuditorAware<String> myAuditorProvider() { | |
| return () -> ReactiveSecurityContextHolder.getContext() | |
| .map(SecurityContext::getAuthentication) | |
| .filter(Authentication::isAuthenticated) | |
| .map(Authentication::getName); | |
| } |
...ot-jooq-r2dbc-sample/src/test/java/com/example/jooq/r2dbc/repository/PostRepositoryTest.java
Show resolved
Hide resolved
rajadilipkolli
changed the title
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java (1)
82-111: Consider enhancing method visibility and documentation.The implementation is well-structured with type-safe record mapping. Consider these improvements:
- Add return value details in JavaDoc explaining the fields in Record7
- Consider making the method protected instead of private for better testability, as it contains core query logic
- private SelectJoinStep< + protected SelectJoinStep<* @return SelectJoinStep configured with the base query + * Returns a SelectJoinStep with Record7 containing: + * - UUID: Post ID + * - String: Post Title + * - String: Post Content + * - String: Created By + * - String: Status + * - List<PostCommentResponse>: Comments + * - List<String>: Tagsr2dbc/boot-jooq-r2dbc-sample/src/test/java/com/example/jooq/r2dbc/repository/PostRepositoryTest.java (1)
141-155: Great addition of constraint and concurrency tests!The new test methods effectively cover important scenarios:
- Unique constraint violations
- Foreign key constraints
- Optimistic locking for concurrent updates
Good use of StepVerifier and clear test arrangements.
Consider standardizing the timeout duration across tests:
- .verify(Duration.ofSeconds(5)); + .verify(Duration.ofSeconds(30));Also applies to: 157-174, 176-205
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/Initializer.java(1 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/WebFluxConfig.java(2 hunks)r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java(3 hunks)r2dbc/boot-jooq-r2dbc-sample/src/test/java/com/example/jooq/r2dbc/repository/PostRepositoryTest.java(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/Initializer.java
🧰 Additional context used
📓 Learnings (1)
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java (1)
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1627
File: r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java:86-96
Timestamp: 2025-01-08T16:38:06.730Z
Learning: In jOOQ, use generic type parameters with `record.get()` method for type-safe record mapping (e.g., `record.<List<String>>get("field")`). For multiset conversions, use `records.into(Class)` pattern (e.g., `records.into(PostCommentResponse.class)`).
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/repository/custom/impl/CustomPostRepositoryImpl.java (4)
12-12: LGTM!The new imports are correctly added to support the type-safe record mapping in the commonSelectJoinStep method.
Also applies to: 17-18
52-58: LGTM! Well-structured query composition.The refactoring improves code reusability by leveraging the commonSelectJoinStep method while maintaining proper pagination, sorting, and error handling.
75-75: LGTM! Good simplification.The method has been simplified by removing the unused Condition parameter and reusing the common query logic while maintaining the required ordering.
Also applies to: 78-78
120-121: LGTM! Proper ordering of comments.The implementation correctly orders comments by creation date in descending order, ensuring latest comments appear first.
r2dbc/boot-jooq-r2dbc-sample/src/test/java/com/example/jooq/r2dbc/repository/PostRepositoryTest.java (3)
95-95: LGTM! Method signature improvementGood cleanup - removing the unnecessary null parameter from
retrievePostsWithCommentsAndTags()calls makes the API cleaner.Also applies to: 125-125
227-227: Assertions are in incorrect orderThe test creates comments in the order "test comments", "test comments 2" using
Flux.fromArraywhich maintains insertion order. The assertions should be:
- First comment (getFirst): should expect "test comments"
- Last comment (getLast): should expect "test comments 2"
Also applies to: 236-236
33-38: Verify the necessity of both JooqConfiguration and R2dbcConfigurationThe test imports both
JooqConfigurationandR2dbcConfiguration. Consider if both configurations are necessary or if they can be consolidated to reduce potential configuration overlap.✅ Verification successful
Both configurations serve distinct purposes and are necessary
The configurations handle different aspects of the application:
JooqConfiguration: Sets up jOOQ's DSL context and exception translationR2dbcConfiguration: Manages R2DBC auditing and connection factory customization🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential configuration overlap rg -B 5 -A 10 "class (Jooq|R2dbc)Configuration" --type javaLength of output: 4364
r2dbc/boot-jooq-r2dbc-sample/src/main/java/com/example/jooq/r2dbc/config/WebFluxConfig.java (2)
Line range hint
15-21: LGTM on CORS configuration implementation.The CORS configuration is well-structured and properly uses injected properties for flexibility.
5-5: Verify the relevance of CORS changes to R2DBC proxy implementation.While adding @nonnull to improve null-safety is good practice, this change appears unrelated to the PR's main objective of adding R2DBC proxy support for query logging. Consider:
- Moving this CORS-related change to a separate PR
- Adding the missing R2DBC proxy configuration in this package
Let's verify if R2DBC proxy configuration exists:
Also applies to: 15-15
Summary by CodeRabbit
Based on the comprehensive changes across multiple files, here's a concise release note:
New Features
Improvements
Refactoring
Testing