From 553eefa480752e6f08201979d3a372fa6c6b02fb Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 8 Mar 2018 11:35:50 +0100 Subject: [PATCH] =?UTF-8?q?DATACMNS-1277=20-=20Fix=20invocation=20of=20red?= =?UTF-8?q?eclared=20delete(=E2=80=A6)=20method=20in=20ReflectionRepositor?= =?UTF-8?q?yInvoker.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As a consequence of our renaming efforts in CRUD methods, we lost some generics lookup information to distinguish deleteById(ID) from delete(T) solely based on the method parameter type in case of an intermediate generic repository redeclaring the method as the bound is now expanded to Object and not Serializable, as it did in 1.x. We now rather check for the method name ending in …ById which is much simpler anyway. --- .../support/ReflectionRepositoryInvoker.java | 4 +- .../ReflectionRepositoryInvokerUnitTests.java | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java index 0a183cd5e..b8b9813f8 100644 --- a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java @@ -165,10 +165,8 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { Method method = methods.getDeleteMethod() .orElseThrow(() -> new IllegalStateException("Repository doesn't have a delete-method declared!")); - Class parameterType = method.getParameterTypes()[0]; - List> idTypes = Arrays.asList(idType, Object.class); - if (idTypes.contains(parameterType)) { + if (method.getName().endsWith("ById")) { invoke(method, convertId(id)); } else { invoke(method, this. invokeFindById(id).orElse(null)); diff --git a/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java b/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java index 15cdb4cee..73f91d7ed 100755 --- a/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java @@ -272,6 +272,28 @@ public class ReflectionRepositoryInvokerUnitTests { }); } + @Test // DATACMNS-1277 + public void invokesFindByIdBeforeDeletingOnOverride() { + + DeleteByEntityOverrideSubRepository mock = mock(DeleteByEntityOverrideSubRepository.class); + doReturn(Optional.of(new Domain())).when(mock).findById(any()); + + getInvokerFor(mock).invokeDeleteById(1L); + + verify(mock).findById(1L); + verify(mock).delete(any(Domain.class)); + } + + @Test // DATACMNS-1277 + public void invokesDeleteByIdOnOverride() { + + DeleteByIdOverrideSubRepository mock = mock(DeleteByIdOverrideSubRepository.class); + + getInvokerFor(mock).invokeDeleteById(1L); + + verify(mock).deleteById(1L); + } + private static RepositoryInvoker getInvokerFor(Object repository) { RepositoryMetadata metadata = new DefaultRepositoryMetadata(repository.getClass().getInterfaces()[0]); @@ -336,4 +358,19 @@ public class ReflectionRepositoryInvokerUnitTests { com.google.common.base.Optional findById(Long id); } + + // DATACMNS-1277 + interface DeleteByEntityOverrideRepository extends CrudRepository { + @Override + void delete(T entity); + } + + interface DeleteByEntityOverrideSubRepository extends DeleteByEntityOverrideRepository {} + + // DATACMNS-1277 + interface DeleteByIdOverrideRepository extends Repository { + void deleteById(ID entity); + } + + interface DeleteByIdOverrideSubRepository extends DeleteByIdOverrideRepository {} }