This is an automated email from the ASF dual-hosted git repository. dschneider pushed a commit to branch feature/GEODE-6194 in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-6194 by this push: new 6fa5726 added integration test support 6fa5726 is described below commit 6fa57265122adb2a8b142144a918d6327a13c395 Author: Darrel Schneider <dschnei...@pivotal.io> AuthorDate: Fri Dec 14 16:37:18 2018 -0800 added integration test support Need to add unit tests for SqlHandler to cover some new exceptions related to JSONObject. Also need to add test coverage in JdbcAsyncWriterIntegrationTest and JdbcDistributedTest for composite keys. --- .../connectors/jdbc/JdbcLoaderIntegrationTest.java | 35 ++++++- .../connectors/jdbc/JdbcWriterIntegrationTest.java | 103 ++++++++++++++++++--- .../jdbc/internal/TestConfigService.java | 17 +++- .../geode/connectors/jdbc/internal/SqlHandler.java | 14 ++- .../jdbc/internal/cli/CreateMappingCommand.java | 2 +- 5 files changed, 151 insertions(+), 20 deletions(-) diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java index 71858f7..241e2ee 100644 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java @@ -24,6 +24,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.Date; +import org.json.JSONObject; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -118,6 +119,26 @@ public abstract class JdbcLoaderIntegrationTest { } @Test + public void verifyGetWithPdxClassNameAndCompositeKey() throws Exception { + createEmployeeTable(); + statement + .execute("Insert into " + REGION_TABLE_NAME + "(id, name, age) values('1', 'Emp1', 21)"); + String ids = "id,name"; + Region<String, Employee> region = + createRegionWithJDBCLoader(REGION_TABLE_NAME, Employee.class.getName(), ids); + createPdxType(); + + JSONObject key = new JSONObject(); + key.put("id", "1"); + key.put("name", "Emp1"); + Employee value = region.get(key.toString()); + + assertThat(value.getId()).isEqualTo("1"); + assertThat(value.getName()).isEqualTo("Emp1"); + assertThat(value.getAge()).isEqualTo(21); + } + + @Test public void verifyGetWithSupportedFieldsWithPdxClassName() throws Exception { createClassWithSupportedPdxFieldsTable(statement, REGION_TABLE_NAME); ClassWithSupportedPdxFields classWithSupportedPdxFields = @@ -149,23 +170,29 @@ public abstract class JdbcLoaderIntegrationTest { assertThat(pdx).isNull(); } - private SqlHandler createSqlHandler(String pdxClassName) + private SqlHandler createSqlHandler(String pdxClassName, String ids) throws RegionMappingExistsException { return new SqlHandler(new TableMetaDataManager(), TestConfigService.getTestConfigService((InternalCache) cache, pdxClassName, - getConnectionUrl()), + getConnectionUrl(), ids), testDataSourceFactory); } - private <K, V> Region<K, V> createRegionWithJDBCLoader(String regionName, String pdxClassName) + private <K, V> Region<K, V> createRegionWithJDBCLoader(String regionName, String pdxClassName, + String ids) throws RegionMappingExistsException { JdbcLoader<K, V> jdbcLoader = - new JdbcLoader<>(createSqlHandler(pdxClassName), cache); + new JdbcLoader<>(createSqlHandler(pdxClassName, ids), cache); RegionFactory<K, V> regionFactory = cache.createRegionFactory(REPLICATE); regionFactory.setCacheLoader(jdbcLoader); return regionFactory.create(regionName); } + private <K, V> Region<K, V> createRegionWithJDBCLoader(String regionName, String pdxClassName) + throws RegionMappingExistsException { + return createRegionWithJDBCLoader(regionName, pdxClassName, null); + } + private ClassWithSupportedPdxFields createClassWithSupportedPdxFieldsForInsert(String key) { ClassWithSupportedPdxFields classWithSupportedPdxFields = new ClassWithSupportedPdxFields(key, true, (byte) 1, (short) 2, 3, 4, 5.5f, 6.0, "BigEmp", diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java index 79ff90d..4db143f 100644 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java @@ -24,6 +24,7 @@ import java.sql.Statement; import java.util.HashMap; import java.util.Map; +import org.json.JSONObject; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -38,6 +39,7 @@ import org.apache.geode.connectors.jdbc.internal.TableMetaDataManager; import org.apache.geode.connectors.jdbc.internal.TestConfigService; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.pdx.PdxInstance; +import org.apache.geode.pdx.WritablePdxInstance; public abstract class JdbcWriterIntegrationTest { @@ -60,20 +62,25 @@ public abstract class JdbcWriterIntegrationTest { public void setUp() throws Exception { cache = (InternalCache) new CacheFactory().set("locators", "").set("mcast-port", "0") .setPdxReadSerialized(false).create(); - employees = createRegionWithJDBCSynchronousWriter(REGION_TABLE_NAME); connection = getConnection(); statement = connection.createStatement(); statement.execute("Create Table " + REGION_TABLE_NAME + " (id varchar(10) primary key not null, name varchar(10), age int)"); - pdx1 = cache.createPdxInstanceFactory(Employee.class.getName()).writeString("name", "Emp1") + pdx1 = cache.createPdxInstanceFactory(Employee.class.getName()).writeString("id", "1") + .writeString("name", "Emp1") .writeInt("age", 55).create(); - pdx2 = cache.createPdxInstanceFactory(Employee.class.getName()).writeString("name", "Emp2") + pdx2 = cache.createPdxInstanceFactory(Employee.class.getName()).writeString("id", "2") + .writeString("name", "Emp2") .writeInt("age", 21).create(); employee1 = (Employee) pdx1.getObject(); employee2 = (Employee) pdx2.getObject(); } + private void setupRegion(String ids) throws RegionMappingExistsException { + employees = createRegionWithJDBCSynchronousWriter(REGION_TABLE_NAME, ids); + } + @After public void tearDown() throws Exception { cache.close(); @@ -102,6 +109,7 @@ public abstract class JdbcWriterIntegrationTest { @Test public void canInsertIntoTable() throws Exception { + setupRegion(null); employees.put("1", pdx1); employees.put("2", pdx2); @@ -113,7 +121,29 @@ public abstract class JdbcWriterIntegrationTest { } @Test + public void canInsertIntoTableWithCompositeKey() throws Exception { + setupRegion("id,age"); + JSONObject compositeKey1 = new JSONObject(); + compositeKey1.put("id", pdx1.getField("id")); + compositeKey1.put("age", pdx1.getField("age")); + String actualKey = compositeKey1.toString(); + JSONObject compositeKey2 = new JSONObject(); + compositeKey2.put("id", pdx2.getField("id")); + compositeKey2.put("age", pdx2.getField("age")); + + employees.put(actualKey, pdx1); + employees.put(compositeKey2.toString(), pdx2); + + ResultSet resultSet = + statement.executeQuery("select * from " + REGION_TABLE_NAME + " order by id asc"); + assertRecordMatchesEmployee(resultSet, "1", employee1); + assertRecordMatchesEmployee(resultSet, "2", employee2); + assertThat(resultSet.next()).isFalse(); + } + + @Test public void canPutAllInsertIntoTable() throws Exception { + setupRegion(null); Map<String, PdxInstance> putAllMap = new HashMap<>(); putAllMap.put("1", pdx1); putAllMap.put("2", pdx2); @@ -128,6 +158,7 @@ public abstract class JdbcWriterIntegrationTest { @Test public void verifyThatPdxFieldNamedSameAsPrimaryKeyIsIgnored() throws Exception { + setupRegion(null); PdxInstance pdxInstanceWithId = cache.createPdxInstanceFactory(Employee.class.getName()) .writeString("name", "Emp1").writeInt("age", 55).writeString("id", "3").create(); employees.put("1", pdxInstanceWithId); @@ -139,14 +170,17 @@ public abstract class JdbcWriterIntegrationTest { } @Test - public void putNonPdxInstanceFails() { + public void putNonPdxInstanceFails() throws RegionMappingExistsException { + setupRegion(null); Region nonPdxEmployees = this.employees; Throwable thrown = catchThrowable(() -> nonPdxEmployees.put("1", "non pdx instance")); assertThat(thrown).isInstanceOf(IllegalArgumentException.class); } @Test - public void putNonPdxInstanceThatIsPdxSerializable() throws SQLException { + public void putNonPdxInstanceThatIsPdxSerializable() + throws SQLException, RegionMappingExistsException { + setupRegion(null); Region nonPdxEmployees = this.employees; Employee value = new Employee("2", "Emp2", 22); nonPdxEmployees.put("2", value); @@ -159,6 +193,7 @@ public abstract class JdbcWriterIntegrationTest { @Test public void canDestroyFromTable() throws Exception { + setupRegion(null); employees.put("1", pdx1); employees.put("2", pdx2); @@ -171,7 +206,28 @@ public abstract class JdbcWriterIntegrationTest { } @Test + public void canDestroyFromTableWithCompositeKey() throws Exception { + setupRegion("id,age"); + JSONObject compositeKey1 = new JSONObject(); + compositeKey1.put("id", pdx1.getField("id")); + compositeKey1.put("age", pdx1.getField("age")); + JSONObject compositeKey2 = new JSONObject(); + compositeKey2.put("id", pdx2.getField("id")); + compositeKey2.put("age", pdx2.getField("age")); + employees.put(compositeKey1.toString(), pdx1); + employees.put(compositeKey2.toString(), pdx2); + + employees.destroy(compositeKey1.toString()); + + ResultSet resultSet = + statement.executeQuery("select * from " + REGION_TABLE_NAME + " order by id asc"); + assertRecordMatchesEmployee(resultSet, "2", employee2); + assertThat(resultSet.next()).isFalse(); + } + + @Test public void canUpdateTable() throws Exception { + setupRegion(null); employees.put("1", pdx1); employees.put("1", pdx2); @@ -182,7 +238,30 @@ public abstract class JdbcWriterIntegrationTest { } @Test + public void canUpdateTableWithCompositeKey() throws Exception { + setupRegion("id,age"); + PdxInstance myPdx = cache.createPdxInstanceFactory(Employee.class.getName()) + .writeString("id", "1").writeString("name", "Emp1") + .writeInt("age", 55).create(); + JSONObject compositeKey1 = new JSONObject(); + compositeKey1.put("id", myPdx.getField("id")); + compositeKey1.put("age", myPdx.getField("age")); + employees.put(compositeKey1.toString(), myPdx); + WritablePdxInstance updatedPdx = myPdx.createWriter(); + updatedPdx.setField("name", "updated"); + Employee updatedEmployee = (Employee) updatedPdx.getObject(); + + employees.put(compositeKey1.toString(), updatedPdx); + + ResultSet resultSet = + statement.executeQuery("select * from " + REGION_TABLE_NAME + " order by id asc"); + assertRecordMatchesEmployee(resultSet, "1", updatedEmployee); + assertThat(resultSet.next()).isFalse(); + } + + @Test public void canUpdateBecomeInsert() throws Exception { + setupRegion(null); employees.put("1", pdx1); statement.execute("delete from " + REGION_TABLE_NAME + " where id = '1'"); @@ -198,6 +277,7 @@ public abstract class JdbcWriterIntegrationTest { @Test public void canInsertBecomeUpdate() throws Exception { + setupRegion(null); statement.execute("Insert into " + REGION_TABLE_NAME + " values('1', 'bogus', 11)"); validateTableRowCount(1); @@ -209,9 +289,10 @@ public abstract class JdbcWriterIntegrationTest { assertThat(resultSet.next()).isFalse(); } - private Region<String, PdxInstance> createRegionWithJDBCSynchronousWriter(String regionName) + private Region<String, PdxInstance> createRegionWithJDBCSynchronousWriter(String regionName, + String ids) throws RegionMappingExistsException { - jdbcWriter = new JdbcWriter(createSqlHandler(), cache); + jdbcWriter = new JdbcWriter(createSqlHandler(ids), cache); RegionFactory<String, PdxInstance> regionFactory = cache.createRegionFactory(RegionShortcut.REPLICATE); @@ -226,17 +307,17 @@ public abstract class JdbcWriterIntegrationTest { assertThat(size).isEqualTo(expected); } - private SqlHandler createSqlHandler() + private SqlHandler createSqlHandler(String ids) throws RegionMappingExistsException { return new SqlHandler(new TableMetaDataManager(), - TestConfigService.getTestConfigService(getConnectionUrl()), + TestConfigService.getTestConfigService(getConnectionUrl(), ids), testDataSourceFactory); } - private void assertRecordMatchesEmployee(ResultSet resultSet, String key, Employee employee) + private void assertRecordMatchesEmployee(ResultSet resultSet, String id, Employee employee) throws SQLException { assertThat(resultSet.next()).isTrue(); - assertThat(resultSet.getString("id")).isEqualTo(key); + assertThat(resultSet.getString("id")).isEqualTo(id); assertThat(resultSet.getString("name")).isEqualTo(employee.getName()); assertThat(resultSet.getInt("age")).isEqualTo(employee.getAge()); } diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestConfigService.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestConfigService.java index 2441ae8..cd31ce6 100644 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestConfigService.java +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestConfigService.java @@ -34,13 +34,24 @@ public class TestConfigService { return getTestConfigService(createMockCache(), null, connectionUrl); } + public static JdbcConnectorServiceImpl getTestConfigService(String connectionUrl, String ids) + throws RegionMappingExistsException { + return getTestConfigService(createMockCache(), null, connectionUrl, ids); + } + public static JdbcConnectorServiceImpl getTestConfigService(InternalCache cache, String pdxClassName, String connectionUrl) throws RegionMappingExistsException { + return getTestConfigService(cache, pdxClassName, connectionUrl, null); + } + + public static JdbcConnectorServiceImpl getTestConfigService(InternalCache cache, + String pdxClassName, String connectionUrl, String ids) + throws RegionMappingExistsException { JdbcConnectorServiceImpl service = new JdbcConnectorServiceImpl(); service.init(cache); - service.createRegionMapping(createRegionMapping(pdxClassName)); + service.createRegionMapping(createRegionMapping(pdxClassName, ids)); return service; } @@ -50,8 +61,8 @@ public class TestConfigService { return cache; } - private static RegionMapping createRegionMapping(String pdxClassName) { + private static RegionMapping createRegionMapping(String pdxClassName, String ids) { return new RegionMapping(REGION_NAME, pdxClassName, REGION_TABLE_NAME, - CONNECTION_CONFIG_NAME, null); + CONNECTION_CONFIG_NAME, ids); } } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java index 4040ab5..173d65a 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java @@ -26,6 +26,7 @@ import java.util.Set; import javax.sql.DataSource; +import org.json.JSONException; import org.json.JSONObject; import org.apache.geode.InternalGemFireException; @@ -260,7 +261,18 @@ public class SqlHandler { new ColumnData(keyColumnName, key, tableMetaData.getColumnDataType(keyColumnName)); result.add(columnData); } else { - JSONObject compositeKey = new JSONObject((String) key); + if (!(key instanceof String)) { + throw new JdbcConnectorException("The key \"" + key + "\" of class \"" + key.getClass() + + "\" must be a java.lang.String because multiple columns are configured as ids."); + } + JSONObject compositeKey = null; + try { + compositeKey = new JSONObject((String) key); + } catch (JSONException ex) { + throw new JdbcConnectorException("The key \"" + key + + "\" must be a valid JSON string because multiple columns are configured as ids. Details: " + + ex.getMessage()); + } Set<String> fieldNames = compositeKey.keySet(); if (fieldNames.size() != keyColumnNames.size()) { throw new JdbcConnectorException("The key \"" + key + "\" should have " diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java index 6bcefc5..5f78a2c 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java @@ -65,7 +65,7 @@ public class CreateMappingCommand extends SingleGfshCommand { "By default, writes will be asynchronous. If true, writes will be synchronous."; static final String CREATE_MAPPING__ID_NAME = "id"; static final String CREATE_MAPPING__ID_NAME__HELP = - "The table column name to use as the region key for this JDBC mapping."; + "The table column names to use as the region key for this JDBC mapping. If more than one column name is given then they must be separated by commas."; public static String createAsyncEventQueueName(String regionPath) { if (regionPath.startsWith("/")) {