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 <[email protected]>
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
