From 38f8e116fab0d4425ee0dc196e5c0aca4d523184 Mon Sep 17 00:00:00 2001 From: Matt Blenkinsop Date: Thu, 26 Oct 2023 09:09:10 +0000 Subject: [PATCH] Bug 34587: Fix ->validation->output in API endpoints Signed-off-by: Jessica Zairo Signed-off-by: Michaela Sieber Signed-off-by: Nick Clemens Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/ERM/EUsage/CounterRegistry.pm | 5 +- Koha/REST/V1/ERM/EUsage/CustomReports.pm | 58 +++++------ .../REST/V1/ERM/EUsage/DefaultUsageReports.pm | 62 +----------- Koha/REST/V1/ERM/EUsage/SushiServices.pm | 2 +- .../paths/erm_default_usage_reports.yaml | 98 ------------------- 5 files changed, 30 insertions(+), 195 deletions(-) diff --git a/Koha/REST/V1/ERM/EUsage/CounterRegistry.pm b/Koha/REST/V1/ERM/EUsage/CounterRegistry.pm index 906e878ea4..d8ed673195 100644 --- a/Koha/REST/V1/ERM/EUsage/CounterRegistry.pm +++ b/Koha/REST/V1/ERM/EUsage/CounterRegistry.pm @@ -39,11 +39,12 @@ use Koha::Exceptions; sub list { my $c = shift->openapi->valid_input or return; - my $args = $c->validation->output; + my $args = $c->param('q'); my $json = JSON->new; my @query_params_array = - map { $_ ? $json->decode($_) : () } @{ $args->{q} }; + map { $_ ? $json->decode($_) : () } $args; + my $search_string = $query_params_array[0]->{name}; my $url = 'https://registry.projectcounter.org/api/v1/platform/'; diff --git a/Koha/REST/V1/ERM/EUsage/CustomReports.pm b/Koha/REST/V1/ERM/EUsage/CustomReports.pm index d7a9cdba76..4647a1ce0a 100644 --- a/Koha/REST/V1/ERM/EUsage/CustomReports.pm +++ b/Koha/REST/V1/ERM/EUsage/CustomReports.pm @@ -49,13 +49,13 @@ sub monthly_report { return try { - my $args = $c->param('q'); - my @query_params_array; my $json = JSON->new; + my $args = $json->decode($c->param('q')); + my @query_params_array; if ( ref($args) eq 'ARRAY' ) { foreach my $q ( @{$args} ) { - push @query_params_array, $json->decode($q) + push @query_params_array, $q if $q; } } @@ -67,10 +67,10 @@ sub monthly_report { my $usage_data_providers = Koha::ERM::EUsage::UsageDataProviders->search( {}, {} )->unblessed; my $metric_types = - $query_params_array[0][0]->{'erm_usage_muses.metric_type'}; + $query_params_array[0]->{'erm_usage_muses.metric_type'}; my $access_types = - $query_params_array[0][0]->{'erm_usage_muses.access_type'} - ? $query_params_array[0][0]->{'erm_usage_muses.access_type'} + $query_params_array[0]->{'erm_usage_muses.access_type'} + ? $query_params_array[0]->{'erm_usage_muses.access_type'} : (); # Objects with no data in the selected range will not be returned by the API - we still want to include them if they have been requested @@ -117,16 +117,9 @@ sub yearly_report { my $c = shift->openapi->valid_input or return; return try { - my $args = $c->param('q'); - my @query_params_array; my $json = JSON->new; - - if ( ref($args) eq 'ARRAY' ) { - foreach my $q ( @{$args} ) { - push @query_params_array, $json->decode($q) - if $q; - } - } + my $args = $json->decode( $c->param('q') ); + my @query_params_array; my $data_type = $c->param('data_type'); my $data_set = _get_data_set($data_type); @@ -137,7 +130,7 @@ sub yearly_report { # Titles with no data in the selected range will not be returned by the API - we still want to include them if they have been requested my $requested_ids = _get_correct_query_param( - $data_type, \@query_params_array, + $data_type, $args, 'yearly' ); for my $id ( @{$requested_ids} ) { @@ -151,10 +144,10 @@ sub yearly_report { push @{$data}, $missing_result if $missing_result; } - my $metric_types = $query_params_array[0]->{'erm_usage_yuses.metric_type'}; + my $metric_types = $args->{'erm_usage_yuses.metric_type'}; my $access_types = - $query_params_array[0]->{'erm_usage_yuses.access_type'} - ? $query_params_array[0]->{'erm_usage_yuses.access_type'} + $args->{'erm_usage_yuses.access_type'} + ? $args->{'erm_usage_yuses.access_type'} : (); my $report_data = _get_report_data( @@ -186,13 +179,13 @@ sub metric_types_report { return try { - my $args = $c->param('q'); - my @query_params_array; my $json = JSON->new; + my $args = $json->decode( $c->param('q') ); + my @query_params_array; if ( ref($args) eq 'ARRAY' ) { foreach my $q ( @{$args} ) { - push @query_params_array, $json->decode($q) + push @query_params_array, $q if $q; } } @@ -220,7 +213,7 @@ sub metric_types_report { push @{$data}, $missing_result if $missing_result; } - my @metric_types = ('metric_types_report'); + my @metric_types = ('metric_types_report'); # Dummy value to ensure the loop triggers at least once in _create_report_rows my $report_data = _get_report_data( { data_type => $data_type, @@ -250,13 +243,13 @@ sub provider_rollup_report { return try { - my $args = $c->param('q'); - my @query_params_array; my $json = JSON->new; + my $args = $json->decode( $c->param('q') ); + my @query_params_array; if ( ref($args) eq 'ARRAY' ) { foreach my $q ( @{$args} ) { - push @query_params_array, $json->decode($q) + push @query_params_array, $q if $q; } } @@ -264,11 +257,11 @@ sub provider_rollup_report { my $usage_data_providers_set = Koha::ERM::EUsage::UsageDataProviders->new; my $usage_data_providers = $c->objects->search($usage_data_providers_set); - my $data_type = $c->validation->param('data_type'); + my $data_type = $c->param('data_type'); my $key = 'erm_usage_' . $data_type . 's'; my $metric_types = - $query_params_array[0][0]->{ $key . '.erm_usage_muses.metric_type' }; - + $query_params_array[0]->{ $key . '.erm_usage_muses.metric_type' }; + my @usage_data_provider_report_data; for my $usage_data_provider ( @{$usage_data_providers} ) { @@ -377,18 +370,17 @@ e.g. If it is a titles report and the user has requested titles with the ids 1,2 =cut sub _get_correct_query_param { - my ( $data_type, $array_ref, $period ) = @_; + my ( $data_type, $parameters, $period ) = @_; my $param; - my @query_params = @$array_ref; my $prefix = $period eq 'monthly' ? 'erm_usage_muses' : 'erm_usage_yuses'; my $key = $prefix . "." . $data_type . "_id"; if ( $period eq 'monthly' ) { - $param = $query_params[0][0]->{$key}; + $param = @$parameters[0]->{$key}; } else { - $param = $query_params[0]->{$key}; + $param = $parameters->{$key}; } return $param; diff --git a/Koha/REST/V1/ERM/EUsage/DefaultUsageReports.pm b/Koha/REST/V1/ERM/EUsage/DefaultUsageReports.pm index 8e0d2f2ed4..7d87c14bb1 100644 --- a/Koha/REST/V1/ERM/EUsage/DefaultUsageReports.pm +++ b/Koha/REST/V1/ERM/EUsage/DefaultUsageReports.pm @@ -61,7 +61,7 @@ sub add { Koha::Database->new->schema->txn_do( sub { - my $body = $c->param('body'); + my $body = $c->req->json; my $default_report = Koha::ERM::EUsage::DefaultUsageReport->new_from_api($body)->store; @@ -105,66 +105,6 @@ sub add { }; } -=head3 update - -Controller function that handles updating a Koha::ERM::EUsage::DefaultUsageReport object - -=cut - -sub update { - my $c = shift->openapi->valid_input or return; - - my $default_report = Koha::ERM::EUsage::DefaultUsageReports->find( $c->param('erm_default_usage_report_id') ); - - unless ($default_report) { - return $c->render( - status => 404, - openapi => { error => "Default report not found" } - ); - } - - return try { - Koha::Database->new->schema->txn_do( - sub { - - my $body = $c->req->json; - - $default_report->set_from_api($body)->store; - - $c->res->headers->location( - $c->req->url->to_string . '/' . $default_report->erm_default_usage_report_id ); - return $c->render( - status => 200, - openapi => $default_report->to_api - ); - } - ); - } catch { - my $to_api_mapping = Koha::ERM::EUsage::DefaultUsageReport->new->to_api_mapping; - - if ( blessed $_ ) { - if ( $_->isa('Koha::Exceptions::Object::FKConstraint') ) { - return $c->render( - status => 400, - openapi => { error => "Given " . $to_api_mapping->{ $_->broken_fk } . " does not exist" } - ); - } elsif ( $_->isa('Koha::Exceptions::BadParameter') ) { - return $c->render( - status => 400, - openapi => { error => "Given " . $to_api_mapping->{ $_->parameter } . " does not exist" } - ); - } elsif ( $_->isa('Koha::Exceptions::PayloadTooLarge') ) { - return $c->render( - status => 413, - openapi => { error => $_->error } - ); - } - } - - $c->unhandled_exception($_); - }; -} - =head3 delete =cut diff --git a/Koha/REST/V1/ERM/EUsage/SushiServices.pm b/Koha/REST/V1/ERM/EUsage/SushiServices.pm index e0b85b1747..8253543ff3 100644 --- a/Koha/REST/V1/ERM/EUsage/SushiServices.pm +++ b/Koha/REST/V1/ERM/EUsage/SushiServices.pm @@ -43,7 +43,7 @@ sub get { my $json = JSON->new; my @query_params_array = - map { $_ ? $json->decode($_) : () } @{ $args }; + map { $_ ? $json->decode($_) : () } $args; my $service_url = $query_params_array[0]->{url}; diff --git a/api/v1/swagger/paths/erm_default_usage_reports.yaml b/api/v1/swagger/paths/erm_default_usage_reports.yaml index 1b987face0..19c71fc875 100644 --- a/api/v1/swagger/paths/erm_default_usage_reports.yaml +++ b/api/v1/swagger/paths/erm_default_usage_reports.yaml @@ -119,104 +119,6 @@ permissions: erm: 1 "/erm/default_usage_reports/{erm_default_usage_report_id}": - get: - x-mojo-to: ERM::EUsage::DefaultUsageReports#get - operationId: getERMDefaultUsageReport - tags: - - default_usage_report - summary: get default_usage_report - produces: - - application/json - parameters: - - $ref: "../swagger.yaml#/parameters/erm_default_usage_report_id_pp" - responses: - 200: - description: default_usage_report - schema: - items: - $ref: "../swagger.yaml#/definitions/erm_default_usage_report" - 401: - description: authentication required - schema: - $ref: "../swagger.yaml#/definitions/error" - 403: - description: access forbidden - schema: - $ref: "../swagger.yaml#/definitions/error" - 404: - description: ressource not found - schema: - $ref: "../swagger.yaml#/definitions/error" - 500: - description: |- - internal server error. possible `error_code` attribute values: - * `internal_server_error` - schema: - $ref: "../swagger.yaml#/definitions/error" - 503: - description: under maintenance - schema: - $ref: "../swagger.yaml#/definitions/error" - x-koha-authorization: - permissions: - erm: 1 - put: - x-mojo-to: ERM::EUsage::DefaultUsageReports#update - operationId: updateERMDefaultUsageReports - tags: - - default_usage_report - summary: update default_usage_report - consumes: - - application/json - produces: - - application/json - parameters: - - $ref: "../swagger.yaml#/parameters/erm_default_usage_report_id_pp" - - name: body - in: body - description: a json object containing new information about existing default_usage_report - required: true - schema: - $ref: "../swagger.yaml#/definitions/erm_default_usage_report" - responses: - 200: - description: a successfully updated default_usage_report - schema: - items: - $ref: "../swagger.yaml#/definitions/erm_default_usage_report" - 400: - description: bad parameter - schema: - $ref: "../swagger.yaml#/definitions/error" - 403: - description: access forbidden - schema: - $ref: "../swagger.yaml#/definitions/error" - 404: - description: ressource not found - schema: - $ref: "../swagger.yaml#/definitions/error" - 409: - description: conflict in updating resource - schema: - $ref: "../swagger.yaml#/definitions/error" - 413: - description: Payload too large - schema: - $ref: "../swagger.yaml#/definitions/error" - 500: - description: |- - internal server error. possible `error_code` attribute values: - * `internal_server_error` - schema: - $ref: "../swagger.yaml#/definitions/error" - 503: - description: under maintenance - schema: - $ref: "../swagger.yaml#/definitions/error" - x-koha-authorization: - permissions: - erm: 1 delete: x-mojo-to: ERM::EUsage::DefaultUsageReports#delete operationId: deleteERMDefaultUsageReports -- 2.39.5