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 4477013 GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing (#5187) 4477013 is described below commit 4477013a1656f3d94c8d36286fa497e4406da005 Author: Alberto Bustamante Reyes <alb3rt...@users.noreply.github.com> AuthorDate: Fri Jun 12 23:34:43 2020 +0200 GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing (#5187) --- .../commands/QueryCommandIntegrationTestBase.java | 15 +++++- .../geode/management/internal/cli/GfshParser.java | 24 +++++---- .../internal/cli/GfshParserJUnitTest.java | 62 ++++++++++++++++++++++ 3 files changed, 90 insertions(+), 11 deletions(-) diff --git a/geode-dunit/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommandIntegrationTestBase.java b/geode-dunit/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommandIntegrationTestBase.java index 34d807f..3444612 100644 --- a/geode-dunit/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommandIntegrationTestBase.java +++ b/geode-dunit/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommandIntegrationTestBase.java @@ -16,6 +16,7 @@ package org.apache.geode.management.internal.cli.commands; import static org.apache.geode.cache.Region.SEPARATOR; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; import java.io.File; import java.io.Serializable; @@ -54,7 +55,8 @@ public class QueryCommandIntegrationTestBase { new ServerStarterRule().withJMXManager() .withHttpService() .withRegion(RegionShortcut.REPLICATE, "simpleRegion") - .withRegion(RegionShortcut.REPLICATE, "complexRegion"); + .withRegion(RegionShortcut.REPLICATE, "complexRegion") + .withRegion(RegionShortcut.REPLICATE, "intRegion"); @Rule public GfshCommandRule gfsh = new GfshCommandRule(); @@ -67,12 +69,14 @@ public class QueryCommandIntegrationTestBase { Cache cache = server.getCache(); Region<String, String> simpleRegion = cache.getRegion("simpleRegion"); Region<String, Customer> complexRegion = cache.getRegion("complexRegion"); + Region<Integer, Integer> intRegion = cache.getRegion("intRegion"); for (int i = 0; i < Gfsh.DEFAULT_APP_FETCH_SIZE + 1; i++) { String key = "key" + i; simpleRegion.put(key, "value" + i); complexRegion.put(key, new Customer("name" + i, "Main Street " + i, "Hometown")); + intRegion.put(new Integer(i), new Integer(i)); } } @@ -93,6 +97,15 @@ public class QueryCommandIntegrationTestBase { } @Test + public void queryWithAndWithoutEqualsReturnSameResult() throws Exception { + String resultWithEquals = + gfsh.execute("query --query='select * from " + SEPARATOR + "intRegion i where i <= 3'"); + String resultWithoutEquals = + gfsh.execute("query --query 'select * from " + SEPARATOR + "intRegion i where i <= 3'"); + assertEquals(resultWithEquals, resultWithoutEquals); + } + + @Test public void doesNotShowLimitIfLimitInQuery() throws Exception { gfsh.executeAndAssertThat( "query --query='select * from " + SEPARATOR + "simpleRegion limit 50'") diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java index 9550ac9..eed2695 100755 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java @@ -123,19 +123,24 @@ public class GfshParser extends SimpleParser { List<String> furtherSplitWithEquals = new ArrayList<>(); for (String token : splitWithWhiteSpaces) { + // do not split with "=" if this part starts with quotes or is part of -D + if (token.startsWith("'") || token.startsWith("\"") || token.startsWith("-D")) { + furtherSplitWithEquals.add(token); + continue; + } // if this token has equal sign, split around the first occurrence of it int indexOfFirstEqual = token.indexOf('='); if (indexOfFirstEqual < 0) { furtherSplitWithEquals.add(token); - } else { - String left = token.substring(0, indexOfFirstEqual); - String right = token.substring(indexOfFirstEqual + 1); - if (left.length() > 0) { - furtherSplitWithEquals.add(left); - } - if (right.length() > 0) { - furtherSplitWithEquals.add(right); - } + continue; + } + String left = token.substring(0, indexOfFirstEqual); + String right = token.substring(indexOfFirstEqual + 1); + if (left.length() > 0) { + furtherSplitWithEquals.add(left); + } + if (right.length() > 0) { + furtherSplitWithEquals.add(right); } } return furtherSplitWithEquals; @@ -200,7 +205,6 @@ public class GfshParser extends SimpleParser { @Override public GfshParseResult parse(String userInput) { String rawInput = convertToSimpleParserInput(userInput); - // this tells the simpleParser not to interpret backslash as escaping character rawInput = rawInput.replace("\\", "\\\\"); // User SimpleParser to parse the input diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java index 7674f75..09fa005 100644 --- a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java +++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java @@ -60,6 +60,46 @@ public class GfshParserJUnitTest { } @Test + public void testQuerySplitUserInputDoubleQuotesWithoutEquals() { + input = "query --query \"select * from " + SEPARATOR + "region\""; + tokens = GfshParser.splitUserInput(input); + assertThat(tokens.size()).isEqualTo(3); + assertThat(tokens.get(0)).isEqualTo("query"); + assertThat(tokens.get(1)).isEqualTo("--query"); + assertThat(tokens.get(2)).isEqualTo("\"select * from " + SEPARATOR + "region\""); + } + + @Test + public void testQuerySplitUserInputSingleQuotesWithoutEquals() { + input = "query --query 'select * from " + SEPARATOR + "region'"; + tokens = GfshParser.splitUserInput(input); + assertThat(tokens.size()).isEqualTo(3); + assertThat(tokens.get(0)).isEqualTo("query"); + assertThat(tokens.get(1)).isEqualTo("--query"); + assertThat(tokens.get(2)).isEqualTo("'select * from " + SEPARATOR + "region'"); + } + + @Test + public void testQuerySplitUserInputWithEqualsInQuery() { + input = "query --query 'select * from " + SEPARATOR + "region r where r <= 5'"; + tokens = GfshParser.splitUserInput(input); + assertThat(tokens.size()).isEqualTo(3); + assertThat(tokens.get(0)).isEqualTo("query"); + assertThat(tokens.get(1)).isEqualTo("--query"); + assertThat(tokens.get(2)).isEqualTo("'select * from " + SEPARATOR + "region r where r <= 5'"); + } + + @Test + public void testQuerySplitUserInputWithDoubleEqualsInQuery() { + input = "query --query 'select * from " + SEPARATOR + "region r where r == 5'"; + tokens = GfshParser.splitUserInput(input); + assertThat(tokens.size()).isEqualTo(3); + assertThat(tokens.get(0)).isEqualTo("query"); + assertThat(tokens.get(1)).isEqualTo("--query"); + assertThat(tokens.get(2)).isEqualTo("'select * from " + SEPARATOR + "region r where r == 5'"); + } + + @Test public void testSplitUserInputWithMixedQuotes() { input = "command option='test1 \"test \" test1'"; tokens = GfshParser.splitUserInput(input); @@ -83,6 +123,18 @@ public class GfshParserJUnitTest { } @Test + public void testSplitUserInputWithJWithNoEquals() { + input = + "start server --name=server1 --J \"-Dgemfire.start-dev-rest-api=true\" --J '-Dgemfire.http-service-port=8080' --J '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000'"; + tokens = GfshParser.splitUserInput(input); + assertThat(tokens.size()).isEqualTo(10); + assertThat(tokens.get(5)).isEqualTo("\"-Dgemfire.start-dev-rest-api=true\""); + assertThat(tokens.get(7)).isEqualTo("'-Dgemfire.http-service-port=8080'"); + assertThat(tokens.get(9)) + .isEqualTo("'-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000'"); + } + + @Test public void splitWithWhiteSpacesExceptQuoted() { input = "create region --cache-writer=\"my.abc{'k1' : 'v 1', 'k2' : 'v2'}\""; tokens = GfshParser.splitUserInput(input); @@ -101,6 +153,16 @@ public class GfshParserJUnitTest { } @Test + public void testSplitUserInputWithJNoQuotesNoEquals() { + input = + "start server --name=server1 --J -Dgemfire.start-dev-rest-api=true --J -Dgemfire.http-service-port=8080"; + tokens = GfshParser.splitUserInput(input); + assertThat(tokens.size()).isEqualTo(8); + assertThat(tokens.get(5)).isEqualTo("-Dgemfire.start-dev-rest-api=true"); + assertThat(tokens.get(7)).isEqualTo("-Dgemfire.http-service-port=8080"); + } + + @Test public void testSplitJsonValue() throws Exception { input = "get --key=('id':'testKey0') --region=regionA"; tokens = GfshParser.splitUserInput(input);