Merge pull request #94 from olilo/aeb3b9710ca7f3aa400f

3.6 - Envers - Bugfix for ValidityAuditStrategy

hibernate-envers/src/main/java/org/hibernate/envers/strategy/ValidityAuditStrategy.java

@@ -63,7 +63,13 @@ public class ValidityAuditStrategy implements AuditStrategy {
             IdMapper idMapper = auditCfg.getEntCfg().get(entityName).getIdMapper();
             idMapper.addIdEqualsToQuery(qb.getRootParameters(), id, auditCfg.getAuditEntCfg().getOriginalIdPropName(), true);
 
-            updateLastRevision(session, auditCfg, qb, id, auditedEntityName, revision);
+            // e.end_rev is null
+            qb.getRootParameters().addWhere(auditCfg.getAuditEntCfg().getRevisionEndFieldName(), true, "is", "null", false);
+
+            @SuppressWarnings({"unchecked"})
+            List<Object> l = qb.toQuery(session).list();
+
+            updateLastRevision(session, auditCfg, l, id, auditedEntityName, revision);
         }
 
         // Save the audit data
@@ -73,27 +79,28 @@ public class ValidityAuditStrategy implements AuditStrategy {
     @SuppressWarnings({"unchecked"})
     public void performCollectionChange(Session session, AuditConfiguration auditCfg,
                                         PersistentCollectionChangeData persistentCollectionChangeData, Object revision) {
-        // Update the end date of the previous row if this operation is expected to have a previous row
-        if (getRevisionType(auditCfg, persistentCollectionChangeData.getData()) != RevisionType.ADD) {
-            /*
-             Constructing a query (there are multiple id fields):
-             select e from audited_middle_ent e where e.end_rev is null and e.id1 = :id1 and e.id2 = :id2 ...
-             */
 
-            QueryBuilder qb = new QueryBuilder(persistentCollectionChangeData.getEntityName(), "e");
+        final QueryBuilder qb = new QueryBuilder(persistentCollectionChangeData.getEntityName(), "e");
 
-            // Adding a parameter for each id component, except the rev number
+        // Adding a parameter for each id component, except the rev number
-            String originalIdPropName = auditCfg.getAuditEntCfg().getOriginalIdPropName();
+        final String originalIdPropName = auditCfg.getAuditEntCfg().getOriginalIdPropName();
-            Map<String, Object> originalId = (Map<String, Object>) persistentCollectionChangeData.getData().get(
+        final Map<String, Object> originalId = (Map<String, Object>) persistentCollectionChangeData.getData().get(
-                    originalIdPropName);
+                originalIdPropName);
-            for (Map.Entry<String, Object> originalIdEntry : originalId.entrySet()) {
+        for (Map.Entry<String, Object> originalIdEntry : originalId.entrySet()) {
-                if (!auditCfg.getAuditEntCfg().getRevisionFieldName().equals(originalIdEntry.getKey())) {
+            if (!auditCfg.getAuditEntCfg().getRevisionFieldName().equals(originalIdEntry.getKey())) {
-                    qb.getRootParameters().addWhereWithParam(originalIdPropName + "." + originalIdEntry.getKey(),
+                qb.getRootParameters().addWhereWithParam(originalIdPropName + "." + originalIdEntry.getKey(),
-                            true, "=", originalIdEntry.getValue());
+                        true, "=", originalIdEntry.getValue());
-                }
             }
+        }
 
-            updateLastRevision(session, auditCfg, qb, originalId, persistentCollectionChangeData.getEntityName(), revision);
+        // e.end_rev is null
+        qb.getRootParameters().addWhere(auditCfg.getAuditEntCfg().getRevisionEndFieldName(), true, "is", "null", false);
+
+        final List<Object> l = qb.toQuery(session).list();
+
+        // Update the last revision if there exists such a last revision
+        if (l.size() > 0) {
+            updateLastRevision(session, auditCfg, l, originalId, persistentCollectionChangeData.getEntityName(), revision);
         }
 
         // Save the audit data
@@ -136,19 +143,14 @@ public class ValidityAuditStrategy implements AuditStrategy {
     }
 
     @SuppressWarnings({"unchecked"})
-    private void updateLastRevision(Session session, AuditConfiguration auditCfg, QueryBuilder qb,
+    private void updateLastRevision(Session session, AuditConfiguration auditCfg, List<Object> l,
                                     Object id, String auditedEntityName, Object revision) {
-        String revisionEndFieldName = auditCfg.getAuditEntCfg().getRevisionEndFieldName();
-        
-        // e.end_rev is null
-        qb.getRootParameters().addWhere(revisionEndFieldName, true, "is", "null", false);
-
-        List<Object> l = qb.toQuery(session).list();
 
         // There should be one entry
         if (l.size() == 1) {
             // Setting the end revision to be the current rev
             Object previousData = l.get(0);
+            String revisionEndFieldName = auditCfg.getAuditEntCfg().getRevisionEndFieldName();
             ((Map<String, Object>) previousData).put(revisionEndFieldName, revision);
 
             if (auditCfg.getAuditEntCfg().isRevisionEndTimestampEnabled()) {

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/strategy/ValidityAuditStrategyManyToManyTest.java

@@ -0,0 +1,113 @@
+package org.hibernate.envers.test.integration.strategy;
+
+import java.util.HashSet;
+
+import javax.persistence.EntityManager;
+
+import org.hibernate.ejb.Ejb3Configuration;
+import org.hibernate.envers.test.AbstractEntityTest;
+import org.hibernate.envers.test.entities.manytomany.SetOwnedEntity;
+import org.hibernate.envers.test.entities.manytomany.SetOwningEntity;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+/**
+ * Tests the ValidityAuditStrategy on many-to-many Sets.
+ * It was first introduced because of a bug when adding and removing the same element
+ * from the set multiple times between database persists.
+ * Created on: 24.05.11
+ *
+ * @author Oliver Lorenz
+ * @since 3.6.5
+ */
+public class ValidityAuditStrategyManyToManyTest extends AbstractEntityTest {
+
+    private Integer ing_id;
+
+    private Integer ed_id;
+
+    @Override
+    public void configure(Ejb3Configuration cfg) {
+        cfg.addAnnotatedClass(SetOwningEntity.class);
+        cfg.addAnnotatedClass(SetOwnedEntity.class);
+
+        cfg.setProperty("org.hibernate.envers.audit_strategy",
+                "org.hibernate.envers.strategy.ValidityAuditStrategy");
+    }
+
+    @BeforeClass(enabled = true, dependsOnMethods = "init")
+    public void initData() {
+        final EntityManager em = getEntityManager();
+
+        final SetOwningEntity setOwningEntity = new SetOwningEntity(1, "parent");
+        final SetOwnedEntity setOwnedEntity = new SetOwnedEntity(2, "child");
+
+        // Initial persist
+        em.getTransaction().begin();
+
+        em.persist(setOwningEntity);
+        em.persist(setOwnedEntity);
+
+        em.getTransaction().commit();
+        em.clear();
+
+        ing_id = setOwningEntity.getId();
+        ed_id = setOwnedEntity.getId();
+    }
+
+    @Test(enabled = true)
+    public void testMultipleAddAndRemove() {
+        final EntityManager em = getEntityManager();
+
+        // add child for first time
+        em.getTransaction().begin();
+
+        SetOwningEntity owningEntity = getEntityManager().find(SetOwningEntity.class, ing_id);
+        SetOwnedEntity ownedEntity = getEntityManager().find(SetOwnedEntity.class, ed_id);
+
+        owningEntity.setReferences(new HashSet<SetOwnedEntity>());
+        owningEntity.getReferences().add(ownedEntity);
+
+        em.getTransaction().commit();
+        em.clear();
+
+        // remove child
+        em.getTransaction().begin();
+
+        owningEntity = getEntityManager().find(SetOwningEntity.class, ing_id);
+        ownedEntity = getEntityManager().find(SetOwnedEntity.class, ed_id);
+
+        owningEntity.getReferences().remove(ownedEntity);
+
+        em.getTransaction().commit();
+        em.clear();
+
+        // add child again
+        em.getTransaction().begin();
+
+        owningEntity = getEntityManager().find(SetOwningEntity.class, ing_id);
+        ownedEntity = getEntityManager().find(SetOwnedEntity.class, ed_id);
+
+        owningEntity.getReferences().add(ownedEntity);
+
+        em.getTransaction().commit();
+        em.clear();
+
+        // remove child again
+        em.getTransaction().begin();
+
+        owningEntity = getEntityManager().find(SetOwningEntity.class, ing_id);
+        ownedEntity = getEntityManager().find(SetOwnedEntity.class, ed_id);
+
+        owningEntity.getReferences().remove(ownedEntity);
+
+        em.getTransaction().commit();
+        em.clear();
+
+        // now the set owning entity list should be empty again
+        owningEntity = getEntityManager().find(SetOwningEntity.class, ing_id);
+        assert owningEntity.getReferences().size() == 0;
+    }
+
+
+}