DATACMNS-406 - Tighten contract for RepositoryMetadata implementations.

We now resolve the domain and id-types eagerly within the constructor of AbstractRepositoryMetadata implementations to prevent a RepositoryMetadata instance to be created in an invalid state. Pulled-up repositoryInterface property to AbstractRepositoryMetadata.

Original pull request: #58.

Author: Thomas Darimont

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 the original author or authors.
+ * Copyright 2011-2013 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.
@@ -26,10 +26,12 @@
  * Base class for {@link RepositoryMetadata} implementations.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 public abstract class AbstractRepositoryMetadata implements RepositoryMetadata {
 
 	private final TypeInformation<?> typeInformation;
+	private final Class<?> repositoryInterface;
 
 	/**
 	 * Creates a new {@link AbstractRepositoryMetadata}.
@@ -40,6 +42,8 @@ public AbstractRepositoryMetadata(Class<?> repositoryInterface) {
 
 		Assert.notNull(repositoryInterface, "Given type must not be null!");
 		Assert.isTrue(repositoryInterface.isInterface(), "Given type must be an interface!");
+
+		this.repositoryInterface = repositoryInterface;
 		this.typeInformation = ClassTypeInformation.from(repositoryInterface);
 	}
 
@@ -54,4 +58,11 @@ public Class<?> getReturnedDomainClass(Method method) {
 
 		return Iterable.class.isAssignableFrom(rawType) ? returnTypeInfo.getComponentType().getType() : rawType;
 	}
+
+	/* (non-Javadoc)
+	 * @see org.springframework.data.repository.core.RepositoryMetadata#getRepositoryInterface()
+	 */
+	public Class<?> getRepositoryInterface() {
+		return this.repositoryInterface;
+	}
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2012 the original author or authors.
+ * Copyright 2011-2013 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.
@@ -26,13 +26,15 @@
  * {@link RepositoryDefinition} annotation.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 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;
+	private final Class<? extends Serializable> idType;
+	private final Class<?> domainType;
 
 	/**
 	 * Creates a new {@link AnnotationRepositoryMetadata} instance looking up repository types from a
@@ -41,34 +43,59 @@ public class AnnotationRepositoryMetadata extends AbstractRepositoryMetadata {
 	 * @param repositoryInterface must not be {@literal null}.
 	 */
 	public AnnotationRepositoryMetadata(Class<?> repositoryInterface) {
+
 		super(repositoryInterface);
 		Assert.isTrue(repositoryInterface.isAnnotationPresent(RepositoryDefinition.class), NO_ANNOTATION_FOUND);
-		this.repositoryInterface = repositoryInterface;
+
+		this.idType = resolveIdType(repositoryInterface);
+		this.domainType = resolveDomainType(repositoryInterface);
 	}
 
-	/*
+	/* 
 	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.support.RepositoryMetadata#getIdClass()
+	 * @see org.springframework.data.repository.core.RepositoryMetadata#getIdType()
 	 */
+	@Override
 	public Class<? extends Serializable> getIdType() {
-		RepositoryDefinition annotation = repositoryInterface.getAnnotation(RepositoryDefinition.class);
-		return annotation == null ? null : annotation.idClass();
+		return this.idType;
 	}
 
-	/*
+	/* 
 	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.support.RepositoryMetadata#getDomainClass()
+	 * @see org.springframework.data.repository.core.RepositoryMetadata#getDomainType()
 	 */
+	@Override
 	public Class<?> getDomainType() {
+		return this.domainType;
+	}
+
+	/**
+	 * @param repositoryInterface must not be {@literal null}.
+	 * @return the resolved domain type, never {@literal null}.
+	 */
+	private Class<? extends Serializable> resolveIdType(Class<?> repositoryInterface) {
+
+		Assert.notNull(repositoryInterface, "Repository interface must not be null!");
+
 		RepositoryDefinition annotation = repositoryInterface.getAnnotation(RepositoryDefinition.class);
-		return annotation == null ? null : annotation.domainClass();
+		Assert.isTrue(annotation != null && annotation.idClass() != null,
+				String.format("Could not resolve id type of %s!", repositoryInterface));
+
+		return annotation.idClass();
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.support.RepositoryMetadata#getRepositoryInterface()
+	/**
+	 * @param repositoryInterface must not be {@literal null}.
+	 * @return the resolved domain type, never {@literal null}.
 	 */
-	public Class<?> getRepositoryInterface() {
-		return repositoryInterface;
+	private Class<?> resolveDomainType(Class<?> repositoryInterface) {
+
+		Assert.notNull(repositoryInterface, "Repository interface must not be null!");
+
+		RepositoryDefinition annotation = repositoryInterface.getAnnotation(RepositoryDefinition.class);
+		Assert.isTrue(annotation != null && annotation.domainClass() != null,
+				String.format("Could not resolve domain type of %s!", repositoryInterface));
+
+		return annotation.domainClass();
 	}
 }

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

@@ -40,6 +40,7 @@
  * Default implementation of {@link RepositoryInformation}.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 class DefaultRepositoryInformation extends AbstractRepositoryMetadata implements RepositoryInformation {
 
@@ -76,15 +77,6 @@ public DefaultRepositoryInformation(RepositoryMetadata metadata, Class<?> reposi
 		this.crudMethods = new DefaultCrudMethods(this);
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.support.RepositoryMetadata#getRepositoryInterface()
-	 */
-	@Override
-	public Class<?> getRepositoryInterface() {
-		return metadata.getRepositoryInterface();
-	}
-
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.repository.support.RepositoryMetadata#getDomainClass()

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2012 the original author or authors.
+ * Copyright 2011-2013 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.
@@ -28,10 +28,15 @@
  * about domain and id class.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 public class DefaultRepositoryMetadata extends AbstractRepositoryMetadata {
 
-	private final Class<?> repositoryInterface;
+	private static final String MUST_BE_A_REPOSITORY = String.format("Given type must be assignable to %s!",
+			Repository.class);
+
+	private final Class<? extends Serializable> idType;
+	private final Class<?> domainType;
 
 	/**
 	 * Creates a new {@link DefaultRepositoryMetadata} for the given repository interface.
@@ -41,38 +46,57 @@ public class DefaultRepositoryMetadata extends AbstractRepositoryMetadata {
 	public DefaultRepositoryMetadata(Class<?> repositoryInterface) {
 
 		super(repositoryInterface);
-		Assert.isTrue(repositoryInterface.isInterface());
-		Assert.isTrue(Repository.class.isAssignableFrom(repositoryInterface));
-		this.repositoryInterface = repositoryInterface;
+		Assert.isTrue(Repository.class.isAssignableFrom(repositoryInterface), MUST_BE_A_REPOSITORY);
+
+		this.idType = resolveIdType(repositoryInterface);
+		this.domainType = resolveDomainType(repositoryInterface);
 	}
 
-	/*
+	/* 
 	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.support.RepositoryMetadata#getRepositoryInterface()
+	 * @see org.springframework.data.repository.core.RepositoryMetadata#getDomainType()
 	 */
-	public Class<?> getRepositoryInterface() {
-
-		return repositoryInterface;
+	@Override
+	public Class<?> getDomainType() {
+		return this.domainType;
 	}
 
-	/*
+	/* 
 	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.support.RepositoryMetadata#getDomainClass()
+	 * @see org.springframework.data.repository.core.RepositoryMetadata#getIdType()
 	 */
-	public Class<?> getDomainType() {
+	@Override
+	public Class<? extends Serializable> getIdType() {
+		return this.idType;
+	}
+
+	/**
+	 * @param repositoryInterface must not be {@literal null}.
+	 * @return the resolved domain type, never {@literal null}.
+	 */
+	private Class<?> resolveDomainType(Class<?> repositoryInterface) {
+
+		Assert.notNull(repositoryInterface, "Repository interface must not be null!");
 
 		Class<?>[] arguments = resolveTypeArguments(repositoryInterface, Repository.class);
-		return arguments == null ? null : arguments[0];
+		Assert.isTrue(arguments != null && arguments[0] != null,
+				String.format("Could not resolve domain type of %s!", repositoryInterface));
+
+		return arguments[0];
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.support.RepositoryMetadata#getIdClass()
+	/**
+	 * @param repositoryInterface must not be {@literal null}.
+	 * @return the resolved id type, never {@literal null}.
 	 */
-	@SuppressWarnings("unchecked")
-	public Class<? extends Serializable> getIdType() {
+	private Class<? extends Serializable> resolveIdType(Class<?> repositoryInterface) {
+
+		Assert.notNull(repositoryInterface, "Repository interface must not be null!");
 
 		Class<?>[] arguments = resolveTypeArguments(repositoryInterface, Repository.class);
-		return (Class<? extends Serializable>) (arguments == null ? null : arguments[1]);
+		Assert.isTrue(arguments != null && arguments[1] != null,
+				String.format("Could not resolve id type of %s!", repositoryInterface));
+
+		return (Class<? extends Serializable>) arguments[1];
 	}
 }

src/test/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadataUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2012 the original author or authors.
+ * Copyright 2011-2013 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.
@@ -34,6 +34,7 @@
  * Unit tests for {@link AbstractRepositoryMetadata}.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 public class AbstractRepositoryMetadataUnitTests {
 
@@ -118,10 +119,6 @@ public Class<? extends Serializable> getIdType() {
 		public Class<?> getDomainType() {
 			return null;
 		}
-
-		public Class<?> getRepositoryInterface() {
-			return null;
-		}
 	}
 
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2012 the original author or authors.
+ * Copyright 2011-2013 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.
@@ -24,13 +24,15 @@
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.repository.CrudRepository;
+import org.springframework.data.repository.Repository;
 import org.springframework.data.repository.core.RepositoryMetadata;
 import org.springframework.data.repository.util.ClassUtils;
 
 /**
  * Unit tests for {@link DefaultRepositoryMetadata}.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 public class DefaultRepositoryMetadataUnitTests {
 
@@ -50,6 +52,14 @@ public void rejectsNonRepositoryInterface() {
 		new DefaultRepositoryMetadata(Collection.class);
 	}
 
+	/**
+	 * @see DATACMNS-406
+	 */
+	@Test(expected = IllegalArgumentException.class)
+	public void rejectsUnparameterizedRepositoryInterface() {
+		new DefaultRepositoryMetadata(Repository.class);
+	}
+
 	@Test
 	public void looksUpDomainClassCorrectly() throws Exception {
 
@@ -137,10 +147,7 @@ public T findOne(ID id) {
 	 * 
 	 * @author Oliver Gierke
 	 */
-	static class GenericEntity<T> {
-	}
+	static class GenericEntity<T> {}
 
-	static interface GenericEntityRepository extends CrudRepository<GenericEntity<String>, Long> {
-
-	}
+	static interface GenericEntityRepository extends CrudRepository<GenericEntity<String>, Long> {}
 }