diff --git a/envers/src/main/java/org/hibernate/envers/configuration/EntitiesConfigurator.java b/envers/src/main/java/org/hibernate/envers/configuration/EntitiesConfigurator.java index 47539b1bda..e72d136df4 100644 --- a/envers/src/main/java/org/hibernate/envers/configuration/EntitiesConfigurator.java +++ b/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 diff --git a/envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditEntityNameRegister.java b/envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditEntityNameRegister.java new file mode 100644 index 0000000000..606d478bd4 --- /dev/null +++ b/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 auditEntityNames = new HashSet(); + + /** + * @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; + } +} diff --git a/envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditMetadataGenerator.java b/envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditMetadataGenerator.java index a0bcf29f5c..5dafd953ca 100644 --- a/envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditMetadataGenerator.java +++ b/envers/src/main/java/org/hibernate/envers/configuration/metadata/AuditMetadataGenerator.java @@ -69,12 +69,15 @@ public final class AuditMetadataGenerator { private final Map entitiesConfigurations; private final Map notAuditedEntitiesConfigurations; + private final AuditEntityNameRegister auditEntityNameRegister; + // Map entity name -> (join descriptor -> element describing the "versioned" join) private final Map> 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(); notAuditedEntitiesConfigurations = new HashMap(); entitiesJoins = new HashMap>(); @@ -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 + "'."; diff --git a/envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java b/envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java index 1eceb2c8ae..ac90b2289d 100644 --- a/envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java +++ b/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; diff --git a/envers/src/main/java/org/hibernate/envers/configuration/metadata/MetadataTools.java b/envers/src/main/java/org/hibernate/envers/configuration/metadata/MetadataTools.java index 363bb0beea..d8c8835223 100644 --- a/envers/src/main/java/org/hibernate/envers/configuration/metadata/MetadataTools.java +++ b/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"); - if (nameAttribute == null) { - column_mapping.addAttribute("name", name); - } else { - nameAttribute.setValue(name); - } + addOrModifyAttribute(column_mapping, "name", name); } return column_mapping; diff --git a/envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning1Entity.java b/envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning1Entity.java index 4bc691f40e..69dc8c6167 100644 --- a/envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning1Entity.java +++ b/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; diff --git a/envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning2Entity.java b/envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning2Entity.java index c4c87ceccd..bd1491a134 100644 --- a/envers/src/test/java/org/hibernate/envers/test/entities/manytomany/biowned/ListBiowning2Entity.java +++ b/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; diff --git a/envers/src/test/java/org/hibernate/envers/test/integration/manytomany/biowned/BasicBiowned.java b/envers/src/test/java/org/hibernate/envers/test/integration/manytomany/biowned/BasicBiowned.java index 8405eae634..1e040592b4 100644 --- a/envers/src/test/java/org/hibernate/envers/test/integration/manytomany/biowned/BasicBiowned.java +++ b/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(ListBiowning2Entity.class); + cfg.addAnnotatedClass(ListBiowning1Entity.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_1.getReferences().remove(o2_1); - o1_2.getReferences().add(o2_2); - o2_2.getReferences().add(o1_2); + o1_2.getReferences().remove(o2_1); + o1_1.getReferences().add(o2_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)); - assert Arrays.asList(1, 2, 4, 5).equals(getAuditReader().getRevisions(ListBiowning1Entity.class, o1_2_id)); + // Although it would seem that when modifying references both entities should be marked as modified, because + // 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)); - assert Arrays.asList(1, 2, 3, 4, 5).equals(getAuditReader().getRevisions(ListBiowning2Entity.class, o2_2_id)); + assertEquals(Arrays.asList(1, 2, 3, 5), getAuditReader().getRevisions(ListBiowning1Entity.class, o1_1_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(rev5.getReferences()); + assert TestTools.checkList(rev4.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(rev5.getReferences(), o2_1, o2_2); + assert TestTools.checkList(rev4.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(rev5.getReferences(), o1_2); + assert TestTools.checkList(rev4.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(rev5.getReferences(), o1_2); + assert TestTools.checkList(rev4.getReferences(), o1_2); + assert TestTools.checkList(rev5.getReferences(), o1_1, o1_2); } } \ No newline at end of file diff --git a/envers/src/test/resources/hibernate.test.cfg.xml b/envers/src/test/resources/hibernate.test.cfg.xml index db36d18262..9ea50a9d33 100644 --- a/envers/src/test/resources/hibernate.test.cfg.xml +++ b/envers/src/test/resources/hibernate.test.cfg.xml @@ -17,13 +17,13 @@ sa - + + + + - 100--> +