From cc80f2fc4f93db61106c58037d7523312b9e60d8 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Fri, 17 Nov 2023 16:23:56 +0300 Subject: [PATCH 1/8] Fix string scalar encoding --- .../cayenne/exp/CayenneExpressionVisitor.java | 25 ++++++++++++++++--- .../cayenne/exp/CayenneExpParserTest.java | 12 ++++----- .../AgExpressionParserTokenManager.java | 4 +-- .../java/io/agrest/exp/parser/ExpScalar.java | 10 +++++--- .../io/agrest/exp/parser/ExpScalarList.java | 2 +- .../agrest/exp/parser/AgExpressionParser.jjt | 4 +-- .../agrest/exp/parser/CompositeExpTest.java | 2 +- .../java/io/agrest/exp/parser/ExpInTest.java | 4 +-- .../exp/parser/ExpScalarStringTest.java | 9 +++---- .../test/java/io/agrest/protocol/ExpTest.java | 24 +++++++++--------- .../runtime/protocol/ExpParserTest.java | 6 ++--- 11 files changed, 61 insertions(+), 41 deletions(-) diff --git a/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java b/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java index f0720c906..19451e30e 100644 --- a/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java +++ b/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java @@ -8,7 +8,9 @@ import org.apache.cayenne.exp.parser.*; import java.lang.reflect.Constructor; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.function.BiFunction; class CayenneExpressionVisitor implements AgExpressionParserVisitor { @@ -116,10 +118,20 @@ public Expression visit(ExpNotBetween node, Expression data) { @Override public Expression visit(ExpScalarList node, Expression data) { // NOTE: we are skipping all the List children as they are processed by the getValue() call - Collection value = node.getValue(); - Object[] cayenneValues = new Object[value.size()]; + Collection values = node.getValue(); + List preparedValues = new ArrayList<>(); + for (Object value : values) { + if (!(value instanceof CharSequence)) { + preparedValues.add(value); + continue; + } + String stringValue = value.toString(); + preparedValues.add(stringValue.substring(1, stringValue.length() - 1)); + } + + Object[] cayenneValues = new Object[preparedValues.size()]; int i = 0; - for (Object next : value) { + for (Object next : preparedValues) { if (next instanceof ExpNamedParameter) { cayenneValues[i++] = new ExpressionParameter(((ExpNamedParameter) next).getName()); } else { @@ -137,7 +149,12 @@ public Expression visit(ExpScalarList node, Expression data) { @Override public Expression visit(ExpScalar node, Expression data) { - return process(node, data, new ASTScalar(node.jjtGetValue())); + Object scalarValue = node.jjtGetValue(); + if (scalarValue instanceof CharSequence) { + String value = scalarValue.toString(); + return process(node, data, new ASTScalar(value.substring(1, value.length() - 1))); + } + return process(node, data, new ASTScalar(scalarValue)); } @Override diff --git a/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java b/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java index 791a405c6..254547be9 100644 --- a/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java +++ b/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java @@ -16,19 +16,19 @@ public class CayenneExpParserTest { @Test public void parseNamedParams() { - Expression e = parser.parse(Exp.parse("a = $a").namedParams(Map.of("a", "x"))); + Expression e = parser.parse(Exp.parse("a = $a").namedParams(Map.of("a", "'x'"))); assertEquals(ExpressionFactory.exp("a = 'x'"), e); } @Test public void parsePositionalParams() { - Expression e = parser.parse(Exp.parse("a = $a").positionalParams("x")); + Expression e = parser.parse(Exp.parse("a = $a").positionalParams("'x'")); assertEquals(ExpressionFactory.exp("a = 'x'"), e); } @Test public void parsePositionalParams_NullAndParam() { - Exp agExp = Exp.parse("a = null or a.b = $b").positionalParams("B"); + Exp agExp = Exp.parse("a = null or a.b = $b").positionalParams("'B'"); Expression e = parser.parse(agExp); assertEquals(ExpressionFactory.exp("a = null or a.b = 'B'"), e); } @@ -64,7 +64,7 @@ public void parseIn() { Expression e1 = parser.parse(Exp.in("a", 5, 6, 7)); assertEquals(ExpressionFactory.exp("a in (5, 6, 7)"), e1); - Expression e2 = parser.parse(Exp.in("a", "x", "y", "z")); + Expression e2 = parser.parse(Exp.in("a", "'x'", "'y'", "'z'")); assertEquals(ExpressionFactory.exp("a in ('x','y','z')"), e2); Expression e3 = parser.parse(Exp.in("a", 5, 6, 7)); @@ -81,8 +81,8 @@ public void parseEqual_DbPath() { public void parseCompositeCondition() { Exp e0 = Exp.parse("a = 'b'"); - Exp e1 = Exp.parse("b = $a").namedParams(Map.of("a", "x")); - Exp e2 = Exp.parse("c = $a").positionalParams("y"); + Exp e1 = Exp.parse("b = $a").namedParams(Map.of("a", "'x'")); + Exp e2 = Exp.parse("c = $a").positionalParams("'y'"); // multilevel composite with heterogeneous params Exp e3 = Exp.parse("d = 'z'") diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java index 27d6d2ea3..a624049c3 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java @@ -2555,11 +2555,11 @@ void TokenLexicalActions(Token matchedToken) { case 71 : image.append(input_stream.GetSuffix(jjimageLen + (lengthOfMatch = jjmatchedPos + 1))); - literalValue = stringBuffer.toString(); + literalValue = "'" + stringBuffer.toString() + "'"; break; case 74 : image.append(input_stream.GetSuffix(jjimageLen + (lengthOfMatch = jjmatchedPos + 1))); - literalValue = stringBuffer.toString(); + literalValue = "\"" + stringBuffer.toString() + "\""; break; case 75 : image.append(input_stream.GetSuffix(jjimageLen + (lengthOfMatch = jjmatchedPos + 1))); diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java index 2625888ba..23e9ae852 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java @@ -43,10 +43,14 @@ protected ExpScalar shallowCopy() { @Override public String toString() { if (value instanceof CharSequence) { - return "'" + value + "'"; - } else { - return String.valueOf(value); + String stringValue = value.toString(); + String quoteChar = stringValue.substring(0, 1); + String escapedContent = stringValue + .substring(1, stringValue.length() - 1) + .replaceAll(quoteChar, "\\\\" + quoteChar); + return quoteChar + escapedContent + quoteChar; } + return String.valueOf(value); } } /* JavaCC - OriginalChecksum=21004db13a44c6b16cc9797a6f36a4af (do not edit this line) */ diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java index 9025df7e1..ca1effc07 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java @@ -72,7 +72,7 @@ public String toString() { // children can be either an internal collection, or child nodes, so must use "getValue()" return getValue() .stream() - .map(v -> v instanceof CharSequence ? "'" + v + "'" : String.valueOf(v)) + .map(String::valueOf) .collect(Collectors.joining(", ")); } diff --git a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt index 12b813ba0..de7fc8e62 100644 --- a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt +++ b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt @@ -624,7 +624,7 @@ MORE: TOKEN : { - { literalValue = stringBuffer.toString(); } + { literalValue = "'" + stringBuffer.toString() + "'"; } : DEFAULT } @@ -640,7 +640,7 @@ MORE: TOKEN: { - { literalValue = stringBuffer.toString(); } + { literalValue = "\"" + stringBuffer.toString() + "\""; } : DEFAULT } diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java index fcbc20461..e3f9c3b9e 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java @@ -211,7 +211,7 @@ public void greaterAndLike() { .addChild(expFromType(ExpPath.class) .withValue("t.name")) .addChild(expFromType(ExpScalar.class) - .withValue("%story"))) + .withValue("'%story'"))) .build(); assertEquals(expected, AgExpressionParser.parse("t.amount > 0 and t.name like '%story'")); diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java index 6239cbc3b..2fb6c6c92 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java @@ -55,12 +55,12 @@ public void parseInvalidGrammar(String expString) { @Test public void parameterizedToString() { - assertEquals("a in ('x', 'y')", Exp.parse("a in $l").positionalParams(List.of("x", "y")).toString()); + assertEquals("a in ('x', 'y')", Exp.parse("a in $l").positionalParams(List.of("'x'", "'y'")).toString()); } @Test public void manualToString() { - assertEquals("a in ('x', 'y')", Exp.in("a", "x", "y").toString()); + assertEquals("a in ('x', 'y')", Exp.in("a", "'x'", "'y'").toString()); } @Test diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java index a2e8b6283..5e0431be2 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java @@ -28,13 +28,12 @@ void parse(String expString) { @CsvSource(delimiter = '|', quoteCharacter = 'X', value = { "'example'|'example'", " 'example' |'example'", - "\"example\"|'example'", + "\"example\"|\"example\"", "''|''", "' '|' '", - "\"\\\"example\\\"\"|'\"example\"'", - // TODO: this is wrong, single quote must be escaped or double quotes used in output - "\"a'b\"|'a'b'", - "'a\"b'|'a\"b'" + "\"\\\"example\\\"\"|\"\\\"example\\\"\"", + "\"a\\\"'b\"|\"a\\\"'b\"", + "'a\"\\'b'|'a\"\\'b'" }) public void parsedToString(String expString, String expected) { assertEquals(expected, Exp.parse(expString).toString()); diff --git a/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java b/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java index 141dd96c3..e85f607f5 100644 --- a/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java +++ b/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java @@ -82,8 +82,8 @@ public void parentImmutable() { @Test public void paramsImmutable() { Exp raw = Exp.parse("a = $1"); - Exp e1 = raw.positionalParams("b"); - Exp e2 = raw.namedParams(Map.of("1", "c")); + Exp e1 = raw.positionalParams("'b'"); + Exp e2 = raw.namedParams(Map.of("1", "'c'")); assertEquals("(a) = ($1)", raw.toString()); assertEquals("(a) = ('b')", e1.toString()); @@ -146,12 +146,12 @@ public void notBetween() { @Test public void equal() { - assertEquals("(a) = ('b')", Exp.equal("a", "b").toString()); + assertEquals("(a) = ('b')", Exp.equal("a", "'b'").toString()); } @Test public void notEqual() { - assertEquals("(a) != ('b')", Exp.notEqual("a", "b").toString()); + assertEquals("(a) != ('b')", Exp.notEqual("a", "'b'").toString()); } @Test @@ -161,42 +161,42 @@ public void lessOrEqual() { @Test public void like() { - assertEquals("a like 'x%'", Exp.like("a", "x%").toString()); + assertEquals("a like 'x%'", Exp.like("a", "'x%'").toString()); } @Test public void notLike() { - assertEquals("a not like 'x%'", Exp.notLike("a", "x%").toString()); + assertEquals("a not like 'x%'", Exp.notLike("a", "'x%'").toString()); } @Test public void likeIgnoreCase() { - assertEquals("a likeIgnoreCase 'x%'", Exp.likeIgnoreCase("a", "x%").toString()); + assertEquals("a likeIgnoreCase 'x%'", Exp.likeIgnoreCase("a", "'x%'").toString()); } @Test public void notLikeIgnoreCase() { - assertEquals("a not likeIgnoreCase 'x%'", Exp.notLikeIgnoreCase("a", "x%").toString()); + assertEquals("a not likeIgnoreCase 'x%'", Exp.notLikeIgnoreCase("a", "'x%'").toString()); } @Test public void in() { - assertEquals("a in ('a', 4, 'b')", Exp.in("a", "a", 4, "b").toString()); + assertEquals("a in ('a', 4, 'b')", Exp.in("a", "'a'", 4, "'b'").toString()); } @Test public void inCollection() { - assertEquals("a in ('a', 4, 'b')", Exp.inCollection("a", List.of("a", 4, "b")).toString()); + assertEquals("a in ('a', 4, 'b')", Exp.inCollection("a", List.of("'a'", 4, "'b'")).toString()); } @Test public void notIn() { - assertEquals("a not in ('a', 4, 'b')", Exp.notIn("a", "a", 4, "b").toString()); + assertEquals("a not in ('a', 4, 'b')", Exp.notIn("a", "'a'", 4, "'b'").toString()); } @Test public void notInCollection() { - assertEquals("a not in ('a', 4, 'b')", Exp.notInCollection("a", List.of("a", 4, "b")).toString()); + assertEquals("a not in ('a', 4, 'b')", Exp.notInCollection("a", List.of("'a'", 4, "'b'")).toString()); } @Test diff --git a/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java b/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java index fd12ee2fa..beaa09845 100644 --- a/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java +++ b/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java @@ -40,14 +40,14 @@ public void fromString_List() { @Test public void fromString_List_Params_String() { - Exp exp = parser.fromString("[\"b=$s\",\"x\"]"); + Exp exp = parser.fromString("[\"b=$s\",\"'x'\"]"); assertNotNull(exp); assertEquals("(b) = ('x')", exp.toString()); } @Test public void fromString_List_Params_Multiple() { - Exp exp = parser.fromString("[\"b=$s or b =$x or b =$s\",\"x\",\"y\"]"); + Exp exp = parser.fromString("[\"b=$s or b =$x or b =$s\",\"'x'\",\"'y'\"]"); assertNotNull(exp); assertEquals("((b) = ('x')) or ((b) = ('y')) or ((b) = ('x'))", exp.toString()); } @@ -61,7 +61,7 @@ public void fromString_Map() { @Test public void fromString_Map_Params_String() { - Exp exp = parser.fromString("{\"exp\" : \"b=$s\", \"params\":{\"s\":\"x\"}}"); + Exp exp = parser.fromString("{\"exp\" : \"b=$s\", \"params\":{\"s\":\"'x'\"}}"); assertNotNull(exp); assertEquals("(b) = ('x')", exp.toString()); } From 9f686f421b37d6ae0ad602f92a8a64af48457703 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Wed, 31 Jan 2024 15:05:34 +0300 Subject: [PATCH 2/8] Fix numeric scalar encoding --- .../java/io/agrest/exp/parser/ExpScalar.java | 35 ++++++++++++++----- .../agrest/exp/parser/ExpScalarFloatTest.java | 4 +-- .../agrest/exp/parser/ExpScalarIntTest.java | 6 ++-- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java index 23e9ae852..1e75ff644 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java @@ -2,6 +2,8 @@ /* JavaCCOptions:MULTI=true,NODE_USES_PARSER=false,VISITOR=true,TRACK_TOKENS=false,NODE_PREFIX=Exp,NODE_EXTENDS=,NODE_FACTORY=,SUPPORT_CLASS_VISIBILITY_PUBLIC=true */ package io.agrest.exp.parser; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.function.Function; public class ExpScalar extends ExpBaseScalar { @@ -42,14 +44,31 @@ protected ExpScalar shallowCopy() { @Override public String toString() { - if (value instanceof CharSequence) { - String stringValue = value.toString(); - String quoteChar = stringValue.substring(0, 1); - String escapedContent = stringValue - .substring(1, stringValue.length() - 1) - .replaceAll(quoteChar, "\\\\" + quoteChar); - return quoteChar + escapedContent + quoteChar; - } + return stringifyValue(value); + } + + private static String stringifyValue(CharSequence value) { + String stringValue = value.toString(); + String quoteChar = stringValue.substring(0, 1); + String escapedContent = stringValue + .substring(1, stringValue.length() - 1) + .replaceAll(quoteChar, "\\\\" + quoteChar); + return quoteChar + escapedContent + quoteChar; + } + + private static String stringifyValue(Long value) { + return value + "L"; + } + + private static String stringifyValue(BigInteger value) { + return value + "H"; + } + + private static String stringifyValue(BigDecimal value) { + return value + "B"; + } + + private static String stringifyValue(Object value) { return String.valueOf(value); } } diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarFloatTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarFloatTest.java index 5e3fc9544..b0371b74c 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarFloatTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarFloatTest.java @@ -47,9 +47,7 @@ void parse(String expString) { "3.4028235e+38f|3.4028235E38", "1.7976931348623157e+308|1.7976931348623157E308", "1.7976931348623157e+308d|1.7976931348623157E308", - - // TODO: this is wrong, B suffix expected - "1.7976931348623157e+309b|1.7976931348623157E+309" + "1.7976931348623157e+309b|1.7976931348623157E+309B" }) public void parsedToString(String expString, String expected) { assertEquals(expected, Exp.parse(expString).toString()); diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarIntTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarIntTest.java index 765ee4cf0..556323e0f 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarIntTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarIntTest.java @@ -43,10 +43,8 @@ void parse(String expString) { "1|1", " 1 |1", "2147483647|2147483647", - - // TODO: this is wrong, L suffix expected in the output - "2147483648L|2147483648", - "9223372036854775808H|9223372036854775808", + "2147483648L|2147483648L", + "9223372036854775808H|9223372036854775808H", "01234567|342391", "0x12345678|305419896", "0x09abcdef|162254319" From 92c4ba6ed1f0599641936fcf15e705a31a9a4bc5 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Thu, 1 Feb 2024 19:52:08 +0300 Subject: [PATCH 3/8] Revert "Fix string scalar encoding" This reverts commit cc80f2fc --- .../cayenne/exp/CayenneExpressionVisitor.java | 25 +++----------- .../cayenne/exp/CayenneExpParserTest.java | 12 +++---- .../AgExpressionParserTokenManager.java | 4 +-- .../java/io/agrest/exp/parser/ExpScalar.java | 33 +++---------------- .../io/agrest/exp/parser/ExpScalarList.java | 2 +- .../agrest/exp/parser/AgExpressionParser.jjt | 4 +-- .../agrest/exp/parser/CompositeExpTest.java | 2 +- .../java/io/agrest/exp/parser/ExpInTest.java | 4 +-- .../exp/parser/ExpScalarStringTest.java | 9 ++--- .../test/java/io/agrest/protocol/ExpTest.java | 24 +++++++------- .../runtime/protocol/ExpParserTest.java | 6 ++-- 11 files changed, 43 insertions(+), 82 deletions(-) diff --git a/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java b/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java index 19451e30e..f0720c906 100644 --- a/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java +++ b/agrest-cayenne/src/main/java/io/agrest/cayenne/exp/CayenneExpressionVisitor.java @@ -8,9 +8,7 @@ import org.apache.cayenne.exp.parser.*; import java.lang.reflect.Constructor; -import java.util.ArrayList; import java.util.Collection; -import java.util.List; import java.util.function.BiFunction; class CayenneExpressionVisitor implements AgExpressionParserVisitor { @@ -118,20 +116,10 @@ public Expression visit(ExpNotBetween node, Expression data) { @Override public Expression visit(ExpScalarList node, Expression data) { // NOTE: we are skipping all the List children as they are processed by the getValue() call - Collection values = node.getValue(); - List preparedValues = new ArrayList<>(); - for (Object value : values) { - if (!(value instanceof CharSequence)) { - preparedValues.add(value); - continue; - } - String stringValue = value.toString(); - preparedValues.add(stringValue.substring(1, stringValue.length() - 1)); - } - - Object[] cayenneValues = new Object[preparedValues.size()]; + Collection value = node.getValue(); + Object[] cayenneValues = new Object[value.size()]; int i = 0; - for (Object next : preparedValues) { + for (Object next : value) { if (next instanceof ExpNamedParameter) { cayenneValues[i++] = new ExpressionParameter(((ExpNamedParameter) next).getName()); } else { @@ -149,12 +137,7 @@ public Expression visit(ExpScalarList node, Expression data) { @Override public Expression visit(ExpScalar node, Expression data) { - Object scalarValue = node.jjtGetValue(); - if (scalarValue instanceof CharSequence) { - String value = scalarValue.toString(); - return process(node, data, new ASTScalar(value.substring(1, value.length() - 1))); - } - return process(node, data, new ASTScalar(scalarValue)); + return process(node, data, new ASTScalar(node.jjtGetValue())); } @Override diff --git a/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java b/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java index 254547be9..791a405c6 100644 --- a/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java +++ b/agrest-cayenne/src/test/java/io/agrest/cayenne/exp/CayenneExpParserTest.java @@ -16,19 +16,19 @@ public class CayenneExpParserTest { @Test public void parseNamedParams() { - Expression e = parser.parse(Exp.parse("a = $a").namedParams(Map.of("a", "'x'"))); + Expression e = parser.parse(Exp.parse("a = $a").namedParams(Map.of("a", "x"))); assertEquals(ExpressionFactory.exp("a = 'x'"), e); } @Test public void parsePositionalParams() { - Expression e = parser.parse(Exp.parse("a = $a").positionalParams("'x'")); + Expression e = parser.parse(Exp.parse("a = $a").positionalParams("x")); assertEquals(ExpressionFactory.exp("a = 'x'"), e); } @Test public void parsePositionalParams_NullAndParam() { - Exp agExp = Exp.parse("a = null or a.b = $b").positionalParams("'B'"); + Exp agExp = Exp.parse("a = null or a.b = $b").positionalParams("B"); Expression e = parser.parse(agExp); assertEquals(ExpressionFactory.exp("a = null or a.b = 'B'"), e); } @@ -64,7 +64,7 @@ public void parseIn() { Expression e1 = parser.parse(Exp.in("a", 5, 6, 7)); assertEquals(ExpressionFactory.exp("a in (5, 6, 7)"), e1); - Expression e2 = parser.parse(Exp.in("a", "'x'", "'y'", "'z'")); + Expression e2 = parser.parse(Exp.in("a", "x", "y", "z")); assertEquals(ExpressionFactory.exp("a in ('x','y','z')"), e2); Expression e3 = parser.parse(Exp.in("a", 5, 6, 7)); @@ -81,8 +81,8 @@ public void parseEqual_DbPath() { public void parseCompositeCondition() { Exp e0 = Exp.parse("a = 'b'"); - Exp e1 = Exp.parse("b = $a").namedParams(Map.of("a", "'x'")); - Exp e2 = Exp.parse("c = $a").positionalParams("'y'"); + Exp e1 = Exp.parse("b = $a").namedParams(Map.of("a", "x")); + Exp e2 = Exp.parse("c = $a").positionalParams("y"); // multilevel composite with heterogeneous params Exp e3 = Exp.parse("d = 'z'") diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java index 71e2283af..2471bbf6d 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParserTokenManager.java @@ -2692,11 +2692,11 @@ void TokenLexicalActions(Token matchedToken) break; case 71 : image.append(input_stream.GetSuffix(jjimageLen + (lengthOfMatch = jjmatchedPos + 1))); - literalValue = "'" + stringBuffer.toString() + "'"; + literalValue = stringBuffer.toString(); break; case 74 : image.append(input_stream.GetSuffix(jjimageLen + (lengthOfMatch = jjmatchedPos + 1))); - literalValue = "\"" + stringBuffer.toString() + "\""; + literalValue = stringBuffer.toString(); break; case 75 : image.append(input_stream.GetSuffix(jjimageLen + (lengthOfMatch = jjmatchedPos + 1))); diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java index 1e75ff644..2625888ba 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java @@ -2,8 +2,6 @@ /* JavaCCOptions:MULTI=true,NODE_USES_PARSER=false,VISITOR=true,TRACK_TOKENS=false,NODE_PREFIX=Exp,NODE_EXTENDS=,NODE_FACTORY=,SUPPORT_CLASS_VISIBILITY_PUBLIC=true */ package io.agrest.exp.parser; -import java.math.BigDecimal; -import java.math.BigInteger; import java.util.function.Function; public class ExpScalar extends ExpBaseScalar { @@ -44,32 +42,11 @@ protected ExpScalar shallowCopy() { @Override public String toString() { - return stringifyValue(value); - } - - private static String stringifyValue(CharSequence value) { - String stringValue = value.toString(); - String quoteChar = stringValue.substring(0, 1); - String escapedContent = stringValue - .substring(1, stringValue.length() - 1) - .replaceAll(quoteChar, "\\\\" + quoteChar); - return quoteChar + escapedContent + quoteChar; - } - - private static String stringifyValue(Long value) { - return value + "L"; - } - - private static String stringifyValue(BigInteger value) { - return value + "H"; - } - - private static String stringifyValue(BigDecimal value) { - return value + "B"; - } - - private static String stringifyValue(Object value) { - return String.valueOf(value); + if (value instanceof CharSequence) { + return "'" + value + "'"; + } else { + return String.valueOf(value); + } } } /* JavaCC - OriginalChecksum=21004db13a44c6b16cc9797a6f36a4af (do not edit this line) */ diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java index ca1effc07..9025df7e1 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalarList.java @@ -72,7 +72,7 @@ public String toString() { // children can be either an internal collection, or child nodes, so must use "getValue()" return getValue() .stream() - .map(String::valueOf) + .map(v -> v instanceof CharSequence ? "'" + v + "'" : String.valueOf(v)) .collect(Collectors.joining(", ")); } diff --git a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt index 2388abe4e..0a6148530 100644 --- a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt +++ b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt @@ -643,7 +643,7 @@ MORE: TOKEN : { - { literalValue = "'" + stringBuffer.toString() + "'"; } + { literalValue = stringBuffer.toString(); } : DEFAULT } @@ -659,7 +659,7 @@ MORE: TOKEN: { - { literalValue = "\"" + stringBuffer.toString() + "\""; } + { literalValue = stringBuffer.toString(); } : DEFAULT } diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java index e3f9c3b9e..fcbc20461 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/CompositeExpTest.java @@ -211,7 +211,7 @@ public void greaterAndLike() { .addChild(expFromType(ExpPath.class) .withValue("t.name")) .addChild(expFromType(ExpScalar.class) - .withValue("'%story'"))) + .withValue("%story"))) .build(); assertEquals(expected, AgExpressionParser.parse("t.amount > 0 and t.name like '%story'")); diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java index 2fb6c6c92..6239cbc3b 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpInTest.java @@ -55,12 +55,12 @@ public void parseInvalidGrammar(String expString) { @Test public void parameterizedToString() { - assertEquals("a in ('x', 'y')", Exp.parse("a in $l").positionalParams(List.of("'x'", "'y'")).toString()); + assertEquals("a in ('x', 'y')", Exp.parse("a in $l").positionalParams(List.of("x", "y")).toString()); } @Test public void manualToString() { - assertEquals("a in ('x', 'y')", Exp.in("a", "'x'", "'y'").toString()); + assertEquals("a in ('x', 'y')", Exp.in("a", "x", "y").toString()); } @Test diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java index 5e0431be2..a2e8b6283 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java @@ -28,12 +28,13 @@ void parse(String expString) { @CsvSource(delimiter = '|', quoteCharacter = 'X', value = { "'example'|'example'", " 'example' |'example'", - "\"example\"|\"example\"", + "\"example\"|'example'", "''|''", "' '|' '", - "\"\\\"example\\\"\"|\"\\\"example\\\"\"", - "\"a\\\"'b\"|\"a\\\"'b\"", - "'a\"\\'b'|'a\"\\'b'" + "\"\\\"example\\\"\"|'\"example\"'", + // TODO: this is wrong, single quote must be escaped or double quotes used in output + "\"a'b\"|'a'b'", + "'a\"b'|'a\"b'" }) public void parsedToString(String expString, String expected) { assertEquals(expected, Exp.parse(expString).toString()); diff --git a/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java b/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java index e85f607f5..141dd96c3 100644 --- a/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java +++ b/agrest-engine/src/test/java/io/agrest/protocol/ExpTest.java @@ -82,8 +82,8 @@ public void parentImmutable() { @Test public void paramsImmutable() { Exp raw = Exp.parse("a = $1"); - Exp e1 = raw.positionalParams("'b'"); - Exp e2 = raw.namedParams(Map.of("1", "'c'")); + Exp e1 = raw.positionalParams("b"); + Exp e2 = raw.namedParams(Map.of("1", "c")); assertEquals("(a) = ($1)", raw.toString()); assertEquals("(a) = ('b')", e1.toString()); @@ -146,12 +146,12 @@ public void notBetween() { @Test public void equal() { - assertEquals("(a) = ('b')", Exp.equal("a", "'b'").toString()); + assertEquals("(a) = ('b')", Exp.equal("a", "b").toString()); } @Test public void notEqual() { - assertEquals("(a) != ('b')", Exp.notEqual("a", "'b'").toString()); + assertEquals("(a) != ('b')", Exp.notEqual("a", "b").toString()); } @Test @@ -161,42 +161,42 @@ public void lessOrEqual() { @Test public void like() { - assertEquals("a like 'x%'", Exp.like("a", "'x%'").toString()); + assertEquals("a like 'x%'", Exp.like("a", "x%").toString()); } @Test public void notLike() { - assertEquals("a not like 'x%'", Exp.notLike("a", "'x%'").toString()); + assertEquals("a not like 'x%'", Exp.notLike("a", "x%").toString()); } @Test public void likeIgnoreCase() { - assertEquals("a likeIgnoreCase 'x%'", Exp.likeIgnoreCase("a", "'x%'").toString()); + assertEquals("a likeIgnoreCase 'x%'", Exp.likeIgnoreCase("a", "x%").toString()); } @Test public void notLikeIgnoreCase() { - assertEquals("a not likeIgnoreCase 'x%'", Exp.notLikeIgnoreCase("a", "'x%'").toString()); + assertEquals("a not likeIgnoreCase 'x%'", Exp.notLikeIgnoreCase("a", "x%").toString()); } @Test public void in() { - assertEquals("a in ('a', 4, 'b')", Exp.in("a", "'a'", 4, "'b'").toString()); + assertEquals("a in ('a', 4, 'b')", Exp.in("a", "a", 4, "b").toString()); } @Test public void inCollection() { - assertEquals("a in ('a', 4, 'b')", Exp.inCollection("a", List.of("'a'", 4, "'b'")).toString()); + assertEquals("a in ('a', 4, 'b')", Exp.inCollection("a", List.of("a", 4, "b")).toString()); } @Test public void notIn() { - assertEquals("a not in ('a', 4, 'b')", Exp.notIn("a", "'a'", 4, "'b'").toString()); + assertEquals("a not in ('a', 4, 'b')", Exp.notIn("a", "a", 4, "b").toString()); } @Test public void notInCollection() { - assertEquals("a not in ('a', 4, 'b')", Exp.notInCollection("a", List.of("'a'", 4, "'b'")).toString()); + assertEquals("a not in ('a', 4, 'b')", Exp.notInCollection("a", List.of("a", 4, "b")).toString()); } @Test diff --git a/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java b/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java index beaa09845..fd12ee2fa 100644 --- a/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java +++ b/agrest-engine/src/test/java/io/agrest/runtime/protocol/ExpParserTest.java @@ -40,14 +40,14 @@ public void fromString_List() { @Test public void fromString_List_Params_String() { - Exp exp = parser.fromString("[\"b=$s\",\"'x'\"]"); + Exp exp = parser.fromString("[\"b=$s\",\"x\"]"); assertNotNull(exp); assertEquals("(b) = ('x')", exp.toString()); } @Test public void fromString_List_Params_Multiple() { - Exp exp = parser.fromString("[\"b=$s or b =$x or b =$s\",\"'x'\",\"'y'\"]"); + Exp exp = parser.fromString("[\"b=$s or b =$x or b =$s\",\"x\",\"y\"]"); assertNotNull(exp); assertEquals("((b) = ('x')) or ((b) = ('y')) or ((b) = ('x'))", exp.toString()); } @@ -61,7 +61,7 @@ public void fromString_Map() { @Test public void fromString_Map_Params_String() { - Exp exp = parser.fromString("{\"exp\" : \"b=$s\", \"params\":{\"s\":\"'x'\"}}"); + Exp exp = parser.fromString("{\"exp\" : \"b=$s\", \"params\":{\"s\":\"x\"}}"); assertNotNull(exp); assertEquals("(b) = ('x')", exp.toString()); } From dc71bf7a48cfb4fb7647b5452abc13ea555f7b53 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Thu, 1 Feb 2024 20:23:37 +0300 Subject: [PATCH 4/8] Store quoted string in scalarImage --- .../agrest/exp/parser/AgExpressionParser.java | 22 ++++++----- .../java/io/agrest/exp/parser/ExpScalar.java | 38 +++++++++++++++++-- .../src/main/java/io/agrest/protocol/Exp.java | 17 +++++---- .../agrest/exp/parser/AgExpressionParser.jjt | 12 +++++- .../exp/parser/ExpScalarStringTest.java | 9 ++--- 5 files changed, 69 insertions(+), 29 deletions(-) diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java index 819b89d7d..cdb73ca44 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java @@ -967,32 +967,34 @@ final public void stringLiteral() throws ParseException { case SINGLE_QUOTED_STRING:{ jj_consume_token(SINGLE_QUOTED_STRING); ExpScalar jjtn001 = new ExpScalar(JJTSCALAR); - boolean jjtc001 = true; - jjtree.openNodeScope(jjtn001); + boolean jjtc001 = true; + jjtree.openNodeScope(jjtn001); try { jjtree.closeNodeScope(jjtn001, 0); - jjtc001 = false; + jjtc001 = false; jjtn001.jjtSetValue(token_source.literalValue); + jjtn001.setScalarImage("'" + token_source.literalValue + "'"); } finally { if (jjtc001) { - jjtree.closeNodeScope(jjtn001, 0); - } + jjtree.closeNodeScope(jjtn001, 0); + } } break; } case DOUBLE_QUOTED_STRING:{ jj_consume_token(DOUBLE_QUOTED_STRING); ExpScalar jjtn002 = new ExpScalar(JJTSCALAR); - boolean jjtc002 = true; - jjtree.openNodeScope(jjtn002); + boolean jjtc002 = true; + jjtree.openNodeScope(jjtn002); try { jjtree.closeNodeScope(jjtn002, 0); - jjtc002 = false; + jjtc002 = false; jjtn002.jjtSetValue(token_source.literalValue); + jjtn002.setScalarImage("\"" + token_source.literalValue + "\""); } finally { if (jjtc002) { - jjtree.closeNodeScope(jjtn002, 0); - } + jjtree.closeNodeScope(jjtn002, 0); + } } break; } diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java index 2625888ba..da470d038 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java @@ -2,9 +2,14 @@ /* JavaCCOptions:MULTI=true,NODE_USES_PARSER=false,VISITOR=true,TRACK_TOKENS=false,NODE_PREFIX=Exp,NODE_EXTENDS=,NODE_FACTORY=,SUPPORT_CLASS_VISIBILITY_PUBLIC=true */ package io.agrest.exp.parser; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.function.Function; public class ExpScalar extends ExpBaseScalar { + + protected String scalarImage; + public ExpScalar(int id) { super(id); } @@ -20,6 +25,9 @@ public ExpScalar() { public ExpScalar(Object value) { super(AgExpressionParserTreeConstants.JJTSCALAR); setValue(value); + if (value instanceof CharSequence) { + setScalarImage("'" + value + "'"); + } } @Override @@ -31,10 +39,17 @@ protected Object transformExpression(Function transformer) { * Accept the visitor. **/ public T jjtAccept(AgExpressionParserVisitor visitor, T data) { - return visitor.visit(this, data); } + public String getScalarImage() { + return scalarImage; + } + + public void setScalarImage(String scalarImage) { + this.scalarImage = scalarImage; + } + @Override protected ExpScalar shallowCopy() { return new ExpScalar(getValue()); @@ -42,11 +57,26 @@ protected ExpScalar shallowCopy() { @Override public String toString() { + if (value == null) { + return "null"; + } if (value instanceof CharSequence) { - return "'" + value + "'"; - } else { - return String.valueOf(value); + String quoteChar = scalarImage.substring(0, 1); + String escapedContent = scalarImage + .substring(1, scalarImage.length() - 1) + .replaceAll(quoteChar, "\\\\" + quoteChar); + return quoteChar + escapedContent + quoteChar; + } + if (value.getClass() == Long.class) { + return value + "L"; + } + if (value.getClass() == BigInteger.class) { + return value + "H"; + } + if (value.getClass() == BigDecimal.class) { + return value + "B"; } + return String.valueOf(value); } } /* JavaCC - OriginalChecksum=21004db13a44c6b16cc9797a6f36a4af (do not edit this line) */ diff --git a/agrest-engine/src/main/java/io/agrest/protocol/Exp.java b/agrest-engine/src/main/java/io/agrest/protocol/Exp.java index 944908ff2..eada017a6 100644 --- a/agrest-engine/src/main/java/io/agrest/protocol/Exp.java +++ b/agrest-engine/src/main/java/io/agrest/protocol/Exp.java @@ -64,26 +64,27 @@ static Exp path(String path) { */ static Exp scalar(Object value) { if (value == null) { - return new ExpScalar(AgExpressionParserTreeConstants.JJTSCALAR); + return new ExpScalar(); } ExpBaseScalar scalar; if (value instanceof Collection) { - scalar = new ExpScalarList(AgExpressionParserTreeConstants.JJTSCALARLIST); - } else if (value.getClass().isArray()) { + scalar = new ExpScalarList((Collection) value); + return scalar; + } + if (value.getClass().isArray()) { Class componentType = value.getClass().getComponentType(); if (componentType.isPrimitive()) { value = ExpUtils.wrapPrimitiveArray(value); } else { value = Arrays.asList((Object[]) value); } - scalar = new ExpScalarList(AgExpressionParserTreeConstants.JJTSCALARLIST); - } else { - scalar = new ExpScalar(AgExpressionParserTreeConstants.JJTSCALAR); + scalar = new ExpScalarList(); + scalar.jjtSetValue(value); + return scalar; } - scalar.jjtSetValue(value); - return scalar; + return new ExpScalar(value); } /** diff --git a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt index 0a6148530..a961da10e 100644 --- a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt +++ b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt @@ -174,9 +174,17 @@ void stringParameter() : {} void stringLiteral() : {} { - { jjtThis.jjtSetValue(token_source.literalValue); } #Scalar(0) + + { + jjtThis.jjtSetValue(token_source.literalValue); + jjtThis.setScalarImage("'" + token_source.literalValue + "'"); + } #Scalar(0) | - { jjtThis.jjtSetValue(token_source.literalValue); } #Scalar(0) + + { + jjtThis.jjtSetValue(token_source.literalValue); + jjtThis.setScalarImage("\"" + token_source.literalValue + "\""); + } #Scalar(0) } void stringExpression() : {} diff --git a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java index a2e8b6283..5e0431be2 100644 --- a/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java +++ b/agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java @@ -28,13 +28,12 @@ void parse(String expString) { @CsvSource(delimiter = '|', quoteCharacter = 'X', value = { "'example'|'example'", " 'example' |'example'", - "\"example\"|'example'", + "\"example\"|\"example\"", "''|''", "' '|' '", - "\"\\\"example\\\"\"|'\"example\"'", - // TODO: this is wrong, single quote must be escaped or double quotes used in output - "\"a'b\"|'a'b'", - "'a\"b'|'a\"b'" + "\"\\\"example\\\"\"|\"\\\"example\\\"\"", + "\"a\\\"'b\"|\"a\\\"'b\"", + "'a\"\\'b'|'a\"\\'b'" }) public void parsedToString(String expString, String expected) { assertEquals(expected, Exp.parse(expString).toString()); From eed112dd16f155a612f6794b4eb9219bc1e46d24 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Sat, 3 Feb 2024 13:24:22 +0300 Subject: [PATCH 5/8] Refactor 'scalarImage' initialization --- .../agrest/exp/parser/AgExpressionParser.java | 4 +- .../java/io/agrest/exp/parser/ExpScalar.java | 85 +++++++++++++------ .../agrest/exp/parser/AgExpressionParser.jjt | 4 +- 3 files changed, 62 insertions(+), 31 deletions(-) diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java index cdb73ca44..b0d08ae7f 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java @@ -973,7 +973,7 @@ final public void stringLiteral() throws ParseException { jjtree.closeNodeScope(jjtn001, 0); jjtc001 = false; jjtn001.jjtSetValue(token_source.literalValue); - jjtn001.setScalarImage("'" + token_source.literalValue + "'"); + jjtn001.syncScalarImage("'" + token_source.literalValue + "'"); } finally { if (jjtc001) { jjtree.closeNodeScope(jjtn001, 0); @@ -990,7 +990,7 @@ final public void stringLiteral() throws ParseException { jjtree.closeNodeScope(jjtn002, 0); jjtc002 = false; jjtn002.jjtSetValue(token_source.literalValue); - jjtn002.setScalarImage("\"" + token_source.literalValue + "\""); + jjtn002.syncScalarImage("\"" + token_source.literalValue + "\""); } finally { if (jjtc002) { jjtree.closeNodeScope(jjtn002, 0); diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java index da470d038..8dab7647a 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java @@ -8,14 +8,16 @@ public class ExpScalar extends ExpBaseScalar { - protected String scalarImage; + private String scalarImage; public ExpScalar(int id) { super(id); + syncScalarImage(value); } public ExpScalar(AgExpressionParser p, int id) { super(p, id); + syncScalarImage(value); } public ExpScalar() { @@ -25,9 +27,6 @@ public ExpScalar() { public ExpScalar(Object value) { super(AgExpressionParserTreeConstants.JJTSCALAR); setValue(value); - if (value instanceof CharSequence) { - setScalarImage("'" + value + "'"); - } } @Override @@ -35,48 +34,80 @@ protected Object transformExpression(Function transformer) { return transformer.apply(new ExpScalar(getValue())); } - /** - * Accept the visitor. - **/ - public T jjtAccept(AgExpressionParserVisitor visitor, T data) { - return visitor.visit(this, data); + protected void syncScalarImage(CharSequence value) { + if (value.length() < 2) { + scalarImage = "'" + value + "'"; + return; + } + String stringValue = value.toString(); + char firstChar = stringValue.charAt(0); + if (firstChar != '\'' && firstChar != '"') { + scalarImage = "'" + value + "'"; + return; + } + String escapedContent = stringValue + .substring(1, stringValue.length() - 1) + .replaceAll(String.valueOf(firstChar), "\\\\" + firstChar); + scalarImage = firstChar + escapedContent + firstChar; } - public String getScalarImage() { - return scalarImage; + protected void syncScalarImage(Long value) { + scalarImage = value + "L"; } - public void setScalarImage(String scalarImage) { - this.scalarImage = scalarImage; + protected void syncScalarImage(BigInteger value) { + scalarImage = value + "H"; } - @Override - protected ExpScalar shallowCopy() { - return new ExpScalar(getValue()); + protected void syncScalarImage(BigDecimal value) { + scalarImage = value + "B"; + } + + protected void syncScalarImage(Object value) { + scalarImage = String.valueOf(value); + } + + /** + * Accept the visitor. + **/ + public T jjtAccept(AgExpressionParserVisitor visitor, T data) { + return visitor.visit(this, data); } @Override - public String toString() { + public void jjtSetValue(Object value) { + super.jjtSetValue(value); if (value == null) { - return "null"; + scalarImage = "null"; + return; } if (value instanceof CharSequence) { - String quoteChar = scalarImage.substring(0, 1); - String escapedContent = scalarImage - .substring(1, scalarImage.length() - 1) - .replaceAll(quoteChar, "\\\\" + quoteChar); - return quoteChar + escapedContent + quoteChar; + syncScalarImage(((CharSequence) value)); + return; } if (value.getClass() == Long.class) { - return value + "L"; + syncScalarImage(((Long) value)); + return; } if (value.getClass() == BigInteger.class) { - return value + "H"; + syncScalarImage(((BigInteger) value)); + return; } if (value.getClass() == BigDecimal.class) { - return value + "B"; + syncScalarImage(((BigDecimal) value)); + return; } - return String.valueOf(value); + syncScalarImage(value); + } + + @Override + protected ExpScalar shallowCopy() { + return new ExpScalar(getValue()); + } + + @Override + public String toString() { + return scalarImage; } } /* JavaCC - OriginalChecksum=21004db13a44c6b16cc9797a6f36a4af (do not edit this line) */ diff --git a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt index a961da10e..c4a304f90 100644 --- a/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt +++ b/agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt @@ -177,13 +177,13 @@ void stringLiteral() : {} { jjtThis.jjtSetValue(token_source.literalValue); - jjtThis.setScalarImage("'" + token_source.literalValue + "'"); + jjtThis.syncScalarImage("'" + token_source.literalValue + "'"); } #Scalar(0) | { jjtThis.jjtSetValue(token_source.literalValue); - jjtThis.setScalarImage("\"" + token_source.literalValue + "\""); + jjtThis.syncScalarImage("\"" + token_source.literalValue + "\""); } #Scalar(0) } From fea382ca940f566452f36ef0e49b36bb3949ed37 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Tue, 6 Feb 2024 17:09:43 +0300 Subject: [PATCH 6/8] Apply suggestions from code review --- .../src/main/java/io/agrest/exp/parser/ExpScalar.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java index 8dab7647a..41cde3d15 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java @@ -8,16 +8,14 @@ public class ExpScalar extends ExpBaseScalar { - private String scalarImage; + private String scalarImage = String.valueOf(value); public ExpScalar(int id) { super(id); - syncScalarImage(value); } public ExpScalar(AgExpressionParser p, int id) { super(p, id); - syncScalarImage(value); } public ExpScalar() { From 84ee2a6721e71e5d81c9004ed8349d65bef1f639 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Thu, 8 Feb 2024 19:38:33 +0300 Subject: [PATCH 7/8] Fix exp-string conversion for scalar --- .../main/java/io/agrest/exp/parser/ExpStringConverter.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java index f637e93b8..d05cab83c 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java @@ -260,9 +260,7 @@ public static String convert(ExpPath exp) { } public static String convert(ExpScalar exp) { - return exp.value instanceof CharSequence - ? "'" + exp.value + "'" - : String.valueOf(exp.value); + return exp.scalarImage; } public static String convert(ExpScalarList exp) { From b63dcf08ba5a54debef124c301fab883eb11c062 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Thu, 8 Feb 2024 19:40:05 +0300 Subject: [PATCH 8/8] Clean up --- .../src/main/java/io/agrest/exp/parser/ExpStringConverter.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java index d05cab83c..a7d9ca64b 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/ExpStringConverter.java @@ -3,8 +3,6 @@ import io.agrest.protocol.Exp; import java.util.Arrays; -import java.util.HashSet; -import java.util.List; import java.util.Set; import java.util.stream.Collectors;