Bug 25516 - Item/pickup_locations wantarray removed, so dies on Perl >=5.24 where "autoderef" feature absent
Summary: Item/pickup_locations wantarray removed, so dies on Perl >=5.24 where "autode...
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Peter Vashchuk
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 24368
Blocks:
  Show dependency treegraph
 
Reported: 2020-05-15 15:10 UTC by Andrew Nugged
Modified: 2020-05-20 21:04 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 25516: Fix for "Can't call method unblessed on unblessed reference" (2.53 KB, patch)
2020-05-18 11:49 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 25516: Fix for "Can't call method unblessed on unblessed reference" (2.58 KB, patch)
2020-05-18 18:21 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 25516: Fix for "Can't call method unblessed on unblessed reference" (2.74 KB, patch)
2020-05-19 07:19 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Nugged 2020-05-15 15:10:19 UTC
After last updates, until around May 11, "wantarray" in sub pickup_locations was removed, but it might be hidden on Perls before v5.24, because there was autoderef experimental feature (https://www.effectiveperlprogramming.com/2010/11/use-array-references-with-the-array-operators/), the feature removed in Perl v5.24, and Ubuntu 16.04 has Perl v5.22, Ubuntu 18.04 has Perl v5.26.

Since newer Perls it will definitely produce the error because now it uses $arr_reference as a single item and maps over it once, and the process dies with "no method" or whatever. There might be other places in the code that uses this v5.14-5.24 feature for example to keys, map, grep $arr_reference.

The place I bumped in:


1. On the clean devenv after “reset_all” go to /cgi-bin/koha/admin/smart-rules.pl and press “Save” button on “Default checkout, hold and return policy” to save the empty record (GUI allows that), – this is needed to have in the table “circulation_rule” key “hold_fulfillment_policy” appeared with empty string value so code will reach the right place.

Another option to have the same “hold_fulfillment_policy” empty string is to reset DB and go through onboarding.pl (and how I found). So, after onboarding, you don’t ever need to save the empty record, it will be filled automatically.

To check:
    SELECT rule_name, rule_value 
    FROM circulation_rules WHERE rule_name='hold_fulfillment_policy'
it produces: 
    "hold_fulfillment_policy", ""


2. open item record /cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1 - it produced the bug:

Software error:
Can't call method "unblessed" on unblessed reference at ../reserve/request.pl line 581.


The code from request.pl line 581:
```
    $item->{pickup_locations} = join( ', ',
        map { $_->unblessed->{branchname} }
          Koha::Items->find($itemnumber)
          ->pickup_locations( { patron => $patron } ) );
    $item->{pickup_locations_code} = join( ',',
        map { $_->unblessed->{branchcode} }
          Koha::Items->find($itemnumber)
          ->pickup_locations( { patron => $patron } ) );
```

and that's because sub pickup_locations (recent changes, where wantarray removed, so might be more places) now does this:

    return \@pickup_locations;

Yet it might work on Perl <5.24.


Peter: please search through code where we might have similar issues,
and meanwhile, provide a patch for the current one above.
Comment 1 Jonathan Druart 2020-05-18 10:33:00 UTC
Are you planning on submitting a patch soon? I'd like 2. to be part of 20.05.
Comment 2 Peter Vashchuk 2020-05-18 11:49:00 UTC
Created attachment 105012 [details] [review]
Bug 25516: Fix for "Can't call method unblessed on unblessed reference"

Software error:
    Can't call method "unblessed" on unblessed reference at ../reserve/request.pl line 581.
was caused by recent commit with `wantarray` removal in sub pickup_locations in ‘Item.pm’
and visible on fresher Perls (where experimental feature "autoderef" removed https://www.effectiveperlprogramming.com/2010/11/use-array-references-with-the-array-operators/ )

To test:
    1) Get a clean dev environment after "reset_all"
    2) Add an empty record for “Default checkout, hold and return policy” on /cgi-bin/koha/admin/smart-rules.pl.
    3) Open item record, like /cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1
    4) Observe the error (Can't call method "unblessed" on unblessed reference at ../reserve/request.pl line 581.)
    5) Apply patch.
    6) Repeat steps 2 and 3.
    7) Observe that the error is gone.
Comment 3 Victor Grousset/tuxayo 2020-05-18 18:21:45 UTC
Created attachment 105040 [details] [review]
Bug 25516: Fix for "Can't call method unblessed on unblessed reference"

Software error:
    Can't call method "unblessed" on unblessed reference at ../reserve/request.pl line 581.
was caused by recent commit with `wantarray` removal in sub pickup_locations in ‘Item.pm’
and visible on fresher Perls (where experimental feature "autoderef" removed https://www.effectiveperlprogramming.com/2010/11/use-array-references-with-the-array-operators/ )

To test:
    1) Get a clean dev environment after "reset_all"
    2) Add an empty record for “Default checkout, hold and return policy” on /cgi-bin/koha/admin/smart-rules.pl.
    3) Open item record, like /cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1
    4) Observe the error (Can't call method "unblessed" on unblessed reference at ../reserve/request.pl line 581.)
    5) Apply patch.
    6) Repeat steps 2 and 3.
    7) Observe that the error is gone.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 4 Victor Grousset/tuxayo 2020-05-18 18:22:33 UTC
It works ! :D
Comment 5 Jonathan Druart 2020-05-19 07:19:38 UTC
Created attachment 105058 [details] [review]
Bug 25516: Fix for "Can't call method unblessed on unblessed reference"

Software error:
    Can't call method "unblessed" on unblessed reference at ../reserve/request.pl line 581.
was caused by recent commit with `wantarray` removal in sub pickup_locations in ‘Item.pm’
and visible on fresher Perls (where experimental feature "autoderef" removed https://www.effectiveperlprogramming.com/2010/11/use-array-references-with-the-array-operators/ )

To test:
    1) Get a clean dev environment after "reset_all"
    2) Add an empty record for “Default checkout, hold and return policy” on /cgi-bin/koha/admin/smart-rules.pl.
    3) Open item record, like /cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1
    4) Observe the error (Can't call method "unblessed" on unblessed reference at ../reserve/request.pl line 581.)
    5) Apply patch.
    6) Repeat steps 2 and 3.
    7) Observe that the error is gone.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended-patch: replace tab indentation with spaces
Comment 6 Jonathan Druart 2020-05-19 07:20:28 UTC
Peter, thanks for the patch!
Make sure you configure your editor correctly for this project, we use 4 spaces indentation, no tab ;)
I amended the patch.
Comment 7 Martin Renvoize 2020-05-19 07:30:36 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 8 Andrew Nugged 2020-05-19 08:55:56 UTC
(In reply to Jonathan Druart from comment #6)
> Make sure you configure your editor correctly for this project, we use 4 spaces indentation, no tab ;)

actually, git's config for 'whitespace' we all have: 'trailing-space,space-before-tab' to protect ourselves against occasional things like above, but it protects in space+tab, in another order, not tab+space, maybe we should try 'indent-with-non-tab,tab-in-indent' too...

But now we even added this in our REGEX in team's pre-commit hook now :P
Comment 9 Joy Nelson 2020-05-20 21:04:05 UTC
missing dependency - not backported to 19.11.x