diff --git a/README.md b/README.md index 3f80941..08d31c9 100644 --- a/README.md +++ b/README.md @@ -17,8 +17,8 @@ This library is not ready for production use, but it wont take long to finish it ```erlang %% Define your routes Routes = [ - {<<"POST">>, <<"/api/users">>, fun my_handler:create_user/3}, - {<<"GET">>, <<"/api/users/{userId}">>, fun my_handler:get_user/3} + {<<"POST">>, <<"/api/users">>, fun user_handler:create_user/3}, + {<<"GET">>, <<"/api/users/{userId}">>, fun user_handler:get_user/3} ], %% Configure and start Elli, preferably in you supervisor spec. diff --git a/demo.escript b/demo.escript index 4da61d5..bacdd06 100644 --- a/demo.escript +++ b/demo.escript @@ -4,10 +4,12 @@ main(_) -> Routes = [ - {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/3}, - {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/3}, - {<<"POST">>, <<"/api/echo">>, fun elli_openapi_demo:echo_text/3}, - {<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/3} +%% {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/3}, +%% {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/3}, +%% {<<"POST">>, <<"/api/echo">>, fun elli_openapi_demo:echo_text/3}, +%% {<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/3} + {<<"GET">>, <<"/api/users/{userId}">>, fun user_handler:get_user/3}, + {<<"POST">>, <<"/api/users/">>, fun user_handler:create_user/3} ], Port = 3000, ElliOpts = [ diff --git a/rebar.config b/rebar.config index 3c6d3f4..77b4a53 100644 --- a/rebar.config +++ b/rebar.config @@ -18,7 +18,7 @@ {deps, [ {elli, "~> 3.3.0"}, - {spectra, "~> 0.2.0"} + {spectra, "~> 0.4.0"} ]}. {hank, [ diff --git a/rebar.lock b/rebar.lock index 7a86130..ddf761a 100644 --- a/rebar.lock +++ b/rebar.lock @@ -5,12 +5,12 @@ {ref,"c4d1098174cec06bd124855f3a28dfd6eda0a581"}, "eqwalizer_support"}, 0}, - {<<"spectra">>,{pkg,<<"spectra">>,<<"0.1.1">>},0}]}. + {<<"spectra">>,{pkg,<<"spectra">>,<<"0.4.0">>},0}]}. [ {pkg_hash,[ {<<"elli">>, <<"089218762A7FF3D20AE81C8E911BD0F73EE4EE0ED85454226D1FC6B4FFF3B4F6">>}, - {<<"spectra">>, <<"DB3DE5D46B25BF608318E6C8FB8EDEDDE25A21F2AC5DD91B41BBC3800E3C8226">>}]}, + {<<"spectra">>, <<"1A5390E55B34F83693998FD7ECBC448BCDC5FC5EBCCA790C7B0DD2D6A0FED255">>}]}, {pkg_hash_ext,[ {<<"elli">>, <<"698B13B33D05661DB9FE7EFCBA41B84825A379CCE86E486CF6AFF9285BE0CCF8">>}, - {<<"spectra">>, <<"76C684A56D016EA37443D4BCAEEDDBFB20366536F026BDB0B43D3C6C02225D13">>}]} + {<<"spectra">>, <<"E51E88BE6EB3B85A1E3D0A9651A9F76ACA5A83A1EF940467541B77D078C5FB84">>}]} ]. diff --git a/src/elli_openapi.erl b/src/elli_openapi.erl index e3cd89a..cfd1504 100644 --- a/src/elli_openapi.erl +++ b/src/elli_openapi.erl @@ -12,7 +12,6 @@ -include_lib("spectra/include/spectra_internal.hrl"). -include_lib("stdlib/include/ms_transform.hrl"). --include_lib("elli/include/elli.hrl"). -compile(nowarn_unused_type). @@ -110,19 +109,21 @@ check_and_convert_response(HandlerType, {HttpCode, Headers, Body}) -> encode_headers(Module, ReturnHeadersType, Headers) -> spectra_util:fold_until_error( - fun({FieldType, FieldName, Type}, Acc) when - FieldType =:= map_field_exact orelse FieldType =:= map_field_assoc - -> + fun( + #literal_map_field{ + kind = Kind, name = FieldName, binary_name = BinaryName, val_type = Type + }, + Acc + ) -> case maps:find(FieldName, Headers) of {ok, HeaderValue} -> case spectra:encode(binary_string, Module, Type, HeaderValue) of {ok, EncodedHeader} -> - HeaderName = atom_to_binary(FieldName), - {ok, [{HeaderName, EncodedHeader} | Acc]}; + {ok, [{BinaryName, EncodedHeader} | Acc]}; {error, _} = Error -> Error end; - error when FieldType =:= map_field_exact -> + error when Kind =:= exact -> {error, {missing_header, FieldName}}; error -> {ok, Acc} @@ -209,16 +210,16 @@ get_content_type(ElliRequest) -> decode_path_args(Module, PathArgs, PathArgsType) -> spectra_util:fold_until_error( - fun({map_field_exact, FieldName, Type}, Acc) -> - case maps:find(FieldName, PathArgs) of - {ok, PathArg} -> + fun(#literal_map_field{name = FieldName, val_type = Type}, Acc) -> + case PathArgs of + #{FieldName := PathArg} -> case spectra:decode(binary_string, Module, Type, PathArg) of {ok, DecodedPathArgs} -> - {ok, maps:put(FieldName, DecodedPathArgs, Acc)}; + {ok, Acc#{FieldName => DecodedPathArgs}}; {error, _} = Error -> Error end; - error -> + #{} -> {error, {missing_path_arg, FieldName}} end end, @@ -228,18 +229,24 @@ decode_path_args(Module, PathArgs, PathArgsType) -> decode_headers(Module, HeadersType, Headers) -> spectra_util:fold_until_error( - fun({map_field_exact, FieldName, Type}, Acc) -> - HeaderName = atom_to_binary(FieldName), - case lists:keyfind(HeaderName, 1, Headers) of - {HeaderName, HeaderValue} -> + fun( + #literal_map_field{ + kind = Kind, name = FieldName, binary_name = BinaryName, val_type = Type + }, + Acc + ) -> + case lists:keyfind(BinaryName, 1, Headers) of + {BinaryName, HeaderValue} -> case spectra:decode(binary_string, Module, Type, HeaderValue) of {ok, DecodedHeader} -> {ok, maps:put(FieldName, DecodedHeader, Acc)}; {error, _} = Error -> Error end; + false when Kind =:= exact -> + {error, {missing_header, FieldName}}; false -> - {error, {missing_header, FieldName}} + {ok, Acc} end end, #{}, @@ -253,7 +260,7 @@ to_matchspec(RouteEndpoints) -> path_map(RouteEndpoints) -> lists:foldl( fun({{Method, Path, Fun}, Endpoint, HandlerType}, Acc) -> - maps:put({Method, Path}, {Fun, Endpoint, HandlerType}, Acc) + Acc#{{Method, Path} => {Fun, Endpoint, HandlerType}} end, maps:new(), RouteEndpoints @@ -263,7 +270,7 @@ path_map(RouteEndpoints) -> to_handler_type({_HttpMethod, _Path, CallFun}) -> {Module, Function, Arity} = MFA = erlang:fun_info_mfa(CallFun), TypeInfo = spectra_abstract_code:types_in_module(Module), - {ok, FunctionSpecs} = spectra_type_info:get_function(TypeInfo, Function, Arity), + {ok, FunctionSpecs} = spectra_type_info:find_function(TypeInfo, Function, Arity), join_function_specs(MFA, FunctionSpecs). -spec to_endpoint({binary(), binary(), fun()}, #handler_type{}) -> @@ -295,14 +302,14 @@ to_endpoint( end, EndpointWithPath = maps:fold(PathFun, Endpoint0, to_map(PathArgs)), HeaderFun = - fun({FieldType, Name, Type}, EndpointAcc) when - FieldType =:= map_field_exact orelse FieldType =:= map_field_assoc - -> + fun( + #literal_map_field{kind = Kind, binary_name = BinaryName, val_type = Type}, EndpointAcc + ) -> HeaderArg = #{ - name => atom_to_binary(Name), + name => BinaryName, in => header, - required => FieldType =:= map_field_exact, + required => Kind =:= exact, module => Module, schema => Type }, @@ -310,11 +317,17 @@ to_endpoint( end, EndpointWithHeaders = lists:foldl(HeaderFun, EndpointWithPath, HeaderArgs#sp_map.fields), - RequestContentTypeMime = content_type_to_mime(RequestContentType), + %% Only add request body for HTTP methods that support it Endpoint1 = - spectra_openapi:with_request_body( - EndpointWithHeaders, Module, RequestBody, RequestContentTypeMime - ), + case http_method_supports_body(HttpMethod) of + true -> + RequestContentTypeMime = content_type_to_mime(RequestContentType), + spectra_openapi:with_request_body( + EndpointWithHeaders, Module, RequestBody, RequestContentTypeMime + ); + false -> + EndpointWithHeaders + end, %% Add all responses from the responses map ResponseFun = @@ -341,15 +354,14 @@ to_endpoint( add_response_headers(Response, Module, #sp_map{fields = Fields}) -> lists:foldl( - fun({FieldType, Name, Type}, ResponseAcc) when - FieldType =:= map_field_exact orelse FieldType =:= map_field_assoc - -> - HeaderName = atom_to_binary(Name), + fun( + #literal_map_field{kind = Kind, binary_name = BinaryName, val_type = Type}, ResponseAcc + ) -> HeaderSpec = #{ - required => FieldType =:= map_field_exact, + required => Kind =:= exact, schema => Type }, - spectra_openapi:response_with_header(ResponseAcc, HeaderName, Module, HeaderSpec) + spectra_openapi:response_with_header(ResponseAcc, BinaryName, Module, HeaderSpec) end, Response, Fields @@ -357,6 +369,13 @@ add_response_headers(Response, Module, #sp_map{fields = Fields}) -> add_response_headers(Response, _Module, _Other) -> Response. +%% HTTP methods that support request bodies +http_method_supports_body(~"POST") -> true; +http_method_supports_body(~"PUT") -> true; +http_method_supports_body(~"PATCH") -> true; +http_method_supports_body(~"DELETE") -> true; +http_method_supports_body(_) -> false. + to_spectra_http_method(~"GET") -> get; to_spectra_http_method(~"POST") -> post; to_spectra_http_method(~"PUT") -> put; @@ -368,7 +387,9 @@ to_spectra_http_method(~"TRACE") -> trace. to_map(#sp_map{fields = Fields}) -> lists:foldl( - fun({map_field_exact, Name, Type}, Acc) -> Acc#{atom_to_binary(Name) => Type} end, + fun(#literal_map_field{binary_name = BinaryName, val_type = Type}, Acc) -> + Acc#{BinaryName => Type} + end, #{}, Fields ). diff --git a/src/elli_openapi_handler.erl b/src/elli_openapi_handler.erl index a65dab7..210fbcb 100644 --- a/src/elli_openapi_handler.erl +++ b/src/elli_openapi_handler.erl @@ -21,9 +21,8 @@ handle(#req{path = [~"api-docs"]}, _Args) -> handle(ElliRequest, _Args) -> elli_openapi:route_call(ElliRequest). --spec handle_event(Event, Args, Config) -> ok when +-spec handle_event(Event, Args :: term(), Config) -> ok when Event :: elli_handler:event(), - Args :: elli_handler:callback_args(), Config :: [tuple()]. handle_event(elli_startup, [], Routes) -> Modules = @@ -37,5 +36,8 @@ handle_event(elli_startup, [], Routes) -> lists:foreach(fun code:ensure_loaded/1, lists:usort(Modules)), elli_openapi:setup_routes(Routes), ok; -handle_event(_Event, _Args, _Config) -> +handle_event(request_complete, [Req, ReturnCode, _, _, _], _Config) -> + io:format("Req complete: ~s ~p ~n", [elli_request:raw_path(Req), ReturnCode]), + ok; +handle_event(_Event, _Data, _Config) -> ok. diff --git a/src/user_handler.erl b/src/user_handler.erl new file mode 100644 index 0000000..c115aef --- /dev/null +++ b/src/user_handler.erl @@ -0,0 +1,33 @@ +-module(user_handler). + +-export([get_user/3, create_user/3]). + +-record(user, { + id :: binary(), + name :: binary(), + role :: admin | user | guest +}). + +-ignore_xref([create_user/3, get_user/3]). +-hank([{unnecessary_function_arguments, [{get_user, 3}]}]). + +-spec get_user(#{userId := binary()}, #{}, binary()) -> + {200, #{}, #user{}} + | {404, #{}, #{message := binary()}}. +get_user(#{userId := Id}, _Hdrs, _Body) -> + case find_user(Id) of + {ok, User} -> {200, #{}, User}; + not_found -> {404, #{}, #{message => ~"User not found"}} + end. + +-spec create_user(#{}, #{}, #user{}) -> + {201, #{'Location' => binary()}, #user{}}. +create_user(#{}, #{}, User) -> + io:format("Creating user: ~s with role ~p~n", [User#user.name, User#user.role]), + Location = <<"/api/users/", (User#user.id)/binary>>, + {201, #{'Location' => Location}, User}. + +find_user(~"123") -> + {ok, #user{id = ~"123", name = ~"Alice", role = user}}; +find_user(_) -> + not_found. diff --git a/test/elli_openapi_integration_SUITE.erl b/test/elli_openapi_integration_SUITE.erl index 0ff0650..1ed817c 100644 --- a/test/elli_openapi_integration_SUITE.erl +++ b/test/elli_openapi_integration_SUITE.erl @@ -31,6 +31,7 @@ openapi_spec_includes_response_headers/1, openapi_spec_content_types/1, openapi_spec_multi_status/1, + openapi_spec_get_no_request_body/1, swagger_ui_endpoint/1, redoc_endpoint/1, api_docs_endpoint/1 @@ -61,6 +62,7 @@ all() -> openapi_spec_includes_response_headers, openapi_spec_content_types, openapi_spec_multi_status, + openapi_spec_get_no_request_body, swagger_ui_endpoint, redoc_endpoint, api_docs_endpoint @@ -397,37 +399,39 @@ openapi_spec_includes_response_headers(_Config) -> ?assertMatch( #{ - paths := #{ + <<"paths">> := #{ <<"/api/users">> := #{ - post := #{ - responses := #{ + <<"post">> := #{ + <<"responses">> := #{ <<"201">> := #{ - headers := #{ - <<"Location">> := #{schema := #{type := <<"string">>}}, - <<"ETag">> := #{schema := #{type := <<"string">>}} + <<"headers">> := #{ + <<"Location">> := #{<<"schema">> := #{type := <<"string">>}}, + <<"ETag">> := #{<<"schema">> := #{type := <<"string">>}} } } } } }, <<"/api/users/{userId}">> := #{ - get := #{ - responses := #{ + <<"get">> := #{ + <<"responses">> := #{ <<"200">> := #{ - headers := #{ - <<"ETag">> := #{schema := #{type := <<"string">>}}, - <<"Cache-Control">> := #{schema := #{type := <<"string">>}} + <<"headers">> := #{ + <<"ETag">> := #{<<"schema">> := #{type := <<"string">>}}, + <<"Cache-Control">> := #{ + <<"schema">> := #{type := <<"string">>} + } } } } } }, <<"/api/status">> := #{ - post := #{ - requestBody := #{content := #{<<"text/plain">> := _}}, - responses := #{ + <<"post">> := #{ + <<"requestBody">> := #{<<"content">> := #{<<"text/plain">> := _}}, + <<"responses">> := #{ <<"200">> := #{ - content := #{<<"text/plain">> := _} + <<"content">> := #{<<"text/plain">> := _} } } } @@ -452,23 +456,23 @@ openapi_spec_content_types(_Config) -> {ok, Spec} = elli_openapi:generate_openapi_spec(MetaData, Routes), #{ - paths := #{ + <<"paths">> := #{ <<"/api/echo">> := #{ - post := #{ - requestBody := #{content := EchoReqContent}, - responses := #{<<"200">> := #{content := EchoRespContent}} + <<"post">> := #{ + <<"requestBody">> := #{<<"content">> := EchoReqContent}, + <<"responses">> := #{<<"200">> := #{<<"content">> := EchoRespContent}} } }, <<"/api/status">> := #{ - post := #{ - requestBody := #{content := StatusReqContent}, - responses := #{<<"200">> := #{content := StatusRespContent}} + <<"post">> := #{ + <<"requestBody">> := #{<<"content">> := StatusReqContent}, + <<"responses">> := #{<<"200">> := #{<<"content">> := StatusRespContent}} } }, <<"/api/users">> := #{ - post := #{ - requestBody := #{content := UsersReqContent}, - responses := #{<<"201">> := #{content := UsersRespContent}} + <<"post">> := #{ + <<"requestBody">> := #{<<"content">> := UsersReqContent}, + <<"responses">> := #{<<"201">> := #{<<"content">> := UsersRespContent}} } } } @@ -492,26 +496,26 @@ openapi_spec_multi_status(_Config) -> %% Verify all 4 status codes with proper descriptions and content types ?assertMatch( #{ - paths := #{ + <<"paths">> := #{ <<"/api/items/{itemId}">> := #{ - put := #{ - responses := #{ + <<"put">> := #{ + <<"responses">> := #{ <<"200">> := #{ - headers := #{<<"ETag">> := _}, - description := ~"Success", - content := #{<<"application/json">> := _} + <<"headers">> := #{<<"ETag">> := _}, + <<"description">> := ~"Success", + <<"content">> := #{<<"application/json">> := _} }, <<"400">> := #{ - description := ~"Bad Request", - content := #{<<"application/json">> := _} + <<"description">> := ~"Bad Request", + <<"content">> := #{<<"application/json">> := _} }, <<"404">> := #{ - description := ~"Not Found", - content := #{<<"application/json">> := _} + <<"description">> := ~"Not Found", + <<"content">> := #{<<"application/json">> := _} }, <<"409">> := #{ - description := ~"Conflict", - content := #{<<"application/json">> := _} + <<"description">> := ~"Conflict", + <<"content">> := #{<<"application/json">> := _} } } } @@ -523,6 +527,30 @@ openapi_spec_multi_status(_Config) -> ok. +openapi_spec_get_no_request_body(_Config) -> + %% Test that GET requests do not generate requestBody in OpenAPI spec + Routes = [ + {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/3}, + {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/3} + ], + + MetaData = #{title => ~"Test API", version => ~"1.0.0"}, + {ok, Spec} = elli_openapi:generate_openapi_spec(MetaData, Routes), + + #{<<"paths">> := Paths} = Spec, + + %% Verify GET request has no requestBody + #{<<"/api/users/{userId}">> := UserPath} = Paths, + #{<<"get">> := GetEndpoint} = UserPath, + ?assertNot(maps:is_key(<<"requestBody">>, GetEndpoint)), + + %% Verify POST request has requestBody + #{<<"/api/users">> := UsersPath} = Paths, + #{<<"post">> := PostEndpoint} = UsersPath, + ?assert(maps:is_key(<<"requestBody">>, PostEndpoint)), + + ok. + %%==================================================================== %% Test Cases - Documentation Endpoints %%====================================================================