diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index 24c3c2f59..fbd5d82ca 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -1376,6 +1376,7 @@ public class MappingMongoConverter extends AbstractMongoConverter if (typeHint != null && Object.class != typeHint) { + // TODO this is weird and leads to double-conversion in some cases, e.g. BigDecimal -> Decimal128 -> BigDecimal if (conversionService.canConvert(value.getClass(), typeHint)) { value = doConvert(value, typeHint); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverters.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverters.java index 2a8c8042a..5cfd6fe72 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverters.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverters.java @@ -110,9 +110,9 @@ abstract class MongoConverters { * @return * @since 5.0 */ - static Collection getBigNumberDecimal128Converters() { + static Collection> getBigNumberDecimal128Converters() { - List converters = new ArrayList<>(3); + List> converters = new ArrayList<>(3); converters.add(BigDecimalToDecimal128Converter.INSTANCE); converters.add(Decimal128ToBigDecimalConverter.INSTANCE); @@ -127,9 +127,9 @@ abstract class MongoConverters { * @return * @since 5.0 */ - static Collection getBigNumberStringConverters() { + static Collection> getBigNumberStringConverters() { - List converters = new ArrayList<>(4); + List> converters = new ArrayList<>(2); converters.add(BigDecimalToStringConverter.INSTANCE); converters.add(BigIntegerToStringConverter.INSTANCE); @@ -228,6 +228,7 @@ abstract class MongoConverters { } } + @WritingConverter enum BigDecimalToStringConverter implements Converter { INSTANCE; @@ -239,6 +240,7 @@ abstract class MongoConverters { /** * @since 2.2 */ + @WritingConverter enum BigDecimalToDecimal128Converter implements Converter { INSTANCE; @@ -250,6 +252,7 @@ abstract class MongoConverters { /** * @since 5.0 */ + @WritingConverter enum BigIntegerToDecimal128Converter implements Converter { INSTANCE; @@ -258,6 +261,7 @@ abstract class MongoConverters { } } + @ReadingConverter enum StringToBigDecimalConverter implements Converter { INSTANCE; @@ -269,6 +273,7 @@ abstract class MongoConverters { /** * @since 2.2 */ + @ReadingConverter enum Decimal128ToBigDecimalConverter implements Converter { INSTANCE; 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 28bc09288..6915ae5b5 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 @@ -30,13 +30,16 @@ import java.util.List; import java.util.Locale; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Predicate; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.jspecify.annotations.Nullable; + 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.ConverterRegistry; import org.springframework.core.convert.converter.GenericConverter; import org.springframework.data.convert.ConverterBuilder; import org.springframework.data.convert.PropertyValueConversions; @@ -78,6 +81,12 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus STORE_CONVERTERS = Collections.unmodifiableList(converters); } + /** + * Converters to be registered with the {@code ConversionService} but hidden from CustomConversions to avoid + * converter-based type hinting. + */ + private final List> fallbackConversionServiceConverters = new ArrayList<>(); + /** * Creates an empty {@link MongoCustomConversions} object. */ @@ -101,7 +110,12 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus * @since 2.3 */ protected MongoCustomConversions(MongoConverterConfigurationAdapter conversionConfiguration) { - super(conversionConfiguration.createConverterConfiguration()); + this(conversionConfiguration.createConverterConfiguration()); + } + + private MongoCustomConversions(MongoConverterConfiguration converterConfiguration) { + super(converterConfiguration); + this.fallbackConversionServiceConverters.addAll(converterConfiguration.fallbackConversionServiceConverters); } /** @@ -120,6 +134,12 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus return new MongoCustomConversions(adapter); } + @Override + public void registerConvertersIn(ConverterRegistry conversionService) { + this.fallbackConversionServiceConverters.forEach(conversionService::addConverter); + super.registerConvertersIn(conversionService); + } + @WritingConverter private enum CustomToStringConverter implements GenericConverter { @@ -155,7 +175,7 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus LocalDateTime.class); private boolean useNativeDriverJavaTimeCodecs = false; - private BigDecimalRepresentation @Nullable [] bigDecimals; + private @Nullable BigDecimalRepresentation bigDecimals; private final List customConverters = new ArrayList<>(); private final PropertyValueConversions internalValueConversion = PropertyValueConversions.simple(it -> {}); @@ -312,14 +332,14 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus * Configures the representation to for {@link java.math.BigDecimal} and {@link java.math.BigInteger} values in * MongoDB. Defaults to {@link BigDecimalRepresentation#DECIMAL128}. * - * @param representations ordered list of representations to use (first one is default) + * @param representation the representation to use. * @return this. * @since 4.5 */ - public MongoConverterConfigurationAdapter bigDecimal(BigDecimalRepresentation... representations) { + public MongoConverterConfigurationAdapter bigDecimal(BigDecimalRepresentation representation) { - Assert.notEmpty(representations, "BigDecimalDataType must not be null"); - this.bigDecimals = representations; + Assert.notNull(representation, "BigDecimalDataType must not be null"); + this.bigDecimals = representation; return this; } @@ -365,7 +385,7 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus return this.propertyValueConversions; } - ConverterConfiguration createConverterConfiguration() { + MongoConverterConfiguration createConverterConfiguration() { if (hasDefaultPropertyValueConversions() && propertyValueConversions instanceof SimplePropertyValueConversions svc) { @@ -373,19 +393,24 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus } List storeConverters = new ArrayList<>(STORE_CONVERTERS.size() + 10); - - if (bigDecimals != null) { - for (BigDecimalRepresentation representation : bigDecimals) { - switch (representation) { - case STRING -> storeConverters.addAll(MongoConverters.getBigNumberStringConverters()); - case DECIMAL128 -> storeConverters.addAll(MongoConverters.getBigNumberDecimal128Converters()); - } + List> fallbackConversionServiceConverters = new ArrayList<>(5); + fallbackConversionServiceConverters.addAll(MongoConverters.getBigNumberStringConverters()); + fallbackConversionServiceConverters.addAll(MongoConverters.getBigNumberDecimal128Converters()); + + if (bigDecimals == null) { + if (LOGGER.isInfoEnabled()) { + LOGGER.info( + "No BigDecimal/BigInteger representation set. Choose 'BigDecimalRepresentation.DECIMAL128' or 'BigDecimalRepresentation.String' to store values in desired format."); + } + } else { + switch (bigDecimals) { + case STRING -> storeConverters.addAll(MongoConverters.getBigNumberStringConverters()); + case DECIMAL128 -> storeConverters.addAll(MongoConverters.getBigNumberDecimal128Converters()); } - } else if (LOGGER.isInfoEnabled()) { - LOGGER.info( - "No BigDecimal/BigInteger representation set. Choose [DECIMAL128] and/or [String] to store values in desired format."); } + fallbackConversionServiceConverters.removeAll(storeConverters); + if (useNativeDriverJavaTimeCodecs) { /* @@ -397,7 +422,8 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus StoreConversions storeConversions = StoreConversions .of(new SimpleTypeHolder(JAVA_DRIVER_TIME_SIMPLE_TYPES, MongoSimpleTypes.HOLDER), storeConverters); - return new ConverterConfiguration(storeConversions, this.customConverters, convertiblePair -> { + return new MongoConverterConfiguration(storeConversions, fallbackConversionServiceConverters, + this.customConverters, convertiblePair -> { // Avoid default registrations @@ -408,8 +434,10 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus } storeConverters.addAll(STORE_CONVERTERS); - return new ConverterConfiguration(StoreConversions.of(MongoSimpleTypes.createSimpleTypeHolder(), storeConverters), - this.customConverters, convertiblePair -> true, this.propertyValueConversions); + return new MongoConverterConfiguration( + StoreConversions.of(MongoSimpleTypes.createSimpleTypeHolder(), storeConverters), + fallbackConversionServiceConverters, this.customConverters, convertiblePair -> true, + this.propertyValueConversions); } private boolean hasDefaultPropertyValueConversions() { @@ -418,6 +446,19 @@ public class MongoCustomConversions extends org.springframework.data.convert.Cus } + static class MongoConverterConfiguration extends ConverterConfiguration { + + private final List> fallbackConversionServiceConverters; + + public MongoConverterConfiguration(StoreConversions storeConversions, + List> fallbackConversionServiceConverters, List userConverters, + Predicate converterRegistrationFilter, + @Nullable PropertyValueConversions propertyValueConversions) { + super(storeConversions, userConverters, converterRegistrationFilter, propertyValueConversions); + this.fallbackConversionServiceConverters = fallbackConversionServiceConverters; + } + } + /** * Strategy to represent {@link java.math.BigDecimal} and {@link java.math.BigInteger} values in MongoDB. * 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 aeb79fde4..2c1b6af66 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 @@ -15,6 +15,7 @@ */ package org.springframework.data.mongodb.core.mapping; +import java.math.BigDecimal; import java.math.BigInteger; import java.time.Instant; import java.util.Set; @@ -60,7 +61,7 @@ public abstract class MongoSimpleTypes { BsonDocument.class, BsonDouble.class, BsonInt32.class, BsonInt64.class, BsonJavaScript.class, BsonJavaScriptWithScope.class, BsonObjectId.class, BsonRegularExpression.class, BsonString.class, BsonTimestamp.class, Geometry.class, GeometryCollection.class, LineString.class, MultiLineString.class, - MultiPoint.class, MultiPolygon.class, Point.class, Polygon.class); + MultiPoint.class, MultiPolygon.class, Point.class, Polygon.class, BigInteger.class, BigDecimal.class); public static final SimpleTypeHolder HOLDER = createSimpleTypeHolder(); 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 e3982a6ac..d3cc037fe 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 @@ -15,22 +15,10 @@ */ package org.springframework.data.mongodb.core.convert; -import static java.time.ZoneId.systemDefault; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; -import static org.springframework.data.mongodb.core.DocumentTestUtils.assertTypeHint; -import static org.springframework.data.mongodb.core.DocumentTestUtils.getAsDocument; -import static org.springframework.data.mongodb.test.util.Assertions.assertThat; -import static org.springframework.data.mongodb.test.util.Assertions.assertThatExceptionOfType; -import static org.springframework.data.mongodb.test.util.Assertions.assertThatThrownBy; -import static org.springframework.data.mongodb.test.util.Assertions.fail; +import static java.time.ZoneId.*; +import static org.mockito.Mockito.*; +import static org.springframework.data.mongodb.core.DocumentTestUtils.*; +import static org.springframework.data.mongodb.test.util.Assertions.*; import java.math.BigDecimal; import java.math.BigInteger; @@ -39,24 +27,7 @@ import java.nio.ByteBuffer; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.temporal.ChronoUnit; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.EnumMap; -import java.util.EnumSet; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; -import java.util.SortedMap; -import java.util.TreeMap; -import java.util.UUID; +import java.util.*; import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Stream; @@ -82,6 +53,7 @@ import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; + import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.ConversionNotSupportedException; import org.springframework.beans.factory.annotation.Autowired; @@ -90,7 +62,6 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.context.ApplicationContext; import org.springframework.context.support.StaticApplicationContext; -import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; @@ -2207,57 +2178,153 @@ class MappingMongoConverterUnitTests { @Test // GH-5037 @SuppressWarnings("deprecation") - void mapsBigIntegerToDecimal128WhenAnnotatedWithFieldTargetTypeWhenDefaultConversionIsSetToString() { + void mapsBigNumbersToString() { - converter = createConverter(BigDecimalRepresentation.STRING, BigDecimalRepresentation.DECIMAL128); + converter = createConverter(BigDecimalRepresentation.STRING); - WithExplicitTargetTypes source = new WithExplicitTargetTypes(); - source.bigDecimal = BigDecimal.valueOf(3.14159D); + WithoutExplicitTargetTypes source = new WithoutExplicitTargetTypes(); + source.bigInteger = BigInteger.TWO; + source.bigDecimal = new BigDecimal("3.14159"); org.bson.Document target = new org.bson.Document(); converter.write(source, target); - assertThat(target.get("bigDecimal")).isEqualTo(new Decimal128(source.bigDecimal)); + assertThat(target.get("bigInteger")).isEqualTo(source.bigInteger.toString()); + assertThat(target.get("bigDecimal")).isEqualTo(source.bigDecimal.toString()); } @Test // GH-5037 - @SuppressWarnings("deprecation") - void mapsBigIntegerToStringWhenNotAnnotatedWithFieldTargetTypeAndDefaultConversionIsSetToString() { + void mapsBigNumbersToDecimal128() { - converter = createConverter(BigDecimalRepresentation.STRING, BigDecimalRepresentation.DECIMAL128); + converter = createConverter(BigDecimalRepresentation.DECIMAL128); + + WithoutExplicitTargetTypes source = new WithoutExplicitTargetTypes(); + source.bigInteger = BigInteger.TWO; + source.bigDecimal = new BigDecimal("3.14159"); - BigDecimalContainer source = new BigDecimalContainer(); - source.value = BigDecimal.valueOf(3.14159D); org.bson.Document target = new org.bson.Document(); converter.write(source, target); - assertThat(target.get("value")).isInstanceOf(String.class); + + assertThat(target.get("bigInteger")).isEqualTo(new Decimal128(source.bigInteger.longValue())); + assertThat(target.get("bigDecimal")).isEqualTo(new Decimal128(source.bigDecimal)); } - @Test // GH-5037 - void mapsBigIntegerToStringWhenAnnotatedWithFieldTargetTypeEvenWhenDefaultConverterIsSetToDecimal128() { + @ParameterizedTest // GH-5037 + @MethodSource("representations") + void shouldApplyExplicitBigIntegerToStringConversion(BigDecimalRepresentation representation) { - converter = createConverter(BigDecimalRepresentation.DECIMAL128); + converter = createConverter(representation); WithExplicitTargetTypes source = new WithExplicitTargetTypes(); source.bigIntegerAsString = BigInteger.TWO; + source.bigDecimalAsString = new BigDecimal("123.456"); org.bson.Document target = new org.bson.Document(); converter.write(source, target); assertThat(target.get("bigIntegerAsString")).isEqualTo(source.bigIntegerAsString.toString()); + assertThat(target.get("bigDecimalAsString")).isEqualTo(source.bigDecimalAsString.toString()); } - @Test // GH-5037 - void explicitBigNumberConversionErrorsIfConverterNotRegistered() { + @ParameterizedTest // GH-5037 + @MethodSource("representations") + void shouldApplyExplicitDecimal128Conversion(BigDecimalRepresentation representation) { - converter = createConverter(BigDecimalRepresentation.STRING); + converter = createConverter(representation); WithExplicitTargetTypes source = new WithExplicitTargetTypes(); source.bigInteger = BigInteger.TWO; + source.bigDecimal = new BigDecimal("123.456"); + + org.bson.Document target = new org.bson.Document(); + + converter.write(source, target); + + assertThat(target.get("bigInteger")).isEqualTo(new Decimal128(source.bigInteger.longValue())); + assertThat(target.get("bigDecimal")).isEqualTo(new Decimal128(source.bigDecimal)); + } + + static Stream representations() { + return Stream.of(Arguments.argumentSet("None (default)", new Object[] { null }), // + Arguments.argumentSet("STRING", BigDecimalRepresentation.STRING), // + Arguments.argumentSet("DECIMAL128", BigDecimalRepresentation.DECIMAL128)); + } + + @Test // GH-5037 + void shouldWriteBigNumbersAsIsWithoutConfiguration() { + + converter = createConverter(); + + WithoutExplicitTargetTypes source = new WithoutExplicitTargetTypes(); + source.bigInteger = BigInteger.TWO; + source.bigDecimal = new BigDecimal("123.456"); + + org.bson.Document target = new org.bson.Document(); + + converter.write(source, target); + + assertThat(target.get("bigInteger")).isEqualTo(source.bigInteger); + assertThat(target.get("bigDecimal")).isEqualTo(source.bigDecimal); + } + + @Test // GH-5037 + void shouldReadTypedBigNumbersFromDecimal128() { + + converter = createConverter(); org.bson.Document target = new org.bson.Document(); + target.put("bigInteger", new Decimal128(2)); + target.put("bigDecimal", new Decimal128(new BigDecimal("123.456"))); - assertThatExceptionOfType(ConversionFailedException.class).isThrownBy(() -> converter.write(source, target)); + WithExplicitTargetTypes result = converter.read(WithExplicitTargetTypes.class, target); + + assertThat(result.bigInteger).isEqualTo(BigInteger.TWO); + assertThat(result.bigDecimal).isEqualTo(new BigDecimal("123.456")); + } + + @Test // GH-5037 + void shouldReadTypedBigNumbersFromString() { + + converter = createConverter(); + + org.bson.Document target = new org.bson.Document(); + target.put("bigIntegerAsString", "2"); + target.put("bigDecimalAsString", "123.456"); + + WithExplicitTargetTypes result = converter.read(WithExplicitTargetTypes.class, target); + + assertThat(result.bigIntegerAsString).isEqualTo(BigInteger.TWO); + assertThat(result.bigDecimalAsString).isEqualTo(new BigDecimal("123.456")); + } + + @Test // GH-5037 + void shouldReadBigNumbersFromDecimal128() { + + converter = createConverter(); + + org.bson.Document target = new org.bson.Document(); + target.put("bigInteger", new Decimal128(2)); + target.put("bigDecimal", new Decimal128(new BigDecimal("123.456"))); + + WithoutExplicitTargetTypes result = converter.read(WithoutExplicitTargetTypes.class, target); + + assertThat(result.bigInteger).isEqualTo(BigInteger.TWO); + assertThat(result.bigDecimal).isEqualTo(new BigDecimal("123.456")); + } + + @Test // GH-5037 + void shouldReadBigNumbersFromString() { + + converter = createConverter(); + + org.bson.Document target = new org.bson.Document(); + target.put("bigInteger", "2"); + target.put("bigDecimal", "123.456"); + + WithoutExplicitTargetTypes result = converter.read(WithoutExplicitTargetTypes.class, target); + + assertThat(result.bigInteger).isEqualTo(BigInteger.TWO); + assertThat(result.bigDecimal).isEqualTo(new BigDecimal("123.456")); } @Test // DATAMONGO-2328 @@ -3517,11 +3584,21 @@ class MappingMongoConverterUnitTests { assertThat(document).containsEntry("map.foo", "2.5"); } + private MappingMongoConverter createConverter() { + return createConverter(null); + } + private MappingMongoConverter createConverter( - MongoCustomConversions.BigDecimalRepresentation... bigDecimalRepresentation) { + MongoCustomConversions.BigDecimalRepresentation bigDecimalRepresentation) { MongoCustomConversions conversions = MongoCustomConversions.create( - it -> it.registerConverter(new ByteBufferToDoubleHolderConverter()).bigDecimal(bigDecimalRepresentation)); + it -> { + it.registerConverter(new ByteBufferToDoubleHolderConverter()); + + if (bigDecimalRepresentation != null) { + it.bigDecimal(bigDecimalRepresentation); + } + }); MongoMappingContext mappingContext = new MongoMappingContext(); mappingContext.setApplicationContext(context); @@ -4197,6 +4274,9 @@ class MappingMongoConverterUnitTests { @Field(targetType = FieldType.STRING) // BigInteger bigIntegerAsString; + @Field(targetType = FieldType.STRING) // + BigDecimal bigDecimalAsString; + @Field(targetType = FieldType.INT64) // Date dateAsLong; @@ -4210,6 +4290,14 @@ class MappingMongoConverterUnitTests { Date dateAsObjectId; } + static class WithoutExplicitTargetTypes { + + BigDecimal bigDecimal; + + BigInteger bigInteger; + + } + static class WrapperAroundWithUnwrapped { String someValue;