From bf21ca07cf4b28bc0dd262121f1d94eabdaa016e Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Mon, 19 Feb 2024 13:35:19 +0100 Subject: [PATCH 1/7] Add fetching specific messages for MAM pm --- src/mam/mam_iq.erl | 8 +++++++ src/mam/mod_mam_cassandra_arch.erl | 26 +++++++++++++-------- src/mam/mod_mam_elasticsearch_arch.erl | 16 +++++++++++-- src/mam/mod_mam_pm.erl | 9 +++++++- src/mam/mod_mam_rdbms_arch.erl | 3 ++- src/mam/mod_mam_utils.erl | 32 +++++++++++++++++++++++++- 6 files changed, 80 insertions(+), 14 deletions(-) diff --git a/src/mam/mam_iq.erl b/src/mam/mam_iq.erl index 988fffd642e..b0d79a6c0d7 100644 --- a/src/mam/mam_iq.erl +++ b/src/mam/mam_iq.erl @@ -40,6 +40,8 @@ search_text => binary() | undefined, %% Filtering Result Set based on message ids borders => mod_mam:borders() | undefined, + %% Filtering Result Set based on specific message ids + message_ids => [mod_mam:message_id()] | undefined, %% Affects 'policy-violation' for a case when: %% - user does not use forms to query archive %% - user does not provide "set" element @@ -145,6 +147,7 @@ form_to_lookup_params(#iq{sub_el = QueryEl} = IQ, MaxResultLimit, DefaultResultL search_text => mod_mam_utils:form_to_text(KVs), borders => mod_mam_utils:form_borders_decode(KVs), + message_ids => form_to_msg_ids(KVs), %% Whether or not the client query included a element, %% the server MAY simply return its limited results. %% So, disable 'policy-violation'. @@ -205,3 +208,8 @@ include_groupchat(#{<<"include-groupchat">> := [<<"false">>]}) -> false; include_groupchat(_) -> undefined. + +form_to_msg_ids(#{<<"ids">> := IDs}) -> + [mod_mam_utils:external_binary_to_mess_id(ID) || ID <- IDs]; +form_to_msg_ids(_) -> + undefined. diff --git a/src/mam/mod_mam_cassandra_arch.erl b/src/mam/mod_mam_cassandra_arch.erl index 9acf29ae83a..3959b9385f5 100644 --- a/src/mam/mod_mam_cassandra_arch.erl +++ b/src/mam/mod_mam_cassandra_arch.erl @@ -119,7 +119,7 @@ prepared_queries() -> Extra :: gen_hook:extra(). archive_size(Size, #{owner := UserJID}, #{host_type := HostType}) when is_integer(Size) -> Borders = Start = End = WithJID = undefined, - Filter = prepare_filter(UserJID, Borders, Start, End, WithJID), + Filter = prepare_filter(UserJID, Borders, Start, End, WithJID, undefined), {ok, calc_count(pool_name(HostType), UserJID, HostType, Filter)}. @@ -229,13 +229,13 @@ lookup_messages(_Result, #{owner_jid := UserJID, rsm := RSM, borders := Borders, start_ts := Start, end_ts := End, with_jid := WithJID, search_text := undefined, page_size := PageSize, - is_simple := IsSimple}, + is_simple := IsSimple, message_id := MsgID}, #{host_type := HostType}) -> try {ok, lookup_messages2(pool_name(HostType), HostType, UserJID, RSM, Borders, Start, End, WithJID, - PageSize, IsSimple)} + PageSize, MsgID, IsSimple)} catch _Type:Reason:S -> {ok, {error, {Reason, S}}} end. @@ -243,20 +243,20 @@ lookup_messages(_Result, lookup_messages2(PoolName, HostType, UserJID = #jid{}, RSM, Borders, Start, End, WithJID, - PageSize, _IsSimple = true) -> + PageSize, MsgID, _IsSimple = true) -> %% Simple query without calculating offset and total count - Filter = prepare_filter(UserJID, Borders, Start, End, WithJID), + Filter = prepare_filter(UserJID, Borders, Start, End, WithJID, MsgID), lookup_messages_simple(PoolName, HostType, UserJID, RSM, PageSize, Filter); lookup_messages2(PoolName, HostType, UserJID = #jid{}, RSM, Borders, Start, End, WithJID, - PageSize, _IsSimple) -> + PageSize, MsgID, _IsSimple) -> %% Query with offset calculation %% We cannot just use RDBMS code because "LIMIT X, Y" is not supported by cassandra %% Not all queries are optimal. You would like to disable something for production %% once you know how you will call bd Strategy = rsm_to_strategy(RSM), - Filter = prepare_filter(UserJID, Borders, Start, End, WithJID), + Filter = prepare_filter(UserJID, Borders, Start, End, WithJID, MsgID), case Strategy of last_page -> lookup_messages_last_page(PoolName, HostType, UserJID, RSM, PageSize, Filter); @@ -602,9 +602,17 @@ prev_offset_query_cql() -> insert_offset_hint_query_cql() -> "INSERT INTO mam_message_offset(user_jid, with_jid, id, offset) VALUES(?, ?, ?, ?)". -prepare_filter(UserJID, Borders, Start, End, WithJID) -> +prepare_filter(UserJID, Borders, Start, End, WithJID, MsgID) -> BUserJID = bare_jid(UserJID), - {StartID, EndID} = mod_mam_utils:calculate_msg_id_borders(Borders, Start, End), + %% 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'. + {StartID, EndID} = case MsgID of + undefined -> + mod_mam_utils:calculate_msg_id_borders(Borders, Start, End); + ID -> + {ID, ID} + end, BWithJID = maybe_full_jid(WithJID), %% it's NOT optional field prepare_filter_params(BUserJID, BWithJID, StartID, EndID). diff --git a/src/mam/mod_mam_elasticsearch_arch.erl b/src/mam/mod_mam_elasticsearch_arch.erl index 578b88129e9..f90bccb9e4e 100644 --- a/src/mam/mod_mam_elasticsearch_arch.erl +++ b/src/mam/mod_mam_elasticsearch_arch.erl @@ -118,12 +118,17 @@ lookup_messages(Result, lookup_messages(Result, Params, #{host_type := HostType}) -> {ok, do_lookup_messages(Result, HostType, Params)}. -lookup_message_page(AccResult, Host, #rsm_in{id = _ID} = RSM, Params) -> +lookup_message_page(AccResult, Host, #rsm_in{id = _ID} = RSM, #{message_id := MsgID} = Params) -> PageSize = maps:get(page_size, Params), case do_lookup_messages(AccResult, Host, Params#{page_size := 1 + PageSize}) of {error, _} = Err -> Err; {ok, LookupResult} -> - mod_mam_utils:check_for_item_not_found(RSM, PageSize, LookupResult) + case MsgID of + undefined -> + mod_mam_utils:check_for_item_not_found(RSM, PageSize, LookupResult); + _ -> + {ok, LookupResult} + end end. do_lookup_messages(_Result, Host, Params) -> @@ -213,6 +218,7 @@ build_filters(Params) -> Builders = [fun owner_filter/1, fun with_jid_filter/1, fun is_groupchat_filter/1, + fun specific_message_filter/1, fun range_filter/1], lists:flatmap(fun(F) -> F(Params) end, Builders). @@ -233,6 +239,12 @@ is_groupchat_filter(#{include_groupchat := false}) -> is_groupchat_filter(_) -> []. +-spec specific_message_filter(map()) -> [map()]. +specific_message_filter(#{message_id := ID}) when is_integer(ID) -> + [#{term => #{mam_id => ID}}]; +specific_message_filter(_) -> + []. + -spec range_filter(map()) -> [map()]. range_filter(#{end_ts := End, start_ts := Start, borders := Borders, rsm := RSM}) -> {StartId, EndId} = mod_mam_utils:calculate_msg_id_borders(RSM, Borders, Start, End), diff --git a/src/mam/mod_mam_pm.erl b/src/mam/mod_mam_pm.erl index fa6b2e9c1ba..a2a191a5471 100644 --- a/src/mam/mod_mam_pm.erl +++ b/src/mam/mod_mam_pm.erl @@ -599,7 +599,14 @@ lookup_messages_without_policy_violation_check( {error, 'not-supported'}; false -> StartT = erlang:monotonic_time(microsecond), - R = mongoose_hooks:mam_lookup_messages(HostType, Params), + R = case maps:get(message_ids, Params) of + undefined -> + mongoose_hooks:mam_lookup_messages(HostType, + Params#{message_id => undefined}); + IDs -> + mod_mam_utils:lookup_specific_messages(HostType, Params, IDs, + fun mongoose_hooks:mam_lookup_messages/2) + end, Diff = erlang:monotonic_time(microsecond) - StartT, mongoose_metrics:update(HostType, [backends, ?MODULE, lookup], Diff), R diff --git a/src/mam/mod_mam_rdbms_arch.erl b/src/mam/mod_mam_rdbms_arch.erl index ce4a0fcabb8..537834bd1ea 100644 --- a/src/mam/mod_mam_rdbms_arch.erl +++ b/src/mam/mod_mam_rdbms_arch.erl @@ -204,7 +204,8 @@ lookup_fields() -> #lookup_field{op = equal, column = remote_bare_jid, param = remote_bare_jid}, #lookup_field{op = equal, column = remote_resource, param = remote_resource}, #lookup_field{op = like, column = search_body, param = norm_search_text, value_maker = search_words}, - #lookup_field{op = equal, column = is_groupchat, param = include_groupchat}]. + #lookup_field{op = equal, column = is_groupchat, param = include_groupchat}, + #lookup_field{op = equal, column = id, param = message_id}]. -spec env_vars(host_type(), jid:jid()) -> env_vars(). env_vars(HostType, ArcJID) -> diff --git a/src/mam/mod_mam_utils.erl b/src/mam/mod_mam_utils.erl index 09c4f70c77c..5457cf91ee0 100644 --- a/src/mam/mod_mam_utils.erl +++ b/src/mam/mod_mam_utils.erl @@ -84,6 +84,7 @@ check_for_item_not_found/3, maybe_reverse_messages/2, get_msg_id_and_timestamp/1, + lookup_specific_messages/4, is_mam_muc_enabled/2]). %% Ejabberd @@ -416,6 +417,34 @@ get_msg_id_and_timestamp(#{id := MsgID}) -> ExtID = mess_id_to_external_binary(MsgID), {ExtID, list_to_binary(TS)}. +-spec lookup_specific_messages(mongooseim:host_type(), + mam_iq:lookup_params(), + [mod_mam:message_id()], + fun()) -> [mod_mam:message_row()] | {error, item_not_found}. +lookup_specific_messages(HostType, Params, IDs, FetchFun) -> + {FinalOffset, AccumulatedMessages} = lists:foldl( + fun(ID, {_AccOffset, AccMsgs}) -> + {ok, {_, OffsetForID, MessagesForID}} = FetchFun(HostType, Params#{message_id => ID}), + {OffsetForID, AccMsgs ++ MessagesForID} + end, + {0, []}, IDs), + + Result = determine_result(Params, FinalOffset, AccumulatedMessages), + + case length(IDs) == length(AccumulatedMessages) of + true -> Result; + false -> {error, item_not_found} + end. + +determine_result(Params, Offset, Messages) -> + case maps:get(is_simple, Params, false) of + true -> + {ok, {undefined, undefined, Messages}}; + false -> + {ok, {length(Messages), Offset, Messages}} + end. + + tombstone(RetractionInfo = #{packet := Packet}, LocJid) -> Packet#xmlel{children = [retracted_element(RetractionInfo, LocJid)]}. @@ -1243,7 +1272,8 @@ create_lookup_params(RSM, Direction, ArcID, CallerJID, OwnerJID) -> flip_page => false, ordering_direction => Direction, limit_passed => true, - caller_jid => CallerJID}. + caller_jid => CallerJID, + message_ids => undefined}. patch_fun_to_make_result_as_map(F) -> fun(HostType, Params) -> result_to_map(F(HostType, Params)) end. From 686c47502220768dfae537a44e4eca13924a022e Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Mon, 19 Feb 2024 13:35:55 +0100 Subject: [PATCH 2/7] Add fetching specific messages for MAM MUC --- src/mam/mod_mam_muc.erl | 9 ++++++- src/mam/mod_mam_muc_cassandra_arch.erl | 28 ++++++++++++++-------- src/mam/mod_mam_muc_elasticsearch_arch.erl | 18 +++++++++++--- src/mam/mod_mam_muc_rdbms_arch.erl | 3 ++- 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/mam/mod_mam_muc.erl b/src/mam/mod_mam_muc.erl index 98de0354a38..e4ea5a050fd 100644 --- a/src/mam/mod_mam_muc.erl +++ b/src/mam/mod_mam_muc.erl @@ -524,7 +524,14 @@ lookup_messages_without_policy_violation_check(HostType, {error, 'not-supported'}; false -> StartT = erlang:monotonic_time(microsecond), - R = mongoose_hooks:mam_muc_lookup_messages(HostType, Params), + R = case maps:get(message_ids, Params) of + undefined -> + mongoose_hooks:mam_muc_lookup_messages(HostType, + Params#{message_id => undefined}); + IDs -> + mod_mam_utils:lookup_specific_messages(HostType, Params, IDs, + fun mongoose_hooks:mam_muc_lookup_messages/2) + end, Diff = erlang:monotonic_time(microsecond) - StartT, mongoose_metrics:update(HostType, [backends, ?MODULE, lookup], Diff), R diff --git a/src/mam/mod_mam_muc_cassandra_arch.erl b/src/mam/mod_mam_muc_cassandra_arch.erl index 9e7aba451c7..8cbbfc600e0 100644 --- a/src/mam/mod_mam_muc_cassandra_arch.erl +++ b/src/mam/mod_mam_muc_cassandra_arch.erl @@ -120,7 +120,7 @@ prepared_queries() -> archive_size(Size, #{room := RoomJID}, #{host_type := HostType}) when is_integer(Size) -> PoolName = pool_name(HostType), Borders = Start = End = WithNick = undefined, - Filter = prepare_filter(RoomJID, Borders, Start, End, WithNick), + Filter = prepare_filter(RoomJID, Borders, Start, End, WithNick, undefined), {ok, calc_count(PoolName, RoomJID, HostType, Filter)}. @@ -228,7 +228,7 @@ lookup_messages(_Result, #{owner_jid := RoomJID, rsm := RSM, borders := Borders, start_ts := Start, end_ts := End, with_jid := WithJID, search_text := undefined, page_size := PageSize, - is_simple := IsSimple}, + is_simple := IsSimple, message_id := MsgID}, #{host_type := HostType}) -> try WithNick = maybe_jid_to_nick(WithJID), @@ -236,7 +236,7 @@ lookup_messages(_Result, {ok, lookup_messages2(PoolName, HostType, RoomJID, RSM, Borders, Start, End, WithNick, - PageSize, IsSimple)} + PageSize, MsgID, IsSimple)} catch _Type:Reason:Stacktrace -> {ok, {error, {Reason, {stacktrace, Stacktrace}}}} end. @@ -248,20 +248,20 @@ maybe_jid_to_nick(undefined) -> undefined. lookup_messages2(PoolName, HostType, RoomJID = #jid{}, RSM, Borders, Start, End, WithNick, - PageSize, _IsSimple = true) -> + PageSize, MsgID, _IsSimple = true) -> %% Simple query without calculating offset and total count - Filter = prepare_filter(RoomJID, Borders, Start, End, WithNick), + Filter = prepare_filter(RoomJID, Borders, Start, End, WithNick, MsgID), lookup_messages_simple(PoolName, HostType, RoomJID, RSM, PageSize, Filter); lookup_messages2(PoolName, HostType, RoomJID = #jid{}, RSM, Borders, Start, End, WithNick, - PageSize, _IsSimple) -> + PageSize, MsgID, _IsSimple) -> %% Query with offset calculation %% We cannot just use RDBMS code because "LIMIT X, Y" is not supported by cassandra %% Not all queries are optimal. You would like to disable something for production %% once you know how you will call bd Strategy = rsm_to_strategy(RSM), - Filter = prepare_filter(RoomJID, Borders, Start, End, WithNick), + Filter = prepare_filter(RoomJID, Borders, Start, End, WithNick, MsgID), case Strategy of last_page -> lookup_messages_last_page(PoolName, HostType, RoomJID, RSM, PageSize, Filter); @@ -611,12 +611,20 @@ prev_offset_query_cql() -> insert_offset_hint_query_cql() -> "INSERT INTO mam_muc_message_offset(room_jid, with_nick, id, offset) VALUES(?, ?, ?, ?)". -prepare_filter(RoomJID, Borders, Start, End, WithNick) -> +prepare_filter(RoomJID, Borders, Start, End, WithNick, MsgID) -> BRoomJID = mod_mam_utils:bare_jid(RoomJID), StartID = maybe_encode_compact_uuid(Start, 0), EndID = maybe_encode_compact_uuid(End, 255), - StartID2 = apply_start_border(Borders, StartID), - EndID2 = apply_end_border(Borders, EndID), + %% In Cassandra, a column cannot be restricted by both an equality and an inequality relation. + %% When MsgID is defined, it is used as both StartID2 and EndID2 to comply with this limitation. + %% This means that the `ids` filter effectively overrides filters like 'before-id' or 'after-id'. + {StartID2, EndID2} = case MsgID of + undefined -> + {apply_start_border(Borders, StartID), + apply_end_border(Borders, EndID)}; + ID -> + {ID, ID} + end, BWithNick = maybe_nick(WithNick), prepare_filter_params(BRoomJID, BWithNick, StartID2, EndID2). diff --git a/src/mam/mod_mam_muc_elasticsearch_arch.erl b/src/mam/mod_mam_muc_elasticsearch_arch.erl index 4173bd15813..09a2ff31f7e 100644 --- a/src/mam/mod_mam_muc_elasticsearch_arch.erl +++ b/src/mam/mod_mam_muc_elasticsearch_arch.erl @@ -118,13 +118,18 @@ lookup_messages(Result, lookup_messages(Result, Params, #{host_type := HostType}) -> {ok, do_lookup_messages(Result, HostType, Params)}. -lookup_message_page(AccResult, Host, RSM, Params) -> +lookup_message_page(AccResult, Host, RSM, #{message_id := MsgID} = Params) -> PageSize = maps:get(page_size, Params), case do_lookup_messages(AccResult, Host, Params#{page_size := 1 + PageSize}) of {error, _} = Err -> Err; {ok, LookupResult} -> - mod_mam_utils:check_for_item_not_found(RSM, PageSize, LookupResult) + case MsgID of + undefined -> + mod_mam_utils:check_for_item_not_found(RSM, PageSize, LookupResult); + _ -> + {ok, LookupResult} + end end. do_lookup_messages(_Result, Host, Params) -> @@ -208,7 +213,8 @@ build_search_query(Params) -> build_filters(Params) -> Builders = [fun room_filter/1, fun with_jid_filter/1, - fun range_filter/1], + fun range_filter/1, + fun specific_message_filter/1], lists:flatmap(fun(F) -> F(Params) end, Builders). -spec room_filter(map()) -> [map()]. @@ -237,6 +243,12 @@ range_filter(#{end_ts := End, start_ts := Start, borders := Borders, rsm := RSM} range_filter(_) -> []. +-spec specific_message_filter(map()) -> [map()]. +specific_message_filter(#{message_id := ID}) when is_integer(ID) -> + [#{term => #{mam_id => ID}}]; +specific_message_filter(_) -> + []. + -spec maybe_add_end_filter(undefined | mod_mam:message_id(), map()) -> map(). maybe_add_end_filter(undefined, RangeMap) -> RangeMap; diff --git a/src/mam/mod_mam_muc_rdbms_arch.erl b/src/mam/mod_mam_muc_rdbms_arch.erl index 02829c92743..b9e9893139c 100644 --- a/src/mam/mod_mam_muc_rdbms_arch.erl +++ b/src/mam/mod_mam_muc_rdbms_arch.erl @@ -169,7 +169,8 @@ lookup_fields() -> #lookup_field{op = ge, column = id, param = start_id}, #lookup_field{op = le, column = id, param = end_id}, #lookup_field{op = equal, column = nick_name, param = remote_resource}, - #lookup_field{op = like, column = search_body, param = norm_search_text, value_maker = search_words}]. + #lookup_field{op = like, column = search_body, param = norm_search_text, value_maker = search_words}, + #lookup_field{op = equal, column = id, param = message_id}]. -spec env_vars(host_type(), jid:jid()) -> env_vars(). env_vars(HostType, ArcJID) -> From 7283a1dabc7133f7550f8bc45a7c72f6b4a083bd Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Mon, 19 Feb 2024 13:36:25 +0100 Subject: [PATCH 3/7] Add tests for fetching specific messages for MAM --- big_tests/tests/mam_SUITE.erl | 172 ++++++++++++++++++++++++- big_tests/tests/mam_helper.erl | 52 ++++++-- src/mam/mod_mam_cassandra_arch.erl | 2 +- src/mam/mod_mam_muc.erl | 2 +- src/mam/mod_mam_muc_cassandra_arch.erl | 2 +- src/mam/mod_mam_pm.erl | 2 +- 6 files changed, 217 insertions(+), 15 deletions(-) diff --git a/big_tests/tests/mam_SUITE.erl b/big_tests/tests/mam_SUITE.erl index 00b375a973e..999182795ae 100644 --- a/big_tests/tests/mam_SUITE.erl +++ b/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) -> diff --git a/big_tests/tests/mam_helper.erl b/big_tests/tests/mam_helper.erl index ff75e4502bc..b1170bb090b 100644 --- a/big_tests/tests/mam_helper.erl +++ b/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">>, diff --git a/src/mam/mod_mam_cassandra_arch.erl b/src/mam/mod_mam_cassandra_arch.erl index 3959b9385f5..db9f526b2d8 100644 --- a/src/mam/mod_mam_cassandra_arch.erl +++ b/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); diff --git a/src/mam/mod_mam_muc.erl b/src/mam/mod_mam_muc.erl index e4ea5a050fd..625543978c7 100644 --- a/src/mam/mod_mam_muc.erl +++ b/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}); diff --git a/src/mam/mod_mam_muc_cassandra_arch.erl b/src/mam/mod_mam_muc_cassandra_arch.erl index 8cbbfc600e0..0bccd61ec89 100644 --- a/src/mam/mod_mam_muc_cassandra_arch.erl +++ b/src/mam/mod_mam_muc_cassandra_arch.erl @@ -617,7 +617,7 @@ prepare_filter(RoomJID, Borders, Start, End, WithNick, MsgID) -> EndID = maybe_encode_compact_uuid(End, 255), %% In Cassandra, a column cannot be restricted by both an equality and an inequality relation. %% When MsgID is defined, it is used as both StartID2 and EndID2 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. {StartID2, EndID2} = case MsgID of undefined -> {apply_start_border(Borders, StartID), diff --git a/src/mam/mod_mam_pm.erl b/src/mam/mod_mam_pm.erl index a2a191a5471..ac77b776c0e 100644 --- a/src/mam/mod_mam_pm.erl +++ b/src/mam/mod_mam_pm.erl @@ -599,7 +599,7 @@ lookup_messages_without_policy_violation_check( {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_lookup_messages(HostType, Params#{message_id => undefined}); From 3497e3c1a9e5e44bb82da6253d0a8fa926d77799 Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Tue, 20 Feb 2024 10:46:43 +0100 Subject: [PATCH 4/7] Add a new supported field and a disco feature --- big_tests/tests/mam_SUITE.erl | 9 ++++++++- big_tests/tests/mam_helper.erl | 5 +++++ include/mongoose_ns.hrl | 3 +++ src/mam/mod_mam_utils.erl | 12 ++++++++++-- src/mongoose_data_forms.erl | 6 ++++-- 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/big_tests/tests/mam_SUITE.erl b/big_tests/tests/mam_SUITE.erl index 999182795ae..4788bd6be67 100644 --- a/big_tests/tests/mam_SUITE.erl +++ b/big_tests/tests/mam_SUITE.erl @@ -226,10 +226,12 @@ mam_ns_binary/0, mam_ns_binary_v04/0, mam_ns_binary_v06/0, + mam_ns_binary_extended/0, retract_ns/0, retract_tombstone_ns/0, groupchat_field_ns/0, groupchat_available_ns/0, + data_validate_ns/0, make_alice_and_bob_friends/2, run_prefs_case/6, prefs_cases2/0, @@ -2111,9 +2113,13 @@ retrieve_form_fields_extra_features(ConfigIn) -> escalus:assert(is_iq_with_ns, [Namespace], Res), QueryEl = exml_query:subelement(Res, <<"query">>), XEl = exml_query:subelement(QueryEl, <<"x">>), + IDsEl = exml_query:subelement_with_attr(XEl, <<"var">>, <<"ids">>), + ValidateEl = exml_query:path(IDsEl, [{element_with_ns, <<"validate">>, data_validate_ns()}, + {element, <<"open">>}]), escalus:assert(has_field_with_type, [<<"before-id">>, <<"text-single">>], XEl), escalus:assert(has_field_with_type, [<<"after-id">>, <<"text-single">>], XEl), - escalus:assert(has_field_with_type, [<<"include-groupchat">>, <<"boolean">>], XEl) + escalus:assert(has_field_with_type, [<<"include-groupchat">>, <<"boolean">>], XEl), + ?assertNotEqual(ValidateEl, undefined) end). archived(Config) -> @@ -3574,6 +3580,7 @@ discover_features(Config, Client, Service) -> escalus:assert(is_iq_result, Stanza), escalus:assert(has_feature, [mam_ns_binary_v04()], Stanza), escalus:assert(has_feature, [mam_ns_binary_v06()], Stanza), + escalus:assert(has_feature, [mam_ns_binary_extended()], Stanza), escalus:assert(has_feature, [retract_ns()], Stanza), check_include_groupchat_features(Stanza, ?config(configuration, Config), diff --git a/big_tests/tests/mam_helper.erl b/big_tests/tests/mam_helper.erl index b1170bb090b..83919384389 100644 --- a/big_tests/tests/mam_helper.erl +++ b/big_tests/tests/mam_helper.erl @@ -100,11 +100,13 @@ mam_ns_binary/0, mam_ns_binary_v04/0, mam_ns_binary_v06/0, + mam_ns_binary_extended/0, retract_ns/0, retract_esl_ns/0, retract_tombstone_ns/0, groupchat_field_ns/0, groupchat_available_ns/0, + data_validate_ns/0, make_alice_and_bob_friends/2, run_prefs_case/6, prefs_cases2/0, @@ -257,6 +259,7 @@ nick(User) -> namespaces() -> [mam_ns_binary_v04(), mam_ns_binary_v06(), + mam_ns_binary_extended(), retract_ns(), retract_esl_ns(), retract_tombstone_ns()]. @@ -264,11 +267,13 @@ namespaces() -> mam_ns_binary() -> mam_ns_binary_v04(). mam_ns_binary_v04() -> <<"urn:xmpp:mam:1">>. mam_ns_binary_v06() -> <<"urn:xmpp:mam:2">>. +mam_ns_binary_extended() -> <<"urn:xmpp:mam:2#extended">>. retract_ns() -> <<"urn:xmpp:message-retract:0">>. retract_esl_ns() -> <<"urn:esl:message-retract-by-stanza-id:0">>. retract_tombstone_ns() -> <<"urn:xmpp:message-retract:0#tombstone">>. groupchat_field_ns() -> <<"urn:xmpp:mam:2#groupchat-field">>. groupchat_available_ns() -> <<"urn:xmpp:mam:2#groupchat-available">>. +data_validate_ns() -> <<"http://jabber.org/protocol/xdata-validate">>. skip_undefined(Xs) -> [X || X <- Xs, X =/= undefined]. diff --git a/include/mongoose_ns.hrl b/include/mongoose_ns.hrl index ebf1fb5d895..ba3abd6ea1a 100644 --- a/include/mongoose_ns.hrl +++ b/include/mongoose_ns.hrl @@ -58,6 +58,7 @@ -define(NS_SERVERINFO, <<"http://jabber.org/network/serverinfo">>). -define(NS_MAM_04, <<"urn:xmpp:mam:1">>). % MAM 0.4.1 or 0.5 -define(NS_MAM_06, <<"urn:xmpp:mam:2">>). % MAM 0.6 +-define(NS_MAM_EXTENDED, <<"urn:xmpp:mam:2#extended">>). -define(NS_MAM_GC_FIELD, <<"urn:xmpp:mam:2#groupchat-field">>). -define(NS_MAM_GC_AVAILABLE, <<"urn:xmpp:mam:2#groupchat-available">>). -define(NS_HTTP_UPLOAD_030, <<"urn:xmpp:http:upload:0">>). @@ -108,6 +109,8 @@ -define(JINGLE_NS, <<"urn:xmpp:jingle:1">>). +-define(NS_DATA_VALIDATE, <<"http://jabber.org/protocol/xdata-validate">>). + %% Custom extension to accept stanza-ids as retraction IDs -define(NS_ESL_RETRACT, <<"urn:esl:message-retract-by-stanza-id:0">>). diff --git a/src/mam/mod_mam_utils.erl b/src/mam/mod_mam_utils.erl index 5457cf91ee0..ab27b8a91a5 100644 --- a/src/mam/mod_mam_utils.erl +++ b/src/mam/mod_mam_utils.erl @@ -742,7 +742,7 @@ features(Module, HostType) -> ++ groupchat_features(Module, HostType). mam_features() -> - [?NS_MAM_04, ?NS_MAM_06]. + [?NS_MAM_04, ?NS_MAM_06, ?NS_MAM_EXTENDED]. retraction_features(Module, HostType) -> case has_message_retraction(Module, HostType) of @@ -795,7 +795,8 @@ message_form_fields(Mod, HostType, <<"urn:xmpp:mam:2">>) -> #{type => <<"text-single">>, var => <<"end">>}, #{type => <<"text-single">>, var => <<"before-id">>}, #{type => <<"text-single">>, var => <<"after-id">>}, - #{type => <<"boolean">>, var => <<"include-groupchat">>} | TextSearch]. + #{type => <<"boolean">>, var => <<"include-groupchat">>}, + #{type => <<"list-multi">>, var => <<"ids">>, children => [validate_element()]} | TextSearch]. -spec form_to_text(_) -> 'undefined' | binary(). form_to_text(#{<<"full-text-search">> := [Text]}) -> @@ -803,6 +804,13 @@ form_to_text(#{<<"full-text-search">> := [Text]}) -> form_to_text(#{}) -> undefined. +-spec validate_element() -> exml:element(). +validate_element() -> + #xmlel{name = <<"validate">>, + attrs = [{<<"xmlns">>, ?NS_DATA_VALIDATE}, {<<"datatype">>, <<"xs:string">>}], + children = [#xmlel{name = <<"open">>}]}. + % #xmlel{name = <<"enable">>, attrs = [{<<"xmlns">>, ?NS_DATA_VALIDATE}]}. + %% ----------------------------------------------------------------------- %% Text search tokenization %% ----------------------------------------------------------------------- diff --git a/src/mongoose_data_forms.erl b/src/mongoose_data_forms.erl index 5c83190ca90..6eb0ff5dd51 100644 --- a/src/mongoose_data_forms.erl +++ b/src/mongoose_data_forms.erl @@ -128,10 +128,12 @@ form_type_field(NS) when is_binary(NS) -> -spec form_field(field()) -> exml:element(). form_field(M) when is_map(M) -> + Children = maps:get(children, M, []), Values = [form_field_value(Value) || Value <- maps:get(values, M, [])], Options = [form_field_option(Option) || Option <- maps:get(options, M, [])], - Attrs = [{atom_to_binary(K), V} || {K, V} <- maps:to_list(M), K =/= values, K =/= options], - #xmlel{name = <<"field">>, attrs = Attrs, children = Values ++ Options}. + Attrs = [{atom_to_binary(K), V} + || {K, V} <- maps:to_list(M), K =/= values, K =/= options, K =/= children], + #xmlel{name = <<"field">>, attrs = Attrs, children = Values ++ Options ++ Children}. -spec form_title(binary()) -> exml:element(). form_title(Title) -> From 2b8e96f656ffd407e015bb8c3b106e94174c22cc Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Tue, 20 Feb 2024 10:47:22 +0100 Subject: [PATCH 5/7] Update MAM docs --- doc/modules/mod_mam.md | 4 ++-- src/mam/mod_mam.erl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/modules/mod_mam.md b/doc/modules/mod_mam.md index 0e616ee5711..2d23ed44286 100644 --- a/doc/modules/mod_mam.md +++ b/doc/modules/mod_mam.md @@ -5,7 +5,7 @@ It enables a service to store all user messages for one-to-one chats as well as It uses [XEP-0059: Result Set Management](http://xmpp.org/extensions/xep-0059.html) for paging. It is a highly customizable module, that requires some skill and knowledge to operate properly and efficiently. -MongooseIM is compatible with MAM 0.4-0.6. +MongooseIM is compatible with MAM 0.4-1.1.0. Configure MAM with different storage backends: @@ -75,7 +75,7 @@ Database backend to use. * **Default:** `false` * **Example:** `no_stanzaid_element = true` -Do not add a `` element from MAM v0.6. +Do not add a `` element from MAM v1.1.0. ### `modules.mod_mam.is_archivable_message` * **Syntax:** non-empty string diff --git a/src/mam/mod_mam.erl b/src/mam/mod_mam.erl index 33be3ef0341..0f5b0a15b8d 100644 --- a/src/mam/mod_mam.erl +++ b/src/mam/mod_mam.erl @@ -17,7 +17,7 @@ -module(mod_mam). -behaviour(gen_mod). -behaviour(mongoose_module_metrics). --xep([{xep, 313}, {version, "1.1.0"}, {status, partial}, {legacy_versions, ["0.5"]}]). +-xep([{xep, 313}, {version, "1.1.0"}, {legacy_versions, ["0.5"]}]). -xep([{xep, 424}, {version, "0.3.0"}]). -include("mod_mam.hrl"). From a1c80083017de6f737ff8317ca9b207c027f617a Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Mon, 26 Feb 2024 13:00:15 +0100 Subject: [PATCH 6/7] Change verification field in data forms --- big_tests/tests/mam_SUITE.erl | 19 +++---------------- big_tests/tests/mam_helper.erl | 1 - src/mam/mam_iq.erl | 2 +- src/mam/mod_mam_utils.erl | 23 ++++++----------------- src/mongoose_data_forms.erl | 24 +++++++++++++++++++----- 5 files changed, 29 insertions(+), 40 deletions(-) diff --git a/big_tests/tests/mam_SUITE.erl b/big_tests/tests/mam_SUITE.erl index 4788bd6be67..4172d9aa32d 100644 --- a/big_tests/tests/mam_SUITE.erl +++ b/big_tests/tests/mam_SUITE.erl @@ -1093,13 +1093,9 @@ 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) -> +end_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 -> destroy_room(Config), escalus:end_per_testcase(C, Config); end_per_testcase(C=muc_querying_for_all_messages, Config) -> @@ -1753,15 +1749,12 @@ server_returns_item_not_found_for_ids_filter_with_nonexistent_id(Config) -> 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), @@ -1775,8 +1768,6 @@ muc_query_messages_by_ids(Config) -> 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]), @@ -1784,7 +1775,6 @@ muc_simple_query_messages_by_ids(Config) -> 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), @@ -1800,8 +1790,6 @@ muc_simple_query_messages_by_ids(Config) -> 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]), @@ -1809,7 +1797,6 @@ muc_server_returns_item_not_found_for_ids_filter_with_nonexistent_id(Config) -> 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), diff --git a/big_tests/tests/mam_helper.erl b/big_tests/tests/mam_helper.erl index 83919384389..80ad2f51433 100644 --- a/big_tests/tests/mam_helper.erl +++ b/big_tests/tests/mam_helper.erl @@ -623,7 +623,6 @@ get_received_msgs_ids(Response) -> Parsed#forwarded_message.result_id end, Msgs). - %% @doc Result query iq. %% %% [{xmlel,<<"iq">>, diff --git a/src/mam/mam_iq.erl b/src/mam/mam_iq.erl index b0d79a6c0d7..ac9151c2ed6 100644 --- a/src/mam/mam_iq.erl +++ b/src/mam/mam_iq.erl @@ -38,7 +38,7 @@ with_jid => jid:jid() | undefined, %% Filtering by body text search_text => binary() | undefined, - %% Filtering Result Set based on message ids + %% Filtering Result Set before/after specific message ids borders => mod_mam:borders() | undefined, %% Filtering Result Set based on specific message ids message_ids => [mod_mam:message_id()] | undefined, diff --git a/src/mam/mod_mam_utils.erl b/src/mam/mod_mam_utils.erl index ab27b8a91a5..28e9188f0b2 100644 --- a/src/mam/mod_mam_utils.erl +++ b/src/mam/mod_mam_utils.erl @@ -430,20 +430,15 @@ lookup_specific_messages(HostType, Params, IDs, FetchFun) -> {0, []}, IDs), Result = determine_result(Params, FinalOffset, AccumulatedMessages), - case length(IDs) == length(AccumulatedMessages) of true -> Result; false -> {error, item_not_found} end. -determine_result(Params, Offset, Messages) -> - case maps:get(is_simple, Params, false) of - true -> - {ok, {undefined, undefined, Messages}}; - false -> - {ok, {length(Messages), Offset, Messages}} - end. - +determine_result(#{is_simple := true}, _Offset, Messages) -> + {ok, {undefined, undefined, Messages}}; +determine_result(#{}, Offset, Messages) -> + {ok, {length(Messages), Offset, Messages}}. tombstone(RetractionInfo = #{packet := Packet}, LocJid) -> Packet#xmlel{children = [retracted_element(RetractionInfo, LocJid)]}. @@ -796,7 +791,8 @@ message_form_fields(Mod, HostType, <<"urn:xmpp:mam:2">>) -> #{type => <<"text-single">>, var => <<"before-id">>}, #{type => <<"text-single">>, var => <<"after-id">>}, #{type => <<"boolean">>, var => <<"include-groupchat">>}, - #{type => <<"list-multi">>, var => <<"ids">>, children => [validate_element()]} | TextSearch]. + #{type => <<"list-multi">>, var => <<"ids">>, + validate => #{method => open, datatype => <<"xs:string">>}} | TextSearch]. -spec form_to_text(_) -> 'undefined' | binary(). form_to_text(#{<<"full-text-search">> := [Text]}) -> @@ -804,13 +800,6 @@ form_to_text(#{<<"full-text-search">> := [Text]}) -> form_to_text(#{}) -> undefined. --spec validate_element() -> exml:element(). -validate_element() -> - #xmlel{name = <<"validate">>, - attrs = [{<<"xmlns">>, ?NS_DATA_VALIDATE}, {<<"datatype">>, <<"xs:string">>}], - children = [#xmlel{name = <<"open">>}]}. - % #xmlel{name = <<"enable">>, attrs = [{<<"xmlns">>, ?NS_DATA_VALIDATE}]}. - %% ----------------------------------------------------------------------- %% Text search tokenization %% ----------------------------------------------------------------------- diff --git a/src/mongoose_data_forms.erl b/src/mongoose_data_forms.erl index 6eb0ff5dd51..0677aa6d160 100644 --- a/src/mongoose_data_forms.erl +++ b/src/mongoose_data_forms.erl @@ -16,13 +16,14 @@ -type form() :: #{type => binary(), title => binary(), instructions => binary(), ns => binary(), fields => [field()], reported => [field()], items => [[field()]]}. -type field() :: #{var => binary(), type => binary(), label => binary(), - values => [binary()], options => [option()]}. + values => [binary()], options => [option()], validate => validate()}. -type option() :: binary() | {binary(), binary()}. +-type validate() :: #{method => atom(), datatype => binary()}. -type parsed_form() :: #{type => binary(), ns => binary(), kvs := kv_map()}. -type kv_map() :: #{binary() => [binary()]}. --export_type([form/0, field/0, option/0, kv_map/0]). +-export_type([form/0, field/0, option/0, validate/0, kv_map/0]). -ignore_xref([is_form/1]). % exported for consistency, might be used later @@ -128,12 +129,12 @@ form_type_field(NS) when is_binary(NS) -> -spec form_field(field()) -> exml:element(). form_field(M) when is_map(M) -> - Children = maps:get(children, M, []), + Validate = form_field_validate(maps:get(validate, M, [])), Values = [form_field_value(Value) || Value <- maps:get(values, M, [])], Options = [form_field_option(Option) || Option <- maps:get(options, M, [])], Attrs = [{atom_to_binary(K), V} - || {K, V} <- maps:to_list(M), K =/= values, K =/= options, K =/= children], - #xmlel{name = <<"field">>, attrs = Attrs, children = Values ++ Options ++ Children}. + || {K, V} <- maps:to_list(M), K =/= values, K =/= options, K =/= validate], + #xmlel{name = <<"field">>, attrs = Attrs, children = Values ++ Options ++ Validate}. -spec form_title(binary()) -> exml:element(). form_title(Title) -> @@ -162,3 +163,16 @@ form_field_option(Option) -> -spec form_field_value(binary()) -> exml:element(). form_field_value(Value) -> #xmlel{name = <<"value">>, children = [#xmlcdata{content = Value}]}. + +-spec form_field_validate(validate()) -> [exml:element()] | []. +form_field_validate(#{method := Method, datatype := Datatype}) -> + [#xmlel{name = <<"validate">>, + attrs = [{<<"xmlns">>, ?NS_DATA_VALIDATE}, {<<"datatype">>, Datatype}], + children = form_field_validation_method(Method)}]; +form_field_validate(_) -> []. + +-spec form_field_validation_method(atom()) -> [exml:element()] | []. +form_field_validation_method(open) -> + [#xmlel{name = <<"open">>}]; +form_field_validation_method(_) -> + []. From db4ed10f214e4840c30c1a39f43c9339dbacbd18 Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Tue, 27 Feb 2024 12:12:15 +0100 Subject: [PATCH 7/7] Change type specs in form field validate --- src/mongoose_data_forms.erl | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/mongoose_data_forms.erl b/src/mongoose_data_forms.erl index 0677aa6d160..55a4edb8671 100644 --- a/src/mongoose_data_forms.erl +++ b/src/mongoose_data_forms.erl @@ -164,15 +164,13 @@ form_field_option(Option) -> form_field_value(Value) -> #xmlel{name = <<"value">>, children = [#xmlcdata{content = Value}]}. --spec form_field_validate(validate()) -> [exml:element()] | []. +-spec form_field_validate(validate()) -> [exml:element()]. form_field_validate(#{method := Method, datatype := Datatype}) -> [#xmlel{name = <<"validate">>, attrs = [{<<"xmlns">>, ?NS_DATA_VALIDATE}, {<<"datatype">>, Datatype}], children = form_field_validation_method(Method)}]; form_field_validate(_) -> []. --spec form_field_validation_method(atom()) -> [exml:element()] | []. +-spec form_field_validation_method(atom()) -> [exml:element()]. form_field_validation_method(open) -> - [#xmlel{name = <<"open">>}]; -form_field_validation_method(_) -> - []. + [#xmlel{name = <<"open">>}].