From fa76308acb0a3438db024e382874df95d2c3a9ad Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 21 Nov 2017 16:12:08 -0300 Subject: [PATCH] Bug 19410: (QA followup) Move build_query_params_from_api into a helper This patch creates the 'build_query_params' helper, instead of the original function in Koha::Objects. Unit tests are removed for Koha::Objects::_build_query_params_from_api and written for the helper plugin. The objects.search helper gets a call to build_query_params added. Tests for it updated to match this behaviour change. To test: - Apply this patches - Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/Koha/Objects.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Julian Maurice Signed-off-by: Lari Taskula --- Koha/Objects.pm | 41 --------------- Koha/REST/Plugin/Objects.pm | 1 + Koha/REST/Plugin/Query.pm | 47 +++++++++++++++++ t/Koha/REST/Plugin/Query.t | 57 +++++++++++++++++++- t/db_dependent/Koha/Objects.t | 65 +---------------------- t/db_dependent/Koha/REST/Plugin/Objects.t | 86 ++++++++++++++++++++++++------- 6 files changed, 172 insertions(+), 125 deletions(-) diff --git a/Koha/Objects.pm b/Koha/Objects.pm index cfc2475..58a0c68 100644 --- a/Koha/Objects.pm +++ b/Koha/Objects.pm @@ -171,47 +171,6 @@ sub search_related { } } -=head2 _build_query_params_from_api - - my $params = _build_query_params_from_api( $filtered_params, $reserved_params ); - -Builds the params for searching on DBIC based on the selected matching algorithm. -Valid options are I, I, I and I. Default is -I. If other value is passed, a Koha::Exceptions::WrongParameter exception -is raised. - -=cut - -sub _build_query_params_from_api { - - my ( $filtered_params, $reserved_params ) = @_; - - my $params; - my $match = $reserved_params->{_match} // 'contains'; - - foreach my $param ( keys %{$filtered_params} ) { - if ( $match eq 'contains' ) { - $params->{$param} = - { like => '%' . $filtered_params->{$param} . '%' }; - } - elsif ( $match eq 'starts_with' ) { - $params->{$param} = { like => $filtered_params->{$param} . '%' }; - } - elsif ( $match eq 'ends_with' ) { - $params->{$param} = { like => '%' . $filtered_params->{$param} }; - } - elsif ( $match eq 'exact' ) { - $params->{$param} = $filtered_params->{$param}; - } - else { - Koha::Exceptions::WrongParameter->throw( - "Invalid value for _match param ($match)"); - } - } - - return $params; -} - =head3 single my $object = Koha::Objects->search({}, { rows => 1 })->single diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index afd5e8e..eb3f951 100644 --- a/Koha/REST/Plugin/Objects.pm +++ b/Koha/REST/Plugin/Objects.pm @@ -67,6 +67,7 @@ sub register { } ); + $filtered_params = $c->build_query_params( $filtered_params, $reserved_params ); # Perform search my $objects = $objects_set->search( $filtered_params, $attributes ); diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm index 45cfe14..77d2fad 100644 --- a/Koha/REST/Plugin/Query.pm +++ b/Koha/REST/Plugin/Query.pm @@ -19,6 +19,8 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Plugin'; +use Koha::Exceptions; + =head1 NAME Koha::REST::Plugin::Query @@ -88,6 +90,51 @@ Generates the DBIC order_by attributes based on I<$params>, and merges into I<$a return $attributes; } ); + +=head3 _build_query_params_from_api + + my $params = _build_query_params_from_api( $filtered_params, $reserved_params ); + +Builds the params for searching on DBIC based on the selected matching algorithm. +Valid options are I, I, I and I. Default is +I. If other value is passed, a Koha::Exceptions::WrongParameter exception +is raised. + +=cut + + $app->helper( + 'build_query_params' => sub { + + my ( $c, $filtered_params, $reserved_params ) = @_; + + my $params; + my $match = $reserved_params->{_match} // 'contains'; + + foreach my $param ( keys %{$filtered_params} ) { + if ( $match eq 'contains' ) { + $params->{$param} = + { like => '%' . $filtered_params->{$param} . '%' }; + } + elsif ( $match eq 'starts_with' ) { + $params->{$param} = { like => $filtered_params->{$param} . '%' }; + } + elsif ( $match eq 'ends_with' ) { + $params->{$param} = { like => '%' . $filtered_params->{$param} }; + } + elsif ( $match eq 'exact' ) { + $params->{$param} = $filtered_params->{$param}; + } + else { + # We should never reach here, because the OpenAPI plugin should + # prevent invalid params to be passed + Koha::Exceptions::WrongParameter->throw( + "Invalid value for _match param ($match)"); + } + } + + return $params; + } + ); } =head2 Internal methods diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index 2851962..a4c6684 100644 --- a/t/Koha/REST/Plugin/Query.t +++ b/t/Koha/REST/Plugin/Query.t @@ -19,6 +19,7 @@ use Modern::Perl; # Dummy app for testing the plugin use Mojolicious::Lite; +use Try::Tiny; app->log->level('error'); @@ -79,9 +80,26 @@ get '/dbic_merge_sorting' => sub { $c->render( json => $attributes, status => 200 ); }; +get '/build_query' => sub { + my $c = shift; + my ( $filtered_params, $reserved_params ) = + $c->extract_reserved_params( $c->req->params->to_hash ); + my $query; + try { + $query = $c->build_query_params( $filtered_params, $reserved_params ); + $c->render( json => { query => $query }, status => 200 ); + } + catch { + $c->render( + json => { exception_msg => $_->message, exception_type => ref($_) }, + status => 400 + ); + }; +}; + # The tests -use Test::More tests => 2; +use Test::More tests => 3; use Test::Mojo; subtest 'extract_reserved_params() tests' => sub { @@ -123,3 +141,40 @@ subtest 'dbic_merge_sorting() tests' => sub { ->json_is( '/b' => 'b', 'Existing values are kept (b)' ) ->json_is( '/order_by' => [ 'uno', { -desc => 'dos' }, { -asc => 'tres' } ] ); }; + +subtest '_build_query_params_from_api' => sub { + + plan tests => 16; + + my $t = Test::Mojo->new; + + # _match => contains + $t->get_ok('/build_query?_match=contains&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => + { author => { like => '%Orson%' }, title => { like => '%Ender%' } } ); + + # _match => starts_with + $t->get_ok('/build_query?_match=starts_with&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => + { author => { like => 'Orson%' }, title => { like => 'Ender%' } } ); + + # _match => ends_with + $t->get_ok('/build_query?_match=ends_with&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => + { author => { like => '%Orson' }, title => { like => '%Ender' } } ); + + # _match => exact + $t->get_ok('/build_query?_match=exact&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => { author => 'Orson', title => 'Ender' } ); + + # _match => blah + $t->get_ok('/build_query?_match=blah&title=Ender&author=Orson') + ->status_is(400) + ->json_is( '/exception_msg' => 'Invalid value for _match param (blah)' ) + ->json_is( '/exception_type' => 'Koha::Exceptions::WrongParameter' ); + +}; diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t index cf261b2..9c41d9f 100644 --- a/t/db_dependent/Koha/Objects.t +++ b/t/db_dependent/Koha/Objects.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 16; +use Test::More tests => 15; use Test::Exception; use Test::Warn; @@ -239,66 +239,3 @@ subtest '->is_paged and ->pager tests' => sub { $schema->storage->txn_rollback; }; - -subtest '_build_query_params_from_api' => sub { - - plan tests => 5; - - my $filtered_params = { - title => "Ender", - author => "Orson" - }; - - subtest '_match => contains' => sub { - plan tests => 2; - my $reserved_params = { _match => 'contains' }; - my $params = Koha::Objects::_build_query_params_from_api( $filtered_params, $reserved_params ); - - is_deeply( $params->{author}, { like => '%Orson%' } ); - is_deeply( $params->{title}, { like => '%Ender%' } ); - }; - - subtest '_match => starts_with' => sub { - plan tests => 2; - my $reserved_params = { _match => 'starts_with' }; - my $params = Koha::Objects::_build_query_params_from_api( $filtered_params, $reserved_params ); - - is_deeply( $params->{author}, { like => 'Orson%' } ); - is_deeply( $params->{title}, { like => 'Ender%' } ); - }; - - subtest '_match => ends_with' => sub { - plan tests => 2; - my $reserved_params = { _match => 'ends_with' }; - my $params = Koha::Objects::_build_query_params_from_api( $filtered_params, $reserved_params ); - - is_deeply( $params->{author}, { like => '%Orson' } ); - is_deeply( $params->{title}, { like => '%Ender' } ); - }; - - subtest '_match => exact' => sub { - plan tests => 2; - my $reserved_params = { _match => 'exact' }; - my $params = Koha::Objects::_build_query_params_from_api( $filtered_params, $reserved_params ); - - is( $params->{author}, 'Orson' ); - is( $params->{title}, 'Ender' ); - }; - - subtest '_match => blah' => sub { - plan tests => 2; - - my $reserved_params = { _match => 'blah' }; - throws_ok { - Koha::Objects::_build_query_params_from_api( $filtered_params, - $reserved_params ); - } - 'Koha::Exceptions::WrongParameter', - 'Exception thrown on invalid _match parameter'; - - is( $@, - 'Invalid value for _match param (blah)', - 'Exception carries the right message' - ); - }; -}; diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t index 6d305f5..46eed8f 100644 --- a/t/db_dependent/Koha/REST/Plugin/Objects.t +++ b/t/db_dependent/Koha/REST/Plugin/Objects.t @@ -31,12 +31,11 @@ get '/patrons' => sub { my $c = shift; $c->validation->output($c->req->params->to_hash); my $patrons = $c->objects->search(Koha::Patrons->new); - $c->render( status => 200, json => $patrons->TO_JSON ); + $c->render( status => 200, json => $patrons ); }; # The tests - use Test::More tests => 1; use Test::Mojo; @@ -44,34 +43,83 @@ use t::lib::TestBuilder; use Koha::Database; my $schema = Koha::Database->new()->schema(); -$schema->storage->txn_begin(); + my $builder = t::lib::TestBuilder->new; -$builder->build({ - source => 'Borrower', - value => { - firstname => 'Manuel', - }, -}); -$builder->build({ - source => 'Borrower', - value => { - firstname => 'Manuel', - }, -}); subtest 'objects.search helper' => sub { - plan tests => 6; + plan tests => 34; my $t = Test::Mojo->new; - $t->get_ok('/patrons?firstname=Manuel&_per_page=1&_page=1') + $schema->storage->txn_begin; + + # Delete existing patrons + Koha::Patrons->search->delete; + # Create two sample patrons that match the query + $builder->build_object({ + class => 'Koha::Patrons', + value => { + firstname => 'Manuel' + } + }); + $builder->build_object({ + class => 'Koha::Patrons', + value => { + firstname => 'Manuela' + } + }); + + $t->get_ok('/patrons?firstname=manuel&_per_page=1&_page=1') ->status_is(200) ->header_like( 'Link' => qr/; rel="next",/ ) ->json_has('/0') ->json_hasnt('/1') ->json_is('/0/firstname' => 'Manuel'); -}; -$schema->storage->txn_rollback(); + $builder->build_object({ + class => 'Koha::Patrons', + value => { + firstname => 'Emanuel' + } + }); + + # _match=starts_with + $t->get_ok('/patrons?firstname=manuel&_per_page=3&_page=1&_match=starts_with') + ->status_is(200) + ->json_has('/0') + ->json_has('/1') + ->json_hasnt('/2') + ->json_is('/0/firstname' => 'Manuel') + ->json_is('/1/firstname' => 'Manuela'); + + # _match=ends_with + $t->get_ok('/patrons?firstname=manuel&_per_page=3&_page=1&_match=ends_with') + ->status_is(200) + ->json_has('/0') + ->json_has('/1') + ->json_hasnt('/2') + ->json_is('/0/firstname' => 'Manuel') + ->json_is('/1/firstname' => 'Emanuel'); + + # _match=exact + $t->get_ok('/patrons?firstname=manuel&_per_page=3&_page=1&_match=exact') + ->status_is(200) + ->json_has('/0') + ->json_hasnt('/1') + ->json_is('/0/firstname' => 'Manuel'); + + # _match=contains + $t->get_ok('/patrons?firstname=manuel&_per_page=3&_page=1&_match=contains') + ->status_is(200) + ->json_has('/0') + ->json_has('/1') + ->json_has('/2') + ->json_hasnt('/3') + ->json_is('/0/firstname' => 'Manuel') + ->json_is('/1/firstname' => 'Manuela') + ->json_is('/2/firstname' => 'Emanuel'); + + $schema->storage->txn_rollback; +}; -- 2.7.4