Skip to content

Commit 5166997

Browse files
committed
Fix AOT critter model generation for entities incompatible with static analysis
Introduces pre-generation compatibility checks that skip AOT for entities the ClassFile API can't handle from a non-<init> context: - Final fields: putfield on final from injected methods throws IllegalAccessError - Private inherited fields: child class can't reach parent's private field - Array-typed fields: checkcast mismatch when Morphia passes Object[] at runtime - No @id on any field: entity uses getter-based discovery; AOT FIELDS mode would produce a model without @id, breaking tests that run with propertyDiscovery=METHODS - @id on a getter (method path): same problem — entity must be discovered at runtime Skipped entities fall through to Tier 2 (runtime nestmate generation) or Tier 3 (reflection), which already handle all these cases correctly. Also: - Fix critterPackage() inner-class naming collision: use full relative class name (replacing $ with _) so TestLazyIdOnly$ReferencedEntity and TestLazyIdOnlyIgnoreMissing$ReferencedEntity no longer map to the same package - Fix CritterMapper.tryLoadPregenerated() to catch Throwable so LinkageError / IllegalAccessError from stale AOT classes falls back to runtime generation - Clean __morphia dirs before regenerating to prevent stale model files
1 parent 8e73a19 commit 5166997

5 files changed

Lines changed: 88 additions & 5 deletions

File tree

core/src/main/java/dev/morphia/critter/Critter.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,13 @@ public class Critter {
2424
* Returns the package name used for generated Critter classes for the given entity.
2525
*/
2626
public static String critterPackage(Class<?> entity) {
27-
return "%s.__morphia.%s".formatted(entity.getPackageName(), entity.getSimpleName().toLowerCase());
27+
// Use the full class name relative to the package (replacing $ with _) so that
28+
// inner classes with the same simple name in different outer classes don't collide.
29+
String pkg = entity.getPackageName();
30+
String relativeName = pkg.isEmpty()
31+
? entity.getName()
32+
: entity.getName().substring(pkg.length() + 1);
33+
return "%s.__morphia.%s".formatted(pkg, relativeName.replace('$', '_').toLowerCase());
2834
}
2935

3036
/**

core/src/main/java/dev/morphia/critter/parser/PropertyFinder.java

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public List<PropertyModelGenerator> find(Class<?> standinType, ClassModel classM
6969
if (methods.isEmpty()) {
7070
List<FieldInfo> fields = discoverAllFields(standinType, classModel);
7171
if (!runtimeMode) {
72+
checkAotCompatibility(fields, targetType);
7273
classLoader.register(targetType.getName(), critterGenerator.fieldAccessors(targetType, fields));
7374
}
7475
for (FieldInfo field : fields) {
@@ -81,6 +82,7 @@ public List<PropertyModelGenerator> find(Class<?> standinType, ClassModel classM
8182
}
8283
} else {
8384
if (!runtimeMode) {
85+
checkAotMethodCompatibility(methods, targetType, classModel);
8486
classLoader.register(targetType.getName(), critterGenerator.methodAccessors(targetType, methods));
8587
}
8688
for (MethodInfo method : methods) {
@@ -95,6 +97,69 @@ public List<PropertyModelGenerator> find(Class<?> standinType, ClassModel classM
9597
return models;
9698
}
9799

100+
private static final String ID_ANNOTATION_DESC = "Ldev/morphia/annotations/Id;";
101+
102+
private void checkAotCompatibility(List<FieldInfo> fields, Class<?> targetType) {
103+
boolean hasIdOnField = false;
104+
for (FieldInfo field : fields) {
105+
int flags = field.access();
106+
if ((flags & ClassFile.ACC_FINAL) != 0) {
107+
throw new UnsupportedOperationException(
108+
"AOT skip: final field '" + field.name() + "' in " + targetType.getName());
109+
}
110+
if ((flags & ClassFile.ACC_PRIVATE) != 0 && field.declaringClass() != targetType) {
111+
throw new UnsupportedOperationException(
112+
"AOT skip: private inherited field '" + field.name() + "' from "
113+
+ field.declaringClass().getName() + " in " + targetType.getName());
114+
}
115+
if (field.desc().startsWith("[")) {
116+
throw new UnsupportedOperationException(
117+
"AOT skip: array-typed field '" + field.name() + "' in " + targetType.getName());
118+
}
119+
if (field.visibleAnnotations() != null && field.visibleAnnotations().stream()
120+
.anyMatch(a -> ID_ANNOTATION_DESC.equals(a.classSymbol().descriptorString()))) {
121+
hasIdOnField = true;
122+
}
123+
}
124+
if (!hasIdOnField) {
125+
// @Id not on any field — entity likely uses getter-based discovery; skip AOT
126+
// so the runtime can pick the correct property discovery mode.
127+
throw new UnsupportedOperationException(
128+
"AOT skip: no @Id on any field in " + targetType.getName()
129+
+ "; entity requires runtime property discovery");
130+
}
131+
}
132+
133+
private void checkAotMethodCompatibility(List<MethodInfo> methods, Class<?> targetType,
134+
ClassModel classModel) {
135+
// If @Id is missing from the discovered methods, look for it on any getter in the hierarchy.
136+
// If found there, the entity relies on METHODS discovery for @Id — skip AOT so the runtime
137+
// can use the correct discovery mode.
138+
boolean hasIdInMethods = methods.stream()
139+
.anyMatch(m -> m.visibleAnnotations() != null && m.visibleAnnotations().stream()
140+
.anyMatch(a -> ID_ANNOTATION_DESC.equals(a.classSymbol().descriptorString())));
141+
if (hasIdInMethods) {
142+
return;
143+
}
144+
ClassModel current = classModel;
145+
Class<?> cls = targetType;
146+
while (cls != null && cls != Object.class) {
147+
ClassModel model = current != null ? current : readClassModel(cls);
148+
if (model != null) {
149+
for (io.github.dmlloyd.classfile.MethodModel method : model.methods()) {
150+
if (visibleAnnotations(method).stream()
151+
.anyMatch(a -> ID_ANNOTATION_DESC.equals(a.classSymbol().descriptorString()))) {
152+
throw new UnsupportedOperationException(
153+
"AOT skip: @Id on getter in " + targetType.getName()
154+
+ "; entity requires runtime property discovery");
155+
}
156+
}
157+
}
158+
cls = cls.getSuperclass();
159+
current = null;
160+
}
161+
}
162+
98163
private boolean isPropertyAnnotated(List<Annotation> annotations, boolean allowUnannotated) {
99164
List<Annotation> anns = annotations != null ? annotations : List.of();
100165
return allowUnannotated || anns.stream()
@@ -141,9 +206,10 @@ private List<FieldInfo> discoverFields(ClassModel classModel, Class<?> declaring
141206
List<FieldInfo> result = new ArrayList<>();
142207
for (FieldModel field : classModel.fields()) {
143208
List<Annotation> visible = visibleAnnotations(field);
144-
boolean isTransient = (field.flags().flagsMask() & ClassFile.ACC_TRANSIENT) != 0
209+
int flags = field.flags().flagsMask();
210+
boolean isTransient = (flags & ClassFile.ACC_TRANSIENT) != 0
145211
|| visible.stream().map(a -> a.classSymbol().descriptorString()).anyMatch(transientDescs::contains);
146-
boolean isStatic = (field.flags().flagsMask() & ClassFile.ACC_STATIC) != 0;
212+
boolean isStatic = (flags & ClassFile.ACC_STATIC) != 0;
147213
if (!isTransient && !isStatic && isPropertyAnnotated(visible, true)) {
148214
String sig = field.findAttribute(signature())
149215
.map(a -> a.signature().stringValue())

core/src/main/java/dev/morphia/mapping/CritterMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ private EntityModel tryLoadPregenerated(Class<?> type) {
154154
return (EntityModel) ctor.newInstance(this);
155155
} catch (ClassNotFoundException e) {
156156
return null;
157-
} catch (Exception e) {
157+
} catch (Throwable e) {
158158
LOG.warn("Failed to load pre-generated model for {}: {}", type.getName(), e.getMessage());
159159
return null;
160160
}

core/src/test/java/dev/morphia/critter/parser/generator/BytecodeDumpTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,12 @@ public void dumpBytecodeAsText() throws Exception {
4040

4141
for (Class<?> entity : ENTITIES) {
4242
CritterClassLoader loader = new CritterClassLoader();
43-
new CritterGenerator(defaultMapper()).generate(entity, loader, false);
43+
try {
44+
new CritterGenerator(defaultMapper()).generate(entity, loader, false);
45+
} catch (UnsupportedOperationException e) {
46+
// Entity requires runtime discovery; skip AOT dump for it
47+
continue;
48+
}
4449

4550
for (Map.Entry<String, byte[]> entry : loader.getTypeDefinitions().entrySet()) {
4651
String className = entry.getKey();

critter/critter-maven/src/main/kotlin/dev/morphia/critter/maven/CritterTestMojo.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ class CritterTestMojo : AbstractMojo() {
4444
return
4545
}
4646

47+
// Remove stale generated files so skipped entities don't leave orphaned models.
48+
testClassesDirectory
49+
.walkTopDown()
50+
.filter { it.isDirectory && it.name == "__morphia" }
51+
.forEach { it.deleteRecursively() }
52+
4753
CritterProcessor(
4854
classesDirectory = testClassesDirectory,
4955
outputDirectory = testClassesDirectory,

0 commit comments

Comments
 (0)