From d131888605747f5dc33b4010045b6d6f361121dc Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 10 Jan 2022 16:18:57 -0300 Subject: [PATCH] Bug 29844: Remove use of wantarray from Koha::Objects This patch removes the use of `wantarray` from the following methods in the Koha::Objects class: - search - search_related In both cases, the change is trivial. And the tests get the 'list context' portion removed as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Caveat: we broke many things by removing the feature. Check follow-up patches as well. --- Koha/Objects.pm | 41 ++++++-------------------- t/db_dependent/Koha/Objects.t | 54 ++++------------------------------- 2 files changed, 14 insertions(+), 81 deletions(-) diff --git a/Koha/Objects.pm b/Koha/Objects.pm index 3ee0a55742..133d0cb08e 100644 --- a/Koha/Objects.pm +++ b/Koha/Objects.pm @@ -34,7 +34,7 @@ Koha::Objects - Koha Object set base class =head1 SYNOPSIS use Koha::Objects; - my @objects = Koha::Objects->search({ borrowernumber => $borrowernumber}); + my $objects = Koha::Objects->search({ borrowernumber => $borrowernumber}); =head1 DESCRIPTION @@ -122,8 +122,6 @@ sub find_or_create { =head3 search - # list context - my @objects = Koha::Objects->search([$params, $attributes]); # scalar context my $objects = Koha::Objects->search([$params, $attributes]); while (my $object = $objects->next) { @@ -133,31 +131,19 @@ sub find_or_create { This B the I class, and generates a resultset based on the query I<$params> and I<$attributes> that are passed (like in DBIC). -In B it returns an array of I objects. -In B it returns an iterator. - =cut sub search { my ( $self, $params, $attributes ) = @_; - if (wantarray) { - my @dbic_rows = $self->_resultset()->search($params, $attributes); - - return $self->_wrap(@dbic_rows); + my $class = ref($self) ? ref($self) : $self; + my $rs = $self->_resultset()->search($params, $attributes); - } - else { - my $class = ref($self) ? ref($self) : $self; - my $rs = $self->_resultset()->search($params, $attributes); - - return $class->_new_from_dbic($rs); - } + return $class->_new_from_dbic($rs); } =head3 search_related - my @objects = Koha::Objects->search_related( $rel_name, $cond?, \%attrs? ); my $objects = Koha::Objects->search_related( $rel_name, $cond?, \%attrs? ); Searches the specified relationship, optionally specifying a condition and attributes for matching records. @@ -168,22 +154,13 @@ sub search_related { my ( $self, $rel_name, @params ) = @_; return if !$rel_name; - if (wantarray) { - my @dbic_rows = $self->_resultset()->search_related($rel_name, @params); - return if !@dbic_rows; - my $object_class = _get_objects_class( $dbic_rows[0]->result_class ); - eval "require $object_class"; - return _wrap( $object_class, @dbic_rows ); + my $rs = $self->_resultset()->search_related($rel_name, @params); + return if !$rs; + my $object_class = _get_objects_class( $rs->result_class ); - } else { - my $rs = $self->_resultset()->search_related($rel_name, @params); - return if !$rs; - my $object_class = _get_objects_class( $rs->result_class ); - - eval "require $object_class"; - return _new_from_dbic( $object_class, $rs ); - } + eval "require $object_class"; + return _new_from_dbic( $object_class, $rs ); } =head3 delete diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t index 34385dc7f4..a64f9007eb 100755 --- 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 => 24; +use Test::More tests => 23; use Test::Exception; use Test::MockModule; use Test::Warn; @@ -141,7 +141,9 @@ subtest 'find' => sub { }; subtest 'search_related' => sub { - plan tests => 6; + + plan tests => 3; + my $builder = t::lib::TestBuilder->new; my $patron_1 = $builder->build( { source => 'Borrower' } ); my $patron_2 = $builder->build( { source => 'Borrower' } ); @@ -163,26 +165,6 @@ subtest 'search_related' => sub { [ $patron_1->{branchcode}, $patron_2->{branchcode} ] ), 'Koha::Objects->search_related should work as expected' ); - - my @libraries = Koha::Patrons->search( - { - -or => { - borrowernumber => - [ $patron_1->{borrowernumber}, $patron_2->{borrowernumber} ] - } - } - )->search_related('branchcode'); - is( - ref( $libraries[0] ), 'Koha::Library', - 'Koha::Objects->search_related should return a list of Koha::Object-based objects' - ); - is( scalar(@libraries), 2, - 'Koha::Objects->search_related should work as expected' ); - ok( eq_array( - [ map { $_->branchcode } @libraries ], - [ $patron_1->{branchcode}, $patron_2->{branchcode} ] ), - 'Koha::Objects->search_related should work as expected' - ); }; subtest 'single' => sub { @@ -211,7 +193,7 @@ subtest 'last' => sub { subtest 'get_column' => sub { plan tests => 1; - my @cities = Koha::Cities->search; + my @cities = Koha::Cities->search->as_list; my @city_names = map { $_->city_name } @cities; is_deeply( [ Koha::Cities->search->get_column('city_name') ], \@city_names, 'Koha::Objects->get_column should be allowed' ); }; @@ -287,32 +269,6 @@ subtest '->is_paged and ->pager tests' => sub { $schema->storage->txn_rollback; }; -subtest '->search() tests' => sub { - - plan tests => 12; - - $schema->storage->txn_begin; - - my $count = Koha::Patrons->search->count; - - # Create 10 patrons - foreach (1..10) { - $builder->build_object({ class => 'Koha::Patrons' }); - } - - my $patrons = Koha::Patrons->search(); - is( ref($patrons), 'Koha::Patrons', 'search in scalar context returns the Koha::Object-based type' ); - my @patrons = Koha::Patrons->search(); - is( scalar @patrons, $count + 10, 'search in list context returns a list of objects' ); - my $i = 0; - foreach (1..10) { - is( ref($patrons[$i]), 'Koha::Patron', 'Objects in the list have the singular type' ); - $i++; - } - - $schema->storage->txn_rollback; -}; - subtest "to_api() tests" => sub { plan tests => 19; -- 2.32.0