From 70da7b00f8731187f972543167e9c8e52201d10f Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 1 Jun 2011 00:09:19 +0100 Subject: [PATCH] DATACMNS-41 - Refactored DefaultRepositoryInformation to take custom implementation class into account as well. --- spring-data-commons-core/.project | 12 ++ .../support/DefaultRepositoryInformation.java | 145 ++++++++++-------- .../support/RepositoryFactorySupport.java | 18 ++- .../support/RepositoryInformation.java | 8 +- ...DefaultRepositoryInformationUnitTests.java | 63 ++++++-- .../DefaultRepositoryMetadataUnitTests.java | 2 - ...eryExecuterMethodInterceptorUnitTests.java | 2 +- .../RepositoryFactorySupportUnitTests.java | 20 ++- 8 files changed, 178 insertions(+), 92 deletions(-) diff --git a/spring-data-commons-core/.project b/spring-data-commons-core/.project index 7c80c8102..2a756252c 100644 --- a/spring-data-commons-core/.project +++ b/spring-data-commons-core/.project @@ -5,6 +5,11 @@ + + org.eclipse.wst.common.project.facet.core.builder + + + org.eclipse.jdt.core.javabuilder @@ -15,9 +20,16 @@ + + org.eclipse.wst.validation.validationbuilder + + + org.eclipse.jdt.core.javanature org.maven.ide.eclipse.maven2Nature + org.eclipse.wst.common.project.facet.core.nature + org.eclipse.wst.common.modulecore.ModuleCoreNature diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInformation.java b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInformation.java index 4087fc04f..fac6f7054 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInformation.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInformation.java @@ -33,7 +33,7 @@ import org.springframework.util.Assert; * * @author Oliver Gierke */ -public class DefaultRepositoryInformation implements RepositoryInformation { +class DefaultRepositoryInformation implements RepositoryInformation { @SuppressWarnings("rawtypes") private static final TypeVariable>[] PARAMETERS = Repository.class.getTypeParameters(); @@ -44,71 +44,76 @@ public class DefaultRepositoryInformation implements RepositoryInformation { private final RepositoryMetadata metadata; private final Class repositoryBaseClass; + private final Class customImplementationClass; /** * Creates a new {@link DefaultRepositoryMetadata} for the given repository interface and repository base class. * - * @param repositoryInterface + * @param metadata + * @param repositoryBaseClass + * @param customImplementationClass */ - public DefaultRepositoryInformation(RepositoryMetadata metadata, Class repositoryBaseClass) { + public DefaultRepositoryInformation(RepositoryMetadata metadata, Class repositoryBaseClass, Class customImplementationClass) { Assert.notNull(metadata); Assert.notNull(repositoryBaseClass); this.metadata = metadata; this.repositoryBaseClass = repositoryBaseClass; + this.customImplementationClass = customImplementationClass; } - /* (non-Javadoc) - * @see org.springframework.data.repository.support.RepositoryMetadata#getRepositoryInterface() - */ + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryMetadata#getRepositoryInterface() + */ public Class getRepositoryInterface() { return metadata.getRepositoryInterface(); } - /* (non-Javadoc) - * @see org.springframework.data.repository.support.RepositoryMetadata#getDomainClass() - */ + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryMetadata#getDomainClass() + */ public Class getDomainClass() { return metadata.getDomainClass(); } - /* (non-Javadoc) - * @see org.springframework.data.repository.support.RepositoryMetadata#getIdClass() - */ + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryMetadata#getIdClass() + */ public Class getIdClass() { return metadata.getIdClass(); } /* - * (non-Javadoc) - * - * @see org.springframework.data.repository.support.RepositoryMetadata# - * getRepositoryBaseClass() - */ + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInformation#getRepositoryBaseClass() + */ public Class getRepositoryBaseClass() { return this.repositoryBaseClass; } /* - * (non-Javadoc) - * - * @see org.springframework.data.repository.support.RepositoryMetadata# - * getBaseClassMethod(java.lang.reflect.Method) - */ - public Method getBaseClassMethod(Method method) { - - Assert.notNull(method); - - Method result = methodCache.get(method); - - if (null != result) { + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInformation#getTargetClassMethod(java.lang.reflect.Method) + */ + public Method getTargetClassMethod(Method method) { + + if (methodCache.containsKey(method)) { + return methodCache.get(method); + } + + Method result = getTargetClassMethod(method, customImplementationClass); + + if (!result.equals(method)) { + methodCache.put(method, result); return result; } - - result = getBaseClassMethodFor(method); + + result = getTargetClassMethod(method, repositoryBaseClass); methodCache.put(method, result); - return result; } @@ -118,28 +123,31 @@ public class DefaultRepositoryInformation implements RepositoryInformation { * @param method * @return */ - private boolean isBaseClassMethod(Method method) { + private boolean isTargetClassMethod(Method method, Class targetType) { Assert.notNull(method); + + if (targetType == null) { + return false; + } - if (method.getDeclaringClass().isAssignableFrom(repositoryBaseClass)) { + if (method.getDeclaringClass().isAssignableFrom(targetType)) { return true; } - return !method.equals(getBaseClassMethod(method)); + return !method.equals(getTargetClassMethod(method, targetType)); } + /* - * (non-Javadoc) - * - * @see org.springframework.data.repository.support.RepositoryMetadata# - * getFinderMethods() - */ + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInformation#getQueryMethods() + */ public Iterable getQueryMethods() { Set result = new HashSet(); - for (Method method : getRepositoryInterface().getDeclaredMethods()) { + for (Method method : getRepositoryInterface().getMethods()) { if (!isCustomMethod(method) && !isBaseClassMethod(method)) { result.add(method); } @@ -149,34 +157,39 @@ public class DefaultRepositoryInformation implements RepositoryInformation { } /* - * (non-Javadoc) - * - * @see - * org.springframework.data.repository.support.RepositoryMetadata#isCustomMethod - * (java.lang.reflect.Method) - */ + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInformation#isCustomMethod(java.lang.reflect.Method) + */ public boolean isCustomMethod(Method method) { - - Class declaringClass = method.getDeclaringClass(); - - boolean isQueryMethod = declaringClass.equals(getRepositoryInterface()); - boolean isRepositoryInterface = isGenericRepositoryInterface(declaringClass); - boolean isBaseClassMethod = isBaseClassMethod(method); - - return !(isRepositoryInterface || isBaseClassMethod || isQueryMethod); + return isTargetClassMethod(method, customImplementationClass); } - + /** - * Returns the given base class' method if the given method (declared in the repository interface) was also declared - * at the repository base class. Returns the given method if the given base class does not declare the method given. - * Takes generics into account. - * + * Returns whether the given method is a method covered by the base implementation. + * * @param method * @return */ - Method getBaseClassMethodFor(Method method) { + public boolean isBaseClassMethod(Method method) { + return isTargetClassMethod(method, repositoryBaseClass); + } - for (Method baseClassMethod : repositoryBaseClass.getMethods()) { + /** + * Returns the given target class' method if the given method (declared in the repository interface) was also declared + * at the target class. Returns the given method if the given base class does not declare the method given. + * Takes generics into account. + * + * @param method must not be {@literal null} + * @param baseClass + * @return + */ + Method getTargetClassMethod(Method method, Class baseClass) { + + if (baseClass == null) { + return method; + } + + for (Method baseClassMethod : baseClass.getMethods()) { // Wrong name if (!method.getName().equals(baseClassMethod.getName())) { @@ -200,11 +213,9 @@ public class DefaultRepositoryInformation implements RepositoryInformation { } /* - * (non-Javadoc) - * - * @see org.springframework.data.repository.support.RepositoryMetadata# - * hasCustomMethod() - */ + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInformation#hasCustomMethod() + */ public boolean hasCustomMethod() { Class repositoryInterface = getRepositoryInterface(); diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java index bbe9a514e..8e3757f02 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java @@ -125,7 +125,8 @@ public abstract class RepositoryFactorySupport { Object customImplementation) { RepositoryMetadata metadata = getRepositoryMetadata(repositoryInterface); - RepositoryInformation information = getRepositoryInformation(metadata); + Class customImplementationClass = null == customImplementation ? null : customImplementation.getClass(); + RepositoryInformation information = getRepositoryInformation(metadata, customImplementationClass); validate(information, customImplementation); @@ -162,10 +163,11 @@ public abstract class RepositoryFactorySupport { * Returns the {@link RepositoryInformation} for the given repository interface. * * @param repositoryInterface + * @param customImplementationClass * @return */ - private RepositoryInformation getRepositoryInformation(RepositoryMetadata metadata) { - return new DefaultRepositoryInformation(metadata, getRepositoryBaseClass(metadata)); + private RepositoryInformation getRepositoryInformation(RepositoryMetadata metadata, Class customImplementationClass) { + return new DefaultRepositoryInformation(metadata, getRepositoryBaseClass(metadata), customImplementationClass); } @@ -321,9 +323,9 @@ public abstract class RepositoryFactorySupport { Method method = invocation.getMethod(); if (isCustomMethodInvocation(invocation)) { - - makeAccessible(method); - return executeMethodOn(customImplementation, method, + Method actualMethod = repositoryInformation.getTargetClassMethod(method); + makeAccessible(actualMethod); + return executeMethodOn(customImplementation, actualMethod, invocation.getArguments()); } @@ -333,7 +335,7 @@ public abstract class RepositoryFactorySupport { // Lookup actual method as it might be redeclared in the interface // and we have to use the repository instance nevertheless - Method actualMethod = repositoryInformation.getBaseClassMethod(method); + Method actualMethod = repositoryInformation.getTargetClassMethod(method); return executeMethodOn(target, actualMethod, invocation.getArguments()); } @@ -387,6 +389,8 @@ public abstract class RepositoryFactorySupport { if (null == customImplementation) { return false; } + + return repositoryInformation.isCustomMethod(invocation.getMethod()); } diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryInformation.java b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryInformation.java index a7282c3bf..f7ce6ba78 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryInformation.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryInformation.java @@ -22,7 +22,7 @@ import java.lang.reflect.Method; * * @author Oliver Gierke */ -interface RepositoryInformation extends RepositoryMetadata { +public interface RepositoryInformation extends RepositoryMetadata { /** * Returns the base class to be used to create the proxy backing instance. @@ -62,14 +62,14 @@ interface RepositoryInformation extends RepositoryMetadata { /** - * Returns the base class method that is backing the given method. This can + * Returns the target class method that is backing the given method. This can * be necessary if a repository interface redeclares a method of the core * repository interface (e.g. for transaction behaviour customization). - * Returns the method itself if the base class does not implement the given + * Returns the method itself if the target class does not implement the given * method. * * @param method * @return */ - Method getBaseClassMethod(Method method); + Method getTargetClassMethod(Method method); } diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInformationUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInformationUnitTests.java index cf28d2e18..8acdfcf54 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInformationUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInformationUnitTests.java @@ -3,28 +3,36 @@ package org.springframework.data.repository.support; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import java.io.Serializable; import java.lang.reflect.Method; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.support.DefaultRepositoryMetadataUnitTests.DummyGenericRepositorySupport; /** * @author Oliver Gierke */ +@RunWith(MockitoJUnitRunner.class) public class DefaultRepositoryInformationUnitTests { @SuppressWarnings("rawtypes") static final Class REPOSITORY = DummyGenericRepositorySupport.class; + + @Mock + FooRepositoryCustom customImplementation; @Test public void discoversRepositoryBaseClassMethod() throws Exception { - Method method = FooDao.class.getMethod("findOne", Integer.class); - RepositoryMetadata metadata = new DefaultRepositoryMetadata(FooDao.class); - DefaultRepositoryInformation information = new DefaultRepositoryInformation(metadata, REPOSITORY); + Method method = FooRepository.class.getMethod("findOne", Integer.class); + RepositoryMetadata metadata = new DefaultRepositoryMetadata(FooRepository.class); + DefaultRepositoryInformation information = new DefaultRepositoryInformation(metadata, REPOSITORY, null); - Method reference = information.getBaseClassMethodFor(method); + Method reference = information.getTargetClassMethod(method); assertEquals(REPOSITORY, reference.getDeclaringClass()); assertThat(reference.getName(), is("findOne")); } @@ -32,15 +40,36 @@ public class DefaultRepositoryInformationUnitTests { @Test public void discoveresNonRepositoryBaseClassMethod() throws Exception { - Method method = FooDao.class.getMethod("findOne", Long.class); + Method method = FooRepository.class.getMethod("findOne", Long.class); - RepositoryMetadata metadata = new DefaultRepositoryMetadata(FooDao.class); - DefaultRepositoryInformation information = new DefaultRepositoryInformation(metadata, CrudRepository.class); + RepositoryMetadata metadata = new DefaultRepositoryMetadata(FooRepository.class); + DefaultRepositoryInformation information = new DefaultRepositoryInformation(metadata, CrudRepository.class, null); - assertThat(information.getBaseClassMethodFor(method), is(method)); + assertThat(information.getTargetClassMethod(method), is(method)); + } + + @Test + public void discoversCustomlyImplementedCrudMethod() throws SecurityException, NoSuchMethodException { + + RepositoryMetadata metadata = new DefaultRepositoryMetadata(FooRepository.class); + RepositoryInformation information = new DefaultRepositoryInformation(metadata, CrudRepository.class, customImplementation.getClass()); + + Method source = FooRepositoryCustom.class.getMethod("save", User.class); + Method expected = customImplementation.getClass().getMethod("save", User.class); + + assertThat(information.getTargetClassMethod(source), is(expected)); + } + + @Test + public void considersIntermediateMethodsAsFinderMethods() { + + RepositoryMetadata metadata = new DefaultRepositoryMetadata(ConcreteRepository.class); + RepositoryInformation information = new DefaultRepositoryInformation(metadata, CrudRepository.class, null); + + assertThat(information.hasCustomMethod(), is(false)); } - private static interface FooDao extends CrudRepository { + interface FooRepository extends CrudRepository, FooRepositoryCustom { // Redeclared method User findOne(Integer primaryKey); @@ -48,6 +77,11 @@ public class DefaultRepositoryInformationUnitTests { // Not a redeclared method User findOne(Long primaryKey); } + + interface FooRepositoryCustom { + + User save(User user); + } @SuppressWarnings("unused") private class User { @@ -59,4 +93,15 @@ public class DefaultRepositoryInformationUnitTests { return null; } } + + + interface BaseRepository extends CrudRepository { + + T findBySomething(String something); + } + + interface ConcreteRepository extends BaseRepository { + + User findBySomethingDifferent(String somethingDifferent); + } } diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryMetadataUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryMetadataUnitTests.java index 3bc5e10da..bd0b916bd 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryMetadataUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/DefaultRepositoryMetadataUnitTests.java @@ -15,7 +15,6 @@ */ package org.springframework.data.repository.support; -import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; import java.io.Serializable; @@ -24,7 +23,6 @@ import org.junit.Test; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.repository.CrudRepository; -import org.springframework.data.repository.RepositoryDefinition; import org.springframework.data.repository.util.ClassUtils; diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java index 49e15f73d..5609cf3f9 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java @@ -15,7 +15,7 @@ */ package org.springframework.data.repository.support; -import static org.mockito.Matchers.any; +import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import org.junit.Test; diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/RepositoryFactorySupportUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/RepositoryFactorySupportUnitTests.java index 0ae06cc5d..0328f0403 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/RepositoryFactorySupportUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/RepositoryFactorySupportUnitTests.java @@ -44,6 +44,8 @@ public class RepositoryFactorySupportUnitTests { @Mock CrudRepository backingRepo; + @Mock + ObjectRepositoryCustom customImplementation; @Mock MyQueryCreationListener listener; @@ -72,6 +74,16 @@ public class RepositoryFactorySupportUnitTests { verify(backingRepo, times(1)).save(any(Object.class)); } + + @Test + public void invokesCustomMethodIfItRedeclaresACRUDOne() { + + ObjectRepository repository = factory.getRepository(ObjectRepository.class, customImplementation); + repository.findOne(1); + + verify(customImplementation, times(1)).findOne(1); + verify(backingRepo, times(0)).findOne(1); + } class DummyRepositoryFactory extends RepositoryFactorySupport { @@ -114,15 +126,19 @@ public class RepositoryFactorySupportUnitTests { } } - interface ObjectRepository extends Repository { + interface ObjectRepository extends Repository, ObjectRepositoryCustom { Object findByClass(Class clazz); - Object findByFoo(); Object save(Object entity); } + + interface ObjectRepositoryCustom { + + Object findOne(Serializable id); + } interface PlainQueryCreationListener extends QueryCreationListener {