Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Need better way to suppress validation of properties/types during model binding #5642

Closed
Eilon opened this issue Dec 28, 2016 · 9 comments
Closed

Comments

@Eilon
Copy link
Member

Eilon commented Dec 28, 2016

Today it is not easy to statically disable validation explicitly on a particular property of a particular type. It also isn't as easy as it could be to disable validation on a type (it is possible to do today, but perhaps not as easy as it could be).

See related: #5633 (Model binding, BindNeverAttribute and BindAttribute ignored ?)

It is also not easy to decide whether to disable validation of a property at runtime.

See related: #4143 (Ignore validation of model property)

We should come up with a simple solution for the first problem, e.g. a new [ValidateNever] attribute (attribute applies to property + type), and perhaps take a look at how to disable validation dynamically a possibility.

@Eilon
Copy link
Member Author

Eilon commented Dec 28, 2016

@dougbu let's discuss with @rynowak and @danroth27 when everyone is back in the office.

@joacar
Copy link

joacar commented Jan 11, 2017

I would also be great to have an OptionalAttribute that would apply to types in collections. Perhaps I'm doing something wrong from start, but my use case is that I have a Customer object that has list of Contact's. To utilize EditorFor I've to initialize the list with an empty Contact. This causes the ModelState to be invalid if the required properties of Contact hasn't been supplied.

Problem might be how to determine the "placeholder"-object from invalid objects.

Edit: Unrelated - could you provide me some documenation on how to (if possible) override indexing for collections during model binding phase? I'm creating entries client side ideally I would like to just have i guid or something as name/index instead of consecutive integers since maintaining those in consecutive order is a pain :)

@Eilon
Copy link
Member Author

Eilon commented Jan 11, 2017

@joacar can you log this in a new issue so we can discuss it separately?

@Eilon
Copy link
Member Author

Eilon commented Jan 11, 2017

(Actually, probably two new issues because it seems like you have two separate topics. Thanks!)

@dougbu
Copy link
Member

dougbu commented Jan 11, 2017

FYI a [ValidateSometimes] attribute that implements an interface like

public interface IShouldValidate
{
    /// <summary>
    /// Gets an indication whether the <paramref name="entry"/> should be validated.
    /// </summary>
    /// <param name="entry"><see cref="ValidationEntry"/> to check.</param>
    /// <param name="parentEntry"><see cref="ValidationEntry"/> containing <paramref name="entry"/>.</param>
    /// <returns><c>true</c> if <paramref name="entry"/> should be validated; <c>false</c> otherwise.</returns>
    bool ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry);
}

is an alternative to the approach I've taken so far on this bug for dynamic validation decisions. The more I think about it, the more I like it. Would reduce the number of classes users need to write and would avoid DI replacements. Even better, would be possible for [ValidateNever] to use the same extensibility point. I'll prototype this before sending out a PR to see if it really is an improvement.

But [ValidateNever] and the potential IShouldValidate extensibility point are analogous to existing model binding / validation attributes and related controls: Applying them to a class changes how properties in that class are handled, not how properties with that class work e.g. [BindNever] on class A disables binding for A.B and not for public A C { get; set; } in some other class.

So, you'd either need to apply your [ValidateSometimes] to your Customer.Contacts property or replace some bigger parts of the system. Fortunately, it seems checking the Contacts property for a single non-initialized element will work in this case.

@joacar
Copy link

joacar commented Jan 12, 2017

@dougbu Could you point me to some implementation of using that technique? Sounds promising

@dougbu
Copy link
Member

dougbu commented Jan 12, 2017

@joacar see #5676. Note that PR is just in its first iteration.

dougbu added a commit that referenced this issue Jan 14, 2017
- #5642
- lazy-load `ValidationEntry.Model`
  - avoids `Exception`s when moving to a property that will not be validated

nits:
- remove duplicate code in `ValidationVisitor`
- clarify "all properties of" doc comments
  - also add missing `<param>` doc in `ViewDataInfo`
dougbu added a commit that referenced this issue Jan 16, 2017
- #5642
- lazy-load `ValidationEntry.Model`
  - avoids `Exception`s when moving to a property that will not be validated

nits:
- remove duplicate code in `ValidationVisitor`
- clarify "all properties of" doc comments
  - also add missing `<param>` doc in `ViewDataInfo`
@dougbu
Copy link
Member

dougbu commented Jan 16, 2017

ce53675

@joacar there's an outline / example of a ValidateSometimesAttribute here as well as a few examples applying [ValidateNever] in the same test class.

@dougbu dougbu closed this as completed Jan 16, 2017
@joacar
Copy link

joacar commented Jan 16, 2017

Great, thanks. Will check it out

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants