From 3059bd409c3d4c1c0c85e4a0d00988affb7e57b3 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 16 Jun 2023 12:50:07 -0300 Subject: [PATCH] Bug 33974: Make biblioitem columns searchable in a generic way This patch makes biblioitem attributes be searchable on the biblios endpoint. It does so by using the new method in Koha::Biblios, and by adjusting objects.search(_rs) to accept a $query_fixer arrayref of functions to be applied to each query or order_by parameters. The result is cleaner code to write, but complex internals. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Tests pass! Searching for biblioitem attributes works on the API! 3. Sign off :-D Signed-off-by: Sam Lau --- Koha/REST/Plugin/Objects.pm | 68 +++++++++++++++++++++++++-------- Koha/REST/V1/Biblios.pm | 9 +++-- t/db_dependent/api/v1/biblios.t | 20 ++++++++-- 3 files changed, 73 insertions(+), 24 deletions(-) diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index 94baf03753..6313b912c6 100644 --- a/Koha/REST/Plugin/Objects.pm +++ b/Koha/REST/Plugin/Objects.pm @@ -112,10 +112,10 @@ shouldn't be called twice in it. $app->helper( 'objects.search' => sub { - my ( $c, $result_set ) = @_; + my ( $c, $result_set, $query_fixers ) = @_; # Generate the resultset using the HTTP request information - my $objects_rs = $c->objects->search_rs($result_set); + my $objects_rs = $c->objects->search_rs( $result_set, $query_fixers ); # Add pagination headers $c->add_pagination_headers(); @@ -127,13 +127,21 @@ shouldn't be called twice in it. =head3 objects.search_rs my $patrons_object = Koha::Patrons->new; - my $patrons_rs = $c->objects->search_rs( $patrons_object ); + my $patrons_rs = $c->objects->search_rs( $patrons_object [, $query_fixers ] ); . . . 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. +The optional I<$query_fixers> parameter is expected to be a reference to a list of +function references. This functions need to accept two parameters: ( $query, $no_quotes ). + +The I<$query> is a string to be adapted. A modified version will be returned. The +I<$no_quotes> parameter controls if quotes need to be added for matching inside the string. +Quoting should be used by default, for replacing JSON keys e.g "biblio.isbn" would match +and biblio.isbn wouldn't. + Warning: this helper stashes base values for the pagination headers to the calling controller, and thus shouldn't be called twice in it. @@ -141,15 +149,26 @@ controller, and thus shouldn't be called twice in it. $app->helper( 'objects.search_rs' => sub { - my ( $c, $result_set ) = @_; + my ( $c, $result_set, $query_fixers ) = @_; my $args = $c->validation->output; my $attributes = {}; + $query_fixers //= []; + # Extract reserved params my ( $filtered_params, $reserved_params, $path_params ) = $c->extract_reserved_params($args); + if ( exists $reserved_params->{_order_by} ) { + # _order_by passed, fix if required + for my $p ( @{$reserved_params->{_order_by}} ) { + foreach my $qf ( @{$query_fixers} ) { + $p = $qf->($p, 1); # 1 => no quotes on matching + } + } + } + # Merge sorting into query attributes $c->dbic_merge_sorting( { @@ -204,31 +223,48 @@ controller, and thus shouldn't be called twice in it. my @query_params_array; - # query in request body, JSON::Validator already decoded it - push @query_params_array, $reserved_params->{query} - if defined $reserved_params->{query}; - my $json = JSON->new; + # query in request body, JSON::Validator already decoded it + if ( $reserved_params->{query} ) { + my $query = $json->encode( $reserved_params->{query} ); + foreach my $qf ( @{$query_fixers} ) { + $query = $qf->($query); + } + push @query_params_array, $json->decode($query); + } + 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 + foreach my $qf (@{$query_fixers}) { + $q = $qf->($q); + } + push @query_params_array, $json->decode($q); + } } } else { # objects.search called outside OpenAPI context # might be a hashref - push @query_params_array, - $json->decode( $reserved_params->{q} ) - if $reserved_params->{q}; + if ( $reserved_params->{q} ) { + my $q = $reserved_params->{q}; + foreach my $qf (@{$query_fixers}) { + $q = $qf->($q); + } + push @query_params_array, $json->decode( $q ); + } } - push @query_params_array, - $json->decode( $reserved_params->{'x-koha-query'} ) - if defined $reserved_params->{'x-koha-query'}; + if ( $reserved_params->{'x-koha-query'} ) { + my $q = $reserved_params->{'x-koha-query'}; + foreach my $qf (@{$query_fixers}) { + $q = $qf->($q); + } + push @query_params_array, $json->decode( $q ); + } my $query_params; diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index 6457d49d7a..bbfec9c1c7 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -31,6 +31,7 @@ use C4::Context; use Koha::Items; +use Clone qw( clone ); use List::MoreUtils qw( any ); use MARC::Record::MiJ; @@ -797,12 +798,12 @@ Controller function that handles retrieving a single biblio object sub list { my $c = shift->openapi->valid_input or return; - my $attributes; - $attributes = - { prefetch => ['metadata'] } # don't prefetch metadata if not needed + my @prefetch = qw(biblioitem); + push @prefetch, 'metadata' # don't prefetch metadata if not needed unless $c->req->headers->accept =~ m/application\/json/; - my $biblios = $c->objects->search_rs( Koha::Biblios->new ); + my $rs = Koha::Biblios->search( undef, { prefetch => \@prefetch }); + my $biblios = $c->objects->search_rs( $rs, [(sub{ $rs->api_query_fixer( $_[0], '', $_[1] ) })] ); return try { diff --git a/t/db_dependent/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t index eb6dcaa78b..2fd0477fd5 100755 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -36,6 +36,8 @@ use Koha::Database; use Koha::Checkouts; use Koha::Old::Checkouts; +use Mojo::JSON qw(encode_json); + my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; @@ -1617,7 +1619,7 @@ subtest 'put() tests' => sub { subtest 'list() tests' => sub { - plan tests => 15; + plan tests => 17; $schema->storage->txn_begin; @@ -1651,8 +1653,7 @@ subtest 'list() tests' => sub { t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); my $biblionumber2 = $builder->build_sample_biblio->biblionumber; - my $search = -"[{\"biblionumber\": \"$biblionumber1\"}, {\"biblionumber\": \"$biblionumber2\"}]"; + my $search = encode_json( [ { 'me.biblio_id' => $biblionumber1 }, { 'me.biblio_id' => $biblionumber2 } ] ); $t->get_ok( "//$userid:$password@/api/v1/biblios/" => { 'x-koha-query' => $search } )->status_is(403); @@ -1686,6 +1687,17 @@ subtest 'list() tests' => sub { { Accept => 'text/plain', 'x-koha-query' => $search } ) ->status_is(200); + # DELETE any biblio with ISBN = TOMAS + Koha::Biblios->search({ 'biblioitem.isbn' => 'TOMAS' }, { join => [ 'biblioitem' ] }) + ->delete; + + + my $isbn_query = encode_json({ isbn => 'TOMAS' }); + $biblio->biblioitem->set({ isbn => 'TOMAS' })->store; + $t->get_ok( "//$userid:$password@/api/v1/biblios?q=$isbn_query" => + { Accept => 'text/plain' } ) + ->status_is(200); + $schema->storage->txn_rollback; }; @@ -1801,4 +1813,4 @@ subtest 'update_item() tests' => sub { ->json_is('/replacement_price', 30); $schema->storage->txn_rollback; -}; \ No newline at end of file +}; -- 2.30.2