You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jc...@apache.org on 2019/04/08 18:46:16 UTC
[geode] branch develop updated: GEODE-6592: Added --if-not-exists
flag to CreateMappingCommand (#3402)
This is an automated email from the ASF dual-hosted git repository.
jchen21 pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new ed43b95 GEODE-6592: Added --if-not-exists flag to CreateMappingCommand (#3402)
ed43b95 is described below
commit ed43b95860bd37b330fafd5066b1474acc1ff2f7
Author: BenjaminPerryRoss <39...@users.noreply.github.com>
AuthorDate: Mon Apr 8 11:45:58 2019 -0700
GEODE-6592: Added --if-not-exists flag to CreateMappingCommand (#3402)
* Added --if-not-exists flag to CreateMappingCommand and Tests
Co-authored-by: Benjamin Ross <br...@pivotal.io>
Co-authored-by: Xiaojian Zhou <gz...@pivotal.io>
Co-authored-by: Jianxia Chen <jc...@pivotal.io>
---
.../cli/CreateMappingCommandDUnitTest.java | 24 ++++++++-
...reateMappingCommandForProxyRegionDUnitTest.java | 5 ++
.../jdbc/internal/cli/CreateMappingCommand.java | 36 ++++++++++++-
.../internal/cli/CreateMappingCommandTest.java | 63 ++++++++++++++++------
4 files changed, 110 insertions(+), 18 deletions(-)
diff --git a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
index 06043f1..ffa121e 100644
--- a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
+++ b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
@@ -302,6 +302,28 @@ public class CreateMappingCommandDUnitTest {
}
@Test
+ public void createMappingSucceedsWithWarningWhenMappingAlreadyExistsAndIfNotExistsIsTrue() {
+ String regionName = REGION_NAME;
+ setupReplicate(regionName);
+ CommandStringBuilder csb = new CommandStringBuilder(CREATE_MAPPING);
+ csb.addOption(REGION_NAME, regionName);
+ csb.addOption(DATA_SOURCE_NAME, "connection");
+ csb.addOption(TABLE_NAME, "myTable");
+ csb.addOption(PDX_NAME, IdAndName.class.getName());
+ csb.addOption(ID_NAME, "myId");
+ csb.addOption(SCHEMA_NAME, "mySchema");
+
+ gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();
+
+ // do it again and expect to fail
+ gfsh.executeAndAssertThat(csb.toString()).statusIsError()
+ .containsOutput("A JDBC mapping for " + regionName + " already exists.");
+
+ csb.addOption(CliStrings.IFNOTEXISTS, "true");
+ gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess().containsOutput("Skipping: ");
+ }
+
+ @Test
public void createMappingReplicatedUpdatesServiceAndClusterConfigForServerGroup() {
String regionName = GROUP1_REGION;
setupGroupReplicate(regionName, TEST_GROUP1);
@@ -1209,7 +1231,7 @@ public class CreateMappingCommandDUnitTest {
// 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, --pdx-class-file, --synchronous, --id, --catalog, --schema, --group) for this command");
+ "You should specify option (--table, --pdx-name, --pdx-class-file, --synchronous, --id, --catalog, --schema, --if-not-exists, --group) for this command");
}
@Test
diff --git a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandForProxyRegionDUnitTest.java b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandForProxyRegionDUnitTest.java
index 0902f31..e8f795f 100644
--- a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandForProxyRegionDUnitTest.java
+++ b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandForProxyRegionDUnitTest.java
@@ -64,6 +64,7 @@ import org.apache.geode.connectors.util.internal.MappingConstants;
import org.apache.geode.distributed.internal.InternalLocator;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.jndi.JNDIInvoker;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
import org.apache.geode.pdx.FieldType;
import org.apache.geode.pdx.PdxReader;
@@ -455,6 +456,10 @@ public class CreateMappingCommandForProxyRegionDUnitTest {
gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess()
.containsOutput(MappingConstants.THERE_IS_NO_JDBC_MAPPING_ON_PROXY_REGION);
+ // do create jdbc-mapping again
+ csb.addOption(CliStrings.IFNOTEXISTS, "true");
+ gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess().containsOutput("Skipping: ");
+
server1.invoke(() -> {
RegionMapping mapping = getRegionMappingFromService(regionName);
assertValidMappingOnServer(mapping, regionName, false, isPR);
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 8784d9a..84b08d5 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
@@ -101,6 +101,9 @@ public class CreateMappingCommand extends SingleGfshCommand {
private static final String CREATE_MAPPING__PDX_CLASS_FILE = MappingConstants.PDX_CLASS_FILE;
private static final String CREATE_MAPPING__PDX_CLASS_FILE__HELP =
"The file that contains the PDX class. It must be a file with the \".jar\" or \".class\" extension. By default, the PDX class must be on the server's classpath or gfsh deployed.";
+ public static final String CREATE_MAPPING__IFNOTEXISTS__HELP =
+ "By default, an attempt to create a duplicate jdbc mapping is reported as an error. If this option is specified without a value or is specified with a value of true, then gfsh displays a \"Skipping...\" acknowledgement, but does not throw an error.";
+ static final String IF_NOT_EXISTS_SKIPPING_EXCEPTION_MESSAGE = "Skipping: ";
@CliCommand(value = CREATE_MAPPING, help = CREATE_MAPPING__HELP)
@CliMetaData(
@@ -127,6 +130,9 @@ public class CreateMappingCommand extends SingleGfshCommand {
help = CREATE_MAPPING__CATALOG_NAME__HELP) String catalog,
@CliOption(key = CREATE_MAPPING__SCHEMA_NAME,
help = CREATE_MAPPING__SCHEMA_NAME__HELP) String schema,
+ @CliOption(key = CliStrings.IFNOTEXISTS,
+ specifiedDefaultValue = "true", unspecifiedDefaultValue = "false",
+ help = CREATE_MAPPING__IFNOTEXISTS__HELP) boolean ifNotExists,
@CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
optionContext = ConverterHint.MEMBERGROUP,
help = CREATE_MAPPING__GROUPS_NAME__HELP) String[] groups)
@@ -166,9 +172,15 @@ public class CreateMappingCommand extends SingleGfshCommand {
checkForCacheLoader(regionName, regionConfig);
checkForCacheWriter(regionName, synchronous, regionConfig);
checkForAsyncQueue(regionName, synchronous, cacheConfig);
+ checkForAEQIdForAccessor(regionName, synchronous, regionConfig);
}
} catch (PreconditionException ex) {
- return ResultModel.createError(ex.getMessage());
+ if (ifNotExists) {
+ return ResultModel
+ .createInfo(IF_NOT_EXISTS_SKIPPING_EXCEPTION_MESSAGE + ex.getMessage());
+ } else {
+ return ResultModel.createError(ex.getMessage());
+ }
}
if (pdxClassFile != null) {
@@ -289,8 +301,30 @@ public class CreateMappingCommand extends SingleGfshCommand {
}
}
+ private void checkForAEQIdForAccessor(String regionName, boolean synchronous,
+ RegionConfig regionConfig)
+ throws PreconditionException {
+ RegionAttributesType regionAttributesType = regionConfig.getRegionAttributes();
+ if (!synchronous && regionAttributesType != null) {
+ boolean isAccessor = MappingCommandUtils.isAccessor(regionAttributesType);
+ if (!isAccessor) {
+ return;
+ } else {
+ String queueName = MappingCommandUtils.createAsyncEventQueueName(regionName);
+ if (regionAttributesType.getAsyncEventQueueIds() != null && regionAttributesType
+ .getAsyncEventQueueIds().contains(queueName)) {
+ throw new PreconditionException(
+ "An async-event-queue named " + queueName + " must not already exist.");
+ }
+ }
+ }
+ }
+
@Override
public boolean updateConfigForGroup(String group, CacheConfig cacheConfig, Object element) {
+ if (element == null) {
+ return false;
+ }
Object[] arguments = (Object[]) element;
RegionMapping regionMapping = (RegionMapping) arguments[0];
boolean synchronous = (Boolean) arguments[1];
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java
index ffbd419..5afe982 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java
@@ -164,7 +164,7 @@ public class CreateMappingCommandTest {
String schema = "schema";
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, null);
+ tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.OK);
Object[] results = (Object[]) result.getConfigObject();
@@ -192,7 +192,7 @@ public class CreateMappingCommandTest {
String[] groups = {group1Name, group2Name};
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, groups);
+ tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, false, groups);
assertThat(result.getStatus()).isSameAs(Result.Status.OK);
Object[] results = (Object[]) result.getConfigObject();
@@ -221,7 +221,7 @@ public class CreateMappingCommandTest {
this.fieldMappings.add(new FieldMapping("pdx2", "pdx2type", "jdbc2", "jdbc2type", false));
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, null);
+ tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.OK);
Object[] results = (Object[]) result.getConfigObject();
@@ -241,7 +241,7 @@ public class CreateMappingCommandTest {
this.fieldMappings.add(new FieldMapping("pdx2", "pdx2type", "jdbc2", "jdbc2type", false));
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, null);
+ tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.OK);
Object[] results = (Object[]) result.getConfigObject();
@@ -268,7 +268,7 @@ public class CreateMappingCommandTest {
String schema = "schema";
assertThatThrownBy(() -> createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, null))
+ tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, false, null))
.isInstanceOf(IllegalStateException.class);
}
@@ -302,7 +302,7 @@ public class CreateMappingCommandTest {
preconditionOutput[0] = computedIds;
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, null);
+ tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.OK);
Object[] results = (Object[]) result.getConfigObject();
@@ -321,7 +321,7 @@ public class CreateMappingCommandTest {
when(preconditionCheckResults.getStatusMessage()).thenReturn("precondition check failed");
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, null);
+ tableName, pdxClass, pdxClassFile, false, ids, catalog, schema, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
assertThat(result.toString()).contains("precondition check failed");
@@ -333,7 +333,7 @@ public class CreateMappingCommandTest {
results.add(successFunctionResult);
ResultModel result = createRegionMappingCommand.createMapping("/" + regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, false, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.OK);
Object[] results = (Object[]) result.getConfigObject();
@@ -348,7 +348,7 @@ public class CreateMappingCommandTest {
results.clear();
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, false, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
}
@@ -359,7 +359,7 @@ public class CreateMappingCommandTest {
doReturn(null).when(createRegionMappingCommand).getConfigurationPersistenceService();
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, false, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
assertThat(result.toString()).contains("Cluster Configuration must be enabled.");
@@ -378,7 +378,7 @@ public class CreateMappingCommandTest {
when(cacheConfig.getRegions()).thenReturn(Collections.emptyList());
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, false, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
assertThat(result.toString())
@@ -408,13 +408,44 @@ public class CreateMappingCommandTest {
when(matchingRegion.getCustomRegionElements()).thenReturn(customList);
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, false, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
assertThat(result.toString()).contains("A JDBC mapping for " + regionName + " already exists.");
}
@Test
+ public void createsMappingIsSkippedWhenRegionMappingExistsWithIfNotExistsTrue()
+ throws IOException {
+ results.add(successFunctionResult);
+ ConfigurationPersistenceService configurationPersistenceService =
+ mock(ConfigurationPersistenceService.class);
+ doReturn(configurationPersistenceService).when(createRegionMappingCommand)
+ .getConfigurationPersistenceService();
+ when(configurationPersistenceService
+ .getCacheConfig(ConfigurationPersistenceService.CLUSTER_CONFIG)).thenReturn(cacheConfig);
+ List<RegionConfig> list = new ArrayList<>();
+ list.add(matchingRegion);
+ when(cacheConfig.getRegions()).thenReturn(list);
+ RegionAttributesType loaderAttribute = mock(RegionAttributesType.class);
+ DeclarableType loaderDeclarable = mock(DeclarableType.class);
+ when(loaderDeclarable.getClassName()).thenReturn(null);
+ when(loaderAttribute.getCacheLoader()).thenReturn(loaderDeclarable);
+ when(matchingRegion.getRegionAttributes()).thenReturn(loaderAttribute);
+ List<CacheElement> customList = new ArrayList<>();
+ RegionMapping existingMapping = mock(RegionMapping.class);
+ customList.add(existingMapping);
+ when(matchingRegion.getCustomRegionElements()).thenReturn(customList);
+
+ ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
+ tableName, pdxClass, pdxClassFile, false, null, null, null, true, null);
+
+ assertThat(result.getStatus()).isSameAs(Result.Status.OK);
+ assertThat(result.toString())
+ .contains("Skipping: A JDBC mapping for " + regionName + " already exists.");
+ }
+
+ @Test
public void createsMappingReturnsStatusERRORWhenClusterConfigRegionHasLoader()
throws IOException {
results.add(successFunctionResult);
@@ -434,7 +465,7 @@ public class CreateMappingCommandTest {
when(matchingRegion.getRegionAttributes()).thenReturn(loaderAttribute);
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, false, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
assertThat(result.toString()).contains("The existing region " + regionName
@@ -461,7 +492,7 @@ public class CreateMappingCommandTest {
when(matchingRegion.getRegionAttributes()).thenReturn(writerAttribute);
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, true, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, true, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
assertThat(result.toString()).contains("The existing region " + regionName
@@ -492,7 +523,7 @@ public class CreateMappingCommandTest {
when(cacheConfig.getAsyncEventQueues()).thenReturn(asyncEventQueues);
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, true, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, true, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.OK);
}
@@ -522,7 +553,7 @@ public class CreateMappingCommandTest {
when(cacheConfig.getAsyncEventQueues()).thenReturn(asyncEventQueues);
ResultModel result = createRegionMappingCommand.createMapping(regionName, dataSourceName,
- tableName, pdxClass, pdxClassFile, false, null, null, null, null);
+ tableName, pdxClass, pdxClassFile, false, null, null, null, false, null);
assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
assertThat(result.toString())