HHH-11063 - Fix collection change logic for addition/removal of elements with same hash.

This commit is contained in:
Chris Cranford 2016-08-31 13:08:33 -04:00
parent 20ae4920b7
commit 2f3798de03
1 changed files with 138 additions and 28 deletions

View File

@ -13,11 +13,14 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.hibernate.collection.internal.PersistentMap;
import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.engine.spi.CollectionEntry;
import org.hibernate.engine.spi.SessionImplementor;
import org.hibernate.envers.RevisionType;
import org.hibernate.envers.boot.internal.EnversService;
@ -29,6 +32,7 @@ import org.hibernate.envers.internal.entities.mapper.relation.lazy.initializor.I
import org.hibernate.envers.internal.reader.AuditReaderImplementor;
import org.hibernate.envers.internal.tools.ReflectionTools;
import org.hibernate.internal.util.compare.EqualsHelper;
import org.hibernate.persister.collection.CollectionPersister;
import org.hibernate.property.access.spi.Setter;
/**
@ -134,36 +138,33 @@ public abstract class AbstractCollectionMapper<T> implements PropertyMapper {
return null;
}
final List<PersistentCollectionChangeData> collectionChanges = new ArrayList<>();
// Comparing new and old collection content.
final Collection newCollection = getNewCollectionContent( newColl );
final Collection oldCollection = getOldCollectionContent( oldColl );
final Set<Object> added = new HashSet<>();
if ( newColl != null ) {
added.addAll( newCollection );
}
// Re-hashing the old collection as the hash codes of the elements there may have changed, and the
// removeAll in AbstractSet has an implementation that is hashcode-change sensitive (as opposed to addAll).
if ( oldColl != null ) {
added.removeAll( new HashSet( oldCollection ) );
// HHH-11063
final CollectionEntry collectionEntry = session.getPersistenceContext().getCollectionEntry( newColl );
if ( collectionEntry != null ) {
// This next block delegates only to the persiter-based collection change code if
// the following are true:
// 1. New collection is not a PersistentMap.
// 2. The collection has a persister.
// 3. The collection is not indexed, e.g. @IndexColumn
//
// In the case of 1 and 3, the collection is transformed into a set of Pair<> elements where the
// pair's left element is either the map key or the index. In these cases, the key/index do
// affect the change code; hence why they're skipped here and handled at the end.
//
// For all others, the persister based method uses the collection's ElementType#isSame to calculate
// equality between the newColl and oldColl. This enforces the same equality check that core uses
// for element types such as @Entity in cases where the hash code does not use the id field but has
// the same value in both collections. Using #isSame, these will be seen as differing elements and
// changes to the collection will be returned.
if ( !( newColl instanceof PersistentMap ) ) {
final CollectionPersister collectionPersister = collectionEntry.getCurrentPersister();
if ( collectionPersister != null && !collectionPersister.hasIndex() ) {
return mapCollectionChanges( session, newColl, oldColl, id, collectionPersister );
}
}
}
addCollectionChanges( session, collectionChanges, added, RevisionType.ADD, id );
final Set<Object> deleted = new HashSet<>();
if ( oldColl != null ) {
deleted.addAll( oldCollection );
}
// The same as above - re-hashing new collection.
if ( newColl != null ) {
deleted.removeAll( new HashSet( newCollection ) );
}
addCollectionChanges( session, collectionChanges, deleted, RevisionType.DEL, id );
return collectionChanges;
return mapCollectionChanges( session, newColl, oldColl, id );
}
@Override
@ -277,4 +278,113 @@ public abstract class AbstractCollectionMapper<T> implements PropertyMapper {
throw new AuditException( e );
}
}
/**
* Map collection changes using hash identity.
*
* @param session The session.
* @param newColl The new persistent collection.
* @param oldColl The old collection.
* @param id The owning entity identifier.
* @return the persistent collection changes.
*/
@SuppressWarnings("unchecked")
private List<PersistentCollectionChangeData> mapCollectionChanges(
SessionImplementor session,
PersistentCollection newColl,
Serializable oldColl,
Serializable id) {
final List<PersistentCollectionChangeData> collectionChanges = new ArrayList<PersistentCollectionChangeData>();
// Comparing new and old collection content.
final Collection newCollection = getNewCollectionContent( newColl );
final Collection oldCollection = getOldCollectionContent( oldColl );
final Set<Object> added = new HashSet<>();
if ( newColl != null ) {
added.addAll( newCollection );
}
// Re-hashing the old collection as the hash codes of the elements there may have changed, and the
// removeAll in AbstractSet has an implementation that is hashcode-change sensitive (as opposed to addAll).
if ( oldColl != null ) {
added.removeAll( new HashSet( oldCollection ) );
}
addCollectionChanges( session, collectionChanges, added, RevisionType.ADD, id );
final Set<Object> deleted = new HashSet<>();
if ( oldColl != null ) {
deleted.addAll( oldCollection );
}
// The same as above - re-hashing new collection.
if ( newColl != null ) {
deleted.removeAll( new HashSet( newCollection ) );
}
addCollectionChanges( session, collectionChanges, deleted, RevisionType.DEL, id );
return collectionChanges;
}
/**
* Map collection changes using the collection element type equality functionality.
*
* @param session The session.
* @param newColl The new persistent collection.
* @param oldColl The old collection.
* @param id The owning entity identifier.
* @param collectionPersister The collection persister.
* @return the persistent collection changes.
*/
private List<PersistentCollectionChangeData> mapCollectionChanges(
SessionImplementor session,
PersistentCollection newColl,
Serializable oldColl,
Serializable id,
CollectionPersister collectionPersister) {
final List<PersistentCollectionChangeData> collectionChanges = new ArrayList<PersistentCollectionChangeData>();
// Comparing new and old collection content.
final Collection newCollection = getNewCollectionContent( newColl );
final Collection oldCollection = getOldCollectionContent( oldColl );
// take the new collection and remove any that exist in the old collection.
// take the resulting Set<> and generate ADD changes.
final Set<Object> added = new HashSet<>();
if ( newColl != null ) {
added.addAll( newCollection );
}
if ( oldColl != null && collectionPersister != null ) {
for ( Object object : oldCollection ) {
for ( Iterator addedIt = added.iterator(); addedIt.hasNext(); ) {
Object object2 = addedIt.next();
if ( collectionPersister.getElementType().isSame( object, object2 ) ) {
addedIt.remove();
break;
}
}
}
}
addCollectionChanges( session, collectionChanges, added, RevisionType.ADD, id );
// take the old collection and remove any that exist in the new collection.
// take the resulting Set<> and generate DEL changes.
final Set<Object> deleted = new HashSet<>();
if ( oldColl != null ) {
deleted.addAll( oldCollection );
}
if ( newColl != null && collectionPersister != null ) {
for ( Object object : newCollection ) {
for ( Iterator deletedIt = deleted.iterator(); deletedIt.hasNext(); ) {
Object object2 = deletedIt.next();
if ( collectionPersister.getElementType().isSame( object, object2 ) ) {
deletedIt.remove();
break;
}
}
}
}
addCollectionChanges( session, collectionChanges, deleted, RevisionType.DEL, id );
return collectionChanges;
}
}