DATACMNS-98 - Fixed potential ClassCastException for domain type lookup for query methods.

Refactored the definition of what domain class is returned from a query method into a common AbstractRepositoryMetadata and moved ClassUtils.getReturnedDomainClass into RepositoryMetadata. Moved test cases accordingly. Added getReturnType(Method method) to TypeInformation.

spring-data-commons-core/src/main/java/org/springframework/data/repository/core/RepositoryMetadata.java

@@ -15,6 +15,11 @@
  */
 package org.springframework.data.repository.core;
 
+import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.Collection;
+
 /**
  * Metadata for repository interfaces.
  * 
@@ -44,4 +49,13 @@ public interface RepositoryMetadata {
 	 * @return
 	 */
 	Class<?> getRepositoryInterface();
+	
+	/**
+	 * Returns the domain class returned by the given {@link Method}. Will extract the type from {@link Collection}s and
+	 * {@link org.springframework.data.domain.Page} as well.
+	 * 
+	 * @param method
+	 * @return
+	 */
+	Class<?> getReturnedDomainClass(Method method);
 }

spring-data-commons-core/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2011 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 java.lang.reflect.Method;
+
+import org.springframework.data.repository.core.RepositoryMetadata;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.TypeInformation;
+import org.springframework.util.Assert;
+
+/**
+ * Base class for {@link RepositoryMetadata} implementations.
+ * 
+ * @author Oliver Gierke
+ */
+public abstract class AbstractRepositoryMetadata implements RepositoryMetadata {
+
+	private final TypeInformation<?> typeInformation;
+	
+	/**
+	 * Creates a new {@link AbstractRepositoryMetadata}.
+	 * 
+	 * @param repositoryInterface must not be {@literal null} and must be an interface.
+	 */
+	public AbstractRepositoryMetadata(Class<?> repositoryInterface) {
+		
+		Assert.notNull(repositoryInterface, "Given type must not be null!");
+		Assert.isTrue(repositoryInterface.isInterface(), "Given type must be an interface!");
+		this.typeInformation = ClassTypeInformation.from(repositoryInterface);
+	}
+	
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.repository.core.RepositoryMetadata#getReturnedDomainClass(java.lang.reflect.Method)
+	 */
+	public Class<?> getReturnedDomainClass(Method method) {
+
+		TypeInformation<?> returnTypeInfo = typeInformation.getReturnType(method);
+		Class<?> rawType = returnTypeInfo.getType();
+
+		return Iterable.class.isAssignableFrom(rawType) ? returnTypeInfo.getComponentType().getType() : rawType;
+	}
+}

spring-data-commons-core/src/main/java/org/springframework/data/repository/core/support/AnnotationRepositoryMetadata.java

@@ -25,15 +25,21 @@ import org.springframework.util.Assert;
  * 
  * @author Oliver Gierke
  */
-public class AnnotationRepositoryMetadata implements RepositoryMetadata {
+public class AnnotationRepositoryMetadata extends AbstractRepositoryMetadata {
 
 	private static final String NO_ANNOTATION_FOUND = String.format("Interface must be annotated with @%s!",
 			RepositoryDefinition.class.getName());
 
 	private final Class<?> repositoryInterface;
 
+	/**
+	 * Creates a new {@link AnnotationRepositoryMetadata} instance looking up repository types from a
+	 * {@link RepositoryDefinition} annotation.
+	 * 
+	 * @param repositoryInterface must not be {@literal null}.
+	 */
 	public AnnotationRepositoryMetadata(Class<?> repositoryInterface) {
-		Assert.notNull(repositoryInterface, "Repository interface must not be null!");
+		super(repositoryInterface);
 		Assert.isTrue(repositoryInterface.isAnnotationPresent(RepositoryDefinition.class), NO_ANNOTATION_FOUND);
 		this.repositoryInterface = repositoryInterface;
 	}

spring-data-commons-core/src/main/java/org/springframework/data/repository/core/support/DefaultRepositoryInformation.java

@@ -37,7 +37,7 @@ import org.springframework.util.Assert;
  * 
  * @author Oliver Gierke
  */
-class DefaultRepositoryInformation implements RepositoryInformation {
+class DefaultRepositoryInformation extends AbstractRepositoryMetadata implements RepositoryInformation {
 
 	@SuppressWarnings("rawtypes")
 	private static final TypeVariable<Class<Repository>>[] PARAMETERS = Repository.class.getTypeParameters();
@@ -60,8 +60,11 @@ class DefaultRepositoryInformation implements RepositoryInformation {
 	public DefaultRepositoryInformation(RepositoryMetadata metadata, Class<?> repositoryBaseClass,
 			Class<?> customImplementationClass) {
 
+		super(metadata.getRepositoryInterface());
+		
 		Assert.notNull(metadata);
 		Assert.notNull(repositoryBaseClass);
+		
 		this.metadata = metadata;
 		this.repositoryBaseClass = repositoryBaseClass;
 		this.customImplementationClass = customImplementationClass;

spring-data-commons-core/src/main/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadata.java

@@ -27,7 +27,7 @@ import org.springframework.util.Assert;
  * 
  * @author Oliver Gierke
  */
-public class DefaultRepositoryMetadata implements RepositoryMetadata {
+public class DefaultRepositoryMetadata extends AbstractRepositoryMetadata {
 
 	private final Class<?> repositoryInterface;
 
@@ -38,7 +38,8 @@ public class DefaultRepositoryMetadata implements RepositoryMetadata {
 	 */
 	public DefaultRepositoryMetadata(Class<?> repositoryInterface) {
 
-		Assert.notNull(repositoryInterface);
+		super(repositoryInterface);
+		Assert.isTrue(repositoryInterface.isInterface());
 		Assert.isTrue(Repository.class.isAssignableFrom(repositoryInterface));
 		this.repositoryInterface = repositoryInterface;
 	}

spring-data-commons-core/src/main/java/org/springframework/data/repository/query/QueryMethod.java

@@ -25,7 +25,6 @@ import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.repository.core.EntityMetadata;
 import org.springframework.data.repository.core.RepositoryMetadata;
-import org.springframework.data.repository.util.ClassUtils;
 import org.springframework.util.Assert;
 
 /**
@@ -124,7 +123,7 @@ public class QueryMethod {
 	protected Class<?> getDomainClass() {
 
 		Class<?> repositoryDomainClass = metadata.getDomainClass();
-		Class<?> methodDomainClass = ClassUtils.getReturnedDomainClass(method);
+		Class<?> methodDomainClass = metadata.getReturnedDomainClass(method);
 
 		return repositoryDomainClass == null || repositoryDomainClass.isAssignableFrom(methodDomainClass) ? methodDomainClass
 				: repositoryDomainClass;

spring-data-commons-core/src/main/java/org/springframework/data/repository/util/ClassUtils.java

@@ -17,8 +17,6 @@ package org.springframework.data.repository.util;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
 import java.util.Arrays;
 import java.util.Collection;
 
@@ -40,29 +38,6 @@ public abstract class ClassUtils {
 
 	}
 
-	/**
-	 * Returns the domain class returned by the given {@link Method}. Will extract the type from {@link Collection}s and
-	 * {@link org.springframework.data.domain.Page} as well.
-	 * 
-	 * @param method
-	 * @return
-	 */
-	public static Class<?> getReturnedDomainClass(Method method) {
-
-		Class<?> type = method.getReturnType();
-
-		if (Iterable.class.isAssignableFrom(type)) {
-
-			ParameterizedType returnType = (ParameterizedType) method.getGenericReturnType();
-			Type componentType = returnType.getActualTypeArguments()[0];
-
-			return componentType instanceof ParameterizedType ? (Class<?>) ((ParameterizedType) componentType).getRawType()
-					: (Class<?>) componentType;
-		}
-
-		return type;
-	}
-
 	/**
 	 * Returns whether the given class contains a property with the given name.
 	 * 

spring-data-commons-core/src/main/java/org/springframework/data/util/TypeDiscoverer.java

@@ -300,6 +300,16 @@ class TypeDiscoverer<S> implements TypeInformation<S> {
 		return null;
 	}
 
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.util.TypeInformation#getReturnType(java.lang.reflect.Method)
+	 */
+	public TypeInformation<?> getReturnType(Method method) {
+		
+		Assert.notNull(method);
+		return createInfo(method.getGenericReturnType());
+	}
+	
 	/*
 		 * (non-Javadoc)
 		 *

spring-data-commons-core/src/main/java/org/springframework/data/util/TypeInformation.java

@@ -16,7 +16,7 @@
 package org.springframework.data.util;
 
 import java.lang.reflect.Constructor;
-
+import java.lang.reflect.Method;
 import java.util.List;
 
 /**
@@ -83,4 +83,13 @@ public interface TypeInformation<S> {
 	 * @return
 	 */
 	TypeInformation<?> getActualType();
+	
+	/**
+	 * Returns a {@link TypeInformation} for the return type of the given {@link Method}. Will potentially resolve
+	 * generics information against the current types type parameter bindings.
+	 * 
+	 * @param method must not be {@literal null}.
+	 * @return
+	 */
+	TypeInformation<?> getReturnType(Method method);
 }

spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadataUnitTest.java

@@ -0,0 +1,127 @@
+/*
+ * Copyright 2011 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.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.io.Serializable;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Test;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.querydsl.User;
+import org.springframework.data.repository.Repository;
+import org.springframework.data.repository.core.RepositoryMetadata;
+
+/**
+ * Unit tests for {@link AbstractRepositoryMetadata}.
+ * 
+ * @author Oliver Gierke
+ */
+public class AbstractRepositoryMetadataUnitTest {
+
+	@Test
+	public void discoversSimpleReturnTypeCorrectly() throws Exception {
+
+		RepositoryMetadata metadata = new DummyRepositoryMetadata(UserRepository.class);
+		Method method = UserRepository.class.getMethod("findSingle");
+		assertThat(metadata.getReturnedDomainClass(method), is(typeCompatibleWith(User.class)));
+	}
+
+	/**
+	 * @see DATACMNS-98
+	 */
+	@Test
+	public void resolvesTypeParameterReturnType() throws Exception {
+		RepositoryMetadata metadata = new DummyRepositoryMetadata(ConcreteRepository.class);
+		Method method = ConcreteRepository.class.getMethod("intermediateMethod");
+		assertThat(metadata.getReturnedDomainClass(method), is(typeCompatibleWith(User.class)));
+	}
+
+	@Test
+	public void determinesReturnTypeFromPageable() throws Exception {
+
+		RepositoryMetadata metadata = new DummyRepositoryMetadata(ExtendingRepository.class);
+		Method method = ExtendingRepository.class.getMethod("findByFirstname", Pageable.class, String.class);
+		assertThat(metadata.getReturnedDomainClass(method), is(typeCompatibleWith(User.class)));
+	}
+
+	@Test
+	public void determinesReturnTypeFromGenericType() throws Exception {
+		RepositoryMetadata metadata = new DummyRepositoryMetadata(ExtendingRepository.class);
+		Method method = ExtendingRepository.class.getMethod("someMethod");
+		assertThat(metadata.getReturnedDomainClass(method), is(typeCompatibleWith(GenericType.class)));
+	}
+
+	@Test
+	public void handlesGenericTypeInReturnedCollectionCorrectly() throws SecurityException, NoSuchMethodException {
+		
+		RepositoryMetadata metadata = new DummyRepositoryMetadata(ExtendingRepository.class);
+		Method method = ExtendingRepository.class.getMethod("anotherMethod");
+		assertThat(metadata.getReturnedDomainClass(method), is(typeCompatibleWith(Map.class)));
+	}
+
+	interface UserRepository extends Repository<User, Long> {
+
+		User findSingle();
+	}
+
+	interface IntermediateRepository<T> extends Repository<T, Long> {
+
+		List<T> intermediateMethod();
+	}
+
+	interface ConcreteRepository extends IntermediateRepository<User> {
+
+	}
+
+	interface ExtendingRepository extends Serializable, UserRepository {
+
+		Page<User> findByFirstname(Pageable pageable, String firstname);
+
+		GenericType<User> someMethod();
+
+		List<Map<String, Object>> anotherMethod();
+	}
+
+	class GenericType<T> {
+
+	}
+
+	class DummyRepositoryMetadata extends AbstractRepositoryMetadata {
+
+		public DummyRepositoryMetadata(Class<?> repositoryInterface) {
+			super(repositoryInterface);
+		}
+
+		public Class<?> getIdClass() {
+			return null;
+		}
+
+		public Class<?> getDomainClass() {
+			return null;
+		}
+
+		public Class<?> getRepositoryInterface() {
+			return null;
+		}
+	}
+
+}

spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadataUnitTests.java

@@ -18,13 +18,13 @@ package org.springframework.data.repository.core.support;
 import static org.junit.Assert.*;
 
 import java.io.Serializable;
+import java.util.Collection;
 
 import org.junit.Test;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.repository.CrudRepository;
 import org.springframework.data.repository.core.RepositoryMetadata;
-import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
 import org.springframework.data.repository.util.ClassUtils;
 
 
@@ -35,6 +35,22 @@ import org.springframework.data.repository.util.ClassUtils;
  */
 public class DefaultRepositoryMetadataUnitTests {
 
+	@Test(expected = IllegalArgumentException.class)
+	public void preventsNullRepositoryInterface() {
+		
+		new DefaultRepositoryMetadata(null);
+	}
+	
+	@Test(expected = IllegalArgumentException.class)
+	public void rejectsNonInterface() {
+		new DefaultRepositoryMetadata(Object.class);
+	}
+	
+	@Test(expected = IllegalArgumentException.class)
+	public void rejectsNonRepositoryInterface() {
+		new DefaultRepositoryMetadata(Collection.class);
+	}
+	
 	@Test
 	public void looksUpDomainClassCorrectly() throws Exception {
 

spring-data-commons-core/src/test/java/org/springframework/data/repository/util/ClassUtilsUnitTests.java

@@ -42,18 +42,6 @@ public class ClassUtilsUnitTests {
 				Pageable.class, String.class), User.class);
 	}
 
-
-	@Test
-	public void determinesReturnType() throws Exception {
-
-		assertEquals(User.class,
-				getReturnedDomainClass(SomeDao.class.getMethod(
-						"findByFirstname", Pageable.class, String.class)));
-		assertEquals(GenericType.class,
-				getReturnedDomainClass(SomeDao.class.getMethod("someMethod")));
-	}
-
-
 	@Test
 	public void determinesValidFieldsCorrectly() {
 
@@ -62,11 +50,6 @@ public class ClassUtilsUnitTests {
 		assertFalse(hasProperty(User.class, "address"));
 	}
 
-	@Test
-	public void handlesGenericTypeInReturnedCollectionCorrectly() throws SecurityException, NoSuchMethodException {
-		assertEquals(Map.class, getReturnedDomainClass(SomeDao.class.getMethod("anotherMethod")));
-	}
-
 	@SuppressWarnings("unused")
 	private class User {
 
@@ -83,29 +66,16 @@ public class ClassUtilsUnitTests {
 
 	}
 
-	/**
-	 * Sample interface to serve two purposes:
-	 * <ol>
-	 * <li>Check that {@link ClassUtils#getDomainClass(Class)} skips non
-	 * {@link GenericDao} interfaces</li>
-	 * <li>Check that {@link ClassUtils#getDomainClass(Class)} traverses
-	 * interface hierarchy</li>
-	 * </ol>
-	 *
-	 * @author Oliver Gierke
-	 */
-	private interface SomeDao extends Serializable, UserRepository {
+	interface SomeDao extends Serializable, UserRepository {
 
 		Page<User> findByFirstname(Pageable pageable, String firstname);
 
-
 		GenericType<User> someMethod();
 
-
 		List<Map<String, Object>> anotherMethod();
 	}
 
-	private class GenericType<T> {
+	class GenericType<T> {
 
 	}
 }