From 772f3cbc5d73ea56bc1fe98e6a6ab1eccb6b1b5a Mon Sep 17 00:00:00 2001 From: Lukasz Antoniak Date: Sun, 14 Apr 2013 23:56:08 +0200 Subject: [PATCH] HHH-8171 - Cleanup --- .../main/docbook/devguide/en-US/Envers.xml | 2 +- .../AuditEntitiesConfiguration.java | 7 +++--- .../metadata/CollectionMetadataGenerator.java | 25 ++++++++----------- .../relation/AbstractCollectionMapper.java | 23 +++++------------ .../relation/SortedSetCollectionMapper.java | 3 +-- .../collection/embeddable/EmbeddableSet.java | 23 +++++++++++------ 6 files changed, 37 insertions(+), 46 deletions(-) diff --git a/documentation/src/main/docbook/devguide/en-US/Envers.xml b/documentation/src/main/docbook/devguide/en-US/Envers.xml index 9a395b0907..41405c7098 100644 --- a/documentation/src/main/docbook/devguide/en-US/Envers.xml +++ b/documentation/src/main/docbook/devguide/en-US/Envers.xml @@ -323,7 +323,7 @@ SETORDINAL - The name of the column used for storing the ordinal of the change in sets of embeddables. + Name of column used for storing ordinal of the change in sets of embeddable elements. diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/AuditEntitiesConfiguration.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/AuditEntitiesConfiguration.java index 4c575bc033..5dd7084f13 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/AuditEntitiesConfiguration.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/AuditEntitiesConfiguration.java @@ -115,9 +115,10 @@ public class AuditEntitiesConfiguration { revisionNumberPath = originalIdPropName + "." + revisionFieldName + ".id"; revisionPropBasePath = originalIdPropName + "." + revisionFieldName + "."; - embeddableSetOrdinalPropertyName = getProperty( properties, - "org.hibernate.envers.embeddable_set_ordinal_field_name", - "org.hibernate.envers.embeddable_set_ordinal_field_name", "SETORDINAL" ); + embeddableSetOrdinalPropertyName = getProperty( + properties, "org.hibernate.envers.embeddable_set_ordinal_field_name", + "org.hibernate.envers.embeddable_set_ordinal_field_name", "SETORDINAL" + ); } public String getOriginalIdPropName() { diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java index 85177a4b69..6e0df82316 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/CollectionMetadataGenerator.java @@ -87,7 +87,6 @@ import org.hibernate.mapping.ManyToOne; import org.hibernate.mapping.OneToMany; import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Property; -import org.hibernate.mapping.SimpleValue; import org.hibernate.mapping.Table; import org.hibernate.mapping.Value; import org.hibernate.type.BagType; @@ -506,20 +505,16 @@ public final class CollectionMetadataGenerator { ); } - // Add an additional column holding a number to make each entry unique within the set, - // since embeddable properties can be null - if ( propertyValue.getCollectionType() instanceof SetType ) { - final String auditedEmbeddableSetOrdinalPropertyName = mainGenerator.getVerEntCfg() - .getEmbeddableSetOrdinalPropertyName(); - final String auditedEmbeddableSetOrdinalPropertyType = "integer"; - - final SimpleValue simpleValue = new SimpleValue( component.getMappings(), component.getTable() ); - simpleValue.setTypeName( auditedEmbeddableSetOrdinalPropertyType ); - - final Element idProperty = MetadataTools.addProperty( xmlMapping, - auditedEmbeddableSetOrdinalPropertyName, auditedEmbeddableSetOrdinalPropertyType, true, true ); - MetadataTools.addColumn( idProperty, auditedEmbeddableSetOrdinalPropertyName, null, 0, 0, null, null, - null, false ); + // Add an additional column holding a number to make each entry unique within the set. + // Embeddable properties may contain null values, so cannot be stored within composite primary key. + if ( propertyValue.isSet() ) { + final String setOrdinalPropertyName = mainGenerator.getVerEntCfg().getEmbeddableSetOrdinalPropertyName(); + final Element ordinalProperty = MetadataTools.addProperty( + xmlMapping, setOrdinalPropertyName, "integer", true, true + ); + MetadataTools.addColumn( + ordinalProperty, setOrdinalPropertyName, null, null, null, null, null, null, false + ); } return new MiddleComponentData( componentMapper, 0 ); diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/AbstractCollectionMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/AbstractCollectionMapper.java index 5ef9e03869..c8781fa040 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/AbstractCollectionMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/AbstractCollectionMapper.java @@ -87,27 +87,16 @@ public abstract class AbstractCollectionMapper implements PropertyMapper { protected abstract void mapToMapFromObject(SessionImplementor session, Map idData, Map data, Object changed); /** - * Creates a Map for the id. - * - *

- * The ordinal parameter represents the iteration ordinal of the current element, used to add a synthetic id when - * dealing with embeddables since embeddable fields can't be contained within the primary key since they might be - * nullable. - *

- * - * @param ordinal - * The element iteration ordinal. - * - * @return A Map for holding the ID information. + * Creates map for storing identifier data. Ordinal parameter guarantees uniqueness of primary key. + * Composite primary key cannot contain embeddable properties since they might be nullable. + * @param ordinal Iteration ordinal. + * @return Map for holding identifier data. */ protected Map createIdMap(int ordinal) { - final HashMap idMap = new HashMap(); - + final Map idMap = new HashMap(); if ( ordinalInId ) { - idMap.put( this.commonCollectionMapperData.getVerEntCfg().getEmbeddableSetOrdinalPropertyName(), - Integer.valueOf( ordinal ) ); + idMap.put( commonCollectionMapperData.getVerEntCfg().getEmbeddableSetOrdinalPropertyName(), ordinal ); } - return idMap; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/SortedSetCollectionMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/SortedSetCollectionMapper.java index 2c0feabc6f..abf88ccb89 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/SortedSetCollectionMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/SortedSetCollectionMapper.java @@ -41,8 +41,7 @@ public final class SortedSetCollectionMapper extends BasicCollectionMapper collectionClass, Class proxyClass, MiddleComponentData elementComponentData, Comparator comparator, boolean ordinalInId, boolean revisionTypeInId) { - super( commonCollectionMapperData, collectionClass, proxyClass, elementComponentData, ordinalInId, - revisionTypeInId ); + super( commonCollectionMapperData, collectionClass, proxyClass, elementComponentData, ordinalInId, revisionTypeInId ); this.comparator = comparator; } diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/collection/embeddable/EmbeddableSet.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/collection/embeddable/EmbeddableSet.java index a41ae215c7..0243319e9b 100644 --- a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/collection/embeddable/EmbeddableSet.java +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/collection/embeddable/EmbeddableSet.java @@ -24,7 +24,6 @@ package org.hibernate.envers.test.integration.collection.embeddable; import java.util.Arrays; -import java.util.Collections; import javax.persistence.EntityManager; import org.junit.Test; @@ -65,7 +64,7 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { EmbeddableSetEntity ese1 = new EmbeddableSetEntity(); - // Revision 1 (ese1: initially 2 elements) + // Revision 1 (ese1: initially two elements) em.getTransaction().begin(); ese1.getComponentSet().add( c3_1 ); ese1.getComponentSet().add( c3_3 ); @@ -84,33 +83,33 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { ese1.getComponentSet().add( c3_2 ); em.getTransaction().commit(); - // Revision 3 (ese1: adding one existing element) + // Revision (still 2) (ese1: adding one existing element) em.getTransaction().begin(); ese1 = em.find( EmbeddableSetEntity.class, ese1.getId() ); ese1.getComponentSet().add( c3_1 ); em.getTransaction().commit(); - // Revision 4 (ese1: removing one existing element) + // Revision 3 (ese1: removing one existing element) em.getTransaction().begin(); ese1 = em.find( EmbeddableSetEntity.class, ese1.getId() ); ese1.getComponentSet().remove( c3_2 ); em.getTransaction().commit(); - // Revision 5 (ese1: adding two elements) + // Revision 4 (ese1: adding two elements) em.getTransaction().begin(); ese1 = em.find( EmbeddableSetEntity.class, ese1.getId() ); ese1.getComponentSet().add( c3_2 ); ese1.getComponentSet().add( c3_4 ); em.getTransaction().commit(); - // Revision 6 (ese1: removing two elements) + // Revision 5 (ese1: removing two elements) em.getTransaction().begin(); ese1 = em.find( EmbeddableSetEntity.class, ese1.getId() ); ese1.getComponentSet().remove( c3_2 ); ese1.getComponentSet().remove( c3_4 ); em.getTransaction().commit(); - // Revision 7 (ese1: removing and adding two elements) + // Revision 6 (ese1: removing and adding two elements) em.getTransaction().begin(); ese1 = em.find( EmbeddableSetEntity.class, ese1.getId() ); ese1.getComponentSet().remove( c3_1 ); @@ -119,6 +118,12 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { ese1.getComponentSet().add( c3_4 ); em.getTransaction().commit(); + // Revision 7 (ese1: adding one element) + em.getTransaction().begin(); + ese1 = em.find( EmbeddableSetEntity.class, ese1.getId() ); + ese1.getComponentSet().add( c3_1 ); + em.getTransaction().commit(); + ese1_id = ese1.getId(); em.close(); @@ -126,7 +131,7 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { @Test public void testRevisionsCounts() { - assertEquals( Arrays.asList( 1, 2, 3, 4, 5, 6 ), getAuditReader().getRevisions( EmbeddableSetEntity.class, ese1_id ) ); + assertEquals( Arrays.asList( 1, 2, 3, 4, 5, 6, 7 ), getAuditReader().getRevisions( EmbeddableSetEntity.class, ese1_id ) ); } @Test @@ -137,6 +142,7 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { EmbeddableSetEntity rev4 = getAuditReader().find( EmbeddableSetEntity.class, ese1_id, 4 ); EmbeddableSetEntity rev5 = getAuditReader().find( EmbeddableSetEntity.class, ese1_id, 5 ); EmbeddableSetEntity rev6 = getAuditReader().find( EmbeddableSetEntity.class, ese1_id, 6 ); + EmbeddableSetEntity rev7 = getAuditReader().find( EmbeddableSetEntity.class, ese1_id, 7 ); assertEquals( TestTools.makeSet( c3_1, c3_3 ), rev1.getComponentSet() ); assertEquals( TestTools.makeSet( c3_1, c3_2, c3_3 ), rev2.getComponentSet() ); @@ -144,5 +150,6 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { assertEquals( TestTools.makeSet( c3_1, c3_2, c3_3, c3_4 ), rev4.getComponentSet() ); assertEquals( TestTools.makeSet( c3_1, c3_3 ), rev5.getComponentSet() ); assertEquals( TestTools.makeSet( c3_2, c3_4 ), rev6.getComponentSet() ); + assertEquals( TestTools.makeSet( c3_2, c3_4, c3_1 ), rev7.getComponentSet() ); } } \ No newline at end of file