[Bug 32894] New: Objects cache methods' result without invalidation
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Bug ID: 32894 Summary: Objects cache methods' result without invalidation Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: In Discussion Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl, martin.renvoize@ptfs-europe.com, tomascohen@gmail.com In some of our Koha:: objects we have methods that cache their result and return it in subsequent calls. However there is no invalidation of the cache if the object is modified. For instance, in Koha/ArticleRequest.pm sub biblio { my ($self) = @_; $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); return $self->{_biblio}; } This pattern exists in several places. It can lead to confusion and incorrect results, such as: use Koha::ArticleRequests; my $ar = Koha::ArticleRequest->new({ borrowernumber => 42, biblionumber => 42, })->store; say $ar->biblio->biblionumber; # Display 42, correct $ar->set({ biblionumber => 24 })->store; say $ar->biblio->biblionumber; # Display 42, wrong $ar->discard_changes; say $ar->biblio->biblionumber; # Display 42, wrong $ar->delete; We should remove those caching and rely on DBIC/DBMS caching mechanism instead. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- No Discussion here, is the status correct? :) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Katrin Fischer from comment #1)
No Discussion here, is the status correct? :)
Yes, it's still expecting feedback from other devs. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I actually thought we'd alr Ady made he decision to not cache like this. I've not been actively removing such caches but I have generally tried to avoid adding new ones for a while. There are some rare case where this style of caching does make sense to me.. though I I couldn't point you at one off the top of my head... The prevent public embed fetching stuff in the API perhaps? Either way, do we think it's time to start retrospectively cleaning up the existing cases? It could make a good hackfest project to bike them all together. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 147499 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147499&action=edit Bug 32894: Remove wrong caching from Koha:: methods - simple In some of our Koha:: objects we have methods that cache their result and return it in subsequent calls. However there is no invalidation of the cache if the object is modified. For instance, in Koha/ArticleRequest.pm sub biblio { my ($self) = @_; $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); return $self->{_biblio}; } This pattern exists in several places. It can lead to confusion and incorrect results, such as: use Koha::ArticleRequests; my $ar = Koha::ArticleRequest->new({ borrowernumber => 42, biblionumber => 42, })->store; say $ar->biblio->biblionumber; # Display 42, correct $ar->set({ biblionumber => 24 })->store; say $ar->biblio->biblionumber; # Display 42, wrong $ar->discard_changes; say $ar->biblio->biblionumber; # Display 42, wrong $ar->delete; We should remove those caching and rely on DBIC/DBMS caching mechanism instead. This patch is adjusting the trivial occurrences -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 147500 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147500&action=edit Bug 32894: Koha::Item->last_returned_by -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 147502 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147502&action=edit Bug 32894: Koha::Biblio->biblioitem Can we do better here? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 147503 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147503&action=edit Bug 32894: Fix test -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Those patches deal with most of the cases. What about Koha::Item::_bundle_items_cached and Koha::Objects? I would not touch Koha::Objects behaviour here, maybe on a follow-up bug. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Martin, could you have a look at Koha::Item::_bundle_items_cached? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Yes, sorry.. I've just not had a moment to breath yet.. This is certainly still near the top of my list. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |ASSIGNED Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147499|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150016 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150016&action=edit Bug 32894: Remove wrong caching from Koha:: methods - simple In some of our Koha:: objects we have methods that cache their result and return it in subsequent calls. However there is no invalidation of the cache if the object is modified. For instance, in Koha/ArticleRequest.pm sub biblio { my ($self) = @_; $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); return $self->{_biblio}; } This pattern exists in several places. It can lead to confusion and incorrect results, such as: use Koha::ArticleRequests; my $ar = Koha::ArticleRequest->new({ borrowernumber => 42, biblionumber => 42, })->store; say $ar->biblio->biblionumber; # Display 42, correct $ar->set({ biblionumber => 24 })->store; say $ar->biblio->biblionumber; # Display 42, wrong $ar->discard_changes; say $ar->biblio->biblionumber; # Display 42, wrong $ar->delete; We should remove those caching and rely on DBIC/DBMS caching mechanism instead. This patch is adjusting the trivial occurrences Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147500|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150017 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150017&action=edit Bug 32894: Koha::Item->last_returned_by Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147502|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150018 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150018&action=edit Bug 32894: Koha::Biblio->biblioitem Can we do better here? Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147503|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150019 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150019&action=edit Bug 32894: Fix test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150020&action=edit Bug 32894: Remove incorrect caching from bundle_items Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150021 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150021&action=edit Bug 32894: (QA follow-up) Fix typo Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Added a tiny typo fix and also a patch for item_bundles. I agree.. lets to Koha::Objects as it's own bug. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Signed Off |Failed QA --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Something is not right with Koha::Item::last_returned_by It returns a patron object, but the relationship is to ItemsLastBorrower and the call to set it doesn't work prove -v t/db_dependent/Circulation/StoreLastBorrower.t Also Koha/Virtualshelfshare.pm : everywhere else we assign resultset to rs, then create new - it isn't wrong, just inconsistent -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32476 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150827 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150827&action=edit Bug 32894: (follow-up) Koha::Item->last_returned_by -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150828&action=edit Bug 32894: Make ->sharee looks like others -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off Assignee|martin.renvoize@ptfs-europe |jonathan.druart+koha@gmail. |.com |com --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Nick Clemens from comment #18)
Something is not right with Koha::Item::last_returned_by
It returns a patron object, but the relationship is to ItemsLastBorrower and the call to set it doesn't work
prove -v t/db_dependent/Circulation/StoreLastBorrower.t
Oops! Thanks!
Also Koha/Virtualshelfshare.pm : everywhere else we assign resultset to rs, then create new - it isn't wrong, just inconsistent
Done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150016|0 |1 is obsolete| | Attachment #150017|0 |1 is obsolete| | Attachment #150018|0 |1 is obsolete| | Attachment #150019|0 |1 is obsolete| | Attachment #150020|0 |1 is obsolete| | Attachment #150021|0 |1 is obsolete| | Attachment #150827|0 |1 is obsolete| | Attachment #150828|0 |1 is obsolete| | --- Comment #22 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150847&action=edit Bug 32894: Remove wrong caching from Koha:: methods - simple In some of our Koha:: objects we have methods that cache their result and return it in subsequent calls. However there is no invalidation of the cache if the object is modified. For instance, in Koha/ArticleRequest.pm sub biblio { my ($self) = @_; $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); return $self->{_biblio}; } This pattern exists in several places. It can lead to confusion and incorrect results, such as: use Koha::ArticleRequests; my $ar = Koha::ArticleRequest->new({ borrowernumber => 42, biblionumber => 42, })->store; say $ar->biblio->biblionumber; # Display 42, correct $ar->set({ biblionumber => 24 })->store; say $ar->biblio->biblionumber; # Display 42, wrong $ar->discard_changes; say $ar->biblio->biblionumber; # Display 42, wrong $ar->delete; We should remove those caching and rely on DBIC/DBMS caching mechanism instead. This patch is adjusting the trivial occurrences Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #23 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150848&action=edit Bug 32894: Koha::Item->last_returned_by Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #24 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150849&action=edit Bug 32894: Koha::Biblio->biblioitem Can we do better here? Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #25 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150850&action=edit Bug 32894: Fix test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #26 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150851 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150851&action=edit Bug 32894: Remove incorrect caching from bundle_items Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #27 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150852 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150852&action=edit Bug 32894: (QA follow-up) Fix typo Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #28 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150853&action=edit Bug 32894: (follow-up) Koha::Item->last_returned_by Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #29 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150854 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150854&action=edit Bug 32894: Make ->sharee looks like others Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #30 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 150855 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150855&action=edit Bug 32894: (QA follow-up) Fix tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #31 from Nick Clemens <nick@bywatersolutions.com> --- Looks good, t/db_dependent all pass now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #32 from Kyle M Hall <kyle@bywatersolutions.com> --- I do not think this is a great idea long term. This is moving backwards when it comes to efficiency and will introduce far more database calls. It seems like it might be better to fix the code that is making a bad assumption. The short term fix is to use discard_changes when needed. The long term fix is to use Koha objects from the perl scripts and pass them to subroutines so we have no need to re-fetch data from the database. Have you identified any bugs being caused by this, or it just a possibility as you've shown? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #33 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Kyle M Hall from comment #32)
I do not think this is a great idea long term. This is moving backwards when it comes to efficiency and will introduce far more database calls. It seems like it might be better to fix the code that is making a bad assumption. The short term fix is to use discard_changes when needed. The long term fix is to use Koha objects from the perl scripts and pass them to subroutines so we have no need to re-fetch data from the database.
Have you identified any bugs being caused by this, or it just a possibility as you've shown?
This is a bad pattern, we should get rid of it and prevent it from propagating over the codebase. The correct way to fix the refetch is patch like bug 32496 and bug 31735. There is also bug 32476 that can be helpful to cache *calculated* values when specific performance problems are identified. Here we are caching only 1 DB call that is already cached by an underlying mechanism anyway (DBMS or DBIC).
It seems like it might be better to fix the code that is making a bad assumption.
No need to make bad assumption to get incorrect results, see comment 0. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_23_11_candidate --- Comment #34 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Can be backported. Let's push early next cycle. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #35 from Kyle M Hall <kyle@bywatersolutions.com> --- reset_all && multitime -n 10 prove t/db_dependent/Koha Just Master: 1: prove t/db_dependent/Koha Mean Std.Dev. Min Median Max real 129.754 3.631 121.324 130.872 134.770 user 107.146 2.759 100.859 108.025 111.129 sys 17.488 0.720 15.979 17.469 18.793 Master + 32890: 1: prove t/db_dependent/Koha Mean Std.Dev. Min Median Max real 128.499 4.333 123.117 127.620 139.403 user 106.268 3.456 102.384 105.515 114.984 sys 17.237 0.992 15.540 17.337 19.357 I'm no statistician so take from this what you will. I think it's generally positive. Lower average with the patches but a higher standard deviation. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply --- Comment #36 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Please rebase. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150847|0 |1 is obsolete| | Attachment #150848|0 |1 is obsolete| | Attachment #150849|0 |1 is obsolete| | Attachment #150850|0 |1 is obsolete| | Attachment #150851|0 |1 is obsolete| | Attachment #150852|0 |1 is obsolete| | Attachment #150853|0 |1 is obsolete| | Attachment #150854|0 |1 is obsolete| | Attachment #150855|0 |1 is obsolete| | --- Comment #37 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152086&action=edit Bug 32894: Remove wrong caching from Koha:: methods - simple In some of our Koha:: objects we have methods that cache their result and return it in subsequent calls. However there is no invalidation of the cache if the object is modified. For instance, in Koha/ArticleRequest.pm sub biblio { my ($self) = @_; $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); return $self->{_biblio}; } This pattern exists in several places. It can lead to confusion and incorrect results, such as: use Koha::ArticleRequests; my $ar = Koha::ArticleRequest->new({ borrowernumber => 42, biblionumber => 42, })->store; say $ar->biblio->biblionumber; # Display 42, correct $ar->set({ biblionumber => 24 })->store; say $ar->biblio->biblionumber; # Display 42, wrong $ar->discard_changes; say $ar->biblio->biblionumber; # Display 42, wrong $ar->delete; We should remove those caching and rely on DBIC/DBMS caching mechanism instead. This patch is adjusting the trivial occurrences Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #38 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152087 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152087&action=edit Bug 32894: Koha::Item->last_returned_by Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152088 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152088&action=edit Bug 32894: Koha::Biblio->biblioitem Can we do better here? Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #40 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152089 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152089&action=edit Bug 32894: Fix test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #41 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152090 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152090&action=edit Bug 32894: Remove incorrect caching from bundle_items Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #42 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152091 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152091&action=edit Bug 32894: (QA follow-up) Fix typo Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #43 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152092 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152092&action=edit Bug 32894: (follow-up) Koha::Item->last_returned_by Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #44 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152093 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152093&action=edit Bug 32894: Make ->sharee looks like others Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #45 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152094 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152094&action=edit Bug 32894: (QA follow-up) Fix tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #46 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00 |23.11.00,23.05.02 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for all the hard work! Pushed to 23.05.x for the next release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00,23.05.02 |23.11.00,23.05.02,22.11.08 released in| | Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #48 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34603 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34603 [Bug 34603] Deleted biblio causes holdshistory.tt to explode -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34609 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34609 [Bug 34609] Holds history errors 500 if old_reserves.biblionumber is NULL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32894 --- Comment #49 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Stop backporting this. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org