Closed girtsl closed 8 years ago
Could you please provide some tiny sample for this to be reproduced? I have many properties with names containing spaces used across multiple tests and no error is being thrown.
My bad for not trying it out in an MVC application - I mostly work with WebApi. You are right, of course, in MVC it works as expected and validationContext.MemberName
indeed contains the member's name.
I guess it must be a bug in WebAPI's model binder/validator, which puts the DisplayName in MemberName. You can reproduce the issue with the class I posted above and this minimalistic API controller:
public class DefaultController : ApiController
{
[HttpPost]
public IHttpActionResult Index(SampleModel model)
{
return Ok();
}
}
I see that this is not a bug with your lib, as you are targeting MVC, but it also stops your lib from being used in WebAPI projects. Which, prior to version 2.7.0, worked beautifully.
Thanks for clarification. There were previously issues with this MemberName
as well, i.e.
in MVC 4 DataAnnotationsModelValidator::Validate()
method, there is no MemberName
provided, only DisplayName
(from the MVC 4.0.0.0 sources):
ValidationContext context = new ValidationContext(container ?? Metadata.Model, null, null);
context.DisplayName = Metadata.GetDisplayName();
that's why I'm trying to retrieve this MemberName
using DisplayName
through reflection lookup for Display
or DisplayName
attributes. If I find any, I get the name of the decorated property which becomes the MemberName
, i.e. (from the EA 2.7.0 sources)
validationContext.MemberName = validationContext.MemberName
?? validationContext.ObjectType.GetMemberNameByDisplayName(validationContext.DisplayName);
I've just found yet another error here, because when there is no Display
nor DisplayName
annotation, we end up with null
. I've just added one single line, mainly ?? DisplayName
:
validationContext.MemberName = validationContext.MemberName
?? validationContext.ObjectType.GetMemberNameByDisplayName(validationContext.DisplayName)
?? DisplayName;
Why? From the dive into MVC 4.0.0.0 source code:
public string GetDisplayName()
{
return DisplayName ?? PropertyName ?? ModelType.Name;
}
I can see than when there is no DisplayName
in the metadata, the name of the property itself is returned.
in MVC 5 situation is fixed, MemberName
is there (from the MVC 5.2.3.0 sources):
string memberName = Metadata.PropertyName ?? Metadata.ModelType.Name;
ValidationContext context = new ValidationContext(container ?? Metadata.Model)
{
DisplayName = Metadata.GetDisplayName(),
MemberName = memberName
};
in WebAPI, from what you have described, we do suffer because of yet another issue. Due to the fact we have DisplayName
in the place of MemberName
, we probably need to lookup for the name of the property in the same way, it's done for MVC 4 case. So will we end up with something like this:
if (validationContext.MemberName != null) // hack for WebAPI, where `MemberName` is actually `DisplayName`
validationContext.MemberName = validationContext.ObjectType.GetMemberNameByDisplayName(validationContext.DisplayName);
validationContext.MemberName = validationContext.MemberName // hack for old MVC, where `MemberName` is `null`:
?? validationContext.ObjectType.GetMemberNameByDisplayName(validationContext.DisplayName) // get name based on display attributes, or
?? validationContext.DisplayName; // if there weren't any display* attributes, get the display name itself, which may be set to property name in a such case
? Why MemberName
is that important? I need it because a safety check has been added in case of ineffective RequiredIf
annotations for value types. I'd like keep that check, to prevent such mistakes in WebAPI as well.
So we probably need this yet another hack shown above, unless there is any smarter way?
The following commit: https://github.com/jwaliszko/ExpressiveAnnotations/commit/44e2ab792d149abdfe6142a90c0cec84cc4b28a4, will most likely fix both problems mentioned in this topic.
Yes, it fixes the issue(s). Thanks!
Here is a simplified class to reproduce the bug:
I traced it to
RequiredIfAttribute.IsValidInternal()
-validationContext.MemberName
contains the display name "First name", which, of course, is not a property name and thus thevalidationContext.ObjectType.GetProperty()
returns null.Unfortunately I cannot offer a solution as I haven't dealt with MVC validation in depth.
P.S. This was introduced in 2.7.0, commit https://github.com/jwaliszko/ExpressiveAnnotations/commit/b4afd1eef0ef9b4a7d486081812ef8d1d3750b12