From 663ae9b4bf94e0b6df2c4cdbf52866fba1c182c8 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 13 Jun 2017 17:08:52 +0200 Subject: [PATCH] DATACMNS-102 - Polishing. Changed RepositoryConfiguration to return Streamable over Iterable. A couple of code reorganizations, suppression of raw type. Turned abstract helper classes into interfaces where possible. A couple of typos in JavaDoc. Original pull request: #222. --- .../DefaultRepositoryConfiguration.java | 5 +- .../RepositoryBeanDefinitionBuilder.java | 18 ++--- .../config/RepositoryConfiguration.java | 5 +- .../RepositoryFragmentConfiguration.java | 6 +- .../core/support/MethodLookups.java | 33 ++++++--- .../core/support/RepositoryComposition.java | 70 +++++++++---------- .../core/support/RepositoryFragment.java | 57 ++++++++------- .../RepositoryFragmentsFactoryBean.java | 28 ++++---- .../RepositoryCompositionUnitTests.java | 3 +- 9 files changed, 121 insertions(+), 104 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java b/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java index acbeb4b72..58b541002 100644 --- a/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java +++ b/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java @@ -23,6 +23,7 @@ import java.util.Optional; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.core.type.filter.TypeFilter; import org.springframework.data.repository.query.QueryLookupStrategy.Key; +import org.springframework.data.util.Streamable; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -64,7 +65,7 @@ public class DefaultRepositoryConfiguration getBasePackages() { + public Streamable getBasePackages() { return configurationSource.getBasePackages(); } @@ -161,7 +162,7 @@ public class DefaultRepositoryConfiguration getExcludeFilters() { + public Streamable getExcludeFilters() { return configurationSource.getExcludeFilters(); } } diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionBuilder.java b/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionBuilder.java index 8a5627b16..75f0d7ead 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionBuilder.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionBuilder.java @@ -16,11 +16,11 @@ package org.springframework.data.repository.config; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,6 +41,7 @@ import org.springframework.data.repository.NoRepositoryBean; import org.springframework.data.repository.core.support.RepositoryFragment; import org.springframework.data.repository.core.support.RepositoryFragmentsFactoryBean; import org.springframework.data.repository.query.ExtensionAwareEvaluationContextProvider; +import org.springframework.data.util.StreamUtils; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -143,6 +144,7 @@ class RepositoryBeanDefinitionBuilder { return builder; } + @SuppressWarnings("deprecation") private Optional registerCustomImplementation(RepositoryConfiguration configuration) { String beanName = configuration.getImplementationBeanName(); @@ -162,7 +164,6 @@ class RepositoryBeanDefinitionBuilder { } it.setSource(configuration.getSource()); - registry.registerBeanDefinition(beanName, it); return beanName; @@ -215,7 +216,6 @@ class RepositoryBeanDefinitionBuilder { fragmentConfiguration.getBeanDefinition().ifPresent(bd -> { bd.setSource(repositoryConfiguration.getSource()); - registry.registerBeanDefinition(beanName, bd); }); } @@ -245,6 +245,7 @@ class RepositoryBeanDefinitionBuilder { } private ClassMetadata getClassMetadata(String className) { + try { return metadataReaderFactory.getMetadataReader(className).getClassMetadata(); } catch (IOException e) { @@ -254,13 +255,8 @@ class RepositoryBeanDefinitionBuilder { private static List getExclusions(RepositoryConfiguration configuration) { - List exclusions = new ArrayList<>(); - - for (TypeFilter typeFilter : configuration.getExcludeFilters()) { - exclusions.add(typeFilter); - } - - exclusions.add(new AnnotationTypeFilter(NoRepositoryBean.class)); - return exclusions; + return Stream + .concat(configuration.getExcludeFilters().stream(), Stream.of(new AnnotationTypeFilter(NoRepositoryBean.class)))// + .collect(StreamUtils.toUnmodifiableList()); } } diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java index 8fc5c24f7..58162881b 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java @@ -19,6 +19,7 @@ import java.util.Optional; import org.springframework.core.type.filter.TypeFilter; import org.springframework.data.repository.query.QueryLookupStrategy; +import org.springframework.data.util.Streamable; /** * Configuration information for a single repository instance. @@ -33,7 +34,7 @@ public interface RepositoryConfiguration getBasePackages(); + Streamable getBasePackages(); /** * Returns the interface name of the repository. @@ -117,5 +118,5 @@ public interface RepositoryConfiguration getExcludeFilters(); + Streamable getExcludeFilters(); } diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryFragmentConfiguration.java b/src/main/java/org/springframework/data/repository/config/RepositoryFragmentConfiguration.java index 92a01aa54..6ae0aa6e8 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryFragmentConfiguration.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryFragmentConfiguration.java @@ -28,13 +28,13 @@ import org.springframework.util.StringUtils; * Fragment configuration consisting of an interface name and the implementation class name. * * @author Mark Paluch + * @author Oliver Gierke * @since 2.0 */ @Value public class RepositoryFragmentConfiguration { - String interfaceName; - String className; + String interfaceName, className; Optional beanDefinition; /** @@ -67,8 +67,8 @@ public class RepositoryFragmentConfiguration { Assert.notNull(beanDefinition, "Bean definition must not be null!"); this.interfaceName = interfaceName; - this.beanDefinition = Optional.of(beanDefinition); this.className = beanDefinition.getBeanClassName(); + this.beanDefinition = Optional.of(beanDefinition); } /** diff --git a/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java b/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java index 42c46fd72..9b013842a 100644 --- a/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java +++ b/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java @@ -46,10 +46,10 @@ import org.springframework.util.Assert; * Implementations of method lookup functions. * * @author Mark Paluch + * @author Oliver Gierke + * @since 2.0 */ -public abstract class MethodLookups { - - private MethodLookups() {} +interface MethodLookups { /** * Direct method lookup filtering on exact method name, parameter count and parameter types. @@ -102,23 +102,34 @@ public abstract class MethodLookups { * * @author Mark Paluch */ - private static class RepositoryAwareMethodLookup implements MethodLookup { + static class RepositoryAwareMethodLookup implements MethodLookup { - private static final TypeVariable>[] PARAMETERS = Repository.class.getTypeParameters(); + @SuppressWarnings("rawtypes") private static final TypeVariable>[] PARAMETERS = Repository.class + .getTypeParameters(); private static final String DOMAIN_TYPE_NAME = PARAMETERS[0].getName(); private static final String ID_TYPE_NAME = PARAMETERS[1].getName(); - private final ResolvableType entityType; - private final ResolvableType idType; + private final ResolvableType entityType, idType; private final Class repositoryInterface; + /** + * Creates a new {@link RepositoryAwareMethodLookup} for the given {@link RepositoryMetadata}. + * + * @param repositoryMetadata must not be {@literal null}. + */ public RepositoryAwareMethodLookup(RepositoryMetadata repositoryMetadata) { + Assert.notNull(repositoryMetadata, "Repository metadata must not be null!"); + this.entityType = ResolvableType.forClass(repositoryMetadata.getDomainType()); this.idType = ResolvableType.forClass(repositoryMetadata.getIdType()); this.repositoryInterface = repositoryMetadata.getRepositoryInterface(); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.MethodLookup#getLookups() + */ @Override public List getLookups() { @@ -214,15 +225,20 @@ public abstract class MethodLookups { * * @author Mark Paluch */ - private static class ReactiveTypeInteropMethodLookup extends RepositoryAwareMethodLookup { + static class ReactiveTypeInteropMethodLookup extends RepositoryAwareMethodLookup { private final RepositoryMetadata repositoryMetadata; public ReactiveTypeInteropMethodLookup(RepositoryMetadata repositoryMetadata) { + super(repositoryMetadata); this.repositoryMetadata = repositoryMetadata; } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.MethodLookups.RepositoryAwareMethodLookup#getLookups() + */ @Override public List getLookups() { @@ -408,5 +424,4 @@ public abstract class MethodLookups { } } } - } diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryComposition.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryComposition.java index f5e97e3e5..5f97ce333 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryComposition.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryComposition.java @@ -17,6 +17,7 @@ package org.springframework.data.repository.core.support; import lombok.AccessLevel; import lombok.EqualsAndHashCode; +import lombok.Getter; import lombok.RequiredArgsConstructor; import java.lang.reflect.Method; @@ -66,14 +67,13 @@ import org.springframework.util.ReflectionUtils; public class RepositoryComposition { private static final BiFunction PASSTHRU_ARG_CONVERTER = (methodParameter, o) -> o; - private static final RepositoryComposition EMPTY = new RepositoryComposition(RepositoryFragments.empty(), MethodLookups.direct(), PASSTHRU_ARG_CONVERTER); private final Map> methodCache = new ConcurrentReferenceHashMap<>(); - private final RepositoryFragments fragments; - private final MethodLookup methodLookup; - private final BiFunction argumentConverter; + private final @Getter RepositoryFragments fragments; + private final @Getter MethodLookup methodLookup; + private final @Getter BiFunction argumentConverter; /** * Create an empty {@link RepositoryComposition}. @@ -86,7 +86,7 @@ public class RepositoryComposition { /** * Create a {@link RepositoryComposition} for just a single {@code implementation} with {@link MethodLookups#direct()) - * method lokup. + * method lookup. * * @param implementation must not be {@literal null}. * @return the {@link RepositoryComposition} for a single {@code implementation}. @@ -98,7 +98,7 @@ public class RepositoryComposition { /** * Create a {@link RepositoryComposition} from {@link RepositoryFragment fragments} with - * {@link MethodLookups#direct()) method lokup. + * {@link MethodLookups#direct()) method lookup. * * @param fragments must not be {@literal null}. * @return the {@link RepositoryComposition} from {@link RepositoryFragment fragments}. @@ -109,7 +109,7 @@ public class RepositoryComposition { /** * Create a {@link RepositoryComposition} from {@link RepositoryFragment fragments} with - * {@link MethodLookups#direct()) method lokup. + * {@link MethodLookups#direct()) method lookup. * * @param fragments must not be {@literal null}. * @return the {@link RepositoryComposition} from {@link RepositoryFragment fragments}. @@ -121,7 +121,7 @@ public class RepositoryComposition { /** * Create a {@link RepositoryComposition} from {@link RepositoryFragments} and {@link RepositoryMetadata} with - * {@link MethodLookups#direct()) method lokup. + * {@link MethodLookups#direct()) method lookup. * * @param fragments must not be {@literal null}. * @return the {@link RepositoryComposition} from {@link RepositoryFragments fragments}. @@ -173,18 +173,6 @@ public class RepositoryComposition { return new RepositoryComposition(fragments, methodLookup, argumentConverter); } - public MethodLookup getMethodLookup() { - return methodLookup; - } - - public RepositoryFragments getFragments() { - return fragments; - } - - public BiFunction getArgumentConverter() { - return argumentConverter; - } - /** * Return {@literal true} if this {@link RepositoryComposition} contains no {@link RepositoryFragment fragments}. * @@ -341,6 +329,10 @@ public class RepositoryComposition { return fragments.isEmpty(); } + /* + * (non-Javadoc) + * @see java.lang.Iterable#iterator() + */ @Override public Iterator> iterator() { return fragments.iterator(); @@ -349,6 +341,7 @@ public class RepositoryComposition { /** * @return {@link Stream} of {@link RepositoryFragment fragments}. */ + @Override public Stream> stream() { return fragments.stream(); } @@ -360,23 +353,6 @@ public class RepositoryComposition { return stream().flatMap(RepositoryFragment::methods); } - static Optional findMethod(InvokedMethod invokedMethod, MethodLookup lookup, - Supplier> methodStreamSupplier) { - - for (MethodPredicate methodPredicate : lookup.getLookups()) { - - Optional resolvedMethod = methodStreamSupplier.get() - .filter(it -> methodPredicate.test(invokedMethod, it)) // - .findFirst(); - - if (resolvedMethod.isPresent()) { - return resolvedMethod; - } - } - - return Optional.empty(); - } - /** * Invoke {@link Method} by resolving the * @@ -401,7 +377,25 @@ public class RepositoryComposition { return method.invoke(target, args); } - /* (non-Javadoc) + private static Optional findMethod(InvokedMethod invokedMethod, MethodLookup lookup, + Supplier> methodStreamSupplier) { + + for (MethodPredicate methodPredicate : lookup.getLookups()) { + + Optional resolvedMethod = methodStreamSupplier.get() + .filter(it -> methodPredicate.test(invokedMethod, it)) // + .findFirst(); + + if (resolvedMethod.isPresent()) { + return resolvedMethod; + } + } + + return Optional.empty(); + } + + /* + * (non-Javadoc) * @see java.lang.Object#toString() */ @Override diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFragment.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFragment.java index c6ffb5bba..b17e99e56 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFragment.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFragment.java @@ -47,7 +47,7 @@ import org.springframework.util.ReflectionUtils; * @since 2.0 * @see RepositoryComposition */ -public abstract class RepositoryFragment { +public interface RepositoryFragment { /** * Create an implemented {@link RepositoryFragment} backed by the {@code implementation} object. @@ -56,7 +56,7 @@ public abstract class RepositoryFragment { * @return */ public static RepositoryFragment implemented(T implementation) { - return new ImplementedRepositoryFragment<>(Optional.empty(), implementation); + return new ImplementedRepositoryFragment(Optional.empty(), implementation); } /** @@ -87,25 +87,32 @@ public abstract class RepositoryFragment { * @param method must not be {@literal null}. * @return {@literal true} if the method was found or {@literal false} otherwise */ - public boolean hasMethod(Method method) { + default boolean hasMethod(Method method) { Assert.notNull(method, "Method must not be null!"); return ReflectionUtils.findMethod(getSignatureContributor(), method.getName(), method.getParameterTypes()) != null; } - /** - * @return the class/interface providing signatures for this {@link RepositoryFragment}. - */ - protected abstract Class getSignatureContributor(); - /** * @return the optional implementation. Only available for implemented fragments. Structural fragments return always * {@link Optional#empty()}. */ - public Optional getImplementation() { + default Optional getImplementation() { return Optional.empty(); } + /** + * @return a {@link Stream} of methods exposed by this {@link RepositoryFragment}. + */ + default Stream methods() { + return Arrays.stream(getSignatureContributor().getMethods()); + } + + /** + * @return the class/interface providing signatures for this {@link RepositoryFragment}. + */ + Class getSignatureContributor(); + /** * Implement a structural {@link RepositoryFragment} given its {@code implementation} object. Returns an implemented * {@link RepositoryFragment}. @@ -113,22 +120,16 @@ public abstract class RepositoryFragment { * @param implementation must not be {@literal null}. * @return a new implemented {@link RepositoryFragment} for {@code implementation}. */ - public abstract RepositoryFragment withImplementation(T implementation); - - /** - * @return a {@link Stream} of methods exposed by this {@link RepositoryFragment}. - */ - public Stream methods() { - return Arrays.stream(getSignatureContributor().getMethods()); - } + RepositoryFragment withImplementation(T implementation); @RequiredArgsConstructor @EqualsAndHashCode(callSuper = false) - static class StructuralRepositoryFragment extends RepositoryFragment { + static class StructuralRepositoryFragment implements RepositoryFragment { private final @NonNull Class interfaceOrImplementation; - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryFragment#getSignatureContributor() */ @Override @@ -136,7 +137,8 @@ public abstract class RepositoryFragment { return interfaceOrImplementation; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryFragment#withImplementation(java.lang.Object) */ @Override @@ -149,19 +151,19 @@ public abstract class RepositoryFragment { */ @Override public String toString() { - return String.format("StructuralRepositoryFragment %s", ClassUtils.getShortName(interfaceOrImplementation)); } } @RequiredArgsConstructor @EqualsAndHashCode(callSuper = false) - static class ImplementedRepositoryFragment extends RepositoryFragment { + static class ImplementedRepositoryFragment implements RepositoryFragment { private final @NonNull Optional> interfaceClass; private final @NonNull T implementation; - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryFragment#getSignatureContributor() */ @SuppressWarnings({ "rawtypes", "unchecked" }) @@ -169,7 +171,8 @@ public abstract class RepositoryFragment { return interfaceClass.orElse((Class) implementation.getClass()); } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryFragment#getImplementation() */ @Override @@ -177,7 +180,8 @@ public abstract class RepositoryFragment { return Optional.of(implementation); } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryFragment#withImplementation(java.lang.Object) */ @Override @@ -185,7 +189,8 @@ public abstract class RepositoryFragment { return new ImplementedRepositoryFragment<>(interfaceClass, implementation); } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see java.lang.Object#toString() */ @Override diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFragmentsFactoryBean.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFragmentsFactoryBean.java index d53ae3369..dfaf9dd49 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFragmentsFactoryBean.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFragmentsFactoryBean.java @@ -53,20 +53,32 @@ public class RepositoryFragmentsFactoryBean this.fragmentBeanNames = fragmentBeanNames; } - /* (non-Javadoc) + /* + * (non-Javadoc) + * @see org.springframework.beans.factory.BeanFactoryAware#setBeanFactory(org.springframework.beans.factory.BeanFactory) + */ + @Override + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + this.beanFactory = beanFactory; + } + + /* + * (non-Javadoc) * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet() */ @Override @SuppressWarnings({ "unchecked", "rawtypes" }) public void afterPropertiesSet() { - List> fragments = (List) fragmentBeanNames.stream() - .map(it -> beanFactory.getBean(it, RepositoryFragment.class)).collect(Collectors.toList()); + List> fragments = (List) fragmentBeanNames.stream() // + .map(it -> beanFactory.getBean(it, RepositoryFragment.class)) // + .collect(Collectors.toList()); this.repositoryFragments = RepositoryFragments.from(fragments); } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.beans.factory.FactoryBean#getObject() */ @Override @@ -81,12 +93,4 @@ public class RepositoryFragmentsFactoryBean public Class getObjectType() { return RepositoryComposition.class; } - - /* (non-Javadoc) - * @see org.springframework.beans.factory.BeanFactoryAware#setBeanFactory(org.springframework.beans.factory.BeanFactory) - */ - @Override - public void setBeanFactory(BeanFactory beanFactory) throws BeansException { - this.beanFactory = beanFactory; - } } diff --git a/src/test/java/org/springframework/data/repository/core/support/RepositoryCompositionUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/RepositoryCompositionUnitTests.java index 337866d87..aca75526c 100644 --- a/src/test/java/org/springframework/data/repository/core/support/RepositoryCompositionUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/RepositoryCompositionUnitTests.java @@ -39,17 +39,18 @@ import org.springframework.util.ReflectionUtils; * Unit tests for {@link RepositoryComposition}. * * @author Mark Paluch + * @author Oliver Gierke */ @RunWith(MockitoJUnitRunner.class) public class RepositoryCompositionUnitTests { @Mock QueryByExampleExecutor queryByExampleExecutor; - @Mock PersonRepository backingRepo; RepositoryComposition repositoryComposition; @Before + @SuppressWarnings("rawtypes") public void before() { RepositoryInformation repositoryInformation = new DefaultRepositoryInformation(