Skip to content

Commit

Permalink
Merge pull request #2077 from ruby-grape/crush_desclared_params
Browse files Browse the repository at this point in the history
Simplify logic for defining declared params
  • Loading branch information
dblock committed Jun 28, 2020
2 parents 9198f7e + 12786e6 commit 0d5cf40
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* [#1520](https://github.com/ruby-grape/grape/pull/1520): Un-deprecate stream-like objects - [@urkle](https://github.com/urkle).
* [#2060](https://github.com/ruby-grape/grape/pull/2060): Drop support for Ruby 2.4 - [@dblock](https://github.com/dblock).
* [#2060](https://github.com/ruby-grape/grape/pull/2060): Upgraded Rubocop to 0.84.0 - [@dblock](https://github.com/dblock).
* [#2077](https://github.com/ruby-grape/grape/pull/2077): Simplify logic for defining declared params - [@dnesteryuk](https://github.com/dnesteryuk).
* Your contribution here.

#### Fixes
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ def route_options_params_key(params_nested_path)
def optioned_declared_params(**options)
declared_params = if options[:include_parent_namespaces]
# Declared params including parent namespaces
route_setting(:saved_declared_params).flatten | Array(route_setting(:declared_params))
route_setting(:declared_params)
else
# Declared params at current namespace
route_setting(:saved_declared_params).last & Array(route_setting(:declared_params))
namespace_stackable(:declared_params).last || []
end

raise ArgumentError, 'Tried to filter for declared parameters but none exist.' unless declared_params
Expand Down
19 changes: 18 additions & 1 deletion lib/grape/dsl/validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,24 @@ module Validations
include Grape::DSL::Configuration

module ClassMethods
# Clears all defined parameters and validations.
# Clears all defined parameters and validations. The main purpose of it is to clean up
# settings, so next endpoint won't interfere with previous one.
#
# params do
# # params for the endpoint below this block
# end
# post '/current' do
# # whatever
# end
#
# # somewhere between them the reset_validations! method gets called
#
# params do
# # params for the endpoint below this block
# end
# post '/next' do
# # whatever
# end
def reset_validations!
unset_namespace_stackable :declared_params
unset_namespace_stackable :validations
Expand Down
8 changes: 5 additions & 3 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ def initialize(new_settings, options = {}, &block)

self.inheritable_setting = new_settings.point_in_time_copy

route_setting(:saved_declared_params, namespace_stackable(:declared_params))
# now +namespace_stackable(:declared_params)+ contains all params defined for
# this endpoint and its parents, but later it will be cleaned up,
# see +reset_validations!+ in lib/grape/dsl/validations.rb
route_setting(:declared_params, namespace_stackable(:declared_params).flatten)
route_setting(:saved_validations, namespace_stackable(:validations))

namespace_stackable(:representations, []) unless namespace_stackable(:representations)
Expand Down Expand Up @@ -116,7 +119,6 @@ def inherit_settings(namespace_stackable)
parent_declared_params = namespace_stackable[:declared_params]

if parent_declared_params
inheritable_setting.route[:declared_params] ||= []
inheritable_setting.route[:declared_params].concat(parent_declared_params.flatten)
end

Expand Down Expand Up @@ -190,7 +192,7 @@ def prepare_default_route_attributes
requirements: prepare_routes_requirements,
prefix: namespace_inheritable(:root_prefix),
anchor: options[:route_options].fetch(:anchor, true),
settings: inheritable_setting.route.except(:saved_declared_params, :saved_validations),
settings: inheritable_setting.route.except(:declared_params, :saved_validations),
forward_match: options[:forward_match]
}
end
Expand Down
6 changes: 3 additions & 3 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,10 @@ def configure_declared_params
@parent.push_declared_params [element => @declared_params]
else
@api.namespace_stackable(:declared_params, @declared_params)

@api.route_setting(:declared_params, []) unless @api.route_setting(:declared_params)
@api.route_setting(:declared_params, @api.namespace_stackable(:declared_params).flatten)
end

# params were stored in settings, it can be cleaned from the params scope
@declared_params = nil
end

def validates(attrs, validations)
Expand Down
30 changes: 17 additions & 13 deletions spec/grape/validations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ def app
subject
end

def declared_params
subject.namespace_stackable(:declared_params).flatten
end

describe 'params' do
context 'optional' do
before do
Expand Down Expand Up @@ -41,7 +45,7 @@ def app
subject.params do
optional :some_param
end
expect(subject.route_setting(:declared_params)).to eq([:some_param])
expect(declared_params).to eq([:some_param])
end
end

Expand All @@ -61,7 +65,7 @@ def define_optional_using

it 'adds entity documentation to declared params' do
define_optional_using
expect(subject.route_setting(:declared_params)).to eq(%i[field_a field_b])
expect(declared_params).to eq(%i[field_a field_b])
end

it 'works when field_a and field_b are not present' do
Expand Down Expand Up @@ -108,7 +112,7 @@ def define_optional_using
subject.params do
requires :some_param
end
expect(subject.route_setting(:declared_params)).to eq([:some_param])
expect(declared_params).to eq([:some_param])
end

it 'works when required field is present but nil' do
Expand Down Expand Up @@ -193,7 +197,7 @@ def define_requires_all

it 'adds entity documentation to declared params' do
define_requires_all
expect(subject.route_setting(:declared_params)).to eq(%i[required_field optional_field])
expect(declared_params).to eq(%i[required_field optional_field])
end

it 'errors when required_field is not present' do
Expand Down Expand Up @@ -228,7 +232,7 @@ def define_requires_none

it 'adds entity documentation to declared params' do
define_requires_none
expect(subject.route_setting(:declared_params)).to eq(%i[required_field optional_field])
expect(declared_params).to eq(%i[required_field optional_field])
end

it 'errors when required_field is not present' do
Expand Down Expand Up @@ -258,7 +262,7 @@ def define_requires_all

it 'adds only the entity documentation to declared params, nothing more' do
define_requires_all
expect(subject.route_setting(:declared_params)).to eq(%i[required_field optional_field])
expect(declared_params).to eq(%i[required_field optional_field])
end
end

Expand Down Expand Up @@ -324,7 +328,7 @@ def define_requires_none
requires :key
end
end
expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
expect(declared_params).to eq([items: [:key]])
end
end

Expand Down Expand Up @@ -396,7 +400,7 @@ def define_requires_none
requires :key
end
end
expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
expect(declared_params).to eq([items: [:key]])
end
end

Expand Down Expand Up @@ -459,7 +463,7 @@ def define_requires_none
requires :key
end
end
expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
expect(declared_params).to eq([items: [:key]])
end
end

Expand Down Expand Up @@ -813,7 +817,7 @@ def validate_param!(attr_name, params)
requires :key
end
end
expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
expect(declared_params).to eq([items: [:key]])
end
end

Expand Down Expand Up @@ -877,7 +881,7 @@ def validate_param!(attr_name, params)
requires(:required_subitems, type: Array) { requires :value }
end
end
expect(subject.route_setting(:declared_params)).to eq([items: [:key, { optional_subitems: [:value] }, { required_subitems: [:value] }]])
expect(declared_params).to eq([items: [:key, { optional_subitems: [:value] }, { required_subitems: [:value] }]])
end
end

Expand Down Expand Up @@ -1122,14 +1126,14 @@ def validate_param!(attr_name, params)
subject.params do
use :pagination
end
expect(subject.route_setting(:declared_params)).to eq %i[page per_page]
expect(declared_params).to eq %i[page per_page]
end

it 'by #use with multiple params' do
subject.params do
use :pagination, :period
end
expect(subject.route_setting(:declared_params)).to eq %i[page per_page start_date end_date]
expect(declared_params).to eq %i[page per_page start_date end_date]
end
end

Expand Down

0 comments on commit 0d5cf40

Please sign in to comment.