Bug 18179: Forbid list context calls for Koha::Objects->find

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 <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/Koha/Objects.pm b/Koha/Objects.pm
index 7e73297..5080e7d 100644 (file)
--- 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 935dc47..aa897c4 100644 (file)
--- 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;