From 8dfac502a2cc5cf77ff2378f934a7380c5307830 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 2 Nov 2022 12:48:29 -0300 Subject: [PATCH] Bug 26635: Move expand syntax to x-koha-embed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of a global av-expand flag (through a header) this patch proposes to allow specifying +av_expand at the x-koha-embed header level. This allows a more fine-grained control on what objects get avs expanded. e.g.: GET /patrons/123 x-koha-expand: +av_expand,checkouts.item+av_expand => { "_str": { "city": { "str": "Córdoba", ... } }, ... "checkouts": [ { ..., "item": { "_str": { "not_for_loan_status": { "str": "Reference material", ... }, ... }, ... } } ] } To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/Koha/REST/Plugin/Query.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Martin Renvoize --- Koha/Object.pm | 22 ++++++++++------ Koha/REST/Plugin/Objects.pm | 4 +-- Koha/REST/Plugin/Query.pm | 10 ++++++- t/Koha/REST/Plugin/Query.t | 32 ++++++++++++++++++----- t/db_dependent/Koha/Object.t | 26 ++++++++++++++++-- t/db_dependent/Koha/REST/Plugin/Objects.t | 11 ++++---- 6 files changed, 80 insertions(+), 25 deletions(-) diff --git a/Koha/Object.pm b/Koha/Object.pm index 3b2b80407b..187ba00769 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -553,9 +553,17 @@ sub to_api { my ( $self, $params ) = @_; my $json_object = $self->TO_JSON; + # Make sure we duplicate the $params variable to avoid + # breaking calls in a loop (Koha::Objects->to_api) + $params = defined $params ? {%$params} : {}; + + # children should be able to handle without + my $embeds = delete $params->{embed}; + my $av_expand = delete $params->{av_expand}; + # coded values handling my $avs = {}; - if ( $params->{av_expand} and $self->can('api_av_mapping') ) { + if ( $av_expand and $self->can('api_av_mapping') ) { $avs = $self->api_av_mapping($params); } @@ -565,7 +573,7 @@ sub to_api { delete $json_object->{$field} unless any { $_ eq $field } @{ $self->public_read_list }; } - if ( $params->{av_expand} ) { + if ( $av_expand ) { foreach my $field (keys %{$avs}) { delete $avs->{$field} unless any { $_ eq $field } @{ $self->public_read_list }; @@ -598,12 +606,7 @@ sub to_api { } $json_object->{_str} = $avs - if $params->{av_expand}; - - # Make sure we duplicate the $params variable to avoid - # breaking calls in a loop (Koha::Objects->to_api) - $params = {%$params}; - my $embeds = delete $params->{embed}; + if $av_expand; if ($embeds) { foreach my $embed ( keys %{$embeds} ) { @@ -617,6 +620,9 @@ sub to_api { my $curr = $embed; my $next = $embeds->{$curr}->{children}; + $params->{av_expand} = 1 + if $embeds->{$embed}->{av_expand}; + my $children = $self->$curr; if ( defined $children and ref($children) eq 'ARRAY' ) { diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index c9a2a26106..ccc0a08942 100644 --- a/Koha/REST/Plugin/Objects.pm +++ b/Koha/REST/Plugin/Objects.pm @@ -54,7 +54,7 @@ the requested object. It passes through any embeds if specified. # Look for embeds my $embed = $c->stash('koha.embed'); - my $av_expand = $c->req->headers->header('x-koha-av-expand'); + my $av_expand = $c->stash('koha.av_expand'); # Generate prefetches for embedded stuff $c->dbic_merge_prefetch( @@ -100,7 +100,7 @@ shouldn't be called twice in it. my $is_public = $c->stash('is_public'); # Look for embeds my $embed = $c->stash('koha.embed'); - my $av_expand = $c->req->headers->header('x-koha-av-expand'); + my $av_expand = $c->stash('koha.av_expand'); # Merge sorting into query attributes $c->dbic_merge_sorting( diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm index b1418d6c19..f75d284625 100644 --- a/Koha/REST/Plugin/Query.pm +++ b/Koha/REST/Plugin/Query.pm @@ -242,7 +242,11 @@ Unwraps and stashes the x-koha-embed headers for use later query construction if ($embed_header) { my $THE_embed = {}; foreach my $embed_req ( split /\s*,\s*/, $embed_header ) { - _merge_embed( _parse_embed($embed_req), $THE_embed ); + if ( $embed_req eq '+av_expand' ) { # special case + $c->stash( 'koha.av_expand' => 1 ); + } else { + _merge_embed( _parse_embed($embed_req), $THE_embed ); + } } $c->stash( 'koha.embed' => $THE_embed ) @@ -364,6 +368,10 @@ sub _parse_embed { my $key = $+{relation} . "_count"; $result->{$key} = { is_count => 1 }; } + elsif ( $curr =~ m/^(?.*)\+av_expand/ ) { + my $key = $+{relation}; + $result->{$key} = { av_expand => 1 }; + } else { $result->{$curr} = {}; } diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index 5a3774599f..509f4f9ff6 100755 --- a/t/Koha/REST/Plugin/Query.t +++ b/t/Koha/REST/Plugin/Query.t @@ -210,11 +210,15 @@ get '/stash_embed' => sub { my $c = shift; $c->stash_embed(); - my $embed = $c->stash('koha.embed'); + my $embed = $c->stash('koha.embed'); + my $av_expand = $c->stash('koha.av_expand'); $c->render( status => 200, - json => $embed + json => { + av_expand => $av_expand, + embed => $embed, + } ); }; @@ -429,24 +433,38 @@ subtest '_build_query_params_from_api' => sub { subtest 'stash_embed() tests' => sub { - plan tests => 8; + plan tests => 14; my $t = Test::Mojo->new; $t->get_ok( '/stash_embed' => { 'x-koha-embed' => 'checkouts,checkouts.item' } ) - ->json_is( { checkouts => { children => { item => { } } } } ); + ->json_is( '/embed' => { checkouts => { children => { item => { } } } } ); $t->get_ok( '/stash_embed' => { 'x-koha-embed' => 'checkouts,checkouts.item,library' } ) - ->json_is( { checkouts => { children => { item => {} } }, library => {} } ); + ->json_is( '/embed' => { checkouts => { children => { item => {} } }, library => {} } ); $t->get_ok( '/stash_embed' => { 'x-koha-embed' => 'holds+count' } ) - ->json_is( { holds_count => { is_count => 1 } } ); + ->json_is( '/embed' => { holds_count => { is_count => 1 } } ); $t->get_ok( '/stash_embed' => { 'x-koha-embed' => 'checkouts,checkouts.item,patron' } ) - ->json_is({ + ->json_is( '/embed' => { checkouts => { children => { item => {} } }, patron => {} }); + + $t->get_ok( '/stash_embed' => { 'x-koha-embed' => 'checkouts,checkouts.item+av_expand,patron+av_expand' } ) + ->json_is( '/embed' => { + checkouts => { children => { item => { av_expand => 1 } } }, + patron => { av_expand => 1 } + }) + ->json_is( '/av_expand' => undef ); + + $t->get_ok( '/stash_embed' => { 'x-koha-embed' => 'checkouts+av_expand,checkouts.item,patron,+av_expand' } ) + ->json_is( '/embed' => { + checkouts => { children => { item => { } }, av_expand => 1 }, + patron => { } + }) + ->json_is( '/av_expand' => 1 ); }; subtest 'stash_overrides() tests' => sub { diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index 6e7d8a1a9f..47346c5850 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -226,7 +226,7 @@ subtest 'TO_JSON tests' => sub { subtest "to_api() tests" => sub { - plan tests => 30; + plan tests => 31; $schema->storage->txn_begin; @@ -303,14 +303,36 @@ subtest "to_api() tests" => sub { 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 $_str = { + location => { + category => 'ASD', + str => 'Estante alto', + type => 'av' + } + }; + + # mock Koha::Item so it implements 'api_av_mapping' + my $item_mock = Test::MockModule->new('Koha::Item'); + $item_mock->mock( + 'api_av_mapping', + sub { + return $_str; + } + ); + my $hold_api = $hold->to_api( { - embed => { 'item' => {} } + embed => { 'item' => { av_expand => 1 } } } ); is( ref($hold_api->{item}), 'HASH', 'Single nested object works correctly' ); is( $hold_api->{item}->{item_id}, $item->itemnumber, 'Object embedded correctly' ); + is_deeply( + $hold_api->{item}->{_str}, + $_str, + '_str correctly added to nested embed' + ); # biblio with no items my $new_biblio = $builder->build_sample_biblio; diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t index 21f89f0639..487b9f6380 100755 --- a/t/db_dependent/Koha/REST/Plugin/Objects.t +++ b/t/db_dependent/Koha/REST/Plugin/Objects.t @@ -38,6 +38,7 @@ plugin 'Koha::REST::Plugin::Pagination'; get '/cities' => sub { my $c = shift; $c->validation->output($c->req->params->to_hash); + $c->stash_embed; my $cities = $c->objects->search(Koha::Cities->new); $c->render( status => 200, json => $cities ); }; @@ -45,6 +46,7 @@ get '/cities' => sub { get '/cities/:city_id' => sub { my $c = shift; my $id = $c->stash("city_id"); + $c->stash_embed; my $city = $c->objects->find(Koha::Cities->new, $id); $c->render( status => 200, json => $city ); }; @@ -712,7 +714,7 @@ subtest 'objects.find helper with expanded authorised values' => sub { } ); - $t->get_ok( '/cities/' . $manuel->id => { 'x-koha-av-expand' => 1 } ) + $t->get_ok( '/cities/' . $manuel->id => { 'x-koha-embed' => '+av_expand' } ) ->status_is(200)->json_is( '/name' => 'Manuel' ) ->json_has('/_str') ->json_is( '/_str/country/type' => 'av' ) @@ -723,7 +725,7 @@ subtest 'objects.find helper with expanded authorised values' => sub { ->status_is(200)->json_is( '/name' => 'Manuel' ) ->json_hasnt('/_str'); - $t->get_ok( '/cities/' . $manuela->id => { 'x-koha-av-expand' => 1 } ) + $t->get_ok( '/cities/' . $manuela->id => { 'x-koha-embed' => '+av_expand' } ) ->status_is(200)->json_is( '/name' => 'Manuela' ) ->json_has('/_str') ->json_is( '/_str/country/type' => 'av' ) @@ -827,7 +829,7 @@ subtest 'objects.search helper with expanded authorised values' => sub { ); $t->get_ok( '/cities?name=manuel&_per_page=4&_page=1&_match=starts_with' => - { 'x-koha-av-expand' => 1 } )->status_is(200) + { 'x-koha-embed' => '+av_expand' } )->status_is(200) ->json_has('/0')->json_has('/1')->json_hasnt('/2') ->json_is( '/0/name' => 'Manuel' ) ->json_has('/0/_str') @@ -840,8 +842,7 @@ subtest 'objects.search helper with expanded authorised values' => sub { ->json_is( '/1/_str/country/type' => 'av' ) ->json_is( '/1/_str/country/category' => $cat->category_name ); - $t->get_ok( '/cities?name=manuel&_per_page=4&_page=1&_match=starts_with' => - { 'x-koha-av-expand' => 0 } )->status_is(200) + $t->get_ok( '/cities?name=manuel&_per_page=4&_page=1&_match=starts_with' )->status_is(200) ->json_has('/0')->json_has('/1')->json_hasnt('/2') ->json_is( '/0/name' => 'Manuel' )->json_hasnt('/0/_str') ->json_is( '/1/name' => 'Manuela' )->json_hasnt('/1/_str'); -- 2.20.1