Attachment 147469 Details for Bug 33080 – Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api

[patch] Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api

Bug-33080-Introduce-objectssearchrs-objectsfindrs-.patch (text/plain), 12.13 KB, created by Agustín Moyano on 2023-02-27 19:38:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Agustín Moyano

Created: 2023-02-27 19:38:40 UTC

Size: 12.13 KB

patch

obsolete

>From 5100e100387043abdedea5bb338502a8997c96f8 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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
>---
> Koha/REST/Plugin/Objects.pm               | 126 ++++++++++++++++++----
> t/db_dependent/Koha/REST/Plugin/Objects.t | 112 ++++++++++++++++++-
> 2 files changed, 215 insertions(+), 23 deletions(-)
>
>diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm
>index eba63a4ded..496b7cb886 100644
>--- a/Koha/REST/Plugin/Objects.pm
>+++ b/Koha/REST/Plugin/Objects.pm
>@@ -49,6 +49,26 @@ 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);
>+        }
>+    );
>+
>+=head3 objects.find_rs
>+
>+    my $patrons_rs = Koha::Patrons->new;
>+    my $patrons = $c->objects->find_rs( $patrons_rs, $id );
>+
>+Performs a database search using given Koha::Objects object and the $id.
>+
>+Returns a new resultset for further processing. It passes through any embeds if specified.
>+
>+=cut
>+
>+    $app->helper(
>+        'objects.find_rs' => sub {
>+            my ( $c, $result_set, $id ) = @_;
> 
>             my $attributes = {};
> 
>@@ -66,9 +86,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;
>         }
>     );
> 
>@@ -91,16 +109,37 @@ 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 $filtered_patrons_rs = $c->objects->search_rs( $patrons_rs );
>+
>+Performs a database search using given Koha::Objects object and query parameters.
>+
>+Returns a new resultset for further processing.
>+
>+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);
>+            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');
> 
>             # Merge sorting into query attributes
>             $c->dbic_merge_sorting(
>@@ -112,10 +151,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 +178,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 +198,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 +227,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 +247,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 3a4832f2f5..06c0594286 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,53 @@ 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.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 33080: 147469 | 147662 | 147663 | 147720 | 147734 | 147735 | 147736 | 147788 | 147789 | 147790 | 147823 | 147824 | 147825 | 148020 | 148021 | 148022 | 148071 | 148072 | 148073