From 5a171d0f8050eda4e43de82efb7f2508be314ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 11 Apr 2013 17:59:54 +0200 Subject: 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. --- examples/rest_pastebin/src/toppage_handler.erl | 15 ++-- src/cowboy_rest.erl | 115 +++++-------------------- test/http_SUITE.erl | 15 ---- test/rest_created_path_resource.erl | 35 -------- test/rest_forbidden_resource.erl | 15 +--- 5 files changed, 30 insertions(+), 165 deletions(-) delete mode 100644 test/rest_created_path_resource.erl diff --git a/examples/rest_pastebin/src/toppage_handler.erl b/examples/rest_pastebin/src/toppage_handler.erl index 5e904d9..a6a0829 100644 --- a/examples/rest_pastebin/src/toppage_handler.erl +++ b/examples/rest_pastebin/src/toppage_handler.erl @@ -9,8 +9,6 @@ -export([content_types_provided/2]). -export([content_types_accepted/2]). -export([resource_exists/2]). --export([post_is_create/2]). --export([create_path/2]). %% Callback Callbacks -export([create_paste/2]). @@ -47,17 +45,16 @@ resource_exists(Req, _State) -> end end. -post_is_create(Req, State) -> - {true, Req, State}. - -create_path(Req, State) -> - {<<$/, (new_paste_id())/binary>>, Req, State}. - create_paste(Req, State) -> {<<$/, PasteID/binary>>, Req2} = cowboy_req:meta(put_path, Req), {ok, [{<<"paste">>, Paste}], Req3} = cowboy_req:body_qs(Req2), 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) -> {read_file("index.html"), Req, index}; diff --git a/src/cowboy_rest.erl b/src/cowboy_rest.erl index cb4fffb..b11a29a 100644 --- a/src/cowboy_rest.erl +++ b/src/cowboy_rest.erl @@ -236,8 +236,8 @@ content_types_provided(Req, State) -> normalize_content_types({ContentType, Callback}) when is_binary(ContentType) -> {cowboy_http:content_type(ContentType), Callback}; -normalize_content_types(Provided) -> - Provided. +normalize_content_types(Normalized) -> + Normalized. prioritize_accept(Accept) -> lists:sort( @@ -728,16 +728,18 @@ is_post_to_missing_resource(Req, State, OnFalse) -> respond(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">>}) -> delete_resource(Req, State); -method(Req, State=#state{method= <<"POST">>}) -> - post_is_create(Req, State); method(Req, State=#state{method= <<"PUT">>}) -> 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}) when Method =:= <<"GET">>; Method =:= <<"HEAD">> -> set_resp_body_etag(Req, State); @@ -752,79 +754,21 @@ delete_resource(Req, State) -> delete_completed(Req, State) -> 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) -> expect(Req, State, is_conflict, false, fun put_resource/2, 409). 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 %% 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. 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. -put_resource(Req, State, OnTrue) -> +accept_resource(Req, State, OnTrue) -> case call(Req, State, content_types_accepted) of no_call -> respond(Req, State, 415); @@ -838,27 +782,6 @@ put_resource(Req, State, OnTrue) -> choose_content_type(Req3, State2, OnTrue, ContentType, CTA2) 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 %% 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 @@ -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, 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 no_call -> error_logger:error_msg( @@ -898,7 +820,12 @@ process_content_type(Req, next(Req2, State2, OnTrue); {false, Req2, 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. %% Whether we created a new resource, either through PUT or POST. diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl index 911efb8..e33e19a 100644 --- a/test/http_SUITE.erl +++ b/test/http_SUITE.erl @@ -54,7 +54,6 @@ -export([pipeline/1]). -export([pipeline_long_polling/1]). -export([rest_bad_accept/1]). --export([rest_created_path/1]). -export([rest_expires/1]). -export([rest_keepalive/1]). -export([rest_keepalive_post/1]). @@ -124,7 +123,6 @@ groups() -> pipeline, pipeline_long_polling, rest_bad_accept, - rest_created_path, rest_expires, rest_keepalive, rest_keepalive_post, @@ -364,7 +362,6 @@ init_dispatch(Config) -> {"/missing_put_callbacks", rest_missing_callbacks, []}, {"/nodelete", rest_nodelete_resource, []}, {"/patch", rest_patch_resource, []}, - {"/created_path", rest_created_path_resource, []}, {"/resetags", rest_resource_etags, []}, {"/rest_expires", rest_expires, []}, {"/loop_timeout", http_handler_loop_timeout, []}, @@ -882,18 +879,6 @@ rest_bad_accept(Config) -> Client), {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) -> Client = ?config(client, Config), {ok, Client2} = cowboy_client:request(<<"GET">>, diff --git a/test/rest_created_path_resource.erl b/test/rest_created_path_resource.erl deleted file mode 100644 index 5ad8cfc..0000000 --- a/test/rest_created_path_resource.erl +++ /dev/null @@ -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}. - - diff --git a/test/rest_forbidden_resource.erl b/test/rest_forbidden_resource.erl index 63aac7e..287ff62 100644 --- a/test/rest_forbidden_resource.erl +++ b/test/rest_forbidden_resource.erl @@ -1,7 +1,7 @@ -module(rest_forbidden_resource). -export([init/3, rest_init/2, allowed_methods/2, forbidden/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) -> {upgrade, protocol, cowboy_rest}. @@ -23,18 +23,9 @@ content_types_provided(Req, State) -> content_types_accepted(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) -> {<<"This is REST!">>, Req, State}. from_text(Req, State) -> - {true, Req, State}. - - - + {Path, Req2} = cowboy_req:path(Req), + {Path, Req2, State}. -- cgit v1.2.3