Skip to content

Commit

Permalink
Merge pull request #9155 from zmstone/1014-publish-api-should-not-echo
Browse files Browse the repository at this point in the history
fix(api): publish API only returns message ID
  • Loading branch information
zmstone committed Oct 14, 2022
2 parents 6f077c4 + f2db35d commit 1686ca2
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 27 deletions.
3 changes: 3 additions & 0 deletions CHANGES-5.0.md
Expand Up @@ -5,6 +5,9 @@
* Add `cert_common_name` and `cert_subject` placeholder support for authz_http and authz_mongo.[#8973](https://github.com/emqx/emqx/pull/8973)
* Use milliseconds internally in emqx_delayed to store the publish time, improving precision.[#9060](https://github.com/emqx/emqx/pull/9060)
* More rigorous checking of flapping to improve stability of the system. [#9136](https://github.com/emqx/emqx/pull/9136)
* No message(s) echo for the message publish APIs [#9155](https://github.com/emqx/emqx/pull/9155)
Prior to this fix, the message publish APIs (`api/v5/publish` and `api/v5/publish/bulk`) echos the message back to the client in HTTP body.
This change fixed it to only send back the message ID.

## Bug fixes

Expand Down
31 changes: 10 additions & 21 deletions apps/emqx_management/src/emqx_mgmt_api_publish.erl
Expand Up @@ -109,15 +109,15 @@ fields(publish_message_info) ->
[
{id,
hoconsc:mk(binary(), #{
desc => <<"Internal Message ID">>
desc => <<"A globally unique message ID">>
})}
] ++ fields(message).
].

publish(post, #{body := Body}) ->
case message(Body) of
{ok, Message} ->
_ = emqx_mgmt:publish(Message),
{200, format_message(Message)};
{200, format_message_response(Message)};
{error, R} ->
{400, 'BAD_REQUEST', to_binary(R)}
end.
Expand All @@ -126,7 +126,7 @@ publish_batch(post, #{body := Body}) ->
case messages(Body) of
{ok, Messages} ->
_ = [emqx_mgmt:publish(Message) || Message <- Messages],
{200, format_message(Messages)};
{200, format_message_response(Messages)};
{error, R} ->
{400, 'BAD_REQUEST', to_binary(R)}
end.
Expand Down Expand Up @@ -167,21 +167,10 @@ messages([MessageMap | List], Res) ->
{error, R}
end.

format_message(Messages) when is_list(Messages) ->
[format_message(Message) || Message <- Messages];
format_message(#message{
id = ID, qos = Qos, from = From, topic = Topic, payload = Payload, flags = Flags
}) ->
#{
id => emqx_guid:to_hexstr(ID),
qos => Qos,
topic => Topic,
payload => Payload,
retain => maps:get(retain, Flags, false),
clientid => to_binary(From)
}.
format_message_response(Messages) when is_list(Messages) ->
[format_message_response(Message) || Message <- Messages];
format_message_response(#message{id = ID}) ->
#{id => emqx_guid:to_hexstr(ID)}.

to_binary(Data) when is_binary(Data) ->
Data;
to_binary(Data) ->
list_to_binary(io_lib:format("~p", [Data])).
to_binary(Term) ->
list_to_binary(io_lib:format("~p", [Term])).
20 changes: 14 additions & 6 deletions apps/emqx_management/test/emqx_mgmt_api_publish_SUITE.erl
Expand Up @@ -47,8 +47,10 @@ t_publish_api(_) ->
Path = emqx_mgmt_api_test_util:api_path(["publish"]),
Auth = emqx_mgmt_api_test_util:auth_header_(),
Body = #{topic => ?TOPIC1, payload => Payload},
{ok, _} = emqx_mgmt_api_test_util:request_api(post, Path, "", Auth, Body),
?assertEqual(receive_assert(?TOPIC1, 0, Payload), ok),
{ok, Response} = emqx_mgmt_api_test_util:request_api(post, Path, "", Auth, Body),
ResponseMap = emqx_json:decode(Response, [return_maps]),
?assertEqual([<<"id">>], maps:keys(ResponseMap)),
?assertEqual(ok, receive_assert(?TOPIC1, 0, Payload)),
emqtt:disconnect(Client).

t_publish_bulk_api(_) ->
Expand All @@ -63,10 +65,16 @@ t_publish_bulk_api(_) ->
Auth = emqx_mgmt_api_test_util:auth_header_(),
Body = [#{topic => ?TOPIC1, payload => Payload}, #{topic => ?TOPIC2, payload => Payload}],
{ok, Response} = emqx_mgmt_api_test_util:request_api(post, Path, "", Auth, Body),
ResponseMap = emqx_json:decode(Response, [return_maps]),
?assertEqual(2, erlang:length(ResponseMap)),
?assertEqual(receive_assert(?TOPIC1, 0, Payload), ok),
?assertEqual(receive_assert(?TOPIC2, 0, Payload), ok),
ResponseList = emqx_json:decode(Response, [return_maps]),
?assertEqual(2, erlang:length(ResponseList)),
lists:foreach(
fun(ResponseMap) ->
?assertEqual([<<"id">>], maps:keys(ResponseMap))
end,
ResponseList
),
?assertEqual(ok, receive_assert(?TOPIC1, 0, Payload)),
?assertEqual(ok, receive_assert(?TOPIC2, 0, Payload)),
emqtt:disconnect(Client).

receive_assert(Topic, Qos, Payload) ->
Expand Down

0 comments on commit 1686ca2

Please sign in to comment.