Skip to content

Commit eecc888

Browse files
beikovbeikov
committed
HHH-19739 Don't unintentionally deduplicate attributes by property name
1 parent 0506fda commit eecc888

File tree

6 files changed

+284
-59
lines changed

6 files changed

+284
-59
lines changed

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

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public ReflectionOptimizer getReflectionOptimizer(
164164
final Method[] getters = new Method[getterNames.length];
165165
final Method[] setters = new Method[setterNames.length];
166166
try {
167-
findAccessors( clazz, getterNames, setterNames, types, getters, setters );
167+
unwrapPropertyInfos( clazz, getterNames, setterNames, types, getters, setters );
168168
}
169169
catch (InvalidPropertyAccessorException ex) {
170170
LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
@@ -199,6 +199,18 @@ public ReflectionOptimizer getReflectionOptimizer(
199199

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

228-
final Member[] getters = new Member[propertyAccessMap.size()];
229-
final Member[] setters = new Member[propertyAccessMap.size()];
240+
final Member[] getters = new Member[propertyInfos.length];
241+
final Member[] setters = new Member[propertyInfos.length];
242+
final String[] propertyNames = new String[propertyInfos.length];
230243
try {
231-
findAccessors( clazz, propertyAccessMap, getters, setters );
244+
unwrapPropertyInfos( clazz, propertyInfos, propertyNames, getters, setters );
232245
}
233246
catch (InvalidPropertyAccessorException ex) {
234247
LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
235248
return null;
236249
}
237250

238-
Class<?> superClass = determineAccessOptimizerSuperClass( clazz, getters, setters );
239-
240-
final String[] propertyNames = propertyAccessMap.keySet().toArray( new String[0] );
251+
final Class<?> superClass = determineAccessOptimizerSuperClass( clazz, getters, setters );
241252
final Class<?> bulkAccessor = byteBuddyState.load( clazz, byteBuddy -> byteBuddy
242253
.with( new NamingStrategy.SuffixingRandom(
243254
OPTIMIZER_PROXY_NAMING_SUFFIX,
@@ -1148,7 +1159,7 @@ else if ( setterMember instanceof Field ) {
11481159
}
11491160
}
11501161

1151-
private static void findAccessors(
1162+
private static void unwrapPropertyInfos(
11521163
Class<?> clazz,
11531164
String[] getterNames,
11541165
String[] setterNames,
@@ -1190,14 +1201,17 @@ else if ( Modifier.isPrivate( setters[i].getModifiers() ) ) {
11901201
}
11911202
}
11921203

1193-
private static void findAccessors(
1204+
private static void unwrapPropertyInfos(
11941205
Class<?> clazz,
1195-
Map<String, PropertyAccess> propertyAccessMap,
1206+
PropertyInfo[] propertyInfos,
1207+
String[] propertyNames,
11961208
Member[] getters,
11971209
Member[] setters) {
11981210
int i = 0;
1199-
for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
1200-
final PropertyAccess propertyAccess = entry.getValue();
1211+
for ( PropertyInfo propertyInfo : propertyInfos ) {
1212+
final String propertyName = propertyInfo.propertyName();
1213+
final PropertyAccess propertyAccess = propertyInfo.propertyAccess();
1214+
propertyNames[i] = propertyName;
12011215
if ( propertyAccess instanceof PropertyAccessEmbeddedImpl ) {
12021216
getters[i] = EMBEDDED_MEMBER;
12031217
setters[i] = EMBEDDED_MEMBER;
@@ -1206,14 +1220,14 @@ private static void findAccessors(
12061220
}
12071221
final Getter getter = propertyAccess.getGetter();
12081222
if ( getter == null ) {
1209-
throw new InvalidPropertyAccessorException( "invalid getter for property [" + entry.getKey() + "]" );
1223+
throw new InvalidPropertyAccessorException( "invalid getter for property [" + propertyName + "]" );
12101224
}
12111225
final Setter setter = propertyAccess.getSetter();
12121226
if ( setter == null ) {
12131227
throw new InvalidPropertyAccessorException(
12141228
String.format(
12151229
"cannot find a setter for [%s] on type [%s]",
1216-
entry.getKey(),
1230+
propertyName,
12171231
clazz.getName()
12181232
)
12191233
);
@@ -1229,7 +1243,7 @@ else if ( getter instanceof GetterFieldImpl ) {
12291243
throw new InvalidPropertyAccessorException(
12301244
String.format(
12311245
"cannot find a getter for [%s] on type [%s]",
1232-
entry.getKey(),
1246+
propertyName,
12331247
clazz.getName()
12341248
)
12351249
);
@@ -1248,7 +1262,7 @@ else if ( setter instanceof SetterFieldImpl ) {
12481262
throw new InvalidPropertyAccessorException(
12491263
String.format(
12501264
"cannot find a setter for [%s] on type [%s]",
1251-
entry.getKey(),
1265+
propertyName,
12521266
clazz.getName()
12531267
)
12541268
);

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
*/
77
package org.hibernate.bytecode.spi;
88

9+
import java.util.HashMap;
910
import java.util.Map;
11+
import java.util.Objects;
1012

1113
import org.hibernate.bytecode.enhance.spi.EnhancementContext;
1214
import org.hibernate.bytecode.enhance.spi.Enhancer;
@@ -57,9 +59,78 @@ public interface BytecodeProvider extends Service {
5759
* @param clazz The class to be reflected upon.
5860
* @param propertyAccessMap The ordered property access map
5961
* @return The reflection optimization delegate.
62+
* @deprecated Use {@link #getReflectionOptimizer(Class, PropertyInfo[])} instead
6063
*/
64+
@Deprecated(forRemoval = true)
6165
@Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, Map<String, PropertyAccess> propertyAccessMap);
6266

67+
/**
68+
* Retrieve the ReflectionOptimizer delegate for this provider
69+
* capable of generating reflection optimization components.
70+
*
71+
* @param clazz The class to be reflected upon.
72+
* @param propertyInfos The ordered property infos
73+
* @return The reflection optimization delegate.
74+
*/
75+
default @Nullable ReflectionOptimizer getReflectionOptimizer(Class<?> clazz, PropertyInfo[] propertyInfos) {
76+
final Map<String, PropertyAccess> map = new HashMap<>();
77+
for ( int i = 0; i < propertyInfos.length; i++ ) {
78+
map.put( propertyInfos[i].propertyName(), propertyInfos[i].propertyAccess() );
79+
}
80+
return getReflectionOptimizer( clazz, map );
81+
}
82+
83+
/**
84+
* Information about a property of a class, needed for generating reflection optimizers.
85+
*
86+
*/
87+
public static final class PropertyInfo {
88+
private final String propertyName;
89+
private final PropertyAccess propertyAccess;
90+
91+
/**
92+
* @param propertyName The name of the property
93+
* @param propertyAccess The property access
94+
*/
95+
public PropertyInfo(String propertyName, PropertyAccess propertyAccess) {
96+
this.propertyName = propertyName;
97+
this.propertyAccess = propertyAccess;
98+
}
99+
100+
public String propertyName() {
101+
return propertyName;
102+
}
103+
104+
public PropertyAccess propertyAccess() {
105+
return propertyAccess;
106+
}
107+
108+
@Override
109+
public boolean equals(@Nullable Object obj) {
110+
if ( obj == this ) {
111+
return true;
112+
}
113+
if ( obj == null || obj.getClass() != this.getClass() ) {
114+
return false;
115+
}
116+
var that = (PropertyInfo) obj;
117+
return Objects.equals( this.propertyName, that.propertyName ) &&
118+
Objects.equals( this.propertyAccess, that.propertyAccess );
119+
}
120+
121+
@Override
122+
public int hashCode() {
123+
return Objects.hash( propertyName, propertyAccess );
124+
}
125+
126+
@Override
127+
public String toString() {
128+
return "PropertyInfo[" +
129+
"propertyName=" + propertyName + ", " +
130+
"propertyAccess=" + propertyAccess + ']';
131+
}
132+
}
133+
63134
/**
64135
* Returns a byte code enhancer that implements the enhancements described in the supplied enhancement context.
65136
*

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*/
77
package org.hibernate.metamodel.internal;
88

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;
@@ -179,19 +179,17 @@ private ReflectionOptimizer buildReflectionOptimizer(
179179
return null;
180180
}
181181

182-
final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
182+
final List<Property> properties = bootDescriptor.getProperties();
183+
final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
183184

184-
int i = 0;
185-
for ( Property property : bootDescriptor.getProperties() ) {
186-
propertyAccessMap.put( property.getName(), getPropertyAccesses()[i] );
187-
i++;
185+
for ( int i = 0; i < properties.size(); i++ ) {
186+
final Property property = properties.get( i );
187+
propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), getPropertyAccesses()[i] );
188188
}
189-
final BytecodeProvider bytecodeProvider = creationContext.getServiceRegistry().getService( BytecodeProvider.class );
190189

191-
return bytecodeProvider.getReflectionOptimizer(
192-
bootDescriptor.getComponentClass(),
193-
propertyAccessMap
194-
);
190+
return creationContext.getServiceRegistry()
191+
.requireService( BytecodeProvider.class )
192+
.getReflectionOptimizer( bootDescriptor.getComponentClass(), propertyInfos );
195193
}
196194

197195
@Override

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

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
package org.hibernate.metamodel.internal;
88

99
import java.lang.reflect.Method;
10-
import java.util.LinkedHashMap;
10+
import java.util.HashMap;
11+
import java.util.List;
1112
import java.util.Locale;
1213
import java.util.Map;
1314
import java.util.Set;
@@ -72,6 +73,7 @@ public class EntityRepresentationStrategyPojoStandard implements EntityRepresent
7273

7374
private final String identifierPropertyName;
7475
private final PropertyAccess identifierPropertyAccess;
76+
private final BytecodeProvider.PropertyInfo[] propertyInfos;
7577
private final Map<String, PropertyAccess> propertyAccessMap;
7678
private final EmbeddableRepresentationStrategyPojo mapsIdRepresentationStrategy;
7779

@@ -139,8 +141,9 @@ else if ( bootDescriptorIdentifier != null ) {
139141
identifierPropertyAccess = makePropertyAccess( identifierProperty );
140142
}
141143

142-
final BytecodeProvider bytecodeProvider = creationContext.getBootstrapContext().getServiceRegistry().getService( BytecodeProvider.class );
144+
this.strategySelector = creationContext.getServiceRegistry().getService( StrategySelector.class );
143145

146+
final BytecodeProvider bytecodeProvider = creationContext.getBootstrapContext().getServiceRegistry().requireService( BytecodeProvider.class );
144147
final EntityMetamodel entityMetamodel = runtimeDescriptor.getEntityMetamodel();
145148
ProxyFactory proxyFactory = null;
146149
if ( proxyJtd != null && entityMetamodel.isLazy() ) {
@@ -151,16 +154,29 @@ else if ( bootDescriptorIdentifier != null ) {
151154
}
152155
this.proxyFactory = proxyFactory;
153156

154-
// resolveReflectionOptimizer may lead to a makePropertyAccess call which requires strategySelector
155-
this.strategySelector = creationContext.getServiceRegistry().getService( StrategySelector.class );
156-
final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
157-
for ( Property property : bootDescriptor.getPropertyClosure() ) {
158-
propertyAccessMap.put( property.getName(), makePropertyAccess( property ) );
157+
propertyInfos = buildPropertyInfos( bootDescriptor );
158+
propertyAccessMap = buildPropertyAccessMap( propertyInfos );
159+
reflectionOptimizer = resolveReflectionOptimizer( bytecodeProvider );
160+
161+
this.instantiator = determineInstantiator( bootDescriptor, runtimeDescriptor.getEntityMetamodel() );
162+
}
163+
164+
private Map<String, PropertyAccess> buildPropertyAccessMap(BytecodeProvider.PropertyInfo[] propertyInfos) {
165+
final Map<String, PropertyAccess> map = new HashMap<>( propertyInfos.length );
166+
for ( BytecodeProvider.PropertyInfo propertyInfo : propertyInfos ) {
167+
map.put( propertyInfo.propertyName(), propertyInfo.propertyAccess() );
159168
}
160-
this.propertyAccessMap = propertyAccessMap;
161-
this.reflectionOptimizer = resolveReflectionOptimizer( bytecodeProvider );
169+
return map;
170+
}
162171

163-
this.instantiator = determineInstantiator( bootDescriptor, entityMetamodel );
172+
private BytecodeProvider.PropertyInfo[] buildPropertyInfos(PersistentClass bootDescriptor) {
173+
final List<Property> properties = bootDescriptor.getPropertyClosure();
174+
final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
175+
for ( int i = 0; i < properties.size(); i++ ) {
176+
final Property property = properties.get( i );
177+
propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), makePropertyAccess( property ) );
178+
}
179+
return propertyInfos;
164180
}
165181

166182
private EntityInstantiator determineInstantiator(PersistentClass bootDescriptor, EntityMetamodel entityMetamodel) {
@@ -292,7 +308,7 @@ private ReflectionOptimizer resolveReflectionOptimizer(BytecodeProvider bytecode
292308
}
293309
return bytecodeProvider.getReflectionOptimizer(
294310
mappedJtd.getJavaTypeClass(),
295-
propertyAccessMap
311+
propertyInfos
296312
);
297313
}
298314

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

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4798,28 +4798,27 @@ public void prepareMappingModel(MappingModelCreationProcess creationProcess) {
47984798
final NonIdentifierAttribute[] properties = currentEntityMetamodel.getProperties();
47994799
AttributeMappingsMap.Builder mappingsBuilder = AttributeMappingsMap.builder();
48004800
int fetchableIndex = getFetchableIndexOffset();
4801-
for ( int i = 0; i < currentEntityMetamodel.getPropertySpan(); i++ ) {
4802-
final NonIdentifierAttribute runtimeAttrDefinition = properties[i];
4803-
final Property bootProperty = bootEntityDescriptor.getProperty( runtimeAttrDefinition.getName() );
4804-
4805-
if ( superMappingType == null
4806-
|| superMappingType.findAttributeMapping( bootProperty.getName() ) == null ) {
4807-
mappingsBuilder.put(
4808-
runtimeAttrDefinition.getName(),
4809-
generateNonIdAttributeMapping(
4810-
runtimeAttrDefinition,
4811-
bootProperty,
4812-
stateArrayPosition++,
4813-
fetchableIndex++,
4814-
creationProcess
4815-
)
4816-
);
4817-
}
4818-
declaredAttributeMappings = mappingsBuilder.build();
4819-
// else {
4820-
// its defined on the supertype, skip it here
4821-
// }
4801+
// For every property that is "owned" by this persistent class, create a declared attribute mapping
4802+
final List<Property> bootProperties = bootEntityDescriptor.getProperties();
4803+
// EntityMetamodel uses getPropertyClosure(), which includes the properties of the super type,
4804+
// so use that property span as offset when indexing into the EntityMetamodel NonIdentifierAttribute[]
4805+
final int superclassPropertiesOffset = superMappingType == null ? 0
4806+
: superMappingType.getEntityPersister().getEntityMetamodel().getPropertySpan();
4807+
for ( int i = 0; i < bootProperties.size(); i++ ) {
4808+
final Property bootProperty = bootProperties.get( i );
4809+
assert properties[superclassPropertiesOffset + i].getName().equals( bootProperty.getName() );
4810+
mappingsBuilder.put(
4811+
bootProperty.getName(),
4812+
generateNonIdAttributeMapping(
4813+
properties[superclassPropertiesOffset + i],
4814+
bootProperty,
4815+
stateArrayPosition++,
4816+
fetchableIndex++,
4817+
creationProcess
4818+
)
4819+
);
48224820
}
4821+
declaredAttributeMappings = mappingsBuilder.build();
48234822

48244823
getAttributeMappings();
48254824

0 commit comments

Comments
 (0)