From 6b13d49db54ff530982fa0c24d572e4e3a8e2c27 Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Tue, 19 May 2026 22:59:42 +0200 Subject: [PATCH] refactor: default to OTP json, drop thoas dependency Switch the default json_lib from thoas to the OTP json module and remove thoas from the dependency list. The json_lib config remains so users can plug in any module that follows the OTP contract: encode/1 -> binary() | iodata(), decode/1 -> Term (raises on bad input). - nova_request_plugin: wrap decode/1 in try/catch to keep 400 responses on malformed JSON. - nova_jsonlogger: wrap encode/2 output in iolist_to_binary/1 so the formatter still returns a binary regardless of backend. - Update inline tests to the OTP json contract (no {ok, _} wrapper). - Update configuration guide to document the new default and contract. --- guides/configuration.md | 6 +-- rebar.config | 3 +- rebar.lock | 9 ++-- src/controllers/nova_error_controller.erl | 6 +-- src/nova.app.src | 3 +- src/nova_basic_handler.erl | 4 +- src/nova_jsonlogger.erl | 56 ++++++++++------------- src/plugins/nova_request_plugin.erl | 13 +++--- test/nova_error_controller_tests.erl | 2 +- test/nova_test_helper.erl | 4 +- 10 files changed, 48 insertions(+), 58 deletions(-) diff --git a/guides/configuration.md b/guides/configuration.md index f0e26085..7931c17f 100644 --- a/guides/configuration.md +++ b/guides/configuration.md @@ -50,11 +50,11 @@ These parameters can be specified in your *main* application (Eg the one you've ### Configure json_lib -One can configure which json library to use for encoding/decoding json structures. The module defined for this should expose two different functions: +Nova uses the OTP `json` module by default. You can override it by setting `json_lib` to any module that follows the same contract: -`encode(Structure) -> binary() | iolist()` +`encode(Structure) -> binary() | iodata()` -`decode(JsonString) -> {ok, Structure}` +`decode(Binary) -> Structure` (raises on invalid input) ## Handling errors in Nova diff --git a/rebar.config b/rebar.config index 07107efe..48c97978 100644 --- a/rebar.config +++ b/rebar.config @@ -14,8 +14,7 @@ {cowboy, "2.15.0"}, {erlydtl, "0.14.0"}, {jhn_stdlib, "5.11.2"}, - {routing_tree, "1.0.11"}, - {thoas, "1.2.1"} + {routing_tree, "1.0.11"} ]}. {profiles, [ diff --git a/rebar.lock b/rebar.lock index 17476a5f..132afadb 100644 --- a/rebar.lock +++ b/rebar.lock @@ -4,8 +4,7 @@ {<<"erlydtl">>,{pkg,<<"erlydtl">>,<<"0.14.0">>},0}, {<<"jhn_stdlib">>,{pkg,<<"jhn_stdlib">>,<<"5.11.2">>},0}, {<<"ranch">>,{pkg,<<"ranch">>,<<"2.2.0">>},1}, - {<<"routing_tree">>,{pkg,<<"routing_tree">>,<<"1.0.11">>},0}, - {<<"thoas">>,{pkg,<<"thoas">>,<<"1.2.1">>},0}]}. + {<<"routing_tree">>,{pkg,<<"routing_tree">>,<<"1.0.11">>},0}]}. [ {pkg_hash,[ {<<"cowboy">>, <<"9CFE86ED7117BF045E10ADBEDB0170AF7BE57F2A3637E7BE143433D8DD267396">>}, @@ -13,14 +12,12 @@ {<<"erlydtl">>, <<"964B2DC84F8C17ACFAA69C59BA129EF26AC45D2BA898C3C6AD9B5BDC8BA13CED">>}, {<<"jhn_stdlib">>, <<"785074F3CA368EAA8E9AF1592BC19AE9EF1F7AF30B2CD6456A6083173A8F5CCB">>}, {<<"ranch">>, <<"25528F82BC8D7C6152C57666CA99EC716510FE0925CB188172F41CE93117B1B0">>}, - {<<"routing_tree">>, <<"72ACEF2095F0EC804F7AFD07EF781DDE5009425A1CA0A28F0706B1DB334A4812">>}, - {<<"thoas">>, <<"19A25F31177A17E74004D4840F66D791D4298C5738790FA2CC73731EB911F195">>}]}, + {<<"routing_tree">>, <<"72ACEF2095F0EC804F7AFD07EF781DDE5009425A1CA0A28F0706B1DB334A4812">>}]}, {pkg_hash_ext,[ {<<"cowboy">>, <<"179FB65140FB440A17B767AD53B755081506F9596C4DB5C49C0396D8C8643668">>}, {<<"cowlib">>, <<"58F1E425A9E04176F1D30E20116F57C4E90EF0E187552E9741C465BDF4044F70">>}, {<<"erlydtl">>, <<"D80EC044CD8F58809C19D29AC5605BE09E955040911B644505E31E9DD8143431">>}, {<<"jhn_stdlib">>, <<"2329CD16DEE46704AAB6184D09508E59DBA31C4D3255271DBB7D34D115ECA508">>}, {<<"ranch">>, <<"FA0B99A1780C80218A4197A59EA8D3BDAE32FBFF7E88527D7D8A4787EFF4F8E7">>}, - {<<"routing_tree">>, <<"85982C7AC502892C5179CD2A591331003BACD2D2A71723640BA7D23F45408E6E">>}, - {<<"thoas">>, <<"E38697EDFFD6E91BD12CEA41B155115282630075C2A727E7A6B2947F5408B86A">>}]} + {<<"routing_tree">>, <<"85982C7AC502892C5179CD2A591331003BACD2D2A71723640BA7D23F45408E6E">>}]} ]. diff --git a/src/controllers/nova_error_controller.erl b/src/controllers/nova_error_controller.erl index b3fb4645..56ea77eb 100644 --- a/src/controllers/nova_error_controller.erl +++ b/src/controllers/nova_error_controller.erl @@ -23,7 +23,7 @@ not_found(Req) -> lists:member(<<"text/html">>, AcceptList)} of {true, _} -> %% Render a json response - JsonLib = nova:get_env(json_lib, thoas), + JsonLib = nova:get_env(json_lib, json), Json = JsonLib:encode(#{message => "Resource not found"}), {status, 404, #{<<"content-type">> => <<"application/json">>}, Json}; {_, true} -> @@ -47,7 +47,7 @@ server_error(#{crash_info := #{status_code := StatusCode} = CrashInfo} = Req) -> true -> case cowboy_req:header(<<"accept">>, Req) of <<"application/json">> -> - JsonLib = nova:get_env(json_lib, thoas), + JsonLib = nova:get_env(json_lib, json), Json = JsonLib:encode(Variables), {status, StatusCode, #{<<"content-type">> => <<"application/json">>}, Json}; <<"text/html">> -> @@ -72,7 +72,7 @@ server_error(#{crash_info := #{class := Class, reason := Reason}} = Req) -> %% We do show a proper error response case cowboy_req:header(<<"accept">>, Req) of <<"application/json">> -> - JsonLib = nova:get_env(json_lib, thoas), + JsonLib = nova:get_env(json_lib, json), Json = JsonLib:encode(Variables), {status, 500, #{<<"content-type">> => <<"application/json">>}, Json}; <<"text/html">> -> diff --git a/src/nova.app.src b/src/nova.app.src index 2d026633..458aa28b 100644 --- a/src/nova.app.src +++ b/src/nova.app.src @@ -13,8 +13,7 @@ compiler, erlydtl, jhn_stdlib, - routing_tree, - thoas + routing_tree ]}, {env,[]}, {modules,[nova]}, diff --git a/src/nova_basic_handler.erl b/src/nova_basic_handler.erl index 77aae11d..d85d4ca2 100644 --- a/src/nova_basic_handler.erl +++ b/src/nova_basic_handler.erl @@ -43,7 +43,7 @@ handle_json({json, StatusCode, Headers, Req0, JSON}, Callback, _Req) -> handle_json({json, StatusCode, Headers, JSON}, Callback, Req0); handle_json({json, StatusCode, Headers, JSON}, _Callback, Req) -> - JsonLib = nova:get_env(json_lib, thoas), + JsonLib = nova:get_env(json_lib, json), EncodedJSON = JsonLib:encode(JSON), Headers0 = maps:merge(#{<<"content-type">> => <<"application/json">>}, Headers), Req0 = cowboy_req:set_resp_headers(Headers0, Req), @@ -154,7 +154,7 @@ handle_status({status, Status, ExtraHeaders, JSON, Req0}, Callback, _Req) -> handle_status({status, Status, ExtraHeaders, JSON}, Callback, Req0); handle_status({status, Status, ExtraHeaders, JSON}, _Callback, Req) when is_map(JSON) -> %% We do not need to render a status page since we just return a JSON structure - JsonLib = nova:get_env(json_lib, thoas), + JsonLib = nova:get_env(json_lib, json), Headers0 = maps:merge(#{<<"content-type">> => <<"application/json">>}, ExtraHeaders), Req0 = cowboy_req:set_resp_headers(Headers0, Req), Req1 = Req0#{resp_status_code => Status}, diff --git a/src/nova_jsonlogger.erl b/src/nova_jsonlogger.erl index 0f2fe821..093d9fa4 100644 --- a/src/nova_jsonlogger.erl +++ b/src/nova_jsonlogger.erl @@ -85,7 +85,7 @@ merge_meta(Msg, Meta0, Config) -> maps:merge(Msg, Meta2). encode(Data, Config) -> - JsonLib = nova:get_env(json_lib, thoas), + JsonLib = nova:get_env(json_lib, json), Json = JsonLib:encode(Data), case new_line(Config) of true -> [Json, new_line_type(Config)]; @@ -162,7 +162,8 @@ meta_with(Meta, _ConfigNotPresent) -> -include_lib("eunit/include/eunit.hrl"). -define(assertJSONEqual(Expected, Actual), - ?assertEqual(thoas:decode(Expected), thoas:decode(Actual)) + ?assertEqual(json:decode(iolist_to_binary(Expected)), + json:decode(iolist_to_binary(Actual))) ). format_test() -> @@ -260,20 +261,20 @@ meta_without_test() -> meta => #{secret => xyz} }, ?assertEqual( - {ok, #{ + #{ <<"answer">> => 42, <<"level">> => <<"info">>, <<"secret">> => <<"xyz">> - }}, - thoas:decode(format(Error, #{})) + }, + json:decode(iolist_to_binary(format(Error, #{}))) ), Config2 = #{meta_without => [secret]}, ?assertEqual( - {ok, #{ + #{ <<"answer">> => 42, <<"level">> => <<"info">> - }}, - thoas:decode(format(Error, Config2)) + }, + json:decode(iolist_to_binary(format(Error, Config2))) ), ok. @@ -284,59 +285,52 @@ meta_with_test() -> meta => #{secret => xyz} }, ?assertEqual( - {ok, #{ + #{ <<"answer">> => 42, <<"level">> => <<"info">>, <<"secret">> => <<"xyz">> - }}, - thoas:decode(format(Error, #{})) + }, + json:decode(iolist_to_binary(format(Error, #{}))) ), Config2 = #{meta_with => [level]}, ?assertEqual( - {ok, #{ + #{ <<"answer">> => 42, <<"level">> => <<"info">> - }}, - thoas:decode(format(Error, Config2)) + }, + json:decode(iolist_to_binary(format(Error, Config2))) ), ok. newline_test() -> ConfigDefault = #{new_line => true}, ?assertEqual( - [<<"{\"level\":\"alert\",\"text\":\"derp\"}">>, <<"\n">>], - format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigDefault) + <<"{\"level\":\"alert\",\"text\":\"derp\"}\n">>, + iolist_to_binary(format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigDefault)) ), ConfigCRLF = #{ new_line_type => crlf, new_line => true }, ?assertEqual( - [<<"{\"level\":\"alert\",\"text\":\"derp\"}">>, <<"\r\n">>], - format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigCRLF) + <<"{\"level\":\"alert\",\"text\":\"derp\"}\r\n">>, + iolist_to_binary(format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigCRLF)) ). newline_types_test() -> Base = #{level => alert, msg => {string, "x"}, meta => #{}}, - %% nl ?assertEqual(<<"\n">>, lists:last(format(Base, #{new_line => true, new_line_type => nl}))), - %% unix ?assertEqual(<<"\n">>, lists:last(format(Base, #{new_line => true, new_line_type => unix}))), - %% crlf ?assertEqual(<<"\r\n">>, lists:last(format(Base, #{new_line => true, new_line_type => crlf}))), - %% windows ?assertEqual(<<"\r\n">>, lists:last(format(Base, #{new_line => true, new_line_type => windows}))), - %% cr ?assertEqual(<<"\r">>, lists:last(format(Base, #{new_line => true, new_line_type => cr}))), - %% macos9 ?assertEqual(<<"\r">>, lists:last(format(Base, #{new_line => true, new_line_type => macos9}))), - %% default (no new_line_type specified) ?assertEqual(<<"\n">>, lists:last(format(Base, #{new_line => true}))). no_newline_test() -> Base = #{level => alert, msg => {string, "x"}, meta => #{}}, Result = format(Base, #{}), - ?assert(is_binary(Result)). + ?assert(is_binary(iolist_to_binary(Result))). jsonify_types_test() -> %% atom @@ -365,29 +359,29 @@ jsonify_types_test() -> format_keyval_list_test() -> Event = #{level => info, msg => {report, [{key, <<"val">>}]}, meta => #{}}, - {ok, Decoded} = thoas:decode(format(Event, #{})), + Decoded = json:decode(iolist_to_binary(format(Event, #{}))), ?assertEqual(<<"val">>, maps:get(<<"key">>, Decoded)). format_format_terms_test() -> Event = #{level => info, msg => {"hello ~s", ["world"]}, meta => #{}}, - {ok, Decoded} = thoas:decode(format(Event, #{})), + Decoded = json:decode(iolist_to_binary(format(Event, #{}))), ?assertEqual(<<"hello world">>, maps:get(<<"text">>, Decoded)). format_error_logger_test() -> Event = #{level => error, msg => {report, #{format => "error: ~p", args => [bad], label => {error_logger, error}}}, meta => #{}}, - {ok, Decoded} = thoas:decode(format(Event, #{})), + Decoded = json:decode(iolist_to_binary(format(Event, #{}))), ?assert(maps:is_key(<<"text">>, Decoded)). nested_map_test() -> Event = #{level => info, msg => {report, #{outer => #{inner => value}}}, meta => #{}}, - {ok, Decoded} = thoas:decode(format(Event, #{})), + Decoded = json:decode(iolist_to_binary(format(Event, #{}))), ?assertEqual(#{<<"inner">> => <<"value">>}, maps:get(<<"outer">>, Decoded)). list_of_maps_test() -> Event = #{level => info, msg => {report, #{items => [#{a => 1}, #{b => 2}]}}, meta => #{}}, - {ok, Decoded} = thoas:decode(format(Event, #{})), + Decoded = json:decode(iolist_to_binary(format(Event, #{}))), ?assertEqual([#{<<"a">> => 1}, #{<<"b">> => 2}], maps:get(<<"items">>, Decoded)). system_time_to_iso8601_test() -> diff --git a/src/plugins/nova_request_plugin.erl b/src/plugins/nova_request_plugin.erl index 7421d68d..5e1ae2a8 100644 --- a/src/plugins/nova_request_plugin.erl +++ b/src/plugins/nova_request_plugin.erl @@ -78,15 +78,16 @@ modulate_state(Req = #{headers := #{<<"content-type">> := <<"application/json", {stop, Req400, State}; modulate_state(Req = #{headers := #{<<"content-type">> := <<"application/json", _/binary>>}, body := Body}, [{decode_json_body, true}|Tl], State) -> %% Decode the data - JsonLib = nova:get_env(json_lib, thoas), - case JsonLib:decode(Body) of - {ok, JSON} -> - modulate_state(Req#{json => JSON}, Tl, State); - Error -> + JsonLib = nova:get_env(json_lib, json), + try JsonLib:decode(Body) of + JSON -> + modulate_state(Req#{json => JSON}, Tl, State) + catch + Class:Reason -> Req400 = cowboy_req:reply(400, Req), logger:warning(#{status_code => 400, msg => "Failed to decode json.", - error => Error}), + error => {Class, Reason}}), {stop, Req400, State} end; modulate_state(#{headers := #{<<"content-type">> := <<"application/x-www-form-urlencoded", _/binary>>}, body := Body} = Req, diff --git a/test/nova_error_controller_tests.erl b/test/nova_error_controller_tests.erl index 159b0660..e425674e 100644 --- a/test/nova_error_controller_tests.erl +++ b/test/nova_error_controller_tests.erl @@ -69,7 +69,7 @@ not_found_json_accept_test_() -> Req1 = nova_test_helper:with_header(<<"accept">>, <<"application/json">>, Req), {status, 404, Headers, Body} = nova_error_controller:not_found(Req1), ?assertEqual(<<"application/json">>, maps:get(<<"content-type">>, Headers)), - ?assert(is_binary(Body)) + ?assert(is_binary(iolist_to_binary(Body))) end}. %% When no accept header, defaults to JSON diff --git a/test/nova_test_helper.erl b/test/nova_test_helper.erl index bf7ca42f..92e6e385 100644 --- a/test/nova_test_helper.erl +++ b/test/nova_test_helper.erl @@ -32,7 +32,7 @@ with_header(Name, Value, Req = #{headers := Headers}) -> -spec with_json_body(map() | binary(), map()) -> map(). with_json_body(JSON, Req) when is_map(JSON) -> - Body = thoas:encode(JSON), + Body = iolist_to_binary(json:encode(JSON)), with_json_body(Body, Req); with_json_body(Body, Req) when is_binary(Body) -> Req1 = with_content_type(<<"application/json">>, Req), @@ -61,7 +61,7 @@ setup_nova_env() -> Prev = application:get_env(nova, bootstrap_application), application:set_env(nova, bootstrap_application, nova), application:set_env(nova, environment, dev), - application:set_env(nova, json_lib, thoas), + application:set_env(nova, json_lib, json), Prev. -spec cleanup_nova_env(undefined | {ok, atom()}) -> ok.