Skip to content

Commit cffb8c1

Browse files
committed
Deprecates and replaces the allow_include config option
Replaces `allow_includes` with `default_allow_include_to_one` and `default_allow_include_to_many` configuration options. Adds support for `allow_include` option on Relationships to override the config settings.
1 parent f619538 commit cffb8c1

9 files changed

Lines changed: 146 additions & 18 deletions

File tree

lib/jsonapi/configuration.rb

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ class Configuration
1010
:route_format,
1111
:raise_if_parameters_not_allowed,
1212
:warn_on_route_setup_issues,
13-
:allow_include,
13+
:default_allow_include_to_one,
14+
:default_allow_include_to_many,
1415
:allow_sort,
1516
:allow_filter,
1617
:default_paginator,
@@ -50,7 +51,8 @@ def initialize
5051
self.resource_key_type = :integer
5152

5253
# optional request features
53-
self.allow_include = true
54+
self.default_allow_include_to_one = true
55+
self.default_allow_include_to_many = true
5456
self.allow_sort = true
5557
self.allow_filter = true
5658

@@ -227,7 +229,13 @@ def resource_finder=(resource_finder)
227229
@resource_finder = resource_finder
228230
end
229231

230-
attr_writer :allow_include, :allow_sort, :allow_filter
232+
def allow_include=(allow_include)
233+
ActiveSupport::Deprecation.warn('`allow_include` has been replaced by `default_allow_include_to_one` and `default_allow_include_to_many` options.')
234+
@default_allow_include_to_one = allow_include
235+
@default_allow_include_to_many = allow_include
236+
end
237+
238+
attr_writer :allow_sort, :allow_filter, :default_allow_include_to_one, :default_allow_include_to_many
231239

232240
attr_writer :default_paginator
233241

lib/jsonapi/exceptions.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ def errors
342342
title: I18n.translate('jsonapi-resources.exceptions.invalid_include.title',
343343
default: 'Invalid field'),
344344
detail: I18n.translate('jsonapi-resources.exceptions.invalid_include.detail',
345-
default: "#{relationship} is not a valid relationship of #{resource}",
345+
default: "#{relationship} is not a valid includable relationship of #{resource}",
346346
relationship: relationship, resource: resource))]
347347
end
348348
end

lib/jsonapi/relationship.rb

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ class Relationship
33
attr_reader :acts_as_set, :foreign_key, :options, :name,
44
:class_name, :polymorphic, :always_include_linkage_data,
55
:parent_resource, :eager_load_on_include, :custom_methods,
6-
:inverse_relationship
6+
:inverse_relationship, :allow_include
77

88
def initialize(name, options = {})
99
@name = name.to_s
@@ -17,6 +17,7 @@ def initialize(name, options = {})
1717
@polymorphic_relations = options[:polymorphic_relations]
1818
@always_include_linkage_data = options.fetch(:always_include_linkage_data, false) == true
1919
@eager_load_on_include = options.fetch(:eager_load_on_include, true) == true
20+
@allow_include = options[:allow_include]
2021
end
2122

2223
alias_method :polymorphic?, :polymorphic
@@ -100,6 +101,14 @@ def belongs_to?
100101
def polymorphic_type
101102
"#{name}_type" if polymorphic?
102103
end
104+
105+
def allow_include?
106+
if @allow_include.nil?
107+
JSONAPI.configuration.default_allow_include_to_one
108+
else
109+
@allow_include
110+
end
111+
end
103112
end
104113

105114
class ToMany < Relationship
@@ -114,6 +123,14 @@ def initialize(name, options = {})
114123
@inverse_relationship = options.fetch(:inverse_relationship, parent_resource._type.to_s.singularize.to_sym)
115124
end
116125
end
126+
127+
def allow_include?
128+
if @allow_include.nil?
129+
JSONAPI.configuration.default_allow_include_to_one
130+
else
131+
@allow_include
132+
end
133+
end
117134
end
118135
end
119136
end

lib/jsonapi/request_parser.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,10 @@ def check_include(resource_klass, include_parts)
330330

331331
relationship = resource_klass._relationship(relationship_name)
332332
if relationship && format_key(relationship_name) == include_parts.first
333+
unless relationship.allow_include?
334+
fail JSONAPI::Exceptions::InvalidInclude.new(format_key(resource_klass._type), include_parts.first)
335+
end
336+
333337
unless include_parts.last.empty?
334338
check_include(Resource.resource_klass_for(resource_klass.module_path + relationship.class_name.to_s.underscore),
335339
include_parts.last.partition('.'))
@@ -342,10 +346,6 @@ def check_include(resource_klass, include_parts)
342346
def parse_include_directives(resource_klass, raw_include)
343347
return unless raw_include
344348

345-
unless JSONAPI.configuration.allow_include
346-
fail JSONAPI::Exceptions::ParameterNotAllowed.new(:include)
347-
end
348-
349349
included_resources = []
350350
begin
351351
included_resources += raw_include.is_a?(Array) ? raw_include : CSV.parse_line(raw_include) || []

locales/en.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ en:
4848
detail: "%{field} is not a valid field for %{type}."
4949
invalid_include:
5050
title: 'Invalid include'
51-
detail: "%{relationship} is not a valid relationship of %{resource}"
51+
detail: "%{relationship} is not a valid includable relationship of %{resource}"
5252
invalid_sort_criteria:
5353
title: 'Invalid sort criteria'
5454
detail: "%{sort_criteria} is not a valid sort criteria for %{resource}"

test/controllers/controller_test.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -551,11 +551,12 @@ def test_show_single_with_includes
551551
end
552552

553553
def test_show_single_with_include_disallowed
554+
original_config = JSONAPI.configuration.dup
554555
JSONAPI.configuration.allow_include = false
555556
assert_cacheable_get :show, params: {id: '1', include: 'comments'}
556557
assert_response :bad_request
557558
ensure
558-
JSONAPI.configuration.allow_include = true
559+
JSONAPI.configuration = original_config
559560
end
560561

561562
def test_show_single_with_fields
@@ -2122,25 +2123,25 @@ def test_expense_entries_show_include
21222123
def test_expense_entries_show_bad_include_missing_relationship
21232124
assert_cacheable_get :show, params: {id: 1, include: 'isoCurrencies,employees'}
21242125
assert_response :bad_request
2125-
assert_match /isoCurrencies is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
2126+
assert_match /isoCurrencies is not a valid includable relationship of expenseEntries/, json_response['errors'][0]['detail']
21262127
end
21272128

21282129
def test_expense_entries_show_bad_include_missing_sub_relationship
21292130
assert_cacheable_get :show, params: {id: 1, include: 'isoCurrency,employee.post'}
21302131
assert_response :bad_request
2131-
assert_match /post is not a valid relationship of employees/, json_response['errors'][0]['detail']
2132+
assert_match /post is not a valid includable relationship of employees/, json_response['errors'][0]['detail']
21322133
end
21332134

21342135
def test_invalid_include
21352136
assert_cacheable_get :index, params: {include: 'invalid../../../../'}
21362137
assert_response :bad_request
2137-
assert_match /invalid is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
2138+
assert_match /invalid is not a valid includable relationship of expenseEntries/, json_response['errors'][0]['detail']
21382139
end
21392140

21402141
def test_invalid_include_long_garbage_string
21412142
assert_cacheable_get :index, params: {include: 'invalid.foo.bar.dfsdfs,dfsdfs.sdfwe.ewrerw.erwrewrew'}
21422143
assert_response :bad_request
2143-
assert_match /invalid is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
2144+
assert_match /invalid is not a valid includable relationship of expenseEntries/, json_response['errors'][0]['detail']
21442145
end
21452146

21462147
def test_expense_entries_show_fields

test/integration/requests/request_test.rb

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,14 +1125,53 @@ def test_include_parameter_allowed
11251125
assert_cacheable_jsonapi_get '/api/v2/books/1/book_comments?include=author'
11261126
end
11271127

1128-
def test_include_parameter_not_allowed
1128+
def test_deprecated_include_parameter_not_allowed
1129+
original_config = JSONAPI.configuration.dup
11291130
JSONAPI.configuration.allow_include = false
11301131
get '/api/v2/books/1/book_comments?include=author', headers: {
11311132
'Accept' => JSONAPI::MEDIA_TYPE
11321133
}
11331134
assert_jsonapi_response 400
11341135
ensure
1135-
JSONAPI.configuration.allow_include = true
1136+
JSONAPI.configuration = original_config
1137+
end
1138+
1139+
def test_deprecated_include_message
1140+
ActiveSupport::Deprecation.silenced = false
1141+
original_config = JSONAPI.configuration.dup
1142+
_out, err = capture_io do
1143+
eval <<-CODE
1144+
JSONAPI.configuration.allow_include = false
1145+
CODE
1146+
end
1147+
assert_match /DEPRECATION WARNING: `allow_include` has been replaced by `default_allow_include_to_one` and `default_allow_include_to_many` options./, err
1148+
ensure
1149+
JSONAPI.configuration = original_config
1150+
ActiveSupport::Deprecation.silenced = true
1151+
end
1152+
1153+
1154+
def test_to_one_include_parameter_not_allowed
1155+
original_config = JSONAPI.configuration.dup
1156+
JSONAPI.configuration.default_allow_include_to_one = false
1157+
get '/api/v2/books/1/book_comments?include=author', headers: {
1158+
'Accept' => JSONAPI::MEDIA_TYPE
1159+
}
1160+
assert_jsonapi_response 400
1161+
ensure
1162+
JSONAPI.configuration = original_config
1163+
end
1164+
1165+
def test_to_one_include_parameter_allowed
1166+
original_config = JSONAPI.configuration.dup
1167+
JSONAPI.configuration.default_allow_include_to_one = true
1168+
get '/api/v2/books/1/book_comments?include=author', headers: {
1169+
'Accept' => JSONAPI::MEDIA_TYPE
1170+
}
1171+
assert_jsonapi_response 200
1172+
assert_equal 1, json_response['included'].size
1173+
ensure
1174+
JSONAPI.configuration = original_config
11361175
end
11371176

11381177
def test_filter_parameter_not_allowed

test/unit/jsonapi_request/jsonapi_request_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ def test_parse_dasherized_with_underscored_include
8787

8888
request.parse_include_directives(ExpenseEntryResource, params[:include])
8989
refute request.errors.empty?
90-
assert_equal 'iso_currency is not a valid relationship of expense-entries', request.errors[0].detail
90+
assert_equal 'iso_currency is not a valid includable relationship of expense-entries', request.errors[0].detail
9191
end
9292

9393
def test_parse_fields_underscored

test/unit/resource/relationship_test.rb

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,67 @@ def test_polymorphic_type
99
assert_equal(relationship.polymorphic_type, "imageable_type")
1010
end
1111

12+
def test_allow_include_not_set_defaults_to_config_to_one
13+
original_config = JSONAPI.configuration.dup
14+
15+
JSONAPI.configuration.default_allow_include_to_one = true
16+
relationship = JSONAPI::Relationship::ToOne.new("foo")
17+
assert(relationship.allow_include?)
18+
19+
JSONAPI.configuration.default_allow_include_to_one = false
20+
relationship = JSONAPI::Relationship::ToOne.new("foo")
21+
refute(relationship.allow_include?)
22+
23+
ensure
24+
JSONAPI.configuration = original_config
25+
end
26+
27+
def test_allow_include_not_set_defaults_to_config_to_many
28+
original_config = JSONAPI.configuration.dup
29+
30+
JSONAPI.configuration.default_allow_include_to_many = true
31+
relationship = JSONAPI::Relationship::ToMany.new("foobar")
32+
assert(relationship.allow_include?)
33+
34+
JSONAPI.configuration.default_allow_include_to_one = false
35+
relationship = JSONAPI::Relationship::ToOne.new("foobar")
36+
refute(relationship.allow_include?)
37+
38+
ensure
39+
JSONAPI.configuration = original_config
40+
end
41+
42+
def test_allow_include_set_overrides_to_config_to_one
43+
original_config = JSONAPI.configuration.dup
44+
45+
JSONAPI.configuration.default_allow_include_to_one = true
46+
relationship1 = JSONAPI::Relationship::ToOne.new("foo1", allow_include: false)
47+
relationship2 = JSONAPI::Relationship::ToOne.new("foo2", allow_include: true)
48+
refute(relationship1.allow_include?)
49+
assert(relationship2.allow_include?)
50+
51+
JSONAPI.configuration.default_allow_include_to_one = false
52+
refute(relationship1.allow_include?)
53+
assert(relationship2.allow_include?)
54+
55+
ensure
56+
JSONAPI.configuration = original_config
57+
end
58+
59+
def test_allow_include_set_overrides_to_config_to_many
60+
original_config = JSONAPI.configuration.dup
61+
62+
JSONAPI.configuration.default_allow_include_to_one = true
63+
relationship1 = JSONAPI::Relationship::ToMany.new("foobar1", allow_include: false)
64+
relationship2 = JSONAPI::Relationship::ToMany.new("foobar2", allow_include: true)
65+
refute(relationship1.allow_include?)
66+
assert(relationship2.allow_include?)
67+
68+
JSONAPI.configuration.default_allow_include_to_one = false
69+
refute(relationship1.allow_include?)
70+
assert(relationship2.allow_include?)
71+
72+
ensure
73+
JSONAPI.configuration = original_config
74+
end
1275
end

0 commit comments

Comments
 (0)