feat: add support for nested properties #587
Conversation
| /// Maps all properties from a nested path to the root destination. | ||
| /// </summary> | ||
| [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)] | ||
| public class MapperNestedPropertiesAttribute : Attribute |
There was a problem hiding this comment.
Could you rename according to the issue and other mapping related attributes to MapNestedPropertiesAttribute.
| private const char PropertyAccessSeparator = '.'; | ||
|
|
||
| /// <summary> | ||
| /// Maps a specified source property to the specified target property. |
There was a problem hiding this comment.
This comment isn't correct, is it? The same for the other ctor.
| @@ -0,0 +1,3 @@ | |||
| namespace Riok.Mapperly.Configuration; | |||
|
|
|||
| public record NestedPropertyMappingConfiguration(StringMemberPath Source); | |||
There was a problem hiding this comment.
I'd rename to MapNestedPropertiesMappingConfiguration to match other namings and attribute name.
| public void ShouldFlattedNestedToTargetRootMapping() | ||
| { | ||
| var mapperBody = """ | ||
| [MapperNestedProperties(nameof(A.NestedValue)] |
There was a problem hiding this comment.
Could you also add a test for deep nested properties. Eg. [MapNestedProperties(new[] { nameof(A.Nested), nameof(A.Nested.Value) })]? I'm not sure, but I think these won't work yet.
And probably also add a test for q required init property, a constructor parameter and a queryable projection mapping.
| @@ -443,4 +456,33 @@ public void ShouldIgnoreStaticConstructorAndDiagnostic() | |||
| .HaveDiagnostic(DiagnosticDescriptors.CouldNotCreateMapping) | |||
| .HaveAssertedAllDiagnostics(); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void ShouldFlattedNestedToTargetRootMapping() | |||
There was a problem hiding this comment.
Could you move these tests to ObjectPropertyFlatteningTest as they fit better there as this always flattens the values.
| return BuilderContext.SymbolAccessor | ||
| .GetAllAccessibleMappableMembers(Mapping.SourceType) | ||
| .Where(i => i.Name == x.Source.Path.Last()) | ||
| .SelectMany(i => BuilderContext.SymbolAccessor.GetAllAccessibleMappableMembers(i.Type)) | ||
| .Select(i => | ||
| { | ||
| var list = x.Source.Path.ToList(); | ||
| list.Add(i.Name); | ||
| return new PropertyMappingConfiguration(new StringMemberPath(list), new StringMemberPath(new[] { i.Name })); | ||
| }); |
There was a problem hiding this comment.
I think mixing these with the manually configured mappings won't fully work...
IMO this needs to be handled separately. If no matching source member could be found, the member should also be looked up in these MapNestedProperties.
| return BuilderContext.SymbolAccessor | ||
| .GetAllAccessibleMappableMembers(Mapping.SourceType) | ||
| .Where(i => i.Name == x.Source.Path.Last()) | ||
| .SelectMany(i => BuilderContext.SymbolAccessor.GetAllAccessibleMappableMembers(i.Type)) |
There was a problem hiding this comment.
The members included via MapNestedProperties should also be included in _unmappedSourceMemberNames and should also emit a diagnostic if not mapped. This should also be tested.
| @@ -443,4 +456,33 @@ public void ShouldIgnoreStaticConstructorAndDiagnostic() | |||
| .HaveDiagnostic(DiagnosticDescriptors.CouldNotCreateMapping) | |||
| .HaveAssertedAllDiagnostics(); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void ShouldFlattedNestedToTargetRootMapping() | |||
There was a problem hiding this comment.
Since this is a complete new feature, an integration test should be added to ensure this works with all supported Roslyn versions.
|
As there seems to be no updates for several weeks I'm closing this for now. Feel free to reopen / open a new PR with updates 😊 |
Adding support for nested properties
Description
(cont of #534)
If you don’t want explicitly name all properties from nested source bean, you can use MapperNestedPropertiesAttribute. This will tell Mapperly to map every property from source proerty to the target object.
Currently a WIP. Please let me know what to add for this to be merge eventually.
Checklist