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

@@ -61,124 +61,56 @@ __PACKAGE__->set_config({
     allowed_roles => [qw/admin reseller ccareadmin ccare subscriberadmin subscriber/],
 });
 
-sub GET :Allow {
+sub create_item {
-    my ($self, $c) = @_;
+    my ($self, $c, $resource, $form, $process_extras) = @_;
-    my $page = $c->request->params->{page} // 1;
+
-    my $rows = $c->request->params->{rows} // 10;
+    my $schema = $c->model('DB');
-    {
+
-        my $timesets = $self->item_rs($c);
+    my $tset;
-
+
-        (my $total_count, $timesets, my $timesets_rows) = $self->paginate_order_collection($c, $timesets);
+    if($c->user->roles eq "subscriberadmin" || $c->user->roles eq "subscriber") {
-        my (@embedded, @links);
+        $resource->{subscriber_id} = $c->user->voip_subscriber->id;
-        for my $tset (@$timesets_rows) {
+    } elsif(!defined $resource->{subscriber_id}) {
-            push @embedded, $self->hal_from_item($c, $tset, "cftimesets");
+        $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Missing mandatory field 'subscriber_id'");
-            push @links, Data::HAL::Link->new(
+        return;
-                relation => 'ngcp:'.$self->resource_name,
+    }
-                href     => sprintf('%s%d', $self->dispatch_path, $tset->id),
+    my $b_subscriber = $schema->resultset('voip_subscribers')->find({
-            );
+            id => $resource->{subscriber_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,
         });
-        my $response = HTTP::Response->new(HTTP_OK, undef,
+    unless($b_subscriber) {
-            HTTP::Headers->new($hal->http_headers(skip_links => 1)), $hal->as_json);
+        $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid 'subscriber_id'.");
-        $c->response->headers($response->headers);
-        $c->response->body($response->content);
         return;
     }
-    return;
+    my $subscriber = $b_subscriber->provisioning_voip_subscriber;
-}
+    unless($subscriber) {
-
+        $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid subscriber.");
-sub POST :Allow {
+        return;
-    my ($self, $c) = @_;
+    }
-
+    if (! exists $resource->{times} ) {
-    my $guard = $c->model('DB')->txn_scope_guard;
+        $resource->{times} = [];
-    {
+    }
-        my $schema = $c->model('DB');
+    if (ref $resource->{times} ne "ARRAY") {
-        my $resource = $self->get_valid_post_data(
+        $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid field 'times'. Must be an array.");
-            c => $c,
+        return;
-            media_type => 'application/json',
+    }
-        );
+    my $times = $resource->{times};
-        last unless $resource;
+    # enable tz and use_owner_tz params for POST:
-
+    #$times = $self->apply_owner_timezone($c,$b_subscriber,$resource->{times},'deflate');
-        my $form = $self->get_form($c);
+    try {
-        last unless $self->validate_form(
+        $tset = $schema->resultset('voip_cf_time_sets')->create({
-            c => $c,
+                name => $resource->{name},
-            resource => $resource,
+                subscriber_id => $subscriber->id,
-            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},
             });
-        unless($b_subscriber) {
+        for my $t ( @$times ) {
-            $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Invalid 'subscriber_id'.");
+            delete $t->{time_set_id};
-            last;
+            $tset->create_related("voip_cf_periods", $t);
-        }
-        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;
         }
-
+    } catch($e) {
-        last unless $self->add_create_journal_item_hal($c,sub {
+        $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Failed to create cftimeset.", $e);
-            my $self = shift;
+        return;
-            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());
     }
-    return;
+
+    return $tset;
 }
 
 1;

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 {
+sub delete_item {
-    my ($self, $c, $id) = @_;
+    my ($self, $c, $item) = @_;
-    {
-        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 PATCH :Allow {
+    return unless $self->check_subscriber_can_update_item($c, $item);
-    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;
-}
 
-sub PUT :Allow {
+    try {
-    my ($self, $c, $id) = @_;
+        $item->delete;
-    my $guard = $c->model('DB')->txn_scope_guard;
+    } catch($e) {
-    {
+        my $id = $item->id;
-        my $preference = $self->require_preference($c);
+        $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Internal Server Error",
-        last unless $preference;
+                     "Failed to delete cftimeset with id '$id'", $e);
-
+        return;
-        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 );
     }
-    return;
-}
 
-sub DELETE :Allow {
+    return 1;
-    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;
 }
 
 sub get_journal_methods{

lib/NGCP/Panel/Role/API/CFTimeSets.pm

@@ -300,8 +300,9 @@ sub _merge_adjacent {
 }
 
 sub hal_from_item {
-    my ($self, $c, $item, $type) = @_;
+    my ($self, $c, $item, $form, $params) = @_;
-    my $form;
+
+    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) = @_;
 

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;