diff --git a/apps/emqx/src/emqx_config_handler.erl b/apps/emqx/src/emqx_config_handler.erl index 6f8740ba8..153598877 100644 --- a/apps/emqx/src/emqx_config_handler.erl +++ b/apps/emqx/src/emqx_config_handler.erl @@ -18,6 +18,7 @@ -module(emqx_config_handler). -include("logger.hrl"). +-include_lib("hocon/include/hoconsc.hrl"). -behaviour(gen_server). @@ -75,7 +76,7 @@ update_config(SchemaModule, ConfKeyPath, UpdateArgs) -> AtomKeyPath = [atom(Key) || Key <- ConfKeyPath], gen_server:call(?MODULE, {change_config, SchemaModule, AtomKeyPath, UpdateArgs}, infinity). --spec add_handler(emqx_config:config_key_path(), handler_name()) -> ok. +-spec add_handler(emqx_config:config_key_path(), handler_name()) -> ok | {error, {conflict, list()}}. add_handler(ConfKeyPath, HandlerName) -> assert_callback_function(HandlerName), gen_server:call(?MODULE, {add_handler, ConfKeyPath, HandlerName}). @@ -100,8 +101,10 @@ init(_) -> {ok, #{handlers => Handlers#{?MOD => ?MODULE}}}. handle_call({add_handler, ConfKeyPath, HandlerName}, _From, State = #{handlers := Handlers}) -> - {ok, NewHandlers} = deep_put_handler(ConfKeyPath, Handlers, HandlerName), - {reply, ok, State#{handlers => NewHandlers}}; + case deep_put_handler(ConfKeyPath, Handlers, HandlerName) of + {ok, NewHandlers} -> {reply, ok, State#{handlers => NewHandlers}}; + {error, _Reason} = Error -> {reply, Error, State} + end; handle_call({change_config, SchemaModule, ConfKeyPath, UpdateArgs}, _From, #{handlers := Handlers} = State) -> @@ -131,12 +134,36 @@ terminate(_Reason, #{handlers := Handlers}) -> code_change(_OldVsn, State, _Extra) -> {ok, State}. -deep_put_handler([], Handlers, Mod) when is_map(Handlers) -> +deep_put_handler([], Handlers, Mod) -> {ok, Handlers#{?MOD => Mod}}; deep_put_handler([Key | KeyPath], Handlers, Mod) -> SubHandlers = maps:get(Key, Handlers, #{}), - {ok, SubHandlers1} = deep_put_handler(KeyPath, SubHandlers, Mod), - {ok, Handlers#{Key => SubHandlers1}}. + case deep_put_handler(KeyPath, SubHandlers, Mod) of + {ok, NewSubHandlers} -> + NewHandlers = Handlers#{Key => NewSubHandlers}, + case check_handler_conflict(NewHandlers) of + ok -> {ok, NewHandlers}; + {error, Reason} -> {error, Reason} + end; + {error, _Reason} = Error -> Error + end. + +%% Make sure that Specify Key and ?WKEY cannot be on the same level. +check_handler_conflict(Handlers) -> + Keys = filter_top_level_handlers(Handlers), + case lists:member(?WKEY, Keys) of + true when length(Keys) =:= 1 -> ok; + true -> {error, {conflict, Keys}}; + false -> ok + end. + +filter_top_level_handlers(Handlers) -> + maps:fold( + fun + (K, #{?MOD := _}, Acc) -> [K | Acc]; + (_K, #{}, Acc) -> Acc; + (?MOD, _, Acc) -> Acc + end, [], Handlers). handle_update_request(SchemaModule, ConfKeyPath, Handlers, UpdateArgs) -> try @@ -366,8 +393,12 @@ assert_callback_function(Mod) -> schema(SchemaModule, [RootKey | _]) -> Roots = hocon_schema:roots(SchemaModule), - {_, Fields} = lists:keyfind(bin(RootKey), 1, Roots), - #{roots => [root], fields => #{root => [Fields]}}. + Field = + case lists:keyfind(bin(RootKey), 1, Roots) of + {_, {Ref, ?REF(Ref)}} -> {Ref, ?R_REF(SchemaModule, Ref)}; + {_, Field0} -> Field0 + end, + #{roots => [root], fields => #{root => [Field]}}. load_prev_handlers() -> Handlers = application:get_env(emqx, ?MODULE, #{}), diff --git a/apps/emqx/test/emqx_config_handler_SUITE.erl b/apps/emqx/test/emqx_config_handler_SUITE.erl index f55c5589f..bd6bebbb8 100644 --- a/apps/emqx/test/emqx_config_handler_SUITE.erl +++ b/apps/emqx/test/emqx_config_handler_SUITE.erl @@ -77,6 +77,23 @@ t_handler(_Config) -> ok = emqx_config_handler:remove_handler(Wildcard1), ok. +t_conflict_handler(_Config) -> + ok = emqx_config_handler:add_handler([sysmon, '?', '?'], ?MODULE), + ?assertMatch({error, {conflict, _}}, + emqx_config_handler:add_handler([sysmon, '?', cpu_check_interval], ?MODULE)), + ok = emqx_config_handler:remove_handler([sysmon, '?', '?']), + + ok = emqx_config_handler:add_handler([sysmon, '?', cpu_check_interval], ?MODULE), + ?assertMatch({error, {conflict, _}}, + emqx_config_handler:add_handler([sysmon, '?', '?'], ?MODULE)), + ok = emqx_config_handler:remove_handler([sysmon, '?', cpu_check_interval]), + + %% override + ok = emqx_config_handler:add_handler([sysmon], emqx_logger), + ?assertMatch(#{handlers := #{sysmon := #{{mod} := emqx_logger}}}, + emqx_config_handler:info()), + ok. + t_root_key_update(_Config) -> PathKey = [sysmon], Opts = #{rawconf_with_defaults => true}, @@ -246,7 +263,7 @@ t_update_sub(_Config) -> ?assertEqual({ok,#{config => 0.81, post_config_update => #{?MODULE => ok}, raw_config => <<"81%">>}}, - emqx:update_config(SubKey, "81%", Opts#{merge => shallow})), + emqx:update_config(SubKey, "81%", Opts)), ?assertEqual(0.81, emqx:get_config(SubKey)), ?assertEqual("81%", emqx:get_raw_config(SubKey)), diff --git a/apps/emqx_bridge/src/emqx_bridge_app.erl b/apps/emqx_bridge/src/emqx_bridge_app.erl index 9240d4a82..f192cf73c 100644 --- a/apps/emqx_bridge/src/emqx_bridge_app.erl +++ b/apps/emqx_bridge/src/emqx_bridge_app.erl @@ -29,8 +29,8 @@ start(_StartType, _StartArgs) -> {ok, Sup} = emqx_bridge_sup:start_link(), ok = emqx_bridge:load(), ok = emqx_bridge:load_hook(), - emqx_config_handler:add_handler(?LEAF_NODE_HDLR_PATH, ?MODULE), - emqx_config_handler:add_handler(?TOP_LELVE_HDLR_PATH, emqx_bridge), + ok = emqx_config_handler:add_handler(?LEAF_NODE_HDLR_PATH, ?MODULE), + ok = emqx_config_handler:add_handler(?TOP_LELVE_HDLR_PATH, emqx_bridge), {ok, Sup}. stop(_State) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 8f9797843..bbe4a6ecb 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -45,7 +45,7 @@ ]). api_spec() -> - emqx_dashboard_swagger:spec(?MODULE). + emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true}). namespace() -> "configuration". @@ -53,7 +53,6 @@ paths() -> ["/configs", "/configs_reset/:rootname"] ++ lists:map(fun({Name, _Type}) -> ?PREFIX ++ to_list(Name) end, config_list(?EXCLUDES)). - schema("/configs") -> #{ 'operationId' => configs, @@ -156,7 +155,7 @@ config(put, #{body := Body}, Req) -> Path = conf_path(Req), case emqx:update_config(Path, Body, #{rawconf_with_defaults => true}) of {ok, #{raw_config := RawConf}} -> - {200, emqx_map_lib:jsonable_map(RawConf)}; + {200, RawConf}; {error, Reason} -> {400, #{code => 'UPDATE_FAILED', message => ?ERR_MSG(Reason)}} end. @@ -194,8 +193,7 @@ conf_path_reset(Req) -> string:lexemes(Path, "/ "). get_full_config() -> - emqx_map_lib:jsonable_map( - emqx_config:fill_defaults(emqx:get_raw_config([]))). + emqx_config:fill_defaults(emqx:get_raw_config([])). conf_path_from_querystr(Req) -> case proplists:get_value(<<"conf_path">>, cowboy_req:parse_qs(Req)) of diff --git a/apps/emqx_modules/test/emqx_rewrite_SUITE.erl b/apps/emqx_modules/test/emqx_rewrite_SUITE.erl index 5b12208be..96ba4bf86 100644 --- a/apps/emqx_modules/test/emqx_rewrite_SUITE.erl +++ b/apps/emqx_modules/test/emqx_rewrite_SUITE.erl @@ -160,23 +160,28 @@ t_update_disable(_Config) -> t_update_re_failed(_Config) -> ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE), + Re = <<"*^test/*">>, Rules = [#{ <<"source_topic">> => <<"test/#">>, - <<"re">> => <<"*^test/*">>, + <<"re">> => Re, <<"dest_topic">> => <<"test1/$2">>, <<"action">> => <<"publish">> }], - Error = {badmatch, + ?assertError({badmatch, {error, - {#{fields => - #{root => [{"rewrite", - {array, {ref,emqx_modules_schema,"rewrite"}}}]}, - roots => [root]}, - [{validation_error, - #{path => "root.rewrite.1.re", - reason => {<<"*^test/*">>,{"nothing to repeat",0}}, - value => <<"*^test/*">>}}]}}}, - ?assertError(Error, emqx_rewrite:update(Rules)), + {_, + [ + {validation_error, + #{ + path := "root.rewrite.1.re", + reason := {Re, {"nothing to repeat", 0}}, + value := Re + } + } + ] + } + } + }, emqx_rewrite:update(Rules)), ok. %%--------------------------------------------------------------------