Merge pull request #12559 from zmstone/0221-refactor-use-atom-fileds

refactor: use atoms for root config fields

apps/emqx/src/emqx_config.erl

@@ -960,7 +960,7 @@ is_zone_root(Name) ->
 
 -spec zone_roots() -> [atom()].
 zone_roots() ->
-    lists:map(fun list_to_atom/1, emqx_zone_schema:roots()).
+    emqx_zone_schema:roots().
 
 %%%
 %%% @doc During init, ensure order of puts that zone is put after the other global defaults.

apps/emqx/src/emqx_schema.erl

@@ -139,6 +139,8 @@
     get_tombstone_map_value_type/1
 ]).
 
+-export([listeners/0]).
+
 -behaviour(hocon_schema).
 
 -reflect_type([
@@ -193,12 +195,12 @@ roots() ->
 
 roots(high) ->
     [
-        {"listeners",
+        {listeners,
             sc(
                 ref("listeners"),
                 #{importance => ?IMPORTANCE_HIGH}
             )},
-        {"mqtt",
+        {mqtt,
             sc(
                 ref("mqtt"),
                 #{
@@ -207,9 +209,9 @@ roots(high) ->
                     importance => ?IMPORTANCE_MEDIUM
                 }
             )},
-        {"zones",
+        {zones,
             sc(
-                map("name", ref("zone")),
+                map(name, ref("zone")),
                 #{
                     desc => ?DESC(zones),
                     importance => ?IMPORTANCE_HIDDEN
@@ -221,7 +223,7 @@ roots(high) ->
             [
                 %% NOTE: authorization schema here is only to keep emqx app pure
                 %% the full schema for EMQX node is injected in emqx_conf_schema.
-                {?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME,
+                {?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_ATOM,
                     sc(
                         ref(?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME),
                         #{importance => ?IMPORTANCE_HIDDEN}
@@ -238,17 +240,17 @@ roots(medium) ->
                     importance => ?IMPORTANCE_HIDDEN
                 }
             )},
-        {"sys_topics",
+        {sys_topics,
             sc(
                 ref("sys_topics"),
                 #{desc => ?DESC(sys_topics)}
             )},
-        {"force_shutdown",
+        {force_shutdown,
             sc(
                 ref("force_shutdown"),
                 #{}
             )},
-        {"overload_protection",
+        {overload_protection,
             sc(
                 ref("overload_protection"),
                 #{importance => ?IMPORTANCE_HIDDEN}
@@ -256,36 +258,36 @@ roots(medium) ->
     ];
 roots(low) ->
     [
-        {"force_gc",
+        {force_gc,
             sc(
                 ref("force_gc"),
                 #{}
             )},
-        {"conn_congestion",
+        {conn_congestion,
             sc(
                 ref("conn_congestion"),
                 #{
                     importance => ?IMPORTANCE_HIDDEN
                 }
             )},
-        {"stats",
+        {stats,
             sc(
                 ref("stats"),
                 #{
                     importance => ?IMPORTANCE_HIDDEN
                 }
             )},
-        {"sysmon",
+        {sysmon,
             sc(
                 ref("sysmon"),
                 #{}
             )},
-        {"alarm",
+        {alarm,
             sc(
                 ref("alarm"),
                 #{}
             )},
-        {"flapping_detect",
+        {flapping_detect,
             sc(
                 ref("flapping_detect"),
                 #{
@@ -293,7 +295,7 @@ roots(low) ->
                     converter => fun flapping_detect_converter/2
                 }
             )},
-        {"persistent_session_store",
+        {persistent_session_store,
             sc(
                 ref("persistent_session_store"),
                 #{
@@ -303,19 +305,19 @@ roots(low) ->
                     importance => ?IMPORTANCE_HIDDEN
                 }
             )},
-        {"session_persistence",
+        {session_persistence,
             sc(
                 ref("session_persistence"),
                 #{
                     importance => ?IMPORTANCE_HIDDEN
                 }
             )},
-        {"trace",
+        {trace,
             sc(
                 ref("trace"),
                 #{importance => ?IMPORTANCE_HIDDEN}
             )},
-        {"crl_cache",
+        {crl_cache,
             sc(
                 ref("crl_cache"),
                 #{importance => ?IMPORTANCE_HIDDEN}
@@ -441,7 +443,7 @@ fields("stats") ->
     ];
 fields("authorization") ->
     authz_fields();
-fields(authz_cache) ->
+fields("authz_cache") ->
     [
         {enable,
             sc(
@@ -630,55 +632,7 @@ fields("force_gc") ->
             )}
     ];
 fields("listeners") ->
-    [
-        {"tcp",
-            sc(
-                tombstone_map(name, ref("mqtt_tcp_listener")),
-                #{
-                    desc => ?DESC(fields_listeners_tcp),
-                    converter => fun(X, _) ->
-                        ensure_default_listener(X, tcp)
-                    end,
-                    required => {false, recursively}
-                }
-            )},
-        {"ssl",
-            sc(
-                tombstone_map(name, ref("mqtt_ssl_listener")),
-                #{
-                    desc => ?DESC(fields_listeners_ssl),
-                    converter => fun(X, _) -> ensure_default_listener(X, ssl) end,
-                    required => {false, recursively}
-                }
-            )},
-        {"ws",
-            sc(
-                tombstone_map(name, ref("mqtt_ws_listener")),
-                #{
-                    desc => ?DESC(fields_listeners_ws),
-                    converter => fun(X, _) -> ensure_default_listener(X, ws) end,
-                    required => {false, recursively}
-                }
-            )},
-        {"wss",
-            sc(
-                tombstone_map(name, ref("mqtt_wss_listener")),
-                #{
-                    desc => ?DESC(fields_listeners_wss),
-                    converter => fun(X, _) -> ensure_default_listener(X, wss) end,
-                    required => {false, recursively}
-                }
-            )},
-        {"quic",
-            sc(
-                tombstone_map(name, ref("mqtt_quic_listener")),
-                #{
-                    desc => ?DESC(fields_listeners_quic),
-                    converter => fun keep_default_tombstone/2,
-                    required => {false, recursively}
-                }
-            )}
-    ];
+    listeners();
 fields("crl_cache") ->
     %% Note: we make the refresh interval and HTTP timeout global (not
     %% per-listener) because multiple SSL listeners might point to the
@@ -2082,7 +2036,7 @@ desc("authorization") ->
     "Settings for client authorization.";
 desc("mqtt") ->
     "Global MQTT configuration.";
-desc(authz_cache) ->
+desc("authz_cache") ->
     "Settings for the authorization cache.";
 desc("zone") ->
     "A `Zone` defines a set of configuration items (such as the maximum number of connections)"
@@ -2644,7 +2598,7 @@ authz_fields() ->
             )},
         {"cache",
             sc(
-                ref(?MODULE, authz_cache),
+                ref(?MODULE, "authz_cache"),
                 #{}
             )}
     ].
@@ -3592,6 +3546,7 @@ flapping_detect_converter(Conf = #{<<"window_time">> := <<"disable">>}, _Opts) -
     Conf#{<<"window_time">> => ?DEFAULT_WINDOW_TIME, <<"enable">> => false};
 flapping_detect_converter(Conf, _Opts) ->
     Conf.
+
 mqtt_general() ->
     [
         {"idle_timeout",
@@ -3923,3 +3878,54 @@ ensure_unicode_path(Path, _) when is_list(Path) ->
     Path;
 ensure_unicode_path(Path, _) ->
     throw({"not_string", Path}).
+
+listeners() ->
+    [
+        {"tcp",
+            sc(
+                tombstone_map(name, ref("mqtt_tcp_listener")),
+                #{
+                    desc => ?DESC(fields_listeners_tcp),
+                    converter => fun(X, _) ->
+                        ensure_default_listener(X, tcp)
+                    end,
+                    required => {false, recursively}
+                }
+            )},
+        {"ssl",
+            sc(
+                tombstone_map(name, ref("mqtt_ssl_listener")),
+                #{
+                    desc => ?DESC(fields_listeners_ssl),
+                    converter => fun(X, _) -> ensure_default_listener(X, ssl) end,
+                    required => {false, recursively}
+                }
+            )},
+        {"ws",
+            sc(
+                tombstone_map(name, ref("mqtt_ws_listener")),
+                #{
+                    desc => ?DESC(fields_listeners_ws),
+                    converter => fun(X, _) -> ensure_default_listener(X, ws) end,
+                    required => {false, recursively}
+                }
+            )},
+        {"wss",
+            sc(
+                tombstone_map(name, ref("mqtt_wss_listener")),
+                #{
+                    desc => ?DESC(fields_listeners_wss),
+                    converter => fun(X, _) -> ensure_default_listener(X, wss) end,
+                    required => {false, recursively}
+                }
+            )},
+        {"quic",
+            sc(
+                tombstone_map(name, ref("mqtt_quic_listener")),
+                #{
+                    desc => ?DESC(fields_listeners_quic),
+                    converter => fun keep_default_tombstone/2,
+                    required => {false, recursively}
+                }
+            )}
+    ].

apps/emqx/src/emqx_zone_schema.erl

@@ -23,17 +23,17 @@
 
 namespace() -> zone.
 
-%% this schema module is not used at root level.
-%% roots are added only for document generation.
+%% Zone values are never checked as root level.
+%% We need roots defined here because it's used to generate config API schema.
 roots() ->
     [
-        "mqtt",
-        "stats",
-        "flapping_detect",
-        "force_shutdown",
-        "conn_congestion",
-        "force_gc",
-        "overload_protection"
+        mqtt,
+        stats,
+        flapping_detect,
+        force_shutdown,
+        conn_congestion,
+        force_gc,
+        overload_protection
     ].
 
 zones_without_default() ->
@@ -43,22 +43,25 @@ zones_without_default() ->
         fun(F) ->
             case lists:member(F, Hidden) of
                 true ->
-                    {F, ?HOCON(?R_REF(F), #{importance => ?IMPORTANCE_HIDDEN})};
+                    {F,
+                        ?HOCON(?R_REF(?MODULE, atom_to_list(F)), #{importance => ?IMPORTANCE_HIDDEN})};
                 false ->
-                    {F, ?HOCON(?R_REF(F), #{})}
+                    {F, ?HOCON(?R_REF(?MODULE, atom_to_list(F)), #{})}
             end
         end,
         Fields
     ).
 
 global_zone_with_default() ->
-    lists:map(fun(F) -> {F, ?HOCON(?R_REF(emqx_schema, F), #{})} end, roots() -- hidden()).
+    lists:map(
+        fun(F) -> {F, ?HOCON(?R_REF(emqx_schema, atom_to_list(F)), #{})} end, roots() -- hidden()
+    ).
 
 hidden() ->
     [
-        "stats",
-        "overload_protection",
-        "conn_congestion"
+        stats,
+        overload_protection,
+        conn_congestion
     ].
 
 %% zone schemas are clones from the same name from root level

apps/emqx/test/emqx_message_SUITE.erl

@@ -144,12 +144,13 @@ t_undefined_headers(_) ->
 t_is_expired_1(_) ->
     test_msg_expired_property(?MODULE).
 
+make_zone_default_conf() ->
+    maps:from_list([{Root, #{}} || Root <- emqx_zone_schema:roots()]).
+
 t_is_expired_2(_) ->
     %% if the 'Message-Expiry-Interval' property is set, the message_expiry_interval should be ignored
     try
-        emqx_config:put(
-            maps:from_list([{list_to_atom(Root), #{}} || Root <- emqx_zone_schema:roots()])
-        ),
+        emqx_config:put(make_zone_default_conf()),
         emqx_config:put_zone_conf(?MODULE, [mqtt, message_expiry_interval], timer:seconds(10)),
         test_msg_expired_property(?MODULE)
     after
@@ -158,9 +159,7 @@ t_is_expired_2(_) ->
 
 t_is_expired_3(_) ->
     try
-        emqx_config:put(
-            maps:from_list([{list_to_atom(Root), #{}} || Root <- emqx_zone_schema:roots()])
-        ),
+        emqx_config:put(make_zone_default_conf()),
         emqx_config:put_zone_conf(?MODULE, [mqtt, message_expiry_interval], 100),
         Msg = emqx_message:make(<<"clientid">>, <<"topic">>, <<"payload">>),
         ?assertNot(emqx_message:is_expired(Msg, ?MODULE)),

apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl

@@ -72,6 +72,7 @@ roots() -> [].
 
 namespace() -> undefined.
 
+%% "authorization"
 fields(?CONF_NS) ->
     emqx_schema:authz_fields() ++ authz_fields();
 fields("metrics_status_fields") ->
@@ -127,7 +128,7 @@ injected_fields(AuthzSchemaMods) ->
     persistent_term:put(?AUTHZ_MODS_PT_KEY, AuthzSchemaMods),
     #{
         'roots.high' => [
-            {?CONF_NS, ?HOCON(?R_REF(?CONF_NS), #{desc => ?DESC(?CONF_NS)})}
+            {?CONF_NS_ATOM, ?HOCON(?R_REF(?CONF_NS), #{desc => ?DESC(?CONF_NS)})}
         ]
     }.
 

apps/emqx_conf/src/emqx_conf_schema.erl

@@ -99,19 +99,19 @@ roots() ->
     ok = emqx_schema_hooks:inject_from_modules(?INJECTING_CONFIGS),
     emqx_schema_high_prio_roots() ++
         [
-            {"node",
+            {node,
                 sc(
                     ?R_REF("node"),
                     #{
                         translate_to => ["emqx"]
                     }
                 )},
-            {"cluster",
+            {cluster,
                 sc(
                     ?R_REF("cluster"),
                     #{translate_to => ["ekka"]}
                 )},
-            {"log",
+            {log,
                 sc(
                     ?R_REF("log"),
                     #{
@@ -119,7 +119,7 @@ roots() ->
                         importance => ?IMPORTANCE_HIGH
                     }
                 )},
-            {"rpc",
+            {rpc,
                 sc(
                     ?R_REF("rpc"),
                     #{
@@ -1421,19 +1421,19 @@ roots(Module) ->
     lists:map(fun({_BinName, Root}) -> Root end, hocon_schema:roots(Module)).
 
 %% Like authentication schema, authorization schema is incomplete in emqx_schema
-%% module, this function replaces the root field "authorization" with a new schema
+%% module, this function replaces the root field 'authorization' with a new schema
 emqx_schema_high_prio_roots() ->
     Roots = emqx_schema:roots(high),
     Authz =
-        {"authorization",
+        {authorization,
             sc(
                 ?R_REF("authorization"),
                 #{
-                    desc => ?DESC(authorization),
+                    desc => ?DESC("authorization"),
                     importance => ?IMPORTANCE_HIGH
                 }
             )},
-    lists:keyreplace("authorization", 1, Roots, Authz).
+    lists:keyreplace(authorization, 1, Roots, Authz).
 
 validate_time_offset(Offset) ->
     ValidTimeOffset = "^([\\-\\+][0-1][0-9]:[0-6][0-9]|system|utc)$",

apps/emqx_dashboard/src/emqx_dashboard_swagger.erl

@@ -234,7 +234,7 @@ file_schema(FileName) ->
     }.
 
 gen_api_schema_json_iodata(SchemaMod, SchemaInfo, Converter) ->
-    {ApiSpec0, Components0} = emqx_dashboard_swagger:spec(
+    {ApiSpec0, Components0} = spec(
         SchemaMod,
         #{
             schema_converter => Converter,

apps/emqx_enterprise/src/emqx_enterprise.app.src

@@ -1,6 +1,6 @@
 {application, emqx_enterprise, [
     {description, "EMQX Enterprise Edition"},
-    {vsn, "0.1.6"},
+    {vsn, "0.1.7"},
     {registered, []},
     {applications, [
         kernel,

apps/emqx_enterprise/src/emqx_enterprise_schema.erl

@@ -144,8 +144,8 @@ ee_delegate(Method, [], Name) ->
 
 redefine_roots(Roots) ->
     Overrides = [
-        {"node", #{type => hoconsc:ref(?MODULE, "node")}},
-        {"log", #{type => hoconsc:ref(?MODULE, "log")}}
+        {node, #{type => hoconsc:ref(?MODULE, "node")}},
+        {log, #{type => hoconsc:ref(?MODULE, "log")}}
     ],
     override(Roots, Overrides).
 

apps/emqx_management/src/emqx_mgmt_api_configs.erl

@@ -502,7 +502,7 @@ conf_path(Req) ->
     string:lexemes(Path, "/ ").
 
 global_zone_roots() ->
-    lists:map(fun({K, _}) -> list_to_binary(K) end, global_zone_schema()).
+    lists:map(fun({K, _}) -> atom_to_binary(K) end, global_zone_schema()).
 
 global_zone_schema() ->
     emqx_zone_schema:global_zone_with_default().

apps/emqx_management/src/emqx_mgmt_api_listeners.erl

@@ -318,13 +318,10 @@ create_listener_schema(Opts) ->
     ).
 
 listeners_type() ->
-    lists:map(
-        fun({Type, _}) -> list_to_existing_atom(Type) end,
-        hocon_schema:fields(emqx_schema, "listeners")
-    ).
+    lists:map(fun({Type, _}) -> list_to_existing_atom(Type) end, emqx_schema:listeners()).
 
 listeners_info(Opts) ->
-    Listeners = hocon_schema:fields(emqx_schema, "listeners"),
+    Listeners = emqx_schema:listeners(),
     lists:map(
         fun({ListenerType, Schema}) ->
             Type = emqx_schema:get_tombstone_map_value_type(Schema),

apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl

@@ -138,7 +138,7 @@ t_log(_Config) ->
 t_global_zone(_Config) ->
     {ok, Zones} = get_global_zone(),
     ZonesKeys = lists:map(
-        fun({K, _}) -> list_to_binary(K) end, emqx_zone_schema:global_zone_with_default()
+        fun({K, _}) -> atom_to_binary(K) end, emqx_zone_schema:global_zone_with_default()
     ),
     ?assertEqual(lists:usort(ZonesKeys), lists:usort(maps:keys(Zones))),
     ?assertEqual(

apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl

@@ -516,7 +516,7 @@ cert_file(Name) ->
     data_file(filename:join(["certs", Name])).
 
 default_listeners_hocon_text() ->
-    Sc = #{roots => emqx_schema:fields("listeners")},
+    Sc = #{roots => emqx_schema:listeners()},
     Listeners = hocon_tconf:make_serializable(Sc, #{}, #{}),
     Config = #{<<"listeners">> => Listeners},
     hocon_pp:do(Config, #{}).