Skip to content

Commit e08cf35

Browse files
mp911demp911de
committed
Refactor DocumentAccessor, consider readNull/writeNull for null values.
Original pull request: #4728 See #4710
1 parent 2e503cd commit e08cf35

File tree

3 files changed

+109
-56
lines changed

3 files changed

+109
-56
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java

+4-16
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ public void put(MongoPersistentProperty prop, @Nullable Object value) {
9292

9393
Assert.notNull(prop, "MongoPersistentProperty must not be null");
9494

95+
if (value == null && !prop.writeNullValues()) {
96+
return;
97+
}
98+
9599
Iterator<String> parts = Arrays.asList(prop.getMongoField().getName().parts()).iterator();
96100
Bson document = this.document;
97101

@@ -173,20 +177,4 @@ private static Document getOrCreateNestedDocument(String key, Bson source) {
173177
return nested;
174178
}
175179

176-
DocumentAccessor withCheckFieldMapping(boolean checkFieldMapping) {
177-
178-
if(!checkFieldMapping) {
179-
return this;
180-
}
181-
182-
return new DocumentAccessor(this.document) {
183-
@Override
184-
public void put(MongoPersistentProperty prop, @Nullable Object value) {
185-
if(value != null || prop.writeNullValues()) {
186-
super.put(prop, value);
187-
}
188-
}
189-
};
190-
191-
}
192180
}

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

+42-31
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@
5353
import org.springframework.core.env.StandardEnvironment;
5454
import org.springframework.data.annotation.Reference;
5555
import org.springframework.data.convert.CustomConversions;
56+
import org.springframework.data.convert.PropertyValueConverter;
5657
import org.springframework.data.convert.TypeMapper;
58+
import org.springframework.data.convert.ValueConversionContext;
5759
import org.springframework.data.mapping.Association;
5860
import org.springframework.data.mapping.InstanceCreatorMetadata;
5961
import org.springframework.data.mapping.MappingException;
@@ -176,12 +178,11 @@ public MappingMongoConverter(DbRefResolver dbRefResolver,
176178
this.idMapper = new QueryMapper(this);
177179

178180
this.spELContext = new SpELContext(DocumentPropertyAccessor.INSTANCE);
179-
this.dbRefProxyHandler = new DefaultDbRefProxyHandler(mappingContext,
180-
(prop, bson, evaluator, path) -> {
181+
this.dbRefProxyHandler = new DefaultDbRefProxyHandler(mappingContext, (prop, bson, evaluator, path) -> {
181182

182-
ConversionContext context = getConversionContext(path);
183-
return MappingMongoConverter.this.getValueInternal(context, prop, bson, evaluator);
184-
}, expressionEvaluatorFactory::create);
183+
ConversionContext context = getConversionContext(path);
184+
return MappingMongoConverter.this.getValueInternal(context, prop, bson, evaluator);
185+
}, expressionEvaluatorFactory::create);
185186

186187
this.referenceLookupDelegate = new ReferenceLookupDelegate(mappingContext, spELContext);
187188
this.documentPointerFactory = new DocumentPointerFactory(conversionService, mappingContext);
@@ -903,7 +904,10 @@ private void writeProperties(Bson bson, MongoPersistentEntity<?> entity, Persist
903904
Object value = accessor.getProperty(prop);
904905

905906
if (value == null) {
906-
if (prop.writeNullValues()) {
907+
908+
if (conversions.hasValueConverter(prop)) {
909+
dbObjectAccessor.put(prop, applyPropertyConversion(null, prop, accessor));
910+
} else {
907911
dbObjectAccessor.put(prop, null);
908912
}
909913
} else if (!conversions.isSimpleType(value.getClass())) {
@@ -941,14 +945,7 @@ protected void writePropertyInternal(@Nullable Object obj, DocumentAccessor acce
941945
TypeInformation<?> type = prop.getTypeInformation();
942946

943947
if (conversions.hasValueConverter(prop)) {
944-
accessor.put(prop, conversions.getPropertyValueConversions().getValueConverter(prop).write(obj,
945-
new MongoConversionContext(new PropertyValueProvider<>() {
946-
@Nullable
947-
@Override
948-
public <T> T getPropertyValue(MongoPersistentProperty property) {
949-
return (T) persistentPropertyAccessor.getProperty(property);
950-
}
951-
}, prop, this, spELContext)));
948+
accessor.put(prop, applyPropertyConversion(obj, prop, persistentPropertyAccessor));
952949
return;
953950
}
954951

@@ -987,8 +984,8 @@ public <T> T getPropertyValue(MongoPersistentProperty property) {
987984
dbRefObj = proxy.toDBRef();
988985
}
989986

990-
if(obj !=null && conversions.hasCustomWriteTarget(obj.getClass())) {
991-
accessor.withCheckFieldMapping(true).put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get()));
987+
if (obj != null && conversions.hasCustomWriteTarget(obj.getClass())) {
988+
accessor.put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get()));
992989
return;
993990
}
994991

@@ -1290,24 +1287,34 @@ private void writeSimpleInternal(@Nullable Object value, Bson bson, String key)
12901287
private void writeSimpleInternal(@Nullable Object value, Bson bson, MongoPersistentProperty property,
12911288
PersistentPropertyAccessor<?> persistentPropertyAccessor) {
12921289

1293-
DocumentAccessor accessor = new DocumentAccessor(bson).withCheckFieldMapping(true);
1290+
DocumentAccessor accessor = new DocumentAccessor(bson);
12941291

12951292
if (conversions.hasValueConverter(property)) {
1296-
accessor.put(property, conversions.getPropertyValueConversions().getValueConverter(property).write(value,
1297-
new MongoConversionContext(new PropertyValueProvider<>() {
1298-
@Nullable
1299-
@Override
1300-
public <T> T getPropertyValue(MongoPersistentProperty property) {
1301-
return (T) persistentPropertyAccessor.getProperty(property);
1302-
}
1303-
}, property, this, spELContext)));
1293+
accessor.put(property, applyPropertyConversion(value, property, persistentPropertyAccessor));
13041294
return;
13051295
}
13061296

13071297
accessor.put(property, getPotentiallyConvertedSimpleWrite(value,
13081298
property.hasExplicitWriteTarget() ? property.getFieldType() : Object.class));
13091299
}
13101300

1301+
@Nullable
1302+
@SuppressWarnings("unchecked")
1303+
private Object applyPropertyConversion(@Nullable Object value, MongoPersistentProperty property,
1304+
PersistentPropertyAccessor<?> persistentPropertyAccessor) {
1305+
MongoConversionContext context = new MongoConversionContext(new PropertyValueProvider<>() {
1306+
1307+
@Nullable
1308+
@Override
1309+
public <T> T getPropertyValue(MongoPersistentProperty property) {
1310+
return (T) persistentPropertyAccessor.getProperty(property);
1311+
}
1312+
}, property, this, spELContext);
1313+
PropertyValueConverter<Object, Object, ValueConversionContext<MongoPersistentProperty>> valueConverter = conversions
1314+
.getPropertyValueConversions().getValueConverter(property);
1315+
return value != null ? valueConverter.write(value, context) : valueConverter.writeNull(context);
1316+
}
1317+
13111318
/**
13121319
* Checks whether we have a custom conversion registered for the given value into an arbitrary simple Mongo type.
13131320
* Returns the converted value if so. If not, we perform special enum handling or simply return the value as is.
@@ -1948,14 +1955,18 @@ public <T> T getPropertyValue(MongoPersistentProperty property) {
19481955
String expression = property.getSpelExpression();
19491956
Object value = expression != null ? evaluator.evaluate(expression) : accessor.get(property);
19501957

1951-
if (value == null) {
1952-
return null;
1953-
}
1954-
19551958
CustomConversions conversions = context.getCustomConversions();
19561959
if (conversions.hasValueConverter(property)) {
1957-
return (T) conversions.getPropertyValueConversions().getValueConverter(property).read(value,
1958-
new MongoConversionContext(this, property, context.getSourceConverter(), spELContext));
1960+
MongoConversionContext conversionContext = new MongoConversionContext(this, property,
1961+
context.getSourceConverter(), spELContext);
1962+
PropertyValueConverter<Object, Object, ValueConversionContext<MongoPersistentProperty>> valueConverter = conversions
1963+
.getPropertyValueConversions().getValueConverter(property);
1964+
return (T) (value != null ? valueConverter.read(value, conversionContext)
1965+
: valueConverter.readNull(conversionContext));
1966+
}
1967+
1968+
if (value == null) {
1969+
return null;
19591970
}
19601971

19611972
ConversionContext contextToUse = context.forProperty(property);

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java

+63-9
Original file line numberDiff line numberDiff line change
@@ -2700,7 +2700,8 @@ void shouldWriteNullPropertyCorrectly() {
27002700
@Test // GH-4710
27012701
void shouldWriteSimplePropertyCorrectlyAfterConversionReturnsNull() {
27022702

2703-
MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Integer.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList());
2703+
MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder
2704+
.writing(Integer.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList());
27042705

27052706
converter = new MappingMongoConverter(resolver, mappingContext);
27062707
converter.setCustomConversions(conversions);
@@ -2719,7 +2720,8 @@ void shouldWriteSimplePropertyCorrectlyAfterConversionReturnsNull() {
27192720
@Test // GH-4710
27202721
void shouldWriteComplexPropertyCorrectlyAfterConversionReturnsNull() {
27212722

2722-
MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Person.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList());
2723+
MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder
2724+
.writing(Person.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList());
27232725

27242726
converter = new MappingMongoConverter(resolver, mappingContext);
27252727
converter.setCustomConversions(conversions);
@@ -2738,7 +2740,9 @@ void shouldWriteComplexPropertyCorrectlyAfterConversionReturnsNull() {
27382740
@Test // GH-4710
27392741
void shouldDelegateWriteOfDBRefToCustomConversionIfConfigured() {
27402742

2741-
MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Person.class, DBRef.class, it -> new DBRef("persons", "n/a")).andReading(it -> null).getConverters().stream().toList());
2743+
MongoCustomConversions conversions = new MongoCustomConversions(
2744+
ConverterBuilder.writing(Person.class, DBRef.class, it -> new DBRef("persons", "n/a")).andReading(it -> null)
2745+
.getConverters().stream().toList());
27422746

27432747
converter = new MappingMongoConverter(resolver, mappingContext);
27442748
converter.setCustomConversions(conversions);
@@ -2751,13 +2755,14 @@ void shouldDelegateWriteOfDBRefToCustomConversionIfConfigured() {
27512755
org.bson.Document document = new org.bson.Document();
27522756
converter.write(fieldWrite, document);
27532757

2754-
assertThat(document).containsEntry("writeAlwaysPersonDBRef", new DBRef("persons", "n/a"));//.doesNotContainKey("writeNonNullPersonDBRef");
2758+
assertThat(document).containsEntry("writeAlwaysPersonDBRef", new DBRef("persons", "n/a"));// .doesNotContainKey("writeNonNullPersonDBRef");
27552759
}
27562760

27572761
@Test // GH-4710
27582762
void shouldDelegateWriteOfDBRefToCustomConversionIfConfiguredAndCheckNulls() {
27592763

2760-
MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Person.class, DBRef.class, it -> null).andReading(it -> null).getConverters().stream().toList());
2764+
MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder
2765+
.writing(Person.class, DBRef.class, it -> null).andReading(it -> null).getConverters().stream().toList());
27612766

27622767
converter = new MappingMongoConverter(resolver, mappingContext);
27632768
converter.setCustomConversions(conversions);
@@ -2773,6 +2778,50 @@ void shouldDelegateWriteOfDBRefToCustomConversionIfConfiguredAndCheckNulls() {
27732778
assertThat(document).containsEntry("writeAlwaysPersonDBRef", null).doesNotContainKey("writeNonNullPersonDBRef");
27742779
}
27752780

2781+
@Test // GH-4710
2782+
void shouldApplyNullConversionToPropertyValueConverters() {
2783+
2784+
MongoCustomConversions conversions = new MongoCustomConversions(
2785+
MongoCustomConversions.MongoConverterConfigurationAdapter.from(Collections.emptyList())
2786+
.configurePropertyConversions(registrar -> {
2787+
registrar.registerConverter(Person.class, "firstname", new MongoValueConverter<String, String>() {
2788+
@Override
2789+
public String readNull(MongoConversionContext context) {
2790+
return "NULL";
2791+
}
2792+
2793+
@Override
2794+
public String writeNull(MongoConversionContext context) {
2795+
return "NULL";
2796+
}
2797+
2798+
@Override
2799+
public String read(String value, MongoConversionContext context) {
2800+
return "";
2801+
}
2802+
2803+
@Override
2804+
public String write(String value, MongoConversionContext context) {
2805+
return "";
2806+
}
2807+
});
2808+
}));
2809+
2810+
converter = new MappingMongoConverter(resolver, mappingContext);
2811+
converter.setCustomConversions(conversions);
2812+
converter.afterPropertiesSet();
2813+
2814+
org.bson.Document document = new org.bson.Document();
2815+
converter.write(new Person(), document);
2816+
2817+
assertThat(document).containsEntry("foo", "NULL");
2818+
2819+
document = new org.bson.Document("foo", null);
2820+
Person result = converter.read(Person.class, document);
2821+
2822+
assertThat(result.firstname).isEqualTo("NULL");
2823+
}
2824+
27762825
@Test // GH-3686
27772826
void readsCollectionContainingNullValue() {
27782827

@@ -3086,7 +3135,7 @@ void beanConverter() {
30863135
}));
30873136
converter.afterPropertiesSet();
30883137

3089-
WithValueConverters wvc = new WithValueConverters();
3138+
WithContextValueConverters wvc = new WithContextValueConverters();
30903139
wvc.converterBean = "spring";
30913140

30923141
org.bson.Document target = new org.bson.Document();
@@ -3097,7 +3146,7 @@ void beanConverter() {
30973146
assertThat((String) it.get("ooo")).startsWith("spring - ");
30983147
});
30993148

3100-
WithValueConverters read = converter.read(WithValueConverters.class, target);
3149+
WithContextValueConverters read = converter.read(WithContextValueConverters.class, target);
31013150
assertThat(read.converterBean).startsWith("spring -");
31023151
}
31033152

@@ -4180,10 +4229,10 @@ static class WithFieldWrite {
41804229
write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Integer writeAlways;
41814230

41824231
@org.springframework.data.mongodb.core.mapping.Field(
4183-
write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson;
4232+
write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson;
41844233

41854234
@org.springframework.data.mongodb.core.mapping.Field(
4186-
write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson;
4235+
write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson;
41874236

41884237
@org.springframework.data.mongodb.core.mapping.DBRef
41894238
@org.springframework.data.mongodb.core.mapping.Field(
@@ -4201,6 +4250,11 @@ static class WithValueConverters {
42014250

42024251
@ValueConverter(Converter2.class) String converterEnum;
42034252

4253+
String viaRegisteredConverter;
4254+
}
4255+
4256+
static class WithContextValueConverters {
4257+
42044258
@ValueConverter(Converter3.class) String converterBean;
42054259

42064260
String viaRegisteredConverter;

0 commit comments

Comments
 (0)