Feat: 2253 flatten collections#2279
Open
saijaku0 wants to merge 2 commits into
Open
Conversation
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.
Closes #2253
Summary
Adds support for collection flattening in
[MapProperty]paths. When a path segment refers to a collection-typed member (e.g. anICollection<UserRole>from an EF Core m:n join table), Mapperly now resolves the remainder of the path against the collection's element type and generates the appropriateSelect/ToListcalls.This removes the need for a hand-written intermediate user-mapping method when projecting through a join table a common pain point with EF Core m:n relations.
Example
Before this PR, the path
"UserRoles.Role"could not be resolved - the user had to write an extraprivate static RoleDto Map(UserRole) => MapToRoleDto(src.Role). After this PR, the path is resolved automatically.Implementation approach
A new abstract
CollectionElementMember(inSymbols/Members/) acts as a pseudo-member inNonEmptyMemberPath. It is not a real type member it signals toMemberPathGetterthat traversal crosses a collection boundary and that the element type should be used for subsequent path resolution.The path
"UserRoles.Role"is parsed into:MemberPathGetter.BuildAccessthen emits the innerSelect(x.UserRoles, x1 => x1.Role)for the collection-crossing segment, and the existingLinqEnumerableMappinginfrastructure handles the outer projection toRoleDto. This means existing collection mapping, null-handling and diagnostics are reused without duplication.Trade-off: nested
Selectvs flatSelectThe current implementation generates nested
Selectcalls for both regular and queryable-projection mappings:The original issue example shows a flat form, where the navigation through
.Roleis inlined into the projection lambda:For regular method mappings, the nested form compiles to identical IL and runs equivalently - the extra
Selectis fused by the JIT.For
IQueryable<T>projections (EF Core), the flat form translates to cleaner SQL, while the nested form may produce extra subqueries depending on the provider. EF Core 8+ handles both, but flat is the more conservative choice.I chose the nested form for this PR because it cleanly reuses
LinqEnumerableMappingwithout modifying its signature, keeping the change small and contained to the newCollectionElementMembermachinery. A flat-form implementation requires deeper integration (see below).Path to flat
Select(follow-up)If flat
Selectis preferred, the change set looks like this:LinqEnumerableMapping.csMemberPathGetter? elementSourceTransformparameterLinqEnumerableMapping.Build()elementSourceTransformtolambdaCtx.Sourcebefore callingelementMapping.BuildNonEmptyMemberPath.csCollectionPathandElementPathsegments (split byCollectionElementMember)MemberPathGetter.BuildAccess()x.UserRoles) without the innerSelectMappedMemberSourceValuecreation siteLinqEnumerableMappingHappy to do this as a follow-up commit in this PR if you'd prefer flat from the start, or as a separate PR after this one merges. I'd appreciate guidance on which path you'd prefer.
Tests
Added
ObjectPropertyCollectionThroughTestcovering:ICollection<T>,List<T>,IEnumerable<T>,T[]nameofinterpolation[MapProperty]attributes on the same mapperVerifysnapshot)Dept.Teams.Members)All existing tests still pass.
Open questions for reviewers
Select- see trade-off above. Should I rework to flat as part of this PR, or follow up?CollectionElementMember- happy to rename if there's a convention I missed."Specified member UserRoles.NonExistent on source type User was not found". Should the message explicitly mention the collection traversal (e.g."...on element type UserRole of collection UserRoles..."), or is the current form fine?