Merge pull request rails#21838 from jeremy/response-add-header

Response#add_header for adding to multi-valued headers like Vary

Author: jeremy

actionpack/lib/action_dispatch/http/response.rb

@@ -161,6 +161,26 @@ 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/response_test.rb

@@ -293,6 +293,65 @@ def test_only_set_charset_still_defaults_to_text_html
   end
 end
 
+class ResponseHeadersTest < ActiveSupport::TestCase
+  def setup
+    @response = ActionDispatch::Response.create
+    @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
+  end
+
+  test 'get_header' do
+    assert_equal '1', @response.get_header('Foo')
+    assert_nil @response.get_header('foo')
+    assert_nil @response.get_header(nil)
+  end
+
+  test 'set_header' do
+    assert_equal '2', @response.set_header('Foo', '2')
+    assert @response.have_header?('Foo')
+    assert_equal '2', @response.get_header('Foo')
+
+    assert_nil @response.set_header('Foo', nil)
+    assert @response.have_header?('Foo')
+    assert_nil @response.get_header('Foo')
+  end
+
+  test 'delete_header' do
+    assert_nil @response.delete_header(nil)
+
+    assert_nil @response.delete_header('foo')
+    assert @response.have_header?('Foo')
+
+    assert_equal '1', @response.delete_header('Foo')
+    assert_not @response.have_header?('Foo')
+  end
+
+  test 'add_header' do
+    # Add a value to an existing header
+    assert_equal '1,2', @response.add_header('Foo', '2')
+    assert_equal '1,2', @response.get_header('Foo')
+
+    # Add nil to an existing header
+    assert_equal '1,2', @response.add_header('Foo', nil)
+    assert_equal '1,2', @response.get_header('Foo')
+
+    # Add nil to a nonexistent header
+    assert_nil @response.add_header('Bar', nil)
+    assert_not @response.have_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_equal '1', @response.get_header('Bar')
+  end
+end
+
 class ResponseIntegrationTest < ActionDispatch::IntegrationTest
   test "response cache control from railsish app" do
     @app = lambda { |env|