DATACMNS-226 - Fixed bug in ConfigurableTypeInformationMapper caching.

Separated ConfigurableTypeInformationMapper from MappingContextTypeInformationMapper. The latter now lazily picks up the alias information if the cached values do not contain aliases.

Polished JavaDocs and clarified meaning of null values.

spring-data-commons-core/src/main/java/org/springframework/data/convert/ConfigurableTypeInformationMapper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 the original author or authors.
+ * Copyright 2011-12 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.
@@ -36,25 +36,6 @@ public class ConfigurableTypeInformationMapper implements TypeInformationMapper
 
 	private final Map<TypeInformation<?>, Object> typeMap;
 
-	/**
-	 * Creates a {@link ConfigurableTypeInformationMapper} from the given {@link MappingContext}. Inspects all
-	 * {@link PersistentEntity} instances for alias information and builds a {@link Map} of aliases to types from it.
-	 * 
-	 * @param mappingContext
-	 */
-	public ConfigurableTypeInformationMapper(MappingContext<? extends PersistentEntity<?, ?>, ?> mappingContext) {
-
-		Assert.notNull(mappingContext);
-		this.typeMap = new HashMap<TypeInformation<?>, Object>();
-
-		for (PersistentEntity<?, ?> entity : mappingContext.getPersistentEntities()) {
-			Object alias = entity.getTypeAlias();
-			if (alias != null) {
-				typeMap.put(entity.getTypeInformation(), alias);
-			}
-		}
-	}
-
 	/**
 	 * Creates a new {@link ConfigurableTypeMapper} for the given type map.
 	 * 
@@ -63,7 +44,6 @@ public class ConfigurableTypeInformationMapper implements TypeInformationMapper
 	public ConfigurableTypeInformationMapper(Map<? extends Class<?>, String> sourceTypeMap) {
 
 		Assert.notNull(sourceTypeMap);
-
 		this.typeMap = new HashMap<TypeInformation<?>, Object>(sourceTypeMap.size());
 
 		for (Entry<? extends Class<?>, String> entry : sourceTypeMap.entrySet()) {
@@ -79,13 +59,12 @@ public class ConfigurableTypeInformationMapper implements TypeInformationMapper
 		}
 	}
 
-	/* (non-Javadoc)
+	/* 
+	 * (non-Javadoc)
 	 * @see org.springframework.data.convert.TypeInformationMapper#createAliasFor(org.springframework.data.util.TypeInformation)
 	 */
 	public Object createAliasFor(TypeInformation<?> type) {
-
-		Object key = typeMap.get(type);
-		return key == null ? null : key;
+		return typeMap.get(type);
 	}
 
 	/*
@@ -94,6 +73,10 @@ public class ConfigurableTypeInformationMapper implements TypeInformationMapper
 	 */
 	public TypeInformation<?> resolveTypeFrom(Object alias) {
 
+		if (alias == null) {
+			return null;
+		}
+
 		for (Entry<TypeInformation<?>, Object> entry : typeMap.entrySet()) {
 			if (entry.getValue().equals(alias)) {
 				return entry.getKey();

spring-data-commons-core/src/main/java/org/springframework/data/convert/DefaultTypeMapper.java

@@ -39,24 +39,46 @@ public class DefaultTypeMapper<S> implements TypeMapper<S> {
 	private final TypeAliasAccessor<S> accessor;
 	private final List<? extends TypeInformationMapper> mappers;
 
+	/**
+	 * Creates a new {@link DefaultTypeMapper} using the given {@link TypeAliasAccessor}. It will use a
+	 * {@link SimpleTypeInformationMapper} to calculate type aliases.
+	 * 
+	 * @param accessor must not be {@literal null}.
+	 */
 	public DefaultTypeMapper(TypeAliasAccessor<S> accessor) {
 		this(accessor, Arrays.asList(SimpleTypeInformationMapper.INSTANCE));
 	}
 
+	/**
+	 * Creates a new {@link DefaultTypeMapper} using the given {@link TypeAliasAccessor} and {@link TypeInformationMapper}
+	 * s.
+	 * 
+	 * @param accessor must not be {@literal null}.
+	 * @param mappers must not be {@literal null}.
+	 */
 	public DefaultTypeMapper(TypeAliasAccessor<S> accessor, List<? extends TypeInformationMapper> mappers) {
-
 		this(accessor, null, mappers);
 	}
 
+	/**
+	 * Creates a new {@link DefaultTypeMapper} using the given {@link TypeAliasAccessor}, {@link MappingContext} and
+	 * additional {@link TypeInformationMapper}s. Will register a {@link MappingContextTypeInformationMapper} before the
+	 * given additional mappers.
+	 * 
+	 * @param accessor must not be {@literal null}.
+	 * @param mappingContext
+	 * @param additionalMappers must not be {@literal null}.
+	 */
 	public DefaultTypeMapper(TypeAliasAccessor<S> accessor,
 			MappingContext<? extends PersistentEntity<?, ?>, ?> mappingContext,
 			List<? extends TypeInformationMapper> additionalMappers) {
 
 		Assert.notNull(accessor);
+		Assert.notNull(additionalMappers);
 
 		List<TypeInformationMapper> mappers = new ArrayList<TypeInformationMapper>(additionalMappers.size() + 1);
 		if (mappingContext != null) {
-			mappers.add(new ConfigurableTypeInformationMapper(mappingContext));
+			mappers.add(new MappingContextTypeInformationMapper(mappingContext));
 		}
 		mappers.addAll(additionalMappers);
 
@@ -73,6 +95,10 @@ public class DefaultTypeMapper<S> implements TypeMapper<S> {
 		Assert.notNull(source);
 		Object alias = accessor.readAliasFrom(source);
 
+		if (alias == null) {
+			return null;
+		}
+
 		for (TypeInformationMapper mapper : mappers) {
 			TypeInformation<?> type = mapper.resolveTypeFrom(alias);
 

spring-data-commons-core/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java

@@ -0,0 +1,125 @@
+/*
+ * Copyright 2011-2012 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.convert;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.context.MappingContext;
+import org.springframework.data.util.TypeInformation;
+import org.springframework.util.Assert;
+
+/**
+ * {@link TypeInformationMapper} implementation that can be either set up using a {@link MappingContext} or manually set
+ * up {@link Map} of {@link String} aliases to types. If a {@link MappingContext} is used the {@link Map} will be build
+ * inspecting the {@link PersistentEntity} instances for type alias information.
+ * 
+ * @author Oliver Gierke
+ */
+public class MappingContextTypeInformationMapper implements TypeInformationMapper {
+
+	private final Map<TypeInformation<?>, Object> typeMap;
+	private final MappingContext<? extends PersistentEntity<?, ?>, ?> mappingContext;
+
+	/**
+	 * Creates a {@link MappingContextTypeInformationMapper} from the given {@link MappingContext}. Inspects all
+	 * {@link PersistentEntity} instances for alias information and builds a {@link Map} of aliases to types from it.
+	 * 
+	 * @param mappingContext must not be {@literal null}.
+	 */
+	public MappingContextTypeInformationMapper(MappingContext<? extends PersistentEntity<?, ?>, ?> mappingContext) {
+
+		Assert.notNull(mappingContext);
+
+		this.typeMap = new HashMap<TypeInformation<?>, Object>();
+		this.mappingContext = mappingContext;
+
+		for (PersistentEntity<?, ?> entity : mappingContext.getPersistentEntities()) {
+			safelyAddToCache(entity.getTypeInformation(), entity.getTypeAlias());
+		}
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.convert.TypeInformationMapper#createAliasFor(org.springframework.data.util.TypeInformation)
+	 */
+	public Object createAliasFor(TypeInformation<?> type) {
+
+		Object key = typeMap.get(type);
+
+		if (key != null) {
+			return key;
+		}
+
+		PersistentEntity<?, ?> entity = mappingContext.getPersistentEntity(type);
+
+		if (entity == null) {
+			return null;
+		}
+
+		Object alias = entity.getTypeAlias();
+		safelyAddToCache(type, alias);
+
+		return alias;
+	}
+
+	/**
+	 * Adds the given alias to the cache in a {@literal null}-safe manner.
+	 * 
+	 * @param key must not be {@literal null}.
+	 * @param alias can be {@literal null}.
+	 */
+	private void safelyAddToCache(TypeInformation<?> key, Object alias) {
+
+		if (alias == null) {
+			return;
+		}
+
+		if (typeMap.containsValue(alias)) {
+			throw new IllegalArgumentException(String.format(
+					"Detected mapping ambiguity! String %s cannot be mapped to more than one type!", alias));
+		}
+
+		typeMap.put(key, alias);
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.convert.TypeInformationMapper#resolveTypeFrom(java.lang.Object)
+	 */
+	public TypeInformation<?> resolveTypeFrom(Object alias) {
+
+		if (alias == null) {
+			return null;
+		}
+
+		for (Entry<TypeInformation<?>, Object> entry : typeMap.entrySet()) {
+			if (entry.getValue().equals(alias)) {
+				return entry.getKey();
+			}
+		}
+
+		for (PersistentEntity<?, ?> entity : mappingContext.getPersistentEntities()) {
+			if (alias.equals(entity.getTypeAlias())) {
+				return entity.getTypeInformation();
+			}
+		}
+
+		return null;
+	}
+}

spring-data-commons-core/src/main/java/org/springframework/data/convert/SimpleTypeInformationMapper.java

@@ -40,13 +40,13 @@ public class SimpleTypeInformationMapper implements TypeInformationMapper {
 	 * @return the type to be used for the given {@link String} representation or {@literal null} if nothing found or the
 	 *         class cannot be loaded.
 	 */
-	public TypeInformation<?> resolveTypeFrom(Object source) {
+	public TypeInformation<?> resolveTypeFrom(Object alias) {
 
-		if (!(source instanceof String)) {
+		if (!(alias instanceof String)) {
 			return null;
 		}
 
-		String value = (String) source;
+		String value = (String) alias;
 
 		if (!StringUtils.hasText(value)) {
 			return null;

spring-data-commons-core/src/main/java/org/springframework/data/convert/TypeAliasAccessor.java

@@ -26,7 +26,7 @@ public interface TypeAliasAccessor<S> {
 	 * Reads the type alias to be used from the given source.
 	 * 
 	 * @param source
-	 * @return
+	 * @return can be {@literal null} in case no alias was found.
 	 */
 	Object readAliasFrom(S source);
 

spring-data-commons-core/src/main/java/org/springframework/data/convert/TypeInformationMapper.java

@@ -27,7 +27,7 @@ public interface TypeInformationMapper {
 	/**
 	 * Returns the actual {@link TypeInformation} to be used for the given alias.
 	 * 
-	 * @param alias
+	 * @param alias can be {@literal null}.
 	 * @return
 	 */
 	TypeInformation<?> resolveTypeFrom(Object alias);

spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

@@ -409,6 +409,10 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 				return;
 			}
 
+			if (!property.isEntity()) {
+				return;
+			}
+
 			for (TypeInformation<?> candidate : property.getPersistentEntityType()) {
 				addPersistentEntity(candidate);
 			}

spring-data-commons-core/src/test/java/org/springframework/data/convert/ConfigurableTypeInformationMapperUnitTests.java

@@ -18,8 +18,6 @@ package org.springframework.data.convert;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 
-import java.beans.PropertyDescriptor;
-import java.lang.reflect.Field;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -27,18 +25,8 @@ import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
-import org.springframework.context.ApplicationContext;
-import org.springframework.context.event.ContextRefreshedEvent;
-import org.springframework.data.annotation.TypeAlias;
-import org.springframework.data.mapping.Association;
 import org.springframework.data.mapping.PersistentProperty;
-import org.springframework.data.mapping.context.AbstractMappingContext;
-import org.springframework.data.mapping.context.MappingContext;
-import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty;
-import org.springframework.data.mapping.model.BasicPersistentEntity;
-import org.springframework.data.mapping.model.SimpleTypeHolder;
 import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.data.util.TypeInformation;
 
@@ -51,8 +39,6 @@ import org.springframework.data.util.TypeInformation;
 public class ConfigurableTypeInformationMapperUnitTests<T extends PersistentProperty<T>> {
 
 	ConfigurableTypeInformationMapper mapper;
-	@Mock
-	ApplicationContext context;
 
 	@Before
 	public void setUp() {
@@ -61,16 +47,12 @@ public class ConfigurableTypeInformationMapperUnitTests<T extends PersistentProp
 
 	@Test(expected = IllegalArgumentException.class)
 	public void rejectsNullTypeMap() {
-		new ConfigurableTypeInformationMapper((Map<? extends Class<?>, String>) null);
-	}
-
-	@Test(expected = IllegalArgumentException.class)
-	public void rejectsNullMappingContext() {
-		new ConfigurableTypeInformationMapper((MappingContext<?, ?>) null);
+		new ConfigurableTypeInformationMapper(null);
 	}
 
 	@Test(expected = IllegalArgumentException.class)
 	public void rejectsNonBijectionalMap() {
+
 		Map<Class<?>, String> map = new HashMap<Class<?>, String>();
 		map.put(String.class, "1");
 		map.put(Object.class, "1");
@@ -78,40 +60,6 @@ public class ConfigurableTypeInformationMapperUnitTests<T extends PersistentProp
 		new ConfigurableTypeInformationMapper(map);
 	}
 
-	@Test
-	@SuppressWarnings("unchecked")
-	public void extractsAliasInfoFromMappingContext() {
-
-		AbstractMappingContext<BasicPersistentEntity<Object, T>, T> mappingContext = new AbstractMappingContext<BasicPersistentEntity<Object, T>, T>() {
-
-			@Override
-			protected <S> BasicPersistentEntity<Object, T> createPersistentEntity(TypeInformation<S> typeInformation) {
-				return (BasicPersistentEntity<Object, T>) new BasicPersistentEntity<S, T>(typeInformation);
-			}
-
-			@Override
-			protected T createPersistentProperty(Field field, PropertyDescriptor descriptor,
-					BasicPersistentEntity<Object, T> owner, SimpleTypeHolder simpleTypeHolder) {
-				return (T) new AnnotationBasedPersistentProperty<T>(field, descriptor, owner, simpleTypeHolder) {
-					@Override
-					protected Association<T> createAssociation() {
-						return null;
-					}
-				};
-			}
-		};
-
-		ContextRefreshedEvent event = new ContextRefreshedEvent(context);
-
-		mappingContext.setInitialEntitySet(Collections.singleton(Entity.class));
-		mappingContext.setApplicationContext(context);
-		mappingContext.onApplicationEvent(event);
-
-		mapper = new ConfigurableTypeInformationMapper(mappingContext);
-
-		assertThat(mapper.createAliasFor(ClassTypeInformation.from(Entity.class)), is((Object) "foo"));
-	}
-
 	@Test
 	public void writesMapKeyForType() {
 
@@ -126,9 +74,4 @@ public class ConfigurableTypeInformationMapperUnitTests<T extends PersistentProp
 		assertThat(mapper.resolveTypeFrom("1"), is((TypeInformation) ClassTypeInformation.from(String.class)));
 		assertThat(mapper.resolveTypeFrom("unmapped"), is(nullValue()));
 	}
-
-	@TypeAlias("foo")
-	class Entity {
-
-	}
 }

spring-data-commons-core/src/test/java/org/springframework/data/convert/MappingContextTypeInformationMapperUnitTests.java

@@ -0,0 +1,106 @@
+/*
+ * Copyright 2012 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.convert;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+import static org.springframework.data.util.ClassTypeInformation.*;
+
+import java.util.Collections;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.data.annotation.TypeAlias;
+import org.springframework.data.mapping.MappingMetadataTests;
+import org.springframework.data.mapping.MappingMetadataTests.SampleMappingContext;
+import org.springframework.data.mapping.MappingMetadataTests.SampleProperty;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.TypeInformation;
+
+/**
+ * Unit tests for {@link MappingContextTypeInformationMapper}.
+ * 
+ * @author Oliver Gierke
+ */
+public class MappingContextTypeInformationMapperUnitTests {
+
+	SampleMappingContext mappingContext;
+	TypeInformationMapper mapper;
+
+	@Before
+	public void setUp() {
+		mappingContext = new MappingMetadataTests.SampleMappingContext();
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void rejectsNullMappingContext() {
+		new MappingContextTypeInformationMapper(null);
+	}
+
+	@Test
+	public void extractsAliasInfoFromMappingContext() {
+
+		mappingContext.setInitialEntitySet(Collections.singleton(Entity.class));
+		mappingContext.initialize();
+
+		mapper = new MappingContextTypeInformationMapper(mappingContext);
+
+		assertThat(mapper.createAliasFor(ClassTypeInformation.from(Entity.class)), is((Object) "foo"));
+	}
+
+	@Test
+	public void extractsAliasForUnknownType() {
+
+		SampleMappingContext mappingContext = new MappingMetadataTests.SampleMappingContext();
+		mappingContext.initialize();
+
+		mapper = new MappingContextTypeInformationMapper(mappingContext);
+
+		assertThat(mapper.createAliasFor(from(Entity.class)), is((Object) "foo"));
+	}
+
+	@Test
+	public void doesNotReturnTypeAliasForSimpleType() {
+
+		SampleMappingContext mappingContext = new MappingMetadataTests.SampleMappingContext();
+		mappingContext.initialize();
+
+		mapper = new MappingContextTypeInformationMapper(mappingContext);
+		assertThat(mapper.createAliasFor(from(String.class)), is(nullValue()));
+	}
+
+	@Test
+	@SuppressWarnings("rawtypes")
+	public void detectsTypeForUnknownEntity() {
+
+		SampleMappingContext mappingContext = new MappingMetadataTests.SampleMappingContext();
+		mappingContext.initialize();
+
+		mapper = new MappingContextTypeInformationMapper(mappingContext);
+		assertThat(mapper.resolveTypeFrom("foo"), is(nullValue()));
+
+		PersistentEntity<?, SampleProperty> entity = mappingContext.getPersistentEntity(Entity.class);
+
+		assertThat(entity, is(notNullValue()));
+		assertThat(mapper.resolveTypeFrom("foo"), is((TypeInformation) from(Entity.class)));
+	}
+
+	@TypeAlias("foo")
+	static class Entity {
+
+	}
+}

spring-data-commons-core/src/test/java/org/springframework/data/mapping/MappingMetadataTests.java

@@ -73,7 +73,7 @@ public class MappingMetadataTests {
 	public interface SampleProperty extends PersistentProperty<SampleProperty> {
 	}
 
-	public class SampleMappingContext extends
+	public static class SampleMappingContext extends
 			AbstractMappingContext<MutablePersistentEntity<?, SampleProperty>, SampleProperty> {
 
 		@Override
@@ -89,7 +89,8 @@ public class MappingMetadataTests {
 		}
 	}
 
-	public class SamplePropertyImpl extends AnnotationBasedPersistentProperty<SampleProperty> implements SampleProperty {
+	public static class SamplePropertyImpl extends AnnotationBasedPersistentProperty<SampleProperty> implements
+			SampleProperty {
 
 		public SamplePropertyImpl(Field field, PropertyDescriptor propertyDescriptor,
 				PersistentEntity<?, SampleProperty> owner, SimpleTypeHolder simpleTypeHolder) {