Merge pull request #621 from apache/5.0-FIX-CAY-2667-generic-inheritance

CAY-2667 Fix Issues with Generic Vertical Inheritance

cayenne/src/main/java/org/apache/cayenne/access/DataContextObjectCreator.java

@@ -209,7 +209,7 @@ protected void injectInitialValue(Object obj) {
 
         ObjEntity entity;
         try {
-            entity = context.getEntityResolver().getObjEntity(object.getClass());
+            entity = context.getEntityResolver().getObjEntity(object.getObjectId().getEntityName());
         } catch (CayenneRuntimeException ex) {
             // ObjEntity cannot be fetched, ignored
             entity = null;

cayenne/src/main/java/org/apache/cayenne/reflect/ArcProperty.java

@@ -19,7 +19,6 @@
 
 package org.apache.cayenne.reflect;
 
-import org.apache.cayenne.map.DbRelationship;
 import org.apache.cayenne.map.ObjRelationship;
 
 /**
@@ -55,7 +54,7 @@ public interface ArcProperty extends PropertyDescriptor {
      * Returns a ClassDescriptor for the type of graph nodes pointed to by this
      * arc property. Note that considering that a target object may be a
      * subclass of the class handled by the descriptor, users of this method may
-     * need to call {@link ClassDescriptor#getSubclassDescriptor(Class)} before
+     * need to call {@link ClassDescriptor#getSubclassDescriptor(String)} before
      * using the descriptor to access objects.
      */
     ClassDescriptor getTargetDescriptor();

cayenne/src/main/java/org/apache/cayenne/reflect/ClassDescriptor.java

@@ -88,14 +88,25 @@ public interface ClassDescriptor {
     ClassDescriptor getSuperclassDescriptor();
 
     /**
-     * Returns the most "specialized" descriptor for a given class. This method assumes
-     * that the following is true:
+     * Returns the most "specialized" descriptor for a given class.
+     * This method assumes that the following is true:
      * 
      * <pre>
      * this.getObjectClass().isAssignableFrom(objectClass)
      * </pre>
+     * @deprecated since 5.0, will throw UnsupportedOperationException on invocation,
+     *             use {@link #getSubclassDescriptor(String)}
      */
-    ClassDescriptor getSubclassDescriptor(Class<?> objectClass);
+    @Deprecated(since = "5.0", forRemoval = true)
+    default ClassDescriptor getSubclassDescriptor(Class<?> unused) {
+        throw new UnsupportedOperationException("This method is deprecated, use getSubclassDescriptor(entityName) instead");
+    }
+
+    /**
+     * Returns the most "specialized" descriptor for a given entity name.
+     * @since 5.0
+     */
+    ClassDescriptor getSubclassDescriptor(String entityName);
 
     /**
      * Creates a new instance of a class described by this object.

cayenne/src/main/java/org/apache/cayenne/reflect/LazyClassDescriptorDecorator.java

@@ -147,9 +147,10 @@ public PropertyDescriptor getProperty(String propertyName) {
         return descriptor.getProperty(propertyName);
     }
 
-    public ClassDescriptor getSubclassDescriptor(Class<?> objectClass) {
+    @Override
+    public ClassDescriptor getSubclassDescriptor(String entityName) {
         checkDescriptorInitialized();
-        return descriptor.getSubclassDescriptor(objectClass);
+        return descriptor.getSubclassDescriptor(entityName);
     }
 
     public ClassDescriptor getSuperclassDescriptor() {

cayenne/src/main/java/org/apache/cayenne/reflect/PersistentDescriptor.java

@@ -46,9 +46,7 @@
  */
 public class PersistentDescriptor implements ClassDescriptor {
 
-	static final Integer TRANSIENT_STATE = PersistenceState.TRANSIENT;
 	static final Integer HOLLOW_STATE = PersistenceState.HOLLOW;
-	static final Integer COMMITTED_STATE = PersistenceState.COMMITTED;
 
 	protected ClassDescriptor superclassDescriptor;
 
@@ -85,7 +83,7 @@ public PersistentDescriptor() {
 		this.subclassDescriptors = new HashMap<>();
 
 		// must be a set as duplicate addition attempts are expected...
-		this.rootDbEntities = new HashSet<DbEntity>(1);
+		this.rootDbEntities = new HashSet<>(1);
 	}
 
 	public void setDiscriminatorColumns(Collection<ObjAttribute> columns) {
@@ -213,14 +211,11 @@ public void removeDeclaredProperty(String propertyName) {
 	/**
 	 * Adds a subclass descriptor that maps to a given class name.
 	 */
-	public void addSubclassDescriptor(String className, ClassDescriptor subclassDescriptor) {
-		// note that 'className' should be used instead of
-		// "subclassDescriptor.getEntity().getClassName()", as this method is
-		// called in
-		// the early phases of descriptor initialization and we do not want to
-		// trigger
-		// subclassDescriptor resolution just yet to prevent stack overflow.
-		subclassDescriptors.put(className, subclassDescriptor);
+	public void addSubclassDescriptor(String entityName, ClassDescriptor subclassDescriptor) {
+		// NOTE: 'entityName' should be used instead of "subclassDescriptor.getEntity().getName()",
+        // as this method is called in the early phases of descriptor initialization, and we do not want to
+		// trigger subclassDescriptor resolution just yet to prevent stack overflow.
+		subclassDescriptors.put(entityName, subclassDescriptor);
 	}
 
 	public ObjEntity getEntity() {
@@ -259,31 +254,21 @@ void setObjectClass(Class<?> objectClass) {
 		this.objectClass = objectClass;
 	}
 
-	public ClassDescriptor getSubclassDescriptor(Class<?> objectClass) {
-		if (objectClass == null) {
+	public ClassDescriptor getSubclassDescriptor(String entityName) {
+		if (entityName == null) {
 			throw new IllegalArgumentException("Null objectClass");
 		}
 
 		if (subclassDescriptors.isEmpty()) {
 			return this;
 		}
 
-		ClassDescriptor subclassDescriptor = subclassDescriptors.get(objectClass.getName());
-
-		// ascend via the class hierarchy (only doing it if there are multiple
-		// choices)
-		if (subclassDescriptor == null) {
-			Class<?> currentClass = objectClass;
-			while (subclassDescriptor == null && (currentClass = currentClass.getSuperclass()) != null) {
-				subclassDescriptor = subclassDescriptors.get(currentClass.getName());
-			}
-		}
-
+		ClassDescriptor subclassDescriptor = subclassDescriptors.get(entityName);
 		return subclassDescriptor != null ? subclassDescriptor : this;
 	}
 
 	public Collection<ObjAttribute> getDiscriminatorColumns() {
-		return allDiscriminatorColumns != null ? allDiscriminatorColumns : Collections.<ObjAttribute> emptyList();
+		return allDiscriminatorColumns != null ? allDiscriminatorColumns : Collections.emptyList();
 	}
 
 	public Collection<AttributeProperty> getIdProperties() {

cayenne/src/main/java/org/apache/cayenne/reflect/PersistentDescriptorFactory.java

@@ -65,7 +65,8 @@ public ClassDescriptor getDescriptor(String entityName) {
     protected ClassDescriptor getDescriptor(ObjEntity entity, Class<?> entityClass) {
         String superEntityName = entity.getSuperEntityName();
 
-        ClassDescriptor superDescriptor = (superEntityName != null) ? descriptorMap.getDescriptor(superEntityName)
+        ClassDescriptor superDescriptor = (superEntityName != null)
+                ? descriptorMap.getDescriptor(superEntityName)
                 : null;
 
         PersistentDescriptor descriptor = createDescriptor();
@@ -175,7 +176,7 @@ protected void indexSubclassDescriptors(PersistentDescriptor descriptor, EntityI
 
             for (EntityInheritanceTree child : inheritanceTree.getChildren()) {
                 ObjEntity childEntity = child.getEntity();
-                descriptor.addSubclassDescriptor(childEntity.getClassName(),
+                descriptor.addSubclassDescriptor(childEntity.getName(),
                         descriptorMap.getDescriptor(childEntity.getName()));
 
                 indexSubclassDescriptors(descriptor, child);
@@ -200,8 +201,7 @@ private void appendDeclaredRootDbEntity(PersistentDescriptor descriptor, ObjEnti
         DbEntity dbEntity = entity.getDbEntity();
         if (dbEntity != null) {
             // descriptor takes care of weeding off duplicates, which are likely
-            // in cases
-            // of non-horizontal inheritance
+            // in cases of non-horizontal inheritance
             descriptor.addRootDbEntity(dbEntity);
         }
     }

cayenne/src/main/java/org/apache/cayenne/util/DeepMergeOperation.java

@@ -83,7 +83,7 @@ private <T extends Persistent> T merge(
         final T target = shallowMergeOperation.merge(peerInParentContext);
         seen.put(id, target);
 
-        descriptor = descriptor.getSubclassDescriptor(peerInParentContext.getClass());
+        descriptor = descriptor.getSubclassDescriptor(id.getEntityName());
         descriptor.visitProperties(new PropertyVisitor() {
 
             public boolean visitToOne(ToOneProperty property) {

cayenne/src/test/java/org/apache/cayenne/access/VerticalInheritanceIT.java

@@ -20,6 +20,7 @@
 
 import org.apache.cayenne.Cayenne;
 import org.apache.cayenne.ObjectContext;
+import org.apache.cayenne.Persistent;
 import org.apache.cayenne.di.Inject;
 import org.apache.cayenne.query.ColumnSelect;
 import org.apache.cayenne.query.EJBQLQuery;
@@ -1234,4 +1235,37 @@ public void testColumnSelectVerticalInheritance_Sub1Sub1() throws SQLException {
 		assertEquals("mDA", result.getSub1Name());
 		assertEquals("3DQa", result.getSub1Sub1Name());
 	}
+
+	@Test
+	public void testInsertTwoGenericVerticalInheritanceObjects() {
+		// Generic DataObjects play nicer with a DataContext
+		final DataContext dataContext = (DataContext) context;
+
+		final Persistent girlEmma = dataContext.newObject("GenGirl");
+		final Persistent boyLuke = dataContext.newObject("GenBoy");
+
+		assertEquals("Girl is type G", girlEmma.readProperty("type"), "G");
+		assertEquals("Boy is type B", boyLuke.readProperty("type"), "B");
+
+		girlEmma.writeProperty("reference", "g1");
+		girlEmma.writeProperty("name", "Emma");
+		girlEmma.writeProperty("toyDolls", 5);
+
+		boyLuke.writeProperty("reference", "b1");
+		boyLuke.writeProperty("name", "Luke");
+		boyLuke.writeProperty("toyTrucks", 12);
+
+		context.commitChanges();
+
+		assertEquals(2, ObjectSelect.query(Persistent.class, "GenStudent").selectCount(context));
+
+		final List<Persistent> students = ObjectSelect.query(Persistent.class, "GenStudent").select(context);
+		assertTrue(students.contains(girlEmma));
+		assertTrue(students.contains(boyLuke));
+
+		final List<Persistent> girls = ObjectSelect.query(Persistent.class, "GenGirl").select(context);
+		assertEquals(1, girls.size());
+		final List<Persistent> boys = ObjectSelect.query(Persistent.class, "GenBoy").select(context);
+		assertEquals(1, boys.size());
+	}
 }

cayenne/src/test/java/org/apache/cayenne/reflect/generic/PersistentObjectDescriptorFactory_VerticalInheritanceIT.java

@@ -0,0 +1,65 @@
+/*****************************************************************
+ *   Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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
+ *
+ *    https://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.apache.cayenne.reflect.generic;
+
+import org.apache.cayenne.GenericPersistentObject;
+import org.apache.cayenne.access.types.ValueObjectTypeRegistry;
+import org.apache.cayenne.di.Inject;
+import org.apache.cayenne.map.EntityResolver;
+import org.apache.cayenne.reflect.ClassDescriptor;
+import org.apache.cayenne.reflect.SingletonFaultFactory;
+import org.apache.cayenne.unit.di.runtime.CayenneProjects;
+import org.apache.cayenne.unit.di.runtime.RuntimeCase;
+import org.apache.cayenne.unit.di.runtime.UseCayenneRuntime;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.mock;
+
+@UseCayenneRuntime(CayenneProjects.INHERITANCE_VERTICAL_PROJECT)
+public class PersistentObjectDescriptorFactory_VerticalInheritanceIT extends RuntimeCase {
+
+    @Inject
+    private EntityResolver resolver;
+
+    @Test
+    public void testVisitProperties_IterationOrder() {
+
+        PersistentObjectDescriptorFactory factory = new PersistentObjectDescriptorFactory(
+                resolver.getClassDescriptorMap(),
+                new SingletonFaultFactory(),
+                new DefaultValueComparisonStrategyFactory(mock(ValueObjectTypeRegistry.class))
+        );
+
+        ClassDescriptor genStudent = factory.getDescriptor("GenStudent");
+        assertNotNull(genStudent);
+        ClassDescriptor genBoy = genStudent.getSubclassDescriptor("GenBoy");
+        assertNotNull(genBoy);
+        ClassDescriptor genGirl = genStudent.getSubclassDescriptor("GenGirl");
+        assertNotNull(genGirl);
+
+        assertNotSame(genStudent, genBoy);
+        assertNotSame(genStudent, genGirl);
+        assertNotSame(genBoy, genGirl);
+
+        assertEquals(GenericPersistentObject.class, genBoy.getObjectClass());
+        assertEquals(GenericPersistentObject.class, genGirl.getObjectClass());
+        assertEquals(GenericPersistentObject.class, genStudent.getObjectClass());
+    }
+}

cayenne/src/test/resources/inheritance-vertical.map.xml

@@ -83,6 +83,20 @@
 		<db-attribute name="SUB1_NAME" type="VARCHAR" length="100"/>
 		<db-attribute name="SUB1_PRICE" type="DOUBLE"/>
 	</db-entity>
+	<db-entity name="GEN_STUDENT">
+		<db-attribute name="ID" type="INTEGER" isPrimaryKey="true" isMandatory="true"/>
+		<db-attribute name="NAME" type="VARCHAR" isMandatory="true" length="50"/>
+		<db-attribute name="REFERENCE" type="VARCHAR" isMandatory="true" length="10"/>
+		<db-attribute name="TYPE" type="CHAR" isMandatory="true" length="1"/>
+	</db-entity>
+	<db-entity name="GEN_BOY">
+		<db-attribute name="ID" type="INTEGER" isPrimaryKey="true" isMandatory="true"/>
+		<db-attribute name="TOY_TRUCKS" type="SMALLINT" length="4"/>
+	</db-entity>
+	<db-entity name="GEN_GIRL">
+		<db-attribute name="ID" type="INTEGER" isPrimaryKey="true" isMandatory="true"/>
+		<db-attribute name="TOY_DOLLS" type="SMALLINT" length="4"/>
+	</db-entity>
 	<db-entity name="IV_SUB1_SUB1">
 		<db-attribute name="ID" type="INTEGER" isPrimaryKey="true" isMandatory="true"/>
 		<db-attribute name="SUB1_SUB1_NAME" type="VARCHAR" length="100"/>
@@ -176,6 +190,19 @@
 		<qualifier><![CDATA[discriminator = "IvSub3"]]></qualifier>
 		<pre-persist method-name="onPrePersist"/>
 	</obj-entity>
+	<obj-entity name="GenStudent" abstract="true" dbEntityName="GEN_STUDENT">
+		<obj-attribute name="name" type="java.lang.String" db-attribute-path="NAME"/>
+		<obj-attribute name="reference" type="java.lang.String" db-attribute-path="REFERENCE"/>
+		<obj-attribute name="type" type="java.lang.String" db-attribute-path="TYPE"/>
+	</obj-entity>
+	<obj-entity name="GenBoy" superEntityName="GenStudent">
+		<qualifier><![CDATA[type = "B"]]></qualifier>
+		<obj-attribute name="toyTrucks" type="java.lang.Short" db-attribute-path="boy.TOY_TRUCKS"/>
+	</obj-entity>
+	<obj-entity name="GenGirl" superEntityName="GenStudent">
+		<qualifier><![CDATA[type = "G"]]></qualifier>
+		<obj-attribute name="toyDolls" type="java.lang.Short" db-attribute-path="girl.TOY_DOLLS"/>
+	</obj-entity>
 	<db-relationship name="sub1" source="IV1_ROOT" target="IV1_SUB1" toDependentPK="true">
 		<db-attribute-pair source="ID" target="ID"/>
 	</db-relationship>
@@ -290,6 +317,18 @@
 	<db-relationship name="ivRoot1" source="IV_SUB3" target="IV_ROOT">
 		<db-attribute-pair source="IV_ROOT_ID" target="ID"/>
 	</db-relationship>
+	<db-relationship name="student" source="GEN_BOY" target="GEN_STUDENT">
+		<db-attribute-pair source="ID" target="ID"/>
+	</db-relationship>
+	<db-relationship name="student" source="GEN_GIRL" target="GEN_STUDENT">
+		<db-attribute-pair source="ID" target="ID"/>
+	</db-relationship>
+	<db-relationship name="boy" source="GEN_STUDENT" target="GEN_BOY" toDependentPK="true">
+		<db-attribute-pair source="ID" target="ID"/>
+	</db-relationship>
+	<db-relationship name="girl" source="GEN_STUDENT" target="GEN_GIRL" toDependentPK="true">
+		<db-attribute-pair source="ID" target="ID"/>
+	</db-relationship>
 	<obj-relationship name="x" source="Iv2Sub1" target="Iv2X" deleteRule="Nullify" db-relationship-path="sub1.x"/>
 	<obj-relationship name="relatedConcrete" source="IvAbstract" target="IvConcrete" deleteRule="Nullify" db-relationship-path="relatedConcrete.abstract"/>
 	<obj-relationship name="others" source="IvBase" target="IvOther" deleteRule="Deny" db-relationship-path="others"/>