Replace ASM + Gizmo with Java ClassFile API backport#4282
Conversation
Replaces org.ow2.asm and io.quarkus.gizmo dependencies with io.github.dmlloyd:jdk-classfile-backport:25.1 in the critter bytecode generation pipeline. Key changes: - Add jdk-classfile-backport dependency; remove gizmo/asm-tree/asm-util from compile scope (keep asm as test-scoped for ClassfileOutput) - New FieldInfo/MethodInfo records replacing ASM FieldNode/MethodNode - BaseGenerator, AddFieldAccessorMethods, AddMethodAccessorMethods rewritten using ClassFile API transforming pattern - PropertyAccessorGenerator, VarHandleAccessorGenerator rewritten with ClassFile.of().build() - PropertyModelGenerator, GizmoEntityModelGenerator rewritten using ClassFile API; now use Java reflection for annotation/type data instead of ASM SignatureReader/AnnotationNode - GizmoExtensions: new ClassFile API utilities (emitAnnotationOnStack, emitClassRef, emitTypeData, asClass, rawTypeDesc, typeDataFromDesc) - AnnotationNodeExtensions.kt simplified to generate a stub class that no longer depends on ASM/Gizmo - PropertyFinder, ExtensionFunctions, CritterParser updated to use ClassModel/FieldInfo/MethodInfo - Tests updated: TypesTest uses ClassDesc, TestGizmoGeneration removes Gizmo-specific tests, uses ClassFile API for MethodInfo discovery https://claude.ai/code/session_01TLEDKhoUzorXDYRAqVLLYE
136782c to
fc3c714
Compare
- Filter ACC_BRIDGE methods in PropertyFinder.isGetter() and PropertyModelGenerator.findMethod() to prevent compiler-generated covariant bridge methods from overriding the real getter's return type - Remove checkcast to non-public property types in VarHandleAccessorGenerator.set() to avoid IllegalAccessError when accessor classes in CritterClassLoader access inner entity classes - Fix VarHandleAccessorGenerator.set() final-field path to not dead-reference the entity class - Fix GizmoExtensions.emitClassRef() for primitive types using getstatic WrapperClass.TYPE - Merge setter annotations into PropertyModelGenerator's annotation map so annotations like @Version and @text on setter methods are captured for METHODS-mode property discovery - Add CritterPropertyModel.registerFieldAnnotations/registerMethodAnnotations to register non-Morphia annotations (e.g. @nonnull) via reflection in generated property model constructors - Wrap CritterParser lists in Collections.unmodifiableList and fix getter field-type check
There was a problem hiding this comment.
Pull request overview
This PR migrates Morphia’s critter bytecode generation pipeline away from ASM/Gizmo to the JDK ClassFile API backport (io.github.dmlloyd:jdk-classfile-backport), updating both the runtime generators and associated tests/build tooling to match the new model.
Changes:
- Introduces
FieldInfo/MethodInforecords and switches class parsing from ASMClassNode/*NodetoClassModel/attributes. - Rewrites accessor/model generators to emit/transform bytecode using
ClassFile.of().build()andtransformClass(...). - Removes ASM/Gizmo-based build-plugin generators and updates tests to validate the new parsing/generation approach.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Adds backport dependency/version management (and currently still manages Gizmo). |
| core/pom.xml | Swaps compile dependency from Gizmo to classfile-backport; moves ASM artifacts to test scope. |
| core/src/test/java/dev/morphia/critter/parser/TypesTest.java | Updates descriptor/class conversion tests to use ClassDesc. |
| core/src/test/java/dev/morphia/critter/parser/gizmo/TestGizmoGeneration.java | Updates generator tests to parse methods/annotations via ClassFile API. |
| core/src/main/java/dev/morphia/mapping/codec/pojo/critter/CritterPropertyModel.java | Adds reflection-based annotation registration helpers for generated models. |
| core/src/main/java/dev/morphia/critter/parser/PropertyFinder.java | Replaces ASM-based property discovery with ClassModel + attributes. |
| core/src/main/java/dev/morphia/critter/parser/MethodInfo.java | New record replacing ASM MethodNode as a carrier. |
| core/src/main/java/dev/morphia/critter/parser/java/CritterParser.java | Removes ASMifier utilities; keeps descriptor helpers. |
| core/src/main/java/dev/morphia/critter/parser/gizmo/VarHandleAccessorGenerator.java | Rewrites VarHandle accessor generation using ClassFile API. |
| core/src/main/java/dev/morphia/critter/parser/gizmo/PropertyModelGenerator.java | Rewrites property model generation using ClassFile API + reflection for types/annotations. |
| core/src/main/java/dev/morphia/critter/parser/gizmo/PropertyAccessorGenerator.java | Rewrites synthetic-method delegating accessors using ClassFile API. |
| core/src/main/java/dev/morphia/critter/parser/gizmo/GizmoExtensions.java | Replaces ASM/Gizmo utilities with ClassFile-based emission helpers (annotations, TypeData, class refs). |
| core/src/main/java/dev/morphia/critter/parser/gizmo/GizmoEntityModelGenerator.java | Rewrites entity model generation using ClassFile API and reflection-sourced annotations. |
| core/src/main/java/dev/morphia/critter/parser/gizmo/CritterGizmoGenerator.java | Switches orchestration to ClassModel parsing and new generators. |
| core/src/main/java/dev/morphia/critter/parser/gizmo/BaseGizmoGenerator.java | Removes Gizmo ClassCreator plumbing; becomes a lightweight base. |
| core/src/main/java/dev/morphia/critter/parser/FieldInfo.java | New record replacing ASM FieldNode as a carrier. |
| core/src/main/java/dev/morphia/critter/parser/ExtensionFunctions.java | Updates getter→property naming helper to use MethodInfo + MethodTypeDesc. |
| core/src/main/java/dev/morphia/critter/parser/asm/BaseGenerator.java | Replaces ASM read/filter setup with ClassFile parsing entry point. |
| core/src/main/java/dev/morphia/critter/parser/asm/AddMethodAccessorMethods.java | Rewrites method-based accessor injection using transformClass + drop/endHandler. |
| core/src/main/java/dev/morphia/critter/parser/asm/AddFieldAccessorMethods.java | Rewrites field-based accessor injection using transformClass + drop/endHandler. |
| core/src/main/java/dev/morphia/critter/Critter.java | Replaces stored annotation types with descriptor strings. |
| build-plugins/src/main/kotlin/util/AnnotationNodeExtensions.kt | Removed (ASM/Gizmo-based annotation builder generator no longer needed). |
| build-plugins/src/main/java/util/KotlinAnnotationExtensions.java | Removed (no longer required for old annotation emission approach). |
| build-plugins/src/main/java/util/AsmBuilders.java | Removed (ASM builder generation removed). |
…type resolution hasSetter() and emitLoadClass() both called the single-argument Class.forName(), which uses the caller's (system) classloader rather than the entity's classloader. For property types only available in a child classloader (typical in app-server deployments), this caused emit() to crash with ClassNotFoundException and hasSetter() to silently return false, making the property read-only. Fix: use entity.getClassLoader() in hasSetter(), and emit Class.forName(name, false, tccl) in the generated constructor bytecode so property types are resolved at runtime via TCCL, consistent with how the entity class itself is already resolved. Regression test added in TestVarHandleAccessor that dynamically generates an entity whose property type lives only in a child classloader and asserts get/set round-trips.
…THODS mode findSetter() and findSetterInHierarchy() matched setter methods by name and descriptor only, without checking ACC_STATIC. In PropertyDiscovery.METHODS mode a static setXxx method was accepted as the property setter, causing the property to be treated as method-based. VarHandleAccessorGenerator's hasSetter() then correctly rejected the static method (it has a reflection-level isStatic guard), leaving the property with no setter handle — so set() threw UnsupportedOperationException even though the backing field was writable. Fix: add (flags & ACC_STATIC) == 0 guard to both findSetter and findSetterInHierarchy so static methods are never selected as property setters. Properties with getter + static setter (no instance setter) now fall back to field-based VarHandle discovery as expected.
consolidate packages
CritterGenerator.generate() called type.getClassLoader() directly without a null guard, causing NPE for bootstrap-loaded entity classes. Extracted GenerationUtils.safeClassLoader() with the correct null fallback and applied it at both sites (CritterGenerator and VarHandleAccessorGenerator). Consolidated three sets of duplicated code into GenerationUtils: PRIMITIVE_TO_WRAPPER (was in both accessor generators), typeClassName() (same), and emitBooleanMethod() (was in both model generators). All callers updated to use the shared versions.
…iscovery isGetter() accepted any method starting with "is" or "get" regardless of length. A no-arg non-void method named exactly "is" or "get" passed all checks, then getterPropertyName() computed an empty property name and threw StringIndexOutOfBoundsException on charAt(0), aborting property discovery for the entire entity. Added an early-exit guard that rejects exact matches before parsing the property name.
emitAnnotationOnStack used value.equals(defaultValue) to skip builder setter calls for annotation elements matching their defaults. Array-typed elements (String[], Class[], annotation[]) return a fresh defensive copy on every annotation proxy invocation, so two logically-identical arrays are never the same instance and equals() always returned false. Effect: setter calls were emitted for every array element regardless of whether the value matched the default, inflating generated bytecode. Fix: replaced equals() with Objects.deepEquals(), which compares array contents recursively.
…eration Generate a concrete annotation implementation class per annotation instance at code-generation time using the ClassFile API. This eliminates all runtime reflection from property/entity model constructors — Morphia and non-Morphia annotations alike are materialized as bytecode constants, fixing NonNull and other third-party annotations being silently dropped. Also fix AnnotationBuilders equals() using field access instead of method calls, and add Hotel test fixture with varargs/hashCode/equals to exercise the generation pipeline.
…tDeclaredAnnotation for non-Morphia Morphia annotations in generated <init> are emitted via AnnotationBuilder factory/setter chains, encoding all values as bytecode constants with zero runtime reflection. Non-Morphia annotations are embedded via RuntimeVisibleAnnotationsAttribute so getDeclaredAnnotation() works at runtime.
…ative entities Generates readable bytecode text files under target/critter-bytecode/ with full package/directory hierarchy for Example, MethodExample, Author, Book, and CritterMapperTestEntity; replaces the narrower DumpBytecodeTest.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
core/src/main/java/dev/morphia/critter/parser/generator/CritterGenerator.java:51
generate()reads the classfile from anInputStreambut never closes it. Wrap the stream in try-with-resources to avoid leaking jar/file handles during generation.
…GenerationUtils - Add GenerationUtils.readClassModel() as the single place that reads a class file resource and parses it to ClassModel (using safeClassLoader); update CritterGenerator, AccessorMethods, and PropertyFinder to delegate to it, removing duplicated stream-handling and the null-classloader bug in AccessorMethods.readClassFiltering() - Fix getDeclaredAnnotation null guard in PropertyModelGenerator: emitted constructor bytecode now skips the annotation() call when the result is null (non-RUNTIME retention or classloader edge cases), preventing NPE in PropertyModel.annotation() - Fix findDeclaringClass to search getter methods after field lookup fails, so generic return types on method-based properties resolve correctly instead of falling back to Object - Fix primitive array handling in GenerationUtils annotation helpers: use Array.getLength/Array.get instead of (Object[]) cast; emit newarray/arrayStore for primitive component types instead of anewarray/aastore - Fix PropertyAccessorGenerator.set() to skip checkcast for non-public property types, mirroring the existing guard in VarHandleAccessorGenerator that prevents IllegalAccessError on package-private inner classes
Thread a stand-in/target-type distinction through CritterGenerator, PropertyFinder, PropertyModelGenerator, and EntityModelGenerator so that @Externalentity annotated classes generate accessor and entity model bytecode for the target type rather than silently falling back to reflection.
…tion cause Reference types in __readXxx() and __writeXxx() now use Object in their method descriptors so the generated accessor class (in the critter package) never references the concrete property type. The cast to the concrete type lives inside the entity where it has legal access. This prevents VerifyError when the property type is package-private. Also passes the caught exception as the cause when re-throwing from the final-field reflection fallback in VarHandleAccessorGenerator.
…itiveWrapperName AddMethodAccessorMethods now walks getDeclaredMethod up the hierarchy (matching VarHandleAccessorGenerator) so package-private setters are recognized in build mode, eliminating the read-only/writable behavioral split between the two code paths. computeTypeData() in PropertyModelGenerator duplicated TypeData.get(Type) with subtly different wildcard and raw-class behavior; replaced both call sites with TypeData.get() and removed the method. primitiveWrapperName() was a 9-branch if-chain duplicating PRIMITIVE_TO_WRAPPER; replaced with a map lookup plus a void special case.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…nature, synthesize @entity for @Externalentity - Unify findSetter/hasSetter into GenerationUtils.findSetterMethod(), fixing array-typed and inner-class setter parameter resolution that previously failed Class.forName - Fix malformed "Lint;" class signature for primitive properties in PropertyAccessorGenerator by using the wrapper type in the generic class signature - Exclude dev.morphia.annotations.internal.* from Morphia annotation collection in EntityModelGenerator to avoid constructing nonexistent builder class names - Synthesize @entity from @Externalentity in EntityModelGenerator constructor, mirroring MorphiaDefaultsConvention, so getAnnotation(Entity.class) returns non-null for @Externalentity models with the critter mapper - Cache annotation descriptor key list in PropertyFinder instead of rebuilding per call
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
core/src/test/java/dev/morphia/critter/parser/generator/TestGeneration.java:167
inputStreamcan be null if the class resource can't be resolved by the classloader, which would cause a NullPointerException onreadAllBytes(). Make the failure explicit withObjects.requireNonNull(...)(or an assertion) so the test fails with a clear message.
…, dead annotation(), duplication - VarHandleAccessorGenerator.hasSetter(): use propertyClassDesc() instead of ClassDesc.of(), which already handles primitives, arrays, and reference types; fixes IllegalArgumentException when a getter returns an array type in METHODS mode - Remove PRIMITIVE_CLASSES map from VarHandleAccessorGenerator; now fully redundant - Move primitiveUnboxMethod() to GenerationUtils as a shared static helper; remove the duplicate from both PropertyAccessorGenerator and VarHandleAccessorGenerator - CritterGenerator.entityModel(): remove dead classModel parameter from both overloads - EntityModelGenerator: remove dead annotation() method that read from the wrong type for @Externalentity models and had no callers - PropertyFinder.readClassModel(): catch RuntimeException from GenerationUtils.readClassModel() and log a warning, restoring the graceful-degradation behavior from the previous ASM implementation instead of crashing hierarchy traversal on parse failures
…esc() Removes the private primitiveDescriptor() switch from PropertyAccessorGenerator and the duplicate switch in VarHandleAccessorGenerator.propertyClassDesc(), replacing both with a single GenerationUtils.primitiveClassDesc(String) helper that returns the appropriate ConstantDescs.CD_* value directly.
…r, setter param-type check - VarHandleAccessorGenerator: add explicit guard in emit() for array-typed properties with a documenting comment — the BSON codec provides Object[] at runtime which VarHandle rejects; explicit throw replaces the accidental ClassDesc.of() throw that was previously causing the reflection fallback - VarHandleAccessorGenerator: fix fluent setter NoSuchMethodException by storing the reflected setter Method at construction time and emitting its actual return type in the findVirtual lookup instead of hardcoded void; MethodHandle.invoke() silently discards the non-void return - PropertyModelGenerator: replace private findSetterMethod(Class,String) — which ignored parameter type and could pick a wrong overloaded setter — with GenerationUtils.findSetterMethod(Class,String,ClassDesc) which validates the parameter type against the getter's return type
Uses MethodHandles.privateLookupIn(declaringClass).defineHiddenClass(..., NESTMATE) to generate PropertyAccessor impls with direct getfield/putfield bytecode. This grants private-member access across the class hierarchy without requiring --add-opens or VarHandle setup overhead. Key fixes included: - Resolve declaring class via hierarchy walk so superclass private fields are accessible even when the accessor is instantiated from a subclass - Exclude static fields from property discovery to avoid IncompatibleClassChangeError - Guard PropertyModel.getValue against null proxy unwrap result (deleted referent) - Exclude NestmateAccessorRegistry from CritterClassLoader child-first loading so it remains a single shared instance across all classloaders
Summary
Implements #4273: replace
org.ow2.asmandio.quarkus.gizmowithio.github.dmlloyd:jdk-classfile-backport:25.1in Morphia's critter bytecode generation pipeline.jdk-classfile-backport; removed gizmo, asm-tree, asm-util from compile scope (asm kept as test-scope forClassfileOutput)FieldInfoandMethodInforeplace ASMFieldNode/MethodNodethroughoutAddFieldAccessorMethods,AddMethodAccessorMethods): rewritten usingClassFile.of().transformClass()with dropping + endHandler patternPropertyAccessorGenerator,VarHandleAccessorGenerator): rewritten usingClassFile.of().build()PropertyModelGenerator,GizmoEntityModelGenerator): rewritten using ClassFile API; use Java reflection for annotation instances and type data instead of ASMSignatureReader/AnnotationNodeGizmoExtensions: new utility class withemitAnnotationOnStack,emitClassRef,emitTypeData,asClass,rawTypeDesc,typeDataFromDescAnnotationNodeExtensions.kt: simplified to generate a minimal stub (no longer depends on ASM/Gizmo since annotation handling is now done via Java reflection)TypesTestusesClassDesc,TestGizmoGenerationremoves Gizmo-specific tests and uses ClassFile API for method discoveryTest plan
TestGizmoGeneration— 8 tests (type data parsing, annotation building, full entity model generation, method-based accessors)TypesTest— 44 tests (ClassDesc ↔ Class conversion)TestVarHandleAccessor— 6 tests (VarHandle-based runtime accessors)TestCritterMapper— 18 tests (full critter mapper integration)https://claude.ai/code/session_01TLEDKhoUzorXDYRAqVLLYE
Generated by Claude Code