From 4dcda9701786a0dbab6fb5f8e1fe010887c014a9 Mon Sep 17 00:00:00 2001 From: Spring Buildmaster Date: Fri, 5 Sep 2014 01:57:19 -0700 Subject: [PATCH 01/59] DATACMNS-572 - Prepare next development iteration. --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index dac19dd3a4..b6e348a042 100644 --- a/pom.xml +++ b/pom.xml @@ -5,14 +5,14 @@ org.springframework.data spring-data-commons - 1.9.0.RELEASE + 1.9.1.BUILD-SNAPSHOT Spring Data Core org.springframework.data.build spring-data-parent - 1.5.0.RELEASE + 1.5.1.BUILD-SNAPSHOT ../spring-data-build/parent/pom.xml From 4e7ad22263478512004003098b1145e12608670f Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 5 Sep 2014 13:13:16 +0200 Subject: [PATCH 02/59] DATACMNS-572 - After release cleanups. --- pom.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index b6e348a042..24d01f66ce 100644 --- a/pom.xml +++ b/pom.xml @@ -1,4 +1,4 @@ - + 4.0.0 @@ -237,8 +237,8 @@ - spring-libs-release - http://repo.spring.io/libs-release + spring-libs-snapshot + http://repo.spring.io/libs-snapshot From 77fe2b11973209e001f1523c28c30cbdf615387b Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 6 Oct 2014 13:23:37 +0200 Subject: [PATCH 03/59] DATACMNS-530 - Improve JavaDoc for RepositoryConfigurationSourceSupport. Removed misleading JavaDoc. Original pull request: #97. --- .../config/RepositoryConfigurationSourceSupport.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 f8e8908373..140ee627d3 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-2013 the original author or authors. + * Copyright 2012-2014 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. @@ -22,7 +22,6 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.core.env.Environment; -import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.ResourceLoader; import org.springframework.core.type.filter.TypeFilter; import org.springframework.util.Assert; @@ -40,8 +39,7 @@ public abstract class RepositoryConfigurationSourceSupport implements Repository private final Environment environment; /** - * Creates a new {@link RepositoryConfigurationSourceSupport} with the given environment. Defaults to a plain - * {@link StandardEnvironment} in case the given argument is {@literal null}. + * Creates a new {@link RepositoryConfigurationSourceSupport} with the given environment. * * @param environment must not be {@literal null}. */ From 36a6c451fa5ef62a4010fa78ba064290c7ba0536 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 6 Oct 2014 10:53:03 +0200 Subject: [PATCH 04/59] DATACMNS-577 - Improved JavaDoc on QueryDslPredicateExecutor. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clarified return values if predicates don't match or return multiple results (for the findOne(…) method). Copied summary to the @return tag. Original pull request: #96. --- .../querydsl/QueryDslPredicateExecutor.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java b/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java index 54dc33723c..5160b79cfd 100644 --- a/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java +++ b/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java @@ -1,5 +1,5 @@ /* - * Copyright 2011 the original author or authors. + * Copyright 2011-2014 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,6 +15,7 @@ */ package org.springframework.data.querydsl; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -25,40 +26,45 @@ * Interface to allow execution of QueryDsl {@link Predicate} instances. * * @author Oliver Gierke + * @author Thomas Darimont */ public interface QueryDslPredicateExecutor { /** - * Returns a single entity matching the given {@link Predicate}. + * Returns a single entity matching the given {@link Predicate} or {@literal null} if none was found. * - * @param spec - * @return + * @param predicate + * @return a single entity matching the given {@link Predicate} or {@literal null} if none was found. + * @throws IncorrectResultSizeDataAccessException if the predicate yields more than one result. */ T findOne(Predicate predicate); /** - * Returns all entities matching the given {@link Predicate}. + * Returns all entities matching the given {@link Predicate}. In case no match could be found an empty + * {@link Iterable} is returned. * - * @param spec - * @return + * @param predicate + * @return all entities matching the given {@link Predicate}. */ Iterable findAll(Predicate predicate); /** - * Returns all entities matching the given {@link Predicate} applying the given {@link OrderSpecifier}s. + * Returns all entities matching the given {@link Predicate} applying the given {@link OrderSpecifier}s. In case no + * match could be found an empty {@link Iterable} is returned. * * @param predicate * @param orders - * @return + * @return all entities matching the given {@link Predicate} applying the given {@link OrderSpecifier}s. */ Iterable findAll(Predicate predicate, OrderSpecifier... orders); /** - * Returns a {@link Page} of entities matching the given {@link Predicate}. + * Returns a {@link Page} of entities matching the given {@link Predicate}. In case no match could be found, an empty + * {@link Page} is returned. * * @param predicate * @param pageable - * @return + * @return a {@link Page} of entities matching the given {@link Predicate}. */ Page findAll(Predicate predicate, Pageable pageable); From 43793f9b3e4f8bce2fa774f535f74bbbe0faf47a Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 17 Oct 2014 19:02:17 +0200 Subject: [PATCH 05/59] DATACMNS-580 - Add section on limiting the query results to reference documentation. Added section explaining the use of first / top keywords to the reference doc. Original pull request: #99. --- src/main/asciidoc/repositories.adoc | 32 ++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index 37e1077e52..83e54de940 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -296,12 +296,42 @@ List findByLastname(String lastname, Pageable pageable); ---- ==== -The first method allows you to pass an `org.springframework.data.domain.Pageable` instance to the query method to dynamically add paging to your statically defined query. A `Page` knows about the total number of elements and pages available. It does so by the infrastructure triggering a count query to calculate the overall number. As this might be expensive depending on the store used, `Slice` can be used as return instead. A `Slice` only knows about whether there's a next `Slice` available which might be just sufficient when walking thought a larger resut set. +The first method allows you to pass an `org.springframework.data.domain.Pageable` instance to the query method to dynamically add paging to your statically defined query. A `Page` knows about the total number of elements and pages available. It does so by the infrastructure triggering a count query to calculate the overall number. As this might be expensive depending on the store used, `Slice` can be used as return instead. A `Slice` only knows about whether there's a next `Slice` available which might be just sufficient when walking thought a larger result set. Sorting options are handled through the `Pageable` instance too. If you only need sorting, simply add an `org.springframework.data.domain.Sort` parameter to your method. As you also can see, simply returning a `List` is possible as well. In this case the additional metadata required to build the actual `Page` instance will not be created (which in turn means that the additional count query that would have been necessary not being issued) but rather simply restricts the query to look up only the given range of entities. NOTE: To find out how many pages you get for a query entirely you have to trigger an additional count query. By default this query will be derived from the query you actually trigger. +[[repositories.limit-query-result]] +=== Limiting query results + +The results of query methods can be limited via the keywords `first` or `top`, which can be used interchangeably. An optional numeric value can be appended to top/first to specify the maximum result size to be returned. +If the number is left out, a result size of 1 is assumed. + +.Limiting the result size of a query with `Top` and `First` +==== +[source, java] +---- +User findFirstByOrderByLastname(); + +User findTopByOrderByAgeDesc(); + +Page queryFirst10ByLastname(String lastname, Pageable pageable); + +Slice findTop3ByLastname(String lastname, Pageable pageable); + +List findFirst10ByLastname(String lastname, Sort sort); + +List findTop10ByLastname(String lastname, Pageable pageable); +---- +==== + +The limiting expressions also support the `Distinct` keyword. Also, for the queries limiting the result set to one instance, wrapping the result into an `Optional` is supported. + +If pagination or slicing is applied to a limiting query pagination (and the calculation of the number of pages available) then it is applied within the limited result. + +NOTE: Note that limiting the results in combination with dynamic sorting via a `Sort` parameter allows to express query methods for the 'K' smallest as well as for the 'K' biggest elements. + [[repositories.create-instances]] == Creating repository instances In this section you create instances and bean definitions for the repository interfaces defined. One way to do so is using the Spring namespace that is shipped with each Spring Data module that supports the repository mechanism although we generally recommend to use the Java-Config style configuration. From 386644190a3d38f907ae1b23f594331dc18a657c Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Tue, 28 Oct 2014 15:57:34 +0100 Subject: [PATCH 06/59] DATACMNS-583 - DomainClassConverter returns input if given types are the same. DomainClassConverter now returns input when the source type is identical to the target type. Previously we erroneously tried to convert the sourceType to the idType of the targetType's backing repository. As a side effect we also avoid performing some unnecessary converter hops. Original pull request: #101. --- .../repository/support/DomainClassConverter.java | 13 +++++++++++-- .../support/DomainClassConverterUnitTests.java | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java index b2ce5242cd..9fc9dad8e7 100644 --- a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java +++ b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2012 the original author or authors. + * Copyright 2008-2014 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. @@ -31,11 +31,12 @@ /** * {@link org.springframework.core.convert.converter.Converter} to convert arbitrary input into domain classes managed - * by Spring Data {@link CrudRepository} s. The implementation uses a {@link ConversionService} in turn to convert the + * by Spring Data {@link CrudRepository}s. The implementation uses a {@link ConversionService} in turn to convert the * source type into the domain class' id type which is then converted into a domain class object by using a * {@link CrudRepository}. * * @author Oliver Gierke + * @author Thomas Darimont */ public class DomainClassConverter implements ConditionalGenericConverter, ApplicationContextAware { @@ -65,6 +66,10 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t return null; } + if (sourceType.equals(targetType)) { + return source; + } + Class domainType = targetType.getType(); RepositoryInformation info = repositories.getRepositoryInformationFor(domainType); @@ -83,6 +88,10 @@ public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { return false; } + if (sourceType.equals(targetType)) { + return true; + } + return conversionService.canConvert(sourceType.getType(), repositories.getRepositoryInformationFor(targetType.getType()).getIdType()); } diff --git a/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java b/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java index f78778d642..d809c24924 100644 --- a/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java @@ -39,6 +39,7 @@ * Unit test for {@link DomainClassConverter}. * * @author Oliver Gierke + * @author Thomas Darimont */ @RunWith(MockitoJUnitRunner.class) public class DomainClassConverterUnitTests { @@ -143,6 +144,19 @@ public void discoversFactoryAndRepoFromParentApplicationContext() { assertThat(converter.matches(sourceDescriptor, targetDescriptor), is(true)); } + /** + * @DATACMNS-583 + */ + @Test + public void shouldReturnSourceObjectIfSourceAndTargetTypesAreTheSame() { + + ApplicationContext context = initContextWithRepo(); + converter.setApplicationContext(context); + + assertThat(converter.matches(targetDescriptor, targetDescriptor), is(true)); + assertThat((User) converter.convert(USER, targetDescriptor, targetDescriptor), is(USER)); + } + private ApplicationContext initContextWithRepo() { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(DummyRepositoryFactoryBean.class); From 9b210d2c3ee00d79980a8f31ba051503680ae737 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 30 Oct 2014 11:57:34 +0100 Subject: [PATCH 07/59] DATACMNS-585 - Updated changelog. --- src/main/resources/changelog.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/resources/changelog.txt b/src/main/resources/changelog.txt index f9e23a59b2..f7704cae02 100644 --- a/src/main/resources/changelog.txt +++ b/src/main/resources/changelog.txt @@ -1,6 +1,15 @@ Spring Data Commons Changelog ============================= +Changes in version 1.9.1.RELEASE (2014-10-30) +--------------------------------------------- +* DATACMNS-585 - Release 1.9.1. +* DATACMNS-583 - DomainClassConverter.matches(…) should return false when source- and target type are equal. +* DATACMNS-580 - Add section on limiting the query results to reference documentation. +* DATACMNS-577 - Improve JavaDoc on QueryDslPredicateExecutor. +* DATACMNS-530 - JavaDoc should be updated to reflect change of requiring an environment. + + Changes in version 1.9.0.RELEASE (2014-09-05) --------------------------------------------- * DATACMNS-572 - Release 1.9 GA. From bae378de680b37eaf967e6bdcb2ea7dfd5cf1399 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 30 Oct 2014 11:57:46 +0100 Subject: [PATCH 08/59] DATACMNS-585 - Prepare 1.9.1.RELEASE (Evans SR1). --- pom.xml | 6 +++--- src/main/resources/notice.txt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 24d01f66ce..ef5a749fcf 100644 --- a/pom.xml +++ b/pom.xml @@ -12,7 +12,7 @@ org.springframework.data.build spring-data-parent - 1.5.1.BUILD-SNAPSHOT + 1.5.1.RELEASE ../spring-data-build/parent/pom.xml @@ -237,8 +237,8 @@ - spring-libs-snapshot - http://repo.spring.io/libs-snapshot + spring-libs-release + http://repo.spring.io/libs-release diff --git a/src/main/resources/notice.txt b/src/main/resources/notice.txt index e74c3c153d..d6af53d7f4 100644 --- a/src/main/resources/notice.txt +++ b/src/main/resources/notice.txt @@ -1,4 +1,4 @@ -Spring Data Commons 1.9 GA +Spring Data Commons 1.9.1 Copyright (c) [2010-2014] Pivotal Software, Inc. This product is licensed to you under the Apache License, Version 2.0 (the "License"). From b728ea297e4711b0241483e3924eccf3a529cbc7 Mon Sep 17 00:00:00 2001 From: Spring Buildmaster Date: Thu, 30 Oct 2014 04:18:49 -0700 Subject: [PATCH 09/59] DATACMNS-585 - Release version 1.9.1.RELEASE (Evans SR1). --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index ef5a749fcf..68b8509435 100644 --- a/pom.xml +++ b/pom.xml @@ -1,11 +1,11 @@ - + 4.0.0 org.springframework.data spring-data-commons - 1.9.1.BUILD-SNAPSHOT + 1.9.1.RELEASE Spring Data Core From 3471a64f79b8cbda348edddd4b064a5f0ca05af0 Mon Sep 17 00:00:00 2001 From: Spring Buildmaster Date: Thu, 30 Oct 2014 04:18:53 -0700 Subject: [PATCH 10/59] DATACMNS-585 - Prepare next development iteration. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 68b8509435..32b63f5173 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 1.9.1.RELEASE + 1.9.2.BUILD-SNAPSHOT Spring Data Core From fa44b9f807c37e9b95785772389c4fb5b4d25282 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 31 Oct 2014 21:34:45 +0100 Subject: [PATCH 11/59] DATACMNS-585 - After release cleanups. --- pom.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 32b63f5173..076ebb988a 100644 --- a/pom.xml +++ b/pom.xml @@ -1,4 +1,4 @@ - + 4.0.0 @@ -12,7 +12,7 @@ org.springframework.data.build spring-data-parent - 1.5.1.RELEASE + 1.5.2.BUILD-SNAPSHOT ../spring-data-build/parent/pom.xml @@ -237,8 +237,8 @@ - spring-libs-release - http://repo.spring.io/libs-release + spring-libs-snapshot + http://repo.spring.io/libs-snapshot From e943fdeae4e7d5d785607c26a30170b6895fa7cc Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 12 Nov 2014 14:30:34 +0100 Subject: [PATCH 12/59] DATACMNS-590 - Fixed calculation of nested generics in ParentTypeAwareTypeInformation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far, the lookup of the type variable map was preferring the type variable maps detected on parent types in nested structures. This caused the concrete type variables in nested types not being considered correctly which caused the type resolution to fall back to the generic bounds. We now accumulate the type variable maps to avoid having to lookup a certain map in the nesting hierarchy. The core fix is in ParentTypeAwareTypeInformation's constructor and mergeMaps(…) respectively. Simplified the handling of type variable maps and made proper use of generics throughout the class hierarchy. --- .../data/util/ClassTypeInformation.java | 24 +++++++-- .../util/GenericArrayTypeInformation.java | 14 ++++-- .../util/ParameterizedTypeInformation.java | 13 +++-- .../util/ParentTypeAwareTypeInformation.java | 28 ++++++----- .../data/util/TypeDiscoverer.java | 30 +++++++----- .../util/TypeVariableTypeInformation.java | 5 +- .../util/ClassTypeInformationUnitTests.java | 36 +++++++++++++- .../data/util/ParameterizedTypeUnitTests.java | 23 +++++---- .../data/util/TypeDiscovererUnitTests.java | 49 ++++++++++++------- 9 files changed, 152 insertions(+), 70 deletions(-) diff --git a/src/main/java/org/springframework/data/util/ClassTypeInformation.java b/src/main/java/org/springframework/data/util/ClassTypeInformation.java index afecda03af..a3caf9d8b1 100644 --- a/src/main/java/org/springframework/data/util/ClassTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ClassTypeInformation.java @@ -23,8 +23,10 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.WeakHashMap; @@ -98,12 +100,26 @@ public static TypeInformation fromReturnTypeOf(Method method) { * @param type */ ClassTypeInformation(Class type) { - this(type, GenericTypeResolver.getTypeVariableMap(type)); + super(ClassUtils.getUserClass(type), getTypeVariableMap(type)); + this.type = type; } - ClassTypeInformation(Class type, Map typeVariableMap) { - super(ClassUtils.getUserClass(type), typeVariableMap); - this.type = type; + /** + * Little helper to allow us to create a generified map, actually just to satisfy the compiler. + * + * @param type must not be {@literal null}. + * @return + */ + private static Map, Type> getTypeVariableMap(Class type) { + + Map source = GenericTypeResolver.getTypeVariableMap(type); + Map, Type> map = new HashMap, Type>(source.size()); + + for (Entry entry : source.entrySet()) { + map.put(entry.getKey(), entry.getValue()); + } + + return map; } /* diff --git a/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java b/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java index 1eeacf7f05..9f91b123f3 100644 --- a/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java +++ b/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java @@ -18,6 +18,8 @@ import java.lang.reflect.Array; import java.lang.reflect.GenericArrayType; import java.lang.reflect.Type; +import java.lang.reflect.TypeVariable; +import java.util.Map; /** * Special {@link TypeDiscoverer} handling {@link GenericArrayType}s. @@ -26,18 +28,20 @@ */ class GenericArrayTypeInformation extends ParentTypeAwareTypeInformation { - private GenericArrayType type; + private final GenericArrayType type; /** * Creates a new {@link GenericArrayTypeInformation} for the given {@link GenericArrayTypeInformation} and * {@link TypeDiscoverer}. * - * @param type - * @param parent + * @param type must not be {@literal null}. + * @param parent must not be {@literal null}. + * @param typeVariableMap must not be {@literal null}. */ - protected GenericArrayTypeInformation(GenericArrayType type, TypeDiscoverer parent) { + protected GenericArrayTypeInformation(GenericArrayType type, TypeDiscoverer parent, + Map, Type> typeVariableMap) { - super(type, parent, parent.getTypeVariableMap()); + super(type, parent, typeVariableMap); this.type = type; } diff --git a/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java b/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java index 3c301ade35..acae863498 100644 --- a/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java @@ -17,6 +17,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.lang.reflect.TypeVariable; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -24,7 +25,6 @@ import java.util.Map; import java.util.Set; -import org.springframework.core.GenericTypeResolver; import org.springframework.util.StringUtils; /** @@ -44,8 +44,10 @@ class ParameterizedTypeInformation extends ParentTypeAwareTypeInformation * @param type must not be {@literal null} * @param parent must not be {@literal null} */ - public ParameterizedTypeInformation(ParameterizedType type, TypeDiscoverer parent) { - super(type, parent, null); + public ParameterizedTypeInformation(ParameterizedType type, TypeDiscoverer parent, + Map, Type> typeVariableMap) { + + super(type, parent, typeVariableMap); this.type = type; } @@ -72,8 +74,11 @@ public TypeInformation getMapValueType() { supertypes.addAll(Arrays.asList(rawType.getGenericInterfaces())); for (Type supertype : supertypes) { - Class rawSuperType = GenericTypeResolver.resolveType(supertype, getTypeVariableMap()); + + Class rawSuperType = resolveType(supertype); + if (Map.class.isAssignableFrom(rawSuperType)) { + ParameterizedType parameterizedSupertype = (ParameterizedType) supertype; Type[] arguments = parameterizedSupertype.getActualTypeArguments(); return createInfo(arguments[1]); diff --git a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java index 0411b1e696..8fb4bc0937 100644 --- a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java @@ -17,6 +17,7 @@ import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; +import java.util.HashMap; import java.util.Map; import org.springframework.util.ObjectUtils; @@ -33,27 +34,30 @@ public abstract class ParentTypeAwareTypeInformation extends TypeDiscoverer parent, Map map) { - - super(type, map); + protected ParentTypeAwareTypeInformation(Type type, TypeDiscoverer parent, Map, Type> map) { + super(type, mergeMaps(parent, map)); this.parent = parent; } /** - * Considers the parent's type variable map before invoking the super class method. + * Merges the type variable maps of the given parent with the new map. * + * @param parent must not be {@literal null}. + * @param map must not be {@literal null}. * @return */ - @Override - @SuppressWarnings("rawtypes") - protected Map getTypeVariableMap() { - return parent == null ? super.getTypeVariableMap() : parent.getTypeVariableMap(); + private static Map, Type> mergeMaps(TypeDiscoverer parent, Map, Type> map) { + + Map, Type> typeVariableMap = new HashMap, Type>(); + typeVariableMap.putAll(map); + typeVariableMap.putAll(parent.getTypeVariableMap()); + + return typeVariableMap; } /* diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java index c43b3407b5..1f77cc84f6 100644 --- a/src/main/java/org/springframework/data/util/TypeDiscoverer.java +++ b/src/main/java/org/springframework/data/util/TypeDiscoverer.java @@ -30,6 +30,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -47,7 +48,7 @@ class TypeDiscoverer implements TypeInformation { private final Type type; - @SuppressWarnings("rawtypes") private final Map typeVariableMap; + private final Map, Type> typeVariableMap; private final Map> fieldTypes = new ConcurrentHashMap>(); private Class resolvedType; @@ -55,25 +56,24 @@ class TypeDiscoverer implements TypeInformation { /** * Creates a ne {@link TypeDiscoverer} for the given type, type variable map and parent. * - * @param type must not be null. - * @param typeVariableMap + * @param type must not be {@literal null}. + * @param typeVariableMap must not be {@literal null}. */ - @SuppressWarnings("rawtypes") - protected TypeDiscoverer(Type type, Map typeVariableMap) { + protected TypeDiscoverer(Type type, Map, Type> typeVariableMap) { Assert.notNull(type); + Assert.notNull(typeVariableMap); + this.type = type; this.typeVariableMap = typeVariableMap; } /** - * Returns the type variable map. Will traverse the parents up to the root on and use it's map. + * Returns the type variable map. * * @return */ - @SuppressWarnings("rawtypes") - protected Map getTypeVariableMap() { - + protected Map, Type> getTypeVariableMap() { return typeVariableMap; } @@ -98,7 +98,7 @@ protected TypeInformation createInfo(Type fieldType) { if (fieldType instanceof ParameterizedType) { ParameterizedType parameterizedType = (ParameterizedType) fieldType; - return new ParameterizedTypeInformation(parameterizedType, this); + return new ParameterizedTypeInformation(parameterizedType, this, variableMap); } if (fieldType instanceof TypeVariable) { @@ -107,7 +107,7 @@ protected TypeInformation createInfo(Type fieldType) { } if (fieldType instanceof GenericArrayType) { - return new GenericArrayTypeInformation((GenericArrayType) fieldType, this); + return new GenericArrayTypeInformation((GenericArrayType) fieldType, this, variableMap); } if (fieldType instanceof WildcardType) { @@ -135,9 +135,13 @@ protected TypeInformation createInfo(Type fieldType) { * @param type * @return */ - @SuppressWarnings("unchecked") + @SuppressWarnings({ "unchecked", "rawtypes" }) protected Class resolveType(Type type) { - return (Class) GenericTypeResolver.resolveType(type, getTypeVariableMap()); + + Map map = new HashMap(); + map.putAll(getTypeVariableMap()); + + return (Class) GenericTypeResolver.resolveType(type, map); } /* diff --git a/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java b/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java index 794c7ae319..ec36f90710 100644 --- a/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java +++ b/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java @@ -43,11 +43,10 @@ class TypeVariableTypeInformation extends ParentTypeAwareTypeInformation { * @param owningType must not be {@literal null} * @param parent */ - @SuppressWarnings("rawtypes") public TypeVariableTypeInformation(TypeVariable variable, Type owningType, TypeDiscoverer parent, - Map map) { + Map, Type> typeVariableMap) { - super(variable, parent, map); + super(variable, parent, typeVariableMap); Assert.notNull(variable); this.variable = variable; this.owningType = owningType; diff --git a/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java b/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java index 9459797f86..d498afc167 100644 --- a/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java +++ b/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java @@ -93,7 +93,7 @@ public void discoversArraysAndCollections() { TypeInformation information = ClassTypeInformation.from(StringCollectionContainer.class); TypeInformation property = information.getProperty("array"); - assertEquals(property.getComponentType().getType(), String.class); + assertThat(property.getComponentType().getType(), is((Object) String.class)); Class type = property.getType(); assertEquals(String[].class, type); @@ -327,6 +327,20 @@ public void createsToStringRepresentation() { is("org.springframework.data.util.ClassTypeInformationUnitTests$SpecialPerson")); } + /** + * @see DATACMNS-590 + */ + @Test + public void resolvesNestedGenericsToConcreteType() { + + ClassTypeInformation rootType = from(ConcreteRoot.class); + TypeInformation subsPropertyType = rootType.getProperty("subs"); + TypeInformation subsElementType = subsPropertyType.getActualType(); + TypeInformation subSubType = subsElementType.getProperty("subSub"); + + assertThat(subSubType.getType(), is((Object) ConcreteSubSub.class)); + } + static class StringMapContainer extends MapContainer { } @@ -461,4 +475,24 @@ static class SuperGenerics { SortedMap>> seriously; } + + // DATACMNS-590 + + static abstract class GenericRoot> { + List subs; + } + + static abstract class GenericSub { + T subSub; + } + + static abstract class GenericSubSub {} + + static class ConcreteRoot extends GenericRoot {} + + static class ConcreteSub extends GenericSub {} + + static class ConcreteSubSub extends GenericSubSub { + String content; + } } diff --git a/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java b/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java index 3f9222a110..1eae5bd99a 100644 --- a/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java +++ b/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2011 the original author or authors. + * Copyright 2011-2014 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. @@ -22,8 +22,11 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.lang.reflect.TypeVariable; +import java.util.Collections; import java.util.HashMap; import java.util.Locale; +import java.util.Map; import org.junit.Before; import org.junit.Test; @@ -39,6 +42,8 @@ @RunWith(MockitoJUnitRunner.class) public class ParameterizedTypeUnitTests { + static final Map, Type> EMPTY_MAP = Collections.emptyMap(); + @Mock ParameterizedType one; @Before @@ -49,22 +54,22 @@ public void setUp() { @Test public void considersTypeInformationsWithDifferingParentsNotEqual() { - TypeDiscoverer stringParent = new TypeDiscoverer(String.class, null); - TypeDiscoverer objectParent = new TypeDiscoverer(Object.class, null); + TypeDiscoverer stringParent = new TypeDiscoverer(String.class, EMPTY_MAP); + TypeDiscoverer objectParent = new TypeDiscoverer(Object.class, EMPTY_MAP); - ParameterizedTypeInformation first = new ParameterizedTypeInformation(one, stringParent); - ParameterizedTypeInformation second = new ParameterizedTypeInformation(one, objectParent); + ParameterizedTypeInformation first = new ParameterizedTypeInformation(one, stringParent, EMPTY_MAP); + ParameterizedTypeInformation second = new ParameterizedTypeInformation(one, objectParent, EMPTY_MAP); - assertFalse(first.equals(second)); + assertThat(first, is(not(second))); } @Test public void considersTypeInformationsWithSameParentsNotEqual() { - TypeDiscoverer stringParent = new TypeDiscoverer(String.class, null); + TypeDiscoverer stringParent = new TypeDiscoverer(String.class, EMPTY_MAP); - ParameterizedTypeInformation first = new ParameterizedTypeInformation(one, stringParent); - ParameterizedTypeInformation second = new ParameterizedTypeInformation(one, stringParent); + ParameterizedTypeInformation first = new ParameterizedTypeInformation(one, stringParent, EMPTY_MAP); + ParameterizedTypeInformation second = new ParameterizedTypeInformation(one, stringParent, EMPTY_MAP); assertTrue(first.equals(second)); } diff --git a/src/test/java/org/springframework/data/util/TypeDiscovererUnitTests.java b/src/test/java/org/springframework/data/util/TypeDiscovererUnitTests.java index 5b4a034cd1..59800c888f 100644 --- a/src/test/java/org/springframework/data/util/TypeDiscovererUnitTests.java +++ b/src/test/java/org/springframework/data/util/TypeDiscovererUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2012 the original author or authors. + * Copyright 2011-2014 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. @@ -17,11 +17,13 @@ import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; +import static org.springframework.data.util.ClassTypeInformation.*; import java.lang.reflect.Constructor; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; @@ -39,13 +41,10 @@ @RunWith(MockitoJUnitRunner.class) public class TypeDiscovererUnitTests { - @Mock - @SuppressWarnings("rawtypes") - Map firstMap; + static final Map, Type> EMPTY_MAP = Collections.emptyMap(); - @Mock - @SuppressWarnings("rawtypes") - Map secondMap; + @Mock Map, Type> firstMap; + @Mock Map, Type> secondMap; @Test(expected = IllegalArgumentException.class) public void rejectsNullType() { @@ -55,8 +54,8 @@ public void rejectsNullType() { @Test public void isNotEqualIfTypesDiffer() { - TypeDiscoverer objectTypeInfo = new TypeDiscoverer(Object.class, null); - TypeDiscoverer stringTypeInfo = new TypeDiscoverer(String.class, null); + TypeDiscoverer objectTypeInfo = new TypeDiscoverer(Object.class, EMPTY_MAP); + TypeDiscoverer stringTypeInfo = new TypeDiscoverer(String.class, EMPTY_MAP); assertFalse(objectTypeInfo.equals(stringTypeInfo)); } @@ -75,37 +74,44 @@ public void isNotEqualIfTypeVariableMapsDiffer() { @Test public void dealsWithTypesReferencingThemselves() { - TypeInformation information = new ClassTypeInformation(SelfReferencing.class); + TypeInformation information = from(SelfReferencing.class); TypeInformation first = information.getProperty("parent").getMapValueType(); TypeInformation second = first.getProperty("map").getMapValueType(); + assertEquals(first, second); } @Test public void dealsWithTypesReferencingThemselvesInAMap() { - TypeInformation information = new ClassTypeInformation( - SelfReferencingMap.class); + TypeInformation information = from(SelfReferencingMap.class); TypeInformation mapValueType = information.getProperty("map").getMapValueType(); + assertEquals(mapValueType, information); } @Test public void returnsComponentAndValueTypesForMapExtensions() { - TypeInformation discoverer = new TypeDiscoverer(CustomMap.class, null); + + TypeInformation discoverer = new TypeDiscoverer(CustomMap.class, EMPTY_MAP); + assertEquals(Locale.class, discoverer.getMapValueType().getType()); assertEquals(String.class, discoverer.getComponentType().getType()); } @Test public void returnsComponentTypeForCollectionExtension() { - TypeDiscoverer discoverer = new TypeDiscoverer(CustomCollection.class, null); + + TypeDiscoverer discoverer = new TypeDiscoverer(CustomCollection.class, firstMap); + assertEquals(String.class, discoverer.getComponentType().getType()); } @Test public void returnsComponentTypeForArrays() { - TypeDiscoverer discoverer = new TypeDiscoverer(String[].class, null); + + TypeDiscoverer discoverer = new TypeDiscoverer(String[].class, EMPTY_MAP); + assertEquals(String.class, discoverer.getComponentType().getType()); } @@ -117,9 +123,10 @@ public void returnsComponentTypeForArrays() { public void discoveresConstructorParameterTypesCorrectly() throws NoSuchMethodException, SecurityException { TypeDiscoverer discoverer = new TypeDiscoverer(GenericConstructors.class, - null); + firstMap); Constructor constructor = GenericConstructors.class.getConstructor(List.class, Locale.class); List> types = discoverer.getParameterTypes(constructor); + assertThat(types.size(), is(2)); assertThat(types.get(0).getType(), equalTo((Class) List.class)); assertThat(types.get(0).getComponentType().getType(), is(equalTo((Class) String.class))); @@ -128,7 +135,9 @@ public void discoveresConstructorParameterTypesCorrectly() throws NoSuchMethodEx @Test @SuppressWarnings("rawtypes") public void returnsNullForComponentAndValueTypesForRawMaps() { - TypeDiscoverer discoverer = new TypeDiscoverer(Map.class, null); + + TypeDiscoverer discoverer = new TypeDiscoverer(Map.class, EMPTY_MAP); + assertThat(discoverer.getComponentType(), is(nullValue())); assertThat(discoverer.getMapValueType(), is(nullValue())); } @@ -140,14 +149,16 @@ public void returnsNullForComponentAndValueTypesForRawMaps() { @SuppressWarnings("rawtypes") public void doesNotConsiderTypeImplementingIterableACollection() { - TypeDiscoverer discoverer = new TypeDiscoverer(Person.class, null); - TypeInformation reference = ClassTypeInformation.from(Address.class); + TypeDiscoverer discoverer = new TypeDiscoverer(Person.class, EMPTY_MAP); + TypeInformation reference = from(Address.class); TypeInformation addresses = discoverer.getProperty("addresses"); + assertThat(addresses.isCollectionLike(), is(false)); assertThat(addresses.getComponentType(), is(reference)); TypeInformation adressIterable = discoverer.getProperty("addressIterable"); + assertThat(adressIterable.isCollectionLike(), is(true)); assertThat(adressIterable.getComponentType(), is(reference)); } From e14ba802a7160c3bca9e14bd6a683297e40335b8 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 14 Nov 2014 16:34:31 +0100 Subject: [PATCH 13/59] DATACMNS-594 - Fixed creation of type variable map to extend into detected bounds. The initial setup of the type variable map now unfolds the detected types in the map to make sure we detect all type variables present in the current scope. Added caching of component and map value TypeInformation instances to avoid repeated creation. --- .../data/util/ClassTypeInformation.java | 18 +++++++++ .../util/GenericArrayTypeInformation.java | 4 +- .../util/ParameterizedTypeInformation.java | 39 +++++++++++-------- .../data/util/TypeDiscoverer.java | 32 +++++++++++++-- .../util/ClassTypeInformationUnitTests.java | 32 +++++++++++++++ 5 files changed, 103 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/springframework/data/util/ClassTypeInformation.java b/src/main/java/org/springframework/data/util/ClassTypeInformation.java index a3caf9d8b1..aecfc1a8f9 100644 --- a/src/main/java/org/springframework/data/util/ClassTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ClassTypeInformation.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -111,12 +112,29 @@ public static TypeInformation fromReturnTypeOf(Method method) { * @return */ private static Map, Type> getTypeVariableMap(Class type) { + return getTypeVariableMap(type, new HashSet()); + } + + @SuppressWarnings("deprecation") + private static Map, Type> getTypeVariableMap(Class type, Collection visited) { + + if (visited.contains(type)) { + return Collections.emptyMap(); + } else { + visited.add(type); + } Map source = GenericTypeResolver.getTypeVariableMap(type); Map, Type> map = new HashMap, Type>(source.size()); for (Entry entry : source.entrySet()) { + + Type value = entry.getValue(); map.put(entry.getKey(), entry.getValue()); + + if (value instanceof Class) { + map.putAll(getTypeVariableMap((Class) value, visited)); + } } return map; diff --git a/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java b/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java index 9f91b123f3..b51ceb0625 100644 --- a/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java +++ b/src/main/java/org/springframework/data/util/GenericArrayTypeInformation.java @@ -57,10 +57,10 @@ public Class getType() { /* * (non-Javadoc) - * @see org.springframework.data.util.TypeDiscoverer#getComponentType() + * @see org.springframework.data.util.TypeDiscoverer#doGetComponentType() */ @Override - public TypeInformation getComponentType() { + protected TypeInformation doGetComponentType() { Type componentType = type.getGenericComponentType(); return createInfo(componentType); diff --git a/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java b/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java index acae863498..61e0554fb6 100644 --- a/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java @@ -36,7 +36,7 @@ class ParameterizedTypeInformation extends ParentTypeAwareTypeInformation { private final ParameterizedType type; - private TypeInformation componentType; + private Boolean resolved; /** * Creates a new {@link ParameterizedTypeInformation} for the given {@link Type} and parent {@link TypeDiscoverer}. @@ -51,12 +51,12 @@ public ParameterizedTypeInformation(ParameterizedType type, TypeDiscoverer pa this.type = type; } - /* + /* * (non-Javadoc) - * @see org.springframework.data.util.TypeDiscoverer#getMapValueType() + * @see org.springframework.data.util.TypeDiscoverer#doGetMapValueType() */ @Override - public TypeInformation getMapValueType() { + protected TypeInformation doGetMapValueType() { if (Map.class.isAssignableFrom(getType())) { @@ -141,18 +141,13 @@ public boolean isAssignableFrom(TypeInformation target) { return true; } - /* + /* * (non-Javadoc) - * @see org.springframework.data.util.TypeDiscoverer#getComponentType() + * @see org.springframework.data.util.TypeDiscoverer#doGetComponentType() */ @Override - public TypeInformation getComponentType() { - - if (componentType == null) { - this.componentType = createInfo(type.getActualTypeArguments()[0]); - } - - return this.componentType; + protected TypeInformation doGetComponentType() { + return createInfo(type.getActualTypeArguments()[0]); } /* @@ -201,10 +196,14 @@ public String toString() { private boolean isResolvedCompletely() { + if (resolved != null) { + return resolved; + } + Type[] types = type.getActualTypeArguments(); if (types.length == 0) { - return false; + return cacheAndReturn(false); } for (Type type : types) { @@ -213,15 +212,21 @@ private boolean isResolvedCompletely() { if (info instanceof ParameterizedTypeInformation) { if (!((ParameterizedTypeInformation) info).isResolvedCompletely()) { - return false; + return cacheAndReturn(false); } } if (!(info instanceof ClassTypeInformation)) { - return false; + return cacheAndReturn(false); } } - return true; + return cacheAndReturn(true); + } + + private boolean cacheAndReturn(boolean resolved) { + + this.resolved = resolved; + return resolved; } } diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java index 1f77cc84f6..28aac57661 100644 --- a/src/main/java/org/springframework/data/util/TypeDiscoverer.java +++ b/src/main/java/org/springframework/data/util/TypeDiscoverer.java @@ -51,6 +51,12 @@ class TypeDiscoverer implements TypeInformation { private final Map, Type> typeVariableMap; private final Map> fieldTypes = new ConcurrentHashMap>(); + private boolean componentTypeResolved = false; + private TypeInformation componentType; + + private boolean valueTypeResolved = false; + private TypeInformation valueType; + private Class resolvedType; /** @@ -83,7 +89,7 @@ protected Map, Type> getTypeVariableMap() { * @param fieldType * @return */ - @SuppressWarnings({ "rawtypes", "unchecked" }) + @SuppressWarnings({ "rawtypes", "unchecked", "deprecation" }) protected TypeInformation createInfo(Type fieldType) { if (fieldType.equals(this.type)) { @@ -135,7 +141,7 @@ protected TypeInformation createInfo(Type fieldType) { * @param type * @return */ - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings({ "unchecked", "rawtypes", "deprecation" }) protected Class resolveType(Type type) { Map map = new HashMap(); @@ -313,6 +319,16 @@ public boolean isMap() { */ public TypeInformation getMapValueType() { + if (!valueTypeResolved) { + this.valueType = doGetMapValueType(); + this.valueTypeResolved = true; + } + + return this.valueType; + } + + protected TypeInformation doGetMapValueType() { + if (isMap()) { return getTypeArgument(Map.class, 1); } @@ -345,7 +361,17 @@ public boolean isCollectionLike() { * (non-Javadoc) * @see org.springframework.data.util.TypeInformation#getComponentType() */ - public TypeInformation getComponentType() { + public final TypeInformation getComponentType() { + + if (!componentTypeResolved) { + this.componentType = doGetComponentType(); + this.componentTypeResolved = true; + } + + return this.componentType; + } + + protected TypeInformation doGetComponentType() { Class rawType = getType(); diff --git a/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java b/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java index d498afc167..844bea797b 100644 --- a/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java +++ b/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java @@ -341,6 +341,18 @@ public void resolvesNestedGenericsToConcreteType() { assertThat(subSubType.getType(), is((Object) ConcreteSubSub.class)); } + /** + * @see DATACMNS-594 + */ + @Test + public void considersGenericsOfTypeBounds() { + + ClassTypeInformation customer = ClassTypeInformation.from(ConcreteRootIntermediate.class); + TypeInformation leafType = customer.getProperty("intermediate.content.intermediate.content"); + + assertThat(leafType.getType(), is((Object) Leaf.class)); + } + static class StringMapContainer extends MapContainer { } @@ -495,4 +507,24 @@ static class ConcreteSub extends GenericSub {} static class ConcreteSubSub extends GenericSubSub { String content; } + + // DATACMNS-594 + + static class Intermediate { + T content; + } + + static abstract class GenericRootIntermediate { + Intermediate intermediate; + } + + static abstract class GenericInnerIntermediate { + Intermediate intermediate; + } + + static class ConcreteRootIntermediate extends GenericRootIntermediate {} + + static class ConcreteInnerIntermediate extends GenericInnerIntermediate {} + + static class Leaf {} } From 806214e332b832113dca551b29ff367ad6bf958b Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 24 Nov 2014 22:38:48 +0100 Subject: [PATCH 14/59] DATACMNS-591 - RepositoryConfigurationDelegate now uses Environment and ResourceLoader everywhere. We now also use the Environment and ResourceLoader for the component scan to detect whether multiple Spring Data modules are on the classpath. --- .../config/RepositoryConfigurationDelegate.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 237f37c6fa..3af8983ce8 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java @@ -159,12 +159,13 @@ public List registerRepositoriesIn(BeanDefinitionRegist */ private boolean multipleStoresDetected() { - ClassPathScanningCandidateComponentProvider scanner = new ClassPathScanningCandidateComponentProvider(false, - environment); + ClassPathScanningCandidateComponentProvider scanner = new ClassPathScanningCandidateComponentProvider(false); + scanner.setEnvironment(environment); + scanner.setResourceLoader(resourceLoader); scanner.addIncludeFilter(new LenientAssignableTypeFilter(RepositoryFactorySupport.class)); - int numberOfModulesFound = scanner.findCandidateComponents(MODULE_DETECTION_PACKAGE).size(); - if (numberOfModulesFound > 1) { + if (scanner.findCandidateComponents(MODULE_DETECTION_PACKAGE).size() > 1) { + LOGGER.debug(MULTIPLE_MODULES); return true; } From 447aca6e9c102e099618c83e3b43ffd1d639e579 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 26 Nov 2014 09:10:00 +0100 Subject: [PATCH 15/59] DATACMNS-601 - Fixes for most of the SonarQube warnings. --- .../auditing/AnnotationAuditingMetadata.java | 2 +- .../auditing/AuditableBeanWrapperFactory.java | 2 +- .../AuditingHandlerBeanDefinitionParser.java | 4 +-- .../data/convert/CollectionFactory.java | 2 +- .../convert/SimpleTypeInformationMapper.java | 6 ++-- .../data/domain/jaxb/SpringDataJaxb.java | 4 ++- .../org/springframework/data/geo/GeoPage.java | 30 +++++++++++++++++++ .../data/mapping/PropertyPath.java | 8 ++--- .../DefaultPersistentPropertyPath.java | 6 ++-- .../AnnotationBasedPersistentProperty.java | 7 +++-- .../querydsl/QueryDslPredicateExecutor.java | 4 +-- .../RepositoryConfigurationDelegate.java | 6 ++-- .../config/RepositoryConfigurationUtils.java | 2 +- .../support/PersistentEntityInformation.java | 2 +- .../data/repository/query/Parameters.java | 2 +- .../data/repository/query/QueryMethod.java | 6 ++-- .../data/repository/support/Repositories.java | 16 +++++----- .../util/QueryExecutionConverters.java | 4 +-- .../AnnotationDetectionMethodCallback.java | 8 ++--- .../DirectFieldAccessFallbackBeanWrapper.java | 2 +- .../util/ParameterizedTypeInformation.java | 8 ++--- .../data/util/ReflectionUtils.java | 2 +- .../data/util/TypeDiscoverer.java | 22 +++++++------- .../data/web/SpringDataAnnotationUtils.java | 2 +- 24 files changed, 94 insertions(+), 63 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java b/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java index 40b624b5a5..69319ac261 100644 --- a/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java +++ b/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java @@ -39,7 +39,7 @@ * @author Oliver Gierke * @since 1.5 */ -class AnnotationAuditingMetadata { +final class AnnotationAuditingMetadata { private static final AnnotationFieldFilter CREATED_BY_FILTER = new AnnotationFieldFilter(CreatedBy.class); private static final AnnotationFieldFilter CREATED_DATE_FILTER = new AnnotationFieldFilter(CreatedDate.class); diff --git a/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java index 7fea361e45..e90bf799de 100644 --- a/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java @@ -115,7 +115,7 @@ public void setLastModifiedDate(Calendar value) { * @author Oliver Gierke * @since 1.8 */ - static abstract class DateConvertingAuditableBeanWrapper implements AuditableBeanWrapper { + abstract static class DateConvertingAuditableBeanWrapper implements AuditableBeanWrapper { private static final boolean IS_JODA_TIME_PRESENT = ClassUtils.isPresent("org.joda.time.DateTime", ReflectionAuditingBeanWrapper.class.getClassLoader()); diff --git a/src/main/java/org/springframework/data/auditing/config/AuditingHandlerBeanDefinitionParser.java b/src/main/java/org/springframework/data/auditing/config/AuditingHandlerBeanDefinitionParser.java index 1348d148d5..dfdcad44a5 100644 --- a/src/main/java/org/springframework/data/auditing/config/AuditingHandlerBeanDefinitionParser.java +++ b/src/main/java/org/springframework/data/auditing/config/AuditingHandlerBeanDefinitionParser.java @@ -19,7 +19,6 @@ import org.springframework.aop.framework.ProxyFactoryBean; import org.springframework.aop.target.LazyInitTargetSource; -import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; @@ -110,8 +109,7 @@ protected void doParse(Element element, BeanDefinitionBuilder builder) { * @see org.springframework.beans.factory.xml.AbstractBeanDefinitionParser#resolveId(org.w3c.dom.Element, org.springframework.beans.factory.support.AbstractBeanDefinition, org.springframework.beans.factory.xml.ParserContext) */ @Override - protected String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext) - throws BeanDefinitionStoreException { + protected String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext) { this.resolvedBeanName = super.resolveId(element, definition, parserContext); return resolvedBeanName; diff --git a/src/main/java/org/springframework/data/convert/CollectionFactory.java b/src/main/java/org/springframework/data/convert/CollectionFactory.java index 28591a04aa..059735e198 100644 --- a/src/main/java/org/springframework/data/convert/CollectionFactory.java +++ b/src/main/java/org/springframework/data/convert/CollectionFactory.java @@ -28,7 +28,7 @@ * * @author Oliver Gierke */ -public class CollectionFactory { +public abstract class CollectionFactory { private CollectionFactory() {} diff --git a/src/main/java/org/springframework/data/convert/SimpleTypeInformationMapper.java b/src/main/java/org/springframework/data/convert/SimpleTypeInformationMapper.java index 4e0097ba4f..9fe19ba6d1 100644 --- a/src/main/java/org/springframework/data/convert/SimpleTypeInformationMapper.java +++ b/src/main/java/org/springframework/data/convert/SimpleTypeInformationMapper.java @@ -33,7 +33,7 @@ public class SimpleTypeInformationMapper implements TypeInformationMapper { public static final SimpleTypeInformationMapper INSTANCE = new SimpleTypeInformationMapper(); - private static final Map> cache = new ConcurrentHashMap>(); + private static final Map> CACHE = new ConcurrentHashMap>(); /** * Returns the {@link TypeInformation} that shall be used when the given {@link String} value is found as type hint. @@ -56,7 +56,7 @@ public ClassTypeInformation resolveTypeFrom(Object alias) { return null; } - ClassTypeInformation information = cache.get(value); + ClassTypeInformation information = CACHE.get(value); if (information != null) { return information; @@ -69,7 +69,7 @@ public ClassTypeInformation resolveTypeFrom(Object alias) { } if (information != null) { - cache.put(value, information); + CACHE.put(value, information); } return information; diff --git a/src/main/java/org/springframework/data/domain/jaxb/SpringDataJaxb.java b/src/main/java/org/springframework/data/domain/jaxb/SpringDataJaxb.java index 7b86ababd5..ec8d0b7079 100644 --- a/src/main/java/org/springframework/data/domain/jaxb/SpringDataJaxb.java +++ b/src/main/java/org/springframework/data/domain/jaxb/SpringDataJaxb.java @@ -44,10 +44,12 @@ * * @author Oliver Gierke */ -public class SpringDataJaxb { +public abstract class SpringDataJaxb { public static final String NAMESPACE = "http://www.springframework.org/schema/data/jaxb"; + private SpringDataJaxb() {} + /** * The DTO for {@link Pageable}s/{@link PageRequest}s. * diff --git a/src/main/java/org/springframework/data/geo/GeoPage.java b/src/main/java/org/springframework/data/geo/GeoPage.java index e0603360e0..f0d25489b8 100644 --- a/src/main/java/org/springframework/data/geo/GeoPage.java +++ b/src/main/java/org/springframework/data/geo/GeoPage.java @@ -18,6 +18,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; +import org.springframework.util.ObjectUtils; /** * Custom {@link Page} to carry the average distance retrieved from the {@link GeoResults} the {@link GeoPage} is set up @@ -66,4 +67,33 @@ public GeoPage(GeoResults results, Pageable pageable, long total) { public Distance getAverageDistance() { return averageDistance; } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.PageImpl#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + + if (this == obj) { + return true; + } + + if (!(obj instanceof GeoPage)) { + return false; + } + + GeoPage that = (GeoPage) obj; + + return super.equals(obj) && ObjectUtils.nullSafeEquals(this.averageDistance, that.averageDistance); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.PageImpl#hashCode() + */ + @Override + public int hashCode() { + return super.hashCode() + ObjectUtils.nullSafeHashCode(this.averageDistance); + } } diff --git a/src/main/java/org/springframework/data/mapping/PropertyPath.java b/src/main/java/org/springframework/data/mapping/PropertyPath.java index 79d133d8c1..95bc57afd5 100644 --- a/src/main/java/org/springframework/data/mapping/PropertyPath.java +++ b/src/main/java/org/springframework/data/mapping/PropertyPath.java @@ -69,15 +69,15 @@ public class PropertyPath implements Iterable { Assert.notNull(owningType); String propertyName = name.matches(ALL_UPPERCASE) ? name : StringUtils.uncapitalize(name); - TypeInformation type = owningType.getProperty(propertyName); + TypeInformation propertyType = owningType.getProperty(propertyName); - if (type == null) { + if (propertyType == null) { throw new PropertyReferenceException(propertyName, owningType, base); } this.owningType = owningType; - this.isCollection = type.isCollectionLike(); - this.type = type.getActualType(); + this.isCollection = propertyType.isCollectionLike(); + this.type = propertyType.getActualType(); this.name = propertyName; } diff --git a/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java b/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java index 94239f7551..64604f5406 100644 --- a/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java +++ b/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java @@ -158,7 +158,7 @@ public PersistentPropertyPath getExtensionForBaseOf(PersistentPropertyPath return this; } - List properties = new ArrayList(); + List result = new ArrayList(); Iterator iterator = iterator(); for (int i = 0; i < base.getLength(); i++) { @@ -166,10 +166,10 @@ public PersistentPropertyPath getExtensionForBaseOf(PersistentPropertyPath } while (iterator.hasNext()) { - properties.add(iterator.next()); + result.add(iterator.next()); } - return new DefaultPersistentPropertyPath(properties); + return new DefaultPersistentPropertyPath(result); } /* diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index 17efb3ebd3..36c1f66498 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -158,9 +158,10 @@ public String getSpelExpression() { @Override public boolean isTransient() { - if (isTransient == null) { - boolean isTransient = super.isTransient() || isAnnotationPresent(Transient.class); - this.isTransient = isTransient || isAnnotationPresent(Value.class) || isAnnotationPresent(Autowired.class); + if (this.isTransient == null) { + boolean potentiallyTransient = super.isTransient() || isAnnotationPresent(Transient.class); + this.isTransient = potentiallyTransient || isAnnotationPresent(Value.class) + || isAnnotationPresent(Autowired.class); } return this.isTransient; diff --git a/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java b/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java index 5160b79cfd..6963344520 100644 --- a/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java +++ b/src/main/java/org/springframework/data/querydsl/QueryDslPredicateExecutor.java @@ -15,7 +15,6 @@ */ package org.springframework.data.querydsl; -import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -35,7 +34,8 @@ public interface QueryDslPredicateExecutor { * * @param predicate * @return a single entity matching the given {@link Predicate} or {@literal null} if none was found. - * @throws IncorrectResultSizeDataAccessException if the predicate yields more than one result. + * @throws org.springframework.dao.IncorrectResultSizeDataAccessException if the predicate yields more than one + * result. */ T findOne(Predicate predicate); 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 3af8983ce8..57f9bce089 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java @@ -56,7 +56,7 @@ public class RepositoryConfigurationDelegate { private final RepositoryConfigurationSource configurationSource; private final ResourceLoader resourceLoader; private final Environment environment; - private final BeanNameGenerator generator; + private final BeanNameGenerator beanNameGenerator; private final boolean isXml; private final boolean inMultiStoreMode; @@ -81,7 +81,7 @@ public RepositoryConfigurationDelegate(RepositoryConfigurationSource configurati RepositoryBeanNameGenerator generator = new RepositoryBeanNameGenerator(); generator.setBeanClassLoader(resourceLoader.getClassLoader()); - this.generator = generator; + this.beanNameGenerator = generator; this.configurationSource = configurationSource; this.resourceLoader = resourceLoader; this.environment = defaultEnvironment(environment, resourceLoader); @@ -136,7 +136,7 @@ public List registerRepositoriesIn(BeanDefinitionRegist } AbstractBeanDefinition beanDefinition = definitionBuilder.getBeanDefinition(); - String beanName = generator.generateBeanName(beanDefinition, registry); + String beanName = beanNameGenerator.generateBeanName(beanDefinition, registry); if (LOGGER.isDebugEnabled()) { LOGGER.debug(REPOSITORY_REGISTRATION, extension.getModuleName(), beanName, diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationUtils.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationUtils.java index 7d30c4e895..cdbce7d3fb 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationUtils.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationUtils.java @@ -28,7 +28,7 @@ * * @author Oliver Gierke */ -public class RepositoryConfigurationUtils { +public abstract class RepositoryConfigurationUtils { private RepositoryConfigurationUtils() {} diff --git a/src/main/java/org/springframework/data/repository/core/support/PersistentEntityInformation.java b/src/main/java/org/springframework/data/repository/core/support/PersistentEntityInformation.java index 7501821887..005ead2ed2 100644 --- a/src/main/java/org/springframework/data/repository/core/support/PersistentEntityInformation.java +++ b/src/main/java/org/springframework/data/repository/core/support/PersistentEntityInformation.java @@ -23,7 +23,7 @@ /** * {@link EntityInformation} implementation that uses a {@link PersistentEntity} to obtain id type information and uses - * a {@link BeanWrapper} to access the property value if requested. + * a {@link org.springframework.data.mapping.IdentifierAccessor} to access the property value if requested. * * @author Oliver Gierke */ 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 d73c79886e..2965f0b3d4 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameters.java +++ b/src/main/java/org/springframework/data/repository/query/Parameters.java @@ -253,7 +253,7 @@ public T getBindableParameter(int bindableIndex) { * * @param method */ - private void assertEitherAllParamAnnotatedOrNone() { + private final void assertEitherAllParamAnnotatedOrNone() { boolean nameFound = false; int index = 0; 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 a15a9a20a1..f1c704f3a2 100644 --- a/src/main/java/org/springframework/data/repository/query/QueryMethod.java +++ b/src/main/java/org/springframework/data/repository/query/QueryMethod.java @@ -119,9 +119,7 @@ public Class getJavaType() { * @return */ public String getNamedQueryName() { - - Class domainClass = getDomainClass(); - return String.format("%s.%s", domainClass.getSimpleName(), method.getName()); + return String.format("%s.%s", getDomainClass().getSimpleName(), method.getName()); } /** @@ -181,7 +179,7 @@ public boolean isSliceQuery() { * * @return */ - public boolean isPageQuery() { + public final boolean isPageQuery() { Class returnType = method.getReturnType(); return org.springframework.util.ClassUtils.isAssignable(Page.class, returnType); diff --git a/src/main/java/org/springframework/data/repository/support/Repositories.java b/src/main/java/org/springframework/data/repository/support/Repositories.java index 3753756d63..ef9e0b67c3 100644 --- a/src/main/java/org/springframework/data/repository/support/Repositories.java +++ b/src/main/java/org/springframework/data/repository/support/Repositories.java @@ -45,7 +45,9 @@ public class Repositories implements Iterable> { static final Repositories NONE = new Repositories(); + private static final RepositoryFactoryInformation EMPTY_REPOSITORY_FACTORY_INFO = EmptyRepositoryFactoryInformation.INSTANCE; + private static final String DOMAIN_TYPE_MUST_NOT_BE_NULL = "Domain type must not be null!"; private final BeanFactory beanFactory; private final Map, String> repositoryBeanNames; @@ -102,7 +104,7 @@ private void populateRepositoryFactoryInformation(ListableBeanFactory factory) { */ public boolean hasRepositoryFor(Class domainClass) { - Assert.notNull(domainClass, "Domain class must not be null!"); + Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); return repositoryFactoryInfos.containsKey(domainClass); } @@ -115,7 +117,7 @@ public boolean hasRepositoryFor(Class domainClass) { */ public Object getRepositoryFor(Class domainClass) { - Assert.notNull(domainClass, "Domain class must not be null!"); + Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); String repositoryBeanName = repositoryBeanNames.get(domainClass); return repositoryBeanName == null || beanFactory == null ? null : beanFactory.getBean(repositoryBeanName); @@ -131,7 +133,7 @@ public Object getRepositoryFor(Class domainClass) { */ private RepositoryFactoryInformation getRepositoryFactoryInfoFor(Class domainClass) { - Assert.notNull(domainClass, "Domain class must not be null!"); + Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); RepositoryFactoryInformation repositoryInfo = repositoryFactoryInfos.get(ClassUtils .getUserClass(domainClass)); @@ -147,7 +149,7 @@ private RepositoryFactoryInformation getRepositoryFactoryI @SuppressWarnings("unchecked") public EntityInformation getEntityInformationFor(Class domainClass) { - Assert.notNull(domainClass, "Domain class must not be null!"); + Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); return (EntityInformation) getRepositoryFactoryInfoFor(domainClass).getEntityInformation(); } @@ -161,7 +163,7 @@ public EntityInformation getEntityInformationF */ public RepositoryInformation getRepositoryInformationFor(Class domainClass) { - Assert.notNull(domainClass, "Domain class must not be null!"); + Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); RepositoryFactoryInformation information = getRepositoryFactoryInfoFor(domainClass); return information == EMPTY_REPOSITORY_FACTORY_INFO ? null : information.getRepositoryInformation(); @@ -177,7 +179,7 @@ public RepositoryInformation getRepositoryInformationFor(Class domainClass) { */ public PersistentEntity getPersistentEntity(Class domainClass) { - Assert.notNull(domainClass, "Domain class must not be null!"); + Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); return getRepositoryFactoryInfoFor(domainClass).getPersistentEntity(); } @@ -189,7 +191,7 @@ public RepositoryInformation getRepositoryInformationFor(Class domainClass) { */ public List getQueryMethodsFor(Class domainClass) { - Assert.notNull(domainClass, "Domain class must not be null!"); + Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); return getRepositoryFactoryInfoFor(domainClass).getQueryMethods(); } diff --git a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java index f0104e2362..c57c7f7ac7 100644 --- a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java +++ b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java @@ -38,14 +38,14 @@ * @author Oliver Gierke * @since 1.8 */ -public class QueryExecutionConverters { +public abstract class QueryExecutionConverters { private static final boolean GUAVA_PRESENT = ClassUtils.isPresent("com.google.common.base.Optional", QueryExecutionConverters.class.getClassLoader()); private static final boolean JDK_PRESENT = ClassUtils.isPresent("java.util.Optional", QueryExecutionConverters.class.getClassLoader()); - private static Set> WRAPPER_TYPES = new HashSet>(); + private static final Set> WRAPPER_TYPES = new HashSet>(); static { diff --git a/src/main/java/org/springframework/data/util/AnnotationDetectionMethodCallback.java b/src/main/java/org/springframework/data/util/AnnotationDetectionMethodCallback.java index 2c5b46d38c..82c1ac6fa8 100644 --- a/src/main/java/org/springframework/data/util/AnnotationDetectionMethodCallback.java +++ b/src/main/java/org/springframework/data/util/AnnotationDetectionMethodCallback.java @@ -94,16 +94,16 @@ public void doWith(Method method) throws IllegalArgumentException, IllegalAccess return; } - A annotation = AnnotationUtils.findAnnotation(method, annotationType); + A foundAnnotation = AnnotationUtils.findAnnotation(method, annotationType); - if (annotation != null) { + if (foundAnnotation != null) { if (foundMethod != null && enforceUniqueness) { - throw new IllegalStateException(String.format(MULTIPLE_FOUND, annotation.getClass().getName(), foundMethod, + throw new IllegalStateException(String.format(MULTIPLE_FOUND, foundAnnotation.getClass().getName(), foundMethod, method)); } - this.annotation = annotation; + this.annotation = foundAnnotation; this.foundMethod = method; } } diff --git a/src/main/java/org/springframework/data/util/DirectFieldAccessFallbackBeanWrapper.java b/src/main/java/org/springframework/data/util/DirectFieldAccessFallbackBeanWrapper.java index 5234559ca0..28f1710b9e 100644 --- a/src/main/java/org/springframework/data/util/DirectFieldAccessFallbackBeanWrapper.java +++ b/src/main/java/org/springframework/data/util/DirectFieldAccessFallbackBeanWrapper.java @@ -77,7 +77,7 @@ public void setPropertyValue(String propertyName, Object value) { if (field == null) { throw new NotWritablePropertyException(getWrappedClass(), propertyName, - "Could not find field for property during fallback access!"); + "Could not find field for property during fallback access!", e); } makeAccessible(field); diff --git a/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java b/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java index 61e0554fb6..01a0a2647b 100644 --- a/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ParameterizedTypeInformation.java @@ -200,15 +200,15 @@ private boolean isResolvedCompletely() { return resolved; } - Type[] types = type.getActualTypeArguments(); + Type[] typeArguments = type.getActualTypeArguments(); - if (types.length == 0) { + if (typeArguments.length == 0) { return cacheAndReturn(false); } - for (Type type : types) { + for (Type typeArgument : typeArguments) { - TypeInformation info = createInfo(type); + TypeInformation info = createInfo(typeArgument); if (info instanceof ParameterizedTypeInformation) { if (!((ParameterizedTypeInformation) info).isResolvedCompletely()) { diff --git a/src/main/java/org/springframework/data/util/ReflectionUtils.java b/src/main/java/org/springframework/data/util/ReflectionUtils.java index b1255f5eb7..3570c9aa90 100644 --- a/src/main/java/org/springframework/data/util/ReflectionUtils.java +++ b/src/main/java/org/springframework/data/util/ReflectionUtils.java @@ -32,7 +32,7 @@ * @author Oliver Gierke * @since 1.5 */ -public class ReflectionUtils { +public abstract class ReflectionUtils { private ReflectionUtils() {} diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java index 28aac57661..04cf28818f 100644 --- a/src/main/java/org/springframework/data/util/TypeDiscoverer.java +++ b/src/main/java/org/springframework/data/util/TypeDiscoverer.java @@ -159,8 +159,8 @@ public List> getParameterTypes(Constructor constructor) { Assert.notNull(constructor); List> result = new ArrayList>(); - for (Type type : constructor.getGenericParameterTypes()) { - result.add(createInfo(type)); + for (Type parameterType : constructor.getGenericParameterTypes()) { + result.add(createInfo(parameterType)); } return result; @@ -201,14 +201,14 @@ public TypeInformation getProperty(String fieldname) { */ private TypeInformation getPropertyInformation(String fieldname) { - Class type = getType(); - Field field = ReflectionUtils.findField(type, fieldname); + Class rawType = getType(); + Field field = ReflectionUtils.findField(rawType, fieldname); if (field != null) { return createInfo(field.getGenericType()); } - PropertyDescriptor descriptor = findPropertyDescriptor(type, fieldname); + PropertyDescriptor descriptor = findPropertyDescriptor(rawType, fieldname); return descriptor == null ? null : createInfo(getGenericType(descriptor)); } @@ -417,8 +417,8 @@ public List> getParameterTypes(Method method) { Type[] parameterTypes = method.getGenericParameterTypes(); List> result = new ArrayList>(parameterTypes.length); - for (Type type : parameterTypes) { - result.add(createInfo(type)); + for (Type parameterType : parameterTypes) { + result.add(createInfo(parameterType)); } return result; @@ -430,9 +430,9 @@ public List> getParameterTypes(Method method) { */ public TypeInformation getSuperTypeInformation(Class superType) { - Class type = getType(); + Class rawType = getType(); - if (!superType.isAssignableFrom(type)) { + if (!superType.isAssignableFrom(rawType)) { return null; } @@ -442,11 +442,11 @@ public TypeInformation getSuperTypeInformation(Class superType) { List candidates = new ArrayList(); - Type genericSuperclass = type.getGenericSuperclass(); + Type genericSuperclass = rawType.getGenericSuperclass(); if (genericSuperclass != null) { candidates.add(genericSuperclass); } - candidates.addAll(Arrays.asList(type.getGenericInterfaces())); + candidates.addAll(Arrays.asList(rawType.getGenericInterfaces())); for (Type candidate : candidates) { diff --git a/src/main/java/org/springframework/data/web/SpringDataAnnotationUtils.java b/src/main/java/org/springframework/data/web/SpringDataAnnotationUtils.java index 1bcbc45520..0a4905913d 100644 --- a/src/main/java/org/springframework/data/web/SpringDataAnnotationUtils.java +++ b/src/main/java/org/springframework/data/web/SpringDataAnnotationUtils.java @@ -32,7 +32,7 @@ * * @author Oliver Gierke */ -class SpringDataAnnotationUtils { +abstract class SpringDataAnnotationUtils { private SpringDataAnnotationUtils() {} From 0cb1cca58827d763a857ae447dd2bfd3b0deb589 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 30 Nov 2014 17:58:16 +0100 Subject: [PATCH 16/59] DATACMNS-609 - Improved bean definition registration for repository configuration. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bean definitions that were registered for a repository configuration setup we registered once for every usage of the repository configuration element (annotation or XML). This caused multiple registrations of the very same bean definition which - as in case of the RepositoryInterfaceAwareBeanPostProcessor - apparently leads to performance problems in the container startup. Feedback for the latter claim is already asked for but we improved the registration setup nonetheless. Introduced a registerIfNotAlreadyRegistered(…) method on RepositoryConfigurationExtensionSupport to allow easy registration of to-b-registered-once-and-only-once beans. We now hint towards the newly introduced method in registerWithSourceAndGeneratedBeanName(…). --- ...positoryConfigurationExtensionSupport.java | 35 +++++++++++++++---- ...onfigurationExtensionSupportUnitTests.java | 28 +++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java index 25b553a69b..0e11b28837 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java @@ -118,7 +118,7 @@ public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConf AbstractBeanDefinition definition = BeanDefinitionBuilder.rootBeanDefinition(REPOSITORY_INTERFACE_POST_PROCESSOR) .getBeanDefinition(); - registerWithSourceAndGeneratedBeanName(registry, definition, configurationSource.getSource()); + registerIfNotAlreadyRegistered(definition, registry, REPOSITORY_INTERFACE_POST_PROCESSOR, configurationSource); } /** @@ -169,12 +169,15 @@ protected Collection> getIdentifyingTypes() { /** * Sets the given source on the given {@link AbstractBeanDefinition} and registers it inside the given - * {@link BeanDefinitionRegistry}. + * {@link BeanDefinitionRegistry}. For {@link BeanDefinition}s to be registerd once-and-only-once for all + * configuration elements (annotation or XML), prefer calling + * {@link #registerIfNotAlreadyRegistered(AbstractBeanDefinition, BeanDefinitionRegistry, String, Object)} with a + * dedicated bean name to avoid the bead definition being registered multiple times. * * - * @param registry - * @param bean - * @param source - * @return + * @param registry must not be {@literal null}. + * @param bean must not be {@literal null}. + * @param source must not be {@literal null}. + * @return the bean name generated for the given {@link BeanDefinition} */ public static String registerWithSourceAndGeneratedBeanName(BeanDefinitionRegistry registry, AbstractBeanDefinition bean, Object source) { @@ -187,6 +190,26 @@ public static String registerWithSourceAndGeneratedBeanName(BeanDefinitionRegist return beanName; } + /** + * Registers the given {@link AbstractBeanDefinition} with the given registry with the given bean name unless the + * registry already contains a bean with that name. + * + * @param bean must not be {@literal null}. + * @param registry must not be {@literal null}. + * @param beanName must not be {@literal null} or empty. + * @param source must not be {@literal null}. + */ + public static void registerIfNotAlreadyRegistered(AbstractBeanDefinition bean, BeanDefinitionRegistry registry, + String beanName, Object source) { + + if (registry.containsBeanDefinition(beanName)) { + return; + } + + bean.setSource(source); + registry.registerBeanDefinition(beanName, bean); + } + /** * Returns whether the given {@link BeanDefinitionRegistry} already contains a bean of the given type assuming the * bean name has been autogenerated. diff --git a/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupportUnitTests.java b/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupportUnitTests.java index 468fe8d57d..5bee74d04e 100644 --- a/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupportUnitTests.java +++ b/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupportUnitTests.java @@ -23,7 +23,12 @@ import java.util.Collections; import org.junit.Test; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.context.annotation.Primary; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.io.DefaultResourceLoader; +import org.springframework.core.type.AnnotationMetadata; +import org.springframework.core.type.StandardAnnotationMetadata; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.RepositoryFactorySupport; @@ -60,6 +65,26 @@ public void considersRepositoryInterfaceExtendingStoreInterfaceStrictMatch() { assertThat(extension.isStrictRepositoryCandidate(ExtendingInterface.class), is(true)); } + /** + * @see DATACMNS-609 + */ + @Test + public void registersRepositoryInterfaceAwareBeanPostProcessorOnlyOnceForMultipleConfigurations() { + + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + AnnotationMetadata annotationMetadata = new StandardAnnotationMetadata(SampleConfiguration.class, true); + + DefaultResourceLoader resourceLoader = new DefaultResourceLoader(); + StandardEnvironment environment = new StandardEnvironment(); + AnnotationRepositoryConfigurationSource configurationSource = new AnnotationRepositoryConfigurationSource( + annotationMetadata, EnableRepositories.class, resourceLoader, environment); + + extension.registerBeansForRoot(beanFactory, configurationSource); + extension.registerBeansForRoot(beanFactory, configurationSource); + + assertThat(beanFactory.getBeanDefinitionCount(), is(1)); + } + static class SampleRepositoryConfigurationExtension extends RepositoryConfigurationExtensionSupport { @Override @@ -95,4 +120,7 @@ interface PlainTypeRepository extends Repository {} interface StoreInterface {} interface ExtendingInterface extends StoreInterface, Repository {} + + @EnableRepositories + static class SampleConfiguration {} } From 13698073e2800c97a8ff45c4ec99df254d5845e5 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 4 Dec 2014 20:25:11 +0100 Subject: [PATCH 17/59] DATACMNS-610 - Extracted QueryExecutionResultHandler from RepositoryFactorySupport. To be able to verify the conversion of List based query execution results into Sets I extracted a QueryExecutionResultHandler that applies the already implemented handling for Optional types and eventually general prost processing in case the return type declared at the repository query method doesn't match the returned value. Added additional unit tests for Optional handling. --- .../support/QueryExecutionResultHandler.java | 78 +++++++++++ .../support/RepositoryFactorySupport.java | 34 +---- .../QueryExecutionResultHandlerUnitTests.java | 123 ++++++++++++++++++ 3 files changed, 205 insertions(+), 30 deletions(-) create mode 100644 src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java create mode 100644 src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java diff --git a/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java b/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java new file mode 100644 index 0000000000..b1c65a29ab --- /dev/null +++ b/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java @@ -0,0 +1,78 @@ +/* + * Copyright 2014 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.core.support; + +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.core.convert.support.GenericConversionService; +import org.springframework.data.repository.util.NullableWrapper; +import org.springframework.data.repository.util.QueryExecutionConverters; + +/** + * Simple domain service to convert query results into a dedicated type. + * + * @author Oliver Gierke + */ +class QueryExecutionResultHandler { + + private static final TypeDescriptor WRAPPER_TYPE = TypeDescriptor.valueOf(NullableWrapper.class); + + private final GenericConversionService conversionService; + + /** + * Creates a new {@link QueryExecutionResultHandler}. + */ + public QueryExecutionResultHandler() { + + GenericConversionService conversionService = new DefaultConversionService(); + QueryExecutionConverters.registerConvertersIn(conversionService); + + this.conversionService = conversionService; + } + + /** + * Post-processes the given result of a query invocation to the given type. + * + * @param result can be {@literal null}. + * @param returnTypeDesciptor can be {@literal null}, if so, no conversion is performed. + * @return + */ + public Object postProcessInvocationResult(Object result, TypeDescriptor returnTypeDesciptor) { + + if (returnTypeDesciptor == null) { + return result; + } + + Class expectedReturnType = returnTypeDesciptor.getType(); + + if (result != null && expectedReturnType.isInstance(result)) { + return result; + } + + if (QueryExecutionConverters.supports(expectedReturnType) + && conversionService.canConvert(WRAPPER_TYPE, returnTypeDesciptor) + && !conversionService.canBypassConvert(WRAPPER_TYPE, TypeDescriptor.valueOf(expectedReturnType))) { + return conversionService.convert(new NullableWrapper(result), expectedReturnType); + } + + if (result == null) { + return null; + } + + return conversionService.canConvert(result.getClass(), expectedReturnType) ? conversionService.convert(result, + expectedReturnType) : result; + } +} diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java index 29299f885c..42dce390ca 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java @@ -35,8 +35,6 @@ import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.support.DefaultConversionService; -import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.EntityInformation; import org.springframework.data.repository.core.NamedQueries; @@ -49,8 +47,6 @@ import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.util.ClassUtils; -import org.springframework.data.repository.util.NullableWrapper; -import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -63,7 +59,6 @@ */ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware { - private static final TypeDescriptor WRAPPER_TYPE = TypeDescriptor.valueOf(NullableWrapper.class); private static final boolean IS_JAVA_8 = org.springframework.util.ClassUtils.isPresent("java.util.Optional", RepositoryFactorySupport.class.getClassLoader()); @@ -316,7 +311,7 @@ public class QueryExecutorMethodInterceptor implements MethodInterceptor { private final Object customImplementation; private final RepositoryInformation repositoryInformation; - private final GenericConversionService conversionService; + private final QueryExecutionResultHandler resultHandler; private final Object target; /** @@ -329,10 +324,7 @@ public QueryExecutorMethodInterceptor(RepositoryInformation repositoryInformatio Assert.notNull(repositoryInformation, "RepositoryInformation must not be null!"); Assert.notNull(target, "Target must not be null!"); - DefaultConversionService conversionService = new DefaultConversionService(); - QueryExecutionConverters.registerConvertersIn(conversionService); - this.conversionService = conversionService; - + this.resultHandler = new QueryExecutionResultHandler(); this.repositoryInformation = repositoryInformation; this.customImplementation = customImplementation; this.target = target; @@ -380,30 +372,12 @@ public Object invoke(MethodInvocation invocation) throws Throwable { Object result = doInvoke(invocation); - Method method = invocation.getMethod(); - // Looking up the TypeDescriptor for the return type - yes, this way o.O + Method method = invocation.getMethod(); MethodParameter parameter = new MethodParameter(method, -1); TypeDescriptor methodReturnTypeDescriptor = TypeDescriptor.nested(parameter, 0); - Class expectedReturnType = method.getReturnType(); - - if (result != null && expectedReturnType.isInstance(result)) { - return result; - } - - if (QueryExecutionConverters.supports(expectedReturnType) - && conversionService.canConvert(WRAPPER_TYPE, methodReturnTypeDescriptor) - && !conversionService.canBypassConvert(WRAPPER_TYPE, TypeDescriptor.valueOf(expectedReturnType))) { - return conversionService.convert(new NullableWrapper(result), expectedReturnType); - } - - if (result == null) { - return null; - } - - return conversionService.canConvert(result.getClass(), expectedReturnType) ? conversionService.convert(result, - expectedReturnType) : result; + return resultHandler.postProcessInvocationResult(result, methodReturnTypeDescriptor); } private Object doInvoke(MethodInvocation invocation) throws Throwable { diff --git a/src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java new file mode 100644 index 0000000000..773f793779 --- /dev/null +++ b/src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java @@ -0,0 +1,123 @@ +/* + * Copyright 2014 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.core.support; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import org.junit.Test; +import org.springframework.core.MethodParameter; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.data.repository.Repository; + +/** + * Unit tests for {@link QueryExecutionResultHandler}. + * + * @author Oliver Gierke + */ +public class QueryExecutionResultHandlerUnitTests { + + QueryExecutionResultHandler handler = new QueryExecutionResultHandler(); + + /** + * @see DATACMNS-610 + */ + @Test + public void convertsListsToSet() throws Exception { + + TypeDescriptor descriptor = getTypeDescriptorFor("set"); + List source = Collections.singletonList(new Entity()); + + assertThat(handler.postProcessInvocationResult(source, descriptor), is(instanceOf(Set.class))); + } + + /** + * @see DATACMNS-483 + */ + @Test + public void turnsNullIntoJdk8Optional() throws Exception { + + Object result = handler.postProcessInvocationResult(null, getTypeDescriptorFor("jdk8Optional")); + assertThat(result, is((Object) Optional.empty())); + } + + /** + * @see DATACMNS-483 + */ + @Test + @SuppressWarnings("unchecked") + public void wrapsValueIntoJdk8Optional() throws Exception { + + Entity entity = new Entity(); + + Object result = handler.postProcessInvocationResult(entity, getTypeDescriptorFor("jdk8Optional")); + assertThat(result, is(instanceOf(Optional.class))); + + Optional optional = (Optional) result; + assertThat(optional, is(Optional.of(entity))); + } + + /** + * @see DATACMNS-483 + */ + @Test + public void turnsNullIntoGuavaOptional() throws Exception { + + Object result = handler.postProcessInvocationResult(null, getTypeDescriptorFor("guavaOptional")); + assertThat(result, is((Object) com.google.common.base.Optional.absent())); + } + + /** + * @see DATACMNS-483 + */ + @Test + @SuppressWarnings("unchecked") + public void wrapsValueIntoGuavaOptional() throws Exception { + + Entity entity = new Entity(); + + Object result = handler.postProcessInvocationResult(entity, getTypeDescriptorFor("guavaOptional")); + assertThat(result, is(instanceOf(com.google.common.base.Optional.class))); + + com.google.common.base.Optional optional = (com.google.common.base.Optional) result; + assertThat(optional, is(com.google.common.base.Optional.of(entity))); + } + + private static TypeDescriptor getTypeDescriptorFor(String methodName) throws Exception { + + Method method = Sample.class.getMethod(methodName); + MethodParameter parameter = new MethodParameter(method, -1); + + return TypeDescriptor.nested(parameter, 0); + } + + static interface Sample extends Repository { + + Set set(); + + Optional jdk8Optional(); + + com.google.common.base.Optional guavaOptional(); + } + + static class Entity {} +} From 1151ec88cd2547f84e847bb9a63cba2de57417fb Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 8 Dec 2014 17:51:17 +0100 Subject: [PATCH 18/59] DATACMNS-611 - Improved cache lookups in RepositoryInterfaceAwareBeanPostProcessor. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactored predictBeanType(…) to only look up the predicted cached type once. Related pull request: #108. --- .../RepositoryInterfaceAwareBeanPostProcessor.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessor.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessor.java index 17ca5d6bee..b42e51aa23 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessor.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-2014 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. @@ -70,15 +70,15 @@ public Class predictBeanType(Class beanClass, String beanName) { return null; } - BeanDefinition definition = context.getBeanDefinition(beanName); - PropertyValue value = definition.getPropertyValues().getPropertyValue("repositoryInterface"); - Class resolvedBeanClass = cache.get(beanName); - if (cache.containsKey(beanName)) { - return cache.get(beanName); + if (resolvedBeanClass != null) { + return resolvedBeanClass == Void.class ? null : resolvedBeanClass; } + BeanDefinition definition = context.getBeanDefinition(beanName); + PropertyValue value = definition.getPropertyValues().getPropertyValue("repositoryInterface"); + resolvedBeanClass = getClassForPropertyValue(value, beanName); cache.put(beanName, resolvedBeanClass); From 3324b187dd0ab8e24bc20910ac2eea553337ec96 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 16 Dec 2014 12:40:48 +0100 Subject: [PATCH 19/59] DATACMNS-615 - PageImpl now rejects a total less than the amount of items given. PageImpl now makes sure that the total given to the constructor is never less than then number of items given to make sure we do not mask broken count calculation by creating an actually invalid instance. Related ticket: DATAMONGO-1120. --- .../java/org/springframework/data/domain/PageImpl.java | 5 +++++ .../springframework/data/domain/PageImplUnitTests.java | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/main/java/org/springframework/data/domain/PageImpl.java b/src/main/java/org/springframework/data/domain/PageImpl.java index cb6a296a9d..71eece84df 100644 --- a/src/main/java/org/springframework/data/domain/PageImpl.java +++ b/src/main/java/org/springframework/data/domain/PageImpl.java @@ -17,6 +17,8 @@ import java.util.List; +import org.springframework.util.Assert; + /** * Basic {@code Page} implementation. * @@ -39,6 +41,9 @@ public class PageImpl extends Chunk implements Page { public PageImpl(List content, Pageable pageable, long total) { super(content, pageable); + + Assert.isTrue(total >= content.size(), "Total must not be less than the number of elements given!"); + this.total = total; } diff --git a/src/test/java/org/springframework/data/domain/PageImplUnitTests.java b/src/test/java/org/springframework/data/domain/PageImplUnitTests.java index f754e56732..189af6cbc2 100644 --- a/src/test/java/org/springframework/data/domain/PageImplUnitTests.java +++ b/src/test/java/org/springframework/data/domain/PageImplUnitTests.java @@ -126,4 +126,12 @@ public void returnsCorrectTotalPages() { assertThat(page.hasNext(), is(false)); assertThat(page.hasPrevious(), is(false)); } + + /** + * @see DATACMNS-615 + */ + @Test(expected = IllegalArgumentException.class) + public void rejectsTotalLessThanContentLength() { + new PageImpl(Arrays.asList("foo", "bar"), new PageRequest(0, 10), 1); + } } From 846ab10b019fe9add592c9111c12329880423f14 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 17 Dec 2014 16:15:07 +0100 Subject: [PATCH 20/59] DATACMNS-616 - Fixed field value lookup for private fields in AnnotationDetectionFieldCallback. We now make the field detected by the callback accessible so that the value lookup doesn't fail for private fields. Added a few more unit tests to verify behavior. --- .../AnnotationDetectionFieldCallback.java | 7 +- ...tationDetectionFieldCallbackUnitTests.java | 77 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/springframework/data/util/AnnotationDetectionFieldCallbackUnitTests.java diff --git a/src/main/java/org/springframework/data/util/AnnotationDetectionFieldCallback.java b/src/main/java/org/springframework/data/util/AnnotationDetectionFieldCallback.java index be52b13260..d2604fbd38 100755 --- a/src/main/java/org/springframework/data/util/AnnotationDetectionFieldCallback.java +++ b/src/main/java/org/springframework/data/util/AnnotationDetectionFieldCallback.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2014 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. @@ -23,7 +23,7 @@ import org.springframework.util.ReflectionUtils.FieldCallback; /** - * A {@link FieldCallback} that will inspect each field for a given annotation. Thie fields type can then be accessed + * A {@link FieldCallback} that will inspect each field for a given annotation. This field's type can then be accessed * afterwards. * * @author Oliver Gierke @@ -55,8 +55,11 @@ public void doWith(Field field) throws IllegalArgumentException, IllegalAccessEx } Annotation annotation = field.getAnnotation(annotationType); + if (annotation != null) { + this.field = field; + ReflectionUtils.makeAccessible(this.field); } } diff --git a/src/test/java/org/springframework/data/util/AnnotationDetectionFieldCallbackUnitTests.java b/src/test/java/org/springframework/data/util/AnnotationDetectionFieldCallbackUnitTests.java new file mode 100644 index 0000000000..d5b73af2e7 --- /dev/null +++ b/src/test/java/org/springframework/data/util/AnnotationDetectionFieldCallbackUnitTests.java @@ -0,0 +1,77 @@ +/* + * Copyright 2014 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.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.util.ReflectionUtils; + +/** + * Unit tests for {@link AnnotationDetectionFieldCallback}. + * + * @author Oliver Gierke + */ +public class AnnotationDetectionFieldCallbackUnitTests { + + /** + * @see DATACMNS-616 + */ + @Test(expected = IllegalArgumentException.class) + public void rejectsNullAnnotationType() { + new AnnotationDetectionFieldCallback(null); + } + + /** + * @see DATACMNS-616 + */ + @Test + @SuppressWarnings("rawtypes") + public void looksUpValueFromPrivateField() { + + AnnotationDetectionFieldCallback callback = new AnnotationDetectionFieldCallback(Autowired.class); + ReflectionUtils.doWithFields(Sample.class, callback); + + assertThat(callback.getType(), is(equalTo((Class) String.class))); + assertThat(callback.getValue(new Sample("foo")), is((Object) "foo")); + } + + /** + * @see DATACMNS-616 + */ + @Test + public void returnsNullForObjectNotContainingAFieldWithTheConfiguredAnnotation() { + + AnnotationDetectionFieldCallback callback = new AnnotationDetectionFieldCallback(Autowired.class); + ReflectionUtils.doWithFields(Empty.class, callback); + + assertThat(callback.getType(), is(nullValue())); + assertThat(callback.getValue(new Empty()), is(nullValue())); + } + + static class Sample { + + @Autowired private final String value; + + public Sample(String value) { + this.value = value; + } + } + + static class Empty {} +} From a179e4e7dfe4de84d7ecba8efef3eff37fff349b Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 18 Dec 2014 18:53:10 +0100 Subject: [PATCH 21/59] DATACMNS-619 - DefaultCrudMethods now exposes accessible methods. We now make sure that all methods detected are made accessible so that clients using them can reflectively invoke them without additional checks. --- .../core/support/DefaultCrudMethods.java | 23 +++++++++++++++++++ .../support/DefaultCrudMethodsUnitTests.java | 14 +++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java b/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java index 83fd9ed488..1c8b0c9cbc 100644 --- a/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java +++ b/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java @@ -29,6 +29,8 @@ import org.springframework.data.repository.core.CrudMethods; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; /** * Default implementation to discover CRUD methods based on the given {@link RepositoryMetadata}. Will detect methods @@ -167,6 +169,27 @@ private Method selectMostSuitableFindOneMethod(RepositoryMetadata metadata) { return null; } + /** + * Looks up the most specific method for the given method and type and returns an accessible version of discovered + * {@link Method} if found. + * + * @param method + * @param type + * @see ClassUtils#getMostSpecificMethod(Method, Class) + * @return + */ + private static Method getMostSpecificMethod(Method method, Class type) { + + Method result = ClassUtils.getMostSpecificMethod(method, type); + + if (result == null) { + return null; + } + + ReflectionUtils.makeAccessible(result); + return result; + } + /* * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#getSaveMethod() diff --git a/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java index 823ec788ae..1e0ce85fd6 100644 --- a/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java @@ -142,6 +142,20 @@ public void detectsOverriddenDeleteMethodForEntity() throws Exception { RepositoryWithDeleteMethodForEntityOverloaded.class.getMethod("delete", Domain.class)); } + /** + * @see DATACMNS-619 + */ + @Test + public void exposedMethodsAreAccessible() { + + CrudMethods methods = getMethodsFor(RepositoryWithAllCrudMethodOverloaded.class); + + assertThat(methods.getSaveMethod().isAccessible(), is(true)); + assertThat(methods.getDeleteMethod().isAccessible(), is(true)); + assertThat(methods.getFindAllMethod().isAccessible(), is(true)); + assertThat(methods.getFindOneMethod().isAccessible(), is(true)); + } + private static CrudMethods getMethodsFor(Class repositoryInterface) { RepositoryMetadata metadata = new DefaultRepositoryMetadata(repositoryInterface); From f02babd3f8b44193f06dee21a4ee2fa636e7ddf9 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 18 Dec 2014 18:57:17 +0100 Subject: [PATCH 22/59] DATACMNS-620 - Repositories now creates reflective RepositoryInvokers for repositories with redeclared CRUD methods. If repository interfaces redeclare CRUD methods we need to use a ReflectionRepositoryInvoker rather than the CRUD one to make sure we pick up the customizations applied to the method declaration (e.g. caching or transaction annotations). This is already fixed in master due to the move to the Spring Data REST RepositoryInvoker API. --- .../data/repository/support/Repositories.java | 24 ++++++++++++++++++- .../support/RepositoriesIntegrationTests.java | 21 ++++++++++++++-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/support/Repositories.java b/src/main/java/org/springframework/data/repository/support/Repositories.java index ef9e0b67c3..cdf80f3049 100644 --- a/src/main/java/org/springframework/data/repository/support/Repositories.java +++ b/src/main/java/org/springframework/data/repository/support/Repositories.java @@ -16,6 +16,8 @@ package org.springframework.data.repository.support; import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; @@ -29,6 +31,7 @@ import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.core.CrudInvoker; +import org.springframework.data.repository.core.CrudMethods; import org.springframework.data.repository.core.EntityInformation; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.core.support.RepositoryFactoryInformation; @@ -202,7 +205,7 @@ public CrudInvoker getCrudInvoker(Class domainClass) { Assert.notNull(repository, String.format("No repository found for domain class: %s", domainClass)); - if (repository instanceof CrudRepository) { + if (repository instanceof CrudRepository && !hasRedeclaredCrudMethods(domainClass)) { return new CrudRepositoryInvoker((CrudRepository) repository); } else { return new ReflectionRepositoryInvoker(repository, getRepositoryInformationFor(domainClass).getCrudMethods()); @@ -217,6 +220,25 @@ public Iterator> iterator() { return repositoryFactoryInfos.keySet().iterator(); } + /** + * Returns whether any of the CRUD methods of the repository for the given domain type are redeclared. + * + * @param type must not be {@literal null}. + * @return + */ + private boolean hasRedeclaredCrudMethods(Class type) { + + CrudMethods crudMethods = getRepositoryInformationFor(type).getCrudMethods(); + + for (Method method : Arrays.asList(crudMethods.getFindOneMethod(), crudMethods.getSaveMethod())) { + if (!method.getDeclaringClass().equals(CrudRepository.class)) { + return true; + } + } + + return false; + } + /** * Null-object to avoid nasty {@literal null} checks in cache lookups. * diff --git a/src/test/java/org/springframework/data/repository/support/RepositoriesIntegrationTests.java b/src/test/java/org/springframework/data/repository/support/RepositoriesIntegrationTests.java index fce9fef7e5..edb0a3fe1e 100644 --- a/src/test/java/org/springframework/data/repository/support/RepositoriesIntegrationTests.java +++ b/src/test/java/org/springframework/data/repository/support/RepositoriesIntegrationTests.java @@ -78,6 +78,15 @@ public RepositoryFactoryBeanSupport, Product, Long> pr public ProductRepository productRepository() { return mock(ProductRepository.class); } + + @Bean + public RepositoryFactoryBeanSupport, Order, Long> orderRepositoryFactory() { + + DummyRepositoryFactoryBean, Order, Long> factory = new DummyRepositoryFactoryBean, Order, Long>(); + factory.setRepositoryInterface(OrderRepository.class); + + return factory; + } } @Autowired Repositories repositories; @@ -97,6 +106,7 @@ public void createsCrudInvokersCorrectly() { assertThat(repositories, is(notNullValue())); assertThat(repositories.getCrudInvoker(User.class), is(instanceOf(CrudRepositoryInvoker.class))); assertThat(repositories.getCrudInvoker(Product.class), is(instanceOf(ReflectionRepositoryInvoker.class))); + assertThat(repositories.getCrudInvoker(Order.class), is(instanceOf(ReflectionRepositoryInvoker.class))); } /** @@ -132,12 +142,19 @@ interface UserRepository extends CrudRepository { } - public static class Product {} + static class Product {} - public static interface ProductRepository extends Repository { + interface ProductRepository extends Repository { Product findOne(Long id); Product save(Product product); } + + static class Order {} + + interface OrderRepository extends CrudRepository { + + Order findOne(Long id); + } } From a4a6eb1384730efcb2ec391aa8e04903538c962b Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 30 Dec 2014 19:46:37 +0100 Subject: [PATCH 23/59] DATACMNS-625 - Re-enabled querydsl-next profile for Travis. Enabled the container based build infrastructure, too, to benefit from resource caching. --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 708a39d53a..6555bc7859 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,8 +6,10 @@ env: - PROFILE=ci - PROFILE=spring4-next - PROFILE=spring41-next + - PROFILE=querydsl-next cache: directories: - $HOME/.m2 +sudo: false install: true script: "mvn clean dependency:list test -P${PROFILE} -Dsort" \ No newline at end of file From 37fbbd4361d63735f876da1a41e0dc35303a1fe3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 5 Jan 2015 18:23:14 +0100 Subject: [PATCH 24/59] DATACMNS-624 - Reactivated Spring 4.1 build profile for Travis. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 6555bc7859..4e092c3edd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ env: matrix: - PROFILE=ci - PROFILE=spring4-next + - PROFILE=spring41 - PROFILE=spring41-next - PROFILE=querydsl-next cache: From 5528a3bdd6346810790a4dc33c82dab7527243e0 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 8 Jan 2015 09:04:49 +0100 Subject: [PATCH 25/59] DAATCMNS-629 - Fixed sample in documentation section for limiting query results. --- src/main/asciidoc/repositories.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index 83e54de940..fa6fe2ab3c 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -312,7 +312,7 @@ If the number is left out, a result size of 1 is assumed. ==== [source, java] ---- -User findFirstByOrderByLastname(); +User findFirstByOrderByLastnameAsc(); User findTopByOrderByAgeDesc(); From dfdae94f89c210678703b28ab0bd1ef165b4d04b Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 1 Dec 2014 11:33:18 +0100 Subject: [PATCH 26/59] DATACMNS-608 - Updated changelog. --- src/main/resources/changelog.txt | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/main/resources/changelog.txt b/src/main/resources/changelog.txt index f7704cae02..592491730d 100644 --- a/src/main/resources/changelog.txt +++ b/src/main/resources/changelog.txt @@ -1,6 +1,33 @@ Spring Data Commons Changelog ============================= +Changes in version 1.10.0.M1 (2014-12-01) +----------------------------------------- +* DATACMNS-609 - Multiple usage of repository setup means (XML or annotation) creates multiple bean definitions for RepositoryInterfaceAwareBeanPostProcessor. +* DATACMNS-608 - Release 1.10 M1. +* DATACMNS-607 - Inline AnnotationAttributes and MethodParameters to cut dependency to Spring HATEOAS for ReflectionRepositoryInvoker. +* DATACMNS-606 - Provide converter between legacy Date instances an JDK 8 date/time types. +* DATACMNS-605 - Move key-value infrastructure to dedicated project. +* DATACMNS-604 - Move QueryDsl helper methods from QueryDslUtils to Map specific structure. +* DATACMNS-601 - Work on SonarQube warnings. +* DATACMNS-600 - Remove package cycle introduced by the invoker package. +* DATACMNS-599 - Introduce IdentifierAccessor API. +* DATACMNS-597 - BasicPersistentEntity.getPropertyAccessor for target bean does not properly handle inheritance when PersistentEntity instances are interface or abstract class based. +* DATACMNS-596 - Expose PersistenPropertyAccessor API via PersistentEntity. +* DATACMNS-594 - Strange generics behaviour for multiply nested generic types. +* DATACMNS-591 - RepositoryConfigurationDelegate should use ResourceLoader provided by the infrastructure. +* DATACMNS-590 - Bug in generics detection for multiply nested generic types. +* DATACMNS-589 - Unify abstractions to invoke repositories with Spring Data REST. +* DATACMNS-587 - Add findAll variant with QueryDSL OrderSpecifier without a Predicate. +* DATACMNS-583 - DomainClassConverter#matches should return false when source- and targetType are equal. +* DATACMNS-581 - Spring Data support for NOT Containing expression. +* DATACMNS-580 - Add section on limiting the query results to reference documentation. +* DATACMNS-578 - Improve entity instantiation performance by using ASM byte-code generation. +* DATACMNS-577 - Improve javadoc on QueryDslPredicateExecutor. +* DATACMNS-530 - JavaDoc should be updated to reflect change of requiring an environment. +* DATACMNS-525 - In-Memory Repository infrastructure. + + Changes in version 1.9.1.RELEASE (2014-10-30) --------------------------------------------- * DATACMNS-585 - Release 1.9.1. From 3a4c1b026ac2245b56f9a51aa93704cc020c3f54 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 20 Jan 2015 14:19:19 +0100 Subject: [PATCH 27/59] DATACMNS-621 - QSort now treats nested paths correctly. We now inspect intermediate path elements to build sort order accordingly. Original pull request: #111. --- .../springframework/data/querydsl/QSort.java | 29 +++++++++-- .../data/querydsl/QSortUnitTests.java | 50 ++++++++++++++++--- .../data/querydsl/UserWrapper.java | 28 +++++++++++ .../data/querydsl/WrapperForUserWrapper.java | 28 +++++++++++ .../WrapperToWrapWrapperForUserWrapper.java | 29 +++++++++++ 5 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 src/test/java/org/springframework/data/querydsl/UserWrapper.java create mode 100644 src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java create mode 100644 src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java diff --git a/src/main/java/org/springframework/data/querydsl/QSort.java b/src/main/java/org/springframework/data/querydsl/QSort.java index 1b8af09a60..a7c905beb0 100644 --- a/src/main/java/org/springframework/data/querydsl/QSort.java +++ b/src/main/java/org/springframework/data/querydsl/QSort.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2014 the original author or authors. + * Copyright 2013-2015 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. @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Stack; import org.springframework.data.domain.Sort; import org.springframework.util.Assert; @@ -31,6 +32,7 @@ * Sort option for queries that wraps a querydsl {@link OrderSpecifier}. * * @author Thomas Darimont + * @author Christoph Strobl */ public class QSort extends Sort implements Serializable { @@ -87,8 +89,8 @@ private static Order toOrder(OrderSpecifier orderSpecifier) { Assert.notNull(orderSpecifier, "Order specifier must not be null!"); Expression target = orderSpecifier.getTarget(); - Object targetElement = target instanceof Path ? ((com.mysema.query.types.Path) target).getMetadata() - .getElement() : target; + + Object targetElement = target instanceof Path ? preparePropertyPath((Path) target) : target; Assert.notNull(targetElement, "Target element must not be null!"); @@ -142,4 +144,25 @@ public QSort and(OrderSpecifier... orderSpecifiers) { Assert.notEmpty(orderSpecifiers, "OrderSpecifiers must not be null or empty!"); return and(Arrays.asList(orderSpecifiers)); } + + private static String preparePropertyPath(Path path) { + + Stack stack = new Stack(); + Path pathElement = path; + while (pathElement.getMetadata() != null && pathElement.getMetadata().getParent() != null) { + + stack.push(pathElement.getMetadata().getElement().toString()); + pathElement = pathElement.getMetadata().getParent(); + } + + StringBuilder sb = new StringBuilder(); + while (!stack.isEmpty()) { + sb.append(stack.pop()); + if (!stack.isEmpty()) { + sb.append("."); + } + } + return sb.toString(); + } + } diff --git a/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java b/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java index 477ab30839..841283e698 100644 --- a/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java +++ b/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2014 the original author or authors. + * Copyright 2013-2015 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,9 +15,8 @@ */ package org.springframework.data.querydsl; -import static org.hamcrest.Matchers.hasItems; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; import java.util.List; @@ -34,6 +33,7 @@ * * @author Thomas Darimont * @author Oliver Gierke + * @author Christoph Strobl */ public class QSortUnitTests { @@ -147,7 +147,7 @@ public void concatenatesPlainSortCorrectly() { assertThat(result, is(Matchers. iterableWithSize(2))); assertThat(result, hasItems(new Order(Direction.ASC, "lastname"), new Order(Direction.ASC, "firstname"))); } - + /** * @see DATACMNS-566 */ @@ -159,6 +159,44 @@ public void shouldSupportSortByOperatorExpressions() { Sort result = sort.and(new Sort(Direction.ASC, "lastname")); assertThat(result, is(Matchers. iterableWithSize(2))); - assertThat(result, hasItems(new Order(Direction.ASC, "lastname"), new Order(Direction.ASC, user.dateOfBirth.yearMonth().toString()))); + assertThat( + result, + hasItems(new Order(Direction.ASC, "lastname"), + new Order(Direction.ASC, user.dateOfBirth.yearMonth().toString()))); + } + + /** + * @see DATACMNS-621 + */ + @Test + public void shouldCreateSortForNestedPathCorrectly() { + + QSort sort = new QSort(QUserWrapper.userWrapper.user.firstname.asc()); + + assertThat(sort, hasItems(new Order(Direction.ASC, "user.firstname"))); + } + + /** + * @see DATACMNS-621 + */ + @Test + public void shouldCreateSortForDeepNestedPathCorrectly() { + + QSort sort = new QSort(QWrapperForUserWrapper.wrapperForUserWrapper.wrapper.user.firstname.asc()); + + assertThat(sort, hasItems(new Order(Direction.ASC, "wrapper.user.firstname"))); + } + + /** + * @see DATACMNS-621 + */ + @Test + public void shouldCreateSortForReallyDeepNestedPathCorrectly() { + + QSort sort = new QSort( + QWrapperToWrapWrapperForUserWrapper.wrapperToWrapWrapperForUserWrapper.wrapperForUserWrapper.wrapper.user.firstname + .asc()); + + assertThat(sort, hasItems(new Order(Direction.ASC, "wrapperForUserWrapper.wrapper.user.firstname"))); } } diff --git a/src/test/java/org/springframework/data/querydsl/UserWrapper.java b/src/test/java/org/springframework/data/querydsl/UserWrapper.java new file mode 100644 index 0000000000..2cb823f3cc --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/UserWrapper.java @@ -0,0 +1,28 @@ +/* + * Copyright 2015 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.querydsl; + +import com.mysema.query.annotations.QueryEntity; + +/** + * @author Christoph Strobl + */ +@QueryEntity +public class UserWrapper { + + User user; + +} diff --git a/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java b/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java new file mode 100644 index 0000000000..b9f33249bb --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java @@ -0,0 +1,28 @@ +/* + * Copyright 2015 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.querydsl; + +import com.mysema.query.annotations.QueryEntity; + +/** + * @author Christoph Strobl + */ +@QueryEntity +public class WrapperForUserWrapper { + + UserWrapper wrapper; + +} diff --git a/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java b/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java new file mode 100644 index 0000000000..f089b01baf --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java @@ -0,0 +1,29 @@ +/* + * Copyright 2015 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.querydsl; + +import com.mysema.query.annotations.QueryEntity; +import com.mysema.query.annotations.QueryInit; + +/** + * @author Christoph Strobl + */ +@QueryEntity +public class WrapperToWrapWrapperForUserWrapper { + + @QueryInit("wrapper.user")// + WrapperForUserWrapper wrapperForUserWrapper; +} From 65f784b8691d55205f7f7b1406c4219866de7451 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 20 Jan 2015 16:24:30 +0100 Subject: [PATCH 28/59] DATACMNS-621 - Polishing. Simplified implementation of Path conversion. Inlined helper domain types to not to pollute the packages with types that are only used within that very one test class. Original pull request: #111. --- .../springframework/data/querydsl/QSort.java | 31 ++++++++++--------- .../data/querydsl/QSortUnitTests.java | 31 ++++++++++++++++--- .../data/querydsl/UserWrapper.java | 28 ----------------- .../data/querydsl/WrapperForUserWrapper.java | 28 ----------------- .../WrapperToWrapWrapperForUserWrapper.java | 29 ----------------- 5 files changed, 42 insertions(+), 105 deletions(-) delete mode 100644 src/test/java/org/springframework/data/querydsl/UserWrapper.java delete mode 100644 src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java delete mode 100644 src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java diff --git a/src/main/java/org/springframework/data/querydsl/QSort.java b/src/main/java/org/springframework/data/querydsl/QSort.java index a7c905beb0..2043d30f25 100644 --- a/src/main/java/org/springframework/data/querydsl/QSort.java +++ b/src/main/java/org/springframework/data/querydsl/QSort.java @@ -19,14 +19,15 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Stack; import org.springframework.data.domain.Sort; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import com.mysema.query.types.Expression; import com.mysema.query.types.OrderSpecifier; import com.mysema.query.types.Path; +import com.mysema.query.types.PathMetadata; /** * Sort option for queries that wraps a querydsl {@link OrderSpecifier}. @@ -145,24 +146,24 @@ public QSort and(OrderSpecifier... orderSpecifiers) { return and(Arrays.asList(orderSpecifiers)); } + /** + * Recursively creates a dot-separated path for the property path. + * + * @param path must not be {@literal null}. + * @return + */ private static String preparePropertyPath(Path path) { - Stack stack = new Stack(); - Path pathElement = path; - while (pathElement.getMetadata() != null && pathElement.getMetadata().getParent() != null) { + PathMetadata metadata = path.getMetadata(); + Path parent = metadata.getParent(); - stack.push(pathElement.getMetadata().getElement().toString()); - pathElement = pathElement.getMetadata().getParent(); + if (parent == null) { + return ""; } - StringBuilder sb = new StringBuilder(); - while (!stack.isEmpty()) { - sb.append(stack.pop()); - if (!stack.isEmpty()) { - sb.append("."); - } - } - return sb.toString(); - } + String basPath = preparePropertyPath(parent); + String element = metadata.getElement().toString(); + return StringUtils.hasText(basPath) ? basPath.concat(".").concat(element) : basPath.concat(element); + } } diff --git a/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java b/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java index 841283e698..86bf7e1634 100644 --- a/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java +++ b/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java @@ -17,6 +17,9 @@ import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import static org.springframework.data.querydsl.QQSortUnitTests_WrapperToWrapWrapperForUserWrapper.*; +import static org.springframework.data.querydsl.QQSortUnitTests_WrapperToWrapWrapperForUserWrapper_WrapperForUserWrapper.*; +import static org.springframework.data.querydsl.QQSortUnitTests_WrapperToWrapWrapperForUserWrapper_WrapperForUserWrapper_UserWrapper.*; import java.util.List; @@ -26,6 +29,7 @@ import org.springframework.data.domain.Sort.Direction; import org.springframework.data.domain.Sort.Order; +import com.mysema.query.annotations.QueryInit; import com.mysema.query.types.OrderSpecifier; /** @@ -171,7 +175,7 @@ public void shouldSupportSortByOperatorExpressions() { @Test public void shouldCreateSortForNestedPathCorrectly() { - QSort sort = new QSort(QUserWrapper.userWrapper.user.firstname.asc()); + QSort sort = new QSort(userWrapper.user.firstname.asc()); assertThat(sort, hasItems(new Order(Direction.ASC, "user.firstname"))); } @@ -182,7 +186,7 @@ public void shouldCreateSortForNestedPathCorrectly() { @Test public void shouldCreateSortForDeepNestedPathCorrectly() { - QSort sort = new QSort(QWrapperForUserWrapper.wrapperForUserWrapper.wrapper.user.firstname.asc()); + QSort sort = new QSort(wrapperForUserWrapper.wrapper.user.firstname.asc()); assertThat(sort, hasItems(new Order(Direction.ASC, "wrapper.user.firstname"))); } @@ -193,10 +197,27 @@ public void shouldCreateSortForDeepNestedPathCorrectly() { @Test public void shouldCreateSortForReallyDeepNestedPathCorrectly() { - QSort sort = new QSort( - QWrapperToWrapWrapperForUserWrapper.wrapperToWrapWrapperForUserWrapper.wrapperForUserWrapper.wrapper.user.firstname - .asc()); + QSort sort = new QSort(wrapperToWrapWrapperForUserWrapper.wrapperForUserWrapper.wrapper.user.firstname.asc()); assertThat(sort, hasItems(new Order(Direction.ASC, "wrapperForUserWrapper.wrapper.user.firstname"))); } + + @com.mysema.query.annotations.QueryEntity + static class WrapperToWrapWrapperForUserWrapper { + + @QueryInit("wrapper.user")// + WrapperForUserWrapper wrapperForUserWrapper; + + @com.mysema.query.annotations.QueryEntity + static class WrapperForUserWrapper { + + UserWrapper wrapper; + + @com.mysema.query.annotations.QueryEntity + static class UserWrapper { + + User user; + } + } + } } diff --git a/src/test/java/org/springframework/data/querydsl/UserWrapper.java b/src/test/java/org/springframework/data/querydsl/UserWrapper.java deleted file mode 100644 index 2cb823f3cc..0000000000 --- a/src/test/java/org/springframework/data/querydsl/UserWrapper.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2015 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.querydsl; - -import com.mysema.query.annotations.QueryEntity; - -/** - * @author Christoph Strobl - */ -@QueryEntity -public class UserWrapper { - - User user; - -} diff --git a/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java b/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java deleted file mode 100644 index b9f33249bb..0000000000 --- a/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2015 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.querydsl; - -import com.mysema.query.annotations.QueryEntity; - -/** - * @author Christoph Strobl - */ -@QueryEntity -public class WrapperForUserWrapper { - - UserWrapper wrapper; - -} diff --git a/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java b/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java deleted file mode 100644 index f089b01baf..0000000000 --- a/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2015 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.querydsl; - -import com.mysema.query.annotations.QueryEntity; -import com.mysema.query.annotations.QueryInit; - -/** - * @author Christoph Strobl - */ -@QueryEntity -public class WrapperToWrapWrapperForUserWrapper { - - @QueryInit("wrapper.user")// - WrapperForUserWrapper wrapperForUserWrapper; -} From 39556590c34286b5f46bb7a64060af15b190a498 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 18 Jan 2015 22:41:10 +0100 Subject: [PATCH 29/59] DATACMNS-634 - Repositories now als returns repositories for super types of a domain class. In case the repository lookup for a given domain type fails we traverse the given types super-types and try to detect a repository for those. Original pull request: #110. --- .../data/repository/support/Repositories.java | 15 +++++++++--- .../support/RepositoriesUnitTests.java | 24 ++++++++++--------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/support/Repositories.java b/src/main/java/org/springframework/data/repository/support/Repositories.java index cdf80f3049..b5ba4a36e4 100644 --- a/src/main/java/org/springframework/data/repository/support/Repositories.java +++ b/src/main/java/org/springframework/data/repository/support/Repositories.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2015 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. @@ -44,6 +44,7 @@ * * @author Oliver Gierke * @author Thomas Darimont + * @author Thomas Eizinger */ public class Repositories implements Iterable> { @@ -138,11 +139,19 @@ private RepositoryFactoryInformation getRepositoryFactoryI Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); - RepositoryFactoryInformation repositoryInfo = repositoryFactoryInfos.get(ClassUtils - .getUserClass(domainClass)); + Class classToInspect = domainClass; + RepositoryFactoryInformation repositoryInfo = repositoryFactoryInfos + .get(ClassUtils.getUserClass(classToInspect)); + + while (repositoryInfo == null && !classToInspect.equals(Object.class)) { + classToInspect = classToInspect.getSuperclass(); + repositoryInfo = repositoryFactoryInfos.get(ClassUtils.getUserClass(classToInspect)); + } + return repositoryInfo == null ? EMPTY_REPOSITORY_FACTORY_INFO : repositoryInfo; } + /** * Returns the {@link EntityInformation} for the given domain class. * diff --git a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java index fa6e8756e4..078fb35508 100644 --- a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java @@ -125,25 +125,27 @@ public void shouldThrowMeaningfulExceptionWhenTheRepositoryForAGivenDomainClassC repositories.getCrudInvoker(EntityWithoutRepository.class); } - class Person { - - } - - class Address { + /** + * @see DATACMNS-634 + */ + @Test + public void findsRepositoryForSubTypes() { + Repositories repositories = new Repositories(context); + assertThat(repositories.getPersistentEntity(AdvancedAddress.class), is(notNullValue())); } - class EntityWithoutRepository { + class EntityWithoutRepository {} - } + class Person {} - interface PersonRepository extends CrudRepository { + class Address {} - } + class AdvancedAddress extends Address {} - interface AddressRepository extends Repository { + interface PersonRepository extends CrudRepository {} - } + interface AddressRepository extends Repository {} static class SampleRepoFactoryInformation implements RepositoryFactoryInformation { From 229d5648dfb19681dbcbf97c846863dc09a023b3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 20 Jan 2015 17:46:00 +0100 Subject: [PATCH 30/59] DATACMNS-634 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplified type traversal in Repositories.getRepositoryFactoryInfoFor(…) and unit tests. Original pull request: #110. --- .../data/repository/support/Repositories.java | 17 +++++++++-------- .../support/RepositoriesUnitTests.java | 6 ++---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/support/Repositories.java b/src/main/java/org/springframework/data/repository/support/Repositories.java index b5ba4a36e4..4ea4c58654 100644 --- a/src/main/java/org/springframework/data/repository/support/Repositories.java +++ b/src/main/java/org/springframework/data/repository/support/Repositories.java @@ -139,18 +139,19 @@ private RepositoryFactoryInformation getRepositoryFactoryI Assert.notNull(domainClass, DOMAIN_TYPE_MUST_NOT_BE_NULL); - Class classToInspect = domainClass; - RepositoryFactoryInformation repositoryInfo = repositoryFactoryInfos - .get(ClassUtils.getUserClass(classToInspect)); + Class userType = ClassUtils.getUserClass(domainClass); + RepositoryFactoryInformation repositoryInfo = repositoryFactoryInfos.get(userType); - while (repositoryInfo == null && !classToInspect.equals(Object.class)) { - classToInspect = classToInspect.getSuperclass(); - repositoryInfo = repositoryFactoryInfos.get(ClassUtils.getUserClass(classToInspect)); + if (repositoryInfo != null) { + return repositoryInfo; } - return repositoryInfo == null ? EMPTY_REPOSITORY_FACTORY_INFO : repositoryInfo; - } + if (!userType.equals(Object.class)) { + return getRepositoryFactoryInfoFor(userType.getSuperclass()); + } + return EMPTY_REPOSITORY_FACTORY_INFO; + } /** * Returns the {@link EntityInformation} for the given domain class. diff --git a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java index 078fb35508..ecf665a6c1 100644 --- a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except @@ -130,9 +130,7 @@ public void shouldThrowMeaningfulExceptionWhenTheRepositoryForAGivenDomainClassC */ @Test public void findsRepositoryForSubTypes() { - - Repositories repositories = new Repositories(context); - assertThat(repositories.getPersistentEntity(AdvancedAddress.class), is(notNullValue())); + assertThat(new Repositories(context).getPersistentEntity(AdvancedAddress.class), is(notNullValue())); } class EntityWithoutRepository {} From f9a0ce5258eea2c8bb218eb218c659813c29feec Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 20 Jan 2015 20:20:03 +0100 Subject: [PATCH 31/59] DATACMNS-563 - Added unit tests to verify enabling one-indexed parameters are considered correctly when resolving Pageables. --- ...andlerMethodArgumentResolverUnitTests.java | 19 +++++++++++++++++++ ...andlerMethodArgumentResolverUnitTests.java | 16 ++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java index 8145d8a6ce..5cf7628c02 100644 --- a/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java @@ -106,6 +106,25 @@ public void returnsCustomizedTemplateVariables() { assertThat(variables, is("{?foo,size,sort}")); } + /** + * @see DATACMNS-563 + */ + @Test + public void enablingOneIndexedParameterReturnsOneForFirstPage() { + + HateoasPageableHandlerMethodArgumentResolver resolver = getResolver(); + resolver.setOneIndexedParameters(true); + + UriComponentsBuilder builder = UriComponentsBuilder.fromPath("/"); + + resolver.enhance(builder, null, new PageRequest(0, 10)); + + MultiValueMap params = builder.build().getQueryParams(); + + assertThat(params.containsKey(resolver.getPageParameterName()), is(true)); + assertThat(params.getFirst(resolver.getPageParameterName()), is("1")); + } + @Override protected HateoasPageableHandlerMethodArgumentResolver getResolver() { diff --git a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java index 8e7f203987..03bf854605 100644 --- a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java @@ -200,6 +200,22 @@ public void returnsNullIfFallbackIsNullAndOnlySizeIsGiven() throws Exception { is(nullValue())); } + /** + * @see DATACMNS-563 + */ + @Test + public void considersOneIndexedParametersSetting() { + + PageableHandlerMethodArgumentResolver resolver = getResolver(); + resolver.setOneIndexedParameters(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("page", "1"); + + assertThat(resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null) + .getPageNumber(), is(0)); + } + @Override protected PageableHandlerMethodArgumentResolver getResolver() { PageableHandlerMethodArgumentResolver resolver = new PageableHandlerMethodArgumentResolver(); From 8febeb5335109cfa7dfb4806d78fdc9486f4fe3a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sat, 24 Jan 2015 13:22:14 +0100 Subject: [PATCH 32/59] DATACMNS-637 - Performance improvements. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MappingContextTypeInformationMapper now caches detected type aliases. PreferredConstructor.isEnclosingClassParameter(…) now eagerly returns if the parameter itself is not an enclosing one and thus avoids a collection lookup and equals check. Moved equals check for the type to the very end of the equals check to increase the chances that other inequality guards kick in earlier. AbstractMappingContext now leaves the non-null-check for getPersistentEntity(…) to the factory method of ClassTypeInformation. We now pre-calculate the hash codes for TypeInformation implementations as far as possible as the instances are used as cache keys quite a lot. The same applies to AbstractPersistentProperty. BasicPersistentEntity now uses an ArrayList we sort during the verify() phase to mimic the previous behavior wich was implemented using a TreeSet as ArrayLists are way more performant when iterating over all elements which doWithProperties(…) is doing which is used quite a lot. BeanWrapper now avoids the getter lookup if field access is used. SimpleTypeHolder now uses a CopyOnWriteArrySet to leniently add types detected to be simple to the set of simple types to avoid ongoing checks against the inheritance hierarchy. --- .../MappingContextTypeInformationMapper.java | 48 +++++--- .../data/mapping/PreferredConstructor.java | 12 +- .../context/AbstractMappingContext.java | 3 +- .../model/AbstractPersistentProperty.java | 6 +- .../mapping/model/BasicPersistentEntity.java | 27 ++++- .../data/mapping/model/BeanWrapper.java | 8 +- .../data/mapping/model/SimpleTypeHolder.java | 10 +- .../springframework/data/util/CacheValue.java | 107 ++++++++++++++++++ .../util/ParentTypeAwareTypeInformation.java | 12 +- .../data/util/TypeDiscoverer.java | 19 +--- .../model/BasicPersistentEntityUnitTests.java | 12 +- 11 files changed, 202 insertions(+), 62 deletions(-) create mode 100644 src/main/java/org/springframework/data/util/CacheValue.java diff --git a/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java b/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java index 4bfc15287c..b8965e7391 100644 --- a/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java +++ b/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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,12 +15,13 @@ */ package org.springframework.data.convert; -import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.util.CacheValue; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -34,7 +35,7 @@ */ public class MappingContextTypeInformationMapper implements TypeInformationMapper { - private final Map, Object> typeMap; + private final Map, CacheValue> typeMap; private final MappingContext, ?> mappingContext; /** @@ -47,7 +48,7 @@ public MappingContextTypeInformationMapper(MappingContext, Object>(); + this.typeMap = new ConcurrentHashMap, CacheValue>(); this.mappingContext = mappingContext; for (PersistentEntity entity : mappingContext.getPersistentEntities()) { @@ -61,10 +62,10 @@ public MappingContextTypeInformationMapper(MappingContext type) { - Object key = typeMap.get(type); + CacheValue key = typeMap.get(type); if (key != null) { - return key; + return key.getValue(); } PersistentEntity entity = mappingContext.getPersistentEntity(type); @@ -87,26 +88,36 @@ public Object createAliasFor(TypeInformation type) { */ private void safelyAddToCache(ClassTypeInformation key, Object alias) { - if (alias == null) { + CacheValue aliasToBeCached = CacheValue.ofNullable(alias); + + if (alias == null && !typeMap.containsKey(key)) { + typeMap.put(key, aliasToBeCached); return; } - Object existingAlias = typeMap.get(key); + CacheValue alreadyCachedAlias = typeMap.get(key); // Reject second alias for same type - if (existingAlias != null && !alias.equals(existingAlias)) { + if (alreadyCachedAlias != null && alreadyCachedAlias.isPresent() && !alreadyCachedAlias.hasValue(alias)) { throw new IllegalArgumentException(String.format( - "Trying to register alias '%s', but found already registered alias '%s' for type %s!", alias, existingAlias, - key)); + "Trying to register alias '%s', but found already registered alias '%s' for type %s!", alias, + alreadyCachedAlias, key)); } // Reject second type for same alias - if (typeMap.containsValue(alias)) { + if (typeMap.containsValue(aliasToBeCached)) { + + for (Entry, CacheValue> entry : typeMap.entrySet()) { + + CacheValue value = entry.getValue(); - for (Entry, Object> entry : typeMap.entrySet()) { - if (entry.getValue().equals(alias) && !entry.getKey().equals(key)) { + if (!value.isPresent()) { + continue; + } + + if (value.hasValue(alias) && !entry.getKey().equals(key)) { throw new IllegalArgumentException(String.format( "Detected existing type mapping of %s to alias '%s' but attempted to bind the same alias to %s!", key, alias, entry.getKey())); @@ -114,7 +125,7 @@ private void safelyAddToCache(ClassTypeInformation key, Object alias) { } } - typeMap.put(key, alias); + typeMap.put(key, aliasToBeCached); } /* @@ -127,8 +138,11 @@ public ClassTypeInformation resolveTypeFrom(Object alias) { return null; } - for (Entry, Object> entry : typeMap.entrySet()) { - if (entry.getValue().equals(alias)) { + for (Entry, CacheValue> entry : typeMap.entrySet()) { + + CacheValue cachedAlias = entry.getValue(); + + if (cachedAlias.hasValue(alias)) { return entry.getKey(); } } diff --git a/src/main/java/org/springframework/data/mapping/PreferredConstructor.java b/src/main/java/org/springframework/data/mapping/PreferredConstructor.java index 8cfb6e8eb8..9507eff327 100644 --- a/src/main/java/org/springframework/data/mapping/PreferredConstructor.java +++ b/src/main/java/org/springframework/data/mapping/PreferredConstructor.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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. @@ -168,11 +168,11 @@ public boolean isEnclosingClassParameter(Parameter parameter) { Assert.notNull(parameter); - if (parameters.isEmpty()) { + if (parameters.isEmpty() || !parameter.isEnclosingClassParameter()) { return false; } - return parameters.get(0).equals(parameter) && parameter.isEnclosingClassParameter(); + return parameters.get(0).equals(parameter); } /** @@ -312,10 +312,10 @@ public boolean equals(Object obj) { boolean nameEquals = this.name == null ? that.name == null : this.name.equals(that.name); boolean keyEquals = this.key == null ? that.key == null : this.key.equals(that.key); - boolean typeEquals = nullSafeEquals(this.type, that.type); boolean entityEquals = this.entity == null ? that.entity == null : this.entity.equals(that.entity); - return nameEquals && keyEquals && typeEquals && entityEquals; + // Explicitly delay equals check on type as it might be expensive + return nameEquals && keyEquals && entityEquals && this.type.equals(that.type); } /* @@ -329,8 +329,8 @@ public int hashCode() { result += 31 * nullSafeHashCode(this.name); result += 31 * nullSafeHashCode(this.key); - result += 31 * nullSafeHashCode(this.type); result += 31 * nullSafeHashCode(this.entity); + result += 31 * this.type.hashCode(); return result; } diff --git a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index e1219c1e76..d64ff32000 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -137,7 +137,6 @@ public Collection getPersistentEntities() { * @see org.springframework.data.mapping.model.MappingContext#getPersistentEntity(java.lang.Class) */ public E getPersistentEntity(Class type) { - Assert.notNull(type); return getPersistentEntity(ClassTypeInformation.from(type)); } diff --git a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java index 1093c51724..41a85d3756 100644 --- a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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. @@ -53,6 +53,7 @@ public abstract class AbstractPersistentProperty

protected final Association

association; protected final PersistentEntity owner; private final SimpleTypeHolder simpleTypeHolder; + private final int hashCode; public AbstractPersistentProperty(Field field, PropertyDescriptor propertyDescriptor, PersistentEntity owner, SimpleTypeHolder simpleTypeHolder) { @@ -68,6 +69,7 @@ public AbstractPersistentProperty(Field field, PropertyDescriptor propertyDescri this.association = isAssociation() ? createAssociation() : null; this.owner = owner; this.simpleTypeHolder = simpleTypeHolder; + this.hashCode = this.field == null ? this.propertyDescriptor.hashCode() : this.field.hashCode(); } protected abstract Association

createAssociation(); @@ -340,7 +342,7 @@ public boolean equals(Object obj) { */ @Override public int hashCode() { - return this.field == null ? this.propertyDescriptor.hashCode() : this.field.hashCode(); + return this.hashCode; } /* diff --git a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java index 5db6a9a65f..9b99e2a1e7 100644 --- a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,9 +17,12 @@ import java.io.Serializable; import java.lang.annotation.Annotation; +import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -50,7 +53,8 @@ public class BasicPersistentEntity> implement private final PreferredConstructor constructor; private final TypeInformation information; - private final Set

properties; + private final List

properties; + private final Comparator

comparator; private final Set> associations; private final Map propertyCache; @@ -81,7 +85,8 @@ public BasicPersistentEntity(TypeInformation information, Comparator

compa Assert.notNull(information); this.information = information; - this.properties = comparator == null ? new HashSet

() : new TreeSet

(comparator); + this.properties = new ArrayList

(); + this.comparator = comparator; this.constructor = new PreferredConstructorDiscoverer(information, this).getConstructor(); this.associations = comparator == null ? new HashSet>() : new TreeSet>( new AssociationComparator

(comparator)); @@ -169,6 +174,11 @@ public boolean hasVersionProperty() { public void addPersistentProperty(P property) { Assert.notNull(property); + + if (properties.contains(property)) { + return; + } + properties.add(property); if (!propertyCache.containsKey(property.getName())) { @@ -217,7 +227,10 @@ protected P returnPropertyIfBetterIdPropertyCandidateOrNull(P property) { * @see org.springframework.data.mapping.MutablePersistentEntity#addAssociation(org.springframework.data.mapping.model.Association) */ public void addAssociation(Association

association) { - associations.add(association); + + if (!associations.contains(association)) { + associations.add(association); + } } /* @@ -357,11 +370,15 @@ public A findAnnotation(Class annotationType) { return annotation; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.mapping.MutablePersistentEntity#verify() */ public void verify() { + if (comparator != null) { + Collections.sort(properties, comparator); + } } /** diff --git a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java index 464adb36ac..cee5a7564e 100644 --- a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java +++ b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -116,15 +116,17 @@ public S getProperty(PersistentProperty property, Class type try { Object obj = null; - Method getter = property.getGetter(); if (!property.usePropertyAccess()) { Field field = property.getField(); ReflectionUtils.makeAccessible(field); obj = ReflectionUtils.getField(field, bean); + } + + Method getter = property.getGetter(); - } else if (property.usePropertyAccess() && getter != null) { + if (property.usePropertyAccess() && getter != null) { ReflectionUtils.makeAccessible(getter); obj = ReflectionUtils.invokeMethod(getter, bean); diff --git a/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java b/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java index dab3f9ece5..9dbf067ad0 100644 --- a/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java +++ b/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ import java.util.HashSet; import java.util.Locale; import java.util.Set; +import java.util.concurrent.CopyOnWriteArraySet; import org.springframework.util.Assert; @@ -86,7 +87,7 @@ public SimpleTypeHolder() { public SimpleTypeHolder(Set> customSimpleTypes, boolean registerDefaults) { Assert.notNull(customSimpleTypes); - this.simpleTypes = new HashSet>(customSimpleTypes); + this.simpleTypes = new CopyOnWriteArraySet>(customSimpleTypes); if (registerDefaults) { this.simpleTypes.addAll(DEFAULTS); @@ -104,7 +105,7 @@ public SimpleTypeHolder(Set> customSimpleTypes, SimpleTypeHol Assert.notNull(customSimpleTypes); Assert.notNull(source); - this.simpleTypes = new HashSet>(customSimpleTypes); + this.simpleTypes = new CopyOnWriteArraySet>(customSimpleTypes); this.simpleTypes.addAll(source.simpleTypes); } @@ -118,12 +119,13 @@ public boolean isSimpleType(Class type) { Assert.notNull(type); - if (Object.class.equals(type)) { + if (Object.class.equals(type) || simpleTypes.contains(type)) { return true; } for (Class clazz : simpleTypes) { if (clazz.isAssignableFrom(type)) { + simpleTypes.add(type); return true; } } diff --git a/src/main/java/org/springframework/data/util/CacheValue.java b/src/main/java/org/springframework/data/util/CacheValue.java new file mode 100644 index 0000000000..3fc20fb0dc --- /dev/null +++ b/src/main/java/org/springframework/data/util/CacheValue.java @@ -0,0 +1,107 @@ +/* + * Copyright 2014-2015 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; + +/** + * Wrapper to safely store {@literal null} values in the value cache. + * + * @author Patryk Wasik + * @author Oliver Gierke + * @author Thomas Darimont + */ +public class CacheValue { + + private static final CacheValue ABSENT = new CacheValue(null); + + private final T value; + + /** + * Creates a new {@link CacheValue} for the gven value. + * + * @param type can be {@literal null}. + */ + private CacheValue(T type) { + this.value = type; + } + + /** + * Returns the actual underlying value. + * + * @return + */ + public T getValue() { + return value; + } + + /** + * Returns whether the cached value has an actual value. + * + * @return + */ + public boolean isPresent() { + return value != null; + } + + /** + * Returns whether the cached value has the given actual value. + * + * @param value can be {@literal null}; + * @return + */ + public boolean hasValue(T value) { + return isPresent() ? this.value.equals(value) : value == null; + } + + /** + * Returns a new {@link CacheValue} for the given value. + * + * @param value can be {@literal null}. + * @return will never be {@literal null}. + */ + @SuppressWarnings("unchecked") + public static CacheValue ofNullable(T value) { + return value == null ? (CacheValue) ABSENT : new CacheValue(value); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + return isPresent() ? 0 : value.hashCode(); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + + if (this == obj) { + return true; + } + + if (!(obj instanceof CacheValue)) { + return false; + } + + CacheValue that = (CacheValue) obj; + + return this.value == null ? false : this.value.equals(that.value); + } +} diff --git a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java index 8fb4bc0937..9072fe30ab 100644 --- a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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. @@ -20,8 +20,6 @@ import java.util.HashMap; import java.util.Map; -import org.springframework.util.ObjectUtils; - /** * Base class for {@link TypeInformation} implementations that need parent type awareness. * @@ -30,6 +28,7 @@ public abstract class ParentTypeAwareTypeInformation extends TypeDiscoverer { private final TypeDiscoverer parent; + private int hashCode; /** * Creates a new {@link ParentTypeAwareTypeInformation}. @@ -99,6 +98,11 @@ public boolean equals(Object obj) { */ @Override public int hashCode() { - return super.hashCode() + 31 * ObjectUtils.nullSafeHashCode(parent); + + if (this.hashCode == 0) { + this.hashCode = super.hashCode() + 31 * parent.hashCode(); + } + + return this.hashCode; } } diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java index 04cf28818f..ddffff1932 100644 --- a/src/main/java/org/springframework/data/util/TypeDiscoverer.java +++ b/src/main/java/org/springframework/data/util/TypeDiscoverer.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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,8 +15,6 @@ */ package org.springframework.data.util; -import static org.springframework.util.ObjectUtils.*; - import java.beans.PropertyDescriptor; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -50,6 +48,7 @@ class TypeDiscoverer implements TypeInformation { private final Type type; private final Map, Type> typeVariableMap; private final Map> fieldTypes = new ConcurrentHashMap>(); + private final int hashCode; private boolean componentTypeResolved = false; private TypeInformation componentType; @@ -72,6 +71,7 @@ protected TypeDiscoverer(Type type, Map, Type> typeVariableMap) this.type = type; this.typeVariableMap = typeVariableMap; + this.hashCode = 17 + (31 * type.hashCode()) + (31 * typeVariableMap.hashCode()); } /** @@ -513,10 +513,7 @@ public boolean equals(Object obj) { TypeDiscoverer that = (TypeDiscoverer) obj; - boolean typeEqual = nullSafeEquals(this.type, that.type); - boolean typeVariableMapEqual = nullSafeEquals(this.typeVariableMap, that.typeVariableMap); - - return typeEqual && typeVariableMapEqual; + return this.type.equals(that.type) && this.typeVariableMap.equals(that.typeVariableMap); } /* @@ -525,12 +522,6 @@ public boolean equals(Object obj) { */ @Override public int hashCode() { - - int result = 17; - - result += nullSafeHashCode(type); - result += nullSafeHashCode(typeVariableMap); - - return result; + return hashCode; } } diff --git a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java index ff93584c62..46b4dfe10a 100644 --- a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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. @@ -21,7 +21,7 @@ import java.util.Comparator; import java.util.Iterator; -import java.util.SortedSet; +import java.util.List; import org.junit.Rule; import org.junit.Test; @@ -53,7 +53,7 @@ public class BasicPersistentEntityUnitTests> { @Rule public ExpectedException exception = ExpectedException.none(); - @Mock T property; + @Mock T property, anotherProperty; @Test public void assertInvariants() { @@ -110,8 +110,9 @@ public int compare(T o1, T o2) { entity.addPersistentProperty(lastName); entity.addPersistentProperty(firstName); entity.addPersistentProperty(ssn); + entity.verify(); - SortedSet properties = (SortedSet) ReflectionTestUtils.getField(entity, "properties"); + List properties = (List) ReflectionTestUtils.getField(entity, "properties"); assertThat(properties.size(), is(3)); Iterator iterator = properties.iterator(); @@ -143,10 +144,11 @@ public void rejectsIdPropertyIfAlreadySet() { MutablePersistentEntity entity = createEntity(null); when(property.isIdProperty()).thenReturn(true); + when(anotherProperty.isIdProperty()).thenReturn(true); entity.addPersistentProperty(property); exception.expect(MappingException.class); - entity.addPersistentProperty(property); + entity.addPersistentProperty(anotherProperty); } /** From e0b985f536a5fdee73cb53dba3ebb19bb46f7249 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 27 Jan 2015 14:35:07 +0100 Subject: [PATCH 33/59] DATACMNS-632 - Updated changelog. --- src/main/resources/changelog.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/main/resources/changelog.txt b/src/main/resources/changelog.txt index 592491730d..d6905cea7c 100644 --- a/src/main/resources/changelog.txt +++ b/src/main/resources/changelog.txt @@ -1,6 +1,24 @@ Spring Data Commons Changelog ============================= +Changes in version 1.8.5.RELEASE (2015-01-27) +--------------------------------------------- +* DATACMNS-637 - General inspection for performance improvements. +* DATACMNS-634 - Repositories should return repository for entity's super-type if available. +* DATACMNS-632 - Release 1.8.5. +* DATACMNS-624 - Re-enable Travis builds for Spring 4.1, 4.1.next and Querydsl.next. +* DATACMNS-621 - QSort does not treat nested paths correctly. +* DATACMNS-616 - AnnotationRevisionMetadata can't access private fields. +* DATACMNS-615 - PageImpl should reject total less than given content length. +* DATACMNS-611 - Avoid triple cache lookup in RepositoryInterfaceAwareBeanPostProcessor.predictBeanType(…). +* DATACMNS-601 - Work on SonarQube warnings. +* DATACMNS-594 - Strange generics behaviour for multiply nested generic types. +* DATACMNS-590 - Bug in generics detection for multiply nested generic types. +* DATACMNS-583 - DomainClassConverter#matches should return false when source- and targetType are equal. +* DATACMNS-577 - Improve javadoc on QueryDslPredicateExecutor. +* DATACMNS-530 - JavaDoc should be updated to reflect change of requiring an environment. + + Changes in version 1.10.0.M1 (2014-12-01) ----------------------------------------- * DATACMNS-609 - Multiple usage of repository setup means (XML or annotation) creates multiple bean definitions for RepositoryInterfaceAwareBeanPostProcessor. From dc8276a2658a8dd108757b536160d6c3de3b9589 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 28 Jan 2015 11:23:19 +0100 Subject: [PATCH 34/59] DATACMNS-633 - Updated changelog. --- src/main/resources/changelog.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/main/resources/changelog.txt b/src/main/resources/changelog.txt index d6905cea7c..c45a0a4b96 100644 --- a/src/main/resources/changelog.txt +++ b/src/main/resources/changelog.txt @@ -1,6 +1,27 @@ Spring Data Commons Changelog ============================= +Changes in version 1.9.2.RELEASE (2015-01-28) +--------------------------------------------- +* DATACMNS-637 - General inspection for performance improvements. +* DATACMNS-634 - Repositories should return repository for entity's super-type if available. +* DATACMNS-633 - Release 1.9.2. +* DATACMNS-629 - Reference documentation section on limiting query results is showing incorrect order-by-clause. +* DATACMNS-625 - Re-enable querydsl-next profile for Travis. +* DATACMNS-621 - QSort does not treat nested paths correctly. +* DATACMNS-620 - Make sure we reflectively invoke redeclared repository methods in legacy RepositoryInvokers. +* DATACMNS-619 - DefaultCrudMethods should always return accessible methods. +* DATACMNS-616 - AnnotationRevisionMetadata can't access private fields. +* DATACMNS-615 - PageImpl should reject total less than given content length. +* DATACMNS-611 - Avoid triple cache lookup in RepositoryInterfaceAwareBeanPostProcessor.predictBeanType(…). +* DATACMNS-610 - Add converter support for Set return types. +* DATACMNS-609 - Multiple usage of repository setup means (XML or annotation) creates multiple bean definitions for RepositoryInterfaceAwareBeanPostProcessor. +* DATACMNS-601 - Work on SonarQube warnings. +* DATACMNS-594 - Strange generics behaviour for multiply nested generic types. +* DATACMNS-591 - RepositoryConfigurationDelegate should use ResourceLoader provided by the infrastructure. +* DATACMNS-590 - Bug in generics detection for multiply nested generic types. + + Changes in version 1.8.5.RELEASE (2015-01-27) --------------------------------------------- * DATACMNS-637 - General inspection for performance improvements. From 348ca036be62e71039ee0d49f64f73d8af34f901 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 28 Jan 2015 11:24:10 +0100 Subject: [PATCH 35/59] DATACMNS-633 - Prepare 1.9.2.RELEASE (Evans SR2). --- pom.xml | 6 +++--- src/main/resources/notice.txt | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index 076ebb988a..67a46799ca 100644 --- a/pom.xml +++ b/pom.xml @@ -12,7 +12,7 @@ org.springframework.data.build spring-data-parent - 1.5.2.BUILD-SNAPSHOT + 1.5.2.RELEASE ../spring-data-build/parent/pom.xml @@ -237,8 +237,8 @@ - spring-libs-snapshot - http://repo.spring.io/libs-snapshot + spring-libs-release + http://repo.spring.io/libs-release diff --git a/src/main/resources/notice.txt b/src/main/resources/notice.txt index d6af53d7f4..6ad80cf465 100644 --- a/src/main/resources/notice.txt +++ b/src/main/resources/notice.txt @@ -1,5 +1,5 @@ -Spring Data Commons 1.9.1 -Copyright (c) [2010-2014] Pivotal Software, Inc. +Spring Data Commons 1.9.2 +Copyright (c) [2010-2015] Pivotal Software, Inc. This product is licensed to you under the Apache License, Version 2.0 (the "License"). You may not use this product except in compliance with the License. From 020b7fcd21d8619182c88cf6db6a8f57fc820ddf Mon Sep 17 00:00:00 2001 From: Spring Buildmaster Date: Wed, 28 Jan 2015 02:50:22 -0800 Subject: [PATCH 36/59] DATACMNS-633 - Release version 1.9.2.RELEASE (Evans SR2). --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 67a46799ca..cb3ea2d3d7 100644 --- a/pom.xml +++ b/pom.xml @@ -1,11 +1,11 @@ - + 4.0.0 org.springframework.data spring-data-commons - 1.9.2.BUILD-SNAPSHOT + 1.9.2.RELEASE Spring Data Core From 1877047d094c97b55722d9237dc2098b8ed853c6 Mon Sep 17 00:00:00 2001 From: Spring Buildmaster Date: Wed, 28 Jan 2015 02:50:25 -0800 Subject: [PATCH 37/59] DATACMNS-633 - Prepare next development iteration. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index cb3ea2d3d7..45c57bc481 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 1.9.2.RELEASE + 1.9.3.BUILD-SNAPSHOT Spring Data Core From fdbea4c1303d5cb540e2602ef9f3a30d0fb05e8a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 28 Jan 2015 17:35:15 +0100 Subject: [PATCH 38/59] DATACMNS-633 - After release cleanups. --- pom.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 45c57bc481..f51fe6d45c 100644 --- a/pom.xml +++ b/pom.xml @@ -1,4 +1,4 @@ - + 4.0.0 @@ -12,7 +12,7 @@ org.springframework.data.build spring-data-parent - 1.5.2.RELEASE + 1.5.3.BUILD-SNAPSHOT ../spring-data-build/parent/pom.xml @@ -237,8 +237,8 @@ - spring-libs-release - http://repo.spring.io/libs-release + spring-libs-snapshot + http://repo.spring.io/libs-snapshot From 2f923e44191c4b1c26579dc51965bb5b8b6d1719 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 30 Jan 2015 14:39:17 +0100 Subject: [PATCH 39/59] DATACMNS-563 - Added explicit unit test for one-indexed parameters on PagedResourcesAssembler. --- .../web/PagedResourcesAssemblerUnitTests.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java index 9876dd34b2..4c56397afb 100644 --- a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java +++ b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java @@ -18,9 +18,11 @@ import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import java.net.URI; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Map; import org.junit.Before; import org.junit.Test; @@ -183,6 +185,25 @@ public void keepsExistingTemplateVariablesFromBaseLink() { assertThat(result.getVariableNames(), hasItems("projection", "size", "sort")); } + /** + * @see DATAMCNS-563 + */ + @Test + public void createsPaginationLinksForOneIndexedArgumentResolverCorrectly() { + + HateoasPageableHandlerMethodArgumentResolver argumentResolver = new HateoasPageableHandlerMethodArgumentResolver(); + argumentResolver.setOneIndexedParameters(true); + + PagedResourcesAssembler assembler = new PagedResourcesAssembler(argumentResolver, null); + PagedResources> resource = assembler.toResource(createPage(1)); + + assertThat(resource.hasLink("prev"), is(true)); + assertThat(resource.hasLink("next"), is(true)); + + assertThat(getQueryParameters(resource.getLink("prev")), hasEntry("page", "1")); + assertThat(getQueryParameters(resource.getLink("next")), hasEntry("page", "3")); + } + private static Page createPage(int index) { AbstractPageRequest request = new PageRequest(index, 1); @@ -193,6 +214,12 @@ private static Page createPage(int index) { return new PageImpl(Arrays.asList(person), request, 3); } + private static Map getQueryParameters(Link link) { + + UriComponents uriComponents = UriComponentsBuilder.fromUri(URI.create(link.expand().getHref())).build(); + return uriComponents.getQueryParams().toSingleValueMap(); + } + static class Person { String name; } From a17100568c68b9cd16d7fd1ab4e2b3223a6d8afc Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 4 Feb 2015 10:32:17 +0100 Subject: [PATCH 40/59] DATACMNS-640 - Fixed potential NullPointerException in PageableHandlerMethodArgumentResolver. We now double check the fallback Pageable for being null before trying to lookup the Sort to fall back. --- ...PageableHandlerMethodArgumentResolver.java | 6 +++++- ...andlerMethodArgumentResolverUnitTests.java | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java index 6ef5ac48e1..7a5f98e52d 100644 --- a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java @@ -246,7 +246,11 @@ public Pageable resolveArgument(MethodParameter methodParameter, ModelAndViewCon pageSize = pageSize > maxPageSize ? maxPageSize : pageSize; Sort sort = sortResolver.resolveArgument(methodParameter, mavContainer, webRequest, binderFactory); - return new PageRequest(page, pageSize, sort == null ? defaultOrFallback.getSort() : sort); + + // Default if necessary and default configured + sort = sort == null && defaultOrFallback != null ? defaultOrFallback.getSort() : sort; + + return new PageRequest(page, pageSize, sort); } /** diff --git a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java index 03bf854605..2fa3b0715a 100644 --- a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java @@ -216,6 +216,26 @@ public void considersOneIndexedParametersSetting() { .getPageNumber(), is(0)); } + /** + * @see DATACMNS-640 + */ + @Test + public void usesNullSortIfNoDefaultIsConfiguredAndPageAndSizeAreGiven() { + + PageableHandlerMethodArgumentResolver resolver = getResolver(); + resolver.setFallbackPageable(null); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("page", "0"); + request.addParameter("size", "10"); + + Pageable result = resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null); + + assertThat(result.getPageNumber(), is(0)); + assertThat(result.getPageSize(), is(10)); + assertThat(result.getSort(), is(nullValue())); + } + @Override protected PageableHandlerMethodArgumentResolver getResolver() { PageableHandlerMethodArgumentResolver resolver = new PageableHandlerMethodArgumentResolver(); From ff6d0dda9d77bd02a1775d7857c166a32e5f4075 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 4 Feb 2015 17:11:48 +0100 Subject: [PATCH 41/59] DATACMNS-642 - Avoid setter lookup in BeanWrapper for if field access is used. We now delay the lookup of the setter method until we discover we really need to use property access. --- .../springframework/data/mapping/model/BeanWrapper.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java index cee5a7564e..09cfcc953b 100644 --- a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java +++ b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java @@ -66,8 +66,6 @@ public void setProperty(PersistentProperty property, Object value) { Assert.notNull(property, "PersistentProperty must not be null!"); - Method setter = property.getSetter(); - try { if (!property.usePropertyAccess()) { @@ -75,8 +73,12 @@ public void setProperty(PersistentProperty property, Object value) { Object valueToSet = getPotentiallyConvertedValue(value, property.getType()); ReflectionUtils.makeAccessible(property.getField()); ReflectionUtils.setField(property.getField(), bean, valueToSet); + return; + } + + Method setter = property.getSetter(); - } else if (property.usePropertyAccess() && setter != null) { + if (property.usePropertyAccess() && setter != null) { Class[] paramTypes = setter.getParameterTypes(); Object valueToSet = getPotentiallyConvertedValue(value, paramTypes[0]); From d9156ffb7b31eb1e317e3249e58d98d258a2f7f8 Mon Sep 17 00:00:00 2001 From: Arlo Louis O'Keeffe Date: Tue, 10 Feb 2015 11:26:51 +0100 Subject: [PATCH 42/59] DATACMNS-645 - Fix typo in reference documentation. Original pull request: #113. --- src/main/asciidoc/repositories.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index fa6fe2ab3c..cdb3374383 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -205,7 +205,7 @@ NOTE: Note, that the intermediate repository interface is annotated with `@NoRep [[repositories.query-methods.details]] == Defining query methods -The repository proxy has two ways to derive a store-specific query from the method name. It can derive the query from the method name directly, or by using an manually defined query. Available options depend on the actual store. However, there's got to be an strategy that decides what actual query is created. Let's have a look at the available options. +The repository proxy has two ways to derive a store-specific query from the method name. It can derive the query from the method name directly, or by using a manually defined query. Available options depend on the actual store. However, there's got to be an strategy that decides what actual query is created. Let's have a look at the available options. [[repositories.query-methods.query-lookup-strategies]] === Query lookup strategies From 0e35fb8c73ea64a9977d99db6055b4fbe134468d Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 10 Feb 2015 16:57:25 +0100 Subject: [PATCH 43/59] DATACMNS-646 - Enable Spring 4.2 build profile for Travis. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 4e092c3edd..f053084055 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ env: - PROFILE=spring4-next - PROFILE=spring41 - PROFILE=spring41-next + - PROFILE=spring42-next - PROFILE=querydsl-next cache: directories: From 7da5f5baeab7b99287403c2437be41cce84fbb2f Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 24 Feb 2015 16:22:23 +0100 Subject: [PATCH 44/59] DATACMNS-649 - Fixed detail in reference docs on extending all repositories. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The custom factory created to extend all repositories with additional behavior needs to return the type of the custom repository base class from getRepositoryBaseClass(…) to make sure the infrastructure can use it to inspect the CRUD methods correctly. Previously the documentations showed an interface being returned. Code formatting in the example and inline code highlighting. --- src/main/asciidoc/repositories.adoc | 53 ++++++++++++++++------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index cdb3374383..918580efb3 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -505,15 +505,16 @@ The preceding approach is not feasible when you want to add a single method to a ==== [source, java] ---- +@NoRepositoryBean public interface MyRepository - extends JpaRepository { + extends PagingAndSortingRepository { void sharedCustomMethod(ID id); } ---- ==== -. Now your individual repository interfaces will extend this intermediate interface instead of the Repository interface to include the functionality declared. +. Now your individual repository interfaces will extend this intermediate interface instead of the `Repository` interface to include the functionality declared. . Next, create an implementation of the intermediate interface that extends the persistence technology-specific repository base class. This class will then act as a custom base class for the repository proxies. + @@ -524,13 +525,12 @@ public interface MyRepository public class MyRepositoryImpl extends SimpleJpaRepository implements MyRepository { - private EntityManager entityManager; + private final EntityManager entityManager; - // There are two constructors to choose from, either can be used. public MyRepositoryImpl(Class domainClass, EntityManager entityManager) { super(domainClass, entityManager); - // This is the recommended method for accessing inherited class dependencies. + // Keep the EntityManager around to used from the newly introduced methods. this.entityManager = entityManager; } @@ -541,49 +541,45 @@ public class MyRepositoryImpl ---- ==== + -The default behavior of the Spring `` namespace is to provide an implementation for all interfaces that fall under the `base-package`. This means that if left in its current state, an implementation instance of MyRepository will be created by Spring. This is of course not desired as it is just supposed to act as an intermediary between Repository and the actual repository interfaces you want to define for each entity. To exclude an interface that extends Repository from being instantiated as a repository instance, you can either annotate it with @NoRepositoryBean or move it outside of the configured `base-package`. +The default behavior of the Spring `` namespace is to provide an implementation for all interfaces that fall under the `base-package`. This means that if left in its current state, an implementation instance of `MyRepository` will be created by Spring. This is of course not desired as it is just supposed to act as an intermediary between `Repository` and the actual repository interfaces you want to define for each entity. To exclude an interface that extends `Repository` from being instantiated as a repository instance, you can either annotate it with `@NoRepositoryBean` (as seen above) or move it outside of the configured `base-package`. -. Then create a custom repository factory to replace the default RepositoryFactoryBean that will in turn produce a custom RepositoryFactory. The new repository factory will then provide your MyRepositoryImpl as the implementation of any interfaces that extend the Repository interface, replacing the SimpleJpaRepository implementation you just extended. +. Then create a custom repository factory to replace the default `RepositoryFactoryBean` that will in turn produce a custom `RepositoryFactory`. The new repository factory will then provide your `MyRepositoryImpl` as the implementation of any interfaces that extend the `Repository` interface, replacing the `SimpleJpaRepository` implementation you just extended. + .Custom repository factory bean ==== [source, java] ---- -public class MyRepositoryFactoryBean, T, I extends Serializable> - extends JpaRepositoryFactoryBean { - - protected RepositoryFactorySupport createRepositoryFactory(EntityManager entityManager) { +public class MyRepositoryFactoryBean, T, + I extends Serializable> extends JpaRepositoryFactoryBean { - return new MyRepositoryFactory(entityManager); + protected RepositoryFactorySupport createRepositoryFactory(EntityManager em) { + return new MyRepositoryFactory(em); } - private static class MyRepositoryFactory extends JpaRepositoryFactory { + private static class MyRepositoryFactory + extends JpaRepositoryFactory { - private EntityManager entityManager; + private final EntityManager em; - public MyRepositoryFactory(EntityManager entityManager) { - super(entityManager); + public MyRepositoryFactory(EntityManager em) { - this.entityManager = entityManager; + super(em); + this.em = em; } protected Object getTargetRepository(RepositoryMetadata metadata) { - - return new MyRepositoryImpl((Class) metadata.getDomainClass(), entityManager); + return new MyRepositoryImpl((Class) metadata.getDomainClass(), em); } protected Class getRepositoryBaseClass(RepositoryMetadata metadata) { - - // The RepositoryMetadata can be safely ignored, it is used by the JpaRepositoryFactory - //to check for QueryDslJpaRepository's which is out of scope. - return MyRepository.class; + return MyRepositoryImpl.class; } } } ---- ==== -. Finally, either declare beans of the custom factory directly or use the `factory-class` attribute of the Spring namespace to tell the repository infrastructure to use your custom factory implementation. +. Finally, either declare beans of the custom factory directly or use the `factory-class` attribute of the Spring namespace or `@Enable…` annotation to instruct the repository infrastructure to use your custom factory implementation. + .Using the custom factory with the namespace ==== @@ -593,6 +589,15 @@ public class MyRepositoryFactoryBean, T, I extends factory-class="com.acme.MyRepositoryFactoryBean" /> ---- ==== ++ +.Using the custom factory with the `@Enable…` annotation +==== +[source, java] +---- +@EnableJpaRepositories(factoryClass = "com.acme.MyRepositoryFactoryBean") +class Config {} +---- +==== [[core.extensions]] == Spring Data extensions From bf9dfcb386b432a9583ebbe8b04e6b4df96d02a8 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 5 Mar 2015 15:54:17 +0100 Subject: [PATCH 45/59] DATACMNS-652 - Updated changelog. --- src/main/resources/changelog.txt | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/main/resources/changelog.txt b/src/main/resources/changelog.txt index c45a0a4b96..10b9d137a7 100644 --- a/src/main/resources/changelog.txt +++ b/src/main/resources/changelog.txt @@ -1,6 +1,41 @@ Spring Data Commons Changelog ============================= +Changes in version 1.10.0.RC1 (2015-03-05) +------------------------------------------ +* DATACMNS-652 - Release 1.10 RC1. +* DATACMNS-651 - Add domain types to represent a range of values. +* DATACMNS-650 - Provide a CloseableIterator abstraction as a foundation for streaming of results. +* DATACMNS-649 - Incorrect reference docs on extending all repositories causes failures in lookup of default transaction metadata. +* DATACMNS-647 - Add test case for collection parameter invocation on ReflectionRepositoryInvoker. +* DATACMNS-646 - Enable build for Spring 4.2 profile. +* DATACMNS-645 - Typo in reference documentation. +* DATACMNS-643 - Add support for JSR-310 and ThreeTen Backport types in auditing support. +* DATACMNS-642 - Performance improvement - BeanWrapper.setProperty should not lock when unnecessary. +* DATACMNS-641 - Default sort direction to ascending order in case none is specified in derived repository queries. +* DATACMNS-640 - Possible NullPointerException on PageableHandlerMethodArgumentResolver when fallbackPageable is null. +* DATACMNS-638 - Let auditing infrastructure work with PersistentEntities rather than a single MappingContext. +* DATACMNS-637 - General inspection for performance improvements. +* DATACMNS-636 - Add 'exists' method to QueryDslPredicateExecutor which accepts a querydsl Predicate. +* DATACMNS-635 - Provide an easier way to instantiate Page. +* DATACMNS-634 - Repositories should return repository for entity's super-type if available. +* DATACMNS-631 - Make RepositoryFactorySupport.getRepositoryMetadata(…) protected. +* DATACMNS-630 - Move core projection infrastructure from Spring Data REST into Commons. +* DATACMNS-629 - Reference documentation section on limiting query results is showing incorrect order-by-clause. +* DATACMNS-628 - Add converters for ThreeTen backport library. +* DATACMNS-627 - Extend DomainClassConverter to be able to convert entity into its identifier. +* DATACMNS-626 - Port converters for Point and Distance from Spring Data REST and turn them into Formatters. +* DATACMNS-625 - Re-enable querydsl-next profile for Travis. +* DATACMNS-623 - Add converter for Instants to JSR-310 Converters. +* DATACMNS-621 - QSort does not treat nested paths correctly. +* DATACMNS-619 - DefaultCrudMethods should always return accessible methods. +* DATACMNS-616 - AnnotationRevisionMetadata can't access private fields. +* DATACMNS-615 - PageImpl should reject total less than given content length. +* DATACMNS-611 - Avoid triple cache lookup in RepositoryInterfaceAwareBeanPostProcessor.predictBeanType(…). +* DATACMNS-610 - Add converter support for Set return types. +* DATACMNS-554 - Add QueryDslPredicateExecutor.findAll(Predicate, Sort) method. + + Changes in version 1.9.2.RELEASE (2015-01-28) --------------------------------------------- * DATACMNS-637 - General inspection for performance improvements. From 8bf02ac6c96ef611a5b28daeaebdbed844732010 Mon Sep 17 00:00:00 2001 From: Arlo Louis O'Keeffe Date: Tue, 10 Feb 2015 11:39:49 +0100 Subject: [PATCH 46/59] DATACMNS-674 - Fixed typo in reference documentation. --- src/main/asciidoc/repositories.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index 918580efb3..ecbc3b05fa 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -205,7 +205,7 @@ NOTE: Note, that the intermediate repository interface is annotated with `@NoRep [[repositories.query-methods.details]] == Defining query methods -The repository proxy has two ways to derive a store-specific query from the method name. It can derive the query from the method name directly, or by using a manually defined query. Available options depend on the actual store. However, there's got to be an strategy that decides what actual query is created. Let's have a look at the available options. +The repository proxy has two ways to derive a store-specific query from the method name. It can derive the query from the method name directly, or by using a manually defined query. Available options depend on the actual store. However, there's got to be a strategy that decides what actual query is created. Let's have a look at the available options. [[repositories.query-methods.query-lookup-strategies]] === Query lookup strategies From d67c43803c4be128d4494f65af5b143dacd58253 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 7 Apr 2015 08:39:59 +0200 Subject: [PATCH 47/59] DATACMNS-677 - AnnotationBasedPersistentProperty now caches absence of annotations on accessor-only properties. AnnotationBasedPersistentProperty now also caches the absence of properties that are expressed through accessors only. Previously the absence of a field caused us to skip the registration of the absence in the cache. --- .../AnnotationBasedPersistentProperty.java | 7 +++--- ...ationBasedPersistentPropertyUnitTests.java | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index 36c1f66498..8820f1cdaa 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -201,8 +201,9 @@ public boolean isWritable() { } /** - * Returns the annotation found for the current {@link AnnotationBasedPersistentProperty}. Will prefer field - * annotations over ones found at getters or setters. + * Returns the annotation found for the current {@link AnnotationBasedPersistentProperty}. Will prefer getters or + * setters annotations over ones found at the backing field as the former can be used to reconfigure the metadata in + * subclasses. * * @param annotationType must not be {@literal null}. * @return @@ -229,7 +230,7 @@ public A findAnnotation(Class annotationType) { } } - return field == null ? null : cacheAndReturn(annotationType, AnnotationUtils.getAnnotation(field, annotationType)); + return cacheAndReturn(annotationType, field == null ? null : AnnotationUtils.getAnnotation(field, annotationType)); } /* diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index fb8988f94f..8e7e14d1d7 100644 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -203,6 +203,23 @@ public void doesNotRejectNonSpringDataAnnotationsUsedOnBothFieldAndAccessor() { getProperty(TypeWithCustomAnnotationsOnBothFieldAndAccessor.class, "field"); } + /** + * @see DATACMNS-677 + */ + @Test + @SuppressWarnings("unchecked") + public void cachesNonPresenceOfAnnotationOnField() { + + SamplePersistentProperty property = getProperty(Sample.class, "getterWithoutField"); + + assertThat(property.findAnnotation(MyAnnotation.class), is(nullValue())); + + Map, ?> field = (Map, ?>) ReflectionTestUtils.getField(property, "annotationCache"); + + assertThat(field.containsKey(MyAnnotation.class), is(true)); + assertThat(field.get(MyAnnotation.class), is(nullValue())); + } + @SuppressWarnings("unchecked") private Map, Annotation> getAnnotationCache(SamplePersistentProperty property) { return (Map, Annotation>) ReflectionTestUtils.getField(property, "annotationCache"); @@ -250,6 +267,13 @@ public String getDoubleMapping() { public void setDoubleMapping(String doubleMapping) { this.doubleMapping = doubleMapping; } + + @AccessType(Type.PROPERTY) + public Object getGetterWithoutField() { + return null; + } + + public void setGetterWithoutField(Object object) {} } static class InvalidSample { From 97b8d45eeeeea74a32c24adae7d9af531cec9e85 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 13 Apr 2015 13:04:59 +0200 Subject: [PATCH 48/59] DATACMNS-681 - Removed some compiler warnings. Removed unused constants. Removed usage of deprecated method in RepositoryBeanDefinitionParser. --- .../org/springframework/data/repository/query/Parameters.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 2965f0b3d4..5f577a9d4a 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameters.java +++ b/src/main/java/org/springframework/data/repository/query/Parameters.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-2015 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. @@ -37,6 +37,7 @@ */ public abstract class Parameters, T extends Parameter> implements Iterable { + @SuppressWarnings("unchecked")// 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", From 57fca31ac713364f5dff3d8e3b99e467629954bc Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 13 Apr 2015 13:15:18 +0200 Subject: [PATCH 49/59] DATACMNS-679 - Updated Sonargraph architecture description. --- Spring Data Commons.sonargraph | 123 +++++++++++++++++---------------- 1 file changed, 63 insertions(+), 60 deletions(-) diff --git a/Spring Data Commons.sonargraph b/Spring Data Commons.sonargraph index 171fe21744..6784b05809 100644 --- a/Spring Data Commons.sonargraph +++ b/Spring Data Commons.sonargraph @@ -1,6 +1,9 @@ - + + + + @@ -10,16 +13,11 @@ - - - - - @@ -71,6 +69,7 @@ + @@ -93,13 +92,6 @@ - - - - - - - @@ -114,6 +106,13 @@ + + + + + + + @@ -176,6 +175,11 @@ + + + + + @@ -193,12 +197,6 @@ - - - - - - @@ -324,6 +322,11 @@ + + + + + @@ -395,58 +398,58 @@ - - - - - - + + + + + + + - - - - - - - - + - - + + - - - - - - - - - - - - - - - + + + + + - - - - - - - + + + + + + + + + + + + + + + + + + + + + - - - + + + + + From 1924aaf7affa418c6920530a088d9ef9887ab611 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 13 Apr 2015 15:25:33 +0200 Subject: [PATCH 50/59] DATACMNS-682 - Add details of how to customize repository scanning using JavaConfig. Completed description for the packages-to-scan customization with JavaConfig. Original pull request: #121. --- src/main/asciidoc/repositories.adoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index ecbc3b05fa..cd0a6f97aa 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -149,7 +149,9 @@ or via <>: ---- + -The JPA namespace is used in this example. If you are using the repository abstraction for any other store, you need to change this to the appropriate namespace declaration of your store module which should be exchanging `jpa` in favor of, for example, `mongodb`. Also, note that the JavaConfig variant doesn't configure a package explictly as the package of the annotated class is used by default. To customize the package to scan +The JPA namespace is used in this example. If you are using the repository abstraction for any other store, you need to change this to the appropriate namespace declaration of your store module which should be exchanging `jpa` in favor of, for example, `mongodb`. + +Also, note that the JavaConfig variant doesn't configure a package explictly as the package of the annotated class is used by default. To customize the package to scan use one of the `basePackage…` attribute of the data-store specific repository `@Enable…`-annotation. . Get the repository instance injected and use it. + From a61bd1406b419189dab0193e3000b957e6587c90 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 4 May 2015 14:23:39 +0200 Subject: [PATCH 51/59] DATACMNS-687 - Added section on dependency management to reference docs. Added a dedicated dependencies.adoc that outlines the usage of the Spring Data release train BOM as well as the way one would customize the release train to be used with Spring Boot. Also, we now explicitly list the Spring Framework version the release requires. --- src/main/asciidoc/dependencies.adoc | 57 +++++++++++++++++++++++++++++ src/main/asciidoc/index.adoc | 1 + src/main/asciidoc/preface.adoc | 2 +- 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 src/main/asciidoc/dependencies.adoc diff --git a/src/main/asciidoc/dependencies.adoc b/src/main/asciidoc/dependencies.adoc new file mode 100644 index 0000000000..2fd0a0f88b --- /dev/null +++ b/src/main/asciidoc/dependencies.adoc @@ -0,0 +1,57 @@ +[[dependencies]] += Dependencies + +Due to different inception dates of individual Spring Data modules, most of them carry different major and minor version numbers. The easiest way to find compatible ones is by relying on the Spring Data Release Train BOM we ship with the compatible versions defined. In a Maven project you'd declare this dependency in the `` section of your POM: + +.Using the Spring Data release train BOM +==== +[source, xml] +---- + + + + org.springframework.data + spring-data-releasetrain + ${release-train} + import + pom + + + +---- +==== + +The current release train version is {releasetrainVersion}. The train names are ascending alphabetically and currently available ones are listed https://github.com/spring-projects/spring-data-commons/wiki/Release-planning[here]. The version name follows the following pattern: `${name}-${release}` where release can be one of the following: + +* `BUILD-SNAPSHOT` - current snapshots +* `M1`, `M2` etc. - milestones +* `RC1`, `RC2` etc. - release candidates +* `RELEASE` - GA release +* `SR1`, `SR2` etc. - service releases + +A working example of using the BOMs can be found in our https://github.com/spring-projects/spring-data-examples/tree/master/bom[Spring Data examples repository]. + +If that's in place declare the Spring Data modules you'd like to use without a version in the `` block. + +.Declaring a dependency to a Spring Data module +==== +[source, xml] +---- + + + org.springframework.data + spring-data-jpa + + +---- +==== + +[[dependencies.spring-boot]] +== Dependency management with Spring Boot + +Spring Boot already selects a very recent version of Spring Data modules for you. In case you want to upgrade to a newer version nonetheless, simply configure the property `spring-data-releasetrain.version` to the train name and iteration you'd like to use. + +[[dependencies.spring-framework]] +== Spring Framework + +The current version of Spring Data modules require Spring Framework in version {springVersion} or better. The modules might also work with an older bugfix version of that minor version. However, using the most recent version within that generation is highly recommended. diff --git a/src/main/asciidoc/index.adoc b/src/main/asciidoc/index.adoc index 48414ee5e1..2d2c936147 100644 --- a/src/main/asciidoc/index.adoc +++ b/src/main/asciidoc/index.adoc @@ -17,6 +17,7 @@ include::preface.adoc[] = Reference documentation :leveloffset: +1 +include::dependencies.adoc[] include::repositories.adoc[] include::auditing.adoc[] :leveloffset: -1 diff --git a/src/main/asciidoc/preface.adoc b/src/main/asciidoc/preface.adoc index 47788ffa07..226ebd4409 100644 --- a/src/main/asciidoc/preface.adoc +++ b/src/main/asciidoc/preface.adoc @@ -10,4 +10,4 @@ The Spring Data Commons project applies core Spring concepts to the development * Bugtracker - https://jira.spring.io/browse/DATACMNS * Release repository - https://repo.spring.io/libs-release * Milestone repository - https://repo.spring.io/libs-milestone -* Snapshot repository - https://repo.spring.io/libs-snapshot \ No newline at end of file +* Snapshot repository - https://repo.spring.io/libs-snapshot From 26b3516623657fcd77f6459961c7a14c1bfdf4e7 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 7 May 2015 10:44:17 +0200 Subject: [PATCH 52/59] DATACMNS-689 - Improved reference documentation on web pagination. Re-added index and defaulting information to the section that documents the currents state of web pagination. Removed legacy pagination sections as they're not really documenting the very legacy support (that would not even work anymore). Added explicit anchors where missing. --- src/main/asciidoc/repositories.adoc | 148 +--------------------------- 1 file changed, 5 insertions(+), 143 deletions(-) diff --git a/src/main/asciidoc/repositories.adoc b/src/main/asciidoc/repositories.adoc index cd0a6f97aa..e21f5f5e78 100644 --- a/src/main/asciidoc/repositories.adoc +++ b/src/main/asciidoc/repositories.adoc @@ -646,6 +646,7 @@ The configuration setup shown above will register a few basic components: - A `DomainClassConverter` to enable Spring MVC to resolve instances of repository managed domain classes from request parameters or path variables. - `HandlerMethodArgumentResolver` implementations to let Spring MVC resolve Pageable and Sort instances from request parameters. +[[core.web.basic.domain-class-converter]] ===== DomainClassConverter The `DomainClassConverter` allows you to use domain types in your Spring MVC controller method signatures directly, so that you don't have to manually lookup the instances via the repository: @@ -671,6 +672,7 @@ As you can see the method receives a User instance directly and no further looku NOTE: Currently the repository has to implement `CrudRepository` to be eligible to be discovered for conversion. +[[core.web.basic.paging-and-sorting]] ===== HandlerMethodArgumentResolvers for Pageable and Sort The configuration snippet above also registers a `PageableHandlerMethodArgumentResolver` as well as an instance of `SortHandlerMethodArgumentResolver`. The registration enables `Pageable` and `Sort` being valid controller method arguments @@ -699,8 +701,8 @@ This method signature will cause Spring MVC try to derive a Pageable instance fr .Request parameters evaluated for Pageable instances [options = "autowidth"] |=============== -|`page`|Page you want to retrieve. -|`size`|Size of the page you want to retrieve. +|`page`|Page you want to retrieve, 0 indexed and defaults to 0. +|`size`|Size of the page you want to retrieve, defaults to 20. |`sort`|Properties that should be sorted by in the format `property,property(,ASC\|DESC)`. Default sort direction is ascending. Use multiple `sort` parameters if you want to switch directions, e.g. `?sort=firstname&sort=lastname,asc`. |=============== @@ -879,6 +881,7 @@ public class UserController { First you declare a repository dependency for each controller to look up the entity managed by the controller or repository respectively. Looking up the entity is boilerplate as well, as it's always a `findOne(…)` call. Fortunately Spring provides means to register custom components that allow conversion between a `String` value to an arbitrary type. +[[web.legacy.property-editors]] ===== PropertyEditors For Spring versions before 3.0 simple Java `PropertyEditors` had to be used. To integrate with that, Spring Data offers a `DomainClassPropertyEditorRegistrar`, which looks up all Spring Data repositories registered in the `ApplicationContext` and registers a custom `PropertyEditor` for the managed domain class. @@ -913,144 +916,3 @@ public class UserController { } ---- -ConversionServiceIn Spring 3.0 and later the `PropertyEditor` support is superseded by a new conversion infrastructure that eliminates the drawbacks of `PropertyEditors` and uses a stateless X to Y conversion approach. Spring Data now ships with a `DomainClassConverter` that mimics the behavior of `DomainClassPropertyEditorRegistrar`. To configure, simply declare a bean instance and pipe the `ConversionService` being used into its constructor: - -[source, xml] ----- - - - - - ----- - -If you are using JavaConfig, you can simply extend Spring MVC's `WebMvcConfigurationSupport` and hand the `FormatingConversionService` that the configuration superclass provides into the `DomainClassConverter` instance you create. - -[source, java] ----- -class WebConfiguration extends WebMvcConfigurationSupport { - - // Other configuration omitted - - @Bean - public DomainClassConverter domainClassConverter() { - return new DomainClassConverter(mvcConversionService()); - } -} ----- - -[[web-pagination]] -==== Web pagination - -When working with pagination in the web layer you usually have to write a lot of boilerplate code yourself to extract the necessary metadata from the request. The less desirable approach shown in the example below requires the method to contain an `HttpServletRequest` parameter that has to be parsed manually. This example also omits appropriate failure handling, which would make the code even more verbose. - -[source, java] ----- -@Controller -@RequestMapping("/users") -public class UserController { - - // DI code omitted - - @RequestMapping - public String showUsers(Model model, HttpServletRequest request) { - - int page = Integer.parseInt(request.getParameter("page")); - int pageSize = Integer.parseInt(request.getParameter("pageSize")); - - Pageable pageable = new PageRequest(page, pageSize); - - model.addAttribute("users", userService.getUsers(pageable)); - return "users"; - } -} ----- - -The bottom line is that the controller should not have to handle the functionality of extracting pagination information from the request. So Spring Data ships with a `PageableHandlerMethodArgumentResolver` that will do the work for you. The Spring MVC JavaConfig support exposes a `WebMvcConfigurationSupport` helper class to customize the configuration as follows: - -[source, java] ----- -@Configuration -public class WebConfig extends WebMvcConfigurationSupport { - - @Override - protected void addArgumentResolvers(List argumentResolvers) { - argumentResolvers.add(new PageableHandlerMethodArgumentResolver()); - } -} ----- - -If you're stuck with XML configuration you can register the resolver as follows: - -[source, xml] ----- - - - - - - - ----- - -Once you've configured the resolver with Spring MVC it allows you to simplify controllers down to something like this: - -[source, java] ----- -@Controller -@RequestMapping("/users") -public class UserController { - - @RequestMapping - public String showUsers(Model model, Pageable pageable) { - - model.addAttribute("users", userRepository.findAll(pageable)); - return "users"; - } -} ----- - -The `PageableArgumentResolver` automatically resolves request parameters to build a `PageRequest` instance. By default it expects the following structure for the request parameters. - -.Request parameters evaluated by PageableHandlerMethodArgumentResolver -[options = "autowidth"] -|=============== -|`page`|Page you want to retrieve, 0 indexed and defaults to 0. -|`size`|Size of the page you want to retrieve, defaults to 20. -|`sort`|A collection of sort directives in the format `($propertyname,)[asc\|desc]?`. -|=============== - -.Pagination URL parameter examples - -To retrieve the third page with a maximum page size of 100 with the data sorted by the email property in ascending order use the following url parameter: -==== ----- -?page=2&size=100&sort=email,asc ----- -==== -To sort the data by multiple properties in different sort order use the following URL parameter: -==== ----- -?sort=foo,asc&sort=bar,desc ----- -==== - -In case you need multiple `Pageable` instances to be resolved from the request (for multiple tables, for example) you can use Spring's `@Qualifier` annotation to distinguish one from another. The request parameters then have to be prefixed with `${qualifier}_`. So for a method signature like this: - -[source, java] ----- -public String showUsers(Model model, - @Qualifier("foo") Pageable first, - @Qualifier("bar") Pageable second) { … } ----- - -you have to populate `foo_page` and `bar_page` and the related subproperties. - -Configuring a global default on bean declaration the `PageableArgumentResolver` will use a `PageRequest` with the first page and a page size of 10 by default. It will use that value if it cannot resolve a `PageRequest` from the request (because of missing parameters, for example). You can configure a global default on the bean declaration directly. If you might need controller method specific defaults for the `Pageable`, annotate the method parameter with `@PageableDefaults` and specify page (through `pageNumber`), page size (through `value`), `sort` (list of properties to sort by), and `sortDir` (the direction to sort by) as annotation attributes: - -[source, java] ----- -public String showUsers(Model model, - @PageableDefaults(pageNumber = 0, value = 30) Pageable pageable) { … } ----- - From e70cfbfd57658ca7b2bf0a72018bd94e6ca683e4 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 11 May 2015 15:39:01 +0200 Subject: [PATCH 53/59] DATACMNS-691 - Removed Travis build profile for Spring 4.0.next. --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index f053084055..642e672f51 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,6 @@ jdk: env: matrix: - PROFILE=ci - - PROFILE=spring4-next - PROFILE=spring41 - PROFILE=spring41-next - PROFILE=spring42-next From 156f7c36a4e382cbd224b5e99cacf599bebe8f6e Mon Sep 17 00:00:00 2001 From: Tomasz Wysocki Date: Wed, 13 May 2015 12:24:28 +0200 Subject: [PATCH 54/59] DATACMNS-693 - AbstractMappingContext now uses Spring's BeanUtils to lookup PropertyDescriptors. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of manually using Introspector.getBeanInfo(…) we now use Spring's BeanUtils.getPropertyDescriptors(…) to benefit from some caching of descriptor instances as well as advanced support for fluent setters, default methods etc. Original pull request: #122. --- .../data/mapping/context/AbstractMappingContext.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index d64ff32000..47e9dbd85d 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -15,9 +15,6 @@ */ package org.springframework.data.mapping.context; -import java.beans.BeanInfo; -import java.beans.IntrospectionException; -import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Field; import java.lang.reflect.Modifier; @@ -34,6 +31,8 @@ import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantReadWriteLock; +import org.springframework.beans.BeanUtils; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; @@ -63,6 +62,7 @@ * @author Oliver Gierke * @author Michael Hunger * @author Thomas Darimont + * @author Tomasz Wysocki */ public abstract class AbstractMappingContext, P extends PersistentProperty

> implements MappingContext, ApplicationEventPublisherAware, InitializingBean { @@ -281,10 +281,10 @@ protected E addPersistentEntity(TypeInformation typeInformation) { // Eagerly cache the entity as we might have to find it during recursive lookups. persistentEntities.put(typeInformation, entity); - BeanInfo info = Introspector.getBeanInfo(type); + PropertyDescriptor[] pds = BeanUtils.getPropertyDescriptors(type); final Map descriptors = new HashMap(); - for (PropertyDescriptor descriptor : info.getPropertyDescriptors()) { + for (PropertyDescriptor descriptor : pds) { descriptors.put(descriptor.getName(), descriptor); } @@ -308,7 +308,7 @@ protected E addPersistentEntity(TypeInformation typeInformation) { return entity; - } catch (IntrospectionException e) { + } catch (BeansException e) { throw new MappingException(e.getMessage(), e); } finally { write.unlock(); From 9069e72d9f041a8dab229215fdf2e0ac930f88d2 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 13 May 2015 14:41:19 +0200 Subject: [PATCH 55/59] DATACMNS-694 - Removed element from parent POM declaration. --- pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/pom.xml b/pom.xml index f51fe6d45c..08c842d3da 100644 --- a/pom.xml +++ b/pom.xml @@ -13,7 +13,6 @@ org.springframework.data.build spring-data-parent 1.5.3.BUILD-SNAPSHOT - ../spring-data-build/parent/pom.xml From abe0ac13b5af3183791108f457ebb2f1c1c8358a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 13 May 2015 18:00:19 +0200 Subject: [PATCH 56/59] DATACMNS-692 - Fixed web parameter range handling for Pageables. In case the PageableHandlerMethodArgumentResolver was configured to use one-indexed parameters, it wasn't defaulting the lower bounds for the page number. This caused indexes out of the allowed bound submitted causing an invalid index handed tor PageRequest. We now apply better range shifting before the bounds are applied. --- ...PageableHandlerMethodArgumentResolver.java | 15 +++++++-------- ...andlerMethodArgumentResolverUnitTests.java | 19 ++++++++++++++++++- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java index 7a5f98e52d..3fe91dcd85 100644 --- a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java @@ -235,9 +235,9 @@ public Pageable resolveArgument(MethodParameter methodParameter, ModelAndViewCon return null; } - int page = StringUtils.hasText(pageString) ? parseAndApplyBoundaries(pageString, 0, Integer.MAX_VALUE) - - (oneIndexedParameters ? 1 : 0) : defaultOrFallback.getPageNumber(); - int pageSize = StringUtils.hasText(pageSizeString) ? parseAndApplyBoundaries(pageSizeString, 0, maxPageSize) + int page = StringUtils.hasText(pageString) ? parseAndApplyBoundaries(pageString, Integer.MAX_VALUE) + : defaultOrFallback.getPageNumber(); + int pageSize = StringUtils.hasText(pageSizeString) ? parseAndApplyBoundaries(pageSizeString, maxPageSize) : defaultOrFallback.getPageSize(); // Limit lower bound @@ -306,17 +306,16 @@ private static Pageable getDefaultPageRequestFrom(MethodParameter parameter) { * boundary if the {@link String} cannot be parsed. * * @param parameter - * @param lower * @param upper * @return */ - private static int parseAndApplyBoundaries(String parameter, int lower, int upper) { + private int parseAndApplyBoundaries(String parameter, int upper) { try { - int parsed = Integer.parseInt(parameter); - return parsed < lower ? lower : parsed > upper ? upper : parsed; + int parsed = Integer.parseInt(parameter) - (oneIndexedParameters ? 1 : 0); + return parsed < 0 ? 0 : parsed > upper ? upper : parsed; } catch (NumberFormatException e) { - return lower; + return 0; } } } diff --git a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java index 2fa3b0715a..d29e5b23b4 100644 --- a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2015 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. @@ -236,6 +236,23 @@ public void usesNullSortIfNoDefaultIsConfiguredAndPageAndSizeAreGiven() { assertThat(result.getSort(), is(nullValue())); } + /** + * @see DATACMNS-692 + */ + @Test + public void oneIndexedParametersDefaultsIndexOutOfRange() { + + PageableHandlerMethodArgumentResolver resolver = getResolver(); + resolver.setOneIndexedParameters(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("page", "0"); + + Pageable result = resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null); + + assertThat(result.getPageNumber(), is(0)); + } + @Override protected PageableHandlerMethodArgumentResolver getResolver() { PageableHandlerMethodArgumentResolver resolver = new PageableHandlerMethodArgumentResolver(); From a4118e437456f1057bf2e7302414527d993ed58a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 13 May 2015 18:33:58 +0200 Subject: [PATCH 57/59] =?UTF-8?q?DATACMNS-695=20-=20Fixed=20potential=20Nu?= =?UTF-8?q?llPointerException=20in=20AbstractMappingContext.getPersistentP?= =?UTF-8?q?ropertyPath(=E2=80=A6).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When traversing nested property paths, AbstractMappingContext.getPersistentPropertyPath(…) previously used the raw actual property type. If the property path contains a reference to a generically typed property, this causes the deeper paths not being resolved correctly. We now explicitly use the TypeInformation of the property to retain generics information while traversing the path. --- .../context/AbstractMappingContext.java | 2 +- .../AbstractMappingContextUnitTests.java | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index 47e9dbd85d..4db922024e 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -239,7 +239,7 @@ private PersistentPropertyPath

getPersistentPropertyPath(Iterable par result.add(persistentProperty); if (iterator.hasNext()) { - current = getPersistentEntity(persistentProperty.getActualType()); + current = getPersistentEntity(persistentProperty.getTypeInformation().getActualType()); } } diff --git a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java index 7ec9bea929..01bc065c1f 100644 --- a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.TreeMap; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -245,6 +246,15 @@ public void doesNotAddMapImplementationClassesAsPersistentEntity() { assertHasEntityFor(TreeMap.class, context, false); } + /** + * @see DATACMNS-695 + */ + @Test + public void persistentPropertyPathTraversesGenericTypesCorrectly() { + assertThat(context.getPersistentPropertyPath("field.wrapped.field", Outer.class), + is(Matchers. iterableWithSize(3))); + } + private static void assertHasEntityFor(Class type, SampleMappingContext context, boolean expected) { boolean found = false; @@ -283,4 +293,17 @@ static class Base { static class Extension extends Base { @Id String foo; } + + static class Outer { + + Wrapper field; + } + + static class Wrapper { + T wrapped; + } + + static class Inner { + String field; + } } From 5d1019ce9219f14bac93f457d68ec6f479f04395 Mon Sep 17 00:00:00 2001 From: Greg Turnquist Date: Thu, 14 May 2015 12:42:38 -0500 Subject: [PATCH 58/59] DATACMNS-696 - Enable Slack notifications for Travis build. Original pull request: #123. --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 642e672f51..45f747c5d4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,4 +13,7 @@ cache: - $HOME/.m2 sudo: false install: true -script: "mvn clean dependency:list test -P${PROFILE} -Dsort" \ No newline at end of file +script: "mvn clean dependency:list test -P${PROFILE} -Dsort" +notifications: + slack: + secure: PWyr3+7uTnHzZSrJY2DrowwdYODlIeFZB6tzuq+i/vYqMlX2GGXGQN9YmqbKPeaVZBdSI7YrI1UDa5cPAAXoTjn/JewkL0RbdTeIS6PGCpcpAb5rFMYtOhEDBrFB/SRiMH4tw86bRNKq/SrUWmpkzDtTzLrw7JceumgyqnrT6GY= From 267d7d24d65be6ca91861b1a57d5bac2ab0a9915 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sat, 16 May 2015 18:55:47 +0200 Subject: [PATCH 59/59] DATACMNS-697 - TypeDiscoverer now considers field-local generics information. In case we create a ParameterizedTypeInformation for a property we now also analyze the field-local generics information and add the discovered type mappings to the type variable map. --- .../data/util/TypeDiscoverer.java | 13 +++++++- .../data/util/ParameterizedTypeUnitTests.java | 31 ++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java index ddffff1932..b1e54ab2eb 100644 --- a/src/main/java/org/springframework/data/util/TypeDiscoverer.java +++ b/src/main/java/org/springframework/data/util/TypeDiscoverer.java @@ -100,10 +100,21 @@ protected TypeInformation createInfo(Type fieldType) { return new ClassTypeInformation((Class) fieldType); } - Map variableMap = GenericTypeResolver.getTypeVariableMap(resolveType(fieldType)); + Class resolveType = resolveType(fieldType); + Map variableMap = new HashMap(); + variableMap.putAll(GenericTypeResolver.getTypeVariableMap(resolveType)); if (fieldType instanceof ParameterizedType) { + ParameterizedType parameterizedType = (ParameterizedType) fieldType; + + TypeVariable>[] typeParameters = resolveType.getTypeParameters(); + Type[] arguments = parameterizedType.getActualTypeArguments(); + + for (int i = 0; i < typeParameters.length; i++) { + variableMap.put(typeParameters[i], arguments[i]); + } + return new ParameterizedTypeInformation(parameterizedType, this, variableMap); } diff --git a/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java b/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java index 1eae5bd99a..5d3f671065 100644 --- a/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java +++ b/src/test/java/org/springframework/data/util/ParameterizedTypeUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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. @@ -25,6 +25,7 @@ import java.lang.reflect.TypeVariable; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; @@ -125,6 +126,17 @@ public void getActualTypeShouldNotUnwrapParameterizedTypes() { assertThat(type.getActualType(), is(type)); } + /** + * @see DATACMNS-697 + */ + @Test + public void usesLocalGenericInformationOfFields() { + + TypeInformation information = ClassTypeInformation.from(NormalizedProfile.class); + TypeInformation valueType = information.getProperty("education2.data").getComponentType(); + assertThat(valueType.getProperty("value").getType(), is(typeCompatibleWith(Education.class))); + } + @SuppressWarnings("serial") class Localized extends HashMap { S value; @@ -151,4 +163,21 @@ class First { class Second { Parameterized property; } + + // see DATACMNS-697 + + class NormalizedProfile { + + ListField education2; + } + + class ListField { + List> data; + } + + class Value { + T value; + } + + class Education {} }