HHH-17418 Change field reader for private final fields

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

@@ -54,55 +54,64 @@ abstract class FieldReaderAppender implements ByteCodeAppender {
 		TypeDescription dispatcherType = persistentFieldAsDefined.getType().isPrimitive()
 				? persistentFieldAsDefined.getType().asErasure()
 				: TypeDescription.OBJECT;
-		// if ( this.$$_hibernate_getInterceptor() != null )
+		// From `PersistentAttributeTransformer`:
-		methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
+		//     Final fields will only be written to from the constructor,
-		methodVisitor.visitMethodInsn(
+		//     so there's no point trying to replace final field writes with a method call.
-				Opcodes.INVOKEVIRTUAL,
+		// as a result if a field is final then there will be no write method, and we don't want to have this block:
-				managedCtClass.getInternalName(),
+		if ( !persistentField.asDefined().isFinal() ) {
-				EnhancerConstants.INTERCEPTOR_GETTER_NAME,
+			// if ( this.$$_hibernate_getInterceptor() != null )
-				Type.getMethodDescriptor( Type.getType( PersistentAttributeInterceptor.class ) ),
+			methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
-				false
+			methodVisitor.visitMethodInsn(
-		);
+					Opcodes.INVOKEVIRTUAL,
-		Label skip = new Label();
+					managedCtClass.getInternalName(),
-		methodVisitor.visitJumpInsn( Opcodes.IFNULL, skip );
+					EnhancerConstants.INTERCEPTOR_GETTER_NAME,
-		// this (for field write)
+					Type.getMethodDescriptor( Type.getType( PersistentAttributeInterceptor.class ) ),
-		methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
+					false
-		// this.$$_hibernate_getInterceptor();
+			);
-		methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
+			Label skip = new Label();
-		methodVisitor.visitMethodInsn(
+			methodVisitor.visitJumpInsn( Opcodes.IFNULL, skip );
-				Opcodes.INVOKEVIRTUAL,
+			// this (for field write)
-				managedCtClass.getInternalName(),
+			methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
-				EnhancerConstants.INTERCEPTOR_GETTER_NAME,
+			// this.$$_hibernate_getInterceptor();
-				Type.getMethodDescriptor( Type.getType( PersistentAttributeInterceptor.class ) ),
+			methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
-				false
+			methodVisitor.visitMethodInsn(
-		);
+					Opcodes.INVOKEVIRTUAL,
-		// .readXXX( self, fieldName, field );
+					managedCtClass.getInternalName(),
-		methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
+					EnhancerConstants.INTERCEPTOR_GETTER_NAME,
-		methodVisitor.visitLdcInsn( persistentFieldAsDefined.getName() );
+					Type.getMethodDescriptor( Type.getType( PersistentAttributeInterceptor.class ) ),
-		methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
+					false
-		fieldRead( methodVisitor );
+			);
-		methodVisitor.visitMethodInsn(
+			// .readXXX( self, fieldName, field );
-				Opcodes.INVOKEINTERFACE,
+			methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
-				Type.getInternalName( PersistentAttributeInterceptor.class ),
+			methodVisitor.visitLdcInsn( persistentFieldAsDefined.getName() );
-				"read" + EnhancerImpl.capitalize( dispatcherType.getSimpleName() ),
+			methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
-				Type.getMethodDescriptor(
+
-						Type.getType( dispatcherType.getDescriptor() ),
+			fieldRead( methodVisitor );
-						Type.getType( Object.class ),
+			methodVisitor.visitMethodInsn(
-						Type.getType( String.class ),
+					Opcodes.INVOKEINTERFACE,
-						Type.getType( dispatcherType.getDescriptor() )
+					Type.getInternalName( PersistentAttributeInterceptor.class ),
-				),
+					"read" + EnhancerImpl.capitalize( dispatcherType.getSimpleName() ),
-				true
+					Type.getMethodDescriptor(
-		);
+							Type.getType( dispatcherType.getDescriptor() ),
-		// field = (cast) XXX
+							Type.getType( Object.class ),
-		if ( !dispatcherType.isPrimitive() ) {
+							Type.getType( String.class ),
-			methodVisitor.visitTypeInsn( Opcodes.CHECKCAST, persistentFieldAsDefined.getType().asErasure().getInternalName() );
+							Type.getType( dispatcherType.getDescriptor() )
-		}
+					),
-		fieldWrite( methodVisitor );
+					true
-		// end if
+			);
-		methodVisitor.visitLabel( skip );
+			// field = (cast) XXX
-		if ( implementationContext.getClassFileVersion().isAtLeast( ClassFileVersion.JAVA_V6 ) ) {
+			if ( !dispatcherType.isPrimitive() ) {
-			methodVisitor.visitFrame( Opcodes.F_SAME, 0, null, 0, null );
+				methodVisitor.visitTypeInsn(
+						Opcodes.CHECKCAST, persistentFieldAsDefined.getType().asErasure().getInternalName() );
+			}
+			fieldWrite( methodVisitor );
+			// end if
+			methodVisitor.visitLabel( skip );
+			if ( implementationContext.getClassFileVersion().isAtLeast( ClassFileVersion.JAVA_V6 ) ) {
+				methodVisitor.visitFrame( Opcodes.F_SAME, 0, null, 0, null );
+			}
 		}
+
 		// return field
 		methodVisitor.visitVarInsn( Opcodes.ALOAD, 0 );
 		fieldRead( methodVisitor );

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/superclass/MappedSuperclassTest.java

@@ -0,0 +1,74 @@
+package org.hibernate.orm.test.bytecode.enhancement.superclass;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+
+import java.time.LocalDateTime;
+
+import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.orm.junit.JiraKey;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import jakarta.persistence.Column;
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.MappedSuperclass;
+
+@JiraKey("HHH-17418")
+@RunWith(BytecodeEnhancerRunner.class)
+public class MappedSuperclassTest extends BaseCoreFunctionalTestCase {
+	private static final LocalDateTime TEST_DATE_UPDATED_VALUE = LocalDateTime.of( 2023, 11, 10, 0, 0 );
+	private static final long TEST_ID = 1L;
+
+	@Override
+	public Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] { MyEntity.class };
+	}
+
+	@Before
+	public void prepare() {
+		doInHibernate( this::sessionFactory, s -> {
+			MyEntity testEntity = new MyEntity();
+			testEntity.id = TEST_ID;
+			s.persist( testEntity );
+		} );
+	}
+
+	@Test
+	public void test() {
+		doInHibernate( this::sessionFactory, s -> {
+			MyEntity testEntity = s.get( MyEntity.class, TEST_ID );
+			assertThat( testEntity.value() ).isEqualTo( TEST_DATE_UPDATED_VALUE );
+		} );
+	}
+
+	@After
+	public void cleanup() {
+		doInHibernate( this::sessionFactory, s -> {
+			MyEntity testEntity = s.get( MyEntity.class, TEST_ID );
+			s.remove( testEntity );
+		} );
+	}
+
+
+	@MappedSuperclass
+	public static class MappedBase {
+		// field is private on purpose so that enhancer will not use field access
+		@Column
+		private final LocalDateTime updated = TEST_DATE_UPDATED_VALUE;
+
+		public LocalDateTime value() {
+			return updated;
+		}
+	}
+
+	@Entity
+	public static class MyEntity extends MappedBase {
+		@Id
+		Long id;
+	}
+}