From defc3c787413576cc6c49b42e804dddbb91ad3c0 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 3 May 2016 09:27:21 +0200 Subject: [PATCH] DATACMNS-809 - Refactor BasicPersistentEntity to set PersistentPropertyAccessorFactory. PersistentPropertyAccessorFactory can be set on a BasicPersistentEntity. This is done by AbstractMappingContext once the entity is verified. This change reduces the count of PersistentPropertyAccessorFactory instances and performs the isSupported check only once, when initializing the entity. Original pull request: #159. --- .../context/AbstractMappingContext.java | 54 ++++++++++--------- .../mapping/model/BasicPersistentEntity.java | 26 +++++++-- ...lassGeneratingPropertyAccessorFactory.java | 11 ++++ ...aultPersistentPropertyAccessorFactory.java | 12 +++-- .../model/MutablePersistentEntity.java | 22 +++++--- .../PersistentPropertyAccessorFactory.java | 12 ++++- ...gPropertyAccessorFactoryDatatypeTests.java | 14 ++++- ...eneratingPropertyAccessorFactoryTests.java | 14 ++++- 8 files changed, 123 insertions(+), 42 deletions(-) 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 41688422a..be09a24f6 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -37,6 +37,7 @@ 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.MappingException; import org.springframework.data.mapping.model.MutablePersistentEntity; import org.springframework.data.mapping.model.SimpleTypeHolder; @@ -54,7 +55,7 @@ import org.springframework.util.StringUtils; *

* The implementation uses a {@link ReentrantReadWriteLock} to make sure {@link PersistentEntity} are completely * populated before accessing them from outside. - * + * * @param E the concrete {@link PersistentEntity} type the {@link MappingContext} implementation creates * @param P the concrete {@link PersistentProperty} type the {@link MappingContext} implementation creates * @author Jon Brisbin @@ -62,11 +63,13 @@ import org.springframework.util.StringUtils; * @author Michael Hunger * @author Thomas Darimont * @author Tomasz Wysocki + * @author Mark Paluch */ public abstract class AbstractMappingContext, P extends PersistentProperty

> implements MappingContext, ApplicationEventPublisherAware, InitializingBean { private final Map, E> persistentEntities = new HashMap, E>(); + private final DefaultPersistentPropertyAccessorFactory persistentPropertyAccessorFactory = new DefaultPersistentPropertyAccessorFactory(); private ApplicationEventPublisher applicationEventPublisher; @@ -78,7 +81,7 @@ public abstract class AbstractMappingContext> initialEntitySet) { @@ -100,7 +103,7 @@ public abstract class AbstractMappingContext * @param typeInformation * @return @@ -363,7 +369,7 @@ public abstract class AbstractMappingContext> implement private final Map propertyCache; private final Map, Annotation> annotationCache; - private final PersistentPropertyAccessorFactory propertyAccessorFactory; private P idProperty; private P versionProperty; + private PersistentPropertyAccessorFactory propertyAccessorFactory; + /** * Creates a new {@link BasicPersistentEntity} from the given {@link TypeInformation}. * @@ -101,7 +102,6 @@ public class BasicPersistentEntity> implement this.propertyCache = new HashMap(); this.annotationCache = new HashMap, Annotation>(); - this.propertyAccessorFactory = new DefaultPersistentPropertyAccessorFactory(); } /* @@ -391,16 +391,32 @@ public class BasicPersistentEntity> implement } } - /* - * (non-Javadoc) - * @see org.springframework.data.mapping.PersistentEntity#getPropertyAccessor(java.lang.Object) + /* (non-Javadoc) + * @see org.springframework.data.mapping.model.MutablePersistentEntity#setPersistentPropertyAccessorFactory(org.springframework.data.mapping.model.PersistentPropertyAccessorFactory) */ @Override + public void setPersistentPropertyAccessorFactory(PersistentPropertyAccessorFactory factory) { + this.propertyAccessorFactory = factory; + } + + public PersistentPropertyAccessorFactory getPropertyAccessorFactory() { + return propertyAccessorFactory; + } + + /* + * (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/ClassGeneratingPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java index e0fbe3b7d..374bc4f24 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -47,6 +47,7 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.SimpleAssociationHandler; import org.springframework.data.mapping.SimplePropertyHandler; import org.springframework.data.util.TypeInformation; +import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; /** @@ -85,6 +86,16 @@ 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. * diff --git a/src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java index baca16172..b0588f957 100644 --- a/src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java @@ -21,7 +21,7 @@ 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 * @author Oliver Gierke * @since 1.13 @@ -37,7 +37,13 @@ public class DefaultPersistentPropertyAccessorFactory implements PersistentPrope @Override public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity entity, Object bean) { - return ClassGeneratingPropertyAccessorFactory.canGenerateAccessorClass(entity) - ? classGeneratingPropertyAccessorFactory.getPropertyAccessor(entity, bean) : new BeanWrapper(bean); + return classGeneratingPropertyAccessorFactory.getPropertyAccessor(entity, bean); + } + + /* (non-Javadoc) + * @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#isSupported(org.springframework.data.mapping.PersistentEntity) + */ + public boolean isSupported(PersistentEntity entity) { + return classGeneratingPropertyAccessorFactory.isSupported(entity); } } 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 283a43eff..ec89fad83 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 by the original author(s). + * Copyright (c) 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. @@ -18,24 +18,26 @@ package org.springframework.data.mapping.model; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.mapping.PersistentPropertyAccessor; /** * Interface capturing mutator methods for {@link PersistentEntity}s. - * + * * @author Oliver Gierke + * @author Mark Paluch */ public interface MutablePersistentEntity> extends PersistentEntity { /** * Adds a {@link PersistentProperty} to the entity. - * + * * @param property */ void addPersistentProperty(P property); /** * Adds an {@link Association} to the entity. - * + * * @param association */ void addAssociation(Association

association); @@ -43,8 +45,16 @@ public interface MutablePersistentEntity> ext /** * Callback method to trigger validation of the {@link PersistentEntity}. As {@link MutablePersistentEntity} is not * immutable there might be some verification steps necessary after the object has reached is final state. - * + * * @throws MappingException in case the entity is invalid */ void verify() throws MappingException; -} \ No newline at end of file + + /** + * Sets the {@link PersistentPropertyAccessorFactory} for the entity. A {@link PersistentPropertyAccessorFactory} + * creates {@link PersistentPropertyAccessor}s for instances of this entity. + * + * @param factory + */ + void setPersistentPropertyAccessorFactory(PersistentPropertyAccessorFactory factory); +} diff --git a/src/main/java/org/springframework/data/mapping/model/PersistentPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/PersistentPropertyAccessorFactory.java index b68ad0dc4..d28eeb38a 100644 --- a/src/main/java/org/springframework/data/mapping/model/PersistentPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/PersistentPropertyAccessorFactory.java @@ -20,7 +20,7 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; /** * Factory to create {@link PersistentPropertyAccessor} for a given {@link PersistentEntity} and bean instance. - * + * * @author Mark Paluch * @author Oliver Gierke * @since 1.13 @@ -29,10 +29,18 @@ public interface PersistentPropertyAccessorFactory { /** * Returns a {@link PersistentPropertyAccessor} for a given {@link PersistentEntity} and {@code bean}. - * + * * @param entity must not be {@literal null}. * @param bean must not be {@literal null}. * @return will never be {@literal null}. */ PersistentPropertyAccessor getPropertyAccessor(PersistentEntity entity, Object bean); + + /** + * Returns whether given {@link PersistentEntity} is supported by this {@link PersistentPropertyAccessorFactory}. + * + * @param entity must not be {@literal null}. + * @return + */ + boolean isSupported(PersistentEntity entity); } 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 31d2eeda4..925272da1 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,7 @@ */ package org.springframework.data.mapping.model; -import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; @@ -131,6 +131,18 @@ public class ClassGeneratingPropertyAccessorFactoryDatatypeTests { assertThat(persistentPropertyAccessor.getProperty(property), is(equalTo((Object) value))); } + /** + * @see DATACMNS-809 + */ + @Test + public void shouldUseClassPropertyAccessorFactory() throws Exception { + + BasicPersistentEntity persistentEntity = mappingContext + .getPersistentEntity(bean.getClass()); + + assertThat(persistentEntity.getPropertyAccessorFactory(), is(instanceOf(DefaultPersistentPropertyAccessorFactory.class))); + } + private PersistentPropertyAccessor getPersistentPropertyAccessor(Object bean) { return factory.getPropertyAccessor(mappingContext.getPersistentEntity(bean.getClass()), 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 aa59226b9..f38a6b33b 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,7 @@ package org.springframework.data.mapping.model; -import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; @@ -156,6 +156,18 @@ public class ClassGeneratingPropertyAccessorFactoryTests { persistentPropertyAccessor.setProperty(property, null); } + /** + * @see DATACMNS-809 + */ + @Test + public void shouldUseClassPropertyAccessorFactory() throws Exception { + + BasicPersistentEntity persistentEntity = mappingContext + .getPersistentEntity(bean.getClass()); + + assertThat(persistentEntity.getPropertyAccessorFactory(), is(instanceOf(DefaultPersistentPropertyAccessorFactory.class))); + } + private PersistentPropertyAccessor getPersistentPropertyAccessor(Object bean) { return factory.getPropertyAccessor(mappingContext.getPersistentEntity(bean.getClass()), bean); }