From eec63cb11d7647f07caaaa2509f037cca5986e67 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 7 Mar 2017 17:06:58 +0100 Subject: [PATCH] DATACMNS-764 - Disambiguate custom repository implementation if necessary When multiple repository implementations are found based on the class name, the one with a bean name matching the interfaces bean name + implementation postfix is picked. Includes support for CDI. RepositoryBeanNameGenerator now no longer implements BeanNameGenerator since while it produces names it does not behave like the interface suggests, i.e. it can work without a BeanFactory in the first place. It now uses constructor injection and is package private. Original pull request: #201. --- src/main/asciidoc/repositories.adoc | 31 +++++ .../repository/cdi/CdiRepositoryBean.java | 26 +++- ...notationRepositoryConfigurationSource.java | 3 +- ...ustomRepositoryImplementationDetector.java | 52 ++++++-- .../DefaultRepositoryConfiguration.java | 6 +- .../RepositoryBeanDefinitionBuilder.java | 3 +- .../config/RepositoryBeanNameGenerator.java | 47 ++++--- .../RepositoryConfigurationDelegate.java | 13 +- .../config/RepositoryConfigurationSource.java | 10 ++ .../RepositoryConfigurationSourceSupport.java | 29 +++-- .../data/repository/config/SelectionSet.java | 95 ++++++++++++++ .../XmlRepositoryConfigurationSource.java | 5 +- .../cdi/CdiRepositoryBeanUnitTests.java | 63 ++++++++-- ...sitoryImplementationDetectorUnitTests.java | 116 ++++++++++++++++++ ...faultRepositoryConfigurationUnitTests.java | 27 +++- .../RepositoryBeanNameGeneratorUnitTests.java | 13 +- .../config/SelectionSetUnitTests.java | 62 ++++++++++ 17 files changed, 526 insertions(+), 75 deletions(-) create mode 100644 src/main/java/org/springframework/data/repository/config/SelectionSet.java create mode 100644 src/test/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetectorUnitTests.java create mode 100644 src/test/java/org/springframework/data/repository/config/SelectionSetUnitTests.java diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index edf10b3eb..950d88cf9 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -649,6 +649,37 @@ If you use namespace configuration, the repository infrastructure tries to autod The first configuration example will try to look up a class `com.acme.repository.UserRepositoryImpl` to act as custom repository implementation, whereas the second example will try to lookup `com.acme.repository.UserRepositoryFooBar`. +===== Resolution of ambiguity + +If multiple implementations with matching class names get found in different packages Spring Data uses the bean names to identify the correct one to use. + +Given the following two custom implementations for the `UserRepository` introduced above the first implementation will get picked. Its bean name is `userRepositoryImpl` matches that of the repository interface (`userRepository`) plus the postfix `Impl`. + +.Resolution of amibiguous implementations +==== +[source, java] +---- +package com.acme.impl.one; + +class UserRepositoryImpl implements UserRepositoryCustom { + + // Your custom implementation +} +---- +[source, java] +---- +package com.acme.impl.two; + +@Component("specialCustomImpl") +class UserRepositoryImpl implements UserRepositoryCustom { + + // Your custom implementation +} +---- +==== + +If you annotate the `UserRepository` interface with `Component("specialCustom")' the bean name plus `Impl` matches that of the second Repository and it will be picked instead of the first one. + ===== Manual wiring The approach just shown works well if your custom implementation uses annotation-based configuration and autowiring only, as it will be treated as any other Spring bean. If your custom implementation bean needs special wiring, you simply declare the bean and name it after the conventions just described. The infrastructure will then refer to the manually defined bean definition by name instead of creating one itself. diff --git a/src/main/java/org/springframework/data/repository/cdi/CdiRepositoryBean.java b/src/main/java/org/springframework/data/repository/cdi/CdiRepositoryBean.java index df71b23cb..2aaafef30 100644 --- a/src/main/java/org/springframework/data/repository/cdi/CdiRepositoryBean.java +++ b/src/main/java/org/springframework/data/repository/cdi/CdiRepositoryBean.java @@ -38,9 +38,12 @@ import javax.enterprise.inject.spi.PassivationCapable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.context.annotation.AnnotationBeanNameGenerator; import org.springframework.data.repository.config.CustomRepositoryImplementationDetector; import org.springframework.data.repository.config.DefaultRepositoryConfiguration; +import org.springframework.data.repository.config.RepositoryBeanNameGenerator; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -52,6 +55,7 @@ import org.springframework.util.StringUtils; * @author Oliver Gierke * @author Mark Paluch * @author Peter Rietzler + * @author Jens Schauder * @author Christoph Strobl */ public abstract class CdiRepositoryBean implements Bean, PassivationCapable { @@ -67,6 +71,10 @@ public abstract class CdiRepositoryBean implements Bean, PassivationCapabl private transient T repoInstance; + private final AnnotationBeanNameGenerator annotationBeanNameGenerator = new AnnotationBeanNameGenerator(); + private final RepositoryBeanNameGenerator beanNameGenerator = + new RepositoryBeanNameGenerator(getClass().getClassLoader()); + /** * Creates a new {@link CdiRepositoryBean}. * @@ -247,8 +255,13 @@ public abstract class CdiRepositoryBean implements Bean, PassivationCapabl CdiRepositoryConfiguration cdiRepositoryConfiguration, CustomRepositoryImplementationDetector detector) { String className = getCustomImplementationClassName(repositoryType, cdiRepositoryConfiguration); - Optional beanDefinition = detector.detectCustomImplementation(className, - Collections.singleton(repositoryType.getPackage().getName()), Collections.emptySet()); + Optional beanDefinition = detector.detectCustomImplementation( // + className, // + getCustomImplementationBeanName(repositoryType), // + Collections.singleton(repositoryType.getPackage().getName()), // + Collections.emptySet(), // + beanNameGenerator::generateBeanName // + ); return beanDefinition.map(it -> { @@ -261,6 +274,11 @@ public abstract class CdiRepositoryBean implements Bean, PassivationCapabl }); } + private String getCustomImplementationBeanName(Class repositoryType) { + return annotationBeanNameGenerator.generateBeanName(new AnnotatedGenericBeanDefinition(repositoryType), null) + + DEFAULT_CONFIGURATION.getRepositoryImplementationPostfix(); + } + private String getCustomImplementationClassName(Class repositoryType, CdiRepositoryConfiguration cdiRepositoryConfiguration) { @@ -356,8 +374,10 @@ public abstract class CdiRepositoryBean implements Bean, PassivationCapabl * @param creationalContext will never be {@literal null}. * @param repositoryType will never be {@literal null}. * @return + * @deprecated overide {@link #create(CreationalContext, Class, Object)} instead. */ - private T create(CreationalContext creationalContext, Class repositoryType) { + @Deprecated + protected T create(CreationalContext creationalContext, Class repositoryType) { Optional> customImplementationBean = getCustomImplementationBean(repositoryType, beanManager, qualifiers); Optional customImplementation = customImplementationBean diff --git a/src/main/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSource.java b/src/main/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSource.java index 6936b8b2d..60f3f6223 100644 --- a/src/main/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSource.java +++ b/src/main/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSource.java @@ -47,6 +47,7 @@ import org.springframework.util.StringUtils; * @author Oliver Gierke * @author Thomas Darimont * @author Peter Rietzler + * @author Jens Schauder */ public class AnnotationRepositoryConfigurationSource extends RepositoryConfigurationSourceSupport { @@ -77,7 +78,7 @@ public class AnnotationRepositoryConfigurationSource extends RepositoryConfigura public AnnotationRepositoryConfigurationSource(AnnotationMetadata metadata, Class annotation, ResourceLoader resourceLoader, Environment environment) { - super(environment); + super(environment, resourceLoader.getClassLoader()); Assert.notNull(metadata, "Metadata must not be null!"); Assert.notNull(annotation, "Annotation must not be null!"); diff --git a/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java b/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java index 4768d7b0a..df9925006 100644 --- a/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java +++ b/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java @@ -19,9 +19,13 @@ package org.springframework.data.repository.config; import lombok.NonNull; import lombok.RequiredArgsConstructor; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -37,11 +41,12 @@ import org.springframework.util.Assert; /** * Detects the custom implementation for a {@link org.springframework.data.repository.Repository} - * + * * @author Oliver Gierke * @author Mark Paluch * @author Christoph Strobl * @author Peter Rietzler + * @author Jens Schauder */ @RequiredArgsConstructor public class CustomRepositoryImplementationDetector { @@ -54,7 +59,7 @@ public class CustomRepositoryImplementationDetector { /** * Tries to detect a custom implementation for a repository bean by classpath scanning. - * + * * @param configuration the {@link RepositoryConfiguration} to consider. * @return the {@code AbstractBeanDefinition} of the custom implementation or {@literal null} if none found. */ @@ -62,24 +67,44 @@ public class CustomRepositoryImplementationDetector { // TODO 2.0: Extract into dedicated interface for custom implementation lookup configuration. - return detectCustomImplementation(configuration.getImplementationClassName(), // + return detectCustomImplementation( // + configuration.getImplementationClassName(), // + configuration.getImplementationBeanName(), // configuration.getBasePackages(), // - configuration.getExcludeFilters()); + configuration.getExcludeFilters(), // + bd -> configuration.getConfigurationSource().generateBeanName(bd)); } /** * Tries to detect a custom implementation for a repository bean by classpath scanning. - * + * * @param className must not be {@literal null}. + * @param beanName may be {@literal null} * @param basePackages must not be {@literal null}. + * @param excludeFilters must not be {@literal null}. + * @param beanNameGenerator must not be {@literal null}. * @return the {@code AbstractBeanDefinition} of the custom implementation or {@literal null} if none found. */ - public Optional detectCustomImplementation(String className, Iterable basePackages, - Iterable excludeFilters) { + public Optional detectCustomImplementation(String className, String beanName, + Iterable basePackages, Iterable excludeFilters, + Function beanNameGenerator) { Assert.notNull(className, "ClassName must not be null!"); Assert.notNull(basePackages, "BasePackages must not be null!"); + Set definitions = findCandidateBeanDefinitions(className, basePackages, excludeFilters); + + SelectionSet selection = new SelectionSet<>( // + definitions, // + c -> c.isEmpty() ? null : throwAmbiguousCustomImplementationException(c) // + ).filterIfNecessary(bd -> beanName != null && beanName.equals(beanNameGenerator.apply(bd))); + + return Optional.ofNullable((AbstractBeanDefinition) selection.uniqueResult()); + } + + Set findCandidateBeanDefinitions(String className, Iterable basePackages, + Iterable excludeFilters) { + // Build pattern to lookup implementation class Pattern pattern = Pattern.compile(".*\\." + className); @@ -101,12 +126,15 @@ public class CustomRepositoryImplementationDetector { definitions.addAll(provider.findCandidateComponents(basePackage)); } - if (definitions.isEmpty()) { - return Optional.empty(); - } + return definitions; + } - if (definitions.size() == 1) { - return Optional.of((AbstractBeanDefinition) definitions.iterator().next()); + private static AbstractBeanDefinition throwAmbiguousCustomImplementationException( + Collection definitions) { + + List implementationClassNames = new ArrayList(); + for (BeanDefinition bean : definitions) { + implementationClassNames.add(bean.getBeanClassName()); } throw new IllegalStateException( 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 c61ae5cee..acbeb4b72 100644 --- a/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java +++ b/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java @@ -28,8 +28,9 @@ import org.springframework.util.StringUtils; /** * Default implementation of {@link RepositoryConfiguration}. - * + * * @author Oliver Gierke + * @author Jens Schauder */ @RequiredArgsConstructor public class DefaultRepositoryConfiguration @@ -104,7 +105,8 @@ public class DefaultRepositoryConfiguration beanDefinition = implementationDetector.detectCustomImplementation( - configuration.getImplementationClassName(), configuration.getBasePackages(), configuration.getExcludeFilters()); + Optional beanDefinition = implementationDetector.detectCustomImplementation(configuration); return beanDefinition.map(it -> { diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryBeanNameGenerator.java b/src/main/java/org/springframework/data/repository/config/RepositoryBeanNameGenerator.java index 1020be8b8..ccfb265ba 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryBeanNameGenerator.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryBeanNameGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,56 +15,69 @@ */ package org.springframework.data.repository.config; -import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.context.annotation.AnnotationBeanNameGenerator; +import org.springframework.context.annotation.ScannedGenericBeanDefinition; import org.springframework.util.ClassUtils; /** * Special {@link BeanNameGenerator} to create bean names for Spring Data repositories. Will delegate to an * {@link AnnotationBeanNameGenerator} but let the delegate work with a customized {@link BeanDefinition} to make sure * the repository interface is inspected and not the actual bean definition class. - * + * * @author Oliver Gierke + * @author Jens Schauder */ -public class RepositoryBeanNameGenerator implements BeanNameGenerator, BeanClassLoaderAware { +public class RepositoryBeanNameGenerator { private static final BeanNameGenerator DELEGATE = new AnnotationBeanNameGenerator(); - private ClassLoader beanClassLoader; + private final ClassLoader beanClassLoader; - /* - * (non-Javadoc) - * @see org.springframework.beans.factory.BeanClassLoaderAware#setBeanClassLoader(java.lang.ClassLoader) - */ - public void setBeanClassLoader(ClassLoader classLoader) { - this.beanClassLoader = classLoader; + public RepositoryBeanNameGenerator(ClassLoader beanClassLoader) { + this.beanClassLoader = beanClassLoader; } - /* - * (non-Javadoc) - * @see org.springframework.beans.factory.support.BeanNameGenerator#generateBeanName(org.springframework.beans.factory.config.BeanDefinition, org.springframework.beans.factory.support.BeanDefinitionRegistry) + /** + * Generate a bean name for the given bean definition. + * + * @param definition the bean definition to generate a name for + * @return the generated bean name + * @since 2.0 */ - public String generateBeanName(BeanDefinition definition, BeanDefinitionRegistry registry) { + public String generateBeanName(BeanDefinition definition) { AnnotatedBeanDefinition beanDefinition = new AnnotatedGenericBeanDefinition(getRepositoryInterfaceFrom(definition)); - return DELEGATE.generateBeanName(beanDefinition, registry); + return DELEGATE.generateBeanName(beanDefinition, null); } /** * Returns the type configured for the {@code repositoryInterface} property of the given bean definition. Uses a * potential {@link Class} being configured as is or tries to load a class with the given value's {@link #toString()} * representation. - * + * * @param beanDefinition * @return */ private Class getRepositoryInterfaceFrom(BeanDefinition beanDefinition) { + if (beanDefinition instanceof ScannedGenericBeanDefinition) { + try { + return ((ScannedGenericBeanDefinition) beanDefinition).resolveBeanClass(beanClassLoader); + } catch (ClassNotFoundException e) { + throw new IllegalStateException("Could not resolve bean class.", e); + } + } else { + return getRepositoryInterfaceFromFactory(beanDefinition); + } + } + + private Class getRepositoryInterfaceFromFactory(BeanDefinition beanDefinition) { + Object value = beanDefinition.getConstructorArgumentValues().getArgumentValue(0, Class.class).getValue(); if (value instanceof Class) { diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java index fb0b3b067..5070e823f 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,7 +24,6 @@ import org.springframework.beans.factory.parsing.BeanComponentDefinition; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.core.env.Environment; import org.springframework.core.env.EnvironmentCapable; import org.springframework.core.env.StandardEnvironment; @@ -38,8 +37,9 @@ import org.springframework.util.Assert; * providing a configuration format specific {@link RepositoryConfigurationSource} (currently either XML or annotations * are supported). The actual registration can then be triggered for different {@link RepositoryConfigurationExtension} * s. - * + * * @author Oliver Gierke + * @author Jens Schauder */ public class RepositoryConfigurationDelegate { @@ -53,7 +53,6 @@ public class RepositoryConfigurationDelegate { private final RepositoryConfigurationSource configurationSource; private final ResourceLoader resourceLoader; private final Environment environment; - private final BeanNameGenerator beanNameGenerator; private final boolean isXml; private final boolean inMultiStoreMode; @@ -75,10 +74,6 @@ public class RepositoryConfigurationDelegate { "Configuration source must either be an Xml- or an AnnotationBasedConfigurationSource!"); Assert.notNull(resourceLoader, "ResourceLoader must not be null!"); - RepositoryBeanNameGenerator generator = new RepositoryBeanNameGenerator(); - generator.setBeanClassLoader(resourceLoader.getClassLoader()); - - this.beanNameGenerator = generator; this.configurationSource = configurationSource; this.resourceLoader = resourceLoader; this.environment = defaultEnvironment(environment, resourceLoader); @@ -133,7 +128,7 @@ public class RepositoryConfigurationDelegate { } AbstractBeanDefinition beanDefinition = definitionBuilder.getBeanDefinition(); - String beanName = beanNameGenerator.generateBeanName(beanDefinition, registry); + String beanName = configurationSource.generateBeanName(beanDefinition); if (LOGGER.isDebugEnabled()) { LOGGER.debug(REPOSITORY_REGISTRATION, extension.getModuleName(), beanName, diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSource.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSource.java index ca549c6ba..76af7c005 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSource.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSource.java @@ -29,6 +29,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy; * @author Oliver Gierke * @author Thomas Darimont * @author Peter Rietzler + * @author Jens Schauder */ public interface RepositoryConfigurationSource { @@ -115,4 +116,13 @@ public interface RepositoryConfigurationSource { * @return must not be {@literal null}. */ Iterable getExcludeFilters(); + + /** + * Returns a name for the beanDefinition. + * + * @param beanDefinition Must not be {@literal null}. + * @return + * @since 2.0 + */ + String generateBeanName(BeanDefinition beanDefinition); } diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSourceSupport.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSourceSupport.java index 716a100f6..b96857367 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSourceSupport.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSourceSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,26 +28,32 @@ import org.springframework.util.Assert; /** * Base class to implement {@link RepositoryConfigurationSource}s. - * + * * @author Oliver Gierke * @author Thomas Darimont * @author Peter Rietzler + * @author Jens Schauder */ public abstract class RepositoryConfigurationSourceSupport implements RepositoryConfigurationSource { protected static final String DEFAULT_REPOSITORY_IMPL_POSTFIX = "Impl"; private final Environment environment; + private final RepositoryBeanNameGenerator beanNameGenerator; /** * Creates a new {@link RepositoryConfigurationSourceSupport} with the given environment. - * + * * @param environment must not be {@literal null}. + * @param classLoader must not be {@literal null}. */ - public RepositoryConfigurationSourceSupport(Environment environment) { + public RepositoryConfigurationSourceSupport(Environment environment, ClassLoader classLoader) { Assert.notNull(environment, "Environment must not be null!"); + Assert.notNull(classLoader, "ClassLoader must not be null!"); + this.environment = environment; + this.beanNameGenerator = new RepositoryBeanNameGenerator(classLoader); } /* @@ -78,7 +84,7 @@ public abstract class RepositoryConfigurationSourceSupport implements Repository /** * Return the {@link TypeFilter}s to define which types to exclude when scanning for repositories. Default * implementation returns an empty collection. - * + * * @return must not be {@literal null}. */ public Iterable getExcludeFilters() { @@ -88,7 +94,7 @@ public abstract class RepositoryConfigurationSourceSupport implements Repository /** * Return the {@link TypeFilter}s to define which types to include when scanning for repositories. Default * implementation returns an empty collection. - * + * * @return must not be {@literal null}. */ protected Iterable getIncludeFilters() { @@ -98,10 +104,19 @@ public abstract class RepositoryConfigurationSourceSupport implements Repository /** * Returns whether we should consider nested repositories, i.e. repository interface definitions nested in other * classes. - * + * * @return {@literal true} if the container should look for nested repository interface definitions. */ public boolean shouldConsiderNestedRepositories() { return false; } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.config.RepositoryConfigurationSource#getBeanNameGenerator() + */ + @Override + public String generateBeanName(BeanDefinition beanDefinition) { + return beanNameGenerator.generateBeanName(beanDefinition); + } } diff --git a/src/main/java/org/springframework/data/repository/config/SelectionSet.java b/src/main/java/org/springframework/data/repository/config/SelectionSet.java new file mode 100644 index 000000000..08faaf4ca --- /dev/null +++ b/src/main/java/org/springframework/data/repository/config/SelectionSet.java @@ -0,0 +1,95 @@ +/* + * Copyright 2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.repository.config; + +import java.util.Collection; +import java.util.List; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +/** + * Allows filtering of a collection in order to select a unique element. Once a unique element is found all further + * filters are ignored. + * + * @author Jens Schauder + */ +class SelectionSet { + + private final Collection collection; + private final Function, T> fallback; + + /** + * creates a {@link SelectionSet} with a default fallback of {@literal null}, when no element is found and an + * {@link IllegalStateException} when no element is found. + */ + SelectionSet(Collection collection) { + this(collection, defaultFallback()); + } + + /** + * @param collection The collection from which a unique element will get picked. + * @param fallback Will get called once {@literal #uniqueResult} gets called if no unique result can be determined. + */ + SelectionSet(Collection collection, Function, T> fallback) { + + this.collection = collection; + this.fallback = fallback; + } + + /** + * If this SelectionSet contains exactly one element it gets returned. If no unique result can + * be identified the fallback function passed in at the constructor gets called and its return value becomes + * the return value of this method. + * + * @return a unique result, or the result of the callback provided in the constructor. + */ + T uniqueResult() { + T uniqueResult = findUniqueResult(); + + return uniqueResult != null ? uniqueResult : fallback.apply(collection); + } + + /** + * Filters the collection with the predicate if there are still more then one elements in the collection. + * + * @param predicate To be used for filtering. + */ + SelectionSet filterIfNecessary(Predicate predicate) { + + if (findUniqueResult() != null) { + return this; + } + + List fillteredList = collection.stream().filter(predicate).collect(Collectors.toList()); + return new SelectionSet(fillteredList, fallback); + } + + private static Function, S> defaultFallback() { + + return c -> { + if (c.isEmpty()) { + return null; + } else { + throw new IllegalStateException("More then one element in collection."); + } + }; + } + + private T findUniqueResult() { + return collection.size() == 1 ? collection.iterator().next() : null; + } +} diff --git a/src/main/java/org/springframework/data/repository/config/XmlRepositoryConfigurationSource.java b/src/main/java/org/springframework/data/repository/config/XmlRepositoryConfigurationSource.java index 4d8ccd010..031c98320 100644 --- a/src/main/java/org/springframework/data/repository/config/XmlRepositoryConfigurationSource.java +++ b/src/main/java/org/springframework/data/repository/config/XmlRepositoryConfigurationSource.java @@ -37,6 +37,7 @@ import org.w3c.dom.Element; * @author Thomas Darimont * @author Christoph Strobl * @author Peter Rietzler + * @author Jens Schauder */ public class XmlRepositoryConfigurationSource extends RepositoryConfigurationSourceSupport { @@ -63,7 +64,7 @@ public class XmlRepositoryConfigurationSource extends RepositoryConfigurationSou */ public XmlRepositoryConfigurationSource(Element element, ParserContext context, Environment environment) { - super(environment); + super(environment, context.getReaderContext().getBeanClassLoader()); Assert.notNull(element, "Element must not be null!"); Assert.notNull(context, "Context must not be null!"); @@ -128,7 +129,7 @@ public class XmlRepositoryConfigurationSource extends RepositoryConfigurationSou return excludeFilters; } - /* + /* * (non-Javadoc) * @see org.springframework.data.repository.config.RepositoryConfigurationSourceSupport#getIncludeFilters() */ diff --git a/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java b/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java index f760566e4..e9f31090e 100755 --- a/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java +++ b/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java @@ -16,6 +16,7 @@ package org.springframework.data.repository.cdi; import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; import java.io.Serializable; import java.lang.annotation.Annotation; @@ -23,17 +24,21 @@ import java.lang.reflect.Type; import java.util.Collections; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import javax.enterprise.context.ApplicationScoped; import javax.enterprise.context.spi.CreationalContext; import javax.enterprise.inject.spi.Bean; import javax.enterprise.inject.spi.BeanManager; +import javax.inject.Named; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.data.repository.Repository; +import org.springframework.data.repository.config.CustomRepositoryImplementationDetector; /** * Unit tests for {@link CdiRepositoryBean}. @@ -69,8 +74,8 @@ public class CdiRepositoryBeanUnitTests { @Test public void returnsBasicMetadata() { - DummyCdiRepositoryBean bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, - SampleRepository.class, beanManager); + DummyCdiRepositoryBean bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class, + beanManager); assertThat(bean.getBeanClass()).isEqualTo(SampleRepository.class); assertThat(bean.getName()).isEqualTo(SampleRepository.class.getName()); @@ -80,8 +85,8 @@ public class CdiRepositoryBeanUnitTests { @Test public void returnsAllImplementedTypes() { - DummyCdiRepositoryBean bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, - SampleRepository.class, beanManager); + DummyCdiRepositoryBean bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class, + beanManager); Set types = bean.getTypes(); assertThat(types).containsExactlyInAnyOrder(SampleRepository.class, Repository.class); @@ -91,8 +96,8 @@ public class CdiRepositoryBeanUnitTests { @SuppressWarnings("unchecked") public void detectsStereotypes() { - DummyCdiRepositoryBean bean = new DummyCdiRepositoryBean<>( - NO_ANNOTATIONS, StereotypedSampleRepository.class, beanManager); + DummyCdiRepositoryBean bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, + StereotypedSampleRepository.class, beanManager); assertThat(bean.getStereotypes()).containsExactly(StereotypeAnnotation.class); } @@ -101,22 +106,57 @@ public class CdiRepositoryBeanUnitTests { @SuppressWarnings("rawtypes") public void scopeDefaultsToApplicationScoped() { - Bean bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class, - beanManager); + Bean bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class, beanManager); assertThat(bean.getScope()).isEqualTo(ApplicationScoped.class); } @Test // DATACMNS-322 public void createsPassivationId() { - CdiRepositoryBean bean = new DummyCdiRepositoryBean<>(SINGLE_ANNOTATION, - SampleRepository.class, beanManager); + CdiRepositoryBean bean = new DummyCdiRepositoryBean<>( // + SINGLE_ANNOTATION, // + SampleRepository.class, // + beanManager // + ); assertThat(bean.getId()).isEqualTo(PASSIVATION_ID); } + @Test // DATACMNS-764 + public void passesCorrectBeanNameToTheImplementationDetector() { + + CustomRepositoryImplementationDetector detector = mock(CustomRepositoryImplementationDetector.class); + + CdiRepositoryBean bean = new CdiRepositoryBean( // + SINGLE_ANNOTATION, // + SampleRepository.class, // + beanManager, // + Optional.of(detector) // + ) { + + @Override + protected SampleRepository create( // + CreationalContext creationalContext, // + Class repositoryType, // + Optional customImplementation // + ) { + return null; + } + }; + + bean.create(mock(CreationalContext.class), SampleRepository.class); + + verify(detector).detectCustomImplementation( // + eq("CdiRepositoryBeanUnitTests.SampleRepositoryImpl"), // + eq("namedRepositoryImpl"), // + anySet(), // + anySet(), // + Mockito.any(Function.class) // + ); + } + static class DummyCdiRepositoryBean extends CdiRepositoryBean { - public DummyCdiRepositoryBean(Set qualifiers, Class repositoryType, BeanManager beanManager) { + DummyCdiRepositoryBean(Set qualifiers, Class repositoryType, BeanManager beanManager) { super(qualifiers, repositoryType, beanManager); } @@ -127,6 +167,7 @@ public class CdiRepositoryBeanUnitTests { } } + @Named("namedRepository") static interface SampleRepository extends Repository { } diff --git a/src/test/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetectorUnitTests.java b/src/test/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetectorUnitTests.java new file mode 100644 index 000000000..e30a06dab --- /dev/null +++ b/src/test/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetectorUnitTests.java @@ -0,0 +1,116 @@ +/* + * Copyright 2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.repository.config; + +import static java.util.Arrays.*; +import static java.util.Collections.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import java.util.HashSet; +import java.util.Optional; +import java.util.function.Function; + +import org.assertj.core.api.Assertions; +import org.junit.Test; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.core.env.Environment; +import org.springframework.core.io.DefaultResourceLoader; +import org.springframework.core.io.ResourceLoader; +import org.springframework.core.type.classreading.MetadataReaderFactory; +import org.springframework.core.type.classreading.SimpleMetadataReaderFactory; +import org.springframework.core.type.filter.TypeFilter; +import org.springframework.mock.env.MockEnvironment; + +/** + * tests {@link CustomRepositoryImplementationDetector} + * + * @author Jens Schauder + */ +public class CustomRepositoryImplementationDetectorUnitTests { + + MetadataReaderFactory metadataFactory = new SimpleMetadataReaderFactory(); + Environment environment = new MockEnvironment(); + ResourceLoader resourceLoader = new DefaultResourceLoader(); + Function nameGenerator = mock(Function.class); + + CustomRepositoryImplementationDetector detector = spy( + new CustomRepositoryImplementationDetector(metadataFactory, environment, resourceLoader)); + + { + doReturn("notTheBeanYouAreLookingFor").when(nameGenerator).apply(any(BeanDefinition.class)); + } + + @Test // DATACMNS-764 + public void returnsNullWhenNoImplementationFound() { + + doReturn(emptySet()).when(detector).findCandidateBeanDefinitions(anyString(), anyListOf(String.class), + anyListOf(TypeFilter.class)); + + Optional beanDefinition = detector.detectCustomImplementation("className", "beanName", emptyList(), + emptyList(), nameGenerator); + + assertThat(beanDefinition).isEmpty(); + } + + @Test // DATACMNS-764 + public void returnsBeanDefinitionWhenOneImplementationIsFound() { + + AbstractBeanDefinition expectedBeanDefinition = mock(AbstractBeanDefinition.class); + + doReturn(new HashSet<>(singleton(expectedBeanDefinition))).when(detector).findCandidateBeanDefinitions(anyString(), + anyListOf(String.class), anyListOf(TypeFilter.class)); + + Optional beanDefinition = detector.detectCustomImplementation("className", "beanName", emptyList(), + emptyList(), nameGenerator); + + assertThat(beanDefinition).contains(expectedBeanDefinition); + } + + @Test // DATACMNS-764 + public void returnsBeanDefinitionMatchingByNameWhenMultipleImplementationAreFound() { + + AbstractBeanDefinition wrongBeanDefinition = mock(AbstractBeanDefinition.class); + AbstractBeanDefinition expectedBeanDefinition = mock(AbstractBeanDefinition.class); + + doReturn("expected").when(nameGenerator).apply(expectedBeanDefinition); + + doReturn(new HashSet<>(asList(wrongBeanDefinition, expectedBeanDefinition))).when(detector) + .findCandidateBeanDefinitions(anyString(), anyListOf(String.class), anyListOf(TypeFilter.class)); + + Optional beanDefinition = detector.detectCustomImplementation("className", "expected", emptyList(), + emptyList(), nameGenerator); + + assertThat( beanDefinition).contains(expectedBeanDefinition); + } + + @Test(expected = IllegalStateException.class) // DATACMNS-764 + public void throwsExceptionWhenMultipleImplementationAreFound() { + + AbstractBeanDefinition wrongBeanDefinition = mock(AbstractBeanDefinition.class); + AbstractBeanDefinition expectedBeanDefinition = mock(AbstractBeanDefinition.class); + + doReturn("expected").when(nameGenerator).apply(any(BeanDefinition.class)); + + doReturn(new HashSet<>(asList(wrongBeanDefinition, expectedBeanDefinition))).when(detector) + .findCandidateBeanDefinitions(anyString(), anyListOf(String.class), anyListOf(TypeFilter.class)); + + Optional beanDefinition = detector.detectCustomImplementation("className", "expected", emptyList(), + emptyList(), nameGenerator); + } +} diff --git a/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java b/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java index 383c6eab7..42253d1ea 100755 --- a/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java +++ b/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java @@ -23,11 +23,15 @@ import lombok.Value; import java.util.Optional; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.ConstructorArgumentValues; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.data.repository.query.QueryLookupStrategy.Key; @@ -35,6 +39,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy.Key; * Unit tests for {@link DefaultRepositoryConfiguration}. * * @author Oliver Gierke + * @author Jens Schauder */ @RunWith(MockitoJUnitRunner.class) public class DefaultRepositoryConfigurationUnitTests { @@ -44,6 +49,14 @@ public class DefaultRepositoryConfigurationUnitTests { BeanDefinition definition = new RootBeanDefinition("com.acme.MyRepository"); RepositoryConfigurationExtension extension = new SimplerRepositoryConfigurationExtension("factory", "module"); + @Before + public void before() { + + RepositoryBeanNameGenerator generator = new RepositoryBeanNameGenerator(getClass().getClassLoader()); + Answer answer = invocation -> generator.generateBeanName((BeanDefinition) invocation.getArguments()[0]); + when(source.generateBeanName(Mockito.any(BeanDefinition.class))).then(answer); + } + @Test public void supportsBasicConfiguration() { @@ -72,7 +85,8 @@ public class DefaultRepositoryConfigurationUnitTests { private DefaultRepositoryConfiguration getConfiguration( RepositoryConfigurationSource source) { - return new DefaultRepositoryConfiguration<>(source, definition, extension); + RootBeanDefinition beanDefinition = createBeanDefinition(); + return new DefaultRepositoryConfiguration<>(source, beanDefinition, extension); } @Value @@ -80,4 +94,15 @@ public class DefaultRepositoryConfigurationUnitTests { private static class SimplerRepositoryConfigurationExtension extends RepositoryConfigurationExtensionSupport { String repositoryFactoryBeanClassName, modulePrefix; } + + private static RootBeanDefinition createBeanDefinition() { + + RootBeanDefinition beanDefinition = new RootBeanDefinition("com.acme.MyRepository"); + + ConstructorArgumentValues constructorArgumentValues = new ConstructorArgumentValues(); + constructorArgumentValues.addGenericArgumentValue(MyRepository.class); + beanDefinition.setConstructorArgumentValues(constructorArgumentValues); + + return beanDefinition; + } } diff --git a/src/test/java/org/springframework/data/repository/config/RepositoryBeanNameGeneratorUnitTests.java b/src/test/java/org/springframework/data/repository/config/RepositoryBeanNameGeneratorUnitTests.java index 091deaa37..b00bcfead 100755 --- a/src/test/java/org/springframework/data/repository/config/RepositoryBeanNameGeneratorUnitTests.java +++ b/src/test/java/org/springframework/data/repository/config/RepositoryBeanNameGeneratorUnitTests.java @@ -32,30 +32,27 @@ import org.springframework.data.repository.core.support.RepositoryFactoryBeanSup * Unit tests for {@link RepositoryBeanNameGenerator}. * * @author Oliver Gierke + * @author Jens Schauder */ public class RepositoryBeanNameGeneratorUnitTests { - BeanNameGenerator generator; + RepositoryBeanNameGenerator generator; BeanDefinitionRegistry registry; @Before public void setUp() { - RepositoryBeanNameGenerator generator = new RepositoryBeanNameGenerator(); - generator.setBeanClassLoader(Thread.currentThread().getContextClassLoader()); - - this.generator = generator; - this.registry = new DefaultListableBeanFactory(); + this.generator = new RepositoryBeanNameGenerator(Thread.currentThread().getContextClassLoader()); } @Test public void usesPlainClassNameIfNoAnnotationPresent() { - assertThat(generator.generateBeanName(getBeanDefinitionFor(MyRepository.class), registry)).isEqualTo("myRepository"); + assertThat(generator.generateBeanName(getBeanDefinitionFor(MyRepository.class))).isEqualTo("myRepository"); } @Test public void usesAnnotationValueIfAnnotationPresent() { - assertThat(generator.generateBeanName(getBeanDefinitionFor(AnnotatedInterface.class), registry)).isEqualTo("specialName"); + assertThat(generator.generateBeanName(getBeanDefinitionFor(AnnotatedInterface.class))).isEqualTo("specialName"); } private BeanDefinition getBeanDefinitionFor(Class repositoryInterface) { diff --git a/src/test/java/org/springframework/data/repository/config/SelectionSetUnitTests.java b/src/test/java/org/springframework/data/repository/config/SelectionSetUnitTests.java new file mode 100644 index 000000000..fabd37589 --- /dev/null +++ b/src/test/java/org/springframework/data/repository/config/SelectionSetUnitTests.java @@ -0,0 +1,62 @@ +package org.springframework.data.repository.config; + +import static java.util.Arrays.*; +import static java.util.Collections.*; +import static org.junit.Assert.*; + +import org.junit.Test; + +/** + * Unit tests for {@link SelectionSet} + * + * @author Jens Schauder + */ +public class SelectionSetUnitTests { + + @Test // DATACMNS-764 + public void returnsUniqueResult() { + assertEquals("single value", new SelectionSet<>(singleton("single value")).uniqueResult()); + } + + @Test // DATACMNS-764 + public void emptyCollectionReturnsNull() { + assertNull(new SelectionSet(emptySet()).uniqueResult()); + } + + @Test(expected = IllegalStateException.class) // DATACMNS-764 + public void multipleElementsThrowException() { + new SelectionSet<>(asList("one", "two")).uniqueResult(); + } + + @Test(expected = NullPointerException.class) // DATACMNS-764 + public void throwsCustomExceptionWhenConfigured() { + + new SelectionSet<>(asList("one", "two"), c -> { + throw new NullPointerException(); + }).uniqueResult(); + } + + @Test // DATACMNS-764 + public void usesFallbackWhenConfigured() { + + String value = new SelectionSet<>(asList("one", "two"), c -> String.valueOf(c.size())).uniqueResult(); + + assertEquals("2", value); + } + + @Test // DATACMNS-764 + public void returnsUniqueResultAfterFilter() { + + SelectionSet selection = new SelectionSet<>(asList("one", "two", "three")).filterIfNecessary(s -> s.contains("w")); + + assertEquals("two", selection.uniqueResult()); + } + + @Test // DATACMNS-764 + public void ignoresFilterWhenResultIsAlreadyUnique() { + + SelectionSet selection = new SelectionSet<>(asList("one")).filterIfNecessary(s -> s.contains("w")); + + assertEquals("one", selection.uniqueResult()); + } +}