From dd9365f0e8315f953ec52d51b4902f5da529f2b2 Mon Sep 17 00:00:00 2001 From: Irina Peshinskaya Date: Wed, 2 Sep 2015 01:59:19 +0300 Subject: [PATCH] MT#14739 Get newly created kamailio user location after flush MT#14779 Add default value for the q parameter MT#14891 Consider form validation for the POST request (create location) MT#14789 Remove user_agent registration parameter Change-Id: I9f81ec3ec5c308c731b3dfe6aa5f075c7d7b50ab --- .../Controller/API/SubscriberRegistrations.pm | 9 +- lib/NGCP/Panel/Controller/Subscriber.pm | 1 + .../Panel/Form/Subscriber/RegisteredAPI.pm | 26 +++--- lib/NGCP/Panel/Role/API.pm | 3 +- .../Panel/Role/API/SubscriberRegistrations.pm | 26 ++++-- lib/NGCP/Panel/Utils/Kamailio.pm | 18 +++- t/api-rest/api-subscriberregistrations.t | 86 +++++++++++++++++++ t/api-rest/api-vouchers.t | 2 +- t/lib/Test/Collection.pm | 5 ++ 9 files changed, 149 insertions(+), 27 deletions(-) create mode 100644 t/api-rest/api-subscriberregistrations.t diff --git a/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm b/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm index 6bdb8aa694..229831aa35 100644 --- a/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm +++ b/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm @@ -169,12 +169,11 @@ sub POST :Allow { my $form = $self->get_form($c); my $create = 1; - $self->update_item($c, undef, undef, $resource, $form, $create); - + my $item = $self->update_item($c, undef, undef, $resource, $form, $create); + last unless $item; + $c->response->status(HTTP_CREATED); - # TODO: can we somehow get our item back from an xmlrpc call? probably not as - # it is async - #$c->response->header(Location => sprintf('/%s%d', $c->request->path, $item->id)); + $c->response->header(Location => sprintf('/%s%d', $c->request->path, $item->id)); $c->response->body(q()); } return; diff --git a/lib/NGCP/Panel/Controller/Subscriber.pm b/lib/NGCP/Panel/Controller/Subscriber.pm index 0090ed455d..c526005f26 100644 --- a/lib/NGCP/Panel/Controller/Subscriber.pm +++ b/lib/NGCP/Panel/Controller/Subscriber.pm @@ -2082,6 +2082,7 @@ sub master :Chained('base') :PathPart('details') :CaptureArgs(0) { ]); $c->stash->{reg_dt_columns} = NGCP::Panel::Utils::Datatables::set_columns($c, [ { name => "id", search => 1, title => $c->loc('#') }, + #left untouchable, although user_agent is always the same by design, see MT 14789 notes { name => "user_agent", search => 1, title => $c->loc('User Agent') }, { name => "contact", search => 1, title => $c->loc('Contact') }, { name => "expires", search => 1, title => $c->loc('Expires') }, diff --git a/lib/NGCP/Panel/Form/Subscriber/RegisteredAPI.pm b/lib/NGCP/Panel/Form/Subscriber/RegisteredAPI.pm index 4fc79c1927..1617de2493 100644 --- a/lib/NGCP/Panel/Form/Subscriber/RegisteredAPI.pm +++ b/lib/NGCP/Panel/Form/Subscriber/RegisteredAPI.pm @@ -7,6 +7,7 @@ use Moose::Util::TypeConstraints; use HTML::FormHandler::Widget::Block::Bootstrap; has '+widget_wrapper' => ( default => 'Bootstrap' ); +has '+use_fields_for_input_without_param' => ( default => 1 ); has_field 'submitid' => ( type => 'Hidden' ); has_field 'subscriber_id' => ( @@ -36,22 +37,18 @@ has_field 'expires' => ( }, ); -has_field 'user_agent' => ( - type => 'Text', - required => 1, - element_attr => { - rel => ['tooltip'], - title => ['The user agent registered at this contact.'] - }, -); - has_field 'q' => ( type => 'Float', - required => 0, + required => 1, + range_start => -1, + range_end => 1, + decimal_symbol => '.', + default => 1, element_attr => { rel => ['tooltip'], title => ['The priority (q-value) of the registration.'] }, + #validate_method => \&validate_q, ); has_field 'nat' => ( @@ -62,7 +59,14 @@ has_field 'nat' => ( title => ['The registered contact is detected as behind NAT.'] }, ); - +sub validate_q { + my ($self,$field) = @_; + if(($field->value < -1) || ($field->value > 1)){ + $field->add_error('Value of "q" must be a float value between -1 and 1'); + return; + } + return 1; +} =pod sub validate { my $self = shift; diff --git a/lib/NGCP/Panel/Role/API.pm b/lib/NGCP/Panel/Role/API.pm index af94282bc3..299d5ad176 100644 --- a/lib/NGCP/Panel/Role/API.pm +++ b/lib/NGCP/Panel/Role/API.pm @@ -100,6 +100,7 @@ sub validate_form { my $form = $params{form}; my $run = $params{run} // 1; my $exceptions = $params{exceptions} // []; + my $form_params = $params{form_params} // {}; push @{ $exceptions }, "external_id"; my @normalized = (); @@ -119,7 +120,7 @@ sub validate_form { if($run) { # check keys/vals - $form->process(params => $resource, posted => 1); + $form->process(params => $resource, posted => 1, %$form_params ); unless($form->validated) { my $e = join '; ', map { sprintf 'field=\'%s\', input=\'%s\', errors=\'%s\'', diff --git a/lib/NGCP/Panel/Role/API/SubscriberRegistrations.pm b/lib/NGCP/Panel/Role/API/SubscriberRegistrations.pm index 9ce3b2cce1..fa8d0165dd 100644 --- a/lib/NGCP/Panel/Role/API/SubscriberRegistrations.pm +++ b/lib/NGCP/Panel/Role/API/SubscriberRegistrations.pm @@ -152,6 +152,8 @@ sub update_item { form => $form, resource => $resource, exceptions => [ "subscriber_id" ], + run => 1, + #form_params => { 'use_fields_for_input_without_param' => 1 }, ); my $sub = $self->subscriber_from_id($c, $resource->{subscriber_id}); @@ -161,23 +163,30 @@ sub update_item { $self->delete_item($c, $item); } my $cflags = 0; - $cflags |= 64 if($resource->{nat}); + $cflags |= 64 if($form->values->{nat}); NGCP::Panel::Utils::Kamailio::create_location($c, $sub->provisioning_voip_subscriber, - $resource->{contact}, - $resource->{q}, - $resource->{expires}, + $form->values->{contact}, + $form->values->{q}, + $form->values->{expires}, 0, # flags $cflags ); - - unless($create) { + + if($create) { + NGCP::Panel::Utils::Kamailio::flush($c); + my $rs = $self->item_rs($c); + my $aor = NGCP::Panel::Utils::Kamailio::get_aor( $c, $sub->provisioning_voip_subscriber ); + $item = $rs->search_rs(undef, { + 'me.contact' => $form->values->{contact}, + 'me.aor' => $aor, + })->first; + }else{ # we need to reload it since we changed the content via an external # xmlrpc call $item->discard_changes; - - return $item; } + return $item; } sub delete_item { @@ -187,6 +196,7 @@ sub delete_item { return unless($sub); NGCP::Panel::Utils::Kamailio::delete_location_contact($c, $sub, $item->contact); + NGCP::Panel::Utils::Kamailio::flush($c); return 1; } diff --git a/lib/NGCP/Panel/Utils/Kamailio.pm b/lib/NGCP/Panel/Utils/Kamailio.pm index 4d8ac32b14..2f5e960c58 100644 --- a/lib/NGCP/Panel/Utils/Kamailio.pm +++ b/lib/NGCP/Panel/Utils/Kamailio.pm @@ -45,7 +45,7 @@ EOF sub create_location { my ($c, $prov_subscriber, $contact, $q, $expires, $flags, $cflags) = @_; - my $aor = $prov_subscriber->username . '@' . $prov_subscriber->domain->domain; + my $aor = get_aor($c, $prov_subscriber); my $path = $c->config->{sip}->{path} || ''; if($expires) { $expires = NGCP::Panel::Utils::DateTime::from_string($expires)->epoch; @@ -74,6 +74,22 @@ sub create_location { EOF } +sub flush { + my ($c) = @_; + + my $dispatcher = NGCP::Panel::Utils::XMLDispatcher->new; + my $ret = $dispatcher->dispatch($c, "proxy-ng", 1, 1, < + +ul.flush + +EOF +} + +sub get_aor{ + my ($c, $prov_subscriber) = @_; + return $prov_subscriber->username . '@' . $prov_subscriber->domain->domain; +} 1; # vim: set tabstop=4 expandtab: diff --git a/t/api-rest/api-subscriberregistrations.t b/t/api-rest/api-subscriberregistrations.t new file mode 100644 index 0000000000..0c86be4b7f --- /dev/null +++ b/t/api-rest/api-subscriberregistrations.t @@ -0,0 +1,86 @@ +#use Sipwise::Base; +use strict; + +#use Moose; +use Sipwise::Base; +use Test::Collection; +use Test::FakeData; +use Test::More; +use Data::Dumper; + +use NGCP::Panel::Utils::DateTime; + + +#init test_machine +my $test_machine = Test::Collection->new( + name => 'subscriberregistrations', +); +my $fake_data = Test::FakeData->new; + +$test_machine->methods->{collection}->{allowed} = {map {$_ => 1} qw(GET HEAD OPTIONS POST)}; +$test_machine->methods->{item}->{allowed} = {map {$_ => 1} qw(GET HEAD OPTIONS PUT PATCH DELETE)}; + +my $expires = NGCP::Panel::Utils::DateTime::current_local(); + +$fake_data->set_data_from_script({ + 'subscriberregistrations' => { + data => { + 'contact' => 'test', + 'expires' => $expires->ymd('-') . ' ' . $expires->hms(':'), + 'q' => 0.5, + 'subscriber_id' => sub { return shift->get_id('subscribers', @_); }, + }, + }, +}); + +$test_machine->DATA_ITEM_STORE($fake_data->process('subscriberregistrations')); +$test_machine->form_data_item( ); + +# create 3 new vouchers from DATA_ITEM +$test_machine->check_create_correct( 3, sub{ $_[0]->{contact} .= time().'_'.$_[1]->{i} ; } ); + +#order of [check_bundle, check_get2put ] is important here: +#subscriberregistrations really is just a wrapper arounf kamailio rpc calls, +#and update of the existing item is made as delete+create. So, on every PUT or PATCH we delete item, and create new. +#It makes internal Collection list of created items misordered with real data in db, +#because Collection just keeps all created item, and doesn't try to recreate them on every update +$test_machine->check_bundle(); +$test_machine->clear_test_data_all(); + +$test_machine->check_create_correct( 1, sub{ $_[0]->{contact} .= time().'_'.$_[1]->{i} ; } ); +$test_machine->check_get2put(); +$test_machine->clear_data_created(); + +$test_machine->check_create_correct( 1, sub{ $_[0]->{contact} .= time().'_'.$_[1]->{i} ; } ); +{ + my($res, $content) = $test_machine->request_post(sub{$_[0]->{q} = 2;$_[0]->{contact} .= time().'_MT14779_1' ;}); + $test_machine->http_code_msg(422, "check creation of the subscriber registration with q > 1. MT#14779",$res,$content); +} +{ + my($res, $content) = $test_machine->request_post(sub{$_[0]->{q} = -2;$_[0]->{contact} .= time().'_MT14779_2' ;}); + $test_machine->http_code_msg(422, "check creation of the subscriber registration with q < -1. MT#14779",$res,$content); +} +{ + # Default value should be used. + my($res, $content) = $test_machine->request_post(sub{delete $_[0]->{q};$_[0]->{contact} .= time().'_MT14779_3';}); + $test_machine->http_code_msg(201, "check creation of the subscriber registration without q. MT#14779.",$res,$content); +} +{ + my($res, $content) = $test_machine->request_post(sub{delete $_[0]->{expires};$_[0]->{contact} .= time().'_MT14891_1';}); + $test_machine->http_code_msg(422, "check creation of the subscriber registration without required expires. MT#14891.",$res,$content); +} + +#api doesn't deny extra fields +#{ +# my($res, $content) = $test_machine->request_post(sub{$_[0]->{user_agent} = 'Test User Agent';$_[0]->{contact} .= time().'_MT14789' ;}); +# $test_machine->http_code_msg(422, "check creation of the subscriber registration without required expires. MT#14789.",$res,$content); +#} + + + +$test_machine->clear_test_data_all(); + + +done_testing; + +# vim: set tabstop=4 expandtab: diff --git a/t/api-rest/api-vouchers.t b/t/api-rest/api-vouchers.t index d005a881de..9d3373e4c0 100644 --- a/t/api-rest/api-vouchers.t +++ b/t/api-rest/api-vouchers.t @@ -64,7 +64,7 @@ my $voucher_uri; } { my($res,$content) = $test_machine->request_patch( [ { op => 'replace', path => '/valid_until', value => '2099-01-01 00:00:00' } ] ); - $test_machine->http_code_msg(422, "check patched invalid billing_zone_id",$res,,$content); + $test_machine->http_code_msg(422, "check patched invalid billing_zone_id",$res,$content); } $test_machine->clear_test_data_all(); diff --git a/t/lib/Test/Collection.pm b/t/lib/Test/Collection.pm index 8c89ae6c99..10da7e7857 100644 --- a/t/lib/Test/Collection.pm +++ b/t/lib/Test/Collection.pm @@ -462,6 +462,7 @@ sub clear_test_data_all{ my($req,$res,$content) = $self->request_delete($self->base_uri.$del_uri); $self->http_code_msg(204, "check delete item $del_uri",$res,$content); } + $self->clear_data_created(); } sub clear_test_data_dependent{ my($self,$uri) = @_; @@ -544,6 +545,10 @@ sub check_created_listed{ delete $created_items->{$_}; } is(scalar(keys %{$created_items}), 0, "check if all created test items have been foundin the list"); + if(scalar(keys %{$created_items})){ + print Dumper $created_items; + print Dumper $listed; + } } sub check_item_get{