GH-656 - Correctly discover and handle typed and parameterized fields.

Relationships can either live both in a typed and a parameterized fields. This changes the field detection algorithm in FieldsInfo to take of both scenarios and closes #656.

core/src/main/java/org/neo4j/ogm/metadata/FieldInfo.java

@@ -191,8 +191,7 @@ public ObjectAnnotations getAnnotations() {
     public boolean persistableAsProperty() {
 
         return PRIMITIVES.contains(descriptor)
-            || (AUTOBOXERS.contains(descriptor) && typeParameterDescriptor == null)
-            || (typeParameterDescriptor != null && AUTOBOXERS.contains(typeParameterDescriptor))
+            || AUTOBOXERS.contains(getTypeDescriptor())
             || propertyConverter != null
             || compositeConverter != null;
     }

core/src/main/java/org/neo4j/ogm/metadata/FieldsInfo.java

@@ -18,23 +18,21 @@
  */
 package org.neo4j.ogm.metadata;
 
-import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
-import java.lang.reflect.WildcardType;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Predicate;
 import java.util.stream.Stream;
 
-import org.neo4j.ogm.annotation.Relationship;
 import org.neo4j.ogm.annotation.Transient;
 import org.neo4j.ogm.metadata.reflect.GenericUtils;
 
@@ -57,7 +55,7 @@ public class FieldsInfo {
         // all generics fields of possible superclasses that resolve to concrete
         // types through this class.
         List<Field> allFieldsInfluencedByThisClass = new ArrayList<>();
-        allFieldsInfluencedByThisClass.addAll(getGenericFieldsInHierarchyOf(clazz));
+        allFieldsInfluencedByThisClass.addAll(getGenericOrParameterizedFieldsInHierarchyOf(clazz));
         allFieldsInfluencedByThisClass.addAll(Arrays.asList(clazz.getDeclaredFields()));
 
         for (Field field : allFieldsInfluencedByThisClass) {
@@ -68,28 +66,7 @@ public class FieldsInfo {
                     String typeParameterDescriptor = null;
                     final Type genericType = field.getGenericType();
                     if (genericType instanceof ParameterizedType) {
-                        ParameterizedType parameterizedType = (ParameterizedType) genericType;
-                        final Type[] actualTypeArguments = parameterizedType.getActualTypeArguments();
-                        if (actualTypeArguments.length > 0) {
-                            for (Type typeArgument : actualTypeArguments) {
-                                if (typeArgument instanceof ParameterizedType) {
-                                    ParameterizedType parameterizedTypeArgument = (ParameterizedType) typeArgument;
-                                    typeParameterDescriptor = parameterizedTypeArgument.getRawType().getTypeName();
-                                    break;
-                                } else if ((typeArgument instanceof TypeVariable || typeArgument instanceof WildcardType)
-                                    // The type parameter descriptor doesn't matter if we're dealing with an explicit relationship
-                                    // We must not try to persist it as a property if the user explicitly asked for storing it as a node.
-                                    && !objectAnnotations.has(Relationship.class))  {
-                                    typeParameterDescriptor = Object.class.getName();
-                                    break;
-                                } else if (typeArgument instanceof Class) {
-                                    typeParameterDescriptor = ((Class) typeArgument).getName();
-                                }
-                            }
-                        }
-                        if (typeParameterDescriptor == null) {
-                            typeParameterDescriptor = parameterizedType.getRawType().getTypeName();
-                        }
+                        typeParameterDescriptor = GenericUtils.findFieldType(field, clazz).getName();
                     }
                     if (typeParameterDescriptor == null && (genericType instanceof TypeVariable)) {
                         typeParameterDescriptor = field.getType().getTypeName();
@@ -101,13 +78,16 @@ public class FieldsInfo {
         }
     }
 
-    private static List<Field> getGenericFieldsInHierarchyOf(Class<?> clazz) {
+    private static List<Field> getGenericOrParameterizedFieldsInHierarchyOf(Class<?> clazz) {
+
+        Predicate<Field> predicate = GenericUtils::isGenericField;
+        predicate = predicate.or(GenericUtils::isParameterizedField);
 
         List<Field> genericFieldsInHierarchy = new ArrayList<>();
         Class<?> currentClass = clazz.getSuperclass();
         while(currentClass != null) {
             Stream.of(currentClass.getDeclaredFields())
-                .filter(GenericUtils::isGenericField)
+                .filter(predicate)
                 .forEach(genericFieldsInHierarchy::add);
             currentClass = currentClass.getSuperclass();
         }

core/src/main/java/org/neo4j/ogm/metadata/reflect/GenericUtils.java

@@ -48,10 +48,21 @@ public static boolean isGenericField(Field field) {
     }
 
     /**
-     * Tries to discover type of given field
+     * Helper to check whether a given field is a parameterized field (A field described by a type variable presenting a parameter type).
+     *
+     * @param field The field to check for a parameterized type
+     * @return True, if {@code field} is a parameterized field.
+     */
+    public static boolean isParameterizedField(Field field) {
+        return field.getGenericType() instanceof ParameterizedType;
+    }
+
+    /**
+     * Tries to discover type of given field.
      * If the field ha a concrete type then there is nothing to do and it's type is returned.
      * If the field has a generic type then it traverses class hierarchy of the concrete class to discover
-     * ParameterizedType with type parameter
+     * ParameterizedType with type parameter.
+     * If the field is parameterized but no parameter can be discovered, than {@code Object.class} will be returned.
      *
      * @param field         field
      * @param concreteClass concrete class that either declares the field or is a subclass of such class
@@ -61,7 +72,7 @@ public static Class findFieldType(Field field, Class concreteClass) {
 
         Class<?>[] arguments = resolveRawArguments(field.getGenericType(), concreteClass);
         if (arguments == null || arguments.length == 0 || arguments[0] == Unknown.class) {
-            return field.getType();
+            return isParameterizedField(field) ? Object.class : field.getType();
         }
 
         return arguments[0];

test/src/test/java/org/neo4j/ogm/domain/gh656/Group.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.neo4j.ogm.domain.gh656;
+
+import java.util.Set;
+
+import org.neo4j.ogm.annotation.GeneratedValue;
+import org.neo4j.ogm.annotation.Id;
+import org.neo4j.ogm.annotation.NodeEntity;
+import org.neo4j.ogm.annotation.Relationship;
+import org.neo4j.ogm.id.UuidStrategy;
+
+/**
+ * @author Michael J. Simons
+ */
+@NodeEntity(label = "Group")
+public class Group extends VersionedEntity<GroupVersion> {
+    @Id
+    @GeneratedValue(strategy = UuidStrategy.class)
+    private String uuid;
+
+    @Relationship(type = "HAS_VERSION2")
+    private Set<GroupVersion> versions2;
+
+    public String getUuid() {
+        return uuid;
+    }
+
+    public void setUuid(String uuid) {
+        this.uuid = uuid;
+    }
+}

test/src/test/java/org/neo4j/ogm/domain/gh656/GroupVersion.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.neo4j.ogm.domain.gh656;
+
+import org.neo4j.ogm.annotation.GeneratedValue;
+import org.neo4j.ogm.annotation.Id;
+import org.neo4j.ogm.annotation.NodeEntity;
+import org.neo4j.ogm.id.UuidStrategy;
+
+/**
+ * @author Michael J. Simons
+ */
+@NodeEntity(label = "GroupVersion")
+public class GroupVersion {
+    @Id
+    @GeneratedValue(strategy = UuidStrategy.class)
+    private String uuid;
+
+    public String getUuid() {
+        return uuid;
+    }
+
+    public void setUuid(String uuid) {
+        this.uuid = uuid;
+    }
+}

test/src/test/java/org/neo4j/ogm/domain/gh656/VersionedEntity.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.neo4j.ogm.domain.gh656;
+
+import java.util.Set;
+
+import org.neo4j.ogm.annotation.Relationship;
+
+/**
+ * @param <V>
+ * @author Michael J. Simons
+ */
+abstract class VersionedEntity<V> {
+    @Relationship(type = "HAS_VERSION")
+    private Set<V> versions;
+
+    public Set<V> getVersions() {
+        return versions;
+    }
+
+    public void setVersions(Set<V> versions) {
+        this.versions = versions;
+    }
+}

test/src/test/java/org/neo4j/ogm/metadata/GenericsFieldsTest.java

@@ -34,72 +34,72 @@ public class GenericsFieldsTest extends TestMetaDataTypeResolution {
 
     @Test
     public void testUnboundedGeneric() {
-        checkField("genericObject", "java.lang.Object", Object.class);
+        checkField("genericObject", "java.lang.Object", Object.class, null);
     }
 
     @Test
     public void testGenericComparable() { // from java.lang
-        checkField("genericComparable", "java.lang.Comparable", Comparable.class);
+        checkField("genericComparable", "java.lang.Comparable", Comparable.class, null);
     }
 
     @Test
     public void testGenericSerializable() { // from java.io
-        checkField("genericSerializable", "java.io.Serializable", Serializable.class);
+        checkField("genericSerializable", "java.io.Serializable", Serializable.class, null);
     }
 
     @Test
     public void testGenericSelfReference() {
-        checkField("next", "org.neo4j.ogm.metadata.POJO", POJO.class);
+        checkField("next", "org.neo4j.ogm.metadata.POJO", POJO.class, null);
     }
 
     @Test // List<S>
     public void testCollectionWithUnboundGenericParameter() {
-        checkField("elements", "java.lang.Object", Object.class);
+        checkField("elements", "java.lang.Object", Object.class, "java.util.List");
     }
 
     @Test // List<POJO<S, T, U>> neighbours;
     public void testCollectionWithConcreteParameterizedType() {
-        checkField("neighbours", "org.neo4j.ogm.metadata.POJO", POJO.class);
+        checkField("neighbours", "org.neo4j.ogm.metadata.POJO", POJO.class, "java.util.List");
     }
 
     @Test // List<? extends Integer> superIntegers
     public void testCollectionWithExtendedConcreteParameterizedType() {
-        checkField("superIntegers", "java.lang.Object", Object.class);
+        checkField("superIntegers", "java.lang.Object", Object.class, "java.util.List");
     }
 
     @Test    // List<? super Integer> subIntegers;
     public void testCollectionWithReducedConcreteParameterizedType() {
-        checkField("subIntegers", "java.lang.Object", Object.class);
+        checkField("subIntegers", "java.lang.Object", Object.class, "java.util.List");
     }
 
     @Test    // List<? extends S> superS;
     public void testCollectionOfWildcardExtendingGenericType() {
-        checkField("superS", "java.lang.Object", Object.class);
+        checkField("superS", "java.lang.Object", Object.class, "java.util.List");
     }
 
     @Test    // List<? super S> subS;
     public void testCollectionOfWildcardReducingGenericType() {
-        checkField("subS", "java.lang.Object", Object.class);
+        checkField("subS", "java.lang.Object", Object.class, "java.util.List");
     }
 
     @Test    // List<?>;
     public void testListGenericWildcard() {
-        checkField("listOfAnything", "java.lang.Object", Object.class);
+        checkField("listOfAnything", "java.lang.Object", Object.class, "java.util.List");
     }
 
     @Test    // Vector<?>;
     public void testVectorGenericWildcard() {
-        checkField("vectorOfAnything", "java.lang.Object", Object.class);
+        checkField("vectorOfAnything", "java.lang.Object", Object.class, "java.util.Vector");
     }
 
     @Test    // Set<?>;
     public void testSetGenericWildcard() {
-        checkField("setOfAnything", "java.lang.Object", Object.class);
+        checkField("setOfAnything", "java.lang.Object", Object.class, "java.util.Set");
     }
 
     @Test    // Iterable<Map<Class<S>, POJO<S, T, U>>> iterable;
     public void testIterableOfMapOfParameterizedClasses() {
-        checkField("iterable", "java.util.Map", Map.class);
+        checkField("iterable", "java.util.Map", Map.class, "java.lang.Iterable");
     }
 
     @Test // GH-492
@@ -121,4 +121,17 @@ public void shouldDetectWrapperArraysInGenericFields() {
         assertThat(fieldInfo.isArray()).isTrue();
         assertThat(fieldInfo.type()).isEqualTo(Integer[].class);
     }
+
+    @Test // GH-656
+    public void parameterizedFieldsInParentClassesShouldWork() {
+
+        MetaData metaData = new MetaData("org.neo4j.ogm.domain.gh656");
+        ClassInfo classInfo = metaData.classInfo("Group");
+        assertThat(classInfo).isNotNull();
+        assertThat(classInfo.getFieldInfo("uuid")).isNotNull();
+        FieldInfo hasVersionField = classInfo.relationshipField("HAS_VERSION");
+        assertThat(hasVersionField).isNotNull();
+        assertThat(hasVersionField.getCollectionClassname()).isEqualTo("java.util.Set");
+        assertThat(hasVersionField.getTypeDescriptor()).isEqualTo("org.neo4j.ogm.domain.gh656.GroupVersion");
+    }
 }

test/src/test/java/org/neo4j/ogm/metadata/TestMetaDataTypeResolution.java

@@ -23,19 +23,28 @@
 import org.neo4j.ogm.utils.ClassUtils;
 
 /**
- * @author vince
+ * @author Vince Bickers
+ * @author Michael J. Simons
  */
-
 public class TestMetaDataTypeResolution {
 
     private MetaData metaData = new MetaData("org.neo4j.ogm.metadata");
 
     public void checkField(String name, String expectedDescriptor, Class expectedPersistableType) {
+        this.checkField(name, expectedDescriptor, expectedPersistableType, null);
+    }
+
+    public void checkField(String name, String expectedDescriptor, Class expectedPersistableType, String fieldType) {
         ClassInfo classInfo = metaData.classInfo("POJO");
         FieldInfo fieldInfo = classInfo.fieldsInfo().get(name);
         String fieldDescriptor = fieldInfo.getTypeDescriptor();
         assertThat(fieldDescriptor).isEqualTo(expectedDescriptor);
         Class clazz = ClassUtils.getType(fieldDescriptor);
         assertThat(clazz).isEqualTo(expectedPersistableType);
+        if (fieldType != null) {
+            assertThat(fieldInfo.getCollectionClassname()).isEqualTo(fieldType);
+        } else {
+            assertThat(fieldInfo.getCollectionClassname()).isEqualTo(fieldDescriptor);
+        }
     }
 }