From b59c4c34c5cc5ca240152237b48a555f5db4e7dd Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Wed, 4 Jan 2023 16:00:42 +0100 Subject: [PATCH] fix(Bridge REST API): no feedback when deleting bridge This fixes https://emqx.atlassian.net/browse/EMQX-8648. The issue described in `EMQX-8648` is that when deleting a non-existing bridge the server gives a success response. See below: ``` curl --head -u admin:public2 -X 'DELETE' 'http://localhost:18083/api/v5/bridges/webhook:i_do_not_exist' HTTP/1.1 204 No Content date: Tue, 03 Jan 2023 16:59:01 GMT server: Cowboy ``` After the fix, deleting a non existing bridge will give the following response: ``` HTTP/1.1 404 Not Found content-length: 49 content-type: application/json date: Thu, 05 Jan 2023 12:40:35 GMT server: Cowboy ``` Closes: EMQX-8648 --- apps/emqx_bridge/src/emqx_bridge_api.erl | 30 +++++++++++-------- .../test/emqx_bridge_api_SUITE.erl | 9 ++++++ changes/v5.0.14/fix-8648.en.md | 1 + 3 files changed, 28 insertions(+), 12 deletions(-) create mode 100644 changes/v5.0.14/fix-8648.en.md diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 0d0d2e9ad..cf39ebf14 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -328,6 +328,7 @@ schema("/bridges/:id") -> responses => #{ 204 => <<"Bridge deleted">>, 400 => error_schema(['INVALID_ID'], "Update bridge failed"), + 404 => error_schema('NOT_FOUND', "Bridge not found"), 403 => error_schema('FORBIDDEN_REQUEST', "Forbidden operation"), 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable") } @@ -433,19 +434,24 @@ schema("/nodes/:node/bridges/:id/operation/:operation") -> end, ?TRY_PARSE_ID( Id, - case emqx_bridge:check_deps_and_remove(BridgeType, BridgeName, AlsoDeleteActs) of + case emqx_bridge:lookup(BridgeType, BridgeName) of {ok, _} -> - 204; - {error, {rules_deps_on_this_bridge, RuleIds}} -> - {403, - error_msg( - 'FORBIDDEN_REQUEST', - {<<"There're some rules dependent on this bridge">>, RuleIds} - )}; - {error, timeout} -> - {503, error_msg('SERVICE_UNAVAILABLE', <<"request timeout">>)}; - {error, Reason} -> - {500, error_msg('INTERNAL_ERROR', Reason)} + case emqx_bridge:check_deps_and_remove(BridgeType, BridgeName, AlsoDeleteActs) of + {ok, _} -> + 204; + {error, {rules_deps_on_this_bridge, RuleIds}} -> + {403, + error_msg( + 'FORBIDDEN_REQUEST', + {<<"There're some rules dependent on this bridge">>, RuleIds} + )}; + {error, timeout} -> + {503, error_msg('SERVICE_UNAVAILABLE', <<"request timeout">>)}; + {error, Reason} -> + {500, error_msg('INTERNAL_ERROR', Reason)} + end; + {error, not_found} -> + {404, error_msg('NOT_FOUND', <<"Bridge not found">>)} end ). diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index 60f770df8..8f7978f40 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -303,6 +303,15 @@ t_http_crud_apis(Config) -> }, jsx:decode(ErrMsg2) ), + %% Deleting a non-existing bridge should result in an error + {ok, 404, ErrMsg3} = request(delete, uri(["bridges", BridgeID]), []), + ?assertMatch( + #{ + <<"code">> := _, + <<"message">> := <<"Bridge not found">> + }, + jsx:decode(ErrMsg3) + ), ok. t_check_dependent_actions_on_delete(Config) -> diff --git a/changes/v5.0.14/fix-8648.en.md b/changes/v5.0.14/fix-8648.en.md new file mode 100644 index 000000000..ac608a2e1 --- /dev/null +++ b/changes/v5.0.14/fix-8648.en.md @@ -0,0 +1 @@ +When deleting a non-existing bridge the server gave a success response. This has been fixed so that the server instead gives an error response when the user attempts to delete a non-existing bridge.