Skip to content

Commit 4b42d0d

Browse files
Jithendar12Jithendar12
committed
Address Review Comments
1 parent 6ff4887 commit 4b42d0d

File tree

7 files changed

+180
-151
lines changed

7 files changed

+180
-151
lines changed

‎athena-oracle/src/test/java/com/amazonaws/athena/connectors/oracle/OracleEnvironmentPropertiesTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@ public void setUp()
5050
}
5151

5252
@Test
53-
public void testOracleConnectionString_withoutSSL()
53+
public void connectionPropertiesToEnvironment_whenSSLNotEnabled_returnsConnectionStringWithoutSSL()
5454
{
5555
Map<String, String> oracleConnectionProperties = oracleEnvironmentProperties.connectionPropertiesToEnvironment(connectionProperties);
5656
String expectedConnectionString = "oracle://jdbc:oracle:thin:${oracle-secret}@//test.oracle.com:1521/orcl";
5757
assertEquals(expectedConnectionString, oracleConnectionProperties.get(DEFAULT));
5858
}
5959

6060
@Test
61-
public void testOracleConnectionString_withSSL()
61+
public void connectionPropertiesToEnvironment_whenSSLEnabled_returnsConnectionStringWithSSL()
6262
{
6363
connectionProperties.put(ENFORCE_SSL, "true");
6464
Map<String, String> oracleConnectionProperties = oracleEnvironmentProperties.connectionPropertiesToEnvironment(connectionProperties);

‎athena-oracle/src/test/java/com/amazonaws/athena/connectors/oracle/OracleJdbcConnectionFactoryTest.java‎

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -49,24 +49,9 @@ public class OracleJdbcConnectionFactoryTest
4949
private static final String USERNAME = "user";
5050
private static final String PASSWORD = "password";
5151

52-
53-
@Test(expected = RuntimeException.class)
54-
public void testGetConnection() throws SQLException
55-
{
56-
DefaultCredentials expectedCredential = new DefaultCredentials(USERNAME, PASSWORD);
57-
CredentialsProvider credentialsProvider = new StaticCredentialsProvider(expectedCredential);
58-
DatabaseConnectionConfig databaseConnectionConfig = new DatabaseConnectionConfig(TEST_CATALOG, OracleConstants.ORACLE_NAME,
59-
CONNECTION_STRING, TEST_SECRET);
60-
DatabaseConnectionInfo databaseConnectionInfo = new DatabaseConnectionInfo(OracleConstants.ORACLE_DRIVER_CLASS, OracleConstants.ORACLE_DEFAULT_PORT);
61-
Connection connection = new OracleJdbcConnectionFactory(databaseConnectionConfig, databaseConnectionInfo).getConnection(credentialsProvider);
62-
String originalURL = connection.getMetaData().getURL();
63-
Driver drv = DriverManager.getDriver(originalURL);
64-
String driverClass = drv.getClass().getName();
65-
assertEquals("oracle.jdbc.OracleDriver", driverClass);
66-
}
67-
6852
@Test
69-
public void testGetConnection_withSsl() throws Exception {
53+
public void getConnection_whenUsingSslConnection_setsSslProperties() throws Exception
54+
{
7055
Driver mockDriver = mock(Driver.class);
7156

7257
DefaultCredentials expectedCredential = new DefaultCredentials(USERNAME, PASSWORD);
@@ -93,34 +78,38 @@ public void testGetConnection_withSsl() throws Exception {
9378
return mock(Connection.class);
9479
});
9580

96-
DriverManager.registerDriver(mockDriver);
97-
98-
// Create a test connection factory with custom environment
99-
OracleJdbcConnectionFactory connectionFactory = new OracleJdbcConnectionFactory(databaseConnectionConfig, databaseConnectionInfo) {
100-
@Override
101-
protected Map<String, String> getEnvMap() {
102-
Map<String, String> env = new HashMap<>();
103-
env.put(IS_FIPS_ENABLED, "true");
104-
return env;
105-
}
106-
};
107-
108-
connectionFactory.getConnection(credentialsProvider);
109-
110-
Properties sslProps = capturedProps[0];
111-
assertEquals("JKS", sslProps.getProperty("javax.net.ssl.trustStoreType"));
112-
assertEquals("changeit", sslProps.getProperty("javax.net.ssl.trustStorePassword"));
113-
assertEquals("true", sslProps.getProperty("oracle.net.ssl_server_dn_match"));
114-
assertEquals(
115-
"(TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA)",
116-
sslProps.getProperty("oracle.net.ssl_cipher_suites")
117-
);
118-
119-
DriverManager.deregisterDriver(mockDriver);
81+
try {
82+
DriverManager.registerDriver(mockDriver);
83+
84+
// Create a test connection factory with custom environment
85+
OracleJdbcConnectionFactory connectionFactory = new OracleJdbcConnectionFactory(databaseConnectionConfig, databaseConnectionInfo) {
86+
@Override
87+
protected Map<String, String> getEnvMap()
88+
{
89+
Map<String, String> env = new HashMap<>();
90+
env.put(IS_FIPS_ENABLED, "true");
91+
return env;
92+
}
93+
};
94+
95+
connectionFactory.getConnection(credentialsProvider);
96+
97+
Properties sslProps = capturedProps[0];
98+
assertEquals("JKS", sslProps.getProperty("javax.net.ssl.trustStoreType"));
99+
assertEquals("changeit", sslProps.getProperty("javax.net.ssl.trustStorePassword"));
100+
assertEquals("true", sslProps.getProperty("oracle.net.ssl_server_dn_match"));
101+
assertEquals(
102+
"(TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA)",
103+
sslProps.getProperty("oracle.net.ssl_cipher_suites")
104+
);
105+
}
106+
finally {
107+
DriverManager.deregisterDriver(mockDriver);
108+
}
120109
}
121110

122111
@Test(expected = RuntimeException.class)
123-
public void testGetConnection_withNullCredentialsProvider_throwsRuntimeException()
112+
public void getConnection_whenCredentialsProviderIsNull_throwsRuntimeException()
124113
{
125114
DatabaseConnectionConfig config = new DatabaseConnectionConfig(
126115
TEST_CATALOG, OracleConstants.ORACLE_NAME,
@@ -131,7 +120,7 @@ public void testGetConnection_withNullCredentialsProvider_throwsRuntimeException
131120
}
132121

133122
@Test(expected = RuntimeException.class)
134-
public void testGetConnection_whenDriverFailsToConnect_throwsRuntimeException() throws Exception
123+
public void getConnection_whenDriverFailsToConnect_throwsRuntimeException() throws Exception
135124
{
136125
DefaultCredentials credentials = new DefaultCredentials(USERNAME, PASSWORD);
137126
CredentialsProvider credentialsProvider = new StaticCredentialsProvider(credentials);
@@ -144,10 +133,13 @@ public void testGetConnection_whenDriverFailsToConnect_throwsRuntimeException()
144133
when(mockDriver.connect(anyString(), any(Properties.class)))
145134
.thenThrow(new SQLException("DB down", "08001", 1234));
146135

147-
DriverManager.registerDriver(mockDriver);
148-
136+
try {
137+
DriverManager.registerDriver(mockDriver);
149138
new OracleJdbcConnectionFactory(config, info).getConnection(credentialsProvider);
139+
}
140+
finally {
150141
DriverManager.deregisterDriver(mockDriver);
142+
}
151143
}
152144

153145
@Test(expected = RuntimeException.class)

‎athena-oracle/src/test/java/com/amazonaws/athena/connectors/oracle/OracleMetadataHandlerTest.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,15 @@ public void tearDown()
121121
}
122122

123123
@Test
124-
public void getPartitionSchema()
124+
public void getPartitionSchema_whenCalled_returnsPartitionSchema()
125125
{
126126
assertEquals(SchemaBuilder.newBuilder()
127127
.addField(OracleMetadataHandler.BLOCK_PARTITION_COLUMN_NAME, org.apache.arrow.vector.types.Types.MinorType.VARCHAR.getType()).build(),
128128
this.oracleMetadataHandler.getPartitionSchema(CATALOG_NAME));
129129
}
130130

131131
@Test
132-
public void doGetTableLayout()
132+
public void doGetTableLayout_whenTableHasPartitions_returnsPartitionLayout()
133133
throws Exception
134134
{
135135
Constraints constraints = Mockito.mock(Constraints.class);
@@ -169,7 +169,7 @@ public void doGetTableLayout()
169169
}
170170

171171
@Test
172-
public void doGetTableLayoutWithNoPartitions()
172+
public void doGetTableLayout_whenTableHasNoPartitions_returnsDefaultPartition()
173173
throws Exception
174174
{
175175
Constraints constraints = Mockito.mock(Constraints.class);
@@ -209,7 +209,7 @@ public void doGetTableLayoutWithNoPartitions()
209209
}
210210

211211
@Test(expected = RuntimeException.class)
212-
public void doGetTableLayoutWithSQLException()
212+
public void doGetTableLayout_whenSQLExceptionOccurs_throwsRuntimeException()
213213
throws Exception
214214
{
215215
Constraints constraints = Mockito.mock(Constraints.class);
@@ -228,7 +228,7 @@ public void doGetTableLayoutWithSQLException()
228228
}
229229

230230
@Test
231-
public void doGetSplits()
231+
public void doGetSplits_whenPartitionsExist_returnsSplitsForAllPartitions()
232232
throws Exception
233233
{
234234
Constraints constraints = Mockito.mock(Constraints.class);
@@ -264,7 +264,7 @@ public void doGetSplits()
264264
}
265265

266266
@Test
267-
public void doGetSplitsContinuation()
267+
public void doGetSplits_whenContinuationTokenProvided_returnsRemainingSplits()
268268
throws Exception
269269
{
270270
Constraints constraints = Mockito.mock(Constraints.class);
@@ -298,7 +298,7 @@ public void doGetSplitsContinuation()
298298
}
299299

300300
@Test
301-
public void testDoGetSplits_withQueryPassthrough()
301+
public void doGetSplits_whenQueryPassthroughArgsProvided_returnsSplitWithPassthroughArgs()
302302
{
303303
TableName tableName = new TableName("testSchema", "testTable");
304304

@@ -339,7 +339,7 @@ public void testDoGetSplits_withQueryPassthrough()
339339
}
340340

341341
@Test
342-
public void doListPaginatedTables()
342+
public void doListTables_whenCalledWithPagination_returnsPaginatedTables()
343343
throws Exception
344344
{
345345
PreparedStatement preparedStatement = Mockito.mock(PreparedStatement.class);
@@ -371,7 +371,7 @@ blockAllocator, new ListTablesRequest(this.federatedIdentity, "testQueryId",
371371
}
372372

373373
@Test
374-
public void doGetTable()
374+
public void doGetTable_whenCalled_returnsTableSchemaWithColumns()
375375
throws Exception
376376
{
377377
String[] schema = {"DATA_TYPE", "COLUMN_SIZE", "COLUMN_NAME", "DECIMAL_DIGITS", "NUM_PREC_RADIX"};
@@ -410,7 +410,8 @@ public void doGetTable()
410410
}
411411

412412
@Test
413-
public void testDoGetTable_withUnsupported_fallbackToVarchar() throws Exception {
413+
public void doGetTable_whenUnsupportedColumnTypes_fallbackToVarchar() throws Exception
414+
{
414415
String[] schema = {"DATA_TYPE", "COLUMN_SIZE", "COLUMN_NAME", "DECIMAL_DIGITS", "NUM_PREC_RADIX"};
415416

416417
Object[][] values = {
@@ -443,7 +444,7 @@ public void testDoGetTable_withUnsupported_fallbackToVarchar() throws Exception
443444
}
444445

445446
@Test
446-
public void testDoGetDataSourceCapabilities()
447+
public void doGetDataSourceCapabilities_whenCalled_returnsSupportedCapabilities()
447448
{
448449
GetDataSourceCapabilitiesRequest request =
449450
new GetDataSourceCapabilitiesRequest(federatedIdentity, QUERY_ID, CATALOG_NAME);

‎athena-oracle/src/test/java/com/amazonaws/athena/connectors/oracle/OracleMuxJdbcMetadataHandlerTest.java‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public void setup()
7272
}
7373

7474
@Test
75-
public void doListSchemaNames()
75+
public void doListSchemaNames_whenCalled_delegatesToOracleHandler()
7676
throws Exception
7777
{
7878
ListSchemasRequest listSchemasRequest = Mockito.mock(ListSchemasRequest.class);
@@ -82,7 +82,7 @@ public void doListSchemaNames()
8282
}
8383

8484
@Test
85-
public void doListTables()
85+
public void doListTables_whenCalled_delegatesToOracleHandler()
8686
throws Exception
8787
{
8888
ListTablesRequest listTablesRequest = Mockito.mock(ListTablesRequest.class);
@@ -92,7 +92,7 @@ public void doListTables()
9292
}
9393

9494
@Test
95-
public void doGetTable()
95+
public void doGetTable_whenCalled_delegatesToOracleHandler()
9696
throws Exception
9797
{
9898
GetTableRequest getTableRequest = Mockito.mock(GetTableRequest.class);
@@ -102,7 +102,7 @@ public void doGetTable()
102102
}
103103

104104
@Test
105-
public void doGetTableLayout()
105+
public void doGetTableLayout_whenCalled_delegatesToOracleHandler()
106106
throws Exception
107107
{
108108
GetTableLayoutRequest getTableLayoutRequest = Mockito.mock(GetTableLayoutRequest.class);
@@ -113,21 +113,20 @@ public void doGetTableLayout()
113113
}
114114

115115
@Test
116-
public void getPartitionSchema()
116+
public void getPartitionSchema_whenCalled_delegatesToOracleHandler()
117117
{
118118
this.jdbcMetadataHandler.getPartitionSchema("fakedatabase");
119119
Mockito.verify(this.oracleMetadataHandler, Mockito.times(1)).getPartitionSchema(Mockito.eq("fakedatabase"));
120120
}
121121

122122
@Test(expected = RuntimeException.class)
123-
public void getPartitionSchemaForUnsupportedCatalog()
123+
public void getPartitionSchema_whenCatalogIsUnsupported_throwsRuntimeException()
124124
{
125125
this.jdbcMetadataHandler.getPartitionSchema("unsupportedCatalog");
126126
}
127127

128-
129128
@Test
130-
public void getPartitions()
129+
public void getPartitions_whenCalled_delegatesToOracleHandler()
131130
throws Exception
132131
{
133132
GetTableLayoutRequest getTableLayoutRequest = Mockito.mock(GetTableLayoutRequest.class);
@@ -137,7 +136,7 @@ public void getPartitions()
137136
}
138137

139138
@Test
140-
public void doGetSplits()
139+
public void doGetSplits_whenCalled_delegatesToOracleHandler()
141140
{
142141
GetSplitsRequest getSplitsRequest = Mockito.mock(GetSplitsRequest.class);
143142
Mockito.when(getSplitsRequest.getCatalogName()).thenReturn("fakedatabase");

‎athena-oracle/src/test/java/com/amazonaws/athena/connectors/oracle/OracleMuxJdbcRecordHandlerTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public void setup()
6868
}
6969

7070
@Test
71-
public void readWithConstraint()
71+
public void readWithConstraint_whenCalled_delegatesToOracleHandler()
7272
throws Exception
7373
{
7474
BlockSpiller blockSpiller = Mockito.mock(BlockSpiller.class);
@@ -79,7 +79,7 @@ public void readWithConstraint()
7979
}
8080

8181
@Test(expected = RuntimeException.class)
82-
public void readWithConstraintWithUnsupportedCatalog()
82+
public void readWithConstraint_whenCatalogIsUnsupported_throwsRuntimeException()
8383
throws Exception
8484
{
8585
BlockSpiller blockSpiller = Mockito.mock(BlockSpiller.class);
@@ -89,7 +89,7 @@ public void readWithConstraintWithUnsupportedCatalog()
8989
}
9090

9191
@Test
92-
public void buildSplitSql()
92+
public void buildSplitSql_whenCalled_delegatesToOracleHandler()
9393
throws SQLException
9494
{
9595
ReadRecordsRequest readRecordsRequest = Mockito.mock(ReadRecordsRequest.class);

0 commit comments

Comments
 (0)