Apply review

big_tests/tests/sasl2_helper.erl

@@ -176,7 +176,7 @@ buffer_messages_and_die(Config, _Client, #{spec := Spec} = Data) ->
     sm_helper:wait_for_messages(Client, Texts),
     %% Client's connection is violently terminated.
     escalus_client:kill_connection(Config, Client),
-    sm_SUITE:wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
     SMID = sm_helper:client_to_smid(Client),
     {C2SPid, Data#{bob => Bob, smid => SMID, smh => 3, texts => Texts}}.
 

big_tests/tests/sm_SUITE.erl

@@ -513,7 +513,7 @@ preserve_order(Config) ->
     %% kill alice connection
     escalus_connection:kill(Alice),
     C2SPid = mongoose_helper:get_session_pid(Alice),
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
 
     escalus_connection:send(Bob, escalus_stanza:chat_to_short_jid(Alice, <<"2">>)),
     escalus_connection:send(Bob, escalus_stanza:chat_to_short_jid(Alice, <<"3">>)),
@@ -580,7 +580,7 @@ resend_unacked_after_resume_timeout(Config) ->
 
     %% ensure there is no session
     C2SPid = mongoose_helper:get_session_pid(Alice),
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
 
     %% alice come back and receives unacked message
     NewAlice = connect_spec(AliceSpec, session),
@@ -645,7 +645,7 @@ resume_session_state_send_message_generic(Config, AckInitialPresence) ->
     %% kill alice connection
     C2SPid = mongoose_helper:get_session_pid(Alice),
     escalus_connection:kill(Alice),
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
     sm_helper:assert_alive_resources(Alice, 1),
 
     %% send some messages and check if c2s can handle it
@@ -693,7 +693,7 @@ resume_session_state_stop_c2s(Config) ->
     % session should be alive
     sm_helper:assert_alive_resources(Alice, 1),
     rpc(mim(), mongoose_c2s, stop, [C2SPid, normal]),
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
     %% suspend the process to ensure that Alice has enough time to reconnect,
     %% before resumption timeout occurs.
     ok = rpc(mim(), sys, suspend, [C2SPid]),
@@ -724,7 +724,7 @@ wait_for_resumption(Config) ->
     Bob = connect_fresh(Config, bob, session),
     Texts = three_texts(),
     {C2SPid, _} = buffer_unacked_messages_and_die(Config, AliceSpec, Bob, Texts),
-    wait_until_resume_session(C2SPid).
+    sm_helper:wait_until_resume_session(C2SPid).
 
 unacknowledged_message_hook_filter(Config) ->
     FilterText = <<"filter">>,
@@ -742,7 +742,7 @@ unacknowledged_message_hook_filter(Config) ->
     %% kill alice connection
     C2SPid = mongoose_helper:get_session_pid(Alice),
     escalus_connection:kill(Alice),
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
     sm_helper:assert_alive_resources(Alice, 1),
     %% ensure second C2S is registered so all the messages are bounced properly
     NewAlice = connect_spec([{resource, <<"2">>}| AliceSpec], sr_presence, manual),
@@ -830,7 +830,7 @@ unacknowledged_message_hook_common(RestartConnectionFN, Config) ->
     %% kill alice connection
     C2SPid = mongoose_helper:get_session_pid(Alice),
     escalus_connection:kill(Alice),
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
     sm_helper:assert_alive_resources(Alice, 1),
 
     escalus:assert(is_chat_message, [<<"msg-1">>], wait_for_unacked_msg_hook(0, Resource, 100)),
@@ -856,7 +856,7 @@ unacknowledged_message_hook_common(RestartConnectionFN, Config) ->
 
     NewC2SPid = mongoose_helper:get_session_pid(NewAlice),
     escalus_connection:kill(NewAlice),
-    wait_until_resume_session(NewC2SPid),
+    sm_helper:wait_until_resume_session(NewC2SPid),
 
     escalus:assert(is_chat_message, [<<"msg-1">>], wait_for_unacked_msg_hook(1, NewResource, 100)),
     escalus:assert(is_chat_message, [<<"msg-2">>], wait_for_unacked_msg_hook(1, NewResource, 100)),
@@ -936,7 +936,7 @@ resume_session_kills_old_C2S_gracefully(Config) ->
     escalus_client:kill_connection(Config, Alice),
 
     %% Ensure the c2s process is waiting for resumption.
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
 
     %% Resume the session.
     NewAlice = connect_resume(Alice, 1),
@@ -998,7 +998,7 @@ buffer_unacked_messages_and_die(Config, AliceSpec, Bob, Texts, F) ->
     sm_helper:wait_for_messages(Alice, Texts),
     %% Alice's connection is violently terminated.
     escalus_client:kill_connection(Config, Alice),
-    wait_until_resume_session(C2SPid),
+    sm_helper:wait_until_resume_session(C2SPid),
     SMID = sm_helper:client_to_smid(Alice),
     {C2SPid, SMID}.
 
@@ -1131,7 +1131,7 @@ messages_are_properly_flushed_during_resumption(Config) ->
         escalus_client:kill_connection(Config, Alice),
         %% The receiver process would stop now
         C2SPid = mongoose_helper:get_session_pid(Alice),
-        wait_until_resume_session(C2SPid),
+        sm_helper:wait_until_resume_session(C2SPid),
 
         sm_helper:wait_for_queue_length(C2SPid, 0),
         ok = rpc(mim(), sys, suspend, [C2SPid]),
@@ -1317,9 +1317,6 @@ wait_for_session(JID, Retries, SleepTime) ->
             ok
     end.
 
-wait_until_resume_session(C2SPid) ->
-    mongoose_helper:wait_for_c2s_state_name(C2SPid, ?EXT_C2S_STATE(resume_session)).
-
 maybe_ack_initial_presence(Alice, ack) ->
     ack_initial_presence(Alice);
 maybe_ack_initial_presence(_Alice, no_ack) ->

big_tests/tests/sm_helper.erl

@@ -25,6 +25,7 @@
          wait_for_resource_count/2,
          wait_for_c2s_unacked_count/2,
          wait_for_process_termination/1,
+         wait_until_resume_session/1,
          process_initial_stanza/1,
          kill_and_connect_resume/1,
          monitor_session/1]).
@@ -262,6 +263,9 @@ get_us_from_spec(UserSpec) ->
     S = proplists:get_value(server, UserSpec),
     {U, S}.
 
+wait_until_resume_session(C2SPid) ->
+    mongoose_helper:wait_for_c2s_state_name(C2SPid, ?EXT_C2S_STATE(resume_session)).
+
 
 %% Stanza helpers
 

doc/modules/mod_sasl2.md

@@ -1,5 +1,3 @@
 ## Module Description
 
 Implements [XEP-0388: Extensible SASL Profile](http://xmpp.org/extensions/xep-0388.html).
-
-

src/c2s/mongoose_c2s_acc.erl

@@ -174,7 +174,7 @@ to_c2s_acc(C2SAcc = #{socket_send := Stanzas}, {socket_send, NewStanzas}) when i
 to_c2s_acc(C2SAcc = #{socket_send := Stanzas}, {socket_send, Stanza}) ->
     C2SAcc#{socket_send := [Stanza | Stanzas]};
 to_c2s_acc(C2SAcc = #{socket_send := Stanzas}, {socket_send_first, NewStanzas}) when is_list(NewStanzas) ->
-    C2SAcc#{socket_send := Stanzas ++ NewStanzas};
+    C2SAcc#{socket_send := Stanzas ++ lists:reverse(NewStanzas)};
 to_c2s_acc(C2SAcc = #{socket_send := Stanzas}, {socket_send_first, Stanza}) ->
     C2SAcc#{socket_send := Stanzas ++ [Stanza]};
 to_c2s_acc(C2SAcc = #{actions := Actions}, {stop, Reason}) ->

src/mod_sasl2.erl

@@ -1,11 +1,11 @@
 %% Design Notes
 %%
 %% This module has three entry points for the statem: `authenticate', `response', and `abort'.
-%% All three of them will generate one `OriginalStateData' that will remain unchained for the whole
+%% All three of them will generate one `OriginalStateData' that will remain unchanged for the whole
 %% statem event, and a copy of this value will be stored into the `SaslAcc::mongoose_acc:t()', which
 %% hook handlers can modify, and at the end of the processing, they can be compared for changes.
 %%
-%% This module triggers two hooks for before and after the full sasl2 mechanism was executed.
+%% This module triggers two hooks for before and after the full sasl2 mechanism is executed.
 %% Handlers to these hooks can read the original `OriginalStateData', as well as the values
 %% accumulated on the SaslAcc. If a value wants to be updated, it should be careful to fetch the
 %% most recent from the accumulator, modify that one, and reinsert, otherwise it can override
@@ -18,7 +18,7 @@
 
 -define(BIND_RETRIES, 3).
 -define(XMLNS_SASL, {<<"xmlns">>, ?NS_SASL}).
--define(XMLNS_SASL2, {<<"xmlns">>, ?NS_SASL_2}).
+-define(XMLNS_SASL_2, {<<"xmlns">>, ?NS_SASL_2}).
 
 -behaviour(gen_mod).
 -behaviour(gen_statem).
@@ -115,7 +115,7 @@ terminate(Reason, C2SState, C2SData) ->
                  c2s_state => C2SState, c2s_data => C2SData}),
     mongoose_c2s:terminate({shutdown, ?MODULE}, C2SState, C2SData).
 
-%% Hooks
+%% Hook handlers
 -spec c2s_stream_features(Acc, map(), gen_hook:extra()) -> {ok, Acc} when
     Acc :: [exml:element()].
 c2s_stream_features(Acc, #{c2s_data := C2SData}, _) ->
@@ -350,7 +350,7 @@ failure_stanza(Reason) ->
 
 -spec sasl2_ns_stanza(binary(), [exml:element() | exml:cdata()]) -> exml:element().
 sasl2_ns_stanza(Name, Children) ->
-    #xmlel{name = Name, attrs = [?XMLNS_SASL2], children = Children}.
+    #xmlel{name = Name, attrs = [?XMLNS_SASL_2], children = Children}.
 
 -spec success_subelement(binary(), binary()) -> exml:element().
 success_subelement(Name, AuthId) ->
@@ -380,7 +380,7 @@ init_mod_state(El) ->
     end.
 
 -spec if_provided_then_is_not_invalid_uuid_v4(not_provided | binary()) ->
-    invalid_agent | uuid:uuid().
+    not_provided | invalid_agent | uuid:uuid().
 if_provided_then_is_not_invalid_uuid_v4(not_provided) ->
     not_provided;
 if_provided_then_is_not_invalid_uuid_v4(Binary) ->
@@ -398,7 +398,7 @@ feature(C2SData, Mechanisms) ->
     InlineFeatures = mongoose_hooks:sasl2_stream_features(C2SData, []),
     InlineElem = inlines(InlineFeatures),
     #xmlel{name = feature_name(),
-           attrs = [?XMLNS_SASL2],
+           attrs = [?XMLNS_SASL_2],
            children = [InlineElem | Mechanisms]}.
 
 -spec inlines([exml:element()]) -> exml:element().
@@ -432,7 +432,7 @@ update_inline_request(SaslAcc, ModuleRequest, XmlResponse, Status) ->
             mongoose_acc:set(?MODULE, ModuleRequest, Request1, SaslAcc)
     end.
 
-%% Here we extract these two values after all modifications by the inline requests
+%% Here we extract these values after all modifications by the inline requests
 -spec get_state_data(mongoose_acc:t()) -> c2s_state_data().
 get_state_data(SaslAcc) ->
     mongoose_acc:get(?MODULE, c2s_state_data, SaslAcc).