From 7d3222320c4f72a34b3fa956c101506501deb309 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 6 Aug 2018 09:53:22 +0200 Subject: [PATCH] DATACMNS-1364 - Store persistent properties in HashMap. We now use HashMap to store persistent properties of a PersistentEntity. An entity is built in a single thread so no concurrent modification happens. Concurrent reads may happen during entity usage which is fine as the PersistentEntity is not changed anymore. Previously, we used ConcurrentReferenceHashMap defaulting to soft references. Soft references can be cleared at the discretion of the GC in response to memory demand. So a default ConcurrentReferenceHashMap is memory-sensitive and acts like a cache with memory-based eviction rules. Persistent properties are not subject to be cached but elements of a PersistentEntity and cannot be recovered once cleared. Original pull request: #304. --- .../data/mapping/model/BasicPersistentEntity.java | 14 +++++++------- .../model/BasicPersistentEntityUnitTests.java | 11 +++++++---- 2 files changed, 14 insertions(+), 11 deletions(-) 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 6885e3835b..2a63814bc1 100644 --- a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java @@ -104,7 +104,7 @@ public BasicPersistentEntity(TypeInformation information, @Nullable Comparato this.constructor = PreferredConstructorDiscoverer.discover(this); this.associations = comparator == null ? new HashSet<>() : new TreeSet<>(new AssociationComparator<>(comparator)); - this.propertyCache = new ConcurrentReferenceHashMap<>(); + this.propertyCache = new HashMap<>(); this.annotationCache = new ConcurrentReferenceHashMap<>(); this.propertyAnnotationCache = CollectionUtils.toMultiValueMap(new ConcurrentReferenceHashMap<>()); this.propertyAccessorFactory = BeanWrapperPropertyAccessorFactory.INSTANCE; @@ -236,7 +236,7 @@ public void addPersistentProperty(P property) { } } - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.model.MutablePersistentEntity#setEvaluationContextProvider(org.springframework.data.spel.EvaluationContextProvider) */ @@ -469,7 +469,7 @@ public IdentifierAccessor getIdentifierAccessor(Object bean) { return hasIdProperty() ? new IdPropertyIdentifierAccessor(this, bean) : new AbsentIdentifierAccessor(bean); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentEntity#isNew(java.lang.Object) */ @@ -481,7 +481,7 @@ public boolean isNew(Object bean) { return isNewStrategy.get().isNew(bean); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentEntity#isImmutable() */ @@ -490,7 +490,7 @@ public boolean isImmutable() { return isImmutable.get(); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentEntity#requiresPropertyPopulation() */ @@ -516,7 +516,7 @@ protected EvaluationContext getEvaluationContext(Object rootObject) { * Returns the default {@link IsNewStrategy} to be used. Will be a {@link PersistentEntityIsNewStrategy} by default. * Note, that this strategy only gets used if the entity doesn't implement {@link Persistable} as this indicates the * user wants to be in control over whether an entity is new or not. - * + * * @return * @since 2.1 */ @@ -526,7 +526,7 @@ protected IsNewStrategy getFallbackIsNewStrategy() { /** * Verifies the given bean type to no be {@literal null} and of the type of the current {@link PersistentEntity}. - * + * * @param bean must not be {@literal null}. */ private final void verifyBeanType(Object bean) { 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 88672ea24d..baeee9fcf4 100755 --- a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java @@ -138,24 +138,27 @@ public void considersComparatorForPropertyOrder() { assertThat(entity.getPersistentProperty("ssn")).isEqualTo(iterator.next()); } - @Test // DATACMNS-186 + @Test // DATACMNS-18, DATACMNS-1364 public void addingAndIdPropertySetsIdPropertyInternally() { MutablePersistentEntity entity = createEntity(Person.class); assertThat(entity.getIdProperty()).isNull(); + when(property.getName()).thenReturn("id"); when(property.isIdProperty()).thenReturn(true); entity.addPersistentProperty(property); assertThat(entity.getIdProperty()).isEqualTo(property); } - @Test // DATACMNS-186 + @Test // DATACMNS-186, DATACMNS-1364 public void rejectsIdPropertyIfAlreadySet() { MutablePersistentEntity entity = createEntity(Person.class); + when(property.getName()).thenReturn("id"); when(property.isIdProperty()).thenReturn(true); when(anotherProperty.isIdProperty()).thenReturn(true); + when(anotherProperty.getName()).thenReturn("another"); entity.addPersistentProperty(property); exception.expect(MappingException.class); @@ -429,7 +432,7 @@ static class PersistableEntity implements Persistable { private final Long id = 42L; - /* + /* * (non-Javadoc) * @see org.springframework.data.domain.Persistable#getId() */ @@ -438,7 +441,7 @@ public Long getId() { return 4711L; } - /* + /* * (non-Javadoc) * @see org.springframework.data.domain.Persistable#isNew() */