You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2018/12/15 00:38:51 UTC

[geode] branch feature/GEODE-6194 updated: added integration test support

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 <ds...@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("/")) {