Skip to content

Commit

Permalink
Better handling of MemberName extraction for broken circumstances (im…
Browse files Browse the repository at this point in the history
…provement of previous commit).
  • Loading branch information
jwaliszko committed Apr 24, 2016
1 parent 44e2ab7 commit 81c5f24
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 26 deletions.
58 changes: 58 additions & 0 deletions src/ExpressiveAnnotations.Tests/AttribsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,45 @@ public void throw_when_requirement_is_applied_to_field_of_non_nullable_value_typ
e.InnerException.Message);
}

[Fact]
public void verify_retrieval_of_correct_member_names_when_validation_context_is_broken()
{
var model = new HackTestModel();
var attrib = new AssertThatAttribute("false");

// correct path:
Assert.Equal("Value1", GetFinalMemberName(attrib, model, 0, "Value1", "Value1"));
Assert.Equal("Value2", GetFinalMemberName(attrib, model, 0, "Value2", "Value 2"));
Assert.Equal("Value3", GetFinalMemberName(attrib, model, 0, "Value3", "Value 3"));

// first issue: no member name provided (MVC <= 4)
Assert.Equal("Value1", GetFinalMemberName(attrib, model, 0, null, "Value1"));
Assert.Equal("Value2", GetFinalMemberName(attrib, model, 0, null, "Value 2"));
Assert.Equal("Value3", GetFinalMemberName(attrib, model, 0, null, "Value 3"));

// second issue: member name equals to display name (WebAPI 2)
Assert.Equal("Value2", GetFinalMemberName(attrib, model, 0, "Value 2", "Value 2"));
Assert.Equal("Value3", GetFinalMemberName(attrib, model, 0, "Value 3", "Value 3"));
}

[Fact]
public void return_null_in_place_of_member_name_when_it_cannot_be_unambiguously_retrieved()
{
var model = new DisplayDuplicateModel();
var attrib = new AssertThatAttribute("false");

Assert.Equal(null, GetFinalMemberName(attrib, model, 0, null, "duplicate"));
}

private static string GetFinalMemberName(ValidationAttribute attrib, object contextModel, object mamberValue, string givenMemberName, string givenDisplayName)
{
return attrib.GetValidationResult(mamberValue, new ValidationContext(contextModel)
{
MemberName = givenMemberName,
DisplayName = givenDisplayName
})?.MemberNames.Single();
}

private static void AssertErrorMessage(string input, string output)
{
AssertErrorMessage(input, output, output);
Expand Down Expand Up @@ -440,6 +479,25 @@ private class WorkModel
[AssertThat(HeavyExpression)]
public int? Value { get; set; }
}

private class HackTestModel
{
public int? Value1 { get; set; }

[Display(Name = "Value 2")]
public int? Value2 { get; set; }

[DisplayName("Value 3")]
public int? Value3 { get; set; }
}

private class DisplayDuplicateModel
{
[Display(Name = "duplicate")]
public int? Value1 { get; set; }
[Display(Name = "duplicate")]
public int? Value2 { get; set; }
}
}

public static class Helper
Expand Down
6 changes: 3 additions & 3 deletions src/ExpressiveAnnotations.Tests/UtilsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ public void verify_display_names_extraction_from_given_type()
[Fact]
public void verify_fields_names_extraction_based_on_their_display_names() // Display attribute or DisplayName attribute used as a workaround for field name extraction in older versions of MVC where MemberName was not provided in ValidationContext
{
Assert.Equal("Value1", typeof (Model).GetMemberNameByDisplayName("Value_1"));
Assert.Equal("Value2", typeof (Model).GetMemberNameByDisplayName("_{Value2}_"));
Assert.Equal("Internal", typeof (Model).GetMemberNameByDisplayName("internal"));
Assert.Equal("Value1", typeof (Model).GetPropertyByDisplayName("Value_1").Name);
Assert.Equal("Value2", typeof (Model).GetPropertyByDisplayName("_{Value2}_").Name);
Assert.Equal("Internal", typeof (Model).GetPropertyByDisplayName("internal").Name);
}

[Fact]
Expand Down
31 changes: 23 additions & 8 deletions src/ExpressiveAnnotations/Attributes/ExpressiveAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ protected ExpressiveAttribute(string expression, Func<string> errorMessageAccess
/// </summary>
protected Parser Parser { get; private set; }

/// <summary>
/// Gets the annotated property type.
/// </summary>
protected Type PropertyType { get; private set; }

/// <summary>
/// Gets the logical expression based on which specified condition is computed.
/// </summary>
Expand Down Expand Up @@ -272,20 +277,30 @@ protected override ValidationResult IsValid(object value, ValidationContext vali
catch (Exception e)
{
throw new ValidationException(
$"{GetType().Name}: validation applied to {validationContext.MemberName} field failed.", e);
$"{GetType().Name}: validation applied to {validationContext.MemberName ?? "<member unknown>"} field failed.", e);
}
}

private void AdjustMemberName(ValidationContext validationContext)
private void AdjustMemberName(ValidationContext validationContext) // fixes for: MVC <= 4 (MemberName is not provided), WebAPI 2 (MemberName states for display name)
{
if (validationContext.MemberName != null) // hack for WebAPI, where MemberName is set to display name
validationContext.MemberName = validationContext.ObjectType.GetMemberNameByDisplayName(validationContext.DisplayName);
PropertyType = null; // reset value
if (validationContext.MemberName == null && validationContext.DisplayName == null)
return;

validationContext.MemberName = validationContext.MemberName // hack for old MVC, where MemberName is not provided:
?? validationContext.ObjectType.GetMemberNameByDisplayName(validationContext.DisplayName) // extract property using display name through Display or DisplayName annotation
?? validationContext.DisplayName; // return DisplayName, which may contain the proper name if no Display nor DisplayName annotation exists
validationContext.MemberName = validationContext.MemberName ?? validationContext.DisplayName;
var prop = validationContext.ObjectType.GetProperty(validationContext.MemberName);
if (prop == null)
{
prop = validationContext.ObjectType.GetPropertyByDisplayName(validationContext.MemberName);
if (prop == null)
{
validationContext.MemberName = null;
return;
}

Debug.Assert(validationContext.MemberName != null);
validationContext.MemberName = prop.Name;
}
PropertyType = prop.PropertyType;
}

private string PreformatMessage(string displayName, string expression, out IList<FormatItem> items)
Expand Down
15 changes: 11 additions & 4 deletions src/ExpressiveAnnotations/Attributes/RequiredIfAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@ public RequiredIfAttribute(string expression)
/// <exception cref="System.InvalidOperationException"></exception>
protected override ValidationResult IsValidInternal(object value, ValidationContext validationContext)
{
var propType = validationContext.ObjectType.GetProperty(validationContext.MemberName).PropertyType;
if (value != null && propType.IsNonNullableValueType())
throw new InvalidOperationException(
$"{nameof(RequiredIfAttribute)} has no effect when applied to a field of non-nullable value type '{propType.FullName}'. Use nullable '{propType.FullName}?' version instead.");
AssertNonValueType(value, validationContext);

var isEmpty = value is string && string.IsNullOrWhiteSpace((string) value);
if (value == null || (isEmpty && !AllowEmptyStrings))
Expand All @@ -62,5 +59,15 @@ protected override ValidationResult IsValidInternal(object value, ValidationCont

return ValidationResult.Success;
}

private void AssertNonValueType(object value, ValidationContext validationContext)
{
if (PropertyType == null)
return;

if (value != null && PropertyType.IsNonNullableValueType())
throw new InvalidOperationException(
$"{nameof(RequiredIfAttribute)} has no effect when applied to a field of non-nullable value type '{PropertyType.FullName}'. Use nullable '{PropertyType.FullName}?' version instead.");
}
}
}
22 changes: 11 additions & 11 deletions src/ExpressiveAnnotations/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -250,41 +250,41 @@ public static IEnumerable<Type> GetLoadableTypes(this ITypeProvider provider)
}
}

public static string GetMemberNameByDisplayName(this Type type, string displayName)
public static PropertyInfo GetPropertyByDisplayName(this Type type, string displayName)
{
Debug.Assert(type != null);
Debug.Assert(displayName != null);

return type.GetMemberNameFromDisplayAttribute(displayName) ??
type.GetMemberNameFromDisplayNameAttribute(displayName);
return type.GetPropertyByDisplayAttribute(displayName) ??
type.GetPropertyByDisplayNameAttribute(displayName);
}

public static string GetMemberNameFromDisplayAttribute(this Type type, string displayName)
public static PropertyInfo GetPropertyByDisplayAttribute(this Type type, string displayName)
{
Debug.Assert(type != null);
Debug.Assert(displayName != null);

// get member name from Display attribute (if such an attribute exists) based on display name
// get member name through Display attribute (if such an attribute exists) based on display name
var props = type.GetProperties()
.Where(p => p.GetAttributes<DisplayAttribute>().Any(a => a.GetName() == displayName))
.Select(p => p.Name).ToList();
.ToList();

// if there is an ambiguity, return nothing
return props.Count == 1 ? props.SingleOrDefault() : null;
return props.Count == 1 ? props.Single() : null;
}

public static string GetMemberNameFromDisplayNameAttribute(this Type type, string displayName)
public static PropertyInfo GetPropertyByDisplayNameAttribute(this Type type, string displayName)
{
Debug.Assert(type != null);
Debug.Assert(displayName != null);

// get member name from DisplayName attribute (if such an attribute exists) based on display name
// get member name through DisplayName attribute (if such an attribute exists) based on display name
var props = type.GetProperties()
.Where(p => p.GetAttributes<DisplayNameAttribute>().Any(a => a.DisplayName == displayName))
.Select(p => p.Name).ToList();
.ToList();

// if there is an ambiguity, return nothing
return props.Count == 1 ? props.SingleOrDefault() : null;
return props.Count == 1 ? props.Single() : null;
}

public static IEnumerable<T> GetAttributes<T>(this MemberInfo element) where T : Attribute
Expand Down

0 comments on commit 81c5f24

Please sign in to comment.