HHH-8973 - Fix tracking entity modifications to detached entities with the modified flags feature.

hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/EnversIntegrator.java

@@ -18,6 +18,7 @@ import org.hibernate.envers.event.spi.EnversPostInsertEventListenerImpl;
 import org.hibernate.envers.event.spi.EnversPostUpdateEventListenerImpl;
 import org.hibernate.envers.event.spi.EnversPreCollectionRemoveEventListenerImpl;
 import org.hibernate.envers.event.spi.EnversPreCollectionUpdateEventListenerImpl;
+import org.hibernate.envers.event.spi.EnversPreUpdateEventListenerImpl;
 import org.hibernate.event.service.spi.EventListenerRegistry;
 import org.hibernate.event.spi.EventType;
 import org.hibernate.integrator.spi.Integrator;
@@ -29,6 +30,7 @@ import org.jboss.logging.Logger;
  * Hooks up Envers event listeners.
  *
  * @author Steve Ebersole
+ * @author Chris Cranford
  */
 public class EnversIntegrator implements Integrator {
 	private static final Logger log = Logger.getLogger( EnversIntegrator.class );
@@ -89,6 +91,10 @@ public class EnversIntegrator implements Integrator {
 					EventType.POST_INSERT,
 					new EnversPostInsertEventListenerImpl( enversService )
 			);
+			listenerRegistry.appendListeners(
+					EventType.PRE_UPDATE,
+					new EnversPreUpdateEventListenerImpl( enversService )
+			);
 			listenerRegistry.appendListeners(
 					EventType.POST_UPDATE,
 					new EnversPostUpdateEventListenerImpl( enversService )

hibernate-envers/src/main/java/org/hibernate/envers/event/spi/BaseEnversUpdateEventListener.java

@@ -0,0 +1,35 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.event.spi;
+
+import org.hibernate.envers.boot.internal.EnversService;
+import org.hibernate.envers.internal.entities.EntityConfiguration;
+
+/**
+ * @author Chris Cranford
+ */
+public abstract class BaseEnversUpdateEventListener extends BaseEnversEventListener {
+	public BaseEnversUpdateEventListener(EnversService enversService) {
+		super( enversService );
+	}
+
+	/**
+	 * Returns whether the entity has {@code withModifiedFlag} features and has no old state, most likely implying
+	 * it was updated in a detached entity state.
+	 *
+	 * @param entityName The associated entity name.
+	 * @param oldState The event old (likely detached) entity state.
+	 * @return {@code true} if the entity is/has been updated in detached state, otherwise {@code false}.
+	 */
+	protected boolean isDetachedEntityUpdate(String entityName, Object[] oldState) {
+		final EntityConfiguration configuration = getEnversService().getEntitiesConfigurations().get( entityName );
+		if ( configuration.getPropertyMapper() != null && oldState == null ) {
+			return configuration.getPropertyMapper().hasPropertiesWithModifiedFlag();
+		}
+		return false;
+	}
+}

hibernate-envers/src/main/java/org/hibernate/envers/event/spi/EnversPostUpdateEventListenerImpl.java

@@ -20,8 +20,9 @@ import org.hibernate.persister.entity.EntityPersister;
  * @author Adam Warski (adam at warski dot org)
  * @author HernпїЅn Chanfreau
  * @author Steve Ebersole
+ * @author Chris Cranford
  */
-public class EnversPostUpdateEventListenerImpl extends BaseEnversEventListener implements PostUpdateEventListener {
+public class EnversPostUpdateEventListenerImpl extends BaseEnversUpdateEventListener implements PostUpdateEventListener {
 	public EnversPostUpdateEventListenerImpl(EnversService enversService) {
 		super( enversService );
 	}
@@ -34,6 +35,8 @@ public class EnversPostUpdateEventListenerImpl extends BaseEnversEventListener i
 			checkIfTransactionInProgress( event.getSession() );
 
 			final AuditProcess auditProcess = getEnversService().getAuditProcessManager().get( event.getSession() );
+
+			Object[] oldState = getOldDBState( auditProcess, entityName, event );
 			final Object[] newDbState = postUpdateDBState( event );
 			final AuditWorkUnit workUnit = new ModWorkUnit(
 					event.getSession(),
@@ -42,7 +45,7 @@ public class EnversPostUpdateEventListenerImpl extends BaseEnversEventListener i
 					event.getId(),
 					event.getPersister(),
 					newDbState,
-					event.getOldState()
+					oldState
 			);
 			auditProcess.addWorkUnit( workUnit );
 
@@ -52,13 +55,20 @@ public class EnversPostUpdateEventListenerImpl extends BaseEnversEventListener i
 						event.getPersister(),
 						entityName,
 						newDbState,
-						event.getOldState(),
+						oldState,
 						event.getSession()
 				);
 			}
 		}
 	}
 
+	private Object[] getOldDBState(AuditProcess auditProcess, String entityName, PostUpdateEvent event) {
+		if ( isDetachedEntityUpdate( entityName, event.getOldState() ) ) {
+			return auditProcess.getCachedEntityState( event.getId(), entityName );
+		}
+		return event.getOldState();
+	}
+
 	private Object[] postUpdateDBState(PostUpdateEvent event) {
 		final Object[] newDbState = event.getState().clone();
 		if ( event.getOldState() != null ) {

hibernate-envers/src/main/java/org/hibernate/envers/event/spi/EnversPreUpdateEventListenerImpl.java

@@ -0,0 +1,40 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.event.spi;
+
+import org.hibernate.envers.boot.internal.EnversService;
+import org.hibernate.envers.internal.synchronization.AuditProcess;
+import org.hibernate.event.spi.PreUpdateEvent;
+import org.hibernate.event.spi.PreUpdateEventListener;
+
+/**
+ * Envers-specific entity (pre) update event listener.
+ *
+ * @author Chris Cranford
+ */
+public class EnversPreUpdateEventListenerImpl extends BaseEnversUpdateEventListener implements PreUpdateEventListener {
+	public EnversPreUpdateEventListenerImpl(EnversService enversService) {
+		super( enversService );
+	}
+
+	@Override
+	public boolean onPreUpdate(PreUpdateEvent event) {
+		final String entityName = event.getPersister().getEntityName();
+		if ( getEnversService().getEntitiesConfigurations().isVersioned( entityName ) ) {
+			checkIfTransactionInProgress( event.getSession() );
+			if ( isDetachedEntityUpdate( entityName, event.getOldState() ) ) {
+				final AuditProcess auditProcess = getEnversService().getAuditProcessManager().get( event.getSession() );
+				auditProcess.cacheEntityState(
+						event.getId(),
+						entityName,
+						event.getPersister().getDatabaseSnapshot( event.getId(), event.getSession() )
+				);
+			}
+		}
+		return false;
+	}
+}

hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/ComponentPropertyMapper.java

@@ -25,6 +25,7 @@ import org.hibernate.property.access.spi.Setter;
  * @author Adam Warski (adam at warski dot org)
  * @author Michal Skowronek (mskowr at o2 dot pl)
  * @author Lukasz Zuchowski (author at zuchos dot com)
+ * @author Chris Cranford
  */
 public class ComponentPropertyMapper implements PropertyMapper, CompositeMapperBuilder {
 	private final PropertyData propertyData;
@@ -158,4 +159,9 @@ public class ComponentPropertyMapper implements PropertyMapper, CompositeMapperB
 	public Map<PropertyData, PropertyMapper> getProperties() {
 		return delegate.getProperties();
 	}
+
+	@Override
+	public boolean hasPropertiesWithModifiedFlag() {
+		return delegate.hasPropertiesWithModifiedFlag();
+	}
 }

hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/ModifiedFlagMapperSupport.java

@@ -0,0 +1,23 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.internal.entities.mapper;
+
+/**
+ * Contract for {@link PropertyMapper} implementations to expose whether they contain any property
+ * that uses {@link org.hibernate.envers.internal.entities.PropertyData#isUsingModifiedFlag()}.
+ *
+ * @author Chris Cranford
+ */
+public interface ModifiedFlagMapperSupport {
+	/**
+	 * Returns whether the associated {@link PropertyMapper} has any properties that use
+	 * the {@code witModifiedFlag} feature.
+	 *
+	 * @return {@code true} if a property uses {@code withModifiedFlag}, otherwise {@code false}.
+	 */
+	boolean hasPropertiesWithModifiedFlag();
+}

hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/MultiPropertyMapper.java

@@ -233,4 +233,14 @@ public class MultiPropertyMapper implements ExtendedPropertyMapper {
 	public Map<String, PropertyData> getPropertyDatas() {
 		return propertyDatas;
 	}
+
+	@Override
+	public boolean hasPropertiesWithModifiedFlag() {
+		for ( PropertyData property : getProperties().keySet() ) {
+			if ( property.isUsingModifiedFlag() ) {
+				return true;
+			}
+		}
+		return false;
+	}
 }

hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/PropertyMapper.java

@@ -18,8 +18,9 @@ import org.hibernate.envers.internal.reader.AuditReaderImplementor;
 /**
  * @author Adam Warski (adam at warski dot org)
  * @author Michal Skowronek (mskowr at o2 dot pl)
+ * @author Chris Cranford
  */
-public interface PropertyMapper {
+public interface PropertyMapper extends ModifiedFlagMapperSupport {
 	/**
 	 * Maps properties to the given map, basing on differences between properties of new and old objects.
 	 *

hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/SinglePropertyMapper.java

@@ -133,4 +133,8 @@ public class SinglePropertyMapper implements PropertyMapper, SimpleMapperBuilder
 		return null;
 	}
 
+	@Override
+	public boolean hasPropertiesWithModifiedFlag() {
+		return propertyData != null && propertyData.isUsingModifiedFlag();
+	}
 }

hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/SubclassPropertyMapper.java

@@ -23,6 +23,7 @@ import org.hibernate.envers.internal.reader.AuditReaderImplementor;
  *
  * @author Adam Warski (adam at warski dot org)
  * @author Michal Skowronek (mskowr at o2 dot pl)
+ * @author Chris Cranford
  */
 public class SubclassPropertyMapper implements ExtendedPropertyMapper {
 	private ExtendedPropertyMapper main;
@@ -140,4 +141,16 @@ public class SubclassPropertyMapper implements ExtendedPropertyMapper {
 		joinedProperties.putAll( main.getProperties() );
 		return joinedProperties;
 	}
+
+	@Override
+	public boolean hasPropertiesWithModifiedFlag() {
+		// checks all properties, exposed both by the main mapper and parent mapper.
+		for ( PropertyData property : getProperties().keySet() ) {
+			if ( property.isUsingModifiedFlag() ) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 }

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

@@ -38,6 +38,7 @@ import org.hibernate.property.access.spi.Setter;
 /**
  * @author Adam Warski (adam at warski dot org)
  * @author Michal Skowronek (mskowr at o2 dot pl)
+ * @author Chris Cranford
  */
 public abstract class AbstractCollectionMapper<T> implements PropertyMapper {
 	protected final CommonCollectionMapperData commonCollectionMapperData;
@@ -395,4 +396,13 @@ public abstract class AbstractCollectionMapper<T> implements PropertyMapper {
 
 		return collectionChanges;
 	}
+
+	@Override
+	public boolean hasPropertiesWithModifiedFlag() {
+		if ( commonCollectionMapperData != null ) {
+			final PropertyData propertyData = commonCollectionMapperData.getCollectionReferencingPropertyData();
+			return propertyData != null && propertyData.isUsingModifiedFlag();
+		}
+		return false;
+	}
 }

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

@@ -26,6 +26,7 @@ import org.hibernate.service.ServiceRegistry;
  * Base class for property mappers that manage to-one relation.
  *
  * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
+ * @author Chris Cranford
  */
 public abstract class AbstractToOneMapper implements PropertyMapper {
 	private final ServiceRegistry serviceRegistry;
@@ -135,4 +136,9 @@ public abstract class AbstractToOneMapper implements PropertyMapper {
 			return audited;
 		}
 	}
+
+	@Override
+	public boolean hasPropertiesWithModifiedFlag() {
+		return propertyData != null && propertyData.isUsingModifiedFlag();
+	}
 }

hibernate-envers/src/main/java/org/hibernate/envers/internal/synchronization/AuditProcess.java

@@ -16,6 +16,7 @@ import javax.persistence.FlushModeType;
 import org.hibernate.Session;
 import org.hibernate.action.spi.BeforeTransactionCompletionProcess;
 import org.hibernate.engine.spi.SessionImplementor;
+import org.hibernate.envers.exception.AuditException;
 import org.hibernate.envers.internal.revisioninfo.RevisionInfoGenerator;
 import org.hibernate.envers.internal.synchronization.work.AuditWorkUnit;
 import org.hibernate.envers.tools.Pair;
@@ -24,6 +25,7 @@ import org.jboss.logging.Logger;
 
 /**
  * @author Adam Warski (adam at warski dot org)
+ * @author Chris Cranford
  */
 public class AuditProcess implements BeforeTransactionCompletionProcess {
 	private static final Logger log = Logger.getLogger( AuditProcess.class );
@@ -34,8 +36,8 @@ public class AuditProcess implements BeforeTransactionCompletionProcess {
 	private final LinkedList<AuditWorkUnit> workUnits;
 	private final Queue<AuditWorkUnit> undoQueue;
 	private final Map<Pair<String, Object>, AuditWorkUnit> usedIds;
+	private final Map<Pair<String, Object>, Object[]> entityStateCache;
 	private final EntityChangeNotifier entityChangeNotifier;
-
 	private Object revisionData;
 
 	public AuditProcess(RevisionInfoGenerator revisionInfoGenerator, SessionImplementor session) {
@@ -45,9 +47,27 @@ public class AuditProcess implements BeforeTransactionCompletionProcess {
 		workUnits = new LinkedList<>();
 		undoQueue = new LinkedList<>();
 		usedIds = new HashMap<>();
+		entityStateCache = new HashMap<>();
 		entityChangeNotifier = new EntityChangeNotifier( revisionInfoGenerator, session );
 	}
 
+	public void cacheEntityState(Object id, String entityName, Object[] snapshot) {
+		final Pair<String, Object> key = new Pair<>( entityName, id );
+		if ( entityStateCache.containsKey( key ) ) {
+			throw new AuditException( "The entity [" + entityName + "] with id [" + id + "] is already cached." );
+		}
+		entityStateCache.put( key, snapshot );
+	}
+
+	public Object[] getCachedEntityState(Object id, String entityName) {
+		final Pair<String, Object> key = new Pair<>( entityName, id );
+		final Object[] entityState = entityStateCache.get( key );
+		if ( entityState != null ) {
+			entityStateCache.remove( key );
+		}
+		return entityState;
+	}
+
 	private void removeWorkUnit(AuditWorkUnit vwu) {
 		workUnits.remove( vwu );
 		if ( vwu.isPerformed() ) {