From 9e3653341aa1dcfda9d29ce5ccddfad1fb4f45d0 Mon Sep 17 00:00:00 2001
From: Kirill Solomko <ksolomko@sipwise.com>
Date: Thu, 4 Apr 2024 16:18:49 +0200
Subject: [PATCH] MT#59478 adapt /api/cftimesets to global transactions, more
fixes
* /api/cftimesets now fully use Entities/EntitiesItem
* EntitiesItem: delete(): fix delete_item() expression processing
* Entities/EntitiesItem: post/put/patch/delete: go to TX_END
in scenarios where after a method call (e.g. update_item()) there
is a normal return from the function but errors in $c->error, so
that they are also caught correctly
Change-Id: I3bef409ded590796c2bba4f30acd28b02e99065b
---
lib/NGCP/Panel/Controller/API/CFTimeSets.pm | 156 +++++-------------
.../Panel/Controller/API/CFTimeSetsItem.pm | 125 ++------------
lib/NGCP/Panel/Role/API/CFTimeSets.pm | 21 ++-
lib/NGCP/Panel/Role/EntitiesItem.pm | 14 +-
4 files changed, 89 insertions(+), 227 deletions(-)
diff --git a/lib/NGCP/Panel/Controller/API/CFTimeSets.pm b/lib/NGCP/Panel/Controller/API/CFTimeSets.pm
index cfcc121ef5..142c781cba 100644
--- a/lib/NGCP/Panel/Controller/API/CFTimeSets.pm
+++ b/lib/NGCP/Panel/Controller/API/CFTimeSets.pm
@@ -61,124 +61,56 @@ __PACKAGE__->set_config({
allowed_roles => [qw/admin reseller ccareadmin ccare subscriberadmin subscriber/],
});
-sub GET :Allow {
- my ($self, $c) = @_;
- my $page = $c->request->params->{page} // 1;
- my $rows = $c->request->params->{rows} // 10;
- {
- my $timesets = $self->item_rs($c);
-
- (my $total_count, $timesets, my $timesets_rows) = $self->paginate_order_collection($c, $timesets);
- my (@embedded, @links);
- for my $tset (@$timesets_rows) {
- push @embedded, $self->hal_from_item($c, $tset, "cftimesets");
- push @links, Data::HAL::Link->new(
- relation => 'ngcp:'.$self->resource_name,
- href => sprintf('%s%d', $self->dispatch_path, $tset->id),
- );
- }
- push @links,
- Data::HAL::Link->new(
- relation => 'curies',
- href => 'http://purl.org/sipwise/ngcp-api/#rel-{rel}',
- name => 'ngcp',
- templated => true,
- ),
- Data::HAL::Link->new(relation => 'profile', href => 'http://purl.org/sipwise/ngcp-api/'),
- $self->collection_nav_links($c, $page, $rows, $total_count, $c->request->path, $c->request->query_params);
-
- my $hal = Data::HAL->new(
- embedded => [@embedded],
- links => [@links],
- );
- $hal->resource({
- total_count => $total_count,
+sub create_item {
+ my ($self, $c, $resource, $form, $process_extras) = @_;
+
+ my $schema = $c->model('DB');
+
+ my $tset;
+
+ if($c->user->roles eq "subscriberadmin" || $c->user->roles eq "subscriber") {
+ $resource->{subscriber_id} = $c->user->voip_subscriber->id;
+ } elsif(!defined $resource->{subscriber_id}) {
+ $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Missing mandatory field 'subscriber_id'");
+ return;
+ }
+ my $b_subscriber = $schema->resultset('voip_subscribers')->find({
+ id => $resource->{subscriber_id},
});
- my $response = HTTP::Response->new(HTTP_OK, undef,
- HTTP::Headers->new($hal->http_headers(skip_links => 1)), $hal->as_json);
- $c->response->headers($response->headers);
- $c->response->body($response->content);
+ unless($b_subscriber) {
+ $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid 'subscriber_id'.");
return;
}
- return;
-}
-
-sub POST :Allow {
- my ($self, $c) = @_;
-
- my $guard = $c->model('DB')->txn_scope_guard;
- {
- my $schema = $c->model('DB');
- my $resource = $self->get_valid_post_data(
- c => $c,
- media_type => 'application/json',
- );
- last unless $resource;
-
- my $form = $self->get_form($c);
- last unless $self->validate_form(
- c => $c,
- resource => $resource,
- form => $form,
- );
-
- my $tset;
-
- if($c->user->roles eq "subscriberadmin" || $c->user->roles eq "subscriber") {
- $resource->{subscriber_id} = $c->user->voip_subscriber->id;
- } elsif(!defined $resource->{subscriber_id}) {
- $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Missing mandatory field 'subscriber_id'");
- last;
- }
- my $b_subscriber = $schema->resultset('voip_subscribers')->find({
- id => $resource->{subscriber_id},
+ my $subscriber = $b_subscriber->provisioning_voip_subscriber;
+ unless($subscriber) {
+ $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid subscriber.");
+ return;
+ }
+ if (! exists $resource->{times} ) {
+ $resource->{times} = [];
+ }
+ if (ref $resource->{times} ne "ARRAY") {
+ $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid field 'times'. Must be an array.");
+ return;
+ }
+ my $times = $resource->{times};
+ # enable tz and use_owner_tz params for POST:
+ #$times = $self->apply_owner_timezone($c,$b_subscriber,$resource->{times},'deflate');
+ try {
+ $tset = $schema->resultset('voip_cf_time_sets')->create({
+ name => $resource->{name},
+ subscriber_id => $subscriber->id,
});
- unless($b_subscriber) {
- $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid 'subscriber_id'.");
- last;
- }
- my $subscriber = $b_subscriber->provisioning_voip_subscriber;
- unless($subscriber) {
- $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid subscriber.");
- last;
- }
- if (! exists $resource->{times} ) {
- $resource->{times} = [];
- }
- if (ref $resource->{times} ne "ARRAY") {
- $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid field 'times'. Must be an array.");
- last;
- }
- my $times = $resource->{times};
- # enable tz and use_owner_tz params for POST:
- #$times = $self->apply_owner_timezone($c,$b_subscriber,$resource->{times},'deflate');
- try {
- $tset = $schema->resultset('voip_cf_time_sets')->create({
- name => $resource->{name},
- subscriber_id => $subscriber->id,
- });
- for my $t ( @$times ) {
- delete $t->{time_set_id};
- $tset->create_related("voip_cf_periods", $t);
- }
- } catch($e) {
- $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Failed to create cftimeset.", $e);
- last;
+ for my $t ( @$times ) {
+ delete $t->{time_set_id};
+ $tset->create_related("voip_cf_periods", $t);
}
-
- last unless $self->add_create_journal_item_hal($c,sub {
- my $self = shift;
- my ($c) = @_;
- my $_dset = $self->item_by_id($c, $tset->id);
- return $self->hal_from_item($c, $_dset, "cftimesets"); });
-
- $guard->commit;
-
- $c->response->status(HTTP_CREATED);
- $c->response->header(Location => sprintf('/%s%d', $c->request->path, $tset->id));
- $c->response->body(q());
+ } catch($e) {
+ $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Failed to create cftimeset.", $e);
+ return;
}
- return;
+
+ return $tset;
}
1;
diff --git a/lib/NGCP/Panel/Controller/API/CFTimeSetsItem.pm b/lib/NGCP/Panel/Controller/API/CFTimeSetsItem.pm
index 22983bda36..313f095809 100644
--- a/lib/NGCP/Panel/Controller/API/CFTimeSetsItem.pm
+++ b/lib/NGCP/Panel/Controller/API/CFTimeSetsItem.pm
@@ -41,124 +41,25 @@ __PACKAGE__->set_config({
allowed_roles => {
Default => [qw/admin reseller ccareadmin ccare subscriberadmin subscriber/],
Journal => [qw/admin reseller ccareadmin ccare/],
- }
+ },
+ PATCH => { ops => [qw/add replace remove copy/] },
});
-sub GET :Allow {
- my ($self, $c, $id) = @_;
- {
- last unless $self->valid_id($c, $id);
- my $tset = $self->item_by_id($c, $id);
- last unless $self->resource_exists($c, timeset => $tset);
-
- my $hal = $self->hal_from_item($c, $tset, "cftimesets");
-
- my $response = HTTP::Response->new(HTTP_OK, undef, HTTP::Headers->new(
- (map { # XXX Data::HAL must be able to generate links with multiple relations
- s|rel="(http://purl.org/sipwise/ngcp-api/#rel-resellers)"|rel="item $1"|r
- =~ s/rel=self/rel="item self"/r;
- } $hal->http_headers),
- ), $hal->as_json);
- $c->response->headers($response->headers);
- $c->response->body($response->content);
- return;
- }
- return;
-}
+sub delete_item {
+ my ($self, $c, $item) = @_;
-sub PATCH :Allow {
- my ($self, $c, $id) = @_;
- my $guard = $c->model('DB')->txn_scope_guard;
- {
- my $preference = $self->require_preference($c);
- last unless $preference;
-
- my $json = $self->get_valid_patch_data(
- c => $c,
- id => $id,
- media_type => 'application/json-patch+json',
- ops => [qw/add replace remove copy/],
- );
- last unless $json;
-
- my $tset = $self->item_by_id($c, $id);
- last unless $self->resource_exists($c, timeset => $tset);
- my $old_resource = $self->hal_from_item($c, $tset, "cftimesets")->resource;
- my $resource = $self->apply_patch($c, $old_resource, $json);
- last unless $resource;
-
- my $form = $self->get_form($c);
- $tset = $self->update_item($c, $tset, $old_resource, $resource, $form);
- last unless $tset;
-
- my $hal = $self->hal_from_item($c, $tset, "cftimesets");
- last unless $self->add_update_journal_item_hal($c,$hal);
-
- $guard->commit;
-
- $self->return_representation($c, 'hal' => $hal, 'preference' => $preference );
- }
- return;
-}
+ return unless $self->check_subscriber_can_update_item($c, $item);
-sub PUT :Allow {
- my ($self, $c, $id) = @_;
- my $guard = $c->model('DB')->txn_scope_guard;
- {
- my $preference = $self->require_preference($c);
- last unless $preference;
-
- my $tset = $self->item_by_id($c, $id);
- last unless $self->resource_exists($c, timeset => $tset);
- my $resource = $self->get_valid_put_data(
- c => $c,
- id => $id,
- media_type => 'application/json',
- );
- last unless $resource;
- my $old_resource = undef; #unused: { $tset->get_inflated_columns };
-
- my $form = $self->get_form($c);
- $tset = $self->update_item($c, $tset, $old_resource, $resource, $form);
- last unless $tset;
-
- my $hal = $self->hal_from_item($c, $tset, "cftimesets");
- last unless $self->add_update_journal_item_hal($c,$hal);
-
- $guard->commit;
-
- $self->return_representation($c, 'hal' => $hal, 'preference' => $preference );
+ try {
+ $item->delete;
+ } catch($e) {
+ my $id = $item->id;
+ $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Internal Server Error",
+ "Failed to delete cftimeset with id '$id'", $e);
+ return;
}
- return;
-}
-sub DELETE :Allow {
- my ($self, $c, $id) = @_;
- my $guard = $c->model('DB')->txn_scope_guard;
- {
- my $tset = $self->item_by_id($c, $id);
- last unless $self->resource_exists($c, timeset => $tset);
-
- return unless $self->check_subscriber_can_update_item($c, $tset);
-
- last unless $self->add_delete_journal_item_hal($c,sub {
- my $self = shift;
- my ($c) = @_;
- return $self->hal_from_item($c, $tset, "cftimesets"); });
-
- try {
- $tset->delete;
- } catch($e) {
- $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Internal Server Error",
- "Failed to delete cftimeset with id '$id'", $e);
- last;
- }
- $guard->commit;
-
- $c->response->status(HTTP_NO_CONTENT);
- $c->response->body(q());
- }
- return;
+ return 1;
}
sub get_journal_methods{
diff --git a/lib/NGCP/Panel/Role/API/CFTimeSets.pm b/lib/NGCP/Panel/Role/API/CFTimeSets.pm
index 08009451e2..2bedac5fe6 100644
--- a/lib/NGCP/Panel/Role/API/CFTimeSets.pm
+++ b/lib/NGCP/Panel/Role/API/CFTimeSets.pm
@@ -300,8 +300,9 @@ sub _merge_adjacent {
}
sub hal_from_item {
- my ($self, $c, $item, $type) = @_;
- my $form;
+ my ($self, $c, $item, $form, $params) = @_;
+
+ my $type = 'cftimesets';
my %resource = $item->get_inflated_columns;
my @times;
@@ -400,6 +401,22 @@ sub check_subscriber_can_update_item {
return 1;
}
+sub resource_from_item {
+ my ($self, $c, $item) = @_;
+
+ my $resource = { $item->get_inflated_columns };
+
+ $resource->{subscriber_id} = $item->subscriber->voip_subscriber->id;
+ $resource->{times} //= [];
+
+ foreach my $period ($item->voip_cf_periods->all) {
+ push @{$resource->{times}}, { $period->get_inflated_columns };
+ }
+
+ return $resource;
+
+}
+
sub update_item {
my ($self, $c, $item, $old_resource, $resource, $form) = @_;
diff --git a/lib/NGCP/Panel/Role/EntitiesItem.pm b/lib/NGCP/Panel/Role/EntitiesItem.pm
index d1cb2a9413..900865aac9 100644
--- a/lib/NGCP/Panel/Role/EntitiesItem.pm
+++ b/lib/NGCP/Panel/Role/EntitiesItem.pm
@@ -294,6 +294,9 @@ sub patch {
#$old_resource = clone($old_resource);
##without it error: The entity could not be processed: Modification of a read-only value attempted at /usr/share/perl5/JSON/Pointer.pm line 200, <$fh> line 1.\n
$resource = $self->apply_patch($c, $old_resource, $json);
+ if ($c->has_errors) {
+ goto TX_END;
+ }
unless ($resource) {
$self->error($c, HTTP_INTERNAL_SERVER_ERROR, 'Internal Server Error',
'#apply_patch') unless $c->has_errors;
@@ -301,6 +304,9 @@ sub patch {
}
($item, $form, $process_extras) = $self->update_item($c, $item, $old_resource, $resource, $form, $process_extras );
+ if ($c->has_errors) {
+ goto TX_END;
+ }
unless ($item) {
$self->error($c, HTTP_INTERNAL_SERVER_ERROR, 'Internal Server Error',
'#update_item') unless $c->has_errors;
@@ -392,6 +398,9 @@ sub put {
my ($data_processed_result);
if (!$non_json_data || !$data) {
($item, $form, $process_extras) = $self->update_item($c, $item, $old_resource, $resource, $form, $process_extras );
+ if ($c->has_errors) {
+ goto TX_END;
+ }
unless ($item) {
$self->error($c, HTTP_INTERNAL_SERVER_ERROR, 'Internal Server Error',
'#update_item') unless $c->has_errors;
@@ -469,13 +478,16 @@ sub delete { ## no critic (ProhibitBuiltinHomonyms)
}
($hal, $hal_id) = $self->get_journal_item_hal($c, $item);
- unless ($self->delete_item($c, $item)) {
+ if ($self->delete_item($c, $item)) {
unless ($self->add_journal_item_hal($c, { hal => $hal, ($hal_id ? ( id => $hal_id, ) : ()) })) {
$self->error($c, HTTP_INTERNAL_SERVER_ERROR, 'Internal Server Error',
'#add_journal_item_hal');
goto TX_END;
}
} else {
+ if ($c->has_errors) {
+ goto TX_END;
+ }
$self->error($c, HTTP_INTERNAL_SERVER_ERROR, 'Internal Server Error',
'non 1 return from $self->delete_item()');
goto TX_END;