Merge pull request #11307 from paulozulato/fix-oracle-table-check

fix(oracle): discard nested tokens when checking table

apps/emqx_bridge_oracle/test/emqx_bridge_oracle_SUITE.erl

@@ -162,6 +162,9 @@ delete_all_bridges() ->
 sql_insert_template_for_bridge() ->
     "INSERT INTO mqtt_test(topic, msgid, payload, retain) VALUES (${topic}, ${id}, ${payload}, ${retain})".
 
+sql_insert_template_with_nested_token_for_bridge() ->
+    "INSERT INTO mqtt_test(topic, msgid, payload, retain) VALUES (${topic}, ${id}, ${payload.msg}, ${retain})".
+
 sql_create_table() ->
     "CREATE TABLE mqtt_test (topic VARCHAR2(255), msgid VARCHAR2(64), payload NCLOB, retain NUMBER(1))".
 
@@ -533,6 +536,23 @@ t_start_stop(Config) ->
     ),
     ok.
 
+t_probe_with_nested_tokens(Config) ->
+    ResourceId = resource_id(Config),
+    reset_table(Config),
+    ?assertMatch(
+        {ok, _},
+        create_bridge(Config, #{
+            <<"sql">> => sql_insert_template_with_nested_token_for_bridge()
+        })
+    ),
+    %% Since the connection process is async, we give it some time to
+    %% stabilize and avoid flakiness.
+    ?retry(
+        _Sleep = 1_000,
+        _Attempts = 20,
+        ?assertEqual({ok, connected}, emqx_resource_manager:health_check(ResourceId))
+    ).
+
 t_on_get_status(Config) ->
     ProxyPort = ?config(proxy_port, Config),
     ProxyHost = ?config(proxy_host, Config),

apps/emqx_oracle/src/emqx_oracle.app.src

@@ -1,6 +1,6 @@
 {application, emqx_oracle, [
     {description, "EMQX Enterprise Oracle Database Connector"},
-    {vsn, "0.1.3"},
+    {vsn, "0.1.4"},
     {registered, []},
     {applications, [
         kernel,

apps/emqx_oracle/src/emqx_oracle.erl

@@ -9,6 +9,10 @@
 -include_lib("emqx/include/logger.hrl").
 -include_lib("snabbkaffe/include/snabbkaffe.hrl").
 
+-define(UNHEALTHY_TARGET_MSG,
+    "Oracle table is invalid. Please check if the table exists in Oracle Database."
+).
+
 %%====================================================================
 %% Exports
 %%====================================================================
@@ -239,7 +243,7 @@ on_get_status(_InstId, #{pool_name := Pool} = State) ->
                     {connected, NState};
                 {error, {undefined_table, NState}} ->
                     %% return new state indicating that we are connected but the target table is not created
-                    {disconnected, NState, unhealthy_target};
+                    {disconnected, NState, {unhealthy_target, ?UNHEALTHY_TARGET_MSG}};
                 {error, _Reason} ->
                     %% do not log error, it is logged in prepare_sql_to_conn
                     connecting
@@ -408,7 +412,19 @@ prepare_sql_to_conn(Conn, [{Key, SQL} | PrepareList], TokensMap, Statements) whe
             Error
     end.
 
-check_if_table_exists(Conn, SQL, Tokens) ->
+check_if_table_exists(Conn, SQL, Tokens0) ->
+    % Discard nested tokens for checking if table exist. As payload here is defined as
+    % a single string, it would fail if Token is, for instance, ${payload.msg}, causing
+    % bridge probe to fail.
+    Tokens = lists:map(
+        fun
+            ({var, [Token | _DiscardedDeepTokens]}) ->
+                {var, [Token]};
+            (Token) ->
+                Token
+        end,
+        Tokens0
+    ),
     {Event, _Headers} = emqx_rule_events:eventmsg_publish(
         emqx_message:make(<<"t/opic">>, "test query")
     ),

changes/ee/fix-11307.en.md

@@ -0,0 +1 @@
+Fixed check for table existence to return a more friendly message in the Oracle bridge.