Skip to content

Commit 85fe15b

Browse files
beikovbeikov
committed
HHH-19739 Don't unintentionally deduplicate attributes by property name
1 parent cef7452 commit 85fe15b

File tree

6 files changed

+238
-49
lines changed

6 files changed

+238
-49
lines changed

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

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public ReflectionOptimizer getReflectionOptimizer(
144144
final Method[] getters = new Method[getterNames.length];
145145
final Method[] setters = new Method[setterNames.length];
146146
try {
147-
findAccessors( clazz, getterNames, setterNames, types, getters, setters );
147+
unwrapPropertyInfos( clazz, getterNames, setterNames, types, getters, setters );
148148
}
149149
catch (InvalidPropertyAccessorException ex) {
150150
LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
@@ -179,6 +179,18 @@ public ReflectionOptimizer getReflectionOptimizer(
179179

180180
@Override
181181
public @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, Map<String, PropertyAccess> propertyAccessMap) {
182+
final PropertyInfo[] propertyInfos = new PropertyInfo[propertyAccessMap.size()];
183+
int i = 0;
184+
for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
185+
final String propertyName = entry.getKey();
186+
final PropertyAccess propertyAccess = entry.getValue();
187+
propertyInfos[i++] = new PropertyInfo( propertyName, propertyAccess );
188+
}
189+
return getReflectionOptimizer( clazz, propertyInfos );
190+
}
191+
192+
@Override
193+
public @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, PropertyInfo[] propertyInfos) {
182194
final Class<?> fastClass;
183195
if ( !clazz.isInterface() && !Modifier.isAbstract( clazz.getModifiers() ) ) {
184196
// we only provide a fast class instantiator if the class can be instantiated
@@ -203,17 +215,17 @@ public ReflectionOptimizer getReflectionOptimizer(
203215
fastClass = null;
204216
}
205217

206-
final Member[] getters = new Member[propertyAccessMap.size()];
207-
final Member[] setters = new Member[propertyAccessMap.size()];
218+
final Member[] getters = new Member[propertyInfos.length];
219+
final Member[] setters = new Member[propertyInfos.length];
220+
final String[] propertyNames = new String[propertyInfos.length];
208221
try {
209-
findAccessors( clazz, propertyAccessMap, getters, setters );
222+
unwrapPropertyInfos( clazz, propertyInfos, propertyNames, getters, setters );
210223
}
211224
catch (InvalidPropertyAccessorException ex) {
212225
LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
213226
return null;
214227
}
215228

216-
final String[] propertyNames = propertyAccessMap.keySet().toArray( new String[0] );
217229
final Class<?> superClass = determineAccessOptimizerSuperClass( clazz, propertyNames, getters, setters );
218230

219231
final String className = clazz.getName() + "$" + OPTIMIZER_PROXY_NAMING_SUFFIX + NameEncodeHelper.encodeName( propertyNames, getters, setters );
@@ -391,7 +403,7 @@ public ByteBuddyProxyHelper getByteBuddyProxyHelper() {
391403
return byteBuddyProxyHelper;
392404
}
393405

394-
private static void findAccessors(
406+
private static void unwrapPropertyInfos(
395407
Class<?> clazz,
396408
String[] getterNames,
397409
String[] setterNames,
@@ -432,14 +444,17 @@ else if ( Modifier.isPrivate( setters[i].getModifiers() ) ) {
432444
}
433445
}
434446

435-
private static void findAccessors(
447+
private static void unwrapPropertyInfos(
436448
Class<?> clazz,
437-
Map<String, PropertyAccess> propertyAccessMap,
449+
PropertyInfo[] propertyInfos,
450+
String[] propertyNames,
438451
Member[] getters,
439452
Member[] setters) {
440453
int i = 0;
441-
for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
442-
final PropertyAccess propertyAccess = entry.getValue();
454+
for ( PropertyInfo propertyInfo : propertyInfos ) {
455+
final String propertyName = propertyInfo.propertyName();
456+
final PropertyAccess propertyAccess = propertyInfo.propertyAccess();
457+
propertyNames[i] = propertyName;
443458
if ( propertyAccess instanceof PropertyAccessEmbeddedImpl ) {
444459
getters[i] = EMBEDDED_MEMBER;
445460
setters[i] = EMBEDDED_MEMBER;
@@ -448,14 +463,14 @@ private static void findAccessors(
448463
}
449464
final Getter getter = propertyAccess.getGetter();
450465
if ( getter == null ) {
451-
throw new InvalidPropertyAccessorException( "invalid getter for property [" + entry.getKey() + "]" );
466+
throw new InvalidPropertyAccessorException( "invalid getter for property [" + propertyName + "]" );
452467
}
453468
final Setter setter = propertyAccess.getSetter();
454469
if ( setter == null ) {
455470
throw new InvalidPropertyAccessorException(
456471
String.format(
457472
"cannot find a setter for [%s] on type [%s]",
458-
entry.getKey(),
473+
propertyName,
459474
clazz.getName()
460475
)
461476
);
@@ -471,7 +486,7 @@ else if ( getter instanceof GetterFieldImpl getterField ) {
471486
throw new InvalidPropertyAccessorException(
472487
String.format(
473488
"cannot find a getter for [%s] on type [%s]",
474-
entry.getKey(),
489+
propertyName,
475490
clazz.getName()
476491
)
477492
);
@@ -490,7 +505,7 @@ else if ( setter instanceof SetterFieldImpl setterField ) {
490505
throw new InvalidPropertyAccessorException(
491506
String.format(
492507
"cannot find a setter for [%s] on type [%s]",
493-
entry.getKey(),
508+
propertyName,
494509
clazz.getName()
495510
)
496511
);

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55
package org.hibernate.bytecode.spi;
66

7+
import java.util.HashMap;
78
import java.util.Map;
89

910
import org.hibernate.bytecode.enhance.spi.EnhancementContext;
@@ -55,9 +56,35 @@ public interface BytecodeProvider extends Service {
5556
* @param clazz The class to be reflected upon.
5657
* @param propertyAccessMap The ordered property access map
5758
* @return The reflection optimization delegate.
59+
* @deprecated Use {@link #getReflectionOptimizer(Class, PropertyInfo[])} instead
5860
*/
61+
@Deprecated(forRemoval = true)
5962
@Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, Map<String, PropertyAccess> propertyAccessMap);
6063

64+
/**
65+
* Retrieve the ReflectionOptimizer delegate for this provider
66+
* capable of generating reflection optimization components.
67+
*
68+
* @param clazz The class to be reflected upon.
69+
* @param propertyInfos The ordered property infos
70+
* @return The reflection optimization delegate.
71+
*/
72+
default @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, PropertyInfo[] propertyInfos) {
73+
final Map<String, PropertyAccess> map = new HashMap<>();
74+
for ( int i = 0; i < propertyInfos.length; i++ ) {
75+
map.put( propertyInfos[i].propertyName(), propertyInfos[i].propertyAccess() );
76+
}
77+
return getReflectionOptimizer( clazz, map );
78+
}
79+
80+
/**
81+
* Information about a property of a class, needed for generating reflection optimizers.
82+
*
83+
* @param propertyName The name of the property
84+
* @param propertyAccess The property access
85+
*/
86+
record PropertyInfo(String propertyName, PropertyAccess propertyAccess) {}
87+
6188
/**
6289
* Returns a byte code enhancer that implements the enhancements described in the supplied enhancement context.
6390
*

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import java.util.Collection;
88
import java.util.HashMap;
9-
import java.util.LinkedHashMap;
9+
import java.util.List;
1010
import java.util.Locale;
1111
import java.util.Map;
1212
import java.util.function.Supplier;
@@ -197,15 +197,15 @@ private static ReflectionOptimizer buildReflectionOptimizer(
197197
&& bootDescriptor.getCustomInstantiator() == null
198198
&& bootDescriptor.getInstantiator() == null
199199
&& !bootDescriptor.isPolymorphic() ) {
200-
final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
201-
int i = 0;
202-
for ( Property property : bootDescriptor.getProperties() ) {
203-
propertyAccessMap.put( property.getName(), propertyAccesses[i] );
204-
i++;
200+
final List<Property> properties = bootDescriptor.getProperties();
201+
final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
202+
for ( int i = 0; i < properties.size(); i++ ) {
203+
final Property property = properties.get( i );
204+
propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), propertyAccesses[i] );
205205
}
206206
return creationContext.getServiceRegistry()
207207
.requireService( BytecodeProvider.class )
208-
.getReflectionOptimizer( bootDescriptor.getComponentClass(), propertyAccessMap );
208+
.getReflectionOptimizer( bootDescriptor.getComponentClass(), propertyInfos );
209209
}
210210
else {
211211
return null;

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

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
package org.hibernate.metamodel.internal;
66

77
import java.lang.reflect.Method;
8-
import java.util.LinkedHashMap;
8+
import java.util.HashMap;
99
import java.util.LinkedHashSet;
10+
import java.util.List;
1011
import java.util.Locale;
1112
import java.util.Map;
1213
import java.util.Set;
@@ -59,6 +60,7 @@ public class EntityRepresentationStrategyPojoStandard implements EntityRepresent
5960

6061
private final String identifierPropertyName;
6162
private final PropertyAccess identifierPropertyAccess;
63+
private final BytecodeProvider.PropertyInfo[] propertyInfos;
6264
private final Map<String, PropertyAccess> propertyAccessMap;
6365
private final EmbeddableRepresentationStrategyPojo mapsIdRepresentationStrategy;
6466

@@ -119,7 +121,8 @@ public EntityRepresentationStrategyPojoStandard(
119121
creationContext
120122
);
121123

122-
propertyAccessMap = buildPropertyAccessMap( bootDescriptor );
124+
propertyInfos = buildPropertyInfos( bootDescriptor );
125+
propertyAccessMap = buildPropertyAccessMap( propertyInfos );
123126
reflectionOptimizer = resolveReflectionOptimizer( bytecodeProvider );
124127

125128
instantiator = determineInstantiator( bootDescriptor, runtimeDescriptor.getEntityMetamodel() );
@@ -155,12 +158,22 @@ private ProxyFactory resolveProxyFactory(
155158
}
156159
}
157160

158-
private Map<String, PropertyAccess> buildPropertyAccessMap(PersistentClass bootDescriptor) {
159-
final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
160-
for ( Property property : bootDescriptor.getPropertyClosure() ) {
161-
propertyAccessMap.put( property.getName(), makePropertyAccess( property ) );
161+
private Map<String, PropertyAccess> buildPropertyAccessMap(BytecodeProvider.PropertyInfo[] propertyInfos) {
162+
final Map<String, PropertyAccess> map = new HashMap<>( propertyInfos.length );
163+
for ( BytecodeProvider.PropertyInfo propertyInfo : propertyInfos ) {
164+
map.put( propertyInfo.propertyName(), propertyInfo.propertyAccess() );
162165
}
163-
return propertyAccessMap;
166+
return map;
167+
}
168+
169+
private BytecodeProvider.PropertyInfo[] buildPropertyInfos(PersistentClass bootDescriptor) {
170+
final List<Property> properties = bootDescriptor.getPropertyClosure();
171+
final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
172+
for ( int i = 0; i < properties.size(); i++ ) {
173+
final Property property = properties.get( i );
174+
propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), makePropertyAccess( property ) );
175+
}
176+
return propertyInfos;
164177
}
165178

166179
private EntityInstantiator determineInstantiator(PersistentClass bootDescriptor, EntityMetamodel entityMetamodel) {
@@ -303,11 +316,11 @@ private static ProxyFactory instantiateProxyFactory(
303316
}
304317

305318
private ReflectionOptimizer resolveReflectionOptimizer(BytecodeProvider bytecodeProvider) {
306-
return bytecodeProvider.getReflectionOptimizer( mappedJtd.getJavaTypeClass(), propertyAccessMap );
319+
return bytecodeProvider.getReflectionOptimizer( mappedJtd.getJavaTypeClass(), propertyInfos );
307320
}
308321

309322
private PropertyAccess makePropertyAccess(Property bootAttributeDescriptor) {
310-
final Class<?> mappedClass = mappedJtd.getJavaTypeClass();
323+
final Class<?> mappedClass = bootAttributeDescriptor.getPersistentClass().getMappedClass();
311324
final String descriptorName = bootAttributeDescriptor.getName();
312325
final var strategy = propertyAccessStrategy( bootAttributeDescriptor, mappedClass, strategySelector );
313326
if ( strategy == null ) {

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4822,26 +4822,27 @@ private void inheritSupertypeSpecialAttributeMappings() {
48224822
final NonIdentifierAttribute[] properties = entityMetamodel.getProperties();
48234823
final AttributeMappingsMap.Builder mappingsBuilder = AttributeMappingsMap.builder();
48244824
int fetchableIndex = getFetchableIndexOffset();
4825-
for ( int i = 0; i < entityMetamodel.getPropertySpan(); i++ ) {
4826-
final NonIdentifierAttribute runtimeAttributeDefinition = properties[i];
4827-
final String attributeName = runtimeAttributeDefinition.getName();
4828-
final Property bootProperty = bootEntityDescriptor.getProperty( attributeName );
4829-
if ( superMappingType == null
4830-
|| superMappingType.findAttributeMapping( bootProperty.getName() ) == null ) {
4831-
mappingsBuilder.put(
4832-
attributeName,
4833-
generateNonIdAttributeMapping(
4834-
runtimeAttributeDefinition,
4835-
bootProperty,
4836-
stateArrayPosition++,
4837-
fetchableIndex++,
4838-
creationProcess
4839-
)
4840-
);
4841-
}
4842-
declaredAttributeMappings = mappingsBuilder.build();
4843-
// otherwise, it's defined on the supertype, skip it here
4825+
// For every property that is "owned" by this persistent class, create a declared attribute mapping
4826+
final List<Property> bootProperties = bootEntityDescriptor.getProperties();
4827+
// EntityMetamodel uses getPropertyClosure(), which includes the properties of the super type,
4828+
// so use that property span as offset when indexing into the EntityMetamodel NonIdentifierAttribute[]
4829+
final int superclassPropertiesOffset = superMappingType == null ? 0
4830+
: superMappingType.getEntityPersister().getEntityMetamodel().getPropertySpan();
4831+
for ( int i = 0; i < bootProperties.size(); i++ ) {
4832+
final Property bootProperty = bootProperties.get( i );
4833+
assert properties[superclassPropertiesOffset + i].getName().equals( bootProperty.getName() );
4834+
mappingsBuilder.put(
4835+
bootProperty.getName(),
4836+
generateNonIdAttributeMapping(
4837+
properties[superclassPropertiesOffset + i],
4838+
bootProperty,
4839+
stateArrayPosition++,
4840+
fetchableIndex++,
4841+
creationProcess
4842+
)
4843+
);
48444844
}
4845+
declaredAttributeMappings = mappingsBuilder.build();
48454846
}
48464847

48474848
private static BeforeExecutionGenerator createVersionGenerator

0 commit comments

Comments
 (0)