Merge branch 'master' of github.com:rails/rails

* 'master' of github.com:rails/rails: (44 commits)
  Fixed indentation in actionmailer base_test [#6538 state:committed]
  remove unused assigned variable
  removes merge conflicts
  removes Examples headers introduced in 9b96de6
  Revert "Fixed identation in actionmailer base_test"
  Report the correct value of nil.id in the exception message as different ruby implementations may have different values, for example Rubinius returns 53 for nil.id.
  Improve testing of cookies in functional tests: - cookies can be set using string or symbol keys - cookies are preserved across calls to get, post, etc. - cookie names and values are escaped - cookies can be cleared using @request.cookies.clear
  more style changes
  Some style changes
  style changes
  Revert "style changes"
  Raise ArgumentError if route name is invalid [#6517 state:resolved]
  style changes
  Allow model to be inherited from Hash [#6487 state:resolved]
  styles applied for usage
  added failing test for fields_for with a record object that inherits from Hash
  Fixed identation in actionmailer base_test
  wrong SQL statement
  commas to set off expressions that interrupt sentence flow
  typo changes
  ...

actionmailer/lib/rails/generators/mailer/USAGE

@@ -1,4 +1,5 @@
 Description:
+============
     Stubs out a new mailer and its views. Pass the mailer name, either
     CamelCased or under_scored, and an optional list of emails as arguments.
 
@@ -6,10 +7,12 @@ Description:
     engine and test framework generators.
 
 Example:
-    `rails generate mailer Notifications signup forgot_password invoice`
+========
+    rails generate mailer Notifications signup forgot_password invoice
 
     creates a Notifications mailer class, views, test, and fixtures:
         Mailer:     app/mailers/notifications.rb
         Views:      app/views/notifications/signup.erb [...]
         Test:       test/functional/notifications_test.rb
         Fixtures:   test/fixtures/notifications/signup [...]
+

actionmailer/test/base_test.rb

@@ -153,8 +153,8 @@ class BaseTest < ActiveSupport::TestCase
     assert_equal(2, email.parts.length)
     assert_equal("multipart/related", email.mime_type)
     assert_equal("multipart/alternative", email.parts[0].mime_type)
-      assert_equal("text/plain", email.parts[0].parts[0].mime_type)
-      assert_equal("text/html",  email.parts[0].parts[1].mime_type)
+    assert_equal("text/plain", email.parts[0].parts[0].mime_type)
+    assert_equal("text/html",  email.parts[0].parts[1].mime_type)
     assert_equal("logo.png", email.parts[1].filename)
   end
 

actionpack/lib/action_controller/test_case.rb

@@ -172,6 +172,10 @@ module ActionController
     end
 
     def recycle!
+      write_cookies!
+      @env.delete('HTTP_COOKIE') if @cookies.blank?
+      @env.delete('action_dispatch.cookies')
+      @cookies = nil
       @formats = nil
       @env.delete_if { |k, v| k =~ /^(action_dispatch|rack)\.request/ }
       @env.delete_if { |k, v| k =~ /^action_dispatch\.rescue/ }
@@ -301,7 +305,11 @@ module ActionController
   # and cookies, though. For sessions, you just do:
   #
   #   @request.session[:key] = "value"
-  #   @request.cookies["key"] = "value"
+  #   @request.cookies[:key] = "value"
+  #
+  # To clear the cookies for a test just clear the request's cookies hash:
+  #
+  #   @request.cookies.clear
   #
   # == \Testing named routes
   #
@@ -416,6 +424,7 @@ module ActionController
         @controller.process_with_new_base_test(@request, @response)
         @assigns = @controller.respond_to?(:view_assigns) ? @controller.view_assigns : {}
         @request.session.delete('flash') if @request.session['flash'].blank?
+        @request.cookies.merge!(@response.cookies)
         @response
       end
 

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -1,5 +1,6 @@
 require 'rack/mount'
 require 'forwardable'
+require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/object/to_query'
 require 'active_support/core_ext/hash/slice'
 
@@ -330,6 +331,7 @@ module ActionDispatch
       end
 
       def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil, anchor = true)
+        raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i)
         route = Route.new(self, app, conditions, requirements, defaults, name, anchor)
         @set.add_route(*route)
         named_routes[name] = route if name

actionpack/lib/action_dispatch/testing/test_process.rb

@@ -22,7 +22,7 @@ module ActionDispatch
     end
 
     def cookies
-      HashWithIndifferentAccess.new(@request.cookies.merge(@response.cookies))
+      @request.cookies.merge(@response.cookies).with_indifferent_access
     end
 
     def redirect_to_url

actionpack/lib/action_dispatch/testing/test_request.rb

@@ -1,5 +1,6 @@
 require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/hash/reverse_merge'
+require 'rack/utils'
 
 module ActionDispatch
   class TestRequest < Request
@@ -77,10 +78,14 @@ module ActionDispatch
     private
       def write_cookies!
         unless @cookies.blank?
-          @env['HTTP_COOKIE'] = @cookies.map { |name, value| "#{name}=#{value};" }.join(' ')
+          @env['HTTP_COOKIE'] = @cookies.map { |name, value| escape_cookie(name, value) }.join('; ')
         end
       end
 
+      def escape_cookie(name, value)
+        "#{Rack::Utils.escape(name)}=#{Rack::Utils.escape(value)}"
+      end
+
       def delete_nil_values!
         @env.delete_if { |k, v| v.nil? }
       end

actionpack/lib/action_view/helpers/form_helper.rb

@@ -6,6 +6,7 @@ require 'active_support/core_ext/class/attribute'
 require 'active_support/core_ext/hash/slice'
 require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/string/output_safety'
+require 'active_support/core_ext/array/extract_options'
 
 module ActionView
   # = Action View Form Helpers
@@ -880,9 +881,9 @@ module ActionView
 
       private
 
-        def instantiate_builder(record, record_object = nil, options = nil, &block)
-          options, record_object = record_object, nil if record_object.is_a?(Hash)
-          options ||= {}
+        def instantiate_builder(record, *args, &block)
+          options = args.extract_options!
+          record_object = args.shift
 
           case record
           when String, Symbol

actionpack/test/dispatch/cookies_test.rb

@@ -124,6 +124,20 @@ class CookiesTest < ActionController::TestCase
       cookies['user_name'] = "david"
       head :ok
     end
+
+    def symbol_key_mock
+      cookies[:user_name] = "david" if cookies[:user_name] == "andrew"
+      head :ok
+    end
+
+    def string_key_mock
+      cookies['user_name'] = "david" if cookies['user_name'] == "andrew"
+      head :ok
+    end
+
+    def noop
+      head :ok
+    end
   end
 
   tests TestController
@@ -411,6 +425,57 @@ class CookiesTest < ActionController::TestCase
     end
   end
 
+  def test_setting_request_cookies_is_indifferent_access
+    @request.cookies.clear
+    @request.cookies[:user_name] = "andrew"
+    get :string_key_mock
+    assert_equal "david", cookies[:user_name]
+
+    @request.cookies.clear
+    @request.cookies['user_name'] = "andrew"
+    get :symbol_key_mock
+    assert_equal "david", cookies['user_name']
+  end
+
+  def test_cookies_retained_across_requests
+    get :symbol_key
+    assert_equal "user_name=david; path=/", @response.headers["Set-Cookie"]
+    assert_equal "david", cookies[:user_name]
+
+    get :noop
+    assert_nil @response.headers["Set-Cookie"]
+    assert_equal "user_name=david", @request.env['HTTP_COOKIE']
+    assert_equal "david", cookies[:user_name]
+
+    get :noop
+    assert_nil @response.headers["Set-Cookie"]
+    assert_equal "user_name=david", @request.env['HTTP_COOKIE']
+    assert_equal "david", cookies[:user_name]
+  end
+
+  def test_cookies_can_be_cleared
+    get :symbol_key
+    assert_equal "user_name=david; path=/", @response.headers["Set-Cookie"]
+    assert_equal "david", cookies[:user_name]
+
+    @request.cookies.clear
+    get :noop
+    assert_nil @response.headers["Set-Cookie"]
+    assert_nil @request.env['HTTP_COOKIE']
+    assert_nil cookies[:user_name]
+
+    get :symbol_key
+    assert_equal "user_name=david; path=/", @response.headers["Set-Cookie"]
+    assert_equal "david", cookies[:user_name]
+  end
+
+  def test_cookies_are_escaped
+    @request.cookies[:user_ids] = '1;2'
+    get :noop
+    assert_equal "user_ids=1%3B2", @request.env['HTTP_COOKIE']
+    assert_equal "1;2", cookies[:user_ids]
+  end
+
   private
     def assert_cookie_header(expected)
       header = @response.headers["Set-Cookie"]

actionpack/test/dispatch/routing_test.rb

@@ -2313,6 +2313,38 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_invalid_route_name_raises_error
+    assert_raise(ArgumentError) do
+      self.class.stub_controllers do |routes|
+        routes.draw { get '/products', :to => 'products#index', :as => 'products ' }
+      end
+    end
+
+    assert_raise(ArgumentError) do
+      self.class.stub_controllers do |routes|
+        routes.draw { get '/products', :to => 'products#index', :as => ' products' }
+      end
+    end
+
+    assert_raise(ArgumentError) do
+      self.class.stub_controllers do |routes|
+        routes.draw { get '/products', :to => 'products#index', :as => 'products!' }
+      end
+    end
+
+    assert_raise(ArgumentError) do
+      self.class.stub_controllers do |routes|
+        routes.draw { get '/products', :to => 'products#index', :as => 'products index' }
+      end
+    end
+
+    assert_raise(ArgumentError) do
+      self.class.stub_controllers do |routes|
+        routes.draw { get '/products', :to => 'products#index', :as => '1products' }
+      end
+    end
+  end
+
   def test_nested_route_in_nested_resource
     get "/posts/1/comments/2/views"
     assert_equal "comments#views", @response.body

actionpack/test/dispatch/test_request_test.rb

@@ -36,10 +36,10 @@ class TestRequestTest < ActiveSupport::TestCase
 
     req.cookies["user_name"] = "david"
     assert_equal({"user_name" => "david"}, req.cookies)
-    assert_equal "user_name=david;", req.env["HTTP_COOKIE"]
+    assert_equal "user_name=david", req.env["HTTP_COOKIE"]
 
     req.cookies["login"] = "XJ-122"
     assert_equal({"user_name" => "david", "login" => "XJ-122"}, req.cookies)
-    assert_equal %w(login=XJ-122 user_name=david), req.env["HTTP_COOKIE"].split(/; ?/).sort
+    assert_equal %w(login=XJ-122 user_name=david), req.env["HTTP_COOKIE"].split(/; /).sort
   end
 end

actionpack/test/template/form_options_helper_test.rb

@@ -1,6 +1,12 @@
 require 'abstract_unit'
 require 'tzinfo'
 
+class Map < Hash
+  def category
+    "<mus>"
+  end
+end
+
 TZInfo::Timezone.cattr_reader :loaded_zones
 
 class FormOptionsHelperTest < ActionView::TestCase
@@ -394,6 +400,19 @@ class FormOptionsHelperTest < ActionView::TestCase
     )
   end
 
+  def test_fields_for_with_record_inherited_from_hash
+    map = Map.new
+
+    output_buffer = fields_for :map, map do |f|
+      concat f.select(:category, %w( abe <mus> hest))
+    end
+
+    assert_dom_equal(
+      "<select id=\"map_category\" name=\"map[category]\"><option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
+      output_buffer
+    )
+  end
+
   def test_select_under_fields_for_with_index
     @post = Post.new
     @post.category = "<mus>"

activemodel/lib/active_model/observing.rb

@@ -72,7 +72,7 @@ module ActiveModel
         def instantiate_observer(observer) #:nodoc:
           # string/symbol
           if observer.respond_to?(:to_sym)
-            observer = observer.to_s.camelize.constantize.instance
+            observer.to_s.camelize.constantize.instance
           elsif observer.respond_to?(:instance)
             observer.instance
           else

activesupport/lib/active_support/backtrace_cleaner.rb

@@ -8,7 +8,7 @@ module ActiveSupport
   # filter or modify the paths of any lines of the backtrace, you can call BacktraceCleaner#remove_filters! These two methods
   # will give you a completely untouched backtrace.
   #
-  # Example:
+  # ==== Example:
   #
   #   bc = BacktraceCleaner.new
   #   bc.add_filter   { |line| line.gsub(Rails.root, '') }

activesupport/lib/active_support/configurable.rb

@@ -64,21 +64,21 @@ module ActiveSupport
     end
 
     # Reads and writes attributes from a configuration <tt>OrderedHash</tt>.
-    # 
-    #   require 'active_support/configurable'      
-    #  
+    #
+    #   require 'active_support/configurable'
+    #
     #   class User
     #     include ActiveSupport::Configurable
-    #   end 
+    #   end
     #
     #   user = User.new
-    # 
+    #
     #   user.config.allowed_access = true
     #   user.config.level = 1
     #
     #   user.config.allowed_access # => true
     #   user.config.level          # => 1
-    # 
+    #
     def config
       @_config ||= self.class.config.inheritable_copy
     end

activesupport/lib/active_support/hash_with_indifferent_access.rb

@@ -1,7 +1,7 @@
 require 'active_support/core_ext/hash/keys'
 
 # This class has dubious semantics and we only have it so that
-# people can write params[:key] instead of params['key']
+# people can write <tt>params[:key]</tt> instead of <tt>params['key']</tt>
 # and they get the same value for both keys.
 
 module ActiveSupport
@@ -109,7 +109,7 @@ module ActiveSupport
     end
 
     # Performs the opposite of merge, with the keys and values from the first hash taking precedence over the second.
-    # This overloaded definition prevents returning a regular hash, if reverse_merge is called on a HashWithDifferentAccess.
+    # This overloaded definition prevents returning a regular hash, if reverse_merge is called on a <tt>HashWithDifferentAccess</tt>.
     def reverse_merge(other_hash)
       super self.class.new_from_hash_copying_default(other_hash)
     end

activesupport/lib/active_support/i18n_railtie.rb

@@ -14,7 +14,7 @@ module I18n
       @reloader ||= ActiveSupport::FileUpdateChecker.new([]){ I18n.reload! }
     end
 
-    # Add I18n::Railtie.reloader to ActionDispatch callbacks. Since, at this
+    # Add <tt>I18n::Railtie.reloader</tt> to ActionDispatch callbacks. Since, at this
     # point, no path was added to the reloader, I18n.reload! is not triggered
     # on to_prepare callbacks. This will only happen on the config.after_initialize
     # callback below.

activesupport/lib/active_support/log_subscriber.rb

@@ -21,7 +21,7 @@ module ActiveSupport
   #   ActiveRecord::LogSubscriber.attach_to :active_record
   #
   # Since we need to know all instance methods before attaching the log subscriber,
-  # the line above should be called after your ActiveRecord::LogSubscriber definition.
+  # the line above should be called after your <tt>ActiveRecord::LogSubscriber</tt> definition.
   #
   # After configured, whenever a "sql.active_record" notification is published,
   # it will properly dispatch the event (ActiveSupport::Notifications::Event) to

activesupport/lib/active_support/message_encryptor.rb

@@ -7,7 +7,7 @@ module ActiveSupport
   #
   # The cipher text and initialization vector are base64 encoded and returned to you.
   #
-  # This can be used in situations similar to the MessageVerifier, but where you don't
+  # This can be used in situations similar to the <tt>MessageVerifier</tt>, but where you don't
   # want users to be able to determine the value of the payload.
   class MessageEncryptor
     class InvalidMessage < StandardError; end

activesupport/lib/active_support/message_verifier.rb

@@ -2,7 +2,7 @@ require 'active_support/base64'
 require 'active_support/core_ext/object/blank'
 
 module ActiveSupport
-  # MessageVerifier makes it easy to generate and verify messages which are signed
+  # +MessageVerifier+ makes it easy to generate and verify messages which are signed
   # to prevent tampering.
   #
   # This is useful for cases like remember-me tokens and auto-unsubscribe links where the

activesupport/lib/active_support/whiny_nil.rb

@@ -37,7 +37,7 @@ class NilClass
 
   # Raises a RuntimeError when you attempt to call +id+ on +nil+.
   def id
-    raise RuntimeError, "Called id for nil, which would mistakenly be 4 -- if you really wanted the id of nil, use object_id", caller
+    raise RuntimeError, "Called id for nil, which would mistakenly be #{object_id} -- if you really wanted the id of nil, use object_id", caller
   end
 
   private

activesupport/test/whiny_nil_test.rb

@@ -33,9 +33,11 @@ class WhinyNilTest < Test::Unit::TestCase
   end
 
   def test_id
+    nil.stubs(:object_id).returns(999)
     nil.id
   rescue RuntimeError => nme
     assert_no_match(/nil:NilClass/, nme.message)
+    assert_match(/999/, nme.message)
   end
 
   def test_no_to_ary_coercion

railties/guides/source/active_record_querying.textile

@@ -420,7 +420,7 @@ Client.limit(5).offset(30)
 will return instead a maximum of 5 clients beginning with the 31st. The SQL looks like:
 
 <sql>
-SELECT * FROM clients LIMIT 5, 30
+SELECT * FROM clients LIMIT 5 OFFSET 30
 </sql>
 
 h3. Group

railties/guides/source/contributing_to_ruby_on_rails.textile

@@ -364,6 +364,20 @@ Please make sure the patch does not introduce whitespace errors:
 $ git apply --whitespace=error-all mynew_patch.diff
 </shell>
 
+You can check your patches by applying your patch to an different dedicated branch:
+
+<shell>
+$ git checkout -b testing_branch
+$ git apply --check my_new_patch.diff
+</shell>
+
+You can make sure your patches don't add any whitespace by applying it yourself using the --whitespace=error-all option. Make sure you are on your dedicated test branche and:
+
+<shell>
+$ git apply --whitespace=error-all mynew_patch.diff
+</shell>
+
+
 h4. Create a Lighthouse Ticket
 
 Now create a ticket with your patch. Go to the "new ticket":http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/new page at Lighthouse. Fill in a reasonable title and description, remember to attach your patch file, and tag the ticket with the ‘patch’ tag and whatever other subject area tags make sense.