From b4696072d02570123afd0d87303a2afd165a429f Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 --- 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