From 4796553a3f44e2263f7af245a94e4bebd2d43680 Mon Sep 17 00:00:00 2001 From: Kristoffer Lundberg Date: Thu, 11 Apr 2013 20:10:10 +0200 Subject: [PATCH] HHH-8171 - SETORDINAL to support set of embeddables --- .../main/docbook/devguide/en-US/Envers.xml | 11 +++++ .../envers/configuration/EnversSettings.java | 5 +++ .../internal/AuditEntitiesConfiguration.java | 9 ++++ .../metadata/CollectionMetadataGenerator.java | 36 +++++++++++----- .../relation/AbstractCollectionMapper.java | 37 ++++++++++++++-- .../relation/BasicCollectionMapper.java | 8 ++-- .../mapper/relation/ListCollectionMapper.java | 2 +- .../mapper/relation/MapCollectionMapper.java | 2 +- .../relation/SortedSetCollectionMapper.java | 9 ++-- .../collection/embeddable/EmbeddableSet.java | 42 ++++++++++++++++--- 10 files changed, 132 insertions(+), 29 deletions(-) diff --git a/documentation/src/main/docbook/devguide/en-US/Envers.xml b/documentation/src/main/docbook/devguide/en-US/Envers.xml index 1e03a0d1f7..ccc4ef9a78 100644 --- a/documentation/src/main/docbook/devguide/en-US/Envers.xml +++ b/documentation/src/main/docbook/devguide/en-US/Envers.xml @@ -315,6 +315,17 @@ For example: a property called "age", will by default get modified flag with column name "age_MOD". + + + org.hibernate.envers.embeddable_set_ordinal_field_name + + + SETORDINAL + + + The name of the column used for storing the ordinal of the change in sets of embeddables. + + diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java index 8b8d889156..4db8f4b6a4 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java @@ -99,4 +99,9 @@ public interface EnversSettings { * Defaults to {@literal REVEND_TSTMP}. */ public static final String AUDIT_STRATEGY_VALIDITY_REVEND_TIMESTAMP_FIELD_NAME = "org.hibernate.envers.audit_strategy_validity_revend_timestamp_field_name"; + + /** + * The name of the column used for storing the ordinal of the change in sets of embeddables. Defaults to {@literal SETORDINAL}. + */ + public static final String EMBEDDABLE_SET_ORDINAL_FIELD_NAME = "org.hibernate.envers.embeddable_set_ordinal_field_name"; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/AuditEntitiesConfiguration.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/AuditEntitiesConfiguration.java index ac8ca6d5f6..39ea2d7e05 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/AuditEntitiesConfiguration.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/AuditEntitiesConfiguration.java @@ -59,6 +59,8 @@ public class AuditEntitiesConfiguration { private final boolean revisionEndTimestampEnabled; private final String revisionEndTimestampFieldName; + + private final String embeddableSetOrdinalPropertyName; public AuditEntitiesConfiguration(Properties properties, String revisionInfoEntityName) { this.revisionInfoEntityName = revisionInfoEntityName; @@ -100,6 +102,9 @@ public class AuditEntitiesConfiguration { revisionNumberPath = originalIdPropName + "." + revisionFieldName + ".id"; revisionPropBasePath = originalIdPropName + "." + revisionFieldName + "."; + + embeddableSetOrdinalPropertyName = ConfigurationHelper.getString( + EnversSettings.EMBEDDABLE_SET_ORDINAL_FIELD_NAME, properties, "SETORDINAL" ); } public String getOriginalIdPropName() { @@ -167,4 +172,8 @@ public class AuditEntitiesConfiguration { public String getRevisionEndFieldName() { return revisionEndFieldName; } + + public String getEmbeddableSetOrdinalPropertyName() { + return embeddableSetOrdinalPropertyName; + } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java index ef2f1ccfd5..9ded90137c 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java @@ -88,6 +88,7 @@ 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,6 +507,22 @@ 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 ); + } + return new MiddleComponentData( componentMapper, 0 ); } else { // Last but one parameter: collection components are always insertable @@ -529,14 +546,13 @@ public final class CollectionMetadataGenerator { Type type = propertyValue.getType(); boolean embeddableElementType = isEmbeddableElementType(); if (type instanceof SortedSetType) { - currentMapper.addComposite(propertyAuditingData.getPropertyData(), - new SortedSetCollectionMapper(commonCollectionMapperData, - TreeSet.class, SortedSetProxy.class, elementComponentData, propertyValue.getComparator(), - embeddableElementType)); + currentMapper.addComposite( propertyAuditingData.getPropertyData(), new SortedSetCollectionMapper( + commonCollectionMapperData, TreeSet.class, SortedSetProxy.class, elementComponentData, + propertyValue.getComparator(), embeddableElementType, embeddableElementType ) ); } else if (type instanceof SetType) { - currentMapper.addComposite(propertyAuditingData.getPropertyData(), - new BasicCollectionMapper(commonCollectionMapperData, - HashSet.class, SetProxy.class, elementComponentData, embeddableElementType)); + currentMapper.addComposite( propertyAuditingData.getPropertyData(), new BasicCollectionMapper( + commonCollectionMapperData, HashSet.class, SetProxy.class, elementComponentData, + embeddableElementType, embeddableElementType ) ); } else if (type instanceof SortedMapType) { // Indexed collection, so indexComponentData is not null. currentMapper.addComposite(propertyAuditingData.getPropertyData(), @@ -549,9 +565,9 @@ public final class CollectionMetadataGenerator { new MapCollectionMapper(commonCollectionMapperData, HashMap.class, MapProxy.class, elementComponentData, indexComponentData, embeddableElementType)); } else if (type instanceof BagType) { - currentMapper.addComposite(propertyAuditingData.getPropertyData(), - new BasicCollectionMapper(commonCollectionMapperData, - ArrayList.class, ListProxy.class, elementComponentData, embeddableElementType)); + currentMapper.addComposite( propertyAuditingData.getPropertyData(), new BasicCollectionMapper( + commonCollectionMapperData, ArrayList.class, ListProxy.class, elementComponentData, + embeddableElementType, embeddableElementType ) ); } else if (type instanceof ListType) { // Indexed collection, so indexComponentData is not null. currentMapper.addComposite(propertyAuditingData.getPropertyData(), diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/AbstractCollectionMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/AbstractCollectionMapper.java index 8fa96a4113..c7e125fe35 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/AbstractCollectionMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/AbstractCollectionMapper.java @@ -55,15 +55,17 @@ import org.hibernate.property.Setter; public abstract class AbstractCollectionMapper implements PropertyMapper { protected final CommonCollectionMapperData commonCollectionMapperData; protected final Class collectionClass; + protected final boolean ordinalInId; protected final boolean revisionTypeInId; private final Constructor proxyConstructor; - protected AbstractCollectionMapper(CommonCollectionMapperData commonCollectionMapperData, - Class collectionClass, Class proxyClass, - boolean revisionTypeInId) { + protected AbstractCollectionMapper(CommonCollectionMapperData commonCollectionMapperData, + Class collectionClass, Class proxyClass, boolean ordinalInId, + boolean revisionTypeInId) { this.commonCollectionMapperData = commonCollectionMapperData; this.collectionClass = collectionClass; + this.ordinalInId = ordinalInId; this.revisionTypeInId = revisionTypeInId; try { @@ -84,11 +86,38 @@ 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. + */ + protected Map createIdMap(int ordinal) { + final HashMap idMap = new HashMap(); + + if ( ordinalInId ) { + idMap.put( this.commonCollectionMapperData.getVerEntCfg().getEmbeddableSetOrdinalPropertyName(), + Integer.valueOf( ordinal ) ); + } + + return idMap; + } + private void addCollectionChanges(SessionImplementor session, List collectionChanges, Set changed, RevisionType revisionType, Serializable id) { + int ordinal = 0; + for (Object changedObj : changed) { Map entityData = new HashMap(); - Map originalId = new HashMap(); + Map originalId = createIdMap( ordinal++ ); entityData.put(commonCollectionMapperData.getVerEntCfg().getOriginalIdPropName(), originalId); collectionChanges.add(new PersistentCollectionChangeData( diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/BasicCollectionMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/BasicCollectionMapper.java index 49a2db0ed9..7de41ea043 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/BasicCollectionMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/BasicCollectionMapper.java @@ -41,10 +41,10 @@ import org.hibernate.envers.internal.reader.AuditReaderImplementor; public class BasicCollectionMapper extends AbstractCollectionMapper implements PropertyMapper { protected final MiddleComponentData elementComponentData; - public BasicCollectionMapper(CommonCollectionMapperData commonCollectionMapperData, - Class collectionClass, Class proxyClass, - MiddleComponentData elementComponentData, boolean revisionTypeInId) { - super(commonCollectionMapperData, collectionClass, proxyClass, revisionTypeInId); + public BasicCollectionMapper(CommonCollectionMapperData commonCollectionMapperData, + Class collectionClass, Class proxyClass, + MiddleComponentData elementComponentData, boolean ordinalInId, boolean revisionTypeInId) { + super( commonCollectionMapperData, collectionClass, proxyClass, ordinalInId, revisionTypeInId ); this.elementComponentData = elementComponentData; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/ListCollectionMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/ListCollectionMapper.java index 26d6ce3398..112077117d 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/ListCollectionMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/ListCollectionMapper.java @@ -49,7 +49,7 @@ public final class ListCollectionMapper extends AbstractCollectionMapper i public ListCollectionMapper(CommonCollectionMapperData commonCollectionMapperData, MiddleComponentData elementComponentData, MiddleComponentData indexComponentData, boolean revisionTypeInId) { - super(commonCollectionMapperData, List.class, ListProxy.class, revisionTypeInId); + super( commonCollectionMapperData, List.class, ListProxy.class, false, revisionTypeInId ); this.elementComponentData = elementComponentData; this.indexComponentData = indexComponentData; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/MapCollectionMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/MapCollectionMapper.java index a2f6d46ea4..ae8a90bb0c 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/MapCollectionMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/MapCollectionMapper.java @@ -46,7 +46,7 @@ public class MapCollectionMapper extends AbstractCollectionMapper Class collectionClass, Class proxyClass, MiddleComponentData elementComponentData, MiddleComponentData indexComponentData, boolean revisionTypeInId) { - super(commonCollectionMapperData, collectionClass, proxyClass, revisionTypeInId); + super( commonCollectionMapperData, collectionClass, proxyClass, false, revisionTypeInId ); this.elementComponentData = elementComponentData; this.indexComponentData = indexComponentData; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/SortedSetCollectionMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/SortedSetCollectionMapper.java index 66efdf3736..e37bd744f6 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/SortedSetCollectionMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/relation/SortedSetCollectionMapper.java @@ -38,10 +38,11 @@ public final class SortedSetCollectionMapper extends BasicCollectionMapper collectionClass, Class proxyClass, - MiddleComponentData elementComponentData, Comparator comparator, - boolean revisionTypeInId) { - super(commonCollectionMapperData, collectionClass, proxyClass, elementComponentData, revisionTypeInId); + Class collectionClass, Class proxyClass, + MiddleComponentData elementComponentData, Comparator comparator, boolean ordinalInId, + boolean 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 32c93332db..a41ae215c7 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 @@ -50,6 +50,8 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { private final Component4 c4_2 = new Component4( "c42", "c42_value2", "c42_description" ); private final Component3 c3_1 = new Component3( "c31", c4_1, c4_2 ); private final Component3 c3_2 = new Component3( "c32", c4_1, c4_2 ); + private final Component3 c3_3 = new Component3( "c33", c4_1, c4_2 ); + private final Component3 c3_4 = new Component3( "c34", c4_1, c4_2 ); @Override protected Class[] getAnnotatedClasses() { @@ -63,9 +65,10 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { EmbeddableSetEntity ese1 = new EmbeddableSetEntity(); - // Revision 1 (ese1: initially 1 element in both collections) + // Revision 1 (ese1: initially 2 elements) em.getTransaction().begin(); ese1.getComponentSet().add( c3_1 ); + ese1.getComponentSet().add( c3_3 ); em.persist( ese1 ); em.getTransaction().commit(); @@ -93,6 +96,29 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { ese1.getComponentSet().remove( c3_2 ); em.getTransaction().commit(); + // Revision 5 (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) + 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) + em.getTransaction().begin(); + ese1 = em.find( EmbeddableSetEntity.class, ese1.getId() ); + ese1.getComponentSet().remove( c3_1 ); + ese1.getComponentSet().remove( c3_3 ); + ese1.getComponentSet().add( c3_2 ); + ese1.getComponentSet().add( c3_4 ); + em.getTransaction().commit(); + ese1_id = ese1.getId(); em.close(); @@ -100,7 +126,7 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { @Test public void testRevisionsCounts() { - assertEquals( Arrays.asList( 1, 2, 3 ), getAuditReader().getRevisions( EmbeddableSetEntity.class, ese1_id ) ); + assertEquals( Arrays.asList( 1, 2, 3, 4, 5, 6 ), getAuditReader().getRevisions( EmbeddableSetEntity.class, ese1_id ) ); } @Test @@ -108,9 +134,15 @@ public class EmbeddableSet extends BaseEnversJPAFunctionalTestCase { EmbeddableSetEntity rev1 = getAuditReader().find( EmbeddableSetEntity.class, ese1_id, 1 ); EmbeddableSetEntity rev2 = getAuditReader().find( EmbeddableSetEntity.class, ese1_id, 2 ); EmbeddableSetEntity rev3 = getAuditReader().find( EmbeddableSetEntity.class, ese1_id, 3 ); + 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 ); - assertEquals( Collections.singleton( c3_1 ), rev1.getComponentSet() ); - assertEquals( TestTools.makeSet( c3_1, c3_2 ), rev2.getComponentSet() ); - assertEquals( TestTools.makeSet( c3_1 ), rev3.getComponentSet() ); + assertEquals( TestTools.makeSet( c3_1, c3_3 ), rev1.getComponentSet() ); + assertEquals( TestTools.makeSet( c3_1, c3_2, c3_3 ), rev2.getComponentSet() ); + assertEquals( TestTools.makeSet( c3_1, c3_3 ), rev3.getComponentSet() ); + 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() ); } } \ No newline at end of file