Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add new integration tests to ensure topology query doesn't change #910

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
a977881
add base test
jasonli-improving Feb 21, 2024
afe5167
add topology tests and change access to private topology queries
jasonli-improving Feb 27, 2024
694c36b
update expected types in rds test
jasonli-improving Feb 27, 2024
65e3e50
disable unsupported test
jasonli-improving Feb 27, 2024
e7f5ea6
remove change from gradle
jasonli-improving Feb 27, 2024
bea1635
remove docker from tests
jasonli-improving Feb 28, 2024
ee23e30
fix timestamp test
jasonli-improving Feb 28, 2024
82d0685
try renaming
jasonli-improving Feb 28, 2024
32678e0
Revert "try renaming"
jasonli-improving Feb 28, 2024
cc7ee06
change scope of tests
jasonli-improving Feb 28, 2024
b356f63
add check for aurora/rds
jasonli-improving Feb 28, 2024
9001d9f
testing again
jasonli-improving Feb 29, 2024
ae00e64
try other than writer instance
jasonli-improving Feb 29, 2024
c728df6
revert: try other than writer instance
jasonli-improving Feb 29, 2024
4025f3a
remove timestamp test and change back test scope
jasonli-improving Feb 29, 2024
66249f6
remove unused imports
jasonli-improving Feb 29, 2024
0830cf8
Revert "remove timestamp test and change back test scope"
jasonli-improving Feb 29, 2024
33cae6c
Revert "remove unused imports"
jasonli-improving Feb 29, 2024
e3cb6f6
enable on min two instances
jasonli-improving Feb 29, 2024
90e5366
add function for updating instance
jasonli-improving Feb 29, 2024
f5e5c69
fix bug with instance id
jasonli-improving Feb 29, 2024
5ee6c57
skipping first row
jasonli-improving Feb 29, 2024
03d71d8
change test scope
jasonli-improving Feb 29, 2024
df4ebd4
add null check
jasonli-improving Feb 29, 2024
03eb786
clean up code
jasonli-improving Mar 1, 2024
90b47d6
change test scope back to normal
jasonli-improving Mar 1, 2024
d617b72
remove import
jasonli-improving Mar 1, 2024
09496af
clarify code
jasonli-improving Mar 1, 2024
9562e11
remove duplicate comment
jasonli-improving Mar 1, 2024
53237a1
add final
jasonli-improving Mar 1, 2024
6f4ded8
rename function
jasonli-improving Mar 1, 2024
c22a97f
update comment
jasonli-improving Mar 1, 2024
65174ef
use getters instead of changing fields to public
jasonli-improving Mar 1, 2024
0edb182
test fails if update certificate throws exception
jasonlamz Mar 6, 2024
83ed69d
missing bracket from merge
jasonlamz Mar 13, 2024
2681ddf
changes from review
jasonlamz Mar 26, 2024
5fb7f9f
reduce line length
jasonlamz Mar 26, 2024
93895c3
testing scope
jasonlamz Mar 26, 2024
8c8e667
Revert "testing scope"
jasonlamz Mar 26, 2024
e094029
test
jasonlamz Mar 26, 2024
89efa7e
test
jasonlamz Mar 26, 2024
adb82b3
updating instance not required for timestamp
jasonlamz Mar 26, 2024
06ba1bd
revert scope
jasonlamz Mar 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,8 @@ public HostListProviderSupplier getHostListProvider() {
NODE_ID_QUERY,
IS_READER_QUERY);
}

public String getTopologyQuery() {
return TOPOLOGY_QUERY;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,8 @@ public HostListProviderSupplier getHostListProvider() {
NODE_ID_QUERY,
IS_READER_QUERY);
}

public String getTopologyQuery() {
return TOPOLOGY_QUERY;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,8 @@ public void prepareConnectProperties(
? connectionAttributes
: connectProperties.getProperty("connectionAttributes") + "," + connectionAttributes);
}

public String getTopologyQuery() {
return TOPOLOGY_QUERY;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,8 @@ public HostListProviderSupplier getHostListProvider() {
FETCH_WRITER_NODE_QUERY,
FETCH_WRITER_NODE_QUERY_COLUMN_NAME);
}

public String getTopologyQuery() {
return TOPOLOGY_QUERY;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ private HostSpec createHost(final ResultSet resultSet) throws SQLException {
// According to the topology query the result set
// should contain 4 columns: node ID, 1/0 (writer/reader), CPU utilization, node lag in time.
String hostName = resultSet.getString(1);
hostName = hostName == null ? "?" : hostName;
final boolean isWriter = resultSet.getBoolean(2);
final float cpuUtilization = resultSet.getFloat(3);
final float nodeLag = resultSet.getFloat(4);
Expand All @@ -439,8 +440,11 @@ private HostSpec createHost(final ResultSet resultSet) throws SQLException {
return createHost(hostName, isWriter, weight, lastUpdateTime);
}

private HostSpec createHost(String host, final boolean isWriter, final long weight, final Timestamp lastUpdateTime) {
host = host == null ? "?" : host;
private HostSpec createHost(
String host,
final boolean isWriter,
final long weight,
final @Nullable Timestamp lastUpdateTime) {
final String endpoint = getHostEndpoint(host);
final int port = this.clusterInstanceTemplate.isPortSpecified()
? this.clusterInstanceTemplate.getPort()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,275 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package integration.container.tests;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import integration.DatabaseEngine;
import integration.DatabaseEngineDeployment;
import integration.DriverHelper;
import integration.TestEnvironmentFeatures;
import integration.container.ConnectionStringHelper;
import integration.container.TestDriver;
import integration.container.TestDriverProvider;
import integration.container.TestEnvironment;
import integration.container.condition.DisableOnTestFeature;
import integration.container.condition.EnableOnDatabaseEngineDeployment;
import integration.container.condition.EnableOnNumOfInstances;
import integration.util.AuroraTestUtility;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
import java.util.Properties;
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.MethodOrderer.MethodName;
import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;
import software.amazon.jdbc.dialect.AuroraMysqlDialect;
import software.amazon.jdbc.dialect.AuroraPgDialect;
import software.amazon.jdbc.dialect.Dialect;
import software.amazon.jdbc.dialect.RdsMultiAzDbClusterMysqlDialect;
import software.amazon.jdbc.dialect.RdsMultiAzDbClusterPgDialect;

@TestMethodOrder(MethodName.class)
@DisableOnTestFeature({
TestEnvironmentFeatures.PERFORMANCE,
TestEnvironmentFeatures.RUN_HIBERNATE_TESTS_ONLY,
TestEnvironmentFeatures.RUN_AUTOSCALING_TESTS_ONLY})
public class TopologyQueryTests {
private static final Logger LOGGER = Logger.getLogger(TopologyQueryTests.class.getName());
private String query = null;

@TestTemplate
@EnableOnDatabaseEngineDeployment(DatabaseEngineDeployment.AURORA)
@ExtendWith(TestDriverProvider.class)
public void testAuroraTypes(TestDriver testDriver) throws SQLException {
LOGGER.info(testDriver.toString());
List<String> expectedTypes;

final Properties props = ConnectionStringHelper.getDefaultPropertiesWithNoPlugins();
DriverHelper.setConnectTimeout(testDriver, props, 10, TimeUnit.SECONDS);
DriverHelper.setSocketTimeout(testDriver, props, 10, TimeUnit.SECONDS);

String url =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add the final keyword to all your constant local variables?
Also you can just call ConnectionStringHelper.getWrapperUrl() here.

ConnectionStringHelper.getWrapperUrl(
testDriver,
TestEnvironment.getCurrent()
.getInfo()
.getDatabaseInfo()
.getInstances()
.get(0)
.getHost(),
TestEnvironment.getCurrent()
.getInfo()
.getDatabaseInfo()
.getInstances()
.get(0)
.getPort(),
TestEnvironment.getCurrent().getInfo().getDatabaseInfo().getDefaultDbName());

if (TestEnvironment.getCurrent().getCurrentDriver() == TestDriver.MARIADB
&& TestEnvironment.getCurrent().getInfo().getRequest().getDatabaseEngine() == DatabaseEngine.MYSQL) {
props.setProperty("permitMysqlScheme", "1");
}
Comment on lines +99 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getWrapperUrl should already be appending permitMysqlScheme to the wrapper url, why is this if-clause required?


LOGGER.finest("Connecting to " + url);

if (TestEnvironment.getCurrent().getCurrentDriver() == TestDriver.PG) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's about MariaDb? Is it supported by the test?

Copy link
Contributor

@jasonlamz jasonlamz Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supported

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean I don't see a branch of the code that handle the case with mariaDb. I see AuroraMysql and AuroraPG.

AuroraPgDialect dialect = new AuroraPgDialect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could move this if-else to the beginning of the test after List<String> expectedTypes;, before establishing a connection, since you don't need any connections here and you are just setting up for the test.

query = dialect.getTopologyQuery();
expectedTypes = Arrays.asList(
"text",
"bool",
"float4",
"float4",
"timestamptz"
);
} else {
AuroraMysqlDialect dialect = new AuroraMysqlDialect();
query = dialect.getTopologyQuery();
expectedTypes = Arrays.asList(
"VARCHAR",
"BIGINT",
"DOUBLE",
"DOUBLE",
"DATETIME"
);
}

final Connection conn = DriverManager.getConnection(url, props);
assertTrue(conn.isValid(5));
Statement stmt = conn.createStatement();
stmt.executeQuery(query);
ResultSet rs = stmt.getResultSet();
int cols = rs.getMetaData().getColumnCount();
List<String> columnTypes = new ArrayList<>();
for (int i = 1; i <= cols; i++) {
columnTypes.add(rs.getMetaData().getColumnTypeName(i));
}
assertEquals(expectedTypes, columnTypes);
conn.close();
}

@TestTemplate
@ExtendWith(TestDriverProvider.class)
@EnableOnDatabaseEngineDeployment(DatabaseEngineDeployment.AURORA)
@EnableOnNumOfInstances(min = 2)
public void testAuroraTimestamp(TestDriver testDriver) throws SQLException, ParseException {
LOGGER.info(testDriver.toString());

final Properties props = ConnectionStringHelper.getDefaultPropertiesWithNoPlugins();
DriverHelper.setConnectTimeout(testDriver, props, 10, TimeUnit.SECONDS);
DriverHelper.setSocketTimeout(testDriver, props, 10, TimeUnit.SECONDS);

// Get second instance since first one has null timestamps
String url =
ConnectionStringHelper.getWrapperUrl(
testDriver,
TestEnvironment.getCurrent()
.getInfo()
.getDatabaseInfo()
.getInstances()
.get(1)
.getHost(),
TestEnvironment.getCurrent()
.getInfo()
.getDatabaseInfo()
.getInstances()
.get(1)
.getPort(),
TestEnvironment.getCurrent().getInfo().getDatabaseInfo().getDefaultDbName());
LOGGER.finest("Connecting to " + url);

if (TestEnvironment.getCurrent().getCurrentDriver() == TestDriver.MARIADB
&& TestEnvironment.getCurrent().getInfo().getRequest().getDatabaseEngine() == DatabaseEngine.MYSQL) {
props.setProperty("permitMysqlScheme", "1");
}

SimpleDateFormat format;
if (TestEnvironment.getCurrent().getCurrentDriver() == TestDriver.PG) {
AuroraPgDialect dialect = new AuroraPgDialect();
query = dialect.getTopologyQuery();
format = new SimpleDateFormat("yyy-MM-dd HH:mm:ssX");
format.setTimeZone(TimeZone.getTimeZone("GMT"));
} else {
AuroraMysqlDialect dialect = new AuroraMysqlDialect();
query = dialect.getTopologyQuery();
format = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSSSSS");
}

final Connection conn = DriverManager.getConnection(url, props);
assertTrue(conn.isValid(5));
Statement stmt = conn.createStatement();
stmt.executeQuery(query);
Comment on lines +189 to +192
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment for the other locations.

ResultSet rs = stmt.getResultSet();

Date date = null;
while (rs.next()) {
if (rs.getString(5) != null) {
date = format.parse(rs.getString(5));
jasonlamz marked this conversation as resolved.
Show resolved Hide resolved
assertNotNull(date);
}
}
assertNotNull(date);

conn.close();
}

@TestTemplate
@ExtendWith(TestDriverProvider.class)
@EnableOnDatabaseEngineDeployment(DatabaseEngineDeployment.RDS)
@Disabled
// TODO: Disabled due to RdsMultiAz integration tests not being supported yet
public void testRdsMultiAzTypes(TestDriver testDriver) throws SQLException {
LOGGER.info(testDriver.toString());

final Properties props = ConnectionStringHelper.getDefaultPropertiesWithNoPlugins();
DriverHelper.setConnectTimeout(testDriver, props, 10, TimeUnit.SECONDS);
DriverHelper.setSocketTimeout(testDriver, props, 10, TimeUnit.SECONDS);

String url =
ConnectionStringHelper.getWrapperUrl(
testDriver,
TestEnvironment.getCurrent()
.getInfo()
.getDatabaseInfo()
.getInstances()
.get(0)
.getHost(),
TestEnvironment.getCurrent()
.getInfo()
.getDatabaseInfo()
.getInstances()
.get(0)
.getPort(),
TestEnvironment.getCurrent().getInfo().getDatabaseInfo().getDefaultDbName());
LOGGER.finest("Connecting to " + url);

if (TestEnvironment.getCurrent().getCurrentDriver() == TestDriver.MARIADB
&& TestEnvironment.getCurrent().getInfo().getRequest().getDatabaseEngine() == DatabaseEngine.MYSQL) {
props.setProperty("permitMysqlScheme", "1");
}

List<String> expectedTypes;
if (TestEnvironment.getCurrent().getCurrentDriver() == TestDriver.PG) {
RdsMultiAzDbClusterPgDialect dialect = new RdsMultiAzDbClusterPgDialect();
query = dialect.getTopologyQuery();
expectedTypes = Arrays.asList(
"text",
"text",
"int4"
);
} else {
RdsMultiAzDbClusterMysqlDialect dialect = new RdsMultiAzDbClusterMysqlDialect();
query = dialect.getTopologyQuery();
expectedTypes = Arrays.asList(
"VARCHAR",
"VARCHAR",
"INT"
);
}

final Connection conn = DriverManager.getConnection(url, props);
assertTrue(conn.isValid(5));
Statement stmt = conn.createStatement();
stmt.executeQuery(query);
ResultSet rs = stmt.getResultSet();
int cols = rs.getMetaData().getColumnCount();
List<String> columnTypes = new ArrayList<>();

for (int i = 1; i <= cols; i++) {
columnTypes.add(rs.getMetaData().getColumnTypeName(i));
}
assertEquals(expectedTypes, columnTypes);
conn.close();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
import java.sql.Statement;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -80,6 +78,7 @@
import software.amazon.awssdk.services.rds.model.DescribeDbInstancesResponse;
import software.amazon.awssdk.services.rds.model.FailoverDbClusterResponse;
import software.amazon.awssdk.services.rds.model.Filter;
import software.amazon.awssdk.services.rds.model.ModifyDbInstanceRequest;
import software.amazon.awssdk.services.rds.model.Tag;
import software.amazon.awssdk.services.rds.waiters.RdsWaiter;
import software.amazon.jdbc.util.StringUtils;
Expand Down
Loading