From 4906d387a359211b41dda831f91ccd9c468a70b8 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Fri, 28 Apr 2023 10:50:13 +0300 Subject: [PATCH] TD-550: Respect limiter hold business errors (#56) --- apps/ff_transfer/src/ff_limiter.erl | 23 +++- .../ff_transfer/src/ff_withdrawal_routing.erl | 22 +++- .../test/ff_withdrawal_limits_SUITE.erl | 124 ++++++++++++++++++ docker-compose.yml | 8 +- rebar.lock | 4 +- 5 files changed, 162 insertions(+), 19 deletions(-) diff --git a/apps/ff_transfer/src/ff_limiter.erl b/apps/ff_transfer/src/ff_limiter.erl index 19bfde8..8f67dd9 100644 --- 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'}, diff --git a/apps/ff_transfer/src/ff_withdrawal_routing.erl b/apps/ff_transfer/src/ff_withdrawal_routing.erl index 6055c41..3110cd9 100644 --- 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}}}. diff --git a/apps/ff_transfer/test/ff_withdrawal_limits_SUITE.erl b/apps/ff_transfer/test/ff_withdrawal_limits_SUITE.erl index 6a3ae2f..987a8f7 100644 --- 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">>}, diff --git a/docker-compose.yml b/docker-compose.yml index 323439b..60064cf 100644 --- 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: diff --git a/rebar.lock b/rebar.lock index f3eaa35..fa3e089 100644 --- 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",