Skip to content

Commit

Permalink
Merge pull request #3597 from esl/inbox/extend_queries
Browse files Browse the repository at this point in the history 
Inbox/extend queries
  • Loading branch information
@chrzaszcz
chrzaszcz authored Mar 18, 2022
2 parents b430804 + de8d0c9 commit 0aa6cea
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 96 deletions.
90 changes: 31 additions & 59 deletions big_tests/tests/inbox_extensions_SUITE.erl
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
-module(inbox_extensions_SUITE).
-compile([export_all, nowarn_export_all]).

-include_lib("escalus/include/escalus.hrl").
-include_lib("escalus/include/escalus_xmlns.hrl").
Expand All @@ -23,62 +24,6 @@
init_per_testcase/2,
end_per_testcase/2]).

%% ERRORS
-export([
% General
returns_error_when_no_jid_provided/1,
returns_error_when_invalid_jid_provided/1,
returns_error_when_valid_jid_but_no_property/1,
% Set-unread errors
returns_error_when_read_invalid_value/1,
returns_error_when_read_valid_request_but_not_in_inbox/1,
% Archiving errors
returns_error_when_archive_invalid_value/1,
returns_error_when_archive_valid_request_but_not_in_inbox/1,
% Muting errors
returns_error_when_mute_invalid_value/1,
returns_error_when_mute_invalid_seconds/1,
returns_error_when_mute_valid_request_but_not_in_inbox/1,
% Form errors
returns_error_when_archive_field_is_invalid/1,
returns_error_when_max_is_not_a_number/1
]).
%% SUCCESSES
-export([
% read
read_unread_entry_set_to_read/1,
read_unread_entry_set_to_read_iq_id_as_fallback/1,
read_unread_entry_set_to_read_queryid/1,
read_read_entry_set_to_unread/1,
read_unread_entry_with_two_messages_when_set_unread_then_unread_count_stays_in_two/1,
% archive
archive_active_entry_gets_archived/1,
archive_archived_entry_gets_active_on_request/1,
archive_archived_entry_gets_active_for_the_sender_on_new_message/1,
archive_archived_entry_gets_active_for_the_receiver_on_new_message/1,
archive_active_unread_entry_gets_archived_and_still_unread/1,
archive_full_archive_can_be_fetched/1,
archive_full_archive_can_be_fetched_queryid/1,
% mute
mute_unmuted_entry_gets_muted/1,
mute_muted_entry_gets_unmuted/1,
mute_after_timestamp_gets_unmuted/1,
mute_muted_conv_restarts_timestamp/1,
% other
returns_valid_properties_form/1,
properties_can_be_get/1,
properties_many_can_be_set/1,
properties_many_can_be_set_queryid/1,
max_queries_can_be_limited/1,
max_queries_can_fetch_ahead/1,
timestamp_is_not_reset_with_setting_properties/1
]).
%% Groupchats
-export([
groupchat_setunread_stanza_sets_inbox/1 % muclight
]).


all() ->
inbox_helper:skip_or_run_inbox_tests(tests()).

Expand Down Expand Up @@ -135,6 +80,7 @@ groups() ->
% other
returns_valid_properties_form,
properties_can_be_get,
properties_full_entry_can_be_get,
properties_many_can_be_set,
properties_many_can_be_set_queryid,
max_queries_can_be_limited,
Expand Down Expand Up @@ -495,6 +441,17 @@ properties_can_be_get(Config) ->
% Then Bob can just query the properties of this entry at will
query_properties(Bob, Alice, [{archive, false}, {read, true}, {mute, 0}])
end).

properties_full_entry_can_be_get(Config) ->
escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) ->
% Alice sends a message to Bob
Body = <<"Hi Bob">>,
inbox_helper:send_and_mark_msg(Alice, Bob, Body),
inbox_helper:check_inbox(Bob, [#conv{unread = 0, from = Alice, to = Bob, content = Body}]),
% Then Bob can just query the properties of this entry at will
query_properties(Bob, Alice, [{archive, false}, {read, true}, {mute, 0}], full_entry)
end).

properties_many_can_be_set(Config) ->
properties_many_can_be_set(Config, undefined).
properties_many_can_be_set_queryid(Config) ->
Expand Down Expand Up @@ -603,17 +560,32 @@ groupchat_setunread_stanza_sets_inbox(Config) ->

-spec query_properties(escalus:client(), escalus:client(), proplists:proplist()) -> [exml:element()].
query_properties(From, To, Expected) ->
Stanza = make_inbox_get_properties(To),
query_properties(From, To, Expected, none).

-spec query_properties(escalus:client(), escalus:client(), proplists:proplist(), none | full_entry) ->
[exml:element()].
query_properties(From, To, Expected, FullEntry) ->
Stanza = make_inbox_get_properties(To, FullEntry),
escalus:send(From, Stanza),
Result = escalus:wait_for_stanza(From),
?assert(escalus_pred:is_iq_result(Stanza, Result)),
[Props] = exml_query:subelements(Result, <<"query">>),
?assertEqual(inbox_helper:inbox_ns_conversation(), exml_query:attr(Props, <<"xmlns">>)),
maybe_assert_full_entry(Props, FullEntry),
lists:foreach(fun({Key, Val}) -> assert_property(Props, Key, Val) end, Expected).

-spec make_inbox_get_properties(escalus:client()) -> exml:element().
make_inbox_get_properties(To) ->
maybe_assert_full_entry(_, none) ->
ok;
maybe_assert_full_entry(Props, full_entry) ->
?assertNotEqual(undefined, exml_query:path(Props, [{element, <<"forwarded">>}])).

-spec make_inbox_get_properties(escalus:client(), boolean()) -> exml:element().
make_inbox_get_properties(To, none) ->
Query = escalus_stanza:query_el(inbox_helper:inbox_ns_conversation(), jid_attr(To), []),
escalus_stanza:iq(<<"get">>, [Query]);
make_inbox_get_properties(To, full_entry) ->
Attrs = [{<<"complete">>, <<"true">>} | jid_attr(To)],
Query = escalus_stanza:query_el(inbox_helper:inbox_ns_conversation(), Attrs, []),
escalus_stanza:iq(<<"get">>, [Query]).

-spec set_inbox_properties(escalus:client(), escalus:client(), proplists:proplist()) -> ok.
Expand Down
25 changes: 24 additions & 1 deletion doc/open-extensions/inbox.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,29 @@ To which the server will reply, just like before, with:
</iq>
```

If an entire entry wanted to be queried, and not only its attributes, a `complete='true'` can be provided:
```xml
<iq id='some_unique_id' type='get'>
<query xmlns='erlang-solutions.com:xmpp:inbox:0#conversation' jid='bob@localhost' complete='true'/>
</iq>
```
To which the server will reply, just like before, with:
```xml
<iq id='some_unique_id' to='alice@localhost/res1' type='result'>
<query xmlns='erlang-solutions.com:xmpp:inbox:0#conversation'>
<forwarded xmlns="urn:xmpp:forward:0">
<delay xmlns="urn:xmpp:delay" stamp="2018-07-10T23:08:25.123456Z"/>
<message xml:lang="en" type="chat" to="bob@localhost/res1" from="alice@localhost/res1" id=”123”>
<body>Hello</body>
</message>
</forwarded>
<archive>false</archive>
<mute>0</mute>
<read>true</read>
</query>
</iq>
```


## Setting properties
Setting properties is done using the standard XMPP pattern of `iq-query` and `iq-result`, as below:
Expand All @@ -217,7 +240,7 @@ where `Property` and `Value` are a list of key-value pairs as follows:
- `mute`: number of _seconds_ to mute for. Choose `0` for unmuting.
- `read` (adjective, not verb): `true` or `false`. Setting to true essentially sets the unread-count to `0`, `false` sets the unread-count to `1` (if it was equal to `0`, otherwise it lefts it unchanged). No other possibilities are offered, to reduce the risk of inconsistencies or problems induced by a faulty client.

*Note* that resetting the inbox count will not be forwarded. While a chat marker will be forwarded to the interlocutor(s), (including the case of a big groupchat with thousands of participants), this reset stanza will not.
*Note* that resetting the inbox count will not be forwarded. While a chat marker will be forwarded to the interlocutor(s), (including the case of a big groupchat with thousands of participants), this reset stanza will not.

If the query was successful, the server will answer with two stanzas, following the classic pattern of broadcasting state changes. First, it would send a message with a `<x>` children containing all new configuration, to the bare-jid of the user: this facilitates broadcasting to all online resources to successfully synchronise their interfaces.
```xml
Expand Down
13 changes: 1 addition & 12 deletions src/inbox/mod_inbox.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ build_inbox_message(#{msg := Content,

-spec build_result_el(exml:element(), id(), integer(), integer(), boolean(), integer(), integer()) -> exml:element().
build_result_el(Msg, QueryId, Count, Timestamp, Archive, MutedUntil, AccTS) ->
Forwarded = build_forward_el(Msg, Timestamp),
Forwarded = mod_inbox_utils:build_forward_el(Msg, Timestamp),
Properties = mod_inbox_entries:extensions_result(Archive, MutedUntil, AccTS),
QueryAttr = [{<<"queryid">>, QueryId} || QueryId =/= undefined, QueryId =/= <<>>],
#xmlel{name = <<"result">>,
Expand All @@ -338,17 +338,6 @@ build_result_iq(List) ->
build_result_el(Name, CountBin) ->
#xmlel{name = Name, children = [#xmlcdata{content = CountBin}]}.

-spec build_forward_el(exml:element(), integer()) -> exml:element().
build_forward_el(Content, Timestamp) ->
Delay = build_delay_el(Timestamp),
#xmlel{name = <<"forwarded">>, attrs = [{<<"xmlns">>, ?NS_FORWARD}],
children = [Delay, Content]}.

-spec build_delay_el(Timestamp :: integer()) -> exml:element().
build_delay_el(Timestamp) ->
TS = calendar:system_time_to_rfc3339(Timestamp, [{offset, "Z"}, {unit, microsecond}]),
jlib:timestamp_to_xml(TS, undefined, undefined).

%%%%%%%%%%%%%%%%%%%
%% iq-get
-spec build_inbox_form() -> exml:element().
Expand Down
26 changes: 21 additions & 5 deletions src/inbox/mod_inbox_backend.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
%% the backend modules (i.e. mod_inbox_rdbms).
-module(mod_inbox_backend).

-include("mod_inbox.hrl").

-export([init/2,
get_inbox/4,
clear_inbox/3,
Expand All @@ -10,6 +12,7 @@
remove_inbox_row/2,
set_inbox_incr_unread/5,
get_inbox_unread/2,
get_full_entry/2,
get_entry_properties/2,
set_entry_properties/3,
reset_unread/3]).
Expand Down Expand Up @@ -39,7 +42,7 @@
mod_inbox:write_res() when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Content :: binary(),
Content :: content(),
Count :: integer(),
MsgId :: binary(),
Timestamp :: integer().
Expand All @@ -52,7 +55,7 @@
mod_inbox:count_res() when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Content :: binary(),
Content :: content(),
MsgId :: binary(),
Timestamp :: integer().

Expand All @@ -65,6 +68,11 @@
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key().

-callback get_full_entry(HostType, InboxEntryKey) -> Ret when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Ret :: inbox_res() | nil().

-callback get_entry_properties(HostType, InboxEntryKey) -> Ret when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Expand Down Expand Up @@ -112,7 +120,7 @@ remove_domain(HostType, LServer) ->
mod_inbox:write_res() when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Content :: binary(),
Content :: content(),
Count :: integer(),
MsgId :: binary(),
Timestamp :: integer().
Expand All @@ -131,7 +139,7 @@ remove_inbox_row(HostType, InboxEntryKey) ->
mod_inbox:count_res() when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Content :: binary(),
Content :: content(),
MsgId :: binary(),
Timestamp :: integer().
set_inbox_incr_unread(HostType, InboxEntryKey, Content, MsgId, Timestamp) ->
Expand All @@ -153,6 +161,14 @@ get_inbox_unread(HostType, InboxEntryKey) ->
Args = [HostType, InboxEntryKey],
mongoose_backend:call_tracked(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

-spec get_full_entry(HostType, InboxEntryKey) -> Ret when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Ret :: inbox_res() | nil().
get_full_entry(HostType, InboxEntryKey) ->
Args = [HostType, InboxEntryKey],
mongoose_backend:call_tracked(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

-spec get_entry_properties(HostType, InboxEntryKey) -> Ret when
HostType :: mongooseim:host_type(),
InboxEntryKey :: mod_inbox:entry_key(),
Expand All @@ -173,4 +189,4 @@ set_entry_properties(HostType, InboxEntryKey, Params) ->
callback_funs() ->
[get_inbox, set_inbox, set_inbox_incr_unread,
reset_unread, remove_inbox_row, clear_inbox, get_inbox_unread,
get_entry_properties, set_entry_properties, remove_domain].
get_full_entry, get_entry_properties, set_entry_properties, remove_domain].
34 changes: 28 additions & 6 deletions src/inbox/mod_inbox_entries.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
-export([should_be_stored_in_inbox/1]).
-export([extensions_result/3]).

-type get_entry_type() :: full_entry | only_properties.
-export_type([get_entry_type/0]).

-spec process_iq_conversation(Acc :: mongoose_acc:t(),
From :: jid:jid(),
To :: jid:jid(),
Expand All @@ -33,7 +36,15 @@ process_iq_conversation_get(Acc, IQ, From, SubEl) ->
SubElWithForm = SubEl#xmlel{children = [Form]},
{Acc, IQ#iq{type = result, sub_el = SubElWithForm}};
EntryJID ->
get_properties_for_jid(Acc, IQ, From, EntryJID)
FullEntry = maybe_get_full_entry(SubEl),
get_properties_for_jid(Acc, IQ, From, EntryJID, FullEntry)
end.

-spec maybe_get_full_entry(exml:element()) -> get_entry_type().
maybe_get_full_entry(SubEl) ->
case exml_query:attr(SubEl, <<"complete">>) of
<<"true">> -> full_entry;
_ -> only_properties
end.

-spec build_inbox_entry_form() -> exml:element().
Expand All @@ -46,12 +57,17 @@ build_inbox_entry_form() ->
jlib:form_field({<<"read">>, <<"boolean">>, <<"false">>}),
jlib:form_field({<<"mute">>, <<"text-single">>, <<"0">>})]}.

-spec get_properties_for_jid(mongoose_acc:t(), jlib:iq(), jid:jid(), jid:jid()) ->
fetch_right_query(HostType, InboxEntryKey, only_properties) ->
mod_inbox_backend:get_entry_properties(HostType, InboxEntryKey);
fetch_right_query(HostType, InboxEntryKey, full_entry) ->
mod_inbox_backend:get_full_entry(HostType, InboxEntryKey).

-spec get_properties_for_jid(mongoose_acc:t(), jlib:iq(), jid:jid(), jid:jid(), get_entry_type()) ->
{mongoose_acc:t(), jlib:iq()}.
get_properties_for_jid(Acc, IQ, From, EntryJID) ->
get_properties_for_jid(Acc, IQ, From, EntryJID, QueryType) ->
HostType = mongoose_acc:host_type(Acc),
{_, _, BinEntryJID} = InboxEntryKey = mod_inbox_utils:build_inbox_entry_key(From, EntryJID),
case mod_inbox_backend:get_entry_properties(HostType, InboxEntryKey) of
case fetch_right_query(HostType, InboxEntryKey, QueryType) of
[] -> return_error(Acc, IQ, <<"Entry not found">>);
Result ->
CurrentTS = mongoose_acc:timestamp(Acc),
Expand Down Expand Up @@ -143,14 +159,20 @@ process_reset_stanza(Acc, From, IQ, _ResetStanza, InterlocutorJID) ->
%% Helpers
%%--------------------------------------------------------------------

-spec build_result(entry_properties(), integer()) -> [exml:element()].
build_result(#{archive := Archived, unread_count := UnreadCount, muted_until := MutedUntil}, CurrentTS) ->
-spec build_result(inbox_res() | entry_properties(), integer()) -> [exml:element()].
build_result(Entry = #{archive := Archived, unread_count := UnreadCount, muted_until := MutedUntil}, CurrentTS) ->
maybe_full_entry(Entry) ++
[
kv_to_el(<<"archive">>, mod_inbox_utils:bool_to_binary(Archived)),
kv_to_el(<<"read">>, mod_inbox_utils:bool_to_binary(0 =:= UnreadCount)),
kv_to_el(<<"mute">>, mod_inbox_utils:maybe_muted_until(MutedUntil, CurrentTS))
].

maybe_full_entry(#{msg := Msg, timestamp := Timestamp}) ->
[ mod_inbox_utils:build_forward_el(Msg, Timestamp) ];
maybe_full_entry(_) ->
[].

-spec kv_to_el(binary(), binary()) -> exml:element().
kv_to_el(Key, Value) ->
#xmlel{name = Key, children = [#xmlcdata{content = Value}]}.
Expand Down
Loading

0 comments on commit 0aa6cea

Please sign in to comment.