This is an automated email from the ASF dual-hosted git repository. sai_boorlagadda 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 6f8b7f1 GEODE-5971: refactor DescribeXXCommand to use ResultModel (#3265) 6f8b7f1 is described below commit 6f8b7f1e7eda878db047b82d43288038008a2d25 Author: jinmeiliao <jil...@pivotal.io> AuthorDate: Tue Mar 5 07:57:49 2019 -0800 GEODE-5971: refactor DescribeXXCommand to use ResultModel (#3265) --- .../commands/DescribeMembersCommandDUnitTest.java | 6 +- .../DescribeConnectionCommandIntegrationTest.java | 11 ++- .../geode/management/internal/cli/CliUtil.java | 21 ----- .../cli/commands/DescribeClientCommand.java | 3 +- .../cli/commands/DescribeConfigCommand.java | 3 +- .../cli/commands/DescribeConnectionCommand.java | 36 +++---- .../cli/commands/DescribeMemberCommand.java | 103 +++++++++------------ .../management/internal/cli/i18n/CliStrings.java | 2 +- .../test/junit/assertions/CommandResultAssert.java | 17 +++- 9 files changed, 92 insertions(+), 110 deletions(-) diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeMembersCommandDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeMembersCommandDUnitTest.java index b6c61fb..f433f85 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeMembersCommandDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeMembersCommandDUnitTest.java @@ -66,7 +66,7 @@ public class DescribeMembersCommandDUnitTest { .statusIsSuccess() .getCommandResult(); - Map<String, String> memberInfo = result.getMapFromSection("0"); + Map<String, String> memberInfo = result.getMapFromSection("memberInfo"); assertThat(memberInfo.get("Name")).isEqualTo("locator-0"); assertThat(memberInfo.get("Id")).contains("locator-0"); assertThat(memberInfo.get("Host")).as("Host").isNotBlank(); @@ -85,7 +85,7 @@ public class DescribeMembersCommandDUnitTest { .statusIsSuccess() .getCommandResult(); - Map<String, String> memberInfo = result.getMapFromSection("0"); + Map<String, String> memberInfo = result.getMapFromSection("memberInfo"); assertThat(memberInfo.get("Name")).isEqualTo("server-1"); assertThat(memberInfo.get("Id")).contains("server-1"); assertThat(memberInfo.get("Host")).as("Host").isNotBlank(); @@ -96,7 +96,7 @@ public class DescribeMembersCommandDUnitTest { assertThat(memberInfo.get("Log file")).as("Log File").isNotBlank(); assertThat(memberInfo.get("Locators")).as("Locators").isNotBlank(); - Map<String, String> cacheServerInfo = result.getMapFromSection("1"); + Map<String, String> cacheServerInfo = result.getMapFromSection("connectionInfo"); assertThat(cacheServerInfo.get("Server Bind")).as("Server Bind").isBlank(); assertThat(cacheServerInfo.get("Server Port")).as("Server Port").isNotBlank(); assertThat(cacheServerInfo.get("Running")).as("Running").isEqualTo("true"); diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandIntegrationTest.java index e1d02e4..6f16bb9 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandIntegrationTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandIntegrationTest.java @@ -39,14 +39,19 @@ public class DescribeConnectionCommandIntegrationTest { @Test public void describeConnectionTest() throws Exception { gfsh.connectAndVerify(locator.getJmxPort(), PortType.jmxManager); - gfsh.executeAndAssertThat("describe connection").tableHasColumnWithValuesContaining( - "Connection Endpoints", gfsh.getGfsh().getOperationInvoker().toString()); + gfsh.executeAndAssertThat("describe connection").hasTableSection() + .hasColumnSize(1) + .hasColumn("Connection Endpoints") + .containsExactly(gfsh.getGfsh().getOperationInvoker().toString()); } @Test public void executeWhileNotConnected() throws Exception { gfsh.executeAndAssertThat("describe connection") - .tableHasColumnWithValuesContaining("Connection Endpoints", "Not connected"); + .hasTableSection() + .hasColumnSize(1) + .hasColumn("Connection Endpoints") + .containsExactly("Not connected"); } } diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java index fc80fdb..e05b0cc 100755 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java @@ -66,7 +66,6 @@ import org.apache.geode.management.internal.MBeanJMXAdapter; import org.apache.geode.management.internal.SystemManagementService; import org.apache.geode.management.internal.cli.exceptions.UserErrorException; import org.apache.geode.management.internal.cli.i18n.CliStrings; -import org.apache.geode.management.internal.cli.result.ResultBuilder; import org.apache.geode.management.internal.cli.shell.Gfsh; import org.apache.geode.management.internal.configuration.domain.DeclarableTypeInstantiator; @@ -505,26 +504,6 @@ public class CliUtil { : IOUtils.tryGetCanonicalPathElseGetAbsolutePath(new File(pathname))); } - public static Result getFunctionResult(ResultCollector<?, ?> rc, String commandName) { - Result result; - List<Object> results = (List<Object>) rc.getResult(); - if (results != null) { - Object resultObj = results.get(0); - if (resultObj instanceof String) { - result = ResultBuilder.createInfoResult((String) resultObj); - } else if (resultObj instanceof Exception) { - result = ResultBuilder.createGemFireErrorResult(((Exception) resultObj).getMessage()); - } else { - result = ResultBuilder.createGemFireErrorResult( - CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, commandName)); - } - } else { - result = ResultBuilder.createGemFireErrorResult( - CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, commandName)); - } - return result; - } - /*** * Executes a function with arguments on a set of members, ignoring the departed members. * diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommand.java index 1075ea4..d9e9ec9 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommand.java @@ -32,6 +32,7 @@ import org.apache.geode.management.CacheServerMXBean; import org.apache.geode.management.ClientHealthStatus; import org.apache.geode.management.ManagementService; import org.apache.geode.management.cli.CliMetaData; +import org.apache.geode.management.cli.GfshCommand; import org.apache.geode.management.internal.cli.CliUtil; import org.apache.geode.management.internal.cli.LogWrapper; import org.apache.geode.management.internal.cli.functions.ContinuousQueryFunction; @@ -42,7 +43,7 @@ import org.apache.geode.management.internal.cli.result.model.TabularResultModel; import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.geode.security.ResourcePermission; -public class DescribeClientCommand extends InternalGfshCommand { +public class DescribeClientCommand extends GfshCommand { @CliCommand(value = CliStrings.DESCRIBE_CLIENT, help = CliStrings.DESCRIBE_CLIENT__HELP) @CliMetaData(relatedTopic = {CliStrings.TOPIC_CLIENT}) @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER, diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java index fe05625..41c1b51 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java @@ -26,6 +26,7 @@ import org.apache.geode.distributed.DistributedMember; import org.apache.geode.internal.util.ArgumentRedactor; import org.apache.geode.management.cli.CliMetaData; import org.apache.geode.management.cli.ConverterHint; +import org.apache.geode.management.cli.GfshCommand; import org.apache.geode.management.internal.cli.domain.MemberConfigurationInfo; import org.apache.geode.management.internal.cli.functions.CliFunctionResult; import org.apache.geode.management.internal.cli.functions.GetMemberConfigInformationFunction; @@ -36,7 +37,7 @@ import org.apache.geode.management.internal.cli.result.model.TabularResultModel; import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.geode.security.ResourcePermission; -public class DescribeConfigCommand extends InternalGfshCommand { +public class DescribeConfigCommand extends GfshCommand { public static final String API_PROPERTIES_SECTION = "api-properties"; public static final String RUNTIME_PROPERTIES_SECTION = "runtime-properties"; public static final String FILE_PROPERTIES_SECTION = "file-properties"; diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommand.java index a73b3d7..910a537 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommand.java @@ -18,35 +18,29 @@ package org.apache.geode.management.internal.cli.commands; import org.springframework.shell.core.annotation.CliCommand; import org.apache.geode.management.cli.CliMetaData; -import org.apache.geode.management.cli.Result; import org.apache.geode.management.internal.cli.i18n.CliStrings; -import org.apache.geode.management.internal.cli.result.ErrorResultData; -import org.apache.geode.management.internal.cli.result.ResultBuilder; -import org.apache.geode.management.internal.cli.result.TabularResultData; +import org.apache.geode.management.internal.cli.result.model.ResultModel; +import org.apache.geode.management.internal.cli.result.model.TabularResultModel; import org.apache.geode.management.internal.cli.shell.Gfsh; import org.apache.geode.management.internal.cli.shell.OperationInvoker; public class DescribeConnectionCommand extends OfflineGfshCommand { @CliCommand(value = {CliStrings.DESCRIBE_CONNECTION}, help = CliStrings.DESCRIBE_CONNECTION__HELP) @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GFSH, CliStrings.TOPIC_GEODE_JMX}) - public Result describeConnection() { - Result result; - try { - TabularResultData tabularResultData = ResultBuilder.createTabularResultData(); - Gfsh gfshInstance = getGfsh(); - if (gfshInstance.isConnectedAndReady()) { - OperationInvoker operationInvoker = gfshInstance.getOperationInvoker(); - // tabularResultData.accumulate("Monitored GemFire DS", operationInvoker.toString()); - tabularResultData.accumulate("Connection Endpoints", operationInvoker.toString()); - } else { - tabularResultData.accumulate("Connection Endpoints", "Not connected"); - } - result = ResultBuilder.buildResult(tabularResultData); - } catch (Exception e) { - ErrorResultData errorResultData = ResultBuilder.createErrorResultData() - .setErrorCode(ResultBuilder.ERRORCODE_DEFAULT).addLine(e.getMessage()); - result = ResultBuilder.buildResult(errorResultData); + public ResultModel describeConnection() { + ResultModel result = new ResultModel(); + + TabularResultModel table = result.addTable("endPoints"); + table.setColumnHeader("Connection Endpoints"); + Gfsh gfshInstance = getGfsh(); + if (gfshInstance.isConnectedAndReady()) { + OperationInvoker operationInvoker = gfshInstance.getOperationInvoker(); + // tabularResultData.accumulate("Monitored GemFire DS", operationInvoker.toString()); + table.addRow(operationInvoker.toString()); + } else { + table.addRow("Not connected"); } + return result; } } diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeMemberCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeMemberCommand.java index 753e159..919efbb 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeMemberCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeMemberCommand.java @@ -24,22 +24,21 @@ import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; import org.apache.geode.annotations.Immutable; -import org.apache.geode.cache.CacheClosedException; import org.apache.geode.cache.execute.ResultCollector; import org.apache.geode.distributed.DistributedMember; import org.apache.geode.management.cli.CliMetaData; import org.apache.geode.management.cli.ConverterHint; -import org.apache.geode.management.cli.Result; +import org.apache.geode.management.cli.GfshCommand; import org.apache.geode.management.internal.cli.domain.CacheServerInfo; import org.apache.geode.management.internal.cli.domain.MemberInformation; import org.apache.geode.management.internal.cli.functions.GetMemberInformationFunction; import org.apache.geode.management.internal.cli.i18n.CliStrings; -import org.apache.geode.management.internal.cli.result.CompositeResultData; -import org.apache.geode.management.internal.cli.result.ResultBuilder; +import org.apache.geode.management.internal.cli.result.model.DataResultModel; +import org.apache.geode.management.internal.cli.result.model.ResultModel; import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.geode.security.ResourcePermission; -public class DescribeMemberCommand extends InternalGfshCommand { +public class DescribeMemberCommand extends GfshCommand { @Immutable private static final GetMemberInformationFunction getMemberInformation = new GetMemberInformationFunction(); @@ -48,71 +47,59 @@ public class DescribeMemberCommand extends InternalGfshCommand { @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_SERVER) @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER, operation = ResourcePermission.Operation.READ) - public Result describeMember(@CliOption(key = CliStrings.DESCRIBE_MEMBER__IDENTIFIER, + public ResultModel describeMember(@CliOption(key = CliStrings.DESCRIBE_MEMBER__IDENTIFIER, optionContext = ConverterHint.ALL_MEMBER_IDNAME, help = CliStrings.DESCRIBE_MEMBER__HELP, mandatory = true) String memberNameOrId) { - Result result = null; + DistributedMember memberToBeDescribed = getMember(memberNameOrId); - try { - DistributedMember memberToBeDescribed = getMember(memberNameOrId); + ResultCollector<?, ?> rc = executeFunction(getMemberInformation, null, memberToBeDescribed); - ResultCollector<?, ?> rc = executeFunction(getMemberInformation, null, memberToBeDescribed); + ArrayList<?> output = (ArrayList<?>) rc.getResult(); + Object obj = output.get(0); - ArrayList<?> output = (ArrayList<?>) rc.getResult(); - Object obj = output.get(0); - - if (obj != null && (obj instanceof MemberInformation)) { - CompositeResultData crd = ResultBuilder.createCompositeResultData(); - - MemberInformation memberInformation = (MemberInformation) obj; - CompositeResultData.SectionResultData section = crd.addSection(); - section.addData("Name", memberInformation.getName()); - section.addData("Id", memberInformation.getId()); - section.addData("Host", memberInformation.getHost()); - section.addData("Regions", StringUtils.join(memberInformation.getHostedRegions(), '\n')); - section.addData("PID", memberInformation.getProcessId()); - section.addData("Groups", memberInformation.getGroups()); - section.addData("Used Heap", memberInformation.getHeapUsage() + "M"); - section.addData("Max Heap", memberInformation.getMaxHeapSize() + "M"); - - String offHeapMemorySize = memberInformation.getOffHeapMemorySize(); - if (offHeapMemorySize != null && !offHeapMemorySize.isEmpty()) { - section.addData("Off Heap Size", offHeapMemorySize); - } - - section.addData("Working Dir", memberInformation.getWorkingDirPath()); - section.addData("Log file", memberInformation.getLogFilePath()); - - section.addData("Locators", memberInformation.getLocators()); + if (obj == null || !(obj instanceof MemberInformation)) { + return ResultModel.createInfo(String.format( + CliStrings.DESCRIBE_MEMBER__MSG__INFO_FOR__0__COULD_NOT_BE_RETRIEVED, memberNameOrId)); + } + ResultModel result = new ResultModel(); + DataResultModel memberInfo = result.addData("memberInfo"); + + MemberInformation memberInformation = (MemberInformation) obj; + memberInfo.addData("Name", memberInformation.getName()); + memberInfo.addData("Id", memberInformation.getId()); + memberInfo.addData("Host", memberInformation.getHost()); + memberInfo.addData("Regions", StringUtils.join(memberInformation.getHostedRegions(), '\n')); + memberInfo.addData("PID", memberInformation.getProcessId()); + memberInfo.addData("Groups", memberInformation.getGroups()); + memberInfo.addData("Used Heap", memberInformation.getHeapUsage() + "M"); + memberInfo.addData("Max Heap", memberInformation.getMaxHeapSize() + "M"); + + String offHeapMemorySize = memberInformation.getOffHeapMemorySize(); + if (offHeapMemorySize != null && !offHeapMemorySize.isEmpty()) { + memberInfo.addData("Off Heap Size", offHeapMemorySize); + } - if (memberInformation.isServer()) { - CompositeResultData.SectionResultData clientServiceSection = crd.addSection(); - List<CacheServerInfo> csList = memberInformation.getCacheServeInfo(); + memberInfo.addData("Working Dir", memberInformation.getWorkingDirPath()); + memberInfo.addData("Log file", memberInformation.getLogFilePath()); - if (csList != null) { - Iterator<CacheServerInfo> iters = csList.iterator(); - clientServiceSection.setHeader("Cache Server Information"); + memberInfo.addData("Locators", memberInformation.getLocators()); - while (iters.hasNext()) { - CacheServerInfo cacheServerInfo = iters.next(); - clientServiceSection.addData("Server Bind", cacheServerInfo.getBindAddress()); - clientServiceSection.addData("Server Port", cacheServerInfo.getPort()); - clientServiceSection.addData("Running", cacheServerInfo.isRunning()); - } + if (memberInformation.isServer()) { + List<CacheServerInfo> csList = memberInformation.getCacheServeInfo(); + if (csList != null) { + DataResultModel connectionInfo = result.addData("connectionInfo"); + Iterator<CacheServerInfo> iters = csList.iterator(); + connectionInfo.setHeader("Cache Server Information"); - clientServiceSection.addData("Client Connections", memberInformation.getClientCount()); - } + while (iters.hasNext()) { + CacheServerInfo cacheServerInfo = iters.next(); + connectionInfo.addData("Server Bind", cacheServerInfo.getBindAddress()); + connectionInfo.addData("Server Port", cacheServerInfo.getPort()); + connectionInfo.addData("Running", cacheServerInfo.isRunning()); } - result = ResultBuilder.buildResult(crd); - } else { - result = ResultBuilder.createInfoResult( - CliStrings.format(CliStrings.DESCRIBE_MEMBER__MSG__INFO_FOR__0__COULD_NOT_BE_RETRIEVED, - new Object[] {memberNameOrId})); + connectionInfo.addData("Client Connections", memberInformation.getClientCount()); } - } catch (CacheClosedException ignored) { - } catch (Exception e) { - result = ResultBuilder.createGemFireErrorResult(e.getMessage()); } return result; } 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 3b062e3..c0c5d05 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 @@ -1104,7 +1104,7 @@ public class CliStrings { "Name/Id of the member to be described."; public static final String DESCRIBE_MEMBER__MSG__NOT_FOUND = "Member \"{0}\" not found"; public static final String DESCRIBE_MEMBER__MSG__INFO_FOR__0__COULD_NOT_BE_RETRIEVED = - "Information for the member \"{0}\" could not be retrieved."; + "Information for the member %s could not be retrieved."; /* 'describe offline-disk-store' command */ public static final String DESCRIBE_OFFLINE_DISK_STORE = "describe offline-disk-store"; diff --git a/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java b/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java index 2cbeb8b..b2ac7e1 100644 --- a/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java +++ b/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java @@ -71,6 +71,8 @@ public class CommandResultAssert * <pre> * <code> containsKeyValuePair("Key Class", "java.lang.String"); </code> * </pre> + * + * @deprecated use hasDataSection methods to verify the table contents */ public CommandResultAssert containsKeyValuePair(String key, String value) { assertThat(actual.getOutput()).containsPattern("\\Q" + key + "\\E\\s+: \\Q" + value + "\\E"); @@ -89,6 +91,9 @@ public class CommandResultAssert return this; } + /** + * @deprecated use hasDataSection methods to verify the table contents + */ public CommandResultAssert containsOrderedOutput(String dataSectionName, String... expectedOutputs) { @@ -177,6 +182,8 @@ public class CommandResultAssert * <code> tableHasColumnWithExactValuesInExactOrder("Region Path", "/region1", "/region2"); * </code> * </pre> + * + * @deprecated use hasTableSection methods to verify the table contents */ public CommandResultAssert tableHasColumnWithExactValuesInExactOrder(String header, String... expectedValues) { @@ -205,6 +212,8 @@ public class CommandResultAssert * <pre> * <code> tableHasColumnWithExactValuesInAnyOrder("Region Path", "/region2", "/region1"); </code> * </pre> + * + * @deprecated use hasTableSection methods to verify the table contents */ public CommandResultAssert tableHasColumnWithExactValuesInAnyOrder(String header, String... expectedValues) { @@ -214,7 +223,9 @@ public class CommandResultAssert return this; } - + /** + * @deprecated use hasTableSection methods to verify the table contents + */ public CommandResultAssert tableHasRowWithValues(String... headersThenValues) { assertThat(headersThenValues.length % 2) .describedAs("You need to pass even number of parameters.").isEqualTo(0); @@ -265,6 +276,8 @@ public class CommandResultAssert /** * Verifies that each of the actual values in the column with the given header contains at least * one of the expectedValues. + * + * @deprecated use hasTableSection methods to verify the table contents */ public CommandResultAssert tableHasColumnWithValuesContaining(String header, String... expectedValues) { @@ -287,6 +300,8 @@ public class CommandResultAssert /** * Verifies that each of the actual values in the column with the given header contains at least * one of the expectedValues. + * + * @deprecated use hasTableSection methods to verify the table contents */ public CommandResultAssert tableHasColumnOnlyWithValues(String header, String... expectedValues) { List<String> actualValues = actual.getCommandResult().getTableColumnValues(header);