From d8cafae6dba4f638b0647af63f4c1474ffce1240 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 13 Apr 2022 14:59:34 +0200 Subject: [PATCH 1/2] Fix minor indentation and redundant code or comments --- big_tests/tests/inbox_SUITE.erl | 7 +++---- src/inbox/mod_inbox_rdbms_async.erl | 12 ++---------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/big_tests/tests/inbox_SUITE.erl b/big_tests/tests/inbox_SUITE.erl index 106001c9d4d..2281fb959be 100644 --- a/big_tests/tests/inbox_SUITE.erl +++ b/big_tests/tests/inbox_SUITE.erl @@ -569,14 +569,13 @@ check_total_unread_count_and_active_conv_count(Config) -> check_total_unread_count_when_there_are_no_active_conversations(Config) -> escalus:fresh_story(Config, [{kate, 1}, {mike, 1}], fun(Kate, Mike) -> inbox_helper:send_and_mark_msg(Kate, Mike), - inbox_helper:get_inbox(Mike, #{count => 1, unread_messages => 0, active_conversations => 0}) - end). + end). total_unread_count_and_active_convs_are_zero_at_no_activity(Config) -> escalus:fresh_story(Config, [{kate, 1}], fun(Kate) -> inbox_helper:get_inbox(Kate, #{count => 0, unread_messages => 0, active_conversations => 0}) - end). + end). try_to_reset_unread_counter_with_bad_marker(Config) -> escalus:fresh_story(Config, [{kate, 1}, {mike, 1}], fun(Kate, Mike) -> @@ -831,7 +830,7 @@ leave_and_remove_conversation(Config) -> check_inbox(Bob, [], #{box => inbox}), if_async_check_bin(Config, Bob, [#conv{unread = 2, from = RoomJid, to = BobJid, verify = fun inbox_helper:verify_is_none_aff_change/2}]) - end). + end). %% this test combines options: %% ... diff --git a/src/inbox/mod_inbox_rdbms_async.erl b/src/inbox/mod_inbox_rdbms_async.erl index f96c240b106..2c3851fab79 100644 --- a/src/inbox/mod_inbox_rdbms_async.erl +++ b/src/inbox/mod_inbox_rdbms_async.erl @@ -174,23 +174,15 @@ empty_global_bin(HostType, TS) -> mod_inbox_rdbms:empty_global_bin(HostType, TS). -spec aggregate(CurrentlyAccumulatedTask :: task(), NewTask :: task()) -> FinalTask :: task(). -%%% First task being processed, just take that one -aggregate(undefined, Task) -> - Task; - %%% if new task is remove_row, do the previous with an updated box - % {reset_unread, mod_inbox:entry_key(), id(), integer()}. -aggregate({reset_unread, _, _, _}, - {remove_inbox_row, _} = OldTask) -> - OldTask; aggregate({set_inbox, Entry, Content, Count, MsgId, Timestamp, _}, {remove_inbox_row, _}) -> {set_inbox, Entry, Content, Count, MsgId, Timestamp, <<"bin">>}; aggregate({set_inbox_incr_unread, Entry, Content, MsgId, Timestamp, Incrs, _}, {remove_inbox_row, _}) -> {set_inbox_incr_unread, Entry, Content, MsgId, Timestamp, Incrs, <<"bin">>}; -aggregate(_, {remove_inbox_row, _} = OldTask) -> - OldTask; +aggregate(_, {remove_inbox_row, _} = NewTask) -> + NewTask; %%% if the last task was remove_row, this task should now only be an insert aggregate({remove_inbox_row, _} = OldTask, {reset_unread, _, _, _}) -> From e81a92f44622ac51b3b5eced36838f686970e3fd Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 13 Apr 2022 15:02:59 +0200 Subject: [PATCH 2/2] Fix unarchive-or-bin sql command in inbox The command as it was currently implemented would not move a conversation to the bin when aggregating it onto the set_inbox commands, as the sql query was so that it would only unarchive entries on request, but the entry was not on the archive, so the box was not moved. This happens because we enforce automatic moves from the archive to the inbox, and no other box is to be moved automatically. --- big_tests/tests/inbox_extensions_SUITE.erl | 18 ++++++++++++++++++ src/inbox/mod_inbox_rdbms.erl | 7 +++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/big_tests/tests/inbox_extensions_SUITE.erl b/big_tests/tests/inbox_extensions_SUITE.erl index fb55b005d76..58a2fefb189 100644 --- a/big_tests/tests/inbox_extensions_SUITE.erl +++ b/big_tests/tests/inbox_extensions_SUITE.erl @@ -70,6 +70,7 @@ groups() -> % box box_move_to_other_works_successfully, box_active_entry_gets_archived, + box_other_entry_does_not_get_unarchived, box_archived_entry_gets_active_on_request, box_archived_entry_gets_active_for_the_sender_on_new_message, box_archived_entry_gets_active_for_the_receiver_on_new_message, @@ -312,6 +313,23 @@ box_active_entry_gets_archived(Config) -> #{box => archive}) end). +box_other_entry_does_not_get_unarchived(Config) -> + escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) -> + % Alice sends a message to Bob and then she moves the conversation to the 'other' box + Body1 = <<"Hi Bob">>, + inbox_helper:send_msg(Alice, Bob, Body1), + inbox_helper:check_inbox(Alice, [#conv{unread = 0, from = Alice, to = Bob, content = Body1}]), + set_inbox_properties(Alice, Bob, [{box, other}]), + % But then Alice keeps writing + Body2 = <<"Hi Bob again">>, + inbox_helper:send_msg(Alice, Bob, Body2), + inbox_helper:get_inbox(Alice, #{box => all, hidden_read => false}, #{count => 1}), + % Then the conversation is still in the same box + inbox_helper:check_inbox(Alice, [], #{box => inbox}), + inbox_helper:check_inbox(Alice, [#conv{unread = 0, from = Alice, to = Bob, content = Body2}], + #{box => other}) + end). + box_archived_entry_gets_active_on_request(Config) -> escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) -> % Alice sends a message to Bob and Bob archives it immediately diff --git a/src/inbox/mod_inbox_rdbms.erl b/src/inbox/mod_inbox_rdbms.erl index 6e7ea11a747..967b8de83a2 100644 --- a/src/inbox/mod_inbox_rdbms.erl +++ b/src/inbox/mod_inbox_rdbms.erl @@ -67,12 +67,15 @@ init(HostType, _Options) -> mongoose_rdbms:prepare(inbox_delete_domain, inbox, [lserver], <<"DELETE FROM inbox WHERE lserver = ?">>), % upserts + BoxQuery = <<"CASE WHEN ?='bin' THEN 'bin'", + " WHEN inbox.box='archive' THEN 'inbox'", + " ELSE inbox.box END">>, UniqueKeyFields = [<<"luser">>, <<"lserver">>, <<"remote_bare_jid">>], InsertFields = UniqueKeyFields ++ [<<"msg_id">>, <<"box">>, <<"content">>, <<"unread_count">>, <<"timestamp">>], rdbms_queries:prepare_upsert(HostType, inbox_upsert, inbox, InsertFields, [<<"msg_id">>, - {expression, <<"box">>, <<"CASE WHEN inbox.box='archive' THEN ? ELSE inbox.box END">>}, + {expression, <<"box">>, BoxQuery}, <<"content">>, <<"unread_count">>, <<"timestamp">>], @@ -80,7 +83,7 @@ init(HostType, _Options) -> rdbms_queries:prepare_upsert(HostType, inbox_upsert_incr_unread, inbox, InsertFields, [<<"msg_id">>, - {expression, <<"box">>, <<"CASE WHEN inbox.box='archive' THEN ? ELSE inbox.box END">>}, + {expression, <<"box">>, BoxQuery}, <<"content">>, {expression, <<"unread_count">>, <<"inbox.unread_count + ?">>}, <<"timestamp">>],