From f2f7886105542430ae4e2be744475a713316d585 Mon Sep 17 00:00:00 2001 From: Pavel Horal Date: Fri, 28 Feb 2020 16:42:41 +0100 Subject: [PATCH] DATACMNS-1676 - Create shared conversion service for AuditableBeanWrapper. Prevent performance penalty in creating conversion service each time a domain object is being persisted. Original pull request: #432. --- .../DefaultAuditableBeanWrapperFactory.java | 59 +++++++++++-------- .../MappingAuditableBeanWrapperFactory.java | 8 ++- ...eflectionAuditingBeanWrapperUnitTests.java | 8 ++- 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java index 79d938819..537977a4a 100644 --- a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java @@ -21,7 +21,6 @@ import lombok.RequiredArgsConstructor; import java.lang.reflect.Field; import java.time.Instant; import java.time.temporal.TemporalAccessor; -import java.util.Calendar; import java.util.Date; import java.util.Optional; import java.util.stream.Stream; @@ -47,6 +46,12 @@ import org.springframework.util.Assert; */ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory { + protected final ConversionService conversionService; + + public DefaultAuditableBeanWrapperFactory() { + this.conversionService = getDateConversionService(); + } + /** * Returns an {@link AuditableBeanWrapper} if the given object is capable of being equipped with auditing information. * @@ -61,25 +66,39 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return Optional.of(source).map(it -> { if (it instanceof Auditable) { - return (AuditableBeanWrapper) new AuditableInterfaceBeanWrapper((Auditable) it); + return (AuditableBeanWrapper) new AuditableInterfaceBeanWrapper(conversionService, (Auditable) it); } AnnotationAuditingMetadata metadata = AnnotationAuditingMetadata.getMetadata(it.getClass()); if (metadata.isAuditable()) { - return new ReflectionAuditingBeanWrapper(it); + return new ReflectionAuditingBeanWrapper(conversionService, it); } return null; }); } + /** + * Creates conversion service for conversions between {@link TemporalAccessor} values and other supported date type values. + * + * @return + */ + static ConversionService getDateConversionService() { + DefaultFormattingConversionService conversionService = new DefaultFormattingConversionService(); + + JodaTimeConverters.getConvertersToRegister().forEach(conversionService::addConverter); + Jsr310Converters.getConvertersToRegister().forEach(conversionService::addConverter); + ThreeTenBackPortConverters.getConvertersToRegister().forEach(conversionService::addConverter); + + return conversionService; + } + /** * An {@link AuditableBeanWrapper} that works with objects implementing * * @author Oliver Gierke */ - @RequiredArgsConstructor static class AuditableInterfaceBeanWrapper extends DateConvertingAuditableBeanWrapper> { @@ -87,7 +106,8 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory private final Class type; @SuppressWarnings("unchecked") - public AuditableInterfaceBeanWrapper(Auditable auditable) { + public AuditableInterfaceBeanWrapper(ConversionService conversionService, Auditable auditable) { + super(conversionService); this.auditable = auditable; this.type = (Class) ResolvableType.forClass(Auditable.class, auditable.getClass()) @@ -151,7 +171,7 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return value; } - /* + /* * (non-Javadoc) * @see org.springframework.data.auditing.AuditableBeanWrapper#getBean() */ @@ -162,32 +182,19 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory } /** - * Base class for {@link AuditableBeanWrapper} implementations that might need to convert {@link Calendar} values into + * Base class for {@link AuditableBeanWrapper} implementations that might need to convert {@link TemporalAccessor} values into * compatible types when setting date/time information. * * @author Oliver Gierke * @since 1.8 */ + @RequiredArgsConstructor abstract static class DateConvertingAuditableBeanWrapper implements AuditableBeanWrapper { private final ConversionService conversionService; /** - * Creates a new {@link DateConvertingAuditableBeanWrapper}. - */ - public DateConvertingAuditableBeanWrapper() { - - DefaultFormattingConversionService conversionService = new DefaultFormattingConversionService(); - - JodaTimeConverters.getConvertersToRegister().forEach(conversionService::addConverter); - Jsr310Converters.getConvertersToRegister().forEach(conversionService::addConverter); - ThreeTenBackPortConverters.getConvertersToRegister().forEach(conversionService::addConverter); - - this.conversionService = conversionService; - } - - /** - * Returns the {@link Calendar} in a type, compatible to the given field. + * Returns the {@link TemporalAccessor} in a type, compatible to the given field. * * @param value can be {@literal null}. * @param targetType must not be {@literal null}. @@ -221,7 +228,7 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory } /** - * Returns the given object as {@link Calendar}. + * Returns the given object as {@link TemporalAccessor}. * * @param source can be {@literal null}. * @param target must not be {@literal null}. @@ -266,9 +273,11 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory /** * Creates a new {@link ReflectionAuditingBeanWrapper} to set auditing data on the given target object. * + * @param conversionService conversion service for date value type conversions * @param target must not be {@literal null}. */ - public ReflectionAuditingBeanWrapper(T target) { + public ReflectionAuditingBeanWrapper(ConversionService conversionService, T target) { + super(conversionService); Assert.notNull(target, "Target object must not be null!"); @@ -328,7 +337,7 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return setDateField(metadata.getLastModifiedDateField(), value); } - /* + /* * (non-Javadoc) * @see org.springframework.data.auditing.AuditableBeanWrapper#getBean() */ diff --git a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java index 3595212d4..5e2c5da1e 100644 --- a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java @@ -22,6 +22,7 @@ import java.util.Map; import java.util.Optional; import java.util.function.Predicate; +import org.springframework.core.convert.ConversionService; import org.springframework.data.annotation.CreatedBy; import org.springframework.data.annotation.CreatedDate; import org.springframework.data.annotation.LastModifiedBy; @@ -87,7 +88,7 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap key -> new MappingAuditingMetadata(context, it.getClass())); return Optional.> ofNullable(metadata.isAuditable() // - ? new MappingMetadataAuditableBeanWrapper(entity.getPropertyPathAccessor(it), metadata) + ? new MappingMetadataAuditableBeanWrapper(conversionService, entity.getPropertyPathAccessor(it), metadata) : null); }).orElseGet(() -> super.getBeanWrapperFor(source)); @@ -177,8 +178,11 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap * @param accessor must not be {@literal null}. * @param metadata must not be {@literal null}. */ - public MappingMetadataAuditableBeanWrapper(PersistentPropertyPathAccessor accessor, + public MappingMetadataAuditableBeanWrapper( + ConversionService conversionService, + PersistentPropertyPathAccessor accessor, MappingAuditingMetadata metadata) { + super(conversionService); Assert.notNull(accessor, "PersistentPropertyAccessor must not be null!"); Assert.notNull(metadata, "Auditing metadata must not be null!"); diff --git a/src/test/java/org/springframework/data/auditing/ReflectionAuditingBeanWrapperUnitTests.java b/src/test/java/org/springframework/data/auditing/ReflectionAuditingBeanWrapperUnitTests.java index 7da060dfb..5c2ed8af6 100755 --- a/src/test/java/org/springframework/data/auditing/ReflectionAuditingBeanWrapperUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/ReflectionAuditingBeanWrapperUnitTests.java @@ -23,6 +23,7 @@ import java.time.ZoneOffset; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.core.convert.ConversionService; import org.springframework.data.annotation.CreatedDate; import org.springframework.data.annotation.LastModifiedDate; import org.springframework.data.auditing.DefaultAuditableBeanWrapperFactory.ReflectionAuditingBeanWrapper; @@ -36,6 +37,7 @@ import org.springframework.data.convert.Jsr310Converters.LocalDateTimeToDateConv */ class ReflectionAuditingBeanWrapperUnitTests { + ConversionService conversionService; AnnotationAuditingMetadata metadata; AnnotatedUser user; AuditableBeanWrapper wrapper; @@ -44,9 +46,9 @@ class ReflectionAuditingBeanWrapperUnitTests { @BeforeEach void setUp() { - + this.conversionService = DefaultAuditableBeanWrapperFactory.getDateConversionService(); this.user = new AnnotatedUser(); - this.wrapper = new ReflectionAuditingBeanWrapper(user); + this.wrapper = new ReflectionAuditingBeanWrapper(conversionService, user); } @Test @@ -74,7 +76,7 @@ class ReflectionAuditingBeanWrapperUnitTests { } Sample sample = new Sample(); - AuditableBeanWrapper wrapper = new ReflectionAuditingBeanWrapper(sample); + AuditableBeanWrapper wrapper = new ReflectionAuditingBeanWrapper(conversionService, sample); wrapper.setCreatedDate(time); assertThat(sample.createdDate).isEqualTo(time.atZone(ZoneOffset.systemDefault()).toInstant().toEpochMilli());