Skip to content

Commit

Permalink
Parameter validation: Raises error for all missing (#886)
Browse files Browse the repository at this point in the history
* 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

* Run `rubocop --auto-gen-config --exclude-limit 180`

* Make BlockLength 26 in `.rubocop.yml` as well
  • Loading branch information
davidwessman authored Jun 9, 2023
1 parent 8c20f2b commit bb7bdb4
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Metrics/ClassLength:
- spec/dummy/app/controllers/users_controller.rb

Metrics/BlockLength:
Max: 25 # default
Max: 26 # default
Exclude:
- app/controllers/apipie/apipies_controller.rb
- lib/apipie/generator/swagger/param_description/composite.rb
Expand Down
33 changes: 21 additions & 12 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config --exclude-limit 180`
# on 2023-06-02 23:32:14 UTC using RuboCop version 1.52.0.
# on 2023-06-09 05:29:05 UTC using RuboCop version 1.52.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand Down Expand Up @@ -118,7 +118,7 @@ Layout/ElseAlignment:
- 'lib/apipie/param_description.rb'
- 'lib/apipie/resource_description.rb'

# Offense count: 59
# Offense count: 58
# This cop supports safe autocorrection (--autocorrect).
Layout/EmptyLineAfterGuardClause:
Exclude:
Expand Down Expand Up @@ -783,6 +783,12 @@ Lint/Void:
Metrics/AbcSize:
Max: 96

# Offense count: 1
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns, inherit_mode.
# AllowedMethods: refine
Metrics/BlockLength:
Max: 26

# Offense count: 4
# Configuration parameters: CountBlocks.
Metrics/BlockNesting:
Expand All @@ -793,11 +799,16 @@ Metrics/BlockNesting:
Metrics/CyclomaticComplexity:
Max: 24

# Offense count: 78
# Offense count: 79
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
Metrics/MethodLength:
Max: 58

# Offense count: 1
# Configuration parameters: CountComments, CountAsOne.
Metrics/ModuleLength:
Max: 101

# Offense count: 4
# Configuration parameters: CountKeywordArgs.
Metrics/ParameterLists:
Expand Down Expand Up @@ -1048,7 +1059,7 @@ RSpec/EmptyLineAfterHook:
RSpec/ExampleLength:
Max: 85

# Offense count: 158
# Offense count: 159
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: CustomTransform, IgnoredWords, DisallowedExamples.
# DisallowedExamples: works
Expand Down Expand Up @@ -1137,7 +1148,7 @@ RSpec/MessageSpies:
RSpec/MultipleExpectations:
Max: 19

# Offense count: 154
# Offense count: 156
# Configuration parameters: AllowSubject.
RSpec/MultipleMemoizedHelpers:
Max: 15
Expand All @@ -1162,18 +1173,17 @@ RSpec/NamedSubject:
- 'spec/lib/swagger/rake_swagger_spec.rb'
- 'spec/lib/swagger/swagger_dsl_spec.rb'

# Offense count: 93
# Offense count: 94
# Configuration parameters: AllowedGroups.
RSpec/NestedGroups:
Max: 6

# Offense count: 2
# Offense count: 1
# Configuration parameters: AllowedPatterns.
# AllowedPatterns: ^expect_, ^assert_
RSpec/NoExpectationExample:
Exclude:
- 'spec/controllers/users_controller_spec.rb'
- 'spec/test_engine/memes_controller_spec.rb'

# Offense count: 2
# This cop supports safe autocorrection (--autocorrect).
Expand Down Expand Up @@ -1366,7 +1376,7 @@ Style/AndOr:
Exclude:
- 'lib/apipie/param_description.rb'

# Offense count: 17
# Offense count: 18
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle, ProceduralMethods, FunctionalMethods, AllowedMethods, AllowedPatterns, AllowBracesOnProceduralOneLiners, BracesRequiredMethods.
# SupportedStyles: line_count_based, semantic, braces_for_chaining, always_braces
Expand Down Expand Up @@ -1522,7 +1532,7 @@ Style/EmptyElse:
- 'lib/apipie/extractor/recorder.rb'
- 'lib/apipie/extractor/writer.rb'

# Offense count: 26
# Offense count: 27
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: compact, expanded
Expand Down Expand Up @@ -1765,7 +1775,6 @@ Style/Proc:
Style/RaiseArgs:
Exclude:
- 'lib/apipie/application.rb'
- 'lib/apipie/dsl_definition.rb'
- 'lib/apipie/extractor/writer.rb'
- 'lib/apipie/param_description.rb'
- 'lib/apipie/see_description.rb'
Expand Down Expand Up @@ -1917,7 +1926,7 @@ Style/StringConcatenation:
- 'lib/apipie/application.rb'
- 'lib/apipie/extractor/writer.rb'

# Offense count: 1210
# Offense count: 1212
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline.
# SupportedStyles: single_quotes, double_quotes
Expand Down
9 changes: 5 additions & 4 deletions lib/apipie/dsl_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,11 @@ def _apipie_define_validators(description)
method_params = self.class._apipie_get_method_params(action_name)

if Apipie.configuration.validate_presence?
method_params.each do |_, param|
# check if required parameters are present
raise ParamMissing.new(param) if param.required && !params.key?(param.name)
Validator::BaseValidator.raise_if_missing_params do |missing|
method_params.each do |_, param|
# check if required parameters are present
missing << param if param.required && !params.key?(param.name)
end
end
end

Expand Down Expand Up @@ -285,7 +287,6 @@ def _apipie_define_validators(description)
old_method.bind(self).call(*args)
end
end

end
end

Expand Down
14 changes: 14 additions & 0 deletions lib/apipie/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ def initialize(param)
end
end

class ParamMultipleMissing < ParamError
attr_accessor :params

def initialize(params)
@params = params
end

def to_s
params.map do |param|
ParamMissing.new(param).to_s
end.join("\n")
end
end

class ParamMissing < DefinedParamError
def to_s
unless @param.options[:missing_message].nil?
Expand Down
26 changes: 20 additions & 6 deletions lib/apipie/validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ def self.find(param_description, argument, options, block)
return nil
end

def self.raise_if_missing_params
missing_params = []
yield missing_params
if missing_params.size > 1
raise ParamMultipleMissing.new(missing_params)
elsif missing_params.size == 1
raise ParamMissing.new(missing_params.first)
end
end

# check if value is valid
def valid?(value)
if self.validate(value)
Expand Down Expand Up @@ -345,14 +355,18 @@ def params_ordered

def validate(value)
return false if !value.is_a? Hash
@hash_params&.each do |k, p|
if Apipie.configuration.validate_presence?
raise ParamMissing.new(p) if p.required && !value.key?(k)
end
if Apipie.configuration.validate_value?
p.validate(value[k]) if value.key?(k)

BaseValidator.raise_if_missing_params do |missing|
@hash_params&.each do |k, p|
if Apipie.configuration.validate_presence?
missing << p if p.required && !value.key?(k)
end
if Apipie.configuration.validate_value?
p.validate(value[k]) if value.key?(k)
end
end
end

return true
end

Expand Down
10 changes: 10 additions & 0 deletions spec/controllers/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def compare_hashes(h1, h2)
expect(methods.keys).to include(:update)
expect(methods.keys).to include(:two_urls)
expect(methods.keys).to include(:action_with_headers)
expect(methods.keys).to include(:multiple_required_params)
end

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

it "should fail if multiple required parameters are missing" do
expect { get :multiple_required_params }.to raise_error(Apipie::ParamMultipleMissing, /required_param1.*\n.*required_param2|required_param2.*\n.*required_parameter1/)
end

it "should pass if required parameter has wrong type" do
expect { get :show, :params => { :id => 5 , :session => "secret_hash" }}.not_to raise_error
expect { get :show, :params => { :id => "ten" , :session => "secret_hash" }}.not_to raise_error
Expand Down Expand Up @@ -246,6 +251,11 @@ def reload_controllers
post :create, :params => { :user => { :name => "root", :pass => "12345", :membership => "____" } }
}.to raise_error(Apipie::ParamInvalid, /membership/)

# Should include both pass and name
expect {
post :create, :params => { :user => { :membership => "standard" } }
}.to raise_error(Apipie::ParamMultipleMissing, /pass.*\n.*name|name.*\n.*pass/)

expect {
post :create, :params => { :user => { :name => "root" } }
}.to raise_error(Apipie::ParamMissing, /pass/)
Expand Down
6 changes: 6 additions & 0 deletions spec/dummy/app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -301,4 +301,10 @@ def create_route
header :HeaderNameWithDefaultValue, 'Header with default value', required: true, default: 'default value'
def action_with_headers
end

api :GET, '/users/multiple_required_params'
param :required_param1, String, required: true
param :required_param2, String, required: true
def multiple_required_params
end
end
1 change: 1 addition & 0 deletions spec/dummy/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
resources :users do
collection do
post :create_route
get :multiple_required_params
end
end
resources :concerns, :only => [:index, :show]
Expand Down

0 comments on commit bb7bdb4

Please sign in to comment.