Skip to content

Commit 825bb8b

Browse files
committed
Updated BindingModelValidator to use MemberAccessor
Previously, the `BindingModelValidator` was operating directly off of the `PropertyInfo`, instead of using the `MemberAccessor`, since most of the methods only had access to the `PropertyInfo` . As a result, the calls to `SetValue()` and `GetValue()` were going directly through reflection, and not using the optimizations built into the new `MemberAccessor`. This behavior was a holdover from the legacy `TypeMemberInfoCollection` and `MemberDispatcher` architecture. While `BindingModelValidator` doesn't have any dependencies on those classes, it did continue to operate off of `PropertyInfo`. Now, for consistency, I've updated `ValidateModel()` to simply accept a `TypeAccessor` instead of a `sourceType` and a `properties` list. Further, all of its private methods now accept a `MemberAccessor` instead of a `PropertyInfo`. In addition to being more consistent with other libraries, this should also be a bit more performant, and at minimum helps ensure that all reflection access is going through `MemberAccessor`. This also mitigates a dependency on `PropertyConfiguration` needed to support the new constructor signature, which accepts a `MemberAccessor` (49ffd61).
1 parent 3d4c9c4 commit 825bb8b

2 files changed

Lines changed: 26 additions & 35 deletions

File tree

OnTopic/Mapping/Reverse/BindingModelValidator.cs

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ static internal class BindingModelValidator {
5757
/// This helper function is intended to provide reporting to developers about errors in their model. As a result, it
5858
/// will exclusively throw exceptions, as opposed to populating validation object for rendering to the view. Because
5959
/// it's only evaluating the compiled model, which will not change during the application's life cycle, the <paramref
60-
/// name="sourceType"/> and <paramref name="contentTypeDescriptor"/> are stored as a <see cref="Tuple"/> in a static
60+
/// name="typeAccessor"/> and <paramref name="contentTypeDescriptor"/> are stored as a <see cref="Tuple"/> in a static
6161
/// <see cref="ConcurrentBag{T}"/> once a particular combination has passed validation—that way, this check only needs
6262
/// to be executed once for any combination, at least for the current application life cycle.
6363
/// </para>
@@ -72,48 +72,43 @@ static internal class BindingModelValidator {
7272
/// cref="ContentTypeDescriptor"/> may not have undergone explicit testing.
7373
/// </para>
7474
/// </remarks>
75-
/// <param name="sourceType">
76-
/// The binding model <see cref="Type"/> to validate.
77-
/// </param>
78-
/// <param name="properties">
79-
/// A <see cref="IEnumerable{PropertyInfo}"/> describing the <paramref name="sourceType"/>'s properties.
75+
/// <param name="typeAccessor">
76+
/// The <see cref="TypeAccessor"/> of the binding model to validate.
8077
/// </param>
8178
/// <param name="contentTypeDescriptor">
8279
/// The <see cref="ContentTypeDescriptor"/> object against which to validate the model.
8380
/// </param>
8481
/// <param name="attributePrefix">The prefix to apply to the attributes.</param>
8582
static internal void ValidateModel(
86-
[AllowNull]Type sourceType,
87-
[AllowNull]IEnumerable<PropertyInfo> properties,
83+
[AllowNull]TypeAccessor typeAccessor,
8884
[AllowNull]ContentTypeDescriptor contentTypeDescriptor,
8985
[AllowNull]string attributePrefix = ""
9086
) {
9187

9288
/*------------------------------------------------------------------------------------------------------------------------
9389
| Validate parameters
9490
\-----------------------------------------------------------------------------------------------------------------------*/
95-
Contract.Requires(sourceType, nameof(sourceType));
96-
Contract.Requires(properties, nameof(properties));
91+
Contract.Requires(typeAccessor, nameof(typeAccessor));
9792
Contract.Requires(contentTypeDescriptor, nameof(contentTypeDescriptor));
9893

9994
/*------------------------------------------------------------------------------------------------------------------------
10095
| Skip validation if this type has already been validated for this content type
10196
\-----------------------------------------------------------------------------------------------------------------------*/
102-
if (_modelsValidated.Contains((sourceType, contentTypeDescriptor.Key))) {
97+
if (_modelsValidated.Contains((typeAccessor.Type, contentTypeDescriptor.Key))) {
10398
return;
10499
}
105100

106101
/*------------------------------------------------------------------------------------------------------------------------
107102
| Validate
108103
\-----------------------------------------------------------------------------------------------------------------------*/
109-
foreach (var property in properties) {
110-
ValidateProperty(sourceType, property, contentTypeDescriptor, attributePrefix);
104+
foreach (var property in typeAccessor.GetMembers(MemberTypes.Property)) {
105+
ValidateProperty(typeAccessor.Type, property, contentTypeDescriptor, attributePrefix);
111106
}
112107

113108
/*------------------------------------------------------------------------------------------------------------------------
114109
| Add type, content type to model validation cache so it isn't checked again
115110
\-----------------------------------------------------------------------------------------------------------------------*/
116-
_modelsValidated.Add((sourceType, contentTypeDescriptor.Key));
111+
_modelsValidated.Add((typeAccessor.Type, contentTypeDescriptor.Key));
117112

118113
return;
119114

@@ -129,16 +124,16 @@ static internal void ValidateModel(
129124
/// <param name="sourceType">
130125
/// The binding model <see cref="Type"/> to validate.
131126
/// </param>
132-
/// <param name="property">
133-
/// A <see cref="PropertyInfo"/> describing a specific property of the <paramref name="sourceType"/>.
127+
/// <param name="propertyAccessor">
128+
/// A <see cref="MemberAccessor"/> describing a specific property of the <paramref name="sourceType"/>.
134129
/// </param>
135130
/// <param name="contentTypeDescriptor">
136131
/// The <see cref="ContentTypeDescriptor"/> object against which to validate the model.
137132
/// </param>
138133
/// <param name="attributePrefix">The prefix to apply to the attributes.</param>
139134
static internal void ValidateProperty(
140135
[AllowNull]Type sourceType,
141-
[AllowNull]PropertyInfo property,
136+
[AllowNull]MemberAccessor propertyAccessor,
142137
[AllowNull]ContentTypeDescriptor contentTypeDescriptor,
143138
[AllowNull]string attributePrefix = ""
144139
) {
@@ -147,14 +142,13 @@ static internal void ValidateProperty(
147142
| Validate parameters
148143
\-----------------------------------------------------------------------------------------------------------------------*/
149144
Contract.Requires(sourceType, nameof(sourceType));
150-
Contract.Requires(property, nameof(property));
145+
Contract.Requires(propertyAccessor, nameof(propertyAccessor));
151146
Contract.Requires(contentTypeDescriptor, nameof(contentTypeDescriptor));
152147

153148
/*------------------------------------------------------------------------------------------------------------------------
154149
| Define variables
155150
\-----------------------------------------------------------------------------------------------------------------------*/
156-
var propertyType = property.PropertyType;
157-
var configuration = new PropertyConfiguration(property, attributePrefix);
151+
var configuration = new PropertyConfiguration(propertyAccessor, attributePrefix);
158152
var compositeAttributeKey = configuration.AttributeKey;
159153
var attributeDescriptor = contentTypeDescriptor.AttributeDescriptors.GetValue(compositeAttributeKey);
160154
var childCollections = new[] { CollectionType.Children, CollectionType.NestedTopics };
@@ -171,20 +165,18 @@ static internal void ValidateProperty(
171165
/*------------------------------------------------------------------------------------------------------------------------
172166
| Skip properties injected by the compiler for record types
173167
\-----------------------------------------------------------------------------------------------------------------------*/
174-
if (configuration.Property.Name is "EqualityContract") {
168+
if (propertyAccessor.Name is "EqualityContract") {
175169
return;
176170
}
177171

178172
/*------------------------------------------------------------------------------------------------------------------------
179173
| Handle mapping properties from referenced objects
180174
\-----------------------------------------------------------------------------------------------------------------------*/
181175
if (configuration.MapToParent) {
182-
var typeAccessor = TypeAccessorCache.GetTypeAccessor(propertyType);
183-
var childProperties = typeAccessor.GetMembers<PropertyInfo>();
176+
var typeAccessor = TypeAccessorCache.GetTypeAccessor(propertyAccessor.Type);
184177

185178
ValidateModel(
186-
propertyType,
187-
childProperties,
179+
typeAccessor,
188180
contentTypeDescriptor,
189181
configuration.AttributePrefix
190182
);
@@ -194,7 +186,7 @@ static internal void ValidateProperty(
194186
/*------------------------------------------------------------------------------------------------------------------------
195187
| Define list type (if it's a list)
196188
\-----------------------------------------------------------------------------------------------------------------------*/
197-
foreach (var type in configuration.Property.PropertyType.GetInterfaces()) {
189+
foreach (var type in propertyAccessor.Type.GetInterfaces()) {
198190
if (type.IsGenericType && typeof(IList<>) == type.GetGenericTypeDefinition()) {
199191
//Uses last argument in case it's a KeyedCollection; in that case, we want the TItem type
200192
listType = type.GetGenericArguments().Last();
@@ -232,7 +224,7 @@ static internal void ValidateProperty(
232224
throw new MappingModelValidationException(
233225
$"A '{nameof(sourceType)}' object was provided with a content type set to '{contentTypeDescriptor.Key}'. This " +
234226
$"content type does not contain an attribute named '{compositeAttributeKey}', as requested by the " +
235-
$"'{configuration.Property.Name}' property. If this property is not intended to be mapped by the " +
227+
$"'{propertyAccessor.Name}' property. If this property is not intended to be mapped by the " +
236228
$"{nameof(ReverseTopicMappingService)}, then it should be decorated with {nameof(DisableMappingAttribute)}."
237229
);
238230
}
@@ -252,7 +244,7 @@ attributeDescriptor.ModelType is ModelType.NestedTopic &&
252244
!typeof(ITopicBindingModel).IsAssignableFrom(listType)
253245
) {
254246
throw new MappingModelValidationException(
255-
$"The '{property.Name}' property on the '{sourceType.Name}' class has been determined to be a " +
247+
$"The '{propertyAccessor.Name}' property on the '{sourceType.Name}' class has been determined to be a " +
256248
$"{configuration.CollectionType}, but the generic type '{listType.Name}' does not implement the " +
257249
$"{nameof(ITopicBindingModel)} interface. This is required for binding models. If this collection is not intended " +
258250
$"to be mapped as a {ModelType.NestedTopic} then update the definition in the associated " +
@@ -266,11 +258,11 @@ attributeDescriptor.ModelType is ModelType.NestedTopic &&
266258
\-----------------------------------------------------------------------------------------------------------------------*/
267259
if (
268260
attributeDescriptor.ModelType is ModelType.Reference &&
269-
!typeof(IAssociatedTopicBindingModel).IsAssignableFrom(propertyType)
261+
!typeof(IAssociatedTopicBindingModel).IsAssignableFrom(propertyAccessor.Type)
270262
) {
271263
throw new MappingModelValidationException(
272-
$"The '{property.Name}' property on the '{sourceType.Name}' class has been determined to be a " +
273-
$"{ModelType.Reference}, but the generic type '{propertyType.Name}' does not implement the " +
264+
$"The '{propertyAccessor.Name}' property on the '{sourceType.Name}' class has been determined to be a " +
265+
$"{ModelType.Reference}, but the generic type '{propertyAccessor.Type.Name}' does not implement the " +
274266
$"{nameof(IAssociatedTopicBindingModel)} interface. This is required for references. If this property is not " +
275267
$"intended to be mapped as a {ModelType.Reference} then update the definition in the associated " +
276268
$"{nameof(ContentTypeDescriptor)}. If this property is not intended to be mapped at all, include the " +
@@ -314,12 +306,12 @@ [DisallowNull]Type listType
314306
/*------------------------------------------------------------------------------------------------------------------------
315307
| Define variables
316308
\-----------------------------------------------------------------------------------------------------------------------*/
317-
var property = configuration.Property;
309+
var property = configuration.MemberAccessor;
318310

319311
/*------------------------------------------------------------------------------------------------------------------------
320312
| Validate list
321313
\-----------------------------------------------------------------------------------------------------------------------*/
322-
if (!typeof(IList).IsAssignableFrom(property.PropertyType)) {
314+
if (!typeof(IList).IsAssignableFrom(property.Type)) {
323315
throw new MappingModelValidationException(
324316
$"The '{property.Name}' property on the '{sourceType.Name}' class maps to a relationship attribute " +
325317
$"'{attributeDescriptor.Key}', but does not implement {nameof(IList)}. Relationships must implement " +

OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,9 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) {
196196
| Validate model
197197
\-----------------------------------------------------------------------------------------------------------------------*/
198198
var typeAccessor = TypeAccessorCache.GetTypeAccessor(source.GetType());
199-
var properties = typeAccessor.GetMembers<PropertyInfo>();
200199
var contentTypeDescriptor = _contentTypeDescriptors.GetValue(target.ContentType);
201200

202-
BindingModelValidator.ValidateModel(source.GetType(), properties, contentTypeDescriptor, attributePrefix);
201+
BindingModelValidator.ValidateModel(typeAccessor, contentTypeDescriptor, attributePrefix);
203202

204203
/*------------------------------------------------------------------------------------------------------------------------
205204
| Loop through properties, mapping each one

0 commit comments

Comments
 (0)