Skip to content

HHH-19059 Bytecode enhancement fails when inherited fields are mapped using property access in subclass #9925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 28, 2025
Merged

HHH-19059 Bytecode enhancement fails when inherited fields are mapped using property access in subclass #9925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e24c29
HHH-19059 Add test for issue
mbellade Mar 18, 2025
9d821a9
HHH-19059 Fix check for property access fields on hierarchies
mbellade Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 49 additions & 57 deletions ...te-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.MappedSuperclass;
import jakarta.persistence.Transient;
import jakarta.persistence.metamodel.Type;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.annotation.AnnotationDescription;
Expand Down Expand Up @@ -74,6 +75,9 @@
import static net.bytebuddy.matcher.ElementMatchers.isDefaultFinalizer;
import static net.bytebuddy.matcher.ElementMatchers.isGetter;
import static net.bytebuddy.matcher.ElementMatchers.isSetter;
import static net.bytebuddy.matcher.ElementMatchers.isStatic;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.not;

public class EnhancerImpl implements Enhancer {

Expand Down Expand Up @@ -476,21 +480,13 @@ private static boolean hasMappingAnnotation(AnnotationList annotations) {
|| annotations.isAnnotationPresent( Embeddable.class );
}

private static boolean hasPersistenceAnnotation(AnnotationList annotations) {
boolean found = false;
for ( AnnotationDescription annotation : annotations ) {
final String annotationName = annotation.getAnnotationType().getName();
if ( annotationName.startsWith( "jakarta.persistence" ) ) {
if ( annotationName.equals( "jakarta.persistence.Transient" ) ) {
// transient property so ignore it
return false;
}
else if ( !found && !IGNORED_PERSISTENCE_ANNOTATIONS.contains( annotationName ) ) {
found = true;
}
}
private static boolean isPersistentMethod(MethodDescription method) {
final AnnotationList annotations = method.getDeclaredAnnotations();
if ( annotations.isAnnotationPresent( Transient.class ) ) {
return false;
}
return found;

return annotations.stream().noneMatch( a -> IGNORED_PERSISTENCE_ANNOTATIONS.contains( a.getAnnotationType().getName() ) );
}

private static final Set<String> IGNORED_PERSISTENCE_ANNOTATIONS = Set.of(
Expand All @@ -503,6 +499,17 @@ else if ( !found && !IGNORED_PERSISTENCE_ANNOTATIONS.contains( annotationName )
"jakarta.persistence.PreUpdate"
);

private static boolean containsField(Generic type, String fieldName) {
do {
if ( !type.getDeclaredFields().filter( not( isStatic() ).and( named( fieldName ) ) ).isEmpty() ) {
return true;
}
type = type.getSuperClass();
}
while ( type != null && !type.represents( Object.class ) );
return false;
}

/**
* 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.
Expand Down Expand Up @@ -545,61 +552,46 @@ private static boolean checkUnsupportedAttributeNaming(TypeDescription managedCt
.asMethodList()
.filter( isGetter().or( isSetter() ) );
for ( final MethodDescription methodDescription : methods ) {
if ( determineAccessType( methodDescription, defaultAccessType ) != AccessType.PROPERTY ) {
if ( methodDescription.getDeclaringType().represents( Object.class )
|| determineAccessType( methodDescription, defaultAccessType ) != AccessType.PROPERTY ) {
// We only need to check this for AccessType.PROPERTY
continue;
}

final String methodName = methodDescription.getActualName();
String methodFieldName;
String fieldName;
if ( methodName.startsWith( "get" ) || methodName.startsWith( "set" ) ) {
methodFieldName = methodName.substring( 3 );
fieldName = methodName.substring( 3 );
}
else {
assert methodName.startsWith( "is" );
methodFieldName = methodName.substring( 2 );
fieldName = methodName.substring( 2 );
}
// convert first field letter to lower case
methodFieldName = getJavaBeansFieldName( methodFieldName );
if ( methodFieldName != null && hasPersistenceAnnotation( methodDescription.getDeclaredAnnotations() ) ) {
boolean propertyNameMatchesFieldName = false;
for ( final FieldDescription field : methodDescription.getDeclaringType().getDeclaredFields() ) {
if ( !Modifier.isStatic( field.getModifiers() ) ) {
final AnnotatedFieldDescription annotatedField = new AnnotatedFieldDescription(
enhancementContext,
field
fieldName = getJavaBeansFieldName( fieldName );
if ( fieldName != null && isPersistentMethod( methodDescription )
&& !containsField( managedCtClass.asGenericType(), fieldName ) ) {
// We shouldn't even be in this method if using LEGACY, see top of this method.
switch ( strategy ) {
case SKIP:
log.debugf(
"Skipping enhancement of [%s] because no field named [%s] could be found for property accessor method [%s]."
+ " To fix this, make sure all property accessor methods have a matching field.",
managedCtClass.getName(),
fieldName,
methodDescription.getName()
);
if ( enhancementContext.isPersistentField( annotatedField ) ) {
if ( methodFieldName.equals( field.getActualName() ) ) {
propertyNameMatchesFieldName = true;
break;
}
}
}
}
if ( !propertyNameMatchesFieldName ) {
// We shouldn't even be in this method if using LEGACY, see top of this method.
switch ( strategy ) {
case SKIP:
log.debugf(
"Skipping enhancement of [%s] because no field named [%s] could be found for property accessor method [%s]."
+ " To fix this, make sure all property accessor methods have a matching field.",
managedCtClass.getName(),
methodFieldName,
methodDescription.getName()
);
return true;
case FAIL:
throw new EnhancementException( String.format(
"Enhancement of [%s] failed because no field named [%s] could be found for property accessor method [%s]."
+ " To fix this, make sure all property accessor methods have a matching field.",
managedCtClass.getName(),
methodFieldName,
methodDescription.getName()
) );
default:
throw new AssertionFailure( "Unexpected strategy at this point: " + strategy );
}
return true;
case FAIL:
throw new EnhancementException( String.format(
"Enhancement of [%s] failed because no field named [%s] could be found for property accessor method [%s]."
+ " To fix this, make sure all property accessor methods have a matching field.",
managedCtClass.getName(),
fieldName,
methodDescription.getName()
) );
default:
throw new AssertionFailure( "Unexpected strategy at this point: " + strategy );
}
}
}
Expand Down
125 changes: 60 additions & 65 deletions .../java/org/hibernate/orm/test/bytecode/enhancement/access/HierarchyPropertyAccessTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,110 +4,99 @@
*/
package org.hibernate.orm.test.bytecode.enhancement.access;

import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.JiraKey;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;

import jakarta.persistence.Access;
import jakarta.persistence.AccessType;
import jakarta.persistence.Basic;
import jakarta.persistence.DiscriminatorColumn;
import jakarta.persistence.DiscriminatorValue;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Inheritance;
import jakarta.persistence.Table;
import jakarta.persistence.MappedSuperclass;
import jakarta.persistence.Transient;
import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.Jira;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

@DomainModel(
annotatedClasses = {
HierarchyPropertyAccessTest.ChildEntity.class,
}
)
@DomainModel(annotatedClasses = {
HierarchyPropertyAccessTest.AbstractSuperclass.class,
HierarchyPropertyAccessTest.ParentEntity.class,
HierarchyPropertyAccessTest.ChildEntity.class,
})
@SessionFactory
@JiraKey("HHH-19140")
@Jira( "https://hibernate.atlassian.net/browse/HHH-19140" )
@Jira( "https://hibernate.atlassian.net/browse/HHH-19059" )
@BytecodeEnhanced
public class HierarchyPropertyAccessTest {


@Test
public void testParent(SessionFactoryScope scope) {
scope.inTransaction( session -> {
session.persist( new ParentEntity( 1L, "field", "transient: property" ) );
} );
assertThat( scope.getSessionFactory().getMappingMetamodel().findEntityDescriptor( ParentEntity.class )
.getBytecodeEnhancementMetadata().isEnhancedForLazyLoading() ).isTrue();
scope.inTransaction( session -> session.persist( new ParentEntity( 1L, "field", "transient: property" ) ) );

scope.inTransaction( session -> {
ParentEntity entity = session.get( ParentEntity.class, 1L );
assertThat( entity.persistProperty ).isEqualTo( "property" );
assertThat( entity.property ).isEqualTo( "transient: property" );
final ParentEntity entity = session.find( ParentEntity.class, 1L );
assertThat( entity.getPersistProperty() ).isEqualTo( "property" );
assertThat( entity.getProperty() ).isEqualTo( "transient: property" );
assertThat( entity.getSuperProperty() ).isEqualTo( 8 );

entity.setProperty( "transient: updated" );
} );

scope.inTransaction( session -> {
ParentEntity entity = session.get( ParentEntity.class, 1L );
assertThat( entity.persistProperty ).isEqualTo( "updated" );
assertThat( entity.property ).isEqualTo( "transient: updated" );
final ParentEntity entity = session.find( ParentEntity.class, 1L );
assertThat( entity.getPersistProperty() ).isEqualTo( "updated" );
assertThat( entity.getProperty() ).isEqualTo( "transient: updated" );
} );
}

@Test
public void testChild(SessionFactoryScope scope) {
scope.inTransaction( session -> {
session.persist( new ChildEntity( 2L, "field", "transient: property" ) );
} );
assertThat( scope.getSessionFactory().getMappingMetamodel().findEntityDescriptor( ChildEntity.class )
.getBytecodeEnhancementMetadata().isEnhancedForLazyLoading() ).isTrue();
scope.inTransaction( session -> session.persist( new ChildEntity( 2L, "field", "transient: property" ) ) );

scope.inTransaction( session -> {
ChildEntity entity = session.get( ChildEntity.class, 2L );
assertThat( entity.persistProperty ).isEqualTo( "property" );
assertThat( entity.property ).isEqualTo( "transient: property" );
ChildEntity entity = session.find( ChildEntity.class, 2L );
assertThat( entity.getPersistProperty() ).isEqualTo( "property" );
assertThat( entity.getProperty() ).isEqualTo( "transient: property" );
assertThat( entity.getSuperProperty() ).isEqualTo( 8 );

entity.setProperty( "transient: updated" );
} );

scope.inTransaction( session -> {
ChildEntity entity = session.get( ChildEntity.class, 2L );
assertThat( entity.persistProperty ).isEqualTo( "updated" );
assertThat( entity.property ).isEqualTo( "transient: updated" );
ChildEntity entity = session.find( ChildEntity.class, 2L );
assertThat( entity.getPersistProperty() ).isEqualTo( "updated" );
assertThat( entity.getProperty() ).isEqualTo( "transient: updated" );
} );
}

@AfterEach
@AfterAll
public void cleanup(SessionFactoryScope scope) {
scope.inTransaction( session -> {
ParentEntity parentEntity = session.get( ParentEntity.class, 1L );
if (parentEntity != null) {
session.remove( parentEntity );
}
ChildEntity childEntity = session.get( ChildEntity.class, 2L );
if (childEntity != null) {
session.remove( childEntity );
}
} );
scope.getSessionFactory().getSchemaManager().truncateMappedObjects();
}

@MappedSuperclass
static abstract class AbstractSuperclass {
protected Integer superProperty;
}

@Entity
@Table(name = "PARENT_ENTITY")
@Inheritance
@DiscriminatorColumn(name = "type")
@DiscriminatorValue("Parent")
static class ParentEntity {
@Entity(name = "ParentEntity")
@DiscriminatorColumn(name = "entity_type")
static class ParentEntity extends AbstractSuperclass {
@Id
Long id;
private Long id;

@Basic
String field;
private String field;

String persistProperty;
private String persistProperty;

@Transient
String property;
private String property;

public ParentEntity() {
}
Expand All @@ -118,7 +107,6 @@ public ParentEntity(Long id, String field, String property) {
this.property = property;
}

@Basic
@Access(AccessType.PROPERTY)
public String getPersistProperty() {
this.persistProperty = this.property.substring( 11 );
Expand All @@ -137,17 +125,24 @@ public String getProperty() {
public void setProperty(String property) {
this.property = property;
}

@Access(AccessType.PROPERTY)
public Integer getSuperProperty() {
return getPersistProperty().length();
}

public void setSuperProperty(Integer superProperty) {
this.superProperty = superProperty;
}
}

@Entity
@DiscriminatorValue("Child")
@Entity(name = "ChildEntity")
static class ChildEntity extends ParentEntity {

public ChildEntity() {
}

public ChildEntity(Long id, String field, String property) {
super(id, field, property);
super( id, field, property );
}
}
}
Loading
Loading