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] 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,