Phase 4: Move critter-core into morphia-core#4205
Conversation
- Move all Java source files from critter/core to core preserving package structure - Move all test files from critter/core to core - Add optional bytecode-gen deps to core/pom.xml (gizmo, asm-tree, asm-util, roaster-jdt) - Add morphia-annotation-node plugin execution to core/pom.xml - Refactor Generators from static singleton to instance class with constructor injection - Make CritterGizmoGenerator methods static, add generators parameter to generate() - Update GizmoEntityModelGenerator to accept MorphiaConfig in constructor - Update PropertyFinder to pass config from mapper to propertyModelGenerator() - Update CritterClassLoader to accept configurable parent classloader - Update CritterProcessor (critter-maven) to use new Generators instance Closes #4186 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Moves Critter’s core/gizmo generation implementation into morphia-core and refactors generator configuration away from a static singleton into explicitly constructed instances.
Changes:
- Refactors
Generatorsfrom a static singleton to an instance constructed fromMorphiaConfig+Mapper, updating call sites accordingly. - Makes
CritterGizmoGeneratora static utility (incore) and threads config through generation paths (PropertyFinder,GizmoEntityModelGenerator). - Updates
core/pom.xmlwith an annotation-node generator execution and optional bytecode-generation dependencies.
Reviewed changes
Copilot reviewed 13 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| critter/critter-maven/src/main/kotlin/dev/morphia/critter/maven/CritterProcessor.kt | Switches maven plugin to instance-based Generators and updated gizmo generation API. |
| critter/core/src/main/java/dev/morphia/critter/parser/Generators.java | Removes singleton/lazy-loading; requires callers to provide config + mapper. |
| critter/core/src/main/java/dev/morphia/critter/parser/PropertyFinder.java | Threads config into property model generation and moves to static CritterGizmoGenerator methods. |
| critter/core/src/main/java/dev/morphia/critter/parser/gizmo/GizmoEntityModelGenerator.java | Uses injected MorphiaConfig instead of global Generators.INSTANCE. |
| critter/core/src/main/java/dev/morphia/critter/CritterClassLoader.java | Allows configurable parent classloader; defaults to thread context CL. |
| core/src/main/java/dev/morphia/critter/parser/gizmo/CritterGizmoGenerator.java | New static generator entry point in core using instance Generators. |
| core/pom.xml | Adds annotation-node generation execution and optional asm/gizmo/roaster deps. |
| critter/core/src/test/** | Updates tests to construct Generators explicitly and use static gizmo generator APIs. |
core/src/main/java/dev/morphia/critter/parser/gizmo/CritterGizmoGenerator.java
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@evanchooly I've opened a new pull request, #4206, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 38 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
core/src/main/java/dev/morphia/critter/parser/PropertyFinder.java:32
mapperis stored as a field but never used outside the constructor. This is dead state and will trigger warnings; remove the field or use it (e.g., usemapper.getConfig()instead of threading a separateconfigparameter if that’s the intent).
| private val critterClassLoader = CritterClassLoader() | ||
| private val generators = Generators(config, ReflectiveMapper(config)) |
There was a problem hiding this comment.
CritterClassLoader is created with the default constructor (parent = TCCL). In the Maven plugin context this can differ from the project/classesDirectory classLoader passed into CritterProcessor, and can lead to generated/accessed classes failing to resolve entity types. Construct CritterClassLoader with the provided classLoader (or otherwise ensure its parent is the same loader used to load entity classes).
…izmoGenerator Co-authored-by: evanchooly <195021+evanchooly@users.noreply.github.com>
Fix InputStream resource leak in CritterGizmoGenerator
| super(parent, Collections.emptyMap()); | ||
| } | ||
|
|
||
| public CritterClassLoader() { |
There was a problem hiding this comment.
@copilot i think this should take a classloader so that the configure classloader on MorphiaConfig can be passed in. check other uses where we pass in classloaders to methods.
|
@evanchooly I've opened a new pull request, #4207, to work on those changes. Once the pull request is ready, I'll request review from you. |
make fields private
Summary
Closes #4186
critter/coretocorepreserving package structurecritter/coretocorecore/pom.xml: gizmo, asm-tree, asm-util, roaster-jdtmorphia-annotation-nodeplugin execution tocore/pom.xml(generatesAnnotationNodeExtensions.java)Generatorsfrom static singleton to instance class withGenerators(MorphiaConfig, Mapper)constructorCritterGizmoGeneratormethods static; addgeneratorsparameter togenerate()GizmoEntityModelGeneratorto acceptMorphiaConfigin constructorPropertyFinderto pass config from mapper topropertyModelGenerator()callsCritterClassLoaderto accept configurable parent classloader (defaults to thread context CL)CritterProcessor(critter-maven) to construct aGeneratorsinstance from the injected configTest plan
./mvnw install -pl :morphia-core -am -DskipTests -Ddeploy.skip=true -Dinvoker.skip=true— compiles successfully./mvnw test -pl :morphia-core -Dtest="TestGizmoGeneration,TestVarHandleAccessor,TypesTest,TestAccessorsMutators,TestPropertyModelGenerator" -Ddeploy.skip=true— 57 tests pass./mvnw install -pl :critter-maven -DskipTests -Ddeploy.skip=true -Dinvoker.skip=true— compiles successfully🤖 Generated with Claude Code