Replace "jsx" with built-in "json" module (#163)

Co-authored-by: Ivan Bakalov <ivan.bakalov@dnsimple.com>
dnsimple · Dec 19, 2024 · 08f1a8a · 08f1a8a
1 parent 324ab10
commit 08f1a8a
Show file tree

Hide file tree

Showing 10 changed files with 98 additions and 54 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -16,7 +16,7 @@ jobs:
     runs-on: ubuntu-22.04
     strategy:
       matrix:
-        otp_version: ['27.1', '26.2', '25.3']
+        otp_version: ['27.1']
 
     steps:
       - uses: actions/checkout@v4
@@ -40,7 +40,7 @@ jobs:
     runs-on: ubuntu-22.04
     strategy:
       matrix:
-        otp_version: ['27.1', '26.2', '25.3']
+        otp_version: ['27.1']
 
     needs:
       - build

diff --git a/rebar.config b/rebar.config
@@ -14,7 +14,6 @@
     {lager, "3.9.2"},
     recon,
     folsom,
-    {jsx, "3.1.0"},
     {dns_erlang, ".*", {git, "https://github.com/dnsimple/dns_erlang.git", {branch, "main"}}},
     iso8601,
     {nodefinder, "2.0.7"},

diff --git a/rebar.lock b/rebar.lock
@@ -8,7 +8,6 @@
  {<<"folsom">>,{pkg,<<"folsom">>,<<"1.0.0">>},0},
  {<<"goldrush">>,{pkg,<<"goldrush">>,<<"0.1.9">>},1},
  {<<"iso8601">>,{pkg,<<"iso8601">>,<<"1.3.4">>},0},
- {<<"jsx">>,{pkg,<<"jsx">>,<<"3.0.0">>},0},
  {<<"lager">>,{pkg,<<"lager">>,<<"3.9.2">>},0},
  {<<"meck">>,{pkg,<<"meck">>,<<"0.9.2">>},0},
  {<<"nodefinder">>,{pkg,<<"nodefinder">>,<<"2.0.0">>},0},
@@ -21,7 +20,6 @@
  {<<"folsom">>, <<"50ECC998D2149939F1D5E0AA3E32788F8ED16A58E390D81B5C0BE4CC4EF25589">>},
  {<<"goldrush">>, <<"F06E5D5F1277DA5C413E84D5A2924174182FB108DABB39D5EC548B27424CD106">>},
  {<<"iso8601">>, <<"7B1F095F86F6CF65E1E5A77872E8E8BF69BD58D4C3A415B3F77D9CC9423ECBB9">>},
- {<<"jsx">>, <<"20A170ABD4335FC6DB24D5FAD1E5D677C55DADF83D1B20A8A33B5FE159892A39">>},
  {<<"lager">>, <<"4CAB289120EB24964E3886BD22323CB5FEFE4510C076992A23AD18CF85413D8C">>},
  {<<"meck">>, <<"85CCBAB053F1DB86C7CA240E9FC718170EE5BDA03810A6292B5306BF31BAE5F5">>},
  {<<"nodefinder">>, <<"1FA140D4C3A27FF66623E267E66876AF18817BA82AD303E4AE46ADBC941851AA">>},
@@ -33,7 +31,6 @@
  {<<"folsom">>, <<"DD6AB97278E94F9E4CFC43E188224A7B8C7EAEC0DD2E935007005177F3EEBB0E">>},
  {<<"goldrush">>, <<"99CB4128CFFCB3227581E5D4D803D5413FA643F4EB96523F77D9E6937D994CEB">>},
  {<<"iso8601">>, <<"A334469C07F1C219326BC891A95F5EEC8EB12DD8071A3FFF56A7843CB20FAE34">>},
- {<<"jsx">>, <<"37BECA0435F5CA8A2F45F76A46211E76418FBEF80C36F0361C249FC75059DC6D">>},
  {<<"lager">>, <<"7F904D9E87A8CB7E66156ED31768D1C8E26EBA1D54F4BC85B1AA4AC1F6340C28">>},
  {<<"meck">>, <<"81344F561357DC40A8344AFA53767C32669153355B626EA9FCBC8DA6B3045826">>},
  {<<"nodefinder">>, <<"10A31C59A4249BFF181773AC77390A4416AA0EEE93C33C716A113419EBF3B210">>},

diff --git a/src/erldns.app.src b/src/erldns.app.src
@@ -11,7 +11,6 @@
         crypto,
         lager,
         dns_erlang,
-        jsx,
         ssl,
         mnesia,
         bear,

diff --git a/src/erldns_storage.erl b/src/erldns_storage.erl
@@ -170,7 +170,15 @@ load_zones(Filename) when is_list(Filename) ->
     case file:read_file(Filename) of
         {ok, Binary} ->
             lager:debug("Parsing zones JSON"),
-            JsonZones = jsx:decode(Binary, [{return_maps, false}]),
+
+            %% The "object_finish" callback override is here so that we don't return maps
+            %% and instead keep the list of decoded information as a keyword list. Maybe we
+            %% won't need this behavior in the future, but for now it's a good way to keep
+            %% compatibility with what we used before to decode JSON (jsx).
+            {JsonZones, ok, _} = json:decode(Binary, ok, #{
+                object_finish => fun(Acc, OldAcc) -> {lists:reverse(Acc), OldAcc} end
+            }),
+
             lager:debug("Putting zones into cache"),
             lists:foreach(
                 fun(JsonZone) ->

diff --git a/src/erldns_zone_encoder.erl b/src/erldns_zone_encoder.erl
@@ -55,7 +55,7 @@ start_link() ->
 %% @doc Encode a Zone meta data into JSON.
 -spec zone_meta_to_json(#zone{}) -> binary().
 zone_meta_to_json(Zone) ->
-    jsx:encode([
+    json_encode_kw_list([
         {<<"erldns">>, [
             {<<"zone">>, [
                 {<<"name">>, Zone#zone.name},
@@ -126,7 +126,7 @@ code_change(_, State, _) ->
 encode_zone_to_json(Zone, Encoders) ->
     Records = records_to_json(Zone, Encoders),
     FilteredRecords = lists:filter(record_filter(), Records),
-    jsx:encode([
+    json_encode_kw_list([
         {<<"erldns">>, [
             {<<"zone">>, [
                 {<<"name">>, Zone#zone.name},
@@ -140,11 +140,11 @@ encode_zone_to_json(Zone, Encoders) ->
 
 encode_zone_records_to_json(_ZoneName, RecordName, Encoders) ->
     Records = erldns_zone_cache:get_records_by_name(RecordName),
-    jsx:encode(lists:filter(record_filter(), lists:map(encode(Encoders), Records))).
+    json_encode_kw_list(lists:filter(record_filter(), lists:map(encode(Encoders), Records))).
 
 encode_zone_records_to_json(_ZoneName, RecordName, RecordType, Encoders) ->
     Records = erldns_zone_cache:get_records_by_name_and_type(RecordName, erldns_records:name_type(RecordType)),
-    jsx:encode(lists:filter(record_filter(), lists:map(encode(Encoders), Records))).
+    json_encode_kw_list(lists:filter(record_filter(), lists:map(encode(Encoders), Records))).
 
 record_filter() ->
     fun(R) ->
@@ -275,3 +275,9 @@ encode_data({dns_rrdata_rrsig, TypeCovered, Alg, Labels, OriginalTtl, Expiration
 encode_data(Data) ->
     erldns_events:notify({?MODULE, unsupported_rrdata_type, Data}),
     {}.
+
+json_encode_kw_list(KwList) when is_list(KwList) ->
+    iolist_to_binary(json:encode(KwList, fun json_encode_term/2)).
+
+json_encode_term([{_, _} | _] = Value, Encode) -> json:encode_key_value_list(Value, Encode);
+json_encode_term(Other, Encode) -> json:encode_value(Other, Encode).
diff --git a/src/erldns_zone_loader.erl b/src/erldns_zone_loader.erl
@@ -33,7 +33,13 @@ load_zones() ->
     case file:read_file(filename()) of
         {ok, Binary} ->
             lager:info("Parsing zones JSON"),
-            JsonZones = jsx:decode(Binary, [{return_maps, false}]),
+            %% The "object_finish" callback override is here so that we don't return maps
+            %% and instead keep the list of decoded information as a keyword list. Maybe we
+            %% won't need this behavior in the future, but for now it's a good way to keep
+            %% compatibility with what we used before to decode JSON (jsx).
+            {JsonZones, ok, _} = json:decode(Binary, ok, #{
+                object_finish => fun(Acc, OldAcc) -> {lists:reverse(Acc), OldAcc} end
+            }),
             lager:info("Putting zones into cache"),
             lists:foreach(
                 fun(JsonZone) ->
@@ -60,43 +66,3 @@ filename() ->
         _ ->
             ?FILENAME
     end.
-
--ifdef(TEST).
-
-json_jsx_decode_test() ->
-    Zone_RR =
-        [
-            {<<"name">>, <<"example.net">>},
-            {<<"records">>, [
-                [
-                    {<<"name">>, <<"example.net">>},
-                    {<<"type">>, <<"SOA">>},
-                    {<<"ttl">>, 3600},
-                    {<<"data">>, [
-                        {<<"mname">>, <<"ns1.example.net">>},
-                        {<<"rname">>, <<"admin.example.net">>},
-                        {<<"serial">>, 1234567},
-                        {<<"refresh">>, 1},
-                        {<<"retry">>, 1},
-                        {<<"expire">>, 1},
-                        {<<"minimum">>, 1}
-                    ]}
-                ],
-                [
-                    {<<"name">>, <<"ns1.example.net">>},
-                    {<<"type">>, <<"A">>},
-                    {<<"ttl">>, 30},
-                    {<<"data">>, [{<<"ip">>, <<"123.45.67.89">>}]}
-                ]
-            ]}
-        ],
-    JSON_zone =
-        <<
-            "{\"name\":\"example.net\",\"records\":[{\"name\":\"example.net\",\"type\":\"SOA\",\"ttl\":3600,\"data\":{\"m"
-            "name\":\"ns1.example.net\",\"rname\":\"admin.example.net\",\"serial\":1234567,\"refresh\":1,\"retry\":1,\"ex"
-            "pire\":1,\"minimum\":1}},{\"name\":\"ns1.example.net\",\"type\":\"A\",\"ttl\":30,\"data\":{\"ip\":\"123.45.6"
-            "7.89\"}}]}"
-        >>,
-    ?assertEqual(Zone_RR, jsx:decode(JSON_zone, [{return_maps, false}])).
-
--endif.
diff --git a/src/erldns_zone_parser.erl b/src/erldns_zone_parser.erl
@@ -841,7 +841,7 @@ base64_to_bin(Bin) when is_binary(Bin) ->
 
 json_to_erlang_test() ->
     json_to_erlang(
-        jsx:decode(<<
+        json:decode(<<
             "{\"name\":\"example.com\",\"sha\":\"10ea56ad7be9d3e6e75be3a15ef0dfabe9facafba486d74914e7baf8fb36638e\",\"rec"
             "ords\":[{\"name\":\"example.com\",\"type\":\"SOA\",\"data\":{\"mname\":\"ns1.dnsimple.com\",\"rname\":\"admi"
             "n.dnsimple.com\",\"serial\":1597990915,\"refresh\":86400,\"retry\":7200,\"expire\":604800,\"minimum\":300},\""

diff --git a/test/erldns_storage_test.erl b/test/erldns_storage_test.erl
@@ -0,0 +1,35 @@
+-module(erldns_storage_test).
+
+-feature(maybe_expr, enable).
+
+-include_lib("dns_erlang/include/dns.hrl").
+-include_lib("erldns/include/erldns.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+-define(TEST_MODULE, erldns_storage).
+
+load_zones_with_existing_file_test() ->
+    {ok, Pid} = erldns_zone_parser:start_link(),
+    erldns_storage:create(zones),
+    erldns_storage:create(zone_records_typed),
+    Filename = filename:join(tmp_dir(), "zones.json"),
+
+    ok = file:write_file(
+        Filename,
+        <<"[{\"name\": \"example.com\", \"records\": [{\"name\": \"example.com\", \"type\": \"SOA\", \"data\": {\"mname\": \"ns1.example.com\", \"rname\": \"ahu.example.com\", \"serial\": 2000081501, \"refresh\": 28800, \"retry\": 7200, \"expire\": 604800, \"minimum\": 86400}, \"ttl\": 100000}]}]">>
+    ),
+
+    ?assertMatch({ok, 1}, ?TEST_MODULE:load_zones(Filename)),
+    gen_server:stop(Pid).
+
+%% Helpers
+
+tmp_dir() ->
+    maybe
+        false ?= os:getenv("TMPDIR"),
+        false ?= os:getenv("TEMP"),
+        false ?= os:getenv("TMP"),
+        "/tmp"
+    else
+        Dir -> Dir
+    end.
diff --git a/test/erldns_zone_encoder_test.erl b/test/erldns_zone_encoder_test.erl
@@ -0,0 +1,34 @@
+-module(erldns_zone_encoder_test).
+
+-feature(maybe_expr, enable).
+
+-include_lib("dns_erlang/include/dns.hrl").
+-include_lib("erldns/include/erldns.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+-define(TEST_MODULE, erldns_storage).
+
+zone_meta_to_json_with_valid_zone_test() ->
+    {ok, Pid} = erldns_zone_parser:start_link(),
+    erldns_storage:create(zones),
+    erldns_storage:create(zone_records_typed),
+
+    Z = #zone{
+        name = <<"example.com">>,
+        authority = [#dns_rr{name = <<"example.com">>, type = ?DNS_TYPE_SOA}]
+    },
+
+    JSON = erldns_zone_encoder:zone_meta_to_json(Z),
+    ?assert(is_binary(JSON)),
+
+    #{
+        <<"erldns">> := #{
+            <<"zone">> := #{
+                <<"name">> := <<"example.com">>,
+                <<"version">> := _,
+                <<"records_count">> := _
+            }
+        }
+    } = json:decode(JSON),
+
+    gen_server:stop(Pid).
-Original file line number
+Diff line change
@@ Expand Up / @@ -11,7 +11,6 @@ @@
             crypto,
             lager,
             dns_erlang,
-            jsx,
             ssl,
             mnesia,
             bear,
@@ Expand Down @@