Merge pull request #259 from emqx/fix-default-integer-not-from-string

fix: builtin type don't convert from_string when value is integer

Author: zhongwencool

sample-schemas/emqx_management_schema.erl

@@ -4,7 +4,7 @@
 
 -behaviour(hocon_schema).
 
--type endpoint() :: integer() | string().
+-type endpoint() :: string() | integer() .
 -type verify() :: verify_peer | verify_none.
 
 -reflect_type([endpoint/0, verify/0]).
@@ -104,4 +104,3 @@ tr_listeners(Conf) ->
 
 filter(Opts) ->
     [{K, V} || {K, V} <- Opts, V =/= undefined].
-

sample-schemas/emqx_schema.erl

@@ -115,7 +115,7 @@ fields("node") ->
     , {"global_gc_interval", t(duration_s(), "emqx.global_gc_interval", undefined)}
     , {"fullsweep_after", t(non_neg_integer(),
                             "vm_args.-env ERL_FULLSWEEP_AFTER", 1000)}
-    , {"max_ets_tables", t(duration(), "vm_args.+e", 256000)}
+    , {"max_ets_tables", t(integer(), "vm_args.+e", 256000)}
     , {"crash_dump", t(file(), "vm_args.-env ERL_CRASH_DUMP", undefined)}
     , {"dist_net_ticktime", t(integer(), "vm_args.-kernel net_ticktime", undefined)}
     , {"dist_listen_min", t(integer(), "kernel.inet_dist_listen_min", undefined)}
@@ -236,7 +236,7 @@ fields("zone") ->
     , {"upgrade_qos", t(flag(), undefined, false)}
     , {"max_inflight", t(range(0, 65535))}
     , {"retry_interval", t(duration_s(), undefined, "30s")}
-    , {"max_awaiting_rel", t(duration(), undefined, 0)}
+    , {"max_awaiting_rel", t(hoconsc:union([integer(), duration()]), undefined, 0)}
     , {"await_rel_timeout", t(duration_s(), undefined, "300s")}
     , {"ignore_loop_deliver", t(boolean())}
     , {"session_expiry_interval", t(duration_s(), undefined, "2h")}
@@ -379,24 +379,24 @@ fields("perf") ->
     ];
 
 fields("sysmon") ->
-    [ {"long_gc", t(duration(), undefined, 0)}
-    , {"long_schedule", t(duration(), undefined, 240)}
+    [ {"long_gc", t(hoconsc:union([integer(), duration()]), undefined, 0)}
+    , {"long_schedule", t(duration(), undefined, "240ms")}
     , {"large_heap", t(bytesize(), undefined, "8MB")}
     , {"busy_dist_port", t(boolean(), undefined, true)}
     , {"busy_port", t(boolean(), undefined, false)}
     ];
 
 fields("os_mon") ->
-    [ {"cpu_check_interval", t(duration_s(), undefined, 60)}
+    [ {"cpu_check_interval", t(duration_s(), undefined, "60s")}
     , {"cpu_high_watermark", t(percent(), undefined, "80%")}
     , {"cpu_low_watermark", t(percent(), undefined, "60%")}
-    , {"mem_check_interval", t(duration_s(), undefined, 60)}
+    , {"mem_check_interval", t(duration_s(), undefined, "60s")}
     , {"sysmem_high_watermark", t(percent(), undefined, "70%")}
     , {"procmem_high_watermark", t(percent(), undefined, "5%")}
     ];
 
 fields("vm_mon") ->
-    [ {"check_interval", t(duration_s(), undefined, 30)}
+    [ {"check_interval", t(duration_s(), undefined, "30s")}
     , {"process_high_watermark", t(percent(), undefined, "80%")}
     , {"process_low_watermark", t(percent(), undefined, "60%")}
     ];

src/hocon_schema_builtin.erl

@@ -43,12 +43,7 @@ convert(Symbol, ?ENUM(_OfSymbols)) ->
             Symbol
     end;
 convert(Int, Type) when is_integer(Int) ->
-    case Type =:= string() of
-        true ->
-            integer_to_list(Int);
-        false ->
-            Int
-    end;
+    convert(integer_to_list(Int), Type);
 convert(Bin, Type) when is_binary(Bin) ->
     Str = unicode:characters_to_list(Bin, utf8),
     convert(Str, Type);

test/hocon_schema_builtin_tests.erl

@@ -0,0 +1,122 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2021-2023 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+-module(hocon_schema_builtin_tests).
+
+-include_lib("typerefl/include/types.hrl").
+-include_lib("eunit/include/eunit.hrl").
+-include("hocon_private.hrl").
+-include("hoconsc.hrl").
+
+-export([roots/0, fields/1, to_ip_port/1]).
+
+-type ip_port() :: tuple() | integer().
+
+-typerefl_from_string({ip_port/0, ?MODULE, to_ip_port}).
+-reflect_type([ip_port/0]).
+
+roots() ->
+    [listener].
+
+fields(listener) ->
+    [{"bind", hoconsc:mk(ip_port(), #{default => 80})}].
+
+builtin_check_test() ->
+    Conf = "listener.bind = 1024",
+    ?assertEqual(#{<<"listener">> => #{<<"bind">> => 1024}}, check_plain(Conf)),
+    Conf1 = "listener.bind = 0",
+    ?assertEqual(#{<<"listener">> => #{<<"bind">> => 0}}, check_plain(Conf1)),
+    Conf2 = "listener.bind = 65535",
+    ?assertEqual(#{<<"listener">> => #{<<"bind">> => 65535}}, check_plain(Conf2)),
+    BadConf1 = "listener.bind = 65536",
+    ?assertThrow(
+        #{
+            exception := "port_number_too_large",
+            field := <<"bind">>,
+            path := "listener",
+            reason := failed_to_check_field
+        },
+        check_plain(BadConf1)
+    ),
+    BadConf2 = "listener.bind = -1",
+    ?assertThrow(
+        #{
+            exception := "port_number_must_be_positive",
+            field := <<"bind">>,
+            path := "listener",
+            reason := failed_to_check_field
+        },
+        check_plain(BadConf2)
+    ),
+    BadConf3 = "listener.bind = 1883d",
+    ?assertThrow(
+        #{
+            exception := "bad_port_number",
+            field := <<"bind">>,
+            path := "listener",
+            reason := failed_to_check_field
+        },
+        check_plain(BadConf3)
+    ),
+    ok.
+
+check_plain(Str) ->
+    {ok, Map} = hocon:binary(Str, #{}),
+    hocon_tconf:check_plain(?MODULE, Map, #{}).
+
+to_ip_port(Str) ->
+    case split_ip_port(Str) of
+        {"", Port} ->
+            %% this is a local address
+            {ok, parse_port(Port)};
+        {MaybeIp, Port} ->
+            PortVal = parse_port(Port),
+            case inet:parse_address(MaybeIp) of
+                {ok, IpTuple} ->
+                    {ok, {IpTuple, PortVal}};
+                _ ->
+                    {error, bad_ip_port}
+            end;
+        _ ->
+            {error, bad_ip_port}
+    end.
+
+split_ip_port(Str0) ->
+    Str = re:replace(Str0, " ", "", [{return, list}, global]),
+    case lists:split(string:rchr(Str, $:), Str) of
+        %% no colon
+        {[], Str} ->
+            {"", Str};
+        {IpPlusColon, PortString} ->
+            IpStr0 = lists:droplast(IpPlusColon),
+            case IpStr0 of
+                %% drop head/tail brackets
+                [$[ | S] ->
+                    case lists:last(S) of
+                        $] -> {lists:droplast(S), PortString};
+                        _ -> error
+                    end;
+                _ ->
+                    {IpStr0, PortString}
+            end
+    end.
+
+parse_port(Port) ->
+    case string:to_integer(string:strip(Port)) of
+        {P, ""} when P < 0 -> throw("port_number_must_be_positive");
+        {P, ""} when P > 65535 -> throw("port_number_too_large");
+        {P, ""} -> P;
+        _ -> throw("bad_port_number")
+    end.