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 7abf45ed6..157da489b 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,7 @@ 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.MethodParameter; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.convert.support.GenericConversionService; @@ -341,13 +342,21 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware { public Object invoke(MethodInvocation invocation) throws Throwable { Object result = doInvoke(invocation); - Class expectedReturnType = invocation.getMethod().getReturnType(); + + Method method = invocation.getMethod(); + + // Looking up the TypeDescriptor for the return type - yes, this way o.O + 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 (conversionService.canConvert(NullableWrapper.class, expectedReturnType) + if (QueryExecutionConverters.supports(expectedReturnType) + && conversionService.canConvert(WRAPPER_TYPE, methodReturnTypeDescriptor) && !conversionService.canBypassConvert(WRAPPER_TYPE, TypeDescriptor.valueOf(expectedReturnType))) { return conversionService.convert(new NullableWrapper(result), expectedReturnType); } 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 4c628e566..4b21eee09 100644 --- a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java +++ b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java @@ -20,7 +20,10 @@ import java.util.HashSet; import java.util.Set; import java.util.concurrent.Future; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.converter.GenericConverter; import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.scheduling.annotation.AsyncResult; import org.springframework.util.Assert; @@ -42,29 +45,19 @@ public class QueryExecutionConverters { 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; + private static Set> WRAPPER_TYPES = new HashSet>(); static { - Set> wrapperTypes = new HashSet>(); - Set> converters = new HashSet>(); - - wrapperTypes.add(Future.class); - converters.add(ObjectToFutureConverter.INSTANCE); + WRAPPER_TYPES.add(Future.class); if (GUAVA_PRESENT) { - wrapperTypes.add(ObjectToGuavaOptionalConverter.INSTANCE.getWrapperType()); - converters.add(ObjectToGuavaOptionalConverter.INSTANCE); + WRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType()); } if (JDK_PRESENT) { - wrapperTypes.add(ObjectToJdk8OptionalConverter.INSTANCE.getWrapperType()); - converters.add(ObjectToJdk8OptionalConverter.INSTANCE); + WRAPPER_TYPES.add(NullableWrapperToJdk8OptionalConverter.getWrapperType()); } - - WRAPPER_TYPES = Collections.unmodifiableSet(wrapperTypes); - CONVERTERS = Collections.unmodifiableSet(converters); } /** @@ -88,9 +81,81 @@ public class QueryExecutionConverters { Assert.notNull(conversionService, "ConversionService must not be null!"); - for (Converter converter : CONVERTERS) { - conversionService.addConverter(converter); + if (GUAVA_PRESENT) { + conversionService.addConverter(new NullableWrapperToGuavaOptionalConverter(conversionService)); } + + if (JDK_PRESENT) { + conversionService.addConverter(new NullableWrapperToJdk8OptionalConverter(conversionService)); + } + + conversionService.addConverter(new NullableWrapperToFutureConverter(conversionService)); + } + + /** + * Base class for converters that create instances of wrapper types such as Google Guava's and JDK 8's + * {@code Optional} types. + * + * @author Oliver Gierke + */ + private static abstract class AbstractWrapperTypeConverter implements GenericConverter { + + @SuppressWarnings("unused")// + private final ConversionService conversionService; + private final Class wrapperType; + + /** + * Creates a new {@link AbstractWrapperTypeConverter} using the given {@link ConversionService} and wrapper type. + * + * @param conversionService must not be {@literal null}. + * @param wrapperType must not be {@literal null}. + */ + protected AbstractWrapperTypeConverter(ConversionService conversionService, Class wrapperType) { + + Assert.notNull(conversionService, "ConversionService must not be null!"); + Assert.notNull(wrapperType, "Wrapper type must not be null!"); + + this.conversionService = conversionService; + this.wrapperType = wrapperType; + } + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.GenericConverter#getConvertibleTypes() + */ + @Override + public Set getConvertibleTypes() { + return Collections.singleton(new ConvertiblePair(NullableWrapper.class, wrapperType)); + } + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.GenericConverter#convert(java.lang.Object, org.springframework.core.convert.TypeDescriptor, org.springframework.core.convert.TypeDescriptor) + */ + @Override + public final Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + + NullableWrapper wrapper = (NullableWrapper) source; + Object value = wrapper.getValue(); + + // TODO: Add Recursive conversion once we move to Spring 4 + return value == null ? getNullValue() : wrap(value); + } + + /** + * Return the object that shall be used as a replacement for {@literal null}. + * + * @return must not be {@literal null}. + */ + protected abstract Object getNullValue(); + + /** + * Wrap the given, non-{@literal null} value into the wrapper type. + * + * @param source will never be {@literal null}. + * @return must not be {@literal null}. + */ + protected abstract Object wrap(Object source); } /** @@ -98,20 +163,36 @@ public class QueryExecutionConverters { * * @author Oliver Gierke */ - private static enum ObjectToGuavaOptionalConverter implements Converter> { + private static class NullableWrapperToGuavaOptionalConverter extends AbstractWrapperTypeConverter { - INSTANCE; + /** + * Creates a new {@link NullableWrapperToGuavaOptionalConverter} using the given {@link ConversionService}. + * + * @param conversionService must not be {@literal null}. + */ + public NullableWrapperToGuavaOptionalConverter(ConversionService conversionService) { + super(conversionService, Optional.class); + } /* * (non-Javadoc) - * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) + * @see org.springframework.data.repository.util.QueryExecutionConverters.AbstractWrapperTypeConverter#getNullValue() */ @Override - public Optional convert(NullableWrapper source) { - return Optional.fromNullable(source.getValue()); + protected Object getNullValue() { + return Optional.absent(); } - public Class getWrapperType() { + /* + * (non-Javadoc) + * @see org.springframework.data.repository.util.QueryExecutionConverters.AbstractWrapperTypeConverter#wrap(java.lang.Object) + */ + @Override + protected Object wrap(Object source) { + return Optional.of(source); + } + + public static Class getWrapperType() { return Optional.class; } } @@ -121,20 +202,36 @@ public class QueryExecutionConverters { * * @author Oliver Gierke */ - private static enum ObjectToJdk8OptionalConverter implements Converter> { + private static class NullableWrapperToJdk8OptionalConverter extends AbstractWrapperTypeConverter { - INSTANCE; + /** + * Creates a new {@link NullableWrapperToJdk8OptionalConverter} using the given {@link ConversionService}. + * + * @param conversionService must not be {@literal null}. + */ + public NullableWrapperToJdk8OptionalConverter(ConversionService conversionService) { + super(conversionService, java.util.Optional.class); + } /* * (non-Javadoc) - * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) + * @see org.springframework.data.repository.util.QueryExecutionConverters.AbstractWrapperTypeConverter#getNullValue() */ @Override - public java.util.Optional convert(NullableWrapper source) { - return java.util.Optional.ofNullable(source.getValue()); + protected Object getNullValue() { + return java.util.Optional.empty(); } - public Class getWrapperType() { + /* + * (non-Javadoc) + * @see org.springframework.data.repository.util.QueryExecutionConverters.AbstractWrapperTypeConverter#wrap(java.lang.Object) + */ + @Override + protected Object wrap(Object source) { + return java.util.Optional.of(source); + } + + public static Class getWrapperType() { return java.util.Optional.class; } } @@ -144,17 +241,35 @@ public class QueryExecutionConverters { * * @author Oliver Gierke */ - private static enum ObjectToFutureConverter implements Converter> { + private static class NullableWrapperToFutureConverter extends AbstractWrapperTypeConverter { - INSTANCE; + private final AsyncResult NULL_OBJECT = new AsyncResult(null); + + /** + * Creates a new {@link NullableWrapperToFutureConverter} using the given {@link ConversionService}. + * + * @param conversionService must not be {@literal null}. + */ + public NullableWrapperToFutureConverter(ConversionService conversionService) { + super(conversionService, Future.class); + } /* * (non-Javadoc) - * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) + * @see org.springframework.data.repository.util.QueryExecutionConverters.AbstractWrapperTypeConverter#getNullValue() */ @Override - public Future convert(NullableWrapper source) { - return new AsyncResult(source.getValue()); + protected Object getNullValue() { + return NULL_OBJECT; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.util.QueryExecutionConverters.AbstractWrapperTypeConverter#wrap(java.lang.Object) + */ + @Override + protected Object wrap(Object source) { + return new AsyncResult(source); } } } diff --git a/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.java b/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.java index d107c62ce..ee67e46cd 100644 --- a/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.java +++ b/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.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. @@ -29,15 +29,27 @@ import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.QueryLookupStrategy.Key; import org.springframework.data.repository.query.RepositoryQuery; +/** + * Dummy implementation for {@link RepositoryFactorySupport} that is equipped with mocks to simulate behavior for test + * cases. + * + * @author Oliver Gierke + */ public class DummyRepositoryFactory extends RepositoryFactorySupport { public final MyRepositoryQuery queryOne = mock(MyRepositoryQuery.class); public final RepositoryQuery queryTwo = mock(RepositoryQuery.class); + public final QueryLookupStrategy strategy = mock(QueryLookupStrategy.class); private final Object repository; public DummyRepositoryFactory(Object repository) { + this.repository = repository; + + when( + strategy.resolveQuery(Mockito.any(Method.class), Mockito.any(RepositoryMetadata.class), + Mockito.any(NamedQueries.class))).thenReturn(queryOne); } /* @@ -74,13 +86,6 @@ public class DummyRepositoryFactory extends RepositoryFactorySupport { */ @Override protected QueryLookupStrategy getQueryLookupStrategy(Key key) { - - QueryLookupStrategy strategy = mock(QueryLookupStrategy.class); - - when( - strategy.resolveQuery(Mockito.any(Method.class), Mockito.any(RepositoryMetadata.class), - Mockito.any(NamedQueries.class))).thenReturn(queryOne, queryTwo); - return strategy; } } diff --git a/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java index c2c4e48ff..b2d20c168 100644 --- a/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.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,12 +15,15 @@ */ package org.springframework.data.repository.core.support; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.*; import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.concurrent.Future; import org.junit.Assume; @@ -41,6 +44,8 @@ import org.springframework.data.domain.Sort; import org.springframework.data.repository.PagingAndSortingRepository; import org.springframework.data.repository.Repository; import org.springframework.data.repository.RepositoryDefinition; +import org.springframework.data.repository.core.NamedQueries; +import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.scheduling.annotation.Async; import org.springframework.scheduling.annotation.AsyncAnnotationBeanPostProcessor; @@ -71,6 +76,12 @@ public class RepositoryFactorySupportUnitTests { @Test public void invokesCustomQueryCreationListenerForSpecialRepositoryQueryOnly() throws Exception { + Mockito.reset(factory.strategy); + + when( + factory.strategy.resolveQuery(Mockito.any(Method.class), Mockito.any(RepositoryMetadata.class), + Mockito.any(NamedQueries.class))).thenReturn(factory.queryOne, factory.queryTwo); + factory.addQueryCreationListener(listener); factory.addQueryCreationListener(otherListener); @@ -148,11 +159,11 @@ public class RepositoryFactorySupportUnitTests { } }); - AsyncRepository repository = factory.getRepository(AsyncRepository.class); + ConvertingRepository repository = factory.getRepository(ConvertingRepository.class); AsyncAnnotationBeanPostProcessor processor = new AsyncAnnotationBeanPostProcessor(); processor.setBeanFactory(new DefaultListableBeanFactory()); - repository = (AsyncRepository) processor.postProcessAfterInitialization(repository, null); + repository = (ConvertingRepository) processor.postProcessAfterInitialization(repository, null); Future future = repository.findByFirstname("Foo"); @@ -167,6 +178,40 @@ public class RepositoryFactorySupportUnitTests { verify(factory.queryOne, times(1)).execute(Mockito.any(Object[].class)); } + /** + * @see DATACMNS-509 + */ + @Test + public void convertsWithSameElementType() { + + List names = Collections.singletonList("Dave"); + + when(factory.queryOne.execute(Mockito.any(Object[].class))).thenReturn(names); + + ConvertingRepository repository = factory.getRepository(ConvertingRepository.class); + Set result = repository.convertListToStringSet(); + + assertThat(result, hasSize(1)); + assertThat(result.iterator().next(), is("Dave")); + } + + /** + * @see DATACMNS-509 + */ + @Test + public void convertsCollectionToOtherCollectionWithElementSuperType() { + + List names = Collections.singletonList("Dave"); + + when(factory.queryOne.execute(Mockito.any(Object[].class))).thenReturn(names); + + ConvertingRepository repository = factory.getRepository(ConvertingRepository.class); + Set result = repository.convertListToObjectSet(); + + assertThat(result, hasSize(1)); + assertThat(result.iterator().next(), is((Object) "Dave")); + } + interface ObjectRepository extends Repository, ObjectRepositoryCustom { Object findByClass(Class clazz); @@ -217,7 +262,11 @@ public class RepositoryFactorySupportUnitTests { } - interface AsyncRepository extends Repository { + interface ConvertingRepository extends Repository { + + Set convertListToStringSet(); + + Set convertListToObjectSet(); @Async Future findByFirstname(String firstname);