DATACMNS-1180 - Fixed accessor lookup for generic properties.

In AbstractPersistentProperty, we now resolve the potentially generic return and parameter types of getters and setters. To achieve that Property has now been made aware of the actual owning type.

Author: odrotbohm

src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

@@ -513,12 +513,13 @@ public PersistentPropertyCreator(E entity, Map<String, PropertyDescriptor> descr
 		public void doWith(Field field) {
 
 			String fieldName = field.getName();
+			TypeInformation<?> type = entity.getTypeInformation();
 
 			ReflectionUtils.makeAccessible(field);
 
 			Property property = Optional.ofNullable(descriptors.get(fieldName))//
-					.map(it -> Property.of(field, it))//
-					.orElseGet(() -> Property.of(field));
+					.map(it -> Property.of(type, field, it))//
+					.orElseGet(() -> Property.of(type, field));
 
 			createAndRegisterProperty(property);
 
@@ -535,7 +536,7 @@ public void addPropertiesForRemainingDescriptors() {
 
 			remainingDescriptors.values().stream() //
 					.filter(Property::supportsStandalone) //
-					.map(Property::of) //
+					.map(it -> Property.of(entity.getTypeInformation(), it)) //
 					.filter(PersistentPropertyFilter.INSTANCE::matches) //
 					.forEach(this::createAndRegisterProperty);
 		}

src/main/java/org/springframework/data/mapping/model/Property.java

@@ -26,6 +26,7 @@
 
 import org.springframework.data.util.Lazy;
 import org.springframework.data.util.Optionals;
+import org.springframework.data.util.TypeInformation;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
@@ -49,68 +50,75 @@ public class Property {
 	private final Lazy<String> name;
 	private final Lazy<String> toString;
 
-	private Property(Optional<Field> field, Optional<PropertyDescriptor> descriptor) {
+	private Property(TypeInformation<?> type, Optional<Field> field, Optional<PropertyDescriptor> descriptor) {
 
+		Assert.notNull(type, "Type must not be null!");
 		Assert.isTrue(Optionals.isAnyPresent(field, descriptor), "Either field or descriptor has to be given!");
 
 		this.field = field;
 		this.descriptor = descriptor;
 
-		this.rawType = withFieldOrDescriptor(Field::getType, PropertyDescriptor::getPropertyType);
+		this.rawType = withFieldOrDescriptor( //
+				it -> type.getRequiredProperty(it.getName()).getType(), //
+				it -> type.getRequiredProperty(it.getName()).getType() //
+		);
 		this.hashCode = Lazy.of(() -> withFieldOrDescriptor(Object::hashCode));
 		this.name = Lazy.of(() -> withFieldOrDescriptor(Field::getName, FeatureDescriptor::getName));
 		this.toString = Lazy.of(() -> withFieldOrDescriptor(Object::toString));
 
 		this.getter = descriptor.map(PropertyDescriptor::getReadMethod)//
 				.filter(it -> getType() != null)//
-				.filter(it -> getType().isAssignableFrom(it.getReturnType()));
+				.filter(it -> getType().isAssignableFrom(type.getReturnType(it).getType()));
 
 		this.setter = descriptor.map(PropertyDescriptor::getWriteMethod)//
 				.filter(it -> getType() != null)//
-				.filter(it -> it.getParameterTypes()[0].isAssignableFrom(getType()));
+				.filter(it -> type.getParameterTypes(it).get(0).getType().isAssignableFrom(getType()));
 	}
 
 	/**
 	 * Creates a new {@link Property} backed by the given field.
 	 *
+	 * @param type the owning type, must not be {@literal null}.
 	 * @param field must not be {@literal null}.
 	 * @return
 	 */
-	public static Property of(Field field) {
+	public static Property of(TypeInformation<?> type, Field field) {
 
 		Assert.notNull(field, "Field must not be null!");
 
-		return new Property(Optional.of(field), Optional.empty());
+		return new Property(type, Optional.of(field), Optional.empty());
 	}
 
 	/**
 	 * Creates a new {@link Property} backed by the given {@link Field} and {@link PropertyDescriptor}.
 	 *
+	 * @param type the owning type, must not be {@literal null}.
 	 * @param field must not be {@literal null}.
 	 * @param descriptor must not be {@literal null}.
 	 * @return
 	 */
-	public static Property of(Field field, PropertyDescriptor descriptor) {
+	public static Property of(TypeInformation<?> type, Field field, PropertyDescriptor descriptor) {
 
 		Assert.notNull(field, "Field must not be null!");
 		Assert.notNull(descriptor, "PropertyDescriptor must not be null!");
 
-		return new Property(Optional.of(field), Optional.of(descriptor));
+		return new Property(type, Optional.of(field), Optional.of(descriptor));
 	}
 
 	/**
 	 * Creates a new {@link Property} for the given {@link PropertyDescriptor}. The creation might fail if the given
 	 * property is not representing a proper property.
 	 *
+	 * @param type the owning type, must not be {@literal null}.
 	 * @param descriptor must not be {@literal null}.
 	 * @return
 	 * @see #supportsStandalone(PropertyDescriptor)
 	 */
-	public static Property of(PropertyDescriptor descriptor) {
+	public static Property of(TypeInformation<?> type, PropertyDescriptor descriptor) {
 
 		Assert.notNull(descriptor, "PropertyDescriptor must not be null!");
 
-		return new Property(Optional.empty(), Optional.of(descriptor));
+		return new Property(type, Optional.empty(), Optional.of(descriptor));
 	}
 
 	/**

src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java

@@ -17,6 +17,9 @@
 
 import static org.assertj.core.api.Assertions.*;
 
+import lombok.Getter;
+import lombok.Setter;
+
 import java.beans.IntrospectionException;
 import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
@@ -36,6 +39,7 @@
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.Person;
 import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.Optionals;
 import org.springframework.data.util.TypeInformation;
 import org.springframework.util.ReflectionUtils;
 
@@ -136,11 +140,13 @@ public void usesCustomSetter() {
 	public void doesNotDiscoverGetterAndSetterIfNoPropertyDescriptorGiven() {
 
 		Field field = ReflectionUtils.findField(AccessorTestClass.class, "id");
-		PersistentProperty<SamplePersistentProperty> property = new SamplePersistentProperty(Property.of(field),
+		Property property = Property.of(ClassTypeInformation.from(AccessorTestClass.class), field);
+
+		PersistentProperty<SamplePersistentProperty> persistentProperty = new SamplePersistentProperty(property,
 				getEntity(AccessorTestClass.class), typeHolder);
 
-		assertThat(property.getGetter()).isNull();
-		assertThat(property.getSetter()).isNull();
+		assertThat(persistentProperty.getGetter()).isNull();
+		assertThat(persistentProperty.getSetter()).isNull();
 	}
 
 	@Test // DATACMNS-337
@@ -211,21 +217,30 @@ public void resolvesGenericsForRawType() {
 		assertThat(property.getRawType()).isEqualTo(String.class);
 	}
 
+	@Test // DATACMNS-1180
+	public void returnsAccessorsForGenericReturnType() {
+
+		SamplePersistentProperty property = getProperty(ConcreteGetter.class, "genericField");
+
+		assertThat(property.getSetter()).isNotNull();
+		assertThat(property.getGetter()).isNotNull();
+	}
+
 	private <T> BasicPersistentEntity<T, SamplePersistentProperty> getEntity(Class<T> type) {
 		return new BasicPersistentEntity<>(ClassTypeInformation.from(type));
 	}
 
 	private <T> SamplePersistentProperty getProperty(Class<T> type, String name) {
 
+		TypeInformation<?> typeInformation = ClassTypeInformation.from(type);
 		Optional<Field> field = Optional.ofNullable(ReflectionUtils.findField(type, name));
 		Optional<PropertyDescriptor> descriptor = getPropertyDescriptor(type, name);
 
-		Property property = field.map(it -> descriptor//
-				.map(foo -> Property.of(it, foo))//
-				.orElseGet(() -> Property.of(it))).orElseGet(() -> getPropertyDescriptor(type, name)//
-						.map(it -> Property.of(it))//
-						.orElseThrow(
-								() -> new IllegalArgumentException(String.format("Couldn't find property %s on %s!", name, type))));
+		Property property = Optionals.firstNonEmpty( //
+				() -> Optionals.mapIfAllPresent(field, descriptor, (left, right) -> Property.of(typeInformation, left, right)), //
+				() -> field.map(it -> Property.of(typeInformation, it)), //
+				() -> descriptor.map(it -> Property.of(typeInformation, it))) //
+				.orElseThrow(() -> new IllegalArgumentException(String.format("Couldn't find property %s on %s!", name, type)));
 
 		return new SamplePersistentProperty(property, getEntity(type), typeHolder);
 	}
@@ -256,6 +271,14 @@ class SecondConcrete extends Generic<Integer> {
 
 	}
 
+	@Getter
+	@Setter
+	class GenericGetter<T> {
+		T genericField;
+	}
+
+	class ConcreteGetter extends GenericGetter<String> {}
+
 	@SuppressWarnings("serial")
 	class TestClassSet extends TreeSet<Object> {}