Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit b5221e2

Browse files
yrodiereyrodiere
committedNov 13, 2024·
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.
1 parent eb6cb31 commit b5221e2

File tree

5 files changed

+223
-8
lines changed

5 files changed

+223
-8
lines changed
 

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import net.bytebuddy.dynamic.scaffold.MethodGraph;
2323
import net.bytebuddy.matcher.ElementMatcher;
2424
import net.bytebuddy.pool.TypePool;
25+
import org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy;
2526

2627
import static net.bytebuddy.matcher.ElementMatchers.isGetter;
2728

@@ -94,6 +95,10 @@ public void registerDiscoveredType(TypeDescription typeDescription, Type.Persist
9495
enhancementContext.registerDiscoveredType( new UnloadedTypeDescription( typeDescription ), type );
9596
}
9697

98+
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
99+
return enhancementContext.getUnsupportedEnhancementStrategy();
100+
}
101+
97102
public void discoverCompositeTypes(TypeDescription managedCtClass, TypePool typePool) {
98103
if ( isDiscoveredType( managedCtClass ) ) {
99104
return;

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

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import net.bytebuddy.implementation.FixedValue;
2424
import net.bytebuddy.implementation.Implementation;
2525
import net.bytebuddy.implementation.StubMethod;
26+
import org.hibernate.AssertionFailure;
2627
import org.hibernate.Version;
2728
import org.hibernate.bytecode.enhance.VersionMismatchException;
2829
import org.hibernate.bytecode.enhance.internal.tracker.CompositeOwnerTracker;
@@ -33,6 +34,7 @@
3334
import org.hibernate.bytecode.enhance.spi.Enhancer;
3435
import org.hibernate.bytecode.enhance.spi.EnhancerConstants;
3536
import org.hibernate.bytecode.enhance.spi.UnloadedField;
37+
import org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy;
3638
import org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeLoadingInterceptor;
3739
import org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState;
3840
import org.hibernate.engine.spi.CompositeOwner;
@@ -171,7 +173,7 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
171173
}
172174

173175
if ( enhancementContext.isEntityClass( managedCtClass ) ) {
174-
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
176+
if ( checkUnsupportedAttributeNaming( managedCtClass ) ) {
175177
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes
176178
return null;
177179
}
@@ -335,7 +337,7 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
335337
return createTransformer( managedCtClass ).applyTo( builder );
336338
}
337339
else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
338-
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
340+
if ( checkUnsupportedAttributeNaming( managedCtClass ) ) {
339341
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes
340342
return null;
341343
}
@@ -375,7 +377,7 @@ else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
375377
else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
376378

377379
// Check for HHH-16572 (PROPERTY attributes with mismatched field and method names)
378-
if ( hasUnsupportedAttributeNaming( managedCtClass ) ) {
380+
if ( checkUnsupportedAttributeNaming( managedCtClass ) ) {
379381
return null;
380382
}
381383

@@ -399,8 +401,22 @@ else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
399401
* Check whether an entity class ({@code managedCtClass}) has mismatched names between a persistent field and its
400402
* getter/setter when using {@link AccessType#PROPERTY}, which Hibernate does not currently support for enhancement.
401403
* See https://hibernate.atlassian.net/browse/HHH-16572
404+
*
405+
* @return {@code true} if enhancement of the class must be {@link org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy#SKIP skipped}
406+
* because it has mismatched names.
407+
* {@code false} if enhancement of the class must proceed, either because it doesn't have any mismatched names,
408+
* or because {@link org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy#LEGACY legacy mode} was opted into.
409+
* @throws EnhancementException if enhancement of the class must {@link org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy#FAIL abort} because it has mismatched names.
402410
*/
403-
private boolean hasUnsupportedAttributeNaming(TypeDescription managedCtClass) {
411+
@SuppressWarnings("deprecation")
412+
private boolean checkUnsupportedAttributeNaming(TypeDescription managedCtClass) {
413+
var strategy = enhancementContext.getUnsupportedEnhancementStrategy();
414+
if ( UnsupportedEnhancementStrategy.LEGACY.equals( strategy ) ) {
415+
// Don't check anything and act as if there was nothing unsupported in the class.
416+
// This is unsafe but that's what LEGACY is about.
417+
return false;
418+
}
419+
404420
// For process access rules, See https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#default-access-type
405421
// and https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a122
406422
//
@@ -462,10 +478,23 @@ else if (methodName.startsWith("get") ||
462478
}
463479
}
464480
}
465-
if (propertyHasAnnotation && !propertyNameMatchesFieldName) {
466-
log.debugf("Skipping enhancement of [%s]: due to class [%s] not having a property accessor method name matching field name [%s]",
467-
managedCtClass, methodDescription.getDeclaringType().getActualName(), methodFieldName);
468-
return true;
481+
if ( propertyHasAnnotation && !propertyNameMatchesFieldName ) {
482+
switch ( strategy ) {
483+
case SKIP:
484+
log.debugf(
485+
"Skipping enhancement of [%s] because no field named [%s] could be found for property accessor method [%s]."
486+
+ " To fix this, make sure all property accessor methods have a matching field.",
487+
managedCtClass.getName(), methodFieldName, methodDescription.getName() );
488+
return true;
489+
case FAIL:
490+
throw new EnhancementException( String.format(
491+
"Enhancement of [%s] failed because no field named [%s] could be found for property accessor method [%s]."
492+
+ " To fix this, make sure all property accessor methods have a matching field.",
493+
managedCtClass.getName(), methodFieldName, methodDescription.getName() ) );
494+
default:
495+
// We shouldn't even be in this method if using LEGACY, see top of this method.
496+
throw new AssertionFailure( "Unexpected strategy at this point: " + strategy );
497+
}
469498
}
470499
}
471500
return false;

‎hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/EnhancementContext.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package org.hibernate.bytecode.enhance.spi;
66

77
import jakarta.persistence.metamodel.Type;
8+
import org.hibernate.Incubating;
89

910
/**
1011
* The context for performing an enhancement. Enhancement can happen in any number of ways:<ul>
@@ -144,4 +145,15 @@ public interface EnhancementContext {
144145
boolean isDiscoveredType(UnloadedClass classDescriptor);
145146

146147
void registerDiscoveredType(UnloadedClass classDescriptor, Type.PersistenceType type);
148+
149+
/**
150+
* @return The expected behavior when encountering a class that cannot be enhanced,
151+
* in particular when attribute names don't match field names.
152+
* @see <a href="https://hibernate.atlassian.net/browse/HHH-16572">HHH-16572</a>
153+
* @see <a href="https://hibernate.atlassian.net/browse/HHH-18833">HHH-18833</a>
154+
*/
155+
@Incubating
156+
default UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
157+
return UnsupportedEnhancementStrategy.SKIP;
158+
}
147159
}

‎hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/UnsupportedEnhancementStrategy.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* SPDX-License-Identifier: LGPL-2.1-or-later
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.bytecode.enhance.spi;
6+
7+
import org.hibernate.Incubating;
8+
9+
/**
10+
* The expected behavior when encountering a class that cannot be enhanced,
11+
* in particular when attribute names don't match field names.
12+
*
13+
* @see org.hibernate.bytecode.enhance.spi.EnhancementContext#getUnsupportedEnhancementStrategy
14+
*/
15+
@Incubating
16+
public enum UnsupportedEnhancementStrategy {
17+
18+
/**
19+
* When a class cannot be enhanced, skip enhancement for that class only.
20+
*/
21+
SKIP,
22+
/**
23+
* When a class cannot be enhanced, throw an exception with an actionable message.
24+
*/
25+
FAIL,
26+
/**
27+
* Legacy behavior: when a class cannot be enhanced, ignore that fact and try to enhance it anyway.
28+
* <p>
29+
* <strong>This is utterly unsafe and may cause errors, unpredictable behavior, and data loss.</strong>
30+
* <p>
31+
* Intended only for internal use in contexts with rigid backwards compatibility requirements.
32+
*
33+
* @deprecated Use {@link #SKIP} or {@link #FAIL} instead.
34+
*/
35+
@Deprecated
36+
LEGACY
37+
38+
}

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* SPDX-License-Identifier: LGPL-2.1-or-later
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.bytecode.enhancement.access;
6+
7+
import jakarta.persistence.Access;
8+
import jakarta.persistence.AccessType;
9+
import jakarta.persistence.Basic;
10+
import jakarta.persistence.Entity;
11+
import jakarta.persistence.Id;
12+
import jakarta.persistence.Table;
13+
import org.hibernate.bytecode.enhance.internal.bytebuddy.EnhancerImpl;
14+
import org.hibernate.bytecode.enhance.spi.EnhancementContext;
15+
import org.hibernate.bytecode.enhance.spi.EnhancementException;
16+
import org.hibernate.bytecode.enhance.spi.Enhancer;
17+
import org.hibernate.bytecode.enhance.spi.UnsupportedEnhancementStrategy;
18+
import org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState;
19+
import org.hibernate.bytecode.spi.ByteCodeHelper;
20+
import org.hibernate.testing.bytecode.enhancement.EnhancerTestContext;
21+
import org.hibernate.testing.orm.junit.JiraKey;
22+
import org.junit.jupiter.api.Test;
23+
24+
import java.io.IOException;
25+
import java.io.InputStream;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
29+
30+
@JiraKey("HHH-18833")
31+
public class UnsupportedEnhancementStrategyTest {
32+
33+
@Test
34+
public void skip() throws IOException {
35+
var context = new EnhancerTestContext() {
36+
@Override
37+
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
38+
// This is currently the default, but we don't care about what's the default here
39+
return UnsupportedEnhancementStrategy.SKIP;
40+
}
41+
};
42+
byte[] originalBytes = getAsBytes( SomeEntity.class );
43+
byte[] enhancedBytes = doEnhance( SomeEntity.class, originalBytes, context );
44+
assertThat( enhancedBytes ).isNull(); // null means "not enhanced"
45+
}
46+
47+
@Test
48+
public void fail() throws IOException {
49+
var context = new EnhancerTestContext() {
50+
@Override
51+
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
52+
return UnsupportedEnhancementStrategy.FAIL;
53+
}
54+
};
55+
byte[] originalBytes = getAsBytes( SomeEntity.class );
56+
assertThatThrownBy( () -> doEnhance( SomeEntity.class, originalBytes, context ) )
57+
.isInstanceOf( EnhancementException.class )
58+
.hasMessageContainingAll(
59+
String.format(
60+
"Enhancement of [%s] failed because no field named [%s] could be found for property accessor method [%s].",
61+
SomeEntity.class.getName(), "propertyMethod", "getPropertyMethod" ),
62+
"To fix this, make sure all property accessor methods have a matching field."
63+
);
64+
}
65+
66+
@Test
67+
@SuppressWarnings("deprecation")
68+
public void legacy() throws IOException {
69+
var context = new EnhancerTestContext() {
70+
@Override
71+
public UnsupportedEnhancementStrategy getUnsupportedEnhancementStrategy() {
72+
// This is currently the default, but we don't care about what's the default here
73+
return UnsupportedEnhancementStrategy.LEGACY;
74+
}
75+
};
76+
byte[] originalBytes = getAsBytes( SomeEntity.class );
77+
byte[] enhancedBytes = doEnhance( SomeEntity.class, originalBytes, context );
78+
assertThat( enhancedBytes ).isNotNull(); // non-null means enhancement _was_ performed
79+
}
80+
81+
private byte[] doEnhance(Class<SomeEntity> someEntityClass, byte[] originalBytes, EnhancementContext context) {
82+
final ByteBuddyState byteBuddyState = new ByteBuddyState();
83+
final Enhancer enhancer = new EnhancerImpl( context, byteBuddyState );
84+
return enhancer.enhance( someEntityClass.getName(), originalBytes );
85+
}
86+
87+
private byte[] getAsBytes(Class<?> clazz) throws IOException {
88+
final String classFile = clazz.getName().replace( '.', '/' ) + ".class";
89+
try (InputStream classFileStream = clazz.getClassLoader().getResourceAsStream( classFile )) {
90+
return ByteCodeHelper.readByteCode( classFileStream );
91+
}
92+
}
93+
94+
@Entity
95+
@Table(name = "SOME_ENTITY")
96+
static class SomeEntity {
97+
@Id
98+
Long id;
99+
100+
@Basic
101+
String field;
102+
103+
String property;
104+
105+
public SomeEntity() {
106+
}
107+
108+
public SomeEntity(Long id, String field, String property) {
109+
this.id = id;
110+
this.field = field;
111+
this.property = property;
112+
}
113+
114+
/**
115+
* The following property accessor methods are purposely named incorrectly to
116+
* not match the "property" field. The HHH-16572 change ensures that
117+
* this entity is not (bytecode) enhanced. Eventually further changes will be made
118+
* such that this entity is enhanced in which case the FailureExpected can be removed
119+
* and the cleanup() uncommented.
120+
*/
121+
@Basic
122+
@Access(AccessType.PROPERTY)
123+
public String getPropertyMethod() {
124+
return "from getter: " + property;
125+
}
126+
127+
public void setPropertyMethod(String property) {
128+
this.property = property;
129+
}
130+
}
131+
}

0 commit comments

Comments
 (0)
Please sign in to comment.