From 856129984b873c2ae901cad53933aa7631d7431c Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 2 May 2023 21:09:36 +0200 Subject: [PATCH] refactor: remove raw_with_default config load option This option was previously only in tests to avoid emqx_conf app overwriting previously set configs with default values. After a03f2dd64bd2f9115d3ef0e59d35e120d7d8bae6, the issue for test cases had been resolved. This commit is to get rid of the option all together --- apps/emqx/src/emqx_config.erl | 62 ++++++++----------- apps/emqx/test/emqx_common_test_helpers.erl | 9 +-- apps/emqx_bridge/test/emqx_bridge_SUITE.erl | 3 +- apps/emqx_conf/src/emqx_conf_app.erl | 2 +- apps/emqx_modules/test/emqx_delayed_SUITE.erl | 4 +- .../test/emqx_delayed_api_SUITE.erl | 5 +- .../test/emqx_modules_conf_SUITE.erl | 4 +- apps/emqx_modules/test/emqx_rewrite_SUITE.erl | 24 ++----- .../test/emqx_rewrite_api_SUITE.erl | 5 +- .../test/emqx_telemetry_SUITE.erl | 24 ++----- .../test/emqx_telemetry_api_SUITE.erl | 5 +- .../test/emqx_topic_metrics_SUITE.erl | 4 +- .../test/emqx_topic_metrics_api_SUITE.erl | 5 +- apps/emqx_statsd/test/emqx_statsd_SUITE.erl | 16 ++--- rel/i18n/emqx_mgmt_api_status.hocon | 6 +- 15 files changed, 55 insertions(+), 123 deletions(-) diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl index 8d9028100..1cf1d873c 100644 --- a/apps/emqx/src/emqx_config.erl +++ b/apps/emqx/src/emqx_config.erl @@ -22,7 +22,6 @@ -export([ init_load/1, init_load/2, - init_load/3, read_override_conf/1, has_deprecated_file/0, delete_override_conf_files/0, @@ -314,44 +313,38 @@ put_raw(KeyPath, Config) -> %%============================================================================ init_load(SchemaMod) -> ConfFiles = application:get_env(emqx, config_files, []), - init_load(SchemaMod, ConfFiles, #{raw_with_default => true}). - -init_load(SchemaMod, Opts) when is_map(Opts) -> - ConfFiles = application:get_env(emqx, config_files, []), - init_load(SchemaMod, ConfFiles, Opts); -init_load(SchemaMod, ConfFiles) -> - init_load(SchemaMod, ConfFiles, #{raw_with_default => false}). + init_load(SchemaMod, ConfFiles). %% @doc Initial load of the given config files. %% NOTE: The order of the files is significant, configs from files ordered %% in the rear of the list overrides prior values. -spec init_load(module(), [string()] | binary() | hocon:config()) -> ok. -init_load(SchemaMod, Conf, Opts) when is_list(Conf) orelse is_binary(Conf) -> - HasDeprecatedFile = has_deprecated_file(), - RawConf = load_config_files(HasDeprecatedFile, Conf), - init_load(HasDeprecatedFile, SchemaMod, RawConf, Opts). - -init_load(true, SchemaMod, RawConf, Opts) when is_map(RawConf) -> +init_load(SchemaMod, Conf) when is_list(Conf) orelse is_binary(Conf) -> ok = save_schema_mod_and_names(SchemaMod), - %% deprecated conf will be removed in 5.1 - %% Merge environment variable overrides on top + HasDeprecatedFile = has_deprecated_file(), + RawConf0 = load_config_files(HasDeprecatedFile, Conf), + RawConf1 = + case HasDeprecatedFile of + true -> + overlay_v0(SchemaMod, RawConf0); + false -> + overlay_v1(SchemaMod, RawConf0) + end, + RawConf = fill_defaults_for_all_roots(SchemaMod, RawConf1), + %% check configs against the schema + {AppEnvs, CheckedConf} = check_config(SchemaMod, RawConf, #{}), + save_to_app_env(AppEnvs), + ok = save_to_config_map(CheckedConf, RawConf). + +%% Merge environment variable overrides on top, then merge with overrides. +overlay_v0(SchemaMod, RawConf) when is_map(RawConf) -> RawConfWithEnvs = merge_envs(SchemaMod, RawConf), Overrides = read_override_confs(), - RawConfWithOverrides = hocon:deep_merge(RawConfWithEnvs, Overrides), - RawConfAll = maybe_fill_defaults(SchemaMod, RawConfWithOverrides, Opts), - %% check configs against the schema - {AppEnvs, CheckedConf} = check_config(SchemaMod, RawConfAll, #{}), - save_to_app_env(AppEnvs), - ok = save_to_config_map(CheckedConf, RawConfAll); -init_load(false, SchemaMod, RawConf, Opts) when is_map(RawConf) -> - ok = save_schema_mod_and_names(SchemaMod), - %% Merge environment variable overrides on top - RawConfWithEnvs = merge_envs(SchemaMod, RawConf), - RawConfAll = maybe_fill_defaults(SchemaMod, RawConfWithEnvs, Opts), - %% check configs against the schema - {AppEnvs, CheckedConf} = check_config(SchemaMod, RawConfAll, #{}), - save_to_app_env(AppEnvs), - ok = save_to_config_map(CheckedConf, RawConfAll). + hocon:deep_merge(RawConfWithEnvs, Overrides). + +%% Merge environment variable overrides on top. +overlay_v1(SchemaMod, RawConf) when is_map(RawConf) -> + merge_envs(SchemaMod, RawConf). %% @doc Read merged cluster + local overrides. read_override_confs() -> @@ -360,8 +353,7 @@ read_override_confs() -> hocon:deep_merge(ClusterOverrides, LocalOverrides). %% keep the raw and non-raw conf has the same keys to make update raw conf easier. -%% TODO: remove raw_with_default as it's now always true. -maybe_fill_defaults(SchemaMod, RawConf0, #{raw_with_default := true}) -> +fill_defaults_for_all_roots(SchemaMod, RawConf0) -> RootSchemas = hocon_schema:roots(SchemaMod), %% the roots which are missing from the loaded configs MissingRoots = lists:filtermap( @@ -380,9 +372,7 @@ maybe_fill_defaults(SchemaMod, RawConf0, #{raw_with_default := true}) -> RawConf0, MissingRoots ), - fill_defaults(RawConf); -maybe_fill_defaults(_SchemaMod, RawConf, _Opts) -> - RawConf. + fill_defaults(RawConf). %% So far, this can only return true when testing. %% e.g. when testing an app, we need to load its config first diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index 926331e56..61373f638 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -55,7 +55,6 @@ is_tcp_server_available/2, is_tcp_server_available/3, load_config/2, - load_config/3, not_wait_mqtt_payload/1, read_schema_configs/2, render_config_file/2, @@ -499,18 +498,14 @@ copy_certs(emqx_conf, Dest0) -> copy_certs(_, _) -> ok. -load_config(SchemaModule, Config, Opts) -> +load_config(SchemaModule, Config) -> ConfigBin = case is_map(Config) of true -> emqx_utils_json:encode(Config); false -> Config end, ok = emqx_config:delete_override_conf_files(), - ok = emqx_config:init_load(SchemaModule, ConfigBin, Opts), - ok. - -load_config(SchemaModule, Config) -> - load_config(SchemaModule, Config, #{raw_with_default => true}). + ok = emqx_config:init_load(SchemaModule, ConfigBin). -spec is_all_tcp_servers_available(Servers) -> Result when Servers :: [{Host, Port}], diff --git a/apps/emqx_bridge/test/emqx_bridge_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_SUITE.erl index ed4807d12..a8864bf00 100644 --- a/apps/emqx_bridge/test/emqx_bridge_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_SUITE.erl @@ -141,8 +141,7 @@ setup_fake_telemetry_data() -> } } }, - Opts = #{raw_with_default => true}, - ok = emqx_common_test_helpers:load_config(emqx_bridge_schema, Conf, Opts), + ok = emqx_common_test_helpers:load_config(emqx_bridge_schema, Conf), ok = snabbkaffe:start_trace(), Predicate = fun(#{?snk_kind := K}) -> K =:= emqx_bridge_loaded end, diff --git a/apps/emqx_conf/src/emqx_conf_app.erl b/apps/emqx_conf/src/emqx_conf_app.erl index 2231b8336..70234b525 100644 --- a/apps/emqx_conf/src/emqx_conf_app.erl +++ b/apps/emqx_conf/src/emqx_conf_app.erl @@ -89,7 +89,7 @@ sync_data_from_node() -> %% ------------------------------------------------------------------------------ init_load() -> - emqx_config:init_load(emqx_conf:schema_module(), #{raw_with_default => true}). + emqx_config:init_load(emqx_conf:schema_module()). init_conf() -> %% Workaround for https://github.com/emqx/mria/issues/94: diff --git a/apps/emqx_modules/test/emqx_delayed_SUITE.erl b/apps/emqx_modules/test/emqx_delayed_SUITE.erl index 59ab0269c..8c271f0c1 100644 --- a/apps/emqx_modules/test/emqx_delayed_SUITE.erl +++ b/apps/emqx_modules/test/emqx_delayed_SUITE.erl @@ -40,9 +40,7 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), emqx_common_test_helpers:start_apps([emqx_conf, emqx_modules]), Config. diff --git a/apps/emqx_modules/test/emqx_delayed_api_SUITE.erl b/apps/emqx_modules/test/emqx_delayed_api_SUITE.erl index 4ae3dec88..b5995a47a 100644 --- a/apps/emqx_modules/test/emqx_delayed_api_SUITE.erl +++ b/apps/emqx_modules/test/emqx_delayed_api_SUITE.erl @@ -32,10 +32,7 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), - + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), ok = emqx_mgmt_api_test_util:init_suite( [emqx_conf, emqx_modules] ), diff --git a/apps/emqx_modules/test/emqx_modules_conf_SUITE.erl b/apps/emqx_modules/test/emqx_modules_conf_SUITE.erl index 666af9ef0..14e477bf9 100644 --- a/apps/emqx_modules/test/emqx_modules_conf_SUITE.erl +++ b/apps/emqx_modules/test/emqx_modules_conf_SUITE.erl @@ -29,9 +29,7 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Conf) -> - emqx_common_test_helpers:load_config(emqx_modules_schema, <<"gateway {}">>, #{ - raw_with_default => true - }), + emqx_common_test_helpers:load_config(emqx_modules_schema, <<"gateway {}">>), emqx_common_test_helpers:start_apps([emqx_conf, emqx_modules]), Conf. diff --git a/apps/emqx_modules/test/emqx_rewrite_SUITE.erl b/apps/emqx_modules/test/emqx_rewrite_SUITE.erl index 1847f876e..aa2c7cad7 100644 --- a/apps/emqx_modules/test/emqx_rewrite_SUITE.erl +++ b/apps/emqx_modules/test/emqx_rewrite_SUITE.erl @@ -73,9 +73,7 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> emqx_common_test_helpers:boot_modules(all), - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, #{}, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, #{}), emqx_common_test_helpers:start_apps([emqx_conf, emqx_modules]), Config. @@ -160,17 +158,13 @@ t_rewrite_re_error(_Config) -> ok. t_list(_Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE), Expect = maps:get(<<"rewrite">>, ?REWRITE), ?assertEqual(Expect, emqx_rewrite:list()), ok. t_update(_Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE), Init = emqx_rewrite:list(), Rules = [ #{ @@ -186,9 +180,7 @@ t_update(_Config) -> ok. t_update_disable(_Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE), ?assertEqual(ok, emqx_rewrite:update([])), timer:sleep(150), @@ -203,9 +195,7 @@ t_update_disable(_Config) -> ok. t_update_re_failed(_Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE), Re = <<"*^test/*">>, Rules = [ #{ @@ -261,9 +251,7 @@ receive_publish(Timeout) -> end. init() -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?REWRITE), ok = emqx_rewrite:enable(), {ok, C} = emqtt:start_link([{clientid, <<"c1">>}, {username, <<"u1">>}]), {ok, _} = emqtt:connect(C), diff --git a/apps/emqx_modules/test/emqx_rewrite_api_SUITE.erl b/apps/emqx_modules/test/emqx_rewrite_api_SUITE.erl index 68d12a2c1..528102d9e 100644 --- a/apps/emqx_modules/test/emqx_rewrite_api_SUITE.erl +++ b/apps/emqx_modules/test/emqx_rewrite_api_SUITE.erl @@ -33,10 +33,7 @@ init_per_testcase(_, Config) -> Config. init_per_suite(Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), - + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), ok = emqx_mgmt_api_test_util:init_suite( [emqx_conf, emqx_modules] ), diff --git a/apps/emqx_modules/test/emqx_telemetry_SUITE.erl b/apps/emqx_modules/test/emqx_telemetry_SUITE.erl index cc9a8b20f..7a461bd0e 100644 --- a/apps/emqx_modules/test/emqx_telemetry_SUITE.erl +++ b/apps/emqx_modules/test/emqx_telemetry_SUITE.erl @@ -42,9 +42,7 @@ init_per_suite(Config) -> emqx_common_test_helpers:deps_path(emqx_authz, "etc/acl.conf") end ), - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), emqx_gateway_test_utils:load_all_gateway_apps(), emqx_common_test_helpers:start_apps( [emqx_conf, emqx_authn, emqx_authz, emqx_modules], @@ -154,9 +152,7 @@ init_per_testcase(t_exhook_info, Config) -> {ok, _} = emqx_exhook_demo_svr:start(), {ok, Sock} = gen_tcp:connect("localhost", 9000, [], 3000), _ = gen_tcp:close(Sock), - ok = emqx_common_test_helpers:load_config(emqx_exhook_schema, ExhookConf, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_exhook_schema, ExhookConf), {ok, _} = application:ensure_all_started(emqx_exhook), Config; init_per_testcase(t_cluster_uuid, Config) -> @@ -177,9 +173,7 @@ init_per_testcase(t_uuid_restored_from_file, Config) -> %% clear the UUIDs in the DB {atomic, ok} = mria:clear_table(emqx_telemetry), emqx_common_test_helpers:stop_apps([emqx_conf, emqx_authn, emqx_authz, emqx_modules]), - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), emqx_common_test_helpers:start_apps( [emqx_conf, emqx_authn, emqx_authz, emqx_modules], fun set_special_configs/1 @@ -332,9 +326,7 @@ t_uuid_saved_to_file(_Config) -> %% clear the UUIDs in the DB {atomic, ok} = mria:clear_table(emqx_telemetry), emqx_common_test_helpers:stop_apps([emqx_conf, emqx_authn, emqx_authz, emqx_modules]), - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), emqx_common_test_helpers:start_apps( [emqx_conf, emqx_authn, emqx_authz, emqx_modules], fun set_special_configs/1 @@ -834,15 +826,11 @@ start_slave(Name) -> (emqx) -> application:set_env(emqx, boot_modules, []), ekka:join(TestNode), - emqx_common_test_helpers:load_config( - emqx_modules_schema, ?BASE_CONF, #{raw_with_default => true} - ), + emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), ok; (_App) -> - emqx_common_test_helpers:load_config( - emqx_modules_schema, ?BASE_CONF, #{raw_with_default => true} - ), + emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), ok end, Opts = #{ diff --git a/apps/emqx_modules/test/emqx_telemetry_api_SUITE.erl b/apps/emqx_modules/test/emqx_telemetry_api_SUITE.erl index ac6d12039..c375810b5 100644 --- a/apps/emqx_modules/test/emqx_telemetry_api_SUITE.erl +++ b/apps/emqx_modules/test/emqx_telemetry_api_SUITE.erl @@ -29,10 +29,7 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), - + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), ok = emqx_mgmt_api_test_util:init_suite( [emqx_conf, emqx_authn, emqx_authz, emqx_modules], fun set_special_configs/1 diff --git a/apps/emqx_modules/test/emqx_topic_metrics_SUITE.erl b/apps/emqx_modules/test/emqx_topic_metrics_SUITE.erl index acf6dfc82..a147f41cd 100644 --- a/apps/emqx_modules/test/emqx_topic_metrics_SUITE.erl +++ b/apps/emqx_modules/test/emqx_topic_metrics_SUITE.erl @@ -28,9 +28,7 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> emqx_common_test_helpers:boot_modules(all), - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?TOPIC, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?TOPIC), emqx_common_test_helpers:start_apps([emqx_conf, emqx_modules]), Config. diff --git a/apps/emqx_modules/test/emqx_topic_metrics_api_SUITE.erl b/apps/emqx_modules/test/emqx_topic_metrics_api_SUITE.erl index 5d64f123d..2e668b6cf 100644 --- a/apps/emqx_modules/test/emqx_topic_metrics_api_SUITE.erl +++ b/apps/emqx_modules/test/emqx_topic_metrics_api_SUITE.erl @@ -40,10 +40,7 @@ init_per_testcase(_, Config) -> Config. init_per_suite(Config) -> - ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF, #{ - raw_with_default => true - }), - + ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF), ok = emqx_mgmt_api_test_util:init_suite( [emqx_conf, emqx_modules] ), diff --git a/apps/emqx_statsd/test/emqx_statsd_SUITE.erl b/apps/emqx_statsd/test/emqx_statsd_SUITE.erl index 1f7c4688e..8b8709c27 100644 --- a/apps/emqx_statsd/test/emqx_statsd_SUITE.erl +++ b/apps/emqx_statsd/test/emqx_statsd_SUITE.erl @@ -59,9 +59,7 @@ init_per_suite(Config) -> [emqx_conf, emqx_dashboard, emqx_statsd], fun set_special_configs/1 ), - ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?BASE_CONF, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?BASE_CONF), Config. end_per_suite(_Config) -> @@ -84,22 +82,16 @@ t_server_validator(_) -> reason := "cannot_be_empty", value := "" }, - emqx_common_test_helpers:load_config(emqx_statsd_schema, ?BAD_CONF, #{ - raw_with_default => true - }) + emqx_common_test_helpers:load_config(emqx_statsd_schema, ?BAD_CONF) ), %% default - ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?DEFAULT_CONF, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?DEFAULT_CONF), DefaultServer = default_server(), ?assertEqual(DefaultServer, emqx_conf:get_raw([statsd, server])), DefaultServerStr = binary_to_list(DefaultServer), ?assertEqual(DefaultServerStr, emqx_conf:get([statsd, server])), %% recover - ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?BASE_CONF, #{ - raw_with_default => true - }), + ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?BASE_CONF), Server2 = emqx_conf:get_raw([statsd, server]), ?assertMatch(Server0, Server2), ok. diff --git a/rel/i18n/emqx_mgmt_api_status.hocon b/rel/i18n/emqx_mgmt_api_status.hocon index 2034d13bc..e70c9dd5e 100644 --- a/rel/i18n/emqx_mgmt_api_status.hocon +++ b/rel/i18n/emqx_mgmt_api_status.hocon @@ -11,8 +11,7 @@ This API was introduced in v5.0.10. The GET `/status` endpoint (without the `/api/...` prefix) is also an alias to this endpoint and works in the same way. This alias has been available since v5.0.0. -Starting from v5.0.25 or e5.0.4, you can also use 'format' parameter to get JSON format information. -""" +Starting from v5.0.25 or e5.0.4, you can also use 'format' parameter to get JSON format information.""" get_status_api.label: """Service health check""" @@ -32,8 +31,7 @@ emqx is running""" get_status_response503.desc: """When EMQX application is temporary not running or being restarted, it may return 'emqx is not_running'. -If the 'format' parameter is provided 'json', the nthe 'app_status' field in the JSON object is 'not_running'. -""" +If the 'format' parameter is provided 'json', the nthe 'app_status' field in the JSON object is 'not_running'.""" get_status_api_format.desc: """Specify the response format, 'text' (default) to return the HTTP body in free text,