From 3b6880edfd91d9012ffbcdf9573a91b13e8a2779 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 25 Nov 2019 11:06:53 +0100 Subject: [PATCH] DATAMONGO-2400 - Consider java.time.Instant a store supported native type and add configuration options for other java.time types. We now use the MongoDB Java driver InstantCodec instead of a dedicated converter. Other java.time types like LocalDate use a different zone offset when writing values which can lead to unexpected behavior. Therefore we added configuration options to MongoCustomConversions that allow to tell the conversion sub system which approach to use when writing those kind of types. Original pull request: #810. --- .../config/MongoConfigurationSupport.java | 28 ++- .../core/convert/MongoCustomConversions.java | 195 +++++++++++++++++- .../core/mapping/MongoSimpleTypes.java | 2 + .../convert/MappingMongoConverterTests.java | 66 +++++- .../MappingMongoConverterUnitTests.java | 21 ++ src/main/asciidoc/reference/mapping.adoc | 33 ++- 6 files changed, 327 insertions(+), 18 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoConfigurationSupport.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoConfigurationSupport.java index a61646783..b3c771bc4 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoConfigurationSupport.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoConfigurationSupport.java @@ -32,6 +32,7 @@ import org.springframework.data.mapping.model.CamelCaseAbbreviatingFieldNamingSt import org.springframework.data.mapping.model.FieldNamingStrategy; import org.springframework.data.mapping.model.PropertyNameFieldNamingStrategy; import org.springframework.data.mongodb.core.convert.MongoCustomConversions; +import org.springframework.data.mongodb.core.convert.MongoCustomConversions.MongoConverterConfigurationAdapter; import org.springframework.data.mongodb.core.mapping.Document; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.util.ClassUtils; @@ -92,14 +93,35 @@ public abstract class MongoConfigurationSupport { /** * Register custom {@link Converter}s in a {@link CustomConversions} object if required. These - * {@link CustomConversions} will be registered with the {@link #mappingMongoConverter()} and - * {@link #mongoMappingContext()}. Returns an empty {@link MongoCustomConversions} instance by default. + * {@link CustomConversions} will be registered with the + * {@link org.springframework.data.mongodb.core.convert.MappingMongoConverter} and {@link #mongoMappingContext()}. + * Returns an empty {@link MongoCustomConversions} instance by default.
+ * NOTE: Use {@link #customConversionsConfiguration(MongoConverterConfigurationAdapter)} to configure + * MongoDB native simple types and register custom {@link Converter converters}. * * @return must not be {@literal null}. */ @Bean public CustomConversions customConversions() { - return new MongoCustomConversions(Collections.emptyList()); + return new MongoCustomConversions(this::customConversionsConfiguration); + } + + /** + * Configuration hook for {@link MongoCustomConversions} creation. + * + * @param converterConfigurationAdapter never {@literal null}. + * @since 2.3 + */ + protected void customConversionsConfiguration(MongoConverterConfigurationAdapter converterConfigurationAdapter) { + + /* + * In case you want to use the MongoDB Java Driver native Codecs for java.time types instead of the converters SpringData + * ships with, then you may want to call the following here. + * + * converterConfigurationAdapter.useNativeDriverJavaTimeCodecs() + * + * But please, be careful! LocalDate, LocalTime and LocalDateTime will be stored with different values by doing so. + */ } /** diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoCustomConversions.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoCustomConversions.java index 7e5659045..ff9023a4f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoCustomConversions.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoCustomConversions.java @@ -15,26 +15,42 @@ */ package org.springframework.data.mongodb.core.convert; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.core.convert.converter.GenericConverter; +import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; import org.springframework.data.convert.JodaTimeConverters; import org.springframework.data.convert.WritingConverter; +import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.mongodb.core.mapping.MongoSimpleTypes; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * Value object to capture custom conversion. {@link MongoCustomConversions} also act as factory for * {@link org.springframework.data.mapping.model.SimpleTypeHolder} * * @author Mark Paluch + * @author Christoph Strobl * @since 2.0 * @see org.springframework.data.convert.CustomConversions * @see org.springframework.data.mapping.model.SimpleTypeHolder @@ -71,7 +87,30 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus * @param converters must not be {@literal null}. */ public MongoCustomConversions(List converters) { - super(STORE_CONVERSIONS, converters); + + this(converterConfigurationAdapter -> { + + converterConfigurationAdapter.useSpringDataJavaTimeCodecs(); + converterConfigurationAdapter.registerConverters(converters); + }); + } + + /** + * Functional style {@link org.springframework.data.convert.CustomConversions} creation giving users a convenient way + * of configuring store specific capabilities by providing deferred hooks to what will be configured when creating the + * {@link org.springframework.data.convert.CustomConversions#CustomConversions(ConverterConfiguration) instance}. + * + * @param conversionConfiguration must not be {@literal null}. + * @since 2.3 + */ + public MongoCustomConversions(Consumer conversionConfiguration) { + + super(() -> { + + MongoConverterConfigurationAdapter adapter = new MongoConverterConfigurationAdapter(); + conversionConfiguration.accept(adapter); + return adapter.createConverterConfiguration(); + }); } @WritingConverter @@ -99,4 +138,158 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus return source != null ? source.toString() : null; } } + + /** + * {@link MongoConverterConfigurationAdapter} encapsulates creation of + * {@link org.springframework.data.convert.CustomConversions.ConverterConfiguration} with MongoDB specifics. + * + * @author Christoph Strobl + * @since 2.3 + */ + public static class MongoConverterConfigurationAdapter { + + /** + * List of {@literal java.time} types having different representation when rendered via the native + * {@link org.bson.codecs.Codec} than the Spring Data {@link Converter}. + */ + private static final List> JAVA_DRIVER_TIME_SIMPLE_TYPES = Arrays.asList(LocalDate.class, LocalTime.class, + LocalDateTime.class); + + private boolean useNativeDriverJavaTimeCodecs = false; + private List customConverters = new ArrayList<>(); + + /** + * Set wether or not to use the native MongoDB Java Driver {@link org.bson.codecs.Codec codes} for + * {@link org.bson.codecs.jsr310.LocalDateCodec LocalDate}, {@link org.bson.codecs.jsr310.LocalTimeCodec LocalTime} + * and {@link org.bson.codecs.jsr310.LocalDateTimeCodec LocalDateTime} using a {@link ZoneOffset#UTC}. + * + * @param useNativeDriverJavaTimeCodecs + * @return this. + */ + public MongoConverterConfigurationAdapter useNativeDriverJavaTimeCodecs(boolean useNativeDriverJavaTimeCodecs) { + + this.useNativeDriverJavaTimeCodecs = useNativeDriverJavaTimeCodecs; + return this; + } + + /** + * Use the native MongoDB Java Driver {@link org.bson.codecs.Codec codes} for + * {@link org.bson.codecs.jsr310.LocalDateCodec LocalDate}, {@link org.bson.codecs.jsr310.LocalTimeCodec LocalTime} + * and {@link org.bson.codecs.jsr310.LocalDateTimeCodec LocalDateTime} using a {@link ZoneOffset#UTC}. + * + * @return this. + * @see #useNativeDriverJavaTimeCodecs(boolean) + */ + public MongoConverterConfigurationAdapter useNativeDriverJavaTimeCodecs() { + return useNativeDriverJavaTimeCodecs(true); + } + + /** + * Use SpringData {@link Converter Jsr310 converters} for + * {@link org.springframework.data.convert.Jsr310Converters.LocalDateToDateConverter LocalDate}, + * {@link org.springframework.data.convert.Jsr310Converters.LocalTimeToDateConverter LocalTime} and + * {@link org.springframework.data.convert.Jsr310Converters.LocalDateTimeToDateConverter LocalDateTime} using the + * {@link ZoneId#systemDefault()}. + * + * @return this. + * @see #useNativeDriverJavaTimeCodecs(boolean) + */ + public MongoConverterConfigurationAdapter useSpringDataJavaTimeCodecs() { + return useNativeDriverJavaTimeCodecs(false); + } + + /** + * Add a custom {@link Converter} implementation. + * + * @param converter must not be {@literal null}. + * @return this. + */ + public MongoConverterConfigurationAdapter registerConverter(Converter converter) { + + Assert.notNull(converter, "Converter must not be null!"); + customConverters.add(converter); + return this; + } + + /** + * Add a custom {@link ConverterFactory} implementation. + * + * @param converterFactory must not be {@literal null}. + * @return this. + */ + public MongoConverterConfigurationAdapter registerConverterFactory(ConverterFactory converterFactory) { + + Assert.notNull(converterFactory, "ConverterFactory must not be null!"); + customConverters.add(converterFactory); + return this; + } + + /** + * Add {@link Converter converters}, {@link ConverterFactory factories}, ... + * + * @param converters must not be {@literal null} nor contain {@literal null} values. + * @return this. + */ + public MongoConverterConfigurationAdapter registerConverters(Collection converters) { + + Assert.noNullElements(converters, "Converters must not be null nor contain null values!"); + customConverters.addAll(converters); + return this; + } + + ConverterConfiguration createConverterConfiguration() { + + if (!useNativeDriverJavaTimeCodecs) { + return new ConverterConfiguration(STORE_CONVERSIONS, this.customConverters); + } + + /* + * We need to have those converters using UTC as the default ones would go on with the systemDefault. + */ + List converters = new ArrayList<>(); + converters.add(DateToUtcLocalDateConverter.INSTANCE); + converters.add(DateToUtcLocalTimeConverter.INSTANCE); + converters.add(DateToUtcLocalDateTimeConverter.INSTANCE); + converters.addAll(STORE_CONVERTERS); + + /* + * Right, good catch! We also need to make sure to remove default writing converters for java.time types. + */ + List skipConverterRegistrationFor = JAVA_DRIVER_TIME_SIMPLE_TYPES.stream() // + .map(it -> new ConvertiblePair(it, Date.class)) // + .collect(Collectors.toList()); // + + StoreConversions storeConversions = StoreConversions + .of(new SimpleTypeHolder(new HashSet<>(JAVA_DRIVER_TIME_SIMPLE_TYPES), MongoSimpleTypes.HOLDER), converters); + + return new ConverterConfiguration(storeConversions, this.customConverters, skipConverterRegistrationFor); + } + + private enum DateToUtcLocalDateTimeConverter implements Converter { + INSTANCE; + + @Override + public LocalDateTime convert(Date source) { + return LocalDateTime.ofInstant(Instant.ofEpochMilli(source.getTime()), ZoneId.of("UTC")); + } + } + + private enum DateToUtcLocalTimeConverter implements Converter { + INSTANCE; + + @Override + public LocalTime convert(Date source) { + return DateToUtcLocalDateTimeConverter.INSTANCE.convert(source).toLocalTime(); + } + } + + private enum DateToUtcLocalDateConverter implements Converter { + INSTANCE; + + @Override + public LocalDate convert(Date source) { + return DateToUtcLocalDateTimeConverter.INSTANCE.convert(source).toLocalDate(); + } + } + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java index 27a4ac9e5..21cee062e 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java @@ -16,6 +16,7 @@ package org.springframework.data.mongodb.core.mapping; import java.math.BigInteger; +import java.time.Instant; import java.util.Collections; import java.util.HashSet; import java.util.Set; @@ -71,6 +72,7 @@ public abstract class MongoSimpleTypes { simpleTypes.add(Pattern.class); simpleTypes.add(Symbol.class); simpleTypes.add(UUID.class); + simpleTypes.add(Instant.class); simpleTypes.add(BsonBinary.class); simpleTypes.add(BsonBoolean.class); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java index f44a367f3..906bcf1a8 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java @@ -24,6 +24,11 @@ import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.ZoneId; import java.util.Arrays; import java.util.List; @@ -40,6 +45,7 @@ import org.springframework.data.mongodb.test.util.Client; import org.springframework.data.mongodb.test.util.MongoClientExtension; import com.mongodb.client.MongoClient; +import com.mongodb.client.MongoCollection; /** * Integration tests for {@link MappingMongoConverter}. @@ -49,6 +55,7 @@ import com.mongodb.client.MongoClient; @ExtendWith(MongoClientExtension.class) public class MappingMongoConverterTests { + public static final String DATABASE = "mapping-converter-tests"; static @Client MongoClient client; MappingMongoConverter converter; @@ -58,21 +65,22 @@ public class MappingMongoConverterTests { @BeforeEach public void setUp() { - client.getDatabase("mapping-converter-tests").drop(); + client.getDatabase(DATABASE).drop(); - MongoDatabaseFactory factory = new SimpleMongoClientDatabaseFactory(client, "mapping-converter-tests"); + MongoDatabaseFactory factory = new SimpleMongoClientDatabaseFactory(client, DATABASE); dbRefResolver = spy(new DefaultDbRefResolver(factory)); mappingContext = new MongoMappingContext(); mappingContext.afterPropertiesSet(); converter = new MappingMongoConverter(dbRefResolver, mappingContext); + converter.afterPropertiesSet(); } @Test // DATAMONGO-2004 public void resolvesLazyDBRefOnAccess() { - client.getDatabase("mapping-converter-tests").getCollection("samples") + client.getDatabase(DATABASE).getCollection("samples") .insertMany(Arrays.asList(new Document("_id", "sample-1").append("value", "one"), new Document("_id", "sample-2").append("value", "two"))); @@ -93,7 +101,7 @@ public class MappingMongoConverterTests { @Test // DATAMONGO-2004 public void resolvesLazyDBRefConstructorArgOnAccess() { - client.getDatabase("mapping-converter-tests").getCollection("samples") + client.getDatabase(DATABASE).getCollection("samples") .insertMany(Arrays.asList(new Document("_id", "sample-1").append("value", "one"), new Document("_id", "sample-2").append("value", "two"))); @@ -111,6 +119,31 @@ public class MappingMongoConverterTests { verify(dbRefResolver).bulkFetch(any()); } + @Test // DATAMONGO-2400 + public void readJavaTimeValuesWrittenViaCodec() { + + configureConverterWithNativeJavaTimeCodec(); + MongoCollection mongoCollection = client.getDatabase(DATABASE).getCollection("java-time-types"); + + Instant now = Instant.now(); + WithJavaTimeTypes source = WithJavaTimeTypes.withJavaTimeTypes(now); + source.id = "id-1"; + + mongoCollection.insertOne(source.toDocument()); + + assertThat(converter.read(WithJavaTimeTypes.class, mongoCollection.find(new Document("_id", source.id)).first())) + .isEqualTo(source); + } + + void configureConverterWithNativeJavaTimeCodec() { + + converter = new MappingMongoConverter(dbRefResolver, mappingContext); + converter.setCustomConversions(new MongoCustomConversions(config -> { + config.useNativeDriverJavaTimeCodecs(); + })); + converter.afterPropertiesSet(); + } + public static class WithLazyDBRef { @Id String id; @@ -145,4 +178,29 @@ public class MappingMongoConverterTests { @Id String id; String value; } + + @Data + static class WithJavaTimeTypes { + + @Id String id; + LocalDate localDate; + LocalTime localTime; + LocalDateTime localDateTime; + + static WithJavaTimeTypes withJavaTimeTypes(Instant instant) { + + WithJavaTimeTypes instance = new WithJavaTimeTypes(); + + instance.localDate = LocalDate.from(instant.atZone(ZoneId.of("CET"))); + instance.localTime = LocalTime.from(instant.atZone(ZoneId.of("CET"))); + instance.localDateTime = LocalDateTime.from(instant.atZone(ZoneId.of("CET"))); + + return instance; + } + + Document toDocument() { + return new Document("_id", id).append("localDate", localDate).append("localTime", localTime) + .append("localDateTime", localDateTime); + } + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index e2a8e12c3..c4ad84f8c 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java @@ -1072,6 +1072,27 @@ public class MappingMongoConverterUnitTests { assertThat(result.attributes).isNotNull(); } + @Test // DATAMONGO-2400 + public void writeJavaTimeValuesViaCodec() { + + configureConverterWithNativeJavaTimeCodec(); + TypeWithLocalDateTime source = new TypeWithLocalDateTime(); + + org.bson.Document target = new org.bson.Document(); + converter.write(source, target); + + assertThat(target).containsEntry("date", source.date); + } + + void configureConverterWithNativeJavaTimeCodec() { + + converter = new MappingMongoConverter(resolver, mappingContext); + converter.setCustomConversions(new MongoCustomConversions(config -> { + config.useNativeDriverJavaTimeCodecs(); + })); + converter.afterPropertiesSet(); + } + private static void assertSyntheticFieldValueOf(Object target, Object expected) { for (int i = 0; i < 10; i++) { diff --git a/src/main/asciidoc/reference/mapping.adoc b/src/main/asciidoc/reference/mapping.adoc index b034cdf67..968000e33 100644 --- a/src/main/asciidoc/reference/mapping.adoc +++ b/src/main/asciidoc/reference/mapping.adoc @@ -177,14 +177,24 @@ calling `get()` before the actual conversion | converter | `{"currencyCode" : "EUR"}` +| `Instant` + +(Java 8) +| native +| `{"date" : ISODate("2019-11-12T23:00:00.809Z")}` + +| `Instant` + +(Joda, JSR310-BackPort) +| converter +| `{"date" : ISODate("2019-11-12T23:00:00.809Z")}` + | `LocalDate` + (Joda, Java 8, JSR310-BackPort) -| converter +| converter / native (Java8)footnote:[Uses UTC zone offset. Configure via <>] | `{"date" : ISODate("2019-11-12T00:00:00.000Z")}` -| `LocalDateTime`, `LocalTime`, `Instant` + +| `LocalDateTime`, `LocalTime` + (Joda, Java 8, JSR310-BackPort) -| converter +| converter / native (Java8)footnote:[Uses UTC zone offset. Configure via <>] | `{"date" : ISODate("2019-11-12T23:00:00.809Z")}` | `DateTime` (Joda) @@ -258,6 +268,8 @@ You can configure the `MappingMongoConverter` as well as `com.mongodb.client.Mon ==== [source,java] ---- +import com.mongodb.client.MongoClients; + @Configuration public class GeoSpatialAppConfig extends AbstractMongoClientConfiguration { @@ -271,6 +283,8 @@ public class GeoSpatialAppConfig extends AbstractMongoClientConfiguration { return "database"; } + // the following are optional + @Override public String getMappingBasePackage() { return "com.bigbank.domain"; @@ -278,13 +292,11 @@ public class GeoSpatialAppConfig extends AbstractMongoClientConfiguration { // the following are optional - @Bean @Override - public CustomConversions customConversions() throws Exception { - List> converterList = new ArrayList>(); - converterList.add(new org.springframework.data.mongodb.test.PersonReadConverter()); - converterList.add(new org.springframework.data.mongodb.test.PersonWriteConverter()); - return new CustomConversions(converterList); + void customConversionsConfiguration(MongoConverterConfigurationAdapter adapter) { + + adapter.registerConverter(new org.springframework.data.mongodb.test.PersonReadConverter()); + adapter.registerConverter(new org.springframework.data.mongodb.test.PersonWriteConverter()); } @Bean @@ -297,7 +309,8 @@ public class GeoSpatialAppConfig extends AbstractMongoClientConfiguration { `AbstractMongoClientConfiguration` requires you to implement methods that define a `com.mongodb.client.MongoClient` as well as provide a database name. `AbstractMongoClientConfiguration` also has a method named `getMappingBasePackage(…)` that you can override to tell the converter where to scan for classes annotated with the `@Document` annotation. -You can add additional converters to the converter by overriding the `customConversions` method. Also shown in the preceding example is a `LoggingEventListener`, which logs `MongoMappingEvent` instances that are posted onto Spring's `ApplicationContextEvent` infrastructure. +You can add additional converters to the converter by overriding the `customConversionsConfiguration` method. +Also shown in the preceding example is a `LoggingEventListener`, which logs `MongoMappingEvent` instances that are posted onto Spring's `ApplicationContextEvent` infrastructure. NOTE: `AbstractMongoClientConfiguration` creates a `MongoTemplate` instance and registers it with the container under the name `mongoTemplate`.