Skip to content

Commit 38a4113

Browse files
odrotbohmodrotbohm
committed
DATACMNS-809 - Polishing.
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.
1 parent defc3c7 commit 38a4113

File tree

7 files changed

+41
-46
lines changed

7 files changed

+41
-46
lines changed

‎src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,10 @@
3737
import org.springframework.data.mapping.PersistentEntity;
3838
import org.springframework.data.mapping.PersistentProperty;
3939
import org.springframework.data.mapping.PropertyPath;
40-
import org.springframework.data.mapping.model.DefaultPersistentPropertyAccessorFactory;
40+
import org.springframework.data.mapping.model.ClassGeneratingPropertyAccessorFactory;
4141
import org.springframework.data.mapping.model.MappingException;
4242
import org.springframework.data.mapping.model.MutablePersistentEntity;
43+
import org.springframework.data.mapping.model.PersistentPropertyAccessorFactory;
4344
import org.springframework.data.mapping.model.SimpleTypeHolder;
4445
import org.springframework.data.util.ClassTypeInformation;
4546
import org.springframework.data.util.TypeInformation;
@@ -69,7 +70,7 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
6970
implements MappingContext<E, P>, ApplicationEventPublisherAware, InitializingBean {
7071

7172
private final Map<TypeInformation<?>, E> persistentEntities = new HashMap<TypeInformation<?>, E>();
72-
private final DefaultPersistentPropertyAccessorFactory persistentPropertyAccessorFactory = new DefaultPersistentPropertyAccessorFactory();
73+
private final PersistentPropertyAccessorFactory persistentPropertyAccessorFactory = new ClassGeneratingPropertyAccessorFactory();
7374

7475
private ApplicationEventPublisher applicationEventPublisher;
7576

@@ -126,6 +127,7 @@ public void setSimpleTypeHolder(SimpleTypeHolder simpleTypes) {
126127
* @see org.springframework.data.mapping.model.MappingContext#getPersistentEntities()
127128
*/
128129
public Collection<E> getPersistentEntities() {
130+
129131
try {
130132
read.lock();
131133
return Collections.unmodifiableSet(new HashSet<E>(persistentEntities.values()));
@@ -322,6 +324,7 @@ protected E addPersistentEntity(TypeInformation<?> typeInformation) {
322324
if (persistentPropertyAccessorFactory.isSupported(entity)) {
323325
entity.setPersistentPropertyAccessorFactory(persistentPropertyAccessorFactory);
324326
}
327+
325328
} catch (MappingException e) {
326329
persistentEntities.remove(typeInformation);
327330
throw e;

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ public class BasicPersistentEntity<T, P extends PersistentProperty<P>> implement
6969

7070
private P idProperty;
7171
private P versionProperty;
72-
7372
private PersistentPropertyAccessorFactory propertyAccessorFactory;
7473

7574
/**
@@ -102,6 +101,7 @@ public BasicPersistentEntity(TypeInformation<T> information, Comparator<P> compa
102101

103102
this.propertyCache = new HashMap<String, P>();
104103
this.annotationCache = new HashMap<Class<? extends Annotation>, Annotation>();
104+
this.propertyAccessorFactory = BeanWrapperPropertyAccessorFactory.INSTANCE;
105105
}
106106

107107
/*
@@ -391,32 +391,25 @@ public void verify() {
391391
}
392392
}
393393

394-
/* (non-Javadoc)
394+
/*
395+
* (non-Javadoc)
395396
* @see org.springframework.data.mapping.model.MutablePersistentEntity#setPersistentPropertyAccessorFactory(org.springframework.data.mapping.model.PersistentPropertyAccessorFactory)
396397
*/
397398
@Override
398399
public void setPersistentPropertyAccessorFactory(PersistentPropertyAccessorFactory factory) {
399400
this.propertyAccessorFactory = factory;
400401
}
401402

402-
public PersistentPropertyAccessorFactory getPropertyAccessorFactory() {
403-
return propertyAccessorFactory;
404-
}
405-
406403
/*
407-
* (non-Javadoc)
408-
* @see org.springframework.data.mapping.PersistentEntity#getPropertyAccessor(java.lang.Object)
409-
*/
404+
* (non-Javadoc)
405+
* @see org.springframework.data.mapping.PersistentEntity#getPropertyAccessor(java.lang.Object)
406+
*/
410407
@Override
411408
public PersistentPropertyAccessor getPropertyAccessor(Object bean) {
412409

413410
Assert.notNull(bean, "Target bean must not be null!");
414411
Assert.isTrue(getType().isInstance(bean), "Target bean is not of type of the persistent entity!");
415412

416-
if (propertyAccessorFactory == null) {
417-
return new BeanWrapper<Object>(bean);
418-
}
419-
420413
return propertyAccessorFactory.getPropertyAccessor(this, bean);
421414
}
422415

‎src/main/java/org/springframework/data/mapping/model/DefaultPersistentPropertyAccessorFactory.java renamed to ‎src/main/java/org/springframework/data/mapping/model/BeanWrapperPropertyAccessorFactory.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,31 +19,29 @@
1919
import org.springframework.data.mapping.PersistentPropertyAccessor;
2020

2121
/**
22-
* Default implementation of {@link PersistentPropertyAccessorFactory}. Accessors can access bean properties either via
23-
* reflection or use generated classes with direct field/method access.
24-
*
25-
* @author Mark Paluch
22+
* PersistentPropertyAccessorFactory that uses a {@link BeanWrapper}.
23+
*
2624
* @author Oliver Gierke
27-
* @since 1.13
2825
*/
29-
public class DefaultPersistentPropertyAccessorFactory implements PersistentPropertyAccessorFactory {
26+
enum BeanWrapperPropertyAccessorFactory implements PersistentPropertyAccessorFactory {
3027

31-
private final ClassGeneratingPropertyAccessorFactory classGeneratingPropertyAccessorFactory = new ClassGeneratingPropertyAccessorFactory();
28+
INSTANCE;
3229

33-
/*
30+
/*
3431
* (non-Javadoc)
3532
* @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#getPropertyAccessor(org.springframework.data.mapping.PersistentEntity, java.lang.Object)
3633
*/
3734
@Override
3835
public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity<?, ?> entity, Object bean) {
39-
40-
return classGeneratingPropertyAccessorFactory.getPropertyAccessor(entity, bean);
36+
return new BeanWrapper<Object>(bean);
4137
}
4238

43-
/* (non-Javadoc)
39+
/*
40+
* (non-Javadoc)
4441
* @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#isSupported(org.springframework.data.mapping.PersistentEntity)
4542
*/
43+
@Override
4644
public boolean isSupported(PersistentEntity<?, ?> entity) {
47-
return classGeneratingPropertyAccessorFactory.isSupported(entity);
45+
return true;
4846
}
4947
}

‎src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,16 @@
5959
* @author Oliver Gierke
6060
* @since 1.13
6161
*/
62-
class ClassGeneratingPropertyAccessorFactory implements PersistentPropertyAccessorFactory {
62+
public class ClassGeneratingPropertyAccessorFactory implements PersistentPropertyAccessorFactory {
6363

6464
private static final boolean IS_JAVA_7_OR_BETTER = org.springframework.util.ClassUtils
6565
.isPresent("java.lang.invoke.MethodHandle", ClassGeneratingPropertyAccessorFactory.class.getClassLoader());
6666

6767
private volatile Map<TypeInformation<?>, Class<PersistentPropertyAccessor>> propertyAccessorClasses = new HashMap<TypeInformation<?>, Class<PersistentPropertyAccessor>>(
6868
32);
6969

70-
/* (non-Javadoc)
70+
/*
71+
* (non-Javadoc)
7172
* @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#getPropertyAccessor(org.springframework.data.mapping.PersistentEntity, java.lang.Object)
7273
*/
7374
@Override
@@ -86,24 +87,18 @@ public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity<?, ?> ent
8687
}
8788
}
8889

89-
/* (non-Javadoc)
90-
* @see org.springframework.data.mapping.model.PersistentPropertyAccessorFactory#isSupported(org.springframework.data.mapping.PersistentEntity)
91-
*/
92-
@Override
93-
public boolean isSupported(PersistentEntity<?, ?> entity) {
94-
95-
Assert.notNull(entity, "PersistentEntity must not be null!");
96-
return canGenerateAccessorClass(entity);
97-
}
98-
9990
/**
10091
* Checks whether an accessor class can be generated.
10192
*
10293
* @param entity
10394
* @return {@literal true} if the runtime is equal or greater to Java 1.7, property name hash codes are unique and the
10495
* type has a class loader we can use to re-inject types.
96+
* @see PersistentPropertyAccessorFactory#isSupported(PersistentEntity)
10597
*/
106-
public static boolean canGenerateAccessorClass(PersistentEntity<?, ?> entity) {
98+
@Override
99+
public boolean isSupported(PersistentEntity<?, ?> entity) {
100+
101+
Assert.notNull(entity, "PersistentEntity must not be null!");
107102

108103
if (!IS_JAVA_7_OR_BETTER) {
109104
return false;

‎src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011-2016 by the original author(s).
2+
* Copyright 2011-2016 by the original author(s).
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -54,7 +54,7 @@ public interface MutablePersistentEntity<T, P extends PersistentProperty<P>> ext
5454
* Sets the {@link PersistentPropertyAccessorFactory} for the entity. A {@link PersistentPropertyAccessorFactory}
5555
* creates {@link PersistentPropertyAccessor}s for instances of this entity.
5656
*
57-
* @param factory
57+
* @param factory must not be {@literal null}.
5858
*/
5959
void setPersistentPropertyAccessorFactory(PersistentPropertyAccessorFactory factory);
6060
}

‎src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
*/
1616
package org.springframework.data.mapping.model;
1717

18-
import static org.hamcrest.CoreMatchers.*;
18+
import static org.hamcrest.CoreMatchers.instanceOf;
19+
import static org.hamcrest.CoreMatchers.is;
1920
import static org.hamcrest.Matchers.equalTo;
2021
import static org.junit.Assert.*;
2122

@@ -35,6 +36,7 @@
3536
import org.springframework.data.mapping.PersistentPropertyAccessor;
3637
import org.springframework.data.mapping.context.SampleMappingContext;
3738
import org.springframework.data.mapping.context.SamplePersistentProperty;
39+
import org.springframework.test.util.ReflectionTestUtils;
3840

3941
/**
4042
* Unit tests for {@link ClassGeneratingPropertyAccessorFactory}
@@ -140,7 +142,8 @@ public void shouldUseClassPropertyAccessorFactory() throws Exception {
140142
BasicPersistentEntity<Object, SamplePersistentProperty> persistentEntity = mappingContext
141143
.getPersistentEntity(bean.getClass());
142144

143-
assertThat(persistentEntity.getPropertyAccessorFactory(), is(instanceOf(DefaultPersistentPropertyAccessorFactory.class)));
145+
assertThat(ReflectionTestUtils.getField(persistentEntity, "propertyAccessorFactory"),
146+
is(instanceOf(ClassGeneratingPropertyAccessorFactory.class)));
144147
}
145148

146149
private PersistentPropertyAccessor getPersistentPropertyAccessor(Object bean) {

‎src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryTests.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616

1717
package org.springframework.data.mapping.model;
1818

19-
import static org.hamcrest.CoreMatchers.*;
19+
import static org.hamcrest.CoreMatchers.instanceOf;
20+
import static org.hamcrest.CoreMatchers.is;
2021
import static org.hamcrest.Matchers.equalTo;
2122
import static org.junit.Assert.*;
2223

@@ -36,6 +37,7 @@
3637
import org.springframework.data.mapping.context.SampleMappingContext;
3738
import org.springframework.data.mapping.context.SamplePersistentProperty;
3839
import org.springframework.data.mapping.model.subpackage.TypeInOtherPackage;
40+
import org.springframework.test.util.ReflectionTestUtils;
3941

4042
/**
4143
* Unit tests for {@link ClassGeneratingPropertyAccessorFactory}
@@ -165,7 +167,8 @@ public void shouldUseClassPropertyAccessorFactory() throws Exception {
165167
BasicPersistentEntity<Object, SamplePersistentProperty> persistentEntity = mappingContext
166168
.getPersistentEntity(bean.getClass());
167169

168-
assertThat(persistentEntity.getPropertyAccessorFactory(), is(instanceOf(DefaultPersistentPropertyAccessorFactory.class)));
170+
assertThat(ReflectionTestUtils.getField(persistentEntity, "propertyAccessorFactory"),
171+
is(instanceOf(ClassGeneratingPropertyAccessorFactory.class)));
169172
}
170173

171174
private PersistentPropertyAccessor getPersistentPropertyAccessor(Object bean) {

0 commit comments

Comments
 (0)