Attachment 96685 Details for Bug 24228 – Bug 24228: Make ->to_api params a hashref

[patch] Bug 24228: Make ->to_api params a hashref

Bug-24228-Make--toapi-params-a-hashref.patch (text/plain), 11.99 KB, created by Tomás Cohen Arazi (tcohen) on 2019-12-30 18:08:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2019-12-30 18:08:33 UTC

Size: 11.99 KB

patch

obsolete

>From b4696072d02570123afd0d87303a2afd165a429f Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Mon, 23 Dec 2019 12:03:12 -0300
>Subject: [PATCH] Bug 24228: Make ->to_api params a hashref
>
>This patch makes the parameters for Koha::Object(s)->to_api a hashref
>preparing the ground for new parameters. The proposed syntax is:
>
>$object->to_api(
>    {
>        embed => {
>            'items' => {
>                'children' => {
>                    'homebranch' => {}
>                }
>            }
>        }
>    }
>);
>
>Tests are added for the Koha::Objects implementation and tests for
>Koha::Object are adjusted to the new syntax, and a test for the single
>result accessor is added as well.
>
>To test:
>1. Apply this patches
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/Koha/Object.t t/db_dependent/Koha/Objects.t
>=> SUCCESS: Tests pass!
>3. Sign off :-D
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Object.pm                | 89 ++++++++++++++++++++++++++---------
> Koha/Objects.pm               |  4 +-
> t/db_dependent/Koha/Object.t  | 76 +++++++++++++++++++++---------
> t/db_dependent/Koha/Objects.t | 65 ++++++++++++++++++++++++-
> 4 files changed, 185 insertions(+), 49 deletions(-)
>
>diff --git a/Koha/Object.pm b/Koha/Object.pm
>index 553e53d28b..3d97087df8 100644
>--- a/Koha/Object.pm
>+++ b/Koha/Object.pm
>@@ -22,7 +22,7 @@ use Modern::Perl;
> 
> use Carp;
> use Mojo::JSON;
>-use Scalar::Util qw( looks_like_number );
>+use Scalar::Util qw( blessed looks_like_number );
> use Try::Tiny;
> 
> use Koha::Database;
>@@ -373,16 +373,37 @@ sub _numeric_column_type {
> 
> =head3 to_api
> 
>-    my $object_for_api = $object->to_api;
>+    my $object_for_api = $object->to_api(
>+        {
>+          [ embed => {
>+                items => {
>+                    children => {
>+                        holds => {,
>+                            children => {
>+                              ...
>+                            }
>+                        }
>+                    }
>+                },
>+                library => {
>+                    ...
>+                }
>+            },
>+            ...
>+         ]
>+        }
>+    );
> 
> Returns a representation of the object, suitable for API output.
> 
> =cut
> 
> sub to_api {
>-    my ( $self, $embeds ) = @_;
>+    my ( $self, $params ) = @_;
>     my $json_object = $self->TO_JSON;
> 
>+    my $embeds = $params->{embed};
>+
>     # Rename attributes if there's a mapping
>     if ( $self->can('to_api_mapping') ) {
>         foreach my $column ( keys %{ $self->to_api_mapping } ) {
>@@ -403,35 +424,28 @@ sub to_api {
>     }
> 
>     if ($embeds) {
>-        foreach my $embed (@$embeds) {
>-            my ( $curr, $next ) = split /\s*\.\s*/, $embed, 2;
>-            my @nxembeds;
>-
>-            @nxembeds = ($next) if $next;
>+        foreach my $embed ( keys %{$embeds} ) {
>+            my $curr = $embed;
>+            my $next = $embeds->{$curr}->{children};
> 
>             my $children = $self->$curr;
>-            if ( ref $children eq 'ARRAY' ) {
>-                my @list;
>-                my $pos = 0;
>-                foreach my $child (@$children) {
>-                    my $res = $child->to_api( \@nxembeds );
>-                    $res = { $json_object->{$curr}->[$pos], $res }
>-                      if defined $json_object->{$curr}
>-                      && defined $json_object->{$curr}->[$pos];
>-                    push @list, $res;
>-                    $pos++;
>-                }
>+
>+            if ( defined $children and ref($children) eq 'ARRAY' ) {
>+                my @list = map {
>+                    $self->_handle_to_api_child(
>+                        { child => $_, next => $next, curr => $curr } )
>+                } @{$children};
>                 $json_object->{$curr} = \@list;
>             }
>             else {
>-                my $res = $children->to_api( \@nxembeds );
>-                $res = { $json_object->{$curr}, $res }
>-                  if defined $json_object->{$curr};
>-                $json_object->{$curr} = $res;
>+                $json_object->{$curr} = $self->_handle_to_api_child(
>+                    { child => $children, next => $next, curr => $curr } );
>             }
>         }
>     }
> 
>+
>+
>     return $json_object;
> }
> 
>@@ -557,6 +571,35 @@ For example, for borrowers, the _type method will return "Borrower".
> 
> sub _type { }
> 
>+=head3 _handle_to_api_child
>+
>+=cut
>+
>+sub _handle_to_api_child {
>+    my ($self, $args ) = @_;
>+
>+    my $child = $args->{child};
>+    my $next  = $args->{next};
>+    my $curr  = $args->{curr};
>+
>+    my $res;
>+
>+    if ( defined $child ) {
>+
>+        Koha::Exceptions::Exception->throw( "Asked to embed $curr but its return value doesn't implement to_api" )
>+            if defined $next and blessed $child and !$child->can('to_api');
>+
>+        if ( blessed $child ) {
>+            $res = $child->to_api({ embed => $next });
>+        }
>+        else {
>+            $res = $child;
>+        }
>+    }
>+
>+    return $res;
>+}
>+
> sub DESTROY { }
> 
> =head1 AUTHOR
>diff --git a/Koha/Objects.pm b/Koha/Objects.pm
>index 05b1de467d..82df97b687 100644
>--- a/Koha/Objects.pm
>+++ b/Koha/Objects.pm
>@@ -315,9 +315,9 @@ Returns a representation of the objects, suitable for API output .
> =cut
> 
> sub to_api {
>-    my ($self, $embeds) = @_;
>+    my ($self, $params) = @_;
> 
>-    return [ map { $_->to_api($embeds) } $self->as_list ];
>+    return [ map { $_->to_api($params) } $self->as_list ];
> }
> 
> =head3 Koha::Objects->_wrap
>diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t
>index af97834a7d..ea987c9ac4 100755
>--- a/t/db_dependent/Koha/Object.t
>+++ b/t/db_dependent/Koha/Object.t
>@@ -215,7 +215,7 @@ subtest 'TO_JSON tests' => sub {
> 
> subtest "to_api() tests" => sub {
> 
>-    plan tests => 18;
>+    plan tests => 26;
> 
>     $schema->storage->txn_begin;
> 
>@@ -262,43 +262,73 @@ subtest "to_api() tests" => sub {
>     my $illrequest = $builder->build_object({ class => 'Koha::Illrequests' });
>     is_deeply( $illrequest->to_api, $illrequest->TO_JSON, 'If no to_api_method present, return TO_JSON' );
> 
>-    my $item_class = Test::MockModule->new('Koha::Item');
>-    $item_class->mock( 'to_api_mapping',
>-        sub {
>-            return {
>-                itemnumber       => 'item_id'
>-            };
>-        }
>-    );
>-
>-    my $hold_class = Test::MockModule->new('Koha::Hold');
>-    $hold_class->mock( 'to_api_mapping',
>-        sub {
>-            return {
>-                reserve_id       => 'hold_id'
>-            };
>-        }
>-    );
>-
>     my $biblio = $builder->build_sample_biblio();
>     my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber });
>     my $hold = $builder->build_object({ class => 'Koha::Holds', value => { itemnumber => $item->itemnumber } });
> 
>-    my @embeds = ('items');
>+    my $embeds = { 'items' => {} };
> 
>-    my $biblio_api = $biblio->to_api(\@embeds);
>+    my $biblio_api = $biblio->to_api({ embed => $embeds });
> 
>     ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>     is($biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item matches');
>     ok(!exists $biblio_api->{items}->[0]->{holds}, 'No holds info should be embedded yet');
> 
>-    @embeds = ('items.holds');
>-    $biblio_api = $biblio->to_api(\@embeds);
>+    $embeds = (
>+        {
>+            'items' => {
>+                'children' => {
>+                    'holds' => {}
>+                }
>+            },
>+            'biblioitem' => {}
>+        }
>+    );
>+    $biblio_api = $biblio->to_api({ embed => $embeds });
> 
>     ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>     is($biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item still matches');
>     ok(exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded');
>     is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $hold->reserve_id, 'Hold matches');
>+    is_deeply($biblio_api->{biblioitem}, $biblio->biblioitem->to_api, 'More than one root');
>+
>+    my $hold_api = $hold->to_api(
>+        {
>+            embed => { 'item' => {} }
>+        }
>+    );
>+
>+    is( ref($hold_api->{item}), 'HASH', 'Single nested object works correctly' );
>+    is( $hold_api->{item}->{item_id}, $item->itemnumber, 'Object embedded correctly' );
>+
>+    # biblio with no items
>+    my $new_biblio = $builder->build_sample_biblio;
>+    my $new_biblio_api = $new_biblio->to_api({ embed => $embeds });
>+
>+    is_deeply( $new_biblio_api->{items}, [], 'Empty list if no items' );
>+
>+    my $biblio_class = Test::MockModule->new('Koha::Biblio');
>+    $biblio_class->mock( 'undef_result', sub { return; } );
>+
>+    $new_biblio_api = $new_biblio->to_api({ embed => ( { 'undef_result' => {} } ) });
>+    ok( exists $new_biblio_api->{undef_result}, 'If a method returns undef, then the attribute is defined' );
>+    is( $new_biblio_api->{undef_result}, undef, 'If a method returns undef, then the attribute is undef' );
>+
>+    $biblio_class->mock( 'items',
>+        sub { return [ bless { itemnumber => 1 }, 'Somethings' ]; } );
>+
>+    throws_ok {
>+        $new_biblio_api = $new_biblio->to_api(
>+            { embed => { 'items' => { children => { asd => {} } } } } );
>+    }
>+    'Koha::Exceptions::Exception',
>+"An exception is thrown if a blessed object to embed doesn't implement to_api";
>+
>+    is(
>+        "$@",
>+        "Asked to embed items but its return value doesn't implement to_api",
>+        "Exception message correct"
>+    );
> 
>     $schema->storage->txn_rollback;
> };
>diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t
>index 592ab40866..e5bd6aa41e 100644
>--- a/t/db_dependent/Koha/Objects.t
>+++ b/t/db_dependent/Koha/Objects.t
>@@ -283,7 +283,7 @@ subtest '->search() tests' => sub {
> 
> subtest "to_api() tests" => sub {
> 
>-    plan tests => 4;
>+    plan tests => 18;
> 
>     $schema->storage->txn_begin;
> 
>@@ -303,6 +303,69 @@ subtest "to_api() tests" => sub {
>     is_deeply( $cities_api->[0], $city_1->to_api, 'to_api returns the individual objects with ->to_api' );
>     is_deeply( $cities_api->[1], $city_2->to_api, 'to_api returns the individual objects with ->to_api' );
> 
>+    my $biblio_1 = $builder->build_sample_biblio();
>+    my $item_1   = $builder->build_sample_item({ biblionumber => $biblio_1->biblionumber });
>+    my $hold_1   = $builder->build_object(
>+        {
>+            class => 'Koha::Holds',
>+            value => { itemnumber => $item_1->itemnumber }
>+        }
>+    );
>+
>+    my $biblio_2 = $builder->build_sample_biblio();
>+    my $item_2   = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber });
>+    my $hold_2   = $builder->build_object(
>+        {
>+            class => 'Koha::Holds',
>+            value => { itemnumber => $item_2->itemnumber }
>+        }
>+    );
>+
>+    my $embed = { 'items' => {} };
>+
>+    my $i = 0;
>+    my @items = ( $item_1, $item_2 );
>+    my @holds = ( $hold_1, $hold_2 );
>+
>+    my $biblios_api = Koha::Biblios->search(
>+        {
>+            biblionumber => [ $biblio_1->biblionumber, $biblio_2->biblionumber ]
>+        }
>+    )->to_api( { embed => $embed } );
>+
>+    foreach my $biblio_api ( @{ $biblios_api } ) {
>+        ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>+        is($biblio_api->{items}->[0]->{item_id}, $items[$i]->itemnumber, 'Item matches');
>+        ok(!exists $biblio_api->{items}->[0]->{holds}, 'No holds info should be embedded yet');
>+
>+        $i++;
>+    }
>+
>+    # One more level
>+    $embed = {
>+        'items' => {
>+            children => { 'holds' => {} }
>+        }
>+    };
>+
>+    $i = 0;
>+
>+    $biblios_api = Koha::Biblios->search(
>+        {
>+            biblionumber => [ $biblio_1->biblionumber, $biblio_2->biblionumber ]
>+        }
>+    )->to_api( { embed => $embed } );
>+
>+    foreach my $biblio_api ( @{ $biblios_api } ) {
>+
>+        ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>+        is($biblio_api->{items}->[0]->{item_id}, $items[$i]->itemnumber, 'Item still matches');
>+        ok(exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded');
>+        is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $holds[$i]->reserve_id, 'Hold matches');
>+
>+        $i++;
>+    }
>+
>     $schema->storage->txn_rollback;
> };
> 
>-- 
>2.24.1

Actions: View | Diff | Splinter Review

Attachments on bug 24228: 96543 | 96612 | 96613 | 96615 | 96622 | 96644 | 96645 | 96646 | 96685 | 96686 | 96687 | 96954 | 96955