Repository navigation
Run lifecycle callbacks once for polymorphic values (plan item 2.6) - #4395
Merged
Merged
Conversation
LifecycleEncoder ran the declared type's @PrePersist/@PostPersist and then encoded the value through EntityEncoder, which hands a subtype to the subtype's codec, whose LifecycleEncoder ran them again. It now hands a subtype to the subtype's codec before running any callbacks.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused dispatch change follows existing codec behavior, with regression coverage for both encoding paths and no unresolved findings.
0 open findings
What changed in this PR
Fixes duplicate lifecycle callbacks when Morphia encodes polymorphic values by delegating to the subtype codec before running callbacks.
Changes:
- Moves subtype dispatch ahead of persistence callbacks.
- Adds nested and root regression tests for both mappers, including load callback counts.
- Marks plan item 2.6 complete; the decoder remains unchanged.
| File | Description |
|---|---|
| design/mapping-game-plan.md | Marks item 2.6 complete. |
| core/src/test/java/dev/morphia/test/mapping/codec/harness/models/HarnessCallbackShape.java | Adds transient callback counters. |
| core/src/test/java/dev/morphia/test/mapping/codec/harness/models/HarnessCallbackHolder.java | Adds a polymorphic list fixture. |
| core/src/test/java/dev/morphia/test/mapping/codec/harness/models/HarnessCallbackCircle.java | Adds the concrete subtype fixture. |
| core/src/test/java/dev/morphia/test/mapping/codec/harness/CodecHarnessTest.java | Verifies callbacks run once for nested and root values. |
| core/src/main/java/dev/morphia/mapping/codec/pojo/LifecycleEncoder.java | Delegates subtype encoding before callbacks. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 2.6 (review B3). Item 2.5 isn't a separate PR: the plan bundles it into 3.8 ("Includes correctness fix 2.5"), because both rewrite the same subtype loop in
PathTarget.The bug
When a type has lifecycle callbacks,
LifecycleEncoderran@PrePersistfor the codec's declared type. It then encoded throughEntityEncoder, which hands a subtype value to the subtype's codec. The subtype inherits the callbacks, so its codec is also aLifecycleEncoderand ran them again. As a result, aCirclein aList<Shape>, or encoded throughShape's codec, got@PrePersistand@PostPersisttwice. Under reflection the counts were[2, 2]instead of[1, 1].Change
LifecycleEncoder.encodehands a subtype value to the subtype's codec before running any callback. That codec runs them once, with the subtype's model.LifecycleDecoderneeded no change. It resolves the subtype's model from the discriminator before any callback runs and never re-dispatches to another codec, so@PreLoadand@PostLoadalready ran once. The new test pins that.Tests
CodecHarnessTest.runsPolymorphicLifecycleCallbacksOnceruns under both mappers on a fresh datastore. It uses new harness models: an abstractHarnessCallbackShapethat counts all four callbacks in@Transientfields, aHarnessCallbackCirclesubclass, and aHarnessCallbackHolderwith aList<HarnessCallbackShape>. It checks:HarnessCallbackShape's codec.Each case expects one
@PrePersistand one@PostPersiston encode, and one@PreLoadand one@PostLoadon decode. The nested case failed before the fix with[2, 2, 0, 0]. The counters are@Transient, so the recorded fixtures are unchanged.The core and Kotlin suites pass with
-Dmorphia.mapper=critterand with reflection: 1481 + 5 tests, 0 failures.Decisions to review
LifecycleEncoderandLifecycleDecoder. The decoder already resolved the subtype before calling back and was never double-calling, which the test confirms.