HHH-18833 Introduce EnhancementContext#getUnsupportedEnhancementStrategy

This method allows custom contexts to pick the behavior they want when
a class contains getters/setters that do not have a matching field,
making enhancement impossible.

Three behaviors are available:

* SKIP (the default), which will skip enhancement of such classes.
* FAIL, which will throw an exception upon encountering such classes.
* LEGACY, which will restore the pre-HHH-16572 behavior.

I do not think LEGACY is useful at the moment, but I wanted to have
that option in case it turns out HHH-16572 does more harm than good in
Quarkus 3.15.
This commit is contained in:
Yoann Rodière 2024-11-08 15:11:58 +01:00
parent eb6cb31fdf
commit b5221e2ec6
5 changed files with 223 additions and 8 deletions

View File

@ -22,6 +22,7 @@ import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.dynamic.scaffold.MethodGraph; import net.bytebuddy.dynamic.scaffold.MethodGraph;
import net.bytebuddy.matcher.ElementMatcher; import net.bytebuddy.matcher.ElementMatcher;
import net.bytebuddy.pool.TypePool; import net.bytebuddy.pool.TypePool;
import org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy;
import static net.bytebuddy.matcher.ElementMatchers.isGetter; import static net.bytebuddy.matcher.ElementMatchers.isGetter;
@ -94,6 +95,10 @@ class ByteBuddyEnhancementContext {
enhancementContext.registerDiscoveredType( new UnloadedTypeDescription( typeDescription ), type ); enhancementContext.registerDiscoveredType( new UnloadedTypeDescription( typeDescription ), type );
} }
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
return enhancementContext.getUnsupportedEnhancementStrategy();
}
public void discoverCompositeTypes(TypeDescription managedCtClass, TypePool typePool) { public void discoverCompositeTypes(TypeDescription managedCtClass, TypePool typePool) {
if ( isDiscoveredType( managedCtClass ) ) { if ( isDiscoveredType( managedCtClass ) ) {
return; return;

View File

@ -23,6 +23,7 @@ import net.bytebuddy.implementation.FieldAccessor;
import net.bytebuddy.implementation.FixedValue; import net.bytebuddy.implementation.FixedValue;
import net.bytebuddy.implementation.Implementation; import net.bytebuddy.implementation.Implementation;
import net.bytebuddy.implementation.StubMethod; import net.bytebuddy.implementation.StubMethod;
import org.hibernate.AssertionFailure;
import org.hibernate.Version; import org.hibernate.Version;
import org.hibernate.bytecode.enhance.VersionMismatchException; import org.hibernate.bytecode.enhance.VersionMismatchException;
import org.hibernate.bytecode.enhance.internal.tracker.CompositeOwnerTracker; import org.hibernate.bytecode.enhance.internal.tracker.CompositeOwnerTracker;
@ -33,6 +34,7 @@ import org.hibernate.bytecode.enhance.spi.EnhancementInfo;
import org.hibernate.bytecode.enhance.spi.Enhancer; import org.hibernate.bytecode.enhance.spi.Enhancer;
import org.hibernate.bytecode.enhance.spi.EnhancerConstants; import org.hibernate.bytecode.enhance.spi.EnhancerConstants;
import org.hibernate.bytecode.enhance.spi.UnloadedField; import org.hibernate.bytecode.enhance.spi.UnloadedField;
import org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy;
import org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeLoadingInterceptor; import org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeLoadingInterceptor;
import org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState; import org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState;
import org.hibernate.engine.spi.CompositeOwner; import org.hibernate.engine.spi.CompositeOwner;
@ -171,7 +173,7 @@ public class EnhancerImpl implements Enhancer {
} }
if ( enhancementContext.isEntityClass( managedCtClass ) ) { if ( enhancementContext.isEntityClass( managedCtClass ) ) {
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) { if ( checkUnsupportedAttributeNaming( managedCtClass ) ) {
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes // do not enhance classes with mismatched names for PROPERTY-access persistent attributes
return null; return null;
} }
@ -335,7 +337,7 @@ public class EnhancerImpl implements Enhancer {
return createTransformer( managedCtClass ).applyTo( builder ); return createTransformer( managedCtClass ).applyTo( builder );
} }
else if ( enhancementContext.isCompositeClass( managedCtClass ) ) { else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) { if ( checkUnsupportedAttributeNaming( managedCtClass ) ) {
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes // do not enhance classes with mismatched names for PROPERTY-access persistent attributes
return null; return null;
} }
@ -375,7 +377,7 @@ public class EnhancerImpl implements Enhancer {
else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) { else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
// Check for HHH-16572 (PROPERTY attributes with mismatched field and method names) // Check for HHH-16572 (PROPERTY attributes with mismatched field and method names)
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) { if ( checkUnsupportedAttributeNaming( managedCtClass ) ) {
return null; return null;
} }
@ -399,8 +401,22 @@ public class EnhancerImpl implements Enhancer {
* Check whether an entity class ({@code managedCtClass}) has mismatched names between a persistent field and its * 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. * getter/setter when using {@link AccessType#PROPERTY}, which Hibernate does not currently support for enhancement.
* See https://hibernate.atlassian.net/browse/HHH-16572 * See https://hibernate.atlassian.net/browse/HHH-16572
*
* @return {@code true} if enhancement of the class must be {@link org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy#SKIP skipped}
* because it has mismatched names.
* {@code false} if enhancement of the class must proceed, either because it doesn't have any mismatched names,
* or because {@link org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy#LEGACY legacy mode} was opted into.
* @throws EnhancementException if enhancement of the class must {@link org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy#FAIL abort} because it has mismatched names.
*/ */
private boolean hasUnsupportedAttributeNaming(TypeDescription managedCtClass) { @SuppressWarnings("deprecation")
private boolean checkUnsupportedAttributeNaming(TypeDescription managedCtClass) {
var strategy = enhancementContext.getUnsupportedEnhancementStrategy();
if ( UnsupportedEnhancementStrategy.LEGACY.equals( strategy ) ) {
// Don't check anything and act as if there was nothing unsupported in the class.
// This is unsafe but that's what LEGACY is about.
return false;
}
// For process access rules, See https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#default-access-type // 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 // and https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a122
// //
@ -462,10 +478,23 @@ public class EnhancerImpl implements Enhancer {
} }
} }
} }
if (propertyHasAnnotation && !propertyNameMatchesFieldName) { if ( propertyHasAnnotation && !propertyNameMatchesFieldName ) {
log.debugf("Skipping enhancement of [%s]: due to class [%s] not having a property accessor method name matching field name [%s]", switch ( strategy ) {
managedCtClass, methodDescription.getDeclaringType().getActualName(), methodFieldName); 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; 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:
// We shouldn't even be in this method if using LEGACY, see top of this method.
throw new AssertionFailure( "Unexpected strategy at this point: " + strategy );
}
} }
} }
return false; return false;

View File

@ -5,6 +5,7 @@
package org.hibernate.bytecode.enhance.spi; package org.hibernate.bytecode.enhance.spi;
import jakarta.persistence.metamodel.Type; import jakarta.persistence.metamodel.Type;
import org.hibernate.Incubating;
/** /**
* The context for performing an enhancement. Enhancement can happen in any number of ways:<ul> * The context for performing an enhancement. Enhancement can happen in any number of ways:<ul>
@ -144,4 +145,15 @@ public interface EnhancementContext {
boolean isDiscoveredType(UnloadedClass classDescriptor); boolean isDiscoveredType(UnloadedClass classDescriptor);
void registerDiscoveredType(UnloadedClass classDescriptor, Type.PersistenceType type); void registerDiscoveredType(UnloadedClass classDescriptor, Type.PersistenceType type);
/**
* @return The expected behavior when encountering a class that cannot be enhanced,
* in particular when attribute names don't match field names.
* @see <a href="https://hibernate.atlassian.net/browse/HHH-16572">HHH-16572</a>
* @see <a href="https://hibernate.atlassian.net/browse/HHH-18833">HHH-18833</a>
*/
@Incubating
default UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
return UnsupportedEnhancementStrategy.SKIP;
}
} }

View File

@ -0,0 +1,38 @@
/*
* SPDX-License-Identifier: LGPL-2.1-or-later
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.bytecode.enhance.spi;
import org.hibernate.Incubating;
/**
* The expected behavior when encountering a class that cannot be enhanced,
* in particular when attribute names don't match field names.
*
* @see org.hibernate.bytecode.enhance.spi.EnhancementContext#getUnsupportedEnhancementStrategy
*/
@Incubating
public enum UnsupportedEnhancementStrategy {
/**
* When a class cannot be enhanced, skip enhancement for that class only.
*/
SKIP,
/**
* When a class cannot be enhanced, throw an exception with an actionable message.
*/
FAIL,
/**
* Legacy behavior: when a class cannot be enhanced, ignore that fact and try to enhance it anyway.
* <p>
* <strong>This is utterly unsafe and may cause errors, unpredictable behavior, and data loss.</strong>
* <p>
* Intended only for internal use in contexts with rigid backwards compatibility requirements.
*
* @deprecated Use {@link #SKIP} or {@link #FAIL} instead.
*/
@Deprecated
LEGACY
}

View File

@ -0,0 +1,131 @@
/*
* 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.Access;
import jakarta.persistence.AccessType;
import jakarta.persistence.Basic;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import org.hibernate.bytecode.enhance.internal.bytebuddy.EnhancerImpl;
import org.hibernate.bytecode.enhance.spi.EnhancementContext;
import org.hibernate.bytecode.enhance.spi.EnhancementException;
import org.hibernate.bytecode.enhance.spi.Enhancer;
import org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy;
import org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState;
import org.hibernate.bytecode.spi.ByteCodeHelper;
import org.hibernate.testing.bytecode.enhancement.EnhancerTestContext;
import org.hibernate.testing.orm.junit.JiraKey;
import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.io.InputStream;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@JiraKey("HHH-18833")
public class UnsupportedEnhancementStrategyTest {
@Test
public void skip() throws IOException {
var context = new EnhancerTestContext() {
@Override
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
// This is currently the default, but we don't care about what's the default here
return UnsupportedEnhancementStrategy.SKIP;
}
};
byte[] originalBytes = getAsBytes( SomeEntity.class );
byte[] enhancedBytes = doEnhance( SomeEntity.class, originalBytes, context );
assertThat( enhancedBytes ).isNull(); // null means "not enhanced"
}
@Test
public void fail() throws IOException {
var context = new EnhancerTestContext() {
@Override
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
return UnsupportedEnhancementStrategy.FAIL;
}
};
byte[] originalBytes = getAsBytes( SomeEntity.class );
assertThatThrownBy( () -> doEnhance( SomeEntity.class, originalBytes, context ) )
.isInstanceOf( EnhancementException.class )
.hasMessageContainingAll(
String.format(
"Enhancement of [%s] failed because no field named [%s] could be found for property accessor method [%s].",
SomeEntity.class.getName(), "propertyMethod", "getPropertyMethod" ),
"To fix this, make sure all property accessor methods have a matching field."
);
}
@Test
@SuppressWarnings("deprecation")
public void legacy() throws IOException {
var context = new EnhancerTestContext() {
@Override
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
// This is currently the default, but we don't care about what's the default here
return UnsupportedEnhancementStrategy.LEGACY;
}
};
byte[] originalBytes = getAsBytes( SomeEntity.class );
byte[] enhancedBytes = doEnhance( SomeEntity.class, originalBytes, context );
assertThat( enhancedBytes ).isNotNull(); // non-null means enhancement _was_ performed
}
private byte[] doEnhance(Class<SomeEntity> someEntityClass, byte[] originalBytes, EnhancementContext context) {
final ByteBuddyState byteBuddyState = new ByteBuddyState();
final Enhancer enhancer = new EnhancerImpl( context, byteBuddyState );
return enhancer.enhance( someEntityClass.getName(), originalBytes );
}
private byte[] getAsBytes(Class<?> clazz) throws IOException {
final String classFile = clazz.getName().replace( '.', '/' ) + ".class";
try (InputStream classFileStream = clazz.getClassLoader().getResourceAsStream( classFile )) {
return ByteCodeHelper.readByteCode( classFileStream );
}
}
@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;
}
}
}