Mailing-List: contact commits-help@couchdb.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@couchdb.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: kxepal@apache.org
To: commits@couchdb.apache.org
Message-Id: <2a00b49f002e4a6abd01cae31a5e3dcc@git.apache.org>
Subject: couch commit: updated refs/heads/master to c63796e
Date: Thu, 17 Sep 2015 10:26:44 +0000 (UTC)

Repository: couchdb-couch
Updated Branches:
  refs/heads/master b8b996832 -> c63796e4e


Fix duplicated Content-Type for show/update functions

When a show/update function returned data it added a
"Content-Type: application/json" header by itself. Then
couch_httpd added some default headers (including a yet
another "application/json" one).
This diff will add default chttpd headers only when
same headers are not yet present in the response.

This closes #55

COUCHDB-1876

Signed-off-by: Alexander Shorin <kxepal@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/couchdb-couch/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-couch/commit/c63796e4
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-couch/tree/c63796e4
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-couch/diff/c63796e4

Branch: refs/heads/master
Commit: c63796e4e5b7f649d461f6b72494dc9f9246a59e
Parents: b8b9968
Author: Artur Mazurek <artur.s.mazurek@gmail.com>
Authored: Mon May 25 15:19:53 2015 +0100
Committer: Alexander Shorin <kxepal@apache.org>
Committed: Thu Sep 17 13:24:54 2015 +0300

----------------------------------------------------------------------
 src/couch_httpd.erl | 103 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 93 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/couchdb-couch/blob/c63796e4/src/couch_httpd.erl
----------------------------------------------------------------------
diff --git a/src/couch_httpd.erl b/src/couch_httpd.erl
index eee1001..d1e9447 100644
--- a/src/couch_httpd.erl
+++ b/src/couch_httpd.erl
@@ -747,23 +747,17 @@ send_json(Req, Code, Value) ->
 
 send_json(Req, Code, Headers, Value) ->
     initialize_jsonp(Req),
-    DefaultHeaders = [
-        {"Content-Type", negotiate_content_type(Req)},
-        {"Cache-Control", "must-revalidate"}
-    ],
+    AllHeaders = maybe_add_default_headers(Req, Headers),
     Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
-    send_response(Req, Code, DefaultHeaders ++ Headers, Body).
+    send_response(Req, Code, AllHeaders, Body).
 
 start_json_response(Req, Code) ->
     start_json_response(Req, Code, []).
 
 start_json_response(Req, Code, Headers) ->
     initialize_jsonp(Req),
-    DefaultHeaders = [
-        {"Content-Type", negotiate_content_type(Req)},
-        {"Cache-Control", "must-revalidate"}
-    ],
-    {ok, Resp} = start_chunked_response(Req, Code, DefaultHeaders ++ Headers),
+    AllHeaders = maybe_add_default_headers(Req, Headers),
+    {ok, Resp} = start_chunked_response(Req, Code, AllHeaders),
     case start_jsonp() of
         [] -> ok;
         Start -> send_chunk(Resp, Start)
@@ -774,6 +768,19 @@ end_json_response(Resp) ->
     send_chunk(Resp, end_jsonp() ++ [$\n]),
     last_chunk(Resp).
 
+maybe_add_default_headers(ForRequest, ToHeaders) ->
+    DefaultHeaders = [
+        {"Content-Type", negotiate_content_type(ForRequest)},
+        {"Cache-Control", "must-revalidate"}
+    ],
+    lists:foldl(fun maybe_add_header/2, ToHeaders, DefaultHeaders).
+
+maybe_add_header({HeaderName, HeaderValue}, ToHeaders) ->
+    case lists:keyfind(HeaderName, 1, ToHeaders) of
+        false -> ToHeaders ++ [{HeaderName, HeaderValue}];
+        _Found -> ToHeaders
+    end.
+
 initialize_jsonp(Req) ->
     case get(jsonp) of
         undefined -> put(jsonp, qs_value(Req, "callback", no_jsonp));
@@ -1120,3 +1127,79 @@ validate_bind_address(Address) ->
         {ok, _} -> ok;
         _ -> throw({error, invalid_bind_address})
     end.
+
+
+%%%%%%%% module tests below %%%%%%%%
+
+-ifdef(TEST).
+-include_lib("couch/include/couch_eunit.hrl").
+
+maybe_add_header_test_() ->
+    Cases = [
+        {[],                    % initial headers
+         {"K1", "V1"},          % header to add
+          [{"K1", "V1"}],       % expected result
+           "Adding to empty headers"},
+
+        {[{"K1", "V1"}],
+         {"K2", "V2"},
+          [{"K1", "V1"}, {"K2", "V2"}],
+           "Adding header to 1 element headers list"},
+
+        {[{"K1", "V1"}],
+         {"K1", "V2"},
+          [{"K1", "V1"}],
+           "Trying to add same header to 1 element headers list"},
+
+        {[{"K1", "V1"}, {"K2", "V2"}],
+         {"K1", "V2"},
+          [{"K1", "V1"}, {"K2", "V2"}],
+           "Trying to add same header to 2 element headers list"},
+
+        {[{"K1", "V1"}, {"K2", "V2"}],
+         {"K3", "V3"},
+          [{"K1", "V1"}, {"K2", "V2"}, {"K3", "V3"}],
+           "Adding header to 2 elements headers list"}
+    ],
+    Tests = lists:map(fun({InitialHeaders, HeaderToAdd, ProperResult, Desc}) ->
+        {Desc,
+        ?_assertEqual(ProperResult,
+            maybe_add_header(HeaderToAdd, InitialHeaders))}
+    end, Cases),
+    {"Tests adding a header to a list of headers", Tests}.
+
+maybe_add_default_headers_test_() ->
+    DummyRequest = [],
+    NoCache = {"Cache-Control", "no-cache"},
+    ApplicationJson = {"Content-Type", "application/json"},
+    % couch_httpd uses process dictionary to check if currently in a
+    % json serving method. Defaults to 'application/javascript' otherwise.
+    % Therefore must-revalidate and application/javascript should be added
+    % by chttpd if such headers are not present
+    MustRevalidate = {"Cache-Control", "must-revalidate"},
+    ApplicationJavascript = {"Content-Type", "application/javascript"},
+    Cases = [
+        {[],
+         [ApplicationJavascript, MustRevalidate],
+          "Should add Content-Type and Cache-Control to empty heaeders"},
+
+        {[NoCache],
+         [NoCache, ApplicationJavascript],
+          "Should add Content-Type only if Cache-Control is present"},
+
+        {[ApplicationJson],
+         [ApplicationJson, MustRevalidate],
+          "Should add Cache-Control if Content-Type is present"},
+
+        {[NoCache, ApplicationJson],
+         [NoCache, ApplicationJson],
+          "Should not add headers if Cache-Control and Content-Type are there"}
+    ],
+    Tests = lists:map(fun({InitialHeaders, ProperResult, Desc}) ->
+        {Desc,
+        ?_assertEqual(ProperResult,
+         maybe_add_default_headers(DummyRequest, InitialHeaders))}
+    end, Cases),
+    {"Tests adding default headers", Tests}.
+
+-endif.