feat: reference deduplication and cycle-safe decoding via DecodeSession#4257
Open
evanchooly wants to merge 2 commits into
Open
feat: reference deduplication and cycle-safe decoding via DecodeSession#4257evanchooly wants to merge 2 commits into
evanchooly wants to merge 2 commits into
Conversation
Add a per-document ThreadLocal decode session that caches entity instances by (collection, id). Nested activate() calls reuse the existing session so all @reference fetches within one document decode share the same cache. - DecodeSession: lightweight ThreadLocal cache with activate/deactivate; activate() returns true only for the root caller (owns deactivation) - MorphiaCursor: activates a session around next()/tryNext(); only deactivates if it was the root activator - EntityDecoder: pre-registers the entity instance via peekId() (mark/reset) before decodeProperties(), enabling cycle detection - ReferenceCodec: checks session cache before firing DB queries for both single and collection references - TestReferences: adds testReferenceDeduplication (within-document) and testCyclicReferenceDoesNotStackOverflow https://claude.ai/code/session_01NRjHAe69rLVqEZYZHnBsBF
…Cursor The original implementation activated the session in MorphiaCursor.next()/ tryNext(), but document decoding actually occurs inside FindIterable.iterator() during prepareCursor() — before MorphiaCursor is even returned. As a result, ReferenceCodec.decode() was called with session=null, making the cache ineffective. Moving session activation to MorphiaCodec.decode() ensures a session is always active during any entity decode, regardless of the cursor path. Nested activate() calls (e.g. reference fetches) reuse the existing session and do not deactivate it, preserving the cache for the full outer-document decode. https://claude.ai/code/session_01NRjHAe69rLVqEZYZHnBsBF
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a per-decode ThreadLocal cache (DecodeSession) that maps (collection, id) → entity so that, within a single document decode, repeated @Reference lookups resolve to the same Java instance and self-referential graphs do not stack-overflow. MorphiaCursor and MorphiaCodec activate/deactivate the session, EntityDecoder pre-registers each decoded entity (using a "peek the id" trick), and ReferenceCodec consults the session before issuing DB fetches.
Changes:
- Introduce
DecodeSessionThreadLocal withactivate/deactivate/register/lookup/contains. - Hook activation into
MorphiaCursor.next/tryNextandMorphiaCodec.decode; register early viapeekIdinEntityDecoder. - Make
ReferenceCodec.fetchconsult the session cache for both single and collection references, and add TestNG tests for deduplication and cyclic references.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
core/src/main/java/dev/morphia/mapping/codec/DecodeSession.java |
New ThreadLocal-backed cache for decoded entities. |
core/src/main/java/dev/morphia/query/MorphiaCursor.java |
Activates/deactivates a session around each next/tryNext. |
core/src/main/java/dev/morphia/mapping/codec/pojo/MorphiaCodec.java |
Activates/deactivates a session around decode. |
core/src/main/java/dev/morphia/mapping/codec/pojo/EntityDecoder.java |
Pre-reads _id, registers entity instance, and post-registers via mapper id. |
core/src/main/java/dev/morphia/mapping/codec/references/ReferenceCodec.java |
Looks up single and collection references in the session before fetching from DB. |
core/src/test/java/dev/morphia/test/mapping/TestReferences.java |
Adds tests for in-document dedup and cyclic graph decoding. |
Comment on lines
+49
to
60
| T instance = (T) instanceCreator.getInstance(); | ||
|
|
||
| DecodeSession session = DecodeSession.current(); | ||
| Object prereadId = null; | ||
| if (session != null) { | ||
| prereadId = peekId(reader); | ||
| if (prereadId != null) { | ||
| session.register(classModel.collectionName(), prereadId, instance); | ||
| } | ||
| } | ||
|
|
||
| decodeProperties(reader, decoderContext, instanceCreator, classModel); |
| } | ||
| } | ||
| return null; | ||
| } catch (Exception e) { |
Comment on lines
+63
to
+64
| PropertyModel idProp = classModel.getIdProperty(); | ||
| if (idProp != null) { |
Comment on lines
+81
to
+88
| boolean root = DecodeSession.activate(); | ||
| try { | ||
| return getDecoder().decode(reader, decoderContext); | ||
| } finally { | ||
| if (root) { | ||
| DecodeSession.deactivate(); | ||
| } | ||
| } |
Comment on lines
+51
to
+70
| DecodeSession session = DecodeSession.current(); | ||
| Object prereadId = null; | ||
| if (session != null) { | ||
| prereadId = peekId(reader); | ||
| if (prereadId != null) { | ||
| session.register(classModel.collectionName(), prereadId, instance); | ||
| } | ||
| } | ||
|
|
||
| decodeProperties(reader, decoderContext, instanceCreator, classModel); | ||
|
|
||
| if (session != null && prereadId == null) { | ||
| PropertyModel idProp = classModel.getIdProperty(); | ||
| if (idProp != null) { | ||
| Object id = morphiaCodec.getDatastore().getMapper().getId(instance); | ||
| if (id != null) { | ||
| session.register(classModel.collectionName(), id, instance); | ||
| } | ||
| } | ||
| } |
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.
Add a per-document ThreadLocal decode session that caches entity instances
by (collection, id). Nested activate() calls reuse the existing session so
all @reference fetches within one document decode share the same cache.
activate() returns true only for the root caller (owns deactivation)
deactivates if it was the root activator
before decodeProperties(), enabling cycle detection
single and collection references
testCyclicReferenceDoesNotStackOverflow
https://claude.ai/code/session_01NRjHAe69rLVqEZYZHnBsBF