Browse Source

Remove process_post, post_is_create, create_path, created_path callbacks

Instead it will always go through content_types_accepted and it is
up to the resource code to do any creation and to return the created
path if the method is POST and the client should be redirected to the
created resource's location.

This removes the meta value 'put_path' as it is not needed anymore.

This fixes an issue with PATCH where content types were not normalized.
Loïc Hoguin 12 years ago
parent
commit
5a171d0f80

+ 6 - 9
examples/rest_pastebin/src/toppage_handler.erl

@@ -9,8 +9,6 @@
 -export([content_types_provided/2]).
 -export([content_types_provided/2]).
 -export([content_types_accepted/2]).
 -export([content_types_accepted/2]).
 -export([resource_exists/2]).
 -export([resource_exists/2]).
--export([post_is_create/2]).
--export([create_path/2]).
 
 
 %% Callback Callbacks
 %% Callback Callbacks
 -export([create_paste/2]).
 -export([create_paste/2]).
@@ -47,17 +45,16 @@ resource_exists(Req, _State) ->
 			end
 			end
 	end.
 	end.
 
 
-post_is_create(Req, State) ->
-	{true, Req, State}.
-
-create_path(Req, State) ->
-	{<<$/, (new_paste_id())/binary>>, Req, State}.
-
 create_paste(Req, State) ->
 create_paste(Req, State) ->
 	{<<$/, PasteID/binary>>, Req2} = cowboy_req:meta(put_path, Req),
 	{<<$/, PasteID/binary>>, Req2} = cowboy_req:meta(put_path, Req),
 	{ok, [{<<"paste">>, Paste}], Req3} = cowboy_req:body_qs(Req2),
 	{ok, [{<<"paste">>, Paste}], Req3} = cowboy_req:body_qs(Req2),
 	ok = file:write_file(full_path(PasteID), Paste),
 	ok = file:write_file(full_path(PasteID), Paste),
-	{true, Req3, State}.
+	case cowboy_req:method(Req3) of
+		{<<"POST">>, Req4} ->
+			{<<$/, (new_paste_id())/binary>>, Req4, State};
+		{_, Req4} ->
+			{true, Req4, State}
+	end.
 
 
 paste_html(Req, index) ->
 paste_html(Req, index) ->
 	{read_file("index.html"), Req, index};
 	{read_file("index.html"), Req, index};

+ 21 - 94
src/cowboy_rest.erl

@@ -236,8 +236,8 @@ content_types_provided(Req, State) ->
 normalize_content_types({ContentType, Callback})
 normalize_content_types({ContentType, Callback})
 		when is_binary(ContentType) ->
 		when is_binary(ContentType) ->
     {cowboy_http:content_type(ContentType), Callback};
     {cowboy_http:content_type(ContentType), Callback};
-normalize_content_types(Provided) ->
-	Provided.
+normalize_content_types(Normalized) ->
+	Normalized.
 
 
 prioritize_accept(Accept) ->
 prioritize_accept(Accept) ->
 	lists:sort(
 	lists:sort(
@@ -728,16 +728,18 @@ is_post_to_missing_resource(Req, State, OnFalse) ->
 	respond(Req, State, OnFalse).
 	respond(Req, State, OnFalse).
 
 
 allow_missing_post(Req, State, OnFalse) ->
 allow_missing_post(Req, State, OnFalse) ->
-	expect(Req, State, allow_missing_post, true, fun post_is_create/2, OnFalse).
+	expect(Req, State, allow_missing_post, true, fun post_resource/2, OnFalse).
+
+post_resource(Req, State) ->
+	accept_resource(Req, State, 204).
 
 
 method(Req, State=#state{method= <<"DELETE">>}) ->
 method(Req, State=#state{method= <<"DELETE">>}) ->
 	delete_resource(Req, State);
 	delete_resource(Req, State);
-method(Req, State=#state{method= <<"POST">>}) ->
-	post_is_create(Req, State);
 method(Req, State=#state{method= <<"PUT">>}) ->
 method(Req, State=#state{method= <<"PUT">>}) ->
 	is_conflict(Req, State);
 	is_conflict(Req, State);
-method(Req, State=#state{method= <<"PATCH">>}) ->
-	patch_resource(Req, State);
+method(Req, State=#state{method=Method})
+		when Method =:= <<"POST">>; Method =:= <<"PATCH">> ->
+	accept_resource(Req, State, 204);
 method(Req, State=#state{method=Method})
 method(Req, State=#state{method=Method})
 		when Method =:= <<"GET">>; Method =:= <<"HEAD">> ->
 		when Method =:= <<"GET">>; Method =:= <<"HEAD">> ->
 	set_resp_body_etag(Req, State);
 	set_resp_body_etag(Req, State);
@@ -752,79 +754,21 @@ delete_resource(Req, State) ->
 delete_completed(Req, State) ->
 delete_completed(Req, State) ->
 	expect(Req, State, delete_completed, true, fun has_resp_body/2, 202).
 	expect(Req, State, delete_completed, true, fun has_resp_body/2, 202).
 
 
-%% post_is_create/2 indicates whether the POST method can create new resources.
-post_is_create(Req, State) ->
-	expect(Req, State, post_is_create, false, fun process_post/2, fun create_path/2).
-
-%% When the POST method can create new resources, create_path/2 will be called
-%% and is expected to return the full path to the new resource
-%% (including the leading /).
-create_path(Req, State) ->
-	case call(Req, State, create_path) of
-		no_call ->
-			put_resource(Req, State, fun created_path/2);
-		{halt, Req2, HandlerState} ->
-			terminate(Req2, State#state{handler_state=HandlerState});
-		{Path, Req2, HandlerState} ->
-			{HostURL, Req3} = cowboy_req:host_url(Req2),
-			State2 = State#state{handler_state=HandlerState},
-			Req4 = cowboy_req:set_resp_header(
-				<<"location">>, << HostURL/binary, Path/binary >>, Req3),
-			put_resource(cowboy_req:set_meta(put_path, Path, Req4),
-				State2, 303)
-	end.
-
-%% Called after content_types_accepted is called for POST methods
-%% when create_path did not exist. Expects the full path to
-%% be returned and MUST exist in the case that create_path
-%% does not.
-created_path(Req, State) ->
-	case call(Req, State, created_path) of
-		{halt, Req2, HandlerState} ->
-			terminate(Req2, State#state{handler_state=HandlerState});
-		{Path, Req2, HandlerState} ->
-			{HostURL, Req3} = cowboy_req:host_url(Req2),
-			State2 = State#state{handler_state=HandlerState},
-			Req4 = cowboy_req:set_resp_header(
-				<<"location">>, << HostURL/binary, Path/binary >>, Req3),
-			respond(cowboy_req:set_meta(put_path, Path, Req4),
-				State2, 303)
-	end.
-
-%% process_post should return true when the POST body could be processed
-%% and false when it hasn't, in which case a 500 error is sent.
-process_post(Req, State) ->
-	case call(Req, State, process_post) of
-		{halt, Req2, HandlerState} ->
-			terminate(Req2, State#state{handler_state=HandlerState});
-		{true, Req2, HandlerState} ->
-			State2 = State#state{handler_state=HandlerState},
-			next(Req2, State2, fun is_new_resource/2);
-		{false, Req2, HandlerState} ->
-			State2 = State#state{handler_state=HandlerState},
-			respond(Req2, State2, 500)
-	end.
-
 is_conflict(Req, State) ->
 is_conflict(Req, State) ->
 	expect(Req, State, is_conflict, false, fun put_resource/2, 409).
 	expect(Req, State, is_conflict, false, fun put_resource/2, 409).
 
 
 put_resource(Req, State) ->
 put_resource(Req, State) ->
-	Path = cowboy_req:get(path, Req),
-	put_resource(cowboy_req:set_meta(put_path, Path, Req),
-		State, fun is_new_resource/2).
+	accept_resource(Req, State, fun is_new_resource/2).
 
 
 %% content_types_accepted should return a list of media types and their
 %% content_types_accepted should return a list of media types and their
 %% associated callback functions in the same format as content_types_provided.
 %% associated callback functions in the same format as content_types_provided.
 %%
 %%
 %% The callback will then be called and is expected to process the content
 %% The callback will then be called and is expected to process the content
-%% pushed to the resource in the request body. The path to the new resource
-%% may be different from the request path, and is stored as request metadata.
-%% It is always defined past this point. It can be retrieved as demonstrated:
-%%     {PutPath, Req2} = cowboy_req:meta(put_path, Req)
+%% pushed to the resource in the request body.
 %%
 %%
-%%content_types_accepted SHOULD return a different list
+%% content_types_accepted SHOULD return a different list
 %% for each HTTP method.
 %% for each HTTP method.
-put_resource(Req, State, OnTrue) ->
+accept_resource(Req, State, OnTrue) ->
 	case call(Req, State, content_types_accepted) of
 	case call(Req, State, content_types_accepted) of
 		no_call ->
 		no_call ->
 			respond(Req, State, 415);
 			respond(Req, State, 415);
@@ -838,27 +782,6 @@ put_resource(Req, State, OnTrue) ->
 			choose_content_type(Req3, State2, OnTrue, ContentType, CTA2)
 			choose_content_type(Req3, State2, OnTrue, ContentType, CTA2)
 	end.
 	end.
 
 
-%% content_types_accepted should return a list of media types and their
-%% associated callback functions in the same format as content_types_provided.
-%%
-%% The callback will then be called and is expected to process the content
-%% pushed to the resource in the request body. 
-%%
-%% content_types_accepted SHOULD return a different list
-%% for each HTTP method.
-patch_resource(Req, State) ->
-	case call(Req, State, content_types_accepted) of
-		no_call ->
-			respond(Req, State, 415);
-		{halt, Req2, HandlerState} ->
-			terminate(Req2, State#state{handler_state=HandlerState});
-		{CTM, Req2, HandlerState} ->
-			State2 = State#state{handler_state=HandlerState},
-			{ok, ContentType, Req3}
-				= cowboy_req:parse_header(<<"content-type">>, Req2),
-			choose_content_type(Req3, State2, 204, ContentType, CTM)
-	end.
-
 %% The special content type '*' will always match. It can be used as a
 %% The special content type '*' will always match. It can be used as a
 %% catch-all content type for accepting any kind of request content.
 %% catch-all content type for accepting any kind of request content.
 %% Note that because it will always match, it should be the last of the
 %% Note that because it will always match, it should be the last of the
@@ -880,9 +803,8 @@ choose_content_type(Req, State, OnTrue,
 choose_content_type(Req, State, OnTrue, ContentType, [_Any|Tail]) ->
 choose_content_type(Req, State, OnTrue, ContentType, [_Any|Tail]) ->
 	choose_content_type(Req, State, OnTrue, ContentType, Tail).
 	choose_content_type(Req, State, OnTrue, ContentType, Tail).
 
 
-process_content_type(Req,
-		State=#state{handler=Handler, handler_state=HandlerState},
-		OnTrue, Fun) ->
+process_content_type(Req, State=#state{method=Method,
+		handler=Handler, handler_state=HandlerState}, OnTrue, Fun) ->
 	case call(Req, State, Fun) of
 	case call(Req, State, Fun) of
 		no_call ->
 		no_call ->
 			error_logger:error_msg(
 			error_logger:error_msg(
@@ -898,7 +820,12 @@ process_content_type(Req,
 			next(Req2, State2, OnTrue);
 			next(Req2, State2, OnTrue);
 		{false, Req2, HandlerState2} ->
 		{false, Req2, HandlerState2} ->
 			State2 = State#state{handler_state=HandlerState2},
 			State2 = State#state{handler_state=HandlerState2},
-			respond(Req2, State2, 422)
+			respond(Req2, State2, 422);
+		{ResURL, Req2, HandlerState2} when Method =:= <<"POST">> ->
+			State2 = State#state{handler_state=HandlerState2},
+			Req3 = cowboy_req:set_resp_header(
+				<<"location">>, ResURL, Req2),
+			respond(Req3, State2, 303)
 	end.
 	end.
 
 
 %% Whether we created a new resource, either through PUT or POST.
 %% Whether we created a new resource, either through PUT or POST.

+ 0 - 15
test/http_SUITE.erl

@@ -54,7 +54,6 @@
 -export([pipeline/1]).
 -export([pipeline/1]).
 -export([pipeline_long_polling/1]).
 -export([pipeline_long_polling/1]).
 -export([rest_bad_accept/1]).
 -export([rest_bad_accept/1]).
--export([rest_created_path/1]).
 -export([rest_expires/1]).
 -export([rest_expires/1]).
 -export([rest_keepalive/1]).
 -export([rest_keepalive/1]).
 -export([rest_keepalive_post/1]).
 -export([rest_keepalive_post/1]).
@@ -124,7 +123,6 @@ groups() ->
 		pipeline,
 		pipeline,
 		pipeline_long_polling,
 		pipeline_long_polling,
 		rest_bad_accept,
 		rest_bad_accept,
-		rest_created_path,
 		rest_expires,
 		rest_expires,
 		rest_keepalive,
 		rest_keepalive,
 		rest_keepalive_post,
 		rest_keepalive_post,
@@ -364,7 +362,6 @@ init_dispatch(Config) ->
 			{"/missing_put_callbacks", rest_missing_callbacks, []},
 			{"/missing_put_callbacks", rest_missing_callbacks, []},
 			{"/nodelete", rest_nodelete_resource, []},
 			{"/nodelete", rest_nodelete_resource, []},
 			{"/patch", rest_patch_resource, []},
 			{"/patch", rest_patch_resource, []},
-			{"/created_path", rest_created_path_resource, []},
 			{"/resetags", rest_resource_etags, []},
 			{"/resetags", rest_resource_etags, []},
 			{"/rest_expires", rest_expires, []},
 			{"/rest_expires", rest_expires, []},
 			{"/loop_timeout", http_handler_loop_timeout, []},
 			{"/loop_timeout", http_handler_loop_timeout, []},
@@ -882,18 +879,6 @@ rest_bad_accept(Config) ->
 		Client),
 		Client),
 	{ok, 400, _, _} = cowboy_client:response(Client2).
 	{ok, 400, _, _} = cowboy_client:response(Client2).
 
 
-rest_created_path(Config) ->
-	Headers = [{<<"content-type">>, <<"text/plain">>}],
-	Body = <<"Whatever">>,
-	Client = ?config(client, Config),
-	URL = build_url("/created_path", Config),
-	{ok, Client2} = cowboy_client:request(<<"POST">>, URL, Headers,
-		Body, Client),
-	{ok, 303, ResHeaders, _} = cowboy_client:response(Client2),
-	{<<"location">>, _Location} =
-		lists:keyfind(<<"location">>, 1, ResHeaders),
-	ok.
-
 rest_expires(Config) ->
 rest_expires(Config) ->
 	Client = ?config(client, Config),
 	Client = ?config(client, Config),
 	{ok, Client2} = cowboy_client:request(<<"GET">>,
 	{ok, Client2} = cowboy_client:request(<<"GET">>,

+ 0 - 35
test/rest_created_path_resource.erl

@@ -1,35 +0,0 @@
--module(rest_created_path_resource).
--export([init/3]).
--export([allowed_methods/2]).
--export([content_types_provided/2]).
--export([get_text_plain/2]).
--export([post_is_create/2]).
--export([content_types_accepted/2]).
--export([post_text_plain/2]).
--export([created_path/2]).
-
-init(_Transport, _Req, _Opts) ->
-	{upgrade, protocol, cowboy_rest}.
-
-allowed_methods(Req, State) ->
-{[<<"HEAD">>, <<"GET">>, <<"POST">>], Req, State}.
-
-content_types_provided(Req, State) ->
-	{[{{<<"text">>, <<"plain">>, []}, get_text_plain}], Req, State}.
-
-get_text_plain(Req, State) ->
-	{<<"This is REST!">>, Req, State}.
-
-post_is_create(Req, State) ->
-	{true, Req, State}.
-
-content_types_accepted(Req, State) ->
-	{[{{<<"text">>, <<"plain">>, []}, post_text_plain}], Req, State}.
-
-post_text_plain(Req, State) ->
-	{true, Req, State}.
-
-created_path(Req, State) ->
-	{<<"/created">>, Req, State}.
-
-

+ 3 - 12
test/rest_forbidden_resource.erl

@@ -1,7 +1,7 @@
 -module(rest_forbidden_resource).
 -module(rest_forbidden_resource).
 -export([init/3, rest_init/2, allowed_methods/2, forbidden/2,
 -export([init/3, rest_init/2, allowed_methods/2, forbidden/2,
 		content_types_provided/2, content_types_accepted/2,
 		content_types_provided/2, content_types_accepted/2,
-		post_is_create/2, create_path/2, to_text/2, from_text/2]).
+		to_text/2, from_text/2]).
 
 
 init(_Transport, _Req, _Opts) ->
 init(_Transport, _Req, _Opts) ->
 	{upgrade, protocol, cowboy_rest}.
 	{upgrade, protocol, cowboy_rest}.
@@ -23,18 +23,9 @@ content_types_provided(Req, State) ->
 content_types_accepted(Req, State) ->
 content_types_accepted(Req, State) ->
 	{[{{<<"text">>, <<"plain">>, []}, from_text}], Req, State}.
 	{[{{<<"text">>, <<"plain">>, []}, from_text}], Req, State}.
 
 
-post_is_create(Req, State) ->
-	{true, Req, State}.
-
-create_path(Req, State) ->
-	{Path, Req2} = cowboy_req:path(Req),
-	{Path, Req2, State}.
-
 to_text(Req, State) ->
 to_text(Req, State) ->
 	{<<"This is REST!">>, Req, State}.
 	{<<"This is REST!">>, Req, State}.
 
 
 from_text(Req, State) ->
 from_text(Req, State) ->
-	{true, Req, State}.
-
-
-
+	{Path, Req2} = cowboy_req:path(Req),
+	{Path, Req2, State}.