GH-2365 - Apply custom converters to additional fields of DTO projections, too.

michael-simons · michael-simons · commit ef91d6d197d3 · 2021-08-30T12:50:18.000+02:00
Closes #2365.
diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java
@@ -28,6 +28,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.BiFunction;
@@ -279,6 +280,7 @@ <T, R> List<R> doFind(@Nullable String cypherQuery, @Nullable Map<String, Object
 				.map(EntityInstanceWithSource.class::cast)
 				.map(converter::convert)
 				.map(v -> (R) v)
+				.filter(Objects::nonNull)
 				.collect(Collectors.toList());
 	}
 
diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java
@@ -254,7 +254,7 @@ <T, R> Flux<R> doFind(@Nullable String cypherQuery, @Nullable Map<String, Object
 
 		DtoInstantiatingConverter converter = new DtoInstantiatingConverter(resultType, neo4jMappingContext);
 		return (Flux<R>) intermediaResults.map(EntityInstanceWithSource.class::cast)
-				.map(converter::convert);
+				.mapNotNull(converter::convert);
 	}
 
 	@Override
diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DtoInstantiatingConverter.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DtoInstantiatingConverter.java
@@ -89,7 +89,7 @@ public Object convertDirectly(Object entityInstance) {
 		PersistentPropertyAccessor<Object> dtoAccessor = targetEntity.getPropertyAccessor(dto);
 		targetEntity.doWithProperties((SimplePropertyHandler) property -> {
 
-			if (constructor.isConstructorParameter(property)) {
+			if (constructor != null && constructor.isConstructorParameter(property)) {
 				return;
 			}
 
@@ -108,11 +108,14 @@ Object getPropertyValueDirectlyFor(PersistentProperty<?> targetProperty, Persist
 		String targetPropertyName = targetProperty.getName();
 		PersistentProperty<?> sourceProperty = sourceEntity.getPersistentProperty(targetPropertyName);
 
+		if (sourceProperty == null) {
+			return null;
+		}
 		return sourceAccessor.getProperty(sourceProperty);
 	}
 
 	@Override
-	public Object convert(EntityInstanceWithSource entityInstanceAndSource) {
+	public Object convert(@Nullable EntityInstanceWithSource entityInstanceAndSource) {
 
 		if (entityInstanceAndSource == null) {
 			return null;
@@ -126,7 +129,8 @@ public Object convert(EntityInstanceWithSource entityInstanceAndSource) {
 		Neo4jPersistentEntity<?> sourceEntity = context.getRequiredPersistentEntity(entityInstance.getClass());
 		PersistentPropertyAccessor<Object> sourceAccessor = sourceEntity.getPropertyAccessor(entityInstance);
 
-		Neo4jPersistentEntity<?> targetEntity = context.addPersistentEntity(ClassTypeInformation.from(targetType)).get();
+		Neo4jPersistentEntity<?> targetEntity = context.addPersistentEntity(ClassTypeInformation.from(targetType))
+														.orElseThrow(() -> new MappingException("Could not add a persistent entity for the projection target type '" + targetType.getName() + "'."));
 		PreferredConstructor<?, ? extends PersistentProperty<?>> constructor = targetEntity
 				.getPersistenceConstructor();
 
@@ -146,7 +150,7 @@ public Object convert(EntityInstanceWithSource entityInstanceAndSource) {
 
 	private ParameterValueProvider<Neo4jPersistentProperty> getParameterValueProvider(
 			Neo4jPersistentEntity<?> targetEntity,
-			Function<PersistentProperty<?>, Object> extractFromSource
+			Function<Neo4jPersistentProperty, Object> extractFromSource
 	) {
 		return new ParameterValueProvider<Neo4jPersistentProperty>() {
 			@SuppressWarnings("unchecked") // Needed for the last cast. It's easier that way than using the parameter type info and checking for primivites
@@ -157,7 +161,7 @@ public <T> T getParameterValue(Parameter<T, Neo4jPersistentProperty> parameter)
 					throw new MappingException(
 							"Constructor parameter names aren't available, please recompile your domain.");
 				}
-				PersistentProperty<?> targetProperty = targetEntity.getPersistentProperty(parameterName);
+				Neo4jPersistentProperty targetProperty = targetEntity.getPersistentProperty(parameterName);
 				if (targetProperty == null) {
 					throw new MappingException("Cannot map constructor parameter " + parameterName
 											   + " to a property of class " + targetType);
@@ -168,10 +172,10 @@ public <T> T getParameterValue(Parameter<T, Neo4jPersistentProperty> parameter)
 	}
 
 	private void setPropertyOnDtoObject(EntityInstanceWithSource entityInstanceAndSource, PersistentEntity<?, ?> sourceEntity,
-			PersistentPropertyAccessor<Object> sourceAccessor, PreferredConstructor<?, ?> constructor,
-			PersistentPropertyAccessor<Object> dtoAccessor, PersistentProperty<?> property) {
+			PersistentPropertyAccessor<Object> sourceAccessor, @Nullable PreferredConstructor<?, ?> constructor,
+			PersistentPropertyAccessor<Object> dtoAccessor, Neo4jPersistentProperty property) {
 
-		if (constructor.isConstructorParameter(property)) {
+		if (constructor != null && constructor.isConstructorParameter(property)) {
 			return;
 		}
 
@@ -180,7 +184,7 @@ private void setPropertyOnDtoObject(EntityInstanceWithSource entityInstanceAndSo
 	}
 
 	@Nullable
-	Object getPropertyValueFor(PersistentProperty<?> targetProperty, PersistentEntity<?, ?> sourceEntity,
+	Object getPropertyValueFor(Neo4jPersistentProperty targetProperty, PersistentEntity<?, ?> sourceEntity,
 			PersistentPropertyAccessor<?> sourceAccessor, EntityInstanceWithSource entityInstanceAndSource) {
 
 		TypeSystem typeSystem = entityInstanceAndSource.getTypeSystem();
@@ -221,7 +225,7 @@ Object getPropertyValueFor(PersistentProperty<?> targetProperty, PersistentEntit
 					singleValue = p -> context.getEntityConverter().read(actualType, p);
 				} else {
 					ClassTypeInformation<?> actualTargetType = ClassTypeInformation.from(actualType);
-					singleValue = p -> context.getConversionService().readValue(p, actualTargetType, null);
+					singleValue = p -> context.getConversionService().readValue(p, actualTargetType, targetProperty.getOptionalReadingConverter());
 				}
 
 				if (targetProperty.isCollectionLike()) {
diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/OptionalUnwrappingConverter.java b/src/main/java/org/springframework/data/neo4j/repository/query/OptionalUnwrappingConverter.java
@@ -18,7 +18,6 @@
 import java.util.Optional;
 
 import org.springframework.core.convert.converter.Converter;
-import org.springframework.lang.Nullable;
 
 /**
  * Used to unwrap optionals before further processing by a
@@ -30,7 +29,7 @@ enum OptionalUnwrappingConverter implements Converter<Object, Object> {
 	INSTANCE;
 
 	@Override
-	public @Nullable Object convert(Object source) {
+	public Object convert(Object source) {
 		if (source instanceof Optional) {
 			return ((Optional<?>) source).orElse(null);
 		}
diff --git a/src/test/java/org/springframework/data/neo4j/integration/conversion_imperative/CustomTypesIT.java b/src/test/java/org/springframework/data/neo4j/integration/conversion_imperative/CustomTypesIT.java
@@ -19,6 +19,7 @@
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -42,6 +43,7 @@
 import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
 import org.springframework.data.neo4j.core.Neo4jOperations;
 import org.springframework.data.neo4j.core.convert.Neo4jConversions;
+import org.springframework.data.neo4j.core.support.DateLong;
 import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
 import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager;
 import org.springframework.data.neo4j.integration.shared.conversion.PersonWithCustomId;
@@ -90,7 +92,8 @@ void setupData() {
 		try (Session session = driver.session(bookmarkCapture.createSessionConfig())) {
 			session.writeTransaction(transaction -> {
 				transaction.run("MATCH (n) detach delete n").consume();
-				transaction.run("CREATE (:CustomTypes{customType:'XYZ'})").consume();
+				transaction.run("CREATE (:CustomTypes{customType:'XYZ', dateAsLong: 1630311077418})").consume();
+				transaction.run("CREATE (:CustomTypes{customType:'ABC'})").consume();
 				return null;
 			});
 			bookmarkCapture.seedWith(session.lastBookmark());
@@ -124,7 +127,7 @@ void deleteAllByCustomId() {
 				.limit(2)
 				.collect(Collectors.toList());
 		try (
-				Session session = driver.session(bookmarkCapture.createSessionConfig());
+				Session session = driver.session(bookmarkCapture.createSessionConfig())
 		) {
 			ids.forEach(id -> session.writeTransaction(createPersonWithCustomId(id)));
 			bookmarkCapture.seedWith(session.lastBookmark());
@@ -143,7 +146,9 @@ void deleteAllByCustomId() {
 	@Test
 	void findByConvertedCustomType(@Autowired EntityWithCustomTypePropertyRepository repository) {
 
-		assertThat(repository.findByCustomType(ThingWithCustomTypes.CustomType.of("XYZ"))).isNotNull();
+		ThingWithCustomTypes xyz = repository.findByCustomType(ThingWithCustomTypes.CustomType.of("XYZ"));
+		assertThat(xyz).isNotNull();
+		assertThat(xyz.getDateAsLong()).isNotNull();
 	}
 
 	@Test
@@ -152,6 +157,31 @@ void findByConvertedCustomTypeWithCustomQuery(@Autowired EntityWithCustomTypePro
 		assertThat(repository.findByCustomTypeCustomQuery(ThingWithCustomTypes.CustomType.of("XYZ"))).isNotNull();
 	}
 
+	@Test // GH-2365
+	void customConverterShouldBeApplied(@Autowired EntityWithCustomTypePropertyRepository repository) {
+
+		ThingWithCustomTypes xyz = repository.defaultAttributeWithCoalesce(ThingWithCustomTypes.CustomType.of("XYZ"));
+		assertThat(xyz).isNotNull();
+		assertThat(xyz.getDateAsLong()).isInSameDayAs("2021-08-30");
+	}
+
+	@Test // GH-2365
+	void customConverterShouldBeAppliedWithCoalesce(@Autowired EntityWithCustomTypePropertyRepository repository) {
+
+		ThingWithCustomTypes abc = repository.defaultAttributeWithCoalesce(ThingWithCustomTypes.CustomType.of("ABC"));
+		assertThat(abc).isNotNull();
+		assertThat(abc.getDateAsLong()).isInSameDayAs("2021-09-21");
+	}
+
+	@Test // GH-2365
+	void converterAndProjection(@Autowired EntityWithCustomTypePropertyRepository repository) {
+
+		ThingWithCustomTypesProjection projection = repository.converterOnProjection(ThingWithCustomTypes.CustomType.of("XYZ"));
+		assertThat(projection).isNotNull();
+		assertThat(projection.dateAsLong).isInSameDayAs("2021-08-30");
+		assertThat(projection.modified).isInSameDayAs("2021-09-21");
+	}
+
 	@Test
 	void findByConvertedCustomTypeWithSpELPropertyAccessQuery(
 			@Autowired EntityWithCustomTypePropertyRepository repository) {
@@ -172,13 +202,27 @@ void findByConvertedDifferentTypeWithSpELObjectQuery(@Autowired EntityWithCustom
 		assertThat(repository.findByDifferentTypeCustomQuery(ThingWithCustomTypes.DifferentType.of("XYZ"))).isNotNull();
 	}
 
+	static class ThingWithCustomTypesProjection {
+
+		Date dateAsLong;
+
+		@DateLong
+		Date modified;
+	}
+
 	interface EntityWithCustomTypePropertyRepository extends Neo4jRepository<ThingWithCustomTypes, Long> {
 
 		ThingWithCustomTypes findByCustomType(ThingWithCustomTypes.CustomType customType);
 
 		@Query("MATCH (c:CustomTypes) WHERE c.customType = $customType return c")
 		ThingWithCustomTypes findByCustomTypeCustomQuery(@Param("customType") ThingWithCustomTypes.CustomType customType);
 
+		@Query("MATCH (c:CustomTypes) WHERE c.customType = $customType return c, 1632200400000 as modified")
+		ThingWithCustomTypesProjection converterOnProjection(@Param("customType") ThingWithCustomTypes.CustomType customType);
+
+		@Query("MATCH (thingWithCustomTypes:`CustomTypes`) WHERE thingWithCustomTypes.customType = $0 RETURN thingWithCustomTypes{.customType, dateAsLong: COALESCE(thingWithCustomTypes.dateAsLong, 1632200400000), .dateAsString, .id, __nodeLabels__: labels(thingWithCustomTypes), __internalNeo4jId__: id(thingWithCustomTypes)}")
+		ThingWithCustomTypes defaultAttributeWithCoalesce(@Param("customType") ThingWithCustomTypes.CustomType customType);
+
 		@Query("MATCH (c:CustomTypes) WHERE c.customType = $differentType return c")
 		ThingWithCustomTypes findByDifferentTypeCustomQuery(
 				@Param("differentType") ThingWithCustomTypes.DifferentType differentType);
diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/conversion/ThingWithCustomTypes.java b/src/test/java/org/springframework/data/neo4j/integration/shared/conversion/ThingWithCustomTypes.java
@@ -67,6 +67,18 @@ public void setDateAsLong(Date dateAsLong) {
 		this.dateAsLong = dateAsLong;
 	}
 
+	public Long getId() {
+		return id;
+	}
+
+	public Date getDateAsLong() {
+		return dateAsLong;
+	}
+
+	public Date getDateAsString() {
+		return dateAsString;
+	}
+
 	/**
 	 * Custom type to convert
 	 */

Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,7 @@ <T, R> Flux<R> doFind(@Nullable String cypherQuery, @Nullable Map<String, Object`
`254`	`254`
`255`	`255`	`DtoInstantiatingConverter converter = new DtoInstantiatingConverter(resultType, neo4jMappingContext);`
`256`	`256`	`return (Flux<R>) intermediaResults.map(EntityInstanceWithSource.class::cast)`
`257`		`- .map(converter::convert);`
	`257`	`+ .mapNotNull(converter::convert);`
`258`	`258`	`}`
`259`	`259`
`260`	`260`	`@Override`