HHH-4634:

- adding a register of all audit entity names generated so far
- generating a unique entity name in case of a relation owned by both sides
- fixing the test

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18118 1b8cb986-b30d-0410-93ca-fae66ebed9b2

envers/src/main/java/org/hibernate/envers/configuration/EntitiesConfigurator.java

@@ -41,6 +41,7 @@ import org.hibernate.envers.configuration.metadata.reader.AnnotationsMetadataRea
 import org.hibernate.envers.configuration.metadata.EntityXmlMappingData;
 import org.hibernate.envers.configuration.metadata.reader.ClassAuditingData;
 import org.hibernate.envers.configuration.metadata.AuditMetadataGenerator;
+import org.hibernate.envers.configuration.metadata.AuditEntityNameRegister;
 import org.hibernate.envers.entities.EntitiesConfigurations;
 import org.hibernate.envers.tools.StringTools;
 import org.hibernate.envers.tools.graph.GraphTopologicalSort;
@@ -57,8 +58,11 @@ public class EntitiesConfigurator {
     public EntitiesConfigurations configure(Configuration cfg, ReflectionManager reflectionManager,
                                             GlobalConfiguration globalCfg, AuditEntitiesConfiguration verEntCfg,
                                             Document revisionInfoXmlMapping, Element revisionInfoRelationMapping) {
+        // Creating a name register to capture all audit entity names created.
+        AuditEntityNameRegister auditEntityNameRegister = new AuditEntityNameRegister();
+
         AuditMetadataGenerator auditMetaGen = new AuditMetadataGenerator(cfg, globalCfg, verEntCfg,
-                revisionInfoRelationMapping);
+                revisionInfoRelationMapping, auditEntityNameRegister);
         DOMWriter writer = new DOMWriter();
 
         // Sorting the persistent class topologically - superclass always before subclass
@@ -76,6 +80,7 @@ public class EntitiesConfigurator {
                     new AnnotationsMetadataReader(globalCfg, reflectionManager, pc);
             ClassAuditingData auditData = annotationsMetadataReader.getAuditData();
 
+            EntityXmlMappingData xmlMappingData = new EntityXmlMappingData();
             if (auditData.isAudited()) {
                 pcDatas.put(pc, auditData);
 
@@ -83,14 +88,12 @@ public class EntitiesConfigurator {
                     verEntCfg.addCustomAuditTableName(pc.getEntityName(), auditData.getAuditTable().value());
                 }
 
-                EntityXmlMappingData xmlMappingData = new EntityXmlMappingData();
                 auditMetaGen.generateFirstPass(pc, auditData, xmlMappingData, true);
-                xmlMappings.put(pc, xmlMappingData);
 			} else {
-				EntityXmlMappingData xmlMappingData = new EntityXmlMappingData();
 				auditMetaGen.generateFirstPass(pc, auditData, xmlMappingData, false);
-				xmlMappings.put(pc, xmlMappingData);
 			}
+
+            xmlMappings.put(pc, xmlMappingData);
         }
 
         // Second pass

envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditEntityNameRegister.java

@@ -0,0 +1,50 @@
+package org.hibernate.envers.configuration.metadata;
+
+import org.hibernate.MappingException;
+
+import java.util.Set;
+import java.util.HashSet;
+
+/**
+ * A register of all audit entity names used so far.
+ * @author Adam Warski (adam at warski dot org)
+ */
+public class AuditEntityNameRegister {
+    private final Set<String> auditEntityNames = new HashSet<String>();
+
+    /**
+     * @param auditEntityName Name of the audit entity.
+     * @return True if the given audit entity name is already used.
+     */
+    private boolean check(String auditEntityName) {
+        return auditEntityNames.contains(auditEntityName);
+    }
+
+    /**
+     * Register an audit entity name. If the name is already registered, an exception is thrown.
+     * @param auditEntityName Name of the audit entity.
+     */
+    public void register(String auditEntityName) {
+        if (auditEntityNames.contains(auditEntityName)) {
+            throw new MappingException("The audit entity name '" + auditEntityName + "' is already registered.");
+        }
+        
+        auditEntityNames.add(auditEntityName);
+    }
+
+    /**
+     * Creates a unique (not yet registered) audit entity name by appending consecutive numbers to the base
+     * name. If the base name is not yet used, it is returned unmodified.
+     * @param baseAuditEntityName The base entity name.
+     * @return 
+     */
+    public String createUnique(final String baseAuditEntityName) {
+        String auditEntityName = baseAuditEntityName;
+        int count = 1;
+        while (check(auditEntityName)) {
+            auditEntityName = baseAuditEntityName + count++;
+        }
+
+        return auditEntityName;
+    }
+}

envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditMetadataGenerator.java

@@ -69,12 +69,15 @@ public final class AuditMetadataGenerator {
     private final Map<String, EntityConfiguration> entitiesConfigurations;
     private final Map<String, EntityConfiguration> notAuditedEntitiesConfigurations;
 
+    private final AuditEntityNameRegister auditEntityNameRegister;
+
     // Map entity name -> (join descriptor -> element describing the "versioned" join)
     private final Map<String, Map<Join, Element>> entitiesJoins;
 
     public AuditMetadataGenerator(Configuration cfg, GlobalConfiguration globalCfg,
                                   AuditEntitiesConfiguration verEntCfg,
-                                  Element revisionInfoRelationMapping) {
+                                  Element revisionInfoRelationMapping,
+                                  AuditEntityNameRegister auditEntityNameRegister) {
         this.cfg = cfg;
         this.globalCfg = globalCfg;
         this.verEntCfg = verEntCfg;
@@ -85,6 +88,8 @@ public final class AuditMetadataGenerator {
         this.idMetadataGenerator = new IdMetadataGenerator(this);
         this.toOneRelationMetadataGenerator = new ToOneRelationMetadataGenerator(this);
 
+        this.auditEntityNameRegister = auditEntityNameRegister;
+
         entitiesConfigurations = new HashMap<String, EntityConfiguration>();
         notAuditedEntitiesConfigurations = new HashMap<String, EntityConfiguration>();
         entitiesJoins = new HashMap<String, Map<Join, Element>>();
@@ -344,6 +349,9 @@ public final class AuditMetadataGenerator {
         String auditEntityName = verEntCfg.getAuditEntityName(entityName);
         String auditTableName = verEntCfg.getAuditTableName(entityName, pc.getTable().getName());
 
+        // Registering the audit entity name, now that it is known
+        auditEntityNameRegister.register(auditEntityName);
+
         AuditTableData auditTableData = new AuditTableData(auditEntityName, auditTableName, schema, catalog);
 
         // Generating a mapping for the id
@@ -446,6 +454,10 @@ public final class AuditMetadataGenerator {
         return verEntCfg;
     }
 
+    AuditEntityNameRegister getAuditEntityNameRegister() {
+        return auditEntityNameRegister;
+    }
+
     void throwUnsupportedTypeException(Type type, String entityName, String propertyName) {
         String message = "Type not supported for auditing: " + type.getClass().getName() +
                 ", on entity " + entityName + ", property '" + propertyName + "'.";

envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java

@@ -254,7 +254,13 @@ public final class CollectionMetadataGenerator {
         // Generating the XML mapping for the middle entity, only if the relation isn't inverse.
         // If the relation is inverse, will be later checked by comparing middleEntityXml with null.
         Element middleEntityXml;
-        if (!propertyValue.isInverse()) {
+        if (!propertyValue.isInverse()) {            
+            // Generating a unique middle entity name
+            auditMiddleEntityName = mainGenerator.getAuditEntityNameRegister().createUnique(auditMiddleEntityName);
+
+            // Registering the generated name
+            mainGenerator.getAuditEntityNameRegister().register(auditMiddleEntityName);
+
             middleEntityXml = createMiddleEntityXml(auditMiddleTableName, auditMiddleEntityName);
         } else {
             middleEntityXml = null;

envers/src/main/java/org/hibernate/envers/configuration/metadata/MetadataTools.java

@@ -68,6 +68,15 @@ public class MetadataTools {
         return prop_mapping;
     }
 
+    private static void addOrModifyAttribute(Element parent, String name, String value) {
+        Attribute attribute = parent.attribute(name);
+        if (attribute == null) {
+            parent.addAttribute(name, value);
+        } else {
+            attribute.setValue(value);
+        }
+    }
+
     public static Element addOrModifyColumn(Element parent, String name) {
         Element column_mapping = parent.element("column");
 
@@ -76,12 +85,7 @@ public class MetadataTools {
         }
 
         if (!StringTools.isEmpty(name)) {
-            Attribute nameAttribute = column_mapping.attribute("name");
+            addOrModifyAttribute(column_mapping, "name", name);
-            if (nameAttribute == null) {
-                column_mapping.addAttribute("name", name);
-            } else {
-                nameAttribute.setValue(name);
-            }
         }
 
         return column_mapping;

envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning1Entity.java

@@ -36,7 +36,8 @@ import org.hibernate.envers.Audited;
 @Entity
 @Audited
 public class ListBiowning1Entity {
-    @Id
+    @Id    
+    @GeneratedValue
     private Integer id;
 
     private String data;

envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning2Entity.java

@@ -37,6 +37,7 @@ import org.hibernate.envers.Audited;
 @Audited
 public class ListBiowning2Entity {
     @Id
+    @GeneratedValue
     private Integer id;
 
     private String data;

envers/src/test/java/org/hibernate/envers/test/integration/manytomany/biowned/BasicBiowned.java

@@ -30,6 +30,7 @@ import org.hibernate.envers.test.entities.manytomany.biowned.ListBiowning2Entity
 import org.hibernate.envers.test.tools.TestTools;
 import org.testng.annotations.Test;
 import org.testng.annotations.BeforeClass;
+import static org.testng.Assert.assertEquals;
 
 import javax.persistence.EntityManager;
 import java.util.Arrays;
@@ -44,11 +45,11 @@ public class BasicBiowned extends AbstractEntityTest {
     private Integer o2_2_id;
 
     public void configure(Ejb3Configuration cfg) {
-        //cfg.addAnnotatedClass(ListBiowning1Entity.class);
+        cfg.addAnnotatedClass(ListBiowning1Entity.class);
-        //cfg.addAnnotatedClass(ListBiowning2Entity.class);
+        cfg.addAnnotatedClass(ListBiowning2Entity.class);
     }
 
-    //@BeforeClass(dependsOnMethods = "init")
+    @BeforeClass(dependsOnMethods = "init")
     public void initData() {
         EntityManager em = getEntityManager();
 
@@ -66,6 +67,7 @@ public class BasicBiowned extends AbstractEntityTest {
         em.persist(o2_2);
 
         em.getTransaction().commit();
+        em.clear();
 
         // Revision 2 (1_1 <-> 2_1; 1_2 <-> 2_2)
 
@@ -80,6 +82,7 @@ public class BasicBiowned extends AbstractEntityTest {
         o1_2.getReferences().add(o2_2);
 
         em.getTransaction().commit();
+        em.clear();
 
         // Revision 3 (1_1 <-> 2_1, 2_2; 1_2 <-> 2_2)
         em.getTransaction().begin();
@@ -90,8 +93,9 @@ public class BasicBiowned extends AbstractEntityTest {
         o1_1.getReferences().add(o2_2);
 
         em.getTransaction().commit();
+        em.clear();
 
-        // Revision 4 (1_1 <-> 2_1; 1_2 <-> 2_1)
+        // Revision 4 (1_2 <-> 2_1, 2_2)
         em.getTransaction().begin();
 
         o1_1 = em.find(ListBiowning1Entity.class, o1_1.getId());
@@ -101,11 +105,12 @@ public class BasicBiowned extends AbstractEntityTest {
 
         o2_2.getReferences().remove(o1_1);
         o2_1.getReferences().remove(o1_1);
-        o1_2.getReferences().add(o2_1);
+        o2_1.getReferences().add(o1_2);
 
         em.getTransaction().commit();
+        em.clear();
 
-        // Revision 5 (1_2 <-> 2_1, 2_2)
+        // Revision 5 (1_1 <-> 2_2, 1_2 <-> 2_2)
         em.getTransaction().begin();
 
         o1_1 = em.find(ListBiowning1Entity.class, o1_1.getId());
@@ -113,12 +118,11 @@ public class BasicBiowned extends AbstractEntityTest {
         o2_1 = em.find(ListBiowning2Entity.class, o2_1.getId());
         o2_2 = em.find(ListBiowning2Entity.class, o2_2.getId());
 
-        o2_1.getReferences().remove(o1_1);
+        o1_2.getReferences().remove(o2_1);
-        o1_1.getReferences().remove(o2_1);
+        o1_1.getReferences().add(o2_2);
-        o1_2.getReferences().add(o2_2);
-        o2_2.getReferences().add(o1_2);
 
         em.getTransaction().commit();
+        em.clear();
 
         //
 
@@ -128,16 +132,20 @@ public class BasicBiowned extends AbstractEntityTest {
         o2_2_id = o2_2.getId();
     }
 
-    @Test(enabled = false)
+    @Test(enabled = true)
     public void testRevisionsCounts() {
-        assert Arrays.asList(1, 2, 3, 4, 5).equals(getAuditReader().getRevisions(ListBiowning1Entity.class, o1_1_id));
+        // Although it would seem that when modifying references both entities should be marked as modified, because
-        assert Arrays.asList(1, 2, 4, 5).equals(getAuditReader().getRevisions(ListBiowning1Entity.class, o1_2_id));
+        // ownly the owning side is notified (because of the bi-owning mapping), a revision is created only for
+        // the entity where the collection was directly modified.
 
-        assert Arrays.asList(1, 2, 4, 5).equals(getAuditReader().getRevisions(ListBiowning2Entity.class, o2_1_id));
+        assertEquals(Arrays.asList(1, 2, 3, 5), getAuditReader().getRevisions(ListBiowning1Entity.class, o1_1_id));
-        assert Arrays.asList(1, 2, 3, 4, 5).equals(getAuditReader().getRevisions(ListBiowning2Entity.class, o2_2_id));
+        assertEquals(Arrays.asList(1, 2, 5), getAuditReader().getRevisions(ListBiowning1Entity.class, o1_2_id));
+
+        assertEquals(Arrays.asList(1, 4), getAuditReader().getRevisions(ListBiowning2Entity.class, o2_1_id));
+        assertEquals(Arrays.asList(1, 4), getAuditReader().getRevisions(ListBiowning2Entity.class, o2_2_id));
     }
 
-    @Test(enabled = false)
+    @Test(enabled = true)
     public void testHistoryOfO1_1() {
         ListBiowning2Entity o2_1 = getEntityManager().find(ListBiowning2Entity.class, o2_1_id);
         ListBiowning2Entity o2_2 = getEntityManager().find(ListBiowning2Entity.class, o2_2_id);
@@ -151,11 +159,11 @@ public class BasicBiowned extends AbstractEntityTest {
         assert TestTools.checkList(rev1.getReferences());
         assert TestTools.checkList(rev2.getReferences(), o2_1);
         assert TestTools.checkList(rev3.getReferences(), o2_1, o2_2);
-        assert TestTools.checkList(rev4.getReferences(), o2_1);
+        assert TestTools.checkList(rev4.getReferences());
-        assert TestTools.checkList(rev5.getReferences());
+        assert TestTools.checkList(rev5.getReferences(), o2_2);
     }
 
-    @Test(enabled = false)
+    @Test(enabled = true)
     public void testHistoryOfO1_2() {
         ListBiowning2Entity o2_1 = getEntityManager().find(ListBiowning2Entity.class, o2_1_id);
         ListBiowning2Entity o2_2 = getEntityManager().find(ListBiowning2Entity.class, o2_2_id);
@@ -169,11 +177,12 @@ public class BasicBiowned extends AbstractEntityTest {
         assert TestTools.checkList(rev1.getReferences());
         assert TestTools.checkList(rev2.getReferences(), o2_2);
         assert TestTools.checkList(rev3.getReferences(), o2_2);
-        assert TestTools.checkList(rev4.getReferences(), o2_1);
+        assert TestTools.checkList(rev4.getReferences(), o2_1, o2_2);
-        assert TestTools.checkList(rev5.getReferences(), o2_1, o2_2);
+        System.out.println("rev5.getReferences() = " + rev5.getReferences());
+        assert TestTools.checkList(rev5.getReferences(), o2_2);
     }
 
-    @Test(enabled = false)
+    @Test(enabled = true)
     public void testHistoryOfO2_1() {
         ListBiowning1Entity o1_1 = getEntityManager().find(ListBiowning1Entity.class, o1_1_id);
         ListBiowning1Entity o1_2 = getEntityManager().find(ListBiowning1Entity.class, o1_2_id);
@@ -187,11 +196,11 @@ public class BasicBiowned extends AbstractEntityTest {
         assert TestTools.checkList(rev1.getReferences());
         assert TestTools.checkList(rev2.getReferences(), o1_1);
         assert TestTools.checkList(rev3.getReferences(), o1_1);
-        assert TestTools.checkList(rev4.getReferences(), o1_1, o1_2);
+        assert TestTools.checkList(rev4.getReferences(), o1_2);
-        assert TestTools.checkList(rev5.getReferences(), o1_2);
+        assert TestTools.checkList(rev5.getReferences());
     }
 
-    @Test(enabled = false)
+    @Test(enabled = true)
     public void testHistoryOfO2_2() {
         ListBiowning1Entity o1_1 = getEntityManager().find(ListBiowning1Entity.class, o1_1_id);
         ListBiowning1Entity o1_2 = getEntityManager().find(ListBiowning1Entity.class, o1_2_id);
@@ -205,7 +214,7 @@ public class BasicBiowned extends AbstractEntityTest {
         assert TestTools.checkList(rev1.getReferences());
         assert TestTools.checkList(rev2.getReferences(), o1_2);
         assert TestTools.checkList(rev3.getReferences(), o1_1, o1_2);
-        assert TestTools.checkList(rev4.getReferences());
+        assert TestTools.checkList(rev4.getReferences(), o1_2);
-        assert TestTools.checkList(rev5.getReferences(), o1_2);
+        assert TestTools.checkList(rev5.getReferences(), o1_1, o1_2);
     }
 }

envers/src/test/resources/hibernate.test.cfg.xml

@@ -17,13 +17,13 @@
         <property name="connection.username">sa</property>
         <property name="connection.password"></property>
 
-        <!--<property name="dialect">org.hibernate.dialect.MySQL5InnoDBDialect</property>
+        <!--<property name="dialect">org.hibernate.dialect.MySQL5InnoDBDialect</property>-->
-        <property name="connection.url">jdbc:mysql:///hibernate_tests?useUnicode=true&amp;characterEncoding=UTF-8</property>
+        <!--<property name="connection.url">jdbc:mysql:///hibernate_tests?useUnicode=true&amp;characterEncoding=UTF-8</property>-->
-        <property name="connection.driver_class">com.mysql.jdbc.Driver</property>
+        <!--<property name="connection.driver_class">com.mysql.jdbc.Driver</property>-->
-        <property name="connection.username">root</property>
+        <!--<property name="connection.username">root</property>-->
-        <property name="connection.password"></property>
+        <!--<property name="connection.password"></property>-->
 
-        <property name="hibernate.jdbc.batch_size">100</property>-->
+        <!--<property name="hibernate.jdbc.batch_size">100</property>-->
 
         <!--<event type="post-insert">
             <listener class="org.hibernate.envers.event.AuditEventListener" />