Skip to content

Commit 970bc79

Browse files
lgebhardtlgebhardt
committed
Tightens the error handling on request parsing.
Now fails on the first error encountered. (cherry picked from commit d177c22)
1 parent 4499d16 commit 970bc79

5 files changed

Lines changed: 71 additions & 68 deletions

File tree

‎lib/jsonapi/exceptions.rb‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -364,24 +364,21 @@ def errors
364364
end
365365
end
366366

367-
class ParametersNotAllowed < Error
368-
attr_accessor :params
367+
class ParameterNotAllowed < Error
368+
attr_accessor :param
369369

370-
def initialize(params, error_object_overrides = {})
371-
@params = params
370+
def initialize(param, error_object_overrides = {})
371+
@param = param
372372
super(error_object_overrides)
373373
end
374374

375375
def errors
376-
params.collect do |param|
377-
create_error_object(code: JSONAPI::PARAM_NOT_ALLOWED,
378-
status: :bad_request,
379-
title: I18n.translate('jsonapi-resources.exceptions.parameters_not_allowed.title',
380-
default: 'Param not allowed'),
381-
detail: I18n.translate('jsonapi-resources.exceptions.parameters_not_allowed.detail',
382-
default: "#{param} is not allowed.", param: param))
383-
384-
end
376+
[create_error_object(code: JSONAPI::PARAM_NOT_ALLOWED,
377+
status: :bad_request,
378+
title: I18n.translate('jsonapi-resources.exceptions.parameter_not_allowed.title',
379+
default: 'Param not allowed'),
380+
detail: I18n.translate('jsonapi-resources.exceptions.parameter_not_allowed.detail',
381+
default: "#{param} is not allowed.", param: param))]
385382
end
386383
end
387384

‎lib/jsonapi/request_parser.rb‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -170,25 +170,23 @@ def parse_fields(fields)
170170
end
171171
type_resource = Resource.resource_for(@resource_klass.module_path + underscored_type.to_s)
172172
rescue NameError
173-
@errors.concat(JSONAPI::Exceptions::InvalidResource.new(type).errors)
174-
rescue JSONAPI::Exceptions::InvalidResource => e
175-
@errors.concat(e.errors)
173+
fail JSONAPI::Exceptions::InvalidResource.new(type)
176174
end
177175

178176
if type_resource.nil?
179-
@errors.concat(JSONAPI::Exceptions::InvalidResource.new(type).errors)
177+
fail JSONAPI::Exceptions::InvalidResource.new(type)
180178
else
181179
unless values.nil?
182180
valid_fields = type_resource.fields.collect { |key| format_key(key) }
183181
values.each do |field|
184182
if valid_fields.include?(field)
185183
extracted_fields[type].push unformat_key(field)
186184
else
187-
@errors.concat(JSONAPI::Exceptions::InvalidField.new(type, field).errors)
185+
fail JSONAPI::Exceptions::InvalidField.new(type, field)
188186
end
189187
end
190188
else
191-
@errors.concat(JSONAPI::Exceptions::InvalidField.new(type, 'nil').errors)
189+
fail JSONAPI::Exceptions::InvalidField.new(type, 'nil')
192190
end
193191
end
194192
end
@@ -213,7 +211,7 @@ def parse_include_directives(raw_include)
213211
return unless raw_include
214212

215213
unless JSONAPI.configuration.allow_include
216-
fail JSONAPI::Exceptions::ParametersNotAllowed.new([:include])
214+
fail JSONAPI::Exceptions::ParameterNotAllowed.new(:include)
217215
end
218216

219217
included_resources = []
@@ -242,7 +240,7 @@ def parse_filters(filters)
242240
return unless filters
243241

244242
unless JSONAPI.configuration.allow_filter
245-
fail JSONAPI::Exceptions::ParametersNotAllowed.new([:filter])
243+
fail JSONAPI::Exceptions::ParameterNotAllowed.new(:filter)
246244
end
247245

248246
unless filters.class.method_defined?(:each)
@@ -255,7 +253,7 @@ def parse_filters(filters)
255253
if @resource_klass._allowed_filter?(filter)
256254
@filters[filter] = value
257255
else
258-
@errors.concat(JSONAPI::Exceptions::FilterNotAllowed.new(filter).errors)
256+
fail JSONAPI::Exceptions::FilterNotAllowed.new(filter)
259257
end
260258
end
261259
end
@@ -271,7 +269,7 @@ def parse_sort_criteria(sort_criteria)
271269
return unless sort_criteria.present?
272270

273271
unless JSONAPI.configuration.allow_sort
274-
fail JSONAPI::Exceptions::ParametersNotAllowed.new([:sort])
272+
fail JSONAPI::Exceptions::ParameterNotAllowed.new(:sort)
275273
end
276274

277275
sorts = []
@@ -300,9 +298,8 @@ def check_sort_criteria(resource_klass, sort_criteria)
300298
sort_field = sort_criteria[:field]
301299
sortable_fields = resource_klass.sortable_fields(context)
302300

303-
unless sortable_fields.include? sort_field.to_sym
304-
@errors.concat(JSONAPI::Exceptions::InvalidSortCriteria
305-
.new(format_key(resource_klass._type), sort_field).errors)
301+
unless sortable_fields.include?sort_field.to_sym
302+
fail JSONAPI::Exceptions::InvalidSortCriteria.new(format_key(resource_klass._type), sort_field)
306303
end
307304
end
308305

@@ -532,45 +529,52 @@ def verify_permitted_params(params, allowed_fields)
532529
when 'relationships'
533530
value.keys.each do |links_key|
534531
unless formatted_allowed_fields.include?(links_key.to_sym)
535-
params_not_allowed.push(links_key)
536-
unless JSONAPI.configuration.raise_if_parameters_not_allowed
532+
if JSONAPI.configuration.raise_if_parameters_not_allowed
533+
fail JSONAPI::Exceptions::ParameterNotAllowed.new(links_key)
534+
else
535+
params_not_allowed.push(links_key)
537536
value.delete links_key
538537
end
539538
end
540539
end
541540
when 'attributes'
542541
value.each do |attr_key, attr_value|
543542
unless formatted_allowed_fields.include?(attr_key.to_sym)
544-
params_not_allowed.push(attr_key)
545-
unless JSONAPI.configuration.raise_if_parameters_not_allowed
543+
if JSONAPI.configuration.raise_if_parameters_not_allowed
544+
fail JSONAPI::Exceptions::ParameterNotAllowed.new(attr_key)
545+
else
546+
params_not_allowed.push(attr_key)
546547
value.delete attr_key
547548
end
548549
end
549550
end
550551
when 'type'
551552
when 'id'
552553
unless formatted_allowed_fields.include?(:id)
553-
params_not_allowed.push(:id)
554-
unless JSONAPI.configuration.raise_if_parameters_not_allowed
554+
if JSONAPI.configuration.raise_if_parameters_not_allowed
555+
fail JSONAPI::Exceptions::ParameterNotAllowed.new(:id)
556+
else
557+
params_not_allowed.push(:id)
555558
params.delete :id
556559
end
557560
end
558561
else
559-
params_not_allowed.push(key)
562+
if JSONAPI.configuration.raise_if_parameters_not_allowed
563+
fail JSONAPI::Exceptions::ParameterNotAllowed.new(key)
564+
else
565+
params_not_allowed.push(key)
566+
params.delete key
567+
end
560568
end
561569
end
562570

563571
if params_not_allowed.length > 0
564-
if JSONAPI.configuration.raise_if_parameters_not_allowed
565-
fail JSONAPI::Exceptions::ParametersNotAllowed.new(params_not_allowed)
566-
else
567-
params_not_allowed_warnings = params_not_allowed.map do |key|
568-
JSONAPI::Warning.new(code: JSONAPI::PARAM_NOT_ALLOWED,
569-
title: 'Param not allowed',
570-
detail: "#{key} is not allowed.")
571-
end
572-
self.warnings.concat(params_not_allowed_warnings)
572+
params_not_allowed_warnings = params_not_allowed.map do |param|
573+
JSONAPI::Warning.new(code: JSONAPI::PARAM_NOT_ALLOWED,
574+
title: 'Param not allowed',
575+
detail: "#{param} is not allowed.")
573576
end
577+
self.warnings.concat(params_not_allowed_warnings)
574578
end
575579
end
576580

‎locales/en.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ en:
5555
invalid_sort_criteria:
5656
title: 'Invalid sort criteria'
5757
detail: "%{sort_criteria} is not a valid sort criteria for %{resource}"
58-
parameters_not_allowed:
58+
parameter_not_allowed:
5959
title: 'Param not allowed'
6060
detail: "%{param} is not allowed."
6161
parameter_missing:

‎test/controllers/controller_test.rb‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1803,7 +1803,6 @@ def test_update_unpermitted_attributes
18031803
}
18041804

18051805
assert_response :bad_request
1806-
assert_match /author is not allowed./, response.body
18071806
assert_match /subject is not allowed./, response.body
18081807
end
18091808

‎test/unit/jsonapi_request/jsonapi_request_test.rb‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,17 @@ def test_parse_dasherized_with_underscored_fields
129129
}
130130
)
131131

132-
request = JSONAPI::RequestParser.new(
133-
params,
134-
{
135-
context: nil,
136-
key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
137-
}
138-
)
139-
140-
refute request.errors.empty?
141-
assert_equal 'iso_currency is not a valid field for expense-entries.', request.errors[0].detail
132+
e = assert_raises JSONAPI::Exceptions::InvalidField do
133+
JSONAPI::RequestParser.new(
134+
params,
135+
{
136+
context: nil,
137+
key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
138+
}
139+
)
140+
end
141+
refute e.errors.empty?
142+
assert_equal 'iso_currency is not a valid field for expense-entries.', e.errors[0].detail
142143
end
143144

144145
def test_parse_dasherized_with_underscored_resource
@@ -152,16 +153,18 @@ def test_parse_dasherized_with_underscored_resource
152153
}
153154
)
154155

155-
request = JSONAPI::RequestParser.new(
156-
params,
157-
{
158-
context: nil,
159-
key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
160-
}
161-
)
162-
163-
refute request.errors.empty?
164-
assert_equal 'expense_entries is not a valid resource.', request.errors[0].detail
156+
e = assert_raises JSONAPI::Exceptions::InvalidResource do
157+
JSONAPI::RequestParser.new(
158+
params,
159+
{
160+
context: nil,
161+
key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
162+
}
163+
)
164+
parse_fields(params[:fields])
165+
end
166+
refute e.errors.empty?
167+
assert_equal 'expense_entries is not a valid resource.', e.errors[0].detail
165168
end
166169

167170
def test_parse_filters_with_valid_filters
@@ -173,10 +176,10 @@ def test_parse_filters_with_valid_filters
173176

174177
def test_parse_filters_with_non_valid_filter
175178
setup_request
176-
@request.parse_filters({breed: 'Whiskers'}) # breed is not a set filter
177-
assert_equal(@request.filters, {})
178-
assert_equal(@request.errors.count, 1)
179-
assert_equal(@request.errors.first.title, "Filter not allowed")
179+
e = assert_raises JSONAPI::Exceptions::FilterNotAllowed do
180+
@request.parse_filters({breed: 'Whiskers'}) # breed is not a set filter
181+
end
182+
assert_equal 'breed is not allowed.', e.errors[0].detail
180183
end
181184

182185
def test_parse_filters_with_no_filters

0 commit comments

Comments
 (0)