You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2017/10/18 15:59:59 UTC
[geode] branch develop updated: GEODE-3819: correctly set the result status code when building the re… (#935)
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao 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 b16a709 GEODE-3819: correctly set the result status code when building the re… (#935)
b16a709 is described below
commit b16a709bc72589446959740f88de8d25793c9e17
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Wed Oct 18 08:59:57 2017 -0700
GEODE-3819: correctly set the result status code when building the re… (#935)
* GEODE-3819: correctly set the result status code when building the result from json string
---
.../internal/cli/commands/CreateRegionCommand.java | 8 +-------
.../management/internal/cli/result/ErrorResultData.java | 6 ++++--
.../management/internal/cli/result/ResultBuilder.java | 7 +++++++
.../management/internal/cli/result/ResultBuilderTest.java | 9 +++++++++
.../internal/security/GfshCommandsSecurityTest.java | 14 +++++---------
5 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
index e5aced8..708c3f1 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
@@ -29,7 +29,6 @@ import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
import org.apache.geode.cache.DataPolicy;
-import org.apache.geode.cache.PartitionResolver;
import org.apache.geode.cache.Region;
import org.apache.geode.cache.RegionAttributes;
import org.apache.geode.cache.RegionShortcut;
@@ -159,7 +158,6 @@ public class CreateRegionCommand implements GfshCommand {
) {
Result result;
AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
- String lastError = null;
try {
InternalCache cache = getCache();
@@ -297,7 +295,7 @@ public class CreateRegionCommand implements GfshCommand {
if (success) {
xmlEntity.set(regionCreateResult.getXmlEntity());
} else {
- lastError = regionCreateResult.getMessage();
+ tabularResultData.setStatus(Result.Status.ERROR);
}
}
result = ResultBuilder.buildResult(tabularResultData);
@@ -308,10 +306,6 @@ public class CreateRegionCommand implements GfshCommand {
result = ResultBuilder.createUserErrorResult(e.getMessage());
}
- if (lastError != null) {
- result = ResultBuilder.createUserErrorResult(lastError);
- }
-
if (xmlEntity.get() != null) {
persistClusterConfiguration(result,
() -> getSharedConfiguration().addXmlEntity(xmlEntity.get(), groups));
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ErrorResultData.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ErrorResultData.java
index 9edcdbf..9734bf2 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ErrorResultData.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ErrorResultData.java
@@ -98,8 +98,10 @@ public class ErrorResultData extends InfoResultData {
@Override
public void setStatus(final Status status) {
- throw new UnsupportedOperationException(
- "The status of an ErrorResultData result must always be ERROR");
+ if (status != Status.ERROR) {
+ throw new UnsupportedOperationException(
+ "The status of an ErrorResultData result must always be ERROR");
+ }
}
@Override
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java
index 460ade2..22db17c 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java
@@ -230,6 +230,13 @@ public class ResultBuilder {
resultData = errorResultData;
}
+ Integer statusCode = (Integer) jsonObject.get("status");
+ if (Result.Status.OK.getCode() == statusCode) {
+ resultData.setStatus(Result.Status.OK);
+ } else {
+ resultData.setStatus(Result.Status.ERROR);
+ }
+
result = buildResult(resultData);
String fileToDownloadPath = jsonObject.getString("fileToDownload");
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
index 0ce89c1..3b4e371 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
@@ -128,4 +128,13 @@ public class ResultBuilderTest {
result.addSection();
}
+
+ @Test
+ public void errorCodeCorrectlyUpdated() throws Exception {
+ String json =
+ "{\"contentType\":\"table\",\"data\":{\"content\":{\"Member\":[\"server\"],\"Status\":[\"ERROR: Bad.\"]},\"footer\":\"\",\"header\":\"\",\"type-class\":\"org.apache.geode.management.internal.cli.CommandResponse.Data\"},\"debugInfo\":\"\",\"failedToPersist\":false,\"fileToDownload\":null,\"page\":\"1/1\",\"sender\":\"server\",\"status\":-1,\"tokenAccessor\":\"__NULL__\",\"type-class\":\"org.apache.geode.management.internal.cli.CommandResponse\",\"version\":\"1.3.0-SNAPSHOT\",\"whe [...]
+
+ CommandResult result = ResultBuilder.fromJson(json);
+ assertThat(result.getStatus().getCode()).isEqualTo(-1);
+ }
}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
index abbf7c0..648784f 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
@@ -32,11 +32,11 @@ import org.apache.geode.management.internal.cli.result.CommandResult;
import org.apache.geode.management.internal.cli.result.ErrorResultData;
import org.apache.geode.management.internal.cli.result.ResultBuilder;
import org.apache.geode.security.SimpleTestSecurityManager;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
import org.apache.geode.test.junit.rules.ConnectionConfiguration;
import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
import org.apache.geode.test.junit.rules.ServerStarterRule;
-import org.apache.geode.test.junit.categories.IntegrationTest;
-import org.apache.geode.test.junit.categories.SecurityTest;
@Category({IntegrationTest.class, SecurityTest.class})
public class GfshCommandsSecurityTest {
@@ -135,12 +135,9 @@ public class GfshCommandsSecurityTest {
CommandResult result = gfshConnection.executeCommand(permitted.getCommand());
assertThat(result).isNotNull();
- if (result.getResultData() instanceof ErrorResultData) {
- assertThat(ResultBuilder.ERRORCODE_UNAUTHORIZED).describedAs(permitted.getCommand())
- .isNotEqualTo(((ErrorResultData) result.getResultData()).getErrorCode());
- } else {
- assertThat(Result.Status.OK).describedAs(permitted.toString())
- .isEqualTo(result.getStatus());
+ // for permitted commands, if any error happens, it's not an Unauthorized error
+ if (result.getStatus() == Result.Status.ERROR) {
+ assertThat(result.getContent().toString()).doesNotContain("not authorized");
}
}
@@ -148,7 +145,6 @@ public class GfshCommandsSecurityTest {
List<TestCommand> others = TestCommand.getOnlineCommands();
others.removeAll(allPermitted);
for (TestCommand other : others) {
-
System.out.println("Processing unauthorized command: " + other.getCommand());
CommandResult result = gfshConnection.executeCommand(other.getCommand());
int errorCode = ((ErrorResultData) result.getResultData()).getErrorCode();
--
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].