Merge pull request #335 from mgreenwood1001/Issue234

Fix for Issue #234 - Late Binding for Function Parameters

json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java

@@ -146,8 +146,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);
         }
 

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

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

json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/ILateBindingValue.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2011 the original author or authors.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+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
+ *
+ */
+public interface ILateBindingValue {
+    /**
+     * Obtain the value of the parameter at runtime using the parameter state and invocation of other late binding values
+     * rather than maintaining cached state which ends up in a global store and won't change as a result of external
+     * reference changes.
+     *
+     * @return
+     *      The value of evaluating the context at runtime.
+     */
+    Object get();
+}

json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/JsonLateBindingValue.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2011 the original author or authors.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.jayway.jsonpath.internal.function.latebinding;
+
+import com.jayway.jsonpath.internal.function.Parameter;
+import com.jayway.jsonpath.spi.json.JsonProvider;
+
+/**
+ * Defines the JSON document Late binding approach to function arguments.
+ *
+ */
+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;
+    }
+
+    /**
+     * Evaluate the JSON document at the point of need using the JSON parameter and associated document model which may
+     * itself originate from yet another function thus recursively invoking late binding methods.
+     *
+     * @return
+     */
+    @Override
+    public Object get() {
+        return jsonProvider.parse(jsonParameter.getJson());
+    }
+}

json-path/src/main/java/com/jayway/jsonpath/internal/function/latebinding/PathLateBindingValue.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2011 the original author or authors.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+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...
+ *
+ */
+public class PathLateBindingValue implements ILateBindingValue {
+    private final Path path;
+    private final Object rootDocument;
+    private final Configuration configuration;
+
+    public PathLateBindingValue(final Path path, final Object rootDocument, final Configuration configuration) {
+        this.path = path;
+        this.rootDocument = rootDocument;
+        this.configuration = configuration;
+    }
+
+    /**
+     * Evaluate the expression at the point of need for Path type expressions
+     *
+     * @return
+     */
+    public Object get() {
+        return path.evaluate(rootDocument, rootDocument, configuration).getValue();
+    }
+}

json-path/src/main/java/com/jayway/jsonpath/internal/function/numeric/AbstractAggregation.java

@@ -49,10 +49,10 @@ public abstract class AbstractAggregation implements PathFunction {
         }
         if (parameters != null) {
             for (Parameter param : parameters) {
-                if (param.getCachedValue() instanceof Number) {
-                    Number value = (Number)param.getCachedValue();
+                Object value = param.getValue();
+                if (null != value && value instanceof Number) {
                     count++;
-                    next(value);
+                    next((Number)value);
                 }
             }
         }

json-path/src/main/java/com/jayway/jsonpath/internal/function/text/Concatenate.java

@@ -27,8 +27,9 @@ public class Concatenate implements PathFunction {
         }
         if (parameters != null) {
             for (Parameter param : parameters) {
-                if (param.getCachedValue() != null) {
-                    result.append(param.getCachedValue().toString());
+		Object value = param.getValue();
+                if (value != null) {
+                    result.append(value.toString());
                 }
             }
         }

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,11 @@ 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.setLateBinding(new PathLateBindingValue(param.getPath(), ctx.rootDocument(), ctx.configuration()));
                             param.setEvaluated(true);
                             break;
                         case JSON:
-                            param.setCachedValue(ctx.configuration().jsonProvider().parse(param.getJson()));
+                            param.setLateBinding(new JsonLateBindingValue(ctx.configuration().jsonProvider(), param));
                             param.setEvaluated(true);
                             break;
                     }

json-path/src/test/java/com/jayway/jsonpath/internal/function/Issue234.java

@@ -0,0 +1,38 @@
+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.
+ *
+ */
+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");
+    }
+}