DATACMNS-1210 - Fix concurrency issue in BasicPersitentEntity.

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-commons</artifactId>
-	<version>2.1.0.BUILD-SNAPSHOT</version>
+	<version>2.1.0.DATACMNS-1210-SNAPSHOT</version>
 
 	<name>Spring Data Core</name>
 

src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java

@@ -20,7 +20,17 @@
 
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
 
 import org.springframework.core.annotation.AnnotatedElementUtils;
@@ -30,7 +40,8 @@
 import org.springframework.data.util.TypeInformation;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
-import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.CollectionUtils;
+import org.springframework.util.ConcurrentReferenceHashMap;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
 
@@ -93,9 +104,10 @@ public BasicPersistentEntity(TypeInformation<T> information, @Nullable Comparato
 		this.constructor = PreferredConstructorDiscoverer.discover(this);
 		this.associations = comparator == null ? new HashSet<>() : new TreeSet<>(new AssociationComparator<>(comparator));
 
-		this.propertyCache = new HashMap<>();
-		this.annotationCache = new HashMap<>();
-		this.propertyAnnotationCache = new LinkedMultiValueMap<>();
+		this.propertyCache = new ConcurrentReferenceHashMap<>();
+		this.annotationCache = new ConcurrentReferenceHashMap<>();
+		this.propertyAnnotationCache = CollectionUtils
+				.toMultiValueMap(new ConcurrentReferenceHashMap<Class<? extends Annotation>, List<P>>());
 		this.propertyAccessorFactory = BeanWrapperPropertyAccessorFactory.INSTANCE;
 		this.typeAlias = Lazy.of(() -> getAliasFromAnnotation(getType()));
 	}
@@ -193,9 +205,7 @@ public void addPersistentProperty(P property) {
 			persistentPropertiesCache.add(property);
 		}
 
-		if (!propertyCache.containsKey(property.getName())) {
-			propertyCache.put(property.getName(), property);
-		}
+		propertyCache.computeIfAbsent(property.getName(), key -> property);
 
 		P candidate = returnPropertyIfBetterIdPropertyCandidateOrNull(property);
 

src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java

@@ -19,11 +19,14 @@
 import static org.junit.Assume.*;
 import static org.mockito.Mockito.*;
 
+import java.lang.annotation.Annotation;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.hamcrest.CoreMatchers;
 import org.junit.Rule;
@@ -34,9 +37,12 @@
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.core.annotation.AliasFor;
+import org.springframework.data.annotation.AccessType;
+import org.springframework.data.annotation.AccessType.Type;
 import org.springframework.data.annotation.CreatedBy;
 import org.springframework.data.annotation.CreatedDate;
 import org.springframework.data.annotation.LastModifiedBy;
+import org.springframework.data.annotation.Persistent;
 import org.springframework.data.annotation.TypeAlias;
 import org.springframework.data.mapping.Alias;
 import org.springframework.data.mapping.Document;
@@ -49,6 +55,7 @@
 import org.springframework.data.mapping.context.SampleMappingContext;
 import org.springframework.data.mapping.context.SamplePersistentProperty;
 import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.Version;
 import org.springframework.test.util.ReflectionTestUtils;
 
 /**
@@ -276,6 +283,64 @@ public void getRequiredAnnotationThrowsException() {
 		assertThatThrownBy(() -> entity.getRequiredAnnotation(Document.class)).isInstanceOf(IllegalStateException.class);
 	}
 
+	@Test // DATACMNS-1210
+	public void findAnnotationShouldBeThreadSafe() throws InterruptedException {
+
+		assumeTrue("Requires Java 9",
+				Version.parse(System.getProperty("java.version")).isGreaterThanOrEqualTo(Version.parse("9.0")));
+
+		CountDownLatch latch = new CountDownLatch(2);
+		CountDownLatch syncLatch = new CountDownLatch(1);
+
+		final AtomicBoolean failed = new AtomicBoolean(false);
+
+		PersistentEntity<EntityWithAnnotation, T> entity = new BasicPersistentEntity(
+				ClassTypeInformation.from(EntityWithAnnotation.class), null) {
+
+			@Override
+			public Annotation findAnnotation(Class annotationType) {
+
+				try {
+					syncLatch.await();
+				} catch (InterruptedException e) {
+					e.printStackTrace();
+				}
+
+				return super.findAnnotation(annotationType);
+			}
+		};
+
+		Runnable findAccessType = () -> {
+
+			try {
+				entity.findAnnotation(AccessType.class);
+			} catch (Exception e) {
+				failed.set(true);
+			} finally {
+				latch.countDown();
+			}
+		};
+
+		Runnable findPersistent = () -> {
+
+			try {
+				entity.findAnnotation(Persistent.class);
+			} catch (Exception e) {
+				failed.set(true);
+			} finally {
+				latch.countDown();
+			}
+		};
+
+		new Thread(findAccessType).start();
+		new Thread(findPersistent).start();
+
+		syncLatch.countDown();
+		latch.await();
+
+		assertThat(failed.get()).isFalse();
+	}
+
 	private <S> BasicPersistentEntity<S, T> createEntity(Class<S> type) {
 		return createEntity(type, null);
 	}
@@ -315,4 +380,9 @@ public String getProperty() {
 	static class AliasEntityUsingComposedAnnotation {}
 
 	static class Subtype extends Entity {}
+
+	@AccessType(Type.PROPERTY)
+	static class EntityWithAnnotation {
+
+	}
 }