Skip to content

Simplify logic for defining declared params #2077

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

Merged
merged 1 commit into from
Jun 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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