Merge pull request #77 from skowronm/HHH-6177

HHH-6177: fixing problems with mixed inheritance by not using the properties tag when creating audit entities mapping

hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/ToOneRelationMetadataGenerator.java

@@ -86,9 +86,15 @@ public final class ToOneRelationMetadataGenerator {
 
         MetadataTools.prefixNamesInPropertyElement(properties, lastPropertyPrefix,
                 MetadataTools.getColumnNameIterator(value.getColumnIterator()), false, insertable);
-        parent.add(properties);
 
-        // Adding mapper for the id
+		// Extracting related id properties from properties tag
+		for (Object o : properties.content()) {
+			Element element = (Element) o;
+			element.setParent(null);
+			parent.add(element);
+		}
+
+		// Adding mapper for the id
         PropertyData propertyData = propertyAuditingData.getPropertyData();
         mapper.addComposite(propertyData, new ToOneIdMapper(relMapper, propertyData, referencedEntityName, nonInsertableFake));
     }

hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java

@@ -22,10 +22,7 @@
  * Boston, MA  02110-1301  USA
  */
 package org.hibernate.envers.entities.mapper.relation;
-import java.io.Serializable;
+
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
 import org.hibernate.collection.spi.PersistentCollection;
 import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.envers.configuration.AuditConfiguration;
@@ -40,6 +37,11 @@ import org.hibernate.envers.tools.Tools;
 import org.hibernate.envers.tools.reflection.ReflectionTools;
 import org.hibernate.property.Setter;
 
+import java.io.Serializable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 /**
  * @author Adam Warski (adam at warski dot org)
  * @author Hern<EFBFBD>n Chanfreau
@@ -59,13 +61,16 @@ public class ToOneIdMapper implements PropertyMapper {
 
     public boolean mapToMapFromEntity(SessionImplementor session, Map<String, Object> data, Object newObj, Object oldObj) {
         HashMap<String, Object> newData = new HashMap<String, Object>();
-        data.put(propertyData.getName(), newData);
 
         // If this property is originally non-insertable, but made insertable because it is in a many-to-one "fake"
         // bi-directional relation, we always store the "old", unchaged data, to prevent storing changes made
         // to this field. It is the responsibility of the collection to properly update it if it really changed.
         delegate.mapToMapFromEntity(newData, nonInsertableFake ? oldObj : newObj);
 
+		for (Map.Entry<String, Object> entry : newData.entrySet()) {
+			data.put(entry.getKey(), entry.getValue());
+		}
+
         //noinspection SimplifiableConditionalExpression
         return nonInsertableFake ? false : !Tools.entitiesEqual(session, referencedEntityName, newObj, oldObj);
     }
@@ -76,7 +81,8 @@ public class ToOneIdMapper implements PropertyMapper {
             return;
         }
 
-        Object entityId = delegate.mapToIdFromMap((Map) data.get(propertyData.getName()));
+		Object entityId;
+		entityId = delegate.mapToIdFromMap(data);
         Object value;
         if (entityId == null) {
             value = null;
@@ -86,10 +92,10 @@ public class ToOneIdMapper implements PropertyMapper {
             } else {
             	EntityConfiguration entCfg = verCfg.getEntCfg().get(referencedEntityName);
             	if(entCfg == null) {
-            		// a relation marked as RelationTargetAuditMode.NOT_AUDITED 
+            		// a relation marked as RelationTargetAuditMode.NOT_AUDITED
             		entCfg = verCfg.getEntCfg().getNotVersionEntityConfiguration(referencedEntityName);
             	}
-            	
+
                 Class<?> entityClass = ReflectionTools.loadClass(entCfg.getEntityClassName());
 
                 value = versionsReader.getSessionImplementor().getFactory().getEntityPersister(referencedEntityName).

hibernate-envers/src/main/java/org/hibernate/envers/query/criteria/NotNullAuditExpression.java

@@ -45,7 +45,7 @@ public class NotNullAuditExpression implements AuditCriterion {
         if (relatedEntity == null) {
             parameters.addNotNullRestriction(propertyName, true);
         } else {
-            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, null, propertyName, false);
+            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, null, null, false);
         }
     }
 }

hibernate-envers/src/main/java/org/hibernate/envers/query/criteria/NullAuditExpression.java

@@ -45,7 +45,7 @@ public class NullAuditExpression implements AuditCriterion {
         if (relatedEntity == null) {
             parameters.addNullRestriction(propertyName, true);
         } else {
-            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, null, propertyName, true);
+            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, null, null, true);
         }
     }
 }

hibernate-envers/src/main/java/org/hibernate/envers/query/criteria/RelatedAuditExpression.java

@@ -52,7 +52,7 @@ public class RelatedAuditExpression implements AuditCriterion {
             throw new AuditException("This criterion can only be used on a property that is " +
                     "a relation to another property.");
         } else {
-            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, id, propertyName, equals);
+            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, id, null, equals);
         }
     }
 }

hibernate-envers/src/main/java/org/hibernate/envers/query/criteria/SimpleAuditExpression.java

@@ -58,7 +58,7 @@ public class SimpleAuditExpression implements AuditCriterion {
 
             Object id = relatedEntity.getIdMapper().mapToIdFromEntity(value);
 
-            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, id, propertyName, "=".equals(op));
+            relatedEntity.getIdMapper().addIdEqualsToQuery(parameters, id, null, "=".equals(op));
         }
     }
 }

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/inheritance/mixed/MixedInheritanceStrategiesEntityTest.java

@@ -0,0 +1,141 @@
+package org.hibernate.envers.test.integration.inheritance.mixed;
+
+import org.hibernate.ejb.Ejb3Configuration;
+import org.hibernate.envers.test.AbstractEntityTest;
+import org.hibernate.envers.test.Priority;
+import org.hibernate.envers.test.integration.inheritance.mixed.entities.*;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Michal Skowronek (mskowr at o2 pl)
+ */
+public class MixedInheritanceStrategiesEntityTest extends AbstractEntityTest {
+
+	private ActivityId id2;
+	private ActivityId id1;
+	private ActivityId id3;
+
+	@Override
+    public void configure(Ejb3Configuration cfg) {
+        cfg.addAnnotatedClass(AbstractActivity.class);
+        cfg.addAnnotatedClass(AbstractCheckActivity.class);
+        cfg.addAnnotatedClass(CheckInActivity.class);
+        cfg.addAnnotatedClass(NormalActivity.class);
+    }
+
+    @Test
+    @Priority(10)
+    public void initData() {
+        NormalActivity normalActivity = new NormalActivity();
+		id1 = new ActivityId(1, 2);
+		normalActivity.setId(id1);
+        normalActivity.setSequenceNumber(1);
+
+        // Revision 1
+        getEntityManager().getTransaction().begin();
+
+        getEntityManager().persist(normalActivity);
+
+        getEntityManager().getTransaction().commit();
+        // Revision 2
+        getEntityManager().getTransaction().begin();
+
+        normalActivity = getEntityManager().find(NormalActivity.class, id1);
+        CheckInActivity checkInActivity = new CheckInActivity();
+		id2 = new ActivityId(2, 3);
+		checkInActivity.setId(id2);
+        checkInActivity.setSequenceNumber(0);
+        checkInActivity.setDurationInMinutes(30);
+        checkInActivity.setRelatedActivity(normalActivity);
+
+        getEntityManager().persist(checkInActivity);
+
+        getEntityManager().getTransaction().commit();
+
+        // Revision 3
+        normalActivity = new NormalActivity();
+		id3 = new ActivityId(3, 4);
+		normalActivity.setId(id3);
+        normalActivity.setSequenceNumber(2);
+
+        getEntityManager().getTransaction().begin();
+
+        getEntityManager().persist(normalActivity);
+
+        getEntityManager().getTransaction().commit();
+
+        // Revision 4
+        getEntityManager().getTransaction().begin();
+
+        normalActivity = getEntityManager().find(NormalActivity.class, id3);
+        checkInActivity = getEntityManager().find(CheckInActivity.class, id2);
+        checkInActivity.setRelatedActivity(normalActivity);
+
+        getEntityManager().merge(checkInActivity);
+
+        getEntityManager().getTransaction().commit();
+    }
+
+    @Test
+    public void testRevisionsCounts() {
+        assertEquals(Arrays.asList(1), getAuditReader().getRevisions(NormalActivity.class, id1));
+        assertEquals(Arrays.asList(3), getAuditReader().getRevisions(NormalActivity.class, id3));
+        assertEquals(Arrays.asList(2, 4), getAuditReader().getRevisions(CheckInActivity.class, id2));
+    }
+
+    @Test
+    public void testCurrentStateOfCheckInActivity() {
+
+        final CheckInActivity checkInActivity = getEntityManager().find(CheckInActivity.class, id2);
+        final NormalActivity normalActivity = getEntityManager().find(NormalActivity.class, id3);
+
+        assertEquals(id2, checkInActivity.getId());
+        assertEquals(0, checkInActivity.getSequenceNumber().intValue());
+        assertEquals(30, checkInActivity.getDurationInMinutes().intValue());
+        final Activity relatedActivity = checkInActivity.getRelatedActivity();
+        assertEquals(normalActivity.getId(), relatedActivity.getId());
+        assertEquals(normalActivity.getSequenceNumber(), relatedActivity.getSequenceNumber());
+    }
+
+    @Test
+    public void testCheckCurrentStateOfNormalActivities() throws Exception {
+        final NormalActivity normalActivity1 = getEntityManager().find(NormalActivity.class, id1);
+        final NormalActivity normalActivity2 = getEntityManager().find(NormalActivity.class, id3);
+
+        assertEquals(id1, normalActivity1.getId());
+        assertEquals(1, normalActivity1.getSequenceNumber().intValue());
+        assertEquals(id3, normalActivity2.getId());
+        assertEquals(2, normalActivity2.getSequenceNumber().intValue());
+    }
+
+    @Test
+    public void doTestFirstRevisionOfCheckInActivity() throws Exception {
+        CheckInActivity checkInActivity = getAuditReader().find(CheckInActivity.class, id2, 2);
+        NormalActivity normalActivity = getAuditReader().find(NormalActivity.class, id1, 2);
+
+        assertEquals(id2, checkInActivity.getId());
+        assertEquals(0, checkInActivity.getSequenceNumber().intValue());
+        assertEquals(30, checkInActivity.getDurationInMinutes().intValue());
+        Activity relatedActivity = checkInActivity.getRelatedActivity();
+        assertEquals(normalActivity.getId(), relatedActivity.getId());
+        assertEquals(normalActivity.getSequenceNumber(), relatedActivity.getSequenceNumber());
+    }
+
+    @Test
+    public void doTestSecondRevisionOfCheckInActivity() throws Exception {
+        CheckInActivity checkInActivity = getAuditReader().find(CheckInActivity.class, id2, 4);
+        NormalActivity normalActivity = getAuditReader().find(NormalActivity.class, id3, 4);
+
+        assertEquals(id2, checkInActivity.getId());
+        assertEquals(0, checkInActivity.getSequenceNumber().intValue());
+        assertEquals(30, checkInActivity.getDurationInMinutes().intValue());
+        Activity relatedActivity = checkInActivity.getRelatedActivity();
+        assertEquals(normalActivity.getId(), relatedActivity.getId());
+        assertEquals(normalActivity.getSequenceNumber(), relatedActivity.getSequenceNumber());
+    }
+
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/inheritance/mixed/entities/AbstractActivity.java

@@ -0,0 +1,34 @@
+package org.hibernate.envers.test.integration.inheritance.mixed.entities;
+
+import org.hibernate.envers.Audited;
+
+import javax.persistence.EmbeddedId;
+import javax.persistence.Entity;
+import javax.persistence.Inheritance;
+import javax.persistence.InheritanceType;
+
+@Audited
+@Entity
+@Inheritance(strategy = InheritanceType.SINGLE_TABLE)
+public abstract class AbstractActivity implements Activity {
+	@EmbeddedId
+    private ActivityId id;
+
+    private Integer sequenceNumber;
+
+    public ActivityId getId() {
+        return id;
+    }
+
+    public void setId(ActivityId id) {
+        this.id = id;
+    }
+
+	public Integer getSequenceNumber() {
+        return sequenceNumber;
+    }
+
+    public void setSequenceNumber(Integer sequenceNumber) {
+        this.sequenceNumber = sequenceNumber;
+    }
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/inheritance/mixed/entities/AbstractCheckActivity.java

@@ -0,0 +1,36 @@
+package org.hibernate.envers.test.integration.inheritance.mixed.entities;
+
+import org.hibernate.envers.Audited;
+
+import javax.persistence.*;
+
+@Audited
+@Entity
+@DiscriminatorValue(value = "CHECK")
+@SecondaryTable(name = "ACTIVITY_CHECK",
+		pkJoinColumns = {@PrimaryKeyJoinColumn(name = "ACTIVITY_ID"),
+						@PrimaryKeyJoinColumn(name = "ACTIVITY_ID2")})
+public abstract class AbstractCheckActivity extends AbstractActivity {
+    @Column(table = "ACTIVITY_CHECK")
+    private Integer durationInMinutes;
+    @ManyToOne(targetEntity = AbstractActivity.class, cascade = CascadeType.MERGE, fetch = FetchType.LAZY)
+	@JoinColumns({@JoinColumn(table = "ACTIVITY_CHECK", referencedColumnName = "id"),
+			@JoinColumn(table = "ACTIVITY_CHECK", referencedColumnName = "id2")})
+    private Activity relatedActivity;
+
+    public Integer getDurationInMinutes() {
+        return durationInMinutes;
+    }
+
+    public void setDurationInMinutes(Integer durationInMinutes) {
+        this.durationInMinutes = durationInMinutes;
+    }
+
+    public Activity getRelatedActivity() {
+        return relatedActivity;
+    }
+
+    public void setRelatedActivity(Activity relatedActivity) {
+        this.relatedActivity = relatedActivity;
+    }
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/inheritance/mixed/entities/Activity.java

@@ -0,0 +1,9 @@
+package org.hibernate.envers.test.integration.inheritance.mixed.entities;
+
+import java.io.Serializable;
+
+public interface Activity extends Serializable {
+	ActivityId getId();
+
+    Integer getSequenceNumber();
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/inheritance/mixed/entities/ActivityId.java

@@ -0,0 +1,53 @@
+package org.hibernate.envers.test.integration.inheritance.mixed.entities;
+
+import javax.persistence.Embeddable;
+import java.io.Serializable;
+
+@Embeddable
+public class ActivityId implements Serializable {
+	private Integer id;
+	private Integer id2;
+
+	public ActivityId() {
+	}
+
+	public ActivityId(int i, int i1) {
+		id = i;
+		id2 = i1;
+	}
+
+	public Integer getId() {
+		return id;
+	}
+
+	public void setId(Integer id) {
+		this.id = id;
+	}
+
+	public Integer getId2() {
+		return id2;
+	}
+
+	public void setId2(Integer id2) {
+		this.id2 = id2;
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (obj == null) {
+			return true;
+		}
+		if (!(obj instanceof ActivityId)) {
+			return false;
+		}
+		ActivityId id = (ActivityId) obj;
+		return getId().equals(id.getId()) && getId2().equals(id.getId2());
+	}
+
+	@Override
+	public int hashCode() {
+		int result = getId().hashCode();
+		result = 31 * result + getId2().hashCode();
+		return result;
+	}
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/inheritance/mixed/entities/CheckInActivity.java

@@ -0,0 +1,30 @@
+package org.hibernate.envers.test.integration.inheritance.mixed.entities;
+
+import org.hibernate.envers.Audited;
+
+import javax.persistence.DiscriminatorValue;
+import javax.persistence.Entity;
+
+@Audited
+@Entity
+@DiscriminatorValue(value = "CHECK_IN")
+public class CheckInActivity extends AbstractCheckActivity {
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == this) {
+            return true;
+        }
+        if (!(obj instanceof CheckInActivity)) {
+            return false;
+        }
+        CheckInActivity checkInActivity = (CheckInActivity) obj;
+        return getId().equals(checkInActivity.getId());
+    }
+
+    @Override
+    public int hashCode() {
+        return getId().hashCode();
+    }
+
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/inheritance/mixed/entities/NormalActivity.java

@@ -0,0 +1,29 @@
+package org.hibernate.envers.test.integration.inheritance.mixed.entities;
+
+import org.hibernate.envers.Audited;
+
+import javax.persistence.DiscriminatorValue;
+import javax.persistence.Entity;
+
+@Audited
+@Entity
+@DiscriminatorValue(value = "NORMAL")
+public class NormalActivity extends AbstractActivity {
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == this) {
+            return true;
+        }
+        if (!(obj instanceof NormalActivity)) {
+            return false;
+        }
+        NormalActivity normalActivity = (NormalActivity) obj;
+        return getId().equals(normalActivity.getId());
+    }
+
+    @Override
+    public int hashCode() {
+        return getId().hashCode();
+    }
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/naming/JoinNaming.java

@@ -33,6 +33,8 @@ import javax.persistence.EntityManager;
 import java.util.Arrays;
 import java.util.Iterator;
 
+import static junit.framework.Assert.*;
+
 /**
  * @author Adam Warski (adam at warski dot org)
  */
@@ -122,16 +124,11 @@ public class JoinNaming extends AbstractEntityTest {
     @SuppressWarnings({"unchecked"})
     @Test
     public void testJoinColumnName() {
-        Iterator<Column> columns =
+		Iterator<Column> columns =
-                getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.JoinNamingRefIngEntity_AUD")
+				getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.JoinNamingRefIngEntity_AUD")
-                .getProperty("reference").getColumnIterator();
+						.getProperty("reference_id").getColumnIterator();
-
+		assertTrue(columns.hasNext());
-        while (columns.hasNext()) {
+		assertEquals("jnree_column_reference", columns.next().getName());
-            if ("jnree_column_reference".equals(columns.next().getName())) {
+		assertFalse(columns.hasNext());
-                return;
+	}
-            }
-        }
-
-        assert false;
-    }
 }

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/naming/ids/JoinEmbIdNaming.java

@@ -33,6 +33,8 @@ import javax.persistence.EntityManager;
 import java.util.Arrays;
 import java.util.Iterator;
 
+import static junit.framework.Assert.*;
+
 /**
  * @author Adam Warski (adam at warski dot org)
  */
@@ -120,22 +122,18 @@ public class JoinEmbIdNaming extends AbstractEntityTest {
     @SuppressWarnings({"unchecked"})
     @Test
     public void testJoinColumnNames() {
-        Iterator<Column> columns =
+		Iterator<Column> columns =
-                getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.ids.JoinEmbIdNamingRefIngEntity_AUD")
+				getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.ids.JoinEmbIdNamingRefIngEntity_AUD")
-                .getProperty("reference").getColumnIterator();
+						.getProperty("reference_x").getColumnIterator();
+		assertTrue(columns.hasNext());
+		assertEquals("XX_reference", columns.next().getName());
+		assertFalse(columns.hasNext());
 
-        boolean xxFound = false;
+		columns = getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.ids.JoinEmbIdNamingRefIngEntity_AUD")
-        boolean yyFound = false;
+				.getProperty("reference_y").getColumnIterator();
-        while (columns.hasNext()) {
-            if ("XX_reference".equals(columns.next().getName())) {
-                xxFound = true;
-            }
 
-            if ("YY_reference".equals(columns.next().getName())) {
+		assertTrue(columns.hasNext());
-                yyFound = true;
+		assertEquals("YY_reference", columns.next().getName());
-            }
+		assertFalse(columns.hasNext());
-        }
+	}
-
-        assert xxFound && yyFound;
-    }
 }

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/naming/ids/JoinMulIdNaming.java

@@ -33,6 +33,8 @@ import javax.persistence.EntityManager;
 import java.util.Arrays;
 import java.util.Iterator;
 
+import static junit.framework.Assert.*;
+
 /**
  * @author Adam Warski (adam at warski dot org)
  */
@@ -120,22 +122,17 @@ public class JoinMulIdNaming extends AbstractEntityTest {
     @SuppressWarnings({"unchecked"})
     @Test
     public void testJoinColumnNames() {
-        Iterator<Column> columns =
+		Iterator<Column> columns =
-                getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.ids.JoinMulIdNamingRefIngEntity_AUD")
+				getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.ids.JoinMulIdNamingRefIngEntity_AUD")
-                .getProperty("reference").getColumnIterator();
+						.getProperty("reference_id1").getColumnIterator();
+		assertTrue(columns.hasNext());
+		assertEquals("ID1_reference", columns.next().getName());
+		assertFalse(columns.hasNext());
 
-        boolean id1Found = false;
+		columns = getCfg().getClassMapping("org.hibernate.envers.test.integration.naming.ids.JoinMulIdNamingRefIngEntity_AUD")
-        boolean id2Found = false;
+				.getProperty("reference_id2").getColumnIterator();
-        while (columns.hasNext()) {
+		assertTrue(columns.hasNext());
-            if ("ID1_reference".equals(columns.next().getName())) {
+		assertEquals("ID2_reference", columns.next().getName());
-                id1Found = true;
+		assertFalse(columns.hasNext());
-            }
+	}
-
-            if ("ID2_reference".equals(columns.next().getName())) {
-                id2Found = true;
-            }
-        }
-
-        assert id1Found && id2Found;
-    }
 }