chore(deps): Upgrading mongodb driver to mongodb driver sync#27685
Open
josephglerin wants to merge 1 commit intoprestodb:masterfrom
Open
chore(deps): Upgrading mongodb driver to mongodb driver sync#27685josephglerin wants to merge 1 commit intoprestodb:masterfrom
josephglerin wants to merge 1 commit intoprestodb:masterfrom
Conversation
|
|
Contributor
Reviewer's GuideUpgrades the Presto MongoDB connector from the legacy Mongo client to the modern mongodb-driver-sync API, refactoring client/session construction, TLS/read preference handling, and tests to use MongoClientSettings and connection strings, and updating driver dependencies and supported write concerns accordingly. Sequence diagram for MongoSession creation with MongoClientSettings and connection stringsequenceDiagram
participant Caller as MongoClientModule_createMongoSession_caller
participant MCM as MongoClientModule
participant MCSB as MongoClientSettings_Builder
participant CSP as ConnectionStringParser
participant MCP as MongoClientPoolSettings
participant MSS as MongoSocketSettings
participant SSL as SslContextProvider
participant MC as MongoClient
Caller->>MCM: createMongoSession(typeManager, mongoClientConfig)
activate MCM
MCM->>MCM: buildConnectionString(mongoClientConfig)
activate MCM
MCM->>CSP: new ConnectionString(connectionString)
deactivate MCM
MCM->>MCSB: MongoClientSettings.builder()
activate MCSB
MCM->>MCSB: applyConnectionString(connectionString)
MCM->>MCSB: applyToConnectionPoolSettings(maxSize, minSize, maxWaitTime)
activate MCP
MCP-->>MCSB: configuredPoolSettings
deactivate MCP
MCM->>MCSB: applyToSocketSettings(connectTimeout, readTimeout)
activate MSS
MSS-->>MCSB: configuredSocketSettings
deactivate MSS
MCM->>MCM: configureReadPreference(mongoClientConfig)
MCM-->>MCSB: readPreference
MCM->>MCSB: writeConcern(writeConcernFromConfig)
alt replicaSetNamePresent
MCM->>MCSB: applyToClusterSettings(requiredReplicaSetName)
end
alt tlsEnabled
MCM->>SSL: new SslContextProvider(truststorePath, truststorePassword, keystorePath, keystorePassword)
activate SSL
SSL-->>MCM: buildSslContext_optional
deactivate SSL
MCM->>MCSB: applyToSslSettings(enabled=true, context=sslContext)
end
MCSB-->>MCM: MongoClientSettings
deactivate MCSB
MCM->>MC: MongoClients.create(settings)
activate MC
MCM-->>Caller: new MongoSession(typeManager, MC, mongoClientConfig)
deactivate MC
deactivate MCM
Class diagram for updated MongoClientModule and WriteConcernTypeclassDiagram
class MongoClientModule {
+MongoClientModule()
+static MongoSession createMongoSession(TypeManager typeManager, MongoClientConfig config)
-static String buildConnectionString(MongoClientConfig config)
-static ReadPreference configureReadPreference(MongoClientConfig config)
-static void configureSsl(MongoClientSettings_Builder settings, MongoClientConfig config)
}
class MongoClientConfig {
+List~ServerAddress~ getSeeds()
+List~MongoCredential~ getCredentials()
+int getConnectionsPerHost()
+int getMinConnectionsPerHost()
+int getMaxWaitTime()
+int getConnectionTimeout()
+int getSocketTimeout()
+boolean getSocketKeepAlive()
+WriteConcernType getWriteConcern()
+MongoReadPreferenceType getReadPreference()
+List~TagSet~ getReadPreferenceTags()
+String getRequiredReplicaSetName()
+boolean isTlsEnabled()
+String getTruststorePath()
+String getTruststorePassword()
+String getKeystorePath()
+String getKeystorePassword()
}
class MongoSession {
+MongoSession(TypeManager typeManager, MongoClient client, MongoClientConfig config)
+void shutdown()
+MongoDatabase getDatabase(String schemaName)
+MongoCollection getCollection(String schemaName, String tableName)
+void insert(String schemaName, String tableName, List~Document~ documents)
+MongoCursor query(String schemaName, String tableName, Bson filter, Bson projection)
}
class WriteConcernType {
<<enumeration>>
+ACKNOWLEDGED
+JOURNALED
+MAJORITY
+UNACKNOWLEDGED
-WriteConcern writeConcern
+WriteConcernType(WriteConcern writeConcern)
+WriteConcern getWriteConcern()
}
class SslContextProvider {
+SslContextProvider(String truststorePath, String truststorePassword, String keystorePath, String keystorePassword)
+Optional~SSLContext~ buildSslContext()
}
class MongoClientSettings_Builder {
+MongoClientSettings_Builder applyConnectionString(ConnectionString connectionString)
+MongoClientSettings_Builder applyToConnectionPoolSettings(ConnectionPoolSettingsCallback callback)
+MongoClientSettings_Builder applyToSocketSettings(SocketSettingsCallback callback)
+MongoClientSettings_Builder applyToClusterSettings(ClusterSettingsCallback callback)
+MongoClientSettings_Builder applyToSslSettings(SslSettingsCallback callback)
+MongoClientSettings_Builder writeConcern(WriteConcern writeConcern)
+MongoClientSettings_Builder readPreference(ReadPreference readPreference)
+MongoClientSettings build()
}
class MongoClients {
+static MongoClient create(MongoClientSettings settings)
}
class MongoClient {
+MongoDatabase getDatabase(String name)
+void close()
}
MongoClientModule ..> MongoClientConfig : uses
MongoClientModule ..> MongoSession : creates
MongoClientModule ..> MongoClientSettings_Builder : configures
MongoClientModule ..> MongoClients : createsMongoClient
MongoClientModule ..> SslContextProvider : tlsConfiguration
MongoClientModule ..> WriteConcernType : uses
MongoSession ..> MongoClient : holds
MongoClientConfig ..> WriteConcernType : returns
WriteConcernType ..> WriteConcern : wraps
MongoClients ..> MongoClient : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
prestodb-ci
requested review from
a team,
Shreya-ibm and
anees-kt
and removed request for
a team
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
buildConnectionString, the connection string is manually assembled using only the first credential and without URL-encoding username/password or auth DB, which can break for special characters and may silently ignore multiple configured credentials—consider usingConnectionString-compatible formatting helpers or encoding, and either enforcing a single-credential invariant or handling multiple credentials explicitly. - The new
MongoClientSettingsconfiguration dropssocketKeepAlive(config.getSocketKeepAlive())from the oldMongoClientOptionssetup, so if that flag is still meaningful inMongoClientConfigyou may want to map it to the driver’s equivalent (or explicitly deprecate/ignore it in the config). - Since
mongodb-driver-syncalready pulls inmongodb-driver-coreandbson, you may be able to avoid explicitly declaring those extra dependencies in the POM unless there is a strong reason to keep them direct.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `buildConnectionString`, the connection string is manually assembled using only the first credential and without URL-encoding username/password or auth DB, which can break for special characters and may silently ignore multiple configured credentials—consider using `ConnectionString`-compatible formatting helpers or encoding, and either enforcing a single-credential invariant or handling multiple credentials explicitly.
- The new `MongoClientSettings` configuration drops `socketKeepAlive(config.getSocketKeepAlive())` from the old `MongoClientOptions` setup, so if that flag is still meaningful in `MongoClientConfig` you may want to map it to the driver’s equivalent (or explicitly deprecate/ignore it in the config).
- Since `mongodb-driver-sync` already pulls in `mongodb-driver-core` and `bson`, you may be able to avoid explicitly declaring those extra dependencies in the POM unless there is a strong reason to keep them direct.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
nishithakbhaskaran
left a comment
nishithakbhaskaran
left a comment
There was a problem hiding this comment.
@josephglerin Thanks for the PR.
Please update the release note and PR title based on the guidelines.
josephglerin
changed the title
Author
Testing
|
agrawalreetika
requested changes
Apr 30, 2026
Member
agrawalreetika
left a comment
agrawalreetika
left a comment
There was a problem hiding this comment.
Thanks for the PR.
Is it going to be backward compatible? Please check the existing MongoDB doc and update the version requirement if any, accordingly - https://prestodb.io/docs/current/connector/mongodb.html
| .connectionsPerHost(config.getConnectionsPerHost()) | ||
| .connectTimeout(config.getConnectionTimeout()) | ||
| .socketTimeout(config.getSocketTimeout()) | ||
| .socketKeepAlive(config.getSocketKeepAlive()) |
Member
There was a problem hiding this comment.
socketKeepAlive is removed?
|
|
||
| // Add credentials if present | ||
| if (!config.getCredentials().isEmpty()) { | ||
| MongoCredential credential = config.getCredentials().get(0); |
Member
There was a problem hiding this comment.
What if there are multiple credentials provided? I see the config is MongoCredential list
Comment on lines
+98
to
+104
| if (!config.getCredentials().isEmpty()) { | ||
| MongoCredential credential = config.getCredentials().get(0); | ||
| connectionString.append(credential.getUserName()) | ||
| .append(":") | ||
| .append(new String(credential.getPassword())) | ||
| .append("@"); | ||
| } |
Member
There was a problem hiding this comment.
Should we set credentials via settingsBuilder directly instead of exposing it as a String in connectionString?
| String connectionString = buildConnectionString(config); | ||
| settingsBuilder.applyConnectionString(new ConnectionString(connectionString)); | ||
|
|
||
| // Connection pool settings |
Member
There was a problem hiding this comment.
Nit: i think we can remove these one-liner comments in different places
| } | ||
|
|
||
| // directConnection=false forces replica set discovery to find the primary | ||
| connectionString.append("?directConnection=false"); |
Member
There was a problem hiding this comment.
I suppose for single node deployment, we don't need discovery? Should we only use directConnection=false for replica sets?
agrawalreetika
changed the title
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
4 participants
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.
Upgrade MongoDB driver from mongodb-driver-legacy to mongodb-driver-sync
Upgrade Presto MongoDB connector from the unmaintained legacy MongoDB Java driver to the modern MongoDB Java Driver Sync (version 5.6.5)
Motivation and Context
The Presto MongoDB connector currently uses the legacy MongoDB Java driver, which MongoDB officially deprecated and no longer maintains. This upgrade is critical for several reasons:
End of Life & Security
MongoDB Server Compatibility
Performance & Reliability
Future-Proofing
Reference: MongoDB Legacy Driver Documentation
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
Summary by Sourcery
Upgrade the MongoDB connector to use the modern mongodb-driver-sync client and configuration APIs.
Enhancements:
Build:
Tests: