diff --git a/json-path/src/main/java/com/jayway/jsonpath/JsonPath.java b/json-path/src/main/java/com/jayway/jsonpath/JsonPath.java index 8ce853d5..bc4e6144 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/JsonPath.java +++ b/json-path/src/main/java/com/jayway/jsonpath/JsonPath.java @@ -21,6 +21,7 @@ import com.jayway.jsonpath.internal.Path; import com.jayway.jsonpath.internal.PathRef; import com.jayway.jsonpath.internal.Utils; import com.jayway.jsonpath.internal.path.PathCompiler; +import com.jayway.jsonpath.spi.cache.Cache; import com.jayway.jsonpath.spi.json.JsonProvider; import java.io.File; diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java b/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java index 50b84a08..6f2706b1 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java @@ -26,6 +26,7 @@ import com.jayway.jsonpath.ReadContext; import com.jayway.jsonpath.TypeRef; import com.jayway.jsonpath.spi.cache.Cache; import com.jayway.jsonpath.spi.cache.CacheProvider; +import com.jayway.jsonpath.spi.cache.LRUCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -146,8 +147,8 @@ public class JsonContext implements ParseContext, DocumentContext { if(jsonPath != null){ return read(jsonPath); } else { - jsonPath = compile(path, filters); - cache.put(cacheKey, jsonPath); + jsonPath = compile(path, filters); + cache.put(cacheKey, jsonPath); return read(jsonPath); } diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/Parameter.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/Parameter.java index e8eabf97..75824ff8 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/function/Parameter.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/Parameter.java @@ -1,6 +1,8 @@ package com.jayway.jsonpath.internal.function; import com.jayway.jsonpath.internal.Path; +import com.jayway.jsonpath.internal.function.latebinding.ILateBindingValue; +import com.jayway.jsonpath.internal.function.latebinding.PathLateBindingValue; /** * Created by matt@mjgreenwood.net on 12/10/15. @@ -8,7 +10,7 @@ import com.jayway.jsonpath.internal.Path; public class Parameter { private ParamType type; private Path path; - private Object cachedValue; + private ILateBindingValue lateBinding; private Boolean evaluated = false; private String json; @@ -24,12 +26,12 @@ public class Parameter { this.type = ParamType.PATH; } - public Object getCachedValue() { - return cachedValue; + public Object getValue() { + return lateBinding.get(); } - public void setCachedValue(Object cachedValue) { - this.cachedValue = cachedValue; + public void setLateBinding(ILateBindingValue lateBinding) { + this.lateBinding = lateBinding; } public Path getPath() { diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/json/Append.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/json/Append.java index 805226e4..ed39d4a8 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/function/json/Append.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/json/Append.java @@ -22,7 +22,7 @@ public class Append implements PathFunction { for (Parameter param : parameters) { if (jsonProvider.isArray(model)) { int len = jsonProvider.length(model); - jsonProvider.setArrayIndex(model, len, param.getCachedValue()); + jsonProvider.setArrayIndex(model, len, param.getValue()); } } } diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/ILateBindingValue.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/ILateBindingValue.java new file mode 100644 index 00000000..0d0ed403 --- /dev/null +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/ILateBindingValue.java @@ -0,0 +1,10 @@ +package com.jayway.jsonpath.internal.function.latebinding; + +/** + * Obtain the late binding value at runtime rather than storing the value in the cache thus trashing the cache + * + * Created by mattg on 3/27/17. + */ +public interface ILateBindingValue { + Object get(); +} diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/JsonLateBindingValue.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/JsonLateBindingValue.java new file mode 100644 index 00000000..ef09725e --- /dev/null +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/JsonLateBindingValue.java @@ -0,0 +1,22 @@ +package com.jayway.jsonpath.internal.function.latebinding; + +import com.jayway.jsonpath.internal.function.Parameter; +import com.jayway.jsonpath.spi.json.JsonProvider; + +/** + * Created by mattg on 3/27/17. + */ +public class JsonLateBindingValue implements ILateBindingValue { + private final JsonProvider jsonProvider; + private final Parameter jsonParameter; + + public JsonLateBindingValue(JsonProvider jsonProvider, Parameter jsonParameter) { + this.jsonProvider = jsonProvider; + this.jsonParameter = jsonParameter; + } + + @Override + public Object get() { + return jsonProvider.parse(jsonParameter.getJson()); + } +} diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/PathLateBindingValue.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/PathLateBindingValue.java new file mode 100644 index 00000000..58c70018 --- /dev/null +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/PathLateBindingValue.java @@ -0,0 +1,33 @@ +package com.jayway.jsonpath.internal.function.latebinding; + +import com.jayway.jsonpath.Configuration; +import com.jayway.jsonpath.internal.Path; +import com.jayway.jsonpath.internal.function.ParamType; + +/** + * Defines the contract for late bindings, provides document state and enough context to perform the evaluation at a later + * date such that we can operate on a dynamically changing value. + * + * Acts like a lambda function with references, but since we're supporting JDK 6+, we're left doing this... + * + * Created by mattg on 3/27/17. + */ +public class PathLateBindingValue implements ILateBindingValue { + private final Path path; + private final Object rootDocument; + private final Configuration configuration; + + public PathLateBindingValue(ParamType type, Path path, Object rootDocument, Configuration configuration) { + this.path = path; + this.rootDocument = rootDocument; + this.configuration = configuration; + } + + /** + * Evaluate the field type + * @return + */ + public Object get() { + return path.evaluate(rootDocument, rootDocument, configuration).getValue(); + } +} diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/numeric/AbstractAggregation.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/numeric/AbstractAggregation.java index 78d9da99..1215f434 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/function/numeric/AbstractAggregation.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/numeric/AbstractAggregation.java @@ -49,8 +49,8 @@ public abstract class AbstractAggregation implements PathFunction { } if (parameters != null) { for (Parameter param : parameters) { - if (param.getCachedValue() instanceof Number) { - Number value = (Number)param.getCachedValue(); + if (param.getValue() instanceof Number) { + Number value = (Number)param.getValue(); count++; next(value); } diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Concatenate.java b/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Concatenate.java index 6c2dc1c4..19689edf 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Concatenate.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Concatenate.java @@ -27,8 +27,8 @@ public class Concatenate implements PathFunction { } if (parameters != null) { for (Parameter param : parameters) { - if (param.getCachedValue() != null) { - result.append(param.getCachedValue().toString()); + if (param.getValue() != null) { + result.append(param.getValue().toString()); } } } 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 8e9055ed..af8d073d 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 @@ -4,6 +4,8 @@ import com.jayway.jsonpath.internal.PathRef; import com.jayway.jsonpath.internal.function.Parameter; import com.jayway.jsonpath.internal.function.PathFunction; import com.jayway.jsonpath.internal.function.PathFunctionFactory; +import com.jayway.jsonpath.internal.function.latebinding.JsonLateBindingValue; +import com.jayway.jsonpath.internal.function.latebinding.PathLateBindingValue; import java.util.List; @@ -49,11 +51,13 @@ public class FunctionPathToken extends PathToken { if (!param.hasEvaluated()) { switch (param.getType()) { case PATH: - param.setCachedValue(param.getPath().evaluate(ctx.rootDocument(), ctx.rootDocument(), ctx.configuration()).getValue()); + //param.setCachedValue(param.getPath().evaluate(ctx.rootDocument(), ctx.rootDocument(), ctx.configuration()).getValue()); + param.setLateBinding(new PathLateBindingValue(param.getType(), param.getPath(), ctx.rootDocument(), ctx.configuration())); param.setEvaluated(true); break; case JSON: - param.setCachedValue(ctx.configuration().jsonProvider().parse(param.getJson())); +// ctx.configuration().jsonProvider().parse(param.getJson()) + param.setLateBinding(new JsonLateBindingValue(ctx.configuration().jsonProvider(), param)); param.setEvaluated(true); break; } diff --git a/json-path/src/test/java/com/jayway/jsonpath/internal/function/Issue234.java b/json-path/src/test/java/com/jayway/jsonpath/internal/function/Issue234.java new file mode 100644 index 00000000..17e1e54a --- /dev/null +++ b/json-path/src/test/java/com/jayway/jsonpath/internal/function/Issue234.java @@ -0,0 +1,39 @@ +package com.jayway.jsonpath.internal.function; + +import com.jayway.jsonpath.JsonPath; +import org.assertj.core.util.Maps; +import org.junit.Test; + +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * TDD for Issue #234 + * + * Verifies the use-case where-in the function path expression is cached and re-used but the JsonPath includes a function + * whose arguments are then dependent upon state that changes externally from the internal Cache.getCache state. The + * prior implementation had a bug where-in the parameter values were cached -- the present implementation (as of Issue #234) + * now uses a late binding approach to eval the function parameters. Cache invalidation isn't an option given the need + * for nested function calls. + * + * Once this bug is fixed, most of the concern then centers around the need to ensure nested functions are processed + * correctly. + * + * @see NestedFunctionTest for examples of where that is performed. + * + * Created by mattg on 3/27/17. + */ +public class Issue234 { + + @Test + public void testIssue234() { + Map context = Maps.newHashMap(); + context.put("key", "first"); + Object value = JsonPath.read(context, "concat(\"/\", $.key)"); + assertThat(value).isEqualTo("/first"); + context.put("key", "second"); + value = JsonPath.read(context, "concat(\"/\", $.key)"); + assertThat(value).isEqualTo("/second"); + } +}