From 38ad42b42cbf6da6254a2498a2fc553507cb3f18 Mon Sep 17 00:00:00 2001 From: Birte Kristina Friesel Date: Sun, 23 Jul 2023 20:18:10 +0200 Subject: [PATCH] convert checkout to promises (checkout_p) --- lib/Travelynx.pm | 415 ++++++++++++++------------ lib/Travelynx/Command/work.pm | 35 ++- lib/Travelynx/Controller/Traveling.pm | 153 ++++++---- 3 files changed, 347 insertions(+), 256 deletions(-) diff --git a/lib/Travelynx.pm b/lib/Travelynx.pm index 469e1fe..f5f56b7 100755 --- a/lib/Travelynx.pm +++ b/lib/Travelynx.pm @@ -588,7 +588,7 @@ sub startup { ); $self->helper( - 'checkout' => sub { + 'checkout_p' => sub { my ( $self, %opt ) = @_; my $station = $opt{station}; @@ -596,34 +596,29 @@ sub startup { my $arr_eva = $opt{arr_eva}; my $with_related = $opt{with_related} // 0; my $force = $opt{force}; - my $uid = $opt{uid}; - my $db = $opt{db} // $self->pg->db; - my $status = $self->iris->get_departures( - station => $station, - lookbehind => 120, - lookahead => 180, - with_related => $with_related, - ); - $uid //= $self->current_user->{id}; - my $user = $self->get_user_status( $uid, $db ); - my $train_id = $user->{train_id}; + my $uid = $opt{uid} // $self->current_user->{id}; + my $db = $opt{db} // $self->pg->db; + my $user = $self->get_user_status( $uid, $db ); + my $train_id = $user->{train_id}; + + my $promise = Mojo::Promise->new; if ( not $station ) { $self->app->log->error("Checkout($uid): station is empty"); - return ( 1, 'BUG: Checkout station is empty.' ); + return $promise->resolve( 1, + 'BUG: Checkout station is empty.' ); } if ( not $user->{checked_in} and not $user->{cancelled} ) { - return ( 0, 'You are not checked into any train' ); - } - if ( $status->{errstr} and not $force ) { - return ( 1, $status->{errstr} ); + return $promise->resolve( 0, + 'You are not checked into any train' ); } + if ( $dep_eva and $dep_eva != $user->{dep_eva} ) { - return ( 0, 'race condition' ); + return $promise->resolve( 0, 'race condition' ); } if ( $arr_eva and $arr_eva != $user->{arr_eva} ) { - return ( 0, 'race condition' ); + return $promise->resolve( 0, 'race condition' ); } my $now = DateTime->now( time_zone => 'Europe/Berlin' ); @@ -632,196 +627,232 @@ sub startup { with_data => 1 ); - # Note that a train may pass the same station several times. - # Notable example: S41 / S42 ("Ringbahn") both starts and - # terminates at Berlin Südkreuz - my ($train) = List::Util::first { - $_->train_id eq $train_id - and $_->sched_arrival - and $_->sched_arrival->epoch > $user->{sched_departure}->epoch - } - @{ $status->{results} }; + $self->iris->get_departures_p( + station => $station, + lookbehind => 120, + lookahead => 180, + with_related => $with_related, + )->then( + sub { + my ($status) = @_; - $train //= List::Util::first { $_->train_id eq $train_id } - @{ $status->{results} }; + my $new_checkout_station_id = $status->{station_eva}; - my $new_checkout_station_id = $status->{station_eva}; + # Store the intended checkout station regardless of this operation's + # success. + # TODO for with_related == 1, the correct EVA may be different + # and should be fetched from $train later on + $self->in_transit->set_arrival_eva( + uid => $uid, + db => $db, + arrival_eva => $new_checkout_station_id + ); - # Store the intended checkout station regardless of this operation's - # success. - $self->in_transit->set_arrival_eva( - uid => $uid, - db => $db, - arrival_eva => $new_checkout_station_id - ); + # If in_transit already contains arrival data for another estimated + # destination, we must invalidate it. + if ( defined $journey->{checkout_station_id} + and $journey->{checkout_station_id} + != $new_checkout_station_id ) + { + $self->in_transit->unset_arrival_data( + uid => $uid, + db => $db + ); + } - # If in_transit already contains arrival data for another estimated - # destination, we must invalidate it. - if ( defined $journey->{checkout_station_id} - and $journey->{checkout_station_id} - != $new_checkout_station_id ) - { - $self->in_transit->unset_arrival_data( - uid => $uid, - db => $db - ); - } + # Note that a train may pass the same station several times. + # Notable example: S41 / S42 ("Ringbahn") both starts and + # terminates at Berlin Südkreuz + my $train = List::Util::first { + $_->train_id eq $train_id + and $_->sched_arrival + and $_->sched_arrival->epoch + > $user->{sched_departure}->epoch + } + @{ $status->{results} }; - if ( not defined $train ) { + $train //= List::Util::first { $_->train_id eq $train_id } + @{ $status->{results} }; - # Arrival time via IRIS is unknown, so the train probably has not - # arrived yet. Fall back to HAFAS. - # TODO support cases where $station is EVA or DS100 code - if ( - my $station_data - = List::Util::first { $_->[0] eq $station } - @{ $journey->{route} } - ) - { - $station_data = $station_data->[2]; - if ( $station_data->{sched_arr} ) { - my $sched_arr - = epoch_to_dt( $station_data->{sched_arr} ); - my $rt_arr = epoch_to_dt( $station_data->{rt_arr} ); - if ( $rt_arr->epoch == 0 ) { - $rt_arr = $sched_arr->clone; - if ( $station_data->{arr_delay} - and $station_data->{arr_delay} =~ m{^\d+$} ) - { - $rt_arr->add( - minutes => $station_data->{arr_delay} ); + if ( not defined $train ) { + + # Arrival time via IRIS is unknown, so the train probably + # has not arrived yet. Fall back to HAFAS. + # TODO support cases where $station is EVA or DS100 code + if ( + my $station_data + = List::Util::first { $_->[0] eq $station } + @{ $journey->{route} } + ) + { + $station_data = $station_data->[2]; + if ( $station_data->{sched_arr} ) { + my $sched_arr + = epoch_to_dt( $station_data->{sched_arr} ); + my $rt_arr + = epoch_to_dt( $station_data->{rt_arr} ); + if ( $rt_arr->epoch == 0 ) { + $rt_arr = $sched_arr->clone; + if ( $station_data->{arr_delay} + and $station_data->{arr_delay} + =~ m{^\d+$} ) + { + $rt_arr->add( minutes => + $station_data->{arr_delay} ); + } + } + $self->in_transit->set_arrival_times( + uid => $uid, + db => $db, + sched_arrival => $sched_arr, + rt_arrival => $rt_arr + ); } } - $self->in_transit->set_arrival_times( - uid => $uid, - db => $db, - sched_arrival => $sched_arr, - rt_arrival => $rt_arr - ); - } - } - if ( not $force ) { + if ( not $force ) { - # mustn't be called during a transaction - if ( not $opt{in_transaction} ) { - $self->run_hook( $uid, 'update' ); - } - return ( 1, undef ); - } - } - - my $has_arrived = 0; - - eval { - - my $tx; - if ( not $opt{in_transaction} ) { - $tx = $db->begin; - } - - if ( defined $train and not $train->arrival and not $force ) { - my $train_no = $train->train_no; - die("Train ${train_no} has no arrival timestamp\n"); - } - elsif ( defined $train and $train->arrival ) { - $self->in_transit->set_arrival( - uid => $uid, - db => $db, - train => $train, - route => [ $self->iris->route_diff($train) ] - ); - - $has_arrived = $train->arrival->epoch < $now->epoch ? 1 : 0; - if ($has_arrived) { - my @unknown_stations - = $self->stations->grep_unknown( $train->route ); - if (@unknown_stations) { - $self->app->log->warn( - sprintf( -'Route of %s %s (%s -> %s) contains unknown stations: %s', - $train->type, - $train->train_no, - $train->origin, - $train->destination, - join( ', ', @unknown_stations ) - ) - ); + # mustn't be called during a transaction + if ( not $opt{in_transaction} ) { + $self->run_hook( $uid, 'update' ); + } + $promise->resolve( 1, undef ); + return; } } - } + my $has_arrived = 0; - $journey = $self->in_transit->get( - uid => $uid, - db => $db - ); + eval { - if ( $has_arrived or $force ) { - $self->journeys->add_from_in_transit( - db => $db, - journey => $journey - ); - $self->in_transit->delete( - uid => $uid, - db => $db - ); + my $tx; + if ( not $opt{in_transaction} ) { + $tx = $db->begin; + } - my $cache_ts = $now->clone; - if ( $journey->{real_departure} - =~ m{ ^ (? \d{4} ) - (? \d{2} ) }x ) - { - $cache_ts->set( - year => $+{year}, - month => $+{month} + if ( defined $train + and not $train->arrival + and not $force ) + { + my $train_no = $train->train_no; + die("Train ${train_no} has no arrival timestamp\n"); + } + elsif ( defined $train and $train->arrival ) { + $self->in_transit->set_arrival( + uid => $uid, + db => $db, + train => $train, + route => [ $self->iris->route_diff($train) ] + ); + + $has_arrived + = $train->arrival->epoch < $now->epoch ? 1 : 0; + if ($has_arrived) { + my @unknown_stations + = $self->stations->grep_unknown( + $train->route ); + if (@unknown_stations) { + $self->app->log->warn( + sprintf( +'Route of %s %s (%s -> %s) contains unknown stations: %s', + $train->type, + $train->train_no, + $train->origin, + $train->destination, + join( ', ', @unknown_stations ) + ) + ); + } + } + } + + $journey = $self->in_transit->get( + uid => $uid, + db => $db ); + + if ( $has_arrived or $force ) { + $self->journeys->add_from_in_transit( + db => $db, + journey => $journey + ); + $self->in_transit->delete( + uid => $uid, + db => $db + ); + + my $cache_ts = $now->clone; + if ( $journey->{real_departure} + =~ m{ ^ (? \d{4} ) - (? \d{2} ) }x + ) + { + $cache_ts->set( + year => $+{year}, + month => $+{month} + ); + } + $self->journey_stats_cache->invalidate( + ts => $cache_ts, + db => $db, + uid => $uid + ); + } + elsif ( defined $train + and $train->arrival_is_cancelled ) + { + + # This branch is only taken if the deparure was not cancelled, + # i.e., if the train was supposed to go here but got + # redirected or cancelled on the way and not from the start on. + # If the departure itself was cancelled, the user route is + # cancelled_from action -> 'cancelled journey' panel on main page + # -> cancelled_to action -> force checkout (causing the + # previous branch to be taken due to $force) + $journey->{cancelled} = 1; + $self->journeys->add_from_in_transit( + db => $db, + journey => $journey + ); + $self->in_transit->set_cancelled_destination( + uid => $uid, + db => $db, + cancelled_destination => $train->station, + ); + } + + if ( not $opt{in_transaction} ) { + $tx->commit; + } + }; + + if ($@) { + $self->app->log->error("Checkout($uid): $@"); + $promise->resolve( 1, 'Checkout error: ' . $@ ); + return; } - $self->journey_stats_cache->invalidate( - ts => $cache_ts, - db => $db, - uid => $uid - ); - } - elsif ( defined $train and $train->arrival_is_cancelled ) { - # This branch is only taken if the deparure was not cancelled, - # i.e., if the train was supposed to go here but got - # redirected or cancelled on the way and not from the start on. - # If the departure itself was cancelled, the user route is - # cancelled_from action -> 'cancelled journey' panel on main page - # -> cancelled_to action -> force checkout (causing the - # previous branch to be taken due to $force) - $journey->{cancelled} = 1; - $self->journeys->add_from_in_transit( - db => $db, - journey => $journey - ); - $self->in_transit->set_cancelled_destination( - uid => $uid, - db => $db, - cancelled_destination => $train->station, - ); - } + if ( $has_arrived or $force ) { + if ( not $opt{in_transaction} ) { + $self->run_hook( $uid, 'checkout' ); + } + $promise->resolve( 0, undef ); + return; + } + if ( not $opt{in_transaction} ) { + $self->run_hook( $uid, 'update' ); + $self->add_route_timestamps( $uid, $train, 0, 1 ); + } + $promise->resolve( 1, undef ); + return; - if ( not $opt{in_transaction} ) { - $tx->commit; } - }; - - if ($@) { - $self->app->log->error("Checkout($uid): $@"); - return ( 1, 'Checkout error: ' . $@ ); - } - - if ( $has_arrived or $force ) { - if ( not $opt{in_transaction} ) { - $self->run_hook( $uid, 'checkout' ); + )->catch( + sub { + my ($err) = @_; + $promise->resolve( 1, $err ); + return; } - return ( 0, undef ); - } - if ( not $opt{in_transaction} ) { - $self->run_hook( $uid, 'update' ); - $self->add_route_timestamps( $uid, $train, 0, 1 ); - } - return ( 1, undef ); + )->wait; + + return $promise; } ); @@ -1788,13 +1819,17 @@ sub startup { )->then( sub { $self->log->debug("... handled origin"); - my ( undef, $err ) = $self->checkout( + return $self->checkout_p( station => $traewelling->{arr_eva}, train_id => 0, uid => $uid, in_transaction => 1, db => $db ); + } + )->then( + sub { + my ( undef, $err ) = @_; if ($err) { $self->log->debug("... error: $err"); return Mojo::Promise->reject($err); diff --git a/lib/Travelynx/Command/work.pm b/lib/Travelynx/Command/work.pm index 3086737..23d2925 100644 --- a/lib/Travelynx/Command/work.pm +++ b/lib/Travelynx/Command/work.pm @@ -86,15 +86,15 @@ sub run { # train. if ($checked_in) { - # check out (adds a cancelled journey and resets journey state - # to checkin - $self->app->checkout( + # check out (adds a cancelled journey and resets journey state + # to checkin + $self->app->checkout_p( station => $arr, - force => 1, + force => 2, dep_eva => $dep, arr_eva => $arr, uid => $uid - ); + )->wait; } } else { @@ -153,15 +153,15 @@ sub run { if ( $checked_in and $train->arrival_is_cancelled ) { - # check out (adds a cancelled journey and resets journey state - # to destination selection) - $self->app->checkout( + # check out (adds a cancelled journey and resets journey state + # to destination selection) + $self->app->checkout_p( station => $arr, force => 0, dep_eva => $dep, arr_eva => $arr, uid => $uid - ); + )->wait; } else { $self->app->add_route_timestamps( @@ -174,21 +174,24 @@ sub run { } } elsif ( $entry->{real_arr_ts} ) { - my ( undef, $error ) = $self->app->checkout( + my ( undef, $error ) = $self->app->checkout_p( station => $arr, - force => 1, + force => 2, dep_eva => $dep, arr_eva => $arr, uid => $uid - ); - if ($error) { - die("${error}\n"); - } + )->catch( + sub { + my ($error) = @_; + $self->app->log->error("work($uid)/arrival: $@"); + $errors += 1; + } + )->wait; } }; if ($@) { - $errors += 1; $self->app->log->error("work($uid)/arrival: $@"); + $errors += 1; } eval { } diff --git a/lib/Travelynx/Controller/Traveling.pm b/lib/Travelynx/Controller/Traveling.pm index 48fa944..3e051a1 100755 --- a/lib/Travelynx/Controller/Traveling.pm +++ b/lib/Travelynx/Controller/Traveling.pm @@ -622,17 +622,26 @@ sub travel_action { if ( $params->{action} eq 'checkin' ) { my $status = $self->get_user_status; + my $promise; + if ( $status->{checked_in} and $status->{arr_eva} and $status->{arrival_countdown} <= 0 ) { - $self->checkout( station => $status->{arr_eva} ); + $promise = $self->checkout_p( station => $status->{arr_eva} ); + } + else { + $promise = Mojo::Promise->resolve; } $self->render_later; - $self->checkin_p( - station => $params->{station}, - train_id => $params->{train} + $promise->then( + sub { + return $self->checkin_p( + station => $params->{station}, + train_id => $params->{train} + ); + } )->then( sub { my $destination = $params->{dest}; @@ -648,17 +657,26 @@ sub travel_action { # Silently ignore errors -- if they are permanent, the user will see # them when selecting the destination manually. - my ( $still_checked_in, undef ) = $self->checkout( + return $self->checkout_p( station => $destination, force => 0 ); - my $station_link = '/s/' . $destination; - $self->render( - json => { - success => 1, - redirect_to => $still_checked_in ? '/' : $station_link, - }, - ); + } + )->then( + sub { + my ( $still_checked_in, undef ) = @_; + if ( my $destination = $params->{dest} ) { + my $station_link = '/s/' . $destination; + $self->render( + json => { + success => 1, + redirect_to => $still_checked_in + ? '/' + : $station_link, + }, + ); + } + return; } )->catch( sub { @@ -673,28 +691,47 @@ sub travel_action { )->wait; } elsif ( $params->{action} eq 'checkout' ) { - my ( $still_checked_in, $error ) = $self->checkout( + $self->render_later; + $self->checkout_p( station => $params->{station}, force => $params->{force} - ); - my $station_link = '/s/' . $params->{station}; + )->then( + sub { + my ( $still_checked_in, $error ) = @_; + my $station_link = '/s/' . $params->{station}; - if ($error) { - $self->render( - json => { - success => 0, - error => $error, - }, - ); - } - else { - $self->render( - json => { - success => 1, - redirect_to => $still_checked_in ? '/' : $station_link, - }, - ); - } + if ($error) { + $self->render( + json => { + success => 0, + error => $error, + }, + ); + } + else { + $self->render( + json => { + success => 1, + redirect_to => $still_checked_in + ? '/' + : $station_link, + }, + ); + } + return; + } + )->catch( + sub { + my ($error) = @_; + $self->render( + json => { + success => 0, + error => $error, + }, + ); + return; + } + )->wait; } elsif ( $params->{action} eq 'undo' ) { my $status = $self->get_user_status; @@ -747,27 +784,43 @@ sub travel_action { )->wait; } elsif ( $params->{action} eq 'cancelled_to' ) { - my ( undef, $error ) = $self->checkout( + $self->render_later; + $self->checkout_p( station => $params->{station}, force => 1 - ); - - if ($error) { - $self->render( - json => { - success => 0, - error => $error, - }, - ); - } - else { - $self->render( - json => { - success => 1, - redirect_to => '/', - }, - ); - } + )->then( + sub { + my ( undef, $error ) = @_; + if ($error) { + $self->render( + json => { + success => 0, + error => $error, + }, + ); + } + else { + $self->render( + json => { + success => 1, + redirect_to => '/', + }, + ); + } + return; + } + )->catch( + sub { + my ($error) = @_; + $self->render( + json => { + success => 0, + error => $error, + }, + ); + return; + } + )->wait; } elsif ( $params->{action} eq 'delete' ) { my $error = $self->journeys->delete(