diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/AddWorkUnit.java b/hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/AddWorkUnit.java index c3d1ca5780..863e9861f9 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/AddWorkUnit.java +++ b/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 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(); - verCfg.getEntCfg().get(getEntityName()).getPropertyMapper().map(sessionImplementor, data, + this.data = new HashMap(); + 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; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/DelWorkUnit.java b/hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/DelWorkUnit.java index 9622fa03bb..8106cf0860 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/synchronization/work/DelWorkUnit.java +++ b/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) { diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/tools/Tools.java b/hibernate-envers/src/main/java/org/hibernate/envers/tools/Tools.java index 16ddbbcfce..32fc8fdc9b 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/tools/Tools.java +++ b/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 List iteratorToList(Iterator iter) { List ret = new ArrayList(); 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 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; + } } diff --git a/hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/merge/AddDelTest.java b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/merge/AddDelTest.java new file mode 100644 index 0000000000..9212f4ee8e --- /dev/null +++ b/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)); + } +} diff --git a/hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/merge/GivenIdStrEntity.java b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/merge/GivenIdStrEntity.java new file mode 100644 index 0000000000..088df91e39 --- /dev/null +++ b/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; + } +}