Skip to content

Commit

Permalink
GEODE-6156: add --id option to create jdbc-mapping (apache#2969)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dschneider-pivotal
dschneider-pivotal authored Dec 11, 2018
1 parent d7e7e87 commit 56c3593
Show file tree
Hide file tree
Showing 22 changed files with 313 additions and 99 deletions.
50 changes: 41 additions & 9 deletions ...t/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public abstract class TableMetaDataManagerIntegrationTest {
public void setup() throws Exception {
connection = getConnection();
statement = connection.createStatement();
createTable();
manager = new TableMetaDataManager();
}

Expand Down Expand Up @@ -71,39 +70,72 @@ protected void createTable() throws SQLException {
statement.execute("CREATE TABLE " + REGION_TABLE_NAME + " (" + quote + "id" + quote
+ " VARCHAR(10) primary key not null," + quote + "name" + quote + " VARCHAR(10)," + quote
+ "age" + quote + " int)");
}

protected void createTableWithNoPrimaryKey() throws SQLException {
DatabaseMetaData metaData = connection.getMetaData();
String quote = metaData.getIdentifierQuoteString();
statement.execute("CREATE TABLE " + REGION_TABLE_NAME + " (" + quote + "nonprimaryid" + quote
+ " VARCHAR(10)," + quote + "name" + quote + " VARCHAR(10)," + quote
+ "age" + quote + " int)");
}

@Test
public void validateKeyColumnName() {
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME);
public void validateKeyColumnName() throws SQLException {
createTable();
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME, null);

String keyColumnName = metaData.getKeyColumnName();

assertThat(keyColumnName).isEqualTo("id");
}

@Test
public void validateColumnDataTypeForName() {
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME);
public void validateKeyColumnNameOnNonPrimaryKey() throws SQLException {
createTableWithNoPrimaryKey();
TableMetaDataView metaData =
manager.getTableMetaDataView(connection, REGION_TABLE_NAME, "nonprimaryid");

String keyColumnName = metaData.getKeyColumnName();

assertThat(keyColumnName).isEqualTo("nonprimaryid");
}

@Test
public void validateKeyColumnNameOnNonPrimaryKeyWithInExactMatch() throws SQLException {
createTableWithNoPrimaryKey();
TableMetaDataView metaData =
manager.getTableMetaDataView(connection, REGION_TABLE_NAME, "NonPrimaryId");

String keyColumnName = metaData.getKeyColumnName();

assertThat(keyColumnName).isEqualTo("NonPrimaryId");
}

@Test
public void validateColumnDataTypeForName() throws SQLException {
createTable();
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME, null);

int nameDataType = metaData.getColumnDataType("name");

assertThat(nameDataType).isEqualTo(Types.VARCHAR);
}

@Test
public void validateColumnDataTypeForId() {
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME);
public void validateColumnDataTypeForId() throws SQLException {
createTable();
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME, null);

int nameDataType = metaData.getColumnDataType("id");

assertThat(nameDataType).isEqualTo(Types.VARCHAR);
}

@Test
public void validateColumnDataTypeForAge() {
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME);
public void validateColumnDataTypeForAge() throws SQLException {
createTable();
TableMetaDataView metaData = manager.getTableMetaDataView(connection, REGION_TABLE_NAME, null);

int nameDataType = metaData.getColumnDataType("age");

Expand Down
2 changes: 1 addition & 1 deletion .../src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestConfigService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ private static InternalCache createMockCache() {

private static RegionMapping createRegionMapping(String pdxClassName) {
return new RegionMapping(REGION_NAME, pdxClassName, REGION_TABLE_NAME,
CONNECTION_CONFIG_NAME);
CONNECTION_CONFIG_NAME, null);
}
}
6 changes: 5 additions & 1 deletion ...est/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__DATA_SOURCE_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__ID_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__PDX_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__REGION_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__SYNCHRONOUS_NAME;
Expand Down Expand Up @@ -183,6 +184,7 @@ public void createMappingUpdatesServiceAndClusterConfig(String regionName) {
csb.addOption(CREATE_MAPPING__DATA_SOURCE_NAME, "connection");
csb.addOption(CREATE_MAPPING__TABLE_NAME, "myTable");
csb.addOption(CREATE_MAPPING__PDX_NAME, "myPdxClass");
csb.addOption(CREATE_MAPPING__ID_NAME, "myId");

gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();

Expand All @@ -191,6 +193,7 @@ public void createMappingUpdatesServiceAndClusterConfig(String regionName) {
assertThat(mapping.getDataSourceName()).isEqualTo("connection");
assertThat(mapping.getTableName()).isEqualTo("myTable");
assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
assertThat(mapping.getIds()).isEqualTo("myId");
validateRegionAlteredOnServer(regionName, false);
validateAsyncEventQueueCreatedOnServer(regionName, false);
});
Expand All @@ -200,6 +203,7 @@ public void createMappingUpdatesServiceAndClusterConfig(String regionName) {
assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
assertThat(regionMapping.getTableName()).isEqualTo("myTable");
assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
assertThat(regionMapping.getIds()).isEqualTo("myId");
validateRegionAlteredInClusterConfig(regionName, false);
validateAsyncEventQueueCreatedInClusterConfig(regionName, false);
});
Expand Down Expand Up @@ -342,7 +346,7 @@ public void createMappingWithoutPdxNameFails(String regionName) {
// NOTE: --table is optional so it should not be in the output but it is. See GEODE-3468.
gfsh.executeAndAssertThat(csb.toString()).statusIsError()
.containsOutput(
"You should specify option (--table, --pdx-name, --synchronous) for this command");
"You should specify option (--table, --pdx-name, --synchronous, --id) for this command");
}

@Test
Expand Down
6 changes: 5 additions & 1 deletion ...t/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeMappingCommandDUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__DATA_SOURCE_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__ID_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__PDX_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__REGION_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__TABLE_NAME;
Expand Down Expand Up @@ -85,6 +86,7 @@ public void describesExistingMapping(String regionName) throws Exception {
csb.addOption(CREATE_MAPPING__DATA_SOURCE_NAME, "connection");
csb.addOption(CREATE_MAPPING__TABLE_NAME, "testTable");
csb.addOption(CREATE_MAPPING__PDX_NAME, "myPdxClass");
csb.addOption(CREATE_MAPPING__ID_NAME, "myId");

gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();

Expand All @@ -99,6 +101,7 @@ public void describesExistingMapping(String regionName) throws Exception {
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__DATA_SOURCE_NAME, "connection");
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__TABLE_NAME, "testTable");
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__PDX_NAME, "myPdxClass");
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__ID_NAME, "myId");
}

@Test
Expand Down Expand Up @@ -139,13 +142,14 @@ public void reportConfigurationFoundOnMember() throws Exception {
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__DATA_SOURCE_NAME, "connection");
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__TABLE_NAME, "testTable");
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__PDX_NAME, "myPdxClass");
commandResultAssert.containsKeyValuePair(CREATE_MAPPING__ID_NAME, "myId");
}

private void createRegionMapping() throws RegionMappingExistsException {
InternalCache cache = ClusterStartupRule.getCache();
JdbcConnectorService service = cache.getService(JdbcConnectorService.class);
service.createRegionMapping(new RegionMapping(REGION_NAME, "myPdxClass",
"testTable", "connection"));
"testTable", "connection", "myId"));
assertThat(service.getMappingForRegion(REGION_NAME)).isNotNull();
}
}
2 changes: 1 addition & 1 deletion ...dTest/java/org/apache/geode/connectors/jdbc/internal/cli/ListMappingCommandDUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ private void createNRegionMappings(int N) throws RegionMappingExistsException {
for (int i = 1; i <= N; i++) {
String name = regionName + "-" + i;
service.createRegionMapping(
new RegionMapping(name, "x.y.MyPdxClass", "table", "connection"));
new RegionMapping(name, "x.y.MyPdxClass", "table", "connection", null));
assertThat(service.getMappingForRegion(name)).isNotNull();
}
}
Expand Down
4 changes: 2 additions & 2 deletions geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public <K, V> PdxInstance read(Region<K, V> region, K key) throws SQLException {
PdxInstance result;
try (Connection connection = getConnection(regionMapping.getDataSourceName())) {
TableMetaDataView tableMetaData = this.tableMetaDataManager.getTableMetaDataView(connection,
regionMapping.getRegionToTableName());
regionMapping.getRegionToTableName(), regionMapping.getIds());
EntryColumnData entryColumnData =
getEntryColumnData(tableMetaData, regionMapping, key, null, Operation.GET);
try (PreparedStatement statement =
Expand Down Expand Up @@ -163,7 +163,7 @@ public <K, V> void write(Region<K, V> region, Operation operation, K key, PdxIns

try (Connection connection = getConnection(regionMapping.getDataSourceName())) {
TableMetaDataView tableMetaData = this.tableMetaDataManager.getTableMetaDataView(connection,
regionMapping.getRegionToTableName());
regionMapping.getRegionToTableName(), regionMapping.getIds());
EntryColumnData entryColumnData =
getEntryColumnData(tableMetaData, regionMapping, key, value, operation);
int updateCount = 0;
Expand Down
40 changes: 35 additions & 5 deletions ...nectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,20 @@ public class TableMetaDataManager {
private final ConcurrentMap<String, TableMetaDataView> tableToMetaDataMap =
new ConcurrentHashMap<>();

public TableMetaDataView getTableMetaDataView(Connection connection, String tableName) {
public TableMetaDataView getTableMetaDataView(Connection connection, String tableName,
String ids) {
return tableToMetaDataMap.computeIfAbsent(tableName,
k -> computeTableMetaDataView(connection, k));
k -> computeTableMetaDataView(connection, k, ids));
}

private TableMetaDataView computeTableMetaDataView(Connection connection, String tableName) {
private TableMetaDataView computeTableMetaDataView(Connection connection, String tableName,
String ids) {
TableMetaData result;
try {
DatabaseMetaData metaData = connection.getMetaData();
try (ResultSet tables = metaData.getTables(null, null, "%", null)) {
String realTableName = getTableNameFromMetaData(tableName, tables);
String key = getPrimaryKeyColumnNameFromMetaData(realTableName, metaData);
String key = getPrimaryKeyColumnNameFromMetaData(realTableName, metaData, ids);
String quoteString = metaData.getIdentifierQuoteString();
if (quoteString == null) {
quoteString = "";
Expand Down Expand Up @@ -83,8 +85,16 @@ private String getTableNameFromMetaData(String tableName, ResultSet tables) thro
return result;
}

private String getPrimaryKeyColumnNameFromMetaData(String tableName, DatabaseMetaData metaData)
private String getPrimaryKeyColumnNameFromMetaData(String tableName, DatabaseMetaData metaData,
String ids)
throws SQLException {
if (ids != null && !ids.isEmpty()) {
if (!doesColumnExistInTable(tableName, metaData, ids)) {
throw new JdbcConnectorException(
"The table " + tableName + " does not have a column named " + ids);
}
return ids;
}
try (ResultSet primaryKeys = metaData.getPrimaryKeys(null, null, tableName)) {
if (!primaryKeys.next()) {
throw new JdbcConnectorException(
Expand All @@ -109,4 +119,24 @@ private void getDataTypesFromMetaData(String tableName, DatabaseMetaData metaDat
}
}
}

private boolean doesColumnExistInTable(String tableName, DatabaseMetaData metaData,
String columnName) throws SQLException {
int caseInsensitiveMatches = 0;
try (ResultSet columnData = metaData.getColumns(null, null, tableName, "%")) {
while (columnData.next()) {
String realColumnName = columnData.getString("COLUMN_NAME");
if (columnName.equals(realColumnName)) {
return true;
} else if (columnName.equalsIgnoreCase(realColumnName)) {
caseInsensitiveMatches++;
}
}
}
if (caseInsensitiveMatches > 1) {
throw new JdbcConnectorException(
"The table " + tableName + " has more than one column that matches " + columnName);
}
return caseInsensitiveMatches != 0;
}
}
9 changes: 7 additions & 2 deletions ...ors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ public class CreateMappingCommand extends SingleGfshCommand {
static final String CREATE_MAPPING__SYNCHRONOUS_NAME = "synchronous";
static final String CREATE_MAPPING__SYNCHRONOUS_NAME__HELP =
"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.";

public static String createAsyncEventQueueName(String regionPath) {
if (regionPath.startsWith("/")) {
Expand All @@ -86,14 +89,16 @@ public ResultModel createMapping(
help = CREATE_MAPPING__PDX_NAME__HELP) String pdxName,
@CliOption(key = CREATE_MAPPING__SYNCHRONOUS_NAME,
help = CREATE_MAPPING__SYNCHRONOUS_NAME__HELP,
specifiedDefaultValue = "true", unspecifiedDefaultValue = "false") boolean synchronous) {
specifiedDefaultValue = "true", unspecifiedDefaultValue = "false") boolean synchronous,
@CliOption(key = CREATE_MAPPING__ID_NAME,
help = CREATE_MAPPING__ID_NAME__HELP) String id) {
if (regionName.startsWith("/")) {
regionName = regionName.substring(1);
}

// input
Set<DistributedMember> targetMembers = getMembers(null, null);
RegionMapping mapping = new RegionMapping(regionName, pdxName, table, dataSourceName);
RegionMapping mapping = new RegionMapping(regionName, pdxName, table, dataSourceName, id);

try {
ConfigurationPersistenceService configurationPersistenceService =
Expand Down
2 changes: 2 additions & 0 deletions ...s/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeMappingCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.apache.geode.connectors.jdbc.internal.cli;

import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__DATA_SOURCE_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__ID_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__PDX_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__REGION_NAME;
import static org.apache.geode.connectors.jdbc.internal.cli.CreateMappingCommand.CREATE_MAPPING__TABLE_NAME;
Expand Down Expand Up @@ -89,6 +90,7 @@ private void fillResultData(RegionMapping mapping, ResultModel resultModel) {
sectionModel.addData(CREATE_MAPPING__DATA_SOURCE_NAME, mapping.getDataSourceName());
sectionModel.addData(CREATE_MAPPING__TABLE_NAME, mapping.getTableName());
sectionModel.addData(CREATE_MAPPING__PDX_NAME, mapping.getPdxName());
sectionModel.addData(CREATE_MAPPING__ID_NAME, mapping.getIds());
}

@CliAvailabilityIndicator({DESCRIBE_MAPPING})
Expand Down
Loading

0 comments on commit 56c3593

Please sign in to comment.