Skip to content

Introduce support to pass-thru TemporalAccessor auditing values #2874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-commons</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.0-GH-2719-SNAPSHOT</version>

<name>Spring Data Core</name>
<description>Core Spring concepts underpinning every Spring Data module.</description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.springframework.data.auditing;

import java.lang.reflect.Field;
import java.time.temporal.TemporalAccessor;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -58,7 +59,12 @@ final class AnnotationAuditingMetadata {

static {

List<String> types = new ArrayList<>(3);
List<String> types = new ArrayList<>(Jsr310Converters.getSupportedClasses() //
.stream() //
.filter(TemporalAccessor.class::isAssignableFrom) //
.map(Class::getName) //
.toList());

types.add(Date.class.getName());
types.add(Long.class.getName());
types.add(long.class.getName());
Expand Down Expand Up @@ -104,7 +110,7 @@ private void assertValidDateFieldType(Optional<Field> field) {

Class<?> type = it.getType();

if (Jsr310Converters.supports(type)) {
if (TemporalAccessor.class.isAssignableFrom(type)) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ public <T> Optional<AuditableBeanWrapper<T>> getBeanWrapperFor(T source) {
return Optional.of(source).map(it -> {

if (it instanceof Auditable) {
return (AuditableBeanWrapper<T>) new AuditableInterfaceBeanWrapper(conversionService, (Auditable<Object, ?, TemporalAccessor>) it);
return (AuditableBeanWrapper<T>) new AuditableInterfaceBeanWrapper(conversionService,
(Auditable<Object, ?, TemporalAccessor>) it);
}

AnnotationAuditingMetadata metadata = AnnotationAuditingMetadata.getMetadata(it.getClass());
Expand All @@ -98,7 +99,8 @@ static class AuditableInterfaceBeanWrapper
private final Class<? extends TemporalAccessor> type;

@SuppressWarnings("unchecked")
public AuditableInterfaceBeanWrapper(ConversionService conversionService, Auditable<Object, ?, TemporalAccessor> auditable) {
public AuditableInterfaceBeanWrapper(ConversionService conversionService,
Auditable<Object, ?, TemporalAccessor> auditable) {

super(conversionService);

Expand Down Expand Up @@ -151,8 +153,8 @@ public TemporalAccessor setLastModifiedDate(TemporalAccessor value) {
}

/**
* Base class for {@link AuditableBeanWrapper} implementations that might need to convert {@link TemporalAccessor} values into
* compatible types when setting date/time information.
* 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
Expand All @@ -168,15 +170,15 @@ abstract static class DateConvertingAuditableBeanWrapper<T> implements Auditable
/**
* Returns the {@link TemporalAccessor} in a type, compatible to the given field.
*
* @param value can be {@literal null}.
* @param value must not be {@literal null}.
* @param targetType must not be {@literal null}.
* @param source must not be {@literal null}.
* @return
*/
@Nullable
protected Object getDateValueToSet(TemporalAccessor value, Class<?> targetType, Object source) {

if (TemporalAccessor.class.equals(targetType)) {
if (targetType.isInstance(value)) {
return value;
}

Expand All @@ -188,15 +190,15 @@ protected Object getDateValueToSet(TemporalAccessor value, Class<?> targetType,

if (!conversionService.canConvert(value.getClass(), Date.class)) {
throw new IllegalArgumentException(
String.format("Cannot convert date type for member %s; From %s to java.util.Date to %s", source,
String.format("Cannot convert date type for %s; From %s to java.util.Date to %s", source,
value.getClass(), targetType));
}

Date date = conversionService.convert(value, Date.class);
return conversionService.convert(date, targetType);
}

throw rejectUnsupportedType(source);
throw rejectUnsupportedType(value.getClass(), targetType);
}

/**
Expand All @@ -217,19 +219,20 @@ protected <S extends TemporalAccessor> Optional<S> getAsTemporalAccessor(Optiona
}

Class<?> typeToConvertTo = Stream.of(target, Instant.class)//
.filter(type -> target.isAssignableFrom(type))//
.filter(target::isAssignableFrom)//
.filter(type -> conversionService.canConvert(it.getClass(), type))//
.findFirst() //
.orElseThrow(() -> rejectUnsupportedType(source.map(Object.class::cast).orElseGet(() -> source)));
.orElseThrow(() -> rejectUnsupportedType(it.getClass(), target));

return (S) conversionService.convert(it, typeToConvertTo);
});
}
}

private static IllegalArgumentException rejectUnsupportedType(Object source) {
return new IllegalArgumentException(String.format("Invalid date type %s for member %s; Supported types are %s",
source.getClass(), source, AnnotationAuditingMetadata.SUPPORTED_DATE_TYPES));
private static IllegalArgumentException rejectUnsupportedType(Class<?> sourceType, Class<?> targetType) {
return new IllegalArgumentException(
String.format("Cannot convert unsupported date type %s to %s; Supported types are %s", sourceType.getName(),
targetType.getName(), AnnotationAuditingMetadata.SUPPORTED_DATE_TYPES));
}

/**
Expand Down Expand Up @@ -264,7 +267,6 @@ public Object setCreatedBy(Object value) {

@Override
public TemporalAccessor setCreatedDate(TemporalAccessor value) {

return setDateField(metadata.getCreatedDateField(), value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ static class MappingAuditingMetadata {
/**
* Creates a new {@link MappingAuditingMetadata} instance from the given {@link PersistentEntity}.
*
* @param entity must not be {@literal null}.
* @param context must not be {@literal null}.
* @param type must not be {@literal null}.
*/
public <P> MappingAuditingMetadata(MappingContext<?, ? extends PersistentProperty<?>> context, Class<?> type) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.List;

import org.springframework.core.convert.converter.Converter;
import org.springframework.lang.NonNull;

/**
* Helper class to register JSR-310 specific {@link Converter} implementations in case the we're running on Java 8.
* Helper class to register JSR-310 specific {@link Converter} implementations.
*
* @author Oliver Gierke
* @author Barak Schoster
Expand All @@ -51,9 +52,9 @@ public abstract class Jsr310Converters {
Instant.class, ZoneId.class, Duration.class, Period.class);

/**
* Returns the converters to be registered. Will only return converters in case we're running on Java 8.
* Returns the converters to be registered.
*
* @return
* @return the converters to be registered.
*/
public static Collection<Converter<?, ?>> getConvertersToRegister() {

Expand Down Expand Up @@ -82,10 +83,17 @@ public abstract class Jsr310Converters {
}

public static boolean supports(Class<?> type) {

return CLASSES.contains(type);
}

/**
* @return the collection of supported temporal classes.
* @since 3.2
*/
public static Collection<Class<?>> getSupportedClasses() {
return Collections.unmodifiableList(CLASSES);
}

@ReadingConverter
public enum DateToLocalDateTimeConverter implements Converter<Date, LocalDateTime> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@

import java.time.Instant;
import java.time.LocalDateTime;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoField;

import org.junit.jupiter.api.Test;
Expand All @@ -34,7 +36,7 @@
* @author Oliver Gierke
* @author Christoph Strobl
* @author Jens Schauder
* @since 1.5
* @author Mark Paluch
*/
class DefaultAuditableBeanWrapperFactoryUnitTests {

Expand Down Expand Up @@ -135,10 +137,56 @@ void lastModifiedAsLocalDateTimeDateIsAvailableViaWrapperAsLocalDateTime() {
assertThat(result).hasValue(now);
}

@Test
void shouldRejectUnsupportedTemporalConversion() {

var source = new WithZonedDateTime();
AuditableBeanWrapper<WithZonedDateTime> wrapper = factory.getBeanWrapperFor(source).get();

assertThatIllegalArgumentException().isThrownBy(() -> wrapper.setCreatedDate(LocalDateTime.now()))
.withMessageContaining(
"Cannot convert unsupported date type java.time.LocalDateTime to java.time.ZonedDateTime");
}

@Test // GH-2719
void shouldPassthruZonedDateTimeValue() {

var source = new WithZonedDateTime();
var now = ZonedDateTime.now();
AuditableBeanWrapper<WithZonedDateTime> wrapper = factory.getBeanWrapperFor(source).get();

wrapper.setCreatedDate(now);

assertThat(source.created).isEqualTo(now);
}

@Test // GH-2719
void shouldPassthruOffsetDatetimeValue() {

var source = new WithOffsetDateTime();
var now = OffsetDateTime.now();
AuditableBeanWrapper<WithOffsetDateTime> wrapper = factory.getBeanWrapperFor(source).get();

wrapper.setCreatedDate(now);

assertThat(source.created).isEqualTo(now);
}

public static class LongBasedAuditable {

@CreatedDate public Long dateCreated;

@LastModifiedDate public Long dateModified;
}

static class WithZonedDateTime {

@CreatedDate ZonedDateTime created;
}

static class WithOffsetDateTime {

@CreatedDate OffsetDateTime created;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoField;
import java.time.temporal.TemporalAccessor;
import java.util.Arrays;
Expand Down Expand Up @@ -242,6 +243,29 @@ void skipsCollectionPropertiesWhenSettingProperties() {
});
}

@Test // GH-2719
void shouldRejectUnsupportedTemporalConversion() {

var source = new WithZonedDateTime();
AuditableBeanWrapper<WithZonedDateTime> wrapper = factory.getBeanWrapperFor(source).get();

assertThatIllegalArgumentException().isThrownBy(() -> wrapper.setCreatedDate(LocalDateTime.now()))
.withMessageContaining(
"Cannot convert unsupported date type java.time.LocalDateTime to java.time.ZonedDateTime");
}

@Test // GH-2719
void shouldPassthruTemporalValue() {

var source = new WithZonedDateTime();
var now = ZonedDateTime.now();
AuditableBeanWrapper<WithZonedDateTime> wrapper = factory.getBeanWrapperFor(source).get();

wrapper.setCreatedDate(now);

assertThat(source.created).isEqualTo(now);
}

private void assertLastModificationDate(Object source, TemporalAccessor expected) {

var sample = new Sample();
Expand Down Expand Up @@ -302,6 +326,11 @@ static class Embedded {
@LastModifiedBy String modifier;
}

static class WithZonedDateTime {

@CreatedDate ZonedDateTime created;
}

static class WithEmbedded {
Embedded embedded;
Collection<Embedded> embeddeds;
Expand Down