Skip to content

Commit 501ddda

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 5bc96aa commit 501ddda

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-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

4446
/**
@@ -210,6 +212,7 @@ private static String pathNodesToSpEL(String[] pathNodes) {
210212
static class TypedSpelPath extends SpelPath {
211213

212214
private static final String INVALID_PATH_REFERENCE = "Invalid path reference %s on type %s (from source %s)!";
215+
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!";
213216
private static final Map<CacheKey, TypedSpelPath> TYPED_PATHS = new ConcurrentReferenceHashMap<>(32);
214217
private static final EvaluationContext CONTEXT = SimpleEvaluationContext.forReadWriteDataBinding().build();
215218

@@ -286,7 +289,24 @@ public Class<?> getType(Object root) {
286289

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

289-
return expression.getValueType(CONTEXT, root);
292+
try {
293+
294+
return expression.getValueType(CONTEXT, root);
295+
296+
} catch (SpelEvaluationException o_O) {
297+
298+
if (!SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS.equals(o_O.getMessageCode())) {
299+
throw o_O;
300+
}
301+
302+
Object collectionOrArray = getParent().getValue(root);
303+
304+
if (Collection.class.isInstance(collectionOrArray)) {
305+
return CollectionUtils.findCommonElementType(Collection.class.cast(collectionOrArray));
306+
}
307+
}
308+
309+
throw new IllegalArgumentException(String.format("Cannot obtain type for path %s on %s!", path, root));
290310
}
291311

292312
/**
@@ -384,6 +404,11 @@ public void addValue(Object target, Object value) {
384404
} else {
385405

386406
List<Object> list = parentPath.getValue(target);
407+
408+
if (listIndex > list.size()) {
409+
throw new PatchException(String.format(INVALID_COLLECTION_INDEX, listIndex, list.size()));
410+
}
411+
387412
list.add(listIndex >= 0 ? listIndex.intValue() : list.size(), value);
388413
}
389414
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package org.springframework.data.rest.webmvc.json.patch;
1717

18+
import static org.assertj.core.api.Assertions.*;
1819
import static org.assertj.core.api.Assertions.assertThat;
1920
import static org.junit.Assert.*;
2021

@@ -111,4 +112,29 @@ public void initializesNullCollectionsOnAppend() {
111112

112113
assertThat(todo.getUninitialized()).containsExactly("Text");
113114
}
115+
116+
@Test // DATAREST-1273
117+
public void addsItemToTheEndOfACollectionViaIndex() {
118+
119+
List<Todo> todos = new ArrayList<Todo>();
120+
todos.add(new Todo(1L, "A", false));
121+
122+
Todo todo = new Todo(2L, "B", true);
123+
AddOperation.of("/1", todo).perform(todos, Todo.class);
124+
125+
assertThat(todos).element(1).isEqualTo(todo);
126+
}
127+
128+
@Test // DATAREST-1273
129+
public void rejectsAdditionBeyondEndOfList() {
130+
131+
List<Todo> todos = new ArrayList<Todo>();
132+
todos.add(new Todo(1L, "A", false));
133+
134+
assertThatExceptionOfType(PatchException.class) //
135+
.isThrownBy(() -> AddOperation.of("/2", new Todo(2L, "B", true)).perform(todos, Todo.class)) //
136+
.withMessageContaining("index") //
137+
.withMessageContaining("2") //
138+
.withMessageContaining("1");
139+
}
114140
}

0 commit comments

Comments
 (0)