Skip to content

Commit 93301c7

Browse files
committed
Operate on a cloned Hash when specifying presence: true for parameters to prevent issues with re-used entities. Includes spec and fix.
1 parent b59999d commit 93301c7

File tree

3 files changed

+41
-2
lines changed

3 files changed

+41
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#### Fixes
55

66
* [#936](https://github.com/intridea/grape/pull/936): Fixed default params processing for optional groups - [@dm1try](https://github.com/dm1try).
7+
* [#](https://github.com/intridea/grape/pull/): Fixed forced presence for optional params when based on a reused entity that was also required in another context - [@croeck](https://github.com/croeck).
78

89
* Your contribution here.
910

lib/grape/dsl/parameters.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def use(*names)
2121
def requires(*attrs, &block)
2222
orig_attrs = attrs.clone
2323

24-
opts = attrs.last.is_a?(Hash) ? attrs.pop : {}
24+
opts = attrs.last.is_a?(Hash) ? attrs.pop.clone : {}
2525
opts.merge!(presence: true)
2626

2727
if opts[:using]
@@ -37,7 +37,7 @@ def requires(*attrs, &block)
3737
def optional(*attrs, &block)
3838
orig_attrs = attrs.clone
3939

40-
opts = attrs.last.is_a?(Hash) ? attrs.pop : {}
40+
opts = attrs.last.is_a?(Hash) ? attrs.pop.clone : {}
4141
type = opts[:type]
4242

4343
# check type for optional parameter group

spec/grape/validations/validators/presence_spec.rb

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,42 @@ def app
176176
expect(last_response.body).to eq('Nested triple'.to_json)
177177
end
178178
end
179+
180+
context 'with reused parameter documentation once required and once optional' do
181+
before do
182+
docs = { name: { type: String, desc: 'some name' } }
183+
184+
subject.params do
185+
requires :all, using: docs
186+
end
187+
subject.get '/required' do
188+
'Hello required'
189+
end
190+
191+
subject.params do
192+
optional :all, using: docs
193+
end
194+
subject.get '/optional' do
195+
'Hello optional'
196+
end
197+
end
198+
it 'works with required' do
199+
get '/required'
200+
expect(last_response.status).to eq(400)
201+
expect(last_response.body).to eq('{"error":"name is missing"}')
202+
203+
get '/required', name: 'Bob'
204+
expect(last_response.status).to eq(200)
205+
expect(last_response.body).to eq('Hello required'.to_json)
206+
end
207+
it 'works with optional' do
208+
get '/optional'
209+
expect(last_response.status).to eq(200)
210+
expect(last_response.body).to eq('Hello optional'.to_json)
211+
212+
get '/optional', name: 'Bob'
213+
expect(last_response.status).to eq(200)
214+
expect(last_response.body).to eq('Hello optional'.to_json)
215+
end
216+
end
179217
end

0 commit comments

Comments
 (0)