From 92fe684c1b0172d6c63801de671d4abe4d487099 Mon Sep 17 00:00:00 2001 From: Andreas Hasselberg Date: Sun, 22 Mar 2026 21:15:43 +0100 Subject: [PATCH 1/3] Add query params, function docs, and configurable metadata - **4-arity handlers**: action signatures now take (PathArgs, QueryArgs, Headers, Body) matching the phoenix_spectral convention. Query params are decoded from the URL query string using elli_request:get_args/1, type-checked via spectra, and passed as the second argument. - **OpenAPI query parameter generation**: `to_endpoint` emits `in: query` parameters for each field in the QueryArgs map type; required fields (`:=`) are marked required, optional (`=>`) are not. - **Function doc support**: `-spectra(#{summary => ..., description => ...})` placed before a `-spec` populates the OpenAPI operation's summary and description fields via `spectra_openapi:endpoint/3`. - **Configurable OpenAPI metadata**: `setup_routes/2` now accepts a full `openapi_metadata()` map (servers, contact, license, description, etc.). `elli_openapi_handler` accepts `{MetaData, Routes}` as callback_args for backward-compatible metadata configuration. - **Fix double-encoding bug**: `generate_openapi_spec` now uses `pre_encoded` option so spectra returns a raw map, preventing double-encoding of the JSON output. - Demo updated with `list_users/4` showing optional query params (page, per_page). All 28 tests pass, eqwalizer clean. Co-Authored-By: Claude Sonnet 4.6 --- rebar.config | 2 +- rebar.lock | 6 +- src/elli_openapi.erl | 101 ++++++++++++++++---- src/elli_openapi_demo.erl | 118 +++++++++++++++-------- src/elli_openapi_handler.erl | 25 +++-- test/elli_openapi_integration_SUITE.erl | 119 ++++++++++++++++++++---- 6 files changed, 281 insertions(+), 90 deletions(-) diff --git a/rebar.config b/rebar.config index 77b4a53..ca082f7 100644 --- a/rebar.config +++ b/rebar.config @@ -18,7 +18,7 @@ {deps, [ {elli, "~> 3.3.0"}, - {spectra, "~> 0.4.0"} + {spectra, "~> 0.8.2"} ]}. {hank, [ diff --git a/rebar.lock b/rebar.lock index ddf761a..a9f5999 100644 --- a/rebar.lock +++ b/rebar.lock @@ -5,12 +5,12 @@ {ref,"c4d1098174cec06bd124855f3a28dfd6eda0a581"}, "eqwalizer_support"}, 0}, - {<<"spectra">>,{pkg,<<"spectra">>,<<"0.4.0">>},0}]}. + {<<"spectra">>,{pkg,<<"spectra">>,<<"0.8.2">>},0}]}. [ {pkg_hash,[ {<<"elli">>, <<"089218762A7FF3D20AE81C8E911BD0F73EE4EE0ED85454226D1FC6B4FFF3B4F6">>}, - {<<"spectra">>, <<"1A5390E55B34F83693998FD7ECBC448BCDC5FC5EBCCA790C7B0DD2D6A0FED255">>}]}, + {<<"spectra">>, <<"6BDFBB85624628B6F8537A27B0185004713349C908505DD1D9E288AFAE435EDF">>}]}, {pkg_hash_ext,[ {<<"elli">>, <<"698B13B33D05661DB9FE7EFCBA41B84825A379CCE86E486CF6AFF9285BE0CCF8">>}, - {<<"spectra">>, <<"E51E88BE6EB3B85A1E3D0A9651A9F76ACA5A83A1EF940467541B77D078C5FB84">>}]} + {<<"spectra">>, <<"298134F7564A7EA1152FF205927E5D8BB5FCD4845E5A5BE18F87C75D74368303">>}]} ]. diff --git a/src/elli_openapi.erl b/src/elli_openapi.erl index cfd1504..49bef46 100644 --- a/src/elli_openapi.erl +++ b/src/elli_openapi.erl @@ -2,6 +2,7 @@ -export([ setup_routes/1, + setup_routes/2, route_call/1, to_handler_type/1, to_endpoint/2, @@ -27,15 +28,20 @@ -record(handler_type, { mfa :: mfa(), path_args :: #sp_map{}, + query_args :: #sp_map{}, header_args :: #sp_map{}, request_body :: spectra:sp_type(), request_content_type :: content_type(), - responses :: #{integer() => #response_spec{}} + responses :: #{integer() => #response_spec{}}, + doc :: spectra:function_doc() }). -type spectra_openapi__endpoint_spec() :: map(). setup_routes(Routes) -> + setup_routes(#{title => ~"My API", version => ~"1.0.0"}, Routes). + +setup_routes(MetaData, Routes) -> RouteEndpoints = lists:map( fun(Route) -> @@ -44,7 +50,6 @@ setup_routes(Routes) -> end, Routes ), - MetaData = #{title => ~"My API", version => ~"1.0.0"}, {ok, OpenApiSpec} = generate_openapi_spec(MetaData, Routes), OpenApiJson = json:encode(OpenApiSpec), Mref = to_matchspec(RouteEndpoints), @@ -61,8 +66,8 @@ route_call(ElliRequest) -> HttpPathArgs = maps:from_list(HttpPathArgsList), {Fun, _Endpoint, HandlerType} = maps:get({Method, RoutePath}, MyMap), case check_types(HandlerType, HttpPathArgs, ElliRequest) of - {ok, PathArgs, Headers, Body} -> - Response = Fun(PathArgs, Headers, Body), + {ok, PathArgs, QueryArgs, Headers, Body} -> + Response = Fun(PathArgs, QueryArgs, Headers, Body), check_and_convert_response(HandlerType, Response); {error, ErldanticErrors} -> {400, [], spectra_error_to_response_body(ErldanticErrors)} @@ -150,6 +155,7 @@ check_types(HandlerType, PathArgs, ElliRequest) -> #handler_type{ mfa = {Module, _, _}, path_args = PathArgsType, + query_args = QueryArgsType, header_args = HeadersType, request_body = RequestBodyType, request_content_type = RequestContentType @@ -157,12 +163,22 @@ check_types(HandlerType, PathArgs, ElliRequest) -> HandlerType, case decode_path_args(Module, PathArgs, PathArgsType) of - {ok, DecodePathArgs} -> - case decode_headers(Module, HeadersType, elli_request:headers(ElliRequest)) of - {ok, DecodedHeader} -> - case decode_body(Module, RequestBodyType, RequestContentType, ElliRequest) of - {ok, DecodedBody} -> - {ok, DecodePathArgs, DecodedHeader, DecodedBody}; + {ok, DecodedPathArgs} -> + case decode_query_args(Module, QueryArgsType, ElliRequest) of + {ok, DecodedQueryArgs} -> + case decode_headers(Module, HeadersType, elli_request:headers(ElliRequest)) of + {ok, DecodedHeaders} -> + case + decode_body( + Module, RequestBodyType, RequestContentType, ElliRequest + ) + of + {ok, DecodedBody} -> + {ok, DecodedPathArgs, DecodedQueryArgs, DecodedHeaders, + DecodedBody}; + {error, _} = Error -> + Error + end; {error, _} = Error -> Error end; @@ -227,6 +243,33 @@ decode_path_args(Module, PathArgs, PathArgsType) -> PathArgsType#sp_map.fields ). +decode_query_args(Module, QueryArgsType, ElliRequest) -> + QueryParams = elli_request:get_args(ElliRequest), + spectra_util:fold_until_error( + fun( + #literal_map_field{ + kind = Kind, name = FieldName, binary_name = BinaryName, val_type = Type + }, + Acc + ) -> + case lists:keyfind(BinaryName, 1, QueryParams) of + {BinaryName, ParamValue} -> + case spectra:decode(binary_string, Module, Type, ParamValue) of + {ok, DecodedParam} -> + {ok, Acc#{FieldName => DecodedParam}}; + {error, _} = Error -> + Error + end; + false when Kind =:= exact -> + {error, {missing_query_param, FieldName}}; + false -> + {ok, Acc} + end + end, + #{}, + QueryArgsType#sp_map.fields + ). + decode_headers(Module, HeadersType, Headers) -> spectra_util:fold_until_error( fun( @@ -280,13 +323,16 @@ to_endpoint( #handler_type{ mfa = {Module, _Function, _Arity}, path_args = PathArgs, + query_args = QueryArgs, header_args = HeaderArgs, request_body = RequestBody, request_content_type = RequestContentType, - responses = Responses + responses = Responses, + doc = FunctionDoc } ) -> - Endpoint0 = spectra_openapi:endpoint(to_spectra_http_method(HttpMethod), Path), + EndpointDoc = maps:with([summary, description, deprecated], FunctionDoc), + Endpoint0 = spectra_openapi:endpoint(to_spectra_http_method(HttpMethod), Path, EndpointDoc), PathFun = fun(Key, Val, EndpointAcc) -> PathArg = @@ -294,13 +340,25 @@ to_endpoint( name => Key, in => path, required => true, - module => Module, schema => Val }, - spectra_openapi:with_parameter(EndpointAcc, Module, PathArg) end, EndpointWithPath = maps:fold(PathFun, Endpoint0, to_map(PathArgs)), + QueryFun = + fun( + #literal_map_field{kind = Kind, binary_name = BinaryName, val_type = Type}, EndpointAcc + ) -> + QueryArg = + #{ + name => BinaryName, + in => query, + required => Kind =:= exact, + schema => Type + }, + spectra_openapi:with_parameter(EndpointAcc, Module, QueryArg) + end, + EndpointWithQuery = lists:foldl(QueryFun, EndpointWithPath, QueryArgs#sp_map.fields), HeaderFun = fun( #literal_map_field{kind = Kind, binary_name = BinaryName, val_type = Type}, EndpointAcc @@ -310,12 +368,11 @@ to_endpoint( name => BinaryName, in => header, required => Kind =:= exact, - module => Module, schema => Type }, spectra_openapi:with_parameter(EndpointAcc, Module, HeaderArg) end, - EndpointWithHeaders = lists:foldl(HeaderFun, EndpointWithPath, HeaderArgs#sp_map.fields), + EndpointWithHeaders = lists:foldl(HeaderFun, EndpointWithQuery, HeaderArgs#sp_map.fields), %% Only add request body for HTTP methods that support it Endpoint1 = @@ -405,7 +462,7 @@ generate_openapi_spec(MetaData, Routes) -> ), Endpoints = lists:map(fun({_Route, Endpoint, _HandlerType}) -> Endpoint end, RouteEndpoints), - spectra_openapi:endpoints_to_openapi(MetaData, Endpoints). + spectra_openapi:endpoints_to_openapi(MetaData, Endpoints, [pre_encoded]). -spec infer_content_type(spectra:sp_type()) -> content_type(). infer_content_type(#sp_simple_type{type = binary}) -> @@ -451,19 +508,23 @@ join_function_specs( MFA, [ #sp_function_spec{ - args = [PathArgs, HeaderArgs, Body], - return = ReturnType + args = [PathArgs, QueryArgs, HeaderArgs, Body], + return = ReturnType, + meta = Meta } ] ) -> Responses = extract_responses(ReturnType), + Doc = maps:get(doc, Meta, #{}), #handler_type{ mfa = MFA, path_args = PathArgs, + query_args = QueryArgs, header_args = HeaderArgs, request_body = Body, request_content_type = infer_content_type(Body), - responses = Responses + responses = Responses, + doc = Doc }. %% Extract response specifications from return type diff --git a/src/elli_openapi_demo.erl b/src/elli_openapi_demo.erl index 579fd86..73af1ac 100644 --- a/src/elli_openapi_demo.erl +++ b/src/elli_openapi_demo.erl @@ -1,8 +1,8 @@ -module(elli_openapi_demo). --export([create_user/3, get_user/3, echo_text/3, update_status/3, update_item/3]). +-export([create_user/4, get_user/4, echo_text/4, update_status/4, update_item/4, list_users/4]). --ignore_xref([create_user/3, get_user/3, echo_text/3, update_status/3, update_item/3]). +-ignore_xref([create_user/4, get_user/4, echo_text/4, update_status/4, update_item/4, list_users/4]). -compile(nowarn_unused_type). @@ -13,13 +13,74 @@ role :: admin | user | guest }). +-record(item, { + id :: binary(), + name :: binary(), + version :: integer() +}). + +-record(error_response, { + message :: binary(), + code :: binary() +}). + +-record(user_list, { + users :: [#user{}], + total :: non_neg_integer() +}). + +-spectra(#{summary => <<"Create a new user">>, description => <<"Creates a user account">>}). -spec create_user( + #{}, #{}, #{}, #{email := binary(), name := binary(), role => admin | user | guest} ) -> {201, #{'Location' => binary(), 'ETag' => binary()}, #user{}}. -create_user(#{}, #{}, #{email := Email, name := Name} = Body) -> + +-spectra(#{summary => <<"Get a user by ID">>}). +-spec get_user( + #{userId := binary()}, + #{}, + #{'Authorization' := binary()}, + binary() +) -> + {200, #{'ETag' => binary(), 'Cache-Control' => binary()}, #user{}}. + +-spec echo_text(#{}, #{}, #{}, binary()) -> {200, #{}, binary()}. + +-spec update_status(#{}, #{}, #{}, running | stopped | paused) -> + {200, #{}, running | stopped | paused}. + +-spectra(#{ + summary => <<"Update an item">>, + description => <<"Updates item by ID, with conflict detection">> +}). +-spec update_item( + #{itemId := binary()}, + #{}, + #{}, + #{name := binary(), version := integer()} +) -> + {200, #{'ETag' => binary()}, #item{}} + | {400, #{}, #error_response{}} + | {404, #{}, #error_response{}} + | {409, #{}, #error_response{}}. + +-spectra(#{summary => <<"List users">>, description => <<"Returns a paginated list of users">>}). +-spec list_users( + #{}, + #{page => pos_integer(), per_page => pos_integer()}, + #{}, + binary() +) -> + {200, #{}, #user_list{}}. + +%%==================================================================== +%% Function definitions +%%==================================================================== + +create_user(#{}, #{}, #{}, #{email := Email, name := Name} = Body) -> Role = maps:get(role, Body, user), UserId = <<"user-123">>, User = #user{ @@ -32,13 +93,7 @@ create_user(#{}, #{}, #{email := Email, name := Name} = Body) -> ETag = <<"\"v1-", UserId/binary, "\"">>, {201, #{'Location' => Location, 'ETag' => ETag}, User}. --spec get_user( - #{userId := binary()}, - #{'Authorization' := binary()}, - binary() -) -> - {200, #{'ETag' => binary(), 'Cache-Control' => binary()}, #user{}}. -get_user(#{userId := UserId}, #{'Authorization' := _Token}, ~"") -> +get_user(#{userId := UserId}, #{}, #{'Authorization' := _Token}, ~"") -> User = #user{ id = UserId, email = <<"user@example.com">>, @@ -49,58 +104,30 @@ get_user(#{userId := UserId}, #{'Authorization' := _Token}, ~"") -> CacheControl = <<"max-age=300, must-revalidate">>, {200, #{'ETag' => ETag, 'Cache-Control' => CacheControl}, User}. --spec echo_text(#{}, #{}, binary()) -> {200, #{}, binary()}. -echo_text(#{}, #{}, Text) -> +echo_text(#{}, #{}, #{}, Text) -> {200, #{}, <<"Echo: ", Text/binary>>}. --spec update_status(#{}, #{}, running | stopped | paused) -> - {200, #{}, running | stopped | paused}. -update_status(#{}, #{}, Status) -> +update_status(#{}, #{}, #{}, Status) -> {200, #{}, Status}. --record(item, { - id :: binary(), - name :: binary(), - version :: integer() -}). - --record(error_response, { - message :: binary(), - code :: binary() -}). - -%% Demo endpoint with multiple status codes --spec update_item( - #{itemId := binary()}, - #{}, - #{name := binary(), version := integer()} -) -> - {200, #{'ETag' => binary()}, #item{}} - | {400, #{}, #error_response{}} - | {404, #{}, #error_response{}} - | {409, #{}, #error_response{}}. -update_item(#{itemId := ItemId}, #{}, #{name := Name, version := Version}) -> +update_item(#{itemId := ItemId}, #{}, #{}, #{name := Name, version := Version}) -> case {ItemId, Name, Version} of {~"item-notfound", _, _} -> - %% Simulate item not found {404, #{}, #error_response{ message = <<"Item not found">>, code = <<"ITEM_NOT_FOUND">> }}; {_, _, V} when V < 0 -> - %% Invalid version number {400, #{}, #error_response{ message = <<"Version must be non-negative">>, code = <<"INVALID_VERSION">> }}; {~"item-conflict", _, _} -> - %% Simulate version conflict {409, #{}, #error_response{ message = <<"Version conflict detected">>, code = <<"VERSION_CONFLICT">> }}; {_, _, _} -> - %% Success case Item = #item{ id = ItemId, name = Name, @@ -109,3 +136,12 @@ update_item(#{itemId := ItemId}, #{}, #{name := Name, version := Version}) -> ETag = iolist_to_binary(io_lib:format("\"v~p-~s\"", [Version, ItemId])), {200, #{'ETag' => ETag}, Item} end. + +list_users(#{}, QueryParams, #{}, ~"") -> + _Page = maps:get(page, QueryParams, 1), + _PerPage = maps:get(per_page, QueryParams, 20), + Users = [ + #user{id = <<"user-1">>, email = <<"a@example.com">>, name = <<"Alice">>, role = admin}, + #user{id = <<"user-2">>, email = <<"b@example.com">>, name = <<"Bob">>, role = user} + ], + {200, #{}, #user_list{users = Users, total = 2}}. diff --git a/src/elli_openapi_handler.erl b/src/elli_openapi_handler.erl index 210fbcb..36b9c26 100644 --- a/src/elli_openapi_handler.erl +++ b/src/elli_openapi_handler.erl @@ -23,8 +23,22 @@ handle(ElliRequest, _Args) -> -spec handle_event(Event, Args :: term(), Config) -> ok when Event :: elli_handler:event(), - Config :: [tuple()]. + Config :: [tuple()] | {spectra_openapi:openapi_metadata(), [tuple()]}. +handle_event(elli_startup, [], {MetaData, Routes}) when is_map(MetaData) -> + ensure_modules_loaded(Routes), + elli_openapi:setup_routes(MetaData, Routes), + ok; handle_event(elli_startup, [], Routes) -> + ensure_modules_loaded(Routes), + elli_openapi:setup_routes(Routes), + ok; +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. + +ensure_modules_loaded(Routes) -> Modules = lists:map( fun({_, _, CallFun}) -> @@ -33,11 +47,4 @@ handle_event(elli_startup, [], Routes) -> end, Routes ), - lists:foreach(fun code:ensure_loaded/1, lists:usort(Modules)), - elli_openapi:setup_routes(Routes), - ok; -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. + lists:foreach(fun code:ensure_loaded/1, lists:usort(Modules)). diff --git a/test/elli_openapi_integration_SUITE.erl b/test/elli_openapi_integration_SUITE.erl index 1ed817c..68a13a3 100644 --- a/test/elli_openapi_integration_SUITE.erl +++ b/test/elli_openapi_integration_SUITE.erl @@ -28,10 +28,15 @@ update_item_not_found_404/1, update_item_invalid_version_400/1, update_item_conflict_409/1, + list_users_no_query_params/1, + list_users_with_query_params/1, + list_users_invalid_query_param/1, openapi_spec_includes_response_headers/1, openapi_spec_content_types/1, openapi_spec_multi_status/1, openapi_spec_get_no_request_body/1, + openapi_spec_query_params/1, + openapi_spec_function_doc/1, swagger_ui_endpoint/1, redoc_endpoint/1, api_docs_endpoint/1 @@ -59,10 +64,15 @@ all() -> update_item_not_found_404, update_item_invalid_version_400, update_item_conflict_409, + list_users_no_query_params, + list_users_with_query_params, + list_users_invalid_query_param, openapi_spec_includes_response_headers, openapi_spec_content_types, openapi_spec_multi_status, openapi_spec_get_no_request_body, + openapi_spec_query_params, + openapi_spec_function_doc, swagger_ui_endpoint, redoc_endpoint, api_docs_endpoint @@ -75,11 +85,12 @@ init_per_suite(Config) -> 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}, - {<<"POST">>, <<"/api/status">>, fun elli_openapi_demo:update_status/3}, - {<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/3} + {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/4}, + {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/4}, + {<<"POST">>, <<"/api/echo">>, fun elli_openapi_demo:echo_text/4}, + {<<"POST">>, <<"/api/status">>, fun elli_openapi_demo:update_status/4}, + {<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/4}, + {<<"GET">>, <<"/api/users">>, fun elli_openapi_demo:list_users/4} ], Port = 8765, @@ -268,7 +279,7 @@ get_user_missing_auth_header(Config) -> ok. get_user_not_found(Config) -> - Url = url(Config, "/api/users"), + Url = url(Config, "/api/nonexistent"), ?assertMatch( {ok, {{_, 404, _}, _Headers, _ResponseBody}}, @@ -389,9 +400,9 @@ update_item_conflict_409(Config) -> openapi_spec_includes_response_headers(_Config) -> Routes = [ - {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/3}, - {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/3}, - {<<"POST">>, <<"/api/status">>, fun elli_openapi_demo:update_status/3} + {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/4}, + {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/4}, + {<<"POST">>, <<"/api/status">>, fun elli_openapi_demo:update_status/4} ], MetaData = #{title => ~"Test API", version => ~"1.0.0"}, @@ -446,10 +457,10 @@ openapi_spec_includes_response_headers(_Config) -> openapi_spec_content_types(_Config) -> 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}, - {<<"POST">>, <<"/api/status">>, fun elli_openapi_demo:update_status/3} + {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/4}, + {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/4}, + {<<"POST">>, <<"/api/echo">>, fun elli_openapi_demo:echo_text/4}, + {<<"POST">>, <<"/api/status">>, fun elli_openapi_demo:update_status/4} ], MetaData = #{title => ~"Test API", version => ~"1.0.0"}, @@ -488,7 +499,7 @@ openapi_spec_content_types(_Config) -> ok. openapi_spec_multi_status(_Config) -> - Routes = [{<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/3}], + Routes = [{<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/4}], MetaData = #{title => ~"Test API", version => ~"1.0.0"}, {ok, Spec} = elli_openapi:generate_openapi_spec(MetaData, Routes), @@ -530,8 +541,8 @@ openapi_spec_multi_status(_Config) -> 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} + {<<"GET">>, <<"/api/users/{userId}">>, fun elli_openapi_demo:get_user/4}, + {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/4} ], MetaData = #{title => ~"Test API", version => ~"1.0.0"}, @@ -551,6 +562,82 @@ openapi_spec_get_no_request_body(_Config) -> ok. +%%==================================================================== +%% Test Cases - Query Parameters +%%==================================================================== + +list_users_no_query_params(Config) -> + Url = url(Config, "/api/users"), + + ?assertMatch( + {ok, {{_, 200, _}, _Headers, _ResponseBody}}, + http_get(Url) + ), + ok. + +list_users_with_query_params(Config) -> + Url = url(Config, "/api/users?page=2&per_page=5"), + + ?assertMatch( + {ok, {{_, 200, _}, _Headers, _ResponseBody}}, + http_get(Url) + ), + ok. + +list_users_invalid_query_param(Config) -> + Url = url(Config, "/api/users?page=notanumber"), + + ?assertMatch( + {ok, {{_, 400, _}, _Headers, _ResponseBody}}, + http_get(Url) + ), + ok. + +%%==================================================================== +%% Test Cases - OpenAPI Spec: Query Params and Function Docs +%%==================================================================== + +openapi_spec_query_params(_Config) -> + Routes = [{<<"GET">>, <<"/api/users">>, fun elli_openapi_demo:list_users/4}], + + MetaData = #{title => ~"Test API", version => ~"1.0.0"}, + {ok, Spec} = elli_openapi:generate_openapi_spec(MetaData, Routes), + + #{<<"paths">> := #{<<"/api/users">> := #{<<"get">> := GetEndpoint}}} = Spec, + #{<<"parameters">> := Params} = GetEndpoint, + + ParamNames = [maps:get(<<"name">>, P) || P <- Params], + ?assert(lists:member(<<"page">>, ParamNames)), + ?assert(lists:member(<<"per_page">>, ParamNames)), + + [PageParam] = [P || P <- Params, maps:get(<<"name">>, P) =:= <<"page">>], + ?assertEqual(<<"query">>, maps:get(<<"in">>, PageParam)), + ?assertEqual(false, maps:get(<<"required">>, PageParam)), + + ok. + +openapi_spec_function_doc(_Config) -> + Routes = [ + {<<"POST">>, <<"/api/users">>, fun elli_openapi_demo:create_user/4}, + {<<"GET">>, <<"/api/users">>, fun elli_openapi_demo:list_users/4} + ], + + MetaData = #{title => ~"Test API", version => ~"1.0.0"}, + {ok, Spec} = elli_openapi:generate_openapi_spec(MetaData, Routes), + + ?assertMatch( + #{ + <<"paths">> := #{ + <<"/api/users">> := #{ + <<"post">> := #{<<"summary">> := <<"Create a new user">>}, + <<"get">> := #{<<"summary">> := <<"List users">>} + } + } + }, + Spec + ), + ok. + %%==================================================================== %% Test Cases - Documentation Endpoints %%==================================================================== From 29c81a31a7c1bf3f69ac8184de06a6a363980ec3 Mon Sep 17 00:00:00 2001 From: Andreas Hasselberg Date: Sun, 22 Mar 2026 21:39:57 +0100 Subject: [PATCH 2/3] Add search_users handler and missing required query param test - Add search_users/4 with required query param `query := binary()` - Add GET /api/search route to integration test suite - Add search_users_missing_required_query_param test (verifies 400 when required query param is absent) Co-Authored-By: Claude Sonnet 4.6 --- src/elli_openapi.erl | 11 ++++++++- src/elli_openapi_demo.erl | 32 ++++++++++++++++++++----- test/elli_openapi_integration_SUITE.erl | 27 +++++++++++++++------ 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/elli_openapi.erl b/src/elli_openapi.erl index 49bef46..6124d6b 100644 --- a/src/elli_openapi.erl +++ b/src/elli_openapi.erl @@ -525,7 +525,16 @@ join_function_specs( request_content_type = infer_content_type(Body), responses = Responses, doc = Doc - }. + }; +join_function_specs({Module, Function, Arity}, [#sp_function_spec{args = Args}]) -> + erlang:error( + {handler_wrong_arity, #{ + mfa => {Module, Function, Arity}, + expected_args => 4, + got_args => length(Args), + hint => ~"Handler spec must be: (PathArgs, QueryArgs, Headers, Body) -> Response" + }} + ). %% Extract response specifications from return type %% Handles both single tuple: {200, Headers, Body} diff --git a/src/elli_openapi_demo.erl b/src/elli_openapi_demo.erl index 73af1ac..d757fcf 100644 --- a/src/elli_openapi_demo.erl +++ b/src/elli_openapi_demo.erl @@ -1,8 +1,24 @@ -module(elli_openapi_demo). --export([create_user/4, get_user/4, echo_text/4, update_status/4, update_item/4, list_users/4]). - --ignore_xref([create_user/4, get_user/4, echo_text/4, update_status/4, update_item/4, list_users/4]). +-export([ + create_user/4, + get_user/4, + echo_text/4, + update_status/4, + update_item/4, + list_users/4, + search_users/4 +]). + +-ignore_xref([ + create_user/4, + get_user/4, + echo_text/4, + update_status/4, + update_item/4, + list_users/4, + search_users/4 +]). -compile(nowarn_unused_type). @@ -76,6 +92,8 @@ ) -> {200, #{}, #user_list{}}. +-spec search_users(#{}, #{query := binary()}, #{}, binary()) -> {200, #{}, #user_list{}}. + %%==================================================================== %% Function definitions %%==================================================================== @@ -138,10 +156,12 @@ update_item(#{itemId := ItemId}, #{}, #{}, #{name := Name, version := Version}) end. list_users(#{}, QueryParams, #{}, ~"") -> - _Page = maps:get(page, QueryParams, 1), - _PerPage = maps:get(per_page, QueryParams, 20), + PerPage = maps:get(per_page, QueryParams, 20), Users = [ #user{id = <<"user-1">>, email = <<"a@example.com">>, name = <<"Alice">>, role = admin}, #user{id = <<"user-2">>, email = <<"b@example.com">>, name = <<"Bob">>, role = user} ], - {200, #{}, #user_list{users = Users, total = 2}}. + {200, #{}, #user_list{users = Users, total = PerPage}}. + +search_users(#{}, #{query := _Query}, #{}, ~"") -> + {200, #{}, #user_list{users = [], total = 0}}. diff --git a/test/elli_openapi_integration_SUITE.erl b/test/elli_openapi_integration_SUITE.erl index 68a13a3..edb473d 100644 --- a/test/elli_openapi_integration_SUITE.erl +++ b/test/elli_openapi_integration_SUITE.erl @@ -18,7 +18,7 @@ create_user_invalid_role/1, get_user_success/1, get_user_missing_auth_header/1, - get_user_not_found/1, + unknown_route_returns_404/1, create_user_empty_body/1, create_user_wrong_content_type/1, update_status_success/1, @@ -31,6 +31,7 @@ list_users_no_query_params/1, list_users_with_query_params/1, list_users_invalid_query_param/1, + search_users_missing_required_query_param/1, openapi_spec_includes_response_headers/1, openapi_spec_content_types/1, openapi_spec_multi_status/1, @@ -56,7 +57,7 @@ all() -> create_user_wrong_content_type, get_user_success, get_user_missing_auth_header, - get_user_not_found, + unknown_route_returns_404, update_status_success, update_status_invalid_value, update_status_wrong_content_type, @@ -67,6 +68,7 @@ all() -> list_users_no_query_params, list_users_with_query_params, list_users_invalid_query_param, + search_users_missing_required_query_param, openapi_spec_includes_response_headers, openapi_spec_content_types, openapi_spec_multi_status, @@ -90,7 +92,8 @@ init_per_suite(Config) -> {<<"POST">>, <<"/api/echo">>, fun elli_openapi_demo:echo_text/4}, {<<"POST">>, <<"/api/status">>, fun elli_openapi_demo:update_status/4}, {<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/4}, - {<<"GET">>, <<"/api/users">>, fun elli_openapi_demo:list_users/4} + {<<"GET">>, <<"/api/users">>, fun elli_openapi_demo:list_users/4}, + {<<"GET">>, <<"/api/search">>, fun elli_openapi_demo:search_users/4} ], Port = 8765, @@ -278,7 +281,7 @@ get_user_missing_auth_header(Config) -> ), ok. -get_user_not_found(Config) -> +unknown_route_returns_404(Config) -> Url = url(Config, "/api/nonexistent"), ?assertMatch( @@ -578,14 +581,24 @@ list_users_no_query_params(Config) -> list_users_with_query_params(Config) -> Url = url(Config, "/api/users?page=2&per_page=5"), + {ok, {{_, 200, _}, _Headers, ResponseBody}} = http_get(Url), + + %% Verify the decoded per_page value (integer 5) reached the handler — + %% list_users echoes it as total, confirming key matching and type decoding + ?assertMatch(#{~"total" := 5}, json:decode(list_to_binary(ResponseBody))), + ok. + +list_users_invalid_query_param(Config) -> + Url = url(Config, "/api/users?page=notanumber"), + ?assertMatch( - {ok, {{_, 200, _}, _Headers, _ResponseBody}}, + {ok, {{_, 400, _}, _Headers, _ResponseBody}}, http_get(Url) ), ok. -list_users_invalid_query_param(Config) -> - Url = url(Config, "/api/users?page=notanumber"), +search_users_missing_required_query_param(Config) -> + Url = url(Config, "/api/search"), ?assertMatch( {ok, {{_, 400, _}, _Headers, _ResponseBody}}, From 4ec8f59e90af5bcb3dc05e1a548fbc446a04a204 Mon Sep 17 00:00:00 2001 From: Andreas Hasselberg Date: Sun, 22 Mar 2026 21:41:34 +0100 Subject: [PATCH 3/3] Simplify check_types using maybe expression Replace nested case pyramid with a flat maybe block. Co-Authored-By: Claude Sonnet 4.6 --- src/elli_openapi.erl | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/src/elli_openapi.erl b/src/elli_openapi.erl index 6124d6b..f6d54f3 100644 --- a/src/elli_openapi.erl +++ b/src/elli_openapi.erl @@ -159,34 +159,14 @@ check_types(HandlerType, PathArgs, ElliRequest) -> header_args = HeadersType, request_body = RequestBodyType, request_content_type = RequestContentType - } = - HandlerType, - - case decode_path_args(Module, PathArgs, PathArgsType) of - {ok, DecodedPathArgs} -> - case decode_query_args(Module, QueryArgsType, ElliRequest) of - {ok, DecodedQueryArgs} -> - case decode_headers(Module, HeadersType, elli_request:headers(ElliRequest)) of - {ok, DecodedHeaders} -> - case - decode_body( - Module, RequestBodyType, RequestContentType, ElliRequest - ) - of - {ok, DecodedBody} -> - {ok, DecodedPathArgs, DecodedQueryArgs, DecodedHeaders, - DecodedBody}; - {error, _} = Error -> - Error - end; - {error, _} = Error -> - Error - end; - {error, _} = Error -> - Error - end; - {error, _} = Error -> - Error + } = HandlerType, + maybe + {ok, DecodedPathArgs} ?= decode_path_args(Module, PathArgs, PathArgsType), + {ok, DecodedQueryArgs} ?= decode_query_args(Module, QueryArgsType, ElliRequest), + {ok, DecodedHeaders} ?= + decode_headers(Module, HeadersType, elli_request:headers(ElliRequest)), + {ok, DecodedBody} ?= decode_body(Module, RequestBodyType, RequestContentType, ElliRequest), + {ok, DecodedPathArgs, DecodedQueryArgs, DecodedHeaders, DecodedBody} end. decode_body(Module, RequestBodyType, ExpectedContentType, ElliRequest) ->