added validation behind the coercion

Author: schmurfy

README.markdown

@@ -207,14 +207,8 @@ get ':id' do
 end
 ```
 
-Declaring a parameter type causes the value to be coerced into that type, where possible.  
-There is now implicit validation meaning that invalid input such as "foo" for an Integer will be left unmodified and passed through into the API,  
-this might change in a future release.
+When a type is specified an implicit validation is done after the coercion to ensure the output type is what you asked.
 
-However validations are executed in the order defined which allows you to do this:  
-```ruby
-requires :id, regexp: /^[0-9]+$/, type: Integer
-```
 
 ## Headers
 

lib/grape/validations/coerce.rb

@@ -8,14 +8,48 @@ module Validations
 
     class CoerceValidator < SingleOptionValidator
       def validate_param!(attr_name, params)
-        params[attr_name] = coerce_value(@option, params[attr_name])
+        new_value = coerce_value(@option, params[attr_name])
+        if valid_type?(new_value)
+          params[attr_name] = new_value
+        else
+          throw :error, :status => 400, :message => "invalid parameter: #{attr_name}"
+        end
       end
 
     private
+      def _valid_array_type?(type, values)
+        values.all? do |val|
+          _valid_single_type?(type, val)
+        end
+      end
+      
+      
+      def _valid_single_type?(klass, val)
+        if klass == Virtus::Attribute::Boolean
+          val.is_a?(TrueClass) || val.is_a?(FalseClass)
+        else
+          val.is_a?(klass)
+        end
+      end
+      
+      def valid_type?(val)
+        if @option.is_a?(Array)
+          _valid_array_type?(@option[0], val)
+        else
+          _valid_single_type?(@option, val)
+        end
+      end
+      
       def coerce_value(type, val)
         converter = Virtus::Attribute.build(:a, type)
         converter.coerce(val)
+      
+      # not the prettiest but some invalid coercion can currently trigger
+      # errors in Virtus (see coerce_spec.rb)
+      rescue => err
+        nil
       end
+      
     end
 
   end

spec/grape/validations/coerce_spec.rb

@@ -1,8 +1,40 @@
 require 'spec_helper'
 
+require 'virtus'
+
+module CoerceTest
+  class User
+    include Virtus
+    
+    attribute :id, Integer
+    attribute :name, String
+  end
+  
+end
+
+
 class CoerceAPI < Grape::API
   default_format :json
 
+  
+  params do
+    requires :int, :coerce => Integer
+  end
+  get '/single' do
+  end
+  
+  params do
+    requires :ids, type: Array[Integer]
+  end
+  get '/arr' do
+  end
+  
+  params do
+    requires :user, type: CoerceTest::User
+  end
+  get '/user' do
+  end
+  
   params do
     requires :int, :coerce => Integer
     optional :arr, :coerce => Array[Integer]
@@ -24,11 +56,30 @@ def app; @app; end
     @app = CoerceAPI
   end
 
-  # TOOD: Later when virtus can tell us that an input IS invalid
-  # it "should return an error on malformed input" do
-  #   get '/coerce', :int => "43a"
-  #   last_response.status.should == 400
-  # end
+  it "should return an error on malformed input" do
+    get '/single', :int => "43a"
+    last_response.status.should == 400
+    
+    get '/single', :int => "43"
+    last_response.status.should == 200
+  end
+  
+  it "should return an error on malformed input (array)" do
+    get '/arr', :ids => ["1", "2", "az"]
+    last_response.status.should == 400
+    
+    get '/arr', :ids => ["1", "2", "890"]
+    last_response.status.should == 200
+  end
+  
+  it "should return an error on malformed input (complex object)" do
+    # this request does raise an error inside Virtus
+    get '/user', :user => "32"
+    last_response.status.should == 400
+    
+    get '/user', :user => {id: 32, name: "Bob"}
+    last_response.status.should == 200
+  end
 
   it 'should coerce inputs' do
     get('/coerce', :int => "43")
@@ -37,6 +88,7 @@ def app; @app; end
     ret["int"].should == "Fixnum"
 
     get('/coerce', :int => "40", :arr => ["1","20","3"], :bool => [1, 0])
+    # last_response.body.should == ""
     last_response.status.should == 200
     ret = MultiJson.load(last_response.body)
     ret["int"].should == "Fixnum"