This is an automated email from the ASF dual-hosted git repository.
nreich pushed a commit to branch feature/GEODE-3781
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-3781 by this
push:
new 7fd92d5 Refactor long methods and add types
7fd92d5 is described below
commit 7fd92d55896b8abc4f24afe09e667893b0df3be3
Author: Nick Reich <[email protected]>
AuthorDate: Fri Nov 17 10:08:15 2017 -0800
Refactor long methods and add types
---
.../jdbc/internal/ConnectionManager.java | 158 ++++++++++++---------
.../geode/connectors/jdbc/internal/SqlHandler.java | 54 +++----
2 files changed, 115 insertions(+), 97 deletions(-)
diff --git
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java
index 4f4a520..462c51c 100644
---
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java
+++
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java
@@ -57,6 +57,50 @@ class ConnectionManager {
return getNewConnection(config);
}
+ <K> List<ColumnValue> getColumnToValueList(ConnectionConfiguration config,
+ RegionMapping regionMapping, K key,
PdxInstance value, Operation operation) {
+ String keyColumnName = getKeyColumnName(config,
regionMapping.getTableName());
+ ColumnValue keyColumnValue = new ColumnValue(true, keyColumnName, key);
+
+ if (operation.isDestroy() || operation.isGet()) {
+ return Collections.singletonList(keyColumnValue);
+ }
+
+ List<ColumnValue> result = createColumnValueList(regionMapping, value,
keyColumnName);
+ result.add(keyColumnValue);
+ return result;
+ }
+
+ void close() {
+ connectionMap.values().forEach(this::close);
+ }
+
+ String getKeyColumnName(ConnectionConfiguration connectionConfig, String
tableName) {
+ return tableToPrimaryKeyMap.computeIfAbsent(tableName, k ->
computeKeyColumnName(connectionConfig, k));
+ }
+
+ ConnectionConfiguration getConnectionConfig(String connectionConfigName) {
+ return configService.getConnectionConfig(connectionConfigName);
+ }
+
+ PreparedStatement getPreparedStatement(Connection connection,
List<ColumnValue> columnList,
+ String tableName, Operation
operation, int pdxTypeId) {
+ PreparedStatementCache statementCache = preparedStatementCache.get();
+
+ if (statementCache == null) {
+ statementCache = new PreparedStatementCache();
+ preparedStatementCache.set(statementCache);
+ }
+
+ return statementCache.getPreparedStatement(connection, columnList,
tableName, operation,
+ pdxTypeId);
+ }
+
+ // package protected for testing purposes only
+ Connection getSQLConnection(ConnectionConfiguration config) throws
SQLException {
+ return DriverManager.getConnection(config.getUrl(), config.getUser(),
config.getPassword());
+ }
+
private synchronized Connection getNewConnection(ConnectionConfiguration
config) {
Connection connection;
try {
@@ -69,83 +113,72 @@ class ConnectionManager {
return connection;
}
- // package protected for testing purposes only
- Connection getSQLConnection(ConnectionConfiguration config) throws
SQLException {
- return DriverManager.getConnection(config.getUrl(), config.getUser(),
config.getPassword());
- }
-
- List<ColumnValue> getColumnToValueList(ConnectionConfiguration config,
- RegionMapping regionMapping, Object key, PdxInstance value, Operation
operation) {
- String keyColumnName = getKeyColumnName(config,
regionMapping.getTableName());
- ColumnValue keyCV = new ColumnValue(true, keyColumnName, key);
- if (operation.isDestroy() || operation.isGet()) {
- return Collections.singletonList(keyCV);
- }
-
- List<String> fieldNames = value.getFieldNames();
- List<ColumnValue> result = new ArrayList<>(fieldNames.size() + 1);
- for (String fieldName : fieldNames) {
+ private List<ColumnValue> createColumnValueList(RegionMapping regionMapping,
PdxInstance value,
+ String keyColumnName) {
+ List<ColumnValue> result = new ArrayList<>();
+ for (String fieldName : value.getFieldNames()) {
String columnName = regionMapping.getColumnNameForField(fieldName);
if (columnName.equalsIgnoreCase(keyColumnName)) {
continue;
}
- Object columnValue = value.getField(fieldName);
- ColumnValue cv = new ColumnValue(false, columnName, columnValue);
- result.add(cv);
+ ColumnValue columnValue = new ColumnValue(false, columnName,
value.getField(fieldName));
+ result.add(columnValue);
}
- result.add(keyCV);
return result;
}
- String getKeyColumnName(ConnectionConfiguration connectionConfig, String
tableName) {
- return tableToPrimaryKeyMap.computeIfAbsent(tableName, k -> {
- return computeKeyColumnName(connectionConfig, k);
- });
- }
-
private String computeKeyColumnName(ConnectionConfiguration
connectionConfig, String tableName) {
// TODO: check config for key column
- String key;
+ String key = null;
try {
Connection connection = getConnection(connectionConfig);
DatabaseMetaData metaData = connection.getMetaData();
ResultSet tables = metaData.getTables(null, null, "%", null);
- String realTableName = null;
- while (tables.next()) {
- String name = tables.getString("TABLE_NAME");
- if (name.equalsIgnoreCase(tableName)) {
- if (realTableName != null) {
- throw new IllegalStateException("Duplicate tables that match
region name");
- }
- realTableName = name;
- }
- }
- if (realTableName == null) {
- throw new IllegalStateException("no table was found that matches " +
tableName);
- }
- ResultSet primaryKeys = metaData.getPrimaryKeys(null, null,
realTableName);
- if (!primaryKeys.next()) {
- throw new IllegalStateException(
- "The table " + tableName + " does not have a primary key column.");
- }
- key = primaryKeys.getString("COLUMN_NAME");
- if (primaryKeys.next()) {
- throw new IllegalStateException(
- "The table " + tableName + " has more than one primary key
column.");
- }
+
+ String realTableName = getTableNameFromMetaData(tableName, tables);
+ key = getPrimaryKeyColumnNameFromMetaData(realTableName, metaData);
+
} catch (SQLException e) {
- key = null;
handleSQLException(e);
}
return key;
}
- private void handleSQLException(SQLException e) {
- throw new IllegalStateException("NYI: handleSQLException", e);
+ private String getTableNameFromMetaData(String tableName, ResultSet tables)
throws SQLException {
+ String realTableName = null;
+ while (tables.next()) {
+ String name = tables.getString("TABLE_NAME");
+ if (name.equalsIgnoreCase(tableName)) {
+ if (realTableName != null) {
+ throw new IllegalStateException("Duplicate tables that match region
name");
+ }
+ realTableName = name;
+ }
+ }
+
+ if (realTableName == null) {
+ throw new IllegalStateException("no table was found that matches " +
tableName);
+ }
+ return realTableName;
}
- void close() {
- connectionMap.values().forEach(connection -> close(connection));
+ private String getPrimaryKeyColumnNameFromMetaData(String tableName,
DatabaseMetaData metaData)
+ throws SQLException {
+ ResultSet primaryKeys = metaData.getPrimaryKeys(null, null, tableName);
+ if (!primaryKeys.next()) {
+ throw new IllegalStateException(
+ "The table " + tableName + " does not have a primary key column.");
+ }
+ String key = primaryKeys.getString("COLUMN_NAME");
+ if (primaryKeys.next()) {
+ throw new IllegalStateException(
+ "The table " + tableName + " has more than one primary key column.");
+ }
+ return key;
+ }
+
+ private void handleSQLException(SQLException e) {
+ throw new IllegalStateException("NYI: handleSQLException", e);
}
private void close(Connection connection) {
@@ -156,19 +189,4 @@ class ConnectionManager {
}
}
}
-
- ConnectionConfiguration getConnectionConfig(String connectionConfigName) {
- return configService.getConnectionConfig(connectionConfigName);
- }
-
- PreparedStatement getPreparedStatement(Connection connection,
List<ColumnValue> columnList,
- String tableName, Operation operation, int pdxTypeId) {
- PreparedStatementCache statementCache = preparedStatementCache.get();
- if (statementCache == null) {
- statementCache = new PreparedStatementCache();
- preparedStatementCache.set(statementCache);
- }
- return statementCache.getPreparedStatement(connection, columnList,
tableName, operation,
- pdxTypeId);
- }
}
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 13c25de..c61a9db 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
@@ -38,7 +38,7 @@ public class SqlHandler {
manager.close();
}
- public PdxInstance read(Region region, Object key) {
+ public <K, V> PdxInstance read(Region<K, V> region, K key) {
if (key == null) {
throw new IllegalArgumentException("Key for query cannot be null");
}
@@ -57,7 +57,7 @@ public class SqlHandler {
return executeReadStatement(statement, columnList, factory, regionMapping,
keyColumnName);
}
- private PdxInstanceFactory getPdxInstanceFactory(Region region,
RegionMapping regionMapping) {
+ private <K, V> PdxInstanceFactory getPdxInstanceFactory(Region<K, V> region,
RegionMapping regionMapping) {
InternalCache cache = (InternalCache) region.getRegionService();
String valueClassName = regionMapping.getPdxClassName();
PdxInstanceFactory factory;
@@ -75,11 +75,7 @@ public class SqlHandler {
PdxInstance pdxInstance = null;
synchronized (statement) {
try {
- int idx = 0;
- for (ColumnValue columnValue : columnList) {
- idx++;
- statement.setObject(idx, columnValue.getValue());
- }
+ setValuesInStatement(statement, columnList);
ResultSet resultSet = statement.executeQuery();
if (resultSet.next()) {
@@ -109,11 +105,20 @@ public class SqlHandler {
return pdxInstance;
}
+ private void setValuesInStatement(PreparedStatement statement,
List<ColumnValue> columnList)
+ throws SQLException {
+ int index = 0;
+ for (ColumnValue columnValue : columnList) {
+ index++;
+ statement.setObject(index, columnValue.getValue());
+ }
+ }
+
private String mapColumnNameToFieldName(String columnName) {
return columnName.toLowerCase();
}
- public void write(Region region, Operation operation, Object key,
PdxInstance value) {
+ public <K, V> void write(Region<K, V> region, Operation operation, K key,
PdxInstance value) {
if (value == null && operation != Operation.DESTROY) {
throw new IllegalArgumentException("PdxInstance cannot be null for
non-destroy operations");
}
@@ -124,53 +129,48 @@ public class SqlHandler {
List<ColumnValue> columnList =
manager.getColumnToValueList(connectionConfig, regionMapping, key,
value, operation);
- int pdxTypeId = 0;
- if (value != null) {
- pdxTypeId = ((PdxInstanceImpl) value).getPdxType().getTypeId();
- }
+ int pdxTypeId = value == null ? 0 : ((PdxInstanceImpl)
value).getPdxType().getTypeId();
PreparedStatement statement = manager.getPreparedStatement(
manager.getConnection(connectionConfig), columnList, tableName,
operation, pdxTypeId);
int updateCount = executeWriteStatement(statement, columnList, operation,
false);
+
+ // Destroy action not guaranteed to modify any database rows
if (operation.isDestroy()) {
- // TODO: should we check updateCount here? Probably not. It is possible
we have nothing in the
- // table to destroy.
return;
}
+
if (updateCount <= 0) {
- Operation upsertOp;
- if (operation.isUpdate()) {
- upsertOp = Operation.CREATE;
- } else {
- upsertOp = Operation.UPDATE;
- }
+ Operation upsertOp = getOppositeOperation(operation);
PreparedStatement upsertStatement = manager.getPreparedStatement(
manager.getConnection(connectionConfig), columnList, tableName,
upsertOp, pdxTypeId);
updateCount = executeWriteStatement(upsertStatement, columnList,
upsertOp, true);
}
+
if (updateCount != 1) {
throw new IllegalStateException("Unexpected updateCount " + updateCount);
}
}
+ private Operation getOppositeOperation(Operation operation) {
+ return operation.isUpdate() ? Operation.CREATE : Operation.UPDATE;
+ }
+
private int executeWriteStatement(PreparedStatement statement,
List<ColumnValue> columnList,
Operation operation, boolean handleException) {
+ int updateCount = 0;
synchronized (statement) {
try {
- int idx = 0;
- for (ColumnValue columnValue : columnList) {
- idx++;
- statement.setObject(idx, columnValue.getValue());
- }
- return statement.executeUpdate();
+ setValuesInStatement(statement, columnList);
+ updateCount = statement.executeUpdate();
} catch (SQLException e) {
if (handleException || operation.isDestroy()) {
handleSQLException(e);
}
- return 0;
} finally {
clearStatementParameters(statement);
}
}
+ return updateCount;
}
private void clearStatementParameters(PreparedStatement statement) {
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
