DATAGRAPH-1131 - Fix possible ArrayIndexOutOfBoundsException in MetaDataDrivenConversionService during startup.

Due to improvments in DATAGRAPH-1126 MetaDataDrivenConversionService gets actually used when there is no existing conversion services and revealed now that it can’t handle converters extending from abstract converter bases classes and not redclaring their inheritance from an interface.

This commit uses now Spring Commons infrastructure method to determine generic types of a class instead of using a custom solution.

spring-data-neo4j/src/main/java/org/springframework/data/neo4j/conversion/MetaDataDrivenConversionService.java

@@ -13,7 +13,7 @@
 
 package org.springframework.data.neo4j.conversion;
 
-import java.lang.reflect.ParameterizedType;
+import java.util.Optional;
 
 import org.neo4j.ogm.metadata.ClassInfo;
 import org.neo4j.ogm.metadata.FieldInfo;
@@ -23,6 +23,7 @@ import org.neo4j.ogm.typeconversion.ConversionCallback;
 import org.neo4j.ogm.typeconversion.ProxyAttributeConverter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.core.ResolvableType;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.core.convert.support.GenericConversionService;
 
@@ -58,46 +59,25 @@ public class MetaDataDrivenConversionService extends GenericConversionService im
 
 	@SuppressWarnings({ "unchecked", "rawtypes" })
 	private void addWrappedConverter(final AttributeConverter attributeConverter) {
+
 		if (attributeConverter instanceof ProxyAttributeConverter) {
 			return;
 		}
 
-		Converter toGraphConverter = new Converter() {
-			@Override
-			public Object convert(Object source) {
-				return attributeConverter.toGraphProperty(source);
-			}
-		};
-		Converter toEntityConverter = new Converter() {
-			@Override
-			public Object convert(Object source) {
-				return attributeConverter.toEntityAttribute(source);
-			}
-		};
+		EntityToGraphTypeMapping entityToGraphTypeMapping = getEntityToGraphTypeMapping(attributeConverter);
 
-		ParameterizedType pt = (ParameterizedType) attributeConverter.getClass().getGenericInterfaces()[0];
-		Class<?> sourceType, targetType;
-		if (pt.getActualTypeArguments()[0] instanceof Class) {
-			sourceType = (Class<?>) pt.getActualTypeArguments()[0];
-		} else { // the argument may be a Collection for example
-			sourceType = (Class<?>) ((ParameterizedType) pt.getActualTypeArguments()[0]).getActualTypeArguments()[0];
-		}
-
-		if (pt.getActualTypeArguments()[1] instanceof Class) {
-			targetType = (Class<?>) pt.getActualTypeArguments()[1];
-		} else {
-			targetType = (Class<?>) ((ParameterizedType) pt.getActualTypeArguments()[1]).getActualTypeArguments()[1];
-
-		}
-
-		if (canConvert(sourceType, targetType) && canConvert(targetType, sourceType)) {
+		if (canConvert(entityToGraphTypeMapping.entityType, entityToGraphTypeMapping.graphType)
+				&& canConvert(entityToGraphTypeMapping.graphType, entityToGraphTypeMapping.entityType)) {
 			logger.info("Not adding Spring-compatible converter for " + attributeConverter.getClass()
 					+ " because one that does the same job has already been registered with the ConversionService.");
 		} else {
+			Converter toGraphConverter = attributeConverter::toGraphProperty;
+			Converter toEntityConverter = attributeConverter::toEntityAttribute;
+
 			// It could be argued that this is wrong as it potentially overrides a registered converted that doesn't handle
 			// both directions, but I've decided that it's better to ensure the same converter is used for load and save.
-			addConverter(sourceType, targetType, toGraphConverter);
-			addConverter(targetType, sourceType, toEntityConverter);
+			addConverter(entityToGraphTypeMapping.entityType, entityToGraphTypeMapping.graphType, toGraphConverter);
+			addConverter(entityToGraphTypeMapping.graphType, entityToGraphTypeMapping.entityType, toEntityConverter);
 		}
 	}
 
@@ -109,4 +89,42 @@ public class MetaDataDrivenConversionService extends GenericConversionService im
 		return convert(value, targetType);
 	}
 
+	static class EntityToGraphTypeMapping {
+		Class<?> entityType;
+		Class<?> graphType;
+
+		private EntityToGraphTypeMapping(Class<?> entityType, Class<?> target) {
+			this.entityType = entityType;
+			this.graphType = target;
+		}
+	}
+
+	static EntityToGraphTypeMapping getEntityToGraphTypeMapping(AttributeConverter attributeConverter) {
+
+		ResolvableType resolvableType = ResolvableType.forClass(AttributeConverter.class,
+				attributeConverter.getClass());
+
+		if (!resolvableType.hasGenerics()) {
+			throw new IllegalStateException(
+					"Cannot resolve source and target types for the given attribute converter of class "
+							+ attributeConverter.getClass());
+		}
+
+		Class<?> sourceType = nestedTypeOrType(resolvableType.getGeneric(0));
+		Class<?> targetType = nestedTypeOrType(resolvableType.getGeneric(1));
+
+		return new EntityToGraphTypeMapping(sourceType, targetType);
+	}
+
+	/**
+	 * If the type can be resolved to a collection that has generics, we extract the collection type, otherwise we return
+	 * the resolved type.
+	 *
+	 * @param type Type to resolve
+	 * @return The types resolved class or in case of a generic collections, the collections elements class.
+	 */
+	private static Class<?> nestedTypeOrType(ResolvableType type) {
+		return Optional.ofNullable(type.asCollection()).filter(ResolvableType::hasGenerics).map(r -> r.getGeneric(0))
+				.orElse(type).resolve(Object.class);
+	}
 }

spring-data-neo4j/src/test/java/org/springframework/data/neo4j/conversion/MetaDataDrivenConversionServiceIntegrationTests.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c)  [2011-2018] "Pivotal Software, Inc." / "Neo Technology" / "Graph Aware Ltd."
+ *
+ * This product is licensed to you under the Apache License, Version 2.0 (the "License").
+ * You may not use this product except in compliance with the License.
+ *
+ * This product may include a number of subcomponents with
+ * separate copyright notices and license terms. Your use of the source
+ * code for these subcomponents is subject to the terms and
+ * conditions of the subcomponent's license, as noted in the LICENSE file.
+ *
+ */
+
+package org.springframework.data.neo4j.conversion;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.Arrays;
+import java.util.Map;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.neo4j.graphdb.Result;
+import org.neo4j.ogm.session.SessionFactory;
+import org.neo4j.ogm.testutil.MultiDriverTestClass;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.data.neo4j.conversion.support.ConvertedClass;
+import org.springframework.data.neo4j.conversion.support.EntityRepository;
+import org.springframework.data.neo4j.conversion.support.EntityWithConvertedAttributes;
+import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
+import org.springframework.data.neo4j.transaction.Neo4jTransactionManager;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.transaction.PlatformTransactionManager;
+import org.springframework.transaction.annotation.EnableTransactionManagement;
+
+/**
+ * @author Michael J. Simons
+ * @soundtrack Murray Gold - Doctor Who Season 9
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration(classes = MetaDataDrivenConversionServiceIntegrationTests.Config.class)
+public class MetaDataDrivenConversionServiceIntegrationTests extends MultiDriverTestClass {
+
+	@Autowired private EntityRepository entityRepository;
+
+	@Test // DATAGRAPH-1131
+	public void conversionWithConverterHierarchyShouldWork() {
+
+		ConvertedClass convertedClass = new ConvertedClass();
+		convertedClass.setValue("Some value");
+		EntityWithConvertedAttributes entity = new EntityWithConvertedAttributes("name");
+		entity.setConvertedClass(convertedClass);
+		entity.setDoubles(Arrays.asList(21.0, 21.0));
+		entity.setTheDouble(42.0);
+		entityRepository.save(entity);
+
+		Result result = getGraphDatabaseService()
+				.execute("MATCH (e:EntityWithConvertedAttributes) RETURN e.convertedClass, e.doubles, e.theDouble");
+
+		assertThat(result.hasNext()).isTrue();
+		Map<String, Object> row = result.next();
+		assertThat(row) //
+				.containsEntry("e.convertedClass", "n/a") //
+				.containsEntry("e.doubles", "21.0,21.0") //
+				.containsEntry("e.theDouble", "that has been a double");
+
+	}
+
+	@Configuration
+	@EnableNeo4jRepositories(basePackageClasses = EntityWithConvertedAttributes.class)
+	@EnableTransactionManagement
+	public static class Config {
+
+		@Bean
+		public PlatformTransactionManager transactionManager() {
+			return new Neo4jTransactionManager(sessionFactory());
+		}
+
+		@Bean
+		public SessionFactory sessionFactory() {
+			return new SessionFactory(getBaseConfiguration().build(),
+					EntityWithConvertedAttributes.class.getPackage().getName());
+		}
+	}
+}

spring-data-neo4j/src/test/java/org/springframework/data/neo4j/conversion/MetaDataDrivenConversionServiceTests.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c)  [2011-2018] "Pivotal Software, Inc." / "Neo Technology" / "Graph Aware Ltd."
+ *
+ * This product is licensed to you under the Apache License, Version 2.0 (the "License").
+ * You may not use this product except in compliance with the License.
+ *
+ * This product may include a number of subcomponents with
+ * separate copyright notices and license terms. Your use of the source
+ * code for these subcomponents is subject to the terms and
+ * conditions of the subcomponent's license, as noted in the LICENSE file.
+ *
+ */
+
+package org.springframework.data.neo4j.conversion;
+
+import static org.assertj.core.api.Assertions.*;
+
+import org.junit.Test;
+import org.neo4j.ogm.testutil.MultiDriverTestClass;
+import org.springframework.data.neo4j.conversion.support.ConvertedClass;
+import org.springframework.data.neo4j.conversion.support.Converters;
+
+/**
+ * @author Michael J. Simons
+ * @soundtrack Murray Gold - Doctor Who Season 9
+ */
+public class MetaDataDrivenConversionServiceTests extends MultiDriverTestClass {
+
+	@Test
+	public void shouldDetermineConvertersForClasses() {
+		MetaDataDrivenConversionService.EntityToGraphTypeMapping entityToGraphTypeMapping = MetaDataDrivenConversionService
+				.getEntityToGraphTypeMapping(new Converters.DoubleToStringConverter());
+
+		assertThat(entityToGraphTypeMapping.entityType).isEqualTo(Double.class);
+		assertThat(entityToGraphTypeMapping.graphType).isEqualTo(String.class);
+	}
+
+	@Test
+	public void shouldDetermineConvertersForTypedClasses() {
+		MetaDataDrivenConversionService.EntityToGraphTypeMapping entityToGraphTypeMapping = MetaDataDrivenConversionService
+				.getEntityToGraphTypeMapping(new Converters.ListToStringConverter());
+
+		assertThat(entityToGraphTypeMapping.entityType).isEqualTo(Double.class);
+		assertThat(entityToGraphTypeMapping.graphType).isEqualTo(String.class);
+	}
+
+	@Test // DATAGRAPH-1131
+	public void shouldWorkWithConvertersInvolvingAbstractBaseClasses() {
+		MetaDataDrivenConversionService.EntityToGraphTypeMapping entityToGraphTypeMapping = MetaDataDrivenConversionService
+				.getEntityToGraphTypeMapping(new Converters.ConvertedClassToStringConverter());
+
+		assertThat(entityToGraphTypeMapping.entityType).isEqualTo(ConvertedClass.class);
+		assertThat(entityToGraphTypeMapping.graphType).isEqualTo(String.class);
+	}
+
+}

spring-data-neo4j/src/test/java/org/springframework/data/neo4j/conversion/support/ConvertedClass.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright (c)  [2011-2018] "Pivotal Software, Inc." / "Neo Technology" / "Graph Aware Ltd."
+ *
+ * This product is licensed to you under the Apache License, Version 2.0 (the "License").
+ * You may not use this product except in compliance with the License.
+ *
+ * This product may include a number of subcomponents with
+ * separate copyright notices and license terms. Your use of the source
+ * code for these subcomponents is subject to the terms and
+ * conditions of the subcomponent's license, as noted in the LICENSE file.
+ *
+ */
+
+package org.springframework.data.neo4j.conversion.support;
+
+/**
+ * @author Michael J. Simons
+ * @soundtrack Murray Gold - Doctor Who Season 9
+ */
+public class ConvertedClass {
+	private String value;
+
+	public String getValue() {
+		return value;
+	}
+
+	public void setValue(String value) {
+		this.value = value;
+	}
+}

spring-data-neo4j/src/test/java/org/springframework/data/neo4j/conversion/support/Converters.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright (c)  [2011-2018] "Pivotal Software, Inc." / "Neo Technology" / "Graph Aware Ltd."
+ *
+ * This product is licensed to you under the Apache License, Version 2.0 (the "License").
+ * You may not use this product except in compliance with the License.
+ *
+ * This product may include a number of subcomponents with
+ * separate copyright notices and license terms. Your use of the source
+ * code for these subcomponents is subject to the terms and
+ * conditions of the subcomponent's license, as noted in the LICENSE file.
+ *
+ */
+
+package org.springframework.data.neo4j.conversion.support;
+
+import static java.util.stream.Collectors.joining;
+
+import java.util.List;
+
+import org.neo4j.ogm.typeconversion.AttributeConverter;
+
+/**
+ * Wraps a some converters.
+ *
+ * @author Michael J. Simons
+ * @soundtrack Murray Gold - Doctor Who Season 9
+ */
+public final class Converters {
+
+	private Converters() {
+	}
+
+	/**
+	 * Concrete implementation doesn't matter and is meaningless on purpose.
+	 *
+	 * @param <T>
+	 */
+	public static abstract class AbstractObjectToString<T> implements AttributeConverter<T, String> {
+
+		@Override
+		public String toGraphProperty(T value) {
+			return "n/a";
+		}
+
+		@Override
+		public T toEntityAttribute(String value) {
+			try {
+				return getTypeClass().newInstance();
+			} catch (InstantiationException | IllegalAccessException e) {
+				throw new RuntimeException(e);
+			}
+		}
+
+		protected abstract Class<T> getTypeClass();
+	}
+
+	public static class ConvertedClassToStringConverter extends AbstractObjectToString<ConvertedClass> {
+
+		@Override
+		protected Class<ConvertedClass> getTypeClass() {
+			return ConvertedClass.class;
+		}
+	}
+
+	public static class DoubleToStringConverter implements AttributeConverter<Double, String> {
+
+		@Override
+		public String toGraphProperty(Double value) {
+			return "that has been a double";
+		}
+
+		@Override
+		public Double toEntityAttribute(String value) {
+			return null;
+		}
+	}
+
+	public static class ListToStringConverter implements AttributeConverter<List<Double>, String> {
+
+		@Override
+		public String toGraphProperty(List<Double> value) {
+			return value.stream().map(d -> d.toString()).collect(joining(","));
+		}
+
+		@Override
+		public List<Double> toEntityAttribute(String value) {
+			return null;
+		}
+	}
+}

spring-data-neo4j/src/test/java/org/springframework/data/neo4j/conversion/support/EntityRepository.java

@@ -0,0 +1,7 @@
+package org.springframework.data.neo4j.conversion.support;
+
+import org.springframework.data.neo4j.repository.Neo4jRepository;
+
+public interface EntityRepository extends Neo4jRepository<EntityWithConvertedAttributes, Long> {
+}
+

spring-data-neo4j/src/test/java/org/springframework/data/neo4j/conversion/support/EntityWithConvertedAttributes.java

@@ -0,0 +1,57 @@
+package org.springframework.data.neo4j.conversion.support;
+
+import java.util.List;
+
+import org.neo4j.ogm.annotation.GeneratedValue;
+import org.neo4j.ogm.annotation.Id;
+import org.neo4j.ogm.annotation.NodeEntity;
+import org.neo4j.ogm.annotation.Property;
+import org.neo4j.ogm.annotation.typeconversion.Convert;
+
+@NodeEntity
+public class EntityWithConvertedAttributes {
+	@Id @GeneratedValue private Long id;
+
+	private String name;
+
+	@Property
+	@Convert(Converters.ConvertedClassToStringConverter.class)
+	private ConvertedClass convertedClass;
+
+	@Property
+	@Convert(Converters.ListToStringConverter.class)
+	private List<Double> doubles;
+
+	@Convert(Converters.DoubleToStringConverter.class)
+	private Double theDouble;
+
+	public EntityWithConvertedAttributes() {}
+
+	public EntityWithConvertedAttributes(String name) {
+		this.name = name;
+	}
+
+	public Long getId() {
+		return id;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		this.name = name;
+	}
+
+	public void setConvertedClass(ConvertedClass convertedClass) {
+		this.convertedClass = convertedClass;
+	}
+
+	public void setDoubles(List<Double> doubles) {
+		this.doubles = doubles;
+	}
+
+	public void setTheDouble(Double theDouble) {
+		this.theDouble = theDouble;
+	}
+}