From 72409782eb4f943225b871b2f1d4a234dee7d50a Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Mon, 7 Mar 2022 14:32:58 +0800 Subject: [PATCH 1/5] fix: remove the Id field from response of GET, POST /bridges The response body of POST, GET /bridges should be the same as the request body of the POST /bridges: ``` {"type": "mqtt", "name": "my_mqtt_bridge" } ``` We force the user to provide an Id of format `{type}:{name}` when GET, DELETE, PUT a bridge: `GET /bridges/{type}:{name}` --- apps/emqx_bridge/src/emqx_bridge_api.erl | 28 +++++++++---------- .../src/emqx_bridge_http_schema.erl | 5 +--- .../src/emqx_bridge_mqtt_schema.erl | 9 ++---- 3 files changed, 17 insertions(+), 25 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 27171bd08..d373f29ec 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -88,16 +88,18 @@ get_response_body_schema() -> bridge_info_examples(get)). param_path_operation() -> - path_param(operation, enum([start, stop, restart]), <<"start">>). - -param_path_id() -> - path_param(id, binary(), <<"http:my_http_bridge">>). - -path_param(Name, Type, Example) -> - {Name, mk(Type, + {operation, mk(enum([start, stop, restart]), #{ in => path , required => true - , example => Example + , example => <<"start">> + })}. + +param_path_id() -> + {id, mk(binary(), + #{ in => path + , required => true + , example => <<"http:my_http_bridge">> + , desc => <<"The bridge Id. Must be of format {type}:{name}">> })}. bridge_info_array_example(Method) -> @@ -140,7 +142,6 @@ method_example(Type, Direction, get) -> _ -> "my_" ++ SDir ++ "_" ++ SType ++ "_bridge" end, #{ - id => bin(SType ++ ":" ++ SName), type => bin(SType), name => bin(SName), metrics => ?METRICS(0, 0, 0, 0, 0, 0), @@ -216,7 +217,7 @@ schema("/bridges") -> post => #{ tags => [<<"bridges">>], summary => <<"Create Bridge">>, - description => <<"Create a new bridge">>, + description => <<"Create a new bridge by type and name">>, requestBody => emqx_dashboard_swagger:schema_with_examples( emqx_bridge_schema:post_request(), bridge_info_examples(post)), @@ -243,7 +244,7 @@ schema("/bridges/:id") -> put => #{ tags => [<<"bridges">>], summary => <<"Update Bridge">>, - description => <<"Update a bridge">>, + description => <<"Update a bridge by Id">>, parameters => [param_path_id()], requestBody => emqx_dashboard_swagger:schema_with_examples( emqx_bridge_schema:put_request(), @@ -257,7 +258,7 @@ schema("/bridges/:id") -> delete => #{ tags => [<<"bridges">>], summary => <<"Delete Bridge">>, - description => <<"Delete a bridge">>, + description => <<"Delete a bridge by Id">>, parameters => [param_path_id()], responses => #{ 204 => <<"Bridge deleted">> @@ -271,7 +272,7 @@ schema("/bridges/:id/operation/:operation") -> post => #{ tags => [<<"bridges">>], summary => <<"Start/Stop/Restart Bridge">>, - description => <<"Start/Stop/Restart bridges on a specific node">>, + description => <<"Start/Stop/Restart bridges on a specific node.">>, parameters => [ param_path_id(), param_path_operation() @@ -425,7 +426,6 @@ format_resp(#{id := Id, raw_config := RawConf, {Type, BridgeName} = emqx_bridge:parse_bridge_id(Id), IsConnected = fun(connected) -> connected; (_) -> disconnected end, RawConf#{ - id => Id, type => Type, name => maps:get(<<"name">>, RawConf, BridgeName), node => node(), diff --git a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl index ccf9c0939..1ebb12c0e 100644 --- a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl @@ -74,8 +74,7 @@ fields("put") -> fields("bridge"); fields("get") -> - [ id_field() - ] ++ emqx_bridge_schema:metrics_status_fields() ++ fields("post"). + emqx_bridge_schema:metrics_status_fields() ++ fields("post"). basic_config() -> [ {enable, @@ -96,8 +95,6 @@ basic_config() -> ++ proplists:delete(base_url, emqx_connector_http:fields(config)). %%====================================================================================== -id_field() -> - {id, mk(binary(), #{desc => "The Bridge ID", example => "http:my_http_bridge"})}. type_field() -> {type, mk(http, #{desc => "The Bridge Type"})}. diff --git a/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl b/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl index 9522b6120..923da3591 100644 --- a/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl @@ -35,15 +35,10 @@ fields("put_egress") -> proplists:delete(enable, fields("egress")); fields("get_ingress") -> - [ id_field() - ] ++ emqx_bridge_schema:metrics_status_fields() ++ fields("post_ingress"); + emqx_bridge_schema:metrics_status_fields() ++ fields("post_ingress"); fields("get_egress") -> - [ id_field() - ] ++ emqx_bridge_schema:metrics_status_fields() ++ fields("post_egress"). + emqx_bridge_schema:metrics_status_fields() ++ fields("post_egress"). %%====================================================================================== -id_field() -> - {id, mk(binary(), #{desc => "The bridge ID", example => "mqtt:my_mqtt_bridge"})}. - type_field() -> {type, mk(mqtt, #{desc => "The bridge type"})}. From 9a9c92ae880d4cab05158c0474c9257e12688dc1 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Mon, 7 Mar 2022 16:48:36 +0800 Subject: [PATCH 2/5] fix: update testcases for emqx_bridge --- apps/emqx/include/http_api.hrl | 4 +-- apps/emqx_bridge/src/emqx_bridge.erl | 6 ++-- apps/emqx_bridge/src/emqx_bridge_api.erl | 26 ++++++++--------- .../src/emqx_bridge_http_schema.erl | 20 ++++++++----- .../src/emqx_bridge_mqtt_schema.erl | 11 ++++++- apps/emqx_bridge/src/emqx_bridge_schema.erl | 6 +--- .../test/emqx_bridge_api_SUITE.erl | 29 +++++++------------ .../src/emqx_mgmt_api_banned.erl | 4 +-- .../src/emqx_mgmt_api_trace.erl | 2 +- 9 files changed, 56 insertions(+), 52 deletions(-) diff --git a/apps/emqx/include/http_api.hrl b/apps/emqx/include/http_api.hrl index ac09df989..cb0c49df0 100644 --- a/apps/emqx/include/http_api.hrl +++ b/apps/emqx/include/http_api.hrl @@ -17,7 +17,7 @@ %% Bad Request -define(BAD_REQUEST, 'BAD_REQUEST'). --define(ALREADY_EXISTED, 'ALREADY_EXISTED'). +-define(ALREADY_EXISTS, 'ALREADY_EXISTS'). -define(BAD_CONFIG_SCHEMA, 'BAD_CONFIG_SCHEMA'). -define(BAD_LISTENER_ID, 'BAD_LISTENER_ID'). -define(BAD_NODE_NAME, 'BAD_NODE_NAME'). @@ -49,7 +49,7 @@ %% All codes -define(ERROR_CODES, [ {'BAD_REQUEST', <<"Request parameters are not legal">>} - , {'ALREADY_EXISTED', <<"Resource already existed">>} + , {'ALREADY_EXISTS', <<"Resource already existed">>} , {'BAD_CONFIG_SCHEMA', <<"Configuration data is not legal">>} , {'BAD_LISTENER_ID', <<"Bad listener ID">>} , {'BAD_NODE_NAME', <<"Bad Node Name">>} diff --git a/apps/emqx_bridge/src/emqx_bridge.erl b/apps/emqx_bridge/src/emqx_bridge.erl index 446d87a6e..2f8154cf2 100644 --- a/apps/emqx_bridge/src/emqx_bridge.erl +++ b/apps/emqx_bridge/src/emqx_bridge.erl @@ -31,6 +31,7 @@ , bridge_type/1 , resource_id/1 , resource_id/2 + , bridge_id/2 , parse_bridge_id/1 ]). @@ -202,8 +203,9 @@ lookup(Type, Name) -> lookup(Type, Name, RawConf) -> case emqx_resource:get_instance(resource_id(Type, Name)) of {error, not_found} -> {error, not_found}; - {ok, _, Data} -> {ok, #{id => bridge_id(Type, Name), resource_data => Data, - raw_config => RawConf}} + {ok, _, Data} -> + {ok, #{type => Type, name => Name, resource_data => Data, + raw_config => RawConf}} end. start(Type, Name) -> diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index d373f29ec..01a583f4e 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -284,9 +284,8 @@ schema("/bridges/:id/operation/:operation") -> } }. -'/bridges'(post, #{body := #{<<"type">> := BridgeType} = Conf0}) -> +'/bridges'(post, #{body := #{<<"type">> := BridgeType, <<"name">> := BridgeName} = Conf0}) -> Conf = filter_out_request_body(Conf0), - BridgeName = emqx_misc:gen_id(), case emqx_bridge:lookup(BridgeType, BridgeName) of {ok, _} -> {400, error_msg('ALREADY_EXISTS', <<"bridge already exists">>)}; @@ -324,7 +323,7 @@ schema("/bridges/:id/operation/:operation") -> #{override_to => cluster}) of {ok, _} -> {204}; {error, Reason} -> - {500, error_msg('UNKNOWN_ERROR', Reason)} + {500, error_msg('INTERNAL_ERROR', Reason)} end). lookup_from_all_nodes(BridgeType, BridgeName, SuccCode) -> @@ -335,7 +334,7 @@ lookup_from_all_nodes(BridgeType, BridgeName, SuccCode) -> {ok, [{error, not_found} | _]} -> {404, error_msg('NOT_FOUND', <<"not_found">>)}; {error, ErrL} -> - {500, error_msg('UNKNOWN_ERROR', ErrL)} + {500, error_msg('INTERNAL_ERROR', ErrL)} end. lookup_from_local_node(BridgeType, BridgeName) -> @@ -355,7 +354,7 @@ lookup_from_local_node(BridgeType, BridgeName) -> {error, {pre_config_update, _, bridge_not_found}} -> {404, error_msg('NOT_FOUND', <<"bridge not found">>)}; {error, Reason} -> - {500, error_msg('UNKNOWN_ERROR', Reason)} + {500, error_msg('INTERNAL_ERROR', Reason)} end end). @@ -373,17 +372,19 @@ ensure_bridge_created(BridgeType, BridgeName, Conf) -> end. zip_bridges([BridgesFirstNode | _] = BridgesAllNodes) -> - lists:foldl(fun(#{id := Id}, Acc) -> - Bridges = pick_bridges_by_id(Id, BridgesAllNodes), + lists:foldl(fun(#{type := Type, name := Name}, Acc) -> + Bridges = pick_bridges_by_id(Type, Name, BridgesAllNodes), [format_bridge_info(Bridges) | Acc] end, [], BridgesFirstNode). -pick_bridges_by_id(Id, BridgesAllNodes) -> +pick_bridges_by_id(Type, Name, BridgesAllNodes) -> lists:foldl(fun(BridgesOneNode, Acc) -> - case [Bridge || Bridge = #{id := Id0} <- BridgesOneNode, Id0 == Id] of + case [Bridge || Bridge = #{type := Type0, name := Name0} <- BridgesOneNode, + Type0 == Type, Name0 == Name] of [BridgeInfo] -> [BridgeInfo | Acc]; [] -> - ?SLOG(warning, #{msg => "bridge_inconsistent_in_cluster", bridge => Id}), + ?SLOG(warning, #{msg => "bridge_inconsistent_in_cluster", + bridge => emqx_bridge:bridge_id(Type, Name)}), Acc end end, [], BridgesAllNodes). @@ -421,9 +422,8 @@ aggregate_metrics(AllMetrics) -> Rate1 + Rate0, Rate5m1 + Rate5m0, RateMax1 + RateMax0) end, InitMetrics, AllMetrics). -format_resp(#{id := Id, raw_config := RawConf, +format_resp(#{type := Type, name := BridgeName, raw_config := RawConf, resource_data := #{status := Status, metrics := Metrics}}) -> - {Type, BridgeName} = emqx_bridge:parse_bridge_id(Id), IsConnected = fun(connected) -> connected; (_) -> disconnected end, RawConf#{ type => Type, @@ -448,7 +448,7 @@ is_ok(ResL) -> end. filter_out_request_body(Conf) -> - ExtraConfs = [<<"id">>, <<"type">>, <<"status">>, <<"node_status">>, + ExtraConfs = [<<"id">>, <<"type">>, <<"name">>, <<"status">>, <<"node_status">>, <<"node_metrics">>, <<"metrics">>, <<"node">>], maps:without(ExtraConfs, Conf). diff --git a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl index 1ebb12c0e..b88d76db6 100644 --- a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl @@ -10,7 +10,7 @@ %% Hocon Schema Definitions roots() -> []. -fields("bridge") -> +fields("config") -> basic_config() ++ [ {url, mk(binary(), #{ required => true @@ -68,10 +68,11 @@ How long will the HTTP request timeout. fields("post") -> [ type_field() - ] ++ fields("bridge"); + , name_field() + ] ++ fields("config"); fields("put") -> - fields("bridge"); + fields("config"); fields("get") -> emqx_bridge_schema:metrics_status_fields() ++ fields("post"). @@ -82,10 +83,6 @@ basic_config() -> #{ desc => "Enable or disable this bridge" , default => true })} - , {name, - mk(binary(), - #{ desc => "Bridge name, used as a human-readable description of the bridge." - })} , {direction, mk(egress, #{ desc => "The direction of this bridge, MUST be 'egress'" @@ -97,7 +94,14 @@ basic_config() -> %%====================================================================================== type_field() -> - {type, mk(http, #{desc => "The Bridge Type"})}. + {type, mk(http, + #{ desc => "The Bridge Type" + })}. + +name_field() -> + {name, mk(binary(), + #{ desc => "Bridge name, used as a human-readable description of the bridge." + })}. method() -> enum([post, put, get, delete]). diff --git a/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl b/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl index 923da3591..dfe975aca 100644 --- a/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl @@ -24,9 +24,11 @@ fields("egress") -> fields("post_ingress") -> [ type_field() + , name_field() ] ++ proplists:delete(enable, fields("ingress")); fields("post_egress") -> [ type_field() + , name_field() ] ++ proplists:delete(enable, fields("egress")); fields("put_ingress") -> @@ -41,4 +43,11 @@ fields("get_egress") -> %%====================================================================================== type_field() -> - {type, mk(mqtt, #{desc => "The bridge type"})}. + {type, mk(mqtt, + #{ desc => "The bridge type." + })}. + +name_field() -> + {name, mk(binary(), + #{ desc => "Bridge name, used as a human-readable description of the bridge." + })}. diff --git a/apps/emqx_bridge/src/emqx_bridge_schema.erl b/apps/emqx_bridge/src/emqx_bridge_schema.erl index 88f3634b7..012cdca8a 100644 --- a/apps/emqx_bridge/src/emqx_bridge_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_schema.erl @@ -46,10 +46,6 @@ common_bridge_fields() -> #{ desc => "Enable or disable this bridge" , default => true })} - , {name, - mk(binary(), - #{ desc => "Bridge name, used as a human-readable description of the bridge." - })} , {connector, mk(binary(), #{ required => true @@ -86,7 +82,7 @@ direction_field(Dir, Desc) -> roots() -> [bridges]. fields(bridges) -> - [{http, mk(hoconsc:map(name, ref(emqx_bridge_http_schema, "bridge")), #{})}] + [{http, mk(hoconsc:map(name, ref(emqx_bridge_http_schema, "config")), #{})}] ++ [{T, mk(hoconsc:map(name, hoconsc:union([ ref(schema_mod(T), "ingress"), ref(schema_mod(T), "egress") diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index dc5129523..117912275 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -152,8 +152,7 @@ t_http_crud_apis(_) -> ?HTTP_BRIDGE(URL1, ?BRIDGE_TYPE, ?BRIDGE_NAME)), %ct:pal("---bridge: ~p", [Bridge]), - #{ <<"id">> := BridgeID - , <<"type">> := ?BRIDGE_TYPE + #{ <<"type">> := ?BRIDGE_TYPE , <<"name">> := ?BRIDGE_NAME , <<"status">> := _ , <<"node_status">> := [_|_] @@ -162,6 +161,7 @@ t_http_crud_apis(_) -> , <<"url">> := URL1 } = jsx:decode(Bridge), + BridgeID = emqx_bridge:bridge_id(?BRIDGE_TYPE, ?BRIDGE_NAME), %% send an message to emqx and the message should be forwarded to the HTTP server wait_for_resource_ready(BridgeID, 5), Body = <<"my msg">>, @@ -181,8 +181,7 @@ t_http_crud_apis(_) -> URL2 = ?URL(Port, "path2"), {ok, 200, Bridge2} = request(put, uri(["bridges", BridgeID]), ?HTTP_BRIDGE(URL2, ?BRIDGE_TYPE, ?BRIDGE_NAME)), - ?assertMatch(#{ <<"id">> := BridgeID - , <<"type">> := ?BRIDGE_TYPE + ?assertMatch(#{ <<"type">> := ?BRIDGE_TYPE , <<"name">> := ?BRIDGE_NAME , <<"status">> := _ , <<"node_status">> := [_|_] @@ -193,8 +192,7 @@ t_http_crud_apis(_) -> %% list all bridges again, assert Bridge2 is in it {ok, 200, Bridge2Str} = request(get, uri(["bridges"]), []), - ?assertMatch([#{ <<"id">> := BridgeID - , <<"type">> := ?BRIDGE_TYPE + ?assertMatch([#{ <<"type">> := ?BRIDGE_TYPE , <<"name">> := ?BRIDGE_NAME , <<"status">> := _ , <<"node_status">> := [_|_] @@ -205,8 +203,7 @@ t_http_crud_apis(_) -> %% get the bridge by id {ok, 200, Bridge3Str} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{ <<"id">> := BridgeID - , <<"type">> := ?BRIDGE_TYPE + ?assertMatch(#{ <<"type">> := ?BRIDGE_TYPE , <<"name">> := ?BRIDGE_NAME , <<"status">> := _ , <<"node_status">> := [_|_] @@ -251,8 +248,7 @@ t_start_stop_bridges(_) -> {ok, 201, Bridge} = request(post, uri(["bridges"]), ?HTTP_BRIDGE(URL1, ?BRIDGE_TYPE, ?BRIDGE_NAME)), %ct:pal("the bridge ==== ~p", [Bridge]), - #{ <<"id">> := BridgeID - , <<"type">> := ?BRIDGE_TYPE + #{ <<"type">> := ?BRIDGE_TYPE , <<"name">> := ?BRIDGE_NAME , <<"status">> := _ , <<"node_status">> := [_|_] @@ -260,31 +256,28 @@ t_start_stop_bridges(_) -> , <<"node_metrics">> := [_|_] , <<"url">> := URL1 } = jsx:decode(Bridge), + BridgeID = emqx_bridge:bridge_id(?BRIDGE_TYPE, ?BRIDGE_NAME), %% stop it {ok, 200, <<>>} = request(post, operation_path(stop, BridgeID), <<"">>), {ok, 200, Bridge2} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{ <<"id">> := BridgeID - , <<"status">> := <<"disconnected">> + ?assertMatch(#{ <<"status">> := <<"disconnected">> }, jsx:decode(Bridge2)), %% start again {ok, 200, <<>>} = request(post, operation_path(start, BridgeID), <<"">>), {ok, 200, Bridge3} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{ <<"id">> := BridgeID - , <<"status">> := <<"connected">> + ?assertMatch(#{ <<"status">> := <<"connected">> }, jsx:decode(Bridge3)), %% restart an already started bridge {ok, 200, <<>>} = request(post, operation_path(restart, BridgeID), <<"">>), {ok, 200, Bridge3} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{ <<"id">> := BridgeID - , <<"status">> := <<"connected">> + ?assertMatch(#{ <<"status">> := <<"connected">> }, jsx:decode(Bridge3)), %% stop it again {ok, 200, <<>>} = request(post, operation_path(stop, BridgeID), <<"">>), %% restart a stopped bridge {ok, 200, <<>>} = request(post, operation_path(restart, BridgeID), <<"">>), {ok, 200, Bridge4} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{ <<"id">> := BridgeID - , <<"status">> := <<"connected">> + ?assertMatch(#{ <<"status">> := <<"connected">> }, jsx:decode(Bridge4)), %% delete the bridge {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []), diff --git a/apps/emqx_management/src/emqx_mgmt_api_banned.erl b/apps/emqx_management/src/emqx_mgmt_api_banned.erl index 265fb2a9b..65e0cd549 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_banned.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_banned.erl @@ -68,7 +68,7 @@ schema("/banned") -> responses => #{ 200 => [{data, hoconsc:mk(hoconsc:array(hoconsc:ref(ban)), #{})}], 400 => emqx_dashboard_swagger:error_codes( - ['ALREADY_EXISTED', 'BAD_REQUEST'], + ['ALREADY_EXISTS', 'BAD_REQUEST'], <<"Banned already existed, or bad args">>) } } @@ -141,7 +141,7 @@ banned(post, #{body := Body}) -> case emqx_banned:create(Ban) of {ok, Banned} -> {200, format(Banned)}; {error, {already_exist, Old}} -> - {400, 'ALREADY_EXISTED', format(Old)} + {400, 'ALREADY_EXISTS', format(Old)} end end. diff --git a/apps/emqx_management/src/emqx_mgmt_api_trace.erl b/apps/emqx_management/src/emqx_mgmt_api_trace.erl index b9aaec95e..2890262de 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_trace.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_trace.erl @@ -264,7 +264,7 @@ trace(post, #{body := Param}) -> {ok, Trace0} -> {200, format_trace(Trace0)}; {error, {already_existed, Name}} -> {400, #{ - code => 'ALREADY_EXISTED', + code => 'ALREADY_EXISTS', message => ?TO_BIN([Name, " Already Exists"]) }}; {error, {duplicate_condition, Name}} -> From 2897af9650d910a03aa0dac6922868193ace1976 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Mon, 7 Mar 2022 18:39:11 +0800 Subject: [PATCH 3/5] fix: update testcases for emqx_connector --- apps/emqx_bridge/src/emqx_bridge.erl | 6 ++- apps/emqx_connector/src/emqx_connector.erl | 7 ++-- .../test/emqx_connector_api_SUITE.erl | 41 +++++++++---------- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge.erl b/apps/emqx_bridge/src/emqx_bridge.erl index 2f8154cf2..a6f86cfd6 100644 --- a/apps/emqx_bridge/src/emqx_bridge.erl +++ b/apps/emqx_bridge/src/emqx_bridge.erl @@ -47,7 +47,7 @@ , recreate/3 , create_dry_run/2 , remove/1 - , remove/3 + , remove/2 , update/2 , update/3 , start/2 @@ -286,6 +286,10 @@ remove(BridgeId) -> {BridgeType, BridgeName} = parse_bridge_id(BridgeId), remove(BridgeType, BridgeName, #{}). +remove(Type, Name) -> + remove(Type, Name, undefined). + +%% just for perform_bridge_changes/1 remove(Type, Name, _Conf) -> ?SLOG(info, #{msg => "remove_bridge", type => Type, name => Name}), case emqx_resource:remove_local(resource_id(Type, Name)) of diff --git a/apps/emqx_connector/src/emqx_connector.erl b/apps/emqx_connector/src/emqx_connector.erl index c30bbb1c4..184fcc733 100644 --- a/apps/emqx_connector/src/emqx_connector.erl +++ b/apps/emqx_connector/src/emqx_connector.erl @@ -40,16 +40,15 @@ config_key_path() -> -dialyzer([{nowarn_function, [post_config_update/5]}, error_handling]). post_config_update([connectors, Type, Name], '$remove', _, _OldConf, _AppEnvs) -> ConnId = connector_id(Type, Name), - try foreach_linked_bridges(ConnId, fun(#{id := BId}) -> - throw({dependency_bridges_exist, BId}) + try foreach_linked_bridges(ConnId, fun(#{type := BType, name := BName}) -> + throw({dependency_bridges_exist, emqx_bridge:bridge_id(BType, BName)}) end) catch throw:Error -> {error, Error} end; post_config_update([connectors, Type, Name], _Req, NewConf, OldConf, _AppEnvs) -> ConnId = connector_id(Type, Name), foreach_linked_bridges(ConnId, - fun(#{id := BId}) -> - {BType, BName} = emqx_bridge:parse_bridge_id(BId), + fun(#{type := BType, name := BName}) -> BridgeConf = emqx:get_config([bridges, BType, BName]), case emqx_bridge:update(BType, BName, {BridgeConf#{connector => OldConf}, BridgeConf#{connector => NewConf}}) of diff --git a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl index 7c83671a9..58486a809 100644 --- a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl +++ b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl @@ -124,8 +124,8 @@ clear_resources() -> lists:foreach(fun(#{id := Id}) -> ok = emqx_rule_engine:delete_rule(Id) end, emqx_rule_engine:get_rules()), - lists:foreach(fun(#{id := Id}) -> - ok = emqx_bridge:remove(Id) + lists:foreach(fun(#{type := Type, name := Name}) -> + ok = emqx_bridge:remove(Type, Name) end, emqx_bridge:list()), lists:foreach(fun(#{<<"id">> := Id}) -> ok = emqx_connector:delete(Id) @@ -231,10 +231,11 @@ t_mqtt_conn_bridge_ingress(_) -> <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_INGRESS }), - #{ <<"id">> := BridgeIDIngress - , <<"type">> := <<"mqtt">> + #{ <<"type">> := ?CONNECTR_TYPE + , <<"name">> := ?BRIDGE_NAME_INGRESS , <<"connector">> := ConnctorID } = jsx:decode(Bridge), + BridgeIDIngress = emqx_bridge:bridge_id(?CONNECTR_TYPE, ?BRIDGE_NAME_INGRESS), wait_for_resource_ready(BridgeIDIngress, 5), %% we now test if the bridge works as expected @@ -298,11 +299,11 @@ t_mqtt_conn_bridge_egress(_) -> <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_EGRESS }), - #{ <<"id">> := BridgeIDEgress - , <<"type">> := ?CONNECTR_TYPE + #{ <<"type">> := ?CONNECTR_TYPE , <<"name">> := ?BRIDGE_NAME_EGRESS , <<"connector">> := ConnctorID } = jsx:decode(Bridge), + BridgeIDEgress = emqx_bridge:bridge_id(?CONNECTR_TYPE, ?BRIDGE_NAME_EGRESS), wait_for_resource_ready(BridgeIDEgress, 5), %% we now test if the bridge works as expected @@ -330,8 +331,7 @@ t_mqtt_conn_bridge_egress(_) -> %% verify the metrics of the bridge {ok, 200, BridgeStr} = request(get, uri(["bridges", BridgeIDEgress]), []), - ?assertMatch(#{ <<"id">> := BridgeIDEgress - , <<"metrics">> := ?metrics(1, 1, 0, _, _, _) + ?assertMatch(#{ <<"metrics">> := ?metrics(1, 1, 0, _, _, _) , <<"node_metrics">> := [#{<<"node">> := _, <<"metrics">> := ?metrics(1, 1, 0, _, _, _)}] }, jsx:decode(BridgeStr)), @@ -368,11 +368,11 @@ t_mqtt_conn_update(_) -> <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_EGRESS }), - #{ <<"id">> := BridgeIDEgress - , <<"type">> := <<"mqtt">> + #{ <<"type">> := ?CONNECTR_TYPE , <<"name">> := ?BRIDGE_NAME_EGRESS , <<"connector">> := ConnctorID } = jsx:decode(Bridge), + BridgeIDEgress = emqx_bridge:bridge_id(?CONNECTR_TYPE, ?BRIDGE_NAME_EGRESS), wait_for_resource_ready(BridgeIDEgress, 5), %% then we try to update 'server' of the connector, to an unavailable IP address @@ -410,12 +410,12 @@ t_mqtt_conn_update2(_) -> <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_EGRESS }), - #{ <<"id">> := BridgeIDEgress - , <<"type">> := <<"mqtt">> + #{ <<"type">> := ?CONNECTR_TYPE , <<"name">> := ?BRIDGE_NAME_EGRESS , <<"status">> := <<"disconnected">> , <<"connector">> := ConnctorID } = jsx:decode(Bridge), + BridgeIDEgress = emqx_bridge:bridge_id(?CONNECTR_TYPE, ?BRIDGE_NAME_EGRESS), %% We try to fix the 'server' parameter, to another unavailable server.. %% The update should success: we don't check the connectivity of the new config %% if the resource is now disconnected. @@ -426,8 +426,7 @@ t_mqtt_conn_update2(_) -> ?MQTT_CONNECTOR2(<<"127.0.0.1:1883">>)), wait_for_resource_ready(BridgeIDEgress, 5), {ok, 200, BridgeStr} = request(get, uri(["bridges", BridgeIDEgress]), []), - ?assertMatch(#{ <<"id">> := BridgeIDEgress - , <<"status">> := <<"connected">> + ?assertMatch(#{ <<"status">> := <<"connected">> }, jsx:decode(BridgeStr)), %% delete the bridge {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeIDEgress]), []), @@ -453,9 +452,9 @@ t_mqtt_conn_update3(_) -> <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_EGRESS }), - #{ <<"id">> := BridgeIDEgress - , <<"connector">> := ConnctorID + #{ <<"connector">> := ConnctorID } = jsx:decode(Bridge), + BridgeIDEgress = emqx_bridge:bridge_id(?CONNECTR_TYPE, ?BRIDGE_NAME_EGRESS), wait_for_resource_ready(BridgeIDEgress, 5), %% delete the connector should fail because it is in use by a bridge @@ -486,12 +485,12 @@ t_ingress_mqtt_bridge_with_rules(_) -> }), #{ <<"id">> := ConnctorID } = jsx:decode(Connector), - {ok, 201, Bridge} = request(post, uri(["bridges"]), + {ok, 201, _} = request(post, uri(["bridges"]), ?MQTT_BRIDGE_INGRESS(ConnctorID)#{ <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_INGRESS }), - #{ <<"id">> := BridgeIDIngress } = jsx:decode(Bridge), + BridgeIDIngress = emqx_bridge:bridge_id(?CONNECTR_TYPE, ?BRIDGE_NAME_INGRESS), {ok, 201, Rule} = request(post, uri(["rules"]), #{<<"name">> => <<"A rule get messages from a source mqtt bridge">>, @@ -572,7 +571,8 @@ t_egress_mqtt_bridge_with_rules(_) -> <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_EGRESS }), - #{ <<"id">> := BridgeIDEgress } = jsx:decode(Bridge), + #{ <<"type">> := ?CONNECTR_TYPE, <<"name">> := ?BRIDGE_NAME_EGRESS } = jsx:decode(Bridge), + BridgeIDEgress = emqx_bridge:bridge_id(?CONNECTR_TYPE, ?BRIDGE_NAME_EGRESS), {ok, 201, Rule} = request(post, uri(["rules"]), #{<<"name">> => <<"A rule send messages to a sink mqtt bridge">>, @@ -647,8 +647,7 @@ t_egress_mqtt_bridge_with_rules(_) -> %% verify the metrics of the bridge {ok, 200, BridgeStr} = request(get, uri(["bridges", BridgeIDEgress]), []), - ?assertMatch(#{ <<"id">> := BridgeIDEgress - , <<"metrics">> := ?metrics(2, 2, 0, _, _, _) + ?assertMatch(#{ <<"metrics">> := ?metrics(2, 2, 0, _, _, _) , <<"node_metrics">> := [#{<<"node">> := _, <<"metrics">> := ?metrics(2, 2, 0, _, _, _)}] }, jsx:decode(BridgeStr)), From b20902ebfe5b2d848378a5d7daf919ee0e479161 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Mon, 7 Mar 2022 19:33:32 +0800 Subject: [PATCH 4/5] fix: remove the Id field from response of GET, POST /connectors --- .../src/emqx_bridge_http_schema.erl | 6 +- .../src/emqx_bridge_mqtt_schema.erl | 6 +- apps/emqx_connector/src/emqx_connector.erl | 5 +- .../emqx_connector/src/emqx_connector_api.erl | 44 ++++++--------- .../src/emqx_connector_mqtt.erl | 15 +++-- .../src/mqtt/emqx_connector_mqtt_schema.erl | 5 -- .../test/emqx_connector_api_SUITE.erl | 55 ++++++++----------- 7 files changed, 61 insertions(+), 75 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl index b88d76db6..76a30f871 100644 --- a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl @@ -95,12 +95,14 @@ basic_config() -> type_field() -> {type, mk(http, - #{ desc => "The Bridge Type" + #{ required => true + , desc => "The Bridge Type" })}. name_field() -> {name, mk(binary(), - #{ desc => "Bridge name, used as a human-readable description of the bridge." + #{ required => true + , desc => "Bridge name, used as a human-readable description of the bridge." })}. method() -> diff --git a/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl b/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl index dfe975aca..67a3806b2 100644 --- a/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_mqtt_schema.erl @@ -44,10 +44,12 @@ fields("get_egress") -> %%====================================================================================== type_field() -> {type, mk(mqtt, - #{ desc => "The bridge type." + #{ required => true + , desc => "The bridge type." })}. name_field() -> {name, mk(binary(), - #{ desc => "Bridge name, used as a human-readable description of the bridge." + #{ required => true + , desc => "Bridge name, used as a human-readable description of the bridge." })}. diff --git a/apps/emqx_connector/src/emqx_connector.erl b/apps/emqx_connector/src/emqx_connector.erl index 184fcc733..60d1630b4 100644 --- a/apps/emqx_connector/src/emqx_connector.erl +++ b/apps/emqx_connector/src/emqx_connector.erl @@ -71,7 +71,7 @@ parse_connector_id(ConnectorId) -> list() -> lists:foldl(fun({Type, NameAndConf}, Connectors) -> lists:foldl(fun({Name, RawConf}, Acc) -> - [RawConf#{<<"id">> => connector_id(Type, Name)} | Acc] + [RawConf#{<<"type">> => Type, <<"name">> => Name} | Acc] end, Connectors, maps:to_list(NameAndConf)) end, [], maps:to_list(emqx:get_raw_config(config_key_path(), #{}))). @@ -80,10 +80,9 @@ lookup(Id) when is_binary(Id) -> lookup(Type, Name). lookup(Type, Name) -> - Id = connector_id(Type, Name), case emqx:get_raw_config(config_key_path() ++ [Type, Name], not_found) of not_found -> {error, not_found}; - Conf -> {ok, Conf#{<<"id">> => Id}} + Conf -> {ok, Conf#{<<"type">> => Type, <<"name">> => Name}} end. create_dry_run(Type, Conf) -> diff --git a/apps/emqx_connector/src/emqx_connector_api.erl b/apps/emqx_connector/src/emqx_connector_api.erl index 031492d86..c9f9509c9 100644 --- a/apps/emqx_connector/src/emqx_connector_api.erl +++ b/apps/emqx_connector/src/emqx_connector_api.erl @@ -85,15 +85,7 @@ info_example(Type, Method) -> maps:merge(info_example_basic(Type), method_example(Type, Method)). -method_example(Type, get) -> - SType = atom_to_list(Type), - SName = "my_" ++ SType ++ "_connector", - #{ - id => bin(SType ++ ":" ++ SName), - type => bin(SType), - name => bin(SName) - }; -method_example(Type, post) -> +method_example(Type, Method) when Method == get; Method == post -> SType = atom_to_list(Type), SName = "my_" ++ SType ++ "_connector", #{ @@ -122,7 +114,11 @@ info_example_basic(mqtt) -> }. param_path_id() -> - [{id, mk(binary(), #{in => path, example => <<"mqtt:my_mqtt_connector">>})}]. + [{id, mk(binary(), + #{ in => path + , example => <<"mqtt:my_mqtt_connector">> + , desc => <<"The connector Id. Must be of format {type}:{name}">> + })}]. schema("/connectors_test") -> #{ @@ -211,8 +207,7 @@ schema("/connectors/:id") -> '/connectors'(get, _Request) -> {200, [format_resp(Conn) || Conn <- emqx_connector:list()]}; -'/connectors'(post, #{body := #{<<"type">> := ConnType} = Params}) -> - ConnName = emqx_misc:gen_id(), +'/connectors'(post, #{body := #{<<"type">> := ConnType, <<"name">> := ConnName} = Params}) -> case emqx_connector:lookup(ConnType, ConnName) of {ok, _} -> {400, error_msg('ALREADY_EXISTS', <<"connector already exists">>)}; @@ -220,8 +215,8 @@ schema("/connectors/:id") -> case emqx_connector:update(ConnType, ConnName, filter_out_request_body(Params)) of {ok, #{raw_config := RawConf}} -> - Id = emqx_connector:connector_id(ConnType, ConnName), - {201, format_resp(Id, RawConf)}; + {201, format_resp(RawConf#{<<"type">> => ConnType, + <<"name">> => ConnName})}; {error, Error} -> {400, error_msg('ALREADY_EXISTS', Error)} end @@ -231,7 +226,7 @@ schema("/connectors/:id") -> ?TRY_PARSE_ID(Id, case emqx_connector:lookup(ConnType, ConnName) of {ok, Conf} -> - {200, format_resp(Id, Conf)}; + {200, format_resp(Conf)}; {error, not_found} -> {404, error_msg('NOT_FOUND', <<"connector not found">>)} end); @@ -243,7 +238,8 @@ schema("/connectors/:id") -> {ok, _} -> case emqx_connector:update(ConnType, ConnName, Params) of {ok, #{raw_config := RawConf}} -> - {200, format_resp(Id, RawConf)}; + {200, format_resp(RawConf#{<<"type">> => ConnType, + <<"name">> => ConnName})}; {error, Error} -> {500, error_msg('INTERNAL_ERROR', Error)} end; @@ -274,21 +270,17 @@ error_msg(Code, Msg) when is_binary(Msg) -> error_msg(Code, Msg) -> #{code => Code, message => bin(io_lib:format("~p", [Msg]))}. -format_resp(#{<<"id">> := Id} = RawConf) -> - format_resp(Id, RawConf). - -format_resp(ConnId, RawConf) -> - NumOfBridges = length(emqx_bridge:list_bridges_by_connector(ConnId)), - {Type, ConnName} = emqx_connector:parse_connector_id(ConnId), +format_resp(#{<<"type">> := ConnType, <<"name">> := ConnName} = RawConf) -> + NumOfBridges = length(emqx_bridge:list_bridges_by_connector( + emqx_connector:connector_id(ConnType, ConnName))), RawConf#{ - <<"id">> => ConnId, - <<"type">> => Type, - <<"name">> => maps:get(<<"name">>, RawConf, ConnName), + <<"type">> => ConnType, + <<"name">> => ConnName, <<"num_of_bridges">> => NumOfBridges }. filter_out_request_body(Conf) -> - ExtraConfs = [<<"clientid">>, <<"num_of_bridges">>, <<"type">>], + ExtraConfs = [<<"clientid">>, <<"num_of_bridges">>, <<"type">>, <<"name">>], maps:without(ExtraConfs, Conf). bin(S) when is_list(S) -> diff --git a/apps/emqx_connector/src/emqx_connector_mqtt.erl b/apps/emqx_connector/src/emqx_connector_mqtt.erl index 68dfe66bd..24c0de653 100644 --- a/apps/emqx_connector/src/emqx_connector_mqtt.erl +++ b/apps/emqx_connector/src/emqx_connector_mqtt.erl @@ -54,11 +54,7 @@ fields("config") -> emqx_connector_mqtt_schema:fields("config"); fields("get") -> - [ {id, mk(binary(), - #{ desc => "The connector Id" - , example => <<"mqtt:my_mqtt_connector">> - })} - , {num_of_bridges, mk(integer(), + [ {num_of_bridges, mk(integer(), #{ desc => "The current number of bridges that are using this connector" })} ] ++ fields("post"); @@ -67,7 +63,14 @@ fields("put") -> emqx_connector_mqtt_schema:fields("connector"); fields("post") -> - [ {type, mk(mqtt, #{desc => "The Connector Type"})} + [ {type, mk(mqtt, + #{ required => true + , desc => "The Connector Type" + })} + , {name, mk(binary(), + #{ required => true + , desc => "Connector name, used as a human-readable description of the connector." + })} ] ++ fields("put"). %% =================================================================== diff --git a/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl b/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl index ddf3b8bc2..2c7cfe012 100644 --- a/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl +++ b/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl @@ -55,11 +55,6 @@ clientid conflicts between different nodes. And we can only use shared subscript topic filters for 'remote_topic' of ingress connections. """ })} - , {name, - sc(binary(), - #{ required => false - , desc => "Connector name, used as a human-readable description of the connector." - })} , {server, sc(emqx_schema:ip_port(), #{ default => "127.0.0.1:1883" diff --git a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl index 58486a809..438341978 100644 --- a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl +++ b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl @@ -127,8 +127,8 @@ clear_resources() -> lists:foreach(fun(#{type := Type, name := Name}) -> ok = emqx_bridge:remove(Type, Name) end, emqx_bridge:list()), - lists:foreach(fun(#{<<"id">> := Id}) -> - ok = emqx_connector:delete(Id) + lists:foreach(fun(#{type := Type, name := Name}) -> + ok = emqx_connector:delete(Type, Name) end, emqx_connector:list()). %%------------------------------------------------------------------------------ @@ -147,8 +147,7 @@ t_mqtt_crud_apis(_) -> , <<"name">> => ?CONNECTR_NAME }), - #{ <<"id">> := ConnctorID - , <<"type">> := ?CONNECTR_TYPE + #{ <<"type">> := ?CONNECTR_TYPE , <<"name">> := ?CONNECTR_NAME , <<"server">> := <<"127.0.0.1:1883">> , <<"username">> := User1 @@ -156,12 +155,13 @@ t_mqtt_crud_apis(_) -> , <<"proto_ver">> := <<"v4">> , <<"ssl">> := #{<<"enable">> := false} } = jsx:decode(Connector), - + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), %% update the request-path of the connector User2 = <<"user2">>, {ok, 200, Connector2} = request(put, uri(["connectors", ConnctorID]), ?MQTT_CONNECTOR(User2)), - ?assertMatch(#{ <<"id">> := ConnctorID + ?assertMatch(#{ <<"type">> := ?CONNECTR_TYPE + , <<"name">> := ?CONNECTR_NAME , <<"server">> := <<"127.0.0.1:1883">> , <<"username">> := User2 , <<"password">> := <<"">> @@ -171,8 +171,7 @@ t_mqtt_crud_apis(_) -> %% list all connectors again, assert Connector2 is in it {ok, 200, Connector2Str} = request(get, uri(["connectors"]), []), - ?assertMatch([#{ <<"id">> := ConnctorID - , <<"type">> := ?CONNECTR_TYPE + ?assertMatch([#{ <<"type">> := ?CONNECTR_TYPE , <<"name">> := ?CONNECTR_NAME , <<"server">> := <<"127.0.0.1:1883">> , <<"username">> := User2 @@ -183,8 +182,7 @@ t_mqtt_crud_apis(_) -> %% get the connector by id {ok, 200, Connector3Str} = request(get, uri(["connectors", ConnctorID]), []), - ?assertMatch(#{ <<"id">> := ConnctorID - , <<"type">> := ?CONNECTR_TYPE + ?assertMatch(#{ <<"type">> := ?CONNECTR_TYPE , <<"name">> := ?CONNECTR_NAME , <<"server">> := <<"127.0.0.1:1883">> , <<"username">> := User2 @@ -214,7 +212,8 @@ t_mqtt_conn_bridge_ingress(_) -> , <<"name">> => ?CONNECTR_NAME }), - #{ <<"id">> := ConnctorID + #{ <<"type">> := ?CONNECTR_TYPE + , <<"name">> := ?CONNECTR_NAME , <<"server">> := <<"127.0.0.1:1883">> , <<"num_of_bridges">> := 0 , <<"username">> := User1 @@ -222,7 +221,7 @@ t_mqtt_conn_bridge_ingress(_) -> , <<"proto_ver">> := <<"v4">> , <<"ssl">> := #{<<"enable">> := false} } = jsx:decode(Connector), - + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), %% ... and a MQTT bridge, using POST %% we bind this bridge to the connector created just now timer:sleep(50), @@ -262,8 +261,7 @@ t_mqtt_conn_bridge_ingress(_) -> %% get the connector by id, verify the num_of_bridges now is 1 {ok, 200, Connector1Str} = request(get, uri(["connectors", ConnctorID]), []), - ?assertMatch(#{ <<"id">> := ConnctorID - , <<"num_of_bridges">> := 1 + ?assertMatch(#{ <<"num_of_bridges">> := 1 }, jsx:decode(Connector1Str)), %% delete the bridge @@ -284,14 +282,13 @@ t_mqtt_conn_bridge_egress(_) -> }), %ct:pal("---connector: ~p", [Connector]), - #{ <<"id">> := ConnctorID - , <<"server">> := <<"127.0.0.1:1883">> + #{ <<"server">> := <<"127.0.0.1:1883">> , <<"username">> := User1 , <<"password">> := <<"">> , <<"proto_ver">> := <<"v4">> , <<"ssl">> := #{<<"enable">> := false} } = jsx:decode(Connector), - + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), %% ... and a MQTT bridge, using POST %% we bind this bridge to the connector created just now {ok, 201, Bridge} = request(post, uri(["bridges"]), @@ -357,10 +354,9 @@ t_mqtt_conn_update(_) -> }), %ct:pal("---connector: ~p", [Connector]), - #{ <<"id">> := ConnctorID - , <<"server">> := <<"127.0.0.1:1883">> + #{ <<"server">> := <<"127.0.0.1:1883">> } = jsx:decode(Connector), - + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), %% ... and a MQTT bridge, using POST %% we bind this bridge to the connector created just now {ok, 201, Bridge} = request(post, uri(["bridges"]), @@ -399,10 +395,9 @@ t_mqtt_conn_update2(_) -> , <<"name">> => ?CONNECTR_NAME }), - #{ <<"id">> := ConnctorID - , <<"server">> := <<"127.0.0.1:2603">> + #{ <<"server">> := <<"127.0.0.1:2603">> } = jsx:decode(Connector), - + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), %% ... and a MQTT bridge, using POST %% we bind this bridge to the connector created just now {ok, 201, Bridge} = request(post, uri(["bridges"]), @@ -438,13 +433,12 @@ t_mqtt_conn_update2(_) -> t_mqtt_conn_update3(_) -> %% we add a mqtt connector, using POST - {ok, 201, Connector} = request(post, uri(["connectors"]), + {ok, 201, _} = request(post, uri(["connectors"]), ?MQTT_CONNECTOR2(<<"127.0.0.1:1883">>) #{ <<"type">> => ?CONNECTR_TYPE , <<"name">> => ?CONNECTR_NAME }), - #{ <<"id">> := ConnctorID } = jsx:decode(Connector), - + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), %% ... and a MQTT bridge, using POST %% we bind this bridge to the connector created just now {ok, 201, Bridge} = request(post, uri(["bridges"]), @@ -479,11 +473,11 @@ t_mqtt_conn_testing(_) -> }). t_ingress_mqtt_bridge_with_rules(_) -> - {ok, 201, Connector} = request(post, uri(["connectors"]), + {ok, 201, _} = request(post, uri(["connectors"]), ?MQTT_CONNECTOR(<<"user1">>)#{ <<"type">> => ?CONNECTR_TYPE , <<"name">> => ?CONNECTR_NAME }), - #{ <<"id">> := ConnctorID } = jsx:decode(Connector), + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), {ok, 201, _} = request(post, uri(["bridges"]), ?MQTT_BRIDGE_INGRESS(ConnctorID)#{ @@ -560,12 +554,11 @@ t_ingress_mqtt_bridge_with_rules(_) -> {ok, 204, <<>>} = request(delete, uri(["connectors", ConnctorID]), []). t_egress_mqtt_bridge_with_rules(_) -> - {ok, 201, Connector} = request(post, uri(["connectors"]), + {ok, 201, _} = request(post, uri(["connectors"]), ?MQTT_CONNECTOR(<<"user1">>)#{ <<"type">> => ?CONNECTR_TYPE , <<"name">> => ?CONNECTR_NAME }), - #{ <<"id">> := ConnctorID } = jsx:decode(Connector), - + ConnctorID = emqx_connector:connector_id(?CONNECTR_TYPE, ?CONNECTR_NAME), {ok, 201, Bridge} = request(post, uri(["bridges"]), ?MQTT_BRIDGE_EGRESS(ConnctorID)#{ <<"type">> => ?CONNECTR_TYPE, From 8a0565a53bd44df2e7365d357a1234174f76c22b Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Tue, 8 Mar 2022 10:47:48 +0800 Subject: [PATCH 5/5] chore(elvis): please the elvis --- apps/emqx_bridge/src/emqx_bridge.erl | 3 +- apps/emqx_bridge/src/emqx_bridge_api.erl | 40 +++++++++---------- .../src/emqx_bridge_http_schema.erl | 5 ++- apps/emqx_bridge/src/emqx_bridge_schema.erl | 20 ++++------ .../test/emqx_bridge_api_SUITE.erl | 3 +- .../emqx_connector/src/emqx_connector_api.erl | 12 +++--- .../test/emqx_connector_api_SUITE.erl | 6 ++- .../src/emqx_mgmt_api_trace.erl | 6 ++- elvis.config | 8 ++-- 9 files changed, 51 insertions(+), 52 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge.erl b/apps/emqx_bridge/src/emqx_bridge.erl index a6f86cfd6..e56b694d5 100644 --- a/apps/emqx_bridge/src/emqx_bridge.erl +++ b/apps/emqx_bridge/src/emqx_bridge.erl @@ -224,7 +224,8 @@ create(BridgeId, Conf) -> create(Type, Name, Conf) -> ?SLOG(info, #{msg => "create bridge", type => Type, name => Name, config => Conf}), - case emqx_resource:create_local(resource_id(Type, Name), <<"emqx_bridge">>, emqx_bridge:resource_type(Type), + case emqx_resource:create_local(resource_id(Type, Name), <<"emqx_bridge">>, + emqx_bridge:resource_type(Type), parse_confs(Type, Name, Conf), #{async_create => true}) of {ok, already_created} -> maybe_disable_bridge(Type, Name, Conf); {ok, _} -> maybe_disable_bridge(Type, Name, Conf); diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 01a583f4e..6a0b1a16a 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -134,35 +134,31 @@ info_example(Type, Direction, Method) -> maps:merge(info_example_basic(Type, Direction), method_example(Type, Direction, Method)). -method_example(Type, Direction, get) -> +method_example(Type, Direction, Method) when Method == get; Method == post -> SType = atom_to_list(Type), SDir = atom_to_list(Direction), SName = case Type of http -> "my_" ++ SType ++ "_bridge"; _ -> "my_" ++ SDir ++ "_" ++ SType ++ "_bridge" end, - #{ + TypeNameExamp = #{ type => bin(SType), - name => bin(SName), + name => bin(SName) + }, + maybe_with_metrics_example(TypeNameExamp, Method); +method_example(_Type, _Direction, put) -> + #{}. + +maybe_with_metrics_example(TypeNameExamp, get) -> + TypeNameExamp#{ metrics => ?METRICS(0, 0, 0, 0, 0, 0), node_metrics => [ #{node => node(), - metrics => ?METRICS(0, 0, 0, 0, 0, 0)} + metrics => ?METRICS(0, 0, 0, 0, 0, 0)} ] }; -method_example(Type, Direction, post) -> - SType = atom_to_list(Type), - SDir = atom_to_list(Direction), - SName = case Type of - http -> "my_" ++ SType ++ "_bridge"; - _ -> "my_" ++ SDir ++ "_" ++ SType ++ "_bridge" - end, - #{ - type => bin(SType), - name => bin(SName) - }; -method_example(_Type, _Direction, put) -> - #{}. +maybe_with_metrics_example(TypeNameExamp, _) -> + TypeNameExamp. info_example_basic(http, _) -> #{ @@ -203,7 +199,7 @@ info_example_basic(mqtt, egress) -> schema("/bridges") -> #{ - operationId => '/bridges', + 'operationId' => '/bridges', get => #{ tags => [<<"bridges">>], summary => <<"List Bridges">>, @@ -218,7 +214,7 @@ schema("/bridges") -> tags => [<<"bridges">>], summary => <<"Create Bridge">>, description => <<"Create a new bridge by type and name">>, - requestBody => emqx_dashboard_swagger:schema_with_examples( + 'requestBody' => emqx_dashboard_swagger:schema_with_examples( emqx_bridge_schema:post_request(), bridge_info_examples(post)), responses => #{ @@ -230,7 +226,7 @@ schema("/bridges") -> schema("/bridges/:id") -> #{ - operationId => '/bridges/:id', + 'operationId' => '/bridges/:id', get => #{ tags => [<<"bridges">>], summary => <<"Get Bridge">>, @@ -246,7 +242,7 @@ schema("/bridges/:id") -> summary => <<"Update Bridge">>, description => <<"Update a bridge by Id">>, parameters => [param_path_id()], - requestBody => emqx_dashboard_swagger:schema_with_examples( + 'requestBody' => emqx_dashboard_swagger:schema_with_examples( emqx_bridge_schema:put_request(), bridge_info_examples(put)), responses => #{ @@ -268,7 +264,7 @@ schema("/bridges/:id") -> schema("/bridges/:id/operation/:operation") -> #{ - operationId => '/bridges/:id/operation/:operation', + 'operationId' => '/bridges/:id/operation/:operation', post => #{ tags => [<<"bridges">>], summary => <<"Start/Stop/Restart Bridge">>, diff --git a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl index 76a30f871..2cd6b13e3 100644 --- a/apps/emqx_bridge/src/emqx_bridge_http_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_http_schema.erl @@ -27,8 +27,9 @@ is not allowed. #{ desc =>""" The MQTT topic filter to be forwarded to the HTTP server. All MQTT 'PUBLISH' messages with the topic matching the local_topic will be forwarded.
-NOTE: if this bridge is used as the output of a rule (EMQX rule engine), and also local_topic is configured, then both the data got from the rule and the MQTT messages that match -local_topic will be forwarded. +NOTE: if this bridge is used as the output of a rule (EMQX rule engine), and also local_topic is +configured, then both the data got from the rule and the MQTT messages that match local_topic +will be forwarded. """ })} , {method, mk(method(), diff --git a/apps/emqx_bridge/src/emqx_bridge_schema.erl b/apps/emqx_bridge/src/emqx_bridge_schema.erl index 012cdca8a..1c9f848b9 100644 --- a/apps/emqx_bridge/src/emqx_bridge_schema.erl +++ b/apps/emqx_bridge/src/emqx_bridge_schema.erl @@ -53,7 +53,8 @@ common_bridge_fields() -> , desc =>""" The connector ID to be used for this bridge. Connector IDs must be of format: {type}:{name}.
-In config files, you can find the corresponding config entry for a connector by such path: 'connectors.{type}.{name}'.
+In config files, you can find the corresponding config entry for a connector by such path: +'connectors.{type}.{name}'.
""" })} ]. @@ -63,7 +64,7 @@ metrics_status_fields() -> , {"node_metrics", mk(hoconsc:array(ref(?MODULE, "node_metrics")), #{ desc => "The metrics of the bridge for each node" })} - , {"status", mk(ref(?MODULE, "status"), #{desc => "The status of the bridge"})} + , {"status", mk(status(), #{desc => "The status of the bridge"})} , {"node_status", mk(hoconsc:array(ref(?MODULE, "node_status")), #{ desc => "The status of the bridge for each node" })} @@ -103,21 +104,14 @@ fields("node_metrics") -> , {"metrics", mk(ref(?MODULE, "metrics"), #{})} ]; -fields("status") -> - [ {"matched", mk(integer(), #{desc => "Count of this bridge is queried"})} - , {"success", mk(integer(), #{desc => "Count of query success"})} - , {"failed", mk(integer(), #{desc => "Count of query failed"})} - , {"rate", mk(float(), #{desc => "The rate of matched, times/second"})} - , {"rate_max", mk(float(), #{desc => "The max rate of matched, times/second"})} - , {"rate_last5m", mk(float(), - #{desc => "The average rate of matched in the last 5 minutes, times/second"})} - ]; - fields("node_status") -> [ node_name() - , {"status", mk(ref(?MODULE, "status"), #{})} + , {"status", mk(status(), #{})} ]. +status() -> + hoconsc:enum([connected, disconnected, connecting]). + node_name() -> {"node", mk(binary(), #{desc => "The node name", example => "emqx@127.0.0.1"})}. diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index 117912275..65a369a3d 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -55,7 +55,8 @@ init_per_suite(Config) -> %% some testcases (may from other app) already get emqx_connector started _ = application:stop(emqx_resource), _ = application:stop(emqx_connector), - ok = emqx_common_test_helpers:start_apps([emqx_bridge, emqx_dashboard], fun set_special_configs/1), + ok = emqx_common_test_helpers:start_apps([emqx_bridge, emqx_dashboard], + fun set_special_configs/1), ok = emqx_common_test_helpers:load_config(emqx_bridge_schema, ?CONF_DEFAULT), Config. diff --git a/apps/emqx_connector/src/emqx_connector_api.erl b/apps/emqx_connector/src/emqx_connector_api.erl index c9f9509c9..5262aded3 100644 --- a/apps/emqx_connector/src/emqx_connector_api.erl +++ b/apps/emqx_connector/src/emqx_connector_api.erl @@ -122,13 +122,13 @@ param_path_id() -> schema("/connectors_test") -> #{ - operationId => '/connectors_test', + 'operationId' => '/connectors_test', post => #{ tags => [<<"connectors">>], description => <<"Test creating a new connector by given Id
" "The ID must be of format '{type}:{name}'">>, summary => <<"Test creating connector">>, - requestBody => post_request_body_schema(), + 'requestBody' => post_request_body_schema(), responses => #{ 204 => <<"Test connector OK">>, 400 => error_schema(['TEST_FAILED'], "connector test failed") @@ -138,7 +138,7 @@ schema("/connectors_test") -> schema("/connectors") -> #{ - operationId => '/connectors', + 'operationId' => '/connectors', get => #{ tags => [<<"connectors">>], description => <<"List all connectors">>, @@ -153,7 +153,7 @@ schema("/connectors") -> tags => [<<"connectors">>], description => <<"Create a new connector">>, summary => <<"Create connector">>, - requestBody => post_request_body_schema(), + 'requestBody' => post_request_body_schema(), responses => #{ 201 => get_response_body_schema(), 400 => error_schema(['ALREADY_EXISTS'], "connector already exists") @@ -163,7 +163,7 @@ schema("/connectors") -> schema("/connectors/:id") -> #{ - operationId => '/connectors/:id', + 'operationId' => '/connectors/:id', get => #{ tags => [<<"connectors">>], description => <<"Get the connector by Id">>, @@ -179,7 +179,7 @@ schema("/connectors/:id") -> description => <<"Update an existing connector by Id">>, summary => <<"Update connector">>, parameters => param_path_id(), - requestBody => put_request_body_schema(), + 'requestBody' => put_request_body_schema(), responses => #{ 200 => get_response_body_schema(), 404 => error_schema(['NOT_FOUND'], "Connector not found") diff --git a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl index 438341978..e119189b8 100644 --- a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl +++ b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl @@ -91,12 +91,14 @@ init_per_suite(Config) -> ok = emqx_common_test_helpers:start_apps([emqx_rule_engine, emqx_connector, emqx_bridge, emqx_dashboard], fun set_special_configs/1), ok = emqx_common_test_helpers:load_config(emqx_connector_schema, <<"connectors: {}">>), - ok = emqx_common_test_helpers:load_config(emqx_rule_engine_schema, <<"rule_engine {rules {}}">>), + ok = emqx_common_test_helpers:load_config(emqx_rule_engine_schema, + <<"rule_engine {rules {}}">>), ok = emqx_common_test_helpers:load_config(emqx_bridge_schema, ?BRIDGE_CONF_DEFAULT), Config. end_per_suite(_Config) -> - emqx_common_test_helpers:stop_apps([emqx_rule_engine, emqx_connector, emqx_bridge, emqx_dashboard]), + emqx_common_test_helpers:stop_apps([emqx_rule_engine, emqx_connector, emqx_bridge, + emqx_dashboard]), ok. set_special_configs(emqx_dashboard) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_trace.erl b/apps/emqx_management/src/emqx_mgmt_api_trace.erl index 2890262de..a6e392a0f 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_trace.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_trace.erl @@ -343,7 +343,8 @@ group_trace_file(ZipDir, TraceLog, TraceFiles) -> _ -> Acc end; {error, Node, Reason} -> - ?SLOG(error, #{msg => "download_trace_log_error", node => Node, log => TraceLog, reason => Reason}), + ?SLOG(error, #{msg => "download_trace_log_error", node => Node, + log => TraceLog, reason => Reason}), Acc end end, [], TraceFiles). @@ -375,7 +376,8 @@ stream_log_file(get, #{bindings := #{name := Name}, query_string := Query}) -> {200, #{meta => Meta, items => <<"">>}}; {error, Reason} -> ?SLOG(error, #{msg => "read_file_failed", - node => Node, name => Name, reason => Reason, position => Position, bytes => Bytes}), + node => Node, name => Name, reason => Reason, + position => Position, bytes => Bytes}), {400, #{code => 'READ_FILE_ERROR', message => Reason}}; {badrpc, nodedown} -> {400, #{code => 'RPC_ERROR', message => "BadRpc node down"}} diff --git a/elvis.config b/elvis.config index 9387b8fc2..dff44c31d 100644 --- a/elvis.config +++ b/elvis.config @@ -9,6 +9,8 @@ filter => "*.erl", ruleset => erl_files, rules => [ + {elvis_style, macro_names, disable}, + {elvis_style, function_naming_convention, disable}, {elvis_style, state_record_and_type, disable}, {elvis_style, no_common_caveats_call, #{}}, {elvis_style, no_debug_call, #{ debug_functions => [ {ct, pal} @@ -19,15 +21,15 @@ {right, "||"}, {left, "||"}]}}, {elvis_style, dont_repeat_yourself, #{ min_complexity => 20 }}, - {elvis_style, god_modules, #{ignore => [emqx_authentication, - emqx_resource]}} + {elvis_style, god_modules, #{limit => 100}} ] }, #{dirs => ["test", "apps/**/test"], filter => "*.erl", rules => [ {elvis_text_style, line_length, #{ limit => 100 - , skip_comments => false }}, + , skip_comments => false + }}, {elvis_style, dont_repeat_yourself, #{ min_complexity => 100 }} ] },