From d6fd555eb0f4d54dbef41116974bebe315e2b8d0 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 10 Nov 2025 10:03:14 +0100 Subject: [PATCH 1/2] Prepare branch 4346-property-value-convert-cce --- pom.xml | 2 +- spring-data-mongodb-distribution/pom.xml | 2 +- spring-data-mongodb/pom.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 From 5e5459b383ff5c2d6d554ed9d0f083d6c6cb24e7 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 10 Nov 2025 10:14:08 +0100 Subject: [PATCH 2/2] Invoke PropertyValueConverter only for matching types. When the type does not match we simply use the unconverted value. This happens when comparing properties to regexes. Applying the conversion to the String value of the regex doesn't makes sense since we are not dealing with complete values. Users may still provide converters taking Objects and mangle Regex patterns as they desire. Also fixed the null handling of PropertyValueConverters. Removed a few superfluous `@Nullable` annotations where they became obvious Closes #4346 --- .../mongodb/core/convert/QueryMapper.java | 38 ++++++---- .../MappingMongoConverterUnitTests.java | 21 +++++- .../convert/NullReplacingValueConverter.java | 44 ++++++++++++ .../core/convert/QueryMapperUnitTests.java | 70 +++++++++++++++---- .../core/convert/ReversingValueConverter.java | 9 +-- 5 files changed, 145 insertions(+), 37 deletions(-) create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/NullReplacingValueConverter.java 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(); } }