diff --git a/big_tests/tests/inbox_extensions_SUITE.erl b/big_tests/tests/inbox_extensions_SUITE.erl index 3a39e9d1556..af4c63f72e6 100644 --- a/big_tests/tests/inbox_extensions_SUITE.erl +++ b/big_tests/tests/inbox_extensions_SUITE.erl @@ -99,12 +99,16 @@ groups() -> properties_full_entry_can_be_get, properties_many_can_be_set, properties_many_can_be_set_queryid, - inbox_pagination_overrides_form, - inbox_can_paginate_forwards, - inbox_can_paginate_backwards, + timestamp_is_not_reset_with_setting_properties, + {group, pagination} + ]}, + {pagination, [parallel], [ + bad_id_throws_error, + pagination_overrides_form, + can_paginate_forwards, + can_paginate_backwards, max_queries_can_be_limited, - max_queries_can_fetch_ahead, - timestamp_is_not_reset_with_setting_properties + max_queries_can_fetch_ahead ]}, {muclight, [], [ groupchat_setunread_stanza_sets_inbox @@ -669,7 +673,18 @@ properties_many_can_be_set(Config, QueryId) -> end}], #{box => archive}) end). -inbox_pagination_overrides_form(Config) -> +bad_id_throws_error(Config) -> + escalus:fresh_story(Config, [{alice, 1}], fun(Alice) -> + % We set start and end to return Convs with Mike, but using RSM we override that + TS = erlang:system_time(millisecond), + AliceJid = escalus_utils:get_short_jid(Alice), + IdNotDividing = <<(integer_to_binary(TS))/binary, (base64:encode(AliceJid))/binary>>, + verify_returns_error(Alice, #{box => inbox, 'after' => IdNotDividing}), + BadInt = <<(integer_to_binary(-10))/binary, "/", (base64:encode(AliceJid))/binary>>, + verify_returns_error(Alice, #{box => inbox, 'after' => BadInt}) + end). + +pagination_overrides_form(Config) -> escalus:fresh_story(Config, [{alice, 1}, {bob, 1}, {kate, 1}, {mike, 1}], fun(Alice, Bob, Kate, Mike) -> % Several people write to Alice @@ -687,7 +702,7 @@ inbox_pagination_overrides_form(Config) -> inbox_helper:check_inbox(Alice, AliceConvs, Params) end). -inbox_can_paginate_forwards(Config) -> +can_paginate_forwards(Config) -> escalus:fresh_story(Config, [{alice, 1}, {bob, 1}, {kate, 1}, {mike, 1}], fun(Alice, Bob, Kate, Mike) -> % Several people write to Alice @@ -699,11 +714,11 @@ inbox_can_paginate_forwards(Config) -> #{respond_iq := Iq} = inbox_helper:check_inbox(Alice, [ConvWithMike], Params1), AfterPrevious = exml_query:path(Iq, [{element_with_ns, <<"fin">>, inbox_helper:inbox_ns()}, {element, <<"set">>}, {element, <<"last">>}, cdata]), - Params2 = #{box => inbox, 'after' => AfterPrevious}, + Params2 = #{box => inbox, 'after' => AfterPrevious, limit => 2}, inbox_helper:check_inbox(Alice, [ConvWithKate, ConvWithBob], Params2) end). -inbox_can_paginate_backwards(Config) -> +can_paginate_backwards(Config) -> escalus:fresh_story(Config, [{alice, 1}, {bob, 1}, {kate, 1}, {mike, 1}], fun(Alice, Bob, Kate, Mike) -> % Several people write to Alice @@ -955,3 +970,7 @@ muted_status(MutedOrUnmuted, Outer) -> Diff = erlang:convert_time_unit(MutedDiff, second, microsecond), ?assert(Now + Diff < GivenMutedUntil) end. + +verify_returns_error(User, Params) -> + Stanza = inbox_helper:make_inbox_stanza(Params), + assert_invalid_request(User, Stanza, <<"bad-request">>). diff --git a/doc/open-extensions/inbox.md b/doc/open-extensions/inbox.md index 2160482863f..3ba855ecdfb 100644 --- a/doc/open-extensions/inbox.md +++ b/doc/open-extensions/inbox.md @@ -148,6 +148,7 @@ It can happen that the amount of inbox entries is too big for a given user, even ``` where `Max` is a non-negative integer. +Inbox also has partial support for pagination as described in [XEP-0059](https://xmpp.org/extensions/xep-0059.html). If specifying `before` or `after`, the `start` and `end` form fields will be overridden. ## Properties of an entry Given an entry, certain properties are defined for such an entry: diff --git a/src/inbox/mod_inbox.erl b/src/inbox/mod_inbox.erl index e705f280883..b3f4dc09237 100644 --- a/src/inbox/mod_inbox.erl +++ b/src/inbox/mod_inbox.erl @@ -47,7 +47,8 @@ hidden_read => true | false, box => binary(), limit => undefined | pos_integer(), - rsm => jlib:rsm_in() + rsm => jlib:rsm_in(), + filter_on_jid => binary() }. -type count_res() :: ok | {ok, non_neg_integer()} | {error, term()}. @@ -209,11 +210,26 @@ process_iq(Acc, From, _To, #iq{type = set, sub_el = QueryEl} = IQ, _Extra) -> {Acc, Res} end. +-spec with_rsm([inbox_res()], get_inbox_params()) -> [inbox_res()]. +with_rsm(List, #{order := asc, start := TS, filter_on_jid := BinJid, rsm := #rsm_in{}}) -> + lists:reverse(drop_filter_on_jid(List, BinJid, TS, List)); with_rsm(List, #{order := asc, rsm := #rsm_in{}}) -> lists:reverse(List); +with_rsm(List, #{order := desc, 'end' := TS, filter_on_jid := BinJid}) -> + drop_filter_on_jid(List, BinJid, TS, List); with_rsm(List, _) -> List. +%% As IDs must be unique but timestamps are not, and SQL queries and orders by timestamp alone, +%% we query max+1 and then match to remove the entry that matches the ID given before. +-spec drop_filter_on_jid([inbox_res()], binary(), integer(), [inbox_res()]) -> [inbox_res()]. +drop_filter_on_jid(_List, BinJid, TS, [#{remote_jid := BinJid, timestamp := TS} | Rest]) -> + Rest; +drop_filter_on_jid(List, BinJid, TS, [_ | Rest]) -> + drop_filter_on_jid(List, BinJid, TS, Rest); +drop_filter_on_jid(List, _, _, []) -> + List. + -spec forward_messages(Acc :: mongoose_acc:t(), List :: [inbox_res()], QueryEl :: jlib:iq(), @@ -408,10 +424,10 @@ build_result_iq(List) -> -spec result_set([inbox_res()]) -> exml:element(). result_set([]) -> #xmlel{name = <<"set">>, attrs = [{<<"xmlns">>, ?NS_RSM}]}; -result_set([#{timestamp := First} | _] = List) -> - #{timestamp := Last} = lists:last(List), - BFirst = integer_to_binary(First), - BLast = integer_to_binary(Last), +result_set([#{remote_jid := FirstBinJid, timestamp := FirstTS} | _] = List) -> + #{remote_jid := LastBinJid, timestamp := LastTS} = lists:last(List), + BFirst = mod_inbox_utils:encode_rsm_id(FirstTS, FirstBinJid), + BLast = mod_inbox_utils:encode_rsm_id(LastTS, LastBinJid), mod_mam_utils:result_set(BFirst, BLast, undefined, undefined). %%%%%%%%%%%%%%%%%%% @@ -473,15 +489,26 @@ build_params_with_rsm(Params, #rsm_in{max = Max, id = <<>>, direction = before}) build_params_with_rsm(Params, #rsm_in{max = Max, id = <<>>, direction = aft}) -> Params#{limit => Max}; build_params_with_rsm(Params, #rsm_in{max = Max, id = Id, direction = Dir}) when is_binary(Id) -> - case {mod_inbox_utils:maybe_binary_to_positive_integer(Id), Dir} of - {{error, _}, _} -> {error, bad_request, <<"bad-request">>}; - {Stamp, aft} -> Params#{limit => Max, 'end' => Stamp - 1}; - {Stamp, undefined} -> Params#{limit => Max, 'end' => Stamp - 1}; - {Stamp, before} -> Params#{limit => Max, order => asc, start => Stamp + 1} + case {mod_inbox_utils:decode_rsm_id(Id), Dir} of + {error, _} -> + {error, bad_request, <<"bad-request">>}; + {{Stamp, Jid}, aft} -> + Params#{limit => expand_limit(Max), filter_on_jid => Jid, 'end' => Stamp}; + {{Stamp, Jid}, undefined} -> + Params#{limit => expand_limit(Max), filter_on_jid => Jid, 'end' => Stamp}; + {{Stamp, Jid}, before} -> + Params#{limit => expand_limit(Max), order => asc, filter_on_jid => Jid, start => Stamp} end; build_params_with_rsm(Params, _Rsm) -> Params. +-spec expand_limit(undefined) -> undefined; + (integer()) -> integer(). +expand_limit(undefined) -> + undefined; +expand_limit(Max) -> + Max + 1. + -spec form_to_params(mongooseim:host_type(), FormEl :: exml:element() | undefined) -> get_inbox_params() | {error, bad_request, Msg :: binary()}. form_to_params(_, undefined) -> diff --git a/src/inbox/mod_inbox_utils.erl b/src/inbox/mod_inbox_utils.erl index 534df68c6c2..cd0866d093c 100644 --- a/src/inbox/mod_inbox_utils.erl +++ b/src/inbox/mod_inbox_utils.erl @@ -31,6 +31,8 @@ get_option_remove_on_kicked/1, extract_attr_jid/1, maybe_binary_to_positive_integer/1, + encode_rsm_id/2, + decode_rsm_id/1, binary_to_bool/1, bool_to_binary/1, build_inbox_entry_key/2, @@ -197,6 +199,26 @@ maybe_binary_to_positive_integer(Bin) -> catch error:badarg -> {error, 'NaN'} end. +-spec encode_rsm_id(integer(), binary()) -> binary(). +encode_rsm_id(Int, BinJid) -> + BinInt = integer_to_binary(Int), + EncodedJid = base64:encode(BinJid), + <>. + +-spec decode_rsm_id(binary()) -> {integer(), binary()} | error. +decode_rsm_id(Bin) -> + case binary:split(Bin, <<"/">>) of + [BinInt, BinJid] -> + Int = maybe_binary_to_positive_integer(BinInt), + case Int of + Int when is_integer(Int) -> + Jid = base64:decode(BinJid), + {Int, Jid}; + _ -> error + end; + _ -> error + end. + -spec binary_to_bool(binary()) -> true | false | error. binary_to_bool(<<"true">>) -> true; binary_to_bool(<<"false">>) -> false;