Updates to validation and coercion: Fix #211 and force order of operations for presence and coercion.

[adamgotterer]

This pull request turned into something slightly larger then originally planned. While fixing this bug a few other bugs and improvements popped up that were addressed as well.

• Updated validator to only run custom optional validation when the param is present. Fixes Custom validator shouldn't be triggered when optional and not present #211.
• Re-wrote validations_spec and coercion_spec to be a more robust and have added coverage.
• Added section to documentation for custom validation.
• Changed order of validation run. Check for presence first, then run coercion. After both of those succeed the other validation rules can be processed. Coercion and presence validation were treated like all other validation rules accept there is no guarantee of the order its ran. It was possible that coercion happened after a validation rule had already seen and approved a value.

[adamgotterer]

Don't pull this yet. Just realized that this bug also exists when you set coerce on an optional param as well. Will fix tomorrow and update those test as well. And will double check these work with setting type.

[adamgotterer]

Spent a little more time looking into that bug and I don't believe its an issue. This pull request is ready for review and pull.

[adamgotterer]

Scratch that, I just resurfaced the bug. It seems that coercion isn't guarantee to happen before the validators run. If this is the case the validators could be working with incorrect types. I'm updating the code now to run coercion tests first.

[adamgotterer]

@dblock I'm done with all the changes here. Please review.

@schmurfy you should take a look as well and make sure I didn't break anything obvious. I was also playing a bit with the custom coercion classes. Do you think those should be validated as well? For example, I wrote the following tests expecting them to pass, but they didn't. After further investigation I realized it wasn't supported and outside the scope of the changes I was making. What do you think?

module CoerceValidatorSpec
  class User
    include Virtus
    attribute :id, Integer
    attribute :name, String
  end
end 

it 'validates a custom coercion' do
  subject.params { requires :user, :type => CoerceValidatorSpec::User }
  subject.get '/user' do 'complex works'; end   

  # Expected it to throw a 400 but throws a 200
  get '/user', :user => { :not_there => '32' }
  last_response.status.should == 400
  last_response.body.should == 'invalid parameter: user'

  # Should't complex object types validate types? 
  Not sure what the response would be here? Invalid user->id?
  get '/user', :user => { :id => 'not and int', :name => 'Bob' }
  last_response.status.should == 400    
end 

[schmurfy]

I did not see any breaking change, good patch :)
There are some things I wanted to like refactor required / optional but I just did not find the time, glad you did !

Validating a sub object like your example works but only partially, from what I remember your first test should succeed, the coercion should fails if the hash does not include any of the object attributes.
I will try to have a look tomorrow to confirm this theory.

[dblock]

This is a bit of a contrived example. It would be nice to find something more "useful".

[adamgotterer]

I'll take a shot at updated the whole section and improving my example. I've learned a lot of things that you can do that we haven't documented.

[dblock]

I am going to merge this, thank you. My comments are minor, feel free to make pulls to improve those!

[adamgotterer]

Just realized that I need to remove presence from the validation array. Shouldn't affect it negatively. Would just cause it to validate twice. I'll fix that with the next batch of updates.

[dblock]

Thanks for your hard work, Adam.

[adamgotterer]

Will make the documentation updated discussed here after #221 gets finalized and merged.

[schmurfy]

@adamgotterer I did some tests on validating sub objects and the case which is working (by returning 400) is passing an empty hash but otherwise you are right we cannot really implement this only with Virtus.

But there is hope ;)
One last thing I want to implement is allow validating these but differently:

params do
  requires 'user.id', :type => Integer
  requires 'user.login', :type => String
end

All the work I did on grape was inspired by what I did on goliath, since goliath now longer suits my needs due to recent changes I turned to grape which was the closest match (I was really surprised that really no frameworks suitable for writing quick REST services already exists with inputs validation).
Nested validation is the last thing I implemented in goliath which is not yet in grape and would allow validating sub objects, It could even be auto-generated from this notation as long as User is a Virtus objects:

params do
  requires :user, type: User
end

I may have time to give it a try this week end.

[schmurfy]

Here we go: #222