From 792c2bccf7123d5bb50f8bbe473a1722b4cf3561 Mon Sep 17 00:00:00 2001 From: Kirill Solomko Date: Fri, 25 Aug 2017 16:29:42 +0200 Subject: [PATCH] TT#20095 improve sms billing handling and journaling * billing is split into 2 parts: - init_prepaid_billing (reserves billing/creates session) - perform_prepaid_billing (commits or cancels the session) * when sms fails to be sent the billing session is canceled (was committed before, regardless) * NGCP::Panel::Entities::post - do not do $self->return_representation_post() if api_error_message is present, therefore, the actual error is returned * sms_journal item is now created also in case of an error * improved errors indication Change-Id: I454b8c6a1c04d743ee82e03f8621c7cc4c7c4f98 --- lib/NGCP/Panel/Controller/API/SMS.pm | 73 +++++++++++++-------- lib/NGCP/Panel/Role/API/SMS.pm | 6 +- lib/NGCP/Panel/Role/Entities.pm | 4 +- lib/NGCP/Panel/Utils/SMS.pm | 97 ++++++++++++++++++++-------- 4 files changed, 122 insertions(+), 58 deletions(-) diff --git a/lib/NGCP/Panel/Controller/API/SMS.pm b/lib/NGCP/Panel/Controller/API/SMS.pm index 204a7efe01..3b280966cd 100644 --- a/lib/NGCP/Panel/Controller/API/SMS.pm +++ b/lib/NGCP/Panel/Controller/API/SMS.pm @@ -115,34 +115,51 @@ sub create_item { return; } - my $parts = NGCP::Panel::Utils::SMS::get_number_of_parts($resource->{text}); + my $session; try { - unless(NGCP::Panel::Utils::SMS::perform_prepaid_billing(c => $c, + my $parts = NGCP::Panel::Utils::SMS::get_number_of_parts($resource->{text}); + $session = NGCP::Panel::Utils::SMS::init_prepaid_billing(c => $c, prov_subscriber => $subscriber, parts => $parts, caller => $resource->{caller}, callee => $resource->{callee} - )) { - $self->error($c, HTTP_PAYMENT_REQUIRED, "Not enough credit to send sms"); - return; + ); + unless ($session && $session->{status} eq 'ok') { + die($session->{reason} // + "Internal server error when preparing sms billing"); } - } catch($e) { - $c->log->error("Failed to determine credit: $e"); - $self->error($c, HTTP_PAYMENT_REQUIRED, "Failed to determine credit"); - return; - } - my $error_msg = ""; - my $coding = NGCP::Panel::Utils::SMS::get_coding($resource->{text}); - NGCP::Panel::Utils::SMS::send_sms( - c => $c, - caller => $resource->{caller}, - callee => $resource->{callee}, - text => $resource->{text}, - coding => $coding, - err_code => sub {$error_msg = shift;}, + $session->{coding} = NGCP::Panel::Utils::SMS::get_coding($resource->{text}); + + NGCP::Panel::Utils::SMS::send_sms( + c => $c, + caller => $resource->{caller}, + callee => $resource->{callee}, + text => $resource->{text}, + coding => $session->{coding}, + err_code => sub { + $session->{reason} = shift; + $session->{status} = 'failed'; + } + ); + + NGCP::Panel::Utils::SMS::perform_prepaid_billing(c => $c, + session => $session ); + if ($session->{status} eq 'failed') { + die $session->{reason}."\n"; + } + } catch($e) { + $c->log->error($e); + if ($session && $session->{reason} eq 'insufficient credit') { + $self->error($c, HTTP_PAYMENT_REQUIRED, "Not enough credit to send the sms"); + } else { + $self->error($c, HTTP_INTERNAL_SERVER_ERROR, + "An internal error has occured when sending the sms, please contact the platform administrator or try again later"); + } + } + # TODO: agranig: we need to return an item here, otherwise it fails #if($c->user->roles eq "admin" || $c->user->roles eq "reseller") { # if($c->req->params->{skip_journal} eq "true") { @@ -152,14 +169,16 @@ sub create_item { my $rs = $self->item_rs($c); my $item = $rs->create({ - subscriber_id => $resource->{subscriber_id}, - direction => 'out', - caller => $resource->{caller}, - callee => $resource->{callee}, - text => $resource->{text}, - coding => $coding, - $error_msg ? (status => $error_msg) : (), - }); + subscriber_id => $resource->{subscriber_id}, + direction => 'out', + caller => $resource->{caller}, + callee => $resource->{callee}, + text => $resource->{text}, + coding => $session->{coding}, + status => $session->{status} // '', + reason => $session->{reason} // '', + }); + return $item; } diff --git a/lib/NGCP/Panel/Role/API/SMS.pm b/lib/NGCP/Panel/Role/API/SMS.pm index 6f856b4952..47acc7176f 100644 --- a/lib/NGCP/Panel/Role/API/SMS.pm +++ b/lib/NGCP/Panel/Role/API/SMS.pm @@ -16,7 +16,7 @@ sub item_name { return 'sms'; } -sub resource_name{ +sub resource_name { return 'sms'; } @@ -66,7 +66,7 @@ sub _item_rs { return $item_rs; } -sub check_resource{ +sub check_resource { my($self, $c, $item, $old_resource, $resource, $form) = @_; unless(defined $resource->{subscriber_id}) { # TODO: might check in form @@ -91,7 +91,7 @@ sub check_resource{ c => $c, prov_subscriber => $subscriber ); - $lock //= 0; + $lock ||= 0; # can be an empty string my $lockstr = NGCP::Panel::Utils::Subscriber::get_lock_string($lock); unless($lockstr eq 'none') { $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Subscriber is locked."); diff --git a/lib/NGCP/Panel/Role/Entities.pm b/lib/NGCP/Panel/Role/Entities.pm index a185f62cdf..f501712683 100644 --- a/lib/NGCP/Panel/Role/Entities.pm +++ b/lib/NGCP/Panel/Role/Entities.pm @@ -141,7 +141,9 @@ sub post { $self->complete_transaction($c); $self->post_process_commit($c, 'create', $item, undef, $resource, $form, $process_extras); - + + return if defined $c->stash->{api_error_message}; + $self->return_representation_post($c, 'item' => $item, 'form' => $form, 'form_exceptions' => $form_exceptions ); } return; diff --git a/lib/NGCP/Panel/Utils/SMS.pm b/lib/NGCP/Panel/Utils/SMS.pm index bf21d5a14d..31d6895780 100644 --- a/lib/NGCP/Panel/Utils/SMS.pm +++ b/lib/NGCP/Panel/Utils/SMS.pm @@ -70,7 +70,7 @@ sub send_sms { if ($res->is_success) { return 1; } else { - &{$err_code}("Error with send_sms: " . $res->status_line); + &{$err_code}("Error sending sms: " . $res->status_line); return; } } @@ -180,7 +180,7 @@ sub get_number_of_parts { return ceil(length($text) / $maxlen); } -sub perform_prepaid_billing { +sub init_prepaid_billing { my (%args) = @_; my $c = $args{c}; my $prov_subscriber = $args{prov_subscriber}; @@ -192,6 +192,16 @@ sub perform_prepaid_billing { UUID::generate($uuid); UUID::unparse($uuid, $session_id); + my $session = { + caller => $caller, + callee => $callee, + status => 'ok', + reason => 'accepted', + parts => [], + sid => $session_id, + rpc => $parts, + }; + my ($prepaid_lib, $is_prepaid); my $prepaid_pref_rs = NGCP::Panel::Utils::Preferences::get_dom_preference_rs( c => $c, attribute => 'prepaid_library', @@ -212,11 +222,21 @@ sub perform_prepaid_billing { } # currently only inew rating supported, let others pass - return 1 unless($is_prepaid && $prepaid_lib eq "libinewrate"); + unless ($is_prepaid && $prepaid_lib eq "libinewrate") { + $session->{reason} = 'not prepaid/libinewrate'; + return $session; + } my $use_list = { 'NGCP::Rating::Inew::SmsSession' => undef }; unless(can_load(modules => $use_list, nocache => 0, autoload => 0)) { - $c->log->error("Failed to load NGCP::Rating::Inew::SmsSession for sms from $caller to $callee"); + $c->log->error(sprintf + "Failed to load NGCP::Rating::Inew::SmsSession for sms=%s from=%s to=%s", + $session_id, $caller, $callee + ); + $session->{status} = 'failed'; + $session->{reason} = + sprintf 'failed to init sms session sid=%s from=%s to=%s', + $session_id, $caller,$callee; return; } my $amqr = NGCP::Rating::Inew::SmsSession::init( @@ -224,62 +244,85 @@ sub perform_prepaid_billing { $c->config->{libinewrate}->{openwire_uri}, ); unless($amqr) { - $c->log->error("Failed to create sms amqr handle from $caller to $callee"); + $c->log->error(sprintf + "Failed to create sms amqr handle for sms sid=%s from=%s to=%s", + $session_id, $caller, $callee + ); + $session->{status} = 'failed'; + $session->{reason} = + sprintf 'failed to create sms session sid=%s from=%s to=%s', + $session_id, $caller, $callee; return; } + $session->{amqr_h} = $amqr; + # Reserve credit for each part, and then commit each reservation. # If we can charge multiple times within one session - perfect. # Otherwise we have to create one session per part, store it in an # array, then after all reservations were successful, commit each # of them! - my @sessions = (); - my @failed_sessions = (); - my $cancel_reason; for(my $i = 0; $i < $parts; ++$i) { my $has_credit = 1; my $this_session_id = $session_id."-".$i; + my $sess = NGCP::Rating::Inew::SmsSession::session_create( $amqr, $this_session_id, $caller, $callee, sub { $has_credit = 0; }); + unless($sess) { $c->log->error("Failed to create sms rating session from $caller to $callee with session id $this_session_id"); + $session->{status} = 'failed'; + $session->{reason} = 'failed to create sms session'; last; } - unless(NGCP::Rating::Inew::SmsSession::session_sms_reserve($sess)) { - $c->log->error("Failed to reserve sms session from $caller to $callee with session id $this_session_id"); - push @failed_sessions, $sess; - last; - } + + push @{$session->{parts}}, $sess; + unless($has_credit) { $c->log->info("No credit for sms from $caller to $callee with session id $this_session_id"); - push @failed_sessions, $sess; - $cancel_reason = 'insufficient credit'; + $session->{status} = 'failed'; + $session->{reason} = 'insufficient credit'; + last; + } + + unless(NGCP::Rating::Inew::SmsSession::session_sms_reserve($sess)) { + $c->log->error("Failed to reserve sms session from $caller to $callee with session id $this_session_id"); + $session->{status} = 'failed'; + $session->{reason} = 'failed to reserve sms session'; last; } - push @sessions, $sess; } - if(@sessions == $parts) { - foreach my $sess(@sessions) { + return $session; +} + +sub perform_prepaid_billing { + my (%args) = @_; + my $c = $args{c}; + my $session = $args{session} // return; + my ($amqr, $rpc, $status, $reason, $parts) = + @{$session}{qw(amqr_h rpc status reason parts)}; + + return unless $session; + return unless $amqr; + + $reason //= 'unknown'; + + if ($status eq 'ok' && $rpc == $#{$parts}+1) { + foreach my $sess (@{$parts}) { NGCP::Rating::Inew::SmsSession::session_sms_commit($sess); NGCP::Rating::Inew::SmsSession::session_destroy($sess); } NGCP::Rating::Inew::SmsSession::destroy($amqr); - return 1; } else { - foreach my $sess(@sessions) { - if (defined($cancel_reason)) { - NGCP::Rating::Inew::SmsSession::session_set_cancel_reason($sess, $cancel_reason); - } + foreach my $sess (@{$parts}) { + NGCP::Rating::Inew::SmsSession::session_set_cancel_reason($sess, $reason); NGCP::Rating::Inew::SmsSession::session_sms_discard($sess); NGCP::Rating::Inew::SmsSession::session_destroy($sess); } - foreach my $sess(@failed_sessions) { - NGCP::Rating::Inew::SmsSession::session_destroy($sess); - } NGCP::Rating::Inew::SmsSession::destroy($amqr); - return; } + return; } 1;