geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jche...@apache.org
Subject [geode] branch develop updated: GEODE-6592: Added --if-not-exists flag to CreateMappingCommand (#3402)
Date Mon, 08 Apr 2019 18:46:16 GMT
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 <39068135+BenjaminPerryRoss@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 <bross@pivotal.io>
    Co-authored-by: Xiaojian Zhou <gzhou@pivotal.io>
    Co-authored-by: Jianxia Chen <jchen@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())


Mime
View raw message