Merge pull request #13817 from zmstone/0917-refactor-new-api-to-check-resource-exist

refactor: add is_exist API for resource existence check

apps/emqx_bridge/src/emqx_bridge.erl

@@ -40,6 +40,7 @@
     unload/0,
     lookup/1,
     lookup/2,
+    is_exist_v1/2,
     get_metrics/2,
     create/3,
     disable_enable/3,
@@ -327,6 +328,9 @@ lookup(Id) ->
     {Type, Name} = emqx_bridge_resource:parse_bridge_id(Id),
     lookup(Type, Name).
 
+is_exist_v1(Type, Name) ->
+    emqx_resource:is_exist(emqx_bridge_resource:resource_id(Type, Name)).
+
 lookup(Type, Name) ->
     case emqx_bridge_v2:is_bridge_v2_type(Type) of
         true ->

apps/emqx_bridge/src/emqx_bridge_v2.erl

@@ -44,6 +44,8 @@
     list/1,
     lookup/2,
     lookup/3,
+    lookup_raw_conf/3,
+    is_exist/3,
     create/3,
     create/4,
     %% The remove/2 function is only for internal use as it may create
@@ -56,7 +58,7 @@
     check_deps_and_remove/3,
     check_deps_and_remove/4
 ]).
--export([lookup_action/2, lookup_source/2]).
+-export([is_action_exist/2, is_source_exist/2]).
 
 %% Operations
 
@@ -234,11 +236,22 @@ unload_bridges(ConfRooKey) ->
 lookup(Type, Name) ->
     lookup(?ROOT_KEY_ACTIONS, Type, Name).
 
-lookup_action(Type, Name) ->
-    lookup(?ROOT_KEY_ACTIONS, Type, Name).
+is_action_exist(Type, Name) ->
+    is_exist(?ROOT_KEY_ACTIONS, Type, Name).
+
+is_source_exist(Type, Name) ->
+    is_exist(?ROOT_KEY_SOURCES, Type, Name).
 
-lookup_source(Type, Name) ->
-    lookup(?ROOT_KEY_SOURCES, Type, Name).
+is_exist(ConfRootName, Type, Name) ->
+    {error, not_found} =/= lookup_raw_conf(ConfRootName, Type, Name).
+
+lookup_raw_conf(ConfRootName, Type, Name) ->
+    case emqx:get_raw_config([ConfRootName, Type, Name], not_found) of
+        not_found ->
+            {error, not_found};
+        #{<<"connector">> := _} = RawConf ->
+            {ok, RawConf}
+    end.
 
 -spec lookup(root_cfg_key(), bridge_v2_type(), bridge_v2_name()) ->
     {ok, bridge_v2_info()} | {error, not_found}.

apps/emqx_bridge/src/emqx_bridge_v2_api.erl

@@ -796,10 +796,10 @@ handle_list(ConfRootKey) ->
     end.
 
 handle_create(ConfRootKey, Type, Name, Conf0) ->
-    case emqx_bridge_v2:lookup(ConfRootKey, Type, Name) of
-        {ok, _} ->
+    case emqx_bridge_v2:is_exist(ConfRootKey, Type, Name) of
+        true ->
             ?BAD_REQUEST('ALREADY_EXISTS', <<"bridge already exists">>);
-        {error, not_found} ->
+        false ->
             Conf = filter_out_request_body(Conf0),
             create_bridge(ConfRootKey, Type, Name, Conf)
     end.
@@ -808,12 +808,12 @@ handle_update(ConfRootKey, Id, Conf0) ->
     Conf1 = filter_out_request_body(Conf0),
     ?TRY_PARSE_ID(
         Id,
-        case emqx_bridge_v2:lookup(ConfRootKey, BridgeType, BridgeName) of
-            {ok, _} ->
+        case emqx_bridge_v2:is_exist(ConfRootKey, BridgeType, BridgeName) of
+            true ->
                 RawConf = emqx:get_raw_config([ConfRootKey, BridgeType, BridgeName], #{}),
                 Conf = emqx_utils:deobfuscate(Conf1, RawConf),
                 update_bridge(ConfRootKey, BridgeType, BridgeName, Conf);
-            {error, not_found} ->
+            false ->
                 ?BRIDGE_NOT_FOUND(BridgeType, BridgeName)
         end
     ).
@@ -821,8 +821,8 @@ handle_update(ConfRootKey, Id, Conf0) ->
 handle_delete(ConfRootKey, Id, QueryStringOpts) ->
     ?TRY_PARSE_ID(
         Id,
-        case emqx_bridge_v2:lookup(ConfRootKey, BridgeType, BridgeName) of
-            {ok, _} ->
+        case emqx_bridge_v2:is_exist(ConfRootKey, BridgeType, BridgeName) of
+            true ->
                 AlsoDeleteActions =
                     case maps:get(<<"also_delete_dep_actions">>, QueryStringOpts, <<"false">>) of
                         <<"true">> -> true;
@@ -851,7 +851,7 @@ handle_delete(ConfRootKey, Id, QueryStringOpts) ->
                     {error, Reason} ->
                         ?INTERNAL_ERROR(Reason)
                 end;
-            {error, not_found} ->
+            false ->
                 ?BRIDGE_NOT_FOUND(BridgeType, BridgeName)
         end
     ).
@@ -920,7 +920,7 @@ handle_probe(ConfRootKey, Request) ->
     RequestMeta = #{module => ?MODULE, method => post, path => Path},
     case emqx_dashboard_swagger:filter_check_request_and_translate_body(Request, RequestMeta) of
         {ok, #{body := #{<<"type">> := Type} = Params}} ->
-            Params1 = maybe_deobfuscate_bridge_probe(Params),
+            Params1 = maybe_deobfuscate_bridge_probe(ConfRootKey, Params),
             Params2 = maps:remove(<<"type">>, Params1),
             case emqx_bridge_v2:create_dry_run(ConfRootKey, Type, Params2) of
                 ok ->
@@ -942,9 +942,11 @@ handle_probe(ConfRootKey, Request) ->
     end.
 
 %%% API helpers
-maybe_deobfuscate_bridge_probe(#{<<"type">> := ActionType, <<"name">> := BridgeName} = Params) ->
-    case emqx_bridge_v2:lookup(ActionType, BridgeName) of
-        {ok, #{raw_config := RawConf}} ->
+maybe_deobfuscate_bridge_probe(
+    ConfRootKey, #{<<"type">> := ActionType, <<"name">> := BridgeName} = Params
+) ->
+    case emqx_bridge_v2:lookup_raw_conf(ConfRootKey, ActionType, BridgeName) of
+        {ok, RawConf} ->
             %% TODO check if RawConf obtained above is compatible with the commented out code below
             %% RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}),
             emqx_utils:deobfuscate(Params, RawConf);
@@ -952,7 +954,7 @@ maybe_deobfuscate_bridge_probe(#{<<"type">> := ActionType, <<"name">> := BridgeN
             %% A bridge may be probed before it's created, so not finding it here is fine
             Params
     end;
-maybe_deobfuscate_bridge_probe(Params) ->
+maybe_deobfuscate_bridge_probe(_ConfRootKey, Params) ->
     Params.
 
 is_ok(ok) ->

apps/emqx_connector/src/emqx_connector.erl

@@ -32,6 +32,7 @@
     get_metrics/2,
     list/0,
     load/0,
+    is_exist/2,
     lookup/1,
     lookup/2,
     remove/2,
@@ -235,6 +236,9 @@ lookup(Type, Name, RawConf) ->
             }}
     end.
 
+is_exist(Type, Name) ->
+    emqx_resource:is_exist(emqx_connector_resource:resource_id(Type, Name)).
+
 get_metrics(Type, Name) ->
     emqx_resource:get_metrics(emqx_connector_resource:resource_id(Type, Name)).
 

apps/emqx_connector/src/emqx_connector_api.erl

@@ -318,10 +318,10 @@ schema("/connectors_probe") ->
     }.
 
 '/connectors'(post, #{body := #{<<"type">> := ConnectorType, <<"name">> := ConnectorName} = Conf0}) ->
-    case emqx_connector:lookup(ConnectorType, ConnectorName) of
-        {ok, _} ->
+    case emqx_connector:is_exist(ConnectorType, ConnectorName) of
+        true ->
             ?BAD_REQUEST('ALREADY_EXISTS', <<"connector already exists">>);
-        {error, not_found} ->
+        false ->
             Conf = filter_out_request_body(Conf0),
             create_connector(ConnectorType, ConnectorName, Conf)
     end;
@@ -345,20 +345,20 @@ schema("/connectors_probe") ->
     Conf1 = filter_out_request_body(Conf0),
     ?TRY_PARSE_ID(
         Id,
-        case emqx_connector:lookup(ConnectorType, ConnectorName) of
-            {ok, _} ->
+        case emqx_connector:is_exist(ConnectorType, ConnectorName) of
+            true ->
                 RawConf = emqx:get_raw_config([connectors, ConnectorType, ConnectorName], #{}),
                 Conf = emqx_utils:deobfuscate(Conf1, RawConf),
                 update_connector(ConnectorType, ConnectorName, Conf);
-            {error, not_found} ->
+            false ->
                 ?CONNECTOR_NOT_FOUND(ConnectorType, ConnectorName)
         end
     );
 '/connectors/:id'(delete, #{bindings := #{id := Id}}) ->
     ?TRY_PARSE_ID(
         Id,
-        case emqx_connector:lookup(ConnectorType, ConnectorName) of
-            {ok, _} ->
+        case emqx_connector:is_exist(ConnectorType, ConnectorName) of
+            true ->
                 case emqx_connector:remove(ConnectorType, ConnectorName) of
                     ok ->
                         ?NO_CONTENT;
@@ -372,7 +372,7 @@ schema("/connectors_probe") ->
                     {error, Reason} ->
                         ?INTERNAL_ERROR(Reason)
                 end;
-            {error, not_found} ->
+            false ->
                 ?CONNECTOR_NOT_FOUND(ConnectorType, ConnectorName)
         end
     ).
@@ -406,11 +406,11 @@ schema("/connectors_probe") ->
 maybe_deobfuscate_connector_probe(
     #{<<"type">> := ConnectorType, <<"name">> := ConnectorName} = Params
 ) ->
-    case emqx_connector:lookup(ConnectorType, ConnectorName) of
-        {ok, _} ->
+    case emqx_connector:is_exist(ConnectorType, ConnectorName) of
+        true ->
             RawConf = emqx:get_raw_config([connectors, ConnectorType, ConnectorName], #{}),
             emqx_utils:deobfuscate(Params, RawConf);
-        _ ->
+        false ->
             %% A connector may be probed before it's created, so not finding it here is fine
             Params
     end;

apps/emqx_resource/src/emqx_resource.erl

@@ -123,6 +123,7 @@
     list_instances_verbose/0,
     %% return the data of the instance
     get_instance/1,
+    is_exist/1,
     get_metrics/1,
     fetch_creation_opts/1,
     %% return all the instances of the same resource type
@@ -457,6 +458,10 @@ set_resource_status_connecting(ResId) ->
 get_instance(ResId) ->
     emqx_resource_manager:lookup_cached(ResId).
 
+-spec is_exist(resource_id()) -> boolean().
+is_exist(ResId) ->
+    emqx_resource_manager:is_exist(ResId).
+
 -spec get_metrics(resource_id()) ->
     emqx_metrics_worker:metrics().
 get_metrics(ResId) ->

apps/emqx_resource/src/emqx_resource_manager.erl

@@ -47,6 +47,7 @@
     list_all/0,
     list_group/1,
     lookup_cached/1,
+    is_exist/1,
     get_metrics/1,
     reset_metrics/1,
     channel_status_is_channel_added/1,
@@ -387,6 +388,10 @@ lookup_cached(ResId) ->
             {error, not_found}
     end.
 
+%% @doc Check if the resource is cached.
+is_exist(ResId) ->
+    {error, not_found} =/= lookup_cached(ResId).
+
 %% @doc Get the metrics for the specified resource
 get_metrics(ResId) ->
     emqx_metrics_worker:get_metrics(?RES_METRICS, ResId).
@@ -1246,8 +1251,7 @@ continue_resource_health_check_not_connected(NewStatus, Data0) ->
 
 handle_manual_channel_health_check(From, #data{state = undefined}, _ChannelId) ->
     {keep_state_and_data, [
-        {reply, From,
-            maps:remove(config, channel_status({error, resource_disconnected}, undefined))}
+        {reply, From, channel_error_status(resource_disconnected)}
     ]};
 handle_manual_channel_health_check(
     From,
@@ -1281,14 +1285,14 @@ handle_manual_channel_health_check(
     is_map_key(ChannelId, Channels)
 ->
     %% No ongoing health check: reply with current status.
-    {keep_state_and_data, [{reply, From, maps:remove(config, maps:get(ChannelId, Channels))}]};
+    {keep_state_and_data, [{reply, From, without_channel_config(maps:get(ChannelId, Channels))}]};
 handle_manual_channel_health_check(
     From,
     _Data,
     _ChannelId
 ) ->
     {keep_state_and_data, [
-        {reply, From, maps:remove(config, channel_status({error, channel_not_found}, undefined))}
+        {reply, From, channel_error_status(channel_not_found)}
     ]}.
 
 -spec channels_health_check(resource_status(), data()) -> data().
@@ -1595,7 +1599,7 @@ handle_channel_health_check_worker_down_new_channels_and_status(
 reply_pending_channel_health_check_callers(
     ChannelId, Status0, Data0 = #data{hc_pending_callers = Pending0}
 ) ->
-    Status = maps:remove(config, Status0),
+    Status = without_channel_config(Status0),
     #{channel := CPending0} = Pending0,
     Pending = maps:get(ChannelId, CPending0, []),
     Actions = [{reply, From, Status} || From <- Pending],
@@ -1678,7 +1682,7 @@ maybe_alarm(_Status, false, ResId, Error, _PrevError) ->
             {error, Reason} ->
                 emqx_utils:readable_error_msg(Reason);
             _ ->
-                Error1 = redact_config_from_error_status(Error),
+                Error1 = without_channel_config(Error),
                 emqx_utils:readable_error_msg(Error1)
         end,
     emqx_alarm:safe_activate(
@@ -1688,10 +1692,8 @@ maybe_alarm(_Status, false, ResId, Error, _PrevError) ->
     ),
     ?tp(resource_activate_alarm, #{resource_id => ResId}).
 
-redact_config_from_error_status(#{config := _} = ErrorStatus) ->
-    maps:remove(config, ErrorStatus);
-redact_config_from_error_status(Error) ->
-    Error.
+without_channel_config(Map) ->
+    maps:without([config], Map).
 
 -spec maybe_resume_resource_workers(resource_id(), resource_status()) -> ok.
 maybe_resume_resource_workers(ResId, ?status_connected) ->
@@ -1744,7 +1746,7 @@ maybe_reply(Actions, From, Reply) ->
 data_record_to_external_map(Data) ->
     AddedChannelsWithoutConfigs =
         maps:map(
-            fun(_ChanID, Status) -> maps:remove(config, Status) end,
+            fun(_ChanID, Status) -> without_channel_config(Status) end,
             Data#data.added_channels
         ),
     #{
@@ -1851,10 +1853,13 @@ channel_status({?status_connected, Error}, ChannelConfig) ->
         config => ChannelConfig
     };
 channel_status({error, Reason}, ChannelConfig) ->
+    S = channel_error_status(Reason),
+    S#{config => ChannelConfig}.
+
+channel_error_status(Reason) ->
     #{
         status => ?status_disconnected,
-        error => Reason,
-        config => ChannelConfig
+        error => Reason
     }.
 
 channel_status_is_channel_added(#{

apps/emqx_rule_engine/src/emqx_rule_engine.erl

@@ -667,17 +667,14 @@ validate_bridge_existence_in_actions(#{actions := Actions, from := Froms} = _Rul
     NonExistentBridgeIDs =
         lists:filter(
             fun({Kind, Type, Name}) ->
-                LookupFn =
+                IsExist =
                     case Kind of
-                        action -> fun emqx_bridge_v2:lookup_action/2;
-                        source -> fun emqx_bridge_v2:lookup_source/2;
-                        bridge_v1 -> fun emqx_bridge:lookup/2
+                        action -> fun emqx_bridge_v2:is_action_exist/2;
+                        source -> fun emqx_bridge_v2:is_source_exist/2;
+                        bridge_v1 -> fun emqx_bridge:is_exist_v1/2
                     end,
                 try
-                    case LookupFn(Type, Name) of
-                        {ok, _} -> false;
-                        {error, _} -> true
-                    end
+                    not IsExist(Type, Name)
                 catch
                     _:_ -> true
                 end