Skip to content

Commit 3e651ab

Browse files
sebersolesebersole
committed
HHH-19662 - Define granularity of EnhancementContext options matching reality
HHH-19168 - Disallow re-enhancement of entities with different configuration # Conflicts: # hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/EnhancementContext.java
1 parent dbee9cb commit 3e651ab

15 files changed

+522
-155
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ static Implementation wrap(
4747
ByteBuddyEnhancementContext enhancementContext,
4848
AnnotatedFieldDescription persistentField,
4949
Implementation implementation) {
50-
if ( !enhancementContext.doBiDirectionalAssociationManagement( persistentField ) ) {
50+
if ( !enhancementContext.doBiDirectionalAssociationManagement() ) {
5151
return implementation;
5252
}
5353

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,12 @@ public boolean isMappedSuperclassClass(TypeDescription classDescriptor) {
5151
return enhancementContext.isMappedSuperclassClass( new UnloadedTypeDescription( classDescriptor ) );
5252
}
5353

54-
public boolean doDirtyCheckingInline(TypeDescription classDescriptor) {
55-
return enhancementContext.doDirtyCheckingInline( new UnloadedTypeDescription( classDescriptor ) );
54+
public boolean doDirtyCheckingInline() {
55+
return enhancementContext.doDirtyCheckingInline();
5656
}
5757

58-
public boolean doExtendedEnhancement(TypeDescription classDescriptor) {
59-
return enhancementContext.doExtendedEnhancement( new UnloadedTypeDescription( classDescriptor ) );
58+
public boolean doExtendedEnhancement() {
59+
return enhancementContext.doExtendedEnhancement();
6060
}
6161

6262
public boolean hasLazyLoadableAttributes(TypeDescription classDescriptor) {
@@ -83,8 +83,8 @@ public boolean isMappedCollection(AnnotatedFieldDescription field) {
8383
return enhancementContext.isMappedCollection( field );
8484
}
8585

86-
public boolean doBiDirectionalAssociationManagement(AnnotatedFieldDescription field) {
87-
return enhancementContext.doBiDirectionalAssociationManagement( field );
86+
public boolean doBiDirectionalAssociationManagement() {
87+
return enhancementContext.doBiDirectionalAssociationManagement();
8888
}
8989

9090
public boolean isDiscoveredType(TypeDescription typeDescription) {

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

Lines changed: 89 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import net.bytebuddy.implementation.FixedValue;
3131
import net.bytebuddy.implementation.Implementation;
3232
import net.bytebuddy.implementation.StubMethod;
33-
3433
import org.checkerframework.checker.nullness.qual.Nullable;
3534
import org.hibernate.AssertionFailure;
3635
import org.hibernate.Version;
@@ -76,15 +75,17 @@
7675
import static net.bytebuddy.matcher.ElementMatchers.isStatic;
7776
import static net.bytebuddy.matcher.ElementMatchers.named;
7877
import static net.bytebuddy.matcher.ElementMatchers.not;
78+
import static org.hibernate.bytecode.enhance.internal.bytebuddy.FeatureMismatchException.Feature.ASSOCIATION_MANAGEMENT;
79+
import static org.hibernate.bytecode.enhance.internal.bytebuddy.FeatureMismatchException.Feature.DIRTY_CHECK;
7980

8081
public class EnhancerImpl implements Enhancer {
81-
8282
private static final CoreMessageLogger log = CoreLogging.messageLogger( Enhancer.class );
8383

8484
protected final ByteBuddyEnhancementContext enhancementContext;
8585
private final ByteBuddyState byteBuddyState;
8686
private final EnhancerClassLocator typePool;
8787
private final EnhancerImplConstants constants;
88+
private final List<? extends Annotation> infoAnnotationList;
8889

8990
/**
9091
* Constructs the Enhancer, using the given context.
@@ -109,8 +110,11 @@ public EnhancerImpl(final EnhancementContext enhancementContext, final ByteBuddy
109110
this.byteBuddyState = Objects.requireNonNull( byteBuddyState );
110111
this.typePool = Objects.requireNonNull( classLocator );
111112
this.constants = byteBuddyState.getEnhancerConstants();
113+
114+
this.infoAnnotationList = List.of( createInfoAnnotation( enhancementContext ) );
112115
}
113116

117+
114118
/**
115119
* Performs the enhancement.
116120
*
@@ -133,7 +137,7 @@ public byte[] enhance(String className, byte[] originalBytes) throws Enhancement
133137
return byteBuddyState.rewrite( typePool, safeClassName, byteBuddy -> doEnhance(
134138
() -> byteBuddy.ignore( isDefaultFinalizer() )
135139
.redefine( typeDescription, typePool.asClassFileLocator() )
136-
.annotateType( constants.HIBERNATE_VERSION_ANNOTATION ),
140+
.annotateType( infoAnnotationList ),
137141
typeDescription
138142
) );
139143
}
@@ -167,14 +171,17 @@ public void discoverTypes(String className, byte[] originalBytes) {
167171
}
168172

169173
private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builderSupplier, TypeDescription managedCtClass) {
170-
// skip if the class was already enhanced. This is very common in WildFly as classloading is highly concurrent.
171-
// We need to ensure that no class is instrumented multiple times as that might result in incorrect bytecode.
172-
// N.B. there is a second check below using a different approach: checking for the marker interfaces,
173-
// which does not address the case of extended bytecode enhancement
174-
// (because it enhances classes that do not end up with these marker interfaces).
175-
// I'm currently inclined to keep both checks, as one is safer and the other has better backwards compatibility.
176-
if ( managedCtClass.getDeclaredAnnotations().isAnnotationPresent( EnhancementInfo.class ) ) {
177-
verifyVersions( managedCtClass, enhancementContext );
174+
if ( alreadyEnhanced( managedCtClass ) ) {
175+
// the class already implements `Managed`. there are 2 broad cases -
176+
// 1. the user manually implemented `Managed`
177+
// 2. the class was previously enhanced
178+
// in either case, look for `@EnhancementInfo` and, if found, verify we can "re-enhance" the class
179+
final AnnotationDescription.Loadable<EnhancementInfo> infoAnnotation = managedCtClass.getDeclaredAnnotations().ofType( EnhancementInfo.class );
180+
if ( infoAnnotation != null ) {
181+
// throws an exception if there is a mismatch...
182+
verifyReEnhancement( managedCtClass, infoAnnotation.load(), enhancementContext );
183+
}
184+
// verification succeeded (or not done) - we can simply skip the enhancement
178185
log.tracef( "Skipping enhancement of [%s]: it's already annotated with @EnhancementInfo", managedCtClass.getName() );
179186
return null;
180187
}
@@ -191,14 +198,6 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
191198
return null;
192199
}
193200

194-
// handle already enhanced classes
195-
if ( alreadyEnhanced( managedCtClass ) ) {
196-
verifyVersions( managedCtClass, enhancementContext );
197-
198-
log.tracef( "Skipping enhancement of [%s]: it's already implementing 'Managed'", managedCtClass.getName() );
199-
return null;
200-
}
201-
202201
if ( enhancementContext.isEntityClass( managedCtClass ) ) {
203202
if ( checkUnsupportedAttributeNaming( managedCtClass, enhancementContext ) ) {
204203
// do not enhance classes with mismatched names for PROPERTY-access persistent attributes
@@ -258,7 +257,7 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
258257

259258
builder = addInterceptorHandling( builder, managedCtClass );
260259

261-
if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
260+
if ( enhancementContext.doDirtyCheckingInline() ) {
262261
List<AnnotatedFieldDescription> collectionFields = collectCollectionFields( managedCtClass );
263262

264263
if ( collectionFields.isEmpty() ) {
@@ -390,7 +389,7 @@ else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
390389
builder = builder.implement( ManagedComposite.class );
391390
builder = addInterceptorHandling( builder, managedCtClass );
392391

393-
if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
392+
if ( enhancementContext.doDirtyCheckingInline() ) {
394393
builder = builder.implement( CompositeTracker.class )
395394
.defineField(
396395
EnhancerConstants.TRACKER_COMPOSITE_FIELD_NAME,
@@ -428,7 +427,7 @@ else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
428427
builder = builder.implement( ManagedMappedSuperclass.class );
429428
return createTransformer( managedCtClass ).applyTo( builder );
430429
}
431-
else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
430+
else if ( enhancementContext.doExtendedEnhancement() ) {
432431
log.tracef( "Extended enhancement of [%s]", managedCtClass.getName() );
433432
return createTransformer( managedCtClass ).applyExtended( builderSupplier.get() );
434433
}
@@ -438,6 +437,36 @@ else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
438437
}
439438
}
440439

440+
private void verifyReEnhancement(
441+
TypeDescription managedCtClass,
442+
EnhancementInfo existingInfo,
443+
ByteBuddyEnhancementContext enhancementContext) {
444+
// first, make sure versions match
445+
final String enhancementVersion = existingInfo.version();
446+
if ( "ignore".equals( enhancementVersion ) ) {
447+
// for testing
448+
log.debugf( "Skipping re-enhancement version check for %s due to `ignore`", managedCtClass.getName() );
449+
}
450+
else if ( !Version.getVersionString().equals( enhancementVersion ) ) {
451+
throw new VersionMismatchException( managedCtClass, enhancementVersion, Version.getVersionString() );
452+
}
453+
454+
FeatureMismatchException.checkFeatureEnablement(
455+
managedCtClass,
456+
DIRTY_CHECK,
457+
enhancementContext.doDirtyCheckingInline(),
458+
existingInfo.includesDirtyChecking()
459+
);
460+
461+
FeatureMismatchException.checkFeatureEnablement(
462+
managedCtClass,
463+
ASSOCIATION_MANAGEMENT,
464+
enhancementContext.doBiDirectionalAssociationManagement(),
465+
existingInfo.includesAssociationManagement()
466+
);
467+
}
468+
469+
441470
/**
442471
* Utility that determines the access-type of a mapped class based on an explicit annotation
443472
* or guessing it from the placement of its identifier property. Implementation should be
@@ -629,31 +658,6 @@ private static boolean checkUnsupportedAttributeNaming(TypeDescription managedCt
629658
return new String( chars );
630659
}
631660

632-
private static void verifyVersions(TypeDescription managedCtClass, ByteBuddyEnhancementContext enhancementContext) {
633-
final AnnotationDescription.Loadable<EnhancementInfo> existingInfo = managedCtClass
634-
.getDeclaredAnnotations()
635-
.ofType( EnhancementInfo.class );
636-
if ( existingInfo == null ) {
637-
// There is an edge case here where a user manually adds `implement Managed` to
638-
// their domain class, in which case there will most likely not be a
639-
// `EnhancementInfo` annotation. Such cases should simply not do version checking.
640-
//
641-
// However, there is also ambiguity in this case with classes that were enhanced
642-
// with old versions of Hibernate which did not add that annotation as part of
643-
// enhancement. But overall we consider this condition to be acceptable
644-
return;
645-
}
646-
647-
final String enhancementVersion = extractVersion( existingInfo );
648-
if ( !Version.getVersionString().equals( enhancementVersion ) ) {
649-
throw new VersionMismatchException( managedCtClass, enhancementVersion, Version.getVersionString() );
650-
}
651-
}
652-
653-
private static String extractVersion(AnnotationDescription.Loadable<EnhancementInfo> annotation) {
654-
return annotation.load().version();
655-
}
656-
657661
private PersistentAttributeTransformer createTransformer(TypeDescription typeDescription) {
658662
return PersistentAttributeTransformer.collectPersistentFields( typeDescription, enhancementContext, typePool );
659663
}
@@ -872,4 +876,42 @@ else if ( access != null && access.load().value() == AccessType.FIELD ) {
872876
}
873877
}
874878

879+
880+
private static EnhancementInfo createInfoAnnotation(EnhancementContext enhancementContext) {
881+
return new EnhancementInfoImpl( enhancementContext.doDirtyCheckingInline(), enhancementContext.doBiDirectionalAssociationManagement() );
882+
}
883+
884+
private static class EnhancementInfoImpl implements EnhancementInfo {
885+
private final String version;
886+
private final boolean includesDirtyChecking;
887+
private final boolean includesAssociationManagement;
888+
889+
public EnhancementInfoImpl(boolean includesDirtyChecking, boolean includesAssociationManagement) {
890+
this.version = Version.getVersionString();
891+
this.includesDirtyChecking = includesDirtyChecking;
892+
this.includesAssociationManagement = includesAssociationManagement;
893+
}
894+
895+
@Override
896+
public String version() {
897+
return version;
898+
}
899+
900+
@Override
901+
public boolean includesDirtyChecking() {
902+
return includesDirtyChecking;
903+
}
904+
905+
@Override
906+
public boolean includesAssociationManagement() {
907+
return includesAssociationManagement;
908+
}
909+
910+
@Override
911+
public Class<? extends Annotation> annotationType() {
912+
return EnhancementInfo.class;
913+
}
914+
}
915+
916+
875917
}

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,10 @@
1616
import net.bytebuddy.implementation.Implementation;
1717
import net.bytebuddy.implementation.StubMethod;
1818

19-
import java.lang.annotation.Annotation;
2019
import java.util.Collection;
2120
import java.util.List;
2221

23-
import org.hibernate.Version;
2422
import org.hibernate.bytecode.enhance.spi.CollectionTracker;
25-
import org.hibernate.bytecode.enhance.spi.EnhancementInfo;
2623
import org.hibernate.engine.spi.EntityEntry;
2724
import org.hibernate.engine.spi.ManagedEntity;
2825
import org.hibernate.engine.spi.PersistentAttributeInterceptor;
@@ -60,17 +57,6 @@ public final class EnhancerImplConstants {
6057
//Frequently used annotations, declared as collections as otherwise they get wrapped into them over and over again:
6158
final Collection<? extends AnnotationDescription> TRANSIENT_ANNOTATION = List.of(
6259
AnnotationDescription.Builder.ofType( Transient.class ).build() );
63-
final List<Annotation> HIBERNATE_VERSION_ANNOTATION = List.of( new EnhancementInfo() {
64-
@Override
65-
public String version() {
66-
return Version.getVersionString();
67-
}
68-
69-
@Override
70-
public Class<? extends Annotation> annotationType() {
71-
return EnhancementInfo.class;
72-
}
73-
} );
7460

7561
//Frequently used Types for method signatures:
7662
final TypeDefinition TypeVoid = TypeDescription.ForLoadedType.of( void.class );

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.bytecode.enhance.internal.bytebuddy;
6+
7+
import net.bytebuddy.description.type.TypeDescription;
8+
import org.hibernate.bytecode.enhance.spi.EnhancementException;
9+
import org.hibernate.bytecode.enhance.spi.EnhancementInfo;
10+
11+
import java.util.Locale;
12+
13+
/**
14+
* Indicates a mismatch in either {@linkplain EnhancementInfo#includesDirtyChecking() dirty tracking}
15+
* or {@linkplain EnhancementInfo#includesAssociationManagement() association management}
16+
* between consecutive attempts to enhance a class.
17+
*
18+
* @author Steve Ebersole
19+
*/
20+
public class FeatureMismatchException extends EnhancementException {
21+
public enum Feature { DIRTY_CHECK, ASSOCIATION_MANAGEMENT }
22+
23+
private final String className;
24+
private final Feature mismatchedFeature;
25+
private final boolean previousValue;
26+
27+
public FeatureMismatchException(
28+
String className,
29+
Feature mismatchedFeature,
30+
boolean previousValue) {
31+
super( String.format(
32+
Locale.ROOT,
33+
"Support for %s was enabled during enhancement, but `%s` was previously enhanced with that support %s.",
34+
featureText( mismatchedFeature ),
35+
className,
36+
decode( previousValue )
37+
) );
38+
this.className = className;
39+
this.mismatchedFeature = mismatchedFeature;
40+
this.previousValue = previousValue;
41+
}
42+
43+
public String getClassName() {
44+
return className;
45+
}
46+
47+
public Feature getMismatchedFeature() {
48+
return mismatchedFeature;
49+
}
50+
51+
public boolean wasPreviouslyEnabled() {
52+
return previousValue;
53+
}
54+
55+
public static void checkFeatureEnablement(
56+
TypeDescription managedCtClass,
57+
Feature feature,
58+
boolean currentlyEnabled,
59+
boolean previouslyEnabled) {
60+
if ( currentlyEnabled != previouslyEnabled ) {
61+
throw new FeatureMismatchException( managedCtClass.getName(), feature, previouslyEnabled );
62+
}
63+
}
64+
65+
private static String featureText(Feature mismatchedFeature) {
66+
return switch ( mismatchedFeature ) {
67+
case DIRTY_CHECK -> "inline dirty checking";
68+
case ASSOCIATION_MANAGEMENT -> "bidirectional association management";
69+
};
70+
}
71+
72+
private static String decode(boolean previousValue) {
73+
return previousValue ? "enabled" : "disabled";
74+
}
75+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ static Implementation wrap(
5757
ByteBuddyEnhancementContext enhancementContext,
5858
AnnotatedFieldDescription persistentField,
5959
Implementation implementation) {
60-
if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
60+
if ( enhancementContext.doDirtyCheckingInline() ) {
6161

6262
if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
6363
implementation = Advice.to( CodeTemplates.CompositeDirtyCheckingHandler.class ).wrap( implementation );

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ DynamicType.Builder<?> applyTo(DynamicType.Builder<?> builder) {
281281
if ( !compositeOwner
282282
&& !enhancementContext.isMappedSuperclassClass( managedCtClass )
283283
&& enhancementContext.isCompositeField( enhancedField )
284-
&& enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
284+
&& enhancementContext.doDirtyCheckingInline() ) {
285285
compositeOwner = true;
286286
}
287287
}
@@ -296,7 +296,7 @@ DynamicType.Builder<?> applyTo(DynamicType.Builder<?> builder) {
296296
}
297297
}
298298

299-
if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
299+
if ( enhancementContext.doExtendedEnhancement() ) {
300300
builder = applyExtended( builder );
301301
}
302302

0 commit comments

Comments
 (0)