From 4c53ac4d7d624e6b46a40b8f0a8f0a8f9f2614a6 Mon Sep 17 00:00:00 2001 From: Irina Peshinskaya Date: Thu, 22 Mar 2018 21:11:17 +0100 Subject: [PATCH] TT#34564 Add csv download and upload to PhonebookEntries API Change-Id: Ie4bd350348370ba51fe868cf478244a549dec930 --- .../Panel/Controller/API/PhonebookEntries.pm | 71 ++++++++----------- .../Controller/API/PhonebookEntriesItem.pm | 5 -- lib/NGCP/Panel/Role/API.pm | 45 +++++++----- lib/NGCP/Panel/Role/API/PhonebookEntries.pm | 58 ++++++--------- lib/NGCP/Panel/Role/Entities.pm | 54 ++++++++------ t/api-rest/api-subscribers.t | 2 +- t/lib/Test/Collection.pm | 45 ++++++++---- 7 files changed, 147 insertions(+), 133 deletions(-) diff --git a/lib/NGCP/Panel/Controller/API/PhonebookEntries.pm b/lib/NGCP/Panel/Controller/API/PhonebookEntries.pm index 0a1b8588cf..0917e0b101 100644 --- a/lib/NGCP/Panel/Controller/API/PhonebookEntries.pm +++ b/lib/NGCP/Panel/Controller/API/PhonebookEntries.pm @@ -1,19 +1,17 @@ package NGCP::Panel::Controller::API::PhonebookEntries; -use NGCP::Panel::Utils::Generic qw(:all); use Sipwise::Base; - -use boolean qw(true); -use Data::HAL qw(); -use Data::HAL::Link qw(); -use HTTP::Headers qw(); -use HTTP::Status qw(:constants); +use NGCP::Panel::Utils::Generic qw(:all); use parent qw/NGCP::Panel::Role::Entities NGCP::Panel::Role::API::PhonebookEntries/; +use NGCP::Panel::Utils::Phonebook; + __PACKAGE__->set_config({ POST => { - 'ContentType' => ['text/csv', 'application/json'], + 'ContentType' => ['text/csv', 'application/json'], + #request_params are taken as native hash and doesn't require any json validation or decoding + 'ResourceContentType' => 'native', }, allowed_roles => [qw/admin reseller subscriberadmin subscriber/], }); @@ -35,16 +33,10 @@ sub query_params { { param => 'reseller_id', description => 'Filter for Phonebook entries belonging to a specific reseeller', - query => { - first => sub { - my $q = shift; - { reseller_id => $q }; - }, - second => sub {}, - }, + query_type => 'string_eq', }, { - param => 'contract_id', + param => 'customer_id', description => 'Filter for Phonebook entries belonging to a specific contract', query => { first => sub { @@ -57,24 +49,17 @@ sub query_params { { param => 'subscriber_id', description => 'Filter for Phonebook entries belonging to a specific subscriber', - query => { - first => sub { - my $q = shift; - { subscriber_id => $q }; - }, - second => sub {}, - }, + query_type => 'string_eq', }, { param => 'number', - description => 'Filter for LNP numbers with a specific number (wildcards possible)', - query => { - first => sub { - my $q = shift; - { 'me.number' => { like => $q } }; - }, - second => sub {}, - }, + description => 'Filter for Phonebook numbers with a specific number (wildcards possible)', + query_type => 'string_like', + }, + { + param => 'name', + description => 'Filter for Phonebook numbers with a specific name (wildcards possible)', + query_type => 'string_like', }, ]; } @@ -86,17 +71,23 @@ sub check_create_csv :Private { sub create_csv :Private { my ($self, $c) = @_; - NGCP::Panel::Utils::Phonebook::create_csv( - c => $c, - ); + my($owner_obj,$owner_type,$owner_parameter,$owner_id) = $self->check_owner_params($c); + return unless $owner_obj; + my $rs = $self->_item_rs($c); + NGCP::Panel::Utils::Phonebook::download_csv($c, $rs, $owner_type, $owner_id); } -sub create_item { - my ($self, $c, $resource, $form, $process_extras) = @_; - my $rs = $self->_item_rs($c,undef,$resource);#maybe copy-paste it here? - return unless $rs; - my $item = $rs->create($resource); - return $item; +sub process_data :Private { + my ($self, %params) = @_; + my ($c,$data_ref,$resource,$form,$process_extras) = @params{qw/c data resource form process_extra/}; + my($owner_obj,$owner_type,$owner_parameter,$owner_id) = $self->check_owner_params($c); + return unless $owner_obj; + my $rs = $self->_item_rs($c); + + my ($entries, $fails, $text) = + NGCP::Panel::Utils::Phonebook::upload_csv($c, $rs, $owner_type, $owner_id, + $c->req->params->{purge_existing}, $data_ref); + $c->log->info( $$text ); } 1; diff --git a/lib/NGCP/Panel/Controller/API/PhonebookEntriesItem.pm b/lib/NGCP/Panel/Controller/API/PhonebookEntriesItem.pm index c0d1d4dadf..6a3da74f71 100644 --- a/lib/NGCP/Panel/Controller/API/PhonebookEntriesItem.pm +++ b/lib/NGCP/Panel/Controller/API/PhonebookEntriesItem.pm @@ -5,11 +5,6 @@ use Sipwise::Base; use parent qw/NGCP::Panel::Role::EntitiesItem NGCP::Panel::Role::API::PhonebookEntries/; -use Data::HAL qw(); -use Data::HAL::Link qw(); -use HTTP::Headers qw(); -use HTTP::Status qw(:constants); - __PACKAGE__->set_config({ allowed_roles => [qw/admin reseller subscriberadmin subscriber/], }); diff --git a/lib/NGCP/Panel/Role/API.pm b/lib/NGCP/Panel/Role/API.pm index 4ae668f404..2d16ff7e86 100644 --- a/lib/NGCP/Panel/Role/API.pm +++ b/lib/NGCP/Panel/Role/API.pm @@ -36,14 +36,14 @@ sub get_valid_data{ my $c = $params{c}; my $method = $params{method} // uc($c->request->method); my $media_type = $params{media_type}; - my $json_media_type = $params{json_media_type};#for rare specific cases, like text/csv + my $resource_media_type = $params{resource_media_type};#for rare specific cases, like text/csv return unless $self->forbid_link_header($c); if ($method =~ /^(GET|PUT|POST)$/) { - $json_media_type //= 'application/json'; + $resource_media_type //= 'application/json'; } elsif ($method eq 'PATCH') { - $json_media_type //= 'application/json-patch+json'; + $resource_media_type //= 'application/json-patch+json'; } return unless $self->valid_media_type($c, $media_type); @@ -52,30 +52,38 @@ sub get_valid_data{ return unless $self->valid_id($c, $id); } - my ($json_raw,$json); + my ($json_raw,$json_decoded); if ($c->req->headers->content_type eq 'multipart/form-data') { return unless $self->require_uploads($c); $json_raw = $c->req->param('json'); - } else { + } elsif ($c->req->headers->content_type eq 'application/json' + && 'GET' ne $method) { return unless $self->require_body($c); - $data = $c->stash->{body}; + #overwrite for the first variant of the dual upload + $resource_media_type = 'application/json'; + $json_raw = $c->stash->{body}; + } else { + if ('GET' ne $method) { + return unless $self->require_body($c); + $data = $c->stash->{body}; + } $resource = $c->req->query_params; $non_json_data = 1; } - if ($json_media_type eq 'application/json' || - $json_media_type eq 'application/json-patch+json' ) { + if ($resource_media_type eq 'application/json' || + $resource_media_type eq 'application/json-patch+json' ) { $json_raw //= $data; - return unless $self->require_wellformed_json($c, $json_media_type, $json_raw); - $json = JSON::from_json($json_raw, { utf8 => 1 }); + return unless $self->require_wellformed_json($c, $resource_media_type, $json_raw); + $json_decoded = JSON::from_json($json_raw, { utf8 => 1 }); if ($method eq 'PATCH') { my $ops = $params{ops} // [qw/replace copy/]; - return unless $self->require_valid_patch($c, $json, $ops); + return unless $self->require_valid_patch($c, $json_decoded, $ops); } - return unless $self->get_uploads($c, $json, $params{uploads}, $params{form}); - $resource = $json; + return unless $self->get_uploads($c, $json_decoded, $params{uploads}, $params{form}); + $resource = $json_decoded; $non_json_data = 0; } @@ -90,9 +98,9 @@ sub get_info_data { my ($self, $c) = @_; my $ctype = $self->get_content_type($c) // ''; my $resource = $c->request->params; - my ($resource_json,$resource_json_raw); + my ($resource_json,$resource_json_raw) = (undef,''); if ('multipart/form-data' eq $ctype) { - $resource_json_raw = delete $resource->{json}; + $resource_json = delete $resource->{json}; } elsif ('application/json' eq $ctype) { if ($self->require_body($c)) { $resource_json_raw = $c->stash->{body}; @@ -1276,7 +1284,12 @@ sub return_representation_post{ $response //= HTTP::Response->new(HTTP_OK, undef, HTTP::Headers->new( $hal->http_headers, ), $hal->as_json); - $c->response->header(Location => sprintf('/%s%d', $c->request->path, $self->get_item_id($c, $item))); + $c->response->header( + Location => sprintf('/%s%s', + $c->request->path, + $self->get_item_id( + $c,$item, undef, undef, { purpose => 'hal_links_href' }) + )); } if ('minimal' eq $preference || !$response) { diff --git a/lib/NGCP/Panel/Role/API/PhonebookEntries.pm b/lib/NGCP/Panel/Role/API/PhonebookEntries.pm index f233d60723..d18ae3c787 100644 --- a/lib/NGCP/Panel/Role/API/PhonebookEntries.pm +++ b/lib/NGCP/Panel/Role/API/PhonebookEntries.pm @@ -66,12 +66,24 @@ sub validate_request { return 1; } +sub get_item_id{ + my($self, $c, $item, $resource, $form, $params) = @_; + my $id = int($item->id); + if(('HASH' eq ref $params) && 'hal_links_href' eq $params->{purpose}){ + my($owner,$type,$parameter,$value) = $self->check_owner_params($c); + return unless $owner; + return $id = $id.'?'.$parameter.'='.$value; + } + return $id ; +} + + sub check_owner_params { my($self, $c, $params) = @_; if ($c->stash->{check_owner_params}) { return (@{$c->stash->{check_owner_params}}); } - + my @allowed_params; if ($c->user->roles eq "admin") { @allowed_params = qw/reseller_id customer_id subscriber_id/; @@ -166,15 +178,9 @@ sub check_owner_params { sub get_reseller_phonebook_rs { my ($c, $reseller_id, $context) = @_; - my $list_rs = $c->model('DB')->resultset('reseller_phonebook')->search_rs({ - reseller_id => $reseller_id, - },{ - columns => [qw/id name number/, - { 'owner_id' => 'me.reseller_id' } , - { 'shared' => \'0'}, - ], - }); - + my $list_rs = $c->model('DB')->resultset('resellers')->find({ + id => $reseller_id, + })->phonebook; my $item_rs = $c->model('DB')->resultset('reseller_phonebook'); return ($list_rs,$item_rs); } @@ -182,20 +188,10 @@ sub get_reseller_phonebook_rs { sub get_contract_phonebook_rs { my ($c, $contract_id, $context) = @_; - my $contract_rs = $c->model('DB')->resultset('contracts')->search({ + my $list_rs = $c->model('DB')->resultset('contracts')->find({ id => $contract_id, - })->first; - - my $contract_pb_rs = $c->model('DB')->resultset('contract_phonebook')->search_rs({ - contract_id => $contract_id, - },{ - columns => [qw/id name number/, - { 'owner_id' => 'me.contract_id' } , - { 'shared' => \'0'}, - ], - }); - - my $list_rs = $contract_pb_rs; + })->phonebook; + my $item_rs = $c->model('DB')->resultset('contract_phonebook'); return ($list_rs,$item_rs); } @@ -203,21 +199,9 @@ sub get_contract_phonebook_rs { sub get_subscriber_phonebook_rs { my ($c, $subscriber_id) = @_; - my $subscriber_rs = $c->model('DB')->resultset('voip_subscribers')->search({ + my $list_rs = $c->model('DB')->resultset('voip_subscribers')->find({ id => $subscriber_id, - })->first; - - my $subscriber_pb_rs = $c->model('DB')->resultset('subscriber_phonebook')->search_rs({ - subscriber_id => $subscriber_id, - },{ - columns => [qw/id name number/, - { 'owner_id' => 'me.subscriber_id' } , - { 'shared' => 'me.shared'}, - ], - 'join' => 'subscriber', - }); - - my $list_rs = $subscriber_pb_rs; + })->phonebook; my $item_rs = $c->model('DB')->resultset('subscriber_phonebook'); return ($list_rs,$item_rs); } diff --git a/lib/NGCP/Panel/Role/Entities.pm b/lib/NGCP/Panel/Role/Entities.pm index de9ca93320..b53b4b8580 100644 --- a/lib/NGCP/Panel/Role/Entities.pm +++ b/lib/NGCP/Panel/Role/Entities.pm @@ -66,10 +66,11 @@ sub set_config { #DoesAdd #Method #Path - #ContentType => ['multipart/form-data'],#allowed REQUEST content type - #Uploads => [qw/front_image mac_image/],#uploads filenames + #ContentType => ['multipart/form-data'],#allowed REQUEST content type + #ResourceContentType => 'native' for request_params for csv upload cases to avoid default upload behavior when resource is taken from multipart/form-data {json} body part + #Uploads => [qw/front_image mac_image/],#uploads filenames # or - #Uploads => {'greetingfile' => ['audio/x-wav', 'application/octet-stream']}, + #Uploads => {'greetingfile' => ['audio/x-wav', 'application/octet-stream']}, #own_transaction_control->{PUT|POST|PATCH|DELETE|ALL} = 0|1 - don't start transaction guard in parent classes, implementation need to control it #ReturnContentType => 'binary'#mostly for GET. value different from 'application/json' says that method is going to return binary data using get_item_binary_data #} @@ -110,11 +111,11 @@ sub set_config { AllowedRole => $allowed_roles_by_methods->{$_}, Args => ( $params->{interface_type} eq 'item' ? 1 : 0 ), Does => [ - 'ACL', - ( $params->{interface_type} eq 'item' ? () : ('CheckTrailingSlash') ), + 'ACL', + ( $params->{interface_type} eq 'item' ? () : ('CheckTrailingSlash') ), 'RequireSSL', - ( ref $params_all_methods->{DoesAdd} eq 'ARRAY' ? @$params_all_methods->{DoesAdd} : () ), - ( ref $params_method->{DoesAdd} eq 'ARRAY' ? @$params_method->{DoesAdd} : () ), + ( ref $params_all_methods->{DoesAdd} eq 'ARRAY' ? @$params_all_methods->{DoesAdd} : () ), + ( ref $params_method->{DoesAdd} eq 'ARRAY' ? @$params_method->{DoesAdd} : () ), ], Method => $_, Path => $self->dispatch_path($params->{resource_name}), @@ -143,7 +144,7 @@ sub set_config { AllowedRole => $allowed_roles_by_methods->{'Journal'}, Does => [qw(ACL RequireSSL)], }) } - : + : () ), log_response => 1, log_request => 1, @@ -178,8 +179,8 @@ sub options { my $post_allowed = grep { $_ eq 'POST' } @{ $allowed_methods }; $c->response->headers(HTTP::Headers->new( Allow => join(', ', @{ $allowed_methods }), -# $post_allowed -# ? ( +# $post_allowed +# ? ( Accept_Post => 'application/hal+json; profile=http://purl.org/sipwise/ngcp-api/#rel-'.$self->resource_name, # ) : (), )); @@ -206,6 +207,7 @@ sub get { my @items = 'ARRAY' eq ref $items ? @$items : $items->all; for my $item (@items) { push @embedded, $self->hal_from_item($c, $item, $form, {}); + #TODO: replace hal_links_href by separated method that utilize get_item_id. push @links, Data::HAL::Link->new( relation => 'ngcp:'.$self->resource_name, href => sprintf('/%s%s', $c->request->path, $self->get_item_id($c, @@ -254,11 +256,12 @@ sub post { my $method_config = $self->config->{action}->{POST}; my $process_extras= {}; my ($resource, $data, $non_json_data) = $self->get_valid_data( - c => $c, - method => 'POST', - media_type => $method_config->{ContentType} // 'application/json', - uploads => $method_config->{Uploads} // [] , - form => $form, + c => $c, + method => 'POST', + media_type => $method_config->{ContentType} // 'application/json', + uploads => $method_config->{Uploads} // [] , + form => $form, + resource_media_type => $method_config->{ResourceContentType}, ); last unless $resource; my ($item,$data_processed_result); @@ -280,9 +283,9 @@ sub post { } else { try { #$processed_ok(array), $processed_failed(array), $info, $error - $data_processed_result = $self->process_data( - c => $c, - data => \$data, + $data_processed_result = $self->process_data( + c => $c, + data => \$data, resource => $resource, form => $form, process_extras => $process_extras, @@ -299,14 +302,21 @@ sub post { return if defined $c->stash->{api_error_message}; - $self->return_representation_post($c, - 'item' => $item, - 'form' => $form, - data_processed_result => $data_processed_result ); + $self->return_representation_post($c, + 'item' => $item, + 'form' => $form + ); } return; } +sub create_item { + my ($self, $c, $resource, $form, $process_extras) = @_; + my $rs = $self->_item_rs($c); + return unless $rs; + my $item = $rs->create($resource); + return $item; +} sub POST { my ($self) = shift; diff --git a/t/api-rest/api-subscribers.t b/t/api-rest/api-subscribers.t index 5e5a11c8e8..1afc1e2e22 100644 --- a/t/api-rest/api-subscribers.t +++ b/t/api-rest/api-subscribers.t @@ -28,7 +28,7 @@ $fake_data->set_data_from_script({ username => 'api_test_username', password => 'api_test_password', webusername => 'api_test_webusername', - webpassword => undef, + webpassword => 'web_password_1', domain_id => sub { return shift->get_id('domains',@_); }, #domain_id => email => undef, diff --git a/t/lib/Test/Collection.pm b/t/lib/Test/Collection.pm index 549a9dca05..06b3823b7a 100644 --- a/t/lib/Test/Collection.pm +++ b/t/lib/Test/Collection.pm @@ -669,6 +669,7 @@ sub get_request_put{ $req->header('Prefer' => 'return=representation'); return $req; } + sub get_request_patch{ my($self,$uri,$content) = @_; $uri ||= $self->get_uri_current; @@ -681,6 +682,20 @@ sub get_request_patch{ return $req; } +sub get_request_post{ + my($self,$content,$uri) = @_; + $uri ||= $self->get_uri_current; + $uri = $self->normalize_uri($uri); + #This is for multipart/form-data cases + my $content_type; + ($content,$content_type) = $self->encode_content($content, $self->content_type->{POST}); + my $req = POST $uri, + Content_Type => $content_type, + $content ? ( Content => $content ) : (); + $req->header('Prefer' => 'return=representation'); + return $req; +} + sub request_put{ my($self,$content,$uri) = @_; $uri ||= $self->get_uri_current; @@ -707,16 +722,20 @@ sub request_patch{ sub request_post{ my($self, $content, $uri, $req) = @_; - $uri ||= $self->get_uri_collection; - $uri = $self->normalize_uri($uri); - diag("request_post: uri: $uri;"); - my $content_type; - ($content,$content_type) = $self->encode_content($content, $self->content_type->{POST}); - #form-data is set automatically, despite on $self->content_type->{POST} - $req ||= POST $uri, - Content_Type => $content_type, - Content => $content; - $req->header('Prefer' => 'return=representation'); + if(!$req){ + $uri ||= $self->get_uri_collection; + $uri = $self->normalize_uri($uri); + my $content_type; + ($content,$content_type) = $self->encode_content($content, $self->content_type->{POST}); + #form-data is set automatically, despite on $self->content_type->{POST} + $req ||= POST $uri, + Content_Type => $content_type, + Content => $content; + $req->header('Prefer' => 'return=representation'); + } + diag("request_post: uri: ".$req->uri.";"); + diag("request_post: content: ".$req->content.";"); + diag("request_post: content_type: ".$req->header('Content-Type').";"); my $res = $self->request($req); my $rescontent = $self->get_response_content($res); my $location = $res->header('Location') // ''; @@ -773,8 +792,10 @@ sub request_delete{ sub request_get{ my($self,$uri,$req,$headers) = @_; - $uri = $self->normalize_uri($uri); - $req //= $self->get_request_get($uri,$headers); + if (!$req) { + $uri = $self->normalize_uri($uri); + $req //= $self->get_request_get($uri,$headers); + } my $res = $self->request($req); my $content = $self->get_response_content($res); return wantarray ? ($res, $content, $req) : $res;