[6.2] HHH-19739 Don't unintentionally deduplicate attributes by property name

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

@@ -164,7 +164,7 @@ public ReflectionOptimizer getReflectionOptimizer(
 		final Method[] getters = new Method[getterNames.length];
 		final Method[] setters = new Method[setterNames.length];
 		try {
-			findAccessors( clazz, getterNames, setterNames, types, getters, setters );
+			unwrapPropertyInfos( clazz, getterNames, setterNames, types, getters, setters );
 		}
 		catch (InvalidPropertyAccessorException ex) {
 			LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
@@ -199,6 +199,18 @@ public ReflectionOptimizer getReflectionOptimizer(
 
 	@Override
 	public @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, Map<String, PropertyAccess> propertyAccessMap) {
+		final PropertyInfo[] propertyInfos = new PropertyInfo[propertyAccessMap.size()];
+		int i = 0;
+		for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
+			final String propertyName = entry.getKey();
+			final PropertyAccess propertyAccess = entry.getValue();
+			propertyInfos[i++] = new PropertyInfo( propertyName, propertyAccess );
+		}
+		return getReflectionOptimizer( clazz, propertyInfos );
+	}
+
+	@Override
+	public @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, PropertyInfo[] propertyInfos) {
 		final Class<?> fastClass;
 		if ( !clazz.isInterface() && !Modifier.isAbstract( clazz.getModifiers() ) ) {
 			// we only provide a fast class instantiator if the class can be instantiated
@@ -225,19 +237,18 @@ public ReflectionOptimizer getReflectionOptimizer(
 			fastClass = null;
 		}
 
-		final Member[] getters = new Member[propertyAccessMap.size()];
-		final Member[] setters = new Member[propertyAccessMap.size()];
+		final Member[] getters = new Member[propertyInfos.length];
+		final Member[] setters = new Member[propertyInfos.length];
+		final String[] propertyNames = new String[propertyInfos.length];
 		try {
-			findAccessors( clazz, propertyAccessMap, getters, setters );
+			unwrapPropertyInfos( clazz, propertyInfos, propertyNames, getters, setters );
 		}
 		catch (InvalidPropertyAccessorException ex) {
 			LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
 			return null;
 		}
 
-		Class<?> superClass = determineAccessOptimizerSuperClass( clazz, getters, setters );
-
-		final String[] propertyNames = propertyAccessMap.keySet().toArray( new String[0] );
+		final Class<?> superClass = determineAccessOptimizerSuperClass( clazz, getters, setters );
 		final Class<?> bulkAccessor = byteBuddyState.load( clazz, byteBuddy -> byteBuddy
 				.with( new NamingStrategy.SuffixingRandom(
 						OPTIMIZER_PROXY_NAMING_SUFFIX,
@@ -1148,7 +1159,7 @@ else if ( setterMember instanceof Field ) {
 		}
 	}
 
-	private static void findAccessors(
+	private static void unwrapPropertyInfos(
 			Class<?> clazz,
 			String[] getterNames,
 			String[] setterNames,
@@ -1190,14 +1201,17 @@ else if ( Modifier.isPrivate( setters[i].getModifiers() ) ) {
 		}
 	}
 
-	private static void findAccessors(
+	private static void unwrapPropertyInfos(
 			Class<?> clazz,
-			Map<String, PropertyAccess> propertyAccessMap,
+			PropertyInfo[] propertyInfos,
+			String[] propertyNames,
 			Member[] getters,
 			Member[] setters) {
 		int i = 0;
-		for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
-			final PropertyAccess propertyAccess = entry.getValue();
+		for ( PropertyInfo propertyInfo : propertyInfos ) {
+			final String propertyName = propertyInfo.propertyName();
+			final PropertyAccess propertyAccess = propertyInfo.propertyAccess();
+			propertyNames[i] = propertyName;
 			if ( propertyAccess instanceof PropertyAccessEmbeddedImpl ) {
 				getters[i] = EMBEDDED_MEMBER;
 				setters[i] = EMBEDDED_MEMBER;
@@ -1206,14 +1220,14 @@ private static void findAccessors(
 			}
 			final Getter getter = propertyAccess.getGetter();
 			if ( getter == null ) {
-				throw new InvalidPropertyAccessorException( "invalid getter for property [" + entry.getKey() + "]" );
+				throw new InvalidPropertyAccessorException( "invalid getter for property [" + propertyName + "]" );
 			}
 			final Setter setter = propertyAccess.getSetter();
 			if ( setter == null ) {
 				throw new InvalidPropertyAccessorException(
 						String.format(
 								"cannot find a setter for [%s] on type [%s]",
-								entry.getKey(),
+								propertyName,
 								clazz.getName()
 						)
 				);
@@ -1229,7 +1243,7 @@ else if ( getter instanceof GetterFieldImpl ) {
 				throw new InvalidPropertyAccessorException(
 						String.format(
 								"cannot find a getter for [%s] on type [%s]",
-								entry.getKey(),
+								propertyName,
 								clazz.getName()
 						)
 				);
@@ -1248,7 +1262,7 @@ else if ( setter instanceof SetterFieldImpl ) {
 				throw new InvalidPropertyAccessorException(
 						String.format(
 								"cannot find a setter for [%s] on type [%s]",
-								entry.getKey(),
+								propertyName,
 								clazz.getName()
 						)
 				);

hibernate-core/src/main/java/org/hibernate/bytecode/spi/BytecodeProvider.java

@@ -6,7 +6,9 @@
  */
 package org.hibernate.bytecode.spi;
 
+import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import org.hibernate.bytecode.enhance.spi.EnhancementContext;
 import org.hibernate.bytecode.enhance.spi.Enhancer;
@@ -57,9 +59,78 @@
 	 * @param clazz The class to be reflected upon.
 	 * @param propertyAccessMap The ordered property access map
 	 * @return The reflection optimization delegate.
+	 * @deprecated Use {@link #getReflectionOptimizer(Class, PropertyInfo[])} instead
 	 */
+	@Deprecated(forRemoval = true)
 	@Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, Map<String, PropertyAccess> propertyAccessMap);
 
+	/**
+	 * Retrieve the ReflectionOptimizer delegate for this provider
+	 * capable of generating reflection optimization components.
+	 *
+	 * @param clazz The class to be reflected upon.
+	 * @param propertyInfos The ordered property infos
+	 * @return The reflection optimization delegate.
+	 */
+	default @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, PropertyInfo[] propertyInfos) {
+		final Map<String, PropertyAccess> map = new HashMap<>();
+		for ( int i = 0; i < propertyInfos.length; i++ ) {
+			map.put( propertyInfos[i].propertyName(), propertyInfos[i].propertyAccess() );
+		}
+		return getReflectionOptimizer( clazz, map );
+	}
+
+	/**
+	 * Information about a property of a class, needed for generating reflection optimizers.
+	 *
+	 */
+	public static final class PropertyInfo {
+		private final String propertyName;
+		private final PropertyAccess propertyAccess;
+
+		/**
+		 * @param propertyName The name of the property
+		 * @param propertyAccess The property access
+		 */
+		public PropertyInfo(String propertyName, PropertyAccess propertyAccess) {
+			this.propertyName = propertyName;
+			this.propertyAccess = propertyAccess;
+		}
+
+		public String propertyName() {
+			return propertyName;
+		}
+
+		public PropertyAccess propertyAccess() {
+			return propertyAccess;
+		}
+
+		@Override
+		public boolean equals(@Nullable Object obj) {
+			if ( obj == this ) {
+				return true;
+			}
+			if ( obj == null || obj.getClass() != this.getClass() ) {
+				return false;
+			}
+			var that = (PropertyInfo) obj;
+			return Objects.equals( this.propertyName, that.propertyName ) &&
+					Objects.equals( this.propertyAccess, that.propertyAccess );
+		}
+
+		@Override
+		public int hashCode() {
+			return Objects.hash( propertyName, propertyAccess );
+		}
+
+		@Override
+		public String toString() {
+			return "PropertyInfo[" +
+					"propertyName=" + propertyName + ", " +
+					"propertyAccess=" + propertyAccess + ']';
+		}
+	}
+
 	/**
 	 * Returns a byte code enhancer that implements the enhancements described in the supplied enhancement context.
 	 *

hibernate-core/src/main/java/org/hibernate/metamodel/internal/EmbeddableRepresentationStrategyPojo.java

@@ -6,7 +6,7 @@
  */
 package org.hibernate.metamodel.internal;
 
-import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.function.Supplier;
@@ -179,19 +179,17 @@ private ReflectionOptimizer buildReflectionOptimizer(
 			return null;
 		}
 
-		final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
+		final List<Property> properties = bootDescriptor.getProperties();
+			final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
 
-		int i = 0;
-		for ( Property property : bootDescriptor.getProperties() ) {
-			propertyAccessMap.put( property.getName(), getPropertyAccesses()[i] );
-			i++;
+		for ( int i = 0; i < properties.size(); i++ ) {
+		final Property property = properties.get( i );
+			propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), getPropertyAccesses()[i] );
 		}
-		final BytecodeProvider bytecodeProvider = creationContext.getServiceRegistry().getService( BytecodeProvider.class );
 
-		return bytecodeProvider.getReflectionOptimizer(
-				bootDescriptor.getComponentClass(),
-				propertyAccessMap
-		);
+		return creationContext.getServiceRegistry()
+				.requireService( BytecodeProvider.class )
+				.getReflectionOptimizer( bootDescriptor.getComponentClass(), propertyInfos );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/internal/EntityRepresentationStrategyPojoStandard.java

@@ -7,7 +7,8 @@
 package org.hibernate.metamodel.internal;
 
 import java.lang.reflect.Method;
-import java.util.LinkedHashMap;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -72,6 +73,7 @@ public class EntityRepresentationStrategyPojoStandard implements EntityRepresent
 
 	private final String identifierPropertyName;
 	private final PropertyAccess identifierPropertyAccess;
+	private final BytecodeProvider.PropertyInfo[] propertyInfos;
 	private final Map<String, PropertyAccess> propertyAccessMap;
 	private final EmbeddableRepresentationStrategyPojo mapsIdRepresentationStrategy;
 
@@ -139,8 +141,9 @@ else if ( bootDescriptorIdentifier != null ) {
 			identifierPropertyAccess = makePropertyAccess( identifierProperty );
 		}
 
-		final BytecodeProvider bytecodeProvider = creationContext.getBootstrapContext().getServiceRegistry().getService( BytecodeProvider.class );
+		this.strategySelector = creationContext.getServiceRegistry().getService( StrategySelector.class );
 
+		final BytecodeProvider bytecodeProvider = creationContext.getBootstrapContext().getServiceRegistry().requireService( BytecodeProvider.class );
 		final EntityMetamodel entityMetamodel = runtimeDescriptor.getEntityMetamodel();
 		ProxyFactory proxyFactory = null;
 		if ( proxyJtd != null && entityMetamodel.isLazy() ) {
@@ -151,16 +154,29 @@ else if ( bootDescriptorIdentifier != null ) {
 		}
 		this.proxyFactory = proxyFactory;
 
-		// resolveReflectionOptimizer may lead to a makePropertyAccess call which requires strategySelector
-		this.strategySelector = creationContext.getServiceRegistry().getService( StrategySelector.class );
-		final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
-		for ( Property property : bootDescriptor.getPropertyClosure() ) {
-			propertyAccessMap.put( property.getName(), makePropertyAccess( property ) );
+		propertyInfos = buildPropertyInfos( bootDescriptor );
+		propertyAccessMap = buildPropertyAccessMap( propertyInfos );
+		reflectionOptimizer = resolveReflectionOptimizer( bytecodeProvider );
+
+		this.instantiator = determineInstantiator( bootDescriptor, runtimeDescriptor.getEntityMetamodel() );
+	}
+
+	private Map<String, PropertyAccess> buildPropertyAccessMap(BytecodeProvider.PropertyInfo[] propertyInfos) {
+		final Map<String, PropertyAccess> map = new HashMap<>( propertyInfos.length );
+		for ( BytecodeProvider.PropertyInfo propertyInfo : propertyInfos ) {
+			map.put( propertyInfo.propertyName(), propertyInfo.propertyAccess() );
 		}
-		this.propertyAccessMap = propertyAccessMap;
-		this.reflectionOptimizer = resolveReflectionOptimizer( bytecodeProvider );
+		return map;
+	}
 
-		this.instantiator = determineInstantiator( bootDescriptor, entityMetamodel );
+	private BytecodeProvider.PropertyInfo[] buildPropertyInfos(PersistentClass bootDescriptor) {
+		final List<Property> properties = bootDescriptor.getPropertyClosure();
+		final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
+		for ( int i = 0; i < properties.size(); i++ ) {
+			final Property property = properties.get( i );
+			propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), makePropertyAccess( property ) );
+		}
+		return propertyInfos;
 	}
 
 	private EntityInstantiator determineInstantiator(PersistentClass bootDescriptor, EntityMetamodel entityMetamodel) {
@@ -292,12 +308,13 @@ private ReflectionOptimizer resolveReflectionOptimizer(BytecodeProvider bytecode
 		}
 		return bytecodeProvider.getReflectionOptimizer(
 				mappedJtd.getJavaTypeClass(),
-				propertyAccessMap
+				propertyInfos
 		);
 	}
 
 	private PropertyAccess makePropertyAccess(Property bootAttributeDescriptor) {
-		PropertyAccessStrategy strategy = bootAttributeDescriptor.getPropertyAccessStrategy( mappedJtd.getJavaTypeClass() );
+		final Class<?> mappedClass = bootAttributeDescriptor.getPersistentClass().getMappedClass();
+		PropertyAccessStrategy strategy = bootAttributeDescriptor.getPropertyAccessStrategy( mappedClass );
 
 		if ( strategy == null ) {
 			final String propertyAccessorName = bootAttributeDescriptor.getPropertyAccessorName();
@@ -336,7 +353,7 @@ else if ( bootAttributeDescriptor instanceof IndexBackref ) {
 			);
 		}
 
-		return strategy.buildPropertyAccess( mappedJtd.getJavaTypeClass(), bootAttributeDescriptor.getName(), true );
+		return strategy.buildPropertyAccess( mappedClass, bootAttributeDescriptor.getName(), true );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -4798,28 +4798,27 @@ public void prepareMappingModel(MappingModelCreationProcess creationProcess) {
 		final NonIdentifierAttribute[] properties = currentEntityMetamodel.getProperties();
 		AttributeMappingsMap.Builder mappingsBuilder = AttributeMappingsMap.builder();
 		int fetchableIndex = getFetchableIndexOffset();
-		for ( int i = 0; i < currentEntityMetamodel.getPropertySpan(); i++ ) {
-			final NonIdentifierAttribute runtimeAttrDefinition = properties[i];
-			final Property bootProperty = bootEntityDescriptor.getProperty( runtimeAttrDefinition.getName() );
-
-			if ( superMappingType == null
-					|| superMappingType.findAttributeMapping( bootProperty.getName() ) == null ) {
-				mappingsBuilder.put(
-						runtimeAttrDefinition.getName(),
-						generateNonIdAttributeMapping(
-								runtimeAttrDefinition,
-								bootProperty,
-								stateArrayPosition++,
-								fetchableIndex++,
-								creationProcess
-						)
-				);
-			}
-			declaredAttributeMappings = mappingsBuilder.build();
-//			else {
-				// its defined on the supertype, skip it here
-//			}
+		// For every property that is "owned" by this persistent class, create a declared attribute mapping
+		final List<Property> bootProperties = bootEntityDescriptor.getProperties();
+		// EntityMetamodel uses getPropertyClosure(), which includes the properties of the super type,
+		// so use that property span as offset when indexing into the EntityMetamodel NonIdentifierAttribute[]
+		final int superclassPropertiesOffset = superMappingType == null ? 0
+				: superMappingType.getEntityPersister().getEntityMetamodel().getPropertySpan();
+		for ( int i = 0; i < bootProperties.size(); i++ ) {
+			final Property bootProperty = bootProperties.get( i );
+			assert properties[superclassPropertiesOffset + i].getName().equals( bootProperty.getName() );
+			mappingsBuilder.put(
+					bootProperty.getName(),
+					generateNonIdAttributeMapping(
+							properties[superclassPropertiesOffset + i],
+							bootProperty,
+							stateArrayPosition++,
+							fetchableIndex++,
+							creationProcess
+					)
+			);
 		}
+		declaredAttributeMappings = mappingsBuilder.build();
 
 		getAttributeMappings();