Revert "Merge pull request #2759 from esl/amp-using-acc"

This reverts commit 028603d, reversing
changes made to f2709d0.

src/amp.erl

@@ -12,6 +12,7 @@
 -export([extract_requested_rules/1,
          make_response/3,
          make_error_response/4,
+         update_rules/2,
          rule_to_xmlel/1,
          strip_amp_el/1,
 
@@ -110,6 +111,16 @@ make_error_el(Errors, Rules) ->
                     {<<"code">>, error_code(hd(Errors))}],
            children = [ErrorMarker, RuleContainer]}.
 
+-spec update_rules(#xmlel{}, [amp_rule()]) -> #xmlel{}.
+update_rules(Packet = #xmlel{children = Children}, Rules) ->
+    Packet#xmlel{children = [update_rules_in_element(El, Rules) || El <- Children]}.
+
+update_rules_in_element(El, Rules) ->
+    case is_amp_el(El) of
+        true -> El#xmlel{children = [rule_to_xmlel(R) || R <- Rules]};
+        false -> El
+    end.
+
 -spec rule_to_xmlel(amp_any_rule()) -> #xmlel{}.
 rule_to_xmlel(#amp_rule{condition=C, value=V, action=A}) ->
     #xmlel{name = <<"rule">>,

src/ejabberd_c2s.erl

@@ -1274,8 +1274,8 @@ response_negative(<<"iq">>, deny, From, To, Acc) ->
     IqType = mongoose_acc:stanza_type(Acc),
     response_iq_deny(IqType, From, To, Acc);
 response_negative(<<"message">>, deny, From, To, Acc) ->
-    Acc1 = mod_amp:check_packet(Acc, delivery_failed),
-    send_back_error(mongoose_xmpp_errors:service_unavailable(), From, To, Acc1);
+    mod_amp:check_packet(Acc, From, delivery_failed),
+    send_back_error(mongoose_xmpp_errors:service_unavailable(), From, To, Acc);
 response_negative(_, _, _, _, Acc) ->
     Acc.
 
@@ -1725,24 +1725,25 @@ send_trailer(StateData) ->
 -spec send_and_maybe_buffer_stanza(mongoose_acc:t(), packet(), state()) ->
     {ok | resume, mongoose_acc:t(), state()}.
 send_and_maybe_buffer_stanza(Acc, {J1, J2, El}, State)->
+    StrippedStanza = mod_amp:strip_amp_el_from_request(El),
     {SendResult, _, BufferedStateData} = send_and_maybe_buffer_stanza_no_ack(Acc,
-                                                                          {J1, J2, El},
+                                                                          {J1, J2, StrippedStanza},
                                                                           State),
-    Acc1 = mod_amp:check_packet(Acc, result_to_amp_event(SendResult)),
+    mod_amp:check_packet(El, result_to_amp_event(SendResult)),
     case SendResult of
         ok ->
-            try maybe_send_ack_request(Acc1, BufferedStateData) of
+            try maybe_send_ack_request(Acc, BufferedStateData) of
                 ResAcc ->
                     {ok, ResAcc, BufferedStateData}
             catch
                 _:E ->
                     ?DEBUG("event=send_ack_request_error,error=~p", [E]),
                     ?DEBUG("event=enter_resume_session", []),
-                    {resume, Acc1, BufferedStateData}
+                    {resume, Acc, BufferedStateData}
             end;
         _ ->
             ?DEBUG("Send element error: ~p, try enter resume session", [SendResult]),
-            {resume, Acc1, BufferedStateData}
+            {resume, Acc, BufferedStateData}
     end.
 
 result_to_amp_event(ok) -> delivered;

src/ejabberd_sm.erl

@@ -855,7 +855,7 @@ route_message_by_type(_, From, To, Acc, Packet) ->
                 true ->
                     mongoose_hooks:offline_message_hook(LServer, Acc, From, To, Packet);
                 false ->
-                    mongoose_hooks:failed_to_store_message(LServer, Acc)
+                    mongoose_hooks:failed_to_store_message(LServer, Acc, From, Packet)
             end;
         _ ->
             {Acc1, Err} = jlib:make_error_reply(

src/mam/mod_mam.erl

@@ -289,14 +289,16 @@ user_send_packet(Acc, From, To, Packet) ->
 filter_packet(drop) ->
     drop;
 filter_packet({From, To=#jid{luser=LUser, lserver=LServer}, Acc, Packet}) ->
+    % let them to their amp-related mambo jumbo on stanza
     ?DEBUG("Receive packet~n    from ~p ~n    to ~p~n    packet ~p.",
            [From, To, Packet]),
     {AmpEvent, PacketAfterArchive} =
         case ejabberd_users:does_user_exist(LUser, LServer) of
             false ->
                 {mam_failed, Packet};
             true ->
-                case process_incoming_packet(From, To, Packet) of
+                PacketWithoutAmp = mod_amp:strip_amp_el_from_request(Packet),
+                case process_incoming_packet(From, To, PacketWithoutAmp) of
                     undefined -> {mam_failed, Packet};
                     MessID -> {archived, maybe_add_arcid_elems(
                                            To, MessID, Packet,
@@ -306,9 +308,10 @@ filter_packet({From, To=#jid{luser=LUser, lserver=LServer}, Acc, Packet}) ->
     Acc1 = mongoose_acc:update_stanza(#{ element => PacketAfterArchive,
                                          from_jid => From,
                                          to_jid => To }, Acc),
-    Acc2 = mod_amp:check_packet(Acc1, AmpEvent),
+    Acc2 = mod_amp:check_packet(Acc1, From, AmpEvent),
     {From, To, Acc2, mongoose_acc:element(Acc2)}.
 
+
 process_incoming_packet(From, To, Packet) ->
     handle_package(incoming, true, To, From, From, Packet).
 

src/mod_amp.erl

@@ -9,10 +9,13 @@
 -behaviour(mongoose_module_metrics).
 -xep([{xep, 79}, {version, "1.2"}, {comment, "partially implemented."}]).
 -export([start/2, stop/1]).
--export([run_initial_check/2,
-         check_packet/2,
+-export([check_packet/2,
+         check_packet/3,
          add_local_features/5,
-         add_stream_feature/2
+         add_stream_feature/2,
+         run_initial_check/2,
+         amp_check_packet/3,
+         strip_amp_el_from_request/1
         ]).
 
 -include("amp.hrl").
@@ -37,26 +40,57 @@ hooks(Host) ->
     [{c2s_stream_features, Host, ?MODULE, add_stream_feature, 50},
      {disco_local_features, Host, ?MODULE, add_local_features, 99},
      {c2s_preprocessing_hook, Host, ?MODULE, run_initial_check, 10},
+     {amp_check_packet, Host, ?MODULE, amp_check_packet, 10},
      {amp_verify_support, Host, ?AMP_RESOLVER, verify_support, 10},
      {amp_check_condition, Host, ?AMP_RESOLVER, check_condition, 10},
      {amp_determine_strategy, Host, ?AMP_STRATEGY, determine_strategy, 10}].
+%% Business API
 
-%% API
 
 -spec run_initial_check(mongoose_acc:t(), ejabberd_c2s:state()) -> mongoose_acc:t().
 run_initial_check(#{result := drop} = Acc, _C2SState) ->
     Acc;
 run_initial_check(Acc, _C2SState) ->
-    case mongoose_acc:stanza_name(Acc) of
-        <<"message">> -> run_initial_check(Acc);
-        _ -> Acc
+    Acc1 = mod_amp:check_packet(Acc, mongoose_acc:from_jid(Acc), initial_check),
+    case mongoose_acc:get(amp, check_result, ok, Acc1) of
+        drop -> mongoose_acc:set(hook, result, drop, Acc1);
+        _ -> Acc1
     end.
 
--spec check_packet(mongoose_acc:t(), amp_event()) -> mongoose_acc:t().
+
+-spec check_packet(mongoose_acc:t() | exml:element(), amp_event()) ->
+    mongoose_acc:t() | exml:element().
+check_packet(Packet = #xmlel{attrs = Attrs}, Event) ->
+    % it is called this way only from ejabberd_c2s:send_and_maybe_buffer_stanza/3, line 1666
+    % maybe Paweł Chrząszcz knows why and can advise what to do about it
+    case xml:get_attr(<<"from">>, Attrs) of
+        {value, From} ->
+            check_packet(Packet, jid:from_binary(From), Event);
+        _ ->
+            Packet
+    end;
 check_packet(Acc, Event) ->
-    case mongoose_acc:get(amp, rules, none, Acc) of
-        none -> Acc;
-        Rules -> process_event(Acc, Rules, Event)
+    check_packet(Acc, mongoose_acc:from_jid(Acc), Event).
+
+-spec check_packet(exml:element()|mongoose_acc:t(), jid:jid(), amp_event()) ->
+    exml:element() | mongoose_acc:t().
+check_packet(Packet = #xmlel{name = <<"message">>}, From, Event) ->
+    Acc = mongoose_acc:new(#{ location => ?LOCATION,
+                              lserver => From#jid.lserver,
+                              from_jid => From,
+                              element => Packet }),
+    mongoose_acc:element(check_packet(Acc, From, Event));
+check_packet(Packet = #xmlel{}, _, _) ->
+    Packet;
+check_packet(Acc, #jid{lserver = Host} = From, Event) ->
+    case mongoose_acc:stanza_name(Acc) of
+        <<"message">> ->
+            % this hook replaces original element with something modified by amp
+            % which is a hack, but since we have accumulator here we have a chance
+            % to fix implementation
+            mongoose_hooks:amp_check_packet(Host, Acc, From, Event);
+        _ ->
+            Acc
     end.
 
 -spec add_local_features(Acc :: {result, [exml:element()]} | empty | {error, any()},
@@ -73,49 +107,39 @@ add_local_features(Acc, _From, _To, _NS, _Lang) ->
 add_stream_feature(Acc, _Host) ->
     lists:keystore(<<"amp">>, #xmlel.name, Acc, ?AMP_FEATURE).
 
-%% Internal
+-spec amp_check_packet(mongoose_acc:t(), jid:jid(), amp_event()) -> mongoose_acc:t().
+amp_check_packet(Acc, From, Event) ->
+    Res = mongoose_acc:get(amp, check_result, ok, Acc),
+    amp_check_packet(Res, mongoose_acc:stanza_name(Acc), Acc, From, Event).
 
--spec run_initial_check(mongoose_acc:t()) -> mongoose_acc:t().
-run_initial_check(Acc) ->
+amp_check_packet(drop, _, Acc, _, _) ->
+    Acc;
+amp_check_packet(_, <<"message">>, Acc, From, Event) ->
     Packet = mongoose_acc:element(Acc),
-    From = mongoose_acc:from_jid(Acc),
-    To = mongoose_acc:to_jid(Acc),
-    Result = case amp:extract_requested_rules(Packet) of
-                 none -> nothing_to_do;
-                 {rules, Rules} -> validate_and_process_rules(Packet, From, Rules);
-                 {errors, Errors} -> send_errors_and_drop(Packet, From, Errors)
-             end,
-    case Result of
-        nothing_to_do ->
-            Acc;
+    Res = case amp:extract_requested_rules(Packet) of
+              none                    -> Packet;
+              {rules, Rules}          -> process_amp_rules(Packet, From, Event, Rules);
+              {errors, Errors}        -> send_errors_and_drop(Packet, From, Errors)
+          end,
+    case Res of
         drop ->
-            mongoose_acc:set(hook, result, drop, Acc);
-        NewRules ->
-            Acc1 = mongoose_acc:set_permanent(amp, rules, NewRules, Acc),
-            mongoose_acc:update_stanza(#{element => amp:strip_amp_el(Packet),
-                                         from_jid => From,
-                                         to_jid => To}, Acc1)
-    end.
+            mongoose_acc:set(amp, check_result, drop, Acc);
+        NewElem ->
+            mongoose_acc:update_stanza(#{ element => NewElem,
+                                          from_jid => From,
+                                          to_jid => mongoose_acc:to_jid(Acc) }, Acc)
+    end;
+amp_check_packet(_, _, Acc, _, _) ->
+    Acc.
 
--spec validate_and_process_rules(exml:element(), jid:jid(), amp_rules()) -> amp_rules() | drop.
-validate_and_process_rules(Packet, From, Rules) ->
-    VerifiedRules = verify_support(host(From), Rules),
-    {Good, Bad} = lists:partition(fun is_supported_rule/1, VerifiedRules),
-    ValidRules = [ Rule || {supported, Rule} <- Good ],
-    case Bad of
-        [{error, ValidationError, InvalidRule} | _] ->
-            send_error_and_drop(Packet, From, ValidationError, InvalidRule);
-        [] ->
-            process_rules(Packet, From, initial_check, ValidRules)
+strip_amp_el_from_request(Packet) ->
+    % this will probably be removed - we have accumulator so we won't need anymore to store amp
+    % markers in stanza
+    case amp:is_amp_request(Packet) of
+        true -> amp:strip_amp_el(Packet);
+        false -> Packet
     end.
 
--spec process_event(mongoose_acc:t(), amp_rules(), amp_event()) -> mongoose_acc:t().
-process_event(Acc, Rules, Event) when Event =/= initial_check ->
-    Packet = mongoose_acc:element(Acc),
-    From = mongoose_acc:from_jid(Acc),
-    NewRules = process_rules(Packet, From, Event, Rules),
-    mongoose_acc:set_permanent(amp, rules, NewRules, Acc).
-
 %% @doc This may eventually be configurable, but for now we return a constant list.
 amp_features() ->
     [<<"http://jabber.org/protocol/amp">>
@@ -125,14 +149,22 @@ amp_features() ->
    , <<"http://jabber.org/protocol/amp?condition=match-resource">>
     ].
 
--spec process_rules(exml:element(), jid:jid(), amp_event(), amp_rules()) -> amp_rules() | drop.
-process_rules(Packet, From, Event, Rules) ->
-    Strategy = determine_strategy(Packet, From, Event),
-    RulesWithResults = apply_rules(fun(Rule) ->
-                                           resolve_condition(From, Strategy, Event, Rule)
-                                   end, Rules),
-    PacketResult = take_action(Packet, From, RulesWithResults),
-    return_result(PacketResult, Event, RulesWithResults).
+-spec process_amp_rules(exml:element(), jid:jid(), amp_event(), amp_rules()) -> exml:element() | drop.
+process_amp_rules(Packet, From, Event, Rules) ->
+    VerifiedRules = verify_support(host(From), Rules),
+    {Good, Bad} = lists:partition(fun is_supported_rule/1, VerifiedRules),
+    ValidRules = [ Rule || {supported, Rule} <- Good ],
+    case Bad of
+        [{error, ValidationError, InvalidRule} | _] ->
+            send_error_and_drop(Packet, From, ValidationError, InvalidRule);
+        [] ->
+            Strategy = determine_strategy(Packet, From, Event),
+            RulesWithResults = apply_rules(fun(Rule) ->
+                                                   resolve_condition(From, Strategy, Event, Rule)
+                                           end, ValidRules),
+            PacketResult = take_action(Packet, From, RulesWithResults),
+            return_result(PacketResult, Packet, RulesWithResults)
+    end.
 
 %% @doc ejabberd_hooks helpers
 -spec verify_support(binary(), amp_rules()) -> [amp_rule_support()].
@@ -159,18 +191,20 @@ match_undecided_for_final_event(#amp_rule{condition = deliver}, Event, undecided
        Event =:= delivery_failed -> match;
 match_undecided_for_final_event(_, _, Result) -> Result.
 
--spec take_action(exml:element(), jid:jid(), amp_rules()) -> pass | drop.
 take_action(Packet, From, Rules) ->
     case find(fun(#amp_rule{result = Result}) -> Result =:= match end, Rules) of
         not_found -> pass;
         {found, Rule} -> take_action_for_matched_rule(Packet, From, Rule)
     end.
 
-return_result(drop, initial_check, _Rules) -> drop;
-return_result(pass, _Event, Rules) ->
-    lists:filter(fun(#amp_rule{result = Result}) ->
-                         Result =:= undecided
-                 end, Rules).
+return_result(drop, _Packet, _Rules) -> drop;
+return_result(pass, Packet, Rules) ->
+    update_rules(Packet, lists:filter(fun(#amp_rule{result = Result}) ->
+                                              Result =:= undecided
+                                      end, Rules)).
+
+update_rules(Packet, []) -> amp:strip_amp_el(Packet);
+update_rules(Packet, Rules) -> amp:update_rules(Packet, Rules).
 
 -spec take_action_for_matched_rule(exml:element(), jid:jid(), amp_rule()) -> pass | drop.
 take_action_for_matched_rule(Packet, From, #amp_rule{action = notify} = Rule) ->

src/mongoose_hooks.erl

@@ -10,7 +10,7 @@
          anonymous_purge_hook/3,
          auth_failed/2,
          ejabberd_ctl_process/2,
-         failed_to_store_message/2,
+         failed_to_store_message/4,
          filter_local_packet/2,
          filter_packet/1,
          host_config_update/4,
@@ -204,12 +204,17 @@ auth_failed(Server, Username) ->
 ejabberd_ctl_process(Acc, Args) ->
     ejabberd_hooks:run_fold(ejabberd_ctl_process, Acc, [Args]).
 
--spec failed_to_store_message(LServer, Acc) -> Result when
+-spec failed_to_store_message(LServer, Acc, From, Packet) -> Result when
     LServer :: jid:lserver(),
     Acc :: mongoose_acc:t(),
+    From :: jid:jid(),
+    Packet :: exml:element(),
     Result :: mongoose_acc:t().
-failed_to_store_message(LServer, Acc) ->
-    ejabberd_hooks:run_fold(failed_to_store_message, LServer, Acc, []).
+failed_to_store_message(LServer, Acc, From, Packet) ->
+    ejabberd_hooks:run_fold(failed_to_store_message,
+                            LServer,
+                            Acc,
+                            [From, Packet]).
 
 %%% @doc The `filter_local_packet' hook is called to filter out stanzas routed with `mongoose_local_delivery'.
 -spec filter_local_packet(Server, {From, To, Acc, Packet}) -> Result when