Skip to content

Commit 5798d48

Browse files
beikovbeikov
committed
HHH-19739 Don't unintentionally deduplicate attributes by property name
1 parent 193b15c commit 5798d48

File tree

6 files changed

+278
-50
lines changed

6 files changed

+278
-50
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
@@ -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
@@ -223,17 +235,17 @@ public ReflectionOptimizer getReflectionOptimizer(
223235
fastClass = null;
224236
}
225237

226-
final Member[] getters = new Member[propertyAccessMap.size()];
227-
final Member[] setters = new Member[propertyAccessMap.size()];
238+
final Member[] getters = new Member[propertyInfos.length];
239+
final Member[] setters = new Member[propertyInfos.length];
240+
final String[] propertyNames = new String[propertyInfos.length];
228241
try {
229-
findAccessors( clazz, propertyAccessMap, getters, setters );
242+
unwrapPropertyInfos( clazz, propertyInfos, propertyNames, getters, setters );
230243
}
231244
catch (InvalidPropertyAccessorException ex) {
232245
LOG.unableToGenerateReflectionOptimizer( clazz.getName(), ex.getMessage() );
233246
return null;
234247
}
235248

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

239251
final String className = clazz.getName() + "$" + OPTIMIZER_PROXY_NAMING_SUFFIX + encodeName( propertyNames, getters, setters );
@@ -1201,7 +1213,7 @@ else if ( setterMember instanceof Field ) {
12011213
}
12021214
}
12031215

1204-
private static void findAccessors(
1216+
private static void unwrapPropertyInfos(
12051217
Class<?> clazz,
12061218
String[] getterNames,
12071219
String[] setterNames,
@@ -1243,14 +1255,17 @@ else if ( Modifier.isPrivate( setters[i].getModifiers() ) ) {
12431255
}
12441256
}
12451257

1246-
private static void findAccessors(
1258+
private static void unwrapPropertyInfos(
12471259
Class<?> clazz,
1248-
Map<String, PropertyAccess> propertyAccessMap,
1260+
PropertyInfo[] propertyInfos,
1261+
String[] propertyNames,
12491262
Member[] getters,
12501263
Member[] setters) {
12511264
int i = 0;
1252-
for ( Map.Entry<String, PropertyAccess> entry : propertyAccessMap.entrySet() ) {
1253-
final PropertyAccess propertyAccess = entry.getValue();
1265+
for ( PropertyInfo propertyInfo : propertyInfos ) {
1266+
final String propertyName = propertyInfo.propertyName();
1267+
final PropertyAccess propertyAccess = propertyInfo.propertyAccess();
1268+
propertyNames[i] = propertyName;
12541269
if ( propertyAccess instanceof PropertyAccessEmbeddedImpl ) {
12551270
getters[i] = EMBEDDED_MEMBER;
12561271
setters[i] = EMBEDDED_MEMBER;
@@ -1259,14 +1274,14 @@ private static void findAccessors(
12591274
}
12601275
final Getter getter = propertyAccess.getGetter();
12611276
if ( getter == null ) {
1262-
throw new InvalidPropertyAccessorException( "invalid getter for property [" + entry.getKey() + "]" );
1277+
throw new InvalidPropertyAccessorException( "invalid getter for property [" + propertyName + "]" );
12631278
}
12641279
final Setter setter = propertyAccess.getSetter();
12651280
if ( setter == null ) {
12661281
throw new InvalidPropertyAccessorException(
12671282
String.format(
12681283
"cannot find a setter for [%s] on type [%s]",
1269-
entry.getKey(),
1284+
propertyName,
12701285
clazz.getName()
12711286
)
12721287
);
@@ -1282,7 +1297,7 @@ else if ( getter instanceof GetterFieldImpl ) {
12821297
throw new InvalidPropertyAccessorException(
12831298
String.format(
12841299
"cannot find a getter for [%s] on type [%s]",
1285-
entry.getKey(),
1300+
propertyName,
12861301
clazz.getName()
12871302
)
12881303
);
@@ -1301,7 +1316,7 @@ else if ( setter instanceof SetterFieldImpl ) {
13011316
throw new InvalidPropertyAccessorException(
13021317
String.format(
13031318
"cannot find a setter for [%s] on type [%s]",
1304-
entry.getKey(),
1319+
propertyName,
13051320
clazz.getName()
13061321
)
13071322
);

‎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: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import java.util.Collection;
1010
import java.util.HashMap;
11-
import java.util.LinkedHashMap;
11+
import java.util.List;
1212
import java.util.Locale;
1313
import java.util.Map;
1414
import java.util.function.Supplier;
@@ -236,17 +236,18 @@ private static ReflectionOptimizer buildReflectionOptimizer(
236236
return null;
237237
}
238238

239-
final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
239+
final List<Property> properties = bootDescriptor.getProperties();
240+
final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
241+
242+
for ( int i = 0; i < properties.size(); i++ ) {
243+
final Property property = properties.get( i );
244+
propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), propertyAccesses[i] );
240245

241-
int i = 0;
242-
for ( Property property : bootDescriptor.getProperties() ) {
243-
propertyAccessMap.put( property.getName(), propertyAccesses[i] );
244-
i++;
245246
}
246247

247248
return creationContext.getServiceRegistry()
248249
.requireService( BytecodeProvider.class )
249-
.getReflectionOptimizer( bootDescriptor.getComponentClass(), propertyAccessMap );
250+
.getReflectionOptimizer( bootDescriptor.getComponentClass(), propertyInfos );
250251
}
251252

252253
private static Map<String, Class<?>> getSubclassesByName(

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

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

99
import java.lang.reflect.Method;
10+
import java.util.HashMap;
11+
import java.util.LinkedHashSet;
12+
import java.util.List;
1013
import java.util.LinkedHashMap;
1114
import java.util.Locale;
1215
import java.util.Map;
@@ -71,6 +74,7 @@ public class EntityRepresentationStrategyPojoStandard implements EntityRepresent
7174

7275
private final String identifierPropertyName;
7376
private final PropertyAccess identifierPropertyAccess;
77+
private final BytecodeProvider.PropertyInfo[] propertyInfos;
7478
private final Map<String, PropertyAccess> propertyAccessMap;
7579
private final EmbeddableRepresentationStrategyPojo mapsIdRepresentationStrategy;
7680

@@ -150,8 +154,9 @@ else if ( bootDescriptorIdentifier != null ) {
150154
creationContext
151155
);
152156

153-
this.propertyAccessMap = buildPropertyAccessMap( bootDescriptor );
154-
this.reflectionOptimizer = resolveReflectionOptimizer( bytecodeProvider );
157+
propertyInfos = buildPropertyInfos( bootDescriptor );
158+
propertyAccessMap = buildPropertyAccessMap( propertyInfos );
159+
reflectionOptimizer = resolveReflectionOptimizer( bytecodeProvider );
155160

156161
this.instantiator = determineInstantiator( bootDescriptor, runtimeDescriptor.getEntityMetamodel() );
157162
}
@@ -186,12 +191,22 @@ private ProxyFactory resolveProxyFactory(
186191
return null;
187192
}
188193

189-
private Map<String, PropertyAccess> buildPropertyAccessMap(PersistentClass bootDescriptor) {
190-
final Map<String, PropertyAccess> propertyAccessMap = new LinkedHashMap<>();
191-
for ( Property property : bootDescriptor.getPropertyClosure() ) {
192-
propertyAccessMap.put( property.getName(), makePropertyAccess( property ) );
194+
private Map<String, PropertyAccess> buildPropertyAccessMap(BytecodeProvider.PropertyInfo[] propertyInfos) {
195+
final Map<String, PropertyAccess> map = new HashMap<>( propertyInfos.length );
196+
for ( BytecodeProvider.PropertyInfo propertyInfo : propertyInfos ) {
197+
map.put( propertyInfo.propertyName(), propertyInfo.propertyAccess() );
193198
}
194-
return propertyAccessMap;
199+
return map;
200+
}
201+
202+
private BytecodeProvider.PropertyInfo[] buildPropertyInfos(PersistentClass bootDescriptor) {
203+
final List<Property> properties = bootDescriptor.getPropertyClosure();
204+
final BytecodeProvider.PropertyInfo[] propertyInfos = new BytecodeProvider.PropertyInfo[properties.size()];
205+
for ( int i = 0; i < properties.size(); i++ ) {
206+
final Property property = properties.get( i );
207+
propertyInfos[i] = new BytecodeProvider.PropertyInfo( property.getName(), makePropertyAccess( property ) );
208+
}
209+
return propertyInfos;
195210
}
196211

197212
private EntityInstantiator determineInstantiator(PersistentClass bootDescriptor, EntityMetamodel entityMetamodel) {
@@ -324,7 +339,7 @@ private ProxyFactory createProxyFactory(
324339
private ReflectionOptimizer resolveReflectionOptimizer(BytecodeProvider bytecodeProvider) {
325340
return bytecodeProvider.getReflectionOptimizer(
326341
mappedJtd.getJavaTypeClass(),
327-
propertyAccessMap
342+
propertyInfos
328343
);
329344
}
330345

‎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
@@ -5218,28 +5218,27 @@ public void prepareMappingModel(MappingModelCreationProcess creationProcess) {
52185218
final NonIdentifierAttribute[] properties = currentEntityMetamodel.getProperties();
52195219
AttributeMappingsMap.Builder mappingsBuilder = AttributeMappingsMap.builder();
52205220
int fetchableIndex = getFetchableIndexOffset();
5221-
for ( int i = 0; i < currentEntityMetamodel.getPropertySpan(); i++ ) {
5222-
final NonIdentifierAttribute runtimeAttrDefinition = properties[i];
5223-
final Property bootProperty = bootEntityDescriptor.getProperty( runtimeAttrDefinition.getName() );
5224-
5225-
if ( superMappingType == null
5226-
|| superMappingType.findAttributeMapping( bootProperty.getName() ) == null ) {
5227-
mappingsBuilder.put(
5228-
runtimeAttrDefinition.getName(),
5229-
generateNonIdAttributeMapping(
5230-
runtimeAttrDefinition,
5231-
bootProperty,
5232-
stateArrayPosition++,
5233-
fetchableIndex++,
5234-
creationProcess
5235-
)
5236-
);
5237-
}
5238-
declaredAttributeMappings = mappingsBuilder.build();
5239-
// else {
5240-
// its defined on the supertype, skip it here
5241-
// }
5221+
// For every property that is "owned" by this persistent class, create a declared attribute mapping
5222+
final List<Property> bootProperties = bootEntityDescriptor.getProperties();
5223+
// EntityMetamodel uses getPropertyClosure(), which includes the properties of the super type,
5224+
// so use that property span as offset when indexing into the EntityMetamodel NonIdentifierAttribute[]
5225+
final int superclassPropertiesOffset = superMappingType == null ? 0
5226+
: superMappingType.getEntityPersister().getEntityMetamodel().getPropertySpan();
5227+
for ( int i = 0; i < bootProperties.size(); i++ ) {
5228+
final Property bootProperty = bootProperties.get( i );
5229+
assert properties[superclassPropertiesOffset + i].getName().equals( bootProperty.getName() );
5230+
mappingsBuilder.put(
5231+
bootProperty.getName(),
5232+
generateNonIdAttributeMapping(
5233+
properties[superclassPropertiesOffset + i],
5234+
bootProperty,
5235+
stateArrayPosition++,
5236+
fetchableIndex++,
5237+
creationProcess
5238+
)
5239+
);
52425240
}
5241+
declaredAttributeMappings = mappingsBuilder.build();
52435242

52445243
getAttributeMappings();
52455244

0 commit comments

Comments
 (0)