Skip to content

Commit

Permalink
Corrected the endpoint parameter name generation.
Browse files Browse the repository at this point in the history
Signed-off-by: Hermann Mayer <[email protected]>
  • Loading branch information
Jack12816 committed Sep 10, 2021
1 parent 0ff93fb commit 7bf552b
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ Metrics/BlockLength:
# Offense count: 11
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 304
Max: 306

# Offense count: 30
# Configuration parameters: IgnoredMethods.
Expand Down
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -995,8 +995,10 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d
````json
{
"declared_params": {
"first_name": "first name",
"last_name": null
"user": {
"first_name": "first name",
"last_name": null
}
}
}
````
Expand Down
33 changes: 19 additions & 14 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Grape
module Validations
class ParamsScope
attr_accessor :element, :parent, :index
attr_accessor :element, :original_element, :parent, :index
attr_reader :type

include Grape::DSL::Parameters
Expand All @@ -13,6 +13,8 @@ class ParamsScope
# @param opts [Hash] options for this scope
# @option opts :element [Symbol] the element that contains this scope; for
# this to be relevant, @parent must be set
# @option opts :original_element [Symbol, nil] the original element name
# before it was renamed through +:as+
# @option opts :parent [ParamsScope] the scope containing this scope
# @option opts :api [API] the API endpoint to modify
# @option opts :optional [Boolean] whether or not this scope needs to have
Expand All @@ -23,13 +25,14 @@ class ParamsScope
# validate if this param is present in the parent scope
# @yield the instance context, open for parameter definitions
def initialize(opts, &block)
@element = opts[:element]
@parent = opts[:parent]
@api = opts[:api]
@optional = opts[:optional] || false
@type = opts[:type]
@group = opts[:group] || {}
@dependent_on = opts[:dependent_on]
@element = opts[:element]
@original_element = opts[:original_element]
@parent = opts[:parent]
@api = opts[:api]
@optional = opts[:optional] || false
@type = opts[:type]
@group = opts[:group] || {}
@dependent_on = opts[:dependent_on]
@declared_params = []
@index = nil

Expand Down Expand Up @@ -82,7 +85,8 @@ def meets_dependency?(params, request_params)
def full_name(name, index: nil)
if nested?
# Find our containing element's name, and append ours.
"#{@parent.full_name(@element)}#{brackets(@index || index)}#{brackets(name)}"
element_name = @original_element || @element
"#{@parent.full_name(element_name)}#{brackets(@index || index)}#{brackets(name)}"
elsif lateral?
# Find the name of the element as if it was at the same nesting level
# as our parent. We need to forward our index upward to achieve this.
Expand Down Expand Up @@ -191,11 +195,12 @@ def new_scope(attrs, optional = false, &block)
end

self.class.new(
api: @api,
element: attrs[1][:as] || attrs.first,
parent: self,
optional: optional,
type: type || Array,
api: @api,
element: attrs[1][:as] || attrs.first,
original_element: attrs[1][:as] ? attrs.first : nil,
parent: self,
optional: optional,
type: type || Array,
&block
)
end
Expand Down
168 changes: 168 additions & 0 deletions spec/grape/endpoint/declared_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -598,4 +598,172 @@ def app
expect(json.key?(:artist_id)).not_to be_truthy
end
end

describe 'parameter renaming' do
context 'with a renamed hash with nested parameters' do
before do
subject.format :json
subject.params do
optional :email_address, type: String, regexp: /.+@.+/, as: :email
end
subject.post '/test' do
declared(params, include_missing: false)
end
end

it 'generates the correct parameter names for documentation' do
expect(subject.routes.first.params.keys).to match(%w[email_address])
end

it 'maps the renamed parameter correctly (original name)' do
post '/test', email_address: '[email protected]'
expect(JSON.parse(last_response.body)).to \
match('email' => '[email protected]')
end

it 'maps the renamed parameter correctly (as name)' do
post '/test', email: '[email protected]'
expect(JSON.parse(last_response.body)).to \
match('email' => '[email protected]')
end

it 'validates the renamed parameter correctly (original name)' do
post '/test', email_address: 'bad[at]example.com'
expect(JSON.parse(last_response.body)).to \
match('error' => 'email_address is invalid')
end

it 'validates the renamed parameter correctly (as name)' do
# NOTE: This is mostly unexpected behaviour, because when we "know" the
# renamed parameter name we can bypass the parameter validation
# here as client.
post '/test', email: 'bad[at]example.com'
expect(JSON.parse(last_response.body)).to \
match('email' => 'bad[at]example.com')
# Should be: match('error' => 'email_address is invalid')
end
end

context 'with a renamed hash with nested parameters' do
before do
subject.format :json
subject.params do
optional :address, type: Hash, as: :address_attributes do
optional :street, type: String, values: ['Street 1', 'Street 2'],
default: 'Street 1'
optional :city, type: String
end
end
subject.post '/test' do
declared(params, include_missing: false)
end
end

it 'generates the correct parameter names for documentation' do
expect(subject.routes.first.params.keys).to \
match(%w[address address[street] address[city]])
end

it 'maps the renamed parameter correctly (original name)' do
post '/test', address: { city: 'Berlin', street: 'Street 2', t: 't' }
expect(JSON.parse(last_response.body)).to \
match('address_attributes' => { 'city' => 'Berlin',
'street' => 'Street 2' })
end

it 'maps the renamed parameter correctly (as name)' do
post '/test', address_attributes: { city: 'Berlin', unknown: '1' }
expect(JSON.parse(last_response.body)).to \
match('address_attributes' => { 'city' => 'Berlin',
'street' => 'Street 1' })
end

it 'validates the renamed parameter correctly (original name)' do
post '/test', address: { street: 'unknown' }
expect(JSON.parse(last_response.body)).to \
match('error' => 'address[street] does not have a valid value')
end

it 'validates the renamed parameter correctly (as name)' do
post '/test', address_attributes: { street: 'unknown' }
expect(JSON.parse(last_response.body)).to \
match('error' => 'address[street] does not have a valid value')
end
end

context 'with a renamed hash with nested renamed parameter' do
before do
subject.format :json
subject.params do
optional :user, type: Hash, as: :user_attributes do
optional :email_address, type: String, regexp: /.+@.+/, as: :email
end
end
subject.post '/test' do
declared(params, include_missing: false)
end
end

it 'generates the correct parameter names for documentation' do
expect(subject.routes.first.params.keys).to \
match(%w[user user[email_address]])
end

it 'maps the renamed parameter correctly (original name)' do
post '/test', user: { email_address: '[email protected]' }
expect(JSON.parse(last_response.body)).to \
match('user_attributes' => { 'email' => '[email protected]' })
end

it 'maps the renamed parameter correctly (as name, 1)' do
post '/test', user: { email: '[email protected]' }
expect(JSON.parse(last_response.body)).to \
match('user_attributes' => { 'email' => '[email protected]' })
end

it 'maps the renamed parameter correctly (as name, 2)' do
post '/test', user_attributes: { email_address: '[email protected]' }
expect(JSON.parse(last_response.body)).to \
match('user_attributes' => { 'email' => '[email protected]' })
end

it 'maps the renamed parameter correctly (as name, 3)' do
post '/test', user_attributes: { email: '[email protected]' }
expect(JSON.parse(last_response.body)).to \
match('user_attributes' => { 'email' => '[email protected]' })
end

it 'validates the renamed parameter correctly (original name)' do
post '/test', user: { email_address: 'bad[at]example.com' }
expect(JSON.parse(last_response.body)).to \
match('error' => 'user[email_address] is invalid')
end

it 'validates the renamed parameter correctly (as name, 1)' do
# NOTE: This is mostly unexpected behaviour, because when we "know" the
# renamed parameter name we can bypass the parameter validation
# here as client.
post '/test', user: { email: 'bad[at]example.com' }
expect(JSON.parse(last_response.body)).to \
match('user_attributes' => { 'email' => 'bad[at]example.com' })
# Should be: match('error' => 'user[email_address] is invalid')
end

it 'validates the renamed parameter correctly (as name, 2)' do
post '/test', user_attributes: { email_address: 'bad[at]example.com' }
expect(JSON.parse(last_response.body)).to \
match('error' => 'user[email_address] is invalid')
end

it 'validates the renamed parameter correctly (as name, 3)' do
# NOTE: This is mostly unexpected behaviour, because when we "know" the
# renamed parameter name we can bypass the parameter validation
# here as client.
post '/test', user_attributes: { email: 'bad[at]example.com' }
expect(JSON.parse(last_response.body)).to \
match('user_attributes' => { 'email' => 'bad[at]example.com' })
# Should be: match('error' => 'user[email_address] is invalid')
end
end
end
end

0 comments on commit 7bf552b

Please sign in to comment.