GH-2280 - Allow custom composite converters to delete decomposed properties.

Custom composite converters must be notified when a property to be decomposed is `null`.
This means we must remove the `nullSafeWrite` decorator for all composite properties first.

To be compatible with existing converters that trust us not to get a `null`, we must check for any NPE raising from such a converter.

A custom `Neo4jPersistentPropertyToMapConverter` needs to set _all_ properties it normally writes when decomposing a property to `Values.NULL`, so that they are removed from the node or relationship in question.

This fixes #2280.

Author: michael-simons

src/main/java/org/springframework/data/neo4j/core/convert/Neo4jPersistentPropertyConverter.java

@@ -19,7 +19,7 @@
 import org.neo4j.driver.Value;
 
 /**
- * This  interface represents a pair of methods capable of converting values of type {@code T} to and from {@link Value values}.
+ * This interface represents a pair of methods capable of converting values of type {@code T} to and from {@link Value values}.
  *
  * @param <T> The type of the property to convert (the type of the actual attribute).
  * @author Michael J. Simons

src/main/java/org/springframework/data/neo4j/core/convert/Neo4jPersistentPropertyToMapConverter.java

@@ -19,6 +19,7 @@
 
 import org.apiguardian.api.API;
 import org.neo4j.driver.Value;
+import org.springframework.lang.Nullable;
 
 /**
  * You need to provide an implementation of this interface in case you want to store a property of an entity as separate
@@ -43,7 +44,7 @@ public interface Neo4jPersistentPropertyToMapConverter<K, P> {
 	 * @param neo4jConversionService The conversion service to delegate to if necessary
 	 * @return The decomposed object.
 	 */
-	Map<K, Value> decompose(P property, Neo4jConversionService neo4jConversionService);
+	Map<K, Value> decompose(@Nullable P property, Neo4jConversionService neo4jConversionService);
 
 	/**
 	 * Composes the object back from the map. The map contains the raw driver values, as SDN cannot know how you want to

src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jConversionService.java

@@ -103,14 +103,20 @@ public Value writeValue(@Nullable Object value, TypeInformation<?> sourceType,
 
 		Function<Object, Value> conversion =
 				writingConverter == null ? v -> conversionService.convert(v, Value.class) : writingConverter;
+
 		return writeValueImpl(value, sourceType, conversion);
 	}
 
 	private Value writeValueImpl(@Nullable Object value, TypeInformation<?> type,
 			Function<Object, Value> conversion) {
 
 		if (value == null) {
-			return Values.NULL;
+			try {
+				// Some conversion services may treat null special, so we pass it anyway and ask for forgiveness
+				return conversion.apply(null);
+			} catch (NullPointerException e) {
+				return Values.NULL;
+			}
 		}
 
 		if (isCollection(type)) {

src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java

@@ -195,7 +195,12 @@ private static Function<Object, Value> nullSafeWrite(Function<Object, Value> del
 
 	@Override
 	public Function<Object, Value> getOptionalWritingConverter() {
-		return customConversion.getOptional().map(c -> nullSafeWrite(c::write)).orElse(null);
+		return customConversion.getOptional()
+				.map(c -> {
+					Function<Object, Value> originalConversion = c::write;
+					return this.isComposite() ? originalConversion : nullSafeWrite(originalConversion);
+				})
+				.orElse(null);
 	}
 
 	private static Function<Value, Object> nullSafeRead(Function<Value, Object> delegate) {

src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentProperty.java

@@ -24,6 +24,7 @@
 import org.springframework.data.neo4j.core.schema.CompositeProperty;
 import org.springframework.data.neo4j.core.schema.DynamicLabels;
 import org.springframework.data.neo4j.core.schema.RelationshipProperties;
+import org.springframework.lang.Nullable;
 
 /**
  * A {@link org.springframework.data.mapping.PersistentProperty} interface with additional methods for metadata related
@@ -76,8 +77,10 @@ default boolean isRelationshipWithProperties() {
 		return isAssociation() && isCollectionLike() && getActualType().isAnnotationPresent(RelationshipProperties.class);
 	}
 
+	@Nullable
 	Function<Object, Value> getOptionalWritingConverter();
 
+	@Nullable
 	Function<Value, Object> getOptionalReadingConverter();
 
 	/**

src/main/java/org/springframework/data/neo4j/core/schema/CompositeProperty.java

@@ -21,6 +21,7 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.lang.reflect.Type;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.BiFunction;
@@ -128,7 +129,12 @@ final class DefaultToMapConverter<K> implements Neo4jPersistentPropertyToMapConv
 		}
 
 		@Override
-		public Map<K, Value> decompose(Map<K, Object> property, Neo4jConversionService conversionService) {
+		public Map<K, Value> decompose(@Nullable Map<K, Object> property, Neo4jConversionService conversionService) {
+
+			if (property == null) {
+				return Collections.emptyMap();
+			}
+
 			Map<K, Value> decomposed = new HashMap<>(property.size());
 			property.forEach(
 					(k, v) -> decomposed.put(k, conversionService.writeValue(v, typeInformationForValues, null)));

src/test/java/org/springframework/data/neo4j/integration/conversion_imperative/ImperativeCompositePropertiesIT.java

@@ -19,6 +19,9 @@
 
 import org.junit.jupiter.api.Test;
 import org.neo4j.driver.Driver;
+import org.neo4j.driver.Record;
+import org.neo4j.driver.Session;
+import org.neo4j.driver.types.Node;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -76,6 +79,28 @@ void compositePropertiesOnNodesShouldBeRead(@Autowired Repository repository) {
 		assertThat(repository.findById(id)).isPresent().hasValueSatisfying(this::assertNodePropertiesOn);
 	}
 
+	@Test // GH-2280
+	void compositePropertiesOnNodesShouldBeDeleted(@Autowired Repository repository) {
+
+		Long id = createNodeWithCompositeProperties();
+		ThingWithCompositeProperties thing = repository.findById(id).get();
+		thing.setDatesWithTransformedKey(Collections.singletonMap("Test", null));
+		thing.setSomeDatesByEnumA(Collections.singletonMap(ThingWithCompositeProperties.EnumA.VALUE_AA, null));
+		thing.setSomeOtherDTO(null);
+		repository.save(thing);
+
+		try (Session session = driver.session()) {
+			Record r = session.readTransaction(tx -> tx.run("MATCH (t:CompositeProperties) WHERE id(t) = $id RETURN t",
+					Collections.singletonMap("id", id)).single());
+			Node n = r.get("t").asNode();
+			assertThat(n.asMap()).doesNotContainKeys(
+					"someDatesByEnumA.VALUE_AA",
+					"datesWithTransformedKey.test",
+					"dto.x", "dto.y", "dto.z"
+			);
+		}
+	}
+
 	public interface Repository extends Neo4jRepository<ThingWithCompositeProperties, Long> {
 	}
 

src/test/java/org/springframework/data/neo4j/integration/shared/conversion/ThingWithCompositeProperties.java

@@ -31,6 +31,8 @@
 import org.springframework.data.neo4j.core.schema.Id;
 import org.springframework.data.neo4j.core.schema.Node;
 import org.springframework.data.neo4j.core.schema.Relationship;
+import org.springframework.lang.NonNull;
+import org.springframework.lang.Nullable;
 
 /**
  * Test class verifying composite properties behaviour.
@@ -235,12 +237,19 @@ public String apply(CompositeProperty.Phase phase, String s) {
 	 */
 	static class SomeOtherDTOToMapConverter implements Neo4jPersistentPropertyToMapConverter<String, SomeOtherDTO> {
 
-		@Override
-		public Map<String, Value> decompose(SomeOtherDTO property, Neo4jConversionService conversionService) {
+		@NonNull @Override
+		public Map<String, Value> decompose(@Nullable SomeOtherDTO property, Neo4jConversionService conversionService) {
+
 			final HashMap<String, Value> decomposed = new HashMap<>();
-			decomposed.put("x", Values.value(property.x));
-			decomposed.put("y", Values.value(property.y));
-			decomposed.put("z", Values.value(property.z));
+			if (property == null) {
+				decomposed.put("x", Values.NULL);
+				decomposed.put("y", Values.NULL);
+				decomposed.put("z", Values.NULL);
+			} else {
+				decomposed.put("x", Values.value(property.x));
+				decomposed.put("y", Values.value(property.y));
+				decomposed.put("z", Values.value(property.z));
+			}
 			return decomposed;
 		}