Attachment 62304 Details for Bug 18179 – Bug 18179: Forbid list context calls for Koha::Objects->find

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

Bug-18179-Forbid-list-context-calls-for-KohaObject.patch (text/plain), 2.38 KB, created by Jonathan Druart on 2017-04-18 17:02:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-04-18 17:02:06 UTC

Size: 2.38 KB

patch

obsolete

>From 4042e87d492cec809265b0ba9f6e508a14523478 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Tue, 18 Apr 2017 13:49:18 -0300
>Subject: [PATCH] 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
>---
> 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 7e73297..5080e7d 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 935dc47..aa897c4 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.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 18179: 62216 | 62218 | 62304 | 62305 | 62319 | 62321 | 62656 | 62657 | 62879 | 62880 | 63106 | 63107 | 63108 | 63109 | 63832