From 38a41139fcd08546e8ae528a22ee35614a085ba6 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 3 May 2016 16:44:38 +0200 Subject: [PATCH] DATACMNS-809 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed DefaultPersistentPropertyAccessorFactory as it only delegates to the class generating one. Tweaked AbstractMappingContext to actually use the latter in the first place. Introduced BeanWrapperPropertyAccessorFactory to implement the default behavior of using a BeanWrapper and initialize BasicPersistentEntity to avoid a null clause in getPersistentPropertyAccessor(…). Removed getPersistentPropertyAccessorFactory and rather rely on ReflectionTestUtils in tests to avoid additional API being exposed. Original pull request: #159. --- .../context/AbstractMappingContext.java | 7 ++++-- .../mapping/model/BasicPersistentEntity.java | 19 +++++----------- ...> BeanWrapperPropertyAccessorFactory.java} | 22 +++++++++---------- ...lassGeneratingPropertyAccessorFactory.java | 21 +++++++----------- .../model/MutablePersistentEntity.java | 4 ++-- ...gPropertyAccessorFactoryDatatypeTests.java | 7 ++++-- ...eneratingPropertyAccessorFactoryTests.java | 7 ++++-- 7 files changed, 41 insertions(+), 46 deletions(-) rename src/main/java/org/springframework/data/mapping/model/{DefaultPersistentPropertyAccessorFactory.java => BeanWrapperPropertyAccessorFactory.java} (67%) 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 be09a24f6..a74a7224b 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -37,9 +37,10 @@ import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PropertyPath; -import org.springframework.data.mapping.model.DefaultPersistentPropertyAccessorFactory; +import org.springframework.data.mapping.model.ClassGeneratingPropertyAccessorFactory; import org.springframework.data.mapping.model.MappingException; import org.springframework.data.mapping.model.MutablePersistentEntity; +import org.springframework.data.mapping.model.PersistentPropertyAccessorFactory; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; @@ -69,7 +70,7 @@ public abstract class AbstractMappingContext, ApplicationEventPublisherAware, InitializingBean { private final Map, E> persistentEntities = new HashMap, E>(); - private final DefaultPersistentPropertyAccessorFactory persistentPropertyAccessorFactory = new DefaultPersistentPropertyAccessorFactory(); + private final PersistentPropertyAccessorFactory persistentPropertyAccessorFactory = new ClassGeneratingPropertyAccessorFactory(); private ApplicationEventPublisher applicationEventPublisher; @@ -126,6 +127,7 @@ public abstract class AbstractMappingContext getPersistentEntities() { + try { read.lock(); return Collections.unmodifiableSet(new HashSet(persistentEntities.values())); @@ -322,6 +324,7 @@ public abstract class AbstractMappingContext> implement private P idProperty; private P versionProperty; - private PersistentPropertyAccessorFactory propertyAccessorFactory; /** @@ -102,6 +101,7 @@ public class BasicPersistentEntity> implement this.propertyCache = new HashMap(); this.annotationCache = new HashMap, Annotation>(); + this.propertyAccessorFactory = BeanWrapperPropertyAccessorFactory.INSTANCE; } /* @@ -391,7 +391,8 @@ public class BasicPersistentEntity> implement } } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.mapping.model.MutablePersistentEntity#setPersistentPropertyAccessorFactory(org.springframework.data.mapping.model.PersistentPropertyAccessorFactory) */ @Override @@ -399,24 +400,16 @@ public class BasicPersistentEntity> implement this.propertyAccessorFactory = factory; } - public PersistentPropertyAccessorFactory getPropertyAccessorFactory() { - return propertyAccessorFactory; - } - /* - * (non-Javadoc) - * @see org.springframework.data.mapping.PersistentEntity#getPropertyAccessor(java.lang.Object) - */ + * (non-Javadoc) + * @see org.springframework.data.mapping.PersistentEntity#getPropertyAccessor(java.lang.Object) + */ @Override public PersistentPropertyAccessor getPropertyAccessor(Object bean) { Assert.notNull(bean, "Target bean must not be null!"); Assert.isTrue(getType().isInstance(bean), "Target bean is not of type of the persistent entity!"); - if (propertyAccessorFactory == null) { - return new BeanWrapper(bean); - } - return propertyAccessorFactory.getPropertyAccessor(this, bean); } diff --git a/src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/BeanWrapperPropertyAccessorFactory.java similarity index 67% rename from src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java rename to src/main/java/org/springframework/data/mapping/model/BeanWrapperPropertyAccessorFactory.java index b0588f957..aa311bfc6 100644 --- a/src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/BeanWrapperPropertyAccessorFactory.java @@ -19,31 +19,29 @@ import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentPropertyAccessor; /** - * Default implementation of {@link PersistentPropertyAccessorFactory}. Accessors can access bean properties either via - * reflection or use generated classes with direct field/method access. - * - * @author Mark Paluch + * PersistentPropertyAccessorFactory that uses a {@link BeanWrapper}. + * * @author Oliver Gierke - * @since 1.13 */ -public class DefaultPersistentPropertyAccessorFactory implements PersistentPropertyAccessorFactory { +enum BeanWrapperPropertyAccessorFactory implements PersistentPropertyAccessorFactory { - private final ClassGeneratingPropertyAccessorFactory classGeneratingPropertyAccessorFactory = new ClassGeneratingPropertyAccessorFactory(); + INSTANCE; - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#getPropertyAccessor(org.springframework.data.mapping.PersistentEntity, java.lang.Object) */ @Override public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity entity, Object bean) { - - return classGeneratingPropertyAccessorFactory.getPropertyAccessor(entity, bean); + return new BeanWrapper(bean); } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#isSupported(org.springframework.data.mapping.PersistentEntity) */ + @Override public boolean isSupported(PersistentEntity entity) { - return classGeneratingPropertyAccessorFactory.isSupported(entity); + return true; } } diff --git a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java index 374bc4f24..10c590a04 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -59,7 +59,7 @@ import org.springframework.util.ReflectionUtils; * @author Oliver Gierke * @since 1.13 */ -class ClassGeneratingPropertyAccessorFactory implements PersistentPropertyAccessorFactory { +public class ClassGeneratingPropertyAccessorFactory implements PersistentPropertyAccessorFactory { private static final boolean IS_JAVA_7_OR_BETTER = org.springframework.util.ClassUtils .isPresent("java.lang.invoke.MethodHandle", ClassGeneratingPropertyAccessorFactory.class.getClassLoader()); @@ -67,7 +67,8 @@ class ClassGeneratingPropertyAccessorFactory implements PersistentPropertyAccess private volatile Map, Class> propertyAccessorClasses = new HashMap, Class>( 32); - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#getPropertyAccessor(org.springframework.data.mapping.PersistentEntity, java.lang.Object) */ @Override @@ -86,24 +87,18 @@ class ClassGeneratingPropertyAccessorFactory implements PersistentPropertyAccess } } - /* (non-Javadoc) - * @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#isSupported(org.springframework.data.mapping.PersistentEntity) - */ - @Override - public boolean isSupported(PersistentEntity entity) { - - Assert.notNull(entity, "PersistentEntity must not be null!"); - return canGenerateAccessorClass(entity); - } - /** * Checks whether an accessor class can be generated. * * @param entity * @return {@literal true} if the runtime is equal or greater to Java 1.7, property name hash codes are unique and the * type has a class loader we can use to re-inject types. + * @see PersistentPropertyAccessorFactory#isSupported(PersistentEntity) */ - public static boolean canGenerateAccessorClass(PersistentEntity entity) { + @Override + public boolean isSupported(PersistentEntity entity) { + + Assert.notNull(entity, "PersistentEntity must not be null!"); if (!IS_JAVA_7_OR_BETTER) { return false; diff --git a/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java b/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java index ec89fad83..f7073cc9d 100644 --- a/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2016 by the original author(s). + * Copyright 2011-2016 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. @@ -54,7 +54,7 @@ public interface MutablePersistentEntity> ext * Sets the {@link PersistentPropertyAccessorFactory} for the entity. A {@link PersistentPropertyAccessorFactory} * creates {@link PersistentPropertyAccessor}s for instances of this entity. * - * @param factory + * @param factory must not be {@literal null}. */ void setPersistentPropertyAccessorFactory(PersistentPropertyAccessorFactory factory); } diff --git a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java index 925272da1..018477770 100644 --- a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java +++ b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java @@ -15,7 +15,8 @@ */ package org.springframework.data.mapping.model; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; @@ -35,6 +36,7 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.context.SampleMappingContext; import org.springframework.data.mapping.context.SamplePersistentProperty; +import org.springframework.test.util.ReflectionTestUtils; /** * Unit tests for {@link ClassGeneratingPropertyAccessorFactory} @@ -140,7 +142,8 @@ public class ClassGeneratingPropertyAccessorFactoryDatatypeTests { BasicPersistentEntity persistentEntity = mappingContext .getPersistentEntity(bean.getClass()); - assertThat(persistentEntity.getPropertyAccessorFactory(), is(instanceOf(DefaultPersistentPropertyAccessorFactory.class))); + assertThat(ReflectionTestUtils.getField(persistentEntity, "propertyAccessorFactory"), + is(instanceOf(ClassGeneratingPropertyAccessorFactory.class))); } private PersistentPropertyAccessor getPersistentPropertyAccessor(Object bean) { diff --git a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryTests.java b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryTests.java index f38a6b33b..61ffed0c8 100644 --- a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryTests.java +++ b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryTests.java @@ -16,7 +16,8 @@ package org.springframework.data.mapping.model; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; @@ -36,6 +37,7 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.context.SampleMappingContext; import org.springframework.data.mapping.context.SamplePersistentProperty; import org.springframework.data.mapping.model.subpackage.TypeInOtherPackage; +import org.springframework.test.util.ReflectionTestUtils; /** * Unit tests for {@link ClassGeneratingPropertyAccessorFactory} @@ -165,7 +167,8 @@ public class ClassGeneratingPropertyAccessorFactoryTests { BasicPersistentEntity persistentEntity = mappingContext .getPersistentEntity(bean.getClass()); - assertThat(persistentEntity.getPropertyAccessorFactory(), is(instanceOf(DefaultPersistentPropertyAccessorFactory.class))); + assertThat(ReflectionTestUtils.getField(persistentEntity, "propertyAccessorFactory"), + is(instanceOf(ClassGeneratingPropertyAccessorFactory.class))); } private PersistentPropertyAccessor getPersistentPropertyAccessor(Object bean) {