Bug fix in bean wiring for mongoclient#729
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
WalkthroughMongoDB configuration consolidated to use URI-based connection strings with embedded database names. Primary and secondary Spring bean configurations refactored from abstract superclass inheritance to explicit bean declarations. Deployment manifests and application properties updated to provide versioned database names ( ChangesMongoDB URI-based Configuration Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In
`@openlap-analyticsframework/src/main/java/com/openlap/configurations/PrimaryMongoConfig.java`:
- Around line 41-42: PrimaryMongoConfig currently calls new
SimpleMongoClientDbFactory(client, db) where db is obtained via new
ConnectionString(primaryUri).getDatabase(); validate that db is not null or
empty before creating the factory: call
ConnectionString(primaryUri).getDatabase(), check StringUtils.hasText(db) (or db
!= null && !db.isEmpty()), and if missing throw an IllegalArgumentException with
a clear message like "Database name must be specified in primaryUri" (do the
same validation pattern in SecondaryMongoConfig for its primary/secondaryUri and
corresponding factory creation). Ensure you reference
ConnectionString.getDatabase, primaryUri, SimpleMongoClientDbFactory,
PrimaryMongoConfig and SecondaryMongoConfig when making the changes.
In
`@openlap-analyticsframework/src/main/java/com/openlap/configurations/SecondaryMongoConfig.java`:
- Around line 33-34: The code currently calls new
ConnectionString(secondaryUri).getDatabase() and passes the result directly into
new SimpleMongoClientDbFactory(client, db); add a validation step after
ConnectionString.getDatabase() to ensure the returned db is not null/empty
(e.g., if getDatabase() == null || db.isBlank()) and if it is, throw an
IllegalArgumentException (or a clear IllegalStateException) with a message
referencing secondaryUri so startup failure is explicit; keep using
ConnectionString, secondaryUri, getDatabase(), client and
SimpleMongoClientDbFactory but ensure the factory is only created when a valid
database name is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7f5f4936-22b9-48d1-b646-a6e66bef763b
📒 Files selected for processing (7)
.gitignore.k8s/base/openlap-analyticsframework/web/deployment.yamlREADME.mdopenlap-analyticsframework/compose.yamlopenlap-analyticsframework/src/main/java/com/openlap/configurations/PrimaryMongoConfig.javaopenlap-analyticsframework/src/main/java/com/openlap/configurations/SecondaryMongoConfig.javaopenlap-analyticsframework/src/main/resources/application.properties
| String db = new ConnectionString(primaryUri).getDatabase(); | ||
| return new SimpleMongoClientDbFactory(client, db); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for usage patterns and documentation of ConnectionString.getDatabase()
# and SimpleMongoClientDbFactory to confirm null handling behavior.
# Search for existing null checks on ConnectionString database extraction
rg -nP --type=java -C3 'ConnectionString.*getDatabase\(\)' -g '!**/test/**'
# Search for any validation of database names in MongoDB configuration
rg -nP --type=java -C3 'SimpleMongoClientDbFactory.*null|isEmpty' -g '!**/test/**'Repository: ude-soco/openlap-official
Length of output: 50381
🏁 Script executed:
# Find the MongoDB driver version and related dependencies
fd -e pom.xml | head -5 | xargs grep -A2 -B2 "mongo" -iRepository: ude-soco/openlap-official
Length of output: 53
🌐 Web query:
MongoDB Java Driver ConnectionString.getDatabase() null behavior
💡 Result:
The MongoDB Java Driver's ConnectionString.getDatabase method returns null when no database name is specified in the connection string. The method is annotated with @Nullable and documented simply as "Gets the database name", which explicitly allows for null if absent [1][2]. The connection string format is mongodb://[username:password@]host1[:port1]...[/database.collection][?options], where /database is optional and only relevant for authentication; if omitted, getDatabase yields null [2][3]. For example, "mongodb://localhost:27017" has no database part, so ConnectionString cs = new ConnectionString("mongodb://localhost:27017"); String db = cs.getDatabase; would give db == null. This behavior is consistent across driver versions including 5.4.0 [1], 4.0.1 [2], and 3.x [4][3]. Note that MongoClient.getDatabase(name) is separate and requires a provided name; ConnectionString.getDatabase only parses what's in the URI [5][6].
Citations:
- 1: https://mongodb.github.io/mongo-java-driver/5.4/apidocs/mongodb-driver-core/com/mongodb/ConnectionString.html
- 2: https://mongodb.github.io/mongo-java-driver/4.0/apidocs/mongodb-driver-core/com/mongodb/ConnectionString.html
- 3: https://developer.adobe.com/experience-manager/reference-materials/6-5/javadoc/com/mongodb/ConnectionString.html
- 4: https://www.javadoc.io/doc/org.mongodb/mongo-java-driver/latest/com/mongodb/ConnectionString.html
- 5: https://www.mongodb.com/docs/drivers/java/sync/upcoming/connection/connect/
- 6: https://www.mongodb.com/docs/drivers/java/sync/v4.8/fundamentals/connection/connect/
🌐 Web query:
SimpleMongoClientDbFactory null database parameter Spring Data MongoDB
💡 Result:
The SimpleMongoClientDatabaseFactory (or SimpleMongoClientDbFactory) in Spring Data MongoDB requires a non-null, non-empty database name parameter in its constructors. Passing null for the database parameter results in an IllegalArgumentException, typically with a message like "Database name must not be null or empty" or "Parameter databaseName must not be null or empty" [1][2][3][4]. Key evidence from official sources: - Constructor javadoc: "@param databaseName must not be {@literal null} or empty." [1][2][3] - Source code explicitly documents this requirement and throws exceptions for validation failures [1]. - Common runtime error when misconfigured: "Factory method 'mongoDatabaseFactory' threw exception with message: Database name must not be empty" [4]. To avoid this: - Provide a valid database name, e.g., new SimpleMongoClientDatabaseFactory(mongoClient, "myDatabase") [5]. - When using spring.data.mongodb.uri, ensure it includes the database name at the end (e.g., mongodb://localhost:27017/myDatabase), as spring.data.mongodb.database may be ignored if uri is set [4]. - For connection strings without dbname, use MongoTemplate directly with explicit db selection or ensure config provides it [5][6].
Citations:
- 1: https://github.com/spring-projects/spring-data-mongodb/blob/master/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleMongoClientDatabaseFactory.java
- 2: http://www.springframework.org/spring-data/data-mongodb/reference/api/java/org/springframework/data/mongodb/core/SimpleMongoClientDatabaseFactory.html
- 3: https://docs.spring.io/spring-data/mongodb/docs/current/api/org/springframework/data/mongodb/core/SimpleMongoClientDatabaseFactory.html
- 4: https://stackoverflow.com/questions/76335121/how-to-resolve-the-factory-method-mongodatabasefactory-threw-exception-with-m
- 5: https://docs.spring.io/spring-data/data-mongo/reference/mongodb/configuration.html
- 6: https://docs.spring.io/spring-data/mongodb/docs/current-SNAPSHOT/reference/html/
🏁 Script executed:
# Check if there are any test files that show expected behavior
fd -e java -path "*test*" | xargs rg -l "SimpleMongoClientDbFactory" 2>/dev/null | head -3Repository: ude-soco/openlap-official
Length of output: 442
Add validation for database name in MongoDB URI.
ConnectionString.getDatabase() returns null if the database is not specified in the URI. According to Spring Data MongoDB documentation, passing null to SimpleMongoClientDbFactory throws IllegalArgumentException: "Database name must not be null or empty" at application startup. Validate that the database name is present in the primary MongoDB URI before creating the factory.
Proposed fix
public MongoDbFactory primaryMongoDbFactory(`@Qualifier`("primaryMongoClient") MongoClient client) {
String db = new ConnectionString(primaryUri).getDatabase();
+ if (db == null || db.isEmpty()) {
+ throw new IllegalStateException(
+ "Database name must be specified in primary MongoDB URI: " + primaryUri);
+ }
return new SimpleMongoClientDbFactory(client, db);
}Note: The same issue exists in SecondaryMongoConfig.java at line 33.
📝 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.
| String db = new ConnectionString(primaryUri).getDatabase(); | |
| return new SimpleMongoClientDbFactory(client, db); | |
| String db = new ConnectionString(primaryUri).getDatabase(); | |
| if (db == null || db.isEmpty()) { | |
| throw new IllegalStateException( | |
| "Database name must be specified in primary MongoDB URI: " + primaryUri); | |
| } | |
| return new SimpleMongoClientDbFactory(client, db); |
🤖 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
`@openlap-analyticsframework/src/main/java/com/openlap/configurations/PrimaryMongoConfig.java`
around lines 41 - 42, PrimaryMongoConfig currently calls new
SimpleMongoClientDbFactory(client, db) where db is obtained via new
ConnectionString(primaryUri).getDatabase(); validate that db is not null or
empty before creating the factory: call
ConnectionString(primaryUri).getDatabase(), check StringUtils.hasText(db) (or db
!= null && !db.isEmpty()), and if missing throw an IllegalArgumentException with
a clear message like "Database name must be specified in primaryUri" (do the
same validation pattern in SecondaryMongoConfig for its primary/secondaryUri and
corresponding factory creation). Ensure you reference
ConnectionString.getDatabase, primaryUri, SimpleMongoClientDbFactory,
PrimaryMongoConfig and SecondaryMongoConfig when making the changes.
| String db = new ConnectionString(secondaryUri).getDatabase(); | ||
| return new SimpleMongoClientDbFactory(client, db); |
There was a problem hiding this comment.
Validate that the URI contains a database name before creating the factory.
ConnectionString.getDatabase() returns null if the database is not present in the URI. Passing null to SimpleMongoClientDbFactory will likely cause a runtime failure during application startup. Add validation to ensure the database name is present in the URI.
🛡️ Proposed fix to add validation
public MongoDbFactory secondaryMongoDbFactory(
`@Qualifier`("secondaryMongoClient") MongoClient client) {
String db = new ConnectionString(secondaryUri).getDatabase();
+ if (db == null || db.isEmpty()) {
+ throw new IllegalStateException(
+ "Database name must be specified in secondary MongoDB URI: " + secondaryUri);
+ }
return new SimpleMongoClientDbFactory(client, db);
}📝 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.
| String db = new ConnectionString(secondaryUri).getDatabase(); | |
| return new SimpleMongoClientDbFactory(client, db); | |
| public MongoDbFactory secondaryMongoDbFactory( | |
| `@Qualifier`("secondaryMongoClient") MongoClient client) { | |
| String db = new ConnectionString(secondaryUri).getDatabase(); | |
| if (db == null || db.isEmpty()) { | |
| throw new IllegalStateException( | |
| "Database name must be specified in secondary MongoDB URI: " + secondaryUri); | |
| } | |
| return new SimpleMongoClientDbFactory(client, db); | |
| } |
🤖 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
`@openlap-analyticsframework/src/main/java/com/openlap/configurations/SecondaryMongoConfig.java`
around lines 33 - 34, The code currently calls new
ConnectionString(secondaryUri).getDatabase() and passes the result directly into
new SimpleMongoClientDbFactory(client, db); add a validation step after
ConnectionString.getDatabase() to ensure the returned db is not null/empty
(e.g., if getDatabase() == null || db.isBlank()) and if it is, throw an
IllegalArgumentException (or a clear IllegalStateException) with a message
referencing secondaryUri so startup failure is explicit; keep using
ConnectionString, secondaryUri, getDatabase(), client and
SimpleMongoClientDbFactory but ensure the factory is only created when a valid
database name is present.
2 participants
No description provided.