Skip to content

Commit 758bd7c

Browse files
committed
Parameter validation: Raises error for all missing
- Instead of just raising `ParamMissing` for the first missing, instead raise a compound `ParamMultipleMissing` if there are more than one missing. - Adds specs for both POST and GET requests. - Fixes #802
1 parent 8c20f2b commit 758bd7c

File tree

6 files changed

+48
-2
lines changed

6 files changed

+48
-2
lines changed

lib/apipie/dsl_definition.rb

+7-1
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,15 @@ def _apipie_define_validators(description)
241241
method_params = self.class._apipie_get_method_params(action_name)
242242

243243
if Apipie.configuration.validate_presence?
244+
invalid_params = []
244245
method_params.each do |_, param|
245246
# check if required parameters are present
246-
raise ParamMissing.new(param) if param.required && !params.key?(param.name)
247+
invalid_params << param if param.required && !params.key?(param.name)
248+
end
249+
if invalid_params.size == 1
250+
raise ParamMissing.new(invalid_params.first)
251+
elsif invalid_params.size > 1
252+
raise ParamMultipleMissing.new(invalid_params)
247253
end
248254
end
249255

lib/apipie/errors.rb

+14
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,20 @@ def initialize(param)
2424
end
2525
end
2626

27+
class ParamMultipleMissing < ParamError
28+
attr_accessor :params
29+
30+
def initialize(params)
31+
@params = params
32+
end
33+
34+
def to_s
35+
params.map do |param|
36+
ParamMissing.new(param).to_s
37+
end.join("\n")
38+
end
39+
end
40+
2741
class ParamMissing < DefinedParamError
2842
def to_s
2943
unless @param.options[:missing_message].nil?

lib/apipie/validator.rb

+10-1
Original file line numberDiff line numberDiff line change
@@ -345,14 +345,23 @@ def params_ordered
345345

346346
def validate(value)
347347
return false if !value.is_a? Hash
348+
349+
missing_params = []
348350
@hash_params&.each do |k, p|
349351
if Apipie.configuration.validate_presence?
350-
raise ParamMissing.new(p) if p.required && !value.key?(k)
352+
missing_params << p if p.required && !value.key?(k)
351353
end
352354
if Apipie.configuration.validate_value?
353355
p.validate(value[k]) if value.key?(k)
354356
end
355357
end
358+
359+
if missing_params.size == 1
360+
raise ParamMissing.new(missing_params.first)
361+
elsif missing_params.size > 1
362+
raise ParamMultipleMissing.new(missing_params)
363+
end
364+
356365
return true
357366
end
358367

spec/controllers/users_controller_spec.rb

+10
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ def compare_hashes(h1, h2)
3737
expect(methods.keys).to include(:update)
3838
expect(methods.keys).to include(:two_urls)
3939
expect(methods.keys).to include(:action_with_headers)
40+
expect(methods.keys).to include(:multiple_required_params)
4041
end
4142

4243
it "should contain info about resource" do
@@ -101,6 +102,10 @@ def reload_controllers
101102
expect { get :show, :params => { :id => 5 }}.to raise_error(Apipie::ParamMissing, /session_parameter_is_required/)
102103
end
103104

105+
it "should fail if multiple required parameters are missing" do
106+
expect { get :multiple_required_params }.to raise_error(Apipie::ParamMultipleMissing, /[required_parameter1|required_parameter2]{2}/)
107+
end
108+
104109
it "should pass if required parameter has wrong type" do
105110
expect { get :show, :params => { :id => 5 , :session => "secret_hash" }}.not_to raise_error
106111
expect { get :show, :params => { :id => "ten" , :session => "secret_hash" }}.not_to raise_error
@@ -246,6 +251,11 @@ def reload_controllers
246251
post :create, :params => { :user => { :name => "root", :pass => "12345", :membership => "____" } }
247252
}.to raise_error(Apipie::ParamInvalid, /membership/)
248253

254+
# Should include both pass and name
255+
expect {
256+
post :create, :params => { :user => { :membership => "standard" } }
257+
}.to raise_error(Apipie::ParamMultipleMissing, /pass.*\n.*name|name.*\n.*pass/)
258+
249259
expect {
250260
post :create, :params => { :user => { :name => "root" } }
251261
}.to raise_error(Apipie::ParamMissing, /pass/)

spec/dummy/app/controllers/users_controller.rb

+6
Original file line numberDiff line numberDiff line change
@@ -301,4 +301,10 @@ def create_route
301301
header :HeaderNameWithDefaultValue, 'Header with default value', required: true, default: 'default value'
302302
def action_with_headers
303303
end
304+
305+
api :GET, '/users/multiple_required_params'
306+
param :required_param1, String, required: true
307+
param :required_param2, String, required: true
308+
def multiple_required_params
309+
end
304310
end

spec/dummy/config/routes.rb

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
resources :users do
99
collection do
1010
post :create_route
11+
get :multiple_required_params
1112
end
1213
end
1314
resources :concerns, :only => [:index, :show]

0 commit comments

Comments
 (0)