Skip to content

Commit 34d52e4

Browse files
committed
code review
1 parent 478e1d1 commit 34d52e4

15 files changed

+29
-152
lines changed

wrapper/src/main/java/software/amazon/jdbc/ConnectionPluginManager.java

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ protected <T, E extends Exception> T executeWithSubscribedPlugins(
213213
final String methodName,
214214
final PluginPipeline<T, E> pluginPipeline,
215215
final JdbcCallable<T, E> jdbcMethodFunc,
216-
final boolean useForceConnectPipeline,
217216
final @Nullable ConnectionPlugin pluginToSkip)
218217
throws E {
219218

@@ -227,13 +226,13 @@ protected <T, E extends Exception> T executeWithSubscribedPlugins(
227226

228227
// noinspection unchecked
229228
PluginChainJdbcCallable<T, E> pluginChainFunc =
230-
useForceConnectPipeline
229+
FORCE_CONNECT_METHOD.equals(methodName)
231230
? this.pluginForceConnectChainFuncMap.get(methodName)
232231
: this.pluginChainFuncMap.get(methodName);
233232

234233
if (pluginChainFunc == null) {
235-
pluginChainFunc = this.makePluginChainFunc(methodName, useForceConnectPipeline);
236-
if (useForceConnectPipeline) {
234+
pluginChainFunc = this.makePluginChainFunc(methodName);
235+
if (FORCE_CONNECT_METHOD.equals(methodName)) {
237236
this.pluginForceConnectChainFuncMap.put(methodName, pluginChainFunc);
238237
} else {
239238
this.pluginChainFuncMap.put(methodName, pluginChainFunc);
@@ -262,8 +261,7 @@ protected <T, E extends Exception> T executeWithTelemetry(
262261

263262
@Nullable
264263
protected <T, E extends Exception> PluginChainJdbcCallable<T, E> makePluginChainFunc(
265-
final @NonNull String methodName,
266-
final boolean useForceConnectPipeline) {
264+
final @NonNull String methodName) {
267265

268266
PluginChainJdbcCallable<T, E> pluginChainFunc = null;
269267

@@ -272,7 +270,9 @@ protected <T, E extends Exception> PluginChainJdbcCallable<T, E> makePluginChain
272270
final Set<String> pluginSubscribedMethods = plugin.getSubscribedMethods();
273271
final String pluginName = pluginNameByClass.getOrDefault(plugin.getClass(), plugin.getClass().getSimpleName());
274272
final boolean isSubscribed =
275-
(!useForceConnectPipeline || (plugin instanceof AuthenticationConnectionPlugin))
273+
(!FORCE_CONNECT_METHOD.equals(methodName)
274+
|| (plugin instanceof AuthenticationConnectionPlugin)
275+
|| (plugin instanceof DefaultConnectionPlugin))
276276
&& (pluginSubscribedMethods.contains(ALL_METHODS) || pluginSubscribedMethods.contains(methodName));
277277

278278
if (isSubscribed) {
@@ -361,7 +361,6 @@ public <T, E extends Exception> T execute(
361361
plugin.execute(
362362
resultType, exceptionClass, methodInvokeOn, methodName, func, jdbcMethodArgs),
363363
jdbcMethodFunc,
364-
false,
365364
null);
366365
}
367366

@@ -383,6 +382,7 @@ public <T, E extends Exception> T execute(
383382
* @param isInitialConnection a boolean indicating whether the current {@link Connection} is
384383
* establishing an initial physical connection to the database or has
385384
* already established a physical connection in the past
385+
* @param pluginToSkip the plugin that needs to be skipped while executing this pipeline
386386
* @return a {@link Connection} to the requested host
387387
* @throws SQLException if there was an error establishing a {@link Connection} to the requested
388388
* host
@@ -404,7 +404,6 @@ public Connection connect(
404404
() -> {
405405
throw new SQLException("Shouldn't be called.");
406406
},
407-
false,
408407
pluginToSkip);
409408
} catch (final SQLException | RuntimeException e) {
410409
throw e;
@@ -451,7 +450,6 @@ public Connection forceConnect(
451450
() -> {
452451
throw new SQLException("Shouldn't be called.");
453452
},
454-
true,
455453
pluginToSkip);
456454
} catch (SQLException | RuntimeException e) {
457455
throw e;
@@ -567,7 +565,6 @@ public void initHostProvider(
567565
() -> {
568566
throw new SQLException("Shouldn't be called.");
569567
},
570-
false,
571568
null);
572569
} finally {
573570
context.closeContext();
@@ -670,7 +667,7 @@ private interface PluginPipeline<T, E extends Exception> {
670667
T call(final @NonNull ConnectionPlugin plugin, final @Nullable JdbcCallable<T, E> jdbcMethodFunc) throws E;
671668
}
672669

673-
private interface PluginChainJdbcCallable<T, E extends Exception> {
670+
interface PluginChainJdbcCallable<T, E extends Exception> {
674671

675672
T call(
676673
final @NonNull PluginPipeline<T, E> pipelineFunc,

wrapper/src/main/java/software/amazon/jdbc/plugin/AuroraConnectionTrackerPlugin.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ public class AuroraConnectionTrackerPlugin extends AbstractConnectionPlugin impl
5050
{
5151
addAll(SubscribedMethodHelper.NETWORK_BOUND_METHODS);
5252
add("connect");
53-
add("forceConnect");
5453
add("notifyNodeListChanged");
5554
}
5655
});
@@ -83,12 +82,6 @@ public Set<String> getSubscribedMethods() {
8382
@Override
8483
public Connection connect(final String driverProtocol, final HostSpec hostSpec, final Properties props,
8584
final boolean isInitialConnection, final JdbcCallable<Connection, SQLException> connectFunc) throws SQLException {
86-
return connectInternal(hostSpec, connectFunc);
87-
}
88-
89-
public Connection connectInternal(
90-
final HostSpec hostSpec, final JdbcCallable<Connection, SQLException> connectFunc)
91-
throws SQLException {
9285

9386
final Connection conn = connectFunc.call();
9487

@@ -104,12 +97,6 @@ public Connection connectInternal(
10497
return conn;
10598
}
10699

107-
@Override
108-
public Connection forceConnect(String driverProtocol, HostSpec hostSpec, Properties props,
109-
boolean isInitialConnection, JdbcCallable<Connection, SQLException> forceConnectFunc) throws SQLException {
110-
return connectInternal(hostSpec, forceConnectFunc);
111-
}
112-
113100
@Override
114101
public <T, E extends Exception> T execute(final Class<T> resultClass, final Class<E> exceptionClass,
115102
final Object methodInvokeOn, final String methodName, final JdbcCallable<T, E> jdbcMethodFunc,

wrapper/src/main/java/software/amazon/jdbc/plugin/AuroraInitialConnectionStrategyPlugin.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,6 @@ public Connection connect(
108108
final JdbcCallable<Connection, SQLException> connectFunc)
109109
throws SQLException {
110110

111-
return this.connectInternal(hostSpec, props, isInitialConnection, connectFunc);
112-
}
113-
114-
private Connection connectInternal(
115-
final HostSpec hostSpec,
116-
final Properties props,
117-
final boolean isInitialConnection,
118-
final JdbcCallable<Connection, SQLException> connectFunc)
119-
throws SQLException {
120-
121111
final RdsUrlType type = this.rdsUtils.identifyRdsType(hostSpec.getHost());
122112

123113
if (!type.isRdsCluster()) {

wrapper/src/main/java/software/amazon/jdbc/plugin/AwsSecretsManagerConnectionPlugin.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@
5656
import software.amazon.jdbc.util.telemetry.TelemetryFactory;
5757
import software.amazon.jdbc.util.telemetry.TelemetryTraceLevel;
5858

59-
public class AwsSecretsManagerConnectionPlugin extends AbstractConnectionPlugin implements
60-
AuthenticationConnectionPlugin {
59+
public class AwsSecretsManagerConnectionPlugin extends AbstractConnectionPlugin
60+
implements AuthenticationConnectionPlugin {
6161
private static final Logger LOGGER = Logger.getLogger(AwsSecretsManagerConnectionPlugin.class.getName());
6262
private static final String TELEMETRY_UPDATE_SECRETS = "fetch credentials";
6363
private static final String TELEMETRY_FETCH_CREDENTIALS_COUNTER = "secretsManager.fetchCredentials.count";

wrapper/src/main/java/software/amazon/jdbc/plugin/ConnectTimeConnectionPlugin.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,14 @@
2424
import java.util.Properties;
2525
import java.util.Set;
2626
import java.util.logging.Logger;
27+
import software.amazon.jdbc.AuthenticationConnectionPlugin;
2728
import software.amazon.jdbc.HostSpec;
2829
import software.amazon.jdbc.JdbcCallable;
2930
import software.amazon.jdbc.util.Messages;
3031

31-
public class ConnectTimeConnectionPlugin extends AbstractConnectionPlugin {
32+
public class ConnectTimeConnectionPlugin extends AbstractConnectionPlugin
33+
implements AuthenticationConnectionPlugin {
34+
3235
private static final Set<String> subscribedMethods =
3336
Collections.unmodifiableSet(new HashSet<>(Arrays.asList("connect", "forceConnect")));
3437
private static long connectTime = 0L;

wrapper/src/main/java/software/amazon/jdbc/plugin/DefaultConnectionPlugin.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
* This connection plugin will always be the last plugin in the connection plugin chain, and will
5656
* invoke the JDBC method passed down the chain.
5757
*/
58-
public final class DefaultConnectionPlugin implements ConnectionPlugin, AuthenticationConnectionPlugin {
58+
public final class DefaultConnectionPlugin implements ConnectionPlugin {
5959

6060
private static final Logger LOGGER = Logger.getLogger(DefaultConnectionPlugin.class.getName());
6161
private static final Set<String> subscribedMethods = Collections.unmodifiableSet(new HashSet<>(

wrapper/src/main/java/software/amazon/jdbc/plugin/dev/DeveloperConnectionPlugin.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,15 @@
2424
import java.util.Set;
2525
import java.util.logging.Logger;
2626
import org.checkerframework.checker.nullness.qual.NonNull;
27+
import software.amazon.jdbc.AuthenticationConnectionPlugin;
2728
import software.amazon.jdbc.HostSpec;
2829
import software.amazon.jdbc.JdbcCallable;
2930
import software.amazon.jdbc.plugin.AbstractConnectionPlugin;
3031
import software.amazon.jdbc.util.StringUtils;
3132
import software.amazon.jdbc.util.WrapperUtils;
3233

33-
public class DeveloperConnectionPlugin extends AbstractConnectionPlugin implements ExceptionSimulator {
34+
public class DeveloperConnectionPlugin extends AbstractConnectionPlugin
35+
implements AuthenticationConnectionPlugin, ExceptionSimulator {
3436

3537
private static final Logger LOGGER =
3638
Logger.getLogger(DeveloperConnectionPlugin.class.getName());

wrapper/src/main/java/software/amazon/jdbc/plugin/efm/HostMonitoringConnectionPlugin.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -276,11 +276,7 @@ public Connection connect(
276276
final boolean isInitialConnection,
277277
final @NonNull JdbcCallable<Connection, SQLException> connectFunc)
278278
throws SQLException {
279-
return connectInternal(driverProtocol, hostSpec, connectFunc);
280-
}
281279

282-
private Connection connectInternal(String driverProtocol, HostSpec hostSpec,
283-
JdbcCallable<Connection, SQLException> connectFunc) throws SQLException {
284280
final Connection conn = connectFunc.call();
285281

286282
if (conn != null) {
@@ -294,17 +290,6 @@ private Connection connectInternal(String driverProtocol, HostSpec hostSpec,
294290
return conn;
295291
}
296292

297-
@Override
298-
public Connection forceConnect(
299-
final @NonNull String driverProtocol,
300-
final @NonNull HostSpec hostSpec,
301-
final @NonNull Properties props,
302-
final boolean isInitialConnection,
303-
final @NonNull JdbcCallable<Connection, SQLException> forceConnectFunc)
304-
throws SQLException {
305-
return connectInternal(driverProtocol, hostSpec, forceConnectFunc);
306-
}
307-
308293
public HostSpec getMonitoringHostSpec() {
309294
if (this.monitoringHostSpec == null) {
310295
this.monitoringHostSpec = this.pluginService.getCurrentHostSpec();

wrapper/src/main/java/software/amazon/jdbc/plugin/efm2/HostMonitoringConnectionPlugin.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,7 @@ public Connection connect(
226226
final boolean isInitialConnection,
227227
final @NonNull JdbcCallable<Connection, SQLException> connectFunc)
228228
throws SQLException {
229-
return connectInternal(driverProtocol, hostSpec, connectFunc);
230-
}
231229

232-
private Connection connectInternal(String driverProtocol, HostSpec hostSpec,
233-
JdbcCallable<Connection, SQLException> connectFunc) throws SQLException {
234230
final Connection conn = connectFunc.call();
235231

236232
if (conn != null) {
@@ -244,17 +240,6 @@ private Connection connectInternal(String driverProtocol, HostSpec hostSpec,
244240
return conn;
245241
}
246242

247-
@Override
248-
public Connection forceConnect(
249-
final @NonNull String driverProtocol,
250-
final @NonNull HostSpec hostSpec,
251-
final @NonNull Properties props,
252-
final boolean isInitialConnection,
253-
final @NonNull JdbcCallable<Connection, SQLException> forceConnectFunc)
254-
throws SQLException {
255-
return connectInternal(driverProtocol, hostSpec, forceConnectFunc);
256-
}
257-
258243
public HostSpec getMonitoringHostSpec() {
259244
if (this.monitoringHostSpec == null) {
260245
this.monitoringHostSpec = this.pluginService.getCurrentHostSpec();

wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ public class FailoverConnectionPlugin extends AbstractConnectionPlugin {
7373
add("Connection.close");
7474
add("initHostProvider");
7575
add("connect");
76-
add("forceConnect");
7776
add("notifyConnectionChanged");
7877
add("notifyNodeListChanged");
7978
}
@@ -792,12 +791,7 @@ public Connection connect(
792791
final boolean isInitialConnection,
793792
final JdbcCallable<Connection, SQLException> connectFunc)
794793
throws SQLException {
795-
return connectInternal(driverProtocol, hostSpec, props, isInitialConnection, connectFunc, false);
796-
}
797794

798-
private Connection connectInternal(String driverProtocol, HostSpec hostSpec, Properties props,
799-
boolean isInitialConnection, JdbcCallable<Connection, SQLException> connectFunc, boolean isForceConnect)
800-
throws SQLException {
801795
this.initFailoverMode();
802796
if (this.readerFailoverHandler == null) {
803797
if (this.readerFailoverHandlerSupplier == null) {
@@ -819,7 +813,7 @@ private Connection connectInternal(String driverProtocol, HostSpec hostSpec, Pro
819813
this.staleDnsHelper.getVerifiedConnection(isInitialConnection, this.hostListProviderService,
820814
driverProtocol, hostSpec, props, connectFunc);
821815
} catch (final SQLException e) {
822-
if (!this.enableConnectFailover || isForceConnect || !shouldExceptionTriggerConnectionSwitch(e)) {
816+
if (!this.enableConnectFailover || !shouldExceptionTriggerConnectionSwitch(e)) {
823817
throw e;
824818
}
825819

@@ -842,17 +836,6 @@ private Connection connectInternal(String driverProtocol, HostSpec hostSpec, Pro
842836
return conn;
843837
}
844838

845-
@Override
846-
public Connection forceConnect(
847-
final String driverProtocol,
848-
final HostSpec hostSpec,
849-
final Properties props,
850-
final boolean isInitialConnection,
851-
final JdbcCallable<Connection, SQLException> forceConnectFunc)
852-
throws SQLException {
853-
return connectInternal(driverProtocol, hostSpec, props, isInitialConnection, forceConnectFunc, true);
854-
}
855-
856839
// The below methods are for testing purposes
857840
void setRdsUrlType(final RdsUrlType rdsUrlType) {
858841
this.rdsUrlType = rdsUrlType;

0 commit comments

Comments
 (0)