Merge pull request #370 from gniquil/remove-extra-params

added option to ignore params that are not allowed in create/update

Author: barelyknown

lib/jsonapi/acts_as_resource_controller.rb

@@ -87,6 +87,7 @@ def ensure_correct_media_type
 
     def setup_request
       @request = JSONAPI::Request.new(params, context: context, key_formatter: key_formatter)
+
       render_errors(@request.errors) unless @request.errors.empty?
     rescue => e
       handle_exceptions(e)
@@ -121,6 +122,14 @@ def base_response_meta
       {}
     end
 
+    def base_meta
+      if @request.nil? || @request.warnings.empty?
+        base_response_meta
+      else
+        base_response_meta.merge(warnings: @request.warnings)
+      end
+    end
+
     def base_response_links
       {}
     end
@@ -147,7 +156,7 @@ def create_response_document(operation_results)
         base_url: base_url,
         key_formatter: key_formatter,
         route_formatter: route_formatter,
-        base_meta: base_response_meta,
+        base_meta: base_meta,
         base_links: base_response_links,
         resource_serializer_klass: resource_serializer_klass,
         request: @request

lib/jsonapi/configuration.rb

@@ -8,6 +8,7 @@ class Configuration
                 :key_formatter,
                 :route_format,
                 :route_formatter,
+                :raise_if_parameters_not_allowed,
                 :operations_processor,
                 :allow_include,
                 :allow_sort,
@@ -38,6 +39,8 @@ def initialize
       self.allow_sort = true
       self.allow_filter = true
 
+      self.raise_if_parameters_not_allowed = true
+
       # :none, :offset, :paged, or a custom paginator name
       self.default_paginator = :none
 
@@ -105,6 +108,8 @@ def operations_processor=(operations_processor)
     attr_writer :always_include_to_one_linkage_data
 
     attr_writer :always_include_to_many_linkage_data
+
+    attr_writer :raise_if_parameters_not_allowed
   end
 
   class << self

lib/jsonapi/error.rb

@@ -17,4 +17,17 @@ def initialize(options = {})
       @status         = options[:status]
     end
   end
+
+  class Warning
+    attr_accessor :title, :detail, :code
+    def initialize(options = {})
+      @title          = options[:title]
+      @detail         = options[:detail]
+      @code           = if JSONAPI.configuration.use_text_errors
+                          TEXT_ERRORS[options[:code]]
+                        else
+                          options[:code]
+                        end
+    end
+  end
 end

lib/jsonapi/request.rb

@@ -5,13 +5,14 @@ module JSONAPI
   class Request
     attr_accessor :fields, :include, :filters, :sort_criteria, :errors, :operations,
                   :resource_klass, :context, :paginator, :source_klass, :source_id,
-                  :include_directives, :params
+                  :include_directives, :params, :warnings
 
     def initialize(params = nil, options = {})
       @params = params
       @context = options[:context]
       @key_formatter = options.fetch(:key_formatter, JSONAPI.configuration.key_formatter)
       @errors = []
+      @warnings = []
       @operations = []
       @fields = {}
       @filters = {}
@@ -487,19 +488,40 @@ def verify_permitted_params(params, allowed_fields)
         case key.to_s
         when 'relationships'
           value.each_key do |links_key|
-            params_not_allowed.push(links_key) unless formatted_allowed_fields.include?(links_key.to_sym)
+            unless formatted_allowed_fields.include?(links_key.to_sym)
+              params_not_allowed.push(links_key)
+              unless JSONAPI.configuration.raise_if_parameters_not_allowed
+                value.delete links_key
+              end
+            end
           end
         when 'attributes'
-          value.each do |attr_key, _attr_value|
-            params_not_allowed.push(attr_key) unless formatted_allowed_fields.include?(attr_key.to_sym)
+          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
+                value.delete attr_key
+              end
+            end
           end
         when 'type', 'id'
         else
           params_not_allowed.push(key)
         end
       end
 
-      fail JSONAPI::Exceptions::ParametersNotAllowed.new(params_not_allowed) if params_not_allowed.length > 0
+      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)
+        end
+      end
     end
 
     # TODO: Please remove after `updateable_fields` is removed

test/controllers/controller_test.rb

@@ -5,6 +5,10 @@ def set_content_type_header!
 end
 
 class PostsControllerTest < ActionController::TestCase
+  def setup
+    JSONAPI.configuration.raise_if_parameters_not_allowed = true
+  end
+
   def test_index
     get :index
     assert_response :success
@@ -401,6 +405,38 @@ def test_create_extra_param
     assert_match /asdfg is not allowed/, response.body
   end
 
+  def test_create_extra_param_allow_extra_params
+    JSONAPI.configuration.raise_if_parameters_not_allowed = false
+
+    set_content_type_header!
+    post :create,
+         {
+           data: {
+             type: 'posts',
+             attributes: {
+               asdfg: 'aaaa',
+               title: 'JR is Great',
+               body: 'JSONAPIResources is the greatest thing since unsliced bread.'
+             },
+             relationships: {
+               author: {data: {type: 'people', id: '3'}}
+             }
+           },
+           include: 'author'
+         }
+
+    assert_response :created
+    assert json_response['data'].is_a?(Hash)
+    assert_equal '3', json_response['data']['relationships']['author']['data']['id']
+    assert_equal 'JR is Great', json_response['data']['attributes']['title']
+    assert_equal 'JSONAPIResources is the greatest thing since unsliced bread.', json_response['data']['attributes']['body']
+
+    assert_equal 1, json_response['meta']["warnings"].count
+    assert_equal "Param not allowed", json_response['meta']["warnings"][0]["title"]
+    assert_equal "asdfg is not allowed.", json_response['meta']["warnings"][0]["detail"]
+    assert_equal 105, json_response['meta']["warnings"][0]["code"]
+  end
+
   def test_create_with_invalid_data
     set_content_type_header!
     post :create,
@@ -575,6 +611,40 @@ def test_create_simple_unpermitted_attributes
     assert_match /subject/, json_response['errors'][0]['detail']
   end
 
+  def test_create_simple_unpermitted_attributes_allow_extra_params
+    JSONAPI.configuration.raise_if_parameters_not_allowed = false
+
+    set_content_type_header!
+    post :create,
+         {
+           data: {
+             type: 'posts',
+             attributes: {
+               title: 'JR is Great',
+               subject: 'JR is SUPER Great',
+               body: 'JSONAPIResources is the greatest thing since unsliced bread.'
+             },
+             relationships: {
+               author: {data: {type: 'people', id: '3'}}
+             }
+           },
+           include: 'author'
+         }
+
+    assert_response :created
+    assert json_response['data'].is_a?(Hash)
+    assert_equal '3', json_response['data']['relationships']['author']['data']['id']
+    assert_equal 'JR is Great', json_response['data']['attributes']['title']
+    assert_equal 'JR is Great', json_response['data']['attributes']['subject']
+    assert_equal 'JSONAPIResources is the greatest thing since unsliced bread.', json_response['data']['attributes']['body']
+
+
+    assert_equal 1, json_response['meta']["warnings"].count
+    assert_equal "Param not allowed", json_response['meta']["warnings"][0]["title"]
+    assert_equal "subject is not allowed.", json_response['meta']["warnings"][0]["detail"]
+    assert_equal 105, json_response['meta']["warnings"][0]["code"]
+  end
+
   def test_create_with_links_to_many_type_ids
     set_content_type_header!
     post :create,
@@ -704,6 +774,46 @@ def test_update_with_internal_server_error
     assert_equal title, post_object.title
   end
 
+  def test_update_with_links_allow_extra_params
+    JSONAPI.configuration.raise_if_parameters_not_allowed = false
+
+    set_content_type_header!
+    javascript = Section.find_by(name: 'javascript')
+
+    put :update,
+        {
+          id: 3,
+          data: {
+            id: '3',
+            type: 'posts',
+            attributes: {
+              title: 'A great new Post',
+              subject: 'A great new Post',
+            },
+            relationships: {
+              section: {data: {type: 'sections', id: "#{javascript.id}"}},
+              tags: {data: [{type: 'tags', id: 3}, {type: 'tags', id: 4}]}
+            }
+          },
+          include: 'tags,author,section'
+        }
+
+    assert_response :success
+    assert json_response['data'].is_a?(Hash)
+    assert_equal '3', json_response['data']['relationships']['author']['data']['id']
+    assert_equal javascript.id.to_s, json_response['data']['relationships']['section']['data']['id']
+    assert_equal 'A great new Post', json_response['data']['attributes']['title']
+    assert_equal 'AAAA', json_response['data']['attributes']['body']
+    assert matches_array?([{'type' => 'tags', 'id' => '3'}, {'type' => 'tags', 'id' => '4'}],
+                          json_response['data']['relationships']['tags']['data'])
+
+
+    assert_equal 1, json_response['meta']["warnings"].count
+    assert_equal "Param not allowed", json_response['meta']["warnings"][0]["title"]
+    assert_equal "subject is not allowed.", json_response['meta']["warnings"][0]["detail"]
+    assert_equal 105, json_response['meta']["warnings"][0]["code"]
+  end
+
   def test_update_remove_links
     set_content_type_header!
     put :update,
@@ -1150,6 +1260,33 @@ def test_update_extra_param_in_links
     assert_match /asdfg is not allowed/, response.body
   end
 
+  def test_update_extra_param_in_links_allow_extra_params
+    JSONAPI.configuration.raise_if_parameters_not_allowed = false
+
+    set_content_type_header!
+    javascript = Section.find_by(name: 'javascript')
+
+    put :update,
+        {
+          id: 3,
+          data: {
+            type: 'posts',
+            id: '3',
+            attributes: {
+              title: 'A great new Post'
+            },
+            relationships: {
+              asdfg: 'aaaa'
+            }
+          }
+        }
+
+    assert_response :success
+    assert_equal "A great new Post", json_response["data"]["attributes"]["title"]
+    assert_equal "Param not allowed", json_response["meta"]["warnings"][0]["title"]
+    assert_equal "asdfg is not allowed.", json_response["meta"]["warnings"][0]["detail"]
+  end
+
   def test_update_missing_param
     set_content_type_header!
     javascript = Section.find_by(name: 'javascript')