Merge pull request #25891 from abpframework/maliming/fix-mapperly-single-parameter-extra-properties-defaults

Fix `ExtraProperties` defaults lost in Mapperly single-parameter `Map`

framework/src/Volo.Abp.Mapperly/Volo/Abp/Mapperly/MapperlyAutoObjectMappingProvider.cs

@@ -7,6 +7,7 @@ using System.Linq.Expressions;
 using System.Reflection;
 using Microsoft.Extensions.DependencyInjection;
 using Volo.Abp.Data;
+using Volo.Abp.DynamicProxy;
 using Volo.Abp.ObjectExtending;
 using Volo.Abp.ObjectMapping;
 
@@ -44,9 +45,13 @@ public class MapperlyAutoObjectMappingProvider : IAutoObjectMappingProvider
         var mapper = ServiceProvider.GetService<IAbpMapperlyMapper<TSource, TDestination>>();
         if (mapper != null)
         {
+            var mapExtraPropertiesAttribute = mapper.GetType().GetSingleAttributeOrNull<MapExtraPropertiesAttribute>();
             mapper.BeforeMap((TSource)source);
             var destination = mapper.Map((TSource)source);
-            TryMapExtraProperties(mapper.GetType().GetSingleAttributeOrNull<MapExtraPropertiesAttribute>(), (TSource)source, destination, new ExtraPropertyDictionary());
+            var destinationExtraProperties = mapExtraPropertiesAttribute == null
+                ? new ExtraPropertyDictionary()
+                : GetDestinationExtraPropertiesSeed((TSource)source, destination);
+            TryMapExtraProperties(mapExtraPropertiesAttribute, (TSource)source, destination, destinationExtraProperties);
             mapper.AfterMap((TSource)source, destination);
             return destination;
         }
@@ -54,9 +59,13 @@ public class MapperlyAutoObjectMappingProvider : IAutoObjectMappingProvider
         var reverseMapper = ServiceProvider.GetService<IAbpReverseMapperlyMapper<TDestination, TSource>>();
         if (reverseMapper != null)
         {
+            var mapExtraPropertiesAttribute = reverseMapper.GetType().GetSingleAttributeOrNull<MapExtraPropertiesAttribute>();
             reverseMapper.BeforeReverseMap((TSource)source);
             var destination = reverseMapper.ReverseMap((TSource)source);
-            TryMapExtraProperties(reverseMapper.GetType().GetSingleAttributeOrNull<MapExtraPropertiesAttribute>(), (TSource)source, destination, new ExtraPropertyDictionary());
+            var destinationExtraProperties = mapExtraPropertiesAttribute == null
+                ? new ExtraPropertyDictionary()
+                : GetDestinationExtraPropertiesSeed((TSource)source, destination);
+            TryMapExtraProperties(mapExtraPropertiesAttribute, (TSource)source, destination, destinationExtraProperties);
             reverseMapper.AfterReverseMap((TSource)source, destination);
             return destination;
         }
@@ -219,6 +228,38 @@ public class MapperlyAutoObjectMappingProvider : IAutoObjectMappingProvider
         return Expression.Lambda<Func<object, object, object, object?>>(callConvert, instanceParam, sourceParam, destinationParam).Compile();
     }
 
+    protected virtual ExtraPropertyDictionary GetDestinationExtraPropertiesSeed<TSource, TDestination>(TSource source, TDestination destination)
+    {
+        var extraProperties = new ExtraPropertyDictionary();
+        if (source is not IHasExtraProperties sourceHasExtraProperties ||
+            destination is not IHasExtraProperties destinationHasExtraProperties ||
+            destinationHasExtraProperties.ExtraProperties is null)
+        {
+            return extraProperties;
+        }
+
+        //Keys that don't exist in the source can only be set by the destination's constructor
+        foreach (var property in destinationHasExtraProperties.ExtraProperties)
+        {
+            if (sourceHasExtraProperties.ExtraProperties == null || !sourceHasExtraProperties.ExtraProperties.ContainsKey(property.Key))
+            {
+                extraProperties[property.Key] = property.Value;
+            }
+        }
+
+        //Source keys may be copied by the generated mapper, reset registered ones to their default value and let the filter map the source value
+        var destinationType = ProxyHelper.UnProxy(destinationHasExtraProperties).GetType();
+        foreach (var property in ObjectExtensionManager.Instance.GetProperties(destinationType))
+        {
+            if (!extraProperties.ContainsKey(property.Name) && destinationHasExtraProperties.ExtraProperties.ContainsKey(property.Name))
+            {
+                extraProperties[property.Name] = property.GetDefaultValue();
+            }
+        }
+
+        return extraProperties;
+    }
+
     protected virtual ExtraPropertyDictionary GetExtraProperties<TDestination>(TDestination destination)
     {
         var extraProperties = new ExtraPropertyDictionary();

framework/test/Volo.Abp.Mapperly.Tests/Mapperly/AbpAutoMapperExtensibleDtoExtensions_Tests.cs

@@ -53,8 +53,10 @@ public class AbpAutoMapperExtensibleDtoExtensions_Tests : AbpIntegratedTest<Mapp
         var personDto = _objectMapper.Map<ExtensibleTestPerson, ExtensibleTestPersonDto>(person);
 
         personDto.GetProperty<string>("Name").ShouldBe("John"); //Defined in both classes
-        personDto.GetProperty<int>("ChildCount").ShouldBe(0); //Not defined in the source, but was set to the default value by ExtensibleTestPersonDto constructor
-        personDto.GetProperty("CityName").ShouldBeNull(); //Ignored, but was set to the default value by ExtensibleTestPersonDto constructor
+        personDto.HasProperty("ChildCount").ShouldBeTrue(); //Not defined in the source, but was set to the default value by ExtensibleTestPersonDto constructor
+        personDto.GetProperty<int>("ChildCount").ShouldBe(0);
+        personDto.HasProperty("CityName").ShouldBeTrue(); //Ignored, but was set to the default value by ExtensibleTestPersonDto constructor
+        personDto.GetProperty("CityName").ShouldBeNull();
         personDto.HasProperty("Age").ShouldBeFalse(); //Not defined on the destination
         personDto.HasProperty("Sex").ShouldBeFalse(); //Not defined in both classes
     }

framework/test/Volo.Abp.Mapperly.Tests/Volo/Abp/Mapperly/AbpReverseMapperly_Tests.cs

@@ -97,6 +97,7 @@ public class AbpReverseMapperly_Tests : AbpIntegratedTest<MapperlyTestModule>
         var entity = _objectMapper.Map<ExtensibleReverseDto, ExtensibleReverseEntity>(dto);
 
         entity.GetProperty<string>("Tag").ShouldBe("ok");
-        entity.HasProperty("Secret").ShouldBeFalse();
+        entity.HasProperty("Secret").ShouldBeTrue(); //Ignored, but was set to the default value by the ExtensibleObject constructor
+        entity.GetProperty<string>("Secret").ShouldBeNull();
     }
 }

framework/test/Volo.Abp.Mapperly.Tests/Volo/Abp/Mapperly/MapExtraPropertiesDefaultSeed_Tests.cs

@@ -0,0 +1,87 @@
+using System;
+using Microsoft.Extensions.DependencyInjection;
+using Shouldly;
+using Volo.Abp.Data;
+using Volo.Abp.ObjectMapping;
+using Volo.Abp.Testing;
+using Xunit;
+
+namespace Volo.Abp.Mapperly;
+
+public class MapExtraPropertiesDefaultSeed_Tests : AbpIntegratedTest<MapperlyTestModule>
+{
+    private readonly IObjectMapper _objectMapper;
+
+    public MapExtraPropertiesDefaultSeed_Tests()
+    {
+        _objectMapper = ServiceProvider.GetRequiredService<IObjectMapper>();
+    }
+
+    [Fact]
+    public void Single_Parameter_Map_Should_Preserve_Constructor_Seeded_Properties()
+    {
+        var entity = new ExtensibleSeededEntity { Id = Guid.NewGuid() }
+            .SetProperty("Tag", "ok");
+
+        var dto = _objectMapper.Map<ExtensibleSeededEntity, ExtensibleSeededDto>(entity);
+
+        dto.GetProperty<string>("Tag").ShouldBe("ok"); //Defined in both classes
+        dto.GetProperty<string>("CreatedBy").ShouldBe("system"); //Set by the ExtensibleSeededDto constructor
+        dto.HasProperty("DtoOnly").ShouldBeTrue(); //Not defined in the source, but was set to the default value by the ExtensibleObject constructor
+        dto.GetProperty("DtoOnly").ShouldBeNull();
+    }
+
+    [Fact]
+    public void Single_Parameter_Map_Should_Not_Change_The_Source_Extra_Properties()
+    {
+        var entity = new ExtensibleSeededEntity { Id = Guid.NewGuid() }
+            .SetProperty("Tag", "ok")
+            .SetProperty("CreatedBy", "leaked");
+        var originalReference = entity.ExtraProperties;
+
+        _objectMapper.Map<ExtensibleSeededEntity, ExtensibleSeededDto>(entity);
+
+        ReferenceEquals(entity.ExtraProperties, originalReference).ShouldBeTrue();
+        entity.GetProperty<string>("Tag").ShouldBe("ok");
+        entity.GetProperty<string>("CreatedBy").ShouldBe("leaked");
+    }
+
+    [Fact]
+    public void Single_Parameter_Map_Should_Not_Seed_Defaults_When_Destination_Disables_Them()
+    {
+        var entity = new ExtensibleSeededEntity { Id = Guid.NewGuid() }
+            .SetProperty("Tag", "ok");
+
+        var dto = _objectMapper.Map<ExtensibleSeededEntity, ExtensibleNonSeededDto>(entity);
+
+        dto.GetProperty<string>("Tag").ShouldBe("ok"); //Defined in both classes
+        dto.HasProperty("DtoOnly").ShouldBeFalse(); //ExtensibleNonSeededDto constructor disables the default values seeding
+    }
+
+    [Fact]
+    public void Single_Parameter_Map_Should_Not_Leak_Filtered_Source_Values_Into_Registered_Keys()
+    {
+        var entity = new ExtensibleSeededEntity { Id = Guid.NewGuid() }
+            .SetProperty("Tag", "ok")
+            .SetProperty("DtoOnly", "leaked");
+
+        var dto = _objectMapper.Map<ExtensibleSeededEntity, ExtensibleNonSeededDto>(entity);
+
+        dto.GetProperty<string>("Tag").ShouldBe("ok"); //Defined in both classes
+        dto.GetProperty("DtoOnly").ShouldBeNull(); //Not defined in the source, the source value must not leak
+    }
+
+    [Fact]
+    public void Single_Parameter_Map_Should_Not_Invoke_Default_Value_Factories_Without_MapExtraProperties_Attribute()
+    {
+        var entity = new ExtensibleSeededEntity { Id = Guid.NewGuid() }
+            .SetProperty("Tag", "ok");
+
+        var callsBefore = ExtensibleNoAttributeDto.CountedDefaultValueFactoryCalls;
+
+        var dto = _objectMapper.Map<ExtensibleSeededEntity, ExtensibleNoAttributeDto>(entity);
+
+        (ExtensibleNoAttributeDto.CountedDefaultValueFactoryCalls - callsBefore).ShouldBe(1); //Only the ExtensibleNoAttributeDto constructor seeding
+        dto.HasProperty("Counted").ShouldBeTrue();
+    }
+}

framework/test/Volo.Abp.Mapperly.Tests/Volo/Abp/Mapperly/MapperlyTestModule.cs

@@ -21,7 +21,20 @@ public class MapperlyTestModule : AbpModule
                 .AddOrUpdateProperty<ExtensibleReverseEntity, string>("Tag")
                 .AddOrUpdateProperty<ExtensibleReverseEntity, string>("Secret")
                 .AddOrUpdateProperty<ExtensibleReverseDto, string>("Tag")
-                .AddOrUpdateProperty<ExtensibleReverseDto, string>("Secret");
+                .AddOrUpdateProperty<ExtensibleReverseDto, string>("Secret")
+                .AddOrUpdateProperty<ExtensibleSeededEntity, string>("Tag")
+                .AddOrUpdateProperty<ExtensibleSeededDto, string>("Tag")
+                .AddOrUpdateProperty<ExtensibleSeededDto, string>("DtoOnly")
+                .AddOrUpdateProperty<ExtensibleNonSeededDto, string>("Tag")
+                .AddOrUpdateProperty<ExtensibleNonSeededDto, string>("DtoOnly")
+                .AddOrUpdateProperty<ExtensibleNoAttributeDto, string>("Counted", options =>
+                {
+                    options.DefaultValueFactory = () =>
+                    {
+                        ExtensibleNoAttributeDto.CountedDefaultValueFactoryCalls++;
+                        return null!;
+                    };
+                });
         });
     }
 }

framework/test/Volo.Abp.Mapperly.Tests/Volo/Abp/Mapperly/SampleClasses/MapperlyMappers.cs

@@ -105,4 +105,62 @@ public partial class ExtensibleReverseMapper : TwoWayMapperBase<ExtensibleRevers
     public override partial ExtensibleReverseEntity ReverseMap(ExtensibleReverseDto destination);
 
     public override partial void ReverseMap(ExtensibleReverseDto destination, ExtensibleReverseEntity source);
+}
+
+public class ExtensibleSeededEntity : ExtensibleObject
+{
+    public Guid Id { get; set; }
+}
+
+public class ExtensibleSeededDto : ExtensibleObject
+{
+    public Guid Id { get; set; }
+
+    public ExtensibleSeededDto()
+    {
+        this.SetProperty("CreatedBy", "system");
+    }
+}
+
+public class ExtensibleNonSeededDto : ExtensibleObject
+{
+    public Guid Id { get; set; }
+
+    public ExtensibleNonSeededDto()
+        : base(false)
+    {
+    }
+}
+
+public class ExtensibleNoAttributeDto : ExtensibleObject
+{
+    public static int CountedDefaultValueFactoryCalls;
+
+    public Guid Id { get; set; }
+}
+
+[Mapper]
+[MapExtraProperties]
+public partial class ExtensibleSeededMapper : MapperBase<ExtensibleSeededEntity, ExtensibleSeededDto>
+{
+    public override partial ExtensibleSeededDto Map(ExtensibleSeededEntity source);
+
+    public override partial void Map(ExtensibleSeededEntity source, ExtensibleSeededDto destination);
+}
+
+[Mapper]
+[MapExtraProperties]
+public partial class ExtensibleNonSeededMapper : MapperBase<ExtensibleSeededEntity, ExtensibleNonSeededDto>
+{
+    public override partial ExtensibleNonSeededDto Map(ExtensibleSeededEntity source);
+
+    public override partial void Map(ExtensibleSeededEntity source, ExtensibleNonSeededDto destination);
+}
+
+[Mapper]
+public partial class ExtensibleNoAttributeMapper : MapperBase<ExtensibleSeededEntity, ExtensibleNoAttributeDto>
+{
+    public override partial ExtensibleNoAttributeDto Map(ExtensibleSeededEntity source);
+
+    public override partial void Map(ExtensibleSeededEntity source, ExtensibleNoAttributeDto destination);
 }