DATACMNS-432 - Make Repositories effectively immutable.

We now eagerly populate the repository factory information in Repositories to provide a read only view on the discovered repository information. Previously lookup operations could also change some maps that held information about the until then discovered repository meta data which could lead to ConcurrentModifcationExceptions in multi-threaded environments. Since we don't allow any modification after construction this won't happen anymore.

We also cache the computed RepositoryMetadata in RepositoryFactoryBeanSupport.

Original pull request: #63.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2013 the original author or authors.
+ * Copyright 2008-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.
@@ -40,6 +40,7 @@ import org.springframework.util.Assert;
  * 
  * @param <T> the type of the repository
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 public abstract class RepositoryFactoryBeanSupport<T extends Repository<S, ID>, S, ID extends Serializable> implements
 		InitializingBean, RepositoryFactoryInformation<S, ID>, FactoryBean<T>, BeanClassLoaderAware {
@@ -56,6 +57,8 @@ public abstract class RepositoryFactoryBeanSupport<T extends Repository<S, ID>,
 
 	private T repository;
 
+	private RepositoryMetadata repositoryMetadata;
+
 	/**
 	 * Setter to inject the repository interface to implement.
 	 * 
@@ -130,7 +133,6 @@ public abstract class RepositoryFactoryBeanSupport<T extends Repository<S, ID>,
 	@SuppressWarnings("unchecked")
 	public EntityInformation<S, ID> getEntityInformation() {
 
-		RepositoryMetadata repositoryMetadata = factory.getRepositoryMetadata(repositoryInterface);
 		return (EntityInformation<S, ID>) factory.getEntityInformation(repositoryMetadata.getDomainType());
 	}
 
@@ -140,9 +142,8 @@ public abstract class RepositoryFactoryBeanSupport<T extends Repository<S, ID>,
 	 */
 	public RepositoryInformation getRepositoryInformation() {
 
-		RepositoryMetadata metadata = factory.getRepositoryMetadata(repositoryInterface);
-		return this.factory.getRepositoryInformation(metadata,
-				customImplementation == null ? null : customImplementation.getClass());
+		return this.factory.getRepositoryInformation(repositoryMetadata, customImplementation == null ? null
+				: customImplementation.getClass());
 	}
 
 	/* 
@@ -155,8 +156,7 @@ public abstract class RepositoryFactoryBeanSupport<T extends Repository<S, ID>,
 			return null;
 		}
 
-		RepositoryMetadata metadata = factory.getRepositoryMetadata(repositoryInterface);
-		return mappingContext.getPersistentEntity(metadata.getDomainType());
+		return mappingContext.getPersistentEntity(repositoryMetadata.getDomainType());
 	}
 
 	/* (non-Javadoc)
@@ -197,11 +197,15 @@ public abstract class RepositoryFactoryBeanSupport<T extends Repository<S, ID>,
 	 */
 	public void afterPropertiesSet() {
 
+		Assert.notNull(repositoryInterface, "Repository interface must not be null on initialization!");
+
 		this.factory = createRepositoryFactory();
 		this.factory.setQueryLookupStrategyKey(queryLookupStrategyKey);
 		this.factory.setNamedQueries(namedQueries);
 		this.factory.setBeanClassLoader(classLoader);
 
+		this.repositoryMetadata = this.factory.getRepositoryMetadata(repositoryInterface);
+
 		if (!lazyInit) {
 			initAndReturn();
 		}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2013 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.
@@ -16,10 +16,8 @@
 package org.springframework.data.repository.support;
 
 import java.io.Serializable;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -43,22 +41,25 @@ import org.springframework.util.ClassUtils;
  * Wrapper class to access repository instances obtained from a {@link ListableBeanFactory}.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 public class Repositories implements Iterable<Class<?>> {
 
 	static final Repositories NONE = new Repositories();
-
-	private final Map<Class<?>, RepositoryFactoryInformation<Object, Serializable>> domainClassToBeanName = new HashMap<Class<?>, RepositoryFactoryInformation<Object, Serializable>>();
-	private final Map<RepositoryFactoryInformation<Object, Serializable>, String> repositories = new HashMap<RepositoryFactoryInformation<Object, Serializable>, String>();
+	private static final RepositoryFactoryInformation<Object, Serializable> EMPTY_REPOSITORY_FACTORY_INFO = EmptyRepositoryFactoryInformation.INSTANCE;
 
 	private final BeanFactory beanFactory;
-	private final Set<String> repositoryFactoryBeanNames = new HashSet<String>();
+	private final Map<Class<?>, String> repositoryBeanNames;
+	private final Map<Class<?>, RepositoryFactoryInformation<Object, Serializable>> repositoryFactoryInfos;
 
 	/**
 	 * Constructor to create the {@link #NONE} instance.
 	 */
 	private Repositories() {
+
 		this.beanFactory = null;
+		this.repositoryBeanNames = Collections.<Class<?>, String> emptyMap();
+		this.repositoryFactoryInfos = Collections.<Class<?>, RepositoryFactoryInformation<Object, Serializable>> emptyMap();
 	}
 
 	/**
@@ -70,11 +71,31 @@ public class Repositories implements Iterable<Class<?>> {
 	public Repositories(ListableBeanFactory factory) {
 
 		Assert.notNull(factory);
+
 		this.beanFactory = factory;
+		this.repositoryFactoryInfos = new HashMap<Class<?>, RepositoryFactoryInformation<Object, Serializable>>();
+		this.repositoryBeanNames = new HashMap<Class<?>, String>();
+
+		populateRepositoryFactoryInformation(factory);
+	}
+
+	@SuppressWarnings({ "rawtypes", "unchecked" })
+	private void populateRepositoryFactoryInformation(ListableBeanFactory factory) {
 
-		String[] beanNamesForType = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory,
-				RepositoryFactoryInformation.class, false, false);
-		this.repositoryFactoryBeanNames.addAll(Arrays.asList(beanNamesForType));
+		Set<Map.Entry<String, RepositoryFactoryInformation>> repositoryFactoryBeans = BeanFactoryUtils
+				.beansOfTypeIncludingAncestors(factory, RepositoryFactoryInformation.class).entrySet();
+
+		for (Map.Entry<String, RepositoryFactoryInformation> entry : repositoryFactoryBeans) {
+
+			String beanName = entry.getKey();
+
+			RepositoryFactoryInformation repositoryFactoryInformation = entry.getValue();
+			Class<?> userDomainType = ClassUtils.getUserClass(repositoryFactoryInformation.getRepositoryInformation()
+					.getDomainType());
+
+			this.repositoryFactoryInfos.put(userDomainType, repositoryFactoryInformation);
+			this.repositoryBeanNames.put(userDomainType, BeanFactoryUtils.transformedBeanName(beanName));
+		}
 	}
 
 	/**
@@ -84,8 +105,10 @@ public class Repositories implements Iterable<Class<?>> {
 	 * @return
 	 */
 	public boolean hasRepositoryFor(Class<?> domainClass) {
-		lookupRepositoryFactoryInformationFor(domainClass);
-		return domainClassToBeanName.containsKey(domainClass);
+
+		Assert.notNull(domainClass, "Domain class must not be null!");
+
+		return repositoryFactoryInfos.containsKey(domainClass);
 	}
 
 	/**
@@ -96,13 +119,27 @@ public class Repositories implements Iterable<Class<?>> {
 	 */
 	public Object getRepositoryFor(Class<?> domainClass) {
 
-		RepositoryFactoryInformation<Object, Serializable> information = getRepoInfoFor(domainClass);
+		Assert.notNull(domainClass, "Domain class must not be null!");
 
-		if (information == null) {
-			return null;
-		}
+		String repositoryBeanName = repositoryBeanNames.get(domainClass);
+		return repositoryBeanName == null || beanFactory == null ? null : beanFactory.getBean(repositoryBeanName);
+	}
+
+	/**
+	 * Returns the {@link RepositoryFactoryInformation} for the given domain class. The given <code>code</code> is
+	 * converted to the actual user class if necessary, @see ClassUtils#getUserClass.
+	 * 
+	 * @param domainClass must not be {@literal null}.
+	 * @return the {@link RepositoryFactoryInformation} for the given domain class or {@literal null} if no repository
+	 *         registered for this domain class.
+	 */
+	private RepositoryFactoryInformation<Object, Serializable> getRepositoryFactoryInfoFor(Class<?> domainClass) {
 
-		return beanFactory.getBean(repositories.get(information));
+		Assert.notNull(domainClass, "Domain class must not be null!");
+
+		RepositoryFactoryInformation<Object, Serializable> repositoryInfo = repositoryFactoryInfos.get(ClassUtils
+				.getUserClass(domainClass));
+		return repositoryInfo == null ? EMPTY_REPOSITORY_FACTORY_INFO : repositoryInfo;
 	}
 
 	/**
@@ -114,8 +151,9 @@ public class Repositories implements Iterable<Class<?>> {
 	@SuppressWarnings("unchecked")
 	public <T, S extends Serializable> EntityInformation<T, S> getEntityInformationFor(Class<?> domainClass) {
 
-		RepositoryFactoryInformation<Object, Serializable> information = getRepoInfoFor(domainClass);
-		return information == null ? null : (EntityInformation<T, S>) information.getEntityInformation();
+		Assert.notNull(domainClass, "Domain class must not be null!");
+
+		return (EntityInformation<T, S>) getRepositoryFactoryInfoFor(domainClass).getEntityInformation();
 	}
 
 	/**
@@ -127,8 +165,10 @@ public class Repositories implements Iterable<Class<?>> {
 	 */
 	public RepositoryInformation getRepositoryInformationFor(Class<?> domainClass) {
 
-		RepositoryFactoryInformation<Object, Serializable> information = getRepoInfoFor(domainClass);
-		return information == null ? null : information.getRepositoryInformation();
+		Assert.notNull(domainClass, "Domain class must not be null!");
+
+		RepositoryFactoryInformation<Object, Serializable> information = getRepositoryFactoryInfoFor(domainClass);
+		return information == EMPTY_REPOSITORY_FACTORY_INFO ? null : information.getRepositoryInformation();
 	}
 
 	/**
@@ -141,8 +181,8 @@ public class Repositories implements Iterable<Class<?>> {
 	 */
 	public PersistentEntity<?, ?> getPersistentEntity(Class<?> domainClass) {
 
-		RepositoryFactoryInformation<Object, Serializable> information = getRepoInfoFor(domainClass);
-		return information == null ? null : information.getPersistentEntity();
+		Assert.notNull(domainClass, "Domain class must not be null!");
+		return getRepositoryFactoryInfoFor(domainClass).getPersistentEntity();
 	}
 
 	/**
@@ -153,14 +193,13 @@ public class Repositories implements Iterable<Class<?>> {
 	 */
 	public List<QueryMethod> getQueryMethodsFor(Class<?> domainClass) {
 
-		RepositoryFactoryInformation<Object, Serializable> information = getRepoInfoFor(domainClass);
-		return information == null ? Collections.<QueryMethod> emptyList() : information.getQueryMethods();
+		Assert.notNull(domainClass, "Domain class must not be null!");
+		return getRepositoryFactoryInfoFor(domainClass).getQueryMethods();
 	}
 
 	@SuppressWarnings("unchecked")
 	public <T> CrudInvoker<T> getCrudInvoker(Class<T> domainClass) {
 
-		RepositoryInformation information = getRepositoryInformationFor(domainClass);
 		Object repository = getRepositoryFor(domainClass);
 
 		Assert.notNull(repository, String.format("No repository found for domain class: %s", domainClass));
@@ -168,67 +207,45 @@ public class Repositories implements Iterable<Class<?>> {
 		if (repository instanceof CrudRepository) {
 			return new CrudRepositoryInvoker<T>((CrudRepository<T, Serializable>) repository);
 		} else {
-			return new ReflectionRepositoryInvoker<T>(repository, information.getCrudMethods());
+			return new ReflectionRepositoryInvoker<T>(repository, getRepositoryInformationFor(domainClass).getCrudMethods());
 		}
 	}
 
-	private RepositoryFactoryInformation<Object, Serializable> getRepoInfoFor(Class<?> domainClass) {
-
-		Assert.notNull(domainClass);
-
-		// Create defensive copy of the keys to allow threads to potentially add values while iterating over them
-		Set<RepositoryFactoryInformation<Object, Serializable>> keys = Collections.unmodifiableSet(repositories.keySet());
-		Class<?> type = ClassUtils.getUserClass(domainClass);
-
-		for (RepositoryFactoryInformation<Object, Serializable> information : keys) {
-			if (type.equals(information.getEntityInformation().getJavaType())) {
-				return information;
-			}
-		}
-
-		return lookupRepositoryFactoryInformationFor(type);
-	}
-
 	/* 
 	 * (non-Javadoc)
 	 * @see java.lang.Iterable#iterator()
 	 */
 	public Iterator<Class<?>> iterator() {
-		lookupRepositoryFactoryInformationFor(null);
-		return domainClassToBeanName.keySet().iterator();
+		return repositoryFactoryInfos.keySet().iterator();
 	}
 
 	/**
-	 * Looks up the {@link RepositoryFactoryInformation} for a given domain type. Will inspect the {@link BeanFactory} for
-	 * beans implementing {@link RepositoryFactoryInformation} and cache the domain class to repository bean name mappings
-	 * for further lookups. If a {@link RepositoryFactoryInformation} for the given domain type is found we interrupt the
-	 * lookup proces to prevent beans from being looked up early.
+	 * Null-object to avoid nasty {@literal null} checks in cache lookups.
 	 * 
-	 * @param domainType
-	 * @return
+	 * @author Thomas Darimont
 	 */
-	@SuppressWarnings("unchecked")
-	private RepositoryFactoryInformation<Object, Serializable> lookupRepositoryFactoryInformationFor(Class<?> domainType) {
-
-		if (domainClassToBeanName.containsKey(domainType)) {
-			return domainClassToBeanName.get(domainType);
-		}
+	private static enum EmptyRepositoryFactoryInformation implements RepositoryFactoryInformation<Object, Serializable> {
 
-		for (String repositoryFactoryName : repositoryFactoryBeanNames) {
+		INSTANCE;
 
-			RepositoryFactoryInformation<Object, Serializable> information = beanFactory.getBean(repositoryFactoryName,
-					RepositoryFactoryInformation.class);
-
-			RepositoryInformation info = information.getRepositoryInformation();
+		@Override
+		public EntityInformation<Object, Serializable> getEntityInformation() {
+			return null;
+		}
 
-			repositories.put(information, BeanFactoryUtils.transformedBeanName(repositoryFactoryName));
-			domainClassToBeanName.put(info.getDomainType(), information);
+		@Override
+		public RepositoryInformation getRepositoryInformation() {
+			return null;
+		}
 
-			if (info.getDomainType().equals(domainType)) {
-				return information;
-			}
+		@Override
+		public PersistentEntity<?, ?> getPersistentEntity() {
+			return null;
 		}
 
-		return null;
+		@Override
+		public List<QueryMethod> getQueryMethods() {
+			return Collections.<QueryMethod> emptyList();
+		}
 	}
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-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.
@@ -22,12 +22,14 @@ import static org.mockito.Mockito.*;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.springframework.data.repository.CrudRepository;
 import org.springframework.test.util.ReflectionTestUtils;
 
 /**
  * Unit tests for {@link RepositoryFactoryBeanSupport}.
  * 
  * @author Oliver Gierke
+ * @author Thomas Darimont
  */
 public class RepositoryFactoryBeanSupportUnitTests {
 
@@ -37,7 +39,7 @@ public class RepositoryFactoryBeanSupportUnitTests {
 	 * @see DATACMNS-341
 	 */
 	@Test
-	@SuppressWarnings("rawtypes")
+	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public void setsConfiguredClassLoaderOnRepositoryFactory() {
 
 		ClassLoader classLoader = mock(ClassLoader.class);
@@ -45,12 +47,16 @@ public class RepositoryFactoryBeanSupportUnitTests {
 		RepositoryFactoryBeanSupport factoryBean = new DummyRepositoryFactoryBean();
 		factoryBean.setBeanClassLoader(classLoader);
 		factoryBean.setLazyInit(true);
+		factoryBean.setRepositoryInterface(CrudRepository.class);
 		factoryBean.afterPropertiesSet();
 
 		Object factory = ReflectionTestUtils.getField(factoryBean, "factory");
 		assertThat(ReflectionTestUtils.getField(factory, "classLoader"), is((Object) classLoader));
 	}
 
+	/**
+	 * @see DATACMNS-432
+	 */
 	@Test
 	@SuppressWarnings("rawtypes")
 	public void initializationFailsWithMissingRepositoryInterface() {