P2C-8: add attempt limit (#238)

* P2C-8: Attempt limit * Add validation for attempt_limit * Add attempt limit check * Add test, fix logic * Fix images * Remove garbage from test * Limit attempts in case attempt_limit undefined
2024-11-06 02:35:18 +00:00 · 2020-06-26 16:25:24 +03:00 · 2020-06-26 16:25:24 +03:00 · 8500401cff
commit 8500401cff
parent 3fab8903b2
8 changed files with 152 additions and 23 deletions
--- a/apps/ff_cth/src/ct_payment_system.erl
+++ b/apps/ff_cth/src/ct_payment_system.erl
@ -386,6 +386,19 @@ domain_config(Options, C) ->
                        }}},
                        then_ = {value, [?wthdr_prv(4), ?wthdr_prv(5)]}
                    },
+                    #domain_WithdrawalProviderDecision{
+                        if_ = {condition, {cost_in, #domain_CashRange{
+                            upper = {inclusive, #domain_Cash{
+                                amount = 500100,
+                                currency = #domain_CurrencyRef{symbolic_code = <<"RUB">>}
+                            }},
+                            lower = {inclusive, #domain_Cash{
+                                amount = 500100,
+                                currency = #domain_CurrencyRef{symbolic_code = <<"RUB">>}
+                            }}
+                        }}},
+                        then_ = {value, [?wthdr_prv(4), ?wthdr_prv(6), ?wthdr_prv(7), ?wthdr_prv(8)]}
+                    },
                    #domain_WithdrawalProviderDecision{
                        if_ = {
                            condition,
@ -488,6 +501,9 @@ domain_config(Options, C) ->
        ct_domain:withdrawal_provider(?wthdr_prv(3), ?prx(3), dummy_provider_identity_id(Options), C),
        ct_domain:withdrawal_provider(?wthdr_prv(4), ?prx(6), provider_identity_id(Options), C),
        ct_domain:withdrawal_provider(?wthdr_prv(5), ?prx(2), provider_identity_id(Options), C),
+        ct_domain:withdrawal_provider(?wthdr_prv(6), ?prx(6), provider_identity_id(Options), C),
+        ct_domain:withdrawal_provider(?wthdr_prv(7), ?prx(6), provider_identity_id(Options), C),
+        ct_domain:withdrawal_provider(?wthdr_prv(8), ?prx(2), provider_identity_id(Options), C),
        ct_domain:p2p_provider(?p2p_prv(1), ?prx(5), dummy_provider_identity_id(Options), C),

        ct_domain:contract_template(?tmpl(1), ?trms(1)),
@ -530,6 +546,7 @@ default_termset(Options) ->
            ]},
            withdrawals = #domain_WithdrawalServiceTerms{
                currencies = {value, ?ordset([?cur(<<"RUB">>)])},
+                attempt_limit = {value, #domain_AttemptLimit{attempts = 3}},
                cash_limit = {decisions, [
                    #domain_CashLimitDecision{
                        if_   = {condition, {currency_is, ?cur(<<"RUB">>)}},
--- a/apps/ff_transfer/src/ff_withdrawal.erl
+++ b/apps/ff_transfer/src/ff_withdrawal.erl
@ -846,15 +846,15 @@ process_session_creation(Withdrawal) ->
 -spec construct_session_id(withdrawal_state()) -> id().
 construct_session_id(Withdrawal) ->
    ID = id(Withdrawal),
-    Index = ff_withdrawal_route_attempt_utils:get_index(attempts(Withdrawal)),
-    SubID = integer_to_binary(Index),
+    Attempt = ff_withdrawal_route_attempt_utils:get_attempt(attempts(Withdrawal)),
+    SubID = integer_to_binary(Attempt),
    << ID/binary, "/", SubID/binary >>.

 -spec construct_p_transfer_id(withdrawal_state()) -> id().
 construct_p_transfer_id(Withdrawal) ->
    ID = id(Withdrawal),
-    Index = ff_withdrawal_route_attempt_utils:get_index(attempts(Withdrawal)),
-    SubID = integer_to_binary(Index),
+    Attempt = ff_withdrawal_route_attempt_utils:get_attempt(attempts(Withdrawal)),
+    SubID = integer_to_binary(Attempt),
    <<"ff/withdrawal/", ID/binary, "/", SubID/binary >>.

 create_session(ID, TransferData, SessionParams) ->
@ -1447,13 +1447,17 @@ process_route_change(Providers, Withdrawal, Reason) ->
    Attempts = attempts(Withdrawal),
    %% TODO Remove line below after switch to [route()] from [provider_id()]
    Routes = [#{provider_id => ID} || ID <- Providers],
-    case ff_withdrawal_route_attempt_utils:next_route(Routes, Attempts) of
+    AttemptLimit = get_attempt_limit(Withdrawal),
+    case ff_withdrawal_route_attempt_utils:next_route(Routes, Attempts, AttemptLimit) of
        {ok, Route} ->
            {continue, [
                {route_changed, Route}
            ]};
        {error, route_not_found} ->
            %% No more routes, return last error
+            process_transfer_fail(Reason, Withdrawal);
+        {error, attempt_limit_exceeded} ->
+            %% Attempt limit exceeded, return last error
            process_transfer_fail(Reason, Withdrawal)
    end.

@ -1658,6 +1662,47 @@ maybe_migrate({session_finished, SessionID}, MigrateParams) ->
 maybe_migrate(Ev, _MigrateParams) ->
    Ev.

+get_attempt_limit(Withdrawal) ->
+    #{
+        body := Body,
+        params := #{
+            wallet_id := WalletID,
+            destination_id := DestinationID
+        },
+        created_at := Timestamp,
+        party_revision := PartyRevision,
+        domain_revision := DomainRevision,
+        resource := Resource
+    } = Withdrawal,
+    {ok, Wallet} = get_wallet(WalletID),
+    {ok, Destination} = get_destination(DestinationID),
+    Identity = get_wallet_identity(Wallet),
+    PartyID = ff_identity:party(Identity),
+    ContractID = ff_identity:contract(Identity),
+    VarsetParams = genlib_map:compact(#{
+        body => Body,
+        wallet_id => WalletID,
+        party_id => PartyID,
+        destination => Destination,
+        resource => Resource
+    }),
+
+    {ok, Terms} = ff_party:get_contract_terms(
+        PartyID, ContractID, build_party_varset(VarsetParams), Timestamp, PartyRevision, DomainRevision
+    ),
+    #domain_TermSet{wallets = WalletTerms} = Terms,
+    #domain_WalletServiceTerms{withdrawals = WithdrawalTerms} = WalletTerms,
+    #domain_WithdrawalServiceTerms{attempt_limit = AttemptLimit} = WithdrawalTerms,
+    get_attempt_limit_(AttemptLimit).
+
+get_attempt_limit_(undefined) ->
+    %% When attempt_limit is undefined
+    %% do not try all defined providers, if any
+    %% just stop after first one
+    1;
+get_attempt_limit_({value, Limit}) ->
+    ff_dmsl_codec:unmarshal(attempt_limit, Limit).
+
 %% Tests

 -ifdef(TEST).
--- a/apps/ff_transfer/src/ff_withdrawal_route_attempt_utils.erl
+++ b/apps/ff_transfer/src/ff_withdrawal_route_attempt_utils.erl
@ -21,6 +21,7 @@
 -type account()  :: ff_account:account().
 -type route() :: ff_withdrawal:route().
 -type session() :: ff_withdrawal:session().
+-type attempt_limit() :: ff_party:attempt_limit().

 -type attempt() :: #{
    session => session(),
@ -31,7 +32,7 @@
 -opaque attempts() :: #{
    attempts := #{route() => attempt()},
    inversed_routes := [route()],
-    index := non_neg_integer(),
+    attempt := non_neg_integer(),
    current => route()
 }.

@ -39,7 +40,7 @@

 %% API
 -export([new_route/2]).
-export([next_route/2]).
+-export([next_route/3]).
 -export([get_current_session/1]).
 -export([get_current_p_transfer/1]).
 -export([get_current_limit_checks/1]).
@ -48,7 +49,7 @@
 -export([update_current_limit_checks/2]).

 -export([get_sessions/1]).
-export([get_index/1]).
+-export([get_attempt/1]).

 -spec new_route(route(), attempts()) ->
    attempts().
@ -58,17 +59,21 @@ new_route(Route, Existing) ->
    #{
        attempts := Attempts,
        inversed_routes := InvRoutes,
-        index := Index
+        attempt := Attempt
    } = Existing,
    Existing#{
        current => Route,
-        index => Index + 1,
+        attempt => Attempt + 1,
        inversed_routes => [Route | InvRoutes],
        attempts => Attempts#{Route => #{}}
    }.

-spec next_route([route()], attempts()) -> {ok, route()} | {error, route_not_found}.
-next_route(Routes, #{attempts := Existing}) ->
+-spec next_route([route()], attempts(), attempt_limit()) ->
+    {ok, route()} | {error, route_not_found | attempt_limit_exceeded}.
+next_route(_Routes, #{attempt := Attempt}, AttemptLimit)
+    when is_integer(AttemptLimit) andalso Attempt == AttemptLimit ->
+    {error, attempt_limit_exceeded};
+next_route(Routes, #{attempts := Existing}, _AttemptLimit) ->
    PendingRoutes =
        lists:filter(
            fun(R) ->
@ -140,9 +145,9 @@ get_sessions(#{attempts := Attempts, inversed_routes := InvRoutes}) ->
        InvRoutes
    ).

-spec get_index(attempts()) -> non_neg_integer().
-get_index(#{index := Index}) ->
-    Index.
+-spec get_attempt(attempts()) -> non_neg_integer().
+get_attempt(#{attempt := Attempt}) ->
+    Attempt.

 %% Internal

@ -151,7 +156,7 @@ init() ->
    #{
        attempts => #{},
        inversed_routes => [],
-        index => 0
+        attempt => 0
    }.

 %% @private
--- a/apps/ff_transfer/test/ff_withdrawal_routing_SUITE.erl
+++ b/apps/ff_transfer/test/ff_withdrawal_routing_SUITE.erl
@ -35,6 +35,7 @@

 -export([adapter_unreachable_route_test/1]).
 -export([adapter_unreachable_quote_test/1]).
+-export([attempt_limit_test/1]).

 %% Internal types

@ -67,7 +68,8 @@ groups() ->
    [
        {default, [parallel], [
            adapter_unreachable_route_test,
-            adapter_unreachable_quote_test
+            adapter_unreachable_quote_test,
+            attempt_limit_test
        ]}
    ].

@ -162,6 +164,28 @@ adapter_unreachable_quote_test(C) ->
        {failed, #{code => <<"authorization_error">>}},
        await_final_withdrawal_status(WithdrawalID)).

+
+-spec attempt_limit_test(config()) -> test_return().
+attempt_limit_test(C) ->
+    Currency = <<"RUB">>,
+    Cash = {500100, Currency},
+    #{
+        wallet_id := WalletID,
+        destination_id := DestinationID
+    } = prepare_standard_environment(Cash, C),
+    WithdrawalID = generate_id(),
+    WithdrawalParams = #{
+        id => WithdrawalID,
+        destination_id => DestinationID,
+        wallet_id => WalletID,
+        body => Cash,
+        external_id => WithdrawalID
+    },
+    ok = ff_withdrawal_machine:create(WithdrawalParams, ff_entity_context:new()),
+    ?assertEqual(
+        {failed, #{code => <<"authorization_error">>}},
+        await_final_withdrawal_status(WithdrawalID)).
+
 %% Utils

 get_withdrawal(WithdrawalID) ->
--- a/apps/fistful/src/ff_dmsl_codec.erl
+++ b/apps/fistful/src/ff_dmsl_codec.erl
@ -180,6 +180,11 @@ unmarshal(exp_date, #'domain_BankCardExpDate'{
 }) ->
    {unmarshal(integer, Month), unmarshal(integer, Year)};

+unmarshal(attempt_limit, #domain_AttemptLimit{
+    attempts = Attempts
+}) ->
+    unmarshal(integer, Attempts);
+
 unmarshal(amount, V) ->
    unmarshal(integer, V);
 unmarshal(string, V) when is_binary(V) ->
@ -283,6 +288,11 @@ marshal(p2p_tool, {Sender, Receiver}) ->
        receiver = marshal(payment_tool, Receiver)
    };

+marshal(attempt_limit, Limit) ->
+    #domain_AttemptLimit{
+        attempts = Limit
+    };
+
 marshal(risk_score, low) ->
    low;
 marshal(risk_score, high) ->
--- a/apps/fistful/src/ff_party.erl
+++ b/apps/fistful/src/ff_party.erl
@ -17,6 +17,7 @@
 -type clock()       :: ff_transaction:clock().
 -type revision()    :: dmsl_domain_thrift:'PartyRevision'().
 -type terms()       :: dmsl_domain_thrift:'TermSet'().
+-type attempt_limit() :: integer().

 -type party_params() :: #{
    email := binary()
@ -70,6 +71,7 @@
 -export_type([validate_p2p_template_creation_error/0]).
 -export_type([cash/0]).
 -export_type([cash_range/0]).
+-export_type([attempt_limit/0]).

 -type inaccessibility() ::
    {inaccessible, blocked | suspended}.
@ -119,6 +121,7 @@
 -type p2p_forbidden_error() :: {terms_violation, p2p_forbidden}.
 -type w2w_forbidden_error() :: {terms_violation, w2w_forbidden}.
 -type p2p_template_forbidden_error() :: {terms_violation, p2p_template_forbidden}.
+-type attempt_limit_error() :: {terms_violation, {attempt_limit, attempt_limit()}}.

 -type not_reduced_error() :: {not_reduced, {Name :: atom(), TermsPart :: any()}}.

@ -306,7 +309,8 @@ validate_withdrawal_creation(Terms, {_, CurrencyID} = Cash) ->
        valid = unwrap(validate_wallet_terms_currency(CurrencyID, WalletTerms)),
        #domain_WalletServiceTerms{withdrawals = WithdrawalTerms} = WalletTerms,
        valid = unwrap(validate_withdrawal_terms_currency(CurrencyID, WithdrawalTerms)),
-        valid = unwrap(validate_withdrawal_cash_limit(Cash, WithdrawalTerms))
+        valid = unwrap(validate_withdrawal_cash_limit(Cash, WithdrawalTerms)),
+        valid = unwrap(validate_withdrawal_attempt_limit(WithdrawalTerms))
    end).

 -spec validate_deposit_creation(terms(), cash()) -> Result when
@ -603,13 +607,15 @@ validate_withdrawal_terms_is_reduced(Terms) ->
    #domain_WithdrawalServiceTerms{
        currencies = WithdrawalCurrenciesSelector,
        cash_limit = CashLimitSelector,
-        cash_flow = CashFlowSelector
+        cash_flow = CashFlowSelector,
+        attempt_limit = AttemptLimitSelector
    } = WithdrawalTerms,
    do_validate_terms_is_reduced([
        {wallet_currencies, WalletCurrenciesSelector},
        {withdrawal_currencies, WithdrawalCurrenciesSelector},
        {withdrawal_cash_limit, CashLimitSelector},
-        {withdrawal_cash_flow, CashFlowSelector}
+        {withdrawal_cash_flow, CashFlowSelector},
+        {withdrawal_attempt_limit, AttemptLimitSelector}
    ]).

 -spec validate_p2p_terms_is_reduced(wallet_terms() | undefined) ->
@ -752,6 +758,20 @@ validate_withdrawal_cash_limit(Cash, Terms) ->
    } = Terms,
    validate_cash_range(ff_dmsl_codec:marshal(cash, Cash), CashRange).

+-spec validate_withdrawal_attempt_limit(withdrawal_terms()) ->
+    {ok, valid} | {error, attempt_limit_error()}.
+
+validate_withdrawal_attempt_limit(Terms) ->
+    #domain_WithdrawalServiceTerms{
+        attempt_limit = AttemptLimit
+    } = Terms,
+    case AttemptLimit of
+        undefined ->
+            {ok, valid};
+        {value, Limit} ->
+            validate_attempt_limit(ff_dmsl_codec:unmarshal(attempt_limit, Limit))
+    end.
+
 -spec validate_p2p_terms_currency(currency_id(), p2p_terms()) ->
    {ok, valid} | {error, currency_validation_error()}.
 validate_p2p_terms_currency(CurrencyID, Terms) ->
@ -857,6 +877,14 @@ compare_cash(
 ) ->
    Fun(A, Am).

+-spec validate_attempt_limit(attempt_limit()) ->
+    {ok, valid} | {error, attempt_limit_error()}.
+
+validate_attempt_limit(AttemptLimit) when AttemptLimit > 0 ->
+    {ok, valid};
+validate_attempt_limit(AttemptLimit) ->
+    {error, {terms_violation, {attempt_limit, AttemptLimit}}}.
+
 %% Varset stuff

 -spec encode_varset(hg_selector:varset()) ->
--- a/docker-compose.sh
+++ b/docker-compose.sh
@ -48,7 +48,7 @@ services:
      retries: 10

  hellgate:
-    image: dr2.rbkmoney.com/rbkmoney/hellgate:ff2426f008091b9cc81f064a990572a2198c581b
+    image: dr2.rbkmoney.com/rbkmoney/hellgate:59ec5840712eb6e5406a7f69eca7b8f04d831416
    command: /opt/hellgate/bin/hellgate foreground
    depends_on:
      machinegun:
@ -88,7 +88,7 @@ services:
      retries: 20

  dominant:
-    image: dr2.rbkmoney.com/rbkmoney/dominant:9f0da27b9f3c853e63c72b62a47f7f1e76f8a967
+    image: dr2.rbkmoney.com/rbkmoney/dominant:6d389d77f2a65edbf8accbbe7af7840157c7e57d
    command: /opt/dominant/bin/dominant foreground
    depends_on:
      machinegun:
--- a/rebar.lock
+++ b/rebar.lock
@ -39,7 +39,7 @@
 {<<"cowlib">>,{pkg,<<"cowlib">>,<<"2.8.0">>},1},
 {<<"damsel">>,
  {git,"git@github.com:rbkmoney/damsel.git",
-       {ref,"290a7441a55c534899eee020b0f4d4e3b0464f9d"}},
+       {ref,"e9c18627e5cc5d6d95dbd820b323bf6165b0782e"}},
  0},
 {<<"dmt_client">>,
  {git,"git@github.com:rbkmoney/dmt_client.git",