Enhance JDBC connection string parsing for multiple hosts #1082
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1082 +/- ##
============================================
- Coverage 60.74% 60.72% -0.02%
Complexity 87 87
============================================
Files 320 320
Lines 14446 14491 +45
Branches 1988 2014 +26
============================================
+ Hits 8775 8800 +25
- Misses 5091 5102 +11
- Partials 580 589 +9
Continue to review full report at Codecov.
|
| return new ConnectionMetaData(dbVendor, host, port, user); | ||
| } | ||
|
|
||
| private HostPort parseAddressList(String connectionUrl) { |
There was a problem hiding this comment.
[minor] maybe return null when HostPort.host == null || HostPort.port < 0
There was a problem hiding this comment.
[minor] maybe naming this method with Oracle might be relevant as it's very Oracle-specific.
There was a problem hiding this comment.
maybe return
nullwhenHostPort.host == null || HostPort.port < 0
👍 for HostPort.host == null, however I am pretty sure the port is optional, so one may set a host and not the port (in which case the default will be used- there is a test for that)
maybe naming this method with Oracle might be relevant as it's very Oracle-specific.
It's within the Oracle parser (enum)
| } | ||
| default: { | ||
| if (currentValueBuffer == null) { | ||
| logger.warn("Failed to parse Oracle DB address list from: {}", connectionUrl); |
There was a problem hiding this comment.
do we really want to continue parsing in case of error ?
There was a problem hiding this comment.
Could be an error or an unknown use of the connection string. However, I only care about very specific nodes in this tree, so if the parsing yields a valid tree in which I can find the tree nodes I am looking for, I don't care.
| return ConnectionUrlParser.defaultParse(connectionUrl, dbVendor, 3306, user); | ||
| String host = "localhost"; | ||
| int port = 3306; | ||
| HostPort hostPort = parseMySqlFlavor(connectionUrl); |
There was a problem hiding this comment.
[minor] same here, having a null return value would avoid duplication of the null check and port > 0 below.
There was a problem hiding this comment.
I don't think- most (or all) make port configuration optional, each provider using a different default port if not configured.
What does this PR do?
Enhance the JDBC connection string parser to support dedicated syntaxes for multiple hosts in Oracle, MySQL and MariaDB.
Checklist
Related issues
Closes #1031