From a1f68c825c3dd6b29dcdc1233ab2480345865bce Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 27 Aug 2025 10:59:57 +0200 Subject: [PATCH] HHH-19739 Don't unintentionally deduplicate attributes by property name --- .../bytebuddy/BytecodeProviderImpl.java | 46 +++--- .../bytecode/spi/BytecodeProvider.java | 71 +++++++++ .../EmbeddableRepresentationStrategyPojo.java | 20 ++- ...ityRepresentationStrategyPojoStandard.java | 43 ++++-- .../entity/AbstractEntityPersister.java | 41 +++--- .../inheritance/MultipleInheritanceTest.java | 135 ++++++++++++++++++ 6 files changed, 295 insertions(+), 61 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/MultipleInheritanceTest.java 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 index 6b763340bda2..7ecea7545e2d 100644 --- 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 @@ -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 propertyAccessMap) { + final PropertyInfo[] propertyInfos = new PropertyInfo[propertyAccessMap.size()]; + int i = 0; + for ( Map.Entry 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 propertyAccessMap, + PropertyInfo[] propertyInfos, + String[] propertyNames, Member[] getters, Member[] setters) { int i = 0; - for ( Map.Entry 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() ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/spi/BytecodeProvider.java b/hibernate-core/src/main/java/org/hibernate/bytecode/spi/BytecodeProvider.java index 077c9e5869d6..ce07bddd06e5 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/spi/BytecodeProvider.java +++ b/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 @@ public interface BytecodeProvider extends Service { * @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 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 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. * diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/EmbeddableRepresentationStrategyPojo.java b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/EmbeddableRepresentationStrategyPojo.java index ca2b12dcd69f..e78104079654 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/EmbeddableRepresentationStrategyPojo.java +++ b/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 propertyAccessMap = new LinkedHashMap<>(); + final List 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 diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/EntityRepresentationStrategyPojoStandard.java b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/EntityRepresentationStrategyPojoStandard.java index 8f90a387e401..b26060ed2489 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/EntityRepresentationStrategyPojoStandard.java +++ b/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 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 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 buildPropertyAccessMap(BytecodeProvider.PropertyInfo[] propertyInfos) { + final Map 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 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 diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 057d4d31ab08..bc9a1aa9050d 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/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 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(); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/MultipleInheritanceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/MultipleInheritanceTest.java new file mode 100644 index 000000000000..9ab5e8f410ba --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/MultipleInheritanceTest.java @@ -0,0 +1,135 @@ +/* + * 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 . + */ +package org.hibernate.orm.test.inheritance; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Column; +import jakarta.persistence.Embeddable; +import jakarta.persistence.EmbeddedId; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Inheritance; +import jakarta.persistence.InheritanceType; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.MappedSuperclass; +import jakarta.persistence.OneToMany; +import jakarta.persistence.OneToOne; + +import org.hibernate.annotations.NotFound; +import org.hibernate.annotations.NotFoundAction; + +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Test; + +import java.io.Serializable; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class MultipleInheritanceTest extends BaseCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { + CarOptional.class, + CarPart.class, + BasicCar.class, + SuperCar.class, + Car.class + }; + } + + @Test + public void test() { + inTransaction( session -> { + + Car car = new Car(); + CarPart carPart = new CarPart(); + + + CarPK id = new CarPK(); + id.carId1 = "1"; + carPart.id = id; + session.persist( carPart ); + + car.id = id; + car.parts = carPart; + ((BasicCar) car).parts = carPart; + session.persist( car ); + session.flush(); + session.clear(); + + Car loadedCar = session.find( Car.class, id ); + assertNotNull( loadedCar.parts ); + assertNotNull( ( (BasicCar) loadedCar ).parts ); + } ); + } + + @Embeddable + public static class CarPK implements Serializable { + @Column(name = "CAR_ID_1") + protected String carId1; + } + + @Entity(name = "BasicCar") + @Inheritance(strategy = InheritanceType.SINGLE_TABLE) + public static class BasicCar { + @EmbeddedId + protected CarPK id; + + @OneToOne + @JoinColumn(name = "CAR_ID_1", referencedColumnName = "CAR_ID_1", insertable = false, updatable = false) + CarPart parts; + } + + @Entity(name = "SuperCar") + public static class SuperCar extends BasicCar { + @OneToMany(fetch = FetchType.LAZY, cascade = CascadeType.ALL, orphanRemoval = true) +// @JoinColumn(name = "CAR_ID_1", referencedColumnName = "CAR_ID_1") + private List optionals; + } + + @MappedSuperclass + public static abstract class AbstractCar extends BasicCar { + @OneToOne + @NotFound(action = NotFoundAction.IGNORE) + @JoinColumn(name = "CAR_ID_1", referencedColumnName = "CAR_ID_1", insertable = false, updatable = false) + CarPart parts ; + } + + @Entity(name = "CarPart") + public static class CarPart { + @EmbeddedId + private CarPK id; + + String name; + } + + @Entity(name = "Car") + public static class Car extends AbstractCar { + + } + + + + @Entity(name = "CarOptional") + public static class CarOptional { + + @EmbeddedId + private CarOptionalPK id; + + private String name; + + @Embeddable + public class CarOptionalPK implements Serializable { + + @Column(name = "OPTIONAL_ID1") + private String id1; + + } + } +}