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

@@ -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;

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};
     }

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);

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';
         }
     }