diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java index 875e201a5..462efefdc 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java @@ -36,9 +36,11 @@ import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionException; import org.springframework.expression.spel.SpelEvaluationException; +import org.springframework.expression.spel.SpelMessage; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.SimpleEvaluationContext; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.ConcurrentReferenceHashMap; import org.springframework.util.StringUtils; @@ -220,6 +222,7 @@ class SpelPath { static class TypedSpelPath extends SpelPath { private static final String INVALID_PATH_REFERENCE = "Invalid path reference %s on type %s (from source %s)!"; + private static final String INVALID_COLLECTION_INDEX = "Invalid collection index %s for collection of size %s. Use '…/-' or the collection's actual size as index to append to it!"; private static final Map TYPED_PATHS = new ConcurrentReferenceHashMap( 32); private static final EvaluationContext CONTEXT = SimpleEvaluationContext.forReadWriteDataBinding().build(); @@ -306,7 +309,24 @@ class SpelPath { Assert.notNull(root, "Root object must not be null!"); - return expression.getValueType(CONTEXT, root); + try { + + return expression.getValueType(CONTEXT, root); + + } catch (SpelEvaluationException o_O) { + + if (!SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS.equals(o_O.getMessageCode())) { + throw o_O; + } + + Object collectionOrArray = getParent().getValue(root); + + if (Collection.class.isInstance(collectionOrArray)) { + return CollectionUtils.findCommonElementType(Collection.class.cast(collectionOrArray)); + } + } + + throw new IllegalArgumentException(String.format("Cannot obtain type for path %s on %s!", path, root)); } /** @@ -404,6 +424,11 @@ class SpelPath { } else { List list = parentPath.getValue(target); + + if (listIndex > list.size()) { + throw new PatchException(String.format(INVALID_COLLECTION_INDEX, listIndex, list.size())); + } + list.add(listIndex >= 0 ? listIndex.intValue() : list.size(), value); } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationUnitTests.java index 4620ffbc7..41fb3db35 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationUnitTests.java @@ -21,13 +21,17 @@ import static org.junit.Assert.*; import java.util.ArrayList; import java.util.List; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; public class AddOperationUnitTests { + public @Rule ExpectedException exception = ExpectedException.none(); + @Test public void addBooleanPropertyValue() throws Exception { @@ -112,4 +116,30 @@ public class AddOperationUnitTests { assertThat(todo.getUninitialized(), is(notNullValue())); assertThat(todo.getUninitialized(), hasItem("Text")); } + + @Test // DATAREST-1273 + public void addsItemToTheEndOfACollectionViaIndex() { + + List todos = new ArrayList(); + todos.add(new Todo(1L, "A", false)); + + Todo todo = new Todo(2L, "B", true); + AddOperation.of("/1", todo).perform(todos, Todo.class); + + assertThat(todos.get(1), is(todo)); + } + + @Test // DATAREST-1273 + public void rejectsAdditionBeyondEndOfList() { + + List todos = new ArrayList(); + todos.add(new Todo(1L, "A", false)); + + exception.expect(PatchException.class); + exception.expectMessage("index"); + exception.expectMessage("1"); + exception.expectMessage("2"); + + AddOperation.of("/2", new Todo(2L, "B", true)).perform(todos, Todo.class); + } }