TD-550: Respect limiter hold business errors (#56)

2024-11-06 02:35:18 +00:00 · 2023-04-28 10:50:13 +03:00 · 2023-04-28 10:50:13 +03:00 · 4906d387a3
commit 4906d387a3
parent cc3c9ed8ca
5 changed files with 162 additions and 19 deletions
--- a/apps/ff_transfer/src/ff_limiter.erl
+++ b/apps/ff_transfer/src/ff_limiter.erl
@ -65,7 +65,7 @@ check_limits_(T, {Limits, Errors}, Context) ->
            {Limits, [{LimitID, LimitAmount, UpperBoundary} | Errors]}
    end.

-spec hold_withdrawal_limits([turnover_limit()], route(), withdrawal(), pos_integer()) -> ok.
+-spec hold_withdrawal_limits([turnover_limit()], route(), withdrawal(), pos_integer()) -> ok | no_return().
 hold_withdrawal_limits(TurnoverLimits, Route, Withdrawal, Iter) ->
    LimitChanges = gen_limit_changes(TurnoverLimits, Route, Withdrawal, Iter),
    Context = gen_limit_context(Route, Withdrawal),
@ -83,7 +83,7 @@ rollback_withdrawal_limits(TurnoverLimits, Route, Withdrawal, Iter) ->
    Context = gen_limit_context(Route, Withdrawal),
    rollback(LimitChanges, get_latest_clock(), Context).

-spec hold([limit_change()], clock(), context()) -> ok.
+-spec hold([limit_change()], clock(), context()) -> ok | no_return().
 hold(LimitChanges, Clock, Context) ->
    lists:foreach(
        fun(LimitChange) ->
@ -204,11 +204,15 @@ get(LimitID, Version, Clock, Context) ->
            error({invalid_request, Errors})
    end.

-spec call_hold(limit_change(), clock(), context()) -> clock().
+-spec call_hold(limit_change(), clock(), context()) -> clock() | no_return().
 call_hold(LimitChange, Clock, Context) ->
    Args = {LimitChange, Clock, Context},
-    {ok, ClockUpdated} = call('Hold', Args),
-    ClockUpdated.
+    case call('Hold', Args) of
+        {ok, ClockUpdated} ->
+            ClockUpdated;
+        {exception, Exception} ->
+            error(Exception)
+    end.

 -spec call_commit(limit_change(), clock(), context()) -> clock().
 call_commit(LimitChange, Clock, Context) ->
@ -219,8 +223,13 @@ call_commit(LimitChange, Clock, Context) ->
 -spec call_rollback(limit_change(), clock(), context()) -> clock().
 call_rollback(LimitChange, Clock, Context) ->
    Args = {LimitChange, Clock, Context},
-    {ok, ClockUpdated} = call('Rollback', Args),
-    ClockUpdated.
+    case call('Rollback', Args) of
+        {ok, ClockUpdated} -> ClockUpdated;
+        %% Always ignore business exceptions on rollback and compatibility return latest clock
+        {exception, #limiter_InvalidOperationCurrency{}} -> {latest, #limiter_LatestClock{}};
+        {exception, #limiter_OperationContextNotSupported{}} -> {latest, #limiter_LatestClock{}};
+        {exception, #limiter_PaymentToolNotSupported{}} -> {latest, #limiter_LatestClock{}}
+    end.

 call(Func, Args) ->
    Service = {limproto_limiter_thrift, 'Limiter'},
--- a/apps/ff_transfer/src/ff_withdrawal_routing.erl
+++ b/apps/ff_transfer/src/ff_withdrawal_routing.erl
@ -1,6 +1,7 @@
 -module(ff_withdrawal_routing).

 -include_lib("damsel/include/dmsl_domain_thrift.hrl").
+-include_lib("limiter_proto/include/limproto_limiter_thrift.hrl").

 -export([prepare_routes/2]).
 -export([prepare_routes/3]).
@ -391,12 +392,21 @@ validate_cash_limit(_NotReducedSelector, _VS) ->
 validate_turnover_limits(undefined, _VS, _Route, _RoutingContext) ->
    {ok, valid};
 validate_turnover_limits({value, TurnoverLimits}, _VS, Route, #{withdrawal := Withdrawal, iteration := Iter}) ->
-    ok = ff_limiter:hold_withdrawal_limits(TurnoverLimits, Route, Withdrawal, Iter),
-    case ff_limiter:check_limits(TurnoverLimits, Route, Withdrawal) of
-        {ok, _} ->
-            {ok, valid};
-        {error, Error} ->
-            {error, {terms_violation, Error}}
+    try
+        ok = ff_limiter:hold_withdrawal_limits(TurnoverLimits, Route, Withdrawal, Iter),
+        case ff_limiter:check_limits(TurnoverLimits, Route, Withdrawal) of
+            {ok, _} ->
+                {ok, valid};
+            {error, Error} ->
+                {error, {terms_violation, Error}}
+        end
+    catch
+        error:(#limiter_InvalidOperationCurrency{} = LimitError) ->
+            {error, {limit_hold_error, LimitError}};
+        error:(#limiter_OperationContextNotSupported{} = LimitError) ->
+            {error, {limit_hold_error, LimitError}};
+        error:(#limiter_PaymentToolNotSupported{} = LimitError) ->
+            {error, {limit_hold_error, LimitError}}
    end;
 validate_turnover_limits(NotReducedSelector, _VS, _Route, _RoutingContext) ->
    {error, {misconfiguration, {'Could not reduce selector to a value', NotReducedSelector}}}.
--- a/apps/ff_transfer/test/ff_withdrawal_limits_SUITE.erl
+++ b/apps/ff_transfer/test/ff_withdrawal_limits_SUITE.erl
@ -3,6 +3,10 @@
 -include_lib("stdlib/include/assert.hrl").
 -include_lib("ff_cth/include/ct_domain.hrl").
 -include_lib("damsel/include/dmsl_wthd_domain_thrift.hrl").
+-include_lib("limiter_proto/include/limproto_limiter_thrift.hrl").
+-include_lib("damsel/include/dmsl_limiter_config_thrift.hrl").
+-include_lib("limiter_proto/include/limproto_base_thrift.hrl").
+-include_lib("limiter_proto/include/limproto_context_withdrawal_thrift.hrl").

 %% Common test API

@ -18,6 +22,10 @@
 %% Tests
 -export([limit_success/1]).
 -export([limit_overflow/1]).
+-export([limit_hold_currency_error/1]).
+-export([limit_hold_operation_error/1]).
+-export([limit_hold_paytool_error/1]).
+-export([limit_hold_error_two_routes_failure/1]).
 -export([choose_provider_without_limit_overflow/1]).
 -export([provider_limits_exhaust_orderly/1]).
 -export([provider_retry/1]).
@ -45,6 +53,10 @@ groups() ->
        {default, [sequence], [
            limit_success,
            limit_overflow,
+            limit_hold_currency_error,
+            limit_hold_operation_error,
+            limit_hold_paytool_error,
+            limit_hold_error_two_routes_failure,
            choose_provider_without_limit_overflow,
            provider_limits_exhaust_orderly,
            provider_retry,
@ -84,7 +96,19 @@ end_per_group(_, _) ->
 %%

 -spec init_per_testcase(test_case_name(), config()) -> config().
+init_per_testcase(Name, C0) when
+    Name =:= limit_hold_currency_error orelse
+        Name =:= limit_hold_operation_error orelse
+        Name =:= limit_hold_paytool_error orelse
+        Name =:= limit_hold_error_two_routes_failure
+->
+    C1 = do_init_per_testcase(Name, C0),
+    meck:new(ff_woody_client, [no_link, passthrough]),
+    C1;
 init_per_testcase(Name, C0) ->
+    do_init_per_testcase(Name, C0).
+
+do_init_per_testcase(Name, C0) ->
    C1 = ct_helper:makeup_cfg(
        [
            ct_helper:test_case_name(Name),
@ -108,7 +132,18 @@ init_per_testcase(Name, C0) ->
    C2.

 -spec end_per_testcase(test_case_name(), config()) -> _.
+end_per_testcase(Name, C) when
+    Name =:= limit_hold_currency_error orelse
+        Name =:= limit_hold_operation_error orelse
+        Name =:= limit_hold_paytool_error orelse
+        Name =:= limit_hold_error_two_routes_failure
+->
+    meck:unload(ff_woody_client),
+    do_end_per_testcase(Name, C);
 end_per_testcase(Name, C) ->
+    do_end_per_testcase(Name, C).
+
+do_end_per_testcase(Name, C) ->
    case Name of
        Name when
            Name =:= provider_retry orelse
@ -171,6 +206,95 @@ limit_overflow(C) ->
    Withdrawal = get_withdrawal(WithdrawalID),
    ?assertEqual(PreviousAmount, ff_limiter_helper:get_limit_amount(?LIMIT_TURNOVER_NUM_PAYTOOL_ID2, Withdrawal, C)).

+-spec limit_hold_currency_error(config()) -> test_return().
+limit_hold_currency_error(C) ->
+    mock_limiter_trm_hold(?trm(1800), fun(_LimitChange, _Clock, _Context) ->
+        {exception, #limiter_InvalidOperationCurrency{currency = <<"RUB">>, expected_currency = <<"KEK">>}}
+    end),
+    limit_hold_error(C).
+
+-spec limit_hold_operation_error(config()) -> test_return().
+limit_hold_operation_error(C) ->
+    mock_limiter_trm_hold(?trm(1800), fun(_LimitChange, _Clock, _Context) ->
+        {exception, #limiter_OperationContextNotSupported{
+            context_type = {withdrawal_processing, #limiter_config_LimitContextTypeWithdrawalProcessing{}}
+        }}
+    end),
+    limit_hold_error(C).
+
+-spec limit_hold_paytool_error(config()) -> test_return().
+limit_hold_paytool_error(C) ->
+    mock_limiter_trm_hold(?trm(1800), fun(_LimitChange, _Clock, _Context) ->
+        {exception, #limiter_PaymentToolNotSupported{payment_tool = <<"unsupported paytool">>}}
+    end),
+    limit_hold_error(C).
+
+-spec limit_hold_error_two_routes_failure(config()) -> test_return().
+limit_hold_error_two_routes_failure(C) ->
+    mock_limiter_trm_call(?trm(2000), fun(_LimitChange, _Clock, _Context) ->
+        {exception, #limiter_PaymentToolNotSupported{payment_tool = <<"unsupported paytool">>}}
+    end),
+    %% See `?ruleset(?PAYINST1_ROUTING_POLICIES + 18)` with two candidates in `ct_payment_system:domain_config/1`.
+    Cash = {901000, <<"RUB">>},
+    #{
+        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()),
+    Result = await_final_withdrawal_status(WithdrawalID),
+    ?assertMatch({failed, #{code := <<"no_route_found">>}}, Result).
+
+-define(LIMITER_REQUEST(Func, TerminalRef), {
+    {limproto_limiter_thrift, 'Limiter'},
+    Func,
+    {_LimitChange, _Clock, #limiter_LimitContext{
+        withdrawal_processing = #context_withdrawal_Context{
+            withdrawal = #context_withdrawal_Withdrawal{route = #base_Route{terminal = TerminalRef}}
+        }
+    }}
+}).
+mock_limiter_trm_hold(ExpectTerminalRef, ReturnFunc) ->
+    ok = meck:expect(ff_woody_client, call, fun
+        (limiter, {_, _, Args} = ?LIMITER_REQUEST('Hold', TerminalRef)) when TerminalRef =:= ExpectTerminalRef ->
+            apply(ReturnFunc, tuple_to_list(Args));
+        (Service, Request) ->
+            meck:passthrough([Service, Request])
+    end).
+
+mock_limiter_trm_call(ExpectTerminalRef, ReturnFunc) ->
+    ok = meck:expect(ff_woody_client, call, fun
+        (limiter, {_, _, Args} = ?LIMITER_REQUEST(_Func, TerminalRef)) when TerminalRef =:= ExpectTerminalRef ->
+            apply(ReturnFunc, tuple_to_list(Args));
+        (Service, Request) ->
+            meck:passthrough([Service, Request])
+    end).
+
+limit_hold_error(C) ->
+    Cash = {800800, <<"RUB">>},
+    #{
+        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()),
+    Result = await_final_withdrawal_status(WithdrawalID),
+    ?assertMatch({failed, #{code := <<"no_route_found">>}}, Result).
+
 -spec choose_provider_without_limit_overflow(config()) -> test_return().
 choose_provider_without_limit_overflow(C) ->
    Cash = {901000, <<"RUB">>},
--- a/docker-compose.yml
+++ b/docker-compose.yml
@ -28,7 +28,7 @@ services:
    command: /sbin/init

  dominant:
-    image: ghcr.io/valitydev/dominant:sha-e0afa44
+    image: ghcr.io/valitydev/dominant:sha-ecd7531
    command: /opt/dominant/bin/dominant foreground
    depends_on:
      machinegun:
@ -40,7 +40,7 @@ services:
      retries: 10

  machinegun:
-    image: ghcr.io/valitydev/machinegun:sha-fb7fbf9
+    image: ghcr.io/valitydev/machinegun:sha-058bada
    command: /opt/machinegun/bin/machinegun foreground
    volumes:
      - ./test/machinegun/config.yaml:/opt/machinegun/etc/config.yaml
@ -52,7 +52,7 @@ services:
      retries: 10

  limiter:
-    image: ghcr.io/valitydev/limiter:sha-3eff7dd
+    image: ghcr.io/valitydev/limiter:sha-2b8723b
    command: /opt/limiter/bin/limiter foreground
    depends_on:
      machinegun:
@ -87,7 +87,7 @@ services:
      retries: 20

  party-management:
-    image: ghcr.io/valitydev/party-management:sha-57d4d64
+    image: ghcr.io/valitydev/party-management:sha-4a94036
    command: /opt/party-management/bin/party-management foreground
    depends_on:
      machinegun:
--- a/rebar.lock
+++ b/rebar.lock
@ -22,7 +22,7 @@
 {<<"cowlib">>,{pkg,<<"cowlib">>,<<"2.11.0">>},2},
 {<<"damsel">>,
  {git,"https://github.com/valitydev/damsel.git",
-       {ref,"2d6fd01208aa2649b4efef0c1d19abc6a1dc5210"}},
+       {ref,"e12c03c51378a40f63d5c1805b4adeaae73e372b"}},
  0},
 {<<"dmt_client">>,
  {git,"https://github.com/valitydev/dmt_client.git",
@ -50,7 +50,7 @@
 {<<"jsx">>,{pkg,<<"jsx">>,<<"3.1.0">>},1},
 {<<"limiter_proto">>,
  {git,"https://github.com/valitydev/limiter-proto.git",
-       {ref,"9b76200a957c0e91bcdf6f16dfbab90d38a3f173"}},
+       {ref,"bbd2c0dce044dd5b4e424fc8e38a0023a1685a22"}},
  0},
 {<<"machinery">>,
  {git,"https://github.com/valitydev/machinery-erlang.git",