You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kh...@apache.org on 2018/03/19 22:11:46 UTC

[geode] branch develop updated (4b5d42d -> 3fa3074)

This is an automated email from the ASF dual-hosted git repository.

khowe pushed a change to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git.


    from 4b5d42d  GEODE-4888 Document security-udp-dhalgo property (#1642)
     new bafab6b  GEODE-4771: Defaults in ConfigurePDXCommand
     new 54b18ed  GEODE-4771: Changes requested by reviewers.
     new 3fa3074  GEODE-4771: Internal API was renamed

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../internal/cli/commands/ConfigurePDXCommand.java | 181 ++++++----
 .../management/internal/cli/i18n/CliStrings.java   |   5 +-
 .../ConfigurePDXCommandIntegrationTest.java        | 135 ++++++++
 .../cli/commands/ConfigurePDXCommandTest.java      | 382 +++++++++++++++++++++
 4 files changed, 631 insertions(+), 72 deletions(-)
 create mode 100644 geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java
 create mode 100644 geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java

-- 
To stop receiving notification emails like this one, please contact
khowe@apache.org.

[geode] 02/03: GEODE-4771: Changes requested by reviewers.

Posted by kh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

khowe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 54b18eded8802561b8607b4bcdc1525915436e85
Author: Juan Jose Ramos Cassella <ju...@gmail.com>
AuthorDate: Wed Mar 14 09:14:25 2018 +0000

    GEODE-4771: Changes requested by reviewers.
---
 .../internal/cli/commands/ConfigurePDXCommand.java | 118 ++++++++++-----------
 .../ConfigurePDXCommandIntegrationTest.java        |  17 +--
 .../cli/commands/ConfigurePDXCommandTest.java      |  59 ++++++-----
 3 files changed, 97 insertions(+), 97 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java
index 43719f9..1868c15 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java
@@ -90,75 +90,68 @@ public class ConfigurePDXCommand extends GfshCommand {
           help = CliStrings.CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES__HELP) String[] portableClassesPatterns) {
 
     Result result;
+    ReflectionBasedAutoSerializer autoSerializer;
+    CacheCreation cache = getCacheCreation(true);
+    InfoResultData ird = ResultBuilder.createInfoResultData();
 
-    try {
-      ReflectionBasedAutoSerializer autoSerializer;
-      CacheCreation cache = getCacheCreation(true);
-      InfoResultData ird = ResultBuilder.createInfoResultData();
+    if (!getAllNormalMembers().isEmpty()) {
+      ird.addLine(CliStrings.CONFIGURE_PDX__NORMAL__MEMBERS__WARNING);
+    }
 
-      if (!getAllNormalMembers().isEmpty()) {
-        ird.addLine(CliStrings.CONFIGURE_PDX__NORMAL__MEMBERS__WARNING);
-      }
+    // Set persistent and the disk-store
+    if (diskStore != null) {
+      cache.setPdxPersistent(true);
+      ird.addLine(CliStrings.CONFIGURE_PDX__PERSISTENT + " = " + cache.getPdxPersistent());
 
-      // Set persistent and the disk-store
-      if (diskStore != null) {
-        cache.setPdxPersistent(true);
-        ird.addLine(CliStrings.CONFIGURE_PDX__PERSISTENT + " = " + cache.getPdxPersistent());
-
-        if (!diskStore.equals("")) {
-          cache.setPdxDiskStore(diskStore);
-          ird.addLine(CliStrings.CONFIGURE_PDX__DISKSTORE + " = " + cache.getPdxDiskStore());
-        } else {
-          ird.addLine(CliStrings.CONFIGURE_PDX__DISKSTORE + " = " + "DEFAULT");
-        }
+      if (!diskStore.equals("")) {
+        cache.setPdxDiskStore(diskStore);
+        ird.addLine(CliStrings.CONFIGURE_PDX__DISKSTORE + " = " + cache.getPdxDiskStore());
       } else {
-        cache.setPdxPersistent(CacheConfig.DEFAULT_PDX_PERSISTENT);
-        ird.addLine(CliStrings.CONFIGURE_PDX__PERSISTENT + " = " + cache.getPdxPersistent());
-      }
-
-      // Set read-serialized
-      if (readSerialized != null) {
-        cache.setPdxReadSerialized(readSerialized);
-      } else {
-        cache.setPdxReadSerialized(CacheConfig.DEFAULT_PDX_READ_SERIALIZED);
+        ird.addLine(CliStrings.CONFIGURE_PDX__DISKSTORE + " = " + "DEFAULT");
       }
+    } else {
+      cache.setPdxPersistent(CacheConfig.DEFAULT_PDX_PERSISTENT);
+      ird.addLine(CliStrings.CONFIGURE_PDX__PERSISTENT + " = " + cache.getPdxPersistent());
+    }
 
-      ird.addLine(
-          CliStrings.CONFIGURE_PDX__READ__SERIALIZED + " = " + cache.getPdxReadSerialized());
+    // Set read-serialized
+    if (readSerialized != null) {
+      cache.setPdxReadSerialized(readSerialized);
+    } else {
+      cache.setPdxReadSerialized(CacheConfig.DEFAULT_PDX_READ_SERIALIZED);
+    }
 
-      // Set ignoreUnreadFields
-      if (ignoreUnreadFields != null) {
-        cache.setPdxIgnoreUnreadFields(ignoreUnreadFields);
-      } else {
-        cache.setPdxIgnoreUnreadFields(CacheConfig.DEFAULT_PDX_IGNORE_UNREAD_FIELDS);
-      }
+    ird.addLine(CliStrings.CONFIGURE_PDX__READ__SERIALIZED + " = " + cache.getPdxReadSerialized());
 
-      ird.addLine(CliStrings.CONFIGURE_PDX__IGNORE__UNREAD_FIELDS + " = "
-          + cache.getPdxIgnoreUnreadFields());
+    // Set ignoreUnreadFields
+    if (ignoreUnreadFields != null) {
+      cache.setPdxIgnoreUnreadFields(ignoreUnreadFields);
+    } else {
+      cache.setPdxIgnoreUnreadFields(CacheConfig.DEFAULT_PDX_IGNORE_UNREAD_FIELDS);
+    }
 
-      // Auto Serializer Configuration
-      if (portableClassesPatterns != null) {
-        autoSerializer = createReflectionBasedAutoSerializer(true, portableClassesPatterns);
-        cache.setPdxSerializer(autoSerializer);
-        ird.addLine("PDX Serializer = " + cache.getPdxSerializer().getClass().getName());
-        ird.addLine("Portable Classes = " + Arrays.toString(portableClassesPatterns));
-      }
+    ird.addLine(
+        CliStrings.CONFIGURE_PDX__IGNORE__UNREAD_FIELDS + " = " + cache.getPdxIgnoreUnreadFields());
 
-      if (nonPortableClassesPatterns != null) {
-        autoSerializer = createReflectionBasedAutoSerializer(false, nonPortableClassesPatterns);
-        cache.setPdxSerializer(autoSerializer);
-        ird.addLine("PDX Serializer = " + cache.getPdxSerializer().getClass().getName());
-        ird.addLine("Non Portable Classes = " + Arrays.toString(nonPortableClassesPatterns));
-      }
+    // Auto Serializer Configuration
+    if (portableClassesPatterns != null) {
+      autoSerializer = createReflectionBasedAutoSerializer(true, portableClassesPatterns);
+      cache.setPdxSerializer(autoSerializer);
+      ird.addLine("PDX Serializer = " + cache.getPdxSerializer().getClass().getName());
+      ird.addLine("Portable Classes = " + Arrays.toString(portableClassesPatterns));
+    }
 
-      XmlEntity xmlEntity = createXmlEntity(cache);
-      result = ResultBuilder.buildResult(ird);
-      persistClusterConfiguration(result,
-          () -> getSharedConfiguration().addXmlEntity(xmlEntity, null));
-    } catch (Exception e) {
-      return ResultBuilder.createGemFireErrorResult(e.getMessage());
+    if (nonPortableClassesPatterns != null) {
+      autoSerializer = createReflectionBasedAutoSerializer(false, nonPortableClassesPatterns);
+      cache.setPdxSerializer(autoSerializer);
+      ird.addLine("PDX Serializer = " + cache.getPdxSerializer().getClass().getName());
+      ird.addLine("Non Portable Classes = " + Arrays.toString(nonPortableClassesPatterns));
     }
 
+    XmlEntity xmlEntity = createXmlEntity(cache);
+    result = ResultBuilder.buildResult(ird);
+    persistClusterConfiguration(result,
+        () -> getSharedConfiguration().addXmlEntity(xmlEntity, null));
     return result;
   }
 
@@ -169,14 +162,13 @@ public class ConfigurePDXCommand extends GfshCommand {
 
     @Override
     public Result preExecution(GfshParseResult parseResult) {
-      Object portableClassesPatterns =
-          parseResult.getParamValue(CliStrings.CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES);
-      Object nonPortableClassesPatterns =
-          parseResult.getParamValue(CliStrings.CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES);
-
-      if ((nonPortableClassesPatterns != null && ((String[]) nonPortableClassesPatterns).length > 0)
-          && (portableClassesPatterns != null && ((String[]) portableClassesPatterns).length > 0)) {
+      String[] portableClassesPatterns = (String[]) parseResult
+          .getParamValue(CliStrings.CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES);
+      String[] nonPortableClassesPatterns =
+          (String[]) parseResult.getParamValue(CliStrings.CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES);
 
+      if ((nonPortableClassesPatterns != null && nonPortableClassesPatterns.length > 0)
+          && (portableClassesPatterns != null && portableClassesPatterns.length > 0)) {
         return ResultBuilder.createUserErrorResult(CliStrings.CONFIGURE_PDX__ERROR__MESSAGE);
       }
 
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java
index b1288b0..d5f56c8 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java
@@ -42,12 +42,6 @@ public class ConfigurePDXCommandIntegrationTest {
   }
 
   @Test
-  @Ignore("See https://issues.apache.org/jira/browse/GEODE-4794")
-  public void commandShouldSucceedWhenUsingDefaults() {
-    gfsh.executeAndAssertThat(BASE_COMMAND_STRING).statusIsSuccess().hasNoFailToPersistError();
-  }
-
-  @Test
   public void commandShouldFailWhenNotConnected() throws Exception {
     gfsh.disconnect();
     gfsh.executeAndAssertThat(BASE_COMMAND_STRING).statusIsError().containsOutput("Command",
@@ -55,6 +49,17 @@ public class ConfigurePDXCommandIntegrationTest {
   }
 
   @Test
+  @Ignore("See https://issues.apache.org/jira/browse/GEODE-4794")
+  public void commandShouldSucceedWhenUsingDefaults() {
+    gfsh.executeAndAssertThat(BASE_COMMAND_STRING).statusIsSuccess().hasNoFailToPersistError();
+
+    String sharedConfigXml = locator.getLocator().getSharedConfiguration()
+        .getConfiguration("cluster").getCacheXmlContent();
+    assertThat(sharedConfigXml).contains(
+        "<pdx ignore-unread-fields=\"false\" persistent=\"false\" read-serialized=\"false\"></pdx>");
+  }
+
+  @Test
   public void commandShouldSucceedWhenConfiguringAutoSerializableClassesWithPersistence() {
     gfsh.executeAndAssertThat(BASE_COMMAND_STRING
         + "--read-serialized=true --disk-store=myDiskStore --ignore-unread-fields=true --auto-serializable-classes=com.company.DomainObject.*#identity=id")
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
index bfd5a61..9a4d87d 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
@@ -96,17 +96,20 @@ public class ConfigurePDXCommandTest {
     doThrow(new RuntimeException("Can't create CacheCreation.")).when(command)
         .getCacheCreation(anyBoolean());
     gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsError()
-        .containsOutput("Could not process command due to error. Can't create CacheCreation.");
+        .containsOutput("Could not process command due to error.")
+        .containsOutput("Can't create CacheCreation.");
     doReturn(cacheCreation).when(command).getCacheCreation(anyBoolean());
 
     doThrow(new RuntimeException("Can't find members.")).when(command).getAllNormalMembers();
     gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsError()
-        .containsOutput("Could not process command due to error. Can't find members.");
+        .containsOutput("Could not process command due to error.")
+        .containsOutput("Can't find members.");
     doReturn(Collections.emptySet()).when(command).getAllNormalMembers();
 
     doThrow(new RuntimeException("Can't create XmlEntity.")).when(command).createXmlEntity(any());
     gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsError()
-        .containsOutput("Could not process command due to error. Can't create XmlEntity.");
+        .containsOutput("Could not process command due to error.")
+        .containsOutput("Can't create XmlEntity.");
     doReturn(xmlEntity).when(command).createXmlEntity(any());
 
     doThrow(new RuntimeException("Can't create ReflectionBasedAutoSerializer.")).when(command)
@@ -114,13 +117,13 @@ public class ConfigurePDXCommandTest {
     gfshParserRule
         .executeAndAssertThat(command,
             BASE_COMMAND_STRING + "--auto-serializable-classes=" + new String[] {})
-        .statusIsError().containsOutput(
-            "Could not process command due to error. Can't create ReflectionBasedAutoSerializer.");
+        .statusIsError().containsOutput("Could not process command due to error.")
+        .containsOutput("Can't create ReflectionBasedAutoSerializer.");
     gfshParserRule
         .executeAndAssertThat(command,
             BASE_COMMAND_STRING + "--portable-auto-serializable-classes=" + new String[] {})
-        .statusIsError().containsOutput(
-            "Could not process command due to error. Can't create ReflectionBasedAutoSerializer.");
+        .statusIsError().containsOutput("Could not process command due to error.")
+        .containsOutput("Can't create ReflectionBasedAutoSerializer.");
 
     verify(command, times(0)).persistClusterConfiguration(any(), any());
   }
@@ -165,7 +168,7 @@ public class ConfigurePDXCommandTest {
         .statusIsSuccess().hasNoFailToPersistError()
         .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
-        .hasPersistenseConfigured(true, "DEFAULT", cacheCreation);
+        .hasPersistenceConfigured(true, "DEFAULT", cacheCreation);
 
     verify(cacheCreation, times(0)).setPdxSerializer(any());
     verify(command, times(1)).persistClusterConfiguration(any(), any());
@@ -179,7 +182,7 @@ public class ConfigurePDXCommandTest {
         .statusIsSuccess().hasNoFailToPersistError()
         .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
-        .hasPersistenseConfigured(true, "myDiskStore", cacheCreation);
+        .hasPersistenceConfigured(true, "myDiskStore", cacheCreation);
 
     verify(cacheCreation, times(0)).setPdxSerializer(any());
     verify(command, times(1)).persistClusterConfiguration(any(), any());
@@ -193,7 +196,7 @@ public class ConfigurePDXCommandTest {
         .statusIsSuccess().hasNoFailToPersistError()
         .hasReadSerializedConfigured(true, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
-        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+        .hasPersistenceConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
 
     verify(cacheCreation, times(0)).setPdxSerializer(any());
     verify(command, times(1)).persistClusterConfiguration(any(), any());
@@ -207,7 +210,7 @@ public class ConfigurePDXCommandTest {
         .statusIsSuccess().hasNoFailToPersistError()
         .hasReadSerializedConfigured(false, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
-        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+        .hasPersistenceConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
 
     verify(cacheCreation, times(0)).setPdxSerializer(any());
     verify(command, times(1)).persistClusterConfiguration(any(), any());
@@ -222,7 +225,7 @@ public class ConfigurePDXCommandTest {
         .statusIsSuccess().hasNoFailToPersistError()
         .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(true, cacheCreation)
-        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+        .hasPersistenceConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
 
     verify(cacheCreation, times(0)).setPdxSerializer(any());
     verify(command, times(1)).persistClusterConfiguration(any(), any());
@@ -237,7 +240,7 @@ public class ConfigurePDXCommandTest {
         .statusIsSuccess().hasNoFailToPersistError()
         .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(false, cacheCreation)
-        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+        .hasPersistenceConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
 
     verify(cacheCreation, times(0)).setPdxSerializer(any());
     verify(command, times(1)).persistClusterConfiguration(any(), any());
@@ -255,7 +258,7 @@ public class ConfigurePDXCommandTest {
         .statusIsSuccess().hasNoFailToPersistError()
         .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
-        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation)
+        .hasPersistenceConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation)
         .containsOutput("Portable Classes = [com.company.DomainObject.*#identity=id]")
         .containsOutput("PDX Serializer = org.apache.geode.pdx.ReflectionBasedAutoSerializer");
 
@@ -276,7 +279,7 @@ public class ConfigurePDXCommandTest {
         .hasReadSerializedConfigured(CacheConfig.DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
         .hasIgnoreUnreadFieldsConfigured(CacheConfig.DEFAULT_PDX_IGNORE_UNREAD_FIELDS,
             cacheCreation)
-        .hasPersistenseConfigured(CacheConfig.DEFAULT_PDX_PERSISTENT, null, cacheCreation)
+        .hasPersistenceConfigured(CacheConfig.DEFAULT_PDX_PERSISTENT, null, cacheCreation)
         .containsOutput("Non Portable Classes = [com.company.DomainObject.*#identity=id]")
         .containsOutput("PDX Serializer = org.apache.geode.pdx.ReflectionBasedAutoSerializer");
 
@@ -287,47 +290,47 @@ public class ConfigurePDXCommandTest {
 
   static class CustomGfshParserRule extends GfshParserRule {
     @Override
-    public <T> CustomResultAssert executeAndAssertThat(T instance, String command) {
+    public <T> PDXCommandResultAssert executeAndAssertThat(T instance, String command) {
       CommandResultAssert resultAssert = super.executeAndAssertThat(instance, command);;
 
-      return new CustomResultAssert(resultAssert.getCommandResult());
+      return new PDXCommandResultAssert(resultAssert.getCommandResult());
     }
   }
 
-  static class CustomResultAssert extends CommandResultAssert {
-    public CustomResultAssert(CommandResult commandResult) {
+  static class PDXCommandResultAssert extends CommandResultAssert {
+    public PDXCommandResultAssert(CommandResult commandResult) {
       super(commandResult);
     }
 
     @Override
-    public CustomResultAssert statusIsError() {
+    public PDXCommandResultAssert statusIsError() {
       super.statusIsError();
 
       return this;
     }
 
     @Override
-    public CustomResultAssert statusIsSuccess() {
+    public PDXCommandResultAssert statusIsSuccess() {
       super.statusIsSuccess();
 
       return this;
     }
 
     @Override
-    public CustomResultAssert containsOutput(String... expectedOutputs) {
+    public PDXCommandResultAssert containsOutput(String... expectedOutputs) {
       super.containsOutput(expectedOutputs);
 
       return this;
     }
 
     @Override
-    public CustomResultAssert hasNoFailToPersistError() {
+    public PDXCommandResultAssert hasNoFailToPersistError() {
       super.hasNoFailToPersistError();
 
       return this;
     }
 
-    public CustomResultAssert hasPersistenseConfigured(boolean persistenceEnabled,
+    public PDXCommandResultAssert hasPersistenceConfigured(boolean persistenceEnabled,
         String diskStoreName, CacheCreation cache) {
       assertThat(actual.getOutput()).contains("persistent = " + persistenceEnabled);
 
@@ -345,7 +348,7 @@ public class ConfigurePDXCommandTest {
     }
 
 
-    public CustomResultAssert hasReadSerializedConfigured(boolean readSerializedEnabled,
+    public PDXCommandResultAssert hasReadSerializedConfigured(boolean readSerializedEnabled,
         CacheCreation cache) {
       assertThat(actual.getOutput()).contains("read-serialized = " + readSerializedEnabled);
       verify(cache, times(1)).setPdxReadSerialized(readSerializedEnabled);
@@ -353,7 +356,7 @@ public class ConfigurePDXCommandTest {
       return this;
     }
 
-    public CustomResultAssert hasIgnoreUnreadFieldsConfigured(boolean ignoreUnreadFieldsEnabled,
+    public PDXCommandResultAssert hasIgnoreUnreadFieldsConfigured(boolean ignoreUnreadFieldsEnabled,
         CacheCreation cache) {
       assertThat(actual.getOutput())
           .contains("ignore-unread-fields = " + ignoreUnreadFieldsEnabled);
@@ -362,12 +365,12 @@ public class ConfigurePDXCommandTest {
       return this;
     }
 
-    public CustomResultAssert hasDefaultsConfigured(ConfigurePDXCommand command,
+    public PDXCommandResultAssert hasDefaultsConfigured(ConfigurePDXCommand command,
         CacheCreation cacheCreation) {
       hasNoFailToPersistError();
       hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation);
       hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation);
-      hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+      hasPersistenceConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
 
       verify(cacheCreation, times(0)).setPdxSerializer(any());
       verify(command, times(1)).persistClusterConfiguration(any(), any());

-- 
To stop receiving notification emails like this one, please contact
khowe@apache.org.

[geode] 03/03: GEODE-4771: Internal API was renamed

Posted by kh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

khowe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 3fa3074af9b328a33ee2c811a711c36208c8cfce
Author: Ken Howe <kh...@pivotal.io>
AuthorDate: Mon Mar 19 13:14:52 2018 -0700

    GEODE-4771: Internal API was renamed
    
    ClusterConfigurationService was changed to InternalClusterConfigurationService
    after the original PR (#1574) was created.
---
 .../management/internal/cli/commands/ConfigurePDXCommandTest.java   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
index 9a4d87d..f448e6e 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
@@ -39,7 +39,7 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import org.apache.geode.distributed.DistributedMember;
-import org.apache.geode.distributed.internal.ClusterConfigurationService;
+import org.apache.geode.distributed.internal.InternalClusterConfigurationService;
 import org.apache.geode.internal.cache.CacheConfig;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.xmlcache.CacheCreation;
@@ -60,7 +60,7 @@ public class ConfigurePDXCommandTest {
   private XmlEntity xmlEntity;
   private CacheCreation cacheCreation;
   private ConfigurePDXCommand command;
-  private ClusterConfigurationService clusterConfigurationService;
+  private InternalClusterConfigurationService clusterConfigurationService;
 
   @Before
   public void setUp() throws Exception {
@@ -68,7 +68,7 @@ public class ConfigurePDXCommandTest {
     xmlEntity = mock(XmlEntity.class);
     command = spy(ConfigurePDXCommand.class);
     cacheCreation = spy(CacheCreation.class);
-    clusterConfigurationService = mock(ClusterConfigurationService.class);
+    clusterConfigurationService = mock(InternalClusterConfigurationService.class);
 
     doReturn(cache).when(command).getCache();
     doReturn(xmlEntity).when(command).createXmlEntity(any());

-- 
To stop receiving notification emails like this one, please contact
khowe@apache.org.

[geode] 01/03: GEODE-4771: Defaults in ConfigurePDXCommand

Posted by kh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

khowe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit bafab6bb72938f714961c524e3e2800f4a26f827
Author: Juan Jose Ramos Cassella <ju...@gmail.com>
AuthorDate: Wed Mar 7 14:39:19 2018 +0000

    GEODE-4771: Defaults in ConfigurePDXCommand
    
    - Refactored `ConfigurePDXCommand` to allow unit testing.
    - Added a custom `Interceptor` to validate command input.
    - Added unit and integration tests for `ConfigurePDXCommand`.
    - Fixed help strings for `auto-serializable-classes` and
      `portable-auto-serializable-classes`.
    - Fixed `ConfigurePDXCommand` to set `check-portability=false` when
      `--auto-serializable-classes` is used.
---
 .../internal/cli/commands/ConfigurePDXCommand.java | 111 ++++--
 .../management/internal/cli/i18n/CliStrings.java   |   5 +-
 .../ConfigurePDXCommandIntegrationTest.java        | 130 +++++++
 .../cli/commands/ConfigurePDXCommandTest.java      | 379 +++++++++++++++++++++
 4 files changed, 592 insertions(+), 33 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java
index 0c11448..43719f9 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java
@@ -28,6 +28,8 @@ import org.apache.geode.internal.cache.xmlcache.CacheXml;
 import org.apache.geode.internal.cache.xmlcache.CacheXmlGenerator;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
+import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.result.InfoResultData;
 import org.apache.geode.management.internal.cli.result.ResultBuilder;
@@ -37,8 +39,42 @@ import org.apache.geode.pdx.ReflectionBasedAutoSerializer;
 import org.apache.geode.security.ResourcePermission;
 
 public class ConfigurePDXCommand extends GfshCommand {
+
+  /**
+   *
+   * @param checkPortability
+   * @param patterns
+   */
+  protected ReflectionBasedAutoSerializer createReflectionBasedAutoSerializer(
+      boolean checkPortability, String[] patterns) {
+    return new ReflectionBasedAutoSerializer(checkPortability, patterns);
+  }
+
+  /**
+   * @param forParsing if true then this creation is used for parsing xml; if false then it is used
+   *        for generating xml.
+   * @since GemFire 5.7
+   */
+  protected CacheCreation getCacheCreation(boolean forParsing) {
+    return new CacheCreation(forParsing);
+  }
+
+  /**
+   * Creates the XmlEntity associated to the PDX configuration.
+   */
+  protected XmlEntity createXmlEntity(CacheCreation cache) {
+    final StringWriter stringWriter = new StringWriter();
+    final PrintWriter printWriter = new PrintWriter(stringWriter);
+    CacheXmlGenerator.generate(cache, printWriter, true, false, false);
+    printWriter.close();
+    String xmlDefinition = stringWriter.toString();
+
+    return XmlEntity.builder().withType(CacheXml.PDX).withConfig(xmlDefinition).build();
+  }
+
   @CliCommand(value = CliStrings.CONFIGURE_PDX, help = CliStrings.CONFIGURE_PDX__HELP)
-  @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_REGION)
+  @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_REGION,
+      interceptor = "org.apache.geode.management.internal.cli.commands.ConfigurePDXCommand$Interceptor")
   @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
       operation = ResourcePermission.Operation.MANAGE)
   public Result configurePDX(
@@ -49,26 +85,26 @@ public class ConfigurePDXCommand extends GfshCommand {
       @CliOption(key = CliStrings.CONFIGURE_PDX__DISKSTORE, specifiedDefaultValue = "",
           help = CliStrings.CONFIGURE_PDX__DISKSTORE__HELP) String diskStore,
       @CliOption(key = CliStrings.CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES,
-          help = CliStrings.CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES__HELP) String[] patterns,
+          help = CliStrings.CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES__HELP) String[] nonPortableClassesPatterns,
       @CliOption(key = CliStrings.CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES,
-          help = CliStrings.CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES__HELP) String[] portablePatterns) {
+          help = CliStrings.CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES__HELP) String[] portableClassesPatterns) {
+
     Result result;
 
     try {
+      ReflectionBasedAutoSerializer autoSerializer;
+      CacheCreation cache = getCacheCreation(true);
       InfoResultData ird = ResultBuilder.createInfoResultData();
-      CacheCreation cache = new CacheCreation(true);
 
-      if ((portablePatterns != null && portablePatterns.length > 0)
-          && (patterns != null && patterns.length > 0)) {
-        return ResultBuilder.createUserErrorResult(CliStrings.CONFIGURE_PDX__ERROR__MESSAGE);
-      }
       if (!getAllNormalMembers().isEmpty()) {
         ird.addLine(CliStrings.CONFIGURE_PDX__NORMAL__MEMBERS__WARNING);
       }
+
       // Set persistent and the disk-store
       if (diskStore != null) {
         cache.setPdxPersistent(true);
         ird.addLine(CliStrings.CONFIGURE_PDX__PERSISTENT + " = " + cache.getPdxPersistent());
+
         if (!diskStore.equals("")) {
           cache.setPdxDiskStore(diskStore);
           ird.addLine(CliStrings.CONFIGURE_PDX__DISKSTORE + " = " + cache.getPdxDiskStore());
@@ -86,52 +122,65 @@ public class ConfigurePDXCommand extends GfshCommand {
       } else {
         cache.setPdxReadSerialized(CacheConfig.DEFAULT_PDX_READ_SERIALIZED);
       }
+
       ird.addLine(
           CliStrings.CONFIGURE_PDX__READ__SERIALIZED + " = " + cache.getPdxReadSerialized());
 
-
       // Set ignoreUnreadFields
       if (ignoreUnreadFields != null) {
         cache.setPdxIgnoreUnreadFields(ignoreUnreadFields);
       } else {
         cache.setPdxIgnoreUnreadFields(CacheConfig.DEFAULT_PDX_IGNORE_UNREAD_FIELDS);
       }
+
       ird.addLine(CliStrings.CONFIGURE_PDX__IGNORE__UNREAD_FIELDS + " = "
           + cache.getPdxIgnoreUnreadFields());
 
-
-      if (portablePatterns != null) {
-        ReflectionBasedAutoSerializer autoSerializer =
-            new ReflectionBasedAutoSerializer(portablePatterns);
+      // Auto Serializer Configuration
+      if (portableClassesPatterns != null) {
+        autoSerializer = createReflectionBasedAutoSerializer(true, portableClassesPatterns);
         cache.setPdxSerializer(autoSerializer);
-        ird.addLine("PDX Serializer " + cache.getPdxSerializer().getClass().getName());
-        ird.addLine("Portable classes " + Arrays.toString(portablePatterns));
+        ird.addLine("PDX Serializer = " + cache.getPdxSerializer().getClass().getName());
+        ird.addLine("Portable Classes = " + Arrays.toString(portableClassesPatterns));
       }
 
-      if (patterns != null) {
-        ReflectionBasedAutoSerializer nonPortableAutoSerializer =
-            new ReflectionBasedAutoSerializer(true, patterns);
-        cache.setPdxSerializer(nonPortableAutoSerializer);
-        ird.addLine("PDX Serializer : " + cache.getPdxSerializer().getClass().getName());
-        ird.addLine("Non portable classes :" + Arrays.toString(patterns));
+      if (nonPortableClassesPatterns != null) {
+        autoSerializer = createReflectionBasedAutoSerializer(false, nonPortableClassesPatterns);
+        cache.setPdxSerializer(autoSerializer);
+        ird.addLine("PDX Serializer = " + cache.getPdxSerializer().getClass().getName());
+        ird.addLine("Non Portable Classes = " + Arrays.toString(nonPortableClassesPatterns));
       }
 
-      final StringWriter stringWriter = new StringWriter();
-      final PrintWriter printWriter = new PrintWriter(stringWriter);
-      CacheXmlGenerator.generate(cache, printWriter, true, false, false);
-      printWriter.close();
-      String xmlDefinition = stringWriter.toString();
-      // TODO jbarrett - shouldn't this use the same loadXmlDefinition that other constructors use?
-      XmlEntity xmlEntity =
-          XmlEntity.builder().withType(CacheXml.PDX).withConfig(xmlDefinition).build();
-
+      XmlEntity xmlEntity = createXmlEntity(cache);
       result = ResultBuilder.buildResult(ird);
       persistClusterConfiguration(result,
           () -> getSharedConfiguration().addXmlEntity(xmlEntity, null));
-
     } catch (Exception e) {
       return ResultBuilder.createGemFireErrorResult(e.getMessage());
     }
+
     return result;
   }
+
+  /**
+   * Interceptor to validate command parameters.
+   */
+  public static class Interceptor extends AbstractCliAroundInterceptor {
+
+    @Override
+    public Result preExecution(GfshParseResult parseResult) {
+      Object portableClassesPatterns =
+          parseResult.getParamValue(CliStrings.CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES);
+      Object nonPortableClassesPatterns =
+          parseResult.getParamValue(CliStrings.CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES);
+
+      if ((nonPortableClassesPatterns != null && ((String[]) nonPortableClassesPatterns).length > 0)
+          && (portableClassesPatterns != null && ((String[]) portableClassesPatterns).length > 0)) {
+
+        return ResultBuilder.createUserErrorResult(CliStrings.CONFIGURE_PDX__ERROR__MESSAGE);
+      }
+
+      return ResultBuilder.createInfoResult("");
+    }
+  }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
index ba4138d..a1e9e3e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
@@ -3114,11 +3114,12 @@ public class CliStrings {
   public static final String CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES =
       "portable-auto-serializable-classes";
   public static final String CONFIGURE_PDX__PORTABLE__AUTO__SERIALIZER__CLASSES__HELP =
-      "the patterns which are matched against domain class names to determine whether they should be serialized";
+      "The patterns that are matched against domain class names to determine whether they should be serialized. Serialization done by the auto-serializer will throw an exception if the object of these classes are not portable to non-java languages (check-portability=true).";
 
   public static final String CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES = "auto-serializable-classes";
   public static final String CONFIGURE_PDX__AUTO__SERIALIZER__CLASSES__HELP =
-      "the patterns which are matched against domain class names to determine whether they should be serialized, serialization done by the auto-serializer will throw an exception if the object of these classes are not portable to non-java languages";
+      "The patterns that are matched against domain class names to determine whether they should be auto-serialized. Serialization done by the auto-serializer will not throw an exception if the object of these classes are not portable to non-java languages (check-portability=false).";
+
   public static final String CONFIGURE_PDX__NORMAL__MEMBERS__WARNING =
       "The command would only take effect on new data members joining the distributed system. It won't affect the existing data members";
   public static final String CONFIGURE_PDX__ERROR__MESSAGE =
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java
new file mode 100644
index 0000000..b1288b0
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandIntegrationTest.java
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+@Category(IntegrationTest.class)
+public class ConfigurePDXCommandIntegrationTest {
+  private static final String BASE_COMMAND_STRING = "configure pdx ";
+
+  @Rule
+  public GfshCommandRule gfsh = new GfshCommandRule().withTimeout(1);
+
+  @Rule
+  public LocatorStarterRule locator = new LocatorStarterRule().withAutoStart().withJMXManager();
+
+  @Before
+  public void before() throws Exception {
+    gfsh.connectAndVerify(locator);
+  }
+
+  @Test
+  @Ignore("See https://issues.apache.org/jira/browse/GEODE-4794")
+  public void commandShouldSucceedWhenUsingDefaults() {
+    gfsh.executeAndAssertThat(BASE_COMMAND_STRING).statusIsSuccess().hasNoFailToPersistError();
+  }
+
+  @Test
+  public void commandShouldFailWhenNotConnected() throws Exception {
+    gfsh.disconnect();
+    gfsh.executeAndAssertThat(BASE_COMMAND_STRING).statusIsError().containsOutput("Command",
+        "was found but is not currently available");
+  }
+
+  @Test
+  public void commandShouldSucceedWhenConfiguringAutoSerializableClassesWithPersistence() {
+    gfsh.executeAndAssertThat(BASE_COMMAND_STRING
+        + "--read-serialized=true --disk-store=myDiskStore --ignore-unread-fields=true --auto-serializable-classes=com.company.DomainObject.*#identity=id")
+        .statusIsSuccess().hasNoFailToPersistError();
+
+    String sharedConfigXml = locator.getLocator().getSharedConfiguration()
+        .getConfiguration("cluster").getCacheXmlContent();
+    assertThat(sharedConfigXml).contains(
+        "<pdx disk-store-name=\"myDiskStore\" ignore-unread-fields=\"true\" persistent=\"true\" read-serialized=\"true\">");
+    assertThat(sharedConfigXml).contains("<pdx-serializer>",
+        "<class-name>org.apache.geode.pdx.ReflectionBasedAutoSerializer</class-name>",
+        "<parameter name=\"classes\">",
+        "<string>com.company.DomainObject.*, com.company.DomainObject.*#identity=id</string>",
+        "</parameter>", "</pdx-serializer>");
+    assertThat(sharedConfigXml).contains("</pdx>");
+  }
+
+  @Test
+  public void commandShouldSucceedWhenConfiguringAutoSerializableClassesWithoutPersistence() {
+    gfsh.executeAndAssertThat(BASE_COMMAND_STRING
+        + "--read-serialized=false --ignore-unread-fields=false --auto-serializable-classes=com.company.DomainObject.*#identity=id")
+        .statusIsSuccess().hasNoFailToPersistError();
+
+    String sharedConfigXml = locator.getLocator().getSharedConfiguration()
+        .getConfiguration("cluster").getCacheXmlContent();
+    assertThat(sharedConfigXml).contains("<pdx>");
+    assertThat(sharedConfigXml).contains("<pdx-serializer>",
+        "<class-name>org.apache.geode.pdx.ReflectionBasedAutoSerializer</class-name>",
+        "<parameter name=\"classes\">",
+        "<string>com.company.DomainObject.*, com.company.DomainObject.*#identity=id</string>",
+        "</parameter>", "</pdx-serializer>");
+    assertThat(sharedConfigXml).contains("</pdx>");
+  }
+
+  @Test
+  public void commandShouldSucceedWhenConfiguringPortableAutoSerializableClassesWithPersistence() {
+    gfsh.executeAndAssertThat(BASE_COMMAND_STRING
+        + "--read-serialized=true --disk-store=myDiskStore --ignore-unread-fields=true --portable-auto-serializable-classes=com.company.DomainObject.*#identity=id")
+        .statusIsSuccess().hasNoFailToPersistError();
+
+    String sharedConfigXml = locator.getLocator().getSharedConfiguration()
+        .getConfiguration("cluster").getCacheXmlContent();
+    assertThat(sharedConfigXml).contains(
+        "<pdx disk-store-name=\"myDiskStore\" ignore-unread-fields=\"true\" persistent=\"true\" read-serialized=\"true\">");
+    assertThat(sharedConfigXml).contains("<parameter name=\"check-portability\">")
+        .contains("<string>true</string>").contains("</parameter>");
+    assertThat(sharedConfigXml).contains("<pdx-serializer>",
+        "<class-name>org.apache.geode.pdx.ReflectionBasedAutoSerializer</class-name>",
+        "<parameter name=\"classes\">",
+        "<string>com.company.DomainObject.*, com.company.DomainObject.*#identity=id</string>",
+        "</parameter>", "</pdx-serializer>");
+    assertThat(sharedConfigXml).contains("</pdx>");
+  }
+
+  @Test
+  public void commandShouldSucceedWhenConfiguringPortableAutoSerializableClassesWithoutPersistence() {
+    gfsh.executeAndAssertThat(BASE_COMMAND_STRING
+        + "--read-serialized=false --ignore-unread-fields=false --portable-auto-serializable-classes=com.company.DomainObject.*#identity=id")
+        .statusIsSuccess().hasNoFailToPersistError();
+
+    String sharedConfigXml = locator.getLocator().getSharedConfiguration()
+        .getConfiguration("cluster").getCacheXmlContent();
+    assertThat(sharedConfigXml).contains("<pdx>");
+    assertThat(sharedConfigXml).contains("<parameter name=\"check-portability\">")
+        .contains("<string>true</string>").contains("</parameter>");
+    assertThat(sharedConfigXml).contains("<pdx-serializer>",
+        "<class-name>org.apache.geode.pdx.ReflectionBasedAutoSerializer</class-name>",
+        "<parameter name=\"classes\">",
+        "<string>com.company.DomainObject.*, com.company.DomainObject.*#identity=id</string>",
+        "</parameter>", "</pdx-serializer>");
+    assertThat(sharedConfigXml).contains("</pdx>");
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
new file mode 100644
index 0000000..bfd5a61
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommandTest.java
@@ -0,0 +1,379 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.apache.geode.internal.cache.CacheConfig.DEFAULT_PDX_IGNORE_UNREAD_FIELDS;
+import static org.apache.geode.internal.cache.CacheConfig.DEFAULT_PDX_PERSISTENT;
+import static org.apache.geode.internal.cache.CacheConfig.DEFAULT_PDX_READ_SERIALIZED;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.commons.lang.StringUtils;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.ClusterConfigurationService;
+import org.apache.geode.internal.cache.CacheConfig;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.cache.xmlcache.CacheCreation;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.test.junit.assertions.CommandResultAssert;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
+@Category(UnitTest.class)
+public class ConfigurePDXCommandTest {
+  private static final String BASE_COMMAND_STRING = "configure pdx ";
+
+  @ClassRule
+  public static CustomGfshParserRule gfshParserRule = new CustomGfshParserRule();
+
+  private InternalCache cache;
+  private XmlEntity xmlEntity;
+  private CacheCreation cacheCreation;
+  private ConfigurePDXCommand command;
+  private ClusterConfigurationService clusterConfigurationService;
+
+  @Before
+  public void setUp() throws Exception {
+    cache = mock(InternalCache.class);
+    xmlEntity = mock(XmlEntity.class);
+    command = spy(ConfigurePDXCommand.class);
+    cacheCreation = spy(CacheCreation.class);
+    clusterConfigurationService = mock(ClusterConfigurationService.class);
+
+    doReturn(cache).when(command).getCache();
+    doReturn(xmlEntity).when(command).createXmlEntity(any());
+    doReturn(cacheCreation).when(command).getCacheCreation(anyBoolean());
+    doReturn(Collections.emptySet()).when(command).getAllNormalMembers();
+    doReturn(clusterConfigurationService).when(command).getSharedConfiguration();
+  }
+
+  @Test
+  public void parsingShouldSucceedWithoutArguments() {
+    assertThat(gfshParserRule.parse(BASE_COMMAND_STRING)).isNotNull();
+  }
+
+  @Test
+  public void parsingAutoCompleteShouldSucceed() throws Exception {
+    GfshParserRule.CommandCandidate candidate = gfshParserRule.complete(BASE_COMMAND_STRING);
+
+    assertThat(candidate).isNotNull();
+    assertThat(candidate.getCandidates()).isNotNull();
+    assertThat(candidate.getCandidates().size()).isEqualTo(5);
+  }
+
+  @Test
+  public void executionShouldHandleInternalFailures() {
+    doThrow(new RuntimeException("Can't create CacheCreation.")).when(command)
+        .getCacheCreation(anyBoolean());
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsError()
+        .containsOutput("Could not process command due to error. Can't create CacheCreation.");
+    doReturn(cacheCreation).when(command).getCacheCreation(anyBoolean());
+
+    doThrow(new RuntimeException("Can't find members.")).when(command).getAllNormalMembers();
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsError()
+        .containsOutput("Could not process command due to error. Can't find members.");
+    doReturn(Collections.emptySet()).when(command).getAllNormalMembers();
+
+    doThrow(new RuntimeException("Can't create XmlEntity.")).when(command).createXmlEntity(any());
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsError()
+        .containsOutput("Could not process command due to error. Can't create XmlEntity.");
+    doReturn(xmlEntity).when(command).createXmlEntity(any());
+
+    doThrow(new RuntimeException("Can't create ReflectionBasedAutoSerializer.")).when(command)
+        .createReflectionBasedAutoSerializer(anyBoolean(), any());
+    gfshParserRule
+        .executeAndAssertThat(command,
+            BASE_COMMAND_STRING + "--auto-serializable-classes=" + new String[] {})
+        .statusIsError().containsOutput(
+            "Could not process command due to error. Can't create ReflectionBasedAutoSerializer.");
+    gfshParserRule
+        .executeAndAssertThat(command,
+            BASE_COMMAND_STRING + "--portable-auto-serializable-classes=" + new String[] {})
+        .statusIsError().containsOutput(
+            "Could not process command due to error. Can't create ReflectionBasedAutoSerializer.");
+
+    verify(command, times(0)).persistClusterConfiguration(any(), any());
+  }
+
+  @Test
+  public void executionShouldFailIfBothPortableAndNonPortableClassesParametersAreSpecifiedAtTheSameTime() {
+    gfshParserRule
+        .executeAndAssertThat(command,
+            BASE_COMMAND_STRING
+                + "--auto-serializable-classes=org.apache.geode --portable-auto-serializable-classes=org.apache.geode")
+        .statusIsError().containsOutput(
+            "The autoserializer cannot support both portable and non-portable classes at the same time.");
+
+    verify(command, times(0)).persistClusterConfiguration(any(), any());
+  }
+
+  @Test
+  public void executionShouldIncludeWarningMessageWhenThereAreMembersAlreadyRunning() {
+    Set<DistributedMember> members = new HashSet<>();
+    DistributedMember mockMember = mock(DistributedMember.class);
+    when(mockMember.getId()).thenReturn("member0");
+    members.add(mockMember);
+    doReturn(xmlEntity).when(command).createXmlEntity(any());
+    doReturn(members).when(command).getAllNormalMembers();
+
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsSuccess()
+        .hasDefaultsConfigured(command, cacheCreation).containsOutput(
+            "The command would only take effect on new data members joining the distributed system. It won't affect the existing data members");
+  }
+
+  @Test
+  public void executionShouldWorkCorrectlyWhenDefaultsAreUsed() {
+    // Factory Default
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING).statusIsSuccess()
+        .hasNoFailToPersistError().hasDefaultsConfigured(command, cacheCreation);
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigurePersistenceWhenDefaultDiskStoreIsUsed() {
+    // Default Disk Store
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING + "--disk-store")
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
+        .hasPersistenseConfigured(true, "DEFAULT", cacheCreation);
+
+    verify(cacheCreation, times(0)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(0)).createReflectionBasedAutoSerializer(anyBoolean(), any());
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigurePersistenceWhenCustomDiskStoreIsUsed() {
+    // Custom Disk Store
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING + "--disk-store=myDiskStore")
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
+        .hasPersistenseConfigured(true, "myDiskStore", cacheCreation);
+
+    verify(cacheCreation, times(0)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(0)).createReflectionBasedAutoSerializer(anyBoolean(), any());
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigureReadSerializedWhenFlagIsSetAsTrue() {
+    // Custom Configuration as True
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING + "--read-serialized=true")
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(true, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
+        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+
+    verify(cacheCreation, times(0)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(0)).createReflectionBasedAutoSerializer(anyBoolean(), any());
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigureReadSerializedWhenFlagIsSetAsFalse() {
+    // Custom Configuration as False
+    gfshParserRule.executeAndAssertThat(command, BASE_COMMAND_STRING + "--read-serialized=false")
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(false, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
+        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+
+    verify(cacheCreation, times(0)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(0)).createReflectionBasedAutoSerializer(anyBoolean(), any());
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigureIgnoreUnreadFieldsWhenFlagIsSetAsTrue() {
+    // Custom Configuration as True
+    gfshParserRule
+        .executeAndAssertThat(command, BASE_COMMAND_STRING + "--ignore-unread-fields=true")
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(true, cacheCreation)
+        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+
+    verify(cacheCreation, times(0)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(0)).createReflectionBasedAutoSerializer(anyBoolean(), any());
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigureIgnoreUnreadFieldsWhenFlagIsSetAsFalse() {
+    // Custom Configuration as False
+    gfshParserRule
+        .executeAndAssertThat(command, BASE_COMMAND_STRING + "--ignore-unread-fields=false")
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(false, cacheCreation)
+        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+
+    verify(cacheCreation, times(0)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(0)).createReflectionBasedAutoSerializer(anyBoolean(), any());
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigurePortableAutoSerializableClassesWhenUsingCustomPattern() {
+    String[] patterns = new String[] {"com.company.DomainObject.*#identity=id"};
+
+    // Custom Settings
+    gfshParserRule
+        .executeAndAssertThat(command,
+            BASE_COMMAND_STRING + "--portable-auto-serializable-classes=" + patterns[0])
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation)
+        .hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation)
+        .containsOutput("Portable Classes = [com.company.DomainObject.*#identity=id]")
+        .containsOutput("PDX Serializer = org.apache.geode.pdx.ReflectionBasedAutoSerializer");
+
+    verify(cacheCreation, times(1)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(1)).createReflectionBasedAutoSerializer(true, patterns);
+  }
+
+  @Test
+  public void executionShouldCorrectlyConfigureAutoSerializableClassesWhenUsingCustomPattern() {
+    String[] patterns = new String[] {"com.company.DomainObject.*#identity=id"};
+
+    // Custom Settings
+    gfshParserRule
+        .executeAndAssertThat(command,
+            BASE_COMMAND_STRING + "--auto-serializable-classes=" + patterns[0])
+        .statusIsSuccess().hasNoFailToPersistError()
+        .hasReadSerializedConfigured(CacheConfig.DEFAULT_PDX_READ_SERIALIZED, cacheCreation)
+        .hasIgnoreUnreadFieldsConfigured(CacheConfig.DEFAULT_PDX_IGNORE_UNREAD_FIELDS,
+            cacheCreation)
+        .hasPersistenseConfigured(CacheConfig.DEFAULT_PDX_PERSISTENT, null, cacheCreation)
+        .containsOutput("Non Portable Classes = [com.company.DomainObject.*#identity=id]")
+        .containsOutput("PDX Serializer = org.apache.geode.pdx.ReflectionBasedAutoSerializer");
+
+    verify(cacheCreation, times(1)).setPdxSerializer(any());
+    verify(command, times(1)).persistClusterConfiguration(any(), any());
+    verify(command, times(1)).createReflectionBasedAutoSerializer(false, patterns);
+  }
+
+  static class CustomGfshParserRule extends GfshParserRule {
+    @Override
+    public <T> CustomResultAssert executeAndAssertThat(T instance, String command) {
+      CommandResultAssert resultAssert = super.executeAndAssertThat(instance, command);;
+
+      return new CustomResultAssert(resultAssert.getCommandResult());
+    }
+  }
+
+  static class CustomResultAssert extends CommandResultAssert {
+    public CustomResultAssert(CommandResult commandResult) {
+      super(commandResult);
+    }
+
+    @Override
+    public CustomResultAssert statusIsError() {
+      super.statusIsError();
+
+      return this;
+    }
+
+    @Override
+    public CustomResultAssert statusIsSuccess() {
+      super.statusIsSuccess();
+
+      return this;
+    }
+
+    @Override
+    public CustomResultAssert containsOutput(String... expectedOutputs) {
+      super.containsOutput(expectedOutputs);
+
+      return this;
+    }
+
+    @Override
+    public CustomResultAssert hasNoFailToPersistError() {
+      super.hasNoFailToPersistError();
+
+      return this;
+    }
+
+    public CustomResultAssert hasPersistenseConfigured(boolean persistenceEnabled,
+        String diskStoreName, CacheCreation cache) {
+      assertThat(actual.getOutput()).contains("persistent = " + persistenceEnabled);
+
+      if (StringUtils.isNotEmpty(diskStoreName)) {
+        assertThat(actual.getOutput()).contains("disk-store = " + diskStoreName);
+      }
+
+      if (persistenceEnabled) {
+        verify(cache, times(1)).setPdxPersistent(true);
+      } else {
+        verify(cache, times(0)).setPdxPersistent(true);
+      }
+
+      return this;
+    }
+
+
+    public CustomResultAssert hasReadSerializedConfigured(boolean readSerializedEnabled,
+        CacheCreation cache) {
+      assertThat(actual.getOutput()).contains("read-serialized = " + readSerializedEnabled);
+      verify(cache, times(1)).setPdxReadSerialized(readSerializedEnabled);
+
+      return this;
+    }
+
+    public CustomResultAssert hasIgnoreUnreadFieldsConfigured(boolean ignoreUnreadFieldsEnabled,
+        CacheCreation cache) {
+      assertThat(actual.getOutput())
+          .contains("ignore-unread-fields = " + ignoreUnreadFieldsEnabled);
+      verify(cache, times(1)).setPdxIgnoreUnreadFields(ignoreUnreadFieldsEnabled);
+
+      return this;
+    }
+
+    public CustomResultAssert hasDefaultsConfigured(ConfigurePDXCommand command,
+        CacheCreation cacheCreation) {
+      hasNoFailToPersistError();
+      hasReadSerializedConfigured(DEFAULT_PDX_READ_SERIALIZED, cacheCreation);
+      hasIgnoreUnreadFieldsConfigured(DEFAULT_PDX_IGNORE_UNREAD_FIELDS, cacheCreation);
+      hasPersistenseConfigured(DEFAULT_PDX_PERSISTENT, null, cacheCreation);
+
+      verify(cacheCreation, times(0)).setPdxSerializer(any());
+      verify(command, times(1)).persistClusterConfiguration(any(), any());
+      verify(command, times(0)).createReflectionBasedAutoSerializer(anyBoolean(), any());
+
+      return this;
+    }
+  }
+}

-- 
To stop receiving notification emails like this one, please contact
khowe@apache.org.