Skip to content

Commit 8e7665c

Browse files
committed
DATAREST-1273 - Properly support adding to collections via indexes in JsonPatch expressions.
We now support adding a to a collection via JsonPatch expressions by using the collection's size as target index. Contrary to paths pointing to existing collection elements we cannot determine the type of the object to be created. Thus, we fall back to the common element type of all existing collection elements. We also reject indexes pointing to elements with an index greater than the current collection's size.
1 parent 69ee773 commit 8e7665c

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@
3636
import org.springframework.expression.Expression;
3737
import org.springframework.expression.ExpressionException;
3838
import org.springframework.expression.spel.SpelEvaluationException;
39+
import org.springframework.expression.spel.SpelMessage;
3940
import org.springframework.expression.spel.standard.SpelExpressionParser;
4041
import org.springframework.expression.spel.support.SimpleEvaluationContext;
4142
import org.springframework.util.Assert;
43+
import org.springframework.util.CollectionUtils;
4244
import org.springframework.util.ConcurrentReferenceHashMap;
4345
import org.springframework.util.StringUtils;
4446

@@ -220,6 +222,7 @@ private static String pathNodesToSpEL(String[] pathNodes) {
220222
static class TypedSpelPath extends SpelPath {
221223

222224
private static final String INVALID_PATH_REFERENCE = "Invalid path reference %s on type %s (from source %s)!";
225+
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!";
223226
private static final Map<CacheKey, TypedSpelPath> TYPED_PATHS = new ConcurrentReferenceHashMap<CacheKey, TypedSpelPath>(
224227
32);
225228
private static final EvaluationContext CONTEXT = SimpleEvaluationContext.forReadWriteDataBinding().build();
@@ -306,7 +309,24 @@ public Class<?> getType(Object root) {
306309

307310
Assert.notNull(root, "Root object must not be null!");
308311

309-
return expression.getValueType(CONTEXT, root);
312+
try {
313+
314+
return expression.getValueType(CONTEXT, root);
315+
316+
} catch (SpelEvaluationException o_O) {
317+
318+
if (!SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS.equals(o_O.getMessageCode())) {
319+
throw o_O;
320+
}
321+
322+
Object collectionOrArray = getParent().getValue(root);
323+
324+
if (Collection.class.isInstance(collectionOrArray)) {
325+
return CollectionUtils.findCommonElementType(Collection.class.cast(collectionOrArray));
326+
}
327+
}
328+
329+
throw new IllegalArgumentException(String.format("Cannot obtain type for path %s on %s!", path, root));
310330
}
311331

312332
/**
@@ -404,6 +424,11 @@ public void addValue(Object target, Object value) {
404424
} else {
405425

406426
List<Object> list = parentPath.getValue(target);
427+
428+
if (listIndex > list.size()) {
429+
throw new PatchException(String.format(INVALID_COLLECTION_INDEX, listIndex, list.size()));
430+
}
431+
407432
list.add(listIndex >= 0 ? listIndex.intValue() : list.size(), value);
408433
}
409434
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@
2121
import java.util.ArrayList;
2222
import java.util.List;
2323

24+
import org.junit.Rule;
2425
import org.junit.Test;
26+
import org.junit.rules.ExpectedException;
2527

2628
import com.fasterxml.jackson.databind.JsonNode;
2729
import com.fasterxml.jackson.databind.ObjectMapper;
2830

2931
public class AddOperationUnitTests {
3032

33+
public @Rule ExpectedException exception = ExpectedException.none();
34+
3135
@Test
3236
public void addBooleanPropertyValue() throws Exception {
3337

@@ -112,4 +116,30 @@ public void initializesNullCollectionsOnAppend() {
112116
assertThat(todo.getUninitialized(), is(notNullValue()));
113117
assertThat(todo.getUninitialized(), hasItem("Text"));
114118
}
119+
120+
@Test // DATAREST-1273
121+
public void addsItemToTheEndOfACollectionViaIndex() {
122+
123+
List<Todo> todos = new ArrayList<Todo>();
124+
todos.add(new Todo(1L, "A", false));
125+
126+
Todo todo = new Todo(2L, "B", true);
127+
AddOperation.of("/1", todo).perform(todos, Todo.class);
128+
129+
assertThat(todos.get(1), is(todo));
130+
}
131+
132+
@Test // DATAREST-1273
133+
public void rejectsAdditionBeyondEndOfList() {
134+
135+
List<Todo> todos = new ArrayList<Todo>();
136+
todos.add(new Todo(1L, "A", false));
137+
138+
exception.expect(PatchException.class);
139+
exception.expectMessage("index");
140+
exception.expectMessage("1");
141+
exception.expectMessage("2");
142+
143+
AddOperation.of("/2", new Todo(2L, "B", true)).perform(todos, Todo.class);
144+
}
115145
}

0 commit comments

Comments
 (0)