From df484a3cc08509ec945a654ffa5d5a9b4bb2aaed Mon Sep 17 00:00:00 2001 From: Henk Kin Date: Tue, 4 Mar 2025 07:36:52 +0100 Subject: [PATCH 1/5] Partially incorrect reverse map when numerical values of source and destination enums differ #30 Setup testcase with error --- ...uesDifferReverseEnumValueMappingByValue.cs | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs diff --git a/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs b/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs new file mode 100644 index 0000000..8f19088 --- /dev/null +++ b/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs @@ -0,0 +1,119 @@ +using System; +using System.Reflection; +using AutoMapper.Extensions.EnumMapping.Tests.Internal; +using Shouldly; +using Xunit; + +namespace AutoMapper.Extensions.EnumMapping.Tests +{ + public class PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue + { + public class Valid : AutoMapperSpecBase + { + Destination _result; + + // Assume, as is the case for my use case, that both the source + // and destination enumerations are created via code generation + // from sources outside our control, and thus I cannot make updates + // or changes to them to work around the issue. + + // This is idiomatic of how a Protobuf enum is generated, with the Unspecified + // value acting as a stand-in for when the field is not set by the client or server + public enum Source + { + Unspecified = 0, + Bar = 1, + Baz = 2, + } + + // In our case, this is generated from an OpenAPI spec via Kiota + public enum Destination + { + BAR_ALT_NAME, // we can't map by name because the names don't match, even with case insensitivity turned on + BAZ_ALT_NAME, + } + + public class TestEnumProfile : Profile + { + public TestEnumProfile() + { + CreateMap() + .ConvertUsingEnumMapping( + opts => + { + opts + .MapByValue() + .MapValue(Source.Bar, Destination.BAR_ALT_NAME) + .MapValue(Source.Baz, Destination.BAZ_ALT_NAME) + .MapException(Source.Unspecified, () => new InvalidOperationException($"Unspecified values are not supported")); + }) + .ReverseMap(); + } + } + + protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => + { + cfg.EnableEnumMappingValidation(); + cfg.AddMaps(typeof(PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue).GetTypeInfo().Assembly); + }); + + protected override void Because_of() + { + _result = Mapper.Map(Source.Bar); + } + + [Fact] + public void Should_map_enum_by_value() + { + _result.ShouldBe(Destination.BAR_ALT_NAME); + } + + [Fact] + public void TestBarMapping() + { + // Passes + var res = Mapper.Map(Source.Bar); + res.ShouldBe(Destination.BAR_ALT_NAME); + //Assert.That(_mapper.Map(Source.Bar), Is.EqualTo(Destination.BAR_ALT_NAME)); + } + + [Fact] + public void TestBazMapping() + { + // Passes + var res = Mapper.Map(Source.Baz); + res.ShouldBe(Destination.BAZ_ALT_NAME); + //Assert.That(_mapper.Map(Source.Baz), Is.EqualTo(Destination.BAZ_ALT_NAME)); + } + + [Fact] + public void TestUnspecifiedMapping() + { + // Passes + Assert.Throws(() => + { + Mapper.Map(Source.Unspecified); + //_mapper.Map(Source.Unspecified); + }); + } + + [Fact] + public void TestReverseBarMapping() + { + // Passes + var res = Mapper.Map(Destination.BAR_ALT_NAME); + res.ShouldBe(Source.Bar); + //Assert.That(_mapper.Map(Destination.BAR_ALT_NAME), Is.EqualTo(Source.Bar)); + } + + [Fact] + public void TestReverseBazMapping() + { + // Failure: Expected: Baz But was: Bar + var res = Mapper.Map(Destination.BAZ_ALT_NAME); + res.ShouldBe(Source.Baz); + //Assert.That(_mapper.Map(Destination.BAZ_ALT_NAME), Is.EqualTo(Source.Baz)); + } + } + } +} From ee429ff41ea746ed3666c4ec358fb801d66043dc Mon Sep 17 00:00:00 2001 From: Henk Kin Date: Tue, 4 Mar 2025 10:40:49 +0100 Subject: [PATCH 2/5] Added Mapping by custom mappings --- ...uesDifferReverseEnumValueMappingByValue.cs | 2 +- .../IEnumConfigurationExpression.cs | 6 ++ .../Internal/EnumMappingFeature.cs | 63 +++++++++++++------ .../Internal/EnumMappingType.cs | 3 +- 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs b/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs index 8f19088..f3cf39a 100644 --- a/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs +++ b/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs @@ -42,7 +42,7 @@ public TestEnumProfile() opts => { opts - .MapByValue() + .MapByCustom() .MapValue(Source.Bar, Destination.BAR_ALT_NAME) .MapValue(Source.Baz, Destination.BAZ_ALT_NAME) .MapException(Source.Unspecified, () => new InvalidOperationException($"Unspecified values are not supported")); diff --git a/src/AutoMapper.Extensions.EnumMapping/IEnumConfigurationExpression.cs b/src/AutoMapper.Extensions.EnumMapping/IEnumConfigurationExpression.cs index fa698cd..6878a25 100644 --- a/src/AutoMapper.Extensions.EnumMapping/IEnumConfigurationExpression.cs +++ b/src/AutoMapper.Extensions.EnumMapping/IEnumConfigurationExpression.cs @@ -24,6 +24,12 @@ public interface IEnumConfigurationExpression /// Enum configuration options IEnumConfigurationExpression MapByValue(); + /// + /// (default) Map enum values by custom mapping (no default mapping used) + /// + /// Enum configuration options + IEnumConfigurationExpression MapByCustom(); + /// /// Map enum value from source to destination value /// diff --git a/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingFeature.cs b/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingFeature.cs index a585503..aba26df 100644 --- a/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingFeature.cs +++ b/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingFeature.cs @@ -69,7 +69,7 @@ private Dictionary> CreateDefaultEnu } } } - else + else if (EnumMappingType == EnumMappingType.Value) { var sourceEnumValueType = Enum.GetUnderlyingType(sourceType); var destinationEnumValueType = Enum.GetUnderlyingType(destinationType); @@ -89,6 +89,10 @@ private Dictionary> CreateDefaultEnu } } } + else + { + // Custom does not have default mapping + } return enumValueMappings; } @@ -105,6 +109,11 @@ public IEnumConfigurationExpression MapByValue() EnumMappingType = EnumMappingType.Value; return this; } + public IEnumConfigurationExpression MapByCustom() + { + EnumMappingType = EnumMappingType.Custom; + return this; + } public IEnumConfigurationExpression MapValue(TSource source, TDestination destination) { @@ -126,10 +135,14 @@ public IMappingFeature Reverse() { reverseEnumConfigurationExpression.MapByName(IgnoreCase); } - else + else if (EnumMappingType == EnumMappingType.Value) { reverseEnumConfigurationExpression.MapByValue(); } + else + { + reverseEnumConfigurationExpression.MapByCustom(); + } var reverseEnumValueMappingsOverride = new Dictionary(); var sourceEnumValueType = Enum.GetUnderlyingType(typeof(TSource)); @@ -154,31 +167,38 @@ public IMappingFeature Reverse() var sourceValues = destinationsPerSourceMapping.Select(x => x.Key).ToList(); var hasDestinationValueSameValueInSource = destinationValueAsSourceType.HasValue && sourceValues.Contains(destinationValueAsSourceType.Value); - if (hasDestinationValueSameValueInSource) + if (hasDestinationValueSameValueInSource && EnumMappingType != EnumMappingType.Custom) { // if there is a matching source and destination value, then that mapping is preferred and no override is needed continue; } - foreach (var sourceValue in sourceValues) + if (EnumMappingType != EnumMappingType.Custom) { - var hasDestinationSameValueAsSource = HasDestinationSameValueAsSource(sourceValue, destinationEnumValueType, out TDestination? sourceValueAsDestinationType); - - if (!hasDestinationSameValueAsSource) + foreach (var sourceValue in sourceValues) { - continue; - } + var hasDestinationSameValueAsSource = HasDestinationSameValueAsSource(sourceValue, destinationEnumValueType, out TDestination? sourceValueAsDestinationType); - var isSourceValueUsedInDestinationPartOfEnumMapping = sourceValueAsDestinationType.HasValue - && enumValueMappings.Values.Any(x => x.GetDestinationType == GetDestinationType.Value - && Equals(x.GetDestinationFunc.Invoke(), sourceValueAsDestinationType.Value)); - if (!isSourceValueUsedInDestinationPartOfEnumMapping) - { - // if there is a source which is not a destination part of a mapping, then that mapping cannot reversed - continue; - } + if (!hasDestinationSameValueAsSource) + { + continue; + } - reverseEnumValueMappingsOverride.Add(destinationValue, sourceValue); + var isSourceValueUsedInDestinationPartOfEnumMapping = sourceValueAsDestinationType.HasValue + && enumValueMappings.Values.Any(x => x.GetDestinationType == GetDestinationType.Value + && Equals(x.GetDestinationFunc.Invoke(), sourceValueAsDestinationType.Value)); + if (!isSourceValueUsedInDestinationPartOfEnumMapping) + { + // if there is a source which is not a destination part of a mapping, then that mapping cannot reversed + continue; + } + + reverseEnumValueMappingsOverride.Add(destinationValue, sourceValue); + } + } + else if (!reverseEnumValueMappingsOverride.ContainsKey(destinationValue) && sourceValues.Count == 1) + { + reverseEnumValueMappingsOverride.Add(destinationValue, sourceValues.Single()); } } @@ -234,10 +254,15 @@ private bool HasDestinationSameValueAsSource(TSource sourceValue, Type destinati } } } - else + else if(EnumMappingType == EnumMappingType.Value) { destinationValueAsSourceType = (TSource)Convert.ChangeType(destinationValue, sourceEnumValueType); } + else + { + // Custom mapping does not have default + destinationValueAsSourceType = null; + } return destinationValueAsSourceType; } diff --git a/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingType.cs b/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingType.cs index 72010c6..edaf2d1 100644 --- a/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingType.cs +++ b/src/AutoMapper.Extensions.EnumMapping/Internal/EnumMappingType.cs @@ -3,6 +3,7 @@ internal enum EnumMappingType { Value = 0, - Name = 1 + Name = 1, + Custom = 2 } } From 84a81f5e82181800bc4febb1871469fd558b0e8e Mon Sep 17 00:00:00 2001 From: Henk Kin Date: Tue, 4 Mar 2025 16:57:25 +0100 Subject: [PATCH 3/5] Added testcases --- ...cs => ReverseCustomEnumMappingByCustom.cs} | 4 +- .../ReverseEnumValueMappingByCustom.cs | 156 ++++++++++++++++++ 2 files changed, 158 insertions(+), 2 deletions(-) rename src/AutoMapper.Extensions.EnumMapping.Tests/{PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs => ReverseCustomEnumMappingByCustom.cs} (94%) create mode 100644 src/AutoMapper.Extensions.EnumMapping.Tests/ReverseEnumValueMappingByCustom.cs diff --git a/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs b/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseCustomEnumMappingByCustom.cs similarity index 94% rename from src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs rename to src/AutoMapper.Extensions.EnumMapping.Tests/ReverseCustomEnumMappingByCustom.cs index f3cf39a..a4a4140 100644 --- a/src/AutoMapper.Extensions.EnumMapping.Tests/PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue.cs +++ b/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseCustomEnumMappingByCustom.cs @@ -6,7 +6,7 @@ namespace AutoMapper.Extensions.EnumMapping.Tests { - public class PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue + public class ReverseCustomEnumMappingByCustom { public class Valid : AutoMapperSpecBase { @@ -54,7 +54,7 @@ public TestEnumProfile() protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => { cfg.EnableEnumMappingValidation(); - cfg.AddMaps(typeof(PartiallyIncorrectWhenNumericalValuesDifferReverseEnumValueMappingByValue).GetTypeInfo().Assembly); + cfg.AddMaps(typeof(ReverseCustomEnumMappingByCustom).GetTypeInfo().Assembly); }); protected override void Because_of() diff --git a/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseEnumValueMappingByCustom.cs b/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseEnumValueMappingByCustom.cs new file mode 100644 index 0000000..41d97db --- /dev/null +++ b/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseEnumValueMappingByCustom.cs @@ -0,0 +1,156 @@ +using System; +using AutoMapper.Extensions.EnumMapping.Tests.Internal; +using Shouldly; +using Xunit; + +namespace AutoMapper.Extensions.EnumMapping.Tests +{ + public class ReverseEnumValueMappingByCustom + { + public class Valid : AutoMapperSpecBase + { + Destination _result; + public enum Source { Default, Foo, Bar } + public enum Destination { Default, Bar, Foo } + + protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => + { + cfg.EnableEnumMappingValidation(); + cfg.CreateMap() + .ConvertUsingEnumMapping(opt => opt + .MapByCustom() + .MapValue(Source.Default, Destination.Default) + .MapValue(Source.Foo, Destination.Foo) + .MapValue(Source.Bar, Destination.Bar) + ) + .ReverseMap(); + }); + + protected override void Because_of() + { + _result = Mapper.Map(Source.Bar); + } + + [Fact] + public void Should_map_enum_by_custom() + { + _result.ShouldBe(Destination.Bar); + ((int)_result).ShouldBe((int)Source.Foo); + + } + } + + public class ValidCustomMapping : AutoMapperSpecBase + { + Destination _result; + public enum Source { Default, Bar } + public enum Destination { Default, Bar } + + protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => + { + cfg.EnableEnumMappingValidation(); + cfg.CreateMap() + .ConvertUsingEnumMapping(opt => opt + .MapByCustom() + .MapValue(Source.Default, Destination.Default) + .MapValue(Source.Bar, Destination.Bar) + ) + .ReverseMap(); + }); + + protected override void Because_of() + { + _result = Mapper.Map(Source.Bar); + } + + [Fact] + public void Should_map_using_custom_map() + { + _result.ShouldBe(Destination.Bar); + } + } + + public class ValidationErrors : NonValidatingSpecBase + { + public enum Source { Default, Foo, Bar } + public enum Destination { Default, Bar } + + protected override MapperConfiguration Configuration => new MapperConfiguration(cfg => + { + cfg.EnableEnumMappingValidation(); + cfg.CreateMap() + .ConvertUsingEnumMapping(opt => opt + .MapByCustom() + .MapValue(Source.Default, Destination.Default) + .MapValue(Source.Bar, Destination.Bar) + ) + .ReverseMap(); + }); + + [Fact] + public void Should_fail_validation() => + new Action(() => Configuration.AssertConfigurationIsValid()).ShouldThrowException( + ex => ex.Message.ShouldBe( + $@"Missing enum mapping from {typeof(Source).FullName} to {typeof(Destination).FullName} based on Custom{Environment.NewLine}The following source values are not mapped:{Environment.NewLine} - Foo{Environment.NewLine}")); + } + + public class CustomMappingWithValidationErrors : NonValidatingSpecBase + { + public enum Source { Default, Foo, Bar, Error } + public enum Destination { Default, Bar } + + protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => + { + cfg.EnableEnumMappingValidation(); + cfg.CreateMap() + .ConvertUsingEnumMapping(opt => opt + .MapByCustom() + .MapValue(Source.Default, Destination.Default) + .MapException(Source.Foo, () => new NotSupportedException($"Foo is not valid value")) + .MapValue(Source.Bar, Destination.Bar)) + .ReverseMap(); + }); + + [Fact] + public void Should_fail_validation() => + new Action(() => Configuration.AssertConfigurationIsValid()).ShouldThrowException( + ex => ex.Message.ShouldBe( + $@"Missing enum mapping from {typeof(Source).FullName} to {typeof(Destination).FullName} based on Custom{Environment.NewLine}The following source values are not mapped:{Environment.NewLine} - Error{Environment.NewLine}")); + } + + public class ValidCustomReverseMapping : AutoMapperSpecBase + { + Source _resultDefault; + Source _resultFoo; + Source _resultBar; + public enum Source { Default, Bar } + public enum Destination { Default, Foo, Bar } + + protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => + { + cfg.EnableEnumMappingValidation(); + cfg.CreateMap() + .ConvertUsingEnumMapping(opt => opt + .MapByCustom() + .MapValue(Source.Default, Destination.Default) + .MapValue(Source.Bar, Destination.Bar)) + .ReverseMap(optr => optr.MapByCustom().MapValue(Destination.Foo, Source.Bar)); + }); + + protected override void Because_of() + { + _resultDefault = Mapper.Map(Destination.Default); + _resultFoo = Mapper.Map(Destination.Foo); + _resultBar = Mapper.Map(Destination.Bar); + } + + [Fact] + public void Should_map_using_reverse_custom_map() + { + _resultDefault.ShouldBe(Source.Default); + _resultFoo.ShouldBe(Source.Bar); + _resultBar.ShouldBe(Source.Bar); + } + } + } +} From 38dd12b5ebaac25f813f64b3a92d1fdbc0bfd9ef Mon Sep 17 00:00:00 2001 From: Henk Kin Date: Wed, 5 Mar 2025 22:00:43 +0100 Subject: [PATCH 4/5] Updated documentation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 20eaebc..cfe1f60 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ For method `CreateMap` this library provide a `ConvertUsingEnumMapping` method. If you want to change some mappings, then you can use `MapValue` method. This is a chainable method. -Default the enum values are mapped by value, but it is possible to map by name calling `MapByName()` or `MapByValue()`. +Default the enum values are mapped by value (`MapByValue()`), but it is possible to map by name calling `MapByName()`. For enums which does not have same values and names, you can use `MapByCustom()`. Then you have to add a `MapValue` for every source enum value. ```csharp using AutoMapper.Extensions.EnumMapping; From a3d881ff8733ec7656d5d0da3494484b1e033e21 Mon Sep 17 00:00:00 2001 From: Henk Kin Date: Wed, 5 Mar 2025 22:09:23 +0100 Subject: [PATCH 5/5] Cleanup --- .../ReverseCustomEnumMappingByCustom.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseCustomEnumMappingByCustom.cs b/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseCustomEnumMappingByCustom.cs index a4a4140..77f1d0c 100644 --- a/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseCustomEnumMappingByCustom.cs +++ b/src/AutoMapper.Extensions.EnumMapping.Tests/ReverseCustomEnumMappingByCustom.cs @@ -6,6 +6,9 @@ namespace AutoMapper.Extensions.EnumMapping.Tests { + /// + /// Based on issue #30 + /// public class ReverseCustomEnumMappingByCustom { public class Valid : AutoMapperSpecBase @@ -74,7 +77,6 @@ public void TestBarMapping() // Passes var res = Mapper.Map(Source.Bar); res.ShouldBe(Destination.BAR_ALT_NAME); - //Assert.That(_mapper.Map(Source.Bar), Is.EqualTo(Destination.BAR_ALT_NAME)); } [Fact] @@ -83,7 +85,6 @@ public void TestBazMapping() // Passes var res = Mapper.Map(Source.Baz); res.ShouldBe(Destination.BAZ_ALT_NAME); - //Assert.That(_mapper.Map(Source.Baz), Is.EqualTo(Destination.BAZ_ALT_NAME)); } [Fact] @@ -93,7 +94,6 @@ public void TestUnspecifiedMapping() Assert.Throws(() => { Mapper.Map(Source.Unspecified); - //_mapper.Map(Source.Unspecified); }); } @@ -103,16 +103,14 @@ public void TestReverseBarMapping() // Passes var res = Mapper.Map(Destination.BAR_ALT_NAME); res.ShouldBe(Source.Bar); - //Assert.That(_mapper.Map(Destination.BAR_ALT_NAME), Is.EqualTo(Source.Bar)); } [Fact] public void TestReverseBazMapping() { - // Failure: Expected: Baz But was: Bar + // Failure: Expected: Baz But was: Bar => Fixed var res = Mapper.Map(Destination.BAZ_ALT_NAME); res.ShouldBe(Source.Baz); - //Assert.That(_mapper.Map(Destination.BAZ_ALT_NAME), Is.EqualTo(Source.Baz)); } } }