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 2020/06/12 21:35:26 UTC

[geode] branch develop updated: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing (#5187)

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 <al...@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);