diff --git a/pom.xml b/pom.xml index b23a16229f..601181d495 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 5.0.0-SNAPSHOT + 5.0.0-4346-property-value-convert-cce-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index fc88571622..bf46b87197 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-mongodb-parent - 5.0.0-SNAPSHOT + 5.0.0-4346-property-value-convert-cce-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 37730f7d40..c3415ec286 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 5.0.0-SNAPSHOT + 5.0.0-4346-property-value-convert-cce-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index 92fd30b403..0cde0cc757 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -15,17 +15,8 @@ */ package org.springframework.data.mongodb.core.convert; -import java.util.AbstractMap; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Optional; -import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -38,7 +29,7 @@ import org.bson.conversions.Bson; import org.bson.types.ObjectId; import org.jspecify.annotations.Nullable; - +import org.springframework.core.GenericTypeResolver; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Reference; @@ -713,7 +704,7 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers List converted = new ArrayList<>(collection.size()); for (Object o : collection) { - converted.add(valueConverter.write(o, conversionContext)); + converted.add(typeSafeSingleWriteConversion(valueConverter, conversionContext, o)); } return converted; @@ -730,7 +721,28 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers }); } - return value != null ? valueConverter.write(value, conversionContext) : value; + return typeSafeSingleWriteConversion(valueConverter, conversionContext, value); + } + + private static @Nullable Object typeSafeSingleWriteConversion(PropertyValueConverter> valueConverter, MongoConversionContext conversionContext, @Nullable Object o) { + + Class[] typeArguments = GenericTypeResolver.resolveTypeArguments(valueConverter.getClass(), PropertyValueConverter.class); + + if (o == null) { + return valueConverter.writeNull(conversionContext); + } + + // don't apply the converter if we can determine, that the types don't match. + // this happens for regex comparisons. + if (typeArguments != null && typeArguments.length > 0) { + + Class converterWriteArgumentType = typeArguments[0]; + if (!converterWriteArgumentType.isAssignableFrom(o.getClass())) { + return o; + } + } + + return valueConverter.write(o, conversionContext); } @Nullable 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 002876e8cc..ed4bf669fc 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 @@ -3315,6 +3315,21 @@ void enumConverter() { assertThat(read.converterEnum).isEqualTo("spring"); } + @Test // GH-4346 + void nullConverter() { + + WithValueConverters wvc = new WithValueConverters(); + wvc.nullConverter = null; + + org.bson.Document target = new org.bson.Document(); + converter.write(wvc, target); + + assertThat(target).containsEntry("nullConverter", "W"); + + WithValueConverters read = converter.read(WithValueConverters.class, org.bson.Document.parse("{ nullConverter : null}")); + assertThat(read.nullConverter).isEqualTo("R"); + } + @Test // GH-3596 void beanConverter() { @@ -3357,12 +3372,12 @@ void beanConverter() { new PropertyValueConverter() { @Override - public @Nullable String read(org.bson.@Nullable Document nativeValue, MongoConversionContext context) { + public @Nullable String read(org.bson.Document nativeValue, MongoConversionContext context) { return nativeValue.getString("bar"); } @Override - public org.bson.@Nullable Document write(@Nullable String domainValue, MongoConversionContext context) { + public org.bson.Document write(String domainValue, MongoConversionContext context) { return new org.bson.Document("bar", domainValue); } }); @@ -4615,6 +4630,8 @@ static class WithValueConverters { @ValueConverter(Converter2.class) String converterEnum; + @ValueConverter(NullReplacingValueConverter.class) String nullConverter; + String viaRegisteredConverter; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/NullReplacingValueConverter.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/NullReplacingValueConverter.java new file mode 100644 index 0000000000..3081d3bab6 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/NullReplacingValueConverter.java @@ -0,0 +1,44 @@ +/* + * Copyright 2022-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mongodb.core.convert; + +import org.jspecify.annotations.Nullable; + +/** + * @author Jens Schauder + */ +class NullReplacingValueConverter implements MongoValueConverter { + + @Override + public @Nullable String read(String value, MongoConversionContext context) { + return value; + } + + @Override + public @Nullable String readNull(MongoConversionContext context) { + return "R"; + } + + @Override + public @Nullable String write(String value, MongoConversionContext context) { + return value; + } + + @Override + public @Nullable String writeNull(MongoConversionContext context) { + return "W"; + } +} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java index e950036087..fe1062fc99 100755 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java @@ -38,7 +38,6 @@ import org.bson.types.ObjectId; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; - import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; import org.springframework.data.annotation.Transient; @@ -58,17 +57,8 @@ import org.springframework.data.mongodb.core.convert.MongoCustomConversions.MongoConverterConfigurationAdapter; import org.springframework.data.mongodb.core.geo.GeoJsonPoint; import org.springframework.data.mongodb.core.geo.GeoJsonPolygon; -import org.springframework.data.mongodb.core.mapping.DBRef; -import org.springframework.data.mongodb.core.mapping.Document; -import org.springframework.data.mongodb.core.mapping.DocumentReference; -import org.springframework.data.mongodb.core.mapping.Field; +import org.springframework.data.mongodb.core.mapping.*; import org.springframework.data.mongodb.core.mapping.FieldName.Type; -import org.springframework.data.mongodb.core.mapping.FieldType; -import org.springframework.data.mongodb.core.mapping.MongoId; -import org.springframework.data.mongodb.core.mapping.MongoMappingContext; -import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; -import org.springframework.data.mongodb.core.mapping.TextScore; -import org.springframework.data.mongodb.core.mapping.Unwrapped; import org.springframework.data.mongodb.core.query.BasicQuery; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Query; @@ -98,7 +88,8 @@ public class QueryMapperUnitTests { @BeforeEach void beforeEach() { - MongoCustomConversions conversions = new MongoCustomConversions(new MongoConverterConfigurationAdapter().bigDecimal(BigDecimalRepresentation.DECIMAL128)); + MongoCustomConversions conversions = new MongoCustomConversions( + new MongoConverterConfigurationAdapter().bigDecimal(BigDecimalRepresentation.DECIMAL128)); this.context = new MongoMappingContext(); this.context.setSimpleTypeHolder(conversions.getSimpleTypeHolder()); @@ -133,7 +124,8 @@ void convertsStringIntoObjectId() { void handlesBigIntegerIdsCorrectly/*in legacy string format*/() { MappingMongoConverter converter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, context); - converter.setCustomConversions(MongoCustomConversions.create(adapter -> adapter.bigDecimal(BigDecimalRepresentation.STRING))); + converter.setCustomConversions( + MongoCustomConversions.create(adapter -> adapter.bigDecimal(BigDecimalRepresentation.STRING))); converter.afterPropertiesSet(); QueryMapper mapper = new QueryMapper(converter); @@ -1663,7 +1655,7 @@ void usageOfUntypedAggregationShouldRenderOperationsAsIs() { Query query = query(expr(Expr.valueOf(ComparisonOperators.valueOf("field").greaterThan("budget")))); org.bson.Document mappedObject = mapper.getMappedObject(query.getQueryObject(), - context.getPersistentEntity(Object.class)); + context.getPersistentEntity(Object.class)); assertThat(mappedObject).isEqualTo("{ $expr : { $expr : { $gt : [ '$field', '$budget'] } } }"); } @@ -1730,11 +1722,49 @@ void allOperatorShouldConvertIdCollection() { Criteria criteria = new Criteria().andOperator(where("name").isNull().and("id").all(List.of(oid.toString()))); org.bson.Document mappedObject = mapper.getMappedObject(criteria.getCriteriaObject(), - context.getPersistentEntity(Customer.class)); + context.getPersistentEntity(Customer.class)); assertThat(mappedObject).containsEntry("$and.[0]._id.$all", List.of(oid)); } + @Test // GH-4346 + void propertyValueConverterOnlyGetsInvokedOnMatchingType() { + + Criteria criteria = new Criteria().andOperator(where("text").regex("abc")); + org.bson.Document mappedObject = mapper.getMappedObject(criteria.getCriteriaObject(), + context.getPersistentEntity(WithPropertyValueConverter.class)); + + org.bson.Document parsedExpected = org.bson.Document.parse("{ '$and' : [{ 'text' : /abc/ }] }"); + + // We are comparing BsonDocument instances instead of Document instances, because Document.parse applies a Codec, + // which the ObjectMapper doesn't, yielding slightly different results. + // converting both to BsonDocuments applies the Codec to both. + assertThat(mappedObject.toBsonDocument()).isEqualTo(parsedExpected.toBsonDocument()); + } + + @Test // GH-4346 + void nullConversionIsApplied(){ + + org.bson.Document mappedObject = mapper.getMappedObject(new org.bson.Document("text", null), + context.getPersistentEntity(WithNullReplacingPropertyValueConverter.class)); + + assertThat(mappedObject).isEqualTo(new org.bson.Document("text", "W")); + } + + @Test // GH-4346 + void nullConversionIsAppliedToLists(){ + + List listOfStrings = new ArrayList<>(); + listOfStrings.add("alpha"); + listOfStrings.add(null); + listOfStrings.add("beta"); + + org.bson.Document mappedObject = mapper.getMappedObject(new org.bson.Document("text", listOfStrings), + context.getPersistentEntity(WithNullReplacingPropertyValueConverter.class)); + + assertThat(mappedObject).isEqualTo(new org.bson.Document("text", List.of("alpha", "W", "beta"))); + } + class WithSimpleMap { Map simpleMap; } @@ -2021,6 +2051,16 @@ static class WithPropertyValueConverter { @ValueConverter(ReversingValueConverter.class) String text; } + static class WithNullReplacingPropertyValueConverter { + + @ValueConverter(NullReplacingValueConverter.class) String text; + } + + static class WithNullReplacingPropertyValueConverterOnList { + + @ValueConverter(NullReplacingValueConverter.class) List text; + } + @WritingConverter public static class MyAddressToDocumentConverter implements Converter { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReversingValueConverter.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReversingValueConverter.java index 0fe791784d..1812df766e 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReversingValueConverter.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReversingValueConverter.java @@ -24,22 +24,17 @@ class ReversingValueConverter implements MongoValueConverter { @Nullable @Override - public String read(@Nullable String value, MongoConversionContext context) { + public String read(String value, MongoConversionContext context) { return reverse(value); } @Nullable @Override - public String write(@Nullable String value, MongoConversionContext context) { + public String write(String value, MongoConversionContext context) { return reverse(value); } private String reverse(String source) { - - if (source == null) { - return null; - } - return new StringBuilder(source).reverse().toString(); } }