Repository navigation
Emit the context check for the containing type map in CollectionMapper (fixes #4650) - #4651
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change directly addresses the documented mismatch with a minimal, targeted fix and includes a regression test that covers the previously intermittent failure mode.
Pull request overview
Fixes an execution-plan generation bug in CollectionMapper where the OverMaxDepth depth check could run without the corresponding CheckContext guard when MaxDepth was required by the containing type map but not by the element type map—leading to intermittent runtime failures depending on execution plan cache warmup order.
Changes:
- Updated
CollectionMapperto emitCheckContextwhen either the element type map or the containingmemberMap.TypeMaprequires context. - Added a regression test that reproduces the cache-order-dependent failure and verifies both warmup orders succeed.
File summaries
| File | Description |
|---|---|
| src/AutoMapper/Mappers/CollectionMapper.cs | Aligns CheckContext emission with the same containing type map used by OverMaxDepth, preventing default-context depth checks. |
| src/UnitTests/Bug/CollectionMaxDepthContext.cs | Adds a regression test covering both compilation/warmup orders for a shared collection type-pair plan. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CollectionMapper emits the OverMaxDepth check from the containing type map (memberMap.TypeMap) but decided whether to emit CheckContext from the element type map. When the containing map has MaxDepth set and the element map is plain, the generated plan calls context.OverTypeDepth against the default context, which throws "Context.Items are only available when using a Map overload that takes Action<IMappingOperationOptions>!". Collection execution plans are cached per type pair and MapRequest equality ignores MemberMap, so the first member map to compile a given collection pair wins the cache for every other call site. Once a map with MaxDepth compiles the plan, unrelated callers reaching it under the default context fail, which makes the failure depend on warmup order rather than on configuration. This became reachable for many more configurations when self-referential type maps started defaulting to MaxDepth 64 in 0afaf1e. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wm2m61jXrAs4FheXR5TufH
c5135ab to
179a97b
Compare
…4657) (#4675) Reproduces #4657, already fixed by #4651. Unlike that fix's test, this needs no MapAtRuntime and a single Map call: cycle detection turns the collection members into runtime calls, WorkItemModel.Stories compiles the shared List<StoryEntity> -> List<StoryModel> plan with WorkItem's MaxDepth check, and EpicModel.Stories then reuses it under the default context. Registering InitiativeEntity -> InitiativeModel first is what makes the Epic members runtime calls, so the test covers both registration orders. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. |
Fixes #4650.
CollectionMapperemits theOverMaxDepthcheck from the containing type map (memberMap.TypeMap) but decided whether to emitCheckContextfrom the element type map. With a containing map that hasMaxDepth > 0and a plain element map, the generated plan callscontext.OverTypeDepthagainst the default context, which throws:The message is misleading —
ThrowInvalidMapis shared betweenItemsandCheckDefault, and the real accessor isTypeDepth.Because collection execution plans are cached per type pair (
MapRequestequality ignoresMemberMap), the first member map to compile a given collection pair wins the cache for every other call site. Once a map withMaxDepthcompiles the plan, unrelated callers reaching it under the default context fail, so the failure depends on warmup order rather than on configuration. This became reachable for many more configurations in 0afaf1e, where self-referential maps started defaulting toMaxDepth = 64.The fix emits the context check when either the element map or the containing type map requires it, so the guard and the depth check agree. When the containing map only sets
PreserveReferences, the extra check is a no-op at runtime — its own plan has already replaced the default context.Added
CollectionMaxDepthContext, which maps the sameList<Service> -> IList<ServiceDto>pair from a self-referential parent and a plain parent in both orders. The recursive-first case fails onmainwith the exception above and passes with this change; the plain-first case passes either way.Full unit test suite passes (1219 tests, net10.0).
The underlying cache-key issue — member-specific state baked into a type-pair-keyed plan — is not addressed here and is worth a separate look.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Wm2m61jXrAs4FheXR5TufH