-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
validation / coercion support #201
Changes from 9 commits
4800c93
79435d6
d0d7872
fdd865b
bc3318c
b68b777
87b46d7
50b8dc8
beb22f3
8f89489
b5db980
908bc2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
require 'virtus' | ||
|
||
module Grape | ||
|
||
module Validations | ||
|
||
## | ||
# All validators must inherit from this class. | ||
# | ||
class Validator | ||
def initialize(attrs, options) | ||
@attrs = Array(attrs) | ||
|
||
if options.is_a?(Hash) && !options.empty? | ||
raise "unknown options: #{options.keys}" | ||
end | ||
end | ||
|
||
def validate!(params) | ||
@attrs.each do |attr_name| | ||
validate_param!(attr_name, params) | ||
end | ||
end | ||
|
||
private | ||
|
||
def self.convert_to_short_name(klass) | ||
ret = klass.name.gsub(/::/, '/'). | ||
gsub(/([A-Z]+)([A-Z][a-z])/,'\1_\2'). | ||
gsub(/([a-z\d])([A-Z])/,'\1_\2'). | ||
tr("-", "_"). | ||
downcase | ||
File.basename(ret, '_validator') | ||
end | ||
end | ||
|
||
## | ||
# Base class for all validators taking only one param. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you want to create a Update: saw you did this further. I believe the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kept SingleOptionValidator there to clearly separate the foundation classes and the real validators but I can move it too if you prefer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't feel strongly about it. Keep it. |
||
class SingleOptionValidator < Validator | ||
def initialize(attrs, options) | ||
@option = options | ||
super | ||
end | ||
|
||
end | ||
|
||
# we define Validator::inherited here so SingleOptionValidator | ||
# will not be considered a validator. | ||
class Validator | ||
def self.inherited(klass) | ||
short_name = convert_to_short_name(klass) | ||
Validations::register_validator(short_name, klass) | ||
end | ||
end | ||
|
||
|
||
|
||
class <<self | ||
attr_accessor :validators | ||
end | ||
|
||
self.validators = {} | ||
|
||
def self.register_validator(short_name, klass) | ||
validators[short_name] = klass | ||
end | ||
|
||
|
||
class ParamsScope | ||
def initialize(api, &block) | ||
@api = api | ||
instance_eval(&block) | ||
end | ||
|
||
def requires(*attrs) | ||
validations = {:presence => true} | ||
if attrs.last.is_a?(Hash) | ||
validations.merge!(attrs.pop) | ||
end | ||
|
||
validates(attrs, validations) | ||
end | ||
|
||
def optional(*attrs) | ||
validations = {} | ||
if attrs.last.is_a?(Hash) | ||
validations.merge!(attrs.pop) | ||
end | ||
|
||
validates(attrs, validations) | ||
end | ||
|
||
private | ||
def validates(attrs, validations) | ||
doc_attrs = { :required => validations.keys.include?(:presence) } | ||
|
||
# special case (type = coerce) | ||
if validations[:type] | ||
validations[:coerce] = validations.delete(:type) | ||
end | ||
|
||
if coerce_type = validations[:coerce] | ||
doc_attrs[:type] = coerce_type.to_s | ||
end | ||
|
||
if desc = validations.delete(:desc) | ||
doc_attrs[:desc] = desc | ||
end | ||
|
||
@api.document_attribute(attrs, doc_attrs) | ||
|
||
validations.each do |type, options| | ||
validator_class = Validations::validators[type.to_s] | ||
if validator_class | ||
@api.settings[:validations] << validator_class.new(attrs, options) | ||
else | ||
raise "unknown validator: #{type}" | ||
end | ||
end | ||
|
||
end | ||
|
||
end | ||
|
||
# This module is mixed into the API Class. | ||
module ClassMethods | ||
def reset_validations! | ||
settings[:validations] = [] | ||
end | ||
|
||
def params(&block) | ||
ParamsScope.new(self, &block) | ||
end | ||
|
||
def document_attribute(names, opts) | ||
if @last_description | ||
@last_description[:params] ||= {} | ||
|
||
Array(names).each do |name| | ||
@last_description[:params][name.to_sym] ||= {} | ||
@last_description[:params][name.to_sym].merge!(opts) | ||
end | ||
end | ||
end | ||
|
||
end | ||
|
||
end | ||
end | ||
|
||
# load all defined validations | ||
Dir[File.expand_path('../validations/*.rb', __FILE__)].each do |path| | ||
require(path) | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
|
||
module Grape | ||
class API | ||
Boolean = Virtus::Attribute::Boolean | ||
end | ||
|
||
module Validations | ||
|
||
class CoerceValidator < SingleOptionValidator | ||
def validate_param!(attr_name, params) | ||
params[attr_name] = coerce_value(@option, params[attr_name]) | ||
end | ||
|
||
private | ||
def coerce_value(type, val) | ||
converter = Virtus::Attribute.build(:a, type) | ||
converter.coerce(val) | ||
end | ||
end | ||
|
||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
module Grape | ||
module Validations | ||
|
||
class PresenceValidator < Validator | ||
def validate_param!(attr_name, params) | ||
unless params.has_key?(attr_name) | ||
throw :error, :status => 400, :message => "missing parameter: #{attr_name}" | ||
end | ||
end | ||
|
||
end | ||
|
||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
module Grape | ||
module Validations | ||
|
||
class RegexpValidator < SingleOptionValidator | ||
def validate_param!(attr_name, params) | ||
if params[attr_name] && !( params[attr_name].to_s =~ @option ) | ||
throw :error, :status => 400, :message => "invalid parameter: #{attr_name}" | ||
end | ||
end | ||
end | ||
|
||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
require 'spec_helper' | ||
|
||
class CoerceAPI < Grape::API | ||
default_format :json | ||
|
||
params do | ||
requires :int, :coerce => Integer | ||
optional :arr, :coerce => Array[Integer] | ||
optional :bool, :coerce => Array[Boolean] | ||
end | ||
get '/coerce' do | ||
{ | ||
:int => params[:int].class, | ||
:arr => params[:arr] ? params[:arr][0].class : nil, | ||
:bool => params[:bool] ? (params[:bool][0] == true) && (params[:bool][1] == false) : nil | ||
} | ||
end | ||
end | ||
|
||
describe Grape::Validations::CoerceValidator do | ||
def app; @app; end | ||
|
||
before do | ||
@app = CoerceAPI | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move the definition of CoerceAPI into here as a before block just like in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will move the class definition in the CoerseTest module I added, I planned to do that but got distracted. |
||
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 coerce inputs' do | ||
get('/coerce', :int => "43") | ||
last_response.status.should == 200 | ||
ret = MultiJson.load(last_response.body) | ||
ret["int"].should == "Fixnum" | ||
|
||
get('/coerce', :int => "40", :arr => ["1","20","3"], :bool => [1, 0]) | ||
last_response.status.should == 200 | ||
ret = MultiJson.load(last_response.body) | ||
ret["int"].should == "Fixnum" | ||
ret["arr"].should == "Fixnum" | ||
ret["bool"].should == true | ||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
require 'spec_helper' | ||
|
||
describe Grape::Validations::PresenceValidator do | ||
def app; @app; end | ||
|
||
before do | ||
@app = Class.new(Grape::API) do | ||
default_format :json | ||
|
||
params do | ||
requires :id, :regexp => /^[0-9]+$/ | ||
end | ||
|
||
post do | ||
{:ret => params[:id]} | ||
end | ||
|
||
params do | ||
requires :name, :company | ||
end | ||
|
||
get do | ||
"Hello" | ||
end | ||
|
||
end | ||
|
||
end | ||
|
||
it 'validates id' do | ||
post('/') | ||
last_response.status.should == 400 | ||
last_response.body.should == "missing parameter: id" | ||
|
||
post('/', {}, 'rack.input' => StringIO.new('{"id" : "a56b"}')) | ||
last_response.body.should == 'invalid parameter: id' | ||
last_response.status.should == 400 | ||
|
||
post('/', {}, 'rack.input' => StringIO.new('{"id" : 56}')) | ||
last_response.body.should == '{"ret":56}' | ||
last_response.status.should == 201 | ||
end | ||
|
||
it 'validates name, company' do | ||
get('/') | ||
last_response.status.should == 400 | ||
last_response.body.should == "missing parameter: name" | ||
|
||
get('/', :name => "Bob") | ||
last_response.status.should == 400 | ||
last_response.body.should == "missing parameter: company" | ||
|
||
get('/', :name => "Bob", :company => "TestCorp") | ||
last_response.status.should == 200 | ||
last_response.body.should == "Hello" | ||
end | ||
|
||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would even drop Virtus, it's like saying that JSON is created with
multi_json
, who cares, really? :) I would write something like this:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love Virtus and I want to share ;)
Seriously that's fine with me, I based my new version on what you propose and drop any mention of Virtus.
I added a note about a way to do the validation if wanted.
I also added another note saying the behavior might change since I hope we can make the validation automatic when declaring a coercion in the future.