Skip to content

Commit 9829dd8

Browse files
dougbudougbu
committed
Give ValidationContext.MemberName a distinct value from DisplayName
- #120 - revert to previous behavior if `webapi:UseLegacyValidationMemberName` application setting is `true`
1 parent 2b669ca commit 9829dd8

File tree

3 files changed

+274
-13
lines changed

3 files changed

+274
-13
lines changed

‎src/System.Web.Http/System.Web.Http.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
</Reference>
2222
<Reference Include="System" />
2323
<Reference Include="System.ComponentModel.DataAnnotations" />
24+
<Reference Include="System.Configuration" />
2425
<Reference Include="System.Core" />
2526
<Reference Include="System.Data.Linq" />
2627
<Reference Include="System.Net.Http" />

‎src/System.Web.Http/Validation/Validators/DataAnnotationsModelValidator.cs‎

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,23 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Collections.Generic;
5+
using System.Collections.Specialized;
56
using System.ComponentModel.DataAnnotations;
7+
using System.Configuration;
68
using System.Linq;
79
using System.Web.Http.Metadata;
810

911
namespace System.Web.Http.Validation.Validators
1012
{
1113
public class DataAnnotationsModelValidator : ModelValidator
1214
{
13-
public DataAnnotationsModelValidator(IEnumerable<ModelValidatorProvider> validatorProviders, ValidationAttribute attribute)
15+
internal static readonly string UseLegacyValidationMemberNameKey = "webapi:UseLegacyValidationMemberName";
16+
private static bool _useLegacyValidationMemberName =
17+
GetUseLegacyValidationMemberName(ConfigurationManager.AppSettings);
18+
19+
public DataAnnotationsModelValidator(
20+
IEnumerable<ModelValidatorProvider> validatorProviders,
21+
ValidationAttribute attribute)
1422
: base(validatorProviders)
1523
{
1624
if (attribute == null)
@@ -21,6 +29,13 @@ public DataAnnotationsModelValidator(IEnumerable<ModelValidatorProvider> validat
2129
Attribute = attribute;
2230
}
2331

32+
// Internal for testing
33+
internal static bool UseLegacyValidationMemberName
34+
{
35+
get { return _useLegacyValidationMemberName; }
36+
set { _useLegacyValidationMemberName = value; }
37+
}
38+
2439
protected internal ValidationAttribute Attribute { get; private set; }
2540

2641
public override bool IsRequired
@@ -30,25 +45,39 @@ public override bool IsRequired
3045

3146
public override IEnumerable<ModelValidationResult> Validate(ModelMetadata metadata, object container)
3247
{
48+
string memberName;
49+
if (_useLegacyValidationMemberName)
50+
{
51+
// Using member name from a Display or DisplayFormat attribute is generally incorrect. This
52+
// (configuration-controlled) override is provided only for corner cases where strict
53+
// back-compatibility is required.
54+
memberName = metadata.GetDisplayName();
55+
}
56+
else
57+
{
58+
// MemberName expression matches GetDisplayName() except it ignores Display and DisplayName
59+
// attributes. ModelType fallback isn't great and can separate errors and attempted values in the
60+
// ModelState. But, expression matches MVC, avoids a null MemberName, and is mostly back-compatible.
61+
memberName = metadata.PropertyName ?? metadata.ModelType.Name;
62+
}
63+
3364
// Per the WCF RIA Services team, instance can never be null (if you have
3465
// no parent, you pass yourself for the "instance" parameter).
35-
36-
string memberName = metadata.GetDisplayName();
37-
ValidationContext context = new ValidationContext(container ?? metadata.Model)
66+
ValidationContext context = new ValidationContext(instance: container ?? metadata.Model)
3867
{
39-
DisplayName = memberName,
40-
MemberName = memberName
68+
DisplayName = metadata.GetDisplayName(),
69+
MemberName = memberName,
4170
};
4271

4372
ValidationResult result = Attribute.GetValidationResult(metadata.Model, context);
4473

4574
if (result != ValidationResult.Success)
4675
{
47-
// ModelValidationResult.MemberName is used by invoking validators (such as ModelValidationNode) to
48-
// construct the ModelKey for ModelStateDictionary. When validating at type level we want to append the
49-
// returned MemberNames if specified (e.g. person.Address.FirstName). For property validation, the
50-
// ModelKey can be constructed using the ModelMetadata and we should ignore MemberName (we don't want
51-
// (person.Name.Name). However the invoking validator does not have a way to distinguish between these two
76+
// ModelValidationResult.MemberName is used by invoking validators (such as ModelValidationNode) to
77+
// construct the ModelKey for ModelStateDictionary. When validating at type level we want to append the
78+
// returned MemberNames if specified (e.g. person.Address.FirstName). For property validation, the
79+
// ModelKey can be constructed using the ModelMetadata and we should ignore MemberName (we don't want
80+
// (person.Name.Name). However the invoking validator does not have a way to distinguish between these two
5281
// cases. Consequently we'll only set MemberName if this validation returns a MemberName that is different
5382
// from the property being validated.
5483

@@ -63,11 +92,29 @@ public override IEnumerable<ModelValidationResult> Validate(ModelMetadata metada
6392
Message = result.ErrorMessage,
6493
MemberName = errorMemberName
6594
};
66-
95+
6796
return new ModelValidationResult[] { validationResult };
6897
}
6998

7099
return Enumerable.Empty<ModelValidationResult>();
71100
}
101+
102+
// Internal for testing
103+
internal static bool GetUseLegacyValidationMemberName(NameValueCollection appSettings)
104+
{
105+
var useLegacyMemberNameArray = appSettings.GetValues(UseLegacyValidationMemberNameKey);
106+
if (useLegacyMemberNameArray != null &&
107+
useLegacyMemberNameArray.Length > 0)
108+
{
109+
bool useLegacyMemberName;
110+
if (bool.TryParse(useLegacyMemberNameArray[0], out useLegacyMemberName) &&
111+
useLegacyMemberName)
112+
{
113+
return true;
114+
}
115+
}
116+
117+
return false;
118+
}
72119
}
73120
}

‎test/System.Web.Http.Test/Validation/Validators/DataAnnotationsModelValidatorTest.cs‎

Lines changed: 214 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Collections.Generic;
5+
using System.Collections.Specialized;
56
using System.ComponentModel.DataAnnotations;
67
using System.Linq;
78
using System.Web.Http.Metadata;
89
using System.Web.Http.Metadata.Providers;
10+
using System.Web.WebPages.TestUtils;
911
using Microsoft.TestCommon;
1012
using Moq;
1113
using Moq.Protected;
@@ -47,6 +49,90 @@ public void ValuesSet()
4749
Assert.Same(attribute, validator.Attribute);
4850
}
4951

52+
public static TheoryDataSet<NameValueCollection> FalseAppSettingsData
53+
{
54+
get
55+
{
56+
return new TheoryDataSet<NameValueCollection>
57+
{
58+
new NameValueCollection(),
59+
new NameValueCollection
60+
{
61+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "false" },
62+
},
63+
new NameValueCollection
64+
{
65+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "False" },
66+
},
67+
new NameValueCollection
68+
{
69+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "false" },
70+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "true" },
71+
},
72+
new NameValueCollection
73+
{
74+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "garbage" },
75+
},
76+
};
77+
}
78+
}
79+
80+
[Theory]
81+
[PropertyData("FalseAppSettingsData")]
82+
public void GetUseLegacyValidationMemberName_ReturnsFalse(NameValueCollection appSettings)
83+
{
84+
// Arrange & Act
85+
var result = DataAnnotationsModelValidator.GetUseLegacyValidationMemberName(appSettings);
86+
87+
// Assert
88+
Assert.False(result);
89+
}
90+
91+
public static TheoryDataSet<NameValueCollection> TrueAppSettingsData
92+
{
93+
get
94+
{
95+
return new TheoryDataSet<NameValueCollection>
96+
{
97+
new NameValueCollection
98+
{
99+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "true" },
100+
},
101+
new NameValueCollection
102+
{
103+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "True" },
104+
},
105+
new NameValueCollection
106+
{
107+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "true" },
108+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "false" },
109+
},
110+
new NameValueCollection
111+
{
112+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "true" },
113+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "false" },
114+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "garbage" },
115+
},
116+
new NameValueCollection
117+
{
118+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "True" },
119+
{ DataAnnotationsModelValidator.UseLegacyValidationMemberNameKey, "garbage" },
120+
},
121+
};
122+
}
123+
}
124+
125+
[Theory]
126+
[PropertyData("TrueAppSettingsData")]
127+
public void GetUseLegacyValidationMemberName_ReturnsTrue(NameValueCollection appSettings)
128+
{
129+
// Arrange & Act
130+
var result = DataAnnotationsModelValidator.GetUseLegacyValidationMemberName(appSettings);
131+
132+
// Assert
133+
Assert.True(result);
134+
}
135+
50136
public static TheoryDataSet<ModelMetadata, string> ValidateSetsMemberNamePropertyDataSet
51137
{
52138
get
@@ -57,10 +143,14 @@ public static TheoryDataSet<ModelMetadata, string> ValidateSetsMemberNamePropert
57143
_metadataProvider.GetMetadataForProperty(() => 15, typeof(string), "Length"),
58144
"Length"
59145
},
146+
{
147+
_metadataProvider.GetMetadataForProperty(() => string.Empty, typeof(AnnotatedModel), "Name"),
148+
"Name"
149+
},
60150
{
61151
_metadataProvider.GetMetadataForType(() => new object(), typeof(SampleModel)),
62152
"SampleModel"
63-
}
153+
},
64154
};
65155
}
66156
}
@@ -89,6 +179,123 @@ public void ValidateSetsMemberNamePropertyOfValidationContextForProperties(Model
89179
attribute.VerifyAll();
90180
}
91181

182+
[Fact]
183+
public void ValidateSetsMemberNameProperty_UsingDisplayName()
184+
{
185+
AppDomainUtils.RunInSeparateAppDomain(ValidateSetsMemberNameProperty_UsingDisplayName_Inner);
186+
}
187+
188+
private static void ValidateSetsMemberNameProperty_UsingDisplayName_Inner()
189+
{
190+
// Arrange
191+
DataAnnotationsModelValidator.UseLegacyValidationMemberName = true;
192+
var expectedMemberName = "Annotated Name";
193+
var attribute = new Mock<ValidationAttribute> { CallBase = true };
194+
attribute
195+
.Protected()
196+
.Setup<ValidationResult>("IsValid", ItExpr.IsAny<object>(), ItExpr.IsAny<ValidationContext>())
197+
.Callback((object o, ValidationContext context) =>
198+
{
199+
Assert.Equal(expectedMemberName, context.MemberName);
200+
})
201+
.Returns(ValidationResult.Success)
202+
.Verifiable();
203+
var validator = new DataAnnotationsModelValidator(_noValidatorProviders, attribute.Object);
204+
var metadata = _metadataProvider.GetMetadataForProperty(() => string.Empty, typeof(AnnotatedModel), "Name");
205+
206+
// Act
207+
var results = validator.Validate(metadata, container: null);
208+
209+
// Assert
210+
Assert.Empty(results);
211+
attribute.VerifyAll();
212+
}
213+
214+
// Confirm explicit false setting does not change Validate(...)'s behavior from its default.
215+
[Fact]
216+
public void ValidateSetsMemberNameProperty_NotUsingDisplayName()
217+
{
218+
AppDomainUtils.RunInSeparateAppDomain(ValidateSetsMemberNameProperty_NotUsingDisplayName_Inner);
219+
}
220+
221+
private static void ValidateSetsMemberNameProperty_NotUsingDisplayName_Inner()
222+
{
223+
// Arrange
224+
DataAnnotationsModelValidator.UseLegacyValidationMemberName = false;
225+
var expectedMemberName = "Name";
226+
var attribute = new Mock<ValidationAttribute> { CallBase = true };
227+
attribute
228+
.Protected()
229+
.Setup<ValidationResult>("IsValid", ItExpr.IsAny<object>(), ItExpr.IsAny<ValidationContext>())
230+
.Callback((object o, ValidationContext context) =>
231+
{
232+
Assert.Equal(expectedMemberName, context.MemberName);
233+
})
234+
.Returns(ValidationResult.Success)
235+
.Verifiable();
236+
var validator = new DataAnnotationsModelValidator(_noValidatorProviders, attribute.Object);
237+
var metadata = _metadataProvider.GetMetadataForProperty(() => string.Empty, typeof(AnnotatedModel), "Name");
238+
239+
// Act
240+
var results = validator.Validate(metadata, container: null);
241+
242+
// Assert
243+
Assert.Empty(results);
244+
attribute.VerifyAll();
245+
}
246+
247+
public static TheoryDataSet<ModelMetadata, string> ValidateSetsDisplayNamePropertyDataSet
248+
{
249+
get
250+
{
251+
return new TheoryDataSet<ModelMetadata, string>
252+
{
253+
{
254+
_metadataProvider.GetMetadataForProperty(() => 15, typeof(string), "Length"),
255+
"Length"
256+
},
257+
{
258+
_metadataProvider.GetMetadataForProperty(() => string.Empty, typeof(AnnotatedModel), "Name"),
259+
"Annotated Name"
260+
},
261+
{
262+
_metadataProvider.GetMetadataForType(() => new object(), typeof(SampleModel)),
263+
"SampleModel"
264+
},
265+
};
266+
}
267+
}
268+
269+
[Theory]
270+
[PropertyData("ValidateSetsDisplayNamePropertyDataSet")]
271+
public void ValidateSetsDisplayNamePropertyOfValidationContextAsExpected(
272+
ModelMetadata metadata,
273+
string expectedDisplayName)
274+
{
275+
// Arrange
276+
var attribute = new Mock<ValidationAttribute>
277+
{
278+
CallBase = true,
279+
};
280+
attribute
281+
.Protected()
282+
.Setup<ValidationResult>("IsValid", ItExpr.IsAny<object>(), ItExpr.IsAny<ValidationContext>())
283+
.Callback(
284+
(object o, ValidationContext context) => Assert.Equal(expectedDisplayName, context.DisplayName))
285+
.Returns(ValidationResult.Success)
286+
.Verifiable();
287+
DataAnnotationsModelValidator validator = new DataAnnotationsModelValidator(
288+
_noValidatorProviders,
289+
attribute.Object);
290+
291+
// Act
292+
IEnumerable<ModelValidationResult> results = validator.Validate(metadata, container: null);
293+
294+
// Assert
295+
Assert.Empty(results);
296+
attribute.VerifyAll();
297+
}
298+
92299
[Fact]
93300
public void ValidateWithIsValidTrue()
94301
{
@@ -222,5 +429,11 @@ class SampleModel
222429
{
223430
public string Name { get; set; }
224431
}
432+
433+
private class AnnotatedModel
434+
{
435+
[Display(Name = "Annotated Name")]
436+
public string Name { get; set; }
437+
}
225438
}
226439
}

0 commit comments

Comments
 (0)