From 2b99a9b988c336345a1a19bfe98de583a8c01666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9F=90=E6=96=87?= Date: Wed, 17 May 2023 13:31:45 +0800 Subject: [PATCH] feat: hide resource_opts's request_timeout --- apps/emqx_bridge/src/emqx_bridge_api.erl | 13 +++++++--- apps/emqx_bridge/src/emqx_bridge_resource.erl | 15 ++++-------- .../src/schema/emqx_bridge_schema.erl | 23 ++++++++++++++++-- .../src/schema/emqx_bridge_webhook_schema.erl | 8 +++---- .../test/emqx_bridge_api_SUITE.erl | 24 +++++++++++++++++++ .../emqx_bridge_compatible_config_tests.erl | 2 +- changes/ce/feat-10713.en.md | 2 +- 7 files changed, 65 insertions(+), 22 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index c7e48990b..9802d5fe8 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -892,11 +892,18 @@ fill_defaults(Type, RawConf) -> pack_bridge_conf(Type, RawConf) -> #{<<"bridges">> => #{bin(Type) => #{<<"foo">> => RawConf}}}. -unpack_bridge_conf(Type, PackedConf) -> - #{<<"bridges">> := Bridges} = PackedConf, - #{<<"foo">> := RawConf} = maps:get(bin(Type), Bridges), +%% Hide webhook's resource_opts.request_timeout from user. +filter_raw_conf(<<"webhook">>, RawConf0) -> + emqx_utils_maps:deep_remove([<<"resource_opts">>, <<"request_timeout">>], RawConf0); +filter_raw_conf(_TypeBin, RawConf) -> RawConf. +unpack_bridge_conf(Type, PackedConf) -> + TypeBin = bin(Type), + #{<<"bridges">> := Bridges} = PackedConf, + #{<<"foo">> := RawConf} = maps:get(TypeBin, Bridges), + filter_raw_conf(TypeBin, RawConf). + is_ok(ok) -> ok; is_ok(OkResult = {ok, _}) -> diff --git a/apps/emqx_bridge/src/emqx_bridge_resource.erl b/apps/emqx_bridge/src/emqx_bridge_resource.erl index a756f535e..60ee242d1 100644 --- a/apps/emqx_bridge/src/emqx_bridge_resource.erl +++ b/apps/emqx_bridge/src/emqx_bridge_resource.erl @@ -178,7 +178,7 @@ create(Type, Name, Conf, Opts) -> <<"emqx_bridge">>, bridge_to_resource_type(Type), parse_confs(TypeBin, Name, Conf), - parse_opts(TypeBin, Conf, Opts) + parse_opts(Conf, Opts) ), ok. @@ -245,7 +245,7 @@ recreate(Type, Name, Conf, Opts) -> resource_id(Type, Name), bridge_to_resource_type(Type), parse_confs(TypeBin, Name, Conf), - parse_opts(TypeBin, Conf, Opts) + parse_opts(Conf, Opts) ). create_dry_run(Type, Conf0) -> @@ -402,15 +402,8 @@ bin(Bin) when is_binary(Bin) -> Bin; bin(Str) when is_list(Str) -> list_to_binary(Str); bin(Atom) when is_atom(Atom) -> atom_to_binary(Atom, utf8). -parse_opts(Type, Conf, Opts0) -> - Opts1 = override_start_after_created(Conf, Opts0), - override_resource_request_timeout(Type, Conf, Opts1). - -%% Put webhook's http request_timeout into the resource options -override_resource_request_timeout(<<"webhook">>, #{request_timeout := ReqTimeout}, Opts) -> - Opts#{request_timeout => ReqTimeout}; -override_resource_request_timeout(_Type, _Conf, Opts) -> - Opts. +parse_opts(Conf, Opts0) -> + override_start_after_created(Conf, Opts0). override_start_after_created(Config, Opts) -> Enabled = maps:get(enable, Config, true), diff --git a/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl b/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl index d1755bf73..b590f0cd4 100644 --- a/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl +++ b/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl @@ -223,6 +223,25 @@ node_name() -> {"node", mk(binary(), #{desc => ?DESC("desc_node_name"), example => "emqx@127.0.0.1"})}. webhook_bridge_converter(Conf0, _HoconOpts) -> - emqx_bridge_compatible_config:upgrade_pre_ee( + Conf1 = emqx_bridge_compatible_config:upgrade_pre_ee( Conf0, fun emqx_bridge_compatible_config:webhook_maybe_upgrade/1 - ). + ), + case Conf1 of + undefined -> + undefined; + _ -> + maps:map( + fun(_Name, Conf) -> + do_convert_webhook_config(Conf) + end, + Conf1 + ) + end. + +%% We hide resource_opts.request_timeout from user. +do_convert_webhook_config( + #{<<"request_timeout">> := ReqT, <<"resource_opts">> := ResOpts} = Conf +) -> + Conf#{<<"resource_opts">> => ResOpts#{<<"request_timeout">> => ReqT}}; +do_convert_webhook_config(Conf) -> + Conf. diff --git a/apps/emqx_bridge/src/schema/emqx_bridge_webhook_schema.erl b/apps/emqx_bridge/src/schema/emqx_bridge_webhook_schema.erl index 83a3dba9b..de1e09abb 100644 --- a/apps/emqx_bridge/src/schema/emqx_bridge_webhook_schema.erl +++ b/apps/emqx_bridge/src/schema/emqx_bridge_webhook_schema.erl @@ -41,7 +41,7 @@ fields("get") -> emqx_bridge_schema:status_fields() ++ fields("post"); fields("creation_opts") -> [ - deprecated_request_timeout() + hidden_request_timeout() | lists:filter( fun({K, _V}) -> not lists:member(K, unsupported_opts()) @@ -195,12 +195,12 @@ name_field() -> method() -> enum([post, put, get, delete]). -deprecated_request_timeout() -> +hidden_request_timeout() -> {request_timeout, mk( hoconsc:union([infinity, emqx_schema:duration_ms()]), #{ - default => <<"15s">>, - deprecated => {since, "5.0.26"} + required => false, + importance => ?IMPORTANCE_HIDDEN } )}. diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index 1b5e51a11..288b1da29 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -1295,8 +1295,32 @@ t_inconsistent_webhook_request_timeouts(Config) -> Config ), ?assertNot(maps:is_key(<<"request_timeout">>, ResourceOpts)), + validate_resource_request_timeout(proplists:get_value(group, Config), 1000, Name), ok. +validate_resource_request_timeout(single, Timeout, Name) -> + SentData = #{payload => <<"Hello EMQX">>, timestamp => 1668602148000}, + BridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE_HTTP, Name), + ResId = emqx_bridge_resource:resource_id(<<"webhook">>, Name), + ?check_trace( + begin + {ok, Res} = + ?wait_async_action( + emqx_bridge:send_message(BridgeID, SentData), + #{?snk_kind := async_query}, + 1000 + ), + ?assertMatch({ok, #{id := ResId, query_opts := #{timeout := Timeout}}}, Res) + end, + fun(Trace0) -> + Trace = ?of_kind(async_query, Trace0), + ?assertMatch([#{query_opts := #{timeout := Timeout}}], Trace), + ok + end + ); +validate_resource_request_timeout(_Cluster, _Timeout, _Name) -> + ignore. + %% request(Method, URL, Config) -> diff --git a/apps/emqx_bridge/test/emqx_bridge_compatible_config_tests.erl b/apps/emqx_bridge/test/emqx_bridge_compatible_config_tests.erl index 3481ac30c..080ca9107 100644 --- a/apps/emqx_bridge/test/emqx_bridge_compatible_config_tests.erl +++ b/apps/emqx_bridge/test/emqx_bridge_compatible_config_tests.erl @@ -73,7 +73,7 @@ webhook_config_test() -> } } = check(Conf3), ?assertEqual(60_000, RequestTime), - ?assertNot(maps:is_key(<<"requst_timeout">>, ResourceOpts)), + ?assertMatch(#{<<"request_timeout">> := 60_000}, ResourceOpts), ok. up(#{<<"bridges">> := Bridges0} = Conf0) -> diff --git a/changes/ce/feat-10713.en.md b/changes/ce/feat-10713.en.md index 0e28a1a12..6de542be6 100644 --- a/changes/ce/feat-10713.en.md +++ b/changes/ce/feat-10713.en.md @@ -1,3 +1,3 @@ -We deprecated the request_timeout in resource_option of the webhook to keep it consistent with the http request_timeout of the webhook. +We hide the request_timeout in resource_option of the webhook to keep it consistent with the http request_timeout of the webhook. From now on, when configuring a webhook through API or configuration files, it is no longer necessary to configure the request_timeout of the resource. Only configuring the http request_timeout is sufficient, and the request_timeout in the resource will automatically be consistent with the http request_timeout.