chore: address review comments

Author: karenc-bq

wrapper/build.gradle.kts

@@ -315,11 +315,6 @@ tasks.register<Test>("test-all-mysql-aurora") {
         systemProperty("test-no-performance", "true")
         systemProperty("test-no-pg-driver", "true")
         systemProperty("test-no-pg-engine", "true")
-        systemProperty("test-no-mariadb-driver", "true")
-        systemProperty("test-no-mariadb-engine", "true")
-        systemProperty("test-no-iam", "true")
-        systemProperty("test-no-hikari", "true")
-        systemProperty("test-no-secrets-manager", "true")
     }
 }
 

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

@@ -152,6 +152,7 @@ public void removeAlias(final String... alias) {
   public void resetAliases() {
     this.aliases.clear();
     this.allAliases.clear();
+    this.allAliases.add(this.asAlias());
   }
 
   public String getUrl() {

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

@@ -503,7 +503,7 @@ public HostSpec identifyConnection(Connection connection) throws SQLException {
 
   @Override
   public void fillAliases(Connection connection, HostSpec hostSpec) throws SQLException {
-    if (!hostSpec.getAliases().isEmpty()) {
+    if (hostSpec == null || !hostSpec.getAliases().isEmpty()) {
       LOGGER.finest(() -> Messages.get("PluginServiceImpl.nonEmptyAliases", new Object[] {hostSpec.getAliases()}));
       return;
     }
@@ -524,8 +524,8 @@ public void fillAliases(Connection connection, HostSpec hostSpec) throws SQLExce
 
     // Add the instance endpoint if the current connection is associated with a topology aware database cluster.
     final HostSpec host = this.identifyConnection(connection);
-    if (host != this.currentHostSpec) {
-      hostSpec.addAlias(host.asAlias());
+    if (host != null) {
+      hostSpec.addAlias(host.asAliases().toArray(new String[] {}));
     }
   }
 }

wrapper/src/main/java/software/amazon/jdbc/hostlistprovider/AuroraHostListProvider.java

@@ -622,9 +622,7 @@ public HostSpec identifyConnection(Connection connection) throws SQLException {
         final List<HostSpec> topology = this.refresh(connection);
 
         if (topology == null) {
-          final HostSpec host = new HostSpec(getHostEndpoint(instanceName));
-          host.setHostId(instanceName);
-          return host;
+          return null;
         }
         return topology
             .stream()

wrapper/src/main/java/software/amazon/jdbc/hostlistprovider/ConnectionStringHostListProvider.java

@@ -17,10 +17,13 @@
 package software.amazon.jdbc.hostlistprovider;
 
 import java.sql.Connection;
+import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.Statement;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.Properties;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import software.amazon.jdbc.AwsWrapperProperty;
@@ -112,6 +115,27 @@ public HostRole getHostRole(Connection connection) {
 
   @Override
   public HostSpec identifyConnection(Connection connection) throws SQLException {
-    return this.hostList.get(0);
+    try (final Statement stmt = connection.createStatement();
+        final ResultSet resultSet = stmt.executeQuery(this.hostListProviderService.getDialect().getHostAliasQuery())) {
+      if (resultSet.next()) {
+        final String instance = resultSet.getString(1);
+
+        final List<HostSpec> topology = this.refresh(connection);
+
+        if (topology == null) {
+          return null;
+        }
+
+        return topology
+            .stream()
+            .filter(host -> Objects.equals(instance, host.getHostId()))
+            .findAny()
+            .orElse(null);
+      }
+    } catch (final SQLException e) {
+      throw new SQLException(Messages.get("ConnectionStringHostListProvider.errorIdentifyConnection"), e);
+    }
+
+    throw new SQLException(Messages.get("ConnectionStringHostListProvider.errorIdentifyConnection"));
   }
 }

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

@@ -179,7 +179,9 @@ public <T, E extends Exception> T execute(
           this.monitorService.stopMonitoring(monitorContext);
 
           if (monitorContext.isNodeUnhealthy()) {
-            this.pluginService.setAvailability(this.getMonitoringHostSpec().asAliases(), HostAvailability.NOT_AVAILABLE);
+            this.pluginService.setAvailability(
+                this.getMonitoringHostSpec().asAliases(),
+                HostAvailability.NOT_AVAILABLE);
 
             final boolean isConnectionClosed;
             try {
@@ -305,11 +307,19 @@ public HostSpec getMonitoringHostSpec() {
           LOGGER.finest("Monitoring HostSpec is associated with a cluster endpoint, "
               + "plugin needs to identify the cluster connection.");
           this.monitoringHostSpec = this.pluginService.identifyConnection(this.pluginService.getCurrentConnection());
+          if (this.monitoringHostSpec == null) {
+            throw new RuntimeException(Messages.get(
+                "HostMonitoringConnectionPlugin.unableToIdentifyConnection",
+                new Object[] {
+                    this.pluginService.getCurrentHostSpec().getHost(),
+                    this.pluginService.getHostListProvider()}));
+          }
           this.pluginService.fillAliases(this.pluginService.getCurrentConnection(), monitoringHostSpec);
         }
       } catch (SQLException e) {
-        // Log and ignore
+        // Log and throw.
         LOGGER.finest(Messages.get("HostMonitoringConnectionPlugin.errorIdentifyingConnection", new Object[] {e}));
+        throw new RuntimeException(e);
       }
     }
     return this.monitoringHostSpec;

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

@@ -90,16 +90,9 @@ public MonitorConnectionContext startMonitoring(
       final int failureDetectionCount) {
 
     if (nodeKeys.isEmpty()) {
-      LOGGER.warning(
-          () -> Messages.get(
-              "MonitorServiceImpl.emptyAliasSet",
-              new Object[] {hostSpec}));
-      try {
-        this.pluginService.fillAliases(connectionToAbort, hostSpec);
-      } catch (SQLException e) {
-        // Log and ignore the error.
-        LOGGER.finest(Messages.get("MonitorServiceImpl.errorPopulatingAliases", new Object[] {e}));
-      }
+      throw new IllegalArgumentException(Messages.get(
+          "MonitorServiceImpl.emptyAliasSet",
+          new Object[] {hostSpec}));
     }
 
     final Monitor monitor;

wrapper/src/main/resources/messages.properties

@@ -79,6 +79,7 @@ ClusterAwareWriterFailoverHandler.alreadyWriter=Current reader connection is act
 
 # Connection String Host List Provider
 ConnectionStringHostListProvider.parsedListEmpty=Can''t parse connection string: ''{0}''.
+ConnectionStringHostListProvider.errorIdentifyConnection=An error occurred while obtaining the connection's host ID.
 
 # Connection Plugin Manager
 ConnectionPluginManager.configurationProfileNotFound=Configuration profile ''{0}'' not found.
@@ -142,6 +143,7 @@ HostMonitoringConnectionPlugin.activatedMonitoring=Executing method ''{0}'', mon
 HostMonitoringConnectionPlugin.monitoringDeactivated=Monitoring deactivated for method ''{0}''.
 HostMonitoringConnectionPlugin.unavailableNode=Node ''{0}'' is unavailable.
 HostMonitoringConnectionPlugin.errorIdentifyingConnection=Error occurred while identifying connection: ''{0}''.
+HostMonitoringConnectionPlugin.unableToIdentifyConnection=Unable to identify the given connection: ''{0}'', please ensure the correct host list provider is specified. The host list provider in use is: ''{1}''.
 
 # IAM Auth Connection Plugin
 IamAuthConnectionPlugin.unsupportedHostname=Unsupported AWS hostname {0}. Amazon domain name in format *.AWS-Region.rds.amazonaws.com or *.rds.AWS-Region.amazonaws.com.cn is expected.

wrapper/src/test/build.gradle.kts

@@ -86,5 +86,5 @@ tasks.register<Test>("in-container") {
 
     // modify below filter to select specific integration tests
     // see https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/testing/TestFilter.html
-    filter.includeTestsMatching("integration.refactored.container.tests.AuroraFailoverTest.testServerFailoverWithIdleConnections")
+    filter.includeTestsMatching("integration.refactored.container.tests.*")
 }

wrapper/src/test/java/software/amazon/jdbc/hostlistprovider/AuroraHostListProviderTest.java

@@ -435,10 +435,7 @@ void testIdentifyConnectionNullTopology() throws SQLException {
     when(mockResultSet.getString(eq(1))).thenReturn("instance-1");
     when(auroraHostListProvider.refresh(eq(mockConnection))).thenReturn(null);
 
-    final HostSpec actual = auroraHostListProvider.identifyConnection(mockConnection);
-
-    assertEquals("instance-1.pattern", actual.getHost());
-    assertEquals("instance-1", actual.getHostId());
+    assertNull(auroraHostListProvider.identifyConnection(mockConnection));
   }
 
   @Test