Merge pull request #204 from lukasz-antoniak/HHH-6753

HHH-6753 - Envers WorkUnits: Problems with merge

hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/AddWorkUnit.java

@@ -29,20 +29,24 @@ import java.util.Map;
 import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.envers.RevisionType;
 import org.hibernate.envers.configuration.AuditConfiguration;
+import org.hibernate.envers.tools.Tools;
 import org.hibernate.persister.entity.EntityPersister;
 
 /**
  * @author Adam Warski (adam at warski dot org)
+ * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
  */
 public class AddWorkUnit extends AbstractAuditWorkUnit implements AuditWorkUnit {
+    private final Object[] state;
     private final Map<String, Object> data;
 
     public AddWorkUnit(SessionImplementor sessionImplementor, String entityName, AuditConfiguration verCfg,
 					   Serializable id, EntityPersister entityPersister, Object[] state) {
         super(sessionImplementor, entityName, verCfg, id, RevisionType.ADD);
 
-        data = new HashMap<String, Object>();
+        this.data = new HashMap<String, Object>();
-        verCfg.getEntCfg().get(getEntityName()).getPropertyMapper().map(sessionImplementor, data,
+        this.state = state;
+        this.verCfg.getEntCfg().get(getEntityName()).getPropertyMapper().map(sessionImplementor, data,
 				entityPersister.getPropertyNames(), state, null);
     }
 
@@ -51,6 +55,8 @@ public class AddWorkUnit extends AbstractAuditWorkUnit implements AuditWorkUnit
         super(sessionImplementor, entityName, verCfg, id, RevisionType.ADD);
 
         this.data = data;
+        final String[] propertyNames = sessionImplementor.getFactory().getEntityPersister(getEntityName()).getPropertyNames();
+        this.state = Tools.mapToArray(data, propertyNames);
     }
 
     public boolean containsWork() {
@@ -62,6 +68,10 @@ public class AddWorkUnit extends AbstractAuditWorkUnit implements AuditWorkUnit
         return data;
     }
 
+    public Object[] getState() {
+        return state;
+    }
+
     public AuditWorkUnit merge(AddWorkUnit second) {
         return second;
     }

hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/DelWorkUnit.java

@@ -29,13 +29,16 @@ import java.util.Map;
 import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.envers.RevisionType;
 import org.hibernate.envers.configuration.AuditConfiguration;
+import org.hibernate.envers.tools.Tools;
 import org.hibernate.persister.entity.EntityPersister;
 
 /**
  * @author Adam Warski (adam at warski dot org)
+ * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
  */
 public class DelWorkUnit extends AbstractAuditWorkUnit implements AuditWorkUnit {
     private final Object[] state;
+    private final EntityPersister entityPersister;
     private final String[] propertyNames;
 
     public DelWorkUnit(SessionImplementor sessionImplementor, String entityName, AuditConfiguration verCfg,
@@ -43,6 +46,7 @@ public class DelWorkUnit extends AbstractAuditWorkUnit implements AuditWorkUnit
         super(sessionImplementor, entityName, verCfg, id, RevisionType.DEL);
 
         this.state = state;
+        this.entityPersister = entityPersister;
         this.propertyNames = entityPersister.getPropertyNames();
     }
 
@@ -63,7 +67,10 @@ public class DelWorkUnit extends AbstractAuditWorkUnit implements AuditWorkUnit
     }
 
     public AuditWorkUnit merge(AddWorkUnit second) {
-        return null;
+        if (Tools.arraysEqual(second.getState(), state)) {
+            return null; // Return null if object's state has not changed.
+        }
+        return new ModWorkUnit(sessionImplementor, entityName, verCfg, id, entityPersister, second.getState(), state); 
     }
 
     public AuditWorkUnit merge(ModWorkUnit second) {

hibernate-envers/src/main/java/org/hibernate/envers/tools/Tools.java

@@ -125,6 +125,17 @@ public class Tools {
         return obj1.equals(obj2);
     }
 
+    public static boolean arraysEqual(Object[] array1, Object[] array2) {
+        if (array1 == null) return array2 == null;
+        if (array2 == null || array1.length != array2.length) return false;
+        for (int i = 0; i < array1.length; ++i) {
+            if (array1[i] != null ? !array1[i].equals(array2[i]) : array2[i] != null) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     public static <T> List<T> iteratorToList(Iterator<T> iter) {
         List<T> ret = new ArrayList<T>();
         while (iter.hasNext()) {
@@ -189,4 +200,19 @@ public class Tools {
         EntityPersister entityPersister = sessionImplementor.getFactory().getEntityPersister(entityName);
         return entityPersister.getMappedClass();
     }
+
+    /**
+     * Converts map's value set to an array. {@code keys} parameter specifies requested elements and their order. 
+     * @param data Source map.
+     * @param keys Array of keys that represent requested map values.
+     * @return Array of values stored in the map under specified keys. If map does not contain requested key,
+     *         {@code null} is inserted.
+     */
+    public static Object[] mapToArray(Map<String, Object> data, String[] keys) {
+        Object[] ret = new Object[keys.length];
+        for (int i = 0; i < keys.length; ++i) {
+            ret[i] = data.get(keys[i]);
+        }
+        return ret;
+    }
 }

hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/merge/AddDelTest.java

@@ -0,0 +1,60 @@
+package org.hibernate.envers.test.integration.merge;
+
+import org.hibernate.envers.test.AbstractSessionTest;
+import org.hibernate.envers.test.Priority;
+import org.hibernate.envers.test.entities.StrTestEntity;
+import org.hibernate.testing.TestForIssue;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+/**
+ * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
+ */
+@TestForIssue(jiraKey = "HHH-6753")
+public class AddDelTest extends AbstractSessionTest {
+    @Override
+    protected void initMappings() {
+        config.addAnnotatedClass(StrTestEntity.class);
+        config.addAnnotatedClass(GivenIdStrEntity.class);
+    }
+
+    @Test
+    @Priority(10)
+    public void initData() {
+        // Revision 1
+        getSession().getTransaction().begin();
+        GivenIdStrEntity entity = new GivenIdStrEntity(1, "data");
+        getSession().persist(entity);
+        getSession().getTransaction().commit();
+
+        // Revision 2
+        getSession().getTransaction().begin();
+        getSession().persist(new StrTestEntity("another data")); // Just to create second revision.
+        entity = (GivenIdStrEntity) getSession().get(GivenIdStrEntity.class, 1);
+        getSession().delete(entity); // First try to remove the entity.
+        getSession().save(entity); // Then save it.
+        getSession().getTransaction().commit();
+
+        // Revision 3
+        getSession().getTransaction().begin();
+        entity = (GivenIdStrEntity) getSession().get(GivenIdStrEntity.class, 1);
+        getSession().delete(entity); // First try to remove the entity.
+        entity.setData("modified data"); // Then change it's state.
+        getSession().save(entity); // Finally save it.
+        getSession().getTransaction().commit();
+    }
+
+    @Test
+    public void testRevisionsCountOfGivenIdStrEntity() {
+        // Revision 2 has not changed entity's state.
+        Assert.assertEquals(Arrays.asList(1, 3), getAuditReader().getRevisions(GivenIdStrEntity.class, 1));
+    }
+
+    @Test
+    public void testHistoryOfGivenIdStrEntity() {
+        Assert.assertEquals(new GivenIdStrEntity(1, "data"), getAuditReader().find(GivenIdStrEntity.class, 1, 1));
+        Assert.assertEquals(new GivenIdStrEntity(1, "modified data"), getAuditReader().find(GivenIdStrEntity.class, 1, 3));
+    }
+}

hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/merge/GivenIdStrEntity.java

@@ -0,0 +1,67 @@
+package org.hibernate.envers.test.integration.merge;
+
+import org.hibernate.envers.Audited;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+/**
+ * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
+ */
+@Entity
+@Audited
+public class GivenIdStrEntity {
+    @Id
+    private Integer id;
+
+    private String data;
+
+    public GivenIdStrEntity() {
+    }
+
+    public GivenIdStrEntity(Integer id, String data) {
+        this.id = id;
+        this.data = data;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof GivenIdStrEntity)) return false;
+
+        GivenIdStrEntity that = (GivenIdStrEntity) o;
+
+        if (data != null ? !data.equals(that.data) : that.data != null) return false;
+        if (id != null ? !id.equals(that.id) : that.id != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = id != null ? id.hashCode() : 0;
+        result = 31 * result + (data != null ? data.hashCode() : 0);
+        return result;
+    }
+
+    @Override
+    public String toString() {
+        return "GivenIdStrEntity(id = " + id + ", data = " + data + ")";
+    }
+
+    public Integer getId() {
+        return id;
+    }
+
+    public void setId(Integer id) {
+        this.id = id;
+    }
+
+    public String getData() {
+        return data;
+    }
+
+    public void setData(String data) {
+        this.data = data;
+    }
+}