Merge remote-tracking branch 'origin/master' into guides-template-inheritance

16 files changed, 124 insertions, 78 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 4e8e0f41b4..de9722c392 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,23 @@
+*   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
+    and fails for JRuby in 1.9 mode.
+
+    *Peter Suschlik*
+
 *   Fix bug where the router would ignore any constraints added to redirect
     routes.
 
diff --git a/actionpack/Rakefile b/actionpack/Rakefile
index cf20751687..af075f82ad 100644
--- a/actionpack/Rakefile
+++ b/actionpack/Rakefile
@@ -9,7 +9,10 @@ task :default => :test
 # Run the unit tests
 Rake::TestTask.new do |t|
   t.libs << 'test'
-  t.test_files = test_files
+
+  # make sure we include the tests in alphabetical order as on some systems
+  # this will not happen automatically and the tests (as a whole) will error
+  t.test_files = test_files.sort
 
   t.warning = true
   t.verbose = true
diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec
index a39b3e86d4..c752388e28 100644
--- a/actionpack/actionpack.gemspec
+++ b/actionpack/actionpack.gemspec
@@ -23,7 +23,7 @@ Gem::Specification.new do |s|
 
   s.add_dependency 'rack',      '~> 1.6.0.beta'
   s.add_dependency 'rack-test', '~> 0.6.2'
-  s.add_dependency 'rails-deprecated_sanitizer', '~> 1.0', '>= 1.0.2'
+  s.add_dependency 'rails-html-sanitizer', '~> 1.0'
   s.add_dependency 'rails-dom-testing', '~> 1.0', '>= 1.0.2'
   s.add_dependency 'actionview', version
 
diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb
index 02c4e563f5..bc94536c8c 100644
--- a/actionpack/lib/action_controller/metal/renderers.rb
+++ b/actionpack/lib/action_controller/metal/renderers.rb
@@ -34,14 +34,15 @@ module ActionController
     end
 
     def render_to_body(options)
-      _handle_render_options(options) || super
+      _render_to_body_with_renderer(options) || super
     end
 
-    def _handle_render_options(options)
+    def _render_to_body_with_renderer(options)
       _renderers.each do |name|
         if options.key?(name)
           _process_options(options)
-          return send("_render_option_#{name}", options.delete(name), options)
+          method_name = Renderers._render_with_renderer_method_name(name)
+          return send(method_name, options.delete(name), options)
         end
       end
       nil
@@ -51,6 +52,10 @@ module ActionController
     # Default values are <tt>:json</tt>, <tt>:js</tt>, <tt>:xml</tt>.
     RENDERERS = Set.new
 
+    def self._render_with_renderer_method_name(key)
+      "_render_with_renderer_#{key}"
+    end
+
     # Adds a new renderer to call within controller actions.
     # A renderer is invoked by passing its name as an option to
     # <tt>AbstractController::Rendering#render</tt>. To create a renderer
@@ -84,7 +89,7 @@ module ActionController
     # <tt>ActionController::MimeResponds::ClassMethods.respond_to</tt> and
     # <tt>ActionController::MimeResponds#respond_with</tt>
     def self.add(key, &block)
-      define_method("_render_option_#{key}", &block)
+      define_method(_render_with_renderer_method_name(key), &block)
       RENDERERS << key.to_sym
     end
 
@@ -95,8 +100,8 @@ module ActionController
     #   ActionController::Renderers.remove(:csv)
     def self.remove(key)
       RENDERERS.delete(key.to_sym)
-      method = "_render_option_#{key}"
-      remove_method(method) if method_defined?(method)
+      method_name = _render_with_renderer_method_name(key)
+      remove_method(method_name) if method_defined?(method_name)
     end
 
     module All
@@ -112,7 +117,7 @@ module ActionController
       json = json.to_json(options) unless json.kind_of?(String)
 
       if options[:callback].present?
-        if self.content_type.nil? || self.content_type == Mime::JSON
+        if content_type.nil? || content_type == Mime::JSON
           self.content_type = Mime::JS
         end
 
diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb
index 9450be838c..b9d5009683 100644
--- a/actionpack/lib/action_dispatch/http/mime_type.rb
+++ b/actionpack/lib/action_dispatch/http/mime_type.rb
@@ -45,8 +45,8 @@ module Mime
   #
   #       respond_to do |format|
   #         format.html
-  #         format.ics { render text: post.to_ics, mime_type: Mime::Type["text/calendar"]  }
-  #         format.xml { render xml: @people }
+  #         format.ics { render text: @post.to_ics, mime_type: Mime::Type["text/calendar"]  }
+  #         format.xml { render xml: @post }
   #       end
   #     end
   #   end
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index 2fab6be1a5..c5e18048da 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
     #
@@ -369,6 +380,10 @@ module ActionDispatch # :nodoc:
       def to_path
         @response.stream.to_path
       end
+
+      def to_ary
+        nil
+      end
     end
 
     def rack_response(status, header)
diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
index b022fea001..7c4236518d 100644
--- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb
+++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
@@ -155,7 +155,7 @@ module ActionDispatch
             range = IPAddr.new(ip).to_range
             # we want to make sure nobody is sneaking a netmask in
             range.begin == range.end
-          rescue ArgumentError, IPAddr::InvalidAddressError
+          rescue ArgumentError
             nil
           end
         end
diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
index 427a5674bd..f15868d37e 100644
--- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
+++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
@@ -197,7 +197,8 @@ module ActionDispatch
 
           case record_or_hash_or_array
           when Array
-            if record_or_hash_or_array.empty? || record_or_hash_or_array.include?(nil)
+            record_or_hash_or_array = record_or_hash_or_array.compact
+            if record_or_hash_or_array.empty?
               raise ArgumentError, "Nil location provided. Can't build URI."
             end
             if record_or_hash_or_array.first.is_a?(ActionDispatch::Routing::RoutesProxy)
diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
index 2cf38a9c2d..e06f7037c6 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
@@ -38,7 +38,7 @@ module ActionDispatch
       #   # Test a custom route
       #   assert_recognizes({controller: 'items', action: 'show', id: '1'}, 'view/item1')
       def assert_recognizes(expected_options, path, extras={}, msg=nil)
-        request = recognized_request_for(path, extras)
+        request = recognized_request_for(path, extras, msg)
 
         expected_options = expected_options.clone
 
@@ -69,9 +69,9 @@ module ActionDispatch
       #
       #   # Asserts that the generated route gives us our custom route
       #   assert_generates "changesets/12", { controller: 'scm', action: 'show_diff', revision: "12" }
-      def assert_generates(expected_path, options, defaults={}, extras = {}, message=nil)
+      def assert_generates(expected_path, options, defaults={}, extras={}, message=nil)
         if expected_path =~ %r{://}
-          fail_on(URI::InvalidURIError) do
+          fail_on(URI::InvalidURIError, message) do
             uri = URI.parse(expected_path)
             expected_path = uri.path.to_s.empty? ? "/" : uri.path
           end
@@ -174,7 +174,7 @@ module ActionDispatch
 
       private
         # Recognizes the route for a given path.
-        def recognized_request_for(path, extras = {})
+        def recognized_request_for(path, extras = {}, msg)
           if path.is_a?(Hash)
             method = path[:method]
             path   = path[:path]
@@ -186,7 +186,7 @@ module ActionDispatch
           request = ActionController::TestRequest.new
 
           if path =~ %r{://}
-            fail_on(URI::InvalidURIError) do
+            fail_on(URI::InvalidURIError, msg) do
               uri = URI.parse(path)
               request.env["rack.url_scheme"] = uri.scheme || "http"
               request.host = uri.host if uri.host
@@ -200,7 +200,7 @@ module ActionDispatch
 
           request.request_method = method if method
 
-          params = fail_on(ActionController::RoutingError) do
+          params = fail_on(ActionController::RoutingError, msg) do
             @routes.recognize_path(path, { :method => method, :extras => extras })
           end
           request.path_parameters = params.with_indifferent_access
@@ -208,10 +208,10 @@ module ActionDispatch
           request
         end
 
-        def fail_on(exception_class)
+        def fail_on(exception_class, message)
           yield
         rescue exception_class => e
-          raise Minitest::Assertion, e.message
+          raise Minitest::Assertion, message || e.message
         end
     end
   end
diff --git a/actionpack/lib/action_dispatch/testing/assertions/selector.rb b/actionpack/lib/action_dispatch/testing/assertions/selector.rb
index 19eca60f70..7361e6c44b 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/selector.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/selector.rb
@@ -1,3 +1,3 @@
 require 'active_support/deprecation'
 
-ActiveSupport::Deprecation.warn("ActionDispatch::Assertions::SelectorAssertions has been has been extracted to the rails-dom-testing gem.")
\ No newline at end of file
+ActiveSupport::Deprecation.warn("ActionDispatch::Assertions::SelectorAssertions has been extracted to the rails-dom-testing gem.")
diff --git a/actionpack/lib/action_dispatch/testing/assertions/tag.rb b/actionpack/lib/action_dispatch/testing/assertions/tag.rb
index d5348d80e1..5c2905d1ac 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/tag.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/tag.rb
@@ -1,3 +1,3 @@
 require 'active_support/deprecation'
 
-ActiveSupport::Deprecation.warn("ActionDispatch::Assertions::TagAssertions has been has been extracted to the rails-dom-testing gem.")
+ActiveSupport::Deprecation.warn("ActionDispatch::Assertions::TagAssertions has been extracted to the rails-dom-testing gem.")
diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb
index beaf35d3da..280d35adcb 100644
--- a/actionpack/lib/action_pack/gem_version.rb
+++ b/actionpack/lib/action_pack/gem_version.rb
@@ -1,5 +1,5 @@
 module ActionPack
-  # Returns the version of the currently loaded ActionPack as a <tt>Gem::Version</tt>
+  # Returns the version of the currently loaded Action Pack as a <tt>Gem::Version</tt>
   def self.gem_version
     Gem::Version.new VERSION::STRING
   end
@@ -8,7 +8,7 @@ module ActionPack
     MAJOR = 4
     MINOR = 2
     TINY  = 0
-    PRE   = "alpha"
+    PRE   = "beta1"
 
     STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
   end
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 674fb253f4..69312e4c22 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -4,8 +4,6 @@ $:.unshift(File.dirname(__FILE__) + '/lib')
 $:.unshift(File.dirname(__FILE__) + '/fixtures/helpers')
 $:.unshift(File.dirname(__FILE__) + '/fixtures/alternate_helpers')
 
-ENV['TMPDIR'] = File.join(File.dirname(__FILE__), 'tmp')
-
 require 'active_support/core_ext/kernel/reporting'
 
 # These are the normal settings that will be set up by Railties
@@ -466,7 +464,7 @@ class ForkingExecutor
   def initialize size
     @size  = size
     @queue = Server.new
-    file   = File.join Dir.tmpdir, Dir::Tmpname.make_tmpname('tests', 'fd')
+    file   = File.join Dir.tmpdir, Dir::Tmpname.make_tmpname('rails-tests', 'fd')
     @url   = "drbunix://#{file}"
     @pool  = nil
     DRb.start_service @url, @queue
@@ -514,3 +512,8 @@ if ActiveSupport::Testing::Isolation.forking_env? && PROCESS_COUNT > 0
   # Use N processes (N defaults to 4)
   Minitest.parallel_executor = ForkingExecutor.new(PROCESS_COUNT)
 end
+
+# FIXME: we have tests that depend on run order, we should fix that and
+# remove this method call.
+require 'active_support/test_case'
+ActiveSupport::TestCase.test_order = :sorted
diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb
index 187b9a2420..48342e252a 100644
--- a/actionpack/test/dispatch/response_test.rb
+++ b/actionpack/test/dispatch/response_test.rb
@@ -1,4 +1,6 @@
 require 'abstract_unit'
+require 'timeout'
+require 'rack/content_length'
 
 class ResponseTest < ActiveSupport::TestCase
   def setup
@@ -220,9 +222,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 +233,38 @@ 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 "compatibility with Rack::ContentLength" do
+    @response.body = 'Hello'
+    app = lambda { |env| @response.to_a }
+    env = Rack::MockRequest.env_for("/")
+
+    status, headers, body = app.call(env)
+    assert_nil headers['Content-Length']
+
+    status, headers, body = Rack::ContentLength.new(app).call(env)
+    assert_equal '5', headers['Content-Length']
+  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
diff --git a/actionpack/test/dispatch/routing_assertions_test.rb b/actionpack/test/dispatch/routing_assertions_test.rb
index aea4489852..56ea644f22 100644
--- a/actionpack/test/dispatch/routing_assertions_test.rb
+++ b/actionpack/test/dispatch/routing_assertions_test.rb
@@ -74,10 +74,26 @@ class RoutingAssertionsTest < ActionController::TestCase
     assert_recognizes({ :controller => 'query_articles', :action => 'index', :use_query => 'true' }, '/query/articles', { :use_query => 'true' })
   end
 
+  def test_assert_recognizes_raises_message
+    err = assert_raise(Assertion) do
+      assert_recognizes({ :controller => 'secure_articles', :action => 'index' }, 'http://test.host/secure/articles', {}, "This is a really bad msg")
+    end
+
+    assert_match err.message, "This is a really bad msg"
+  end
+
   def test_assert_routing
     assert_routing('/articles', :controller => 'articles', :action => 'index')
   end
 
+  def test_assert_routing_raises_message
+    err = assert_raise(Assertion) do
+      assert_routing('/thisIsNotARoute', { :controller => 'articles', :action => 'edit', :id => '1' }, { :id => '1' }, {}, "This is a really bad msg")
+    end
+
+    assert_match err.message, "This is a really bad msg"
+  end
+
   def test_assert_routing_with_defaults
     assert_routing('/articles/1/edit', { :controller => 'articles', :action => 'edit', :id => '1' }, { :id => '1' })
   end
diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb
index b8b51d86c2..ce9522d12a 100644
--- a/actionpack/test/lib/controller/fake_models.rb
+++ b/actionpack/test/lib/controller/fake_models.rb
@@ -28,30 +28,6 @@ class Customer < Struct.new(:name, :id)
   end
 end
 
-class ValidatedCustomer < Customer
-  def errors
-    if name =~ /Sikachu/i
-      []
-    else
-      [{:name => "is invalid"}]
-    end
-  end
-end
-
-module Quiz
-  class Question < Struct.new(:name, :id)
-    extend ActiveModel::Naming
-    include ActiveModel::Conversion
-
-    def persisted?
-      id.present?
-    end
-  end
-
-  class Store < Question
-  end
-end
-
 class Post < Struct.new(:title, :author_name, :body, :secret, :persisted, :written_on, :cost)
   extend ActiveModel::Naming
   include ActiveModel::Conversion
@@ -95,24 +71,3 @@ class Comment
 
   attr_accessor :body
 end
-
-module Blog
-  def self.use_relative_model_naming?
-    true
-  end
-
-  class Post < Struct.new(:title, :id)
-    extend ActiveModel::Naming
-    include ActiveModel::Conversion
-
-    def persisted?
-      id.present?
-    end
-  end
-end
-
-class RenderJsonTestException < Exception
-  def as_json(options = nil)
-    { :error => self.class.name, :message => self.to_s }
-  end
-end