HHH-16572 - Skip enhancement for PROPERTY attributes with mismatched field and method names

Signed-off-by: Scott Marlow <smarlow@redhat.com>
This commit is contained in:
Scott Marlow 2024-11-03 18:31:45 -05:00 committed by Steve Ebersole
parent 14e0f12654
commit 356ea205ff
3 changed files with 214 additions and 31 deletions

View File

@ -4,17 +4,25 @@
*/
package org.hibernate.bytecode.enhance.internal.bytebuddy;
import java.lang.annotation.Annotation;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Supplier;
import jakarta.persistence.Access;
import jakarta.persistence.AccessType;
import jakarta.persistence.metamodel.Type;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.annotation.AnnotationDescription;
import net.bytebuddy.description.annotation.AnnotationList;
import net.bytebuddy.description.field.FieldDescription;
import net.bytebuddy.description.field.FieldDescription.InDefinedShape;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDefinition;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.description.type.TypeDescription.Generic;
import net.bytebuddy.description.type.TypeList;
import net.bytebuddy.dynamic.DynamicType;
import net.bytebuddy.dynamic.scaffold.MethodGraph;
import net.bytebuddy.implementation.FieldAccessor;
import net.bytebuddy.implementation.FixedValue;
import net.bytebuddy.implementation.Implementation;
import net.bytebuddy.implementation.StubMethod;
import org.hibernate.Version;
import org.hibernate.bytecode.enhance.VersionMismatchException;
import org.hibernate.bytecode.enhance.internal.tracker.CompositeOwnerTracker;
@ -39,23 +47,16 @@ import org.hibernate.engine.spi.SelfDirtinessTracker;
import org.hibernate.internal.CoreLogging;
import org.hibernate.internal.CoreMessageLogger;
import jakarta.persistence.Access;
import jakarta.persistence.AccessType;
import jakarta.persistence.metamodel.Type;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.annotation.AnnotationDescription;
import net.bytebuddy.description.annotation.AnnotationList;
import net.bytebuddy.description.field.FieldDescription;
import net.bytebuddy.description.field.FieldDescription.InDefinedShape;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDefinition;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.description.type.TypeDescription.Generic;
import net.bytebuddy.dynamic.DynamicType;
import net.bytebuddy.implementation.FieldAccessor;
import net.bytebuddy.implementation.FixedValue;
import net.bytebuddy.implementation.Implementation;
import net.bytebuddy.implementation.StubMethod;
import java.lang.annotation.Annotation;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Supplier;
import static net.bytebuddy.matcher.ElementMatchers.isDefaultFinalizer;
@ -170,6 +171,11 @@ public class EnhancerImpl implements Enhancer {
}
if ( enhancementContext.isEntityClass( managedCtClass ) ) {
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes
return null;
}
log.debugf( "Enhancing [%s] as Entity", managedCtClass.getName() );
DynamicType.Builder<?> builder = builderSupplier.get();
builder = builder.implement( ManagedEntity.class )
@ -329,6 +335,11 @@ public class EnhancerImpl implements Enhancer {
return createTransformer( managedCtClass ).applyTo( builder );
}
else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes
return null;
}
log.debugf( "Enhancing [%s] as Composite", managedCtClass.getName() );
DynamicType.Builder<?> builder = builderSupplier.get();
@ -362,6 +373,12 @@ public class EnhancerImpl implements Enhancer {
return createTransformer( managedCtClass ).applyTo( builder );
}
else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
// Check for HHH-16572 (PROPERTY attributes with mismatched field and method names)
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
return null;
}
log.debugf( "Enhancing [%s] as MappedSuperclass", managedCtClass.getName() );
DynamicType.Builder<?> builder = builderSupplier.get();
@ -378,6 +395,82 @@ public class EnhancerImpl implements Enhancer {
}
}
/**
* Check whether an entity class ({@code managedCtClass}) has mismatched names between a persistent field and its
* getter/setter when using {@link AccessType#PROPERTY}, which Hibernate does not currently support for enhancement.
* See https://hibernate.atlassian.net/browse/HHH-16572
*/
private boolean hasUnsupportedAttributeNaming(TypeDescription managedCtClass) {
// For process access rules, See https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#default-access-type
// and https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a122
//
// This check will determine if entity field names do not match Property accessor method name
// For example:
// @Entity
// class Book {
// Integer id;
// String smtg;
//
// @Id Integer getId() { return id; }
// String getSomething() { return smtg; }
// }
//
// Check name of the getter/setter method with persistence annotation and getter/setter method name that doesn't refer to an entity field
// and will return false. If the property accessor method(s) are named to match the field name(s), return true.
boolean propertyHasAnnotation = false;
MethodGraph.Linked methodGraph = MethodGraph.Compiler.Default.forJavaHierarchy().compile((TypeDefinition) managedCtClass);
for (MethodGraph.Node node : methodGraph.listNodes()) {
MethodDescription methodDescription = node.getRepresentative();
if (methodDescription.getDeclaringType().represents(Object.class)) { // skip class java.lang.Object methods
continue;
}
String methodName = methodDescription.getActualName();
if (methodName.equals("") ||
(!methodName.startsWith("get") && !methodName.startsWith("set") && !methodName.startsWith("is"))) {
continue;
}
String methodFieldName;
if (methodName.startsWith("is")) { // skip past "is"
methodFieldName = methodName.substring(2);
}
else if (methodName.startsWith("get") ||
methodName.startsWith("set")) { // skip past "get" or "set"
methodFieldName = methodName.substring(3);
}
else {
// not a property accessor method so ignore it
continue;
}
boolean propertyNameMatchesFieldName = false;
// convert field letter to lower case
methodFieldName = methodFieldName.substring(0, 1).toLowerCase() + methodFieldName.substring(1);
TypeList typeList = methodDescription.getDeclaredAnnotations().asTypeList();
if (typeList.stream().anyMatch(typeDefinitions ->
(typeDefinitions.getName().contains("jakarta.persistence")))) {
propertyHasAnnotation = true;
}
for (FieldDescription ctField : methodDescription.getDeclaringType().getDeclaredFields()) {
if (!Modifier.isStatic(ctField.getModifiers())) {
AnnotatedFieldDescription annotatedField = new AnnotatedFieldDescription(enhancementContext, ctField);
boolean containsPropertyAccessorMethods = false;
if (enhancementContext.isPersistentField(annotatedField)) {
if (methodFieldName.equals(ctField.getActualName())) {
propertyNameMatchesFieldName = true;
break;
}
}
}
}
if (propertyHasAnnotation && !propertyNameMatchesFieldName) {
log.debugf("Skipping enhancement of [%s]: due to class [%s] not having a property accessor method name matching field name [%s]",
managedCtClass, methodDescription.getDeclaringType().getActualName(), methodFieldName);
return true;
}
}
return false;
}
private static void verifyVersions(TypeDescription managedCtClass, ByteBuddyEnhancementContext enhancementContext) {
final AnnotationDescription.Loadable<EnhancementInfo> existingInfo = managedCtClass
.getDeclaredAnnotations()
@ -493,7 +586,7 @@ public class EnhancerImpl implements Enhancer {
AnnotatedFieldDescription annotatedField = new AnnotatedFieldDescription( enhancementContext, ctField );
if ( enhancementContext.isPersistentField( annotatedField ) && enhancementContext.isMappedCollection( annotatedField ) ) {
if ( ctField.getType().asErasure().isAssignableTo( Collection.class ) || ctField.getType().asErasure().isAssignableTo( Map.class ) ) {
collectionList.add( annotatedField );
collectionList.add( annotatedField );
}
}
}

View File

@ -0,0 +1,91 @@
/*
* SPDX-License-Identifier: LGPL-2.1-or-later
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.orm.test.bytecode.enhancement.access;
import jakarta.persistence.*;
import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
import org.hibernate.testing.orm.junit.*;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
@DomainModel(
annotatedClasses = {
InvalidPropertyNameTest.SomeEntity.class,
}
)
@SessionFactory
@JiraKey("HHH-16572")
@BytecodeEnhanced
public class InvalidPropertyNameTest {
@Test
@FailureExpected(jiraKey = "HHH-16572")
public void test(SessionFactoryScope scope) {
scope.inTransaction( session -> {
session.persist( new SomeEntity( 1L, "field", "property" ) );
} );
scope.inTransaction( session -> {
SomeEntity entity = session.get( SomeEntity.class, 1L );
assertThat( entity.property ).isEqualTo( "from getter: property" );
entity.setPropertyMethod( "updated" );
} );
scope.inTransaction( session -> {
SomeEntity entity = session.get( SomeEntity.class, 1L );
assertThat( entity.property ).isEqualTo( "from getter: updated" );
} );
}
@AfterEach
public void cleanup(SessionFactoryScope scope) {
// uncomment the following when @FailureExpected is removed above
// scope.inTransaction( session -> {
// session.remove( session.get( SomeEntity.class, 1L ) );
// PropertyAccessTest} );
}
@Entity
@Table(name = "SOME_ENTITY")
static class SomeEntity {
@Id
Long id;
@Basic
String field;
String property;
public SomeEntity() {
}
public SomeEntity(Long id, String field, String property) {
this.id = id;
this.field = field;
this.property = property;
}
/**
* The following property accessor methods are purposely named incorrectly to
* not match the "property" field. The HHH-16572 change ensures that
* this entity is not (bytecode) enhanced. Eventually further changes will be made
* such that this entity is enhanced in which case the FailureExpected can be removed
* and the cleanup() uncommented.
*/
@Basic
@Access(AccessType.PROPERTY)
public String getPropertyMethod() {
return "from getter: " + property;
}
public void setPropertyMethod(String property) {
this.property = property;
}
}
}

View File

@ -8,7 +8,6 @@ import org.hibernate.cfg.AvailableSettings;
import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.FailureExpected;
import org.hibernate.testing.orm.junit.JiraKey;
import org.hibernate.testing.orm.junit.ServiceRegistry;
import org.hibernate.testing.orm.junit.SessionFactory;
@ -77,7 +76,7 @@ public class LazyLoadingByEnhancerSetterTest {
}
@Test
@FailureExpected( jiraKey = "HHH-10747" )
// failure doesn't occur with HHH-16572 change @FailureExpected( jiraKey = "HHH-10747" )
public void testProperty(SessionFactoryScope scope) {
scope.inTransaction( s -> {
ItemProperty input = new ItemProperty();