From d1f3c6e806872e597c31a463fd177c704cc55e03 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 6 Jan 2023 12:49:14 -0300 Subject: [PATCH] Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 67a81d1328ad5f8720b3bf350ac494a0cd26f516) Signed-off-by: Martin Renvoize --- Koha/REST/Plugin/Objects.pm | 145 +++++++++++++++++----- t/db_dependent/Koha/REST/Plugin/Objects.t | 111 ++++++++++++++++- 2 files changed, 225 insertions(+), 31 deletions(-) diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index eba63a4ded..462fdf97da 100644 --- a/Koha/REST/Plugin/Objects.pm +++ b/Koha/REST/Plugin/Objects.pm @@ -38,10 +38,12 @@ sub register { my $patrons_rs = Koha::Patrons->new; my $patrons = $c->objects->find( $patrons_rs, $id ); + . . . + return $c->render({ status => 200, openapi => $patron }); Performs a database search using given Koha::Objects object and the $id. -Returns I if no object is found Returns the I of +Returns I if no object is found or the I of the requested object. It passes through any embeds if specified. =cut @@ -49,12 +51,32 @@ the requested object. It passes through any embeds if specified. $app->helper( 'objects.find' => sub { my ( $c, $result_set, $id ) = @_; + my $object = $c->objects->find_rs( $result_set, $id ); + return unless $object; + return $c->objects->to_api($object); + } + ); - my $attributes = {}; - # Look for embeds - my $embed = $c->stash('koha.embed'); - my $strings = $c->stash('koha.strings'); +=head3 objects.find_rs + + my $patrons_rs = Koha::Patrons->new; + my $patron_rs = $c->objects->find_rs( $patrons_rs, $id ); + . . . + return $c->render({ status => 200, openapi => $patron_rs->to_api }); + +Returns the passed Koha::Objects resultset filtered to the passed $id and +with any embeds requested by the api applied. + +The resultset can then be used for further processing. + +=cut + + $app->helper( + 'objects.find_rs' => sub { + my ( $c, $result_set, $id ) = @_; + + my $attributes = {}; # Generate prefetches for embedded stuff $c->dbic_merge_prefetch( @@ -66,9 +88,7 @@ the requested object. It passes through any embeds if specified. my $object = $result_set->find( $id, $attributes ); - return unless $object; - - return $object->to_api({ embed => $embed, strings => $strings }); + return $object; } ); @@ -76,8 +96,11 @@ the requested object. It passes through any embeds if specified. my $patrons_rs = Koha::Patrons->new; my $patrons = $c->objects->search( $patrons_rs ); + . . . + return $c->render({ status => 200, openapi => $patrons }); -Performs a database search using given Koha::Objects object and query parameters. +Performs a database search using given Koha::Objects object with any api +query parameters applied. Returns an arrayref of the hashrefs representing the resulting objects for API rendering. @@ -91,16 +114,35 @@ shouldn't be called twice in it. 'objects.search' => sub { my ( $c, $result_set ) = @_; - my $args = $c->validation->output; + return $c->objects->to_api( $c->objects->search_rs($result_set) ); + } + ); + +=head3 objects.search_rs + + my $patrons_rs = Koha::Patrons->new; + my $patrons_rs = $c->objects->search_rs( $patrons_rs ); + . . . + return $c->render({ status => 200, openapi => $patrons_rs->to_api }); + +Returns the passed Koha::Objects resultset filtered as requested by the api query +parameters and with requested embeds applied. + +Warning: this helper adds pagination headers to the calling controller, and thus +shouldn't be called twice in it. + +=cut + + $app->helper( + 'objects.search_rs' => sub { + my ( $c, $result_set ) = @_; + + my $args = $c->validation->output; my $attributes = {}; # Extract reserved params - my ( $filtered_params, $reserved_params, $path_params ) = $c->extract_reserved_params($args); - # Privileged reques? - my $is_public = $c->stash('is_public'); - # Look for embeds - my $embed = $c->stash('koha.embed'); - my $strings = $c->stash('koha.strings'); + my ( $filtered_params, $reserved_params, $path_params ) = + $c->extract_reserved_params($args); # Merge sorting into query attributes $c->dbic_merge_sorting( @@ -112,10 +154,12 @@ shouldn't be called twice in it. ); # If no pagination parameters are passed, default - $reserved_params->{_per_page} //= C4::Context->preference('RESTdefaultPageSize'); - $reserved_params->{_page} //= 1; + $reserved_params->{_per_page} //= + C4::Context->preference('RESTdefaultPageSize'); + $reserved_params->{_page} //= 1; unless ( $reserved_params->{_per_page} == -1 ) { + # Merge pagination into query attributes $c->dbic_merge_pagination( { @@ -137,8 +181,10 @@ shouldn't be called twice in it. if ( defined $filtered_params ) { # Apply the mapping function to the passed params - $filtered_params = $result_set->attributes_from_api($filtered_params); - $filtered_params = $c->build_query_params( $filtered_params, $reserved_params ); + $filtered_params = + $result_set->attributes_from_api($filtered_params); + $filtered_params = + $c->build_query_params( $filtered_params, $reserved_params ); } if ( defined $reserved_params->{q} @@ -155,18 +201,20 @@ shouldn't be called twice in it. my $json = JSON->new; - if ( ref($reserved_params->{q}) eq 'ARRAY' ) { - # q is defined as multi => JSON::Validator generates an array + if ( ref( $reserved_params->{q} ) eq 'ARRAY' ) { + + # q is defined as multi => JSON::Validator generates an array foreach my $q ( @{ $reserved_params->{q} } ) { push @query_params_array, $json->decode($q) - if $q; # skip if exists but is empty + if $q; # skip if exists but is empty } } else { # objects.search called outside OpenAPI context # might be a hashref - push @query_params_array, $json->decode($reserved_params->{q}) - if $reserved_params->{q}; + push @query_params_array, + $json->decode( $reserved_params->{q} ) + if $reserved_params->{q}; } push @query_params_array, @@ -182,16 +230,19 @@ shouldn't be called twice in it. $query_params = $query_params_array[0]; } - $filtered_params = $c->merge_q_params( $filtered_params, $query_params, $result_set ); + $filtered_params = + $c->merge_q_params( $filtered_params, $query_params, + $result_set ); } # request sequence id (i.e. 'draw' Datatables parameter) - $c->res->headers->add( 'x-koha-request-id' => $reserved_params->{'x-koha-request-id'} ) + $c->res->headers->add( + 'x-koha-request-id' => $reserved_params->{'x-koha-request-id'} ) if $reserved_params->{'x-koha-request-id'}; # If search_limited exists, use it $result_set = $result_set->search_limited, - if $result_set->can('search_limited'); + if $result_set->can('search_limited'); # Perform search my $objects = $result_set->search( $filtered_params, $attributes ); @@ -199,13 +250,47 @@ shouldn't be called twice in it. $c->add_pagination_headers( { - total => ($objects->is_paged ? $objects->pager->total_entries : $objects->count), + total => ( + $objects->is_paged + ? $objects->pager->total_entries + : $objects->count + ), base_total => $total, params => $args, } ); - return $objects->to_api({ embed => $embed, public => $is_public, strings => $strings }); + return $objects; + } + ); + +=head3 objects.to_api + + my $patrons_rs = Koha::Patrons->new; + my $api_representation = $c->objects->to_api( $patrons_rs ); + +Returns the API representation of the passed resultset. + +=cut + + $app->helper( + 'objects.to_api' => sub { + my ( $c, $object ) = @_; + + # Privileged request? + my $public = $c->stash('is_public'); + + # Look for embeds + my $embed = $c->stash('koha.embed'); + my $strings = $c->stash('koha.strings'); + + return $object->to_api( + { + embed => $embed, + public => $public, + strings => $strings + } + ); } ); } diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t index 99626ac552..478647a857 100755 --- a/t/db_dependent/Koha/REST/Plugin/Objects.t +++ b/t/db_dependent/Koha/REST/Plugin/Objects.t @@ -43,6 +43,15 @@ get '/cities' => sub { $c->render( status => 200, json => $cities ); }; +get '/cities/rs' => sub { + my $c = shift; + $c->validation->output( $c->req->params->to_hash ); + $c->stash_embed; + my $cities = $c->objects->search_rs( Koha::Cities->new ); + + $c->render( status => 200, json => { count => $cities->count } ); +}; + get '/cities/:city_id' => sub { my $c = shift; my $id = $c->stash("city_id"); @@ -120,8 +129,18 @@ get '/my_patrons' => sub { ); }; +get '/cities/:city_id/rs' => sub { + my $c = shift; + $c->validation->output( $c->req->params->to_hash ); + $c->stash_embed; + my $city_id = $c->param('city_id'); + my $city = $c->objects->find_rs( Koha::Cities->new, $city_id ); + + $c->render( status => 200, json => { name => $city->city_name } ); +}; + # The tests -use Test::More tests => 16; +use Test::More tests => 18; use Test::Mojo; use t::lib::Mocks; @@ -508,6 +527,47 @@ subtest 'objects.search helper order by embedded columns' => sub { $schema->storage->txn_rollback; }; +subtest 'objects.search_rs helper' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + # Remove existing cities to have more control on the search results + Koha::Cities->delete; + + # Create three sample cities that match the query. This makes sure we + # always have a "next" link regardless of Mojolicious::Plugin::OpenAPI version. + $builder->build_object( + { + class => 'Koha::Cities', + value => { + city_name => 'city1' + } + } + ); + $builder->build_object( + { + class => 'Koha::Cities', + value => { + city_name => 'city2' + } + } + ); + $builder->build_object( + { + class => 'Koha::Cities', + value => { + city_name => 'city3' + } + } + ); + + my $t = Test::Mojo->new; + $t->get_ok('/cities/rs')->status_is(200)->json_is( '/count' => 3 ); + + $schema->storage->txn_rollback; +}; + subtest 'objects.find helper' => sub { plan tests => 9; @@ -850,3 +910,52 @@ subtest 'objects.search helper with expanded authorised values' => sub { $schema->storage->txn_rollback; }; + +subtest 'objects.find_rs helper' => sub { + plan tests => 9; + + $schema->storage->txn_begin; + + # Remove existing cities to have more control on the search results + Koha::Cities->delete; + + # Create three sample cities that match the query. This makes sure we + # always have a "next" link regardless of Mojolicious::Plugin::OpenAPI version. + my $city1 = $builder->build_object( + { + class => 'Koha::Cities', + value => { + city_name => 'city1' + } + } + ); + my $city2 = $builder->build_object( + { + class => 'Koha::Cities', + value => { + city_name => 'city2' + } + } + ); + my $city3 = $builder->build_object( + { + class => 'Koha::Cities', + value => { + city_name => 'city3' + } + } + ); + + my $t = Test::Mojo->new; + + $t->get_ok( '/cities/' . $city1->id . '/rs' )->status_is(200) + ->json_is( '/name' => 'city1' ); + + $t->get_ok( '/cities/' . $city2->id . '/rs' )->status_is(200) + ->json_is( '/name' => 'city2' ); + + $t->get_ok( '/cities/' . $city3->id . '/rs' )->status_is(200) + ->json_is( '/name' => 'city3' ); + + $schema->storage->txn_rollback; +}; -- 2.39.5