From c141fb771dc19fc498db506d371753f0b8949afb Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 22 Apr 2014 14:20:54 +0200 Subject: [PATCH] DATACMNS-483 - Support for wrapper types as return types for repository methods. The repository method invocation mechanism now post-processes the result of the actual method invocation in case it's not type-compatible with the return type of the invoked method and invokes a conversion if applicable. This allows us to register custom converters for well-known wrapper types. We currently support Optional from both Google Guava and JDK 8. Implementation notice We use some JDK 8 type stubs from the Spring Data Build project to be able to compile using JDKs < 8. This is necessary as we need to remain compatible with Spring 3.2.x for the Dijkstra release train and our test executions still run into some issues with it a Java 8 runtime (mostly ASM related). These issues were reported in [0, 1] and we'll probably get off the hack for Dijkstra GA by upgrading to Spring 3.2.9. [0] https://jira.spring.io/browse/SPR-11719 [1] https://jira.spring.io/browse/SPR-11718 --- pom.xml | 42 ++++-- .../support/AbstractRepositoryMetadata.java | 4 +- .../support/RepositoryFactorySupport.java | 45 +++++- .../data/repository/query/QueryMethod.java | 15 +- .../data/repository/util/NullableWrapper.java | 58 ++++++++ .../util/QueryExecutionConverters.java | 136 ++++++++++++++++++ .../DefaultRepositoryMetadataUnitTests.java | 21 +++ .../QueryExecutionConvertersUnitTests.java | 68 +++++++++ template.mf | 1 + 9 files changed, 368 insertions(+), 22 deletions(-) create mode 100644 src/main/java/org/springframework/data/repository/util/NullableWrapper.java create mode 100644 src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java create mode 100644 src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java diff --git a/pom.xml b/pom.xml index d1025f514..bf02aaaf9 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 - + org.springframework.data spring-data-commons 1.8.0.BUILD-SNAPSHOT @@ -15,13 +15,14 @@ 1.4.0.BUILD-SNAPSHOT ../spring-data-build/parent/pom.xml - + + 16.0.1 1.9.7 0.10.0.RELEASE DATACMNS - + org.springframework @@ -74,7 +75,7 @@ spring-web true - + javax.servlet servlet-api @@ -109,7 +110,7 @@ ${querydsl} provided - + javax.ejb @@ -126,21 +127,35 @@ provided true - + + + com.google.guava + guava + ${guava} + true + + + + org.springframework.data.build + spring-data-java8-stub + 1.4.0.BUILD-SNAPSHOT + provided + + javax.el el-api ${cdi} test - + org.apache.openwebbeans.test cditest-owb ${webbeans} test - + org.springframework.hateoas spring-hateoas @@ -152,14 +167,14 @@ spring-webmvc true - + com.sun.xml.bind jaxb-impl 2.2.3U1 test - + xmlunit xmlunit @@ -175,9 +190,10 @@ test - + + com.mysema.maven apt-maven-plugin @@ -205,14 +221,14 @@ - + spring-libs-snapshot http://repo.spring.io/libs-snapshot - + spring-plugins-release diff --git a/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java b/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java index a48599577..235186d9e 100644 --- a/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java +++ b/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java @@ -21,6 +21,7 @@ import java.util.Arrays; import org.springframework.data.domain.Pageable; import org.springframework.data.repository.core.CrudMethods; import org.springframework.data.repository.core.RepositoryMetadata; +import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -60,7 +61,8 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata { TypeInformation returnTypeInfo = typeInformation.getReturnType(method); Class rawType = returnTypeInfo.getType(); - boolean needToUnwrap = Iterable.class.isAssignableFrom(rawType) || rawType.isArray(); + boolean needToUnwrap = Iterable.class.isAssignableFrom(rawType) || rawType.isArray() + || QueryExecutionConverters.supports(rawType); return needToUnwrap ? returnTypeInfo.getComponentType().getType() : rawType; } 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 d3af0f90c..7abf45ed6 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 @@ -30,6 +30,9 @@ import org.aopalliance.intercept.MethodInvocation; import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.core.GenericTypeResolver; +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; @@ -40,6 +43,8 @@ import org.springframework.data.repository.query.QueryLookupStrategy.Key; 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; @@ -52,6 +57,8 @@ import org.springframework.util.ObjectUtils; */ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware { + private static final TypeDescriptor WRAPPER_TYPE = TypeDescriptor.valueOf(NullableWrapper.class); + private final Map REPOSITORY_INFORMATION_CACHE = new HashMap(); private final List postProcessors = new ArrayList(); @@ -273,6 +280,7 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware { private final Object customImplementation; private final RepositoryInformation repositoryInformation; + private final GenericConversionService conversionService; private final Object target; /** @@ -282,6 +290,13 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware { public QueryExecutorMethodInterceptor(RepositoryInformation repositoryInformation, Object customImplementation, Object target) { + 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.repositoryInformation = repositoryInformation; this.customImplementation = customImplementation; this.target = target; @@ -325,22 +340,45 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware { */ public Object invoke(MethodInvocation invocation) throws Throwable { + Object result = doInvoke(invocation); + Class expectedReturnType = invocation.getMethod().getReturnType(); + + if (result != null && expectedReturnType.isInstance(result)) { + return result; + } + + if (conversionService.canConvert(NullableWrapper.class, expectedReturnType) + && !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; + } + + private Object doInvoke(MethodInvocation invocation) throws Throwable { + Method method = invocation.getMethod(); + Object[] arguments = invocation.getArguments(); if (isCustomMethodInvocation(invocation)) { Method actualMethod = repositoryInformation.getTargetClassMethod(method); makeAccessible(actualMethod); - return executeMethodOn(customImplementation, actualMethod, invocation.getArguments()); + return executeMethodOn(customImplementation, actualMethod, arguments); } if (hasQueryFor(method)) { - return queries.get(method).execute(invocation.getArguments()); + return queries.get(method).execute(arguments); } // Lookup actual method as it might be redeclared in the interface // and we have to use the repository instance nevertheless Method actualMethod = repositoryInformation.getTargetClassMethod(method); - return executeMethodOn(target, actualMethod, invocation.getArguments()); + return executeMethodOn(target, actualMethod, arguments); } /** @@ -370,7 +408,6 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware { * @return */ private boolean hasQueryFor(Method method) { - return queries.containsKey(method); } 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 ffadf752f..a15a9a20a 100644 --- a/src/main/java/org/springframework/data/repository/query/QueryMethod.java +++ b/src/main/java/org/springframework/data/repository/query/QueryMethod.java @@ -40,6 +40,8 @@ public class QueryMethod { private final Method method; private final Parameters parameters; + private Class domainClass; + /** * Creates a new {@link QueryMethod} from the given parameters. Looks up the correct query to use for following * invocations of the method given. @@ -129,11 +131,16 @@ public class QueryMethod { */ protected Class getDomainClass() { - Class repositoryDomainClass = metadata.getDomainType(); - Class methodDomainClass = metadata.getReturnedDomainClass(method); + if (domainClass == null) { - return repositoryDomainClass == null || repositoryDomainClass.isAssignableFrom(methodDomainClass) ? methodDomainClass - : repositoryDomainClass; + Class repositoryDomainClass = metadata.getDomainType(); + Class methodDomainClass = metadata.getReturnedDomainClass(method); + + this.domainClass = repositoryDomainClass == null || repositoryDomainClass.isAssignableFrom(methodDomainClass) ? methodDomainClass + : repositoryDomainClass; + } + + return domainClass; } /** diff --git a/src/main/java/org/springframework/data/repository/util/NullableWrapper.java b/src/main/java/org/springframework/data/repository/util/NullableWrapper.java new file mode 100644 index 000000000..c4837ac6c --- /dev/null +++ b/src/main/java/org/springframework/data/repository/util/NullableWrapper.java @@ -0,0 +1,58 @@ +/* + * 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.util; + +import org.springframework.core.convert.converter.Converter; + +/** + * Simple value object to wrap a nullable delegate. Used to be able to write {@link Converter} implementations that + * convert {@literal null} into an object of some sort. + * + * @author Oliver Gierke + * @since 1.8 + * @see QueryExecutionConverters + */ +public class NullableWrapper { + + private final Object value; + + /** + * Creates a new {@link NullableWrapper} for the given value. + * + * @param value can be {@literal null}. + */ + public NullableWrapper(Object value) { + this.value = value; + } + + /** + * Returns the type of the contained value. WIll fall back to {@link Object} in case the value is {@literal null}. + * + * @return will never be {@literal null}. + */ + public Class getValueType() { + return value == null ? Object.class : value.getClass(); + } + + /** + * Returns the backing valie + * + * @return the value can be {@literal null}. + */ + public Object getValue() { + return value; + } +} diff --git a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java new file mode 100644 index 000000000..9e326e3ae --- /dev/null +++ b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java @@ -0,0 +1,136 @@ +/* + * 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.util; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.support.ConfigurableConversionService; +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; + +import com.google.common.base.Optional; + +/** + * Converters to potentially wrap the execution of a repository method into a variety of wrapper types potentially being + * available on the classpath. + * + * @author Oliver Gierke + * @since 1.8 + */ +public 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 final Set> WRAPPER_TYPES; + private static final Set> CONVERTERS; + + static { + + Set> wrapperTypes = new HashSet>(); + Set> converters = new HashSet>(); + + if (GUAVA_PRESENT) { + wrapperTypes.add(ObjectToGuavaOptionalConverter.INSTANCE.getWrapperType()); + converters.add(ObjectToGuavaOptionalConverter.INSTANCE); + } + + if (JDK_PRESENT) { + wrapperTypes.add(ObjectToJdk8OptionalConverter.INSTANCE.getWrapperType()); + converters.add(ObjectToJdk8OptionalConverter.INSTANCE); + } + + WRAPPER_TYPES = Collections.unmodifiableSet(wrapperTypes); + CONVERTERS = Collections.unmodifiableSet(converters); + } + + /** + * Returns whether the given type is a supported wrapper type. + * + * @param type must not be {@literal null}. + * @return + */ + public static boolean supports(Class type) { + + Assert.notNull(type, "Type must not be null!"); + return WRAPPER_TYPES.contains(type); + } + + /** + * Registers converters for wrapper types found on the classpath. + * + * @param conversionService must not be {@literal null}. + */ + public static void registerConvertersIn(ConfigurableConversionService conversionService) { + + Assert.notNull(conversionService, "ConversionService must not be null!"); + + for (Converter converter : CONVERTERS) { + conversionService.addConverter(converter); + } + } + + /** + * A Spring {@link Converter} to support Google Guava's {@link Optional}. + * + * @author Oliver Gierke + */ + private static enum ObjectToGuavaOptionalConverter implements Converter> { + + INSTANCE; + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) + */ + @Override + public Optional convert(NullableWrapper source) { + return Optional.fromNullable(source.getValue()); + } + + public Class getWrapperType() { + return Optional.class; + } + } + + /** + * A Spring {@link Converter} to support JDK 8's {@link java.util.Optional}. + * + * @author Oliver Gierke + */ + private static enum ObjectToJdk8OptionalConverter implements Converter> { + + INSTANCE; + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) + */ + @Override + public java.util.Optional convert(NullableWrapper source) { + return java.util.Optional.ofNullable(source.getValue()); + } + + public Class getWrapperType() { + return java.util.Optional.class; + } + } +} diff --git a/src/test/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadataUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadataUnitTests.java index 0bea9f298..dc8601361 100644 --- a/src/test/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadataUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadataUnitTests.java @@ -15,9 +15,11 @@ */ package org.springframework.data.repository.core.support; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import java.io.Serializable; +import java.lang.reflect.Method; import java.util.Collection; import org.junit.Test; @@ -28,6 +30,8 @@ import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.util.ClassUtils; +import com.google.common.base.Optional; + /** * Unit tests for {@link DefaultRepositoryMetadata}. * @@ -101,6 +105,18 @@ public class DefaultRepositoryMetadataUnitTests { assertEquals(Long.class, metadata.getIdType()); } + /** + * @see DATACMNS-483 + */ + @Test + public void discoversDomainTypeOnReturnTypeWrapper() throws Exception { + + RepositoryMetadata metadata = new DefaultRepositoryMetadata(OptionalRepository.class); + + Method method = OptionalRepository.class.getMethod("findByEmailAddress", String.class); + assertThat(metadata.getReturnedDomainClass(method), is(typeCompatibleWith(User.class))); + } + @SuppressWarnings("unused") private class User { @@ -167,4 +183,9 @@ public class DefaultRepositoryMetadataUnitTests { static interface ConcreteRepository extends IdTypeFixingRepository { } + + static interface OptionalRepository extends Repository { + + Optional findByEmailAddress(String emailAddress); + } } diff --git a/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java b/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java new file mode 100644 index 000000000..cf460bb8b --- /dev/null +++ b/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java @@ -0,0 +1,68 @@ +/* + * 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.util; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.util.ClassUtils; + +import com.google.common.base.Optional; + +/** + * Unit tests for {@link QueryExecutionConverters}. + * + * @author Oliver Gierke + */ +public class QueryExecutionConvertersUnitTests { + + DefaultConversionService conversionService; + + @Before + public void setUp() { + + this.conversionService = new DefaultConversionService(); + QueryExecutionConverters.registerConvertersIn(conversionService); + } + + /** + * @see DATACMNS-483 + */ + @Test + @SuppressWarnings("unchecked") + public void turnsNullIntoGuavaOptional() { + + Optional optional = conversionService.convert(new NullableWrapper(null), Optional.class); + assertThat(optional, is(Optional. absent())); + } + + /** + * @see DATACMNS-483 + */ + @Test + @SuppressWarnings("unchecked") + public void turnsNullIntoJdk8Optional() { + + Assume.assumeThat(ClassUtils.isPresent("java.util.Optional", getClass().getClassLoader()), is(true)); + + java.util.Optional optional = conversionService.convert(new NullableWrapper(null), java.util.Optional.class); + assertThat(optional, is(java.util.Optional. empty())); + } +} diff --git a/template.mf b/template.mf index 5a4f7021f..3482dc713 100644 --- a/template.mf +++ b/template.mf @@ -9,6 +9,7 @@ Import-Package: Import-Template: com.fasterxml.jackson.*;version="${jackson:[=.=.=,+1.0.0)}";resolution:=optional, com.mysema.query.*;version="${querydsl:[=.=.=,+1.0.0)}";resolution:=optional, + com.google.common.*;version="${guava:[=.=.=,+1.0.0)}";resolution:=optional, javax.enterprise.*;version="${cdi:[=.=.=,+1.0.0)}";resolution:=optional, javax.inject.*;version="[1.0.0,2.0.0)";resolution:=optional, javax.xml.bind.*;version="0";resolution:=optional,