[6.2] Package-private members access fixes for bytecode enhancement

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

@@ -532,8 +532,23 @@ String getDescriptor() {
 			return fieldDescription.getDescriptor();
 		}
 
-		boolean isVisibleTo(TypeDescription typeDescription) {
-			return fieldDescription.isVisibleTo( typeDescription );
+		boolean isVisibleTo(TypeDescription type) {
+			final var declaringType = fieldDescription.getDeclaringType().asErasure();
+			if ( declaringType.isVisibleTo( type ) ) {
+				if ( fieldDescription.isPublic() || type.equals( declaringType ) ) {
+					return true;
+				}
+				else if ( fieldDescription.isProtected() ) {
+					return declaringType.isAssignableFrom( type );
+				}
+				else if ( fieldDescription.isPrivate() ) {
+					return type.isNestMateOf( declaringType );
+				}
+				// We explicitly consider package-private fields as not visible, as the classes
+				// might have the same package name but be loaded by different class loaders.
+				// (see https://hibernate.atlassian.net/browse/HHH-19784)
+			}
+			return false;
 		}
 
 		FieldDescription getFieldDescription() {

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

@@ -15,7 +15,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;
 
@@ -73,6 +72,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(
@@ -235,9 +235,9 @@ public ReflectionOptimizer getReflectionOptimizer(
 			return null;
 		}
 
-		Class<?> superClass = determineAccessOptimizerSuperClass( clazz, getters, setters );
-
 		final String[] propertyNames = propertyAccessMap.keySet().toArray( new String[0] );
+		final Class<?> superClass = determineAccessOptimizerSuperClass( clazz, propertyNames, getters, setters );
+
 		final Class<?> bulkAccessor = byteBuddyState.load( clazz, byteBuddy -> byteBuddy
 				.with( new NamingStrategy.SuffixingRandom(
 						OPTIMIZER_PROXY_NAMING_SUFFIX,
@@ -264,104 +264,94 @@ 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;
 		}
 	}
 
-	private Class<?> determineAccessOptimizerSuperClass(Class<?> clazz, Member[] getters, Member[] setters) {
+	private Class<?> determineAccessOptimizerSuperClass(Class<?> clazz, String[] propertyNames, Member[] getters, Member[] setters) {
 		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];
-				if ( getter.getDeclaringClass() == foreignPackageClassInfo.clazz && !Modifier.isPublic( getter.getModifiers() ) ) {
-					foreignPackageClassInfo.getters.add( getter );
-				}
-				if ( setter.getDeclaringClass() == foreignPackageClassInfo.clazz && !Modifier.isPublic( setter.getModifiers() ) ) {
-					foreignPackageClassInfo.setters.add( setter );
-				}
-			}
-			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;
+
 			superClass = byteBuddyState.load(
-					foreignPackageClassInfo.clazz,
+					bridgeMembersClassInfo.clazz,
 					byteBuddy -> {
 						DynamicType.Builder<?> builder = byteBuddy.with(
 								new NamingStrategy.SuffixingRandom(
-										OPTIMIZER_PROXY_NAMING_SUFFIX,
+										OPTIMIZER_PROXY_BRIDGE_NAMING_SUFFIX,
 										new NamingStrategy.SuffixingRandom.BaseNameResolver.ForFixedValue(
-												foreignPackageClassInfo.clazz.getName() )
+												bridgeMembersClassInfo.clazz.getName() )
 								)
 						).subclass( newSuperClass );
-						for ( Member getter : foreignPackageClassInfo.getters ) {
-							final Class<?> getterType;
-							if ( getter instanceof Field ) {
-								getterType = ( (Field) getter ).getType();
+						for ( Member getter : bridgeMembersClassInfo.getters ) {
+							if ( !Modifier.isPublic( getter.getModifiers() ) ) {
+								final Class<?> getterType;
+								if ( getter instanceof Field ) {
+									getterType = ( (Field) getter ).getType();
+								}
+								else {
+									getterType = ( (Method) getter ).getReturnType();
+								}
+
+								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
+														)
+												)
+										);
 							}
-							else {
-								getterType = ( (Method) getter ).getReturnType();
-							}
-
-							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 ) {
-								setterType = ( (Field) setter ).getType();
-							}
-							else {
-								setterType = ( (Method) setter ).getParameterTypes()[0];
+						for ( Member setter : bridgeMembersClassInfo.setters ) {
+							if ( !Modifier.isPublic( setter.getModifiers() ) ) {
+								final Class<?> setterType;
+								if ( setter instanceof Field ) {
+									setterType = ( (Field) setter ).getType();
+								}
+								else {
+									setterType = ( (Method) setter ).getParameterTypes()[0];
+								}
+
+								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
+														)
+												)
+										);
 							}
-
-							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
-													)
-											)
-									);
 						}
 
 						return builder;
@@ -371,10 +361,10 @@ private Class<?> determineAccessOptimizerSuperClass(Class<?> clazz, Member[] get
 			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 );
 				}
 			}
@@ -562,16 +552,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,41 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.bytecode;
+
+import org.hibernate.orm.test.bytecode.foreignpackage.ConcreteEntity;
+
+import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner;
+import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.Jira;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@SessionFactory
+@DomainModel(annotatedClasses = {
+		ConcreteEntity.class,
+		SuperclassEntity.class
+})
+@Jira("https://hibernate.atlassian.net/browse/HHH-19369")
+@RunWith( BytecodeEnhancerRunner.class )
+public class ForeignPackageSuperclassAccessorTest extends BaseNonConfigCoreFunctionalTestCase {
+	@Override
+	protected Class[] getAnnotatedClasses() {
+		return new Class[]{
+				ConcreteEntity.class,
+				SuperclassEntity.class
+		};
+	}
+
+	@Test
+	public void test() {
+		inTransaction( session -> {
+			session.find( SuperclassEntity.class, 1L );
+		} );
+	}
+}

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

@@ -0,0 +1,30 @@
+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;
+	}
+}