Make sure a SimpleTypeHolder is used that knows about custom conversions.

Needed as spring-projects/spring-data-commons@b3b590d relies on that information. We didn’t apply this correct beforehand.

Author: michael-simons

src/main/java/org/springframework/data/neo4j/core/PropertyFilterSupport.java

@@ -16,7 +16,6 @@
 package org.springframework.data.neo4j.core;
 
 import org.springframework.data.mapping.PropertyPath;
-import org.springframework.data.neo4j.core.convert.Neo4jSimpleTypes;
 import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
 import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
 import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
@@ -89,7 +88,7 @@ private static void addPropertiesFrom(Class<?> domainType, Class<?> returnedType
 		// 1. Simple types can be added directly
 		// 2. Something that looks like an entity needs to get processed as such
 		// 3. Embedded projection
-		if (Neo4jSimpleTypes.HOLDER.isSimpleType(propertyType) || mappingContext.hasCustomWriteTarget(propertyType)) {
+		if (mappingContext.getConversionService().isSimpleType(propertyType)) {
 			filteredProperties.add(propertyPath);
 		} else if (mappingContext.hasPersistentEntityFor(propertyType)) {
 			// avoid recursion / cycles
@@ -176,7 +175,7 @@ private static void addPropertiesFrom(Collection<PropertyPath> filteredPropertie
 		}
 		Class<?> propertyType = propertyPath.getLeafType();
 		// simple types can get added directly to the list.
-		if (Neo4jSimpleTypes.HOLDER.isSimpleType(propertyType) || mappingContext.hasCustomWriteTarget(propertyType)) {
+		if (mappingContext.getConversionService().isSimpleType(propertyType)) {
 			filteredProperties.add(propertyPath);
 		// Other types are handled also as entities because there cannot be any nested projection within a real entity.
 		} else if (mappingContext.hasPersistentEntityFor(propertyType)) {

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

@@ -82,4 +82,10 @@ Object readValue(
 	Value writeValue(
 			@Nullable Object value, TypeInformation<?> sourceType, @Nullable Function<Object, Value> conversionOverride
 	);
+
+	/**
+	 * @param type A type that should be checked whether it's simple or not.
+	 * @return True if {@code type} is a simple type, according to {@link Neo4jSimpleTypes} and the registered converters.
+	 */
+	boolean isSimpleType(Class<?> type);
 }

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

@@ -87,7 +87,7 @@ public final class Neo4jSimpleTypes {
 	}
 
 	/**
-	 * The simple types we support plus all the simple types recognized by Spring.
+	 * The simple types we support plus all the simple types recognized by Spring. Not taking custom conversions into account.
 	 */
 	public static final SimpleTypeHolder HOLDER = new SimpleTypeHolder(NEO4J_NATIVE_TYPES, true);
 

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

@@ -27,6 +27,7 @@
 import org.springframework.core.convert.support.ConfigurableConversionService;
 import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.dao.TypeMismatchDataAccessException;
+import org.springframework.data.mapping.model.SimpleTypeHolder;
 import org.springframework.data.neo4j.core.convert.Neo4jConversionService;
 import org.springframework.data.neo4j.core.convert.Neo4jConversions;
 import org.springframework.data.util.TypeInformation;
@@ -41,6 +42,7 @@ final class DefaultNeo4jConversionService implements Neo4jConversionService {
 
 	private final ConversionService conversionService;
 	private final Predicate<Class<?>> hasCustomWriteTargetPredicate;
+	private final SimpleTypeHolder simpleTypes;
 
 	DefaultNeo4jConversionService(Neo4jConversions neo4jConversions) {
 
@@ -49,6 +51,7 @@ final class DefaultNeo4jConversionService implements Neo4jConversionService {
 
 		this.conversionService = configurableConversionService;
 		this.hasCustomWriteTargetPredicate = neo4jConversions::hasCustomWriteTarget;
+		this.simpleTypes = neo4jConversions.getSimpleTypeHolder();
 	}
 
 	@Override
@@ -132,4 +135,9 @@ private Value writeValueImpl(@Nullable Object value, TypeInformation<?> type,
 	private static boolean isCollection(TypeInformation<?> type) {
 		return Collection.class.isAssignableFrom(type.getType());
 	}
+
+	@Override
+	public boolean isSimpleType(Class<?> type) {
+		return simpleTypes.isSimpleType(type);
+	}
 }

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

@@ -46,7 +46,6 @@
 import org.springframework.data.neo4j.core.convert.Neo4jConversions;
 import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverter;
 import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverterFactory;
-import org.springframework.data.neo4j.core.convert.Neo4jSimpleTypes;
 import org.springframework.data.neo4j.core.schema.IdGenerator;
 import org.springframework.data.neo4j.core.schema.Node;
 import org.springframework.data.util.ReflectionUtils;
@@ -126,10 +125,10 @@ public void setStrict(boolean strict) {
 	@API(status = API.Status.INTERNAL, since = "6.0")
 	public Neo4jMappingContext(Neo4jConversions neo4jConversions, @Nullable TypeSystem typeSystem) {
 
-		super.setSimpleTypeHolder(Neo4jSimpleTypes.HOLDER);
 		this.conversionService = new DefaultNeo4jConversionService(neo4jConversions);
-
 		this.typeSystem = typeSystem == null ? InternalTypeSystem.TYPE_SYSTEM : typeSystem;
+
+		super.setSimpleTypeHolder(neo4jConversions.getSimpleTypeHolder());
 	}
 
 	public Neo4jEntityConverter getEntityConverter() {

src/main/java/org/springframework/data/neo4j/repository/query/Neo4jQuerySupport.java

@@ -43,7 +43,6 @@
 import org.springframework.data.geo.Distance;
 import org.springframework.data.geo.Metrics;
 import org.springframework.data.neo4j.core.TemplateSupport;
-import org.springframework.data.neo4j.core.convert.Neo4jSimpleTypes;
 import org.springframework.data.neo4j.core.mapping.CypherGenerator;
 import org.springframework.data.neo4j.core.mapping.EntityInstanceWithSource;
 import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
@@ -112,7 +111,7 @@ static Class<?> getDomainType(QueryMethod queryMethod) {
 
 		final BiFunction<TypeSystem, MapAccessor, ?> mappingFunction;
 
-		if (Neo4jSimpleTypes.HOLDER.isSimpleType(returnedType)) {
+		if (mappingContext.getConversionService().isSimpleType(returnedType)) {
 			// Clients automatically selects a single value mapping function.
 			// It will thrown an error if the query contains more than one column.
 			mappingFunction = null;

src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java

@@ -64,6 +64,8 @@
 import org.springframework.data.neo4j.core.schema.RelationshipProperties;
 import org.springframework.data.neo4j.core.schema.TargetNode;
 import org.springframework.data.neo4j.integration.shared.common.FriendshipRelationship;
+import org.springframework.data.neo4j.integration.shared.conversion.ThingWithCompositeProperties;
+import org.springframework.data.neo4j.integration.shared.conversion.ThingWithCustomTypes;
 import org.springframework.data.neo4j.test.LogbackCapture;
 import org.springframework.data.neo4j.test.LogbackCapturingExtension;
 
@@ -496,6 +498,31 @@ void relAnnotationWithoutTypeMustOverwriteDefaultType() {
 		assertThat(entity.getRelationships()).anyMatch(r -> r.getFieldName().equals("theSuperBike") && r.getType().equals("THE_SUPER_BIKE"));
 	}
 
+	@Test // COMMONS-2390
+	void shouldNotCreateEntityForConvertedSimpleTypes() {
+
+		Set<GenericConverter> additionalConverters = new HashSet<>();
+		additionalConverters.add(new ThingWithCustomTypes.CustomTypeConverter());
+		additionalConverters.add(new ThingWithCustomTypes.DifferentTypeConverter());
+
+		Neo4jMappingContext schema = new Neo4jMappingContext(new Neo4jConversions(additionalConverters));
+		schema.setStrict(true);
+		schema.setInitialEntitySet(
+				new HashSet<>(Arrays.asList(ThingWithCustomTypes.class, ThingWithCompositeProperties.class)));
+		schema.initialize();
+
+		assertThat(schema.hasPersistentEntityFor(ThingWithCustomTypes.class)).isTrue();
+		assertThat(schema.hasPersistentEntityFor(ThingWithCompositeProperties.class)).isTrue();
+
+		Neo4jPersistentEntity<?> entity = schema.getPersistentEntity(ThingWithCustomTypes.class);
+		assertThat(entity.getPersistentProperty("customType").isEntity()).isFalse();
+
+		entity = schema.getPersistentEntity(ThingWithCompositeProperties.class);
+		assertThat(entity.getPersistentProperty("customTypeMap").isEntity()).isFalse();
+
+		assertThat(schema.hasPersistentEntityFor(ThingWithCustomTypes.CustomType.class)).isFalse();
+	}
+
 	static class DummyIdGenerator implements IdGenerator<Void> {
 
 		@Override