HHH-9806 - Bytecode-enhancement-based dirty tracking does not work because PersistentAttributeInterceptor is never injected

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/AttributeTypeDescriptor.java

@@ -37,11 +37,10 @@ public abstract class AttributeTypeDescriptor {
 					return "";
 				}
 			}
-			builder.append( String.format( "if (%s() != null", EnhancerConstants.INTERCEPTOR_GETTER_NAME ) );
 
 			// primitives || enums
 			if ( currentValue.getType().isPrimitive() || currentValue.getType().isEnum() ) {
-				builder.append( String.format( " && %s != $1)", currentValue.getName()) );
+				builder.append( String.format( "if (%s != $1)", currentValue.getName()) );
 			}
 			// simple data types
 			else if ( currentValue.getType().getName().startsWith( "java.lang" )
@@ -50,7 +49,7 @@ public abstract class AttributeTypeDescriptor {
 					|| currentValue.getType().getName().startsWith( "java.sql.Date" )
 					|| currentValue.getType().getName().startsWith( "java.util.Date" )
 					|| currentValue.getType().getName().startsWith( "java.util.Calendar" ) ) {
-				builder.append( String.format( " && ((%s == null) || (!%<s.equals($1))))", currentValue.getName() ) );
+				builder.append( String.format( "if (%s == null || !%<s.equals($1))", currentValue.getName() ) );
 			}
 			// all other objects
 			else {
@@ -64,7 +63,7 @@ public abstract class AttributeTypeDescriptor {
 					}
 				}
 				// TODO: for now just call equals, should probably do something else here
-				builder.append( String.format( " && ((%s == null) || (!%<s.equals($1))))", currentValue.getName() ) );
+				builder.append( String.format( "if (%s == null || !%<s.equals($1))", currentValue.getName() ) );
 			}
 			builder.append( String.format( " { %s(\"%s\"); }", EnhancerConstants.TRACKER_CHANGER_NAME, currentValue.getName() ) );
 		}

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/EntityEnhancer.java

@@ -11,6 +11,7 @@ import javassist.CtClass;
 import javassist.CtField;
 import javassist.Modifier;
 import javassist.NotFoundException;
+
 import org.hibernate.bytecode.enhance.internal.tracker.CollectionTracker;
 import org.hibernate.bytecode.enhance.internal.tracker.SimpleDirtyTracker;
 import org.hibernate.bytecode.enhance.spi.EnhancementContext;
@@ -22,6 +23,7 @@ import org.hibernate.engine.spi.SelfDirtinessTracker;
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Locale;
 
 /**
  * enhancer for regular entities
@@ -54,49 +56,63 @@ public class EntityEnhancer extends Enhancer {
 		new PersistentAttributesEnhancer( enhancementContext ).enhance( managedCtClass );
 	}
 
-	/* -- */
-
 	private void addEntityInstanceHandling(CtClass managedCtClass) {
 		try {
-			MethodWriter.write( managedCtClass, "public Object %s() { return this; }", EnhancerConstants.ENTITY_INSTANCE_GETTER_NAME );
+			MethodWriter.write(
+					managedCtClass,
+					"public Object %s() { return this; }",
+					EnhancerConstants.ENTITY_INSTANCE_GETTER_NAME
+			);
 		}
 		catch (CannotCompileException cce) {
-			final String msg = String.format( "Could not enhance entity class [%s] to add EntityEntry getter", managedCtClass.getName() );
+			throw new EnhancementException(
-			throw new EnhancementException(msg, cce);
+					String.format(
+							Locale.ROOT,
+							"Could not enhance entity class [%s] to add EntityEntry getter",
+							managedCtClass.getName()
+					),
+					cce
+			);
 		}
 	}
 
-	/* -- */
-
 	private void addEntityEntryHandling(CtClass managedCtClass) {
-		FieldWriter.addFieldWithGetterAndSetter( managedCtClass, entityEntryCtClass,
+		FieldWriter.addFieldWithGetterAndSetter(
+				managedCtClass, entityEntryCtClass,
 				EnhancerConstants.ENTITY_ENTRY_FIELD_NAME,
 				EnhancerConstants.ENTITY_ENTRY_GETTER_NAME,
-				EnhancerConstants.ENTITY_ENTRY_SETTER_NAME );
+				EnhancerConstants.ENTITY_ENTRY_SETTER_NAME
+		);
 	}
 
 	private void addLinkedPreviousHandling(CtClass managedCtClass) {
-		FieldWriter.addFieldWithGetterAndSetter( managedCtClass, managedEntityCtClass,
+		FieldWriter.addFieldWithGetterAndSetter(
+				managedCtClass, managedEntityCtClass,
 				EnhancerConstants.PREVIOUS_FIELD_NAME,
 				EnhancerConstants.PREVIOUS_GETTER_NAME,
-				EnhancerConstants.PREVIOUS_SETTER_NAME );
+				EnhancerConstants.PREVIOUS_SETTER_NAME
+		);
 	}
 
 	private void addLinkedNextHandling(CtClass managedCtClass) {
-		FieldWriter.addFieldWithGetterAndSetter( managedCtClass, managedEntityCtClass,
+		FieldWriter.addFieldWithGetterAndSetter(
+				managedCtClass, managedEntityCtClass,
 				EnhancerConstants.NEXT_FIELD_NAME,
 				EnhancerConstants.NEXT_GETTER_NAME,
-				EnhancerConstants.NEXT_SETTER_NAME );
+				EnhancerConstants.NEXT_SETTER_NAME
+		);
 	}
 
-	/* --- */
-
 	private void addInLineDirtyHandling(CtClass managedCtClass) {
 		try {
 			managedCtClass.addInterface( classPool.get( SelfDirtinessTracker.class.getName() ) );
 
 			FieldWriter.addField( managedCtClass, classPool.get( TRACKER_IMPL ), EnhancerConstants.TRACKER_FIELD_NAME );
-			FieldWriter.addField( managedCtClass, classPool.get( CollectionTracker.class.getName() ), EnhancerConstants.TRACKER_COLLECTION_NAME );
+			FieldWriter.addField(
+					managedCtClass,
+					classPool.get( CollectionTracker.class.getName() ),
+					EnhancerConstants.TRACKER_COLLECTION_NAME
+			);
 
 			createDirtyTrackerMethods( managedCtClass );
 		}
@@ -107,24 +123,23 @@ public class EntityEnhancer extends Enhancer {
 
 	private void createDirtyTrackerMethods(CtClass managedCtClass) {
 		try {
-			MethodWriter.write( managedCtClass, "" +
+			MethodWriter.write(
+					managedCtClass, "" +
 							"public void %1$s(String name) {%n" +
 							"  if (%2$s == null) { %2$s = new %3$s(); }%n" +
 							"  %2$s.add(name);%n" +
 							"}",
 					EnhancerConstants.TRACKER_CHANGER_NAME,
 					EnhancerConstants.TRACKER_FIELD_NAME,
-					TRACKER_IMPL );
+					TRACKER_IMPL
-
+			);
-			/* --- */
 
 			createCollectionDirtyCheckMethod( managedCtClass );
 			createCollectionDirtyCheckGetFieldsMethod( managedCtClass );
 			createClearDirtyCollectionMethod( managedCtClass );
 
-			/* --- */
+			MethodWriter.write(
-
+					managedCtClass, "" +
-			MethodWriter.write( managedCtClass, "" +
 							"public java.util.Set %1$s() {%n" +
 							"  if (%2$s == null) { %2$s = new %4$s(); }%n" +
 							"  %3$s(%2$s);%n" +
@@ -133,32 +148,37 @@ public class EntityEnhancer extends Enhancer {
 					EnhancerConstants.TRACKER_GET_NAME,
 					EnhancerConstants.TRACKER_FIELD_NAME,
 					EnhancerConstants.TRACKER_COLLECTION_CHANGED_FIELD_NAME,
-					TRACKER_IMPL );
+					TRACKER_IMPL
+			);
 
-			MethodWriter.write( managedCtClass, "" +
+			MethodWriter.write(
+					managedCtClass,
+					"" +
 							"public boolean %1$s() {%n" +
 							"  return (%2$s != null && !%2$s.isEmpty()) || %3$s();%n" +
 							"}",
 					EnhancerConstants.TRACKER_HAS_CHANGED_NAME,
 					EnhancerConstants.TRACKER_FIELD_NAME,
-					EnhancerConstants.TRACKER_COLLECTION_CHANGED_NAME );
+					EnhancerConstants.TRACKER_COLLECTION_CHANGED_NAME
+			);
 
-			MethodWriter.write( managedCtClass, "" +
+			MethodWriter.write(
+					managedCtClass,
+					"" +
 							"public void %1$s() {%n" +
 							"  if (%2$s != null) { %2$s.clear(); }%n" +
 							"  %3$s();%n" +
 							"}",
 					EnhancerConstants.TRACKER_CLEAR_NAME,
 					EnhancerConstants.TRACKER_FIELD_NAME,
-					EnhancerConstants.TRACKER_COLLECTION_CLEAR_NAME );
+					EnhancerConstants.TRACKER_COLLECTION_CLEAR_NAME
+			);
 		}
 		catch (CannotCompileException cce) {
 			cce.printStackTrace();
 		}
 	}
 
-	/* -- */
-
 	private List<CtField> collectCollectionFields(CtClass managedCtClass) {
 		final List<CtField> collectionList = new LinkedList<CtField>();
 		try {
@@ -186,21 +206,30 @@ public class EntityEnhancer extends Enhancer {
 		try {
 			final StringBuilder body = new StringBuilder();
 
-			body.append( String.format( "" +
+			body.append(
+					String.format(
+							"" +
 									"private boolean %1$s() {%n" +
-							"  if (%2$s() == null || %3$s == null) { return false; }%n",
+									"  if (%2$s == null) {%n" +
+									"    return false;%n" +
+									"  }%n",
 							EnhancerConstants.TRACKER_COLLECTION_CHANGED_NAME,
-					EnhancerConstants.INTERCEPTOR_GETTER_NAME,
+							EnhancerConstants.TRACKER_COLLECTION_NAME
-					EnhancerConstants.TRACKER_COLLECTION_NAME ) );
+					)
+			);
 
 			for ( CtField ctField : collectCollectionFields( managedCtClass ) ) {
 				if ( !enhancementContext.isMappedCollection( ctField ) ) {
-					body.append( String.format( "" +
+					body.append(
+							String.format(
+									"" +
 											"  // collection field [%1$s]%n" +
 											"  if (%1$s == null && %2$s.getSize(\"%1$s\") != -1) { return true; }%n" +
 											"  if (%1$s != null && %2$s.getSize(\"%1$s\") != %1$s.size()) { return true; }%n",
 									ctField.getName(),
-							EnhancerConstants.TRACKER_COLLECTION_NAME ) );
+									EnhancerConstants.TRACKER_COLLECTION_NAME
+							)
+					);
 				}
 			}
 			body.append( "  return false;%n}" );
@@ -216,21 +245,29 @@ public class EntityEnhancer extends Enhancer {
 		try {
 			final StringBuilder body = new StringBuilder();
 
-			body.append( String.format( "" +
+			body.append(
+					String.format(
+							"" +
 									"private void %1$s(%3$s tracker) {%n" +
 									"  if (%2$s == null) { return; }%n",
 							EnhancerConstants.TRACKER_COLLECTION_CHANGED_FIELD_NAME,
 							EnhancerConstants.TRACKER_COLLECTION_NAME,
-					TRACKER_IMPL ) );
+							TRACKER_IMPL
+					)
+			);
 
 			for ( CtField ctField : collectCollectionFields( managedCtClass ) ) {
 				if ( !enhancementContext.isMappedCollection( ctField ) ) {
-					body.append( String.format( "" +
+					body.append(
+							String.format(
+									"" +
 											"  // Collection field [%1$s]%n" +
 											"  if (%1$s == null && %2$s.getSize(\"%1$s\") != -1) { tracker.add(\"%1$s\"); }%n" +
 											"  if (%1$s != null && %2$s.getSize(\"%1$s\") != %1$s.size()) { tracker.add(\"%1$s\"); }%n",
 									ctField.getName(),
-							EnhancerConstants.TRACKER_COLLECTION_NAME ) );
+									EnhancerConstants.TRACKER_COLLECTION_NAME
+							)
+					);
 				}
 			}
 			body.append( "}" );
@@ -246,21 +283,29 @@ public class EntityEnhancer extends Enhancer {
 		try {
 			final StringBuilder body = new StringBuilder();
 
-			body.append( String.format( "" +
+			body.append(
+					String.format(
+							"" +
 									"private void %1$s() {%n" +
 									"  if (%2$s == null) { %2$s = new %3$s(); }%n",
 							EnhancerConstants.TRACKER_COLLECTION_CLEAR_NAME,
 							EnhancerConstants.TRACKER_COLLECTION_NAME,
-					CollectionTracker.class.getName()) );
+							CollectionTracker.class.getName()
+					)
+			);
 
 			for ( CtField ctField : collectCollectionFields( managedCtClass ) ) {
 				if ( !enhancementContext.isMappedCollection( ctField ) ) {
-					body.append( String.format( "" +
+					body.append(
+							String.format(
+									"" +
 											"  // Collection field [%1$s]%n" +
 											"  if (%1$s == null) { %2$s.add(\"%1$s\", -1); }%n" +
 											"  else { %2$s.add(\"%1$s\", %1$s.size()); }%n",
 									ctField.getName(),
-							EnhancerConstants.TRACKER_COLLECTION_NAME) );
+									EnhancerConstants.TRACKER_COLLECTION_NAME
+							)
+					);
 				}
 			}
 			body.append( "}" );

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/PersistentAttributesEnhancer.java

@@ -19,6 +19,7 @@ import javassist.bytecode.MethodInfo;
 import javassist.bytecode.Opcode;
 import javassist.bytecode.SignatureAttribute;
 import javassist.bytecode.stackmap.MapMaker;
+
 import org.hibernate.bytecode.enhance.spi.EnhancementContext;
 import org.hibernate.bytecode.enhance.spi.EnhancementException;
 import org.hibernate.bytecode.enhance.spi.Enhancer;
@@ -54,16 +55,18 @@ public class PersistentAttributesEnhancer extends Enhancer {
 		final IdentityHashMap<String, PersistentAttributeAccessMethods> attrDescriptorMap = new IdentityHashMap<String, PersistentAttributeAccessMethods>();
 
 		for ( CtField persistentField : collectPersistentFields( managedCtClass ) ) {
-			attrDescriptorMap.put( persistentField.getName(), enhancePersistentAttribute( managedCtClass, persistentField ) );
+			attrDescriptorMap.put(
+					persistentField.getName(), enhancePersistentAttribute(
+							managedCtClass,
+							persistentField
+					)
+			);
 		}
 
 		// lastly, find all references to the transformed fields and replace with calls to the added reader/writer methods
 		enhanceAttributesAccess( managedCtClass, attrDescriptorMap );
 	}
 
-	/* --- */
-
-	// TODO: drive this from the Hibernate metamodel instance...
 	private CtField[] collectPersistentFields(CtClass managedCtClass) {
 		final List<CtField> persistentFieldList = new LinkedList<CtField>();
 		for ( CtField ctField : managedCtClass.getDeclaredFields() ) {
@@ -78,22 +81,30 @@ public class PersistentAttributesEnhancer extends Enhancer {
 		return enhancementContext.order( persistentFieldList.toArray( new CtField[persistentFieldList.size()] ) );
 	}
 
-	/* --- */
+	private PersistentAttributeAccessMethods enhancePersistentAttribute(
-
+			CtClass managedCtClass,
-	private PersistentAttributeAccessMethods enhancePersistentAttribute(CtClass managedCtClass, CtField persistentField) {
+			CtField persistentField) {
 		try {
 			final AttributeTypeDescriptor typeDescriptor = AttributeTypeDescriptor.resolve( persistentField );
 			return new PersistentAttributeAccessMethods(
 					generateFieldReader( managedCtClass, persistentField, typeDescriptor ),
-					generateFieldWriter( managedCtClass, persistentField, typeDescriptor ) );
+					generateFieldWriter( managedCtClass, persistentField, typeDescriptor )
+			);
 		}
 		catch (Exception e) {
-			final String msg = String.format( "Unable to enhance persistent attribute [%s:%s]", managedCtClass.getName(), persistentField.getName() );
+			final String msg = String.format(
+					"Unable to enhance persistent attribute [%s:%s]",
+					managedCtClass.getName(),
+					persistentField.getName()
+			);
 			throw new EnhancementException( msg, e );
 		}
 	}
 
-	private CtMethod generateFieldReader(CtClass managedCtClass, CtField persistentField, AttributeTypeDescriptor typeDescriptor) {
+	private CtMethod generateFieldReader(
+			CtClass managedCtClass,
+			CtField persistentField,
+			AttributeTypeDescriptor typeDescriptor) {
 		final String fieldName = persistentField.getName();
 		final String readerName = EnhancerConstants.PERSISTENT_FIELD_READER_PREFIX + fieldName;
 
@@ -105,23 +116,36 @@ public class PersistentAttributesEnhancer extends Enhancer {
 
 		// TODO: temporary solution...
 		try {
-			return MethodWriter.write( managedCtClass, "public %s %s() {%n  %s%n  return this.%s;%n}",
+			return MethodWriter.write(
+					managedCtClass, "public %s %s() {%n  %s%n  return this.%s;%n}",
 					persistentField.getType().getName(),
 					readerName,
 					typeDescriptor.buildReadInterceptionBodyFragment( fieldName ),
-					fieldName);
+					fieldName
+			);
 		}
 		catch (CannotCompileException cce) {
-			final String msg = String.format( "Could not enhance entity class [%s] to add field reader method [%s]",  managedCtClass.getName(), readerName );
+			final String msg = String.format(
+					"Could not enhance entity class [%s] to add field reader method [%s]",
+					managedCtClass.getName(),
+					readerName
+			);
 			throw new EnhancementException( msg, cce );
 		}
 		catch (NotFoundException nfe) {
-			final String msg = String.format( "Could not enhance entity class [%s] to add field reader method [%s]",  managedCtClass.getName(), readerName );
+			final String msg = String.format(
+					"Could not enhance entity class [%s] to add field reader method [%s]",
+					managedCtClass.getName(),
+					readerName
+			);
 			throw new EnhancementException( msg, nfe );
 		}
 	}
 
-	private CtMethod generateFieldWriter(CtClass managedCtClass, CtField persistentField, AttributeTypeDescriptor typeDescriptor) {
+	private CtMethod generateFieldWriter(
+			CtClass managedCtClass,
+			CtField persistentField,
+			AttributeTypeDescriptor typeDescriptor) {
 		final String fieldName = persistentField.getName();
 		final String writerName = EnhancerConstants.PERSISTENT_FIELD_WRITER_PREFIX + fieldName;
 
@@ -132,20 +156,32 @@ public class PersistentAttributesEnhancer extends Enhancer {
 				writer = MethodWriter.addSetter( managedCtClass, fieldName, writerName );
 			}
 			else {
-				writer = MethodWriter.write( managedCtClass, "public void %s(%s %s) {%n  %s%n}",
+				writer = MethodWriter.write(
+						managedCtClass,
+						"public void %s(%s %s) {%n  %s%n}",
 						writerName,
 						persistentField.getType().getName(),
 						fieldName,
-						typeDescriptor.buildWriteInterceptionBodyFragment( fieldName ) );
+						typeDescriptor.buildWriteInterceptionBodyFragment( fieldName )
+				);
 			}
 
 			if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
-				writer.insertBefore( String.format( "if (%s != null) { %<s.callOwner(\".%s\"); }%n",
+				writer.insertBefore(
+						String.format(
+								"if (%s != null) { %<s.callOwner(\".%s\"); }%n",
 								EnhancerConstants.TRACKER_COMPOSITE_FIELD_NAME,
-						fieldName ) );
+								fieldName
+						)
+				);
 			}
 			else if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
-				writer.insertBefore( typeDescriptor.buildInLineDirtyCheckingBodyFragment( enhancementContext, persistentField ) );
+				writer.insertBefore(
+						typeDescriptor.buildInLineDirtyCheckingBodyFragment(
+								enhancementContext,
+								persistentField
+						)
+				);
 			}
 
 			handleCompositeField( managedCtClass, persistentField, writer );
@@ -156,29 +192,44 @@ public class PersistentAttributesEnhancer extends Enhancer {
 			return writer;
 		}
 		catch (CannotCompileException cce) {
-			final String msg = String.format( "Could not enhance entity class [%s] to add field writer method [%s]",  managedCtClass.getName(), writerName );
+			final String msg = String.format(
+					"Could not enhance entity class [%s] to add field writer method [%s]",
+					managedCtClass.getName(),
+					writerName
+			);
 			throw new EnhancementException( msg, cce );
 		}
 		catch (NotFoundException nfe) {
-			final String msg = String.format( "Could not enhance entity class [%s] to add field writer method [%s]",  managedCtClass.getName(), writerName );
+			final String msg = String.format(
+					"Could not enhance entity class [%s] to add field writer method [%s]",
+					managedCtClass.getName(),
+					writerName
+			);
 			throw new EnhancementException( msg, nfe );
 		}
 	}
 
-	/* --- */
+	private void handleBiDirectionalAssociation(CtClass managedCtClass, CtField persistentField, CtMethod fieldWriter)
-
+			throws NotFoundException, CannotCompileException {
-	private void handleBiDirectionalAssociation(CtClass managedCtClass, CtField persistentField, CtMethod fieldWriter) throws NotFoundException, CannotCompileException {
 		if ( !isPossibleBiDirectionalAssociation( persistentField ) ) {
 			return;
 		}
 		final CtClass targetEntity = getTargetEntityClass( persistentField );
 		if ( targetEntity == null ) {
-			log.debugf( "Could not find type of bi-directional association for field [%s#%s]", managedCtClass.getName(), persistentField.getName() );
+			log.debugf(
+					"Could not find type of bi-directional association for field [%s#%s]",
+					managedCtClass.getName(),
+					persistentField.getName()
+			);
 			return;
 		}
 		final String mappedBy = getMappedBy( persistentField, targetEntity );
 		if ( mappedBy.isEmpty() ) {
-			log.debugf( "Could not find bi-directional association for field [%s#%s]", managedCtClass.getName(), persistentField.getName() );
+			log.debugf(
+					"Could not find bi-directional association for field [%s#%s]",
+					managedCtClass.getName(),
+					persistentField.getName()
+			);
 			return;
 		}
 
@@ -190,41 +241,73 @@ public class PersistentAttributesEnhancer extends Enhancer {
 
 		if ( persistentField.hasAnnotation( OneToOne.class ) ) {
 			// only unset when $1 != null to avoid recursion
-			fieldWriter.insertBefore( String.format( "if ($0.%s != null && $1 != null) $0.%<s.%s(null);%n",
+			fieldWriter.insertBefore(
+					String.format(
+							"if ($0.%s != null && $1 != null) $0.%<s.%s(null);%n",
 							persistentField.getName(),
-					mappedBySetterName));
+							mappedBySetterName
-			fieldWriter.insertAfter( String.format( "if ($1 != null && $1.%s() != $0) $1.%s($0);%n",
+					)
+			);
+			fieldWriter.insertAfter(
+					String.format(
+							"if ($1 != null && $1.%s() != $0) $1.%s($0);%n",
 							mappedByGetterName,
-					mappedBySetterName) );
+							mappedBySetterName
+					)
+			);
 		}
 		if ( persistentField.hasAnnotation( OneToMany.class ) ) {
 			// only remove elements not in the new collection or else we would loose those elements
-			fieldWriter.insertBefore( String.format( "if ($0.%s != null) for (java.util.Iterator itr = $0.%<s.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if ($1 == null || !$1.contains(target)) target.%s(null); }%n",
+			fieldWriter.insertBefore(
+					String.format(
+							"if ($0.%s != null) for (java.util.Iterator itr = $0.%<s.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if ($1 == null || !$1.contains(target)) target.%s(null); }%n",
 							persistentField.getName(),
 							targetEntity.getName(),
-					mappedBySetterName ) );
+							mappedBySetterName
-			fieldWriter.insertAfter( String.format( "if ($1 != null) for (java.util.Iterator itr = $1.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if (target.%s() != $0) target.%s((%s)$0); }%n",
+					)
+			);
+			fieldWriter.insertAfter(
+					String.format(
+							"if ($1 != null) for (java.util.Iterator itr = $1.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if (target.%s() != $0) target.%s((%s)$0); }%n",
 							targetEntity.getName(),
 							mappedByGetterName,
 							mappedBySetterName,
-					managedCtClass.getName() ) );
+							managedCtClass.getName()
+					)
+			);
 		}
 		if ( persistentField.hasAnnotation( ManyToOne.class ) ) {
-			fieldWriter.insertBefore( String.format( "if ($0.%1$s != null && $0.%1$s.%2$s() != null) $0.%1$s.%2$s().remove($0);%n",
+			fieldWriter.insertBefore(
+					String.format(
+							"if ($0.%1$s != null && $0.%1$s.%2$s() != null) $0.%1$s.%2$s().remove($0);%n",
 							persistentField.getName(),
-					mappedByGetterName) );
+							mappedByGetterName
+					)
+			);
 			// check .contains($0) to avoid double inserts (but preventing duplicates)
-			fieldWriter.insertAfter( String.format( "if ($1 != null && $1.%s() != null && !$1.%<s().contains($0) ) $1.%<s().add($0);%n",
+			fieldWriter.insertAfter(
-					mappedByGetterName));
+					String.format(
+							"if ($1 != null && $1.%s() != null && !$1.%<s().contains($0) ) $1.%<s().add($0);%n",
+							mappedByGetterName
+					)
+			);
 		}
 		if ( persistentField.hasAnnotation( ManyToMany.class ) ) {
-			fieldWriter.insertBefore( String.format( "if ($0.%s != null) for (java.util.Iterator itr = $0.%<s.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if ($1 == null || !$1.contains(target)) target.%s().remove($0); }%n",
+			fieldWriter.insertBefore(
+					String.format(
+							"if ($0.%s != null) for (java.util.Iterator itr = $0.%<s.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if ($1 == null || !$1.contains(target)) target.%s().remove($0); }%n",
 							persistentField.getName(),
 							targetEntity.getName(),
-					mappedByGetterName));
+							mappedByGetterName
-			fieldWriter.insertAfter( String.format( "if ($1 != null) for (java.util.Iterator itr = $1.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if (target.%s() != $0 && target.%<s() != null) target.%<s().add($0); }%n",
+					)
+			);
+			fieldWriter.insertAfter(
+					String.format(
+							"if ($1 != null) for (java.util.Iterator itr = $1.iterator(); itr.hasNext(); ) { %s target = (%<s) itr.next(); if (target.%s() != $0 && target.%<s() != null) target.%<s().add($0); }%n",
 							targetEntity.getName(),
-					mappedByGetterName ) );
+							mappedByGetterName
+					)
+			);
 		}
 		// implementation note: association management @OneToMany and @ManyToMay works for add() operations but for remove() a snapshot of the collection is needed so we know what associations to break.
 		// another approach that could force that behavior would be to return Collections.unmodifiableCollection() ...
@@ -264,7 +347,13 @@ public class PersistentAttributesEnhancer extends Enhancer {
 		// get mappedBy value by searching in the fields of the target entity class
 		for ( CtField f : targetEntity.getDeclaredFields() ) {
 			if ( enhancementContext.isPersistentField( f ) && getMappedByFromAnnotation( f ).equals( persistentField.getName() ) ) {
-				log.debugf( "mappedBy association for field [%s:%s] is [%s:%s]", persistentField.getDeclaringClass().getName(), persistentField.getName(), targetEntity.getName(), f.getName() );
+				log.debugf(
+						"mappedBy association for field [%s:%s] is [%s:%s]",
+						persistentField.getDeclaringClass().getName(),
+						persistentField.getName(),
+						targetEntity.getName(),
+						f.getName()
+				);
 				return f.getName();
 			}
 		}
@@ -300,7 +389,9 @@ public class PersistentAttributesEnhancer extends Enhancer {
 		}
 		if ( persistentField.hasAnnotation( OneToMany.class ) || persistentField.hasAnnotation( ManyToMany.class ) ) {
 			try {
-				final SignatureAttribute.TypeArgument target = ((SignatureAttribute.ClassType) SignatureAttribute.toFieldSignature( persistentField.getGenericSignature() )).getTypeArguments()[0];
+				final SignatureAttribute.TypeArgument target = ( (SignatureAttribute.ClassType) SignatureAttribute.toFieldSignature(
+						persistentField.getGenericSignature()
+				) ).getTypeArguments()[0];
 				return persistentField.getDeclaringClass().getClassPool().get( target.toString() );
 			}
 			catch (BadBytecode ignore) {
@@ -309,9 +400,8 @@ public class PersistentAttributesEnhancer extends Enhancer {
 		return null;
 	}
 
-	/* --- */
+	private void handleCompositeField(CtClass managedCtClass, CtField persistentField, CtMethod fieldWriter)
-
+			throws NotFoundException, CannotCompileException {
-	private void handleCompositeField(CtClass managedCtClass, CtField persistentField, CtMethod fieldWriter) throws NotFoundException, CannotCompileException {
 		if ( !persistentField.hasAnnotation( Embedded.class ) ) {
 			return;
 		}
@@ -321,34 +411,44 @@ public class PersistentAttributesEnhancer extends Enhancer {
 
 		if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
 			// if a composite have a embedded field we need to implement the TRACKER_CHANGER_NAME method as well
-			MethodWriter.write( managedCtClass, "" +
+			MethodWriter.write(
+					managedCtClass, "" +
 							"public void %1$s(String name) {%n" +
 							"  if (%2$s != null) { %2$s.callOwner(\".\" + name) ; }%n}",
 					EnhancerConstants.TRACKER_CHANGER_NAME,
-					EnhancerConstants.TRACKER_COMPOSITE_FIELD_NAME );
+					EnhancerConstants.TRACKER_COMPOSITE_FIELD_NAME
+			);
 		}
 
 		// cleanup previous owner
-		fieldWriter.insertBefore( String.format( "" +
+		fieldWriter.insertBefore(
+				String.format(
+						"" +
 								"if (%1$s != null) { ((%2$s) %1$s).%3$s(\"%1$s\"); }%n",
 						persistentField.getName(),
 						CompositeTracker.class.getName(),
-				EnhancerConstants.TRACKER_COMPOSITE_CLEAR_OWNER) );
+						EnhancerConstants.TRACKER_COMPOSITE_CLEAR_OWNER
+				)
+		);
 
 		// trigger track changes
-		fieldWriter.insertAfter( String.format( "" +
+		fieldWriter.insertAfter(
+				String.format(
+						"" +
 								"((%2$s) %1$s).%4$s(\"%1$s\", (%3$s) this);%n" +
 								"%5$s(\"%1$s\");",
 						persistentField.getName(),
 						CompositeTracker.class.getName(),
 						CompositeOwner.class.getName(),
 						EnhancerConstants.TRACKER_COMPOSITE_SET_OWNER,
-				EnhancerConstants.TRACKER_CHANGER_NAME ) );
+						EnhancerConstants.TRACKER_CHANGER_NAME
+				)
+		);
 	}
 
-	/* --- */
+	protected void enhanceAttributesAccess(
-
+			CtClass managedCtClass,
-	protected void enhanceAttributesAccess(CtClass managedCtClass, IdentityHashMap<String, PersistentAttributeAccessMethods> attributeDescriptorMap) {
+			IdentityHashMap<String, PersistentAttributeAccessMethods> attributeDescriptorMap) {
 		final ConstPool constPool = managedCtClass.getClassFile().getConstPool();
 
 		for ( Object oMethod : managedCtClass.getClassFile().getMethods() ) {
@@ -392,16 +492,16 @@ public class PersistentAttributesEnhancer extends Enhancer {
 				methodInfo.getCodeAttribute().setAttribute( MapMaker.make( classPool, methodInfo ) );
 			}
 			catch (BadBytecode bb) {
-				final String msg = String.format( "Unable to perform field access transformation in method [%s]",  methodName );
+				final String msg = String.format(
+						"Unable to perform field access transformation in method [%s]",
+						methodName
+				);
 				throw new EnhancementException( msg, bb );
 			}
 		}
 	}
 
-	/* --- */
-
 	private static class PersistentAttributeAccessMethods {
-
 		private final CtMethod reader;
 		private final CtMethod writer;
 

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/Enhancer.java

@@ -96,16 +96,6 @@ public class Enhancer {
 		}
 	}
 
-	/**
-	 * @deprecated Should use enhance(String, byte[]) and a proper EnhancementContext
-	 */
-	@Deprecated( )
-	public byte[] enhanceComposite(String className, byte[] originalBytes) throws EnhancementException {
-		return enhance( className, originalBytes );
-	}
-
-	/* --- */
-
 	private ClassPool buildClassPool(EnhancementContext enhancementContext) {
 		final ClassPool classPool = new ClassPool( false );
 		final ClassLoader loadingClassLoader = enhancementContext.getLoadingClassLoader();
@@ -123,8 +113,6 @@ public class Enhancer {
 		return aClass.getName().replace( '.', File.separatorChar ) + JavaFileObject.Kind.CLASS.extension;
 	}
 
-	/* --- */
-
 	private void enhance(CtClass managedCtClass) {
 		// can't effectively enhance interfaces
 		if ( managedCtClass.isInterface() ) {
@@ -172,8 +160,6 @@ public class Enhancer {
 		}
 	}
 
-	/* --- */
-
 	protected void addInterceptorHandling(CtClass managedCtClass) {
 		// interceptor handling is only needed if either:
 		//		a) in-line dirty checking has *not* been requested
@@ -191,4 +177,12 @@ public class Enhancer {
 				EnhancerConstants.INTERCEPTOR_SETTER_NAME );
 	}
 
+
+	/**
+	 * @deprecated Should use enhance(String, byte[]) and a proper EnhancementContext
+	 */
+	@Deprecated( )
+	public byte[] enhanceComposite(String className, byte[] originalBytes) throws EnhancementException {
+		return enhance( className, originalBytes );
+	}
 }

hibernate-core/src/main/java/org/hibernate/engine/spi/SelfDirtinessTracker.java

@@ -9,18 +9,28 @@ package org.hibernate.engine.spi;
 import java.util.Set;
 
 /**
- * Specify if an entity class is instrumented to track field changes
+ * Contract for an entity to report that it tracks the dirtiness of its own state,
+ * as opposed to needing Hibernate to perform state-diff dirty calculations.
+ * <p/>
+ * Entity classes are free to implement this contract themselves.  This contract is
+ * also introduced into the entity when using bytecode enhancement and requesting
+ * that entities track there own dirtiness.
  *
  * @author <a href="mailto:stale.pedersen@jboss.org">Ståle W. Pedersen</a>
  */
 public interface SelfDirtinessTracker {
 	/**
-	 * Return true if any fields has been changed
+	 * Have any of the entity's persistent attributes changed?
+	 *
+	 * @return {@code true} indicates one or more persistent attributes have changed; {@code false}
+	 * indicates none have changed.
 	 */
 	boolean $$_hibernate_hasDirtyAttributes();
 
 	/**
-	 * Get the field names of all the fields thats been changed
+	 * Retrieve the names of all the persistent attributes whose values have changed.
+	 *
+	 * @return The set of changed persistent attribute names
 	 */
 	Set<String> $$_hibernate_getDirtyAttributes();
 

hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java

@@ -15,7 +15,6 @@ import java.util.Set;
 import org.hibernate.bytecode.enhance.spi.EnhancerConstants;
 import org.hibernate.engine.spi.EntityEntry;
 import org.hibernate.engine.spi.ManagedEntity;
-import org.hibernate.engine.spi.PersistentAttributeInterceptor;
 
 import org.hibernate.testing.junit4.BaseUnitTestCase;
 import org.hibernate.test.bytecode.enhancement.entity.Address;
@@ -70,16 +69,6 @@ public class EnhancerTest extends BaseUnitTestCase {
         nextSetter.invoke( entityInstance, entityInstance );
         assertSame( entityInstance, nextGetter.invoke( entityInstance ) );
 
-        // add an attribute interceptor...
-        Method interceptorGetter = entityClass.getMethod(EnhancerConstants.INTERCEPTOR_GETTER_NAME);
-        assertNull(interceptorGetter.invoke(entityInstance));
-        entityClass.getMethod("getId").invoke(entityInstance);
-
-        Method interceptorSetter = entityClass.getMethod(EnhancerConstants.INTERCEPTOR_SETTER_NAME, PersistentAttributeInterceptor.class);
-        interceptorSetter.invoke(entityInstance, new EnhancerTestUtils.LocalPersistentAttributeInterceptor());
-        assertNotNull(interceptorGetter.invoke(entityInstance));
-
-        // dirty checking is unfortunately just printlns for now... just verify the test output
         entityClass.getMethod("getId").invoke(entityInstance);
         entityClass.getMethod("setId", Long.class).invoke(entityInstance, entityClass.getMethod("getId").invoke(entityInstance));
         entityClass.getMethod("setId", Long.class).invoke(entityInstance, 1L);
@@ -109,9 +98,6 @@ public class EnhancerTest extends BaseUnitTestCase {
             entityClass = EnhancerTestUtils.enhanceAndDecompile(entityClassToEnhance, cl);
             entityInstance = entityClass.newInstance();
 
-            interceptorSetter = entityClass.getMethod(EnhancerConstants.INTERCEPTOR_SETTER_NAME, PersistentAttributeInterceptor.class);
-            interceptorSetter.invoke(entityInstance, new EnhancerTestUtils.LocalPersistentAttributeInterceptor());
-
             List<String> strings = new ArrayList<String>();
             strings.add("FooBar");
             entityClass.getMethod("setSomeStrings", List.class).invoke(entityInstance, strings);

hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTestUtils.java

@@ -92,10 +92,17 @@ public abstract class EnhancerTestUtils extends BaseUnitTestCase {
 		try {
 			JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
 			StandardJavaFileManager fileManager = compiler.getStandardFileManager( null, null, null );
-            fileManager.setLocation(StandardLocation.CLASS_OUTPUT, Collections.singletonList(new File(workingDir)));
+			fileManager.setLocation(
+					StandardLocation.CLASS_OUTPUT,
+					Collections.singletonList( new File( workingDir ) )
+			);
 
 			JavapTask javapTask = new JavapTask();
-            for (JavaFileObject jfo : fileManager.getJavaFileObjects(workingDir + File.separator + getFilenameForClassName(className))) {
+			for ( JavaFileObject jfo : fileManager.getJavaFileObjects(
+					workingDir + File.separator + getFilenameForClassName(
+							className
+					)
+			) ) {
 				try {
 					Set<String> interfaceNames = new HashSet<String>();
 					Set<String> fieldNames = new HashSet<String>();
@@ -154,20 +161,29 @@ public abstract class EnhancerTestUtils extends BaseUnitTestCase {
 						assertTrue( fieldNames.contains( EnhancerConstants.TRACKER_CHANGER_NAME ) );
 						assertTrue( methodNames.contains( EnhancerConstants.TRACKER_CHANGER_NAME ) );
 					}
-                } catch (ConstantPoolException e) {
+				}
+				catch (ConstantPoolException e) {
 					e.printStackTrace();
 				}
 			}
-        } catch (IOException ioe) {
+		}
+		catch (IOException ioe) {
 			assertNull( "Failed to open class file", ioe );
-        } catch (RuntimeException re) {
+		}
+		catch (RuntimeException re) {
 			log.warnf( re, "WARNING: UNABLE DECOMPILE DUE TO %s", re.getMessage() );
 		}
 	}
 
 	private static CtClass generateCtClassForAnEntity(Class<?> entityClassToEnhance) throws Exception {
 		ClassPool cp = new ClassPool( false );
-        return cp.makeClass(EnhancerTestUtils.class.getClassLoader().getResourceAsStream(getFilenameForClassName(entityClassToEnhance.getName())));
+		return cp.makeClass(
+				EnhancerTestUtils.class.getClassLoader().getResourceAsStream(
+						getFilenameForClassName(
+								entityClassToEnhance.getName()
+						)
+				)
+		);
 	}
 
 	private static String getFilenameForClassName(String className) {
@@ -178,34 +194,18 @@ public abstract class EnhancerTestUtils extends BaseUnitTestCase {
 	 * clears the dirty set for an entity
 	 */
 	public static void clearDirtyTracking(Object entityInstance) {
-        try {
+		( (SelfDirtinessTracker) entityInstance ).$$_hibernate_clearDirtyAttributes();
-            entityInstance.getClass().getMethod(EnhancerConstants.TRACKER_CLEAR_NAME).invoke(entityInstance);
-            checkDirtyTracking(entityInstance);
-        } catch (InvocationTargetException e) {
-            assertNull("Exception in clear dirty tracking", e);
-        } catch (NoSuchMethodException e) {
-            assertNull("Exception in clear dirty tracking", e);
-        } catch (IllegalAccessException e) {
-            assertNull("Exception in clear  dirty tracking", e);
-        }
 	}
 
 	/**
 	 * compares the dirty fields of an entity with a set of expected values
 	 */
 	public static void checkDirtyTracking(Object entityInstance, String... dirtyFields) {
-        try {
+		final SelfDirtinessTracker selfDirtinessTracker = (SelfDirtinessTracker) entityInstance;
-            assertTrue((dirtyFields.length == 0) != (Boolean) entityInstance.getClass().getMethod(EnhancerConstants.TRACKER_HAS_CHANGED_NAME).invoke(entityInstance));
+		assertEquals( dirtyFields.length > 0, selfDirtinessTracker.$$_hibernate_hasDirtyAttributes() );
-            Set<?> tracked = (Set<?>) entityInstance.getClass().getMethod(EnhancerConstants.TRACKER_GET_NAME).invoke(entityInstance);
+		Set<?> tracked = selfDirtinessTracker.$$_hibernate_getDirtyAttributes();
 		assertEquals( dirtyFields.length, tracked.size() );
 		assertTrue( tracked.containsAll( Arrays.asList( dirtyFields ) ) );
-        } catch (InvocationTargetException e) {
-            assertNull("Exception while checking dirty tracking", e);
-        } catch (NoSuchMethodException e) {
-            assertNull("Exception while checking dirty tracking", e);
-        } catch (IllegalAccessException e) {
-            assertNull("Exception while checking dirty tracking", e);
-        }
 	}
 
 	static EntityEntry makeEntityEntry() {
@@ -226,83 +226,110 @@ public abstract class EnhancerTestUtils extends BaseUnitTestCase {
 
 	public static class LocalPersistentAttributeInterceptor implements PersistentAttributeInterceptor {
 
-        @Override public boolean readBoolean(Object obj, String name, boolean oldValue) {
+		@Override
+		public boolean readBoolean(Object obj, String name, boolean oldValue) {
 			log.infof( "Reading boolean [%s]", name );
 			return oldValue;
 		}
-        @Override public boolean writeBoolean(Object obj, String name, boolean oldValue, boolean newValue) {
+
+		@Override
+		public boolean writeBoolean(Object obj, String name, boolean oldValue, boolean newValue) {
 			log.infof( "Writing boolean []", name );
 			return newValue;
 		}
 
-        @Override public byte readByte(Object obj, String name, byte oldValue) {
+		@Override
+		public byte readByte(Object obj, String name, byte oldValue) {
 			log.infof( "Reading byte [%s]", name );
 			return oldValue;
 		}
-        @Override public byte writeByte(Object obj, String name, byte oldValue, byte newValue) {
+
+		@Override
+		public byte writeByte(Object obj, String name, byte oldValue, byte newValue) {
 			log.infof( "Writing byte [%s]", name );
 			return newValue;
 		}
 
-        @Override public char readChar(Object obj, String name, char oldValue) {
+		@Override
+		public char readChar(Object obj, String name, char oldValue) {
 			log.infof( "Reading char [%s]", name );
 			return oldValue;
 		}
-        @Override public char writeChar(Object obj, String name, char oldValue, char newValue) {
+
+		@Override
+		public char writeChar(Object obj, String name, char oldValue, char newValue) {
 			log.infof( "Writing char [%s]", name );
 			return newValue;
 		}
 
-        @Override public short readShort(Object obj, String name, short oldValue) {
+		@Override
+		public short readShort(Object obj, String name, short oldValue) {
 			log.infof( "Reading short [%s]", name );
 			return oldValue;
 		}
-        @Override public short writeShort(Object obj, String name, short oldValue, short newValue) {
+
+		@Override
+		public short writeShort(Object obj, String name, short oldValue, short newValue) {
 			log.infof( "Writing short [%s]", name );
 			return newValue;
 		}
 
-        @Override public int readInt(Object obj, String name, int oldValue) {
+		@Override
+		public int readInt(Object obj, String name, int oldValue) {
 			log.infof( "Reading int [%s]", name );
 			return oldValue;
 		}
-        @Override public int writeInt(Object obj, String name, int oldValue, int newValue) {
+
+		@Override
+		public int writeInt(Object obj, String name, int oldValue, int newValue) {
 			log.infof( "Writing int [%s]", name );
 			return newValue;
 		}
 
-        @Override public float readFloat(Object obj, String name, float oldValue) {
+		@Override
+		public float readFloat(Object obj, String name, float oldValue) {
 			log.infof( "Reading float [%s]", name );
 			return oldValue;
 		}
-        @Override public float writeFloat(Object obj, String name, float oldValue, float newValue) {
+
+		@Override
+		public float writeFloat(Object obj, String name, float oldValue, float newValue) {
 			log.infof( "Writing float [%s]", name );
 			return newValue;
 		}
 
-        @Override public double readDouble(Object obj, String name, double oldValue) {
+		@Override
+		public double readDouble(Object obj, String name, double oldValue) {
 			log.infof( "Reading double [%s]", name );
 			return oldValue;
 		}
-        @Override public double writeDouble(Object obj, String name, double oldValue, double newValue) {
+
+		@Override
+		public double writeDouble(Object obj, String name, double oldValue, double newValue) {
 			log.infof( "Writing double [%s]", name );
 			return newValue;
 		}
 
-        @Override public long readLong(Object obj, String name, long oldValue) {
+		@Override
+		public long readLong(Object obj, String name, long oldValue) {
 			log.infof( "Reading long [%s]", name );
 			return oldValue;
 		}
-        @Override public long writeLong(Object obj, String name, long oldValue, long newValue) {
+
+		@Override
+		public long writeLong(Object obj, String name, long oldValue, long newValue) {
 			log.infof( "Writing long [%s]", name );
 			return newValue;
 		}
 
-        @Override public Object readObject(Object obj, String name, Object oldValue) {
+		@Override
+		public Object readObject(Object obj, String name, Object oldValue) {
 			log.infof( "Reading Object [%s]", name );
 			return oldValue;
 		}
-        @Override public Object writeObject(Object obj, String name, Object oldValue, Object newValue) {
+
+		@Override
+		public Object writeObject(Object obj, String name, Object oldValue, Object newValue) {
 			log.infof( "Writing Object [%s]", name );
 			return newValue;
 		}