Do not display a warning when processing a message with repeated markers

esl · Jun 7, 2022 · 8cca12d · 8cca12d
1 parent a1399e3
commit 8cca12d
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 11 deletions.
diff --git a/big_tests/tests/smart_markers_SUITE.erl b/big_tests/tests/smart_markers_SUITE.erl
@@ -40,7 +40,8 @@ groups() ->
        marker_for_thread_can_be_fetched,
        marker_after_timestamp_can_be_fetched,
        marker_after_timestamp_for_threadid_can_be_fetched,
-       remove_markers_when_removed_user
+       remove_markers_when_removed_user,
+       repeated_markers_produce_no_warnings
       ]},
      {muclight, [parallel],
       [
@@ -106,8 +107,15 @@ end_per_group(_, Config) ->
     dynamic_modules:restore_modules(Config),
     Config.
 
+init_per_testcase(repeated_markers_produce_no_warnings = TC, Config) ->
+    logger_ct_backend:start(),
+    escalus:init_per_testcase(TC, Config);
 init_per_testcase(Name, Config) ->
     escalus:init_per_testcase(Name, Config).
+
+end_per_testcase(repeated_markers_produce_no_warnings = TC, Config) ->
+    logger_ct_backend:stop(),
+    escalus:end_per_testcase(TC, Config);
 end_per_testcase(Name, Config) ->
     escalus:end_per_testcase(Name, Config).
 
@@ -238,6 +246,23 @@ remove_markers_when_removed_user(Config) ->
         mongoose_helper:wait_until(fun() -> length(fetch_markers_for_users(BobJid, AliceJid)) end, 0)
     end).
 
+repeated_markers_produce_no_warnings(Config) ->
+    escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) ->
+        MsgId = escalus_stanza:id(),
+        Msg = escalus_stanza:set_id(escalus_stanza:chat_to(Bob, <<"Hello!">>), MsgId),
+        escalus:send(Alice, Msg),
+        escalus:wait_for_stanza(Bob),
+        ChatMarker = escalus_stanza:chat_marker(Alice, <<"displayed">>, MsgId),
+        [MarkerEl] = ChatMarker#xmlel.children,
+        RepeatedChatMarker = ChatMarker#xmlel{children =  [MarkerEl, MarkerEl]},
+        logger_ct_backend:capture(warning),
+        escalus:send(Bob, RepeatedChatMarker),
+        escalus:wait_for_stanza(Alice),
+        logger_ct_backend:stop_capture(),
+        FilterFun = fun(_, WMsg) -> re:run(WMsg, "{bad_result,{updated,0}}") /= nomatch end,
+        [] = logger_ct_backend:recv(FilterFun)
+    end).
+
 marker_is_stored_for_room(Config) ->
     escalus:fresh_story(Config, [{alice, 1}, {bob, 1}, {kate, 1}],
                         fun(Alice, Bob, Kate) ->

diff --git a/src/smart_markers/mod_smart_markers_rdbms.erl b/src/smart_markers/mod_smart_markers_rdbms.erl
@@ -9,11 +9,12 @@
 -behavior(mod_smart_markers_backend).
 
 -include("jlib.hrl").
+-include("mongoose_logger.hrl").
 
 -export([init/2, update_chat_marker/2, get_chat_markers/4]).
 -export([get_conv_chat_marker/6]).
 -export([remove_domain/2, remove_user/2, remove_to/2, remove_to_for_user/3]).
--export([encode_jid/1, encode_thread/1, encode_type/1, check_upsert_result/1]).
+-export([encode_jid/1, encode_thread/1, encode_type/1, verify/2]).
 
 %%--------------------------------------------------------------------
 %% API
@@ -54,7 +55,7 @@ init(HostType, _) ->
                          mod_smart_markers:chat_marker()) -> ok.
 update_chat_marker(HostType, #{from := #jid{luser = LU, lserver = LS},
                                to := To, thread := Thread,
-                               type := Type, timestamp := TS, id := Id}) ->
+                               type := Type, timestamp := TS, id := Id} = Marker) ->
     ToEncoded = encode_jid(To),
     ThreadEncoded = encode_thread(Thread),
     TypeEncoded = encode_type(Type),
@@ -63,7 +64,7 @@ update_chat_marker(HostType, #{from := #jid{luser = LU, lserver = LS},
     InsertValues = KeyValues ++ UpdateValues,
     Res = rdbms_queries:execute_upsert(HostType, smart_markers_upsert,
                                        InsertValues, UpdateValues, KeyValues),
-    ok = check_upsert_result(Res).
+    verify(Res, Marker).
 
 -spec get_conv_chat_marker(HostType :: mongooseim:host_type(),
                            From :: jid:jid(),
@@ -143,6 +144,15 @@ remove_to(HostType, To) ->
 remove_to_for_user(HostType, #jid{luser = LU, lserver = LS}, To) ->
     mongoose_rdbms:execute_successfully(HostType, markers_remove_to_for_user, [LS, LU, encode_jid(To)]).
 
+-spec verify(term(), mod_smart_markers:chat_marker()) -> ok.
+verify(Answer, Marker) ->
+    case check_upsert_result(Answer) of
+        {error, Reason} ->
+            ?LOG_WARNING(#{what => smart_marker_insert_failed, reason => Reason,
+                           marker => Marker});
+        _ -> ok
+    end.
+
 %%--------------------------------------------------------------------
 %% local functions
 %%--------------------------------------------------------------------
@@ -157,6 +167,7 @@ encode_type(acknowledged) -> <<"A">>.
 
 %% MySQL returns 1 when an upsert is an insert
 %% and 2, when an upsert acts as update
+check_upsert_result({updated, 0}) -> ok;
 check_upsert_result({updated, 1}) -> ok;
 check_upsert_result({updated, 2}) -> ok;
 check_upsert_result(Result) ->

diff --git a/src/smart_markers/mod_smart_markers_rdbms_async.erl b/src/smart_markers/mod_smart_markers_rdbms_async.erl
@@ -7,7 +7,6 @@
 -behaviour(mongoose_aggregator_worker).
 
 -include("jlib.hrl").
--include("mongoose_logger.hrl").
 
 -export([init/2, stop/1, update_chat_marker/2, get_chat_markers/4, get_conv_chat_marker/6]).
 -export([remove_domain/2, remove_user/2, remove_to/2, remove_to_for_user/3]).
@@ -107,9 +106,4 @@ request(#{from := #jid{luser = LU, lserver = LS}, to := To, thread := Thread,
 
 -spec verify(term(), mod_smart_markers:chat_marker(), mongoose_async_pools:pool_extra()) -> ok.
 verify(Answer, Marker, _Extra) ->
-    case mod_smart_markers_rdbms:check_upsert_result(Answer) of
-        {error, Reason} ->
-            ?LOG_WARNING(#{what => smart_marker_insert_failed, reason => Reason,
-                           marker => Marker});
-        _ -> ok
-    end.
+    mod_smart_markers_rdbms:verify(Answer, Marker).