HHH-18928 Use enhanced field access even when defaulting to PROPERTY

mbellade · mbellade · commit 2d5f845f7dfc · 2025-02-07T12:45:32.000+01:00
This avoids problems with unnecessary initialization of lazy
properties when using instrumented getter/setter methods internally.
diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/internal/bytebuddy/BytecodeProviderImpl.java b/hibernate-core/src/main/java/org/hibernate/bytecode/internal/bytebuddy/BytecodeProviderImpl.java
@@ -1199,8 +1199,8 @@ private static void findAccessors(
 			if ( getter instanceof GetterMethodImpl ) {
 				getterMember = getter.getMethod();
 			}
-			else if ( getter instanceof GetterFieldImpl ) {
-				getterMember = getter.getMember();
+			else if ( getter instanceof GetterFieldImpl getterField ) {
+				getterMember = getterField.getField();
 			}
 			else {
 				throw new InvalidPropertyAccessorException(
diff --git a/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java b/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java
@@ -4,6 +4,9 @@
  */
 package org.hibernate.property.access.internal;
 
+import jakarta.persistence.AccessType;
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.hibernate.property.access.spi.EnhancedGetterFieldImpl;
 import org.hibernate.property.access.spi.EnhancedSetterImpl;
 import org.hibernate.property.access.spi.EnhancedSetterMethodImpl;
 import org.hibernate.property.access.spi.Getter;
@@ -17,9 +20,7 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 
-import jakarta.persistence.AccessType;
-import org.checkerframework.checker.nullness.qual.Nullable;
-
+import static org.hibernate.internal.util.ReflectHelper.findField;
 import static org.hibernate.internal.util.ReflectHelper.findSetterMethod;
 import static org.hibernate.internal.util.ReflectHelper.getterMethodOrNull;
 import static org.hibernate.property.access.internal.AccessStrategyHelper.fieldOrNull;
@@ -41,10 +42,12 @@ public PropertyAccessEnhancedImpl(
 			PropertyAccessStrategy strategy,
 			Class<?> containerJavaType,
 			String propertyName,
-			@Nullable AccessType getterAccessType) {
+			@Nullable AccessType classAccessType) {
 		this.strategy = strategy;
 
-		final AccessType propertyAccessType = resolveAccessType( getterAccessType, containerJavaType, propertyName );
+		final AccessType propertyAccessType = classAccessType == null ?
+				AccessStrategyHelper.getAccessType( containerJavaType, propertyName ) :
+				classAccessType;
 
 		switch ( propertyAccessType ) {
 			case FIELD: {
@@ -65,10 +68,8 @@ public PropertyAccessEnhancedImpl(
 							"Could not locate getter for property named [" + containerJavaType.getName() + "#" + propertyName + "]"
 					);
 				}
-				final Method setterMethod = findSetterMethod( containerJavaType, propertyName, getterMethod.getReturnType() );
-
-				this.getter = new GetterMethodImpl( containerJavaType, propertyName, getterMethod );
-				this.setter = new EnhancedSetterMethodImpl( containerJavaType, propertyName, setterMethod );
+				this.getter = propertyGetter( classAccessType, containerJavaType, propertyName, getterMethod );
+				this.setter = propertySetter( classAccessType, containerJavaType, propertyName, getterMethod.getReturnType() );
 				break;
 			}
 			default: {
@@ -79,12 +80,31 @@ public PropertyAccessEnhancedImpl(
 		}
 	}
 
-	private static AccessType resolveAccessType(@Nullable AccessType getterAccessType, Class<?> containerJavaType, String propertyName) {
-		if ( getterAccessType != null ) {
-			// this should indicate FIELD access
-			return getterAccessType;
+	private static Getter propertyGetter(@Nullable AccessType classAccessType, Class<?> containerJavaType, String propertyName, Method getterMethod) {
+		if ( classAccessType != null ) {
+			final AccessType explicitAccessType = AccessStrategyHelper.getAccessType( containerJavaType, propertyName );
+			if ( explicitAccessType == AccessType.FIELD ) {
+				// We need to default to FIELD unless we have an explicit AccessType to avoid unnecessary initializations
+				final Field field = findField( containerJavaType, propertyName );
+				return new EnhancedGetterFieldImpl( containerJavaType, propertyName, field, getterMethod );
+			}
+		}
+		// when classAccessType is null know PROPERTY is the explicit access type
+		return new GetterMethodImpl( containerJavaType, propertyName, getterMethod );
+	}
+
+	private static Setter propertySetter(@Nullable AccessType classAccessType, Class<?> containerJavaType, String propertyName, Class<?> fieldType) {
+		if ( classAccessType != null ) {
+			final AccessType explicitAccessType = AccessStrategyHelper.getAccessType( containerJavaType, propertyName );
+			if ( explicitAccessType == AccessType.FIELD ) {
+				// We need to default to FIELD unless we have an explicit AccessType to avoid unnecessary initializations
+				final Field field = findField( containerJavaType, propertyName );
+				return new EnhancedSetterImpl( containerJavaType, propertyName, field );
+			}
 		}
-		return AccessStrategyHelper.getAccessType( containerJavaType, propertyName );
+		// when classAccessType is null know PROPERTY is the explicit access type
+		final Method setterMethod = findSetterMethod( containerJavaType, propertyName, fieldType );
+		return new EnhancedSetterMethodImpl( containerJavaType, propertyName, setterMethod );
 	}
 
 	@Override
diff --git a/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessStrategyEnhancedImpl.java b/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessStrategyEnhancedImpl.java
@@ -29,18 +29,18 @@ public static PropertyAccessStrategyEnhancedImpl with(AccessType getterAccessTyp
 		};
 	}
 
-	private final @Nullable AccessType getterAccessType;
+	private final @Nullable AccessType classAccessType;
 
 	public static PropertyAccessStrategyEnhancedImpl STANDARD = new PropertyAccessStrategyEnhancedImpl( null );
 	public static PropertyAccessStrategyEnhancedImpl FIELD = new PropertyAccessStrategyEnhancedImpl( AccessType.FIELD );
 	public static PropertyAccessStrategyEnhancedImpl PROPERTY = new PropertyAccessStrategyEnhancedImpl( AccessType.PROPERTY );
 
-	public PropertyAccessStrategyEnhancedImpl(@Nullable AccessType getterAccessType) {
-		this.getterAccessType = getterAccessType;
+	public PropertyAccessStrategyEnhancedImpl(@Nullable AccessType classAccessType) {
+		this.classAccessType = classAccessType;
 	}
 
 	@Override
 	public PropertyAccess buildPropertyAccess(Class<?> containerJavaType, final String propertyName, boolean setterRequired) {
-		return new PropertyAccessEnhancedImpl( this, containerJavaType, propertyName, getterAccessType );
+		return new PropertyAccessEnhancedImpl( this, containerJavaType, propertyName, classAccessType );
 	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/property/access/spi/EnhancedGetterFieldImpl.java b/hibernate-core/src/main/java/org/hibernate/property/access/spi/EnhancedGetterFieldImpl.java
@@ -0,0 +1,41 @@
+/*
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.property.access.spi;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.hibernate.Internal;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+
+import static org.hibernate.internal.util.NullnessUtil.castNonNull;
+
+/**
+ * A specialized Getter implementation for handling getting values from
+ * a bytecode-enhanced Class. The reason we need specialized handling
+ * is to produce the correct {@link java.lang.reflect.Member} while
+ * using the {@link Field} to access values and ensure correct functionality.
+ *
+ * @author Steve Ebersole
+ * @author Luis Barreiro
+ */
+@Internal
+public class EnhancedGetterFieldImpl extends GetterFieldImpl {
+	public EnhancedGetterFieldImpl(Class<?> containerClass, String propertyName, Field field, Method getterMethod) {
+		super( containerClass, propertyName, field, getterMethod );
+		assert getterMethod != null;
+	}
+
+	@Override
+	public @NonNull Method getMethod() {
+		return castNonNull( super.getMethod() );
+	}
+
+	@Override
+	public Member getMember() {
+		return getMethod();
+	}
+}
diff --git a/hibernate-core/src/main/java/org/hibernate/property/access/spi/GetterFieldImpl.java b/hibernate-core/src/main/java/org/hibernate/property/access/spi/GetterFieldImpl.java
@@ -33,11 +33,14 @@ public class GetterFieldImpl implements Getter {
 	private final @Nullable Method getterMethod;
 
 	public GetterFieldImpl(Class<?> containerClass, String propertyName, Field field) {
+		this ( containerClass, propertyName, field, ReflectHelper.findGetterMethodForFieldAccess( field, propertyName ) );
+	}
+
+	GetterFieldImpl(Class<?> containerClass, String propertyName, Field field, Method getterMethod) {
 		this.containerClass = containerClass;
 		this.propertyName = propertyName;
 		this.field = field;
-
-		this.getterMethod = ReflectHelper.findGetterMethodForFieldAccess( field, propertyName );
+		this.getterMethod = getterMethod;
 	}
 
 	@Override
@@ -76,9 +79,13 @@ public Type getReturnType() {
 		return field.getGenericType();
 	}
 
+	public Field getField() {
+		return field;
+	}
+
 	@Override
 	public Member getMember() {
-		return field;
+		return getField();
 	}
 
 	@Override

Original file line number	Diff line number	Diff line change
`@@ -1199,8 +1199,8 @@ private static void findAccessors(`
`1199`	`1199`	`if ( getter instanceof GetterMethodImpl ) {`
`1200`	`1200`	`getterMember = getter.getMethod();`
`1201`	`1201`	`}`
`1202`		`- else if ( getter instanceof GetterFieldImpl ) {`
`1203`		`- getterMember = getter.getMember();`
	`1202`	`+ else if ( getter instanceof GetterFieldImpl getterField ) {`
	`1203`	`+ getterMember = getterField.getField();`
`1204`	`1204`	`}`
`1205`	`1205`	`else {`
`1206`	`1206`	`throw new InvalidPropertyAccessorException(`
Original file line number	Diff line number	Diff line change
`@@ -29,18 +29,18 @@ public static PropertyAccessStrategyEnhancedImpl with(AccessType getterAccessTyp`
`29`	`29`	`};`
`30`	`30`	`}`
`31`	`31`
`32`		`- private final @Nullable AccessType getterAccessType;`
	`32`	`+ private final @Nullable AccessType classAccessType;`
`33`	`33`
`34`	`34`	`public static PropertyAccessStrategyEnhancedImpl STANDARD = new PropertyAccessStrategyEnhancedImpl( null );`
`35`	`35`	`public static PropertyAccessStrategyEnhancedImpl FIELD = new PropertyAccessStrategyEnhancedImpl( AccessType.FIELD );`
`36`	`36`	`public static PropertyAccessStrategyEnhancedImpl PROPERTY = new PropertyAccessStrategyEnhancedImpl( AccessType.PROPERTY );`
`37`	`37`
`38`		`- public PropertyAccessStrategyEnhancedImpl(@Nullable AccessType getterAccessType) {`
`39`		`- this.getterAccessType = getterAccessType;`
	`38`	`+ public PropertyAccessStrategyEnhancedImpl(@Nullable AccessType classAccessType) {`
	`39`	`+ this.classAccessType = classAccessType;`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`@Override`
`43`	`43`	`public PropertyAccess buildPropertyAccess(Class<?> containerJavaType, final String propertyName, boolean setterRequired) {`
`44`		`- return new PropertyAccessEnhancedImpl( this, containerJavaType, propertyName, getterAccessType );`
	`44`	`+ return new PropertyAccessEnhancedImpl( this, containerJavaType, propertyName, classAccessType );`
`45`	`45`	`}`
`46`	`46`	`}`