From 353f328ad3a99cc5a9ee913cd1f72a1d841705f2 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 Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- Koha/REST/Plugin/Objects.pm | 58 +++++++++++++++++++++++++-------- Koha/REST/V1/Biblios.pm | 9 ++--- t/db_dependent/api/v1/biblios.t | 15 ++++++++- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index ea562f7ec7..f09bf70bd6 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( { @@ -203,26 +222,39 @@ 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 ); + } } 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 04379ddf11..54f6fe52a5 100755 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -38,6 +38,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; @@ -1619,7 +1621,7 @@ subtest 'put() tests' => sub { subtest 'list() tests' => sub { - plan tests => 15; + plan tests => 17; $schema->storage->txn_begin; @@ -1677,6 +1679,17 @@ subtest 'list() tests' => sub { $t->get_ok( "//$userid:$password@/api/v1/biblios?q=$query" => { Accept => 'text/plain' } )->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; }; -- 2.20.1