From 38dbfbf3fb3bb0440d94597a45a6155bd3eeb138 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 13 Apr 2015 17:40:40 +0200 Subject: [PATCH] DATACMNS-683 - Tweaked the matching algorithm for DomainClassConverter. DomainClassConverter as well as its internal ToEntity- and ToIdConverter implementations now actively refrain from matching if the source type is assignable to the target one to prevent unnecessary conversion attempts if the source value already actually is assignable to the target type. Related ticket: DATACMNS-583. --- .../support/DomainClassConverter.java | 14 +++---- .../DomainClassConverterUnitTests.java | 38 +++++++++++++++++-- 2 files changed, 42 insertions(+), 10 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 0770e569b..217ab033d 100644 --- a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java +++ b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java @@ -166,12 +166,12 @@ public class DomainClassConverter rawIdType = repositories.getRepositoryInformationFor(targetType.getType()).getIdType(); @@ -192,7 +192,7 @@ public class DomainClassConverter rawIdType = repositories.getRepositoryInformationFor(sourceType.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 480712d24..f62a18902 100644 --- a/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.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. @@ -20,6 +20,8 @@ import static org.junit.Assert.*; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; +import java.lang.reflect.Method; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -30,10 +32,14 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.context.ApplicationContext; import org.springframework.context.support.GenericApplicationContext; +import org.springframework.core.MethodParameter; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.core.support.DummyRepositoryFactoryBean; +import org.springframework.data.repository.support.DomainClassConverter.ToIdConverter; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.bind.annotation.ModelAttribute; /** * Unit test for {@link DomainClassConverter}. @@ -47,6 +53,7 @@ public class DomainClassConverterUnitTests { static final User USER = new User(); static final TypeDescriptor STRING_TYPE = TypeDescriptor.valueOf(String.class); static final TypeDescriptor USER_TYPE = TypeDescriptor.valueOf(User.class); + static final TypeDescriptor SUB_USER_TYPE = TypeDescriptor.valueOf(SubUser.class); static final TypeDescriptor LONG_TYPE = TypeDescriptor.valueOf(Long.class); @SuppressWarnings("rawtypes") DomainClassConverter converter; @@ -144,11 +151,11 @@ public class DomainClassConverterUnitTests { * @see DATACMNS-583 */ @Test - public void shouldReturnSourceObjectIfSourceAndTargetTypesAreTheSame() { + public void converterDoesntMatchIfTargetTypeIsAssignableFromSource() { converter.setApplicationContext(initContextWithRepo()); - assertThat(converter.matches(USER_TYPE, USER_TYPE), is(true)); + assertThat(converter.matches(SUB_USER_TYPE, USER_TYPE), is(false)); assertThat((User) converter.convert(USER, USER_TYPE, USER_TYPE), is(USER)); } @@ -186,6 +193,24 @@ public class DomainClassConverterUnitTests { assertThat(converter.matches(USER_TYPE, STRING_TYPE), is(true)); } + /** + * @see DATACMNS-683 + */ + @Test + public void toIdConverterDoesNotMatchIfTargetTypeIsAssignableFromSource() throws Exception { + + converter.setApplicationContext(initContextWithRepo()); + + @SuppressWarnings("rawtypes") + DomainClassConverter.ToIdConverter toIdConverter = (ToIdConverter) ReflectionTestUtils.getField(converter, + "toIdConverter"); + + Method method = Wrapper.class.getMethod("foo", User.class); + TypeDescriptor target = TypeDescriptor.nested(new MethodParameter(method, 0), 0); + + assertThat(toIdConverter.matches(SUB_USER_TYPE, target), is(false)); + } + private ApplicationContext initContextWithRepo() { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(DummyRepositoryFactoryBean.class); @@ -199,10 +224,17 @@ public class DomainClassConverterUnitTests { return ctx; } + static interface Wrapper { + + void foo(@ModelAttribute User user); + } + private static class User { } + private static class SubUser extends User {} + private static interface UserRepository extends CrudRepository { }