diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 23c3ba5de..4323abe57 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -15,6 +15,7 @@ * #678 Upgrading Swagger to 2.2.2 * #679 Upgrading Jackson to 2.15.4 * #680 Upgrading SLF4J to 2.0.7 +* #681 Exp: regression in processing (not)in with empty list ## Release 5.0.M19 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 da239f6fa..dde2858d7 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 @@ -77,6 +77,24 @@ public void parseIn() { Expression e3 = parser.parse(Exp.in("a", 5, 6, 7)); assertEquals(ExpressionFactory.exp("a in (5, 6, 7)"), e3); + + Expression e4 = parser.parse(Exp.in("a")); + assertEquals(ExpressionFactory.exp("false"), e4); + } + + @Test + public void parseNotIn() { + Expression e1 = parser.parse(Exp.notIn("a", 5, 6, 7)); + assertEquals(ExpressionFactory.exp("a not in (5, 6, 7)"), e1); + + Expression e2 = parser.parse(Exp.notIn("a", "x", "y", "z")); + assertEquals(ExpressionFactory.exp("a not in ('x','y','z')"), e2); + + Expression e3 = parser.parse(Exp.notIn("a", 5, 6, 7)); + assertEquals(ExpressionFactory.exp("a not in (5, 6, 7)"), e3); + + Expression e4 = parser.parse(Exp.notIn("a")); + assertEquals(ExpressionFactory.exp("true"), e4); } @Test diff --git a/agrest-engine/src/main/java/io/agrest/exp/parser/NamedParamTransformer.java b/agrest-engine/src/main/java/io/agrest/exp/parser/NamedParamTransformer.java index ed4f324a7..dec585fae 100644 --- a/agrest-engine/src/main/java/io/agrest/exp/parser/NamedParamTransformer.java +++ b/agrest-engine/src/main/java/io/agrest/exp/parser/NamedParamTransformer.java @@ -2,6 +2,7 @@ import java.util.Map; import java.util.function.Function; +import java.util.function.Supplier; class NamedParamTransformer implements Function { @@ -17,7 +18,8 @@ class NamedParamTransformer implements Function { public Object apply(Object object) { if (!(object instanceof ExpNamedParameter)) { - return object; + // after parameters are resolved, we may need to shake down the tree a bit + return optimize(object); } String name = ((ExpNamedParameter) object).getName(); @@ -31,4 +33,43 @@ public Object apply(Object object) { return value != null ? SimpleNode.wrapParameterValue(value) : new ExpScalar(null); } } + + private Object optimize(Object object) { + if(object instanceof SimpleNode) { + return ((SimpleNode) object).jjtAccept(new OptimizationVisitor(), null); + } + return object; + } + + static class OptimizationVisitor extends AgExpressionParserDefaultVisitor { + + @Override + public SimpleNode defaultVisit(SimpleNode node, SimpleNode data) { + // note, we do not go down to children, just process this node and that's it + return node; + } + + @Override + public SimpleNode visit(ExpIn node, SimpleNode data) { + return optimizeIn(node, ExpFalse::new); + } + + @Override + public SimpleNode visit(ExpNotIn node, SimpleNode data) { + return optimizeIn(node, ExpTrue::new); + } + + private static SimpleNode optimizeIn(SimpleNode node, Supplier supplier) { + if(node.jjtGetNumChildren() < 2) { + return node; + } + Node child = node.jjtGetChild(1); + if(child instanceof ExpScalarList) { + if(((ExpScalarList) child).getValue().isEmpty()) { + return supplier.get(); + } + } + return node; + } + } } 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 ecb0f44e5..b5635b996 100644 --- a/agrest-engine/src/main/java/io/agrest/protocol/Exp.java +++ b/agrest-engine/src/main/java/io/agrest/protocol/Exp.java @@ -101,14 +101,20 @@ static Exp notBetween(String left, Object right1, Object right2) { /** * @since 5.0 */ - static Exp in(String path, Object... values) { - return ExpUtils.composeBinary(new ExpIn(), path(path), ExpUtils.scalarArray(values)); + static Exp in(String path, Object... scalars) { + if(scalars.length == 0){ + return new ExpFalse(); + } + return ExpUtils.composeBinary(new ExpIn(), path(path), ExpUtils.scalarArray(scalars)); } /** * @since 5.0 */ static Exp notIn(String path, Object... scalars) { + if(scalars.length == 0){ + return new ExpTrue(); + } return ExpUtils.composeBinary(new ExpNotIn(), path(path), ExpUtils.scalarArray(scalars)); } 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..761003ad3 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 @@ -8,6 +8,7 @@ import org.junit.jupiter.params.provider.ValueSource; import java.util.List; +import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -53,6 +54,18 @@ public void parseInvalidGrammar(String expString) { assertThrows(AgException.class, () -> Exp.parse(expString)); } + @Test + public void emptyIn() { + Exp exp = Exp.parse("a in $a").namedParams(Map.of("a", List.of())); + assertEquals("false", exp.toString()); + } + + @Test + public void emptyNotIn() { + Exp exp = Exp.parse("a not in $a").namedParams(Map.of("a", List.of())); + assertEquals("true", exp.toString()); + } + @Test public void parameterizedToString() { assertEquals("a in ('x', 'y')", Exp.parse("a in $l").positionalParams(List.of("x", "y")).toString());