Skip to content
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

Fix error message indices #2463

Merged
merged 3 commits into from
Jun 25, 2024
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 @@ -8,6 +8,7 @@

* [#2459](https://github.com/ruby-grape/grape/pull/2459): Autocorrect cops - [@ericproulx](https://github.com/ericproulx).
* [#3458](https://github.com/ruby-grape/grape/pull/2458): Remove unused Grape::Util::Accept::Header - [@ericproulx](https://github.com/ericproulx).
* [#2463](https://github.com/ruby-grape/grape/pull/2463): Fix error message indices - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

### 2.1.1 (2024-06-22)
Expand Down
1 change: 1 addition & 0 deletions lib/grape/validations/attributes_iterator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def each(&block)
private

def do_each(params_to_process, parent_indicies = [], &block)
@scope.reset_index # gets updated depending on the size of params_to_process
params_to_process.each_with_index do |resource_params, index|
# when we get arrays of arrays it means that target element located inside array
# we need this because we want to know parent arrays indicies
Expand Down
14 changes: 5 additions & 9 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,14 @@ def should_validate?(parameters)

def meets_dependency?(params, request_params)
return true unless @dependent_on

return false if @parent.present? && !@parent.meets_dependency?(@parent.params(request_params), request_params)

return params.any? { |param| meets_dependency?(param, request_params) } if params.is_a?(Array)

meets_hash_dependency?(params)
end

def attr_meets_dependency?(params)
return true unless @dependent_on

return false if @parent.present? && !@parent.attr_meets_dependency?(params)

meets_hash_dependency?(params)
Expand Down Expand Up @@ -169,6 +166,10 @@ def required?
!@optional
end

def reset_index
@index = nil
end

protected

# Adds a parameter declaration to our list of validations.
Expand Down Expand Up @@ -297,12 +298,7 @@ def new_lateral_scope(options, &block)
# `optional` invocation that opened this scope.
# @yield parameter scope
def new_group_scope(attrs, &block)
self.class.new(
api: @api,
parent: self,
group: attrs.first,
&block
)
self.class.new(api: @api, parent: self, group: attrs.first, &block)
end

# Pushes declared params to parent or settings
Expand Down
42 changes: 42 additions & 0 deletions spec/grape/validations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1966,6 +1966,48 @@ def validate_param!(attr_name, params)
expect(last_response.status).to eq(400)
end
end

# Ensure there is no leakage of indices between requests
context 'required with a hash inside an array' do
before do
subject.params do
requires :items, type: Array do
requires :item, type: Hash do
requires :name, type: String
end
end
end
subject.post '/required' do
'required works'
end
end

let(:valid_item) { { item: { name: 'foo' } } }

let(:params) do
{
items: [
valid_item,
valid_item,
{}
]
}
end

it 'makes sure the error message is independent of the previous request' do
post_with_json '/required', {}
expect(last_response).to be_bad_request
expect(last_response.body).to eq('items is missing, items[item][name] is missing')

post_with_json '/required', params
expect(last_response).to be_bad_request
expect(last_response.body).to eq('items[2][item] is missing, items[2][item][name] is missing')

post_with_json '/required', {}
expect(last_response).to be_bad_request
expect(last_response.body).to eq('items is missing, items[item][name] is missing')
end
end
end

describe 'require_validator' do
Expand Down