[Bug 30181] New: Koha::BackgroundJob->_derived_class returns an empty object
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Bug ID: 30181 Summary: Koha::BackgroundJob->_derived_class returns an empty object Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org If you look at Koha::BackgroundJob->process, you will notice some funky stuff. After re-reading how background_jobs_worker.pl does its job, you notice that the main loop can only deal with Koha::BackgrounJob objects (i.e. not Koha::BackgroundJob::BatchUpdateItem). This is because it either gets a job id (STOMP use case) or it uses a Koha::BackgroundJobs->search to find jobs. So, when a job is found (say $job), the worker eventually calls $job->process. At that point, it needs to instantiate the 'real' class to properly call its overridden ->process method. When it does so, it creates a new/empty object, that is passed... the id. So, devs implementing background jobs need to take care of retrieving the job object so they are able to update the job status. This is not cool. -- 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=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- 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=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com, | |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=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131105 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131105&action=edit Bug 30181: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131106 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131106&action=edit Bug 30181: Make Koha::BackgroundJob->_derived_class return the right thing This patch makes _derived_class rely on _new_from_dbic to generate a new object with the right class, but based on the same DB row. Not an empty one as it was before. This way we can remove some biolerplate that is required now when writing background job classes. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/BackgroundJob.t => FAIL: Boo, tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131107 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131107&action=edit Bug 30181: (follow-up) Remove redundant queries Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Missing changes in t/lib/Koha/BackgroundJob/BatchTest.pm -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 158 return $derived_class->process({job_id => $self->id, %$args}); 213 return $derived_class->additional_report({job_id => $self->id}); We do no longer need to pass job_id -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131105|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131106|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131107|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131322 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131322&action=edit Bug 30181: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131323 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131323&action=edit Bug 30181: Make Koha::BackgroundJob->_derived_class return the right thing This patch makes _derived_class rely on _new_from_dbic to generate a new object with the right class, but based on the same DB row. Not an empty one as it was before. This way we can remove some biolerplate that is required now when writing background job classes. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/BackgroundJob.t => FAIL: Boo, tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131324 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131324&action=edit Bug 30181: (follow-up) Remove redundant queries and parameters Now $self is actually an instance of the job class, there's no need to have the job_id parameter passed, or the have the ->process method re-fetch the object from the database. This patch cleans things up. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131325 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131325&action=edit Bug 30181: (QA follow-up) Update BatchTest This patch updates t::lib::Koha::BackgroundJob::BatchTest to the new style, and also removes a couple stray cases in which job_id was still passed as a parameter. Tests are rewritten a bit, so they actually test more of the behaviors. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/BackgroundJobs.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Thanks for the valuable feedback, Jonathan. I have submitted a revised patchset. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29346 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29346 [Bug 29346] Add option for building the holds queue for a single record in background -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131322|0 |1 is obsolete| | --- Comment #11 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 131799 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131799&action=edit Bug 30181: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131323|0 |1 is obsolete| | --- Comment #12 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 131800 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131800&action=edit Bug 30181: Make Koha::BackgroundJob->_derived_class return the right thing This patch makes _derived_class rely on _new_from_dbic to generate a new object with the right class, but based on the same DB row. Not an empty one as it was before. This way we can remove some biolerplate that is required now when writing background job classes. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/BackgroundJob.t => FAIL: Boo, tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131324|0 |1 is obsolete| | --- Comment #13 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 131801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131801&action=edit Bug 30181: (follow-up) Remove redundant queries and parameters Now $self is actually an instance of the job class, there's no need to have the job_id parameter passed, or the have the ->process method re-fetch the object from the database. This patch cleans things up. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131325|0 |1 is obsolete| | --- Comment #14 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 131802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131802&action=edit Bug 30181: (QA follow-up) Update BatchTest This patch updates t::lib::Koha::BackgroundJob::BatchTest to the new style, and also removes a couple stray cases in which job_id was still passed as a parameter. Tests are rewritten a bit, so they actually test more of the behaviors. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/BackgroundJobs.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #15 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Note that the test for the regression tests patch is BackgroundJob.t (singular) while the test for the QA follow-up patch is BackgroundJobs.t (plural). My own inattentive reading confused me here for a bit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Kyle M Hall <kyle@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=30181 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131799|0 |1 is obsolete| | Attachment #131800|0 |1 is obsolete| | Attachment #131801|0 |1 is obsolete| | Attachment #131802|0 |1 is obsolete| | --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 131845 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131845&action=edit Bug 30181: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 131846 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131846&action=edit Bug 30181: Make Koha::BackgroundJob->_derived_class return the right thing This patch makes _derived_class rely on _new_from_dbic to generate a new object with the right class, but based on the same DB row. Not an empty one as it was before. This way we can remove some biolerplate that is required now when writing background job classes. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/BackgroundJob.t => FAIL: Boo, tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 131847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131847&action=edit Bug 30181: (follow-up) Remove redundant queries and parameters Now $self is actually an instance of the job class, there's no need to have the job_id parameter passed, or the have the ->process method re-fetch the object from the database. This patch cleans things up. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 131848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131848&action=edit Bug 30181: (QA follow-up) Update BatchTest This patch updates t::lib::Koha::BackgroundJob::BatchTest to the new style, and also removes a couple stray cases in which job_id was still passed as a parameter. Tests are rewritten a bit, so they actually test more of the behaviors. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/BackgroundJobs.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 --- Comment #20 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30181 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_21_11_candidate -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org