From e4a84dfe5135d0d04c33a8c2823786d8b1a14fd1 Mon Sep 17 00:00:00 2001 From: John Blum Date: Fri, 16 Feb 2018 16:22:26 -0800 Subject: [PATCH] DATACMNS-1261 - Apply Java 8 Functional types and Lambdas to simplify parsing logic in Repository configuration. Additionally, fix up Exception error messages and edit Javadoc. --- .../data/config/ConfigurationUtils.java | 22 ++--- ...notationRepositoryConfigurationSource.java | 5 +- ...ositoryBeanDefinitionRegistrarSupport.java | 10 +-- .../config/RepositoryComponentProvider.java | 32 ++++---- .../RepositoryConfigurationDelegate.java | 35 ++++---- .../RepositoryConfigurationSourceSupport.java | 2 +- .../data/repository/query/Parameters.java | 14 ++-- .../data/repository/query/QueryMethod.java | 7 +- .../data/util/IterableUtils.java | 70 ++++++++++++++++ .../data/util/IterableUtilsUnitTests.java | 81 +++++++++++++++++++ 10 files changed, 213 insertions(+), 65 deletions(-) create mode 100644 src/main/java/org/springframework/data/util/IterableUtils.java create mode 100644 src/test/java/org/springframework/data/util/IterableUtilsUnitTests.java diff --git a/src/main/java/org/springframework/data/config/ConfigurationUtils.java b/src/main/java/org/springframework/data/config/ConfigurationUtils.java index 045289dc14..22218af863 100644 --- a/src/main/java/org/springframework/data/config/ConfigurationUtils.java +++ b/src/main/java/org/springframework/data/config/ConfigurationUtils.java @@ -37,15 +37,13 @@ public interface ConfigurationUtils { * @return * @throws IllegalArgumentException if no {@link ResourceLoader} can be obtained from the {@link XmlReaderContext}. */ - public static ResourceLoader getRequiredResourceLoader(XmlReaderContext context) { + static ResourceLoader getRequiredResourceLoader(XmlReaderContext context) { Assert.notNull(context, "XmlReaderContext must not be null!"); ResourceLoader resourceLoader = context.getResourceLoader(); - if (resourceLoader == null) { - throw new IllegalArgumentException("Could not obtain ResourceLoader from XmlReaderContext!"); - } + Assert.notNull(resourceLoader, "Could not obtain ResourceLoader from XmlReaderContext!"); return resourceLoader; } @@ -57,7 +55,7 @@ public static ResourceLoader getRequiredResourceLoader(XmlReaderContext context) * @return * @throws IllegalArgumentException if no {@link ClassLoader} can be obtained from the given {@link XmlReaderContext}. */ - public static ClassLoader getRequiredClassLoader(XmlReaderContext context) { + static ClassLoader getRequiredClassLoader(XmlReaderContext context) { return getRequiredClassLoader(getRequiredResourceLoader(context)); } @@ -68,15 +66,13 @@ public static ClassLoader getRequiredClassLoader(XmlReaderContext context) { * @return * @throws IllegalArgumentException if the given {@link ResourceLoader} does not expose a {@link ClassLoader}. */ - public static ClassLoader getRequiredClassLoader(ResourceLoader resourceLoader) { + static ClassLoader getRequiredClassLoader(ResourceLoader resourceLoader) { Assert.notNull(resourceLoader, "ResourceLoader must not be null!"); ClassLoader classLoader = resourceLoader.getClassLoader(); - if (classLoader == null) { - throw new IllegalArgumentException("Could not obtain ClassLoader from ResourceLoader!"); - } + Assert.notNull(classLoader, "Could not obtain ClassLoader from ResourceLoader!"); return classLoader; } @@ -88,16 +84,14 @@ public static ClassLoader getRequiredClassLoader(ResourceLoader resourceLoader) * @return * @throws IllegalArgumentException if the given {@link BeanDefinition} does not contain a bean class name. */ - public static String getRequiredBeanClassName(BeanDefinition beanDefinition) { + static String getRequiredBeanClassName(BeanDefinition beanDefinition) { Assert.notNull(beanDefinition, "BeanDefinition must not be null!"); String result = beanDefinition.getBeanClassName(); - if (result == null) { - throw new IllegalArgumentException( - String.format("Could not obtain required bean class name from BeanDefinition!", beanDefinition)); - } + Assert.notNull(result, String.format("Could not obtain required bean class name from BeanDefinition [%s]!", + beanDefinition)); return result; } 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 5eedf8d7bc..53d15de917 100644 --- a/src/main/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSource.java +++ b/src/main/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSource.java @@ -93,9 +93,8 @@ public AnnotationRepositoryConfigurationSource(AnnotationMetadata metadata, Clas Map annotationAttributes = metadata.getAnnotationAttributes(annotation.getName()); - if (annotationAttributes == null) { - throw new IllegalStateException(String.format("Unable to obtain annotation attributes for %s!", annotation)); - } + Assert.state(annotationAttributes != null, + String.format("Unable to obtain annotation attributes for %s!", annotation)); this.attributes = new AnnotationAttributes(annotationAttributes); this.enableAnnotationMetadata = new StandardAnnotationMetadata(annotation); diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java b/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java index f5e4ab14f6..5b025db416 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java @@ -76,13 +76,13 @@ public void registerBeanDefinitions(AnnotationMetadata annotationMetadata, BeanD AnnotationRepositoryConfigurationSource configurationSource = new AnnotationRepositoryConfigurationSource( annotationMetadata, getAnnotation(), resourceLoader, environment, registry); - RepositoryConfigurationExtension extension = getExtension(); - RepositoryConfigurationUtils.exposeRegistration(extension, registry, configurationSource); + RepositoryConfigurationExtension configurationExtension = getExtension(); + RepositoryConfigurationUtils.exposeRegistration(configurationExtension, registry, configurationSource); - RepositoryConfigurationDelegate delegate = new RepositoryConfigurationDelegate(configurationSource, resourceLoader, - environment); + RepositoryConfigurationDelegate delegate = + new RepositoryConfigurationDelegate(configurationSource, resourceLoader, environment); - delegate.registerRepositoriesIn(registry, extension); + delegate.registerRepositoriesIn(registry, configurationExtension); } /** diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryComponentProvider.java b/src/main/java/org/springframework/data/repository/config/RepositoryComponentProvider.java index b9dc29371b..7470d84e4c 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryComponentProvider.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryComponentProvider.java @@ -36,6 +36,7 @@ import org.springframework.data.repository.Repository; import org.springframework.data.repository.RepositoryDefinition; import org.springframework.data.repository.util.ClassUtils; +import org.springframework.data.util.IterableUtils; import org.springframework.util.Assert; /** @@ -61,15 +62,12 @@ public RepositoryComponentProvider(Iterable includeFilters super(false); - Assert.notNull(includeFilters, "Include filters must not be null!"); Assert.notNull(registry, "BeanDefinitionRegistry must not be null!"); this.registry = registry; - if (includeFilters.iterator().hasNext()) { - for (TypeFilter filter : includeFilters) { - addIncludeFilter(filter); - } + if (IterableUtils.isNotEmpty(includeFilters)) { + includeFilters.forEach(this::addIncludeFilter); } else { super.addIncludeFilter(new InterfaceTypeFilter(Repository.class)); super.addIncludeFilter(new AnnotationTypeFilter(RepositoryDefinition.class, true, true)); @@ -79,9 +77,11 @@ public RepositoryComponentProvider(Iterable includeFilters } /** - * Custom extension of {@link #addIncludeFilter(TypeFilter)} to extend the added {@link TypeFilter}. For the - * {@link TypeFilter} handed we'll have two filters registered: one additionally enforcing the - * {@link RepositoryDefinition} annotation, the other one forcing the extension of {@link Repository}. + * Custom extension of {@link ClassPathScanningCandidateComponentProvider#addIncludeFilter(TypeFilter)} + * to extend the added {@link TypeFilter}. + * + * For the {@link TypeFilter} handed we will have two filters registered: one additionally enforcing the + * {@link RepositoryDefinition} annotation and the other forcing the extension of {@link Repository}. * * @see ClassPathScanningCandidateComponentProvider#addIncludeFilter(TypeFilter) */ @@ -123,11 +123,8 @@ public Set findCandidateComponents(String basePackage) { Set candidates = super.findCandidateComponents(basePackage); - for (BeanDefinition candidate : candidates) { - if (candidate instanceof AnnotatedBeanDefinition) { - AnnotationConfigUtils.processCommonDefinitionAnnotations((AnnotatedBeanDefinition) candidate); - } - } + candidates.stream().filter(bd -> bd instanceof AnnotatedBeanDefinition).forEach(bd -> + AnnotationConfigUtils.processCommonDefinitionAnnotations((AnnotatedBeanDefinition) bd)); return candidates; } @@ -153,7 +150,8 @@ public boolean isConsiderNestedRepositoryInterfaces() { * Controls whether nested inner-class {@link Repository} interface definitions should be considered for automatic * discovery. This defaults to {@literal false}. * - * @param considerNestedRepositoryInterfaces + * @param considerNestedRepositoryInterfaces boolean value indicating whether nested inner-class {@link Repository} + * interface definitions should be considered for automatic discovery. */ public void setConsiderNestedRepositoryInterfaces(boolean considerNestedRepositoryInterfaces) { this.considerNestedRepositoryInterfaces = considerNestedRepositoryInterfaces; @@ -170,9 +168,9 @@ private static class InterfaceTypeFilter extends AssignableTypeFilter { /** * Creates a new {@link InterfaceTypeFilter}. * - * @param targetType + * @param targetType {@link Class type} of the interface. */ - public InterfaceTypeFilter(Class targetType) { + private InterfaceTypeFilter(Class targetType) { super(targetType); } @@ -202,7 +200,7 @@ private static class AllTypeFilter implements TypeFilter { * * @param delegates must not be {@literal null}. */ - public AllTypeFilter(List delegates) { + private AllTypeFilter(List delegates) { Assert.notNull(delegates, "TypeFilter deleages must not be null!"); this.delegates = delegates; 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 7384000b61..78f770384c 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java @@ -31,6 +31,7 @@ import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.ResourceLoader; import org.springframework.core.io.support.SpringFactoriesLoader; +import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.RepositoryFactorySupport; import org.springframework.util.Assert; @@ -69,12 +70,13 @@ public class RepositoryConfigurationDelegate { public RepositoryConfigurationDelegate(RepositoryConfigurationSource configurationSource, ResourceLoader resourceLoader, Environment environment) { - this.isXml = configurationSource instanceof XmlRepositoryConfigurationSource; + Assert.notNull(resourceLoader, "ResourceLoader must not be null!"); + boolean isAnnotation = configurationSource instanceof AnnotationRepositoryConfigurationSource; + this.isXml = configurationSource instanceof XmlRepositoryConfigurationSource; - Assert.isTrue(isXml || isAnnotation, - "Configuration source must either be an Xml- or an AnnotationBasedConfigurationSource!"); - Assert.notNull(resourceLoader, "ResourceLoader must not be null!"); + Assert.isTrue(this.isXml || isAnnotation, + "Configuration source must either be an XML-based or an Annotation-based RepositoryConfigurationSource!"); this.configurationSource = configurationSource; this.resourceLoader = resourceLoader; @@ -83,12 +85,14 @@ public RepositoryConfigurationDelegate(RepositoryConfigurationSource configurati } /** - * Defaults the environment in case the given one is null. Used as fallback, in case the legacy constructor was - * invoked. + * Defaults the {@link Environment} in case the given one is {@literal null}. + * + * Used as fallback in case the legacy constructor was invoked. * * @param environment can be {@literal null}. * @param resourceLoader can be {@literal null}. - * @return + * @return the given {@link Environment} if not {@literal null} or an {@link Environment} obtained + * from the {@link ResourceLoader} if {@link ResourceLoader} is {@link EnvironmentCapable}. */ private static Environment defaultEnvironment(@Nullable Environment environment, @Nullable ResourceLoader resourceLoader) { @@ -104,21 +108,22 @@ private static Environment defaultEnvironment(@Nullable Environment environment, /** * Registers the found repositories in the given {@link BeanDefinitionRegistry}. * - * @param registry - * @param extension - * @return {@link BeanComponentDefinition}s for all repository bean definitions found. + * @param registry {@link BeanDefinitionRegistry} used to register {@link Repository Repositories}. + * @param extension {@link RepositoryConfigurationExtension} containing store specific configuration meta-data. + * @return {@link BeanComponentDefinition}s for all {@link Repository} bean definitions found. */ public List registerRepositoriesIn(BeanDefinitionRegistry registry, RepositoryConfigurationExtension extension) { extension.registerBeansForRoot(registry, configurationSource); - RepositoryBeanDefinitionBuilder builder = new RepositoryBeanDefinitionBuilder(registry, extension, resourceLoader, - environment); + RepositoryBeanDefinitionBuilder builder = + new RepositoryBeanDefinitionBuilder(registry, extension, resourceLoader, environment); + List definitions = new ArrayList<>(); - for (RepositoryConfiguration configuration : extension - .getRepositoryConfigurations(configurationSource, resourceLoader, inMultiStoreMode)) { + for (RepositoryConfiguration configuration + : extension.getRepositoryConfigurations(configurationSource, resourceLoader, inMultiStoreMode)) { BeanDefinitionBuilder definitionBuilder = builder.build(configuration); @@ -152,7 +157,7 @@ public List registerRepositoriesIn(BeanDefinitionRegist * than a single type is considered a multi-store configuration scenario which will trigger stricter repository * scanning. * - * @return + * @return a boolean value indicating if multiple Spring Data store implementations are present on the classpath. */ private boolean multipleStoresDetected() { 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 8154ac6499..40108d61d9 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSourceSupport.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationSourceSupport.java @@ -72,7 +72,7 @@ public Streamable getCandidates(ResourceLoader loader) { scanner.setEnvironment(environment); scanner.setResourceLoader(loader); - getExcludeFilters().forEach(it -> scanner.addExcludeFilter(it)); + getExcludeFilters().forEach(scanner::addExcludeFilter); return Streamable.of(() -> getBasePackages().stream()// .flatMap(it -> scanner.findCandidateComponents(it).stream())); diff --git a/src/main/java/org/springframework/data/repository/query/Parameters.java b/src/main/java/org/springframework/data/repository/query/Parameters.java index e36b5d9bfd..9c0dfc949b 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameters.java +++ b/src/main/java/org/springframework/data/repository/query/Parameters.java @@ -15,8 +15,6 @@ */ package org.springframework.data.repository.query; -import static java.lang.String.*; - import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; @@ -41,11 +39,13 @@ public abstract class Parameters, T extends Parameter public static final List> TYPES = Arrays.asList(Pageable.class, Sort.class); - private static final String PARAM_ON_SPECIAL = format("You must not user @%s on a parameter typed %s or %s", - Param.class.getSimpleName(), Pageable.class.getSimpleName(), Sort.class.getSimpleName()); - private static final String ALL_OR_NOTHING = String.format( - "Either use @%s on all parameters except %s and %s typed once, or none at all!", Param.class.getSimpleName(), - Pageable.class.getSimpleName(), Sort.class.getSimpleName()); + private static final String PARAM_ON_SPECIAL = + String.format("You must not use @%s on a parameter typed %s or %s", + Param.class.getSimpleName(), Pageable.class.getSimpleName(), Sort.class.getSimpleName()); + + private static final String ALL_OR_NOTHING = + String.format("Either use @%s on all parameters except parameters of type %s and %s, or none at all!", + Param.class.getSimpleName(), Pageable.class.getSimpleName(), Sort.class.getSimpleName()); private final ParameterNameDiscoverer discoverer = new DefaultParameterNameDiscoverer(); private final int pageableIndex; diff --git a/src/main/java/org/springframework/data/repository/query/QueryMethod.java b/src/main/java/org/springframework/data/repository/query/QueryMethod.java index c4ccb10b8f..68e9c59ed0 100644 --- a/src/main/java/org/springframework/data/repository/query/QueryMethod.java +++ b/src/main/java/org/springframework/data/repository/query/QueryMethod.java @@ -69,10 +69,11 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory Parameters.TYPES.stream()// .filter(type -> getNumberOfOccurences(method, type) > 1)// - .findFirst().ifPresent(type -> { + .findFirst() + .ifPresent(type -> { throw new IllegalStateException( - String.format("Method must only one argument of type %s! Offending method: %s", type.getSimpleName(), - method.toString())); + String.format("Method must only have one argument of type %s! Offending method: %s", + type.getSimpleName(), method.toString())); }); this.method = method; diff --git a/src/main/java/org/springframework/data/util/IterableUtils.java b/src/main/java/org/springframework/data/util/IterableUtils.java new file mode 100644 index 0000000000..66b603a7f2 --- /dev/null +++ b/src/main/java/org/springframework/data/util/IterableUtils.java @@ -0,0 +1,70 @@ +/* + * Copyright 2012-2018 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.util; + +import java.util.Collections; + +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; + +/** + * Utility class for working with {@link Iterable} objects. + * + * @author John Blum + * @see java.lang.Iterable + * @since 2.1.0 + */ +public abstract class IterableUtils { + + /** + * Determines whether the given {@link Iterable} is empty. + * + * @param iterable {@link Iterable} to evaluate; can be {@literal null}. + * @return {@literal true} if the given {@link Iterable} is empty, + * otherwise return {@literal false}. + * @see #nullSafeIterable(Iterable) + * @see java.lang.Iterable + */ + public static boolean isEmpty(@Nullable Iterable iterable) { + return !isNotEmpty(iterable); + } + + /** + * Determines if the given {@link Iterable} is not empty. + * + * @param iterable {@link Iterable} to evaluate; can be {@literal null}. + * @return {@literal true} if the given {@link Iterable} is not empty, + * otherwise return {@literal false}. + * @see #nullSafeIterable(Iterable) + * @see java.lang.Iterable + */ + public static boolean isNotEmpty(@Nullable Iterable iterable) { + return nullSafeIterable(iterable).iterator().hasNext(); + } + + /** + * Protects against {@literal null} {@link Iterable} references. + * + * @param {@link Class type} of elements contained by the {@link Iterable}. + * @param it {@link Iterable} to safe-guard from {@literal null}. + * @return the given {@link Iterable} if not {@literal null} or an empty {@link Iterable}. + * @see java.lang.Iterable + */ + @NonNull + public static Iterable nullSafeIterable(@Nullable Iterable it) { + return it != null ? it : Collections::emptyIterator; + } +} diff --git a/src/test/java/org/springframework/data/util/IterableUtilsUnitTests.java b/src/test/java/org/springframework/data/util/IterableUtilsUnitTests.java new file mode 100644 index 0000000000..65b0b6d8be --- /dev/null +++ b/src/test/java/org/springframework/data/util/IterableUtilsUnitTests.java @@ -0,0 +1,81 @@ +/* + * Copyright 2012-2018 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.util; + +import static org.assertj.core.api.Java6Assertions.assertThat; +import static org.mockito.Mockito.mock; + +import java.util.Collections; + +import org.junit.Test; + +/** + * Unit tests for {@link IterableUtils}. + * + * @author John Blum + * @see org.junit.Test + * @see org.springframework.data.util.IterableUtils + * @since 1.0.0 + */ +public class IterableUtilsUnitTests { + + @Test + public void isEmptyReturnsTrueWithEmptyIterable() { + assertThat(IterableUtils.isEmpty(Collections.emptySet())).isTrue(); + } + + @Test + public void isEmptyReturnsTrueWithNullIterable() { + assertThat(IterableUtils.isEmpty(null)).isTrue(); + } + + @Test + public void isEmptyReturnsFalseWithNonEmptyIterable() { + assertThat(IterableUtils.isEmpty(Collections.singleton(1))).isFalse(); + } + + @Test + public void isNotEmptyReturnsTrueWithNonEmptyIterable() { + assertThat(IterableUtils.isNotEmpty(Collections.singletonList(1))).isTrue(); + } + + @Test + public void isNotEmptyReturnsFalseWithEmptyIterable() { + assertThat(IterableUtils.isNotEmpty(Collections.emptyList())).isFalse(); + } + + @Test + public void isNotEmptyReturnsFalseWithNullIterable() { + assertThat(IterableUtils.isNotEmpty(null)).isFalse(); + } + + @Test + public void nullSafeIterableReturnsNonNullIterable() { + + Iterable mockIterable = mock(Iterable.class); + + assertThat(IterableUtils.nullSafeIterable(mockIterable)).isSameAs(mockIterable); + } + + @Test + public void nullSafeIterableReturnsEmptyIterableForNull() { + + Iterable iterable = IterableUtils.nullSafeIterable(null); + + assertThat(iterable).isNotNull(); + assertThat(iterable).isEmpty(); + } +}