From abfe6b9b556f3748a657f017a7ac2ec45a88197b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Fri, 8 Nov 2024 15:11:58 +0100 Subject: [PATCH] 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. --- .../ByteBuddyEnhancementContext.java | 5 + .../internal/bytebuddy/EnhancerImpl.java | 45 ++++-- .../enhance/spi/EnhancementContext.java | 12 ++ .../spi/UnsupportedEnhancementStrategy.java | 40 ++++++ .../UnsupportedEnhancementStrategyTest.java | 131 ++++++++++++++++++ 5 files changed, 225 insertions(+), 8 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/UnsupportedEnhancementStrategy.java create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/access/UnsupportedEnhancementStrategyTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/ByteBuddyEnhancementContext.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/ByteBuddyEnhancementContext.java index 5e495b30f1..dd2097ae0b 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/ByteBuddyEnhancementContext.java +++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/ByteBuddyEnhancementContext.java @@ -24,6 +24,7 @@ import net.bytebuddy.dynamic.scaffold.MethodGraph; import net.bytebuddy.matcher.ElementMatcher; import net.bytebuddy.pool.TypePool; +import org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy; import static net.bytebuddy.matcher.ElementMatchers.isGetter; @@ -106,6 +107,10 @@ public void registerDiscoveredType(TypeDescription typeDescription, Type.Persist enhancementContext.registerDiscoveredType( new UnloadedTypeDescription( typeDescription ), type ); } + public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() { + return enhancementContext.getUnsupportedEnhancementStrategy(); + } + public void discoverCompositeTypes(TypeDescription managedCtClass, TypePool typePool) { if ( isDiscoveredType( managedCtClass ) ) { return; diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java index 7595a8ae7a..3f6f5c999e 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java @@ -25,6 +25,7 @@ import net.bytebuddy.implementation.FixedValue; import net.bytebuddy.implementation.Implementation; import net.bytebuddy.implementation.StubMethod; +import org.hibernate.AssertionFailure; import org.hibernate.Version; import org.hibernate.bytecode.enhance.VersionMismatchException; import org.hibernate.bytecode.enhance.internal.tracker.CompositeOwnerTracker; @@ -35,6 +36,7 @@ import org.hibernate.bytecode.enhance.spi.Enhancer; import org.hibernate.bytecode.enhance.spi.EnhancerConstants; 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.internal.bytebuddy.ByteBuddyState; import org.hibernate.engine.spi.CompositeOwner; @@ -173,7 +175,7 @@ private DynamicType.Builder doEnhance(Supplier> builde } if ( enhancementContext.isEntityClass( managedCtClass ) ) { - if ( hasUnsupportedAttributeNaming( managedCtClass ) ) { + if ( checkUnsupportedAttributeNaming( managedCtClass ) ) { // do not enhance classes with mismatched names for PROPERTY-access persistent attributes return null; } @@ -337,7 +339,7 @@ private DynamicType.Builder doEnhance(Supplier> builde return createTransformer( managedCtClass ).applyTo( builder ); } else if ( enhancementContext.isCompositeClass( managedCtClass ) ) { - if ( hasUnsupportedAttributeNaming( managedCtClass ) ) { + if ( checkUnsupportedAttributeNaming( managedCtClass ) ) { // do not enhance classes with mismatched names for PROPERTY-access persistent attributes return null; } @@ -377,7 +379,7 @@ else if ( enhancementContext.isCompositeClass( managedCtClass ) ) { else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) { // Check for HHH-16572 (PROPERTY attributes with mismatched field and method names) - if ( hasUnsupportedAttributeNaming( managedCtClass ) ) { + if ( checkUnsupportedAttributeNaming( managedCtClass ) ) { return null; } @@ -401,8 +403,22 @@ else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) { * 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 + * + * @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 // and https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a122 // @@ -464,10 +480,23 @@ else if (methodName.startsWith("get") || } } } - 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; + if ( propertyHasAnnotation && !propertyNameMatchesFieldName ) { + 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: + // 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; diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/EnhancementContext.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/EnhancementContext.java index 06086c65d2..453407f38c 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/EnhancementContext.java +++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/EnhancementContext.java @@ -7,6 +7,7 @@ package org.hibernate.bytecode.enhance.spi; import jakarta.persistence.metamodel.Type; +import org.hibernate.Incubating; /** * The context for performing an enhancement. Enhancement can happen in any number of ways:
    @@ -146,4 +147,15 @@ public interface EnhancementContext { boolean isDiscoveredType(UnloadedClass classDescriptor); 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 HHH-16572 + * @see HHH-18833 + */ + @Incubating + default UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() { + return UnsupportedEnhancementStrategy.SKIP; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/UnsupportedEnhancementStrategy.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/UnsupportedEnhancementStrategy.java new file mode 100644 index 0000000000..ce5c97883f --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/UnsupportedEnhancementStrategy.java @@ -0,0 +1,40 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +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. + *

    + * This is utterly unsafe and may cause errors, unpredictable behavior, and data loss. + *

    + * Intended only for internal use in contexts with rigid backwards compatibility requirements. + * + * @deprecated Use {@link #SKIP} or {@link #FAIL} instead. + */ + @Deprecated + LEGACY + +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/access/UnsupportedEnhancementStrategyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/access/UnsupportedEnhancementStrategyTest.java new file mode 100644 index 0000000000..902b19c959 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/access/UnsupportedEnhancementStrategyTest.java @@ -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 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; + } + } +}