Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Model validation overhaul #1265

Closed
7 tasks done
thecodejunkie opened this issue Oct 15, 2013 · 6 comments
Closed
7 tasks done

Model validation overhaul #1265

thecodejunkie opened this issue Oct 15, 2013 · 6 comments
Assignees
Milestone

Comments

@thecodejunkie
Copy link
Member

It's time to give the model validation support and overlook and see what parts needs overhauling. This issue will only track what is needed and what has been done. Each change should be logged as a different issue and tracked in here

  • Breaking Change Update DataAnnotations and FluentValidation packages so that they register all their required types using an application-registration. These packages should work without the use of the default bootstrapper auto-registration capabilities: See Validator registration improvements #1267
  • Remove the hard-coded factories of IDataAnnotationsValidatorAdapter and IFluentAdapter and make is so that they are registered using an application-registration and then resolved as ctor dependencies where ever they are required. This will make it easier to add custom validation rules. Add implementation -> automatically available -> super-duper-happy-path. See Data annotations improvements #1279 for Data Annotations changes and Fluent validation improvements #1305 for fluent validation changes
  • Breaking Change Pass in NancyContext to IModelValidator.Validate(...) to follow our "context everywhere"-ethos. See Passing NancyContext to IModelValidator.Validate #1311
  • Breaking Change Look over ModelValidationResult.Errors and make it more user-friendly. It needs to have a good way to get a list of errors per field. This is to make it easier to connect it to a form etc. Possibly be able to return the data as Dictionary<string, IEnumerable<string>> where the string is the name of the property and the list is the validation error messages for that property. See Updated ModelValidationResult and ModelValidationError #1318
  • The DataAnnotations project is missing an DataAnnotationsValidatorAdapter implementation for the CompareAttribute. See comment Model validation overhaul #1265 (comment)
  • Breaking Change Revise the ModelValidationRule class and IDataAnnotationsValidatorAdapter.GetRules(ValidationAttribute attribute, PropertyDescriptor descriptor) implementations. The rules needs to be more friendly to consume from a client-side perspective. Something like a collection where the key is the member name and then a collection of applicable rules. The changes should be reflected on ModelValidationDescriptor and should probably be able to return something like Dictionary<string, IEnumerable<ModelValidationRule>>, where the string is the name of the property and the list contains the validation rules that should be enforced for that property. See Grouped validation rules per property #1314
  • Clean up the types in Nancy.Validation namespace. Make sure all types follow the code conventions and styles. Add missing XML comments to public types and members. See Nancy validation cleanup #1306
@ghost ghost assigned thecodejunkie Oct 15, 2013
@thecodejunkie
Copy link
Member Author

With regards to the CompareAttribute, this is a 4.5 only attribute and there are a couple of others that was introduced in 4.5. Currently Nancy.Validation.DataAnnotations (and Nancy in general) target 4.0. What we (or perhaps someone in the community) could do is write a set of adapters for the 4.5-only attributes and publish those as separate package (like Nancy.Validation.DataAnnotations.Extensions)

@thecodejunkie
Copy link
Member Author

Putting this here so I don't forget. #1279 (comment) How do we handle auto registrations of IEnumerable<T> with the Unity-bootstrapper?

@jchannon
Copy link
Member

jchannon commented Nov 5, 2013

Need to be able to bind to instances and to a list of instances if said instance/List is a property on the model. See #1263 to that shows a where binding to a property of a List fails. I've also tried to bind just a T instance and this also fails.

@thecodejunkie
Copy link
Member Author

@jchannon that's not English, try again ;)

@jchannon
Copy link
Member

jchannon commented Nov 6, 2013

@thecodejunkie updated to English from Swedish 😄

@CumpsD
Copy link
Contributor

CumpsD commented Jan 5, 2014

I am so looking forward to this! 👍

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