Skip to content

Commit 9bce058

Browse files
beikovbeikov
committed
HHH-19739 Don't unintentionally deduplicate attributes by property name
1 parent 7cbb301 commit 9bce058

File tree

6 files changed

+237
-48
lines changed

6 files changed

+237
-48
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
@@ -163,7 +163,7 @@ public ReflectionOptimizer getReflectionOptimizer(
163163
final Method[] getters = new Method[getterNames.length];
164164
final Method[] setters = new Method[setterNames.length];
165165
try {
166-
findAccessors( clazz, getterNames, setterNames, types, getters, setters );
166+
unwrapPropertyInfos( clazz, getterNames, setterNames, types, getters, setters );
167167
}
168168
catch (InvalidPropertyAccessorException ex) {
169169
LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
@@ -198,6 +198,18 @@ public ReflectionOptimizer getReflectionOptimizer(
198198

199199
@Override
200200
public @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, Map<String, PropertyAccess> propertyAccessMap) {
201+
final PropertyInfo[] propertyInfos = new PropertyInfo[propertyAccessMap.size()];
202+
int i = 0;
203+
for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
204+
final String propertyName = entry.getKey();
205+
final PropertyAccess propertyAccess = entry.getValue();
206+
propertyInfos[i++] = new PropertyInfo( propertyName, propertyAccess );
207+
}
208+
return getReflectionOptimizer( clazz, propertyInfos );
209+
}
210+
211+
@Override
212+
public @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, PropertyInfo[] propertyInfos) {
201213
final Class<?> fastClass;
202214
if ( !clazz.isInterface() && !Modifier.isAbstract( clazz.getModifiers() ) ) {
203215
// we only provide a fast class instantiator if the class can be instantiated
@@ -222,17 +234,17 @@ public ReflectionOptimizer getReflectionOptimizer(
222234
fastClass = null;
223235
}
224236

225-
final Member[] getters = new Member[propertyAccessMap.size()];
226-
final Member[] setters = new Member[propertyAccessMap.size()];
237+
final Member[] getters = new Member[propertyInfos.length];
238+
final Member[] setters = new Member[propertyInfos.length];
239+
final String[] propertyNames = new String[propertyInfos.length];
227240
try {
228-
findAccessors( clazz, propertyAccessMap, getters, setters );
241+
unwrapPropertyInfos( clazz, propertyInfos, propertyNames, getters, setters );
229242
}
230243
catch (InvalidPropertyAccessorException ex) {
231244
LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
232245
return null;
233246
}
234247

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

238250
final String className = clazz.getName() + "$" + OPTIMIZER_PROXY_NAMING_SUFFIX + encodeName( propertyNames, getters, setters );
@@ -1196,7 +1208,7 @@ else if ( setterMember instanceof Field field ) {
11961208
}
11971209
}
11981210

1199-
private static void findAccessors(
1211+
private static void unwrapPropertyInfos(
12001212
Class<?> clazz,
12011213
String[] getterNames,
12021214
String[] setterNames,
@@ -1238,14 +1250,17 @@ else if ( Modifier.isPrivate( setters[i].getModifiers() ) ) {
12381250
}
12391251
}
12401252

1241-
private static void findAccessors(
1253+
private static void unwrapPropertyInfos(
12421254
Class<?> clazz,
1243-
Map<String, PropertyAccess> propertyAccessMap,
1255+
PropertyInfo[] propertyInfos,
1256+
String[] propertyNames,
12441257
Member[] getters,
12451258
Member[] setters) {
12461259
int i = 0;
1247-
for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
1248-
final PropertyAccess propertyAccess = entry.getValue();
1260+
for ( PropertyInfo propertyInfo : propertyInfos ) {
1261+
final String propertyName = propertyInfo.propertyName();
1262+
final PropertyAccess propertyAccess = propertyInfo.propertyAccess();
1263+
propertyNames[i] = propertyName;
12491264
if ( propertyAccess instanceof PropertyAccessEmbeddedImpl ) {
12501265
getters[i] = EMBEDDED_MEMBER;
12511266
setters[i] = EMBEDDED_MEMBER;
@@ -1254,14 +1269,14 @@ private static void findAccessors(
12541269
}
12551270
final Getter getter = propertyAccess.getGetter();
12561271
if ( getter == null ) {
1257-
throw new InvalidPropertyAccessorException( "invalid getter for property [" + entry.getKey() + "]" );
1272+
throw new InvalidPropertyAccessorException( "invalid getter for property [" + propertyName + "]" );
12581273
}
12591274
final Setter setter = propertyAccess.getSetter();
12601275
if ( setter == null ) {
12611276
throw new InvalidPropertyAccessorException(
12621277
String.format(
12631278
"cannot find a setter for [%s] on type [%s]",
1264-
entry.getKey(),
1279+
propertyName,
12651280
clazz.getName()
12661281
)
12671282
);
@@ -1277,7 +1292,7 @@ else if ( getter instanceof GetterFieldImpl getterField ) {
12771292
throw new InvalidPropertyAccessorException(
12781293
String.format(
12791294
"cannot find a getter for [%s] on type [%s]",
1280-
entry.getKey(),
1295+
propertyName,
12811296
clazz.getName()
12821297
)
12831298
);
@@ -1296,7 +1311,7 @@ else if ( setter instanceof SetterFieldImpl setterField ) {
12961311
throw new InvalidPropertyAccessorException(
12971312
String.format(
12981313
"cannot find a setter for [%s] on type [%s]",
1299-
entry.getKey(),
1314+
propertyName,
13001315
clazz.getName()
13011316
)
13021317
);

‎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: 21 additions & 8 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,7 +316,7 @@ 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) {

‎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)