DATAREST-1039 - Fix appending a complex value appended to a collection via JSON Patch.

When a PATCH call using JSON Patch tried to append an item to an empty collection, it previously failed to look up the type to unmarshal the incoming payload to. We now inspect the declared types on the wrapped object's property to determine that type for append operations.

Also, an uninitialized collection is now initialized using Spring's CollectionFactory.

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java

@@ -15,6 +15,12 @@
  */
 package org.springframework.data.rest.webmvc.json.patch;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import org.springframework.data.mapping.PropertyPath;
+import org.springframework.util.StringUtils;
+
 /**
  * Operation to add a new value to the given "path". Will throw a {@link PatchException} if the path is invalid or if
  * the given value is not assignable to the given path.
@@ -42,4 +48,29 @@ class AddOperation extends PatchOperation {
 	<T> void perform(Object targetObject, Class<T> type) {
 		addValue(targetObject, evaluateValueFromTarget(targetObject, type));
 	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#evaluateValueFromTarget(java.lang.Object, java.lang.Class)
+	 */
+	@Override
+	protected <T> Object evaluateValueFromTarget(Object targetObject, Class<T> entityType) {
+
+		if (!path.endsWith("-")) {
+			return super.evaluateValueFromTarget(targetObject, entityType);
+		}
+
+		List<String> segments = new ArrayList<String>();
+
+		for (String segment : path.split("/")) {
+			if (!(segment.matches("\\d+") || segment.equals("-") || segment.isEmpty())) {
+				segments.add(segment);
+			}
+		}
+
+		PropertyPath propertyPath = PropertyPath.from(StringUtils.collectionToDelimitedString(segments, "."), entityType);
+
+		return value instanceof LateObjectEvaluator ? ((LateObjectEvaluator) value).evaluate(propertyPath.getType())
+				: value;
+	}
 }

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java

@@ -17,8 +17,11 @@ package org.springframework.data.rest.webmvc.json.patch;
 
 import static org.springframework.data.rest.webmvc.json.patch.PathToSpEL.*;
 
+import java.util.Collection;
 import java.util.List;
 
+import org.springframework.core.CollectionFactory;
+import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.expression.Expression;
 import org.springframework.expression.ExpressionException;
 
@@ -129,7 +132,21 @@ public abstract class PatchOperation {
 		Integer listIndex = targetListIndex(path);
 
 		if (parent == null || !(parent instanceof List) || listIndex == null) {
-			spelExpression.setValue(target, value);
+
+			TypeDescriptor descriptor = parentExpression.getValueTypeDescriptor(target);
+
+			// Set as new collection if necessary
+			if (descriptor.isCollection() && !Collection.class.isInstance(value)) {
+
+				Collection<Object> collection = CollectionFactory.createCollection(descriptor.getType(), 1);
+				collection.add(value);
+
+				parentExpression.setValue(target, collection);
+
+			} else {
+				spelExpression.setValue(target, value);
+			}
+
 		} else {
 
 			List<Object> list = (List<Object>) parentExpression.getValue(target);

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PathToSpEL.java

@@ -79,7 +79,12 @@ public class PathToSpEL {
 			}
 
 			if (APPEND_CHARACTERS.contains(pathNode)) {
-				spelBuilder.append("[size() - 1]");
+
+				if (spelBuilder.length() > 0) {
+					spelBuilder.append(".");
+				}
+
+				spelBuilder.append("$[true]");
 				continue;
 			}
 

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationTests.java

@@ -23,6 +23,9 @@ import java.util.List;
 
 import org.junit.Test;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class AddOperationTests {
 
 	@Test
@@ -84,4 +87,29 @@ public class AddOperationTests {
 
 		assertThat(todo.getItems().get(0), is("Some text."));
 	}
+
+	@Test // DATAREST-1039
+	public void addsLazilyEvaluatedObjectToList() throws Exception {
+
+		Todo todo = new Todo(1L, "description", false);
+
+		ObjectMapper mapper = new ObjectMapper();
+		JsonNode node = mapper.readTree("\"Some text.\"");
+		JsonLateObjectEvaluator evaluator = new JsonLateObjectEvaluator(mapper, node);
+
+		new AddOperation("/items/-", evaluator).perform(todo, Todo.class);
+
+		assertThat(todo.getItems().get(0), is("Some text."));
+	}
+
+	@Test // DATAREST-1039
+	public void initializesNullCollectionsOnAppend() {
+
+		Todo todo = new Todo(1L, "description", false);
+
+		new AddOperation("/uninitialized/-", "Text").perform(todo, Todo.class);
+
+		assertThat(todo.getUninitialized(), is(notNullValue()));
+		assertThat(todo.getUninitialized(), hasItem("Text"));
+	}
 }

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/Todo.java

@@ -16,13 +16,13 @@
 
 package org.springframework.data.rest.webmvc.json.patch;
 
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.List;
 
-import lombok.Data;
-import lombok.NoArgsConstructor;
-
 /**
  * @author Roy Clarkson
  * @author Craig Walls
@@ -38,6 +38,7 @@ class Todo {
 	private boolean complete;
 	private TodoType type = new TodoType();
 	private List<String> items = new ArrayList<String>();
+	private List<String> uninitialized;
 	private BigInteger amount;
 
 	public Todo(Long id, String description, boolean complete) {