Merge pull request #21861 from jeremy/rack-add-header

Introduce `Headers#add`. Move `Response#add_header` upstream.

Gemfile.lock

@@ -29,7 +29,7 @@ GIT
 
 GIT
   remote: git://github.com/rack/rack.git
-  revision: c28f271d0c91f45e13bfa8f07bed445ef91f41de
+  revision: c617ea99c12a5bfe026e00476ff37e714e01891a
   branch: master
   specs:
     rack (2.0.0.alpha)

actionpack/lib/action_dispatch/http/cache.rb

@@ -59,7 +59,7 @@ module ActionDispatch
         end
 
         def last_modified?
-          have_header? LAST_MODIFIED
+          has_header? LAST_MODIFIED
         end
 
         def last_modified=(utc_time)
@@ -73,7 +73,7 @@ module ActionDispatch
         end
 
         def date?
-          have_header? DATE
+          has_header? DATE
         end
 
         def date=(utc_time)

actionpack/lib/action_dispatch/http/headers.rb

@@ -49,6 +49,11 @@ module ActionDispatch
         @req.set_header env_name(key), value
       end
 
+      # Add a value to a multivalued header like Vary or Accept-Encoding.
+      def add(key, value)
+        @req.add_header env_name(key), value
+      end
+
       def key?(key)
         @req.has_header? env_name(key)
       end

actionpack/lib/action_dispatch/http/response.rb

@@ -156,31 +156,11 @@ module ActionDispatch # :nodoc:
       yield self if block_given?
     end
 
-    def have_header?(key);  headers.key? key;   end
+    def has_header?(key);   headers.key? key;   end
     def get_header(key);    headers[key];       end
     def set_header(key, v); headers[key] = v;   end
     def delete_header(key); headers.delete key; end
 
-    # Add a header that may have multiple values.
-    #
-    # Example:
-    #   response.add_header 'Vary', 'Accept'
-    #   response.add_header 'Vary', 'Accept-Encoding'
-    #   response.add_header 'Vary', 'Cookie'
-    #
-    #   assert_equal 'Accept,Accept-Encoding,Cookie', response.get_header 'Vary'
-    #
-    # http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
-    def add_header(key, v)
-      if v.nil?
-        get_header key
-      elsif have_header? key
-        set_header key, "#{get_header key},#{v}"
-      else
-        set_header key, v
-      end
-    end
-
     def await_commit
       synchronize do
         @cv.wait_until { @committed }

actionpack/test/dispatch/header_test.rb

@@ -42,6 +42,24 @@ class HeaderTest < ActiveSupport::TestCase
     assert_equal "127.0.0.1", @headers["HTTP_HOST"]
   end
 
+  test "add to multivalued headers" do
+    # Sets header when not present
+    @headers.add 'Foo', '1'
+    assert_equal '1', @headers['Foo']
+
+    # Ignores nil values
+    @headers.add 'Foo', nil
+    assert_equal '1', @headers['Foo']
+
+    # Converts value to string
+    @headers.add 'Foo', 1
+    assert_equal '1,1', @headers['Foo']
+
+    # Case-insensitive
+    @headers.add 'fOo', 2
+    assert_equal '1,1,2', @headers['foO']
+  end
+
   test "headers can contain numbers" do
     @headers["Content-MD5"] = "Q2hlY2sgSW50ZWdyaXR5IQ=="
 

actionpack/test/dispatch/response_test.rb

@@ -299,10 +299,10 @@ class ResponseHeadersTest < ActiveSupport::TestCase
     @response.set_header 'Foo', '1'
   end
 
-  test 'have_header?' do
-    assert @response.have_header? 'Foo'
-    assert_not @response.have_header? 'foo'
-    assert_not @response.have_header? nil
+  test 'has_header?' do
+    assert @response.has_header? 'Foo'
+    assert_not @response.has_header? 'foo'
+    assert_not @response.has_header? nil
   end
 
   test 'get_header' do
@@ -313,11 +313,11 @@ class ResponseHeadersTest < ActiveSupport::TestCase
 
   test 'set_header' do
     assert_equal '2', @response.set_header('Foo', '2')
-    assert @response.have_header?('Foo')
+    assert @response.has_header?('Foo')
     assert_equal '2', @response.get_header('Foo')
 
     assert_nil @response.set_header('Foo', nil)
-    assert @response.have_header?('Foo')
+    assert @response.has_header?('Foo')
     assert_nil @response.get_header('Foo')
   end
 
@@ -325,10 +325,10 @@ class ResponseHeadersTest < ActiveSupport::TestCase
     assert_nil @response.delete_header(nil)
 
     assert_nil @response.delete_header('foo')
-    assert @response.have_header?('Foo')
+    assert @response.has_header?('Foo')
 
     assert_equal '1', @response.delete_header('Foo')
-    assert_not @response.have_header?('Foo')
+    assert_not @response.has_header?('Foo')
   end
 
   test 'add_header' do
@@ -342,12 +342,12 @@ class ResponseHeadersTest < ActiveSupport::TestCase
 
     # Add nil to a nonexistent header
     assert_nil @response.add_header('Bar', nil)
-    assert_not @response.have_header?('Bar')
+    assert_not @response.has_header?('Bar')
     assert_nil @response.get_header('Bar')
 
     # Add a value to a nonexistent header
     assert_equal '1', @response.add_header('Bar', '1')
-    assert @response.have_header?('Bar')
+    assert @response.has_header?('Bar')
     assert_equal '1', @response.get_header('Bar')
   end
 end