Reverts leniency towards notfound limit-configs for v1 (#44)

* Revert "Makes get_batch/get_values lenient to nonexistent limit-configs too (#42)"

This reverts commit 36eb612.

* Partially reverts "Supresses `rollback_batch` exception for notfound limit-configs (#41)"

Author: nanodirijabl

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)),
         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, lenient)),
-                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) ->

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).
 
@@ -65,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).
 

apps/limiter/test/lim_turnover_SUITE.erl

@@ -69,9 +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([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]).
@@ -141,9 +138,6 @@ groups() ->
             batch_hold_ok,
             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 +877,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 =
@@ -921,46 +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 = add_non_existent_limit_config(Request0),
-    {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 =