diff --git a/src/main/java/org/springframework/data/convert/BytecodeGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/convert/BytecodeGeneratingEntityInstantiator.java index 657bb6d13..cdec6d8c5 100644 --- a/src/main/java/org/springframework/data/convert/BytecodeGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/convert/BytecodeGeneratingEntityInstantiator.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 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. @@ -223,7 +223,7 @@ public enum BytecodeGeneratingEntityInstantiator implements EntityInstantiator { /** * @author Thomas Darimont */ - interface ObjectInstantiator { + public interface ObjectInstantiator { Object newInstance(Object... args); } diff --git a/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java b/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java index 4bfc15287..b8965e739 100644 --- a/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java +++ b/src/main/java/org/springframework/data/convert/MappingContextTypeInformationMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-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. @@ -15,12 +15,13 @@ */ package org.springframework.data.convert; -import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.util.CacheValue; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -34,7 +35,7 @@ import org.springframework.util.Assert; */ public class MappingContextTypeInformationMapper implements TypeInformationMapper { - private final Map, Object> typeMap; + private final Map, CacheValue> typeMap; private final MappingContext, ?> mappingContext; /** @@ -47,7 +48,7 @@ public class MappingContextTypeInformationMapper implements TypeInformationMappe Assert.notNull(mappingContext); - this.typeMap = new HashMap, Object>(); + this.typeMap = new ConcurrentHashMap, CacheValue>(); this.mappingContext = mappingContext; for (PersistentEntity entity : mappingContext.getPersistentEntities()) { @@ -61,10 +62,10 @@ public class MappingContextTypeInformationMapper implements TypeInformationMappe */ public Object createAliasFor(TypeInformation type) { - Object key = typeMap.get(type); + CacheValue key = typeMap.get(type); if (key != null) { - return key; + return key.getValue(); } PersistentEntity entity = mappingContext.getPersistentEntity(type); @@ -87,26 +88,36 @@ public class MappingContextTypeInformationMapper implements TypeInformationMappe */ private void safelyAddToCache(ClassTypeInformation key, Object alias) { - if (alias == null) { + CacheValue aliasToBeCached = CacheValue.ofNullable(alias); + + if (alias == null && !typeMap.containsKey(key)) { + typeMap.put(key, aliasToBeCached); return; } - Object existingAlias = typeMap.get(key); + CacheValue alreadyCachedAlias = typeMap.get(key); // Reject second alias for same type - if (existingAlias != null && !alias.equals(existingAlias)) { + if (alreadyCachedAlias != null && alreadyCachedAlias.isPresent() && !alreadyCachedAlias.hasValue(alias)) { throw new IllegalArgumentException(String.format( - "Trying to register alias '%s', but found already registered alias '%s' for type %s!", alias, existingAlias, - key)); + "Trying to register alias '%s', but found already registered alias '%s' for type %s!", alias, + alreadyCachedAlias, key)); } // Reject second type for same alias - if (typeMap.containsValue(alias)) { + if (typeMap.containsValue(aliasToBeCached)) { - for (Entry, Object> entry : typeMap.entrySet()) { - if (entry.getValue().equals(alias) && !entry.getKey().equals(key)) { + for (Entry, CacheValue> entry : typeMap.entrySet()) { + + CacheValue value = entry.getValue(); + + if (!value.isPresent()) { + continue; + } + + if (value.hasValue(alias) && !entry.getKey().equals(key)) { throw new IllegalArgumentException(String.format( "Detected existing type mapping of %s to alias '%s' but attempted to bind the same alias to %s!", key, alias, entry.getKey())); @@ -114,7 +125,7 @@ public class MappingContextTypeInformationMapper implements TypeInformationMappe } } - typeMap.put(key, alias); + typeMap.put(key, aliasToBeCached); } /* @@ -127,8 +138,11 @@ public class MappingContextTypeInformationMapper implements TypeInformationMappe return null; } - for (Entry, Object> entry : typeMap.entrySet()) { - if (entry.getValue().equals(alias)) { + for (Entry, CacheValue> entry : typeMap.entrySet()) { + + CacheValue cachedAlias = entry.getValue(); + + if (cachedAlias.hasValue(alias)) { return entry.getKey(); } } diff --git a/src/main/java/org/springframework/data/mapping/PreferredConstructor.java b/src/main/java/org/springframework/data/mapping/PreferredConstructor.java index 8cfb6e8eb..9507eff32 100644 --- a/src/main/java/org/springframework/data/mapping/PreferredConstructor.java +++ b/src/main/java/org/springframework/data/mapping/PreferredConstructor.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-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. @@ -168,11 +168,11 @@ public class PreferredConstructor> { Assert.notNull(parameter); - if (parameters.isEmpty()) { + if (parameters.isEmpty() || !parameter.isEnclosingClassParameter()) { return false; } - return parameters.get(0).equals(parameter) && parameter.isEnclosingClassParameter(); + return parameters.get(0).equals(parameter); } /** @@ -312,10 +312,10 @@ public class PreferredConstructor> { boolean nameEquals = this.name == null ? that.name == null : this.name.equals(that.name); boolean keyEquals = this.key == null ? that.key == null : this.key.equals(that.key); - boolean typeEquals = nullSafeEquals(this.type, that.type); boolean entityEquals = this.entity == null ? that.entity == null : this.entity.equals(that.entity); - return nameEquals && keyEquals && typeEquals && entityEquals; + // Explicitly delay equals check on type as it might be expensive + return nameEquals && keyEquals && entityEquals && this.type.equals(that.type); } /* @@ -329,8 +329,8 @@ public class PreferredConstructor> { result += 31 * nullSafeHashCode(this.name); result += 31 * nullSafeHashCode(this.key); - result += 31 * nullSafeHashCode(this.type); result += 31 * nullSafeHashCode(this.entity); + result += 31 * this.type.hashCode(); return result; } diff --git a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index e1219c1e7..d64ff3200 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -137,7 +137,6 @@ public abstract class AbstractMappingContext type) { - Assert.notNull(type); return getPersistentEntity(ClassTypeInformation.from(type)); } diff --git a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java index 1093c5172..41a85d375 100644 --- a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-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. @@ -53,6 +53,7 @@ public abstract class AbstractPersistentProperty

protected final Association

association; protected final PersistentEntity owner; private final SimpleTypeHolder simpleTypeHolder; + private final int hashCode; public AbstractPersistentProperty(Field field, PropertyDescriptor propertyDescriptor, PersistentEntity owner, SimpleTypeHolder simpleTypeHolder) { @@ -68,6 +69,7 @@ public abstract class AbstractPersistentProperty

this.association = isAssociation() ? createAssociation() : null; this.owner = owner; this.simpleTypeHolder = simpleTypeHolder; + this.hashCode = this.field == null ? this.propertyDescriptor.hashCode() : this.field.hashCode(); } protected abstract Association

createAssociation(); @@ -340,7 +342,7 @@ public abstract class AbstractPersistentProperty

*/ @Override public int hashCode() { - return this.field == null ? this.propertyDescriptor.hashCode() : this.field.hashCode(); + return this.hashCode; } /* diff --git a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java index 01c277965..e4ed226ef 100644 --- a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,9 +17,12 @@ package org.springframework.data.mapping.model; import java.io.Serializable; import java.lang.annotation.Annotation; +import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -52,7 +55,8 @@ public class BasicPersistentEntity> implement private final PreferredConstructor constructor; private final TypeInformation information; - private final Set

properties; + private final List

properties; + private final Comparator

comparator; private final Set> associations; private final Map propertyCache; @@ -83,7 +87,8 @@ public class BasicPersistentEntity> implement Assert.notNull(information); this.information = information; - this.properties = comparator == null ? new HashSet

() : new TreeSet

(comparator); + this.properties = new ArrayList

(); + this.comparator = comparator; this.constructor = new PreferredConstructorDiscoverer(information, this).getConstructor(); this.associations = comparator == null ? new HashSet>() : new TreeSet>( new AssociationComparator

(comparator)); @@ -171,6 +176,11 @@ public class BasicPersistentEntity> implement public void addPersistentProperty(P property) { Assert.notNull(property); + + if (properties.contains(property)) { + return; + } + properties.add(property); if (!propertyCache.containsKey(property.getName())) { @@ -219,7 +229,10 @@ public class BasicPersistentEntity> implement * @see org.springframework.data.mapping.MutablePersistentEntity#addAssociation(org.springframework.data.mapping.model.Association) */ public void addAssociation(Association

association) { - associations.add(association); + + if (!associations.contains(association)) { + associations.add(association); + } } /* @@ -363,7 +376,12 @@ public class BasicPersistentEntity> implement * (non-Javadoc) * @see org.springframework.data.mapping.MutablePersistentEntity#verify() */ - public void verify() {} + public void verify() { + + if (comparator != null) { + Collections.sort(properties, comparator); + } + } /* * (non-Javadoc) diff --git a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java index 709c222c5..ab902aeeb 100644 --- a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java +++ b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -110,15 +110,16 @@ public class BeanWrapper implements PersistentPropertyAccessor { try { - Method getter = property.getGetter(); - if (!property.usePropertyAccess()) { Field field = property.getField(); ReflectionUtils.makeAccessible(field); return (S) ReflectionUtils.getField(field, bean); + } - } else if (property.usePropertyAccess() && getter != null) { + Method getter = property.getGetter(); + + if (property.usePropertyAccess() && getter != null) { ReflectionUtils.makeAccessible(getter); return (S) ReflectionUtils.invokeMethod(getter, bean); diff --git a/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java b/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java index dab3f9ece..9dbf067ad 100644 --- a/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java +++ b/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 by the original author(s). + * Copyright 2011-2015 by the original author(s). * * 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,7 @@ import java.util.Date; import java.util.HashSet; import java.util.Locale; import java.util.Set; +import java.util.concurrent.CopyOnWriteArraySet; import org.springframework.util.Assert; @@ -86,7 +87,7 @@ public class SimpleTypeHolder { public SimpleTypeHolder(Set> customSimpleTypes, boolean registerDefaults) { Assert.notNull(customSimpleTypes); - this.simpleTypes = new HashSet>(customSimpleTypes); + this.simpleTypes = new CopyOnWriteArraySet>(customSimpleTypes); if (registerDefaults) { this.simpleTypes.addAll(DEFAULTS); @@ -104,7 +105,7 @@ public class SimpleTypeHolder { Assert.notNull(customSimpleTypes); Assert.notNull(source); - this.simpleTypes = new HashSet>(customSimpleTypes); + this.simpleTypes = new CopyOnWriteArraySet>(customSimpleTypes); this.simpleTypes.addAll(source.simpleTypes); } @@ -118,12 +119,13 @@ public class SimpleTypeHolder { Assert.notNull(type); - if (Object.class.equals(type)) { + if (Object.class.equals(type) || simpleTypes.contains(type)) { return true; } for (Class clazz : simpleTypes) { if (clazz.isAssignableFrom(type)) { + simpleTypes.add(type); return true; } } diff --git a/src/main/java/org/springframework/data/util/CacheValue.java b/src/main/java/org/springframework/data/util/CacheValue.java new file mode 100644 index 000000000..3fc20fb0d --- /dev/null +++ b/src/main/java/org/springframework/data/util/CacheValue.java @@ -0,0 +1,107 @@ +/* + * Copyright 2014-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. + * 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.util; + +/** + * Wrapper to safely store {@literal null} values in the value cache. + * + * @author Patryk Wasik + * @author Oliver Gierke + * @author Thomas Darimont + */ +public class CacheValue { + + private static final CacheValue ABSENT = new CacheValue(null); + + private final T value; + + /** + * Creates a new {@link CacheValue} for the gven value. + * + * @param type can be {@literal null}. + */ + private CacheValue(T type) { + this.value = type; + } + + /** + * Returns the actual underlying value. + * + * @return + */ + public T getValue() { + return value; + } + + /** + * Returns whether the cached value has an actual value. + * + * @return + */ + public boolean isPresent() { + return value != null; + } + + /** + * Returns whether the cached value has the given actual value. + * + * @param value can be {@literal null}; + * @return + */ + public boolean hasValue(T value) { + return isPresent() ? this.value.equals(value) : value == null; + } + + /** + * Returns a new {@link CacheValue} for the given value. + * + * @param value can be {@literal null}. + * @return will never be {@literal null}. + */ + @SuppressWarnings("unchecked") + public static CacheValue ofNullable(T value) { + return value == null ? (CacheValue) ABSENT : new CacheValue(value); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + return isPresent() ? 0 : value.hashCode(); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + + if (this == obj) { + return true; + } + + if (!(obj instanceof CacheValue)) { + return false; + } + + CacheValue that = (CacheValue) obj; + + return this.value == null ? false : this.value.equals(that.value); + } +} diff --git a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java index 8fb4bc093..9072fe30a 100644 --- a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-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,8 +20,6 @@ import java.lang.reflect.TypeVariable; import java.util.HashMap; import java.util.Map; -import org.springframework.util.ObjectUtils; - /** * Base class for {@link TypeInformation} implementations that need parent type awareness. * @@ -30,6 +28,7 @@ import org.springframework.util.ObjectUtils; public abstract class ParentTypeAwareTypeInformation extends TypeDiscoverer { private final TypeDiscoverer parent; + private int hashCode; /** * Creates a new {@link ParentTypeAwareTypeInformation}. @@ -99,6 +98,11 @@ public abstract class ParentTypeAwareTypeInformation extends TypeDiscoverer implements TypeInformation { private final Type type; private final Map, Type> typeVariableMap; private final Map> fieldTypes = new ConcurrentHashMap>(); + private final int hashCode; private boolean componentTypeResolved = false; private TypeInformation componentType; @@ -72,6 +71,7 @@ class TypeDiscoverer implements TypeInformation { this.type = type; this.typeVariableMap = typeVariableMap; + this.hashCode = 17 + (31 * type.hashCode()) + (31 * typeVariableMap.hashCode()); } /** @@ -513,10 +513,7 @@ class TypeDiscoverer implements TypeInformation { TypeDiscoverer that = (TypeDiscoverer) obj; - boolean typeEqual = nullSafeEquals(this.type, that.type); - boolean typeVariableMapEqual = nullSafeEquals(this.typeVariableMap, that.typeVariableMap); - - return typeEqual && typeVariableMapEqual; + return this.type.equals(that.type) && this.typeVariableMap.equals(that.typeVariableMap); } /* @@ -525,12 +522,6 @@ class TypeDiscoverer implements TypeInformation { */ @Override public int hashCode() { - - int result = 17; - - result += nullSafeHashCode(type); - result += nullSafeHashCode(typeVariableMap); - - return result; + return hashCode; } } diff --git a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java index 195e0cd80..6d879be6a 100644 --- a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-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. @@ -21,7 +21,7 @@ import static org.mockito.Mockito.*; import java.util.Comparator; import java.util.Iterator; -import java.util.SortedSet; +import java.util.List; import org.junit.Rule; import org.junit.Test; @@ -55,7 +55,7 @@ public class BasicPersistentEntityUnitTests> { @Rule public ExpectedException exception = ExpectedException.none(); - @Mock T property; + @Mock T property, anotherProperty; @Test public void assertInvariants() { @@ -111,8 +111,9 @@ public class BasicPersistentEntityUnitTests> { entity.addPersistentProperty(lastName); entity.addPersistentProperty(firstName); entity.addPersistentProperty(ssn); + entity.verify(); - SortedSet properties = (SortedSet) ReflectionTestUtils.getField(entity, "properties"); + List properties = (List) ReflectionTestUtils.getField(entity, "properties"); assertThat(properties.size(), is(3)); Iterator iterator = properties.iterator(); @@ -144,10 +145,11 @@ public class BasicPersistentEntityUnitTests> { MutablePersistentEntity entity = createEntity(Person.class); when(property.isIdProperty()).thenReturn(true); + when(anotherProperty.isIdProperty()).thenReturn(true); entity.addPersistentProperty(property); exception.expect(MappingException.class); - entity.addPersistentProperty(property); + entity.addPersistentProperty(anotherProperty); } /**