From 2713294427658ff15bd1e55f7e49042a8a32cfa9 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Mon, 6 Oct 2025 15:12:50 +0300 Subject: [PATCH 1/2] Revert "Makes get_batch/get_values lenient to nonexistent limit-configs too (#42)" This reverts commit 36eb61242c5b48cd338a4c5c7247b0c10a5366bc. --- apps/limiter/src/lim_config_machine.erl | 4 +-- apps/limiter/src/lim_liminator.erl | 4 --- apps/limiter/test/lim_turnover_SUITE.erl | 42 ++++-------------------- 3 files changed, 8 insertions(+), 42 deletions(-) diff --git a/apps/limiter/src/lim_config_machine.erl b/apps/limiter/src/lim_config_machine.erl index f56f3ee..dcf46c5 100644 --- a/apps/limiter/src/lim_config_machine.erl +++ b/apps/limiter/src/lim_config_machine.erl @@ -305,7 +305,7 @@ rollback(#limiter_LimitChange{id = ID, version = Version} = LimitChange, LimitCo {ok, [lim_liminator:limit_response()]} | {error, config_error() | {processor(), get_limit_error()}}. get_values(LimitChanges, LimitContext) -> do(fun() -> - Changes = unwrap(collect_changes(hold, LimitChanges, LimitContext, lenient)), + Changes = unwrap(collect_changes(hold, LimitChanges, LimitContext, strict)), Names = lists:map(fun lim_liminator:get_name/1, Changes), unwrap(lim_liminator:get_values(Names, LimitContext)) end). @@ -318,7 +318,7 @@ get_batch(OperationID, LimitChanges, LimitContext) -> OperationID, lim_liminator:get( OperationID, - unwrap(collect_changes(hold, LimitChanges, LimitContext, lenient)), + unwrap(collect_changes(hold, LimitChanges, LimitContext, strict)), LimitContext ) ) diff --git a/apps/limiter/src/lim_liminator.erl b/apps/limiter/src/lim_liminator.erl index d965cdf..a2813cb 100644 --- a/apps/limiter/src/lim_liminator.erl +++ b/apps/limiter/src/lim_liminator.erl @@ -43,15 +43,11 @@ get_name(#liminator_LimitChange{limit_name = Name}) -> -spec get_values([limit_name()], lim_context()) -> {ok, [limit_response()]} | {error, invalid_request_error()}. -get_values([], _LimitContext) -> - {ok, []}; get_values(Names, LimitContext) -> do('GetLastLimitsValues', Names, LimitContext). -spec get(operation_id(), [limit_change()], lim_context()) -> {ok, [limit_response()]} | {error, invalid_request_error()}. -get(_OperationID, [], _LimitContext) -> - {ok, []}; get(OperationID, Changes, LimitContext) -> do('Get', #liminator_LimitRequest{operation_id = OperationID, limit_changes = Changes}, LimitContext). diff --git a/apps/limiter/test/lim_turnover_SUITE.erl b/apps/limiter/test/lim_turnover_SUITE.erl index bdc531f..f418c34 100644 --- a/apps/limiter/test/lim_turnover_SUITE.erl +++ b/apps/limiter/test/lim_turnover_SUITE.erl @@ -70,8 +70,6 @@ -export([batch_commit_ok/1]). -export([batch_rollback_ok/1]). -export([batch_rollback_lenient_to_config_notfound_ok/1]). --export([batch_get_values_lenient_to_config_notfound_ok/1]). --export([batch_hold_strict_to_config_notfound_ok/1]). -export([two_batch_hold_ok/1]). -export([two_batch_commit_ok/1]). -export([two_batch_rollback_ok/1]). @@ -142,8 +140,6 @@ groups() -> batch_commit_ok, batch_rollback_ok, batch_rollback_lenient_to_config_notfound_ok, - batch_get_values_lenient_to_config_notfound_ok, - batch_hold_strict_to_config_notfound_ok, two_batch_hold_ok, two_batch_commit_ok, two_batch_rollback_ok, @@ -883,10 +879,6 @@ construct_request(C) -> ?LIMIT_CHANGE(ID2, 0, Version2) ]). -add_non_existent_limit_config(?LIMIT_REQUEST(_ID, Changes) = Request) -> - NonexistentChange = ?LIMIT_CHANGE(<<"this-does-not-exist">>, 0, dmt_client:get_last_version()), - Request#limiter_LimitRequest{limit_changes = [NonexistentChange | Changes]}. - -spec batch_hold_ok(config()) -> _. batch_hold_ok(C) -> Context = @@ -930,37 +922,15 @@ batch_rollback_lenient_to_config_notfound_ok(C) -> end, Request0 = construct_request(C), ok = hold_and_assert_batch(10, Request0, Context, C), - Request1 = add_non_existent_limit_config(Request0), + Request1 = Request0#limiter_LimitRequest{ + limit_changes = [ + ?LIMIT_CHANGE(<<"this-does-not-exist">>, 0, dmt_client:get_last_version()) + | Request0#limiter_LimitRequest.limit_changes + ] + }, {ok, ok} = lim_client:rollback_batch(Request1, Context, ?config(client, C)), ok = assert_values(0, Request0, Context, C). --spec batch_get_values_lenient_to_config_notfound_ok(config()) -> _. -batch_get_values_lenient_to_config_notfound_ok(C) -> - Context = - case get_group_name(C) of - withdrawals -> ?wthdproc_ctx_withdrawal(?cash(10)); - _Default -> ?payproc_ctx_payment(?cash(10), ?cash(10)) - end, - Request0 = construct_request(C), - ok = hold_and_assert_batch(10, Request0, Context, C), - Request1 = add_non_existent_limit_config(Request0), - ok = assert_batch(10, Request1, Context, C), - ok = assert_values(10, Request1, Context, C). - --spec batch_hold_strict_to_config_notfound_ok(config()) -> _. -batch_hold_strict_to_config_notfound_ok(C) -> - Context = - case get_group_name(C) of - withdrawals -> ?wthdproc_ctx_withdrawal(?cash(10)); - _Default -> ?payproc_ctx_payment(?cash(10), ?cash(10)) - end, - Request0 = construct_request(C), - Request1 = add_non_existent_limit_config(Request0), - ?assertEqual( - {exception, #limiter_LimitNotFound{}}, - lim_client:hold_batch(Request1, Context, ?config(client, C)) - ). - -spec two_batch_hold_ok(config()) -> _. two_batch_hold_ok(C) -> Context = From 5ac9862b353e49558843afc090e3af3f9f9d40a8 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Mon, 6 Oct 2025 15:17:54 +0300 Subject: [PATCH 2/2] Partially reverts "Supresses `rollback_batch` exception for notfound limit-configs (#41)" --- apps/limiter/src/lim_config_machine.erl | 43 ++++++------------------ apps/limiter/src/lim_liminator.erl | 2 -- apps/limiter/test/lim_turnover_SUITE.erl | 20 ----------- 3 files changed, 10 insertions(+), 55 deletions(-) diff --git a/apps/limiter/src/lim_config_machine.erl b/apps/limiter/src/lim_config_machine.erl index dcf46c5..4884c68 100644 --- a/apps/limiter/src/lim_config_machine.erl +++ b/apps/limiter/src/lim_config_machine.erl @@ -305,7 +305,7 @@ rollback(#limiter_LimitChange{id = ID, version = Version} = LimitChange, LimitCo {ok, [lim_liminator:limit_response()]} | {error, config_error() | {processor(), get_limit_error()}}. get_values(LimitChanges, LimitContext) -> do(fun() -> - Changes = unwrap(collect_changes(hold, LimitChanges, LimitContext, strict)), + Changes = unwrap(collect_changes(hold, LimitChanges, LimitContext)), Names = lists:map(fun lim_liminator:get_name/1, Changes), unwrap(lim_liminator:get_values(Names, LimitContext)) end). @@ -316,11 +316,7 @@ get_batch(OperationID, LimitChanges, LimitContext) -> do(fun() -> unwrap( OperationID, - lim_liminator:get( - OperationID, - unwrap(collect_changes(hold, LimitChanges, LimitContext, strict)), - LimitContext - ) + lim_liminator:get(OperationID, unwrap(collect_changes(hold, LimitChanges, LimitContext)), LimitContext) ) end). @@ -330,11 +326,7 @@ hold_batch(OperationID, LimitChanges, LimitContext) -> do(fun() -> unwrap( OperationID, - lim_liminator:hold( - OperationID, - unwrap(collect_changes(hold, LimitChanges, LimitContext, strict)), - LimitContext - ) + lim_liminator:hold(OperationID, unwrap(collect_changes(hold, LimitChanges, LimitContext)), LimitContext) ) end). @@ -344,11 +336,7 @@ commit_batch(OperationID, LimitChanges, LimitContext) -> do(fun() -> unwrap( OperationID, - lim_liminator:commit( - OperationID, - unwrap(collect_changes(commit, LimitChanges, LimitContext, strict)), - LimitContext - ) + lim_liminator:commit(OperationID, unwrap(collect_changes(commit, LimitChanges, LimitContext)), LimitContext) ) end). @@ -358,28 +346,17 @@ rollback_batch(OperationID, LimitChanges, LimitContext) -> do(fun() -> unwrap( OperationID, - lim_liminator:rollback( - OperationID, - unwrap(collect_changes(hold, LimitChanges, LimitContext, lenient)), - LimitContext - ) + lim_liminator:rollback(OperationID, unwrap(collect_changes(hold, LimitChanges, LimitContext)), LimitContext) ) end). -collect_changes(_Stage, [], _LimitContext, _Mode) -> +collect_changes(_Stage, [], _LimitContext) -> {ok, []}; -collect_changes(Stage, [LimitChange = #limiter_LimitChange{id = ID, version = Version} | Other], LimitContext, Mode) -> +collect_changes(Stage, [LimitChange = #limiter_LimitChange{id = ID, version = Version} | Other], LimitContext) -> do(fun() -> - case {Mode, get_handler(ID, Version, LimitContext)} of - {lenient, {error, {config, notfound}}} -> - %% TODO Log that this limit-change was ignored because the - %% mentioned limit-config was not found. - unwrap(collect_changes(Stage, Other, LimitContext, Mode)); - {_, Result} -> - {Handler, Config} = unwrap(Result), - Change = unwrap(Handler, Handler:make_change(Stage, LimitChange, Config, LimitContext)), - [Change | unwrap(collect_changes(Stage, Other, LimitContext, Mode))] - end + {Handler, Config} = unwrap(get_handler(ID, Version, LimitContext)), + Change = unwrap(Handler, Handler:make_change(Stage, LimitChange, Config, LimitContext)), + [Change | unwrap(collect_changes(Stage, Other, LimitContext))] end). get_handler(ID, Version, LimitContext) -> diff --git a/apps/limiter/src/lim_liminator.erl b/apps/limiter/src/lim_liminator.erl index a2813cb..3970658 100644 --- a/apps/limiter/src/lim_liminator.erl +++ b/apps/limiter/src/lim_liminator.erl @@ -61,8 +61,6 @@ commit(OperationID, Changes, LimitContext) -> do('Commit', #liminator_LimitRequest{operation_id = OperationID, limit_changes = Changes}, LimitContext). -spec rollback(operation_id(), [limit_change()], lim_context()) -> {ok, ok} | {error, invalid_request_error()}. -rollback(_OperationID, [], _LimitContext) -> - {ok, ok}; rollback(OperationID, Changes, LimitContext) -> do('Rollback', #liminator_LimitRequest{operation_id = OperationID, limit_changes = Changes}, LimitContext). diff --git a/apps/limiter/test/lim_turnover_SUITE.erl b/apps/limiter/test/lim_turnover_SUITE.erl index f418c34..27ac719 100644 --- a/apps/limiter/test/lim_turnover_SUITE.erl +++ b/apps/limiter/test/lim_turnover_SUITE.erl @@ -69,7 +69,6 @@ -export([batch_hold_ok/1]). -export([batch_commit_ok/1]). -export([batch_rollback_ok/1]). --export([batch_rollback_lenient_to_config_notfound_ok/1]). -export([two_batch_hold_ok/1]). -export([two_batch_commit_ok/1]). -export([two_batch_rollback_ok/1]). @@ -139,7 +138,6 @@ groups() -> batch_hold_ok, batch_commit_ok, batch_rollback_ok, - batch_rollback_lenient_to_config_notfound_ok, two_batch_hold_ok, two_batch_commit_ok, two_batch_rollback_ok, @@ -913,24 +911,6 @@ batch_rollback_ok(C) -> {ok, ok} = lim_client:rollback_batch(Request, Context, ?config(client, C)), ok = assert_values(0, Request, Context, C). --spec batch_rollback_lenient_to_config_notfound_ok(config()) -> _. -batch_rollback_lenient_to_config_notfound_ok(C) -> - Context = - case get_group_name(C) of - withdrawals -> ?wthdproc_ctx_withdrawal(?cash(10)); - _Default -> ?payproc_ctx_payment(?cash(10), ?cash(10)) - end, - Request0 = construct_request(C), - ok = hold_and_assert_batch(10, Request0, Context, C), - Request1 = Request0#limiter_LimitRequest{ - limit_changes = [ - ?LIMIT_CHANGE(<<"this-does-not-exist">>, 0, dmt_client:get_last_version()) - | Request0#limiter_LimitRequest.limit_changes - ] - }, - {ok, ok} = lim_client:rollback_batch(Request1, Context, ?config(client, C)), - ok = assert_values(0, Request0, Context, C). - -spec two_batch_hold_ok(config()) -> _. two_batch_hold_ok(C) -> Context =