From 03bec8af412d8dc99723f3c4e28c1ef9adf052aa Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Mon, 12 Oct 2015 14:52:09 +0300 Subject: [PATCH 01/17] two more tests for Option.ALWAYS_RETURN_LIST --- json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java index 8d7a243f..42ad5d75 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java @@ -2,6 +2,7 @@ package com.jayway.jsonpath; import org.junit.Test; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -45,6 +46,9 @@ public class OptionsTest extends BaseTest { Configuration conf = Configuration.builder().options(ALWAYS_RETURN_LIST).build(); assertThat(using(conf).parse("{\"foo\" : \"bar\"}").read("$.foo")).isInstanceOf(List.class); + assertThat(using(conf).parse("{\"foo\": null}").read("$.foo")).isInstanceOf(List.class); + assertThat(using(conf).parse("{\"foo\": [1, 4, 8]}").read("$.foo")).asList() + .containsExactly(Arrays.asList(1, 4, 8)); } @Test From 9566d846e65e3bc632f41a2567a489bab814cd9c Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Mon, 12 Oct 2015 15:38:09 +0300 Subject: [PATCH 02/17] two more tests for Option.ALWAYS_RETURN_LIST in case of indefinite path --- .../src/test/java/com/jayway/jsonpath/OptionsTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java index 42ad5d75..17d1c0b0 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java @@ -46,9 +46,16 @@ public class OptionsTest extends BaseTest { Configuration conf = Configuration.builder().options(ALWAYS_RETURN_LIST).build(); assertThat(using(conf).parse("{\"foo\" : \"bar\"}").read("$.foo")).isInstanceOf(List.class); + assertThat(using(conf).parse("{\"foo\": null}").read("$.foo")).isInstanceOf(List.class); + List result = using(conf).parse("{\"bar\": {\"foo\": null}}").read("$..foo"); + assertThat(result).hasSize(1); + assertThat(result.get(0)).isNull(); + assertThat(using(conf).parse("{\"foo\": [1, 4, 8]}").read("$.foo")).asList() .containsExactly(Arrays.asList(1, 4, 8)); + assertThat(using(conf).parse("{\"bar\": {\"foo\": [1, 4, 8]}}").read("$..foo")).asList() + .containsExactly(Arrays.asList(1, 4, 8)); } @Test From 296d3578d685322379193ecca336a6c34aba8d9c Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Mon, 12 Oct 2015 18:31:47 +0300 Subject: [PATCH 03/17] tests of Option.ALWAYS_RETURN_LIST in case of indefinite path moved to separate test --- .../test/java/com/jayway/jsonpath/OptionsTest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java index 17d1c0b0..37b54898 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java @@ -48,12 +48,19 @@ public class OptionsTest extends BaseTest { assertThat(using(conf).parse("{\"foo\" : \"bar\"}").read("$.foo")).isInstanceOf(List.class); assertThat(using(conf).parse("{\"foo\": null}").read("$.foo")).isInstanceOf(List.class); + + assertThat(using(conf).parse("{\"foo\": [1, 4, 8]}").read("$.foo")).asList() + .containsExactly(Arrays.asList(1, 4, 8)); + } + + @Test + public void an_indefinite_path_can_be_returned_as_list() { + Configuration conf = Configuration.builder().options(ALWAYS_RETURN_LIST).build(); + List result = using(conf).parse("{\"bar\": {\"foo\": null}}").read("$..foo"); assertThat(result).hasSize(1); assertThat(result.get(0)).isNull(); - assertThat(using(conf).parse("{\"foo\": [1, 4, 8]}").read("$.foo")).asList() - .containsExactly(Arrays.asList(1, 4, 8)); assertThat(using(conf).parse("{\"bar\": {\"foo\": [1, 4, 8]}}").read("$..foo")).asList() .containsExactly(Arrays.asList(1, 4, 8)); } From 630f0c15b344535aaeff3871a7d76bffed2b30a9 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Mon, 12 Oct 2015 19:02:25 +0300 Subject: [PATCH 04/17] fix for PathToken.isUpstreamDefinite --- .../jsonpath/internal/token/PathToken.java | 6 +-- .../com/jayway/jsonpath/PathTokenTest.java | 53 +++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java index 54339fd6..f40c8e7b 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java @@ -30,7 +30,7 @@ public abstract class PathToken { private Boolean definite = null; private Boolean upstreamDefinite = null; - PathToken appendTailToken(PathToken next) { + public PathToken appendTailToken(PathToken next) { this.next = next; this.next.prev = this; return next; @@ -148,13 +148,13 @@ public abstract class PathToken { return prev == null; } - boolean isUpstreamDefinite(){ + public boolean isUpstreamDefinite(){ if(upstreamDefinite != null){ return upstreamDefinite.booleanValue(); } boolean isUpstreamDefinite = isTokenDefinite(); if (isUpstreamDefinite && !isRoot()) { - isUpstreamDefinite = prev.isPathDefinite(); + isUpstreamDefinite = prev.isUpstreamDefinite(); } upstreamDefinite = isUpstreamDefinite; return isUpstreamDefinite; diff --git a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java new file mode 100644 index 00000000..c933505a --- /dev/null +++ b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java @@ -0,0 +1,53 @@ +package com.jayway.jsonpath; + +import com.jayway.jsonpath.internal.token.PathToken; +import com.jayway.jsonpath.internal.token.PropertyPathToken; +import com.jayway.jsonpath.internal.token.ScanPathToken; +import com.jayway.jsonpath.internal.token.WildcardPathToken; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; + +import java.util.Arrays; + +public class PathTokenTest extends BaseTest { + + @Test + public void is_upstream_definite_in_simple_case() { + assertThat(makePathReturningTail(makePPT("foo")).isUpstreamDefinite()).isTrue(); + + assertThat(makePathReturningTail(makePPT("foo"), makePPT("bar")).isUpstreamDefinite()).isTrue(); + + assertThat(makePathReturningTail(makePPT("foo", "foo2"), makePPT("bar")).isUpstreamDefinite()).isFalse(); + + assertThat(makePathReturningTail(new WildcardPathToken(), makePPT("bar")).isUpstreamDefinite()).isFalse(); + + assertThat(makePathReturningTail(new ScanPathToken(), makePPT("bar")).isUpstreamDefinite()).isFalse(); + } + + @Test + public void is_upstream_definite_in_complex_case() { + assertThat(makePathReturningTail(makePPT("foo"), makePPT("bar"), makePPT("baz")).isUpstreamDefinite()).isTrue(); + + assertThat(makePathReturningTail(makePPT("foo"), new WildcardPathToken()).isUpstreamDefinite()).isFalse(); + + assertThat(makePathReturningTail(new WildcardPathToken(), makePPT("bar"), makePPT("baz")).isUpstreamDefinite()).isFalse(); + } + + + private PathToken makePPT(final String ... properties) { + return new PropertyPathToken(Arrays.asList(properties)); + } + + private PathToken makePathReturningTail(final PathToken ... tokens) { + PathToken last = null; + for (final PathToken token : tokens) { + if (last != null) { + last.appendTailToken(token); + } + last = token; + } + return last; + } +} From f2d2a25744442e6eaac8af0e5182f0e2d85d8fbf Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Tue, 13 Oct 2015 16:09:07 +0300 Subject: [PATCH 05/17] multiprops not in leaf are disallowed at the moment --- json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java index c933505a..e88157ab 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java @@ -19,7 +19,7 @@ public class PathTokenTest extends BaseTest { assertThat(makePathReturningTail(makePPT("foo"), makePPT("bar")).isUpstreamDefinite()).isTrue(); - assertThat(makePathReturningTail(makePPT("foo", "foo2"), makePPT("bar")).isUpstreamDefinite()).isFalse(); + // assertThat(makePathReturningTail(makePPT("foo", "foo2"), makePPT("bar")).isUpstreamDefinite()).isFalse(); assertThat(makePathReturningTail(new WildcardPathToken(), makePPT("bar")).isUpstreamDefinite()).isFalse(); From f013e86fb839193aecf0081d17855f43a57d63e4 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Tue, 13 Oct 2015 16:15:28 +0300 Subject: [PATCH 06/17] fix for object property handling in case of Option.SUPPRESS_EXCEPTIONS and indefinite upstream path --- .../jayway/jsonpath/internal/token/PathToken.java | 4 ++-- .../test/java/com/jayway/jsonpath/OptionsTest.java | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java index f40c8e7b..9ef8b7a6 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java @@ -57,8 +57,8 @@ public abstract class PathToken { } } else { if(!isUpstreamDefinite() && - !ctx.options().contains(Option.REQUIRE_PROPERTIES) && - !ctx.options().contains(Option.SUPPRESS_EXCEPTIONS)){ + !ctx.options().contains(Option.REQUIRE_PROPERTIES) || + ctx.options().contains(Option.SUPPRESS_EXCEPTIONS)){ return; } else { throw new PathNotFoundException("Missing property in path " + evalPath); diff --git a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java index 37b54898..4553c36e 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/OptionsTest.java @@ -137,4 +137,17 @@ public class OptionsTest extends BaseTest { } catch (PathNotFoundException pnf){} } + + @Test + public void issue_suppress_exceptions_does_not_break_indefinite_evaluation() { + Configuration conf = Configuration.builder().options(SUPPRESS_EXCEPTIONS).build(); + + assertThat(using(conf).parse("{\"foo2\": [5]}").read("$..foo2[0]")).asList().containsOnly(5); + assertThat(using(conf).parse("{\"foo\" : {\"foo2\": [5]}}").read("$..foo2[0]")).asList().containsOnly(5); + assertThat(using(conf).parse("[null, [{\"foo\" : {\"foo2\": [5]}}]]").read("$..foo2[0]")).asList().containsOnly(5); + + assertThat(using(conf).parse("[null, [{\"foo\" : {\"foo2\": [5]}}]]").read("$..foo.foo2[0]")).asList().containsOnly(5); + + assertThat(using(conf).parse("{\"aoo\" : {}, \"foo\" : {\"foo2\": [5]}, \"zoo\" : {}}").read("$[*].foo2[0]")).asList().containsOnly(5); + } } From 1d259d18576a41cc2c7e4a6abd730a3d95205aad Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Wed, 14 Oct 2015 14:47:58 +0300 Subject: [PATCH 07/17] semantic of PathToken.isUpstreamDefinite changed: current token is not an upstream of itself --- .../jsonpath/internal/token/PathToken.java | 17 ++++++++--------- .../java/com/jayway/jsonpath/PathTokenTest.java | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java index 9ef8b7a6..2ff23e3c 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java @@ -43,6 +43,10 @@ public abstract class PathToken { String evalPath = Utils.concat(currentPath, "['", property, "']"); Object propertyVal = readObjectProperty(property, model, ctx); if(propertyVal == JsonProvider.UNDEFINED){ + // Conditions below heavily depend on current token type (and its logic) and are not "universal", + // so this code is quite dangerous. Better safe than sorry. + assert this instanceof PropertyPathToken : "only PropertyPathToken is supported"; + if(isLeaf()) { if(ctx.options().contains(Option.DEFAULT_PATH_LEAF_TO_NULL)){ propertyVal = null; @@ -148,16 +152,11 @@ public abstract class PathToken { return prev == null; } - public boolean isUpstreamDefinite(){ - if(upstreamDefinite != null){ - return upstreamDefinite.booleanValue(); - } - boolean isUpstreamDefinite = isTokenDefinite(); - if (isUpstreamDefinite && !isRoot()) { - isUpstreamDefinite = prev.isUpstreamDefinite(); + public boolean isUpstreamDefinite() { + if (upstreamDefinite == null) { + upstreamDefinite = isRoot() || prev.isTokenDefinite() && prev.isUpstreamDefinite(); } - upstreamDefinite = isUpstreamDefinite; - return isUpstreamDefinite; + return upstreamDefinite; } public int getTokenCount() { diff --git a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java index e88157ab..0548597f 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java @@ -30,7 +30,7 @@ public class PathTokenTest extends BaseTest { public void is_upstream_definite_in_complex_case() { assertThat(makePathReturningTail(makePPT("foo"), makePPT("bar"), makePPT("baz")).isUpstreamDefinite()).isTrue(); - assertThat(makePathReturningTail(makePPT("foo"), new WildcardPathToken()).isUpstreamDefinite()).isFalse(); + assertThat(makePathReturningTail(makePPT("foo"), new WildcardPathToken()).isUpstreamDefinite()).isTrue(); assertThat(makePathReturningTail(new WildcardPathToken(), makePPT("bar"), makePPT("baz")).isUpstreamDefinite()).isFalse(); } From f03875784ead636624271815d820b81cb005ad13 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Wed, 14 Oct 2015 18:33:59 +0300 Subject: [PATCH 08/17] deep scan fixes and tests --- .../internal/token/ArrayPathToken.java | 16 +++- .../internal/token/PredicatePathToken.java | 4 +- .../internal/token/PropertyPathToken.java | 6 +- .../internal/token/ScanPathToken.java | 6 ++ .../com/jayway/jsonpath/DeepScanTest.java | 88 +++++++++++++++++++ .../java/com/jayway/jsonpath/TestUtils.java | 32 +++++++ 6 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java create mode 100644 json-path/src/test/java/com/jayway/jsonpath/TestUtils.java diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ArrayPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ArrayPathToken.java index 1928d91d..5c0675f6 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ArrayPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ArrayPathToken.java @@ -54,10 +54,18 @@ public class ArrayPathToken extends PathToken { @Override public void evaluate(String currentPath, PathRef parent, Object model, EvaluationContextImpl ctx) { if(model == null){ - throw new PathNotFoundException("The path " + currentPath + " is null"); + if (! isUpstreamDefinite()) { + return; + } else { + throw new PathNotFoundException("The path " + currentPath + " is null"); + } } if (!ctx.jsonProvider().isArray(model)) { - throw new InvalidPathException(format("Filter: %s can only be applied to arrays. Current context is: %s", toString(), model)); + if (! isUpstreamDefinite()) { + return; + } else { + throw new InvalidPathException(format("Filter: %s can only be applied to arrays. Current context is: %s", toString(), model)); + } } try { @@ -143,7 +151,9 @@ public class ArrayPathToken extends PathToken { break; } } catch (IndexOutOfBoundsException e) { - throw new PathNotFoundException("Index out of bounds when evaluating path " + currentPath); + if (isUpstreamDefinite()) { + throw new PathNotFoundException("Index out of bounds when evaluating path " + currentPath); + } } } diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PredicatePathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PredicatePathToken.java index bd6c5100..bfd5af11 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PredicatePathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PredicatePathToken.java @@ -69,7 +69,9 @@ public class PredicatePathToken extends PathToken { idx++; } } else { - throw new InvalidPathException(format("Filter: %s can not be applied to primitives. Current context is: %s", toString(), model)); + if (isUpstreamDefinite()) { + throw new InvalidPathException(format("Filter: %s can not be applied to primitives. Current context is: %s", toString(), model)); + } } } diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java index f7824036..86b16b78 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java @@ -38,7 +38,11 @@ public class PropertyPathToken extends PathToken { @Override public void evaluate(String currentPath, PathRef parent, Object model, EvaluationContextImpl ctx) { if (!ctx.jsonProvider().isMap(model)) { - throw new PathNotFoundException("Property " + getPathFragment() + " not found in path " + currentPath); + if (! isUpstreamDefinite()) { + return; + } else { + throw new PathNotFoundException("Property " + getPathFragment() + " not found in path " + currentPath); + } } handleObjectProperty(currentPath, model, ctx, properties); diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java index 5bb0d0e5..72a7f8fd 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java @@ -14,6 +14,7 @@ */ package com.jayway.jsonpath.internal.token; +import com.jayway.jsonpath.Option; import com.jayway.jsonpath.internal.PathRef; import com.jayway.jsonpath.spi.json.JsonProvider; @@ -167,6 +168,11 @@ public class ScanPathToken extends PathToken { @Override public boolean matches(Object model) { if (ctx.jsonProvider().isMap(model)) { + if (ctx.options().contains(Option.REQUIRE_PROPERTIES)) { + // Have to require properties defined in path when an indefinite path is evaluated, + // so have to go there and search for it. + return true; + } Collection keys = ctx.jsonProvider().getPropertyKeys(model); return keys.containsAll(propertyPathToken.getProperties()); } diff --git a/json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java b/json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java new file mode 100644 index 00000000..e85566d2 --- /dev/null +++ b/json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java @@ -0,0 +1,88 @@ +package com.jayway.jsonpath; + +import org.junit.Test; + +import static com.jayway.jsonpath.TestUtils.assertEvaluationThrows; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Deep scan is indefinite, so certain "illegal" actions become a no-op instead of a path evaluation exception. + */ +public class DeepScanTest extends BaseTest { + + @Test + public void when_deep_scanning_non_array_subscription_is_ignored() { + Object result = JsonPath.parse("{\"x\": [0,1,[0,1,2,3,null],null]}").read("$..[2][3]"); + assertThat(result).asList().containsOnly(3); + result = JsonPath.parse("{\"x\": [0,1,[0,1,2,3,null],null], \"y\": [0,1,2]}").read("$..[2][3]"); + assertThat(result).asList().containsOnly(3); + + result = JsonPath.parse("{\"x\": [0,1,[0,1,2],null], \"y\": [0,1,2]}").read("$..[2][3]"); + assertThat(result).asList().isEmpty(); + } + + @Test + public void when_deep_scanning_null_subscription_is_ignored() { + Object result = JsonPath.parse("{\"x\": [null,null,[0,1,2,3,null],null]}").read("$..[2][3]"); + assertThat(result).asList().containsOnly(3); + result = JsonPath.parse("{\"x\": [null,null,[0,1,2,3,null],null], \"y\": [0,1,null]}").read("$..[2][3]"); + assertThat(result).asList().containsOnly(3); + } + + @Test + public void when_deep_scanning_array_index_oob_is_ignored() { + Object result = JsonPath.parse("{\"x\": [0,1,[0,1,2,3,10],null]}").read("$..[4]"); + assertThat(result).asList().containsOnly(10); + + result = JsonPath.parse("{\"x\": [null,null,[0,1,2,3]], \"y\": [null,null,[0,1]]}").read("$..[2][3]"); + assertThat(result).asList().containsOnly(3); + } + + @Test + public void definite_upstream_illegal_array_access_throws() { + assertEvaluationThrows("{\"foo\": {\"bar\": null}}", "$.foo.bar.[5]", PathNotFoundException.class); + assertEvaluationThrows("{\"foo\": {\"bar\": null}}", "$.foo.bar.[5, 10]", PathNotFoundException.class); + + assertEvaluationThrows("{\"foo\": {\"bar\": 4}}", "$.foo.bar.[5]", InvalidPathException.class); + assertEvaluationThrows("{\"foo\": {\"bar\": 4}}", "$.foo.bar.[5, 10]", InvalidPathException.class); + + assertEvaluationThrows("{\"foo\": {\"bar\": []}}", "$.foo.bar.[5]", PathNotFoundException.class); + } + + @Test + public void when_deep_scanning_illegal_property_access_is_ignored() { + Object result = JsonPath.parse("{\"x\": {\"foo\": {\"bar\": 4}}, \"y\": {\"foo\": 1}}").read("$..foo"); + assertThat(result).asList().hasSize(2); + + result = JsonPath.parse("{\"x\": {\"foo\": {\"bar\": 4}}, \"y\": {\"foo\": 1}}").read("$..foo.bar"); + assertThat(result).asList().containsOnly(4); + result = JsonPath.parse("{\"x\": {\"foo\": {\"bar\": 4}}, \"y\": {\"foo\": 1}}").read("$..[*].foo.bar"); + assertThat(result).asList().containsOnly(4); + result = JsonPath.parse("{\"x\": {\"foo\": {\"baz\": 4}}, \"y\": {\"foo\": 1}}").read("$..[*].foo.bar"); + assertThat(result).asList().isEmpty(); + } + + @Test + public void when_deep_scanning_illegal_predicate_is_ignored() { + Object result = JsonPath.parse("{\"x\": {\"foo\": {\"bar\": 4}}, \"y\": {\"foo\": 1}}").read( + "$..foo[?(@.bar)].bar"); + assertThat(result).asList().containsOnly(4); + + result = JsonPath.parse("{\"x\": {\"foo\": {\"bar\": 4}}, \"y\": {\"foo\": 1}}").read( + "$..[*]foo[?(@.bar)].bar"); + assertThat(result).asList().containsOnly(4); + } + + @Test + public void when_deep_scanning_require_properties_still_counts() { + final Configuration conf = Configuration.defaultConfiguration().addOptions(Option.REQUIRE_PROPERTIES); + + Object result = JsonPath.parse("[{\"x\": {\"foo\": {\"x\": 4}, \"x\": null}, \"y\": {\"x\": 1}}, {\"x\": []}]").read( + "$..x"); + assertThat(result).asList().hasSize(5); + + // foo.bar must be found in every object node after deep scan (which is impossible) + assertEvaluationThrows("{\"foo\": {\"bar\": 4}}", "$..foo.bar", PathNotFoundException.class, conf); + } +} diff --git a/json-path/src/test/java/com/jayway/jsonpath/TestUtils.java b/json-path/src/test/java/com/jayway/jsonpath/TestUtils.java new file mode 100644 index 00000000..1835d65b --- /dev/null +++ b/json-path/src/test/java/com/jayway/jsonpath/TestUtils.java @@ -0,0 +1,32 @@ +package com.jayway.jsonpath; + +import static com.jayway.jsonpath.JsonPath.using; +import static org.assertj.core.api.Assertions.fail; + +public final class TestUtils { + private TestUtils() {} + + public static void assertEvaluationThrows(final String json, final String path, + Class expected) { + assertEvaluationThrows(json, path, expected, Configuration.defaultConfiguration()); + } + + /** + * Shortcut for expected exception testing during path evaluation. + * + * @param conf conf to use during evaluation + * @param json json to parse + * @param path jsonpath do evaluate + * @param expected expected exception class (reference comparison, not an instanceof) + */ + public static void assertEvaluationThrows(final String json, final String path, + Class expected, final Configuration conf) { + try { + using(conf).parse(json).read(path); + fail("Should throw " + expected.getName()); + } catch (JsonPathException exc) { + if (exc.getClass() != expected) + throw exc; + } + } +} From e8af47fa98153391ccde1561eb267b7e73f25597 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Thu, 15 Oct 2015 13:51:37 +0300 Subject: [PATCH 09/17] multiple properties path token at the end of the path: tests and fix --- .../internal/token/ScanPathToken.java | 31 +++++++++++++------ .../com/jayway/jsonpath/DeepScanTest.java | 30 ++++++++++++++++++ .../com/jayway/jsonpath/MultiPropTest.java | 4 +++ 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java index 72a7f8fd..0b4a50c5 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java @@ -167,16 +167,29 @@ public class ScanPathToken extends PathToken { @Override public boolean matches(Object model) { - if (ctx.jsonProvider().isMap(model)) { - if (ctx.options().contains(Option.REQUIRE_PROPERTIES)) { - // Have to require properties defined in path when an indefinite path is evaluated, - // so have to go there and search for it. - return true; - } - Collection keys = ctx.jsonProvider().getPropertyKeys(model); - return keys.containsAll(propertyPathToken.getProperties()); + if (! ctx.jsonProvider().isMap(model)) { + return false; } - return false; + + if (ctx.options().contains(Option.REQUIRE_PROPERTIES)) { + // Have to require properties defined in path when an indefinite path is evaluated, + // so have to go there and search for it. + return true; + } + + if (! propertyPathToken.isTokenDefinite()) { + // It's responsibility of PropertyPathToken code to handle indefinite scenario of properties, + // so we'll allow it to do its job. + return true; + } + + if (ctx.options().contains(Option.DEFAULT_PATH_LEAF_TO_NULL)) { + // In case of DEFAULT_PATH_LEAF_TO_NULL missing properties is not a problem. + return true; + } + + Collection keys = ctx.jsonProvider().getPropertyKeys(model); + return keys.containsAll(propertyPathToken.getProperties()); } } } diff --git a/json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java b/json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java index e85566d2..6724809e 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/DeepScanTest.java @@ -2,10 +2,14 @@ package com.jayway.jsonpath; import org.junit.Test; +import static com.jayway.jsonpath.JsonPath.using; import static com.jayway.jsonpath.TestUtils.assertEvaluationThrows; import static org.assertj.core.api.Assertions.assertThat; +import java.util.List; +import java.util.Map; + /** * Deep scan is indefinite, so certain "illegal" actions become a no-op instead of a path evaluation exception. */ @@ -84,5 +88,31 @@ public class DeepScanTest extends BaseTest { // foo.bar must be found in every object node after deep scan (which is impossible) assertEvaluationThrows("{\"foo\": {\"bar\": 4}}", "$..foo.bar", PathNotFoundException.class, conf); + + assertEvaluationThrows("{\"foo\": {\"bar\": 4}, \"baz\": 2}", "$..['foo', 'baz']", PathNotFoundException.class, conf); + } + + @Test + public void when_deep_scanning_leaf_multi_props_work() { + Object result = JsonPath.parse("[{\"a\": \"a-val\", \"b\": \"b-val\", \"c\": \"c-val\"}, [1, 5], {\"a\": \"a-val\"}]").read( + "$..['a', 'c']"); + // This is current deep scan semantics: only objects containing all properties specified in multiprops token are + // considered. + assertThat(result).asList().hasSize(1); + result = ((List)result).get(0); + + assertThat(result).isInstanceOf(Map.class); + assertThat((Map)result).hasSize(2).containsEntry("a", "a-val").containsEntry("c", "c-val"); + + // But this semantics changes when DEFAULT_PATH_LEAF_TO_NULL comes into play. + Configuration conf = Configuration.defaultConfiguration().addOptions(Option.DEFAULT_PATH_LEAF_TO_NULL); + result = using(conf).parse("[{\"a\": \"a-val\", \"b\": \"b-val\", \"c\": \"c-val\"}, [1, 5], {\"a\": \"a-val\"}]").read( + "$..['a', 'c']"); + // todo: deep equality test, but not tied to any json provider + assertThat(result).asList().hasSize(2); + for (final Object node : (List)result) { + assertThat(node).isInstanceOf(Map.class); + assertThat((Map)node).hasSize(2).containsEntry("a", "a-val"); + } } } diff --git a/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java b/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java index c850eed3..aeb0bffb 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java @@ -24,6 +24,10 @@ public class MultiPropTest { assertThat(using(conf).parse(model).read("$['a', 'b']", Map.class)) .containsEntry("a", "a-val") .containsEntry("b", "b-val"); + + // current semantics: absent props are skipped + assertThat(using(conf).parse(model).read("$['a', 'd']", Map.class)) + .hasSize(1).containsEntry("a", "a-val"); } @Test From 99637ca2d7283cf62f447b2dfef8a741c026f1ba Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Thu, 15 Oct 2015 19:14:35 +0300 Subject: [PATCH 10/17] added support of multiple object properties in non-leaf path node Semantics of this case is the same as semantics of ArrayPathToken with multiple array indices specified. --- .../com/jayway/jsonpath/internal/Utils.java | 23 ++++++++ .../jsonpath/internal/token/PathToken.java | 13 +++-- .../internal/token/PropertyPathToken.java | 39 ++++++++++++- .../com/jayway/jsonpath/MultiPropTest.java | 57 +++++++++++++++++++ .../com/jayway/jsonpath/PathTokenTest.java | 2 +- 5 files changed, 126 insertions(+), 8 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/Utils.java b/json-path/src/main/java/com/jayway/jsonpath/internal/Utils.java index dd46f40b..3305bff9 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/Utils.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/Utils.java @@ -272,6 +272,29 @@ public final class Utils { } } + /** + * Check if one and only one condition is true; otherwise + * throw an exception with the specified message. + * @param message error describing message + * @param expressions the boolean expressions to check + * @throws IllegalArgumentException if zero or more than one expressions are true + */ + public static void onlyOneIsTrue(final String message, final boolean ... expressions) { + if (! onlyOneIsTrueNonThrow(expressions)) { + throw new IllegalArgumentException(message); + } + } + + public static boolean onlyOneIsTrueNonThrow(final boolean ... expressions) { + int count = 0; + for (final boolean expression : expressions) { + if (expression && ++count > 1) { + return false; + } + } + return 1 == count; + } + /** *

Validate that the specified argument character sequence is * neither {@code null} nor a length of zero (no characters); diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java index 2ff23e3c..2076f95f 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java @@ -44,7 +44,9 @@ public abstract class PathToken { Object propertyVal = readObjectProperty(property, model, ctx); if(propertyVal == JsonProvider.UNDEFINED){ // Conditions below heavily depend on current token type (and its logic) and are not "universal", - // so this code is quite dangerous. Better safe than sorry. + // so this code is quite dangerous (I'd rather rewrite it & move to PropertyPathToken and implemented + // WildcardPathToken as a dynamic multi prop case of PropertyPathToken). + // Better safe than sorry. assert this instanceof PropertyPathToken : "only PropertyPathToken is supported"; if(isLeaf()) { @@ -60,9 +62,12 @@ public abstract class PathToken { } } else { - if(!isUpstreamDefinite() && + if (! (isUpstreamDefinite() && isTokenDefinite()) && !ctx.options().contains(Option.REQUIRE_PROPERTIES) || ctx.options().contains(Option.SUPPRESS_EXCEPTIONS)){ + // If there is some indefiniteness in the path and properties are not required - we'll ignore + // absent property. And also in case of exception suppression - so that other path evaluation + // branches could be examined. return; } else { throw new PathNotFoundException("Missing property in path " + evalPath); @@ -78,9 +83,7 @@ public abstract class PathToken { } else { String evalPath = currentPath + "[" + Utils.join(", ", "'", properties) + "]"; - if (!isLeaf()) { - throw new InvalidPathException("Multi properties can only be used as path leafs: " + evalPath); - } + assert isLeaf() : "non-leaf multi props handled elsewhere"; Object merged = ctx.jsonProvider().createMap(); for (String property : properties) { diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java index 86b16b78..1dc64388 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java @@ -14,12 +14,16 @@ */ package com.jayway.jsonpath.internal.token; +import com.jayway.jsonpath.InvalidPathException; import com.jayway.jsonpath.PathNotFoundException; import com.jayway.jsonpath.internal.PathRef; import com.jayway.jsonpath.internal.Utils; +import java.util.ArrayList; import java.util.List; +import static com.jayway.jsonpath.internal.Utils.onlyOneIsTrueNonThrow; + /** * */ @@ -28,6 +32,9 @@ public class PropertyPathToken extends PathToken { private final List properties; public PropertyPathToken(List properties) { + if (properties.isEmpty()) { + throw new InvalidPathException("Empty properties"); + } this.properties = properties; } @@ -35,8 +42,24 @@ public class PropertyPathToken extends PathToken { return properties; } + public boolean singlePropertyCase() { + return properties.size() == 1; + } + + public boolean multiPropertyMergeCase() { + return isLeaf() && properties.size() > 1; + } + + public boolean multiPropertyIterationCase() { + // Semantics of this case is the same as semantics of ArrayPathToken with INDEX_SEQUENCE operation. + return ! isLeaf() && properties.size() > 1; + } + @Override public void evaluate(String currentPath, PathRef parent, Object model, EvaluationContextImpl ctx) { + // Can't assert it in ctor because isLeaf() could be changed later on. + assert onlyOneIsTrueNonThrow(singlePropertyCase(), multiPropertyMergeCase(), multiPropertyIterationCase()); + if (!ctx.jsonProvider().isMap(model)) { if (! isUpstreamDefinite()) { return; @@ -45,12 +68,24 @@ public class PropertyPathToken extends PathToken { } } - handleObjectProperty(currentPath, model, ctx, properties); + if (singlePropertyCase() || multiPropertyMergeCase()) { + handleObjectProperty(currentPath, model, ctx, properties); + return; + } + + assert multiPropertyIterationCase(); + final List currentlyHandledProperty = new ArrayList(1); + currentlyHandledProperty.add(null); + for (final String property : properties) { + currentlyHandledProperty.set(0, property); + handleObjectProperty(currentPath, model, ctx, currentlyHandledProperty); + } } @Override boolean isTokenDefinite() { - return true; + // in case of leaf multiprops will be merged, so it's kinda definite + return singlePropertyCase() || multiPropertyMergeCase(); } @Override diff --git a/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java b/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java index aeb0bffb..2f7d45d7 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/MultiPropTest.java @@ -6,6 +6,8 @@ import java.util.HashMap; import java.util.Map; import static com.jayway.jsonpath.JsonPath.using; +import static com.jayway.jsonpath.TestUtils.assertEvaluationThrows; + import static org.assertj.core.api.Assertions.assertThat; public class MultiPropTest { @@ -60,5 +62,60 @@ public class MultiPropTest { using(conf).parse(model).read("$['a', 'x']", Map.class); } + @Test + public void multi_props_can_be_non_leafs() { + Object result = JsonPath.parse("{\"a\": {\"v\": 5}, \"b\": {\"v\": 4}, \"c\": {\"v\": 1}}").read( + "$['a', 'c'].v"); + assertThat(result).asList().containsOnly(5, 1); + } + + @Test + public void nonexistent_non_leaf_multi_props_ignored() { + Object result = JsonPath.parse("{\"a\": {\"v\": 5}, \"b\": {\"v\": 4}, \"c\": {\"v\": 1}}").read( + "$['d', 'a', 'c', 'm'].v"); + assertThat(result).asList().containsOnly(5, 1); + } + + @Test + public void multi_props_with_post_filter() { + Object result = JsonPath.parse("{\"a\": {\"v\": 5}, \"b\": {\"v\": 4}, \"c\": {\"v\": 1, \"flag\": true}}").read( + "$['a', 'c'][?(@.flag)].v"); + assertThat(result).asList().containsOnly(1); + } + + @Test + public void deep_scan_does_not_affect_non_leaf_multi_props() { + // deep scan + multiprop is quite redundant scenario, but it's not forbidden, so we'd better check + final String json = "{\"v\": [[{}, 1, {\"a\": {\"v\": 5}, \"b\": {\"v\": 4}, \"c\": {\"v\": 1, \"flag\": true}}]]}"; + Object result = JsonPath.parse(json).read("$..['a', 'c'].v"); + assertThat(result).asList().containsOnly(5, 1); + + result = JsonPath.parse(json).read("$..['a', 'c'][?(@.flag)].v"); + assertThat(result).asList().containsOnly(1); + } + @Test + public void multi_props_can_be_in_the_middle() { + final String json = "{\"x\": [null, {\"a\": {\"v\": 5}, \"b\": {\"v\": 4}, \"c\": {\"v\": 1}}]}"; + Object result = JsonPath.parse(json).read("$.x[1]['a', 'c'].v"); + assertThat(result).asList().containsOnly(5, 1); + result = JsonPath.parse(json).read("$.x[*]['a', 'c'].v"); + assertThat(result).asList().containsOnly(5, 1); + result = JsonPath.parse(json).read("$[*][*]['a', 'c'].v"); + assertThat(result).asList().containsOnly(5, 1); + + result = JsonPath.parse(json).read("$.x[1]['d', 'a', 'c', 'm'].v"); + assertThat(result).asList().containsOnly(5, 1); + result = JsonPath.parse(json).read("$.x[*]['d', 'a', 'c', 'm'].v"); + assertThat(result).asList().containsOnly(5, 1); + } + + @Test + public void non_leaf_multi_props_can_be_required() { + final Configuration conf = Configuration.defaultConfiguration().addOptions(Option.REQUIRE_PROPERTIES); + final String json = "{\"a\": {\"v\": 5}, \"b\": {\"v\": 4}, \"c\": {\"v\": 1}}"; + + assertThat(using(conf).parse(json).read("$['a', 'c'].v")).asList().containsOnly(5, 1); + assertEvaluationThrows(json, "$['d', 'a', 'c', 'm'].v", PathNotFoundException.class, conf); + } } diff --git a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java index 0548597f..b7612649 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java @@ -19,7 +19,7 @@ public class PathTokenTest extends BaseTest { assertThat(makePathReturningTail(makePPT("foo"), makePPT("bar")).isUpstreamDefinite()).isTrue(); - // assertThat(makePathReturningTail(makePPT("foo", "foo2"), makePPT("bar")).isUpstreamDefinite()).isFalse(); + assertThat(makePathReturningTail(makePPT("foo", "foo2"), makePPT("bar")).isUpstreamDefinite()).isFalse(); assertThat(makePathReturningTail(new WildcardPathToken(), makePPT("bar")).isUpstreamDefinite()).isFalse(); From 101ffa60902f395604a5ec12ef684052ec9cbb2a Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Thu, 15 Oct 2015 19:42:24 +0300 Subject: [PATCH 11/17] smallfix for deep scan predicate (though, kind of redundant, but logic is clearer) --- .../java/com/jayway/jsonpath/internal/token/ScanPathToken.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java index 0b4a50c5..16e56b1a 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java @@ -183,7 +183,7 @@ public class ScanPathToken extends PathToken { return true; } - if (ctx.options().contains(Option.DEFAULT_PATH_LEAF_TO_NULL)) { + if (propertyPathToken.isLeaf() && ctx.options().contains(Option.DEFAULT_PATH_LEAF_TO_NULL)) { // In case of DEFAULT_PATH_LEAF_TO_NULL missing properties is not a problem. return true; } From 5c0dc5672203643318172ab4045a481dc284b8f8 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Fri, 16 Oct 2015 14:51:53 +0300 Subject: [PATCH 12/17] number of issues discovered in predicate parsing And I dont see a quickfix. I suggest reimplementing entire path compilation using context-free grammar approach. --- .../com/jayway/jsonpath/PredicateTest.java | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java b/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java index 62939070..472cb641 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java @@ -1,5 +1,6 @@ package com.jayway.jsonpath; +import org.junit.Ignore; import org.junit.Test; import java.util.List; @@ -23,4 +24,107 @@ public class PredicateTest extends BaseTest { assertThat(reader.read("$.store.book[?].isbn", List.class, booksWithISBN)).containsOnly("0-395-19395-8", "0-553-21311-3"); } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_escaped_backslash_in_prop() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"it\\\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message == 'it\\\\')].message"); + + assertThat(result).containsExactly("it\\"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_bracket_in_regex() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"(it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message =~ /\\(it/)].message"); + + assertThat(result).containsExactly("(it"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_and_in_regex() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message =~ /&&|it/)].message"); + + assertThat(result).containsExactly("it"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_and_in_prop() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"&& it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message == '&& it')].message"); + + assertThat(result).containsExactly("&& it"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_brackets_must_change_priorities() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message && (@.id == 1 || @.id == 2))].id"); + assertThat(result).isEmpty(); + + result = JsonPath.read(json, "$.logs[?((@.id == 2 || @.id == 1) && @.message)].id"); + assertThat(result).isEmpty(); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_square_bracket_in_prop() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"] it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message == '] it')].message"); + + assertThat(result).containsExactly("] it"); + } } From 97db5795bd9c9037a7d6dea4d8cd0b502fdfc01f Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Fri, 16 Oct 2015 15:10:57 +0300 Subject: [PATCH 13/17] fix for escaped backslash in property inside predicate --- .../java/com/jayway/jsonpath/internal/PathCompiler.java | 9 ++++++++- .../src/test/java/com/jayway/jsonpath/PredicateTest.java | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/PathCompiler.java b/json-path/src/main/java/com/jayway/jsonpath/internal/PathCompiler.java index e6b73385..a92c4a94 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/PathCompiler.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/PathCompiler.java @@ -292,7 +292,7 @@ public class PathCompiler { start--; } - int mem = ' '; + int mem = -1; int curr = start; boolean inProp = false; int openSquareBracket = 0; @@ -317,6 +317,13 @@ public class PathCompiler { } } break; + case '\\': + if (mem == '\\') { // escaped backslash, skip it + mem = -1; + curr++; + continue; + } + break; case '\'': if(mem == '\\') { break; diff --git a/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java b/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java index 472cb641..fb619ec2 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java @@ -25,17 +25,17 @@ public class PredicateTest extends BaseTest { assertThat(reader.read("$.store.book[?].isbn", List.class, booksWithISBN)).containsOnly("0-395-19395-8", "0-553-21311-3"); } - @Ignore("not ready yet (requires compiler reimplementation)") @Test public void issue_predicate_can_have_escaped_backslash_in_prop() { String json = "{\n" + " \"logs\": [\n" + " {\n" - + " \"message\": \"it\\\",\n" + + " \"message\": \"it\\\\\",\n" + " \"id\": 2\n" + " }\n" + " ]\n" + "}"; + // message: it\ -> (after json escaping) -> "it\\" -> (after java escaping) -> "\"it\\\\\"" List result = JsonPath.read(json, "$.logs[?(@.message == 'it\\\\')].message"); From 22bd223f58a5ff3383c1b98c996be5a19672cac2 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Fri, 16 Oct 2015 15:36:08 +0300 Subject: [PATCH 14/17] java 6 support also, to run tests you have to downgrade org.assertj:assertj-core to 1.7.0 --- .../main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java | 2 +- .../src/main/java/com/jayway/jsonpath/internal/PathRef.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java b/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java index 661310b8..8f4bb3e7 100644 --- a/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java +++ b/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java @@ -33,7 +33,7 @@ public class JsonAsserterImpl implements JsonAsserter { try { obj = JsonPath.read(jsonObject, path); } catch (Exception e) { - throw new AssertionError(String.format("Error reading JSON path [%s]", path), e); + throw new AssertionError(String.format("Error reading JSON path [%s]: %s", path, e)); } if (!matcher.matches(obj)) { diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/PathRef.java b/json-path/src/main/java/com/jayway/jsonpath/internal/PathRef.java index 391fb8a1..b5192869 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/PathRef.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/PathRef.java @@ -154,7 +154,7 @@ public abstract class PathRef implements Comparable { public int compareTo(PathRef o) { if(o instanceof ArrayIndexPathRef){ ArrayIndexPathRef pf = (ArrayIndexPathRef) o; - return Integer.compare(pf.index, this.index); + return Integer.valueOf(pf.index).compareTo(this.index); } return super.compareTo(o); } From 5f8dc8ae0b82e55325fa90b116d93ba48490a9b9 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Mon, 19 Oct 2015 15:45:45 +0300 Subject: [PATCH 15/17] path compilation issues tests moved to proper location --- .../com/jayway/jsonpath/PathCompilerTest.java | 103 ++++++++++++++++++ .../com/jayway/jsonpath/PredicateTest.java | 103 ------------------ 2 files changed, 103 insertions(+), 103 deletions(-) diff --git a/json-path/src/test/java/com/jayway/jsonpath/PathCompilerTest.java b/json-path/src/test/java/com/jayway/jsonpath/PathCompilerTest.java index 5a1d05b6..32a14671 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PathCompilerTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/PathCompilerTest.java @@ -3,6 +3,8 @@ package com.jayway.jsonpath; import org.junit.Ignore; import org.junit.Test; +import java.util.List; + import static com.jayway.jsonpath.internal.PathCompiler.compile; import static org.assertj.core.api.Assertions.assertThat; @@ -128,4 +130,105 @@ public class PathCompilerTest { assertThat(compile("$..['prop']..[*]").toString()).isEqualTo("$..['prop']..[*]"); } + @Test + public void issue_predicate_can_have_escaped_backslash_in_prop() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"it\\\\\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + // message: it\ -> (after json escaping) -> "it\\" -> (after java escaping) -> "\"it\\\\\"" + + List result = JsonPath.read(json, "$.logs[?(@.message == 'it\\\\')].message"); + + assertThat(result).containsExactly("it\\"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_bracket_in_regex() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"(it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message =~ /\\(it/)].message"); + + assertThat(result).containsExactly("(it"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_and_in_regex() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message =~ /&&|it/)].message"); + + assertThat(result).containsExactly("it"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_can_have_and_in_prop() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"&& it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message == '&& it')].message"); + + assertThat(result).containsExactly("&& it"); + } + + @Ignore("not ready yet (requires compiler reimplementation)") + @Test + public void issue_predicate_brackets_must_change_priorities() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message && (@.id == 1 || @.id == 2))].id"); + assertThat(result).isEmpty(); + + result = JsonPath.read(json, "$.logs[?((@.id == 2 || @.id == 1) && @.message)].id"); + assertThat(result).isEmpty(); + } + + @Test + public void issue_predicate_can_have_square_bracket_in_prop() { + String json = "{\n" + + " \"logs\": [\n" + + " {\n" + + " \"message\": \"] it\",\n" + + " \"id\": 2\n" + + " }\n" + + " ]\n" + + "}"; + + List result = JsonPath.read(json, "$.logs[?(@.message == '] it')].message"); + + assertThat(result).containsExactly("] it"); + } } diff --git a/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java b/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java index 956ec2a6..62939070 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/PredicateTest.java @@ -1,6 +1,5 @@ package com.jayway.jsonpath; -import org.junit.Ignore; import org.junit.Test; import java.util.List; @@ -24,106 +23,4 @@ public class PredicateTest extends BaseTest { assertThat(reader.read("$.store.book[?].isbn", List.class, booksWithISBN)).containsOnly("0-395-19395-8", "0-553-21311-3"); } - - @Test - public void issue_predicate_can_have_escaped_backslash_in_prop() { - String json = "{\n" - + " \"logs\": [\n" - + " {\n" - + " \"message\": \"it\\\\\",\n" - + " \"id\": 2\n" - + " }\n" - + " ]\n" - + "}"; - // message: it\ -> (after json escaping) -> "it\\" -> (after java escaping) -> "\"it\\\\\"" - - List result = JsonPath.read(json, "$.logs[?(@.message == 'it\\\\')].message"); - - assertThat(result).containsExactly("it\\"); - } - - @Ignore("not ready yet (requires compiler reimplementation)") - @Test - public void issue_predicate_can_have_bracket_in_regex() { - String json = "{\n" - + " \"logs\": [\n" - + " {\n" - + " \"message\": \"(it\",\n" - + " \"id\": 2\n" - + " }\n" - + " ]\n" - + "}"; - - List result = JsonPath.read(json, "$.logs[?(@.message =~ /\\(it/)].message"); - - assertThat(result).containsExactly("(it"); - } - - @Ignore("not ready yet (requires compiler reimplementation)") - @Test - public void issue_predicate_can_have_and_in_regex() { - String json = "{\n" - + " \"logs\": [\n" - + " {\n" - + " \"message\": \"it\",\n" - + " \"id\": 2\n" - + " }\n" - + " ]\n" - + "}"; - - List result = JsonPath.read(json, "$.logs[?(@.message =~ /&&|it/)].message"); - - assertThat(result).containsExactly("it"); - } - - @Ignore("not ready yet (requires compiler reimplementation)") - @Test - public void issue_predicate_can_have_and_in_prop() { - String json = "{\n" - + " \"logs\": [\n" - + " {\n" - + " \"message\": \"&& it\",\n" - + " \"id\": 2\n" - + " }\n" - + " ]\n" - + "}"; - - List result = JsonPath.read(json, "$.logs[?(@.message == '&& it')].message"); - - assertThat(result).containsExactly("&& it"); - } - - @Ignore("not ready yet (requires compiler reimplementation)") - @Test - public void issue_predicate_brackets_must_change_priorities() { - String json = "{\n" - + " \"logs\": [\n" - + " {\n" - + " \"id\": 2\n" - + " }\n" - + " ]\n" - + "}"; - - List result = JsonPath.read(json, "$.logs[?(@.message && (@.id == 1 || @.id == 2))].id"); - assertThat(result).isEmpty(); - - result = JsonPath.read(json, "$.logs[?((@.id == 2 || @.id == 1) && @.message)].id"); - assertThat(result).isEmpty(); - } - - @Test - public void issue_predicate_can_have_square_bracket_in_prop() { - String json = "{\n" - + " \"logs\": [\n" - + " {\n" - + " \"message\": \"] it\",\n" - + " \"id\": 2\n" - + " }\n" - + " ]\n" - + "}"; - - List result = JsonPath.read(json, "$.logs[?(@.message == '] it')].message"); - - assertThat(result).containsExactly("] it"); - } } From 4642a09ae6ad12a9cf9644ce059cadf0779c0b91 Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Tue, 20 Oct 2015 14:26:45 +0300 Subject: [PATCH 16/17] corrected way to specify cause of AssertionError --- .../java/com/jayway/jsonassert/impl/JsonAsserterImpl.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java b/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java index 8f4bb3e7..e068460a 100644 --- a/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java +++ b/json-path-assert/src/main/java/com/jayway/jsonassert/impl/JsonAsserterImpl.java @@ -33,7 +33,9 @@ public class JsonAsserterImpl implements JsonAsserter { try { obj = JsonPath.read(jsonObject, path); } catch (Exception e) { - throw new AssertionError(String.format("Error reading JSON path [%s]: %s", path, e)); + final AssertionError assertionError = new AssertionError(String.format("Error reading JSON path [%s]", path)); + assertionError.initCause(e); + throw assertionError; } if (!matcher.matches(obj)) { From bcd00f7b7bb24971ad5b1dcb2eb3cb51a8e9ddca Mon Sep 17 00:00:00 2001 From: Alexey Makeyev Date: Tue, 20 Oct 2015 15:31:13 +0300 Subject: [PATCH 17/17] test moved to avoid visibility changes --- .../java/com/jayway/jsonpath/internal/token/PathToken.java | 4 ++-- .../com/jayway/jsonpath/internal/token/PropertyPathToken.java | 2 +- .../com/jayway/jsonpath/internal/token/ScanPathToken.java | 2 +- .../com/jayway/jsonpath/internal/token/WildcardPathToken.java | 2 +- .../jayway/jsonpath/{ => internal/token}/PathTokenTest.java | 3 ++- 5 files changed, 7 insertions(+), 6 deletions(-) rename json-path/src/test/java/com/jayway/jsonpath/{ => internal/token}/PathTokenTest.java (95%) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java index 781f23d2..47b76383 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PathToken.java @@ -36,7 +36,7 @@ public abstract class PathToken { private Boolean definite = null; private Boolean upstreamDefinite = null; - public PathToken appendTailToken(PathToken next) { + PathToken appendTailToken(PathToken next) { this.next = next; this.next.prev = this; return next; @@ -161,7 +161,7 @@ public abstract class PathToken { return prev == null; } - public boolean isUpstreamDefinite() { + boolean isUpstreamDefinite() { if (upstreamDefinite == null) { upstreamDefinite = isRoot() || prev.isTokenDefinite() && prev.isUpstreamDefinite(); } diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java index 24918f3e..d4633a67 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/PropertyPathToken.java @@ -27,7 +27,7 @@ import static com.jayway.jsonpath.internal.Utils.onlyOneIsTrueNonThrow; /** * */ -public class PropertyPathToken extends PathToken { +class PropertyPathToken extends PathToken { private final List properties; diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java index a67fcd80..07d48ad2 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/ScanPathToken.java @@ -25,7 +25,7 @@ import java.util.Collection; */ public class ScanPathToken extends PathToken { - public ScanPathToken() { + ScanPathToken() { } @Override diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/token/WildcardPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/token/WildcardPathToken.java index 4325301f..1ce86ddc 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/token/WildcardPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/token/WildcardPathToken.java @@ -25,7 +25,7 @@ import static java.util.Arrays.asList; */ public class WildcardPathToken extends PathToken { - public WildcardPathToken() { + WildcardPathToken() { } @Override diff --git a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java b/json-path/src/test/java/com/jayway/jsonpath/internal/token/PathTokenTest.java similarity index 95% rename from json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java rename to json-path/src/test/java/com/jayway/jsonpath/internal/token/PathTokenTest.java index b7612649..e1e68dc4 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/PathTokenTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/internal/token/PathTokenTest.java @@ -1,5 +1,6 @@ -package com.jayway.jsonpath; +package com.jayway.jsonpath.internal.token; +import com.jayway.jsonpath.BaseTest; import com.jayway.jsonpath.internal.token.PathToken; import com.jayway.jsonpath.internal.token.PropertyPathToken; import com.jayway.jsonpath.internal.token.ScanPathToken;