Skip to content

Commit c87bd82

Browse files
zzakghiculescubyroot
committed
Deprecate ActionController.escape_json_responses= at the method
Instead of emitting the deprecation in an initializer, we should emit the deprecation when calling the writer method. In order to reduce warnings when users are updating their apps to use the new default `true`, we check the value before emitting. This includes the class attribute method as well. In order to set the value for the internal attribute, we need to find out the name of the internal setter. https://github.com/rails/rails/blob/1c094d762d25805cc2d110ee2f7b54b33352e293/activesupport/lib/active_support/core_ext/class/attribute.rb#L95-L102 In the future, it would be nice if `class_attribute` and `mattr_*`, etc had a way to easily deprecate parts of the methods being defined, or provide a callback (so we can emit warnings based on the value, etc). Co-authored-by: Alex Ghiculescu <[email protected]> Co-authored-by: Jean Boussier <[email protected]>
1 parent 4621f71 commit c87bd82

File tree

3 files changed

+105
-18
lines changed

3 files changed

+105
-18
lines changed

actionpack/lib/action_controller/metal/renderers.rb

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,23 @@ module Renderers
2727
# Default values are `:json`, `:js`, `:xml`.
2828
RENDERERS = Set.new
2929

30+
module DeprecatedEscapeJsonResponses # :nodoc:
31+
def escape_json_responses=(value)
32+
if value
33+
ActionController.deprecator.warn(<<~MSG.squish)
34+
Setting action_controller.escape_json_responses = true is deprecated and will have no effect in Rails 8.2.
35+
Set it to `false`, or remove the config.
36+
MSG
37+
end
38+
super
39+
end
40+
end
41+
3042
included do
3143
class_attribute :_renderers, default: Set.new.freeze
32-
class_attribute :escape_json_responses, instance_accessor: false, default: true
44+
class_attribute :escape_json_responses, instance_writer: false, instance_accessor: false, default: true
45+
46+
singleton_class.prepend DeprecatedEscapeJsonResponses
3347
end
3448

3549
# Used in ActionController::Base and ActionController::API to include all

actionpack/lib/action_controller/railtie.rb

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,5 @@ class Railtie < Rails::Railtie # :nodoc:
133133
ActionController::TestCase.executor_around_each_request = app.config.active_support.executor_around_test_case
134134
end
135135
end
136-
137-
initializer "action_controller.escape_json_responses_deprecated_warning" do
138-
config.after_initialize do
139-
ActiveSupport.on_load(:action_controller) do
140-
if ActionController::Base.escape_json_responses
141-
ActionController.deprecator.warn(<<~MSG.squish)
142-
Setting action_controller.escape_json_responses = true is deprecated and will have no effect in Rails 8.2.
143-
Set it to `false` or use `config.load_defaults(8.1)`.
144-
MSG
145-
end
146-
end
147-
end
148-
end
149136
end
150137
end

actionpack/test/controller/render_json_test.rb

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,96 @@ def test_render_json_with_callback_escapes_js_chars
122122
end
123123

124124
def test_render_json_with_new_default_and_without_callback_does_not_escape_js_chars
125-
TestController.with(escape_json_responses: false) do
126-
get :render_json_unsafe_chars_without_callback
127-
assert_equal %({"hello":"\u2028\u2029<script>"}), @response.body
128-
assert_equal "application/json", @response.media_type
125+
msg = <<~MSG.squish
126+
Setting action_controller.escape_json_responses = true is deprecated and will have no effect in Rails 8.2.
127+
Set it to `false`, or remove the config.
128+
MSG
129+
130+
assert_deprecated(msg, ActionController.deprecator) do
131+
TestController.with(escape_json_responses: false) do
132+
get :render_json_unsafe_chars_without_callback
133+
assert_equal %({"hello":"\u2028\u2029<script>"}), @response.body
134+
assert_equal "application/json", @response.media_type
135+
end
136+
end
137+
end
138+
139+
def test_render_json_with_optional_escape_option_is_not_deprecated
140+
@before_escape_json_responses = @controller.class.escape_json_responses
141+
142+
assert_not_deprecated(ActionController.deprecator) do
143+
@controller.class.escape_json_responses = false
144+
end
145+
146+
get :render_json_unsafe_chars_without_callback
147+
assert_equal %({"hello":"\u2028\u2029<script>"}), @response.body
148+
assert_equal "application/json", @response.media_type
149+
ensure
150+
ActionController.deprecator.silence do
151+
@controller.class.escape_json_responses = @before_escape_json_responses
152+
end
153+
end
154+
155+
def test_render_json_with_redundant_escape_option_is_deprecated
156+
@before_escape_json_responses = @controller.class.escape_json_responses
157+
158+
msg = <<~MSG.squish
159+
Setting action_controller.escape_json_responses = true is deprecated and will have no effect in Rails 8.2.
160+
Set it to `false`, or remove the config.
161+
MSG
162+
163+
assert_deprecated(msg, ActionController.deprecator) do
164+
@controller.class.escape_json_responses = true
165+
end
166+
167+
get :render_json_unsafe_chars_without_callback
168+
assert_equal '{"hello":"\\u2028\\u2029\\u003cscript\\u003e"}', @response.body
169+
assert_equal "application/json", @response.media_type
170+
ensure
171+
ActionController.deprecator.silence do
172+
@controller.class.escape_json_responses = @before_escape_json_responses
173+
end
174+
end
175+
176+
def test_set_escape_json_responses_class_method_is_deprecated
177+
@before_escape_json_responses = @controller.class.escape_json_responses
178+
179+
msg = <<~MSG.squish
180+
Setting action_controller.escape_json_responses = true is deprecated and will have no effect in Rails 8.2.
181+
Set it to `false`, or remove the config.
182+
MSG
183+
184+
assert_deprecated(msg, ActionController.deprecator) do
185+
@controller.class.escape_json_responses = true
186+
end
187+
188+
get :render_json_unsafe_chars_without_callback
189+
assert_equal '{"hello":"\\u2028\\u2029\\u003cscript\\u003e"}', @response.body
190+
assert_equal "application/json", @response.media_type
191+
ensure
192+
ActionController.deprecator.silence do
193+
@controller.class.escape_json_responses = @before_escape_json_responses
194+
end
195+
end
196+
197+
def test_set_escape_json_responses_controller_method_is_deprecated
198+
@before_escape_json_responses = @controller.class.escape_json_responses
199+
200+
msg = <<~MSG.squish
201+
Setting action_controller.escape_json_responses = true is deprecated and will have no effect in Rails 8.2.
202+
Set it to `false`, or remove the config.
203+
MSG
204+
205+
assert_deprecated(msg, ActionController.deprecator) do
206+
@controller.class.escape_json_responses = true
207+
end
208+
209+
get :render_json_unsafe_chars_without_callback
210+
assert_equal '{"hello":"\\u2028\\u2029\\u003cscript\\u003e"}', @response.body
211+
assert_equal "application/json", @response.media_type
212+
ensure
213+
ActionController.deprecator.silence do
214+
@controller.class.escape_json_responses = @before_escape_json_responses
129215
end
130216
end
131217

0 commit comments

Comments
 (0)