You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Make annotated lifecycle methods entity-only (plan item 4.14) - #4434
Plan item 4.14: annotated lifecycle methods (@PrePersist, @PostPersist, @PreLoad, @PostLoad) become entity-only. Follows #4432, #4431 and #4429.
Changes
Core
An @EntityListeners class must implement at least one per-event interface and may not declare annotated lifecycle methods. Mapping fails with a MappingException that names the method and points to the interfaces, rather than silently ignoring it.
@EntityListeners instances are created when the entity's model is set up, not on first use, and are called through TypedListeners with no reflection.
EntityListenerAdapter now handles only the entity's own methods; OnEntityListenerAdapter is folded into it.
A listener constructor that throws is reported with its cause, not as a missing no-arg constructor.
Javadoc on EntityListener, @EntityListeners and the four event annotations says so.
Recipe: new AnnotatedListenerMethodMigration, in UpgradeToMorphia30 right after EntityListenerMigration. For each class listed in @EntityListeners, or implementing EntityListener, that isn't itself an entity:
it implements each event's interface (replacing a directly implemented EntityListener), with a method that calls the class's annotated methods for the event in declaration order, and removes the annotations;
the type argument is the class's EntityListener<T> argument, else the most specific entity parameter type, else Object;
a method already shaped like the interface's is reused;
a method annotated for several events is called from each.
It marks: a @PrePersist method that uses its Document (always empty), a checked exception, an entity parameter the type argument doesn't fit, a name clash with the generated method, an event the class already implements, unrelated entity types, and a listener whose superclass is a listener or has annotated methods.
Decisions to review
Removed in 3.0, not deprecated. 3.0 already breaks listeners and ships a recipe for them. With 4.2's Document listeners every annotated method has an interface to move to, so the plan's expected cost (Document-using methods with nowhere to go) no longer applies.
The recipe delegates rather than rewriting methods. The generated interface method calls the existing methods, so their names, parameter order, visibility and return values stay as they are, and callers are untouched.
Inherited listeners are marked, not migrated. A generated method would override the superclass's, or two classes in a hierarchy would implement one interface with different type arguments.
Event masks use the listener's type argument. A model sets an event bit only when an @EntityListeners instance may accept it or a subtype. A PostLoadDocumentListener<Ring> on Figure no longer gives the sibling Block a lifecycle codec at all (TestLifecycleStreaming updated).
Testing
Core with critter and reflection; Kotlin, validation and rewrite (217 tests, 15 new).
Local Copilot CLI review over four rounds before pushing. Valid findings fixed:
inherited listener superclasses;
constructor errors;
a second interface for an implemented event;
multi-event methods.
Static annotated methods were never called and aren't rejected, so they're left alone.
The morphia-docs pages are updated after this merges.
An @EntityListeners class now implements the per-event listener interfaces only.
Mapping rejects one that declares @PrePersist/@PostPersist/@PreLoad/@PostLoad
methods or implements none of the interfaces. Its instances are created when
the model is set up and called through TypedListeners, so EntityListenerAdapter
handles just the entity's own methods (OnEntityListenerAdapter is folded in).
The new AnnotatedListenerMethodMigration recipe, part of UpgradeToMorphia30,
makes such a class implement each event's interface with a method that calls
its annotated methods, and marks what it can't migrate.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The migration can generate uncompilable listener classes, invoke the wrong callback overload, and miss interface conflicts or inherited annotations that cause mapping failures.
Makes annotated lifecycle callbacks entity-only for Morphia 3.0, completing plan item 4.14. External listeners use per-event interfaces, with an OpenRewrite recipe to help migrate existing callbacks.
Changes:
Validates and creates external listeners during model setup, then dispatches them through TypedListeners.
Adds annotated-listener migration and regression tests.
Updates lifecycle documentation, test fixtures, and subtype event-mask expectations.
…ated overload
The recipe reuses a method as the interface's only if it returns void and the class
doesn't implement the event's other interface, recognizes interfaces through
subinterfaces and superclasses, marks a listener that only inherits annotated
methods from a class that isn't a listener, and casts the entity to the annotated
method's parameter type so an overload taking the entity's own type isn't called.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The migration can produce uncompilable listener classes by dropping required type qualification, mishandling generic callbacks and method-scoped type variables, and overlooking inherited final-method collisions.
…types; import cast types
The recipe marks an annotated method when a superclass already has the event's
interface method, when the method that would be reused is generic, or when its
entity parameter is the method's own type variable, and imports the type a cast
names.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Two critical migration defects generate uncompilable code through colliding type names in listener signatures and conflicting raw and parameterized EntityListener inheritance via subinterfaces.
No open issues 🎉
2 previously missed issues
In code that hasn't changed since last review
Generated listener signatures use ambiguous type names
For EntityListener<com.acme.Document> with @PostLoad void check(com.acme.Document entity, org.bson.Document document), this generates postLoad(Document entity, Document document, Datastore datastore), although the two parameters require different classes. The migrated class cannot compile, and entities named Datastore have the same problem. Use collision-safe qualified type references in generated signatures and interface arguments, and add regression coverage for these names.
Raw listener inheritance conflicts with parameterized event interface
For interface Audit extends EntityListener {} and Stamping implements Audit with @PostLoad void check(Order order), the recipe retains Audit and adds PostLoadListener<Order>. The result cannot compile because it inherits both raw EntityListener and EntityListener<Order>. Detect raw marker inheritance through subinterfaces and mark the class for manual migration before adding a parameterized event interface.
…w EntityListener
The recipe marks a class when a type the generated code names by its simple name
is hidden by another of that name in the file, or when the class implements an
interface that inherits EntityListener raw or with another type argument.
Copilot's third review flagged two missed cases in its summary; both are fixed in ded8924164:
Ambiguous type names: a class is now marked when a type the generated code names by its simple name (the entity type, a cast type, Document or Datastore) is hidden by another type of that name in the file. Test: flagsAnEntityTypeNamedLikeTheDocument.
Raw EntityListener through a subinterface: a class that implements an interface inheriting EntityListener raw, or with a type argument other than the event interfaces would get, is marked. Test: flagsAClassThatInheritsTheRawEntityListener.
This was the third review cycle, so I'm stopping the Copilot loop here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan item 4.14: annotated lifecycle methods (
@PrePersist,@PostPersist,@PreLoad,@PostLoad) become entity-only. Follows #4432, #4431 and #4429.Changes
@EntityListenersclass must implement at least one per-event interface and may not declare annotated lifecycle methods. Mapping fails with aMappingExceptionthat names the method and points to the interfaces, rather than silently ignoring it.@EntityListenersinstances are created when the entity's model is set up, not on first use, and are called throughTypedListenerswith no reflection.EntityListenerAdapternow handles only the entity's own methods;OnEntityListenerAdapteris folded into it.EntityListener,@EntityListenersand the four event annotations says so.AnnotatedListenerMethodMigration, inUpgradeToMorphia30right afterEntityListenerMigration. For each class listed in@EntityListeners, or implementingEntityListener, that isn't itself an entity:EntityListener), with a method that calls the class's annotated methods for the event in declaration order, and removes the annotations;EntityListener<T>argument, else the most specific entity parameter type, elseObject;@PrePersistmethod that uses itsDocument(always empty), a checked exception, an entity parameter the type argument doesn't fit, a name clash with the generated method, an event the class already implements, unrelated entity types, and a listener whose superclass is a listener or has annotated methods.Decisions to review
@EntityListenersinstance may accept it or a subtype. APostLoadDocumentListener<Ring>onFigureno longer gives the siblingBlocka lifecycle codec at all (TestLifecycleStreamingupdated).Testing
Core with critter and reflection; Kotlin, validation and rewrite (217 tests, 15 new).
Local Copilot CLI review over four rounds before pushing. Valid findings fixed:
Static annotated methods were never called and aren't rejected, so they're left alone.
The morphia-docs pages are updated after this merges.