From 91c7d1b1991366d3753ec6c5c9c38a74fdecbe4e 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 c9d3717ef..6f0d07f77 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 3ca61f327..26706fe1a 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 {} }