From 88b1d9ba8829b197a17b2b19855ce644ec5a69db Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Wed, 21 Feb 2024 14:28:41 +0100 Subject: [PATCH] refactor: use atoms for root config fields and types --- apps/emqx/src/emqx_config.erl | 2 +- apps/emqx/src/emqx_schema.erl | 146 +++++++++--------- apps/emqx/src/emqx_zone_schema.erl | 33 ++-- apps/emqx/test/emqx_message_SUITE.erl | 11 +- .../src/emqx_authz/emqx_authz_schema.erl | 3 +- apps/emqx_conf/src/emqx_conf_schema.erl | 16 +- .../src/emqx_dashboard_swagger.erl | 2 +- .../src/emqx_enterprise.app.src | 2 +- .../src/emqx_enterprise_schema.erl | 4 +- .../src/emqx_mgmt_api_configs.erl | 2 +- .../src/emqx_mgmt_api_listeners.erl | 7 +- .../test/emqx_mgmt_api_configs_SUITE.erl | 2 +- .../test/emqx_mgmt_api_listeners_SUITE.erl | 2 +- 13 files changed, 119 insertions(+), 113 deletions(-) diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl index 195f116b1..b245cd1e1 100644 --- a/apps/emqx/src/emqx_config.erl +++ b/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. diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index d5989687d..33cf4c213 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/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} + } + )} + ]. diff --git a/apps/emqx/src/emqx_zone_schema.erl b/apps/emqx/src/emqx_zone_schema.erl index 695a4aa4c..41ea5d7e8 100644 --- a/apps/emqx/src/emqx_zone_schema.erl +++ b/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 diff --git a/apps/emqx/test/emqx_message_SUITE.erl b/apps/emqx/test/emqx_message_SUITE.erl index 7bf6c9a7e..db5e5dd6e 100644 --- a/apps/emqx/test/emqx_message_SUITE.erl +++ b/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)), diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl index 8e82ea061..89cec0f3d 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl +++ b/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)})} ] }. diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl index ea35988bd..d99538f2b 100644 --- a/apps/emqx_conf/src/emqx_conf_schema.erl +++ b/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)$", diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index e5810dcc5..cb5dcd345 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/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, diff --git a/apps/emqx_enterprise/src/emqx_enterprise.app.src b/apps/emqx_enterprise/src/emqx_enterprise.app.src index d7bcb1fd5..07ec35151 100644 --- a/apps/emqx_enterprise/src/emqx_enterprise.app.src +++ b/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, diff --git a/apps/emqx_enterprise/src/emqx_enterprise_schema.erl b/apps/emqx_enterprise/src/emqx_enterprise_schema.erl index 66af3206b..fda66409a 100644 --- a/apps/emqx_enterprise/src/emqx_enterprise_schema.erl +++ b/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). diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 8db1ef720..c15a9d93a 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/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(). diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index 1718a14cf..857185aa2 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/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), diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl index c555d3e16..3e816e4fd 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl +++ b/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( diff --git a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl index 244961283..827231e74 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/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, #{}).