Skip to content

Commit 7bf552b

Browse files
Jack12816Jack12816
committed
Corrected the endpoint parameter name generation.
Signed-off-by: Hermann Mayer <[email protected]>
1 parent 0ff93fb commit 7bf552b

File tree

4 files changed

+192
-17
lines changed

4 files changed

+192
-17
lines changed

‎.rubocop_todo.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ Metrics/BlockLength:
167167
# Offense count: 11
168168
# Configuration parameters: CountComments, CountAsOne.
169169
Metrics/ClassLength:
170-
Max: 304
170+
Max: 306
171171

172172
# Offense count: 30
173173
# Configuration parameters: IgnoredMethods.

‎README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -995,8 +995,10 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d
995995
````json
996996
{
997997
"declared_params": {
998-
"first_name": "first name",
999-
"last_name": null
998+
"user": {
999+
"first_name": "first name",
1000+
"last_name": null
1001+
}
10001002
}
10011003
}
10021004
````

‎lib/grape/validations/params_scope.rb

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
module Grape
44
module Validations
55
class ParamsScope
6-
attr_accessor :element, :parent, :index
6+
attr_accessor :element, :original_element, :parent, :index
77
attr_reader :type
88

99
include Grape::DSL::Parameters
@@ -13,6 +13,8 @@ class ParamsScope
1313
# @param opts [Hash] options for this scope
1414
# @option opts :element [Symbol] the element that contains this scope; for
1515
# this to be relevant, @parent must be set
16+
# @option opts :original_element [Symbol, nil] the original element name
17+
# before it was renamed through +:as+
1618
# @option opts :parent [ParamsScope] the scope containing this scope
1719
# @option opts :api [API] the API endpoint to modify
1820
# @option opts :optional [Boolean] whether or not this scope needs to have
@@ -23,13 +25,14 @@ class ParamsScope
2325
# validate if this param is present in the parent scope
2426
# @yield the instance context, open for parameter definitions
2527
def initialize(opts, &block)
26-
@element = opts[:element]
27-
@parent = opts[:parent]
28-
@api = opts[:api]
29-
@optional = opts[:optional] || false
30-
@type = opts[:type]
31-
@group = opts[:group] || {}
32-
@dependent_on = opts[:dependent_on]
28+
@element = opts[:element]
29+
@original_element = opts[:original_element]
30+
@parent = opts[:parent]
31+
@api = opts[:api]
32+
@optional = opts[:optional] || false
33+
@type = opts[:type]
34+
@group = opts[:group] || {}
35+
@dependent_on = opts[:dependent_on]
3336
@declared_params = []
3437
@index = nil
3538

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

193197
self.class.new(
194-
api: @api,
195-
element: attrs[1][:as] || attrs.first,
196-
parent: self,
197-
optional: optional,
198-
type: type || Array,
198+
api: @api,
199+
element: attrs[1][:as] || attrs.first,
200+
original_element: attrs[1][:as] ? attrs.first : nil,
201+
parent: self,
202+
optional: optional,
203+
type: type || Array,
199204
&block
200205
)
201206
end

‎spec/grape/endpoint/declared_spec.rb

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,4 +598,172 @@ def app
598598
expect(json.key?(:artist_id)).not_to be_truthy
599599
end
600600
end
601+
602+
describe 'parameter renaming' do
603+
context 'with a renamed hash with nested parameters' do
604+
before do
605+
subject.format :json
606+
subject.params do
607+
optional :email_address, type: String, regexp: /.+@.+/, as: :email
608+
end
609+
subject.post '/test' do
610+
declared(params, include_missing: false)
611+
end
612+
end
613+
614+
it 'generates the correct parameter names for documentation' do
615+
expect(subject.routes.first.params.keys).to match(%w[email_address])
616+
end
617+
618+
it 'maps the renamed parameter correctly (original name)' do
619+
post '/test', email_address: '[email protected]'
620+
expect(JSON.parse(last_response.body)).to \
621+
match('email' => '[email protected]')
622+
end
623+
624+
it 'maps the renamed parameter correctly (as name)' do
625+
post '/test', email: '[email protected]'
626+
expect(JSON.parse(last_response.body)).to \
627+
match('email' => '[email protected]')
628+
end
629+
630+
it 'validates the renamed parameter correctly (original name)' do
631+
post '/test', email_address: 'bad[at]example.com'
632+
expect(JSON.parse(last_response.body)).to \
633+
match('error' => 'email_address is invalid')
634+
end
635+
636+
it 'validates the renamed parameter correctly (as name)' do
637+
# NOTE: This is mostly unexpected behaviour, because when we "know" the
638+
# renamed parameter name we can bypass the parameter validation
639+
# here as client.
640+
post '/test', email: 'bad[at]example.com'
641+
expect(JSON.parse(last_response.body)).to \
642+
match('email' => 'bad[at]example.com')
643+
# Should be: match('error' => 'email_address is invalid')
644+
end
645+
end
646+
647+
context 'with a renamed hash with nested parameters' do
648+
before do
649+
subject.format :json
650+
subject.params do
651+
optional :address, type: Hash, as: :address_attributes do
652+
optional :street, type: String, values: ['Street 1', 'Street 2'],
653+
default: 'Street 1'
654+
optional :city, type: String
655+
end
656+
end
657+
subject.post '/test' do
658+
declared(params, include_missing: false)
659+
end
660+
end
661+
662+
it 'generates the correct parameter names for documentation' do
663+
expect(subject.routes.first.params.keys).to \
664+
match(%w[address address[street] address[city]])
665+
end
666+
667+
it 'maps the renamed parameter correctly (original name)' do
668+
post '/test', address: { city: 'Berlin', street: 'Street 2', t: 't' }
669+
expect(JSON.parse(last_response.body)).to \
670+
match('address_attributes' => { 'city' => 'Berlin',
671+
'street' => 'Street 2' })
672+
end
673+
674+
it 'maps the renamed parameter correctly (as name)' do
675+
post '/test', address_attributes: { city: 'Berlin', unknown: '1' }
676+
expect(JSON.parse(last_response.body)).to \
677+
match('address_attributes' => { 'city' => 'Berlin',
678+
'street' => 'Street 1' })
679+
end
680+
681+
it 'validates the renamed parameter correctly (original name)' do
682+
post '/test', address: { street: 'unknown' }
683+
expect(JSON.parse(last_response.body)).to \
684+
match('error' => 'address[street] does not have a valid value')
685+
end
686+
687+
it 'validates the renamed parameter correctly (as name)' do
688+
post '/test', address_attributes: { street: 'unknown' }
689+
expect(JSON.parse(last_response.body)).to \
690+
match('error' => 'address[street] does not have a valid value')
691+
end
692+
end
693+
694+
context 'with a renamed hash with nested renamed parameter' do
695+
before do
696+
subject.format :json
697+
subject.params do
698+
optional :user, type: Hash, as: :user_attributes do
699+
optional :email_address, type: String, regexp: /.+@.+/, as: :email
700+
end
701+
end
702+
subject.post '/test' do
703+
declared(params, include_missing: false)
704+
end
705+
end
706+
707+
it 'generates the correct parameter names for documentation' do
708+
expect(subject.routes.first.params.keys).to \
709+
match(%w[user user[email_address]])
710+
end
711+
712+
it 'maps the renamed parameter correctly (original name)' do
713+
post '/test', user: { email_address: '[email protected]' }
714+
expect(JSON.parse(last_response.body)).to \
715+
match('user_attributes' => { 'email' => '[email protected]' })
716+
end
717+
718+
it 'maps the renamed parameter correctly (as name, 1)' do
719+
post '/test', user: { email: '[email protected]' }
720+
expect(JSON.parse(last_response.body)).to \
721+
match('user_attributes' => { 'email' => '[email protected]' })
722+
end
723+
724+
it 'maps the renamed parameter correctly (as name, 2)' do
725+
post '/test', user_attributes: { email_address: '[email protected]' }
726+
expect(JSON.parse(last_response.body)).to \
727+
match('user_attributes' => { 'email' => '[email protected]' })
728+
end
729+
730+
it 'maps the renamed parameter correctly (as name, 3)' do
731+
post '/test', user_attributes: { email: '[email protected]' }
732+
expect(JSON.parse(last_response.body)).to \
733+
match('user_attributes' => { 'email' => '[email protected]' })
734+
end
735+
736+
it 'validates the renamed parameter correctly (original name)' do
737+
post '/test', user: { email_address: 'bad[at]example.com' }
738+
expect(JSON.parse(last_response.body)).to \
739+
match('error' => 'user[email_address] is invalid')
740+
end
741+
742+
it 'validates the renamed parameter correctly (as name, 1)' do
743+
# NOTE: This is mostly unexpected behaviour, because when we "know" the
744+
# renamed parameter name we can bypass the parameter validation
745+
# here as client.
746+
post '/test', user: { email: 'bad[at]example.com' }
747+
expect(JSON.parse(last_response.body)).to \
748+
match('user_attributes' => { 'email' => 'bad[at]example.com' })
749+
# Should be: match('error' => 'user[email_address] is invalid')
750+
end
751+
752+
it 'validates the renamed parameter correctly (as name, 2)' do
753+
post '/test', user_attributes: { email_address: 'bad[at]example.com' }
754+
expect(JSON.parse(last_response.body)).to \
755+
match('error' => 'user[email_address] is invalid')
756+
end
757+
758+
it 'validates the renamed parameter correctly (as name, 3)' do
759+
# NOTE: This is mostly unexpected behaviour, because when we "know" the
760+
# renamed parameter name we can bypass the parameter validation
761+
# here as client.
762+
post '/test', user_attributes: { email: 'bad[at]example.com' }
763+
expect(JSON.parse(last_response.body)).to \
764+
match('user_attributes' => { 'email' => 'bad[at]example.com' })
765+
# Should be: match('error' => 'user[email_address] is invalid')
766+
end
767+
end
768+
end
601769
end

0 commit comments

Comments
 (0)