Merge pull request #1096 from cerebris/includes_config

Allow more control over allowing includes

Author: dgeb

lib/jsonapi/configuration.rb

@@ -10,7 +10,8 @@ class Configuration
                 :route_format,
                 :raise_if_parameters_not_allowed,
                 :warn_on_route_setup_issues,
-                :allow_include,
+                :default_allow_include_to_one,
+                :default_allow_include_to_many,
                 :allow_sort,
                 :allow_filter,
                 :default_paginator,
@@ -50,7 +51,8 @@ def initialize
       self.resource_key_type = :integer
 
       # optional request features
-      self.allow_include = true
+      self.default_allow_include_to_one = true
+      self.default_allow_include_to_many = true
       self.allow_sort = true
       self.allow_filter = true
 
@@ -227,7 +229,13 @@ def resource_finder=(resource_finder)
       @resource_finder = resource_finder
     end
 
-    attr_writer :allow_include, :allow_sort, :allow_filter
+    def allow_include=(allow_include)
+      ActiveSupport::Deprecation.warn('`allow_include` has been replaced by `default_allow_include_to_one` and `default_allow_include_to_many` options.')
+      @default_allow_include_to_one = allow_include
+      @default_allow_include_to_many = allow_include
+    end
+
+    attr_writer :allow_sort, :allow_filter, :default_allow_include_to_one, :default_allow_include_to_many
 
     attr_writer :default_paginator
 

lib/jsonapi/exceptions.rb

@@ -342,7 +342,7 @@ def errors
                              title: I18n.translate('jsonapi-resources.exceptions.invalid_include.title',
                                                    default: 'Invalid field'),
                              detail: I18n.translate('jsonapi-resources.exceptions.invalid_include.detail',
-                                                    default: "#{relationship} is not a valid relationship of #{resource}",
+                                                    default: "#{relationship} is not a valid includable relationship of #{resource}",
                                                     relationship: relationship, resource: resource))]
       end
     end

lib/jsonapi/relationship.rb

@@ -3,7 +3,9 @@ class Relationship
     attr_reader :acts_as_set, :foreign_key, :options, :name,
                 :class_name, :polymorphic, :always_include_linkage_data,
                 :parent_resource, :eager_load_on_include, :custom_methods,
-                :inverse_relationship
+                :inverse_relationship, :allow_include
+
+    attr_writer :allow_include
 
     def initialize(name, options = {})
       @name = name.to_s
@@ -17,6 +19,7 @@ def initialize(name, options = {})
       @polymorphic_relations = options[:polymorphic_relations]
       @always_include_linkage_data = options.fetch(:always_include_linkage_data, false) == true
       @eager_load_on_include = options.fetch(:eager_load_on_include, true) == true
+      @allow_include = options[:allow_include]
     end
 
     alias_method :polymorphic?, :polymorphic
@@ -100,6 +103,22 @@ def belongs_to?
       def polymorphic_type
         "#{name}_type" if polymorphic?
       end
+
+      def allow_include?(context = nil)
+        strategy = if @allow_include.nil?
+                     JSONAPI.configuration.default_allow_include_to_one
+                   else
+                     @allow_include
+                   end
+
+        if !!strategy == strategy #check for boolean
+          return strategy
+        elsif strategy.is_a?(Symbol) || strategy.is_a?(String)
+          parent_resource.send(strategy, context)
+        else
+          strategy.call(context)
+        end
+      end
     end
 
     class ToMany < Relationship
@@ -114,6 +133,23 @@ def initialize(name, options = {})
           @inverse_relationship = options.fetch(:inverse_relationship, parent_resource._type.to_s.singularize.to_sym)
         end
       end
+
+      def allow_include?(context = nil)
+        strategy = if @allow_include.nil?
+                     JSONAPI.configuration.default_allow_include_to_many
+                   else
+                     @allow_include
+                   end
+
+        if !!strategy == strategy #check for boolean
+          return strategy
+        elsif strategy.is_a?(Symbol) || strategy.is_a?(String)
+          parent_resource.send(strategy, context)
+        else
+          strategy.call(context)
+        end
+
+      end
     end
   end
 end

lib/jsonapi/request_parser.rb

@@ -330,22 +330,23 @@ def check_include(resource_klass, include_parts)
 
       relationship = resource_klass._relationship(relationship_name)
       if relationship && format_key(relationship_name) == include_parts.first
+        unless relationship.allow_include?(context)
+          fail JSONAPI::Exceptions::InvalidInclude.new(format_key(resource_klass._type), include_parts.first)
+        end
+
         unless include_parts.last.empty?
           check_include(Resource.resource_klass_for(resource_klass.module_path + relationship.class_name.to_s.underscore),
                         include_parts.last.partition('.'))
         end
       else
         fail JSONAPI::Exceptions::InvalidInclude.new(format_key(resource_klass._type), include_parts.first)
       end
+      true
     end
 
     def parse_include_directives(resource_klass, raw_include)
       return unless raw_include
 
-      unless JSONAPI.configuration.allow_include
-        fail JSONAPI::Exceptions::ParameterNotAllowed.new(:include)
-      end
-
       included_resources = []
       begin
         included_resources += raw_include.is_a?(Array) ? raw_include : CSV.parse_line(raw_include) || []

locales/en.yml

@@ -48,7 +48,7 @@ en:
         detail: "%{field} is not a valid field for %{type}."
       invalid_include:
         title: 'Invalid include'
-        detail: "%{relationship} is not a valid relationship of %{resource}"
+        detail: "%{relationship} is not a valid includable relationship of %{resource}"
       invalid_sort_criteria:
         title: 'Invalid sort criteria'
         detail: "%{sort_criteria} is not a valid sort criteria for %{resource}"

test/controllers/controller_test.rb

@@ -551,11 +551,12 @@ def test_show_single_with_includes
   end
 
   def test_show_single_with_include_disallowed
+    original_config = JSONAPI.configuration.dup
     JSONAPI.configuration.allow_include = false
     assert_cacheable_get :show, params: {id: '1', include: 'comments'}
     assert_response :bad_request
   ensure
-    JSONAPI.configuration.allow_include = true
+    JSONAPI.configuration = original_config
   end
 
   def test_show_single_with_fields
@@ -2122,25 +2123,25 @@ def test_expense_entries_show_include
   def test_expense_entries_show_bad_include_missing_relationship
     assert_cacheable_get :show, params: {id: 1, include: 'isoCurrencies,employees'}
     assert_response :bad_request
-    assert_match /isoCurrencies is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
+    assert_match /isoCurrencies is not a valid includable relationship of expenseEntries/, json_response['errors'][0]['detail']
   end
 
   def test_expense_entries_show_bad_include_missing_sub_relationship
     assert_cacheable_get :show, params: {id: 1, include: 'isoCurrency,employee.post'}
     assert_response :bad_request
-    assert_match /post is not a valid relationship of employees/, json_response['errors'][0]['detail']
+    assert_match /post is not a valid includable relationship of employees/, json_response['errors'][0]['detail']
   end
 
   def test_invalid_include
     assert_cacheable_get :index, params: {include: 'invalid../../../../'}
     assert_response :bad_request
-    assert_match /invalid is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
+    assert_match /invalid is not a valid includable relationship of expenseEntries/, json_response['errors'][0]['detail']
   end
 
   def test_invalid_include_long_garbage_string
     assert_cacheable_get :index, params: {include: 'invalid.foo.bar.dfsdfs,dfsdfs.sdfwe.ewrerw.erwrewrew'}
     assert_response :bad_request
-    assert_match /invalid is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
+    assert_match /invalid is not a valid includable relationship of expenseEntries/, json_response['errors'][0]['detail']
   end
 
   def test_expense_entries_show_fields

test/integration/requests/request_test.rb

@@ -1125,14 +1125,53 @@ def test_include_parameter_allowed
     assert_cacheable_jsonapi_get '/api/v2/books/1/book_comments?include=author'
   end
 
-  def test_include_parameter_not_allowed
+  def test_deprecated_include_parameter_not_allowed
+    original_config = JSONAPI.configuration.dup
     JSONAPI.configuration.allow_include = false
     get '/api/v2/books/1/book_comments?include=author', headers: {
       'Accept' => JSONAPI::MEDIA_TYPE
     }
     assert_jsonapi_response 400
   ensure
-    JSONAPI.configuration.allow_include = true
+    JSONAPI.configuration = original_config
+  end
+
+  def test_deprecated_include_message
+    ActiveSupport::Deprecation.silenced = false
+    original_config = JSONAPI.configuration.dup
+    _out, err = capture_io do
+      eval <<-CODE
+        JSONAPI.configuration.allow_include = false
+      CODE
+    end
+    assert_match /DEPRECATION WARNING: `allow_include` has been replaced by `default_allow_include_to_one` and `default_allow_include_to_many` options./, err
+  ensure
+    JSONAPI.configuration = original_config
+    ActiveSupport::Deprecation.silenced = true
+  end
+
+
+  def test_to_one_include_parameter_not_allowed
+    original_config = JSONAPI.configuration.dup
+    JSONAPI.configuration.default_allow_include_to_one = false
+    get '/api/v2/books/1/book_comments?include=author', headers: {
+        'Accept' => JSONAPI::MEDIA_TYPE
+    }
+    assert_jsonapi_response 400
+  ensure
+    JSONAPI.configuration = original_config
+  end
+
+  def test_to_one_include_parameter_allowed
+    original_config = JSONAPI.configuration.dup
+    JSONAPI.configuration.default_allow_include_to_one = true
+    get '/api/v2/books/1/book_comments?include=author', headers: {
+        'Accept' => JSONAPI::MEDIA_TYPE
+    }
+    assert_jsonapi_response 200
+    assert_equal 1, json_response['included'].size
+  ensure
+    JSONAPI.configuration = original_config
   end
 
   def test_filter_parameter_not_allowed

test/unit/jsonapi_request/jsonapi_request_test.rb

@@ -47,6 +47,92 @@ def test_parse_blank_includes
     assert_empty include_directives.model_includes
   end
 
+  def test_check_include_allowed
+    reset_includes
+    assert JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "isoCurrency".partition('.'))
+  ensure
+    reset_includes
+  end
+
+  def test_check_nested_include_allowed
+    reset_includes
+    assert JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "employee.expenseEntries".partition('.'))
+  ensure
+    reset_includes
+  end
+
+  def test_check_include_relationship_does_not_exist
+    reset_includes
+
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      assert JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "foo".partition('.'))
+    end
+  ensure
+    reset_includes
+  end
+
+  def test_check_nested_include_relationship_does_not_exist_wrong_format
+    reset_includes
+
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      assert JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "employee.expense-entries".partition('.'))
+    end
+  ensure
+    reset_includes
+  end
+
+  def test_check_include_has_one_not_allowed_default
+    reset_includes
+
+    assert JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "isoCurrency".partition('.'))
+    JSONAPI.configuration.default_allow_include_to_one = false
+
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "isoCurrency".partition('.'))
+    end
+  ensure
+      reset_includes
+  end
+
+  def test_check_include_has_one_not_allowed_resource
+    reset_includes
+
+    assert JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "isoCurrency".partition('.'))
+    ExpenseEntryResource._relationship(:iso_currency).allow_include = false
+
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      JSONAPI::RequestParser.new.check_include(ExpenseEntryResource, "isoCurrency".partition('.'))
+    end
+  ensure
+    reset_includes
+  end
+
+  def test_check_include_has_many_not_allowed_default
+    reset_includes
+
+    assert JSONAPI::RequestParser.new.check_include(EmployeeResource, "expenseEntries".partition('.'))
+    JSONAPI.configuration.default_allow_include_to_many = false
+
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      JSONAPI::RequestParser.new.check_include(EmployeeResource, "expenseEntries".partition('.'))
+    end
+  ensure
+    reset_includes
+  end
+
+  def test_check_include_has_many_not_allowed_resource
+    reset_includes
+
+    assert JSONAPI::RequestParser.new.check_include(EmployeeResource, "expenseEntries".partition('.'))
+    EmployeeResource._relationship(:expense_entries).allow_include = false
+
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      JSONAPI::RequestParser.new.check_include(EmployeeResource, "expenseEntries".partition('.'))
+    end
+  ensure
+    reset_includes
+  end
+
   def test_parse_dasherized_with_dasherized_include
     params = ActionController::Parameters.new(
       {
@@ -87,7 +173,7 @@ def test_parse_dasherized_with_underscored_include
 
     request.parse_include_directives(ExpenseEntryResource, params[:include])
     refute request.errors.empty?
-    assert_equal 'iso_currency is not a valid relationship of expense-entries', request.errors[0].detail
+    assert_equal 'iso_currency is not a valid includable relationship of expense-entries', request.errors[0].detail
   end
 
   def test_parse_fields_underscored
@@ -242,4 +328,12 @@ def test_parse_sort_with_relationships
   def setup_request
     @request = JSONAPI::RequestParser.new
   end
+
+  def reset_includes
+    JSONAPI.configuration.json_key_format = :camelized_key
+    JSONAPI.configuration.default_allow_include_to_one = true
+    JSONAPI.configuration.default_allow_include_to_many = true
+    ExpenseEntryResource._relationship(:iso_currency).allow_include = nil
+    EmployeeResource._relationship(:expense_entries).allow_include = nil
+  end
 end