Skip to content

Commit

Permalink
GEODE-4771: Defaults in ConfigurePDXCommand
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
jujoramos committed Mar 13, 2018
1 parent 94586e1 commit a1cb166
Show file tree
Hide file tree
Showing 4 changed files with 592 additions and 33 deletions.
Expand Up @@ -28,6 +28,8 @@
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;
Expand All @@ -37,8 +39,42 @@
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(
Expand All @@ -49,26 +85,26 @@ public Result configurePDX(
@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());
Expand All @@ -86,52 +122,65 @@ public Result configurePDX(
} 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("");
}
}
}
Expand Up @@ -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 =
Expand Down
@@ -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>");
}
}

0 comments on commit a1cb166

Please sign in to comment.