Browse Source

Change the order of set_resp_cookie arguments

The Opts value is put last, to be more consistent with the
rest of the cowboy_req module.

Additionally a test handler was fixed which reduced the number
of errors in http_SUITE.
Loïc Hoguin 8 years ago
parent
commit
91ae70b06c

+ 12 - 15
doc/src/guide/cookies.asciidoc

@@ -33,24 +33,21 @@ update the expiration time and avoid losing a cookie.
 
 === Setting cookies
 
-// @todo So I am not particularly happy about set_resp_cookie/4
-// having Opts as a *third* argument, instead of the *last* like
-// all other functions that come with an Opts argument. We will
-// probably need to change this before 2.0.
-
 By default cookies are defined for the duration of the session:
 
 [source,erlang]
+----
 SessionID = generate_session_id(),
 Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, Req0).
+----
 
 They can also be set for a duration in seconds:
 
 [source,erlang]
 ----
 SessionID = generate_session_id(),
-Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID,
-    #{max_age => 3600}, Req0).
+Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, Req0,
+    #{max_age => 3600}).
 ----
 
 To delete cookies, set `max_age` to 0:
@@ -58,8 +55,8 @@ To delete cookies, set `max_age` to 0:
 [source,erlang]
 ----
 SessionID = generate_session_id(),
-Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID,
-    #{max_age => 0}, Req0).
+Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, Req0,
+    #{max_age => 0}).
 ----
 
 To restrict cookies to a specific domain and path, the options
@@ -67,8 +64,8 @@ of the same name can be used:
 
 [source,erlang]
 ----
-Req = cowboy_req:set_resp_cookie(<<"inaccount">>, <<"1">>,
-    #{domain => "my.example.org", path => "/account"}, Req0).
+Req = cowboy_req:set_resp_cookie(<<"inaccount">>, <<"1">>, Req0,
+    #{domain => "my.example.org", path => "/account"}).
 ----
 
 Cookies will be sent with requests to this domain and all
@@ -81,8 +78,8 @@ available over HTTPS):
 [source,erlang]
 ----
 SessionID = generate_session_id(),
-Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID,
-    #{secure => true}, Req0).
+Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, Req0,
+    #{secure => true}).
 ----
 
 To prevent client-side scripts from accessing a cookie:
@@ -90,8 +87,8 @@ To prevent client-side scripts from accessing a cookie:
 [source,erlang]
 ----
 SessionID = generate_session_id(),
-Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID,
-    #{http_only => true}, Req0).
+Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, Req0,
+    #{http_only => true}).
 ----
 
 Cookies may also be set client-side, for example using

+ 15 - 21
doc/src/manual/cowboy_req.set_resp_cookie.asciidoc

@@ -11,7 +11,7 @@ cowboy_req:set_resp_cookie - Set a cookie
 set_resp_cookie(Name, Value, Req :: cowboy_req:req())
     -> set_resp_cookie(Name, Value, [], Req)
 
-set_resp_cookie(Name, Value, Opts, Req :: cowboy_req:req())
+set_resp_cookie(Name, Value, Req :: cowboy_req:req(), Opts)
     -> Req
 
 Name  :: binary()                  %% case sensitive
@@ -33,14 +33,14 @@ Value::
 
 Cookie value.
 
-Opts::
-
-Optional cookie options.
-
 Req::
 
 The Req object.
 
+Opts::
+
+Cookie options.
+
 == Return value
 
 A new Req object is returned.
@@ -66,44 +66,38 @@ Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, Req0).
 .Set a cookie with an expiration time
 [source,erlang]
 ----
-Req = cowboy_req:set_resp_cookie(<<"lang">>, <<"fr-FR">>, [
-    {max_age, 3600}
-], Req0).
+Req = cowboy_req:set_resp_cookie(<<"lang">>, <<"fr-FR">>,
+    Req0, #{max_age => 3600}).
 ----
 
 .Delete a cookie
 [source,erlang]
 ----
-Req = cowboy_req:set_resp_cookie(<<"sessionid">>, <<>>, [
-    {max_age, 0}
-], Req0).
+Req = cowboy_req:set_resp_cookie(<<"sessionid">>, <<>>,
+    Req0, #{max_age => 0}).
 ----
 
 .Set a cookie for a specific domain and path
 [source,erlang]
 ----
-Req = cowboy_req:set_resp_cookie(<<"inaccount">>, <<"1">>, [
-    {domain, "my.example.org"},
-    {path, "/account"}
-], Req0).
+Req = cowboy_req:set_resp_cookie(<<"inaccount">>, <<"1">>,
+    Req0, #{domain => "my.example.org", path => "/account"}).
 ----
 
 .Restrict a cookie to HTTPS
 [source,erlang]
 ----
 SessionID = base64:encode(crypto:strong_rand_bytes(32)),
-Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, [
-    {secure, true}
-], Req0).
+Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID,
+    Req0, #{secure => true}).
 ----
 
 .Restrict a cookie to HTTP
 [source,erlang]
 ----
 SessionID = base64:encode(crypto:strong_rand_bytes(32)),
-Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID, [
-    {http_only, true}
-], Req0).
+Req = cowboy_req:set_resp_cookie(<<"sessionid">>, SessionID,
+    Req0, #{http_only => true}).
 ----
 
 == See also

+ 1 - 1
examples/cookie/src/toppage_handler.erl

@@ -8,7 +8,7 @@
 init(Req0, Opts) ->
 	NewValue = integer_to_list(rand:uniform(1000000)),
 	Req1 = cowboy_req:set_resp_cookie(<<"server">>, NewValue,
-		#{path => <<"/">>}, Req0),
+		Req0, #{path => <<"/">>}),
 	#{client := ClientCookie, server := ServerCookie}
 		= cowboy_req:match_cookies([{client, [], <<>>}, {server, [], <<>>}], Req1),
 	{ok, Body} = toppage_dtl:render([

+ 3 - 3
src/cowboy_req.erl

@@ -548,7 +548,7 @@ stream_multipart(Req=#{multipart := {Boundary, Buffer}}, _) ->
 -spec set_resp_cookie(iodata(), iodata(), Req)
 	-> Req when Req::req().
 set_resp_cookie(Name, Value, Req) ->
-	set_resp_cookie(Name, Value, #{}, Req).
+	set_resp_cookie(Name, Value, Req, #{}).
 
 %% The cookie name cannot contain any of the following characters:
 %%   =,;\s\t\r\n\013\014
@@ -556,9 +556,9 @@ set_resp_cookie(Name, Value, Req) ->
 %% The cookie value cannot contain any of the following characters:
 %%   ,; \t\r\n\013\014
 %% @todo Fix the cookie_opts() type.
--spec set_resp_cookie(binary(), iodata(), cookie_opts(), Req)
+-spec set_resp_cookie(binary(), iodata(), Req, cookie_opts())
 	-> Req when Req::req().
-set_resp_cookie(Name, Value, Opts, Req) ->
+set_resp_cookie(Name, Value, Req, Opts) ->
 	Cookie = cow_cookie:setcookie(Name, Value, maps:to_list(Opts)),
 	RespCookies = maps:get(resp_cookies, Req, #{}),
 	Req#{resp_cookies => RespCookies#{Name => Cookie}}.

+ 2 - 1
test/handlers/resp_h.erl

@@ -20,7 +20,8 @@ do(<<"set_resp_cookie3">>, Req0, Opts) ->
 	end,
 	{ok, cowboy_req:reply(200, #{}, "OK", Req), Opts};
 do(<<"set_resp_cookie4">>, Req0, Opts) ->
-	Req = cowboy_req:set_resp_cookie(<<"mycookie">>, "myvalue", #{path => cowboy_req:path(Req0)}, Req0),
+	Req = cowboy_req:set_resp_cookie(<<"mycookie">>, "myvalue", Req0,
+		#{path => cowboy_req:path(Req0)}),
 	{ok, cowboy_req:reply(200, #{}, "OK", Req), Opts};
 do(<<"set_resp_header">>, Req0, Opts) ->
 	Req = cowboy_req:set_resp_header(<<"content-type">>, <<"text/plain">>, Req0),

+ 1 - 1
test/http_SUITE_data/http_set_resp.erl

@@ -12,7 +12,7 @@ init(Req, Opts) ->
 	end, Req, maps:to_list(Headers)),
 	Req3 = cowboy_req:set_resp_body(Body, Req2),
 	Req4 = cowboy_req:set_resp_header(<<"x-cowboy-test">>, <<"ok">>, Req3),
-	Req5 = cowboy_req:set_resp_cookie(<<"cake">>, <<"lie">>, [], Req4),
+	Req5 = cowboy_req:set_resp_cookie(<<"cake">>, <<"lie">>, Req4),
 	case cowboy_req:has_resp_header(<<"x-cowboy-test">>, Req5) of
 		false -> {ok, Req5, Opts};
 		true ->