Tightens the error handling on request parsing.

Now fails on the first error encountered.

(cherry picked from commit d177c22)

Author: lgebhardt

lib/jsonapi/exceptions.rb

@@ -364,24 +364,21 @@ def errors
       end
     end
 
-    class ParametersNotAllowed < Error
-      attr_accessor :params
+    class ParameterNotAllowed < Error
+      attr_accessor :param
 
-      def initialize(params, error_object_overrides = {})
-        @params = params
+      def initialize(param, error_object_overrides = {})
+        @param = param
         super(error_object_overrides)
       end
 
       def errors
-        params.collect do |param|
-          create_error_object(code: JSONAPI::PARAM_NOT_ALLOWED,
-                              status: :bad_request,
-                              title: I18n.translate('jsonapi-resources.exceptions.parameters_not_allowed.title',
-                                                    default: 'Param not allowed'),
-                              detail: I18n.translate('jsonapi-resources.exceptions.parameters_not_allowed.detail',
-                                                     default: "#{param} is not allowed.", param: param))
-
-        end
+        [create_error_object(code: JSONAPI::PARAM_NOT_ALLOWED,
+                             status: :bad_request,
+                             title: I18n.translate('jsonapi-resources.exceptions.parameter_not_allowed.title',
+                                                   default: 'Param not allowed'),
+                             detail: I18n.translate('jsonapi-resources.exceptions.parameter_not_allowed.detail',
+                                                    default: "#{param} is not allowed.", param: param))]
       end
     end
 

lib/jsonapi/request_parser.rb

@@ -170,25 +170,23 @@ def parse_fields(fields)
           end
           type_resource = Resource.resource_for(@resource_klass.module_path + underscored_type.to_s)
         rescue NameError
-          @errors.concat(JSONAPI::Exceptions::InvalidResource.new(type).errors)
-        rescue JSONAPI::Exceptions::InvalidResource => e
-          @errors.concat(e.errors)
+          fail JSONAPI::Exceptions::InvalidResource.new(type)
         end
 
         if type_resource.nil?
-          @errors.concat(JSONAPI::Exceptions::InvalidResource.new(type).errors)
+          fail JSONAPI::Exceptions::InvalidResource.new(type)
         else
           unless values.nil?
             valid_fields = type_resource.fields.collect { |key| format_key(key) }
             values.each do |field|
               if valid_fields.include?(field)
                 extracted_fields[type].push unformat_key(field)
               else
-                @errors.concat(JSONAPI::Exceptions::InvalidField.new(type, field).errors)
+                fail JSONAPI::Exceptions::InvalidField.new(type, field)
               end
             end
           else
-            @errors.concat(JSONAPI::Exceptions::InvalidField.new(type, 'nil').errors)
+            fail JSONAPI::Exceptions::InvalidField.new(type, 'nil')
           end
         end
       end
@@ -213,7 +211,7 @@ def parse_include_directives(raw_include)
       return unless raw_include
 
       unless JSONAPI.configuration.allow_include
-        fail JSONAPI::Exceptions::ParametersNotAllowed.new([:include])
+        fail JSONAPI::Exceptions::ParameterNotAllowed.new(:include)
       end
 
       included_resources = []
@@ -242,7 +240,7 @@ def parse_filters(filters)
       return unless filters
 
       unless JSONAPI.configuration.allow_filter
-        fail JSONAPI::Exceptions::ParametersNotAllowed.new([:filter])
+        fail JSONAPI::Exceptions::ParameterNotAllowed.new(:filter)
       end
 
       unless filters.class.method_defined?(:each)
@@ -255,7 +253,7 @@ def parse_filters(filters)
         if @resource_klass._allowed_filter?(filter)
           @filters[filter] = value
         else
-          @errors.concat(JSONAPI::Exceptions::FilterNotAllowed.new(filter).errors)
+          fail JSONAPI::Exceptions::FilterNotAllowed.new(filter)
         end
       end
     end
@@ -271,7 +269,7 @@ def parse_sort_criteria(sort_criteria)
       return unless sort_criteria.present?
 
       unless JSONAPI.configuration.allow_sort
-        fail JSONAPI::Exceptions::ParametersNotAllowed.new([:sort])
+        fail JSONAPI::Exceptions::ParameterNotAllowed.new(:sort)
       end
 
       sorts = []
@@ -300,9 +298,8 @@ def check_sort_criteria(resource_klass, sort_criteria)
       sort_field = sort_criteria[:field]
       sortable_fields = resource_klass.sortable_fields(context)
 
-      unless sortable_fields.include? sort_field.to_sym
-        @errors.concat(JSONAPI::Exceptions::InvalidSortCriteria
-                         .new(format_key(resource_klass._type), sort_field).errors)
+      unless sortable_fields.include?sort_field.to_sym
+        fail JSONAPI::Exceptions::InvalidSortCriteria.new(format_key(resource_klass._type), sort_field)
       end
     end
 
@@ -532,45 +529,52 @@ def verify_permitted_params(params, allowed_fields)
         when 'relationships'
           value.keys.each do |links_key|
             unless formatted_allowed_fields.include?(links_key.to_sym)
-              params_not_allowed.push(links_key)
-              unless JSONAPI.configuration.raise_if_parameters_not_allowed
+              if JSONAPI.configuration.raise_if_parameters_not_allowed
+                fail JSONAPI::Exceptions::ParameterNotAllowed.new(links_key)
+              else
+                params_not_allowed.push(links_key)
                 value.delete links_key
               end
             end
           end
         when 'attributes'
           value.each do |attr_key, attr_value|
             unless formatted_allowed_fields.include?(attr_key.to_sym)
-              params_not_allowed.push(attr_key)
-              unless JSONAPI.configuration.raise_if_parameters_not_allowed
+              if JSONAPI.configuration.raise_if_parameters_not_allowed
+                fail JSONAPI::Exceptions::ParameterNotAllowed.new(attr_key)
+              else
+                params_not_allowed.push(attr_key)
                 value.delete attr_key
               end
             end
           end
         when 'type'
         when 'id'
           unless formatted_allowed_fields.include?(:id)
-            params_not_allowed.push(:id)
-            unless JSONAPI.configuration.raise_if_parameters_not_allowed
+            if JSONAPI.configuration.raise_if_parameters_not_allowed
+              fail JSONAPI::Exceptions::ParameterNotAllowed.new(:id)
+            else
+              params_not_allowed.push(:id)
               params.delete :id
             end
           end
         else
-          params_not_allowed.push(key)
+          if JSONAPI.configuration.raise_if_parameters_not_allowed
+            fail JSONAPI::Exceptions::ParameterNotAllowed.new(key)
+          else
+            params_not_allowed.push(key)
+            params.delete key
+          end
         end
       end
 
       if params_not_allowed.length > 0
-        if JSONAPI.configuration.raise_if_parameters_not_allowed
-          fail JSONAPI::Exceptions::ParametersNotAllowed.new(params_not_allowed)
-        else
-          params_not_allowed_warnings = params_not_allowed.map do |key|
-            JSONAPI::Warning.new(code: JSONAPI::PARAM_NOT_ALLOWED,
-                                 title: 'Param not allowed',
-                                 detail: "#{key} is not allowed.")
-          end
-          self.warnings.concat(params_not_allowed_warnings)
+        params_not_allowed_warnings = params_not_allowed.map do |param|
+          JSONAPI::Warning.new(code: JSONAPI::PARAM_NOT_ALLOWED,
+                               title: 'Param not allowed',
+                               detail: "#{param} is not allowed.")
         end
+        self.warnings.concat(params_not_allowed_warnings)
       end
     end
 

locales/en.yml

@@ -55,7 +55,7 @@ en:
       invalid_sort_criteria:
         title: 'Invalid sort criteria'
         detail: "%{sort_criteria} is not a valid sort criteria for %{resource}"
-      parameters_not_allowed:
+      parameter_not_allowed:
         title: 'Param not allowed'
         detail: "%{param} is not allowed."
       parameter_missing:

test/controllers/controller_test.rb

@@ -1803,7 +1803,6 @@ def test_update_unpermitted_attributes
       }
 
     assert_response :bad_request
-    assert_match /author is not allowed./, response.body
     assert_match /subject is not allowed./, response.body
   end
 

test/unit/jsonapi_request/jsonapi_request_test.rb

@@ -129,16 +129,17 @@ def test_parse_dasherized_with_underscored_fields
       }
     )
 
-    request = JSONAPI::RequestParser.new(
-      params,
-      {
-        context: nil,
-        key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
-      }
-    )
-
-    refute request.errors.empty?
-    assert_equal 'iso_currency is not a valid field for expense-entries.', request.errors[0].detail
+    e = assert_raises JSONAPI::Exceptions::InvalidField do
+      JSONAPI::RequestParser.new(
+        params,
+        {
+          context: nil,
+          key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
+        }
+      )
+    end
+    refute e.errors.empty?
+    assert_equal 'iso_currency is not a valid field for expense-entries.', e.errors[0].detail
   end
 
   def test_parse_dasherized_with_underscored_resource
@@ -152,16 +153,18 @@ def test_parse_dasherized_with_underscored_resource
       }
     )
 
-    request = JSONAPI::RequestParser.new(
-      params,
-      {
-        context: nil,
-        key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
-      }
-    )
-
-    refute request.errors.empty?
-    assert_equal 'expense_entries is not a valid resource.', request.errors[0].detail
+    e = assert_raises JSONAPI::Exceptions::InvalidResource do
+      JSONAPI::RequestParser.new(
+        params,
+        {
+          context: nil,
+          key_formatter: JSONAPI::Formatter.formatter_for(:dasherized_key)
+        }
+      )
+      parse_fields(params[:fields])
+    end
+    refute e.errors.empty?
+    assert_equal 'expense_entries is not a valid resource.', e.errors[0].detail
   end
 
   def test_parse_filters_with_valid_filters
@@ -173,10 +176,10 @@ def test_parse_filters_with_valid_filters
 
   def test_parse_filters_with_non_valid_filter
     setup_request
-    @request.parse_filters({breed: 'Whiskers'}) # breed is not a set filter
-    assert_equal(@request.filters, {})
-    assert_equal(@request.errors.count, 1)
-    assert_equal(@request.errors.first.title, "Filter not allowed")
+    e = assert_raises JSONAPI::Exceptions::FilterNotAllowed do
+        @request.parse_filters({breed: 'Whiskers'}) # breed is not a set filter
+    end
+    assert_equal 'breed is not allowed.', e.errors[0].detail
   end
 
   def test_parse_filters_with_no_filters