Skip to content

Commit f8aa915

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

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
@@ -49,7 +49,9 @@
4949
import org.springframework.core.convert.support.DefaultConversionService;
5050
import org.springframework.data.annotation.Reference;
5151
import org.springframework.data.convert.CustomConversions;
52+
import org.springframework.data.convert.PropertyValueConverter;
5253
import org.springframework.data.convert.TypeMapper;
54+
import org.springframework.data.convert.ValueConversionContext;
5355
import org.springframework.data.mapping.Association;
5456
import org.springframework.data.mapping.InstanceCreatorMetadata;
5557
import org.springframework.data.mapping.MappingException;
@@ -164,12 +166,11 @@ public MappingMongoConverter(DbRefResolver dbRefResolver,
164166
this.idMapper = new QueryMapper(this);
165167

166168
this.spELContext = new SpELContext(DocumentPropertyAccessor.INSTANCE);
167-
this.dbRefProxyHandler = new DefaultDbRefProxyHandler(spELContext, mappingContext,
168-
(prop, bson, evaluator, path) -> {
169+
this.dbRefProxyHandler = new DefaultDbRefProxyHandler(spELContext, mappingContext, (prop, bson, evaluator, path) -> {
169170

170-
ConversionContext context = getConversionContext(path);
171-
return MappingMongoConverter.this.getValueInternal(context, prop, bson, evaluator);
172-
});
171+
ConversionContext context = getConversionContext(path);
172+
return MappingMongoConverter.this.getValueInternal(context, prop, bson, evaluator);
173+
});
173174

174175
this.referenceLookupDelegate = new ReferenceLookupDelegate(mappingContext, spELContext);
175176
this.documentPointerFactory = new DocumentPointerFactory(conversionService, mappingContext);
@@ -880,7 +881,10 @@ private void writeProperties(Bson bson, MongoPersistentEntity<?> entity, Persist
880881
Object value = accessor.getProperty(prop);
881882

882883
if (value == null) {
883-
if (prop.writeNullValues()) {
884+
885+
if (conversions.hasValueConverter(prop)) {
886+
dbObjectAccessor.put(prop, applyPropertyConversion(null, prop, accessor));
887+
} else {
884888
dbObjectAccessor.put(prop, null);
885889
}
886890
} else if (!conversions.isSimpleType(value.getClass())) {
@@ -918,14 +922,7 @@ protected void writePropertyInternal(@Nullable Object obj, DocumentAccessor acce
918922
TypeInformation<?> type = prop.getTypeInformation();
919923

920924
if (conversions.hasValueConverter(prop)) {
921-
accessor.put(prop, conversions.getPropertyValueConversions().getValueConverter(prop).write(obj,
922-
new MongoConversionContext(new PropertyValueProvider<>() {
923-
@Nullable
924-
@Override
925-
public <T> T getPropertyValue(MongoPersistentProperty property) {
926-
return (T) persistentPropertyAccessor.getProperty(property);
927-
}
928-
}, prop, this, spELContext)));
925+
accessor.put(prop, applyPropertyConversion(obj, prop, persistentPropertyAccessor));
929926
return;
930927
}
931928

@@ -964,8 +961,8 @@ public <T> T getPropertyValue(MongoPersistentProperty property) {
964961
dbRefObj = proxy.toDBRef();
965962
}
966963

967-
if(obj !=null && conversions.hasCustomWriteTarget(obj.getClass())) {
968-
accessor.withCheckFieldMapping(true).put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get()));
964+
if (obj != null && conversions.hasCustomWriteTarget(obj.getClass())) {
965+
accessor.put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get()));
969966
return;
970967
}
971968

@@ -1267,24 +1264,34 @@ private void writeSimpleInternal(@Nullable Object value, Bson bson, String key)
12671264
private void writeSimpleInternal(@Nullable Object value, Bson bson, MongoPersistentProperty property,
12681265
PersistentPropertyAccessor<?> persistentPropertyAccessor) {
12691266

1270-
DocumentAccessor accessor = new DocumentAccessor(bson).withCheckFieldMapping(true);
1267+
DocumentAccessor accessor = new DocumentAccessor(bson);
12711268

12721269
if (conversions.hasValueConverter(property)) {
1273-
accessor.put(property, conversions.getPropertyValueConversions().getValueConverter(property).write(value,
1274-
new MongoConversionContext(new PropertyValueProvider<>() {
1275-
@Nullable
1276-
@Override
1277-
public <T> T getPropertyValue(MongoPersistentProperty property) {
1278-
return (T) persistentPropertyAccessor.getProperty(property);
1279-
}
1280-
}, property, this, spELContext)));
1270+
accessor.put(property, applyPropertyConversion(value, property, persistentPropertyAccessor));
12811271
return;
12821272
}
12831273

12841274
accessor.put(property, getPotentiallyConvertedSimpleWrite(value,
12851275
property.hasExplicitWriteTarget() ? property.getFieldType() : Object.class));
12861276
}
12871277

1278+
@Nullable
1279+
@SuppressWarnings("unchecked")
1280+
private Object applyPropertyConversion(@Nullable Object value, MongoPersistentProperty property,
1281+
PersistentPropertyAccessor<?> persistentPropertyAccessor) {
1282+
MongoConversionContext context = new MongoConversionContext(new PropertyValueProvider<>() {
1283+
1284+
@Nullable
1285+
@Override
1286+
public <T> T getPropertyValue(MongoPersistentProperty property) {
1287+
return (T) persistentPropertyAccessor.getProperty(property);
1288+
}
1289+
}, property, this, spELContext);
1290+
PropertyValueConverter<Object, Object, ValueConversionContext<MongoPersistentProperty>> valueConverter = conversions
1291+
.getPropertyValueConversions().getValueConverter(property);
1292+
return value != null ? valueConverter.write(value, context) : valueConverter.writeNull(context);
1293+
}
1294+
12881295
/**
12891296
* Checks whether we have a custom conversion registered for the given value into an arbitrary simple Mongo type.
12901297
* Returns the converted value if so. If not, we perform special enum handling or simply return the value as is.
@@ -1925,14 +1932,18 @@ public <T> T getPropertyValue(MongoPersistentProperty property) {
19251932
String expression = property.getSpelExpression();
19261933
Object value = expression != null ? evaluator.evaluate(expression) : accessor.get(property);
19271934

1928-
if (value == null) {
1929-
return null;
1930-
}
1931-
19321935
CustomConversions conversions = context.getCustomConversions();
19331936
if (conversions.hasValueConverter(property)) {
1934-
return (T) conversions.getPropertyValueConversions().getValueConverter(property).read(value,
1935-
new MongoConversionContext(this, property, context.getSourceConverter(), spELContext));
1937+
MongoConversionContext conversionContext = new MongoConversionContext(this, property,
1938+
context.getSourceConverter(), spELContext);
1939+
PropertyValueConverter<Object, Object, ValueConversionContext<MongoPersistentProperty>> valueConverter = conversions
1940+
.getPropertyValueConversions().getValueConverter(property);
1941+
return (T) (value != null ? valueConverter.read(value, conversionContext)
1942+
: valueConverter.readNull(conversionContext));
1943+
}
1944+
1945+
if (value == null) {
1946+
return null;
19361947
}
19371948

19381949
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)