HHH-8171 - SETORDINAL to support set of embeddables

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".
                         </entry>
                     </row>
+                    <row>
+                        <entry>
+                            <property>org.hibernate.envers.embeddable_set_ordinal_field_name</property>
+                        </entry>
+                        <entry>
+                            SETORDINAL
+                        </entry>
+                        <entry>
+                            The name of the column used for storing the ordinal of the change in sets of embeddables. 
+                        </entry>
+                    </row>
                 </tbody>
             </tgroup>
         </table>

hibernate-envers/src/main/java/org/hibernate/envers/configuration/AuditEntitiesConfiguration.java

@@ -58,6 +58,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;
 
@@ -112,6 +114,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" );
     }
 
     public String getOriginalIdPropName() {
@@ -182,4 +188,8 @@ public class AuditEntitiesConfiguration {
     public String getRevisionEndFieldName() {
         return revisionEndFieldName;
     }
+
+	public String getEmbeddableSetOrdinalPropertyName() {
+		return embeddableSetOrdinalPropertyName;
+	}
 }

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

@@ -87,6 +87,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;
@@ -505,6 +506,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
@@ -528,14 +545,13 @@ public final class CollectionMetadataGenerator {
         Type type = propertyValue.getType();
 		boolean embeddableElementType = isEmbeddableElementType();
         if (type instanceof SortedSetType) {
-			currentMapper.addComposite(propertyAuditingData.getPropertyData(),
+			currentMapper.addComposite( propertyAuditingData.getPropertyData(), new SortedSetCollectionMapper(
-					new SortedSetCollectionMapper(commonCollectionMapperData,
+					commonCollectionMapperData, TreeSet.class, SortedSetProxy.class, elementComponentData,
-							TreeSet.class, SortedSetProxy.class, elementComponentData, propertyValue.getComparator(),
+					propertyValue.getComparator(), embeddableElementType, embeddableElementType ) );
-							embeddableElementType));
 		} else if (type instanceof SetType) {
-			currentMapper.addComposite(propertyAuditingData.getPropertyData(),
+			currentMapper.addComposite( propertyAuditingData.getPropertyData(), new BasicCollectionMapper<Set>(
-                    new BasicCollectionMapper<Set>(commonCollectionMapperData,
+					commonCollectionMapperData, HashSet.class, SetProxy.class, elementComponentData,
-                    HashSet.class, SetProxy.class, elementComponentData, embeddableElementType));
+					embeddableElementType, embeddableElementType ) );
         } else if (type instanceof SortedMapType) {
             // Indexed collection, so <code>indexComponentData</code> is not null.
 			currentMapper.addComposite(propertyAuditingData.getPropertyData(),
@@ -548,9 +564,9 @@ public final class CollectionMetadataGenerator {
                     new MapCollectionMapper<Map>(commonCollectionMapperData,
                     HashMap.class, MapProxy.class, elementComponentData, indexComponentData, embeddableElementType));
         } else if (type instanceof BagType) {
-            currentMapper.addComposite(propertyAuditingData.getPropertyData(),
+			currentMapper.addComposite( propertyAuditingData.getPropertyData(), new BasicCollectionMapper<List>(
-                    new BasicCollectionMapper<List>(commonCollectionMapperData,
+					commonCollectionMapperData, ArrayList.class, ListProxy.class, elementComponentData,
-                    ArrayList.class, ListProxy.class, elementComponentData, embeddableElementType));
+					embeddableElementType, embeddableElementType ) );
         } else if (type instanceof ListType) {
             // Indexed collection, so <code>indexComponentData</code> is not null.
             currentMapper.addComposite(propertyAuditingData.getPropertyData(),

hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/AbstractCollectionMapper.java

@@ -55,15 +55,17 @@ import org.hibernate.property.Setter;
 public abstract class AbstractCollectionMapper<T> implements PropertyMapper {
     protected final CommonCollectionMapperData commonCollectionMapperData;    
     protected final Class<? extends T> collectionClass;
+    protected final boolean ordinalInId;
 	protected final boolean revisionTypeInId;
 
     private final Constructor<? extends T> proxyConstructor;
 
 	protected AbstractCollectionMapper(CommonCollectionMapperData commonCollectionMapperData,
-                                       Class<? extends T> collectionClass, Class<? extends T> proxyClass,
+			Class<? extends T> collectionClass, Class<? extends T> 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<T> implements PropertyMapper {
      */
     protected abstract void mapToMapFromObject(SessionImplementor session, Map<String, Object> idData, Map<String, Object> data, Object changed);
 
+	/**
+	 * Creates a Map for the id.
+	 * 
+	 * <p>
+	 * 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.
+	 * </p>
+	 * 
+	 * @param ordinal
+	 *            The element iteration ordinal.
+	 * 
+	 * @return A Map for holding the ID information.
+	 */
+	protected Map<String, Object> createIdMap(int ordinal) {
+		final HashMap<String, Object> idMap = new HashMap<String, Object>();
+
+		if ( ordinalInId ) {
+			idMap.put( this.commonCollectionMapperData.getVerEntCfg().getEmbeddableSetOrdinalPropertyName(),
+					Integer.valueOf( ordinal ) );
+		}
+
+		return idMap;
+	}
+
     private void addCollectionChanges(SessionImplementor session, List<PersistentCollectionChangeData> collectionChanges,
 									  Set<Object> changed, RevisionType revisionType, Serializable id) {
+		int ordinal = 0;
+
         for (Object changedObj : changed) {
             Map<String, Object> entityData = new HashMap<String, Object>();
-            Map<String, Object> originalId = new HashMap<String, Object>();
+			Map<String, Object> originalId = createIdMap( ordinal++ );
             entityData.put(commonCollectionMapperData.getVerEntCfg().getOriginalIdPropName(), originalId);
 
             collectionChanges.add(new PersistentCollectionChangeData(

hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/BasicCollectionMapper.java

@@ -43,8 +43,8 @@ public class BasicCollectionMapper<T extends Collection> extends AbstractCollect
 
 	public BasicCollectionMapper(CommonCollectionMapperData commonCollectionMapperData,
 			Class<? extends T> collectionClass, Class<? extends T> proxyClass,
-                                 MiddleComponentData elementComponentData, boolean revisionTypeInId) {
+			MiddleComponentData elementComponentData, boolean ordinalInId, boolean revisionTypeInId) {
-        super(commonCollectionMapperData, collectionClass, proxyClass, revisionTypeInId);
+		super( commonCollectionMapperData, collectionClass, proxyClass, ordinalInId, revisionTypeInId );
         this.elementComponentData = elementComponentData;
     }
 

hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ListCollectionMapper.java

@@ -48,7 +48,7 @@ public final class ListCollectionMapper extends AbstractCollectionMapper<List> 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;
     }

hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/MapCollectionMapper.java

@@ -46,7 +46,7 @@ public class MapCollectionMapper<T extends Map> extends AbstractCollectionMapper
                                Class<? extends T> collectionClass, Class<? extends T> proxyClass,
                                MiddleComponentData elementComponentData, MiddleComponentData indexComponentData,
 							   boolean revisionTypeInId) {
-        super(commonCollectionMapperData, collectionClass, proxyClass, revisionTypeInId);
+		super( commonCollectionMapperData, collectionClass, proxyClass, false, revisionTypeInId );
         this.elementComponentData = elementComponentData;
         this.indexComponentData = indexComponentData;
     }

hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/SortedSetCollectionMapper.java

@@ -39,9 +39,10 @@ public final class SortedSetCollectionMapper extends BasicCollectionMapper<Sorte
 
 	public SortedSetCollectionMapper(CommonCollectionMapperData commonCollectionMapperData,
 			Class<? extends SortedSet> collectionClass, Class<? extends SortedSet> proxyClass,
-									 MiddleComponentData elementComponentData, Comparator comparator,
+			MiddleComponentData elementComponentData, Comparator comparator, boolean ordinalInId,
 			boolean revisionTypeInId) {
-		super(commonCollectionMapperData, collectionClass, proxyClass, elementComponentData, revisionTypeInId);
+		super( commonCollectionMapperData, collectionClass, proxyClass, elementComponentData, ordinalInId,
+				revisionTypeInId );
 		this.comparator = comparator;
 	}
 

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_3 ), rev1.getComponentSet() );
-		assertEquals( TestTools.makeSet( c3_1, c3_2 ), rev2.getComponentSet() );
+		assertEquals( TestTools.makeSet( c3_1, c3_2, c3_3 ), rev2.getComponentSet() );
-		assertEquals( TestTools.makeSet( c3_1 ), rev3.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() );
 	}
 }