HHH-16572 Incorrect enhancement for PROPERTY attributes with mismatched field and method names

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

@@ -17,6 +17,8 @@
 import java.util.Optional;
 import java.util.function.Supplier;
 
+import net.bytebuddy.description.type.TypeList;
+import net.bytebuddy.dynamic.scaffold.MethodGraph;
 import org.hibernate.Version;
 import org.hibernate.bytecode.enhance.VersionMismatchException;
 import org.hibernate.bytecode.enhance.internal.tracker.CompositeOwnerTracker;
@@ -172,6 +174,12 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
 		}
 
 		if ( enhancementContext.isEntityClass( managedCtClass ) ) {
+
+			// Check for HHH-16572 (PROPERTY attributes with mismatched field and method names)
+			if ( !allowedEnhancementCheck( managedCtClass ) ) {
+				return null;
+			}
+
 			log.debugf( "Enhancing [%s] as Entity", managedCtClass.getName() );
 			DynamicType.Builder<?> builder = builderSupplier.get();
 			builder = builder.implement( ManagedEntity.class )
@@ -331,6 +339,12 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
 			return createTransformer( managedCtClass ).applyTo( builder );
 		}
 		else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
+
+			// Check for HHH-16572 (PROPERTY attributes with mismatched field and method names)
+			if ( !allowedEnhancementCheck( managedCtClass ) ) {
+				return null;
+			}
+
 			log.debugf( "Enhancing [%s] as Composite", managedCtClass.getName() );
 
 			DynamicType.Builder<?> builder = builderSupplier.get();
@@ -364,6 +378,12 @@ else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
 			return createTransformer( managedCtClass ).applyTo( builder );
 		}
 		else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
+
+			// Check for HHH-16572 (PROPERTY attributes with mismatched field and method names)
+			if ( !allowedEnhancementCheck( managedCtClass ) ) {
+				return null;
+			}
+
 			log.debugf( "Enhancing [%s] as MappedSuperclass", managedCtClass.getName() );
 
 			DynamicType.Builder<?> builder = builderSupplier.get();
@@ -380,6 +400,79 @@ else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
 		}
 	}
 
+	// See HHH-16572
+	// return true if enhancement is supported
+	private boolean allowedEnhancementCheck(TypeDescription managedCtClass) {
+		// For process access rules, See https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#default-access-type
+		// and https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a122
+		//
+		// This check will determine if entity field names do not match Property accessor method name
+		// For example:
+		// @Entity
+		// class Book {
+		//   Integer id;
+		//   String smtg;
+		//
+		//   @Id Integer getId() { return id; }
+		//   String getSomething() { return smtg; }
+		// }
+		//
+		// Check name of the getter/setter method with Peristence annotation and getter/setter method name that doesn't refer to an entity field
+		// and will return false.  If the property accessor method(s) are named to match the field name(s), return true.
+		boolean propertyHasAnnotation = false;
+		MethodGraph.Linked methodGraph = MethodGraph.Compiler.Default.forJavaHierarchy().compile((TypeDefinition) managedCtClass);
+		for (MethodGraph.Node node : methodGraph.listNodes()) {
+			MethodDescription methodDescription = node.getRepresentative();
+			if (methodDescription.getDeclaringType().represents(Object.class)) { // skip class java.lang.Object methods
+				continue;
+			}
+
+			String methodName = methodDescription.getActualName();
+			if (methodName.equals("") ||
+					(!methodName.startsWith("get") && !methodName.startsWith("set") && !methodName.startsWith("is"))) {
+				continue;
+			}
+			String methodFieldName;
+			if (methodName.startsWith("is")) { // skip past "is"
+				methodFieldName = methodName.substring(2);
+			}
+			else if (methodName.startsWith("get") ||
+					methodName.startsWith("set")) { // skip past "get" or "set"
+				methodFieldName = methodName.substring(3);
+			}
+			else {
+				// not a property accessor method so ignore it
+				continue;
+			}
+			boolean propertyNameMatchesFieldName = false;
+			// convert field letter to lower case
+			methodFieldName = methodFieldName.substring(0, 1).toLowerCase() + methodFieldName.substring(1);
+			TypeList typeList = methodDescription.getDeclaredAnnotations().asTypeList();
+			if (typeList.stream().anyMatch(typeDefinitions ->
+					(typeDefinitions.getName().contains("jakarta.persistence")))) {
+				propertyHasAnnotation = true;
+			}
+			for (FieldDescription ctField : methodDescription.getDeclaringType().getDeclaredFields()) {
+				if (!Modifier.isStatic(ctField.getModifiers())) {
+					AnnotatedFieldDescription annotatedField = new AnnotatedFieldDescription(enhancementContext, ctField);
+					boolean containsPropertyAccessorMethods = false;
+					if (enhancementContext.isPersistentField(annotatedField)) {
+						if (methodFieldName.equals(ctField.getActualName())) {
+							propertyNameMatchesFieldName = true;
+							break;
+						}
+					}
+				}
+			}
+			if (propertyHasAnnotation && !propertyNameMatchesFieldName) {
+				log.debugf("Skipping enhancement of [%s]: due to class [%s] not having a property accessor method name matching field name [%s]",
+						managedCtClass, methodDescription.getDeclaringType().getActualName(), methodFieldName);
+				return false;
+			}
+		}
+		return true;
+	}
+
 	private static void verifyVersions(TypeDescription managedCtClass, ByteBuddyEnhancementContext enhancementContext) {
 		final AnnotationDescription.Loadable<EnhancementInfo> existingInfo = managedCtClass
 				.getDeclaredAnnotations()

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/access/InvalidPropertyNameTest.java

@@ -0,0 +1,87 @@
+package org.hibernate.orm.test.bytecode.enhancement.access;
+
+import jakarta.persistence.*;
+import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
+import org.hibernate.testing.orm.junit.*;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@DomainModel(
+		annotatedClasses = {
+				InvalidPropertyNameTest.SomeEntity.class,
+		}
+)
+@SessionFactory
+@JiraKey("HHH-16572")
+@BytecodeEnhanced
+public class InvalidPropertyNameTest {
+
+
+	@Test
+	@FailureExpected(jiraKey = "HHH-16572")
+	public void test(SessionFactoryScope scope) {
+		scope.inTransaction( session -> {
+			session.persist( new SomeEntity( 1L, "field", "property" ) );
+		} );
+
+		scope.inTransaction( session -> {
+			SomeEntity entity = session.get( SomeEntity.class, 1L );
+			assertThat( entity.property ).isEqualTo( "from getter: property" );
+
+			entity.setPropertyMethod( "updated" );
+		} );
+
+		scope.inTransaction( session -> {
+			SomeEntity entity = session.get( SomeEntity.class, 1L );
+			assertThat( entity.property ).isEqualTo( "from getter: updated" );
+		} );
+	}
+
+	@AfterEach
+	public void cleanup(SessionFactoryScope scope) {
+		// uncomment the following when @FailureExpected is removed above
+		// scope.inTransaction( session -> {
+		//	session.remove( session.get( SomeEntity.class, 1L ) );
+		// PropertyAccessTest} );
+	}
+
+	@Entity
+	@Table(name = "SOME_ENTITY")
+	static class SomeEntity {
+		@Id
+		Long id;
+
+		@Basic
+		String field;
+
+		String property;
+
+		public SomeEntity() {
+		}
+
+		public SomeEntity(Long id, String field, String property) {
+			this.id = id;
+			this.field = field;
+			this.property = property;
+		}
+
+		/**
+		 * The following property accessor methods are purposely named incorrectly to
+		 * not match the "property" field.  The HHH-16572 change ensures that
+		 * this entity is not (bytecode) enhanced.  Eventually further changes will be made
+		 * such that this entity is enhanced in which case the FailureExpected can be removed
+		 * and the cleanup() uncommented.
+		 */
+		@Basic
+		@Access(AccessType.PROPERTY)
+		public String getPropertyMethod() {
+			return "from getter: " + property;
+		}
+
+		public void setPropertyMethod(String property) {
+			this.property = property;
+		}
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/lazy/LazyLoadingByEnhancerSetterTest.java

@@ -10,7 +10,6 @@
 
 import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
 import org.hibernate.testing.orm.junit.DomainModel;
-import org.hibernate.testing.orm.junit.FailureExpected;
 import org.hibernate.testing.orm.junit.JiraKey;
 import org.hibernate.testing.orm.junit.ServiceRegistry;
 import org.hibernate.testing.orm.junit.SessionFactory;
@@ -79,7 +78,7 @@ public void testField(SessionFactoryScope scope) {
     }
 
     @Test
-    @FailureExpected( jiraKey = "HHH-10747" )
+    // failure doesn't occur with HHH-16572 change @FailureExpected( jiraKey = "HHH-10747" )
     public void testProperty(SessionFactoryScope scope) {
         scope.inTransaction( s -> {
             ItemProperty input = new ItemProperty();