Skip to content

Commit faebabd

Browse files
scottmarlowsebersole
scottmarlow
authored and
sebersole
committed
HHH-16572 - Skip enhancement for PROPERTY attributes with mismatched field and method names
Signed-off-by: Scott Marlow <[email protected]>
1 parent ab9e671 commit faebabd

File tree

3 files changed

+209
-30
lines changed

3 files changed

+209
-30
lines changed

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

Lines changed: 121 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,25 @@
66
*/
77
package org.hibernate.bytecode.enhance.internal.bytebuddy;
88

9-
import java.lang.annotation.Annotation;
10-
import java.lang.reflect.Modifier;
11-
import java.util.ArrayList;
12-
import java.util.Collection;
13-
import java.util.Collections;
14-
import java.util.List;
15-
import java.util.Map;
16-
import java.util.Objects;
17-
import java.util.Optional;
18-
import java.util.function.Supplier;
19-
9+
import jakarta.persistence.Access;
10+
import jakarta.persistence.AccessType;
11+
import jakarta.persistence.metamodel.Type;
12+
import net.bytebuddy.asm.Advice;
13+
import net.bytebuddy.description.annotation.AnnotationDescription;
14+
import net.bytebuddy.description.annotation.AnnotationList;
15+
import net.bytebuddy.description.field.FieldDescription;
16+
import net.bytebuddy.description.field.FieldDescription.InDefinedShape;
17+
import net.bytebuddy.description.method.MethodDescription;
18+
import net.bytebuddy.description.type.TypeDefinition;
19+
import net.bytebuddy.description.type.TypeDescription;
20+
import net.bytebuddy.description.type.TypeDescription.Generic;
21+
import net.bytebuddy.description.type.TypeList;
22+
import net.bytebuddy.dynamic.DynamicType;
23+
import net.bytebuddy.dynamic.scaffold.MethodGraph;
24+
import net.bytebuddy.implementation.FieldAccessor;
25+
import net.bytebuddy.implementation.FixedValue;
26+
import net.bytebuddy.implementation.Implementation;
27+
import net.bytebuddy.implementation.StubMethod;
2028
import org.hibernate.Version;
2129
import org.hibernate.bytecode.enhance.VersionMismatchException;
2230
import org.hibernate.bytecode.enhance.internal.tracker.CompositeOwnerTracker;
@@ -41,23 +49,16 @@
4149
import org.hibernate.internal.CoreLogging;
4250
import org.hibernate.internal.CoreMessageLogger;
4351

44-
import jakarta.persistence.Access;
45-
import jakarta.persistence.AccessType;
46-
import jakarta.persistence.metamodel.Type;
47-
import net.bytebuddy.asm.Advice;
48-
import net.bytebuddy.description.annotation.AnnotationDescription;
49-
import net.bytebuddy.description.annotation.AnnotationList;
50-
import net.bytebuddy.description.field.FieldDescription;
51-
import net.bytebuddy.description.field.FieldDescription.InDefinedShape;
52-
import net.bytebuddy.description.method.MethodDescription;
53-
import net.bytebuddy.description.type.TypeDefinition;
54-
import net.bytebuddy.description.type.TypeDescription;
55-
import net.bytebuddy.description.type.TypeDescription.Generic;
56-
import net.bytebuddy.dynamic.DynamicType;
57-
import net.bytebuddy.implementation.FieldAccessor;
58-
import net.bytebuddy.implementation.FixedValue;
59-
import net.bytebuddy.implementation.Implementation;
60-
import net.bytebuddy.implementation.StubMethod;
52+
import java.lang.annotation.Annotation;
53+
import java.lang.reflect.Modifier;
54+
import java.util.ArrayList;
55+
import java.util.Collection;
56+
import java.util.Collections;
57+
import java.util.List;
58+
import java.util.Map;
59+
import java.util.Objects;
60+
import java.util.Optional;
61+
import java.util.function.Supplier;
6162

6263
import static net.bytebuddy.matcher.ElementMatchers.isDefaultFinalizer;
6364

@@ -172,6 +173,11 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
172173
}
173174

174175
if ( enhancementContext.isEntityClass( managedCtClass ) ) {
176+
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
177+
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes
178+
return null;
179+
}
180+
175181
log.debugf( "Enhancing [%s] as Entity", managedCtClass.getName() );
176182
DynamicType.Builder<?> builder = builderSupplier.get();
177183
builder = builder.implement( ManagedEntity.class )
@@ -331,6 +337,11 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
331337
return createTransformer( managedCtClass ).applyTo( builder );
332338
}
333339
else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
340+
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
341+
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes
342+
return null;
343+
}
344+
334345
log.debugf( "Enhancing [%s] as Composite", managedCtClass.getName() );
335346

336347
DynamicType.Builder<?> builder = builderSupplier.get();
@@ -364,6 +375,12 @@ else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
364375
return createTransformer( managedCtClass ).applyTo( builder );
365376
}
366377
else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
378+
379+
// Check for HHH-16572 (PROPERTY attributes with mismatched field and method names)
380+
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
381+
return null;
382+
}
383+
367384
log.debugf( "Enhancing [%s] as MappedSuperclass", managedCtClass.getName() );
368385

369386
DynamicType.Builder<?> builder = builderSupplier.get();
@@ -380,6 +397,82 @@ else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
380397
}
381398
}
382399

400+
/**
401+
* Check whether an entity class ({@code managedCtClass}) has mismatched names between a persistent field and its
402+
* getter/setter when using {@link AccessType#PROPERTY}, which Hibernate does not currently support for enhancement.
403+
* See https://hibernate.atlassian.net/browse/HHH-16572
404+
*/
405+
private boolean hasUnsupportedAttributeNaming(TypeDescription managedCtClass) {
406+
// For process access rules, See https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#default-access-type
407+
// and https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a122
408+
//
409+
// This check will determine if entity field names do not match Property accessor method name
410+
// For example:
411+
// @Entity
412+
// class Book {
413+
// Integer id;
414+
// String smtg;
415+
//
416+
// @Id Integer getId() { return id; }
417+
// String getSomething() { return smtg; }
418+
// }
419+
//
420+
// Check name of the getter/setter method with persistence annotation and getter/setter method name that doesn't refer to an entity field
421+
// and will return false. If the property accessor method(s) are named to match the field name(s), return true.
422+
boolean propertyHasAnnotation = false;
423+
MethodGraph.Linked methodGraph = MethodGraph.Compiler.Default.forJavaHierarchy().compile((TypeDefinition) managedCtClass);
424+
for (MethodGraph.Node node : methodGraph.listNodes()) {
425+
MethodDescription methodDescription = node.getRepresentative();
426+
if (methodDescription.getDeclaringType().represents(Object.class)) { // skip class java.lang.Object methods
427+
continue;
428+
}
429+
430+
String methodName = methodDescription.getActualName();
431+
if (methodName.equals("") ||
432+
(!methodName.startsWith("get") && !methodName.startsWith("set") && !methodName.startsWith("is"))) {
433+
continue;
434+
}
435+
String methodFieldName;
436+
if (methodName.startsWith("is")) { // skip past "is"
437+
methodFieldName = methodName.substring(2);
438+
}
439+
else if (methodName.startsWith("get") ||
440+
methodName.startsWith("set")) { // skip past "get" or "set"
441+
methodFieldName = methodName.substring(3);
442+
}
443+
else {
444+
// not a property accessor method so ignore it
445+
continue;
446+
}
447+
boolean propertyNameMatchesFieldName = false;
448+
// convert field letter to lower case
449+
methodFieldName = methodFieldName.substring(0, 1).toLowerCase() + methodFieldName.substring(1);
450+
TypeList typeList = methodDescription.getDeclaredAnnotations().asTypeList();
451+
if (typeList.stream().anyMatch(typeDefinitions ->
452+
(typeDefinitions.getName().contains("jakarta.persistence")))) {
453+
propertyHasAnnotation = true;
454+
}
455+
for (FieldDescription ctField : methodDescription.getDeclaringType().getDeclaredFields()) {
456+
if (!Modifier.isStatic(ctField.getModifiers())) {
457+
AnnotatedFieldDescription annotatedField = new AnnotatedFieldDescription(enhancementContext, ctField);
458+
boolean containsPropertyAccessorMethods = false;
459+
if (enhancementContext.isPersistentField(annotatedField)) {
460+
if (methodFieldName.equals(ctField.getActualName())) {
461+
propertyNameMatchesFieldName = true;
462+
break;
463+
}
464+
}
465+
}
466+
}
467+
if (propertyHasAnnotation && !propertyNameMatchesFieldName) {
468+
log.debugf("Skipping enhancement of [%s]: due to class [%s] not having a property accessor method name matching field name [%s]",
469+
managedCtClass, methodDescription.getDeclaringType().getActualName(), methodFieldName);
470+
return true;
471+
}
472+
}
473+
return false;
474+
}
475+
383476
private static void verifyVersions(TypeDescription managedCtClass, ByteBuddyEnhancementContext enhancementContext) {
384477
final AnnotationDescription.Loadable<EnhancementInfo> existingInfo = managedCtClass
385478
.getDeclaredAnnotations()

‎hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/access/InvalidPropertyNameTest.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package org.hibernate.orm.test.bytecode.enhancement.access;
2+
3+
import jakarta.persistence.*;
4+
import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
5+
import org.hibernate.testing.orm.junit.*;
6+
import org.junit.jupiter.api.AfterEach;
7+
import org.junit.jupiter.api.Test;
8+
9+
import static org.assertj.core.api.Assertions.assertThat;
10+
11+
@DomainModel(
12+
annotatedClasses = {
13+
InvalidPropertyNameTest.SomeEntity.class,
14+
}
15+
)
16+
@SessionFactory
17+
@JiraKey("HHH-16572")
18+
@BytecodeEnhanced
19+
public class InvalidPropertyNameTest {
20+
21+
22+
@Test
23+
@FailureExpected(jiraKey = "HHH-16572")
24+
public void test(SessionFactoryScope scope) {
25+
scope.inTransaction( session -> {
26+
session.persist( new SomeEntity( 1L, "field", "property" ) );
27+
} );
28+
29+
scope.inTransaction( session -> {
30+
SomeEntity entity = session.get( SomeEntity.class, 1L );
31+
assertThat( entity.property ).isEqualTo( "from getter: property" );
32+
33+
entity.setPropertyMethod( "updated" );
34+
} );
35+
36+
scope.inTransaction( session -> {
37+
SomeEntity entity = session.get( SomeEntity.class, 1L );
38+
assertThat( entity.property ).isEqualTo( "from getter: updated" );
39+
} );
40+
}
41+
42+
@AfterEach
43+
public void cleanup(SessionFactoryScope scope) {
44+
// uncomment the following when @FailureExpected is removed above
45+
// scope.inTransaction( session -> {
46+
// session.remove( session.get( SomeEntity.class, 1L ) );
47+
// PropertyAccessTest} );
48+
}
49+
50+
@Entity
51+
@Table(name = "SOME_ENTITY")
52+
static class SomeEntity {
53+
@Id
54+
Long id;
55+
56+
@Basic
57+
String field;
58+
59+
String property;
60+
61+
public SomeEntity() {
62+
}
63+
64+
public SomeEntity(Long id, String field, String property) {
65+
this.id = id;
66+
this.field = field;
67+
this.property = property;
68+
}
69+
70+
/**
71+
* The following property accessor methods are purposely named incorrectly to
72+
* not match the "property" field. The HHH-16572 change ensures that
73+
* this entity is not (bytecode) enhanced. Eventually further changes will be made
74+
* such that this entity is enhanced in which case the FailureExpected can be removed
75+
* and the cleanup() uncommented.
76+
*/
77+
@Basic
78+
@Access(AccessType.PROPERTY)
79+
public String getPropertyMethod() {
80+
return "from getter: " + property;
81+
}
82+
83+
public void setPropertyMethod(String property) {
84+
this.property = property;
85+
}
86+
}
87+
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/lazy/LazyLoadingByEnhancerSetterTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
1212
import org.hibernate.testing.orm.junit.DomainModel;
13-
import org.hibernate.testing.orm.junit.FailureExpected;
1413
import org.hibernate.testing.orm.junit.JiraKey;
1514
import org.hibernate.testing.orm.junit.ServiceRegistry;
1615
import org.hibernate.testing.orm.junit.SessionFactory;
@@ -79,7 +78,7 @@ public void testField(SessionFactoryScope scope) {
7978
}
8079

8180
@Test
82-
@FailureExpected( jiraKey = "HHH-10747" )
81+
// failure doesn't occur with HHH-16572 change @FailureExpected( jiraKey = "HHH-10747" )
8382
public void testProperty(SessionFactoryScope scope) {
8483
scope.inTransaction( s -> {
8584
ItemProperty input = new ItemProperty();

0 commit comments

Comments
 (0)