Skip to content

Commit

Permalink
fix unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@stepansergeevitch
stepansergeevitch committed Sep 2, 2024
1 parent 801f805 commit 0030da0
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 92 deletions.
6 changes: 0 additions & 6 deletions src/integrationTest/java/integration/EnvironmentVersionCondition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.util.function.Supplier;

public class EnvironmentVersionCondition implements ExecutionCondition {
private static int infraVersion = -1;
private static String databaseVersion;
private static String protocolVersion;
private static String fireboltVersion;
Expand All @@ -27,7 +26,6 @@ public class EnvironmentVersionCondition implements ExecutionCondition {
);

private static Map<Attribute, Supplier<String>> attributeValueGetter = Map.of(
Attribute.infraVersion, () -> Integer.toString(infraVersion),
Attribute.databaseVersion, () -> databaseVersion,
Attribute.protocolVersion, () -> protocolVersion,
Attribute.fireboltVersion, () -> fireboltVersion
Expand All @@ -45,9 +43,6 @@ public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext ext
if (environmentConditions.length == 0) {
return ConditionEvaluationResult.enabled("Test enabled");
}
if (infraVersion < 0) {
retrieveVersionAttributes((IntegrationTest)testCase);
}

boolean enabled = Arrays.stream(environmentConditions)
.map(condition -> condition.comparison().test(attributeValueGetter.get(condition.attribute()).get(), condition.value()))
Expand All @@ -59,7 +54,6 @@ public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext ext

private void retrieveVersionAttributes(IntegrationTest test) {
try (FireboltConnection conn = (FireboltConnection)test.createConnection()) {
infraVersion = conn.getInfraVersion();
databaseVersion = conn.getMetaData().getDatabaseProductVersion();
fireboltVersion = connectionVersions.get(conn.getClass());
protocolVersion = conn.getProtocolVersion();
Expand Down
2 changes: 0 additions & 2 deletions src/integrationTest/java/integration/tests/SystemEngineTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,9 @@ private String getTableDbName(Connection connection, String table) throws SQLExc
void shouldExecuteEngineManagementQueries() throws SQLException {
try (Connection connection = createConnection(getSystemEngineName())) {
try {
boolean attachEngineToDb = ((FireboltConnection)connection).getInfraVersion() < 2;
List<String> queries = Stream.of(
entry(true, format("CREATE DATABASE IF NOT EXISTS \"%s\"", SECOND_DATABASE_NAME)),
entry(true, format("CREATE ENGINE \"%s\"", ENGINE_NAME)),
entry(attachEngineToDb, format("ATTACH ENGINE \"%s\" TO \"%s\";", ENGINE_NAME, SECOND_DATABASE_NAME)),
entry(true, format("ALTER DATABASE \"%s\" SET DESCRIPTION = 'JDBC Integration test'", SECOND_DATABASE_NAME)),
entry(true, format("ALTER ENGINE \"%s\" RENAME TO \"%s\"", ENGINE_NAME, ENGINE_NEW_NAME)),
entry(true, format("START ENGINE \"%s\"", ENGINE_NEW_NAME)))
Expand Down
5 changes: 2 additions & 3 deletions src/integrationTest/java/integration/tests/TimeoutTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
@CustomLog
class TimeoutTest extends IntegrationTest {
private static final int MIN_TIME_SECONDS = 350;
private static final Map<Integer, Long> SERIES_SIZE = Map.of(1, 80000000000L, 2, 180000000000L);
private static final Long SERIES_SIZE = 80000000000L;
private long startTime;

@BeforeEach
Expand Down Expand Up @@ -59,8 +59,7 @@ void shouldExecuteRequestWithoutTimeoutV2() throws SQLException {

private void shouldExecuteRequestWithoutTimeout() throws SQLException {
try (Connection con = createConnection(); Statement stmt = con.createStatement()) {
int infraVersion = ((FireboltConnection)con).getInfraVersion();
stmt.executeQuery(format("SELECT (max(x) - min(x))/count(x) + avg(x) FROM generate_series(1,%d) r(x)", SERIES_SIZE.get(infraVersion)));
stmt.executeQuery(format("SELECT (max(x) - min(x))/count(x) + avg(x) FROM generate_series(1,%d) r(x)", SERIES_SIZE));
}
}
}
4 changes: 0 additions & 4 deletions src/main/java/com/firebolt/jdbc/client/query/StatementClientImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,6 @@ private Map<String, String> getAllParameters(FireboltProperties fireboltProperti
// In this case it will be in additionalProperties anyway.
params.put(FireboltQueryParameterKey.ACCOUNT_ID.getKey(), accountId);
} else {
if (accountId != null) {
params.put(FireboltQueryParameterKey.ACCOUNT_ID.getKey(), accountId);
params.put(FireboltQueryParameterKey.ENGINE.getKey(), fireboltProperties.getEngine());
}
params.put(FireboltQueryParameterKey.QUERY_LABEL.getKey(), statementInfoWrapper.getLabel()); //QUERY_LABEL
params.put(FireboltQueryParameterKey.COMPRESS.getKey(), fireboltProperties.isCompress() ? "1" : "0");

Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/firebolt/jdbc/statement/rawstatement/SetValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
import static java.util.stream.Collectors.toMap;

public class SetValidator implements StatementValidator {
private static final Map<String, String> forbiddenParameters1 = caseInsensitiveNameSet(DATABASE, ENGINE, ACCOUNT_ID, OUTPUT_FORMAT);
private static final Map<String, String> forbiddenParameters2 = caseInsensitiveNameSet(DATABASE, ENGINE, OUTPUT_FORMAT);
private static final Map<String, String> useSupporting = caseInsensitiveNameSet(DATABASE, ENGINE);
private static final String FORBIDDEN_PROPERTY_ERROR_PREFIX = "Could not set parameter. Set parameter '%s' is not allowed. ";
private static final String FORBIDDEN_PROPERTY_ERROR_USE_SUFFIX = "Try again with 'USE %s' instead of SET.";
Expand All @@ -27,7 +25,7 @@ public class SetValidator implements StatementValidator {
private final Map<String, String> forbiddenParameters;

public SetValidator(FireboltConnection connection) {
forbiddenParameters = connection.getInfraVersion() < 2 ? forbiddenParameters1 : forbiddenParameters2;
forbiddenParameters = caseInsensitiveNameSet(DATABASE, ENGINE, OUTPUT_FORMAT);
}

@Override
Expand Down
31 changes: 7 additions & 24 deletions src/test/java/com/firebolt/jdbc/client/gateway/FireboltGatewayUrlClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,21 +80,6 @@ void shouldGetGatewayUrlWhenResponseIsOk() throws SQLException, IOException {
assertEquals(engineUrl, fireboltGatewayUrlClient.retrieve("access_token", "account").getEngineUrl());
}

@Test
void shouldGetAccountId() throws SQLException, IOException {
fireboltAccountIdResolver.cleanup();
FireboltAccount account = new FireboltAccount("12345", "central");
injectMockedResponse(httpClient, HTTP_OK, "{\"id\": \"12345\", \"region\":\"central\", \"infraVersion\":2}");
assertEquals(account, fireboltAccountIdResolver.retrieve("access_token", "account"));
Mockito.verify(httpClient, times(1)).newCall(any());
// Do this again. The response is cached, so the invocation count will remain 1
assertEquals(account, fireboltAccountIdResolver.retrieve("access_token", "account"));
// now clean the cache and call resolve() again. The invocation counter will be incremented.
fireboltAccountIdResolver.cleanup();
assertEquals(account, fireboltAccountIdResolver.retrieve("access_token", "account"));
Mockito.verify(httpClient, times(2)).newCall(any());
}

@Test
void shouldRuntimeExceptionUponRuntimeException() {
when(httpClient.newCall(any())).thenThrow(new IllegalArgumentException("ex"));
Expand All @@ -106,20 +91,19 @@ void shouldThrowFireboltExceptionUponIOException() throws IOException {
Call call = mock(Call.class);
when(httpClient.newCall(any())).thenReturn(call);
when(call.execute()).thenThrow(new IOException("io error"));
assertEquals("Failed to get engineUrl url for account acc: io error", assertThrows(FireboltException.class, () -> fireboltGatewayUrlClient.retrieve("token", "acc")).getMessage());
assertEquals("Failed to get engine url for account acc: io error", assertThrows(FireboltException.class, () -> fireboltGatewayUrlClient.retrieve("token", "acc")).getMessage());
}

@ParameterizedTest(name = "{0}:{1}")
@CsvSource({
"resolve, com.firebolt.jdbc.client.account.FireboltAccount, {}, JSONObject[\"id\"] not found.",
"engineUrl, com.firebolt.jdbc.client.gateway.GatewayUrlResponse, {}, JSONObject[\"engineUrl\"] not found."
"com.firebolt.jdbc.client.gateway.GatewayUrlResponse, {}, JSONObject[\"engineUrl\"] not found."
})
<T> void shouldThrowFireboltExceptionUponWrongJsonFormat(String path, Class<T> clazz, String json, String expectedErrorMessage) throws IOException {
FireboltAccountRetriever<T> fireboltAccountIdResolver = mockAccountRetriever(path, clazz, json);
assertEquals(format("Failed to get %s url for account acc: %s", path, expectedErrorMessage), assertThrows(FireboltException.class, () -> fireboltAccountIdResolver.retrieve("token", "acc")).getMessage());
<T> void shouldThrowFireboltExceptionUponWrongJsonFormat(Class<T> clazz, String json, String expectedErrorMessage) throws IOException {
FireboltAccountRetriever<T> fireboltAccountIdResolver = mockAccountRetriever(clazz, json);
assertEquals(format("Failed to get engine url for account acc: %s", expectedErrorMessage), assertThrows(FireboltException.class, () -> fireboltAccountIdResolver.retrieve("token", "acc")).getMessage());
}

private <T> FireboltAccountRetriever<T> mockAccountRetriever(String path, Class<T> clazz, String json) throws IOException {
private <T> FireboltAccountRetriever<T> mockAccountRetriever(Class<T> clazz, String json) throws IOException {
try (Response response = mock(Response.class)) {
when(response.code()).thenReturn(200);
ResponseBody responseBody = mock(ResponseBody.class);
Expand All @@ -129,7 +113,7 @@ private <T> FireboltAccountRetriever<T> mockAccountRetriever(String path, Class<
Call call = mock();
when(call.execute()).thenReturn(response);
when(okHttpClient.newCall(any())).thenReturn(call);
return new FireboltAccountRetriever<>(okHttpClient, mock(), null, null, "test-firebolt.io", path, clazz);
return new FireboltAccountRetriever<>(okHttpClient, mock(), null, null, "test-firebolt.io", clazz);
}
}

Expand Down Expand Up @@ -164,7 +148,6 @@ private <T> FireboltAccountRetriever<T> mockAccountRetriever(String path, Class<
})
void testFailedAccountDataRetrieving(int statusCode, String errorMessageTemplate) throws IOException {
injectMockedResponse(httpClient, statusCode, null);
assertErrorMessage(fireboltAccountIdResolver, "one", format(errorMessageTemplate, "one", "resolve"));
assertErrorMessage(fireboltGatewayUrlClient, "two", format(errorMessageTemplate, "two", "engineUrl"));
}

Expand Down
60 changes: 22 additions & 38 deletions src/test/java/com/firebolt/jdbc/client/query/StatementClientImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@
import com.firebolt.jdbc.statement.StatementInfoWrapper;
import com.firebolt.jdbc.statement.StatementUtil;
import lombok.NonNull;
import okhttp3.Call;
import okhttp3.Dispatcher;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.ResponseBody;
import okhttp3.*;
import okio.Buffer;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -50,9 +45,7 @@
import static com.firebolt.jdbc.client.query.StatementClientImpl.HEADER_UPDATE_PARAMETER;
import static java.lang.String.format;
import static java.util.Optional.ofNullable;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.argThat;
Expand Down Expand Up @@ -80,7 +73,9 @@ class StatementClientImplTest {
"true,http://firebolt1:555/?database=db1&account_id=12345&output_format=TabSeparatedWithNamesAndTypes"
})
void shouldPostSqlQueryWithExpectedUrl(boolean systemEngine, String expectedUrl) throws SQLException, IOException {
assertEquals(expectedUrl, shouldPostSqlQuery(systemEngine).getValue());
String res = shouldPostSqlQuery(systemEngine).getValue();
res = res.replaceAll("&query_label=[0-9a-f]{8}(-[0-9a-f]{4}){3}-[0-9a-f]{12}", "");
assertEquals(expectedUrl, res);
}

private Entry<String, String> shouldPostSqlQuery(boolean systemEngine) throws SQLException, IOException {
Expand All @@ -106,10 +101,8 @@ private Entry<String, String> shouldPostSqlQuery(boolean systemEngine) throws SQ
return Map.entry(statementInfoWrapper.getLabel(), actualRequest.url().toString());
}

@ParameterizedTest(name = "infra version:{0}")
@ValueSource(ints = {0, 1, 2})
void shouldCancelSqlQuery(int infraVersion) throws SQLException, IOException {
when(connection.getInfraVersion()).thenReturn(infraVersion);
@Test
void shouldCancelSqlQuery() throws SQLException, IOException {
String id = "12345";
StatementClient statementClient = new StatementClientImpl(okHttpClient, connection, "", "");
PreparedStatement ps = mock(PreparedStatement.class);
Expand All @@ -128,10 +121,8 @@ void shouldCancelSqlQuery(int infraVersion) throws SQLException, IOException {
requestArgumentCaptor.getValue().url().uri().toString());
}

@ParameterizedTest(name = "infra version:{0}")
@ValueSource(ints = {0, 1, 2})
void shouldIgnoreIfStatementIsNotFoundInDbWhenCancelSqlQuery(int infraVersion) throws SQLException, IOException {
when(connection.getInfraVersion()).thenReturn(infraVersion);
@Test
void shouldIgnoreIfStatementIsNotFoundInDbWhenCancelSqlQuery() throws SQLException, IOException {
String id = "12345";
StatementClient statementClient = new StatementClientImpl(okHttpClient, connection, "", "");
PreparedStatement ps = mock(PreparedStatement.class);
Expand All @@ -150,10 +141,8 @@ void shouldIgnoreIfStatementIsNotFoundInDbWhenCancelSqlQuery(int infraVersion) t
requestArgumentCaptor.getValue().url().uri().toString());
}

@ParameterizedTest(name = "infra version:{0}")
@ValueSource(ints = {0, 1, 2})
void cannotGetStatementIdWhenCancelling(int infraVersion) throws SQLException {
when(connection.getInfraVersion()).thenReturn(infraVersion);
@Test
void cannotGetStatementIdWhenCancelling() throws SQLException {
String id = "12345";
StatementClient statementClient = new StatementClientImpl(okHttpClient, connection, "", "");
PreparedStatement ps = mock(PreparedStatement.class);
Expand All @@ -168,13 +157,10 @@ void cannotGetStatementIdWhenCancelling(int infraVersion) throws SQLException {

@ParameterizedTest
@CsvSource({
"1,401,The operation is not authorized",
"1,500, Server failed to execute query",
"2,401,The operation is not authorized",
"2,500, Server failed to execute query"
"401,The operation is not authorized",
"500, Server failed to execute query"
})
void shouldFailToCancelSqlQuery(int infraVersion, int httpStatus, String errorMessage) throws SQLException, IOException {
when(connection.getInfraVersion()).thenReturn(infraVersion);
void shouldFailToCancelSqlQuery(int httpStatus, String errorMessage) throws SQLException, IOException {
String id = "12345";
StatementClient statementClient = new StatementClientImpl(okHttpClient, connection, "", "");
PreparedStatement ps = mock(PreparedStatement.class);
Expand Down Expand Up @@ -256,7 +242,7 @@ void useChangeSeveralProperties() throws SQLException, IOException {
props.setProperty("engine", "e1");
props.setProperty("account_id", "a1");
props.setProperty("compress", "false");
try (FireboltConnection connection = use(1, props, "does not matter", Map.of(
try (FireboltConnection connection = use(props, "does not matter", Map.of(
HEADER_UPDATE_PARAMETER, List.of("database=db2", "engine=e2", "account_id=a1", "addition=something else"),
HEADER_UPDATE_ENDPOINT, List.of("http://other.com?foo=bar")))) {
FireboltProperties fbProps = connection.getSessionProperties();
Expand All @@ -270,18 +256,17 @@ void useChangeSeveralProperties() throws SQLException, IOException {
}
}

@ParameterizedTest(name = "infra version:{0}")
@ParameterizedTest
@CsvSource({
"1,https://api.app.firebolt.io/?database=db1&two=second&three=third&compress=1&one=first,https://api.app.firebolt.io/?database=db1&output_format=TabSeparatedWithNamesAndTypes&compress=1",
"2,https://api.app.firebolt.io/?database=db1&account_id=a1&engine=e1&compress=1&one=first&two=second&three=third&query_label=[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12},https://api.app.firebolt.io/?database=db1&account_id=a1&output_format=TabSeparatedWithNamesAndTypes&engine=e1&compress=1&query_label=[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12}"
"https://api.app.firebolt.io/?database=db1&compress=1&one=first&two=second&three=third&query_label=[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12},https://api.app.firebolt.io/?database=db1&output_format=TabSeparatedWithNamesAndTypes&compress=1&query_label=[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12}"
})
void useResetSession(int infraVersion, String useUrl, String select1Url) throws SQLException, IOException {
void useResetSession(String useUrl, String select1Url) throws SQLException, IOException {
Properties props = new Properties();
props.setProperty("database", "db1");
props.setProperty("engine", "e1");
props.setProperty("account_id", "a1");
props.setProperty("one", "first");
try (FireboltConnection connection = use(infraVersion, props, "does not matter", Map.of(HEADER_UPDATE_PARAMETER, List.of("two=second")))) {
try (FireboltConnection connection = use(props, "does not matter", Map.of(HEADER_UPDATE_PARAMETER, List.of("two=second")))) {
assertEquals(Map.of("one", "first", "two", "second"), connection.getSessionProperties().getAdditionalProperties());

// run set statement
Expand Down Expand Up @@ -339,19 +324,18 @@ private FireboltConnection use(String propName, String propValue, String command
Map<String, List<String>> responseHeaders = responseHeadersStr == null ? Map.of() : Map.of(HEADER_UPDATE_PARAMETER, List.of(responseHeadersStr.split("\\s*,\\s*")));
Properties props = new Properties();
props.setProperty(propName, propValue);
try (FireboltConnection connection = use(1, props, command, responseHeaders)) {
try (FireboltConnection connection = use(props, command, responseHeaders)) {
return connection;
}
}

private FireboltConnection use(int mockedInfraVersion, Properties props, String useCommand, Map<String, List<String>> responseHeaders) throws SQLException, IOException {
private FireboltConnection use(Properties props, String useCommand, Map<String, List<String>> responseHeaders) throws SQLException, IOException {
props.setProperty(FireboltSessionProperty.CONNECTION_TIMEOUT_MILLIS.getKey(), "0"); // simplifies mocking
Call useCall = getMockedCallWithResponse(200, "", responseHeaders);
Call select1Call = getMockedCallWithResponse(200, "");
when(okHttpClient.newCall(any())).thenReturn(useCall, select1Call);
FireboltConnection connection = new FireboltConnection("url", props, "0") {
{
this.infraVersion = mockedInfraVersion;
try {
connect();
} catch (SQLException e) {
Expand Down Expand Up @@ -474,6 +458,6 @@ private String getActualRequestString(Request actualRequest) throws IOException
}

private void assertSqlStatement(String expected, String actual) {
assertTrue(actual.matches(expected + "--label:[0-9a-f]{8}(-[0-9a-f]{4}){3}-[0-9a-f]{12}"));
assertTrue(actual.matches(expected));
}
}
Loading

0 comments on commit 0030da0

Please sign in to comment.