feat(database): Add DatabaseConfig support to DataSourceDatabaseTester - #909
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideAdds constructor-injected DatabaseConfig support to DataSourceDatabaseTester so a shared config is applied to every created connection, introduces a DatabaseConfig.copyPropertiesInto helper and fixes a previously unset feature flag property, along with corresponding tests and changelog entry. Sequence diagram for DataSourceDatabaseTester connection creation with DatabaseConfigsequenceDiagram
actor Test
participant Tester as DataSourceDatabaseTester
participant Provider as CachingConnectionProvider
participant DS as DataSource
participant Conn as DatabaseConnection
participant Config as DatabaseConfig
Test->>Tester: getConnection()
alt connectionProvider not null
Tester->>Provider: getConnection()
Provider-->>Tester: IDatabaseConnection
else connectionProvider null
Tester->>Tester: createConnection()
Tester->>DS: getConnection()
DS-->>Tester: java.sql.Connection
Tester->>Conn: new DatabaseConnection(Connection, schema)
Note over Tester: databaseConfig not null
Tester->>Config: copyPropertiesInto(Conn.getConfig())
Config-->>Tester: (properties applied)
Tester-->>Test: IDatabaseConnection
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe change adds ChangesConfigurable database connections
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DataSourceDatabaseTester
participant CachingConnectionProvider
participant DatabaseConnection
participant DatabaseConfig
DataSourceDatabaseTester->>CachingConnectionProvider: obtain connection
CachingConnectionProvider-->>DataSourceDatabaseTester: return DatabaseConnection
DataSourceDatabaseTester->>DatabaseConfig: copyPropertiesInto(connection configuration)
DatabaseConfig->>DatabaseConnection: set copied properties and features
DataSourceDatabaseTester-->>DataSourceDatabaseTester: return configured connection
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/org/dbunit/database/DatabaseConfig.java (1)
284-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the new JavaDoc parameter descriptions.
Line 287 uses a sentence fragment. Lines 104-110 use sentence fragments and omit final periods. Use complete sentences that begin with a capital letter and end with a period.
src/main/java/org/dbunit/database/DatabaseConfig.java#L284-L290: Rewrite@param targetas a complete sentence.src/main/java/org/dbunit/DataSourceDatabaseTester.java#L97-L114: Rewrite every new@paramdescription as a complete sentence.As per coding guidelines, “use complete sentences beginning with a capital letter and ending with a period for topic text, parameters, and return descriptions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/dbunit/database/DatabaseConfig.java` around lines 284 - 290, Update the `@param` target description in DatabaseConfig.copyPropertiesInto so it begins with a capital letter and ends with a period. In DataSourceDatabaseTester.java lines 97-114, rewrite every new `@param` description as a complete, capitalized sentence with a final period; apply the changes in both affected files.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/org/dbunit/database/DatabaseConfig.java`:
- Around line 284-290: Update the `@param` target description in
DatabaseConfig.copyPropertiesInto so it begins with a capital letter and ends
with a period. In DataSourceDatabaseTester.java lines 97-114, rewrite every new
`@param` description as a complete, capitalized sentence with a final period;
apply the changes in both affected files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df1a24c9-8e89-4dff-819a-b235a569e671
📒 Files selected for processing (5)
src/changes/changes.xmlsrc/main/java/org/dbunit/DataSourceDatabaseTester.javasrc/main/java/org/dbunit/database/DatabaseConfig.javasrc/test/java/org/dbunit/DataSourceDatabaseTesterIT.javasrc/test/java/org/dbunit/database/DatabaseConfigTest.java
* Add DataSourceDatabaseTester(DataSource, String, CachingConnectionProvider, DatabaseConfig) constructor that applies the given config's properties and features to every connection created via createConnection(), so callers no longer need an IOperationListener#connectionRetrieved() override just to reach into each connection's DatabaseConfig (e.g. to set PROPERTY_DATATYPE_FACTORY for a specific database). * Add DatabaseConfig#copyPropertiesInto(DatabaseConfig) to copy every ALL_PROPERTIES entry from one config into another. * Fix DatabaseConfig's constructor, which left FEATURE_SKIP_ORACLE_RECYCLEBIN_TABLES's underlying property value unset despite ALL_PROPERTIES declaring it non-nullable - harmless until copyPropertiesInto()'s full iteration became the first caller to round-trip every declared property through setProperty(), which enforces that constraint strictly. Refs: 707
jeffjensen
force-pushed
the
707-datasourcedatabasetester-databaseconfig
branch
from
August 6, 2026 00:18
daa551e to
234a47d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DataSourceDatabaseTester(DataSource, String schema, CachingConnectionProvider, DatabaseConfig)constructor that applies the given config's properties and features to every connection created viacreateConnection(), so callers no longer need anIOperationListener#connectionRetrieved()override just to reach into each connection'sDatabaseConfig(e.g. to setPROPERTY_DATATYPE_FACTORYfor a specific database), matching the workaround shown in the issue.DatabaseConfig#copyPropertiesInto(DatabaseConfig)to copy everyALL_PROPERTIESentry from one config into another.DatabaseConfig's constructor, which leftFEATURE_SKIP_ORACLE_RECYCLEBIN_TABLES's underlying property value unset despiteALL_PROPERTIESdeclaring it non-nullable - harmless untilcopyPropertiesInto()'s full iteration became the first caller to round-trip every declared property throughsetProperty(), which enforces that constraint strictly.Fixes #707
Test plan
./mvnw clean test- full unit suite passes./mvnw clean verify -Phsqldb-2-7- full unit + IT suite passesDatabaseConfigTestcoverage forcopyPropertiesInto()(configured values copied; nullable property overwritten with null)DataSourceDatabaseTesterITcoverage: config applied with and without aCachingConnectionProvider; 4-arg constructor still rejects a nullDataSource🤖 Generated with Claude Code
https://claude.ai/code/session_0136dJj6sBAMovFVNgGkyaDd
Summary by Sourcery
Add constructor-injected DatabaseConfig support to DataSourceDatabaseTester and introduce a utility to copy database configuration properties between connections.
New Features:
Bug Fixes:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Bug Fixes