refactor: decouple dependencies and clean up initialization logic (#1541)

Author: aaron-congo

benchmarks/src/jmh/java/software/amazon/jdbc/benchmarks/ConnectionPluginManagerBenchmarks.java

@@ -75,7 +75,6 @@
 import software.amazon.jdbc.util.telemetry.TelemetryCounter;
 import software.amazon.jdbc.util.telemetry.TelemetryFactory;
 import software.amazon.jdbc.util.telemetry.TelemetryGauge;
-import software.amazon.jdbc.wrapper.ConnectionWrapper;
 
 @State(Scope.Benchmark)
 @Fork(3)
@@ -94,7 +93,6 @@ public class ConnectionPluginManagerBenchmarks {
   private ConnectionPluginManager pluginManagerWithNoPlugins;
 
   @Mock ConnectionProvider mockConnectionProvider;
-  @Mock ConnectionWrapper mockConnectionWrapper;
   @Mock FullServicesContainer mockServicesContainer;
   @Mock PluginService mockPluginService;
   @Mock PluginManagerService mockPluginManagerService;
@@ -162,15 +160,12 @@ public void setUpIteration() throws Exception {
 
     TelemetryFactory telemetryFactory = new DefaultTelemetryFactory(propertiesWithPlugins);
 
-    pluginManager = new ConnectionPluginManager(mockConnectionProvider,
-        null,
-        mockConnectionWrapper,
-        telemetryFactory);
-    pluginManager.init(mockServicesContainer, propertiesWithPlugins, mockPluginManagerService, configurationProfile);
+    pluginManager = new ConnectionPluginManager(propertiesWithPlugins, telemetryFactory, mockConnectionProvider, null);
+    pluginManager.initPlugins(mockServicesContainer, configurationProfile);
 
-    pluginManagerWithNoPlugins = new ConnectionPluginManager(mockConnectionProvider, null,
-        mockConnectionWrapper, telemetryFactory);
-    pluginManagerWithNoPlugins.init(mockServicesContainer, propertiesWithoutPlugins, mockPluginManagerService, null);
+    pluginManagerWithNoPlugins =
+        new ConnectionPluginManager(propertiesWithoutPlugins, telemetryFactory, mockConnectionProvider, null);
+    pluginManagerWithNoPlugins.initPlugins(mockServicesContainer, null);
   }
 
   @TearDown(Level.Iteration)
@@ -180,17 +175,17 @@ public void tearDownIteration() throws Exception {
 
   @Benchmark
   public ConnectionPluginManager initConnectionPluginManagerWithNoPlugins() throws SQLException {
-    final ConnectionPluginManager manager = new ConnectionPluginManager(mockConnectionProvider, null,
-        mockConnectionWrapper, mockTelemetryFactory);
-    manager.init(mockServicesContainer, propertiesWithoutPlugins, mockPluginManagerService, configurationProfile);
+    final ConnectionPluginManager manager =
+        new ConnectionPluginManager(propertiesWithoutPlugins, mockTelemetryFactory, mockConnectionProvider, null);
+    manager.initPlugins(mockServicesContainer, configurationProfile);
     return manager;
   }
 
   @Benchmark
   public ConnectionPluginManager initConnectionPluginManagerWithPlugins() throws SQLException {
-    final ConnectionPluginManager manager = new ConnectionPluginManager(mockConnectionProvider, null,
-        mockConnectionWrapper, mockTelemetryFactory);
-    manager.init(mockServicesContainer, propertiesWithPlugins, mockPluginManagerService, configurationProfile);
+    final ConnectionPluginManager manager =
+        new ConnectionPluginManager(propertiesWithPlugins, mockTelemetryFactory, mockConnectionProvider, null);
+    manager.initPlugins(mockServicesContainer, configurationProfile);
     return manager;
   }
 

benchmarks/src/jmh/java/software/amazon/jdbc/benchmarks/PluginBenchmarks.java

@@ -63,8 +63,6 @@
 import software.amazon.jdbc.dialect.Dialect;
 import software.amazon.jdbc.hostavailability.SimpleHostAvailabilityStrategy;
 import software.amazon.jdbc.targetdriverdialect.TargetDriverDialect;
-import software.amazon.jdbc.util.monitoring.MonitorService;
-import software.amazon.jdbc.util.storage.StorageService;
 import software.amazon.jdbc.util.telemetry.GaugeCallable;
 import software.amazon.jdbc.util.telemetry.TelemetryContext;
 import software.amazon.jdbc.util.telemetry.TelemetryCounter;
@@ -84,17 +82,15 @@ public class PluginBenchmarks {
   private static final String FIELD_SERVER_ID = "SERVER_ID";
   private static final String FIELD_SESSION_ID = "SESSION_ID";
   private static final String CONNECTION_STRING = "jdbc:postgresql://my.domain.com";
+  private static final String PG_PROTOCOL = "jdbc:postgresql://";
   private static final String PG_CONNECTION_STRING =
       "jdbc:aws-wrapper:postgresql://instance-0.XYZ.us-east-2.rds.amazonaws.com";
   private static final String TEST_HOST = "instance-0";
   private static final int TEST_PORT = 5432;
   private final HostSpec writerHostSpec = new HostSpecBuilder(new SimpleHostAvailabilityStrategy())
       .host(TEST_HOST).port(TEST_PORT).build();
 
-  @Mock private StorageService mockStorageService;
-  @Mock private MonitorService mockMonitorService;
   @Mock private PluginService mockPluginService;
-  @Mock private TargetDriverDialect mockTargetDriverDialect;
   @Mock private Dialect mockDialect;
   @Mock private ConnectionPluginManager mockConnectionPluginManager;
   @Mock private TelemetryFactory mockTelemetryFactory;
@@ -173,15 +169,11 @@ private ConnectionWrapper getConnectionWrapper(Properties props, String connStri
     return new TestConnectionWrapper(
         props,
         connString,
-        mockConnectionProvider,
-        mockTargetDriverDialect,
+        PG_PROTOCOL,
         mockConnectionPluginManager,
-        mockTelemetryFactory,
         mockPluginService,
         mockHostListProviderService,
-        mockPluginManagerService,
-        mockStorageService,
-        mockMonitorService);
+        mockPluginManagerService);
   }
 
   @Benchmark

benchmarks/src/jmh/java/software/amazon/jdbc/benchmarks/testplugin/TestConnectionWrapper.java

@@ -20,43 +20,29 @@
 import java.util.Properties;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import software.amazon.jdbc.ConnectionPluginManager;
-import software.amazon.jdbc.ConnectionProvider;
 import software.amazon.jdbc.HostListProviderService;
 import software.amazon.jdbc.PluginManagerService;
 import software.amazon.jdbc.PluginService;
-import software.amazon.jdbc.targetdriverdialect.TargetDriverDialect;
-import software.amazon.jdbc.util.monitoring.MonitorService;
-import software.amazon.jdbc.util.storage.StorageService;
-import software.amazon.jdbc.util.telemetry.TelemetryFactory;
 import software.amazon.jdbc.wrapper.ConnectionWrapper;
 
 // Test class allowing for mocks to be used with ConnectionWrapper logic
 public class TestConnectionWrapper extends ConnectionWrapper {
-
   public TestConnectionWrapper(
       @NonNull final Properties props,
       @NonNull final String url,
-      @NonNull final ConnectionProvider defaultConnectionProvider,
-      @NonNull final TargetDriverDialect driverDialect,
+      @NonNull final String protocol,
       @NonNull final ConnectionPluginManager connectionPluginManager,
-      @NonNull final TelemetryFactory telemetryFactory,
       @NonNull final PluginService pluginService,
       @NonNull final HostListProviderService hostListProviderService,
-      @NonNull final PluginManagerService pluginManagerService,
-      @NonNull final StorageService storageService,
-      @NonNull final MonitorService monitorService)
+      @NonNull final PluginManagerService pluginManagerService)
       throws SQLException {
     super(
         props,
         url,
-        defaultConnectionProvider,
-        driverDialect,
+        protocol,
         connectionPluginManager,
-        telemetryFactory,
         pluginService,
         hostListProviderService,
-        pluginManagerService,
-        storageService,
-        monitorService);
+        pluginManagerService);
   }
 }

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

@@ -146,7 +146,6 @@ public List<ConnectionPlugin> getPlugins(
       final FullServicesContainer servicesContainer,
       final ConnectionProvider defaultConnProvider,
       final ConnectionProvider effectiveConnProvider,
-      final PluginManagerService pluginManagerService,
       final Properties props,
       @Nullable ConfigurationProfile configurationProfile) throws SQLException {
 
@@ -234,7 +233,7 @@ public List<ConnectionPlugin> getPlugins(
         servicesContainer.getPluginService(),
         defaultConnProvider,
         effectiveConnProvider,
-        pluginManagerService);
+        servicesContainer.getPluginManagerService());
 
     plugins.add(defaultPlugin);
 

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

@@ -51,11 +51,9 @@
 import software.amazon.jdbc.util.FullServicesContainer;
 import software.amazon.jdbc.util.Messages;
 import software.amazon.jdbc.util.Utils;
-import software.amazon.jdbc.util.WrapperUtils;
 import software.amazon.jdbc.util.telemetry.TelemetryContext;
 import software.amazon.jdbc.util.telemetry.TelemetryFactory;
 import software.amazon.jdbc.util.telemetry.TelemetryTraceLevel;
-import software.amazon.jdbc.wrapper.ConnectionWrapper;
 
 /**
  * This class creates and handles a chain of {@link ConnectionPlugin} for each connection.
@@ -92,29 +90,25 @@ public class ConnectionPluginManager implements CanReleaseResources, Wrapper {
         }
       };
 
-  private final ReentrantLock lock = new ReentrantLock();
-
-  protected Properties props = new Properties();
-  protected List<ConnectionPlugin> plugins;
-  protected final @NonNull ConnectionProvider defaultConnProvider;
-  protected final @Nullable ConnectionProvider effectiveConnProvider;
-  protected final ConnectionWrapper connectionWrapper;
-  protected FullServicesContainer servicesContainer;
-  protected PluginService pluginService;
-  protected TelemetryFactory telemetryFactory;
-  protected boolean isTelemetryInUse;
   @SuppressWarnings("rawtypes")
   protected final PluginChainJdbcCallableInfo[] pluginChainFuncMap =
       new PluginChainJdbcCallableInfo[JdbcMethod.ALL.id + 1]; // it should be the last element in JdbcMethod enum
+  protected final ReentrantLock lock = new ReentrantLock();
+  protected final Properties props;
+  protected final TelemetryFactory telemetryFactory;
+  protected final boolean isTelemetryInUse;
+  protected final ConnectionProvider defaultConnProvider;
+  protected final @Nullable ConnectionProvider effectiveConnProvider;
+  protected List<ConnectionPlugin> plugins;
 
   public ConnectionPluginManager(
+      final @NonNull Properties props,
+      final @NonNull TelemetryFactory telemetryFactory,
       final @NonNull ConnectionProvider defaultConnProvider,
-      final @Nullable ConnectionProvider effectiveConnProvider,
-      final @Nullable ConnectionWrapper connectionWrapper,
-      final @NonNull TelemetryFactory telemetryFactory) {
+      final @Nullable ConnectionProvider effectiveConnProvider) {
+    this.props = props;
     this.defaultConnProvider = defaultConnProvider;
     this.effectiveConnProvider = effectiveConnProvider;
-    this.connectionWrapper = connectionWrapper;
     this.telemetryFactory = telemetryFactory;
     this.isTelemetryInUse = telemetryFactory.inUse();
   }
@@ -127,28 +121,11 @@ public ConnectionPluginManager(
       final @Nullable ConnectionProvider effectiveConnProvider,
       final Properties props,
       final List<ConnectionPlugin> plugins,
-      final ConnectionWrapper connectionWrapper,
-      final PluginService pluginService,
-      final TelemetryFactory telemetryFactory) {
-    this(defaultConnProvider, effectiveConnProvider, props, plugins, connectionWrapper, telemetryFactory);
-    this.pluginService = pluginService;
-  }
-
-  /**
-   * This constructor is for testing purposes only.
-   */
-  ConnectionPluginManager(
-      final @NonNull ConnectionProvider defaultConnProvider,
-      final @Nullable ConnectionProvider effectiveConnProvider,
-      final Properties props,
-      final List<ConnectionPlugin> plugins,
-      final ConnectionWrapper connectionWrapper,
       final TelemetryFactory telemetryFactory) {
     this.defaultConnProvider = defaultConnProvider;
     this.effectiveConnProvider = effectiveConnProvider;
     this.props = props;
     this.plugins = plugins;
-    this.connectionWrapper = connectionWrapper;
     this.telemetryFactory = telemetryFactory;
     this.isTelemetryInUse = telemetryFactory.inUse();
   }
@@ -170,30 +147,18 @@ public void unlock() {
    * connection plugin in the chain.
    *
    * @param servicesContainer     the service container for the services required by this class.
-   * @param props                the configuration of the connection
-   * @param pluginManagerService a reference to a plugin manager service
    * @param configurationProfile a profile configuration defined by the user
    * @throws SQLException if errors occurred during the execution
    */
-  public void init(
+  public void initPlugins(
       final FullServicesContainer servicesContainer,
-      final Properties props,
-      final PluginManagerService pluginManagerService,
       @Nullable ConfigurationProfile configurationProfile) throws SQLException {
-
-    this.props = props;
-    this.servicesContainer = servicesContainer;
-    this.pluginService = servicesContainer.getPluginService();
-    this.telemetryFactory = servicesContainer.getTelemetryFactory();
-    this.isTelemetryInUse = telemetryFactory.inUse();
-
     ConnectionPluginChainBuilder pluginChainBuilder = new ConnectionPluginChainBuilder();
     this.plugins = pluginChainBuilder.getPlugins(
-        this.servicesContainer,
+        servicesContainer,
         this.defaultConnProvider,
         this.effectiveConnProvider,
-        pluginManagerService,
-        props,
+        this.props,
         configurationProfile);
   }
 
@@ -223,14 +188,8 @@ protected <T, E extends Exception> T executeWithSubscribedPlugins(
       this.pluginChainFuncMap[jdbcMethod.id] = pluginChainJdbcCallableInfo;
     }
 
-
-    if (pluginChainJdbcCallableInfo == null) {
-      throw new RuntimeException("Error processing this JDBC call.");
-    }
-
     if (jdbcMethod.alwaysUsePipeline || pluginChainJdbcCallableInfo.isSubscribed) {
       // noinspection unchecked
-      @SuppressWarnings("unchecked")
       PluginChainJdbcCallable<T, E> pluginChainFunc = pluginChainJdbcCallableInfo.func;
       return pluginChainFunc.call(pluginPipeline, jdbcMethodFunc, pluginToSkip);
     } else {
@@ -320,10 +279,6 @@ protected <E extends Exception> void notifySubscribedPlugins(
     }
   }
 
-  public ConnectionWrapper getConnectionWrapper() {
-    return this.connectionWrapper;
-  }
-
   public TelemetryFactory getTelemetryFactory() {
     return this.telemetryFactory;
   }
@@ -345,18 +300,6 @@ public <T, E extends Exception> T execute(
       final JdbcCallable<T, E> jdbcMethodFunc,
       final Object[] jdbcMethodArgs)
       throws E {
-
-    // The target driver may block on Statement.getConnection().
-    if (jdbcMethod.shouldLockConnection && jdbcMethod.checkBoundedConnection) {
-      final Connection conn = WrapperUtils.getConnectionFromSqlObject(methodInvokeOn);
-      if (conn != null && conn != this.pluginService.getCurrentConnection()) {
-        throw WrapperUtils.wrapExceptionIfNeeded(
-            exceptionClass,
-            new SQLException(
-                Messages.get("ConnectionPluginManager.invokedAgainstOldConnection", new Object[]{methodInvokeOn})));
-      }
-    }
-
     return executeWithSubscribedPlugins(
         jdbcMethod,
         (plugin, func) ->
@@ -635,9 +578,7 @@ public <T> T unwrap(Class<T> iface) throws SQLException {
     if (iface == ConnectionPluginManager.class) {
       return iface.cast(this);
     }
-    if (iface == PluginService.class) {
-      return iface.cast(this.pluginService);
-    }
+
     if (this.plugins == null) {
       return null;
     }
@@ -647,6 +588,7 @@ public <T> T unwrap(Class<T> iface) throws SQLException {
         return iface.cast(p);
       }
     }
+
     return null;
   }
 

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

@@ -59,10 +59,10 @@
 import software.amazon.jdbc.util.CoreServicesContainer;
 import software.amazon.jdbc.util.DriverInfo;
 import software.amazon.jdbc.util.FullServicesContainer;
-import software.amazon.jdbc.util.FullServicesContainerImpl;
 import software.amazon.jdbc.util.Messages;
 import software.amazon.jdbc.util.PropertyUtils;
 import software.amazon.jdbc.util.RdsUtils;
+import software.amazon.jdbc.util.ServiceUtility;
 import software.amazon.jdbc.util.StringUtils;
 import software.amazon.jdbc.util.WrapperUtils;
 import software.amazon.jdbc.util.monitoring.MonitorService;
@@ -110,6 +110,7 @@ public class Driver implements java.sql.Driver {
 
   private final StorageService storageService;
   private final MonitorService monitorService;
+  private final ConnectionUrlParser urlParser = new ConnectionUrlParser();
 
   public Driver() {
     this(CoreServicesContainer.getInstance());
@@ -239,18 +240,20 @@ public Connection connect(final String url, final Properties info) throws SQLExc
         effectiveConnectionProvider = configurationProfile.getConnectionProvider();
       }
 
-      FullServicesContainer servicesContainer =
-          new FullServicesContainerImpl(storageService, monitorService, defaultConnectionProvider, telemetryFactory);
-
-      return new ConnectionWrapper(
-          servicesContainer,
-          props,
-          driverUrl,
+      String targetDriverProtocol = urlParser.getProtocol(driverUrl);
+      FullServicesContainer servicesContainer = ServiceUtility.getInstance().createStandardServiceContainer(
+          storageService,
+          monitorService,
           defaultConnectionProvider,
           effectiveConnectionProvider,
+          telemetryFactory,
+          driverUrl,
+          targetDriverProtocol,
           targetDriverDialect,
+          props,
           configurationProfile);
 
+      return new ConnectionWrapper(servicesContainer, props, url, targetDriverProtocol, configurationProfile);
     } catch (Exception ex) {
       if (context != null) {
         context.setException(ex);