From c773c10316794c39b9a75f176abb63d4c302a640 Mon Sep 17 00:00:00 2001 From: Kalle Stenflo Date: Tue, 8 Dec 2015 18:57:33 +0100 Subject: [PATCH] Incorrect comparison #145. Equals == and not equals != now compares string and number. Introduced === and !== to do type safe comparison. --- .../internal/filter/EvaluatorFactory.java | 19 ++++++++++ .../internal/filter/RelationalOperator.java | 10 ++++++ .../jsonpath/internal/filter/ValueNode.java | 36 +++++++++++++++---- .../java/com/jayway/jsonpath/FilterTest.java | 15 ++++++++ 4 files changed, 73 insertions(+), 7 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/filter/EvaluatorFactory.java b/json-path/src/main/java/com/jayway/jsonpath/internal/filter/EvaluatorFactory.java index 817b8831..c103ab28 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/filter/EvaluatorFactory.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/filter/EvaluatorFactory.java @@ -13,7 +13,9 @@ public class EvaluatorFactory { static { evaluators.put(RelationalOperator.EXISTS, new ExistsEvaluator()); evaluators.put(RelationalOperator.NE, new NotEqualsEvaluator()); + evaluators.put(RelationalOperator.TSNE, new TypeSafeNotEqualsEvaluator()); evaluators.put(RelationalOperator.EQ, new EqualsEvaluator()); + evaluators.put(RelationalOperator.TSEQ, new TypeSafeEqualsEvaluator()); evaluators.put(RelationalOperator.LT, new LessThanEvaluator()); evaluators.put(RelationalOperator.LTE, new LessThanEqualsEvaluator()); evaluators.put(RelationalOperator.GT, new GreaterThanEvaluator()); @@ -50,6 +52,13 @@ public class EvaluatorFactory { } } + private static class TypeSafeNotEqualsEvaluator implements Evaluator { + @Override + public boolean evaluate(ValueNode left, ValueNode right, Predicate.PredicateContext ctx) { + return !evaluators.get(RelationalOperator.TSEQ).evaluate(left, right, ctx); + } + } + private static class EqualsEvaluator implements Evaluator { @Override public boolean evaluate(ValueNode left, ValueNode right, Predicate.PredicateContext ctx) { @@ -61,6 +70,16 @@ public class EvaluatorFactory { } } + private static class TypeSafeEqualsEvaluator implements Evaluator { + @Override + public boolean evaluate(ValueNode left, ValueNode right, Predicate.PredicateContext ctx) { + if(!left.getClass().equals(right.getClass())){ + return false; + } + return evaluators.get(RelationalOperator.EQ).evaluate(left, right, ctx); + } + } + private static class TypeEvaluator implements Evaluator { @Override public boolean evaluate(ValueNode left, ValueNode right, Predicate.PredicateContext ctx) { diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/filter/RelationalOperator.java b/json-path/src/main/java/com/jayway/jsonpath/internal/filter/RelationalOperator.java index ce2cb766..4575f60b 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/filter/RelationalOperator.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/filter/RelationalOperator.java @@ -7,7 +7,17 @@ public enum RelationalOperator { GTE(">="), LTE("<="), EQ("=="), + + /** + * Type safe equals + */ + TSEQ("==="), NE("!="), + + /** + * Type safe not equals + */ + TSNE("!=="), LT("<"), GT(">"), REGEX("=~"), diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/filter/ValueNode.java b/json-path/src/main/java/com/jayway/jsonpath/internal/filter/ValueNode.java index dc4e8ac0..7352fa04 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/filter/ValueNode.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/filter/ValueNode.java @@ -412,6 +412,17 @@ public abstract class ValueNode { string = escape ? Utils.unescape(charSequence.toString()) : charSequence.toString(); } + @Override + public NumberNode asNumberNode() { + BigDecimal number = null; + try { + number = new BigDecimal(string); + } catch (NumberFormatException nfe){ + return NumberNode.NAN; + } + return new NumberNode(number); + } + public String getString() { return string; } @@ -450,22 +461,33 @@ public abstract class ValueNode { @Override public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof StringNode)) return false; + if (!(o instanceof StringNode) && !(o instanceof NumberNode)) return false; - StringNode that = (StringNode) o; + StringNode that = ((ValueNode) o).asStringNode(); - return !(string != null ? !string.equals(that.string) : that.string != null); + return !(string != null ? !string.equals(that.getString()) : that.getString() != null); } } public static class NumberNode extends ValueNode { + + public static NumberNode NAN = new NumberNode((BigDecimal)null); + private final BigDecimal number; + private NumberNode(BigDecimal number) { + this.number = number; + } private NumberNode(CharSequence num) { number = new BigDecimal(num.toString()); } + @Override + public StringNode asStringNode() { + return new StringNode(number.toString(), false); + } + public BigDecimal getNumber() { return number; } @@ -491,14 +513,14 @@ public abstract class ValueNode { @Override public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof NumberNode)) return false; + if (!(o instanceof NumberNode) && !(o instanceof StringNode)) return false; - ValueNode that = (ValueNode) o; + NumberNode that = ((ValueNode)o).asNumberNode(); - if(!that.isNumberNode()){ + if(that == NumberNode.NAN){ return false; } else { - return number.compareTo(that.asNumberNode().number) == 0; + return number.compareTo(that.number) == 0; } } } diff --git a/json-path/src/test/java/com/jayway/jsonpath/FilterTest.java b/json-path/src/test/java/com/jayway/jsonpath/FilterTest.java index b826f958..58493a35 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/FilterTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/FilterTest.java @@ -42,6 +42,21 @@ public class FilterTest extends BaseTest { assertThat(filter(where("int-key").eq(666)).apply(createPredicateContext(json))).isEqualTo(false); } + @Test + public void int_eq_string_evals() { + assertThat(filter(where("int-key").eq("1")).apply(createPredicateContext(json))).isEqualTo(true); + assertThat(filter(where("int-key").eq("666")).apply(createPredicateContext(json))).isEqualTo(false); + + + assertThat(Filter.parse("[?(1 == '1')]").apply(createPredicateContext(json))).isEqualTo(true); + assertThat(Filter.parse("[?('1' == 1)]").apply(createPredicateContext(json))).isEqualTo(true); + + assertThat(Filter.parse("[?(1 === '1')]").apply(createPredicateContext(json))).isEqualTo(false); + assertThat(Filter.parse("[?('1' === 1)]").apply(createPredicateContext(json))).isEqualTo(false); + + assertThat(Filter.parse("[?(1 === 1)]").apply(createPredicateContext(json))).isEqualTo(true); + } + @Test public void long_eq_evals() { assertThat(filter(where("long-key").eq(3000000000L)).apply(createPredicateContext(json))).isEqualTo(true);