From aeb5cd9c28ad5e6474709a963144d677ed17677f Mon Sep 17 00:00:00 2001 From: Irina Peshinskaya Date: Sun, 11 Nov 2018 22:01:49 +0100 Subject: [PATCH] TT#46183 Add "mode" modifier to "add" PATCH operator Change-Id: Ic23c427ca70d3ed5d633c712468dbf3bda219563 --- .../API/SubscriberPreferencesItem.pm | 18 +++---- lib/NGCP/Panel/Role/API.pm | 48 +++++++++++++++++-- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/lib/NGCP/Panel/Controller/API/SubscriberPreferencesItem.pm b/lib/NGCP/Panel/Controller/API/SubscriberPreferencesItem.pm index 00dacde3e8..30a443e86b 100644 --- a/lib/NGCP/Panel/Controller/API/SubscriberPreferencesItem.pm +++ b/lib/NGCP/Panel/Controller/API/SubscriberPreferencesItem.pm @@ -77,7 +77,7 @@ sub GET :Allow { sub PATCH :Allow { my ($self, $c, $id) = @_; - $c->model('DB')->set_transaction_isolation('READ COMMITTED'); + $c->model('DB')->set_transaction_isolation('READ COMMITTED'); my $guard = $c->model('DB')->txn_scope_guard; { my $preference = $self->require_preference($c); @@ -96,7 +96,7 @@ sub PATCH :Allow { my $balance = NGCP::Panel::Utils::ProfilePackages::get_contract_balance(c => $c, contract => $subscriber->contract, ); #apply underrun lock level - + my $old_resource = $self->get_resource($c, $subscriber, "subscribers"); my $resource = $self->apply_patch($c, $old_resource, $json); last unless $resource; @@ -105,11 +105,11 @@ sub PATCH :Allow { # for proper PATCH behavior $subscriber = $self->update_item($c, $subscriber, $old_resource, $resource, 0, "subscribers"); last unless $subscriber; - + my $hal = $self->hal_from_item($c, $subscriber, "subscribers"); last unless $self->add_update_journal_item_hal($c,$hal); - $guard->commit; + $guard->commit; if ('minimal' eq $preference) { $c->response->status(HTTP_NO_CONTENT); @@ -130,7 +130,7 @@ sub PATCH :Allow { sub PUT :Allow { my ($self, $c, $id) = @_; - $c->model('DB')->set_transaction_isolation('READ COMMITTED'); + $c->model('DB')->set_transaction_isolation('READ COMMITTED'); my $guard = $c->model('DB')->txn_scope_guard; { my $preference = $self->require_preference($c); @@ -140,7 +140,7 @@ sub PUT :Allow { last unless $self->resource_exists($c, systemcontact => $subscriber); my $balance = NGCP::Panel::Utils::ProfilePackages::get_contract_balance(c => $c, contract => $subscriber->contract, - ); #apply underrun lock level + ); #apply underrun lock level my $resource = $self->get_valid_put_data( c => $c, id => $id, @@ -156,8 +156,8 @@ sub PUT :Allow { my $hal = $self->hal_from_item($c, $subscriber, "subscribers"); last unless $self->add_update_journal_item_hal($c,$hal); - - $guard->commit; + + $guard->commit; if ('minimal' eq $preference) { $c->response->status(HTTP_NO_CONTENT); @@ -178,7 +178,7 @@ sub PUT :Allow { sub get_journal_methods{ return [qw/handle_item_base_journal handle_journals_get handle_journalsitem_get handle_journals_options handle_journalsitem_options handle_journals_head handle_journalsitem_head/]; -} +} 1; diff --git a/lib/NGCP/Panel/Role/API.pm b/lib/NGCP/Panel/Role/API.pm index 8e24a1c80b..b5e3f8300a 100644 --- a/lib/NGCP/Panel/Role/API.pm +++ b/lib/NGCP/Panel/Role/API.pm @@ -577,7 +577,11 @@ sub require_valid_patch { 'replace' => { 'path' => 1, 'value' => 1 }, 'copy' => { 'from' => 1, 'path' => 1 }, 'remove' => { 'path' => 1 }, - 'add' => { 'path' => 1, 'value' => 1 }, + 'add' => { 'path' => 1, 'value' => 1, mode => { + required => 0, + allowed_values => [qw/append/], + }, + }, 'test' => { 'path' => 1, 'value' => 1 }, 'move' => { 'from' => 1, 'path' => 1 }, }; @@ -615,6 +619,22 @@ sub require_valid_patch { } delete $tmpops->{$op}->{$k}; } + #remove optional op parameters, so only mandatory will stay + foreach my $k(keys %{ $tmpops->{$op} }) { + if (!ref $tmpops->{$op}->{$k} && !$tmpops->{$op}->{$k}) { + delete $tmpops->{$op}->{$k}; + } elsif (ref $tmpops->{$op}->{$k} eq 'HASH') { + if (defined $tmpops->{$op}->{$k}->{allowed_values} && $elem->{$k}) { + if (!grep {$elem->{$k} eq $_} @{$tmpops->{$op}->{$k}->{allowed_values}}) { + $self->error($c, HTTP_BAD_REQUEST, "Invalid PATCH op '".$tmpops->{$op}."' modifier '$k' value '".$elem->{$k}."'. Allowed values are '". (join("', '", @{$tmpops->{$op}->{$k}->{allowed_values}}) ) . "'"); + } + } + if (exists $tmpops->{$op}->{$k}->{required} && ! $tmpops->{$op}->{$k}->{required}) { + #by default all op spec keys are required, so only those with required = 0 shouldn't be cjecked in $elem + delete $tmpops->{$op}->{$k}; + } + } + } if(keys %{ $tmpops->{$op} }) { $self->error($c, HTTP_BAD_REQUEST, "Missing PATCH keys ". (join(', ', map { "'".$_."'" } keys %{ $tmpops->{$op} }) ) . " for op '$op'"); return; @@ -775,10 +795,32 @@ sub collection_nav_links { return @links; } +#this method expands the newly added ops/modes to the know patch ops. +sub process_patch_description { + my ($self, $c, $entity, $patch) = @_; + my $patch_diff = []; + my $op_iterator = -1; + for my $op (@{ $patch }) { + $op_iterator++; + if ($op->{op} eq 'add' && $op->{mode} && $op->{mode} eq 'append') { + splice @$patch, $op_iterator, 1; + $op->{path} =~s/\/\-$//;#we will add it if element exists + my $value_current = JSON::Pointer->get($entity, $op->{path}); + if (!$value_current) { + push @$patch_diff, {"op" => "add", "path" => $op->{path}, "value" => $op->{value}}; + } else { + push @$patch_diff, map {{"op" => "add", "path" => $op->{path}.'/-', "value" => $_}} ref $op->{value} eq 'ARRAY' ? @{$op->{value}} : ($op->{value}); + } + } + } + push @$patch, @$patch_diff; +} + sub apply_patch { my ($self, $c, $entity, $json, $optional_field_code_ref) = @_; my $patch = JSON::decode_json($json); try { + $self->process_patch_description($c, Storable::dclone($entity), $patch); for my $op (@{ $patch }) { my $coderef = JSON::Pointer->can($op->{op}); die "invalid op '".$op->{op}."' despite schema validation" unless $coderef; @@ -1494,11 +1536,11 @@ sub check_return_type { #while not strict requirement to the config my $result = 1; if ($allowed_types) { - if ( (!ref $allowed_types && $requested_type ne 'binary' && index($requested_type, $allowed_types) < 0) + if ( (!ref $allowed_types && $requested_type ne 'binary' && index($requested_type, $allowed_types) < 0) || ( ref $allowed_types eq 'ARRAY' && !grep {index($requested_type, $_) > -1} @$allowed_types - ) + ) ) { $result = 0; }