From ffed3c6d7e5463d34706166f7a90c362931371b1 Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Sat, 9 May 2015 16:43:34 +0200
Subject: [PATCH] DATAREST-537 - Fixed removing array elements in JSON Patch
 operations.

Before replacing the remove operation with a custom one that nulls out the object, we check whether the JSON Pointer points to an array element. If that's the case we skip the replacement and proceed.
---
 .../rest/webmvc/config/JsonPatchHandler.java  | 29 +++++++++++++++++--
 .../config/JsonPatchHandlerUnitTests.java     | 26 +++++++++++++++--
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java
index 176ff8665..9182abeae 100644
--- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java
+++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 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.
@@ -115,7 +115,8 @@ class JsonPatchHandler {
 
 				// Replace remove operation with replace operation and a value of null.
 				JsonPointer path = (JsonPointer) ReflectionUtils.getField(PATH_FIELD, operation);
-				patchedNode = new ReplaceOperation(path, NullNode.getInstance()).apply(patchedNode);
+				patchedNode = isCollectionElementReference(path) ? operation.apply(patchedNode) : new ReplaceOperation(path,
+						NullNode.getInstance()).apply(patchedNode);
 
 			} else {
 				patchedNode = operation.apply(patchedNode);
@@ -132,4 +133,28 @@ class JsonPatchHandler {
 	<T> T applyPut(ObjectNode source, T existingObject) {
 		return reader.readPut(source, existingObject, mapper);
 	}
+
+	/**
+	 * Returns whether the trailing element of the given {@link JsonPointer} is a pointer into an array or collection.
+	 * 
+	 * @param pointer must not be {@literal null}.
+	 * @return
+	 */
+	private static boolean isCollectionElementReference(JsonPointer pointer) {
+
+		String[] segments = pointer.toString().split("/");
+
+		if (segments.length == 0) {
+			return false;
+		}
+
+		String trailing = segments[segments.length - 1];
+
+		try {
+			Integer.parseInt(trailing);
+			return true;
+		} catch (NumberFormatException o_O) {
+			return false;
+		}
+	}
 }
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java
index 0d5012f5e..b6ebf7f0d 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 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.
@@ -15,7 +15,7 @@
  */
 package org.springframework.data.rest.webmvc.config;
 
-import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.springframework.data.rest.webmvc.util.TestUtils.*;
 
@@ -96,4 +96,26 @@ public class JsonPatchHandlerUnitTests {
 		assertThat(result.lastname, is(nullValue()));
 		assertThat(result.address.zipCode, is("ZIP"));
 	}
+
+	/**
+	 * DATAREST-537
+	 */
+	@Test
+	public void removesArrayItemCorrectly() throws Exception {
+
+		User thomas = new User();
+		thomas.firstname = "Thomas";
+
+		User christoph = new User();
+		christoph.firstname = "Christoph";
+
+		this.user.colleagues = Arrays.asList(thomas, christoph);
+
+		String input = "[{ \"op\": \"remove\", \"path\": \"/colleagues/0\" }]";
+
+		handler.applyPatch(asStream(input), user);
+
+		assertThat(user.colleagues, hasSize(1));
+		assertThat(user.colleagues.get(0).firstname, is(christoph.firstname));
+	}
 }