From 8cca12d54a7df77238a62a4b1f3bcef1308dc11d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Tue, 7 Jun 2022 16:01:36 +0200 Subject: [PATCH] Do not display a warning when processing a message with repeated markers --- big_tests/tests/smart_markers_SUITE.erl | 27 ++++++++++++++++++- src/smart_markers/mod_smart_markers_rdbms.erl | 17 +++++++++--- .../mod_smart_markers_rdbms_async.erl | 8 +----- 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/big_tests/tests/smart_markers_SUITE.erl b/big_tests/tests/smart_markers_SUITE.erl index dca42f63a5..5bb44c9cc8 100644 --- 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 index 3d70f6501d..9200a88a31 100644 --- 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 index 42b1c6b35e..9c19f9bcf6 100644 --- 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).