From 15cbf14f4d4f8039ad7820ce37da202b19e0075d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 18 Apr 2017 13:49:18 -0300 Subject: [PATCH] Bug 18539: Forbid list context calls for Koha::Objects->find MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Reading https://perlmaven.com/how-to-return-undef-from-a-function this sound like the more correct behaviour. Considering: $template->param( stuff => Koha::Stuffs->find( $id ), foo => 1, ); without this patch, if the $id does not represent any rows in the DB, stuff will be assigned to 'foo' and $foo will be undef in the template. That can lead to very bad side-effects. With this patch we make sure that it will never happen again. Test plan: prove t/db_dependent/Koha/Objects.t should return green Signed-off-by: Marc Véron Signed-off-by: Nick Clemens Signed-off-by: Kyle M Hall Signed-off-by: Jonathan Druart --- Koha/Objects.pm | 2 ++ t/db_dependent/Koha/Objects.t | 12 +++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Koha/Objects.pm b/Koha/Objects.pm index 7e73297695..5080e7d0aa 100644 --- a/Koha/Objects.pm +++ b/Koha/Objects.pm @@ -78,6 +78,8 @@ my $object = Koha::Objects->find( { keypart1 => $keypart1, keypart2 => $keypart2 sub find { my ( $self, $id ) = @_; + croak 'Cannot use "->find" in list context' if wantarray; + return unless defined($id); my $result = $self->_resultset()->find($id); diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t index 935dc47bec..aa897c4054 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 => 13; +use Test::More tests => 14; use Test::Warn; use Koha::Authority::Types; @@ -43,6 +43,16 @@ my @columns = Koha::Patrons->columns; my $borrowernumber_exists = grep { /^borrowernumber$/ } @columns; is( $borrowernumber_exists, 1, 'Koha::Objects->columns should return the table columns' ); +subtest 'find' => sub { + plan tests => 2; + my $patron = $builder->build({source => 'Borrower'}); + my $patron_object = Koha::Patrons->find( $patron->{borrowernumber} ); + is( $patron_object->borrowernumber, $patron->{borrowernumber}, '->find should return the correct object' ); + + eval { my @patrons = Koha::Patrons->find( $patron->{borrowernumber} ); }; + like( $@, qr|^Cannot use "->find" in list context|, "->find should not be called in list context to avoid side-effects" ); +}; + subtest 'update' => sub { plan tests => 2; -- 2.39.5