Consider null value settings for types with custom conversion.

This commit fixes an issue where settings regarding storage of null values had been ignored if a custom converter took care of the conversion. Original pull request: #4728 Closes #4710
spring-projects · Aug 28, 2024 · 8077b18 · 8077b18
1 parent 3ec515a
commit 8077b18
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 3 deletions.
diff --git a/...mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java b/...mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java
@@ -172,4 +172,21 @@ private static Document getOrCreateNestedDocument(String key, Bson source) {
 
 		return nested;
 	}
+
+	DocumentAccessor withCheckFieldMapping(boolean checkFieldMapping) {
+
+		if(!checkFieldMapping) {
+			return this;
+		}
+
+		return new DocumentAccessor(this.document) {
+				@Override
+				public void put(MongoPersistentProperty prop, @Nullable Object value) {
+					if(value != null || prop.writeNullValues()) {
+						super.put(prop, value);
+					}
+				}
+			};
+
+	}
 }
diff --git a/...db/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/...db/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java
@@ -964,6 +964,11 @@ public <T> T getPropertyValue(MongoPersistentProperty property) {
 				dbRefObj = proxy.toDBRef();
 			}
 
+			if(obj !=null && conversions.hasCustomWriteTarget(obj.getClass())) {
+				accessor.withCheckFieldMapping(true).put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get()));
+				return;
+			}
+
 			dbRefObj = dbRefObj != null ? dbRefObj : createDBRef(obj, prop);
 
 			accessor.put(prop, dbRefObj);
@@ -1261,7 +1266,8 @@ private void writeSimpleInternal(@Nullable Object value, Bson bson, String key)
 
 	private void writeSimpleInternal(@Nullable Object value, Bson bson, MongoPersistentProperty property,
 			PersistentPropertyAccessor<?> persistentPropertyAccessor) {
-		DocumentAccessor accessor = new DocumentAccessor(bson);
+
+		DocumentAccessor accessor = new DocumentAccessor(bson).withCheckFieldMapping(true);
 
 		if (conversions.hasValueConverter(property)) {
 			accessor.put(property, conversions.getPropertyValueConversions().getValueConverter(property).write(value,

diff --git a/...st/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/...st/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java
@@ -63,6 +63,7 @@
 import org.springframework.data.annotation.PersistenceConstructor;
 import org.springframework.data.annotation.Transient;
 import org.springframework.data.annotation.TypeAlias;
+import org.springframework.data.convert.ConverterBuilder;
 import org.springframework.data.convert.CustomConversions;
 import org.springframework.data.convert.PropertyValueConverter;
 import org.springframework.data.convert.PropertyValueConverterFactory;
@@ -2693,9 +2694,85 @@ void shouldWriteNullPropertyCorrectly() {
 		converter.write(fieldWrite, document);
 
 		assertThat(document).containsEntry("writeAlways", null).doesNotContainKey("writeNonNull");
+		assertThat(document).containsEntry("writeAlwaysPersonDBRef", null).doesNotContainKey("writeNonNullPersonDBRef");
+	}
+
+	@Test // GH-4710
+	void shouldWriteSimplePropertyCorrectlyAfterConversionReturnsNull() {
+
+		MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Integer.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList());
+
+		converter = new MappingMongoConverter(resolver, mappingContext);
+		converter.setCustomConversions(conversions);
+		converter.afterPropertiesSet();
+
+		WithFieldWrite fieldWrite = new WithFieldWrite();
+		fieldWrite.writeAlways = 10;
+		fieldWrite.writeNonNull = 20;
+
+		org.bson.Document document = new org.bson.Document();
+		converter.write(fieldWrite, document);
+
+		assertThat(document).containsEntry("writeAlways", null).doesNotContainKey("writeNonNull");
+	}
+
+	@Test // GH-4710
+	void shouldWriteComplexPropertyCorrectlyAfterConversionReturnsNull() {
+
+		MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Person.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList());
+
+		converter = new MappingMongoConverter(resolver, mappingContext);
+		converter.setCustomConversions(conversions);
+		converter.afterPropertiesSet();
+
+		WithFieldWrite fieldWrite = new WithFieldWrite();
+		fieldWrite.writeAlwaysPerson = new Person();
+		fieldWrite.writeNonNullPerson = new Person();
+
+		org.bson.Document document = new org.bson.Document();
+		converter.write(fieldWrite, document);
+
 		assertThat(document).containsEntry("writeAlwaysPerson", null).doesNotContainKey("writeNonNullPerson");
 	}
 
+	@Test // GH-4710
+	void shouldDelegateWriteOfDBRefToCustomConversionIfConfigured() {
+
+		MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Person.class, DBRef.class, it -> new DBRef("persons", "n/a")).andReading(it -> null).getConverters().stream().toList());
+
+		converter = new MappingMongoConverter(resolver, mappingContext);
+		converter.setCustomConversions(conversions);
+		converter.afterPropertiesSet();
+
+		WithFieldWrite fieldWrite = new WithFieldWrite();
+		fieldWrite.writeAlwaysPersonDBRef = new Person();
+		fieldWrite.writeNonNullPersonDBRef = new Person();
+
+		org.bson.Document document = new org.bson.Document();
+		converter.write(fieldWrite, document);
+
+		assertThat(document).containsEntry("writeAlwaysPersonDBRef", new DBRef("persons", "n/a"));//.doesNotContainKey("writeNonNullPersonDBRef");
+	}
+
+	@Test // GH-4710
+	void shouldDelegateWriteOfDBRefToCustomConversionIfConfiguredAndCheckNulls() {
+
+		MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Person.class, DBRef.class, it -> null).andReading(it -> null).getConverters().stream().toList());
+
+		converter = new MappingMongoConverter(resolver, mappingContext);
+		converter.setCustomConversions(conversions);
+		converter.afterPropertiesSet();
+
+		WithFieldWrite fieldWrite = new WithFieldWrite();
+		fieldWrite.writeAlwaysPersonDBRef = new Person();
+		fieldWrite.writeNonNullPersonDBRef = new Person();
+
+		org.bson.Document document = new org.bson.Document();
+		converter.write(fieldWrite, document);
+
+		assertThat(document).containsEntry("writeAlwaysPersonDBRef", null).doesNotContainKey("writeNonNullPersonDBRef");
+	}
+
 	@Test // GH-3686
 	void readsCollectionContainingNullValue() {
 
@@ -4102,13 +4179,19 @@ static class WithFieldWrite {
 		@org.springframework.data.mongodb.core.mapping.Field(
 				write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Integer writeAlways;
 
+		@org.springframework.data.mongodb.core.mapping.Field(
+			write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson;
+
+		@org.springframework.data.mongodb.core.mapping.Field(
+			write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson;
+
 		@org.springframework.data.mongodb.core.mapping.DBRef
 		@org.springframework.data.mongodb.core.mapping.Field(
-				write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson;
+				write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPersonDBRef;
 
 		@org.springframework.data.mongodb.core.mapping.DBRef
 		@org.springframework.data.mongodb.core.mapping.Field(
-				write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson;
+				write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPersonDBRef;
 
 	}