From afa9289b6633f20f0085dc55ddb530e56d346d10 Mon Sep 17 00:00:00 2001 From: Oleksandr Duts Date: Wed, 30 Mar 2022 13:49:21 +0300 Subject: [PATCH] TT#167900 /api/subscribers improve number duplicate checks * primary and alias numbers are now validated that they do not belong to another subscriber * aliases are now validated that they are not already set as the primary number * reduce amount of related sql queries Change-Id: I4397bbdc4bc9001b7feeef22cb8f85ee0b6ce8ff --- lib/NGCP/Panel/Role/API/Subscribers.pm | 18 ++++-- lib/NGCP/Panel/Utils/Subscriber.pm | 87 +++++++++++++++++--------- 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/lib/NGCP/Panel/Role/API/Subscribers.pm b/lib/NGCP/Panel/Role/API/Subscribers.pm index 89d27d0b89..823e3beb9b 100644 --- a/lib/NGCP/Panel/Role/API/Subscribers.pm +++ b/lib/NGCP/Panel/Role/API/Subscribers.pm @@ -496,14 +496,24 @@ sub update_item { ); } catch(DBIx::Class::Exception $e where { /Duplicate entry '([^']+)' for key 'number_idx'/ }) { $e =~ /Duplicate entry '([^']+)' for key 'number_idx'/; - $c->log->error("failed to update subscriber, number " . $c->qs($1) . " already exists"); # TODO: user, message, trace, ... + $c->log->error("failed to update subscriber, number " . $c->qs($1) . " already exists"); $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Number '" . $1 . "' already exists.", "Number already exists."); return; - } catch($e where { /alias '([^']+)' already exists/ }) { - $e =~ /alias '([^']+)' already exists/; - $c->log->error("failed to update subscriber, alias " . $c->qs($1) . " already exists"); # TODO: user, message, trace, ... + } catch ($e where { /alias \d+ already exists/ }) { + $e =~ /alias (\d+) already exists/; + $c->log->error("failed to update subscriber, alias " . $c->qs($1) . " already exists"); $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Number '" . $1 . "' already exists.", "Number already exists."); return; + } catch ($e where { /aliases \S+ already exist/ }) { + $e =~ /aliases (\S+) already exist/; + $c->log->error("failed to update subscriber, aliases " . $c->qs($1) . " already exist"); + $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Numbers '" . $1 . "' already exist.", "Numbers already exist."); + return; + } catch ($e where { /more than \d+ provided aliases/ }) { + my $err_msg = "more than 10 provided aliases already exist"; + $c->log->error($err_msg); + $self->error($c, HTTP_UNPROCESSABLE_ENTITY, $err_msg); + return; } my $billing_res = { diff --git a/lib/NGCP/Panel/Utils/Subscriber.pm b/lib/NGCP/Panel/Utils/Subscriber.pm index 3ccefa4512..21f2fbbb39 100644 --- a/lib/NGCP/Panel/Utils/Subscriber.pm +++ b/lib/NGCP/Panel/Utils/Subscriber.pm @@ -1384,26 +1384,26 @@ sub update_subscriber_numbers { primary_number_id => $number->id, }); if(defined $prov_subs) { - my $dbalias = $prov_subs->voip_dbaliases->find({ + my $dbalias = $schema->resultset('voip_dbaliases')->search_rs({ username => $cli, - }); + })->first; + + if ($dbalias && $dbalias->subscriber_id != $prov_subs->id) { + die("alias '" . $c->qs($cli) . "' already exists"); + } + if($dbalias) { if(!$dbalias->is_primary) { $dbalias->update({ is_primary => 1 }); } } else { - if ($schema->resultset('voip_dbaliases')->search_rs({ - username => $cli, - })->first) { - die("alias '" . $c->qs($cli) . "' already exists"); - } else { - $dbalias = $prov_subs->voip_dbaliases->create({ - username => $cli, - domain_id => $prov_subs->domain->id, - is_primary => 1, - }); - } + $dbalias = $prov_subs->voip_dbaliases->create({ + username => $cli, + domain_id => $prov_subs->domain->id, + is_primary => 1, + }); } + if(defined $acli_pref) { $acli_pref->search({ value => $old_cli })->delete if($old_cli); if(!$acli_pref->find({ value => $cli })) { @@ -1469,6 +1469,34 @@ sub update_subscriber_numbers { } if(defined $alias_numbers && ref($alias_numbers) eq 'ARRAY') { + + my @alias_numbers_composed = map { + join('', $_->{e164}->{cc}, $_->{e164}->{ac} // '', $_->{e164}->{sn}) + } @$alias_numbers; + + my $foreign_aliases_rs = $schema->resultset('voip_dbaliases')->search_rs({ + username => { 'in' => \@alias_numbers_composed }, + subscriber_id => { '!=' => $prov_subs->id }, + }); + + my $foreign_aliases_count = $foreign_aliases_rs->count(); + + my $current_primary_number; + if (defined $billing_subs->primary_number) { + my %primary_number_parts = $billing_subs->primary_number->get_inflated_columns; + $current_primary_number = join('', @{primary_number_parts}{qw(cc ac sn)}); + } + + if ($foreign_aliases_count) { + if ($foreign_aliases_count == 1) { + die "alias " . $foreign_aliases_rs->first->username . " already exists"; + } elsif ($foreign_aliases_count <= 10) { + die "aliases " . join(',', map {$_->username} $foreign_aliases_rs->all) . " already exist"; + } else { + die "more than 10 provided aliases already exist"; + } + } + my $number; for my $alias(@$alias_numbers) { @@ -1515,29 +1543,28 @@ sub update_subscriber_numbers { $alias->{e164}->{is_devid} = delete $alias->{is_devid}; } - my $dbalias = $prov_subs->voip_dbaliases->search_rs({ + if ($current_primary_number == $cli) { + die "alias '" . $c->qs($cli) . "' is already defined as the primary number"; + } + + my $dbalias = $prov_subs->voip_dbaliases->find({ username => $cli, - is_primary => 0, - })->first; - if($dbalias) { + }); + + if ($dbalias) { $dbalias->update({ - #is_primary => 0, is_devid => $alias->{e164}->{is_devid} // 0, + is_primary => 0, }); } else { - if ($schema->resultset('voip_dbaliases')->search_rs({ - username => $cli, - })->first) { - die("alias '" . $c->qs($cli) . "' already exists"); - } else { - $dbalias = $prov_subs->voip_dbaliases->create({ - username => $cli, - domain_id => $prov_subs->domain->id, - is_primary => 0, - is_devid => $alias->{e164}->{is_devid} // 0, - }); - } + $dbalias = $prov_subs->voip_dbaliases->create({ + username => $cli, + domain_id => $prov_subs->domain->id, + is_primary => 0, + is_devid => $alias->{e164}->{is_devid} // 0, + }); } + if(defined $acli_pref) { $acli_pref->search({ value => $old_cli })->delete if($old_cli); if(!$acli_pref->find({ value => $cli })) {