From 2fcdacddc03d88c4b2900cf7d880647069a9cf1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 21 Oct 2022 14:43:16 +0200 Subject: [PATCH 1/7] Rework mongoose_stanza_api - Move functions from mongoose_stanza_helper, because there is no need for two separate modules here. - Perform a fold over individual steps to organize error handling. The 'fold' function can be later reused, and moved e.g. to mongoose_api_common. - Handle more error cases, that were caught only by graphql-specific code. Now the REST api handles them as well. - Allow stanza without 'from' when 'user' is known. 'from' is set to the user JID later in the routing process anyway. - Use Stanza ID not only for REST. MAM ID was used for GraphQL. We can add MAM ID as a separate GraphQL field when needed. - Set Stanza ID if the stanza does not already have one - this is similar to the behaviour for messages. - Do not treat 0 and <<>> values as undefined - this is a bit unexpected, and now in the unified API the user can always omit them when needed. - Set correct max_result_limit - previously it was set to 1. --- src/mongoose_stanza_api.erl | 200 ++++++++++++++++++++++++++++----- src/mongoose_stanza_helper.erl | 82 -------------- 2 files changed, 174 insertions(+), 108 deletions(-) delete mode 100644 src/mongoose_stanza_helper.erl diff --git a/src/mongoose_stanza_api.erl b/src/mongoose_stanza_api.erl index fa3f09108d2..03adb04843b 100644 --- a/src/mongoose_stanza_api.erl +++ b/src/mongoose_stanza_api.erl @@ -1,39 +1,187 @@ -module(mongoose_stanza_api). --export([lookup_recent_messages/4]). +-export([send_chat_message/4, send_headline_message/5, send_stanza/2, lookup_recent_messages/5]). -include("jlib.hrl"). -include("mongoose_rsm.hrl"). +-include("mongoose_logger.hrl"). -%% TODO fix error handling, do not crash for non-existing users -%% Before is in microseconds --spec lookup_recent_messages( - ArcJID :: jid:jid(), - With :: jid:jid() | undefined, - Before :: mod_mam:unix_timestamp() | undefined, - Limit :: non_neg_integer()) -> - [mod_mam:message_row()]. -lookup_recent_messages(_, _, _, Limit) when Limit > 500 -> - throw({error, message_limit_too_high}); -lookup_recent_messages(ArcJID, WithJID, Before, Limit) -> - #jid{luser = LUser, lserver = LServer} = ArcJID, - {ok, HostType} = mongoose_domain_api:get_domain_host_type(LServer), - EndTS = case Before of - 0 -> undefined; - _ -> Before - end, +%% API + +-spec send_chat_message(User :: jid:jid() | undefined, From :: jid:jid() | undefined, + To :: jid:jid(), Body :: binary()) -> + {unknown_user | invalid_sender, iodata()} | {ok, map()}. +send_chat_message(User, From, To, Body) -> + M = #{user => User, from => From, to => To, body => Body}, + fold(M, [fun check_sender/1, fun get_host_type/1, fun check_user/1, + fun prepare_chat_message/1, fun send/1]). + +-spec send_headline_message(User :: jid:jid() | undefined, From :: jid:jid() | undefined, + To :: jid:jid(), Body :: binary() | undefined, + Subject :: binary() | undefined) -> + {unknown_user | invalid_sender, iodata()} | {ok, map()}. +send_headline_message(User, From, To, Body, Subject) -> + M = #{user => User, from => From, to => To, body => Body, subject => Subject}, + fold(M, [fun check_sender/1, fun get_host_type/1, fun check_user/1, + fun prepare_headline_message/1, fun send/1]). + +-spec send_stanza(User :: jid:jid() | undefined, exml:element()) -> + {unknown_user | invalid_sender | no_sender | + invalid_recipient | no_recipient, iodata()} | {ok, map()}. +send_stanza(User, Stanza) -> + M = #{user => User, stanza => Stanza}, + fold(M, [fun extract_from_jid/1, fun extract_to_jid/1, fun check_sender/1, + fun get_host_type/1, fun check_user/1, fun prepare_stanza/1, fun send/1]). + +-spec lookup_recent_messages(User :: jid:jid(), With :: jid:jid() | undefined, + Before :: mod_mam:unix_timestamp() | undefined, % microseconds + Limit :: non_neg_integer() | undefined, CheckUser :: boolean()) -> + {unknown_user, iodata()} | {ok, {[mod_mam:message_row()], non_neg_integer()}}. +lookup_recent_messages(User, With, Before, Limit, CheckUser) -> + M = #{user => User, with => With, before => Before, limit => Limit, check_user => CheckUser}, + fold(M, [fun get_host_type/1, fun check_user/1, fun lookup_messages/1]). + +%% Steps + +%% @doc Determine the user's bare JID and the 'from' JID, and check if they match +check_sender(M = #{user := User = #jid{}, from := From = #jid{}}) -> + case jid:are_bare_equal(User, From) of + true -> + M#{check_user => false}; + false -> + {invalid_sender, <<"Sender's JID is different from the user's JID">>} + end; +check_sender(M = #{from := From = #jid{}}) -> + M#{user => jid:to_bare(From), check_user => true}; +check_sender(M = #{user := User = #jid{}}) -> + M#{from => User, check_user => false}; +check_sender(#{}) -> + {no_sender, <<"Missing sender JID">>}. + +extract_from_jid(M = #{stanza := Stanza}) -> + case exml_query:attr(Stanza, <<"from">>) of + undefined -> + M; + JidBin -> + case jid:from_binary(JidBin) of + error -> {invalid_sender, <<"Invalid sender JID">>}; + Jid -> M#{from => Jid} + end + end. + +extract_to_jid(M = #{stanza := Stanza}) -> + case exml_query:attr(Stanza, <<"to">>) of + undefined -> + {no_recipient, <<"Missing recipient JID">>}; + JidBin -> + case jid:from_binary(JidBin) of + error -> {invalid_recipient, <<"Invalid recipient JID">>}; + Jid -> M#{to => Jid} + end + end. + +prepare_chat_message(M = #{from := From, to := To, body := Body}) -> + FromBin = jid:to_binary(From), + ToBin = jid:to_binary(To), + M#{stanza => build_chat_message(FromBin, ToBin, Body)}. + +prepare_headline_message(M = #{from := From, to := To, body := Body, subject := Subject}) -> + FromBin = jid:to_binary(From), + ToBin = jid:to_binary(To), + M#{stanza => build_headline_message(FromBin, ToBin, Body, Subject)}. + +prepare_stanza(M = #{stanza := Stanza}) -> + M#{stanza := ensure_id(Stanza)}. + +get_host_type(M = #{user := #jid{lserver = LServer}}) -> + case mongoose_domain_api:get_domain_host_type(LServer) of + {ok, HostType} -> + M#{host_type => HostType}; + {error, not_found} -> + {unknown_user, <<"User's domain does not exist">>} + end. + +check_user(M = #{check_user := false}) -> + M; +check_user(M = #{check_user := true, user := UserJid, host_type := HostType}) -> + case ejabberd_auth:does_user_exist(HostType, UserJid, stored) of + true -> + M; + false -> + {unknown_user, <<"User does not exist">>} + end. + +send(#{host_type := HostType, from := From, to := To, stanza := Stanza}) -> + Acc = mongoose_acc:new(#{from_jid => From, + to_jid => To, + location => ?LOCATION, + host_type => HostType, + lserver => From#jid.lserver, + element => Stanza}), + Acc1 = mongoose_hooks:user_send_packet(Acc, From, To, Stanza), + ejabberd_router:route(From, To, Acc1), + {ok, #{<<"id">> => get_id(Stanza)}}. + +lookup_messages(#{user := UserJid, with := WithJid, before := Before, limit := Limit, + host_type := HostType}) -> + #jid{luser = LUser, lserver = LServer} = UserJid, + MaxResultLimit = mod_mam_params:max_result_limit(mod_mam_pm, HostType), + Limit2 = min(Limit, MaxResultLimit), Params = #{archive_id => mod_mam_pm:archive_id(LServer, LUser), - owner_jid => ArcJID, + owner_jid => UserJid, borders => undefined, rsm => #rsm_in{direction = before, id = undefined}, % last msgs start_ts => undefined, - end_ts => EndTS, + end_ts => Before, now => os:system_time(microsecond), - with_jid => WithJID, + with_jid => WithJid, search_text => undefined, - page_size => Limit, + page_size => Limit2, limit_passed => false, - max_result_limit => 1, + max_result_limit => MaxResultLimit, is_simple => true}, - R = mod_mam_pm:lookup_messages(HostType, Params), - {ok, {_, _, L}} = R, - L. + {ok, {_, _, Rows}} = mod_mam_pm:lookup_messages(HostType, Params), + {ok, {Rows, Limit2}}. + +%% Helpers + +-spec build_chat_message(jid:literal_jid(), jid:literal_jid(), binary()) -> exml:element(). +build_chat_message(From, To, Body) -> + #xmlel{name = <<"message">>, + attrs = add_id([{<<"type">>, <<"chat">>}, {<<"from">>, From}, {<<"to">>, To}]), + children = [#xmlel{name = <<"body">>, + children = [#xmlcdata{content = Body}]}] + }. + +-spec build_headline_message(jid:literal_jid(), jid:literal_jid(), + binary() | undefined, binary() | undefined) -> exml:element(). +build_headline_message(From, To, Body, Subject) -> + Children = maybe_cdata_elem(<<"subject">>, Subject) ++ + maybe_cdata_elem(<<"body">>, Body), + Attrs = add_id([{<<"type">>, <<"headline">>}, {<<"from">>, From}, {<<"to">>, To}]), + #xmlel{name = <<"message">>, attrs = Attrs, children = Children}. + +-spec ensure_id(exml:element()) -> exml:element(). +ensure_id(Stanza) -> + case get_id(Stanza) of + undefined -> Stanza#xmlel{attrs = add_id(Stanza#xmlel.attrs)}; + _ -> Stanza + end. + +maybe_cdata_elem(_, undefined) -> []; +maybe_cdata_elem(Name, Text) when is_binary(Text) -> + [cdata_elem(Name, Text)]. + +cdata_elem(Name, Text) when is_binary(Name), is_binary(Text) -> + #xmlel{name = Name, children = [#xmlcdata{content = Text}]}. + +add_id(Attrs) -> + [{<<"id">>, mongoose_bin:gen_from_crypto()} | Attrs]. + +-spec get_id(exml:element()) -> binary() | undefined. +get_id(Stanza) -> + exml_query:attr(Stanza, <<"id">>, undefined). + +fold({_, _} = Result, _) -> + Result; +fold(M, [Step | Rest]) when is_map(M) -> + fold(Step(M), Rest). diff --git a/src/mongoose_stanza_helper.erl b/src/mongoose_stanza_helper.erl deleted file mode 100644 index ea1025c4f1c..00000000000 --- a/src/mongoose_stanza_helper.erl +++ /dev/null @@ -1,82 +0,0 @@ --module(mongoose_stanza_helper). --export([build_message/3]). --export([build_message_with_headline/3]). --export([get_last_messages/4]). --export([route/4]). - --include("jlib.hrl"). --include("mongoose_logger.hrl"). - --spec build_message(From :: binary(), To :: binary(), Body :: binary()) -> exml:element(). -build_message(From, To, Body) -> - #xmlel{name = <<"message">>, - attrs = [{<<"type">>, <<"chat">>}, - {<<"id">>, mongoose_bin:gen_from_crypto()}, - {<<"from">>, From}, - {<<"to">>, To}], - children = [#xmlel{name = <<"body">>, - children = [#xmlcdata{content = Body}]}] - }. - -build_message_with_headline(FromBin, ToBin, - #{<<"body">> := Body, <<"subject">> := Subject}) -> - Children = maybe_cdata_elem(<<"subject">>, Subject) ++ - maybe_cdata_elem(<<"body">>, Body), - Attrs = [{<<"type">>, <<"headline">>}, - {<<"id">>, mongoose_bin:gen_from_crypto()}, - {<<"from">>, FromBin}, - {<<"to">>, ToBin}], - #xmlel{name = <<"message">>, attrs = Attrs, children = Children}. - -maybe_cdata_elem(_, null) -> []; -maybe_cdata_elem(_, <<>>) -> []; -maybe_cdata_elem(Name, Text) when is_binary(Text) -> - [cdata_elem(Name, Text)]. - -cdata_elem(Name, Text) when is_binary(Name), is_binary(Text) -> - #xmlel{name = Name, children = [#xmlcdata{content = Text}]}. - --spec get_last_messages(Caller :: jid:jid(), - Limit :: non_neg_integer(), - With :: null | jid:jid(), - Before :: null | mod_mam:unix_timestamp()) -> {ok, map()}. -get_last_messages(Caller, Limit, With, Before) -> - With2 = null_as_undefined(With), - Before2 = null_as_undefined(Before), %% Before is in microseconds - Limit2 = min(500, Limit), - Rows = mongoose_stanza_api:lookup_recent_messages(Caller, With2, Before2, Limit2), - Maps = lists:map(fun row_to_map/1, Rows), - {ok, #{<<"stanzas">> => Maps, <<"limit">> => Limit2}}. - -null_as_undefined(null) -> undefined; -null_as_undefined(Value) -> Value. - --spec row_to_map(mod_mam:message_row()) -> {ok, map()}. -row_to_map(#{id := Id, jid := From, packet := Msg}) -> - {Microseconds, _} = mod_mam_utils:decode_compact_uuid(Id), - StanzaID = mod_mam_utils:mess_id_to_external_binary(Id), - Map = #{<<"sender">> => From, <<"timestamp">> => Microseconds, - <<"stanza_id">> => StanzaID, <<"stanza">> => Msg}, - {ok, Map}. - --spec route(From :: jid:jid(), To :: jid:jid(), - Packet :: exml:element(), SkipAuth :: boolean()) -> - {ok, map()} | {error, term()}. -route(From = #jid{lserver = LServer}, To, Packet, SkipAuth) -> - case mongoose_graphql_helper:check_user(From, SkipAuth) of - {ok, HostType} -> - do_route(HostType, LServer, From, To, Packet); - Error -> - Error - end. - -do_route(HostType, LServer, From, To, Packet) -> - %% Based on mod_commands:do_send_packet/3 - Acc = mongoose_acc:new(#{location => ?LOCATION, - host_type => HostType, - lserver => LServer, - element => Packet}), - Acc1 = mongoose_hooks:user_send_packet(Acc, From, To, Packet), - Acc2 = ejabberd_router:route(From, To, Acc1), - MessID = mod_mam_utils:get_mam_id_ext(Acc2), - {ok, #{ <<"id">> => MessID }}. From 73f777f2c9b4cb3290445e704b4b56d0801fada7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 21 Oct 2022 14:51:16 +0200 Subject: [PATCH 2/7] Update graphql stanza handlers with the new mongoose_stanza_api - Call the new API, which makes it possible to ged rid of some checks. - Remove unused helper functions. - Make format_error/2 more flexible. --- ...mongoose_graphql_stanza_admin_mutation.erl | 23 ++++---- .../mongoose_graphql_stanza_admin_query.erl | 11 ++-- src/graphql/mongoose_graphql_helper.erl | 24 +-------- .../mongoose_graphql_stanza_helper.erl | 20 ++++++- .../mongoose_graphql_stanza_user_mutation.erl | 52 +++++-------------- .../mongoose_graphql_stanza_user_query.erl | 6 ++- 6 files changed, 51 insertions(+), 85 deletions(-) diff --git a/src/graphql/admin/mongoose_graphql_stanza_admin_mutation.erl b/src/graphql/admin/mongoose_graphql_stanza_admin_mutation.erl index 44d30afe2f8..050ba09a374 100644 --- a/src/graphql/admin/mongoose_graphql_stanza_admin_mutation.erl +++ b/src/graphql/admin/mongoose_graphql_stanza_admin_mutation.erl @@ -6,8 +6,8 @@ -ignore_xref([execute/4]). -include("../mongoose_graphql_types.hrl"). --include("mongoose_logger.hrl"). --include("jlib.hrl"). + +-import(mongoose_graphql_helper, [null_to_undefined/1, format_result/2]). execute(_Ctx, _Obj, <<"sendMessage">>, Args) -> send_message(Args); @@ -17,16 +17,15 @@ execute(_Ctx, _Obj, <<"sendStanza">>, Args) -> send_stanza(Args). send_message(#{<<"from">> := From, <<"to">> := To, <<"body">> := Body}) -> - Packet = mongoose_stanza_helper:build_message( - jid:to_binary(From), jid:to_binary(To), Body), - mongoose_stanza_helper:route(From, To, Packet, true). + Res = mongoose_stanza_api:send_chat_message(undefined, From, To, Body), + format_result(Res, #{from => jid:to_binary(From)}). -send_message_headline(Args = #{<<"from">> := From, <<"to">> := To}) -> - Packet = mongoose_stanza_helper:build_message_with_headline( - jid:to_binary(From), jid:to_binary(To), Args), - mongoose_stanza_helper:route(From, To, Packet, true). +send_message_headline(#{<<"from">> := From, <<"to">> := To, + <<"body">> := Body, <<"subject">> := Subject}) -> + Res = mongoose_stanza_api:send_headline_message( + undefined, From, To, null_to_undefined(Body), null_to_undefined(Subject)), + format_result(Res, #{from => jid:to_binary(From)}). send_stanza(#{<<"stanza">> := Packet}) -> - From = jid:from_binary(exml_query:attr(Packet, <<"from">>)), - To = jid:from_binary(exml_query:attr(Packet, <<"to">>)), - mongoose_stanza_helper:route(From, To, Packet, true). + Res = mongoose_stanza_api:send_stanza(undefined, Packet), + format_result(Res, #{}). diff --git a/src/graphql/admin/mongoose_graphql_stanza_admin_query.erl b/src/graphql/admin/mongoose_graphql_stanza_admin_query.erl index 2a63292663e..26916591785 100644 --- a/src/graphql/admin/mongoose_graphql_stanza_admin_query.erl +++ b/src/graphql/admin/mongoose_graphql_stanza_admin_query.erl @@ -1,12 +1,13 @@ -module(mongoose_graphql_stanza_admin_query). -behaviour(mongoose_graphql). +-import(mongoose_graphql_helper, [format_result/2]). + -export([execute/4]). -ignore_xref([execute/4]). -include("../mongoose_graphql_types.hrl"). --include("mongoose_logger.hrl"). execute(_Ctx, _Obj, <<"getLastMessages">>, Args) -> get_last_messages(Args). @@ -14,9 +15,5 @@ execute(_Ctx, _Obj, <<"getLastMessages">>, Args) -> get_last_messages(#{<<"caller">> := Caller, <<"limit">> := Limit, <<"with">> := With, <<"before">> := Before}) when is_integer(Limit) -> - case mongoose_graphql_helper:check_user(Caller, true) of - {ok, _HostType} -> - mongoose_stanza_helper:get_last_messages(Caller, Limit, With, Before); - Error -> - Error - end. + Res = mongoose_graphql_stanza_helper:get_last_messages(Caller, Limit, With, Before, true), + format_result(Res, #{user => jid:to_binary(Caller)}). diff --git a/src/graphql/mongoose_graphql_helper.erl b/src/graphql/mongoose_graphql_helper.erl index bd446473fd3..1e6988f36b6 100644 --- a/src/graphql/mongoose_graphql_helper.erl +++ b/src/graphql/mongoose_graphql_helper.erl @@ -1,12 +1,9 @@ -module(mongoose_graphql_helper). --export([check_user/2]). - -export([null_to_default/2, null_to_undefined/1]). -export([format_result/2, make_error/2, make_error/3]). --include("jlib.hrl"). -include("mongoose_graphql_types.hrl"). -spec format_result(InResult, Context) -> OutResult when @@ -15,7 +12,7 @@ OutResult :: {ok, binary() | integer()} | {error, resolver_error()}. format_result(Result, Context) -> case Result of - {ok, Val} when is_integer(Val) -> {ok, Val}; + {ok, Val} when is_integer(Val) orelse is_map(Val) -> {ok, Val}; {ok, Msg} -> {ok, iolist_to_binary(Msg)}; {ErrCode, Msg} -> make_error(ErrCode, Msg, Context) end. @@ -28,25 +25,6 @@ make_error({Reason, Msg}, Context) -> make_error(Reason, Msg, Context) -> {error, #resolver_error{reason = Reason, msg = iolist_to_binary(Msg), context = Context}}. --spec check_user(jid:jid(), boolean()) -> {ok, mongooseim:host_type()} | {error, term()}. -check_user(#jid{lserver = LServer} = Jid, CheckAuth) -> - case mongoose_domain_api:get_domain_host_type(LServer) of - {ok, HostType} -> - check_auth(HostType, Jid, CheckAuth); - _ -> - {error, #{what => unknown_domain, domain => LServer}} - end. - -check_auth(HostType, _Jid, false) -> - {ok, HostType}; -check_auth(HostType, Jid, true) -> - case ejabberd_auth:does_user_exist(HostType, Jid, stored) of - true -> - {ok, HostType}; - false -> - {error, #{what => unknown_user, jid => jid:to_binary(Jid)}} - end. - null_to_default(null, Default) -> Default; null_to_default(Value, _Default) -> diff --git a/src/graphql/mongoose_graphql_stanza_helper.erl b/src/graphql/mongoose_graphql_stanza_helper.erl index 438dedcd058..44ff40be168 100644 --- a/src/graphql/mongoose_graphql_stanza_helper.erl +++ b/src/graphql/mongoose_graphql_stanza_helper.erl @@ -1,6 +1,24 @@ -module(mongoose_graphql_stanza_helper). --export([row_to_map/1]). +-import(mongoose_graphql_helper, [null_to_undefined/1, make_error/2]). + +-export([get_last_messages/5, row_to_map/1]). + +-spec get_last_messages(Caller :: jid:jid(), + Limit :: null | non_neg_integer(), + With :: null | jid:jid(), + Before :: null | mod_mam:unix_timestamp(), boolean()) -> + {ok, map()} | {unknown_user, iodata()}. +get_last_messages(Caller, Limit, With, Before, CheckUser) -> + case mongoose_stanza_api:lookup_recent_messages( + Caller, null_to_undefined(With), null_to_undefined(Before), + null_to_undefined(Limit), CheckUser) of + {ok, {Rows, Limit2}} -> + Maps = lists:map(fun row_to_map/1, Rows), + {ok, #{<<"stanzas">> => Maps, <<"limit">> => Limit2}}; + Error -> + Error + end. -spec row_to_map(mod_mam:message_row()) -> {ok, map()}. row_to_map(#{id := Id, jid := From, packet := Msg}) -> diff --git a/src/graphql/user/mongoose_graphql_stanza_user_mutation.erl b/src/graphql/user/mongoose_graphql_stanza_user_mutation.erl index 74972ed6140..ae4a0c30b8a 100644 --- a/src/graphql/user/mongoose_graphql_stanza_user_mutation.erl +++ b/src/graphql/user/mongoose_graphql_stanza_user_mutation.erl @@ -6,8 +6,8 @@ -ignore_xref([execute/4]). -include("../mongoose_graphql_types.hrl"). --include("mongoose_logger.hrl"). --include("jlib.hrl"). + +-import(mongoose_graphql_helper, [null_to_undefined/1, format_result/2]). execute(Ctx, _Obj, <<"sendMessage">>, Args) -> send_message(Ctx, Args); @@ -16,44 +16,16 @@ execute(Ctx, _Obj, <<"sendMessageHeadLine">>, Args) -> execute(Ctx, _Obj, <<"sendStanza">>, Args) -> send_stanza(Ctx, Args). -send_message(Ctx, Args) -> - with_from(Ctx, Args, fun send_message2/1). - -send_message_headline(Ctx, Args) -> - with_from(Ctx, Args, fun send_message_headline2/1). - -send_message2(#{<<"from">> := From, <<"to">> := To, <<"body">> := Body}) -> - Packet = mongoose_stanza_helper:build_message(jid:to_binary(From), jid:to_binary(To), Body), - %% SkipAuth = false, because we already checked if From exists - mongoose_stanza_helper:route(From, To, Packet, false). +send_message(#{user := User}, #{<<"from">> := From, <<"to">> := To, <<"body">> := Body}) -> + Res = mongoose_stanza_api:send_chat_message(User, null_to_undefined(From), To, Body), + format_result(Res, #{}). -send_message_headline2(Args = #{<<"from">> := From, <<"to">> := To}) -> - Packet = mongoose_stanza_helper:build_message_with_headline( - jid:to_binary(From), jid:to_binary(To), Args), - mongoose_stanza_helper:route(From, To, Packet, false). +send_message_headline(#{user := User}, #{<<"from">> := From, <<"to">> := To, <<"body">> := Body, + <<"subject">> := Subject}) -> + Res = mongoose_stanza_api:send_headline_message( + User, null_to_undefined(From), To, null_to_undefined(Body), null_to_undefined(Subject)), + format_result(Res, #{}). send_stanza(#{user := User}, #{<<"stanza">> := Packet}) -> - From = jid:from_binary(exml_query:attr(Packet, <<"from">>)), - To = jid:from_binary(exml_query:attr(Packet, <<"to">>)), - case jid:are_bare_equal(User, From) of - true -> - mongoose_stanza_helper:route(From, To, Packet, false); - false -> - {error, #{what => bad_from_jid}} - end. - -with_from(_Ctx = #{user := User}, Args, Next) -> - case maps:get(<<"from">>, Args, null) of - null -> - Next(Args#{<<"from">> => User}); - From -> - case jid:are_bare_equal(User, From) of - true -> - %% We still can allow a custom resource - Next(Args#{<<"from">> => From}); - false -> - ?LOG_ERROR(#{what => bad_from_jid, - user_jid => User, from_jid => From}), - {error, #{what => bad_from_jid}} - end - end. + Res = mongoose_stanza_api:send_stanza(User, Packet), + format_result(Res, #{}). diff --git a/src/graphql/user/mongoose_graphql_stanza_user_query.erl b/src/graphql/user/mongoose_graphql_stanza_user_query.erl index 50c19ca4edd..c11873a3592 100644 --- a/src/graphql/user/mongoose_graphql_stanza_user_query.erl +++ b/src/graphql/user/mongoose_graphql_stanza_user_query.erl @@ -1,12 +1,13 @@ -module(mongoose_graphql_stanza_user_query). -behaviour(mongoose_graphql). +-import(mongoose_graphql_helper, [format_result/2]). + -export([execute/4]). -ignore_xref([execute/4]). -include("../mongoose_graphql_types.hrl"). --include("mongoose_logger.hrl"). execute(#{user := User}, _Obj, <<"getLastMessages">>, Args) -> get_last_messages(Args, User). @@ -14,4 +15,5 @@ execute(#{user := User}, _Obj, <<"getLastMessages">>, Args) -> get_last_messages(#{<<"limit">> := Limit, <<"with">> := With, <<"before">> := Before}, Caller) when is_integer(Limit) -> - mongoose_stanza_helper:get_last_messages(Caller, Limit, With, Before). + Res = mongoose_graphql_stanza_helper:get_last_messages(Caller, Limit, With, Before, false), + format_result(Res, #{user => jid:to_binary(Caller)}). From cf7d0b86f13fa7594faa9e9793cfa081b5e82ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 21 Oct 2022 14:53:31 +0200 Subject: [PATCH 3/7] Update REST API for stanzas with the new mongoose_stanza_api - The new API makes it possible to avoid some checks --- .../mongoose_admin_api_messages.erl | 26 ++++++------- .../mongoose_admin_api_stanzas.erl | 37 +++---------------- .../mongoose_client_api_messages.erl | 14 +++---- 3 files changed, 25 insertions(+), 52 deletions(-) diff --git a/src/mongoose_admin_api/mongoose_admin_api_messages.erl b/src/mongoose_admin_api/mongoose_admin_api_messages.erl index 3455f78cd96..729b8d98629 100644 --- a/src/mongoose_admin_api/mongoose_admin_api_messages.erl +++ b/src/mongoose_admin_api/mongoose_admin_api_messages.erl @@ -69,24 +69,24 @@ handle_get(Req, State) -> Args = parse_qs(Req), Limit = get_limit(Args), Before = get_before(Args), - Rows = mongoose_stanza_api:lookup_recent_messages(OwnerJid, WithJid, Before, Limit), - Messages = lists:map(fun row_to_map/1, Rows), - {jiffy:encode(Messages), Req, State}. + case mongoose_stanza_api:lookup_recent_messages(OwnerJid, WithJid, Before, Limit, true) of + {ok, {Rows, _Limit}} -> + Messages = lists:map(fun row_to_map/1, Rows), + {jiffy:encode(Messages), Req, State}; + {unknown_user, Msg} -> + throw_error(bad_request, Msg) + end. handle_post(Req, State) -> Args = parse_body(Req), From = get_caller(Args), To = get_to(Args), Body = get_body(Args), - Packet = mongoose_stanza_helper:build_message( - jid:to_binary(From), jid:to_binary(To), Body), - case mongoose_stanza_helper:route(From, To, Packet, true) of - {error, #{what := unknown_domain}} -> - throw_error(bad_request, <<"Unknown domain">>); - {error, #{what := unknown_user}} -> - throw_error(bad_request, <<"Unknown user">>); + case mongoose_stanza_api:send_chat_message(undefined, From, To, Body) of {ok, _} -> - {true, Req, State} + {true, Req, State}; + {_Error, Msg} -> + throw_error(bad_request, Msg) end. -spec row_to_map(mod_mam:message_row()) -> map(). @@ -103,7 +103,7 @@ row_to_map(#{id := Id, jid := From, packet := Msg}) -> get_limit(#{limit := LimitBin}) -> try Limit = binary_to_integer(LimitBin), - true = Limit >= 0 andalso Limit =< 500, + true = Limit >= 0, Limit catch _:_ -> throw_error(bad_request, <<"Invalid limit">>) @@ -116,7 +116,7 @@ get_before(#{before := BeforeBin}) -> catch _:_ -> throw_error(bad_request, <<"Invalid value of 'before'">>) end; -get_before(#{}) -> 0. +get_before(#{}) -> undefined. get_owner_jid(#{owner := Owner}) -> case jid:from_binary(Owner) of diff --git a/src/mongoose_admin_api/mongoose_admin_api_stanzas.erl b/src/mongoose_admin_api/mongoose_admin_api_stanzas.erl index d892f0da8a8..309602883e1 100644 --- a/src/mongoose_admin_api/mongoose_admin_api_stanzas.erl +++ b/src/mongoose_admin_api/mongoose_admin_api_stanzas.erl @@ -50,15 +50,11 @@ from_json(Req, State) -> handle_post(Req, State) -> Args = parse_body(Req), Stanza = get_stanza(Args), - From = get_from_jid(Stanza), - To = get_to_jid(Stanza), - case mongoose_stanza_helper:route(From, To, Stanza, true) of - {error, #{what := unknown_domain}} -> - throw_error(bad_request, <<"Unknown domain">>); - {error, #{what := unknown_user}} -> - throw_error(bad_request, <<"Unknown user">>); + case mongoose_stanza_api:send_stanza(undefined, Stanza) of {ok, _} -> - {true, Req, State} + {true, Req, State}; + {_Error, Msg} -> + throw_error(bad_request, Msg) end. get_stanza(#{stanza := BinStanza}) -> @@ -68,26 +64,5 @@ get_stanza(#{stanza := BinStanza}) -> {error, _} -> throw_error(bad_request, <<"Malformed stanza">>) end; -get_stanza(#{}) -> throw_error(bad_request, <<"Missing stanza">>). - -get_from_jid(Stanza) -> - case exml_query:attr(Stanza, <<"from">>) of - undefined -> - throw_error(bad_request, <<"Missing sender JID">>); - JidBin -> - case jid:from_binary(JidBin) of - error -> throw_error(bad_request, <<"Invalid sender JID">>); - Jid -> Jid - end - end. - -get_to_jid(Stanza) -> - case exml_query:attr(Stanza, <<"to">>) of - undefined -> - throw_error(bad_request, <<"Missing recipient JID">>); - JidBin -> - case jid:from_binary(JidBin) of - error -> throw_error(bad_request, <<"Invalid recipient JID">>); - Jid -> Jid - end - end. +get_stanza(#{}) -> + throw_error(bad_request, <<"Missing stanza">>). diff --git a/src/mongoose_client_api/mongoose_client_api_messages.erl b/src/mongoose_client_api/mongoose_client_api_messages.erl index c56b8c3b05a..20ccaec84a3 100644 --- a/src/mongoose_client_api/mongoose_client_api_messages.erl +++ b/src/mongoose_client_api/mongoose_client_api_messages.erl @@ -74,25 +74,23 @@ handle_get(Req, State = #{jid := OwnerJid}) -> Args = parse_qs(Req), Limit = get_limit(Args), Before = get_before(Args), - Rows = mongoose_stanza_api:lookup_recent_messages(OwnerJid, WithJid, Before, Limit), + {ok, {Rows, _Limit}} = + mongoose_stanza_api:lookup_recent_messages(OwnerJid, WithJid, Before, Limit, false), Resp = [make_json_msg(Msg, MAMId) || #{id := MAMId, packet := Msg} <- Rows], {jiffy:encode(Resp), Req, State}. -handle_post(Req, State = #{jid := From}) -> +handle_post(Req, State = #{jid := UserJid}) -> Args = parse_body(Req), To = get_to(Args), Body = get_body(Args), - Packet = mongoose_stanza_helper:build_message(jid:to_binary(From), jid:to_binary(To), Body), - {ok, _} = mongoose_stanza_helper:route(From, To, Packet, true), - Id = exml_query:attr(Packet, <<"id">>), - Resp = #{<<"id">> => Id}, + {ok, Resp} = mongoose_stanza_api:send_chat_message(UserJid, undefined, To, Body), Req2 = cowboy_req:set_resp_body(jiffy:encode(Resp), Req), {true, Req2, State}. get_limit(#{limit := LimitBin}) -> try Limit = binary_to_integer(LimitBin), - true = Limit >= 0 andalso Limit =< 500, + true = Limit >= 0, Limit catch _:_ -> throw_error(bad_request, <<"Invalid limit">>) @@ -105,7 +103,7 @@ get_before(#{before := BeforeBin}) -> catch _:_ -> throw_error(bad_request, <<"Invalid value of 'before'">>) end; -get_before(#{}) -> 0. +get_before(#{}) -> undefined. get_with_jid(#{with := With}) -> case jid:from_binary(With) of From 2c19b3dd95253bc9fd5056858e978eac824881a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 21 Oct 2022 14:54:49 +0200 Subject: [PATCH 4/7] Remove unused function --- src/mam/mod_mam_utils.erl | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/mam/mod_mam_utils.erl b/src/mam/mod_mam_utils.erl index f9e8cc213f8..8da1275314b 100644 --- a/src/mam/mod_mam_utils.erl +++ b/src/mam/mod_mam_utils.erl @@ -10,7 +10,6 @@ %% UID -export([get_or_generate_mam_id/1, - get_mam_id_ext/1, generate_message_id/1, encode_compact_uuid/2, decode_compact_uuid/1, @@ -177,15 +176,6 @@ get_or_generate_mam_id(Acc) -> mod_mam_utils:external_binary_to_mess_id(ExtMessId) end. --spec get_mam_id_ext(mongoose_acc:t()) -> binary(). -get_mam_id_ext(Acc) -> - case mongoose_acc:get(mam, mam_id, undefined, Acc) of - undefined -> - <<>>; - ExtMessId -> - ExtMessId - end. - -spec generate_message_id(integer()) -> integer(). generate_message_id(CandidateStamp) -> {ok, NodeId} = ejabberd_node_id:node_id(), From bb6aa5cd8dae8a8f088d7efb911688f308eecda3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 21 Oct 2022 15:00:17 +0200 Subject: [PATCH 5/7] Update GraphQL tests with the new mongoose_stanza_api - The Id is a stanza Id now. - It is possible to send a stanza with predefined Id and/or without 'from'. - Update error messages. --- big_tests/tests/graphql_stanza_SUITE.erl | 85 ++++++++++++++---------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/big_tests/tests/graphql_stanza_SUITE.erl b/big_tests/tests/graphql_stanza_SUITE.erl index d0210f405a8..54598ce6fae 100644 --- a/big_tests/tests/graphql_stanza_SUITE.erl +++ b/big_tests/tests/graphql_stanza_SUITE.erl @@ -1,5 +1,6 @@ -module(graphql_stanza_SUITE). +-include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("exml/include/exml.hrl"). @@ -63,6 +64,7 @@ user_stanza_cases() -> user_send_message_headline, user_send_message_headline_with_spoofed_from, user_send_stanza, + user_send_stanza_without_from_with_id, user_send_stanza_with_spoofed_from] ++ user_get_last_messages_cases(). @@ -112,9 +114,10 @@ init_mam(Config) when is_list(Config) -> disabled -> Config; Backend -> - Mods = [{mod_mam, mam_helper:config_opts(#{backend => Backend, pm => #{}})}], + MAMOpts = mam_helper:config_opts(#{backend => Backend, pm => #{}}), + Mods = [{mod_mam, MAMOpts}], dynamic_modules:ensure_modules(domain_helper:host_type(), Mods), - [{has_mam, true}|Config] + Config end; init_mam(Other) -> Other. @@ -129,8 +132,8 @@ admin_send_message_story(Config, Alice, Bob) -> From = escalus_client:full_jid(Alice), To = escalus_client:short_jid(Bob), Res = send_message(From, To, <<"Hi!">>, Config), - #{<<"id">> := MamID} = get_ok_value([data, stanza, sendMessage], Res), - assert_not_empty(MamID, Config), + #{<<"id">> := StanzaId} = get_ok_value([data, stanza, sendMessage], Res), + assert_not_empty(StanzaId), escalus:assert(is_message, escalus:wait_for_stanza(Bob)). user_send_message(Config) -> @@ -141,8 +144,8 @@ user_send_message_story(Config, Alice, Bob) -> From = escalus_client:full_jid(Alice), To = escalus_client:short_jid(Bob), Res = user_send_message(Alice, From, To, <<"Hi!">>, Config), - #{<<"id">> := MamID} = get_ok_value([data, stanza, sendMessage], Res), - assert_not_empty(MamID, Config), + #{<<"id">> := StanzaId} = get_ok_value([data, stanza, sendMessage], Res), + assert_not_empty(StanzaId), escalus:assert(is_message, escalus:wait_for_stanza(Bob)). user_send_message_without_from(Config) -> @@ -152,8 +155,8 @@ user_send_message_without_from(Config) -> user_send_message_without_from_story(Config, Alice, Bob) -> To = escalus_client:short_jid(Bob), Res = user_send_message(Alice, null, To, <<"Hi!">>, Config), - #{<<"id">> := MamID} = get_ok_value([data, stanza, sendMessage], Res), - assert_not_empty(MamID, Config), + #{<<"id">> := StanzaId} = get_ok_value([data, stanza, sendMessage], Res), + assert_not_empty(StanzaId), escalus:assert(is_message, escalus:wait_for_stanza(Bob)). user_send_message_with_spoofed_from(Config) -> @@ -185,9 +188,12 @@ admin_send_message_headline_story(Config, Alice, Bob) -> From = escalus_client:full_jid(Alice), To = escalus_client:short_jid(Bob), Res = send_message_headline(From, To, <<"Welcome">>, <<"Hi!">>, Config), - #{<<"id">> := MamID} = get_ok_value([data, stanza, sendMessageHeadLine], Res), - % Headlines are not stored in MAM - <<>> = MamID, + #{<<"id">> := StanzaId} = get_ok_value([data, stanza, sendMessageHeadLine], Res), + assert_not_empty(StanzaId), + escalus:assert(is_message, escalus:wait_for_stanza(Bob)), + Res2 = send_message_headline(From, To, null, null, Config), + #{<<"id">> := StanzaId2} = get_ok_value([data, stanza, sendMessageHeadLine], Res2), + assert_not_empty(StanzaId2), escalus:assert(is_message, escalus:wait_for_stanza(Bob)). user_send_message_headline(Config) -> @@ -198,9 +204,8 @@ user_send_message_headline_story(Config, Alice, Bob) -> From = escalus_client:full_jid(Alice), To = escalus_client:short_jid(Bob), Res = user_send_message_headline(Alice, From, To, <<"Welcome">>, <<"Hi!">>, Config), - #{<<"id">> := MamID} = get_ok_value([data, stanza, sendMessageHeadLine], Res), - % Headlines are not stored in MAM - <<>> = MamID, + #{<<"id">> := StanzaId} = get_ok_value([data, stanza, sendMessageHeadLine], Res), + assert_not_empty(StanzaId), escalus:assert(is_message, escalus:wait_for_stanza(Bob)). user_send_message_headline_with_spoofed_from(Config) -> @@ -271,9 +276,11 @@ admin_send_stanza_story(Config, Alice, Bob) -> Body = <<"Hi!">>, Stanza = escalus_stanza:from(escalus_stanza:chat_to_short_jid(Bob, Body), Alice), Res = send_stanza(exml:to_binary(Stanza), Config), - #{<<"id">> := MamID} = get_ok_value([data, stanza, sendStanza], Res), - assert_not_empty(MamID, Config), - escalus:assert(is_message, escalus:wait_for_stanza(Bob)). + #{<<"id">> := StanzaId} = get_ok_value([data, stanza, sendStanza], Res), + assert_not_empty(StanzaId), + StanzaIn = escalus:wait_for_stanza(Bob), + escalus:assert(is_message, StanzaIn), + ?assertEqual(StanzaId, exml_query:attr(StanzaIn, <<"id">>)). user_send_stanza(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}], @@ -283,9 +290,25 @@ user_send_stanza_story(Config, Alice, Bob) -> Body = <<"Hi!">>, Stanza = escalus_stanza:from(escalus_stanza:chat_to_short_jid(Bob, Body), Alice), Res = user_send_stanza(Alice, exml:to_binary(Stanza), Config), - #{<<"id">> := MamID} = get_ok_value([data, stanza, sendStanza], Res), - assert_not_empty(MamID, Config), - escalus:assert(is_message, escalus:wait_for_stanza(Bob)). + #{<<"id">> := StanzaId} = get_ok_value([data, stanza, sendStanza], Res), + assert_not_empty(StanzaId), + StanzaIn = escalus:wait_for_stanza(Bob), + escalus:assert(is_message, StanzaIn), + ?assertEqual(StanzaId, exml_query:attr(StanzaIn, <<"id">>)). + +user_send_stanza_without_from_with_id(Config) -> + escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}], + fun user_send_stanza_without_from_with_id_story/3). + +user_send_stanza_without_from_with_id_story(Config, Alice, Bob) -> + Body = <<"Hi!">>, + StanzaId = base16:encode(crypto:strong_rand_bytes(8)), + Stanza = escalus_stanza:set_id(escalus_stanza:chat_to_short_jid(Bob, Body), StanzaId), + Res = user_send_stanza(Alice, exml:to_binary(Stanza), Config), + ?assertEqual(#{<<"id">> => StanzaId}, get_ok_value([data, stanza, sendStanza], Res)), + StanzaIn = escalus:wait_for_stanza(Bob), + escalus:assert(is_message, StanzaIn), + ?assertEqual(StanzaId, exml_query:attr(StanzaIn, <<"id">>)). user_send_stanza_with_spoofed_from(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}], @@ -314,7 +337,7 @@ admin_send_stanza_from_unknown_user_story(Config, Bob) -> Stanza = escalus_stanza:from(escalus_stanza:chat_to_short_jid(Bob, Body), From), Res = send_stanza(exml:to_binary(Stanza), Config), ?assertEqual(<<"unknown_user">>, get_err_code(Res)), - ?assertEqual(<<"Given user does not exist">>, get_err_msg(Res)). + ?assertEqual(<<"User does not exist">>, get_err_msg(Res)). admin_send_stanza_from_unknown_domain(Config) -> escalus:fresh_story_with_config(Config, [{bob, 1}], @@ -325,8 +348,8 @@ admin_send_stanza_from_unknown_domain_story(Config, Bob) -> From = <<"YeeeAH@oopsie">>, Stanza = escalus_stanza:from(escalus_stanza:chat_to_short_jid(Bob, Body), From), Res = send_stanza(exml:to_binary(Stanza), Config), - ?assertEqual(<<"unknown_domain">>, get_err_code(Res)), - ?assertEqual(<<"Given domain does not exist">>, get_err_msg(Res)). + ?assertEqual(<<"unknown_user">>, get_err_code(Res)), + ?assertEqual(<<"User's domain does not exist">>, get_err_msg(Res)). admin_get_last_messages(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}], @@ -367,7 +390,7 @@ admin_get_last_messages_for_unknown_user(Config) -> Jid = <<"maybemaybebutnot@", Domain/binary>>, Res = get_last_messages(Jid, null, null, Config), ?assertEqual(<<"unknown_user">>, get_err_code(Res)), - ?assertEqual(<<"Given user does not exist">>, get_err_msg(Res)). + ?assertEqual(<<"User does not exist">>, get_err_msg(Res)). admin_get_last_messages_with(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}, {kate, 1}], @@ -423,7 +446,7 @@ admin_get_last_messages_limit_enforced_story(Config, Alice, Bob) -> Caller = escalus_client:full_jid(Alice), Res = get_last_messages(Caller, null, 1000, null, Config), %% The actual limit is returned - #{<<"stanzas">> := [M1], <<"limit">> := 500} = + #{<<"stanzas">> := [M1], <<"limit">> := 50} = get_ok_value([data, stanza, getLastMessages], Res), check_stanza_map(M1, Alice). @@ -487,14 +510,6 @@ user_get_last_messages(User, With, Before, Config) -> %% Helpers -assert_not_empty(Bin, Config) -> - case proplists:get_value(has_mam, Config) of - true -> - assert_not_empty(Bin); - _ -> - skip - end. - assert_not_empty(Bin) when byte_size(Bin) > 0 -> ok. ok_result(What1, What2, {{<<"200">>, <<"OK">>}, #{<<"data">> := Data}}) -> @@ -512,5 +527,5 @@ check_stanza_map(#{<<"sender">> := SenderJID, spoofed_error(Call, Response) -> null = graphql_helper:get_err_value([data, stanza, Call], Response), - ?assertEqual(<<"Sending from this JID is not allowed">>, get_err_msg(Response)), - ?assertEqual(<<"bad_from_jid">>, get_err_code(Response)). + ?assertEqual(<<"Sender's JID is different from the user's JID">>, get_err_msg(Response)), + ?assertEqual(<<"invalid_sender">>, get_err_code(Response)). From d0bc1133febd9d9ed89ca0ceb14a6cb2ced61b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 21 Oct 2022 15:04:13 +0200 Subject: [PATCH 6/7] Update REST tests with the new mongoose_stanza_api - Update error messages - Test getting messages for a non-existent user --- big_tests/tests/rest_SUITE.erl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/big_tests/tests/rest_SUITE.erl b/big_tests/tests/rest_SUITE.erl index 811048981a0..243c7fab014 100644 --- a/big_tests/tests/rest_SUITE.erl +++ b/big_tests/tests/rest_SUITE.erl @@ -267,9 +267,9 @@ message_errors(Config) -> send_message_bin(AliceJID, <<"@noway">>), {?BAD_REQUEST, <<"Invalid sender JID">>} = send_message_bin(<<"@noway">>, BobJID), - {?BAD_REQUEST, <<"Unknown user">>} = + {?BAD_REQUEST, <<"User does not exist">>} = send_message_bin(<<"baduser@", (domain())/binary>>, BobJID), - {?BAD_REQUEST, <<"Unknown domain">>} = + {?BAD_REQUEST, <<"User's domain does not exist">>} = send_message_bin(<<"baduser@baddomain">>, BobJID). stanzas_are_sent_and_received(Config) -> @@ -297,9 +297,9 @@ stanza_errors(Config) -> send_stanza(extended_message([{<<"from">>, AliceJid}, {<<"to">>, <<"@invalid">>}])), {?BAD_REQUEST, <<"Invalid sender JID">>} = send_stanza(extended_message([{<<"from">>, <<"@invalid">>}, {<<"to">>, BobJid}])), - {?BAD_REQUEST, <<"Unknown domain">>} = + {?BAD_REQUEST, <<"User's domain does not exist">>} = send_stanza(extended_message([{<<"from">>, <<"baduser@baddomain">>}, {<<"to">>, BobJid}])), - {?BAD_REQUEST, <<"Unknown user">>} = + {?BAD_REQUEST, <<"User does not exist">>} = send_stanza(extended_message([{<<"from">>, UnknownJid}, {<<"to">>, BobJid}])), {?BAD_REQUEST, <<"Malformed stanza">>} = send_stanza(broken_message([{<<"from">>, AliceJid}, {<<"to">>, BobJid}])), @@ -347,10 +347,13 @@ messages_are_archived(Config) -> message_archive_errors(Config) -> Config1 = escalus_fresh:create_users(Config, [{alice, 1}]), User = binary_to_list(escalus_users:get_username(Config1, alice)), + Domain = binary_to_list(domain_helper:domain()), {?NOT_FOUND, <<"Missing owner JID">>} = gett(admin, "/messages"), {?BAD_REQUEST, <<"Invalid owner JID">>} = gett(admin, "/messages/@invalid"), + {?BAD_REQUEST, <<"User does not exist">>} = + gett(admin, "/messages/baduser@" ++ Domain), {?BAD_REQUEST, <<"Invalid interlocutor JID">>} = gett(admin, "/messages/" ++ User ++ "/@invalid"), {?BAD_REQUEST, <<"Invalid limit">>} = From 4c81efb684ebc0d41ffcc1ccb0c46895552a41b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 21 Oct 2022 12:39:59 +0200 Subject: [PATCH 7/7] Prevent get_last_messages from crashing when MAM is disabled --- big_tests/tests/graphql_stanza_SUITE.erl | 91 ++++++++++++++++-------- priv/graphql/schemas/admin/stanza.gql | 3 +- priv/graphql/schemas/user/stanza.gql | 3 +- 3 files changed, 65 insertions(+), 32 deletions(-) diff --git a/big_tests/tests/graphql_stanza_SUITE.erl b/big_tests/tests/graphql_stanza_SUITE.erl index 54598ce6fae..88ca4fef9bc 100644 --- a/big_tests/tests/graphql_stanza_SUITE.erl +++ b/big_tests/tests/graphql_stanza_SUITE.erl @@ -9,7 +9,7 @@ -import(distributed_helper, [mim/0, require_rpc_nodes/1]). -import(graphql_helper, [execute_user_command/5, execute_command/4, get_ok_value/2, get_err_code/1, get_err_msg/1, get_coercion_err_msg/1, - get_unauthorized/1]). + get_unauthorized/1, get_not_loaded/1]). suite() -> require_rpc_nodes([mim]) ++ escalus:suite(). @@ -21,10 +21,19 @@ all() -> {group, user_stanza}]. groups() -> - [{admin_stanza_http, [parallel], admin_stanza_cases()}, - {admin_stanza_cli, [], admin_stanza_cases()}, - {domain_admin_stanza, [], domain_admin_stanza_cases()}, - {user_stanza, [parallel], user_stanza_cases()}]. + [{admin_stanza_http, [], [{group, admin_mam}, + {group, admin_no_mam}]}, + {admin_stanza_cli, [], [{group, admin_mam}, + {group, admin_no_mam}]}, + {domain_admin_stanza, [], [{group, admin_mam}, % same as for admin + {group, domain_admin_no_mam}]}, + {user_stanza, [], [{group, user_mam}, + {group, user_no_mam}]}, + {admin_mam, [parallel], admin_mam_cases()}, + {admin_no_mam, [parallel], admin_stanza_cases() ++ admin_no_mam_cases()}, + {domain_admin_no_mam, [parallel], domain_admin_stanza_cases() ++ admin_no_mam_cases()}, + {user_mam, [parallel], user_mam_cases()}, + {user_no_mam, [parallel], user_stanza_cases() ++ user_no_mam_cases()}]. admin_stanza_cases() -> [admin_send_message, @@ -33,10 +42,9 @@ admin_stanza_cases() -> admin_send_stanza, admin_send_unparsable_stanza, admin_send_stanza_from_unknown_user, - admin_send_stanza_from_unknown_domain] - ++ admin_get_last_messages_cases(). + admin_send_stanza_from_unknown_domain]. -admin_get_last_messages_cases() -> +admin_mam_cases() -> [admin_get_last_messages, admin_get_last_messages_for_unknown_user, admin_get_last_messages_with, @@ -45,6 +53,9 @@ admin_get_last_messages_cases() -> admin_get_last_messages_limit_enforced, admin_get_last_messages_before]. +admin_no_mam_cases() -> + [admin_get_last_messages_no_mam]. + domain_admin_stanza_cases() -> [admin_send_message, admin_send_message_to_unparsable_jid, @@ -54,8 +65,7 @@ domain_admin_stanza_cases() -> admin_send_unparsable_stanza, domain_admin_send_stanza_from_unknown_user, domain_admin_send_stanza_from_unknown_domain, - domain_admin_get_last_messages_no_permission] - ++ admin_get_last_messages_cases(). + domain_admin_get_last_messages_no_permission]. user_stanza_cases() -> [user_send_message, @@ -65,17 +75,18 @@ user_stanza_cases() -> user_send_message_headline_with_spoofed_from, user_send_stanza, user_send_stanza_without_from_with_id, - user_send_stanza_with_spoofed_from] - ++ user_get_last_messages_cases(). + user_send_stanza_with_spoofed_from]. -user_get_last_messages_cases() -> +user_mam_cases() -> [user_get_last_messages]. +user_no_mam_cases() -> + [user_get_last_messages_no_mam]. + init_per_suite(Config) -> Config1 = ejabberd_node_utils:init(mim(), Config), Config2 = escalus:init_per_suite(Config1), - dynamic_modules:save_modules(domain_helper:host_type(), Config2), - init_mam(Config2). + dynamic_modules:save_modules(domain_helper:host_type(), Config2). end_per_suite(Config) -> escalus_fresh:clean(), @@ -89,22 +100,28 @@ init_per_group(admin_stanza_cli, Config) -> init_per_group(domain_admin_stanza, Config) -> graphql_helper:init_domain_admin_handler(Config); init_per_group(user_stanza, Config) -> - graphql_helper:init_user(Config). - -end_per_group(_, _Config) -> - graphql_helper:clean(). + graphql_helper:init_user(Config); +init_per_group(GN, Config) when GN =:= admin_mam; + GN =:= domain_admin_mam; + GN =:= user_mam -> + init_mam(Config); +init_per_group(GN, Config) when GN =:= admin_no_mam; + GN =:= domain_admin_no_mam; + GN =:= user_no_mam -> + Mods = [{mod_mam, stopped}], + dynamic_modules:ensure_modules(domain_helper:host_type(), Mods), + Config. + +end_per_group(GN, _Config) when GN =:= admin_stanza_http; + GN =:= admin_stanza_cli; + GN =:= domain_admin_stanza; + GN =:= user_stanza -> + graphql_helper:clean(); +end_per_group(_GN, _Config) -> + ok. init_per_testcase(CaseName, Config) -> - HasMam = proplists:get_value(has_mam, Config, false), - MamOnly = lists:member(CaseName, - user_get_last_messages_cases() - ++ admin_get_last_messages_cases()), - case {HasMam, MamOnly} of - {false, true} -> - {skip, no_mam}; - _ -> - escalus:init_per_testcase(CaseName, Config) - end. + escalus:init_per_testcase(CaseName, Config). end_per_testcase(CaseName, Config) -> escalus:end_per_testcase(CaseName, Config). @@ -112,7 +129,7 @@ end_per_testcase(CaseName, Config) -> init_mam(Config) when is_list(Config) -> case mam_helper:backend() of disabled -> - Config; + {skip, no_mam}; Backend -> MAMOpts = mam_helper:config_opts(#{backend => Backend, pm => #{}}), Mods = [{mod_mam, MAMOpts}], @@ -368,6 +385,12 @@ admin_get_last_messages_story(Config, Alice, Bob) -> get_ok_value([data, stanza, getLastMessages], Res2), check_stanza_map(M2, Alice). +admin_get_last_messages_no_mam(Config) -> + FreshConfig = escalus_fresh:create_users(Config, [{alice, 1}]), + AliceJid = escalus_users:get_jid(FreshConfig, alice), + Res = get_last_messages(AliceJid, null, null, Config), + get_not_loaded(Res). + user_get_last_messages(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}], fun user_get_last_messages_story/3). @@ -385,6 +408,14 @@ user_get_last_messages_story(Config, Alice, Bob) -> get_ok_value([data, stanza, getLastMessages], Res2), check_stanza_map(M2, Alice). +user_get_last_messages_no_mam(Config) -> + escalus:fresh_story_with_config(Config, [{alice, 1}], + fun user_get_last_messages_no_mam_story/2). + +user_get_last_messages_no_mam_story(Config, Alice) -> + Res = user_get_last_messages(Alice, null, null, Config), + get_not_loaded(Res). + admin_get_last_messages_for_unknown_user(Config) -> Domain = domain_helper:domain(), Jid = <<"maybemaybebutnot@", Domain/binary>>, diff --git a/priv/graphql/schemas/admin/stanza.gql b/priv/graphql/schemas/admin/stanza.gql index 151c1629b89..460b1733701 100644 --- a/priv/graphql/schemas/admin/stanza.gql +++ b/priv/graphql/schemas/admin/stanza.gql @@ -3,8 +3,9 @@ type StanzaAdminQuery @protected{ Get last 50 messages to/from a given contact, optionally you can change the limit, specify a date or select only messages exchanged with a specific contact """ - getLastMessages(caller: JID!, with: JID, limit: Int = 50, before: DateTime): StanzasPayload + getLastMessages(caller: JID!, with: JID, limit: PosInt = 50, before: DateTime): StanzasPayload @protected(type: DOMAIN, args: ["caller"]) + @use(modules: ["mod_mam_pm"], arg: "caller") } type StanzaAdminMutation @protected{ diff --git a/priv/graphql/schemas/user/stanza.gql b/priv/graphql/schemas/user/stanza.gql index 66fba77d513..47e78590280 100644 --- a/priv/graphql/schemas/user/stanza.gql +++ b/priv/graphql/schemas/user/stanza.gql @@ -3,7 +3,8 @@ Allow user to query MAM archive. """ type StanzaUserQuery @protected{ "Get n last messages to/from a given contact (optional) with limit and optional date" - getLastMessages(with: JID, limit: Int = 50, before: DateTime): StanzasPayload + getLastMessages(with: JID, limit: PosInt = 50, before: DateTime): StanzasPayload + @use(modules: ["mod_mam_pm"]) } """