Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apps/ff_cth/src/ct_domain_config.erl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

-export([head/0]).

-export([all/1]).
-export([checkout_object/2]).
-export([commit/2]).
-export([insert/1]).
-export([update/1]).
Expand All @@ -21,6 +23,7 @@

-type revision() :: dmt_client:version().
-type object() :: dmsl_domain_thrift:'DomainObject'().
-type object_ref() :: dmt_client:object_ref().

-spec head() -> revision().
head() ->
Expand All @@ -31,6 +34,11 @@ all(Revision) ->
#'Snapshot'{domain = Domain} = dmt_client:checkout(Revision),
Domain.

-spec checkout_object(revision(), object_ref()) -> object() | no_return().
checkout_object(Revision, ObjectRef) ->
#'Snapshot'{domain = Domain} = dmt_client:checkout(Revision),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В dmt_client теперь же есть оптимальный checkout_object.

maps:get(ObjectRef, Domain).

-spec commit(revision(), dmt_client:commit()) -> revision() | no_return().
commit(Revision, Commit) ->
dmt_client:commit(Revision, Commit).
Expand Down
2 changes: 1 addition & 1 deletion apps/ff_server/src/ff_server.erl
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ init([]) ->
{withdrawal_session_management, ff_withdrawal_session_handler},
{deposit_management, ff_deposit_handler},
{withdrawal_session_repairer, ff_withdrawal_session_repair},
{withdrawal_repairer, ff_withdrawal_repair},
{withdrawal_repairer, ff_withdrawal_repair_handler},
{deposit_repairer, ff_deposit_repair},
{w2w_transfer_management, ff_w2w_transfer_handler},
{w2w_transfer_repairer, ff_w2w_transfer_repair}
Expand Down
6 changes: 6 additions & 0 deletions apps/ff_server/src/ff_withdrawal_codec.erl
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,12 @@ unmarshal(repair_scenario, {add_events, #wthd_AddEventsRepair{events = Events, a
events => unmarshal({list, change}, Events),
action => maybe_unmarshal(complex_action, Action)
})};
unmarshal(repair_scenario, {routing, RoutingScenarioType}) ->
{routing, unmarshal(repair_scenario_routing, RoutingScenarioType)};
unmarshal(repair_scenario_routing, {route_changed, #wthd_RoutingRepairRouteChanged{route = Route}}) ->
{route_changed, unmarshal(route, Route)};
unmarshal(repair_scenario_routing, {route_not_found, #wthd_RoutingRepairRouteNotFound{reason = Reason}}) ->
{route_not_found, maybe_unmarshal(string, Reason)};
unmarshal(change, {created, #wthd_CreatedChange{withdrawal = Withdrawal}}) ->
{created, unmarshal(withdrawal, Withdrawal)};
unmarshal(change, {status_changed, #wthd_StatusChange{status = Status}}) ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-module(ff_withdrawal_repair).
-module(ff_withdrawal_repair_handler).

-behaviour(ff_woody_wrapper).

Expand Down
59 changes: 51 additions & 8 deletions apps/ff_transfer/src/ff_withdrawal.erl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
-include_lib("damsel/include/dmsl_payment_processing_thrift.hrl").
-include_lib("damsel/include/dmsl_withdrawals_provider_adapter_thrift.hrl").

-include_lib("fistful_proto/include/ff_proto_withdrawal_thrift.hrl").

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А зачем тут новый include? Чё-то не вижу сходу потребности.(как кстати и в предыдущем include)


-type id() :: binary().
-type clock() :: ff_transaction:clock().

Expand All @@ -24,6 +26,7 @@
attempts => attempts(),
resource => destination_resource(),
adjustments => adjustments_index(),
repair_scenario => repair_scenario(),
status => status(),
metadata => metadata(),
external_id => id()
Expand Down Expand Up @@ -170,7 +173,7 @@
-type invalid_withdrawal_status_error() ::
{invalid_withdrawal_status, status()}.

-type action() :: sleep | continue | undefined.
-type action() :: sleep | continue | undefined | {set_timer, integer()}.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тогда это изменение тоже можно убрать.


-export_type([withdrawal/0]).
-export_type([withdrawal_state/0]).
Expand Down Expand Up @@ -204,6 +207,7 @@
-export([id/1]).
-export([body/1]).
-export([status/1]).
-export([activity/1]).
-export([route/1]).
-export([attempts/1]).
-export([external_id/1]).
Expand All @@ -223,6 +227,7 @@
-export([start_adjustment/2]).
-export([find_adjustment/2]).
-export([adjustments/1]).
-export([start_repair/2]).
-export([effective_final_cash_flow/1]).
-export([sessions/1]).
-export([session_id/1]).
Expand Down Expand Up @@ -260,6 +265,7 @@
-type adjustment() :: ff_adjustment:adjustment().
-type adjustment_id() :: ff_adjustment:id().
-type adjustments_index() :: ff_adjustment_utils:index().
-type repair_scenario() :: ff_repair:scenario().
-type currency_id() :: ff_currency:id().
-type party_revision() :: ff_party:revision().
-type domain_revision() :: ff_domain_config:revision().
Expand Down Expand Up @@ -307,7 +313,7 @@

-type fail_type() ::
limit_check
| route_not_found
| route_not_found | {route_not_found, binary()}
| {inconsistent_quote_route, {provider_id, provider_id()} | {terminal_id, terminal_id()}}
| session.

Expand Down Expand Up @@ -355,6 +361,10 @@ status(T) ->
route(T) ->
maps:get(route, T, undefined).

-spec activity(withdrawal_state()) -> activity().
activity(T) ->
deduce_activity(T).

-spec attempts(withdrawal_state()) -> attempts().
attempts(#{attempts := Attempts}) ->
Attempts;
Expand Down Expand Up @@ -446,7 +456,7 @@ create(Params) ->
destination_id => DestinationID,
quote => Quote
}),
[
Result = [
{created,
genlib_map:compact(#{
version => ?ACTUAL_FORMAT_VERSION,
Expand All @@ -462,7 +472,8 @@ create(Params) ->
})},
{status_changed, pending},
{resource_got, Resource}
]
],
Result

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Какое-то немного бессмысленное изменение, не?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Плохо почистил всё после того как вставлял printf'ы, еще приберусь.

end).

create_resource(
Expand Down Expand Up @@ -537,12 +548,31 @@ is_finished(#{status := {failed, _}}) ->
is_finished(#{status := pending}) ->
false.

-spec start_repair(repair_scenario(), withdrawal_state()) -> {ok, process_result()}.
start_repair(Scenario, St) ->
Activity = ff_withdrawal:activity(St),
%TODO: после вызова process_transfer активити заново высчитывается, надо продумать правильно ли здесь проверять совместимость со сценарием
ok = check_activity_compatibility(Scenario, Activity),
RepairState = St#{repair_scenario => Scenario},
{ok, process_transfer(RepairState)}.

check_activity_compatibility({routing, _}, Activity) when Activity =:= routing ->
ok;
%TODO: activity_not_compatible_with_scenario - или что-то другое? Реализовать в протоколе и тут
check_activity_compatibility(Scenario, Activity) ->
throw({exception, {activity_not_compatible_with_scenario, Activity, Scenario}}).

%% Transfer callbacks

-spec process_transfer(withdrawal_state()) -> process_result().
process_transfer(Withdrawal) ->
Activity = deduce_activity(Withdrawal),
do_process_transfer(Activity, Withdrawal).
case Withdrawal of
#{repair_scenario := RepairScenario} ->
do_process_repair(RepairScenario, Withdrawal);
_ ->
do_process_transfer(Activity, Withdrawal)
end.

%%

Expand Down Expand Up @@ -729,6 +759,10 @@ do_finished_activity(#{status := succeeded, p_transfer := committed}) ->
do_finished_activity(#{status := {failed, _}, p_transfer := cancelled}) ->
stop.

-spec do_process_repair(repair_scenario(), withdrawal_state()) -> process_result().
do_process_repair(Scenario, Withdrawal) ->
process_repair(Scenario, Withdrawal).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нахожу эту функцию немного бессмысленной. 🤔 Зачем она тут?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это небольшой артефакт, который остался после того как я пробовал разные варианты запуска сценария починки. Сначала я эту функцию завел для единообразия с do_process_transfer, а потом оказалось что здесь делать нечего, но пока я это оставил. Выпилю, если она пустой останется.


-spec do_process_transfer(activity(), withdrawal_state()) -> process_result().
do_process_transfer(routing, Withdrawal) ->
process_routing(Withdrawal);
Expand Down Expand Up @@ -761,6 +795,12 @@ do_process_transfer(adjustment, Withdrawal) ->
do_process_transfer(stop, _Withdrawal) ->
{undefined, []}.

-spec process_repair(repair_scenario(), withdrawal_state()) -> process_result().
process_repair({routing, {route_changed, Route}}, _Withdrawal) ->
{{set_timer, 0}, [{route_changed, Route}]};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А зачем новый action? Чем это от continue отличается?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process_repair({routing, {route_not_found, _} = FailType}, Withdrawal) ->
process_transfer_fail(FailType, Withdrawal).

-spec process_routing(withdrawal_state()) -> process_result().
process_routing(Withdrawal) ->
case do_process_routing(Withdrawal) of
Expand Down Expand Up @@ -1350,6 +1390,7 @@ get_current_session_status(Withdrawal) ->
pending
end.


%% Withdrawal validators

-spec validate_withdrawal_creation(terms(), body(), wallet(), destination()) ->
Expand Down Expand Up @@ -1726,9 +1767,11 @@ build_failure(limit_check, Withdrawal) ->
}
end;
build_failure(route_not_found, _Withdrawal) ->
#{
code => <<"no_route_found">>
};
#{code => <<"no_route_found">>};
build_failure({route_not_found, undefined}, Withdrawal) ->
build_failure(route_not_found, Withdrawal);
build_failure({route_not_found, Reason}, _Withdrawal) ->
#{code => Reason};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вообще выглядит немного странно: сценарий называется route_not_found, но при этом withdrawal просто завершится с произвольной ошибкой. Причём тут тогда route_not_found? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Задумка была в том, что тот кто запустит восстановление по сценарию route_not_found напишет Reason, и это можно будет легко найти в логах, либо спустя время вспомнить что это был не просто "no_route_found", а что-то типа "Hand repair with no_route_found". Я упустил, что в этом случае в Reason'е может не оказаться слов про no_route_found (по умолчанию если инициатор поленится указать Reason, то будет дефолтное <<"no_route_found">>).

Тут два варианта, либо вообще Reason выпилить, либо вместо #{code => Reason} делать что-то типа #{code => <<"no_route_found, reason: ", Reason">>}. Вопрос в полезности этой фичи. Как считаешь?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(подозреваю, что поле 'code' задумывалось как нехудожественное короткое обозначение ошибки, и значит не надо там писать никаких ризонов, короче наверное надо просто выпилить Reason и всё)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут два варианта, либо вообще Reason выпилить, либо вместо #{code => Reason} делать что-то типа #{code => <<"no_route_found, reason: ", Reason">>}. Вопрос в полезности этой фичи. Как считаешь?

Я думаю, что для такого сценарий должен по-другому называться, типа MakeFailed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

надо просто выпилить Reason и всё)

Для текущего сценария определённо.

build_failure({inconsistent_quote_route, {Type, FoundID}}, Withdrawal) ->
Details =
{inconsistent_quote_route, #{
Expand Down
16 changes: 14 additions & 2 deletions apps/ff_transfer/src/ff_withdrawal_machine.erl
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,18 @@ process_call(CallArgs, _Machine, _, _Opts) ->

-spec process_repair(ff_repair:scenario(), machine(), handler_args(), handler_opts()) ->
{ok, {repair_response(), result()}} | {error, repair_error()}.
process_repair({add_events, _} = Scenario, Machine, _Args, _Opts) ->
ff_repair:apply_scenario(ff_withdrawal, Machine, Scenario);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кажется по задумке автора ff_repair ты просто процессор должен был добавить для этого сценария и передать его в ff_repair:apply_scenario/4.

process_repair(Scenario, Machine, _Args, _Opts) ->
ff_repair:apply_scenario(ff_withdrawal, Machine, Scenario).
St = ff_machine:collapse(ff_withdrawal, Machine),
{ok, ScenarioResult} = ff_withdrawal:start_repair(Scenario, withdrawal(St)),
ProcessedResult = process_result(ScenarioResult, St),
case ff_repair:validate_scenario_result(ff_withdrawal, Machine, ProcessedResult) of
{ok, valid} ->
{ok, {ok, ProcessedResult}};
{error, _Reason} = Error ->
{Error, #{}}
end.

-spec do_start_adjustment(adjustment_params(), machine()) -> {Response, result()} when
Response :: ok | {error, ff_withdrawal:start_adjustment_error()}.
Expand Down Expand Up @@ -229,7 +239,9 @@ set_action(continue, _St) ->
set_action(undefined, _St) ->
undefined;
set_action(sleep, _St) ->
unset_timer.
unset_timer;
set_action({set_timer, Timeout}, _St) ->
{set_timer, {timeout, Timeout}}.

call(ID, Call) ->
case machinery:call(?NS, ID, Call, backend()) of
Expand Down
Loading