DATAREST-335 - ReflectionRepositoryInvoker correctly handles overridden delete(…) methods.

If a delete(…) method was overridden using the concrete id type, the check on invocation of course has to accommodate the concrete id type and not serializable only.

We previously erroneously attempted a to-entity-conversion by invoking findOne(…) and then handed that result to the delete(…) method which caused an IllegalArgumentException as the argument wasn't matching the parameter type. We now explicitly check for the concrete id type in the first place and even bypass any kind of parameter conversion.

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvoker.java

@@ -203,8 +203,11 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker {
 	public void invokeDelete(Serializable id) {
 
 		Method method = methods.getDeleteMethod();
+		Class<?> parameterType = method.getParameterTypes()[0];
 
-		if (method.getParameterTypes()[0].equals(Serializable.class)) {
+		if (parameterType.equals(information.getIdType())) {
+			invoke(method, id);
+		} else if (parameterType.equals(Serializable.class)) {
 			invoke(method, convertId(id));
 		} else {
 			invoke(method, invokeFindOne(id));

spring-data-rest-core/src/test/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvokerIntegrationTests.java

@@ -17,6 +17,7 @@ package org.springframework.data.rest.core.invoke;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 import java.lang.reflect.Method;
 import java.util.Collections;
@@ -25,13 +26,21 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.persistence.EntityManager;
+
+import org.bson.types.ObjectId;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Matchers;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.mongodb.MongoDbFactory;
+import org.springframework.data.mongodb.core.MongoTemplate;
+import org.springframework.data.mongodb.repository.support.MongoRepositoryFactoryBean;
+import org.springframework.data.repository.CrudRepository;
 import org.springframework.data.repository.core.RepositoryInformation;
 import org.springframework.data.repository.support.Repositories;
 import org.springframework.data.rest.core.AbstractIntegrationTests;
@@ -52,6 +61,7 @@ public class ReflectionRepositoryInvokerIntegrationTests extends AbstractIntegra
 	@Autowired ConversionService conversionService;
 	@Autowired PersonRepository repository;
 	@Autowired OrderRepository orderRepository;
+	@Autowired EntityManager em;
 
 	RepositoryInformation information;
 	RepositoryInvoker invoker;
@@ -139,4 +149,35 @@ public class ReflectionRepositoryInvokerIntegrationTests extends AbstractIntegra
 				conversionService);
 		invoker.invokeQueryMethod(method, parameters, null, null);
 	}
+
+	/**
+	 * @see DATAREST-335
+	 */
+	@Test
+	public void invokesOverriddenDeleteMethodCorrectly() {
+
+		MyRepo repository = mock(MyRepo.class);
+
+		MongoRepositoryFactoryBean<MyRepo, Object, ObjectId> factory = new MongoRepositoryFactoryBean<MyRepo, Object, ObjectId>();
+		factory.setMongoOperations(new MongoTemplate(mock(MongoDbFactory.class)));
+		factory.setRepositoryInterface(MyRepo.class);
+		factory.setLazyInit(true);
+		factory.afterPropertiesSet();
+
+		ObjectId id = new ObjectId();
+
+		ReflectionRepositoryInvoker invoker = new ReflectionRepositoryInvoker(repository,
+				factory.getRepositoryInformation(), conversionService);
+		invoker.invokeDelete(id);
+
+		verify((CustomRepo) repository, times(1)).delete(id);
+		verify(repository, times(0)).findOne(Matchers.any(ObjectId.class));
+
+	}
+
+	interface MyRepo extends CustomRepo, CrudRepository<Object, ObjectId> {}
+
+	interface CustomRepo {
+		void delete(ObjectId id);
+	}
 }