From e6b08709331d100fd1285ec2a1c5bfccd500c842 Mon Sep 17 00:00:00 2001 From: curforever <77817413+curforever@users.noreply.github.com> Date: Thu, 8 Oct 2026 08:20:48 +0800 Subject: [PATCH] Preserve wildcard arguments when reusing compiled functions Move the cleanup of Length's temporary wildcard into its own finally block. Removing the first wildcard after every function call otherwise changes a compiled aggregation's original argument and breaks subsequent reads. Add regression coverage across all eight test providers for numeric aggregation, cached queries, repeated length/size calls, and recovery after provider failure. Fixes #1038. --- .../internal/function/text/Length.java | 18 ++- .../internal/path/FunctionPathToken.java | 19 --- .../function/CompiledFunctionReuseTest.java | 111 ++++++++++++++++++ 3 files changed, 124 insertions(+), 24 deletions(-) create mode 100644 json-path/src/test/java/com/jayway/jsonpath/internal/function/CompiledFunctionReuseTest.java diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Length.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Length.java index ea8ab1ffe..b0452b496 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Length.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Length.java @@ -47,11 +47,12 @@ public Object invoke(String currentPath, PathRef parent, Object model, Evaluatio // for length - to the wildcard such that we request all of its children so we can get back an array and // take its length. Parameter lengthOfParameter = parameters.get(0); + PathToken tail = null; if (!lengthOfParameter.getPath().isFunctionPath()) { Path path = lengthOfParameter.getPath(); if (path instanceof CompiledPath) { RootPathToken root = ((CompiledPath) path).getRoot(); - PathToken tail = root.getNext(); + tail = root.getNext(); while (null != tail && null != tail.getNext()) { tail = tail.getNext(); } @@ -60,9 +61,16 @@ public Object invoke(String currentPath, PathRef parent, Object model, Evaluatio } } } - Object innerModel = parameters.get(0).getPath().evaluate(model, model, ctx.configuration()).getValue(); - if (ctx.configuration().jsonProvider().isArray(innerModel)) { - return ctx.configuration().jsonProvider().length(innerModel); + try { + Object innerModel = lengthOfParameter.getPath().evaluate(model, model, ctx.configuration()).getValue(); + if (ctx.configuration().jsonProvider().isArray(innerModel)) { + return ctx.configuration().jsonProvider().length(innerModel); + } + } finally { + // Remove only the wildcard appended by this invocation, including when evaluation fails. + if (null != tail) { + tail.setNext(null); + } } } if (ctx.configuration().jsonProvider().isArray(model)) { @@ -72,4 +80,4 @@ public Object invoke(String currentPath, PathRef parent, Object model, Evaluatio } return null; } -} \ No newline at end of file +} diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/path/FunctionPathToken.java b/json-path/src/main/java/com/jayway/jsonpath/internal/path/FunctionPathToken.java index 0f4498958..2d24b70d2 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/path/FunctionPathToken.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/path/FunctionPathToken.java @@ -1,6 +1,5 @@ package com.jayway.jsonpath.internal.path; -import com.jayway.jsonpath.internal.Path; import com.jayway.jsonpath.internal.PathRef; import com.jayway.jsonpath.internal.function.Parameter; import com.jayway.jsonpath.internal.function.PathFunction; @@ -40,29 +39,11 @@ public void evaluate(String currentPath, PathRef parent, Object model, Evaluatio evaluateParameters(currentPath, parent, model, ctx); Object result = pathFunction.invoke(currentPath, parent, model, ctx, functionParams); ctx.addResult(currentPath + "." + functionName, parent, result); - cleanWildcardPathToken(); if (!isLeaf()) { next().evaluate(currentPath, parent, result, ctx); } } - private void cleanWildcardPathToken() { - if (null != functionParams && functionParams.size() > 0) { - Path path = functionParams.get(0).getPath(); - if (null != path && !path.isFunctionPath() && path instanceof CompiledPath) { - RootPathToken root = ((CompiledPath) path).getRoot(); - PathToken tail = root.getNext(); - while (null != tail && null != tail.getNext() ) { - if(tail.getNext() instanceof WildcardPathToken){ - tail.setNext(tail.getNext().getNext()); - break; - } - tail = tail.getNext(); - } - } - } - } - private void evaluateParameters(String currentPath, PathRef parent, Object model, EvaluationContextImpl ctx) { if (null != functionParams) { diff --git a/json-path/src/test/java/com/jayway/jsonpath/internal/function/CompiledFunctionReuseTest.java b/json-path/src/test/java/com/jayway/jsonpath/internal/function/CompiledFunctionReuseTest.java new file mode 100644 index 000000000..e836bb045 --- /dev/null +++ b/json-path/src/test/java/com/jayway/jsonpath/internal/function/CompiledFunctionReuseTest.java @@ -0,0 +1,111 @@ +package com.jayway.jsonpath.internal.function; + +import com.jayway.jsonpath.Configuration; +import com.jayway.jsonpath.Configurations; +import com.jayway.jsonpath.JsonPath; +import com.jayway.jsonpath.spi.json.JsonSmartJsonProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class CompiledFunctionReuseTest { + + private static final String FIRST = "{\"dataset\":[{\"value\":1},{\"value\":3},{\"value\":2}]}"; + private static final String SECOND = "{\"dataset\":[{\"value\":5},{\"value\":11},{\"value\":2}]}"; + + static Iterable configurations() { + return Configurations.configurations(); + } + + static List numericAggregations() { + List arguments = new ArrayList<>(); + for (Configuration configuration : configurations()) { + arguments.add(Arguments.of(configuration, "min", 1.0, 2.0)); + arguments.add(Arguments.of(configuration, "max", 3.0, 11.0)); + arguments.add(Arguments.of(configuration, "sum", 6.0, 18.0)); + arguments.add(Arguments.of(configuration, "avg", 2.0, 6.0)); + arguments.add(Arguments.of(configuration, "stddev", Math.sqrt(2.0 / 3.0), Math.sqrt(14.0))); + } + return arguments; + } + + @ParameterizedTest + @MethodSource("numericAggregations") + void compiledAggregationRetainsItsWildcard(Configuration configuration, String function, + double firstExpected, double secondExpected) { + JsonPath compiled = JsonPath.compile(function + "($.dataset[*].value)"); + + assertEquals(firstExpected, ((Number) compiled.read(FIRST, configuration)).doubleValue(), 0.000001); + assertEquals(secondExpected, ((Number) compiled.read(SECOND, configuration)).doubleValue(), 0.000001); + assertEquals(firstExpected, ((Number) compiled.read(FIRST, configuration)).doubleValue(), 0.000001); + } + + @ParameterizedTest + @MethodSource("configurations") + void cachedAggregationCanReadAnotherDocument(Configuration configuration) { + String first = "{\"cachedDataset\":[{\"value\":1},{\"value\":3}]}"; + String second = "{\"cachedDataset\":[{\"value\":5},{\"value\":11}]}"; + String expression = "max($.cachedDataset[*].value)"; + + assertEquals(3.0, ((Number) JsonPath.using(configuration).parse(first).read(expression)).doubleValue()); + assertEquals(11.0, ((Number) JsonPath.using(configuration).parse(second).read(expression)).doubleValue()); + } + + @ParameterizedTest + @MethodSource("configurations") + void lengthAndSizeRetainAnExistingWildcard(Configuration configuration) { + String first = "{\"groups\":[{\"items\":[1,2]},{\"items\":[3]}]}"; + String second = "{\"groups\":[{\"items\":[9]},{\"items\":[8,7,6]}]}"; + for (String function : new String[]{"length", "size"}) { + JsonPath compiled = JsonPath.compile(function + "($.groups[*].items)"); + assertEquals(3, ((Number) compiled.read(first, configuration)).intValue()); + assertEquals(4, ((Number) compiled.read(second, configuration)).intValue()); + assertEquals(3, ((Number) compiled.read(first, configuration)).intValue()); + } + } + + @ParameterizedTest + @MethodSource("configurations") + void repeatedScanLengthDoesNotAccumulateWildcards(Configuration configuration) { + String first = "{\"store\":{\"book\":[{\"value\":1},{\"value\":2}]}}"; + String second = "{\"store\":{\"book\":[{\"value\":3},{\"value\":4},{\"value\":5}]}}"; + for (String function : new String[]{"length", "size"}) { + JsonPath compiled = JsonPath.compile("$..book." + function + "()"); + assertEquals(2, ((Number) compiled.read(first, configuration)).intValue()); + assertEquals(3, ((Number) compiled.read(second, configuration)).intValue()); + assertEquals(2, ((Number) compiled.read(first, configuration)).intValue()); + } + } + + @Test + void lengthCanBeRetriedAfterProviderFailure() { + JsonSmartJsonProvider provider = new JsonSmartJsonProvider() { + private boolean failOnce = true; + + @Override + public Object getArrayIndex(Object value, int index) { + if (failOnce && length(value) == 3) { + failOnce = false; + throw new IllegalStateException("provider failure"); + } + return super.getArrayIndex(value, index); + } + }; + Configuration configuration = Configuration.builder().jsonProvider(provider).build(); + JsonPath compiled = JsonPath.compile("length($.groups[*].items)"); + String json = "{\"groups\":[{\"items\":[1,2,3]}]}"; + + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> compiled.read(json, configuration)); + assertEquals("provider failure", failure.getMessage()); + assertEquals(3, (int) compiled.read(json, configuration)); + assertEquals(3, (int) compiled.read(json, configuration)); + } +}