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.

src/main/java/org/springframework/data/repository/support/DomainClassConverter.java

@@ -166,12 +166,12 @@ public class DomainClassConverter<T extends ConversionService & ConverterRegistr
 		@Override
 		public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
 
-			if (!repositories.hasRepositoryFor(targetType.getType())) {
+			if (sourceType.isAssignableTo(targetType)) {
 				return false;
 			}
 
-			if (sourceType.equals(targetType)) {
-				return true;
+			if (!repositories.hasRepositoryFor(targetType.getType())) {
+				return false;
 			}
 
 			Class<?> rawIdType = repositories.getRepositoryInformationFor(targetType.getType()).getIdType();
@@ -192,7 +192,7 @@ public class DomainClassConverter<T extends ConversionService & ConverterRegistr
 	 * @author Oliver Gierke
 	 * @since 1.10
 	 */
-	private class ToIdConverter implements ConditionalGenericConverter {
+	class ToIdConverter implements ConditionalGenericConverter {
 
 		/*
 		 * (non-Javadoc)
@@ -232,12 +232,12 @@ public class DomainClassConverter<T extends ConversionService & ConverterRegistr
 		@Override
 		public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
 
-			if (!repositories.hasRepositoryFor(sourceType.getType())) {
+			if (sourceType.isAssignableTo(targetType)) {
 				return false;
 			}
 
-			if (sourceType.equals(targetType)) {
-				return true;
+			if (!repositories.hasRepositoryFor(sourceType.getType())) {
+				return false;
 			}
 
 			Class<?> rawIdType = repositories.getRepositoryInformationFor(sourceType.getType()).getIdType();

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<User, Long> {
 
 	}