geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject [geode] branch feature/GEODE-6291 updated: SqlHandler now initializes SqlToPdxInstance on the first read instead of in the constructor
Date Fri, 01 Feb 2019 18:45:40 GMT
This is an automated email from the ASF dual-hosted git repository.

dschneider pushed a commit to branch feature/GEODE-6291
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-6291 by this push:
     new feeb540  SqlHandler now initializes SqlToPdxInstance on the first read instead of
in the constructor
feeb540 is described below

commit feeb5404a5e5b3c86d0bf2de9e7f8ffbcc77e685
Author: Darrel Schneider <dschneider@pivotal.io>
AuthorDate: Fri Feb 1 10:44:50 2019 -0800

    SqlHandler now initializes SqlToPdxInstance on the first read
    instead of in the constructor
---
 .../jdbc/JdbcAsyncWriterIntegrationTest.java       |  2 +-
 .../connectors/jdbc/JdbcLoaderIntegrationTest.java |  2 +-
 .../connectors/jdbc/JdbcWriterIntegrationTest.java |  2 +-
 .../apache/geode/connectors/jdbc/JdbcLoader.java   |  5 ---
 .../jdbc/internal/AbstractJdbcCallback.java        | 12 +++----
 .../geode/connectors/jdbc/internal/SqlHandler.java | 40 +++++++++++++---------
 .../jdbc/internal/AbstractJdbcCallbackTest.java    | 10 +++++-
 .../connectors/jdbc/internal/SqlHandlerTest.java   |  7 ++--
 8 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
index d2c78cb..89f2af6 100644
--- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
+++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
@@ -346,7 +346,7 @@ public abstract class JdbcAsyncWriterIntegrationTest {
       throws RegionMappingExistsException {
     return new SqlHandler(cache, regionName, new TableMetaDataManager(),
         TestConfigService.getTestConfigService(ids, fieldMappings),
-        testDataSourceFactory, false);
+        testDataSourceFactory);
   }
 
 }
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 f9538bf..b818641 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
@@ -235,7 +235,7 @@ public abstract class JdbcLoaderIntegrationTest {
     return new SqlHandler(cache, regionName, new TableMetaDataManager(),
         TestConfigService.getTestConfigService((InternalCache) cache, pdxClassName, ids,
catalog,
             schema, fieldMappings),
-        testDataSourceFactory, true);
+        testDataSourceFactory);
   }
 
   protected <K, V> Region<K, V> createRegionWithJDBCLoader(String regionName,
String pdxClassName,
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 d95699d..86d163f 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
@@ -422,7 +422,7 @@ public abstract class JdbcWriterIntegrationTest {
       throws RegionMappingExistsException {
     return new SqlHandler(cache, regionName, new TableMetaDataManager(),
         TestConfigService.getTestConfigService(cache, null, ids, catalog, schema, fieldMappings),
-        testDataSourceFactory, false);
+        testDataSourceFactory);
   }
 
   protected void assertRecordMatchesEmployee(ResultSet resultSet, String id, Employee employee)
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/JdbcLoader.java
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/JdbcLoader.java
index de1da33..925937a 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/JdbcLoader.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/JdbcLoader.java
@@ -42,11 +42,6 @@ public class JdbcLoader<K, V> extends AbstractJdbcCallback implements
CacheLoade
     super(sqlHandler, cache);
   }
 
-  @Override
-  protected boolean supportsReads() {
-    return true;
-  }
-
   /**
    * @return this method always returns a PdxInstance. It does not matter what the V generic
    *         parameter is set to.
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java
index 5282d20..230e169 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java
@@ -49,17 +49,17 @@ public abstract class AbstractJdbcCallback implements CacheCallback {
     return operation.isLoad();
   }
 
-  protected boolean supportsReads() {
-    return false;
-  }
-
   private synchronized void initialize(Region<?, ?> region) {
     if (sqlHandler == null) {
       this.cache = (InternalCache) region.getRegionService();
       JdbcConnectorService service = cache.getService(JdbcConnectorService.class);
       TableMetaDataManager tableMetaDataManager = new TableMetaDataManager();
-      sqlHandler =
-          new SqlHandler(cache, region.getName(), tableMetaDataManager, service, supportsReads());
+      sqlHandler = createSqlHandler(cache, region.getName(), tableMetaDataManager, service);
     }
   }
+
+  SqlHandler createSqlHandler(InternalCache cache, String regionName,
+      TableMetaDataManager tableMetaDataManager, JdbcConnectorService service) {
+    return new SqlHandler(cache, regionName, tableMetaDataManager, service);
+  }
 }
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 a5ab6e1..bc5fe1b 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
@@ -36,18 +36,19 @@ import org.apache.geode.pdx.PdxInstance;
 
 @Experimental
 public class SqlHandler {
+  private final InternalCache cache;
   private final TableMetaDataManager tableMetaDataManager;
   private final RegionMapping regionMapping;
   private final DataSource dataSource;
-  private final SqlToPdxInstance sqlToPdxInstance;
+  private volatile SqlToPdxInstance sqlToPdxInstance;
 
   public SqlHandler(InternalCache cache, String regionName,
       TableMetaDataManager tableMetaDataManager, JdbcConnectorService configService,
-      DataSourceFactory dataSourceFactory, boolean supportReading) {
+      DataSourceFactory dataSourceFactory) {
+    this.cache = cache;
     this.tableMetaDataManager = tableMetaDataManager;
     this.regionMapping = getMappingForRegion(configService, regionName);
     this.dataSource = getDataSource(dataSourceFactory, this.regionMapping.getDataSourceName());
-    this.sqlToPdxInstance = createSqlToPdxInstance(supportReading, cache, regionMapping);
   }
 
   private static RegionMapping getMappingForRegion(JdbcConnectorService configService,
@@ -71,21 +72,10 @@ public class SqlHandler {
     return dataSource;
   }
 
-  private static SqlToPdxInstance createSqlToPdxInstance(boolean supportReading,
-      InternalCache cache, RegionMapping regionMapping) {
-    if (!supportReading) {
-      return null;
-    }
-    SqlToPdxInstanceCreator sqlToPdxInstanceCreator =
-        new SqlToPdxInstanceCreator(cache, regionMapping);
-    return sqlToPdxInstanceCreator.create();
-  }
-
   public SqlHandler(InternalCache cache, String regionName,
-      TableMetaDataManager tableMetaDataManager, JdbcConnectorService configService,
-      boolean supportReading) {
+      TableMetaDataManager tableMetaDataManager, JdbcConnectorService configService) {
     this(cache, regionName, tableMetaDataManager, configService,
-        dataSourceName -> JNDIInvoker.getDataSource(dataSourceName), supportReading);
+        dataSourceName -> JNDIInvoker.getDataSource(dataSourceName));
   }
 
   Connection getConnection() throws SQLException {
@@ -106,13 +96,29 @@ public class SqlHandler {
       try (PreparedStatement statement =
           getPreparedStatement(connection, tableMetaData, entryColumnData, Operation.GET))
{
         try (ResultSet resultSet = executeReadQuery(statement, entryColumnData)) {
-          result = sqlToPdxInstance.create(resultSet);
+          result = getSqlToPdxInstance().create(resultSet);
         }
       }
     }
     return result;
   }
 
+  private SqlToPdxInstance getSqlToPdxInstance() {
+    SqlToPdxInstance result = this.sqlToPdxInstance;
+    if (result == null) {
+      result = initializeSqlToPdxInstance();
+    }
+    return result;
+  }
+
+  private synchronized SqlToPdxInstance initializeSqlToPdxInstance() {
+    SqlToPdxInstanceCreator sqlToPdxInstanceCreator =
+        new SqlToPdxInstanceCreator(cache, regionMapping);
+    SqlToPdxInstance result = sqlToPdxInstanceCreator.create();
+    this.sqlToPdxInstance = result;
+    return result;
+  }
+
   private ResultSet executeReadQuery(PreparedStatement statement, EntryColumnData entryColumnData)
       throws SQLException {
     setValuesInStatement(statement, entryColumnData, Operation.GET);
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java
index 3030f74..34356f8 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java
@@ -17,7 +17,11 @@ package org.apache.geode.connectors.jdbc.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.same;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import org.junit.Before;
@@ -55,7 +59,8 @@ public class AbstractJdbcCallbackTest {
 
   @Test
   public void initializedSqlHandlerIfNoneExists() {
-    jdbcCallback = new AbstractJdbcCallback() {};
+    jdbcCallback = spy(AbstractJdbcCallback.class);
+    // jdbcCallback = spy(new AbstractJdbcCallback() {});
     InternalCache cache = mock(InternalCache.class);
     Region region = mock(Region.class);
     when(region.getRegionService()).thenReturn(cache);
@@ -65,6 +70,9 @@ public class AbstractJdbcCallbackTest {
     assertThat(jdbcCallback.getSqlHandler()).isNull();
     RegionMapping regionMapping = mock(RegionMapping.class);
     when(service.getMappingForRegion("regionName")).thenReturn(regionMapping);
+    SqlHandler sqlHandler = mock(SqlHandler.class);
+    doReturn(sqlHandler).when(jdbcCallback).createSqlHandler(same(cache), eq("regionName"),
any(),
+        same(service));
 
     jdbcCallback.checkInitialized(region);
 
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java
index 2112daa..1c6c4e1 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java
@@ -119,7 +119,7 @@ public class SqlHandlerTest {
     statement = mock(PreparedStatement.class);
     when(this.connection.prepareStatement(any())).thenReturn(statement);
     handler = new SqlHandler(cache, REGION_NAME, tableMetaDataManager, connectorService,
-        dataSourceFactory, true);
+        dataSourceFactory);
   }
 
   @Test
@@ -135,7 +135,7 @@ public class SqlHandlerTest {
         "JDBC mapping for region regionWithNoMapping not found. Create the mapping with the
gfsh command 'create jdbc-mapping'.");
 
     new SqlHandler(cache, "regionWithNoMapping", tableMetaDataManager, connectorService,
-        dataSourceFactory, true);
+        dataSourceFactory);
   }
 
   @Test
@@ -145,8 +145,7 @@ public class SqlHandlerTest {
     thrown.expectMessage(
         "JDBC data-source named \"bogus data source name\" not found. Create it with gfsh
'create data-source --pooled --name=bogus data source name'.");
 
-    new SqlHandler(cache, REGION_NAME, tableMetaDataManager, connectorService, dataSourceFactory,
-        true);
+    new SqlHandler(cache, REGION_NAME, tableMetaDataManager, connectorService, dataSourceFactory);
   }
 
   @Test


Mime
View raw message