If overriding a accessor method in a child class of Koha::Object, and calling parent by invoking $self->SUPER->accessor_name, $AUTOLOAD in the AUTOLOAD method of KOHA::Object will be assigned the value Koha::ChildClass::SUPER::accessor_name instead of Koha::ChildClass::accessor_name. The next time $self->SUPER->accessor_name is called it will not be picked up, and AUTOLOAD will run again with a SubroutineKoha::ChildClass::SUPER::accessor_name redefined" warning, so obviously definition of lazy accessors does not work for those cases. I tried digging around to see if there is some way of defining autoloaded accessors that handles this case, but unfortunately did not find any relevant information on this. The only, probably safe, workaround is to not insert an entry into the dispatch table when $AUTOLOAD contains '::SUPER::', so until we have a better solution I think that probably is the way to go.
Created attachment 157678 [details] [review] Bug 35133: Don't lazily define accessor methods when invoked through SUPER
I will attach a test case tomorrow, it's a shame it seems so hard to properly implement lazy accessors without resorting to this ugly fix. If someone can come up with some other suggestions that would be great.
It would be a good idea to link to the previous dev via "Depends on" or at least "See Also"
Created attachment 157793 [details] [review] Bug 35133: Don't lazily define accessor methods when invoked through SUPER To test: 1) Apply Bug 32476 2) Run tests in t/db_dependent/Patrons.t 3) This should produce multiple "Subroutine Koha::Patron::SUPER::dateexpiry redefined at ..." warnings 4) Apply patch 5) Run tests in t/db_dependent/Patrons.t again 6) The warnings in 3) should now no longer appear
Created attachment 160777 [details] [review] Bug 35133: Don't lazily define accessor methods when invoked through SUPER To test: 1) Apply Bug 32476 2) Run tests in t/db_dependent/Patrons.t 3) This should produce multiple "Subroutine Koha::Patron::SUPER::dateexpiry redefined at ..." warnings 4) Apply patch 5) Run tests in t/db_dependent/Patrons.t again 6) The warnings in 3) should now no longer appear Signed-off-by: matthias le gac <matthias@babel.inlibro.net>
Created attachment 160778 [details] [review] Bug 35133: Don't lazily define accessor methods when invoked through SUPER To test: 1) Apply Bug 32476 2) Run tests in t/db_dependent/Patrons.t 3) This should produce multiple "Subroutine Koha::Patron::SUPER::dateexpiry redefined at ..." warnings 4) Apply patch 5) Run tests in t/db_dependent/Patrons.t again 6) The warnings in 3) should now no longer appear Signed-off-by: matthias le gac <matthias@babel.inlibro.net> Signed-off-by: Phan Tung Bui <phan-tung.bui@inlibro.com> Phan Tung Bui
Created attachment 161152 [details] [review] Bug 35133: Don't lazily define accessor methods when invoked through SUPER To test: 1) Apply Bug 32476 2) Run tests in t/db_dependent/Patrons.t 3) This should produce multiple "Subroutine Koha::Patron::SUPER::dateexpiry redefined at ..." warnings 4) Apply patch 5) Run tests in t/db_dependent/Patrons.t again 6) The warnings in 3) should now no longer appear Signed-off-by: matthias le gac <matthias@babel.inlibro.net> Signed-off-by: Phan Tung Bui <phan-tung.bui@inlibro.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Looks like this has been a bug in Perl since version 5.18 - https://www.perlmonks.org/?node_id=1171033 It makes sense to me to avoid adding the method reference to the symbol table when handling a SUPER call, since the reference will not work and the next call will fall back to AUTOLOAD anyway. This shouldn't have any side effects, and it passes the QA checks. Thanks for including a clear explanation in the comments, too!
Pushed for 24.05! Well done everyone, thank you!
Not backported to 23.11.x
No changes required to the manual.