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/09/01 03:41:14 UTC
geode git commit: GEODE-3549: fix the constantly failing flaky tests
in CommandOverHttpDUnitTest
Repository: geode
Updated Branches:
refs/heads/develop c62e4cf69 -> 4ce922095
GEODE-3549: fix the constantly failing flaky tests in CommandOverHttpDUnitTest
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/4ce92209
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/4ce92209
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/4ce92209
Branch: refs/heads/develop
Commit: 4ce922095bd726347d01e3f20bae7d3aa8672ec1
Parents: c62e4cf
Author: Jinmei Liao <ji...@pivotal.io>
Authored: Thu Aug 31 14:20:16 2017 -0700
Committer: Jinmei Liao <ji...@pivotal.io>
Committed: Thu Aug 31 20:40:09 2017 -0700
----------------------------------------------------------------------
.../internal/cli/GfshParseResult.java | 25 +++++++----
.../cli/commands/ExecuteFunctionCommand.java | 47 +++++---------------
.../web/controllers/DataCommandsController.java | 3 ++
.../controllers/FunctionCommandsController.java | 8 ++--
.../internal/cli/GfshParserParsingTest.java | 22 +++++++++
.../commands/GemfireDataCommandsDUnitTest.java | 4 +-
6 files changed, 59 insertions(+), 50 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/4ce92209/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
index e91e365..b228b28 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
@@ -14,20 +14,21 @@
*/
package org.apache.geode.management.internal.cli;
-import org.apache.commons.lang.StringUtils;
-import org.apache.geode.management.cli.CliMetaData;
-import org.apache.geode.management.internal.cli.shell.GfshExecutionStrategy;
-import org.apache.geode.management.internal.cli.shell.OperationInvoker;
-import org.springframework.shell.core.annotation.CliCommand;
-import org.springframework.shell.core.annotation.CliOption;
-import org.springframework.shell.event.ParseResult;
-
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import org.apache.commons.lang.StringUtils;
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+import org.springframework.shell.event.ParseResult;
+
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.internal.cli.shell.GfshExecutionStrategy;
+import org.apache.geode.management.internal.cli.shell.OperationInvoker;
+
/**
* Immutable representation of the outcome of parsing a given shell line. * Extends
* {@link ParseResult} to add a field to specify the command string that was input by the user.
@@ -85,8 +86,14 @@ public class GfshParseResult extends ParseResult {
// these will be used for the http request parameters, when turned into the
// commands again, the options will be quoted.
if (argumentAsString.contains(" ")) {
- argumentAsString = "'" + argumentAsString + "'";
+ if (argumentAsString.contains("'")) {
+ argumentAsString = "\"" + argumentAsString + "\"";
+ } else {
+ argumentAsString = "'" + argumentAsString + "'";
+ }
}
+ // this always uses the first variation of the option as the key, so all the controllers
+ // should use this as the parameter key
paramValueStringMap.put(cliOption.key()[0], argumentAsString);
}
}
http://git-wip-us.apache.org/repos/asf/geode/blob/4ce92209/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
index 46ad6e5..8dbcbe6 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
@@ -63,8 +63,9 @@ public class ExecuteFunctionCommand implements GfshCommand {
@CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
optionContext = ConverterHint.MEMBERGROUP,
help = CliStrings.EXECUTE_FUNCTION__ONGROUPS__HELP) String[] onGroups,
- @CliOption(key = CliStrings.MEMBER, optionContext = ConverterHint.MEMBERIDNAME,
- help = CliStrings.EXECUTE_FUNCTION__ONMEMBER__HELP) String onMember,
+ @CliOption(key = {CliStrings.MEMBER, CliStrings.MEMBERS},
+ optionContext = ConverterHint.MEMBERIDNAME,
+ help = CliStrings.EXECUTE_FUNCTION__ONMEMBER__HELP) String[] onMembers,
@CliOption(key = CliStrings.EXECUTE_FUNCTION__ONREGION,
optionContext = ConverterHint.REGION_PATH,
help = CliStrings.EXECUTE_FUNCTION__ONREGION__HELP) String onRegion,
@@ -87,9 +88,6 @@ public class ExecuteFunctionCommand implements GfshCommand {
if (onRegion != null) {
onRegion = onRegion.trim();
}
- if (onMember != null) {
- onMember = onMember.trim();
- }
if (filterString != null) {
filterString = filterString.trim();
}
@@ -103,7 +101,7 @@ public class ExecuteFunctionCommand implements GfshCommand {
return ResultBuilder.buildResult(errorResultData);
}
- if (isMoreThanOneTrue(onRegion != null, onMember != null, onGroups != null)) {
+ if (isMoreThanOneTrue(onRegion != null, onMembers != null, onGroups != null)) {
// Provide Only one of region/member/groups
ErrorResultData errorResultData =
ResultBuilder.createErrorResultData().setErrorCode(ResultBuilder.ERRORCODE_DEFAULT)
@@ -127,7 +125,7 @@ public class ExecuteFunctionCommand implements GfshCommand {
filters.add(filterString);
}
- if (onRegion == null && onMember == null && onGroups == null) {
+ if (onRegion == null && onMembers == null && onGroups == null) {
// run function on all the members excluding locators bug#46113
// if user wish to execute on locator then he can choose --member or --group option
Set<DistributedMember> dsMembers = CliUtil.getAllNormalMembers(cache);
@@ -221,40 +219,19 @@ public class ExecuteFunctionCommand implements GfshCommand {
CliStrings.EXECUTE_FUNCTION__MSG__ERROR_IN_RETRIEVING_EXECUTOR));
}
}
- } else if (onGroups != null) {
- // execute on group members
- Set<DistributedMember> dsMembers = new HashSet<>();
- for (String grp : onGroups) {
- dsMembers.addAll(cache.getDistributedSystem().getGroupMembers(grp));
- }
+ } else if (onGroups != null || onMembers != null) {
+ Set<DistributedMember> dsMembers = CliUtil.findMembers(onGroups, onMembers);
- if (dsMembers.size() > 0) {
- for (DistributedMember member : dsMembers) {
- executeAndGetResults(functionId, filterString, resultCollector, arguments, cache,
- member, resultTable, onRegion);
- }
- return ResultBuilder.buildResult(resultTable);
- } else {
- StringBuilder grps = new StringBuilder();
- for (String grp : onGroups) {
- grps.append(grp);
- grps.append(", ");
- }
- return ResultBuilder.createUserErrorResult(
- CliStrings.format(CliStrings.EXECUTE_FUNCTION__MSG__GROUPS_0_HAS_NO_MEMBERS,
- grps.toString().substring(0, grps.toString().length() - 1)));
+ if (dsMembers.size() == 0) {
+ return ResultBuilder.createUserErrorResult("No members found.");
}
- } else if (onMember != null && onMember.length() > 0) {
- DistributedMember member = CliUtil.getDistributedMemberByNameOrId(onMember); // fix for bug
- // 45658
- if (member != null) {
+
+ for (DistributedMember member : dsMembers) {
executeAndGetResults(functionId, filterString, resultCollector, arguments, cache, member,
resultTable, onRegion);
- } else {
- toTabularResultData(resultTable, onMember, CliStrings
- .format(CliStrings.EXECUTE_FUNCTION__MSG__NO_ASSOCIATED_MEMBER + " " + onMember));
}
return ResultBuilder.buildResult(resultTable);
+
}
} catch (Exception e) {
ErrorResultData errorResultData = ResultBuilder.createErrorResultData()
http://git-wip-us.apache.org/repos/asf/geode/blob/4ce92209/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
index 8a16aa4..3c58f50 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
@@ -151,12 +151,15 @@ public class DataCommandsController extends AbstractCommandsController {
public Callable<ResponseEntity<String>> importData(
@PathVariable("member") final String memberNameId,
@PathVariable("region") final String regionNamePath,
+ @RequestParam(value = CliStrings.IMPORT_DATA__INVOKE_CALLBACKS,
+ required = false) final boolean invokeCallbacks,
@RequestParam(CliStrings.IMPORT_DATA__FILE) final String file) {
final CommandStringBuilder command = new CommandStringBuilder(CliStrings.IMPORT_DATA);
command.addOption(CliStrings.MEMBER, decode(memberNameId));
command.addOption(CliStrings.IMPORT_DATA__REGION, decode(regionNamePath));
command.addOption(CliStrings.IMPORT_DATA__FILE, decode(file));
+ command.addOption(CliStrings.IMPORT_DATA__INVOKE_CALLBACKS, invokeCallbacks + "");
return getProcessCommandCallable(command.toString());
}
http://git-wip-us.apache.org/repos/asf/geode/blob/4ce92209/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java
index da3fc65..e8885da 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java
@@ -77,7 +77,7 @@ public class FunctionCommandsController extends AbstractCommandsController {
@RequestMapping(method = RequestMethod.POST, value = "/functions/{id}")
public Callable<ResponseEntity<String>> executeFunction(
@PathVariable("id") final String functionId,
- @RequestParam(value = CliStrings.GROUPS, required = false) final String groupName,
+ @RequestParam(value = CliStrings.GROUP, required = false) final String groupName,
@RequestParam(value = CliStrings.MEMBER, required = false) final String memberNameId,
@RequestParam(value = CliStrings.EXECUTE_FUNCTION__ONREGION,
required = false) final String regionNamePath,
@@ -92,7 +92,7 @@ public class FunctionCommandsController extends AbstractCommandsController {
command.addOption(CliStrings.EXECUTE_FUNCTION__ID, decode(functionId));
if (hasValue(groupName)) {
- command.addOption(CliStrings.GROUPS, groupName);
+ command.addOption(CliStrings.GROUP, groupName);
}
if (hasValue(memberNameId)) {
@@ -121,14 +121,14 @@ public class FunctionCommandsController extends AbstractCommandsController {
@RequestMapping(method = RequestMethod.DELETE, value = "/functions/{id}")
@ResponseBody
public String destroyFunction(@PathVariable("id") final String functionId,
- @RequestParam(value = CliStrings.GROUPS, required = false) final String groupName,
+ @RequestParam(value = CliStrings.GROUP, required = false) final String groupName,
@RequestParam(value = CliStrings.MEMBER, required = false) final String memberNameId) {
final CommandStringBuilder command = new CommandStringBuilder(CliStrings.DESTROY_FUNCTION);
command.addOption(CliStrings.DESTROY_FUNCTION__ID, decode(functionId));
if (hasValue(groupName)) {
- command.addOption(CliStrings.GROUPS, groupName);
+ command.addOption(CliStrings.GROUP, groupName);
}
if (hasValue(memberNameId)) {
http://git-wip-us.apache.org/repos/asf/geode/blob/4ce92209/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
index 961cdf8..0357105 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
@@ -301,4 +301,26 @@ public class GfshParserParsingTest {
assertThat(result.getParamValue("cache-listener")).isNotNull().isEmpty();
assertThat(result.getParamValue("cache-loader")).isNotNull().isEmpty();
}
+
+ @Test
+ public void testValueOfJsonWithoutOuterQuoteAndSpace() throws Exception {
+ String command = "put --key=('name':'id') --value=456 --region=/test";
+ GfshParseResult result = parser.parse(command);
+ assertThat(result.getParamValue("key")).isEqualTo("('name':'id')");
+ }
+
+ @Test
+ public void testValueOfJsonWithSpace() throws Exception {
+ // this is considerred an invalid command
+ String command = "put --key=('name' : 'id') --value=456 --region=/test";
+ GfshParseResult result = parser.parse(command);
+ assertThat(result).isNull();
+ }
+
+ @Test
+ public void testValueOfJsonWithSpaceAndOuterQuotes() throws Exception {
+ String command = "put --key=\"('name' : 'id')\" --value=456 --region=/test";
+ GfshParseResult result = parser.parse(command);
+ assertThat(result.getParamValue("key")).isEqualTo("\"('name' : 'id')\"");
+ }
}
http://git-wip-us.apache.org/repos/asf/geode/blob/4ce92209/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java
index 0247c79..f2a7432 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java
@@ -556,7 +556,7 @@ public class GemfireDataCommandsDUnitTest extends CliCommandTestBase {
int randomInteger = random.nextInt(COUNT);
String query =
"query --query=\"select ID , status , createTime , pk, floatMinValue from ${DATA_REGION} where ID <= ${PORTFOLIO_ID}"
- + " and status='${STATUS}'" + "\" --interactive=false";
+ + " and status=${STATUS}" + "\" --interactive=false";
executeCommand("set variable --name=DATA_REGION --value=" + DATA_REGION_NAME_PATH);
executeCommand("set variable --name=PORTFOLIO_ID --value=" + randomInteger);
executeCommand("set variable --name=STATUS --value=" + (statusActive ? "active" : "inactive"));
@@ -568,7 +568,7 @@ public class GemfireDataCommandsDUnitTest extends CliCommandTestBase {
try {
query =
"query --query=\"select ID , status , createTime , pk, floatMinValue from ${DATA_REGION2} where ID <= ${PORTFOLIO_ID2}"
- + " and status='${STATUS2}'" + "\" --interactive=false";
+ + " and status=${STATUS2}" + "\" --interactive=false";
cmdResult = executeCommand(query);
printCommandOutput(cmdResult);
validateSelectResult(cmdResult, false, 0, null);