From 2a78d6f561e98684a4988cdc616c6096cd4302d1 Mon Sep 17 00:00:00 2001
From: Jeremy Kemper <jeremykemper@gmail.com>
Date: Fri, 5 Sep 2014 13:33:20 -0700
Subject: Deprecate implicit AD::Response splatting and Array conversion
---
actionpack/CHANGELOG.md | 13 +++++++++++++
actionpack/lib/action_dispatch/http/response.rb | 15 +++++++++++++--
actionpack/test/dispatch/response_test.rb | 20 +++++++++++++++++---
3 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index e250450a76..de9722c392 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,16 @@
+* Deprecate implicit Array conversion for Response objects. It was added
+ (using `#to_ary`) so we could conveniently use implicit splatting:
+
+ status, headers, body = response
+
+ But it also means `response + response` works and `[response].flatten`
+ cascades down to the Rack body. Nonsense behavior. Instead, rely on
+ explicit conversion and splatting with `#to_a`:
+
+ status, header, body = *response
+
+ *Jeremy Kemper*
+
* Don't rescue `IPAddr::InvalidAddressError`.
`IPAddr::InvalidAddressError` does not exist in Ruby 1.9.3
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index 2fab6be1a5..a58e904c11 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -1,4 +1,5 @@
require 'active_support/core_ext/module/attribute_accessors'
+require 'active_support/deprecation'
require 'action_dispatch/http/filter_redirect'
require 'monitor'
@@ -274,12 +275,22 @@ module ActionDispatch # :nodoc:
end
# Turns the Response into a Rack-compatible array of the status, headers,
- # and body.
+ # and body. Allows explict splatting:
+ #
+ # status, headers, body = *response
def to_a
rack_response @status, @header.to_hash
end
alias prepare! to_a
- alias to_ary to_a
+
+ # Be super clear that a response object is not an Array. Defining this
+ # would make implicit splatting work, but it also makes adding responses
+ # as arrays work, and "flattening" responses, cascading to the rack body!
+ # Not sensible behavior.
+ def to_ary
+ ActiveSupport::Deprecation.warn 'ActionDispatch::Response#to_ary no longer performs implicit conversion to an Array. Please use response.to_a instead, or a splat like `status, headers, body = *response`'
+ to_a
+ end
# Returns the response cookies, converted to a Hash of (name => value) pairs
#
diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb
index 187b9a2420..af188191e3 100644
--- a/actionpack/test/dispatch/response_test.rb
+++ b/actionpack/test/dispatch/response_test.rb
@@ -220,9 +220,9 @@ class ResponseTest < ActiveSupport::TestCase
assert @response.respond_to?(:method_missing, true)
end
- test "can be destructured into status, headers and an enumerable body" do
+ test "can be explicitly destructured into status, headers and an enumerable body" do
response = ActionDispatch::Response.new(404, { 'Content-Type' => 'text/plain' }, ['Not Found'])
- status, headers, body = response
+ status, headers, body = *response
assert_equal 404, status
assert_equal({ 'Content-Type' => 'text/plain' }, headers)
@@ -231,12 +231,26 @@ class ResponseTest < ActiveSupport::TestCase
test "[response].flatten does not recurse infinitely" do
Timeout.timeout(1) do # use a timeout to prevent it stalling indefinitely
- status, headers, body = [@response].flatten
+ status, headers, body = assert_deprecated { [@response].flatten }
assert_equal @response.status, status
assert_equal @response.headers, headers
assert_equal @response.body, body.each.to_a.join
end
end
+
+ test "implicit destructuring and Array conversion is deprecated" do
+ response = ActionDispatch::Response.new(404, { 'Content-Type' => 'text/plain' }, ['Not Found'])
+
+ assert_deprecated do
+ status, headers, body = response
+
+ assert_equal 404, status
+ assert_equal({ 'Content-Type' => 'text/plain' }, headers)
+ assert_equal ['Not Found'], body.each.to_a
+ end
+
+ assert_deprecated { response.to_ary }
+ end
end
class ResponseIntegrationTest < ActionDispatch::IntegrationTest
--
cgit v1.2.3