HHH-19372 HHH-19369 Test and fix CCE and IAE with access optimizer and inheritance

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

@@ -14,7 +14,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
@@ -75,6 +74,7 @@ public class BytecodeProviderImpl implements BytecodeProvider {
 
 	private static final String INSTANTIATOR_PROXY_NAMING_SUFFIX = "HibernateInstantiator";
 	private static final String OPTIMIZER_PROXY_NAMING_SUFFIX = "HibernateAccessOptimizer";
+	private static final String OPTIMIZER_PROXY_BRIDGE_NAMING_SUFFIX = "HibernateAccessOptimizerBridge";
 	private static final ElementMatcher.Junction<NamedElement> newInstanceMethodName = ElementMatchers.named(
 			"newInstance" );
 	private static final ElementMatcher.Junction<NamedElement> getPropertyValuesMethodName = ElementMatchers.named(
@@ -280,13 +280,13 @@ public ReflectionOptimizer getReflectionOptimizer(
 		}
 	}
 
-	private static class ForeignPackageClassInfo {
+	private static class BridgeMembersClassInfo {
 		final Class<?> clazz;
 		final List<String> propertyNames = new ArrayList<>();
 		final List<Member> getters = new ArrayList<>();
 		final List<Member> setters = new ArrayList<>();
 
-		public ForeignPackageClassInfo(Class<?> clazz) {
+		public BridgeMembersClassInfo(Class<?> clazz) {
 			this.clazz = clazz;
 		}
 	}
@@ -295,99 +295,81 @@ private Class<?> determineAccessOptimizerSuperClass(Class<?> clazz, String[] pro
 		if ( clazz.isInterface() ) {
 			return Object.class;
 		}
-		// generate access optimizer super classes for foreign package super classes that declare fields
+		// generate access optimizer super classes for super classes that declare members requiring bridge methods
 		//  each should declare protected static methods get_FIELDNAME(OWNER)/set_FIELDNAME(OWNER, TYPE)
 		//  which should be called then from within GetPropertyValues/SetPropertyValues
 		//  Since these super classes will be in the correct package, the package-private entity field access is fine
-		final List<ForeignPackageClassInfo> foreignPackageClassInfos = createForeignPackageClassInfos( clazz );
-		for ( Iterator<ForeignPackageClassInfo> iterator = foreignPackageClassInfos.iterator(); iterator.hasNext(); ) {
-			final ForeignPackageClassInfo foreignPackageClassInfo = iterator.next();
-			for ( int i = 0; i < getters.length; i++ ) {
-				final Member getter = getters[i];
-				final Member setter = setters[i];
-				boolean found = false;
-				if ( getter.getDeclaringClass() == foreignPackageClassInfo.clazz && !Modifier.isPublic( getter.getModifiers() ) ) {
-					foreignPackageClassInfo.getters.add( getter );
-					found = true;
-				}
-				if ( setter.getDeclaringClass() == foreignPackageClassInfo.clazz && !Modifier.isPublic( setter.getModifiers() ) ) {
-					foreignPackageClassInfo.setters.add( setter );
-					found = true;
-				}
-				if ( found ) {
-					foreignPackageClassInfo.propertyNames.add( propertyNames[i] );
-				}
-			}
-			if ( foreignPackageClassInfo.getters.isEmpty() && foreignPackageClassInfo.setters.isEmpty() ) {
-				iterator.remove();
-			}
-		}
+		final List<BridgeMembersClassInfo> bridgeMembersClassInfos = createBridgeMembersClassInfos( clazz, getters, setters, propertyNames );
 
 		Class<?> superClass = Object.class;
-		for ( int i = foreignPackageClassInfos.size() - 1; i >= 0; i-- ) {
-			final ForeignPackageClassInfo foreignPackageClassInfo = foreignPackageClassInfos.get( i );
+		for ( int i = bridgeMembersClassInfos.size() - 1; i >= 0; i-- ) {
+			final BridgeMembersClassInfo bridgeMembersClassInfo = bridgeMembersClassInfos.get( i );
 			final Class<?> newSuperClass = superClass;
 
-			final String className = foreignPackageClassInfo.clazz.getName() + "$" + OPTIMIZER_PROXY_NAMING_SUFFIX + encodeName( foreignPackageClassInfo.propertyNames, foreignPackageClassInfo.getters, foreignPackageClassInfo.setters );
+			final String className = bridgeMembersClassInfo.clazz.getName() + "$" + OPTIMIZER_PROXY_BRIDGE_NAMING_SUFFIX + encodeName( bridgeMembersClassInfo.propertyNames, bridgeMembersClassInfo.getters, bridgeMembersClassInfo.setters );
 			superClass = byteBuddyState.load(
-					foreignPackageClassInfo.clazz,
+					bridgeMembersClassInfo.clazz,
 					className,
 					(byteBuddy, namingStrategy) -> {
 						DynamicType.Builder<?> builder = byteBuddy.with( namingStrategy ).subclass( newSuperClass );
-						for ( Member getter : foreignPackageClassInfo.getters ) {
-							final Class<?> getterType;
-							if ( getter instanceof Field field ) {
-								getterType = field.getType();
-							}
-							else if ( getter instanceof Method method ) {
-								getterType = method.getReturnType();
-							}
+						for ( Member getter : bridgeMembersClassInfo.getters ) {
+							if ( !Modifier.isPublic( getter.getModifiers() ) ) {
+								final Class<?> getterType;
+								if ( getter instanceof Field field ) {
+									getterType = field.getType();
+								}
+								else if ( getter instanceof Method method ) {
+									getterType = method.getReturnType();
+								}
 							else {
 								throw new AssertionFailure( "Unexpected member" + getter );
 							}
 
-							builder = builder.defineMethod(
-											"get_" + getter.getName(),
-											TypeDescription.Generic.OfNonGenericType.ForLoadedType.of(
-													getterType
-											),
-											Opcodes.ACC_PROTECTED | Opcodes.ACC_STATIC
-									)
-									.withParameter( foreignPackageClassInfo.clazz )
-									.intercept(
-											new Implementation.Simple(
-													new GetFieldOnArgument(
-															getter
-													)
-											)
-									);
-						}
-						for ( Member setter : foreignPackageClassInfo.setters ) {
-							final Class<?> setterType;
-							if ( setter instanceof Field field ) {
-								setterType = field.getType();
-							}
-							else if ( setter instanceof Method method ) {
-								setterType = method.getParameterTypes()[0];
+								builder = builder.defineMethod(
+												"get_" + getter.getName(),
+												TypeDescription.Generic.OfNonGenericType.ForLoadedType.of(
+														getterType
+												),
+												Opcodes.ACC_PROTECTED | Opcodes.ACC_STATIC
+										)
+										.withParameter( bridgeMembersClassInfo.clazz )
+										.intercept(
+												new Implementation.Simple(
+														new GetFieldOnArgument(
+																getter
+														)
+												)
+										);
 							}
+						}
+						for ( Member setter : bridgeMembersClassInfo.setters ) {
+							if ( !Modifier.isPublic( setter.getModifiers() ) ) {
+								final Class<?> setterType;
+								if ( setter instanceof Field field ) {
+									setterType = field.getType();
+								}
+								else if ( setter instanceof Method method ) {
+									setterType = method.getParameterTypes()[0];
+								}
 							else {
 								throw new AssertionFailure( "Unexpected member" + setter );
 							}
 
-							builder = builder.defineMethod(
-											"set_" + setter.getName(),
-											TypeDescription.Generic.VOID,
-											Opcodes.ACC_PROTECTED | Opcodes.ACC_STATIC
-									)
-									.withParameter( foreignPackageClassInfo.clazz )
-									.withParameter( setterType )
-									.intercept(
-											new Implementation.Simple(
-													new SetFieldOnArgument(
-															setter
-													)
-											)
-									);
+								builder = builder.defineMethod(
+												"set_" + setter.getName(),
+												TypeDescription.Generic.VOID,
+												Opcodes.ACC_PROTECTED | Opcodes.ACC_STATIC
+										)
+										.withParameter( bridgeMembersClassInfo.clazz )
+										.withParameter( setterType )
+										.intercept(
+												new Implementation.Simple(
+														new SetFieldOnArgument(
+																setter
+														)
+												)
+										);
+							}
 						}
 
 						return builder;
@@ -397,10 +379,10 @@ else if ( setter instanceof Method method ) {
 			for ( int j = 0; j < getters.length; j++ ) {
 				final Member getter = getters[j];
 				final Member setter = setters[j];
-				if ( foreignPackageClassInfo.getters.contains( getter ) ) {
+				if ( bridgeMembersClassInfo.getters.contains( getter ) && !Modifier.isPublic( getter.getModifiers() ) ) {
 					getters[j] = new ForeignPackageMember( superClass, getter );
 				}
-				if ( foreignPackageClassInfo.setters.contains( setter ) ) {
+				if ( bridgeMembersClassInfo.setters.contains( setter ) && !Modifier.isPublic( setter.getModifiers() ) ) {
 					setters[j] = new ForeignPackageMember( superClass, setter );
 				}
 			}
@@ -622,16 +604,31 @@ private boolean is64BitType(Class<?> type) {
 		}
 	}
 
-	private List<ForeignPackageClassInfo> createForeignPackageClassInfos(Class<?> clazz) {
-		final List<ForeignPackageClassInfo> foreignPackageClassInfos = new ArrayList<>();
+	private List<BridgeMembersClassInfo> createBridgeMembersClassInfos(
+			Class<?> clazz,
+			Member[] getters,
+			Member[] setters,
+			String[] propertyNames) {
+		final List<BridgeMembersClassInfo> bridgeMembersClassInfos = new ArrayList<>();
 		Class<?> c = clazz.getSuperclass();
 		while (c != Object.class) {
-			if ( !c.getPackageName().equals( clazz.getPackageName() ) ) {
-				foreignPackageClassInfos.add( new ForeignPackageClassInfo( c ) );
+			final BridgeMembersClassInfo bridgeMemberClassInfo = new BridgeMembersClassInfo( c );
+			for ( int i = 0; i < getters.length; i++ ) {
+				final Member getter = getters[i];
+				final Member setter = setters[i];
+				if ( getter.getDeclaringClass() == c && !Modifier.isPublic( getter.getModifiers() )
+						|| setter.getDeclaringClass() == c && !Modifier.isPublic( setter.getModifiers() ) ) {
+					bridgeMemberClassInfo.getters.add( getter );
+					bridgeMemberClassInfo.setters.add( setter );
+					bridgeMemberClassInfo.propertyNames.add( propertyNames[i] );
+				}
+			}
+			if ( !bridgeMemberClassInfo.propertyNames.isEmpty() ) {
+				bridgeMembersClassInfos.add( bridgeMemberClassInfo );
 			}
 			c = c.getSuperclass();
 		}
-		return foreignPackageClassInfos;
+		return bridgeMembersClassInfos;
 	}
 
 	public ByteBuddyProxyHelper getByteBuddyProxyHelper() {

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/ForeignPackageSuperclassAccessorTest.java

@@ -0,0 +1,29 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.bytecode;
+
+import org.hibernate.orm.test.bytecode.foreignpackage.ConcreteEntity;
+
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.Jira;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Test;
+
+@SessionFactory
+@DomainModel(annotatedClasses = {
+		ConcreteEntity.class,
+		SuperclassEntity.class
+})
+@Jira("https://hibernate.atlassian.net/browse/HHH-19369")
+public class ForeignPackageSuperclassAccessorTest {
+
+	@Test
+	public void test(SessionFactoryScope scope) {
+		scope.inTransaction( session -> {
+			session.find( SuperclassEntity.class, 1L );
+		} );
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/SuperclassEntity.java

@@ -0,0 +1,34 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.bytecode;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Inheritance;
+import jakarta.persistence.InheritanceType;
+
+@Entity
+@Inheritance(strategy = InheritanceType.JOINED)
+public class SuperclassEntity {
+	@Id
+	protected long id;
+	protected String name;
+
+	public long getId() {
+		return id;
+	}
+
+	public void setId(long id) {
+		this.id = id;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		this.name = name;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/optimizer/AncestorEntity.java

@@ -0,0 +1,35 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.bytecode.enhancement.optimizer;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Inheritance;
+import jakarta.persistence.InheritanceType;
+import org.hibernate.orm.test.bytecode.enhancement.optimizer.parent.Ancestor;
+
+@Entity(name = "AncestorEntity")
+@Inheritance(strategy = InheritanceType.JOINED)
+public class AncestorEntity extends Ancestor {
+
+	private Long id;
+
+	private String field;
+
+	public Long getId() {
+		return id;
+	}
+
+	public void setId(Long id) {
+		this.id = id;
+	}
+
+	public String getField() {
+		return field;
+	}
+
+	public void setField(String field) {
+		this.field = field;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/optimizer/ChildEntity3.java

@@ -0,0 +1,20 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.bytecode.enhancement.optimizer;
+
+import jakarta.persistence.Entity;
+
+@Entity(name = "ChildEntity3")
+public class ChildEntity3 extends AncestorEntity {
+	private String childField;
+
+	public String getChildField() {
+		return childField;
+	}
+
+	public void setChieldField(String childField) {
+		this.childField = childField;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/optimizer/ChildEntity4.java

@@ -0,0 +1,20 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.bytecode.enhancement.optimizer;
+
+import jakarta.persistence.Entity;
+
+@Entity(name = "ChildEntity4")
+public class ChildEntity4 extends AncestorEntity {
+	private String childField;
+
+	public String getChildField() {
+		return childField;
+	}
+
+	public void setChieldField(String childField) {
+		this.childField = childField;
+	}
+}