From 996221fc85c1d798434563d321d86882d700d3a2 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 27 Jun 2014 17:05:15 +0200 Subject: [PATCH] =?UTF-8?q?DATAREST-335=20-=20ReflectionRepositoryInvoker?= =?UTF-8?q?=20correctly=20handles=20overridden=20delete(=E2=80=A6)=20metho?= =?UTF-8?q?ds.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../invoke/ReflectionRepositoryInvoker.java | 5 ++- ...tionRepositoryInvokerIntegrationTests.java | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvoker.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvoker.java index 8c8f27377..06a164118 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvoker.java +++ b/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)); diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvokerIntegrationTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvokerIntegrationTests.java index 01fc959d3..178369db1 100644 --- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/invoke/ReflectionRepositoryInvokerIntegrationTests.java +++ b/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 factory = new MongoRepositoryFactoryBean(); + 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 {} + + interface CustomRepo { + void delete(ObjectId id); + } }