Add tests for fetching specific messages for MAM

big_tests/tests/mam_SUITE.erl

@@ -99,6 +99,8 @@
          server_returns_item_not_found_for_before_filter_with_nonexistent_id/1,
          server_returns_item_not_found_for_after_filter_with_nonexistent_id/1,
          server_returns_item_not_found_for_after_filter_with_invalid_id/1,
+         server_returns_item_not_found_for_ids_filter_with_nonexistent_id/1,
+         muc_server_returns_item_not_found_for_ids_filter_with_nonexistent_id/1,
          %% complete_flag_cases tests
          before_complete_false_last5/1,
          before_complete_false_before10/1,
@@ -120,6 +122,10 @@
          offline_message/1,
          nostore_hint/1,
          querying_for_all_messages_with_jid/1,
+         query_messages_by_ids/1,
+         simple_query_messages_by_ids/1,
+         muc_query_messages_by_ids/1,
+         muc_simple_query_messages_by_ids/1,
          muc_querying_for_all_messages/1,
          muc_querying_for_all_messages_with_jid/1,
          muc_light_service_discovery_stored_in_pm/1,
@@ -181,6 +187,8 @@
          stanza_archive_request/2,
          stanza_text_search_archive_request/3,
          stanza_include_groupchat_request/3,
+         stanza_fetch_by_id_request/3,
+         stanza_fetch_by_id_request/4,
          stanza_date_range_archive_request_not_empty/3,
          wait_archive_respond/1,
          wait_for_complete_archive_response/3,
@@ -209,6 +217,8 @@
          wait_message_range/3,
          wait_message_range/5,
          message_id/2,
+         get_pre_generated_msgs_ids/2,
+         get_received_msgs_ids/1,
          stanza_prefs_set_request/4,
          stanza_prefs_get_request/1,
          stanza_query_get_request/1,
@@ -353,7 +363,8 @@ basic_groups() ->
            [{mam_metrics, [], mam_metrics_cases()},
             {mam04, [parallel], mam_cases() ++ [retrieve_form_fields] ++ text_search_cases()},
             {mam06, [parallel], mam_cases() ++ [retrieve_form_fields_extra_features]
-                                ++ stanzaid_cases() ++ retract_cases() ++ metadata_cases()},
+                                ++ stanzaid_cases() ++ retract_cases()
+                                ++ metadata_cases() ++ fetch_specific_msgs_cases()},
             {nostore, [parallel], nostore_cases()},
             {archived, [parallel], archived_cases()},
             {configurable_archiveid, [], configurable_archiveid_cases()},
@@ -373,7 +384,7 @@ basic_groups() ->
      {muc_all, [parallel],
            [{muc04, [parallel], muc_cases() ++ muc_text_search_cases()},
             {muc06, [parallel], muc_cases() ++ muc_stanzaid_cases() ++ muc_retract_cases()
-                                ++ muc_metadata_cases()},
+                                ++ muc_metadata_cases() ++ muc_fetch_specific_msgs_cases()},
             {muc_configurable_archiveid, [], muc_configurable_archiveid_cases()},
             {muc_rsm_all, [parallel],
              [{muc_rsm04, [parallel], muc_rsm_cases()}]}]},
@@ -444,6 +455,13 @@ metadata_cases() ->
      metadata_archive_request_one_message
     ].
 
+fetch_specific_msgs_cases() ->
+    [
+     query_messages_by_ids,
+     simple_query_messages_by_ids,
+     server_returns_item_not_found_for_ids_filter_with_nonexistent_id
+    ].
+
 muc_text_search_cases() ->
     [
      muc_text_search_request
@@ -510,6 +528,13 @@ muc_metadata_cases() ->
      muc_metadata_archive_request_one_message
     ].
 
+muc_fetch_specific_msgs_cases() ->
+    [
+     muc_query_messages_by_ids,
+     muc_simple_query_messages_by_ids,
+     muc_server_returns_item_not_found_for_ids_filter_with_nonexistent_id
+    ].
+
 configurable_archiveid_cases() ->
     [no_elements,
      only_stanzaid,
@@ -828,6 +853,11 @@ init_per_testcase(C=filter_forwarded, Config) ->
 init_per_testcase(C=querying_for_all_messages_with_jid, Config) ->
     Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}, {carol, 1}]),
     escalus:init_per_testcase(C, bootstrap_archive(Config1));
+init_per_testcase(C, Config) when C =:= query_messages_by_ids;
+                                  C =:= simple_query_messages_by_ids;
+                                  C =:= server_returns_item_not_found_for_ids_filter_with_nonexistent_id ->
+    Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}, {carol, 1}]),
+    escalus:init_per_testcase(C, bootstrap_archive(Config1));
 init_per_testcase(C=archived, Config) ->
     Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}]),
     escalus:init_per_testcase(C, Config1);
@@ -846,6 +876,11 @@ init_per_testcase(C=offline_message, Config) ->
     escalus:init_per_testcase(C, Config1);
 init_per_testcase(C=nostore_hint, Config) ->
     escalus:init_per_testcase(C, Config);
+init_per_testcase(C, Config) when C =:= muc_query_messages_by_ids;
+                                  C =:= muc_simple_query_messages_by_ids;
+                                  C =:= muc_server_returns_item_not_found_for_ids_filter_with_nonexistent_id ->
+    Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}]),
+    escalus:init_per_testcase(C, muc_bootstrap_archive(start_alice_room(Config1)));
 init_per_testcase(C=muc_querying_for_all_messages, Config) ->
     Config1 = escalus_fresh:create_users(Config, [{alice, 1}, {bob, 1}]),
     escalus:init_per_testcase(C,
@@ -1056,6 +1091,15 @@ end_per_testcase(C=muc_show_x_user_to_moderators_in_anon_rooms, Config) ->
 end_per_testcase(C=muc_show_x_user_for_your_own_messages_in_anon_rooms, Config) ->
     destroy_room(Config),
     escalus:end_per_testcase(C, Config);
+end_per_testcase(C=muc_query_messages_by_ids, Config) ->
+    destroy_room(Config),
+    escalus:end_per_testcase(C, Config);
+end_per_testcase(C=muc_simple_query_messages_by_ids, Config) ->
+    destroy_room(Config),
+    escalus:end_per_testcase(C, Config);
+end_per_testcase(C=muc_server_returns_item_not_found_for_ids_filter_with_nonexistent_id, Config) ->
+    destroy_room(Config),
+    escalus:end_per_testcase(C, Config);
 end_per_testcase(C=muc_querying_for_all_messages, Config) ->
     destroy_room(Config),
     escalus:end_per_testcase(C, Config);
@@ -1648,6 +1692,130 @@ querying_for_all_messages_with_jid(Config) ->
         end,
     escalus:story(Config, [{alice, 1}], F).
 
+query_messages_by_ids(Config) ->
+    P = ?config(props, Config),
+    F = fun(Alice) ->
+        Msgs = ?config(pre_generated_msgs, Config),
+        IDs = get_pre_generated_msgs_ids(Msgs, [5, 10]),
+
+        Stanza = stanza_fetch_by_id_request(P, <<"fetch-msgs-by-ids">>, IDs),
+        escalus:send(Alice, Stanza),
+
+        Result = wait_archive_respond(Alice),
+        ResultIDs = get_received_msgs_ids(Result),
+
+        assert_respond_size(2, Result),
+        ?assert_equal(lists:sort(ResultIDs), lists:sort(IDs)),
+        ok
+        end,
+    escalus:story(Config, [{alice, 1}], F).
+
+simple_query_messages_by_ids(Config) ->
+    P = ?config(props, Config),
+    F = fun(Alice) ->
+        Msgs = ?config(pre_generated_msgs, Config),
+        [ID1, ID2, ID5] = get_pre_generated_msgs_ids(Msgs, [1, 2, 5]),
+
+        RSM = #rsm_in{max = 10, direction = 'after', id = ID1, simple = true},
+        Stanza = stanza_fetch_by_id_request(P, <<"simple-fetch-msgs-by-ids">>, [ID2, ID5], RSM),
+        escalus:send(Alice, Stanza),
+
+        Result = wait_archive_respond(Alice),
+        ParsedIQ = parse_result_iq(Result),
+        ResultIDs = get_received_msgs_ids(Result),
+
+        ?assert_equal(lists:sort(ResultIDs), lists:sort([ID2, ID5])),
+        ?assert_equal(undefined, ParsedIQ#result_iq.count),
+        ?assert_equal(undefined, ParsedIQ#result_iq.first_index),
+        ok
+        end,
+    escalus:story(Config, [{alice, 1}], F).
+
+server_returns_item_not_found_for_ids_filter_with_nonexistent_id(Config) ->
+    P = ?config(props, Config),
+    F = fun(Alice) ->
+        Msgs = ?config(pre_generated_msgs, Config),
+        IDs = get_pre_generated_msgs_ids(Msgs, [3, 12]),
+        NonexistentID = <<"AV25E9SCO50K">>,
+
+        Stanza = stanza_fetch_by_id_request(P, <<"ids-not-found">>, IDs ++ [NonexistentID]),
+        escalus:send(Alice, Stanza),
+        Result = escalus:wait_for_stanza(Alice),
+
+        escalus:assert(is_iq_error, [Stanza], Result),
+        escalus:assert(is_error, [<<"cancel">>, <<"item-not-found">>], Result),
+        ok
+        end,
+    escalus:story(Config, [{alice, 1}], F).
+
+muc_query_messages_by_ids(Config) ->
+    P = ?config(props, Config),
+    F = fun(Alice) ->
+        maybe_wait_for_archive(Config),
+
+        Room = ?config(room, Config),
+        Msgs = ?config(pre_generated_muc_msgs, Config),
+        IDs = get_pre_generated_msgs_ids(Msgs, [5, 10]),
+
+        Stanza = stanza_fetch_by_id_request(P, <<"fetch-muc-msgs-by-ids">>, IDs),
+        escalus:send(Alice, stanza_to_room(Stanza, Room)),
+        maybe_wait_for_archive(Config),
+
+        Result = wait_archive_respond(Alice),
+        ResultIDs = get_received_msgs_ids(Result),
+
+        assert_respond_size(2, Result),
+        ?assert_equal(lists:sort(ResultIDs), lists:sort(IDs)),
+        ok
+        end,
+    escalus:story(Config, [{alice, 1}], F).
+
+muc_simple_query_messages_by_ids(Config) ->
+    P = ?config(props, Config),
+    F = fun(Alice) ->
+        maybe_wait_for_archive(Config),
+
+        Room = ?config(room, Config),
+        Msgs = ?config(pre_generated_muc_msgs, Config),
+        [ID1, ID2, ID5] = get_pre_generated_msgs_ids(Msgs, [1, 2, 5]),
+
+        RSM = #rsm_in{max = 10, direction = 'after', id = ID1, simple = true},
+        Stanza = stanza_fetch_by_id_request(P, <<"muc-simple-fetch-msgs-by-ids">>, [ID2, ID5], RSM),
+        escalus:send(Alice, stanza_to_room(Stanza, Room)),
+        maybe_wait_for_archive(Config),
+
+        Result = wait_archive_respond(Alice),
+        ParsedIQ = parse_result_iq(Result),
+        ResultIDs = get_received_msgs_ids(Result),
+
+        ?assert_equal(lists:sort(ResultIDs), lists:sort([ID2, ID5])),
+        ?assert_equal(undefined, ParsedIQ#result_iq.count),
+        ?assert_equal(undefined, ParsedIQ#result_iq.first_index),
+        ok
+        end,
+    escalus:story(Config, [{alice, 1}], F).
+
+muc_server_returns_item_not_found_for_ids_filter_with_nonexistent_id(Config) ->
+    P = ?config(props, Config),
+    F = fun(Alice) ->
+        maybe_wait_for_archive(Config),
+
+        Room = ?config(room, Config),
+        Msgs = ?config(pre_generated_muc_msgs, Config),
+        IDs = get_pre_generated_msgs_ids(Msgs, [3, 12]),
+        NonexistentID = <<"AV25E9SCO50K">>,
+
+        Stanza = stanza_fetch_by_id_request(P, <<"muc-ids-not-found">>, IDs ++ [NonexistentID]),
+        escalus:send(Alice, stanza_to_room(Stanza, Room)),
+        maybe_wait_for_archive(Config),
+        Result = escalus:wait_for_stanza(Alice),
+
+        escalus:assert(is_iq_error, [Stanza], Result),
+        escalus:assert(is_error, [<<"cancel">>, <<"item-not-found">>], Result),
+        ok
+        end,
+    escalus:story(Config, [{alice, 1}], F).
+
 muc_querying_for_all_messages(Config) ->
     P = ?config(props, Config),
     F = fun(Alice) ->

big_tests/tests/mam_helper.erl

@@ -57,6 +57,8 @@
          stanza_archive_request/2,
          stanza_text_search_archive_request/3,
          stanza_include_groupchat_request/3,
+         stanza_fetch_by_id_request/3,
+         stanza_fetch_by_id_request/4,
          stanza_date_range_archive_request_not_empty/3,
          wait_archive_respond/1,
          wait_for_complete_archive_response/3,
@@ -88,6 +90,8 @@
          wait_message_range/3,
          wait_message_range/5,
          message_id/2,
+         get_pre_generated_msgs_ids/2,
+         get_received_msgs_ids/1,
          stanza_prefs_set_request/4,
          stanza_prefs_get_request/1,
          stanza_query_get_request/1,
@@ -297,21 +301,21 @@ stanza_archive_request(P, QueryId) ->
 
 stanza_date_range_archive_request(P) ->
     Params = #{
-        start => "2010-06-07T00:00:00Z",
-        stop => "2010-07-07T13:23:54Z"
+        start => <<"2010-06-07T00:00:00Z">>,
+        stop => <<"2010-07-07T13:23:54Z">>
     },
     stanza_lookup_messages_iq(P, Params).
 
 stanza_date_range_archive_request_not_empty(P, Start, Stop) ->
     Params = #{
-        start => Start,
-        stop => Stop
+        start => list_to_binary(Start),
+        stop => list_to_binary(Stop)
     },
     stanza_lookup_messages_iq(P, Params).
 
 stanza_limit_archive_request(P) ->
     Params = #{
-        start => "2010-08-07T00:00:00Z",
+        start => <<"2010-08-07T00:00:00Z">>,
         rsm => #rsm_in{max=10}
     },
     stanza_lookup_messages_iq(P, Params).
@@ -349,6 +353,17 @@ stanza_include_groupchat_request(P, QueryId, IncludeGroupChat) ->
     },
     stanza_lookup_messages_iq(P, Params).
 
+stanza_fetch_by_id_request(P, QueryId, IDs) ->
+    stanza_fetch_by_id_request(P, QueryId, IDs, undefined).
+
+stanza_fetch_by_id_request(P, QueryId, IDs, RSM) ->
+    Params = #{
+        query_id => QueryId,
+        messages_ids => IDs,
+        rsm => RSM
+    },
+    stanza_lookup_messages_iq(P, Params).
+
 stanza_lookup_messages_iq(P, Params) ->
     QueryId = maps:get(query_id, Params, undefined),
     BStart = maps:get(start, Params, undefined),
@@ -358,25 +373,27 @@ stanza_lookup_messages_iq(P, Params) ->
     TextSearch = maps:get(text_search, Params, undefined),
     FlipPage = maps:get(flip_page, Params, undefined),
     IncludeGroupChat = maps:get(include_group_chat, Params, undefined),
+    MessagesIDs = maps:get(messages_ids, Params, undefined),
 
     escalus_stanza:iq(<<"set">>, [#xmlel{
        name = <<"query">>,
        attrs = mam_ns_attr(P)
             ++ maybe_attr(<<"queryid">>, QueryId),
        children = skip_undefined([
-           form_x(BStart, BEnd, BWithJID, RSM, TextSearch, IncludeGroupChat),
+           form_x(BStart, BEnd, BWithJID, RSM, TextSearch, IncludeGroupChat, MessagesIDs),
            maybe_rsm_elem(RSM),
            maybe_flip_page(FlipPage)])
     }]).
 
-form_x(undefined, undefined, undefined, undefined, undefined, undefined) ->
+form_x(undefined, undefined, undefined, undefined, undefined, undefined, undefined) ->
     undefined;
-form_x(BStart, BEnd, BWithJID, RSM, TextSearch, IncludeGroupChat) ->
+form_x(BStart, BEnd, BWithJID, RSM, TextSearch, IncludeGroupChat, MessagesIDs) ->
     Fields = skip_undefined([form_field(<<"start">>, BStart),
                              form_field(<<"end">>, BEnd),
                              form_field(<<"with">>, BWithJID),
                              form_field(<<"full-text-search">>, TextSearch),
-                             form_field(<<"include-groupchat">>, IncludeGroupChat)]
+                             form_field(<<"include-groupchat">>, IncludeGroupChat),
+                             form_field(<<"ids">>, MessagesIDs)]
                             ++ form_extra_fields(RSM)
                             ++ form_border_fields(RSM)),
     form_helper:form(#{fields => Fields}).
@@ -397,6 +414,8 @@ form_border_fields(#rsm_in{
 
 form_field(_VarName, undefined) ->
     undefined;
+form_field(VarName, VarValues) when is_list(VarValues) ->
+    #{var => VarName, values => VarValues};
 form_field(VarName, VarValue) ->
     #{var => VarName, values => [VarValue]}.
 
@@ -585,6 +604,21 @@ message_id(Num, Config) ->
     #forwarded_message{result_id=Id} = lists:nth(Num, AllMessages),
     Id.
 
+get_pre_generated_msgs_ids(Msgs, Nums) ->
+    lists:map(fun(N) ->
+        Msg = lists:nth(N, Msgs),
+        {{MsgID, _}, _, _, _, _} = Msg,
+        rpc_apply(mod_mam_utils, mess_id_to_external_binary, [MsgID])
+      end, Nums).
+
+get_received_msgs_ids(Response) ->
+    Msgs = respond_messages(Response),
+    lists:map(fun(M) ->
+                Parsed = parse_forwarded_message(M),
+                Parsed#forwarded_message.result_id
+              end, Msgs).
+
+
 %% @doc Result query iq.
 %%
 %% [{xmlel,<<"iq">>,

src/mam/mod_mam_cassandra_arch.erl

@@ -606,7 +606,7 @@ prepare_filter(UserJID, Borders, Start, End, WithJID, MsgID) ->
     BUserJID = bare_jid(UserJID),
     %% In Cassandra, a column cannot be restricted by both an equality and an inequality relation.
     %% When MsgID is defined, it is used as both StartID and EndID to comply with this limitation.
-    %% This means that the `ids` filter effectively overrides filters like 'before-id' or 'after-id'.
+    %% This means that the `ids` filter effectively overrides any "before" or "after" filters.
     {StartID, EndID} = case MsgID of
                             undefined ->
                                 mod_mam_utils:calculate_msg_id_borders(Borders, Start, End);

src/mam/mod_mam_muc.erl

@@ -524,7 +524,7 @@ lookup_messages_without_policy_violation_check(HostType,
             {error, 'not-supported'};
         false ->
             StartT = erlang:monotonic_time(microsecond),
-            R = case maps:get(message_ids, Params) of
+            R = case maps:get(message_ids, Params, undefined) of
                     undefined ->
                         mongoose_hooks:mam_muc_lookup_messages(HostType,
                             Params#{message_id => undefined});