Browse Source

Fixes Issue #234 using late binding, ideally this might be a lambda that encapsulates its state -- given support for JDK 6+, its encapsulated state is maintained in an implementation of the interface ILateBindingValue, one for PATH functions one for JSON - its likely the JSON version doesn't have a purpose unless the JSON dynamically changes as a result of the function implementation but its better to create another impl for JSON if in the future JSON can be dynamically changed via functions(?)

The fault is in the Parameter object which obviously cached its value and when the outside reference changes its oblivious to that state change due to its internal cached instance of the state.    Cache.getCache()... that singleton call inside of JsonContext then grabs an instance of the cached parameter, it would be thread unsafe to simple invalidate the cache because who knows for whom the cache is being invalidated.

Not caching function paths isn't an answer -- the input to a function could itself be another function - meaning the input (parameter) value would never be observed to the wrapping function and things such as take the $.max(3, 4, 5, $.avg(...)) would yield an answer without average being computed.
pull/335/head
mattg 8 years ago
parent
commit
3a69db83f0
  1. 1
      json-path/src/main/java/com/jayway/jsonpath/JsonPath.java
  2. 5
      json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java
  3. 12
      json-path/src/main/java/com/jayway/jsonpath/internal/function/Parameter.java
  4. 2
      json-path/src/main/java/com/jayway/jsonpath/internal/function/json/Append.java
  5. 10
      json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/ILateBindingValue.java
  6. 22
      json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/JsonLateBindingValue.java
  7. 33
      json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/PathLateBindingValue.java
  8. 4
      json-path/src/main/java/com/jayway/jsonpath/internal/function/numeric/AbstractAggregation.java
  9. 4
      json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Concatenate.java
  10. 8
      json-path/src/main/java/com/jayway/jsonpath/internal/path/FunctionPathToken.java
  11. 39
      json-path/src/test/java/com/jayway/jsonpath/internal/function/Issue234.java

1
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;

5
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);
}

12
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() {

2
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());
}
}
}

10
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();
}

22
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());
}
}

33
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();
}
}

4
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);
}

4
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());
}
}
}

8
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;
}

39
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<String, String> 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");
}
}
Loading…
Cancel
Save