From bb655be001064e2400cf4263f4c8f187c867d34a Mon Sep 17 00:00:00 2001 From: Irina Peshinskaya Date: Wed, 28 Feb 2018 12:50:11 +0100 Subject: [PATCH] TT#32913 Synchronize access to voip_numbers to avoid unique index error in simultaneous requests Change-Id: I0e848445b87ee3104c97f9353b4b9114e51b8b1b --- lib/NGCP/Panel/Controller/API/NumbersItem.pm | 13 +++++--- lib/NGCP/Panel/Role/API.pm | 8 +++++ t/api-rest/api-numbers.t | 31 ++++++++++++++++---- t/lib/Test/Collection.pm | 5 +++- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/lib/NGCP/Panel/Controller/API/NumbersItem.pm b/lib/NGCP/Panel/Controller/API/NumbersItem.pm index c72b06376e..2588fa49d9 100644 --- a/lib/NGCP/Panel/Controller/API/NumbersItem.pm +++ b/lib/NGCP/Panel/Controller/API/NumbersItem.pm @@ -12,7 +12,8 @@ __PACKAGE__->set_config({ allowed_roles => { Default => [qw/admin reseller subscriberadmin/], Journal => [qw/admin reseller/], - } + }, + set_transaction_isolation => 'READ COMMITTED', }); sub allowed_methods{ @@ -48,9 +49,14 @@ sub update_item_model { form => $form, resource => $resource, ); + my @sub_ids = ($resource->{subscriber_id}, $old_resource->{subscriber_id}); + my %subs = map { + $_ => $schema->resultset('voip_subscribers')->find( + {id => $_},{for => 'update'} + ) + } sort {$a <=> $b } @sub_ids; + my ($sub,$old_sub) = @subs{@sub_ids}; - my $sub = $schema->resultset('voip_subscribers') - ->find($resource->{subscriber_id}); unless($sub) { $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid 'subscriber_id', does not exist."); return; @@ -59,7 +65,6 @@ sub update_item_model { $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid 'subscriber_id', does not exist."); return; } - my $old_sub = $schema->resultset('voip_subscribers')->find($old_resource->{subscriber_id}); if($old_sub->primary_number_id == $old_resource->{id}) { $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Cannot reassign primary number, already at subscriber ".$old_sub->id); return; diff --git a/lib/NGCP/Panel/Role/API.pm b/lib/NGCP/Panel/Role/API.pm index b86e172eb3..7871ce6b73 100644 --- a/lib/NGCP/Panel/Role/API.pm +++ b/lib/NGCP/Panel/Role/API.pm @@ -1131,6 +1131,14 @@ sub get_transaction_control{ $step //= 'init'; if($self->check_transaction_control($c, $action, $step, %params)){ #todo: put it into class variables? + if ($self->config->{set_transaction_isolation}) { + my $transaction_isolation_level = + ( (length $self->config->{set_transaction_isolation} > 1 ) + && lc $self->config->{set_transaction_isolation} ne 'default' ) + ? $self->config->{set_transaction_isolation} + : 'READ COMMITTED'; + $c->model('DB')->set_transaction_isolation($transaction_isolation_level); + } $c->stash->{transaction_quard} = $schema->txn_scope_guard; return $c->stash->{transaction_quard}; } diff --git a/t/api-rest/api-numbers.t b/t/api-rest/api-numbers.t index a1e46de440..922471edb2 100644 --- a/t/api-rest/api-numbers.t +++ b/t/api-rest/api-numbers.t @@ -6,6 +6,7 @@ use Test::FakeData; use Test::More; use Data::Dumper; use Clone qw/clone/; +use threads; #use NGCP::Panel::Utils::Subscriber; @@ -43,19 +44,22 @@ $test_machine->form_data_item(); { cc=> '111', ac => $ticket, sn => $time }, { cc=> '112', ac => $ticket, sn => $time }, { cc=> '113', ac => $ticket, sn => $time }, +# { cc=> '114', ac => $ticket, sn => $time }, +# { cc=> '115', ac => $ticket, sn => $time }, ]; $subscriber2->{content}->{alias_numbers} = [ { cc=> '211', ac => $ticket, sn => $time }, { cc=> '212', ac => $ticket, sn => $time }, { cc=> '213', ac => $ticket, sn => $time }, +# { cc=> '214', ac => $ticket, sn => $time }, +# { cc=> '215', ac => $ticket, sn => $time }, ]; my ($res,$content,$request); ($res,$content,$request) = $subscriber_test_machine->request_put(@{$subscriber1}{qw/content location/}); ($res,$content,$request) = $subscriber_test_machine->request_put(@{$subscriber2}{qw/content location/}); - my ($alias1) = $test_machine->get_item_hal('numbers', '/api/numbers/?type=alias&subscriber_id='.$subscriber1->{content}->{id}); - my ($alias2) = $test_machine->get_item_hal('numbers','/api/numbers/?type=alias&subscriber_id='.$subscriber2->{content}->{id}); - - test_numbers_reassign($alias1,$alias2,$subscriber1,$subscriber2); + my ($aliases1) = $test_machine->get_collection_hal('numbers', '/api/numbers/?type=alias&subscriber_id='.$subscriber1->{content}->{id}, 1)->{collection}; + my ($aliases2) = $test_machine->get_collection_hal('numbers','/api/numbers/?type=alias&subscriber_id='.$subscriber2->{content}->{id}, 1)->{collection}; + test_numbers_reassign($aliases1->[0],$aliases2->[0],$subscriber1,$subscriber2); my $pbxsubscriberadmin = $fake_data->create('subscribers')->[0]; ($res) = $subscriber_test_machine->request_patch([ @@ -69,8 +73,14 @@ $test_machine->form_data_item(); $test_machine->set_subscriber_credentials($pbxsubscriberadmin->{content}); $test_machine->runas('subscriber'); - test_numbers_reassign($alias1,$alias2,$subscriber1,$subscriber2); - + test_numbers_reassign($aliases1->[0],$aliases2->[0],$subscriber1,$subscriber2); + my @threads; + for(my $i = 0; $i < @{$aliases1} && $i < @{$aliases2}; $i++ ){ + push @threads, threads->create(\&test_numbers_reassign,$aliases1->[$i],$aliases2->[$i],$subscriber1,$subscriber2); + } + for(my $i=0; $i < @threads; $i++ ){ + $threads[$i]->join(); + } $test_machine->runas('admin'); $subscriber_test_machine->clear_test_data_all();#fake data aren't registered in this test @@ -86,6 +96,15 @@ done_testing; sub test_numbers_reassign{ my($alias1,$alias2,$subscriber1,$subscriber2) = @_; my $res; + + #print Dumper [ + # [ { op => 'replace', path => '/subscriber_id', value => $subscriber2->{content}->{id} } ] , $alias1, + # [ { op => 'replace', path => '/subscriber_id', value => $subscriber1->{content}->{id} } ] , $alias1, + # [ { op => 'replace', path => '/subscriber_id', value => $subscriber1->{content}->{id} } ] , $alias2, + # [ { op => 'replace', path => '/subscriber_id', value => $subscriber2->{content}->{id} } ] , $alias2 + # + #]; + $alias1->{content}->{subscriber_id} = $subscriber2->{content}->{id}; ($res) = $test_machine->request_patch([ { op => 'replace', path => '/subscriber_id', value => $subscriber2->{content}->{id} } ] , $alias1->{location}); $test_machine->http_code_msg(200, "PATCH for /numbers/", $res); diff --git a/t/lib/Test/Collection.pm b/t/lib/Test/Collection.pm index f7b6844a55..72c8a0d59e 100644 --- a/t/lib/Test/Collection.pm +++ b/t/lib/Test/Collection.pm @@ -515,6 +515,7 @@ sub get_hal_from_collection{ } return ($reshal,$location,$total_count,$reshal_collection); } + sub get_collection_hal{ my($self,$name, $uri, $reload, $page, $rows) = @_; my (@reshals, $location,$total_count,$reshal_collection,$rescollection,$firstitem,$res,$list_collection,$req); @@ -602,7 +603,9 @@ sub encode_content{ ]; $content_type_res = 'multipart/form-data'; }elsif( $json_types{$type} && (('HASH' eq ref $content) ||('ARRAY' eq ref $content)) ){ - $content_res = JSON::to_json($content); + #print Dumper $content; + my $json = JSON->new->allow_nonref; + $content_res = $json->encode($content); $type eq 'json' and $content_type_res = 'application/json'; } }