Skip to content

Commit 3dc9611

Browse files
lbargaoanulbargaoanu
committed
don't throw and catch on validation
1 parent b521096 commit 3dc9611

File tree

4 files changed

+119
-75
lines changed

4 files changed

+119
-75
lines changed

‎.editorconfig‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ end_of_line = lf
66
indent_style = space
77
indent_size = 4
88

9+
csharp_space_around_binary_operators = before_and_after
10+
911
#### Naming styles ####
1012

1113
# Constants are PascalCase

‎src/AutoMapper/Configuration/ConfigurationValidator.cs‎

Lines changed: 75 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,119 +1,125 @@
11
using AutoMapper.Internal.Mappers;
22
namespace AutoMapper.Configuration;
33
[EditorBrowsable(EditorBrowsableState.Never)]
4-
public readonly record struct ConfigurationValidator(IGlobalConfigurationExpression Expression)
4+
public class ConfigurationValidator(IGlobalConfiguration config)
55
{
6-
private void Validate(ValidationContext context)
7-
{
8-
foreach (var validator in Expression.Validators)
9-
{
10-
validator(context);
11-
}
12-
}
13-
public void AssertConfigurationExpressionIsValid(IGlobalConfiguration config, TypeMap[] typeMaps)
6+
IGlobalConfigurationExpression Expression => ((MapperConfiguration)config).ConfigurationExpression;
7+
public void AssertConfigurationExpressionIsValid(TypeMap[] typeMaps)
148
{
159
var duplicateTypeMapConfigs = Expression.Profiles.Append((Profile)Expression)
1610
.SelectMany(p => p.TypeMapConfigs, (profile, typeMap) => (profile, typeMap))
1711
.GroupBy(x => x.typeMap.Types)
1812
.Where(g => g.Count() > 1)
19-
.Select(g => (TypePair : g.Key, ProfileNames : g.Select(tmc => tmc.profile.ProfileName).ToArray()))
13+
.Select(g => (TypePair: g.Key, ProfileNames: g.Select(tmc => tmc.profile.ProfileName).ToArray()))
2014
.Select(g => new DuplicateTypeMapConfigurationException.TypeMapConfigErrors(g.TypePair, g.ProfileNames))
2115
.ToArray();
22-
if (duplicateTypeMapConfigs.Any())
16+
if(duplicateTypeMapConfigs.Length != 0)
2317
{
2418
throw new DuplicateTypeMapConfigurationException(duplicateTypeMapConfigs);
2519
}
26-
AssertConfigurationIsValid(config, typeMaps);
20+
AssertConfigurationIsValid(typeMaps);
2721
}
28-
public void AssertConfigurationIsValid(IGlobalConfiguration config, TypeMap[] typeMaps)
22+
public void AssertConfigurationIsValid(TypeMap[] typeMaps)
2923
{
24+
List<Exception> configExceptions = [];
3025
var badTypeMaps =
3126
(from typeMap in typeMaps
32-
where typeMap.ShouldCheckForValid
33-
let unmappedPropertyNames = typeMap.GetUnmappedPropertyNames()
34-
let canConstruct = typeMap.PassesCtorValidation
35-
where unmappedPropertyNames.Length > 0 || !canConstruct
36-
select new AutoMapperConfigurationException.TypeMapConfigErrors(typeMap, unmappedPropertyNames, canConstruct)
27+
where typeMap.ShouldCheckForValid
28+
let unmappedPropertyNames = typeMap.GetUnmappedPropertyNames()
29+
let canConstruct = typeMap.PassesCtorValidation
30+
where unmappedPropertyNames.Length > 0 || !canConstruct
31+
select new AutoMapperConfigurationException.TypeMapConfigErrors(typeMap, unmappedPropertyNames, canConstruct)
3732
).ToArray();
38-
if (badTypeMaps.Length > 0)
33+
if(badTypeMaps.Length > 0)
3934
{
40-
throw new AutoMapperConfigurationException(badTypeMaps);
35+
configExceptions.Add(new AutoMapperConfigurationException(badTypeMaps));
4136
}
4237
HashSet<TypeMap> typeMapsChecked = [];
43-
List<Exception> configExceptions = [];
44-
foreach (var typeMap in typeMaps)
38+
foreach(var typeMap in typeMaps)
4539
{
46-
try
47-
{
48-
DryRunTypeMap(config, typeMapsChecked, typeMap.Types, typeMap, null);
49-
}
50-
catch (Exception e)
51-
{
52-
configExceptions.Add(e);
53-
}
40+
DryRunTypeMap(typeMap.Types, typeMap, null);
5441
}
55-
if (configExceptions.Count > 1)
42+
if(configExceptions.Count > 1)
5643
{
5744
throw new AggregateException(configExceptions);
5845
}
59-
if (configExceptions.Count > 0)
46+
if(configExceptions.Count > 0)
6047
{
6148
throw configExceptions[0];
6249
}
63-
}
64-
private void DryRunTypeMap(IGlobalConfiguration config, HashSet<TypeMap> typeMapsChecked, TypePair types, TypeMap typeMap, MemberMap memberMap)
65-
{
66-
if(typeMap == null)
50+
void DryRunTypeMap(TypePair types, TypeMap typeMap, MemberMap memberMap)
6751
{
68-
if (types.ContainsGenericParameters)
52+
if(typeMap == null)
6953
{
70-
return;
54+
if(types.ContainsGenericParameters)
55+
{
56+
return;
57+
}
58+
typeMap = config.ResolveTypeMap(types.SourceType, types.DestinationType);
7159
}
72-
typeMap = config.ResolveTypeMap(types.SourceType, types.DestinationType);
73-
}
74-
if (typeMap != null)
75-
{
76-
if (typeMapsChecked.Contains(typeMap))
60+
if(typeMap != null)
7761
{
78-
return;
62+
if(typeMapsChecked.Contains(typeMap))
63+
{
64+
return;
65+
}
66+
typeMapsChecked.Add(typeMap);
67+
if(!Validate(new(types, memberMap, typeMap)) || !typeMap.ShouldCheckForValid)
68+
{
69+
return;
70+
}
71+
CheckPropertyMaps(typeMap);
7972
}
80-
typeMapsChecked.Add(typeMap);
81-
Validate(new(types, memberMap, typeMap));
82-
if(!typeMap.ShouldCheckForValid)
73+
else
8374
{
84-
return;
75+
var mapperToUse = config.FindMapper(types);
76+
if(mapperToUse == null)
77+
{
78+
configExceptions.Add(new AutoMapperConfigurationException(memberMap.TypeMap.Types) { MemberMap = memberMap });
79+
return;
80+
}
81+
if(!Validate(new(types, memberMap, ObjectMapper: mapperToUse)))
82+
{
83+
return;
84+
}
85+
if(mapperToUse.GetAssociatedTypes(types) is TypePair newTypes && newTypes != types)
86+
{
87+
DryRunTypeMap(newTypes, null, memberMap);
88+
}
8589
}
86-
CheckPropertyMaps(config, typeMapsChecked, typeMap);
8790
}
88-
else
91+
void CheckPropertyMaps(TypeMap typeMap)
8992
{
90-
var mapperToUse = config.FindMapper(types);
91-
if (mapperToUse == null)
93+
foreach(var memberMap in typeMap.MemberMaps)
9294
{
93-
throw new AutoMapperConfigurationException(memberMap.TypeMap.Types) { MemberMap = memberMap };
94-
}
95-
Validate(new(types, memberMap, ObjectMapper: mapperToUse));
96-
if (mapperToUse.GetAssociatedTypes(types) is TypePair newTypes && newTypes != types)
97-
{
98-
DryRunTypeMap(config, typeMapsChecked, newTypes, null, memberMap);
95+
if(memberMap.Ignored || (memberMap is PropertyMap && typeMap.ConstructorParameterMatches(memberMap.DestinationName)))
96+
{
97+
continue;
98+
}
99+
var sourceType = memberMap.SourceType;
100+
// when we don't know what the source type is, bail
101+
if(sourceType.IsGenericParameter || sourceType == typeof(object))
102+
{
103+
continue;
104+
}
105+
DryRunTypeMap(new(sourceType, memberMap.DestinationType), null, memberMap);
99106
}
100107
}
101-
}
102-
private void CheckPropertyMaps(IGlobalConfiguration config, HashSet<TypeMap> typeMapsChecked, TypeMap typeMap)
103-
{
104-
foreach (var memberMap in typeMap.MemberMaps)
108+
bool Validate(ValidationContext context)
105109
{
106-
if(memberMap.Ignored || (memberMap is PropertyMap && typeMap.ConstructorParameterMatches(memberMap.DestinationName)))
110+
try
107111
{
108-
continue;
112+
foreach(var validator in Expression.Validators)
113+
{
114+
validator(context);
115+
}
109116
}
110-
var sourceType = memberMap.SourceType;
111-
// when we don't know what the source type is, bail
112-
if (sourceType.IsGenericParameter || sourceType == typeof(object))
117+
catch(Exception e)
113118
{
114-
continue;
119+
configExceptions.Add(e);
120+
return false;
115121
}
116-
DryRunTypeMap(config, typeMapsChecked, new(sourceType, memberMap.DestinationType), null, memberMap);
122+
return true;
117123
}
118124
}
119125
}

‎src/AutoMapper/Configuration/MapperConfiguration.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public sealed class MapperConfiguration : IGlobalConfiguration
4343
private readonly LockingConcurrentDictionary<TypePair, TypeMap> _runtimeMaps;
4444
private LazyValue<ProjectionBuilder> _projectionBuilder;
4545
private readonly LockingConcurrentDictionary<MapRequest, Delegate> _executionPlans;
46-
private readonly ConfigurationValidator _validator;
46+
private readonly MapperConfigurationExpression _configurationExpression;
4747
private readonly Features<IRuntimeFeature> _features = new();
4848
private readonly bool _hasOpenMaps;
4949
private readonly HashSet<TypeMap> _typeMapsPath = [];
@@ -58,14 +58,14 @@ public sealed class MapperConfiguration : IGlobalConfiguration
5858
private readonly List<Type> _typesInheritance = [];
5959
public MapperConfiguration(MapperConfigurationExpression configurationExpression)
6060
{
61+
_configurationExpression=configurationExpression;
6162
var configuration = (IGlobalConfigurationExpression)configurationExpression;
6263
if (configuration.MethodMappingEnabled != false)
6364
{
6465
configuration.IncludeSourceExtensionMethods(typeof(Enumerable));
6566
}
6667
_mappers = [..configuration.Mappers];
6768
_executionPlans = new(CompileExecutionPlan);
68-
_validator = new(configuration);
6969
_projectionBuilder = new(CreateProjectionBuilder);
7070
Configuration = new((IProfileConfiguration)configuration);
7171
int typeMapsCount = Configuration.TypeMapsCount;
@@ -155,7 +155,8 @@ static MapperConfigurationExpression Build(Action<IMapperConfigurationExpression
155155
configure(expr);
156156
return expr;
157157
}
158-
public void AssertConfigurationIsValid() => _validator.AssertConfigurationExpressionIsValid(this, [.._configuredMaps.Values]);
158+
public void AssertConfigurationIsValid() => Validator().AssertConfigurationExpressionIsValid([.._configuredMaps.Values]);
159+
ConfigurationValidator Validator() => new(this);
159160
public IMapper CreateMapper() => new Mapper(this);
160161
public IMapper CreateMapper(Func<Type, object> serviceCtor) => new Mapper(this, serviceCtor);
161162
public void CompileMappings()
@@ -218,7 +219,7 @@ LambdaExpression GenerateObjectMapperExpression(in MapRequest mapRequest, IObjec
218219
return Lambda(fullExpression, source, destination, ContextParameter);
219220
}
220221
}
221-
IGlobalConfigurationExpression ConfigurationExpression => _validator.Expression;
222+
internal IGlobalConfigurationExpression ConfigurationExpression => _configurationExpression;
222223
ProjectionBuilder CreateProjectionBuilder() => new(this, [..ConfigurationExpression.ProjectionMappers]);
223224
IProjectionBuilder IGlobalConfiguration.ProjectionBuilder => _projectionBuilder.Value;
224225
Func<Type, object> IGlobalConfiguration.ServiceCtor => ConfigurationExpression.ServiceCtor;
@@ -471,14 +472,14 @@ IObjectMapper FindMapper(TypePair types)
471472
return null;
472473
}
473474
void IGlobalConfiguration.RegisterTypeMap(TypeMap typeMap) => _configuredMaps[typeMap.Types] = typeMap;
474-
void IGlobalConfiguration.AssertConfigurationIsValid(TypeMap typeMap) => _validator.AssertConfigurationIsValid(this, [typeMap]);
475+
void IGlobalConfiguration.AssertConfigurationIsValid(TypeMap typeMap) => Validator().AssertConfigurationIsValid([typeMap]);
475476
void IGlobalConfiguration.AssertConfigurationIsValid(string profileName)
476477
{
477478
if (Array.TrueForAll(Profiles, x => x.Name != profileName))
478479
{
479480
throw new ArgumentOutOfRangeException(nameof(profileName), $"Cannot find any profiles with the name '{profileName}'.");
480481
}
481-
_validator.AssertConfigurationIsValid(this, _configuredMaps.Values.Where(typeMap => typeMap.Profile.Name == profileName).ToArray());
482+
Validator().AssertConfigurationIsValid(_configuredMaps.Values.Where(typeMap => typeMap.Profile.Name == profileName).ToArray());
482483
}
483484
void IGlobalConfiguration.AssertConfigurationIsValid<TProfile>() => this.Internal().AssertConfigurationIsValid(typeof(TProfile).FullName);
484485
void IGlobalConfiguration.RegisterAsMap(TypeMapConfiguration typeMapConfiguration) =>

‎src/UnitTests/ConfigurationValidation.cs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,40 @@
11
namespace AutoMapper.UnitTests.ConfigurationValidation;
2+
public class When_testing_a_dto_with_mismatched_member_names_and_mismatched_types : AutoMapperSpecBase
3+
{
4+
public class Source
5+
{
6+
public decimal Foo { get; set; }
7+
}
8+
9+
public class Destination
10+
{
11+
public Type Foo { get; set; }
12+
public string Bar { get; set; }
13+
}
214

15+
protected override MapperConfiguration CreateConfiguration() =>
16+
new(cfg => { cfg.CreateMap<Source, Destination>(); });
17+
18+
[Fact]
19+
public void Should_throw_unmapped_member_and_mismatched_type_exceptions()
20+
{
21+
new Action(AssertConfigurationIsValid)
22+
.ShouldThrow<AggregateException>()
23+
.ShouldSatisfyAllConditions(
24+
aex => aex.InnerExceptions.ShouldBeOfLength(2),
25+
aex => aex.InnerExceptions[0]
26+
.ShouldBeOfType<AutoMapperConfigurationException>()
27+
.ShouldSatisfyAllConditions(
28+
ex => ex.Errors.ShouldBeOfLength(1),
29+
ex => ex.Errors[0].UnmappedPropertyNames.ShouldContain("Bar")),
30+
aex => aex.InnerExceptions[1]
31+
.ShouldBeOfType<AutoMapperConfigurationException>()
32+
.ShouldSatisfyAllConditions(
33+
ex => ex.MemberMap.ShouldNotBeNull(),
34+
ex => ex.MemberMap.DestinationName.ShouldBe("Foo"))
35+
);
36+
}
37+
}
338
public class ConstructorMappingValidation : NonValidatingSpecBase
439
{
540
public class Destination

0 commit comments

Comments
 (0)