From d6c5fd3625e02683dab724e7e13b4e04047d59ec Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 25 Feb 2022 07:00:23 -0300 Subject: [PATCH] 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 Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Kyle M Hall Signed-off-by: Fridolin Somers --- Koha/BackgroundJob.pm | 4 +-- Koha/BackgroundJob/BatchCancelHold.pm | 20 ++++++--------- Koha/BackgroundJob/BatchDeleteAuthority.pm | 21 ++++++---------- Koha/BackgroundJob/BatchDeleteBiblio.pm | 29 +++++++++------------- Koha/BackgroundJob/BatchDeleteItem.pm | 21 ++++++---------- Koha/BackgroundJob/BatchUpdateAuthority.pm | 16 ++++++------ Koha/BackgroundJob/BatchUpdateBiblio.pm | 19 ++++++-------- Koha/BackgroundJob/BatchUpdateItem.pm | 23 +++++++---------- 8 files changed, 62 insertions(+), 91 deletions(-) diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index 2a53702195..b2d61291d0 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -155,7 +155,7 @@ sub process { $args ||= {}; - return $derived_class->process({job_id => $self->id, %$args}); + return $derived_class->process( $args ); } =head3 job_type @@ -210,7 +210,7 @@ sub additional_report { my $derived_class = $self->_derived_class; - return $derived_class->additional_report({job_id => $self->id}); + return $derived_class->additional_report; } =head3 cancel diff --git a/Koha/BackgroundJob/BatchCancelHold.pm b/Koha/BackgroundJob/BatchCancelHold.pm index 5216119ac1..ba85cfc63d 100644 --- a/Koha/BackgroundJob/BatchCancelHold.pm +++ b/Koha/BackgroundJob/BatchCancelHold.pm @@ -18,7 +18,6 @@ package Koha::BackgroundJob::BatchCancelHold; use Modern::Perl; use JSON qw( encode_json decode_json ); -use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); use Koha::Holds; use Koha::Patrons; @@ -55,14 +54,12 @@ Process the modification. sub process { my ( $self, $args ) = @_; - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + if ( $self->status eq 'cancelled' ) { return; } my $job_progress = 0; - $job->started_on(dt_from_string)->progress($job_progress) + $self->started_on(dt_from_string)->progress($job_progress) ->status('started')->store; my @hold_ids = @{ $args->{hold_ids} }; @@ -109,16 +106,16 @@ sub process { }; $report->{total_success}++; } - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; } - my $job_data = decode_json $job->data; + my $job_data = decode_json $self->data; $job_data->{messages} = \@messages; $job_data->{report} = $report; - $job->ended_on(dt_from_string)->data( encode_json $job_data); - $job->status('finished') if $job->status ne 'cancelled'; - $job->store; + $self->ended_on(dt_from_string)->data( encode_json $job_data); + $self->status('finished') if $self->status ne 'cancelled'; + $self->store; } @@ -153,8 +150,7 @@ Pass the biblio's title and patron's name sub additional_report { my ( $self, $args ) = @_; - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - my $messages = $job->messages; + my $messages = $self->messages; for my $m ( @$messages ) { $m->{patron} = Koha::Patrons->find($m->{patron_id}); $m->{biblio} = Koha::Biblios->find($m->{biblio_id}); diff --git a/Koha/BackgroundJob/BatchDeleteAuthority.pm b/Koha/BackgroundJob/BatchDeleteAuthority.pm index 99833abcd6..0f2151976f 100644 --- a/Koha/BackgroundJob/BatchDeleteAuthority.pm +++ b/Koha/BackgroundJob/BatchDeleteAuthority.pm @@ -3,7 +3,6 @@ package Koha::BackgroundJob::BatchDeleteAuthority; use Modern::Perl; use JSON qw( encode_json decode_json ); -use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); use C4::AuthoritiesMarc; @@ -16,11 +15,7 @@ sub job_type { sub process { my ( $self, $args ) = @_; - my $job_type = $args->{job_type}; - - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + if ( $self->status eq 'cancelled' ) { return; } @@ -28,7 +23,7 @@ sub process { # Then we will start from scratch and so double delete the same records my $job_progress = 0; - $job->started_on(dt_from_string) + $self->started_on(dt_from_string) ->progress($job_progress) ->status('started') ->store; @@ -44,7 +39,7 @@ sub process { my $schema = Koha::Database->new->schema; RECORD_IDS: for my $record_id ( sort { $a <=> $b } @record_ids ) { - last if $job->get_from_storage->status eq 'cancelled'; + last if $self->get_from_storage->status eq 'cancelled'; next unless $record_id; @@ -71,17 +66,17 @@ sub process { $schema->storage->txn_commit; } - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; } - my $job_data = decode_json $job->data; + my $job_data = decode_json $self->data; $job_data->{messages} = \@messages; $job_data->{report} = $report; - $job->ended_on(dt_from_string) + $self->ended_on(dt_from_string) ->data(encode_json $job_data); - $job->status('finished') if $job->status ne 'cancelled'; - $job->store; + $self->status('finished') if $self->status ne 'cancelled'; + $self->store; } sub enqueue { diff --git a/Koha/BackgroundJob/BatchDeleteBiblio.pm b/Koha/BackgroundJob/BatchDeleteBiblio.pm index 8c94720ac1..15246feae0 100644 --- a/Koha/BackgroundJob/BatchDeleteBiblio.pm +++ b/Koha/BackgroundJob/BatchDeleteBiblio.pm @@ -3,7 +3,6 @@ package Koha::BackgroundJob::BatchDeleteBiblio; use Modern::Perl; use JSON qw( encode_json decode_json ); -use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); use C4::Biblio; @@ -38,11 +37,7 @@ Process the job. sub process { my ( $self, $args ) = @_; - my $job_type = $args->{job_type}; - - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + if ( $self->status eq 'cancelled' ) { return; } @@ -50,7 +45,7 @@ sub process { # Then we will start from scratch and so double delete the same records my $job_progress = 0; - $job->started_on(dt_from_string) + $self->started_on(dt_from_string) ->progress($job_progress) ->status('started') ->store; @@ -66,7 +61,7 @@ sub process { my $schema = Koha::Database->new->schema; RECORD_IDS: for my $record_id ( sort { $a <=> $b } @record_ids ) { - last if $job->get_from_storage->status eq 'cancelled'; + last if $self->get_from_storage->status eq 'cancelled'; next unless $record_id; @@ -85,7 +80,7 @@ sub process { biblionumber => $biblionumber, }; $schema->storage->txn_rollback; - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; next; } @@ -104,7 +99,7 @@ sub process { error => "$@", }; $schema->storage->txn_rollback; - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; next RECORD_IDS; } } @@ -122,7 +117,7 @@ sub process { error => @{$deleted->messages}[0]->message, }; $schema->storage->txn_rollback; - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; next RECORD_IDS; } } @@ -139,7 +134,7 @@ sub process { error => ($@ ? $@ : $error), }; $schema->storage->txn_rollback; - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; next; } @@ -150,17 +145,17 @@ sub process { }; $report->{total_success}++; $schema->storage->txn_commit; - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; } - my $job_data = decode_json $job->data; + my $job_data = decode_json $self->data; $job_data->{messages} = \@messages; $job_data->{report} = $report; - $job->ended_on(dt_from_string) + $self->ended_on(dt_from_string) ->data(encode_json $job_data); - $job->status('finished') if $job->status ne 'cancelled'; - $job->store; + $self->status('finished') if $self->status ne 'cancelled'; + $self->store; } =head3 enqueue diff --git a/Koha/BackgroundJob/BatchDeleteItem.pm b/Koha/BackgroundJob/BatchDeleteItem.pm index b5147e7829..b4bcbfa6a8 100644 --- a/Koha/BackgroundJob/BatchDeleteItem.pm +++ b/Koha/BackgroundJob/BatchDeleteItem.pm @@ -26,7 +26,6 @@ use JSON qw( encode_json decode_json ); use List::MoreUtils qw( uniq ); use Try::Tiny; -use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); use Koha::Items; @@ -75,11 +74,7 @@ The generated report will be: sub process { my ( $self, $args ) = @_; - my $job_type = $args->{job_type}; - - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + if ( $self->status eq 'cancelled' ) { return; } @@ -87,7 +82,7 @@ sub process { # Then we will start from scratch and so double delete the same records my $job_progress = 0; - $job->started_on(dt_from_string)->progress($job_progress) + $self->started_on(dt_from_string)->progress($job_progress) ->status('started')->store; my @record_ids = @{ $args->{record_ids} }; @@ -109,7 +104,7 @@ sub process { my (@biblionumbers); for my $record_id ( sort { $a <=> $b } @record_ids ) { - last if $job->get_from_storage->status eq 'cancelled'; + last if $self->get_from_storage->status eq 'cancelled'; my $item = Koha::Items->find($record_id) || next; @@ -136,7 +131,7 @@ sub process { push @biblionumbers, $item->biblionumber; $report->{total_success}++; - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; } # If there are no items left, delete the biblio @@ -183,13 +178,13 @@ sub process { $report->{not_deleted_itemnumbers} = \@not_deleted_itemnumbers; $report->{deleted_biblionumbers} = \@deleted_biblionumbers; - my $job_data = decode_json $job->data; + my $job_data = decode_json $self->data; $job_data->{messages} = \@messages; $job_data->{report} = $report; - $job->ended_on(dt_from_string)->data( encode_json $job_data); - $job->status('finished') if $job->status ne 'cancelled'; - $job->store; + $self->ended_on(dt_from_string)->data( encode_json $job_data); + $self->status('finished') if $self->status ne 'cancelled'; + $self->store; } =head3 enqueue diff --git a/Koha/BackgroundJob/BatchUpdateAuthority.pm b/Koha/BackgroundJob/BatchUpdateAuthority.pm index 9eacf58a5f..482acf892c 100644 --- a/Koha/BackgroundJob/BatchUpdateAuthority.pm +++ b/Koha/BackgroundJob/BatchUpdateAuthority.pm @@ -55,14 +55,12 @@ Process the modification. sub process { my ( $self, $args ) = @_; - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + if ( $self->status eq 'cancelled' ) { return; } my $job_progress = 0; - $job->started_on(dt_from_string) + $self->started_on(dt_from_string) ->progress($job_progress) ->status('started') ->store; @@ -100,17 +98,17 @@ sub process { }; $report->{total_success}++; } - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; } - my $job_data = decode_json $job->data; + my $job_data = decode_json $self->data; $job_data->{messages} = \@messages; $job_data->{report} = $report; - $job->ended_on(dt_from_string) + $self->ended_on(dt_from_string) ->data(encode_json $job_data); - $job->status('finished') if $job->status ne 'cancelled'; - $job->store; + $self->status('finished') if $self->status ne 'cancelled'; + $self->store; } diff --git a/Koha/BackgroundJob/BatchUpdateBiblio.pm b/Koha/BackgroundJob/BatchUpdateBiblio.pm index 480928c41d..21e938f191 100644 --- a/Koha/BackgroundJob/BatchUpdateBiblio.pm +++ b/Koha/BackgroundJob/BatchUpdateBiblio.pm @@ -18,7 +18,6 @@ package Koha::BackgroundJob::BatchUpdateBiblio; use Modern::Perl; use JSON qw( decode_json encode_json ); -use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); use Koha::Virtualshelves; @@ -57,9 +56,7 @@ Process the modification. sub process { my ( $self, $args ) = @_; - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + if ( $self->status eq 'cancelled' ) { return; } @@ -67,7 +64,7 @@ sub process { # Then we will start from scratch and so double process the same records my $job_progress = 0; - $job->started_on(dt_from_string) + $self->started_on(dt_from_string) ->progress($job_progress) ->status('started') ->store; @@ -82,7 +79,7 @@ sub process { my @messages; RECORD_IDS: for my $biblionumber ( sort { $a <=> $b } @record_ids ) { - last if $job->get_from_storage->status eq 'cancelled'; + last if $self->get_from_storage->status eq 'cancelled'; next unless $biblionumber; @@ -110,17 +107,17 @@ sub process { }; $report->{total_success}++; } - $job->progress( ++$job_progress )->store; + $self->progress( ++$job_progress )->store; } - my $job_data = decode_json $job->data; + my $job_data = decode_json $self->data; $job_data->{messages} = \@messages; $job_data->{report} = $report; - $job->ended_on(dt_from_string) + $self->ended_on(dt_from_string) ->data(encode_json $job_data); - $job->status('finished') if $job->status ne 'cancelled'; - $job->store; + $self->status('finished') if $self->status ne 'cancelled'; + $self->store; } =head3 enqueue diff --git a/Koha/BackgroundJob/BatchUpdateItem.pm b/Koha/BackgroundJob/BatchUpdateItem.pm index fb50893991..7e7ae38fb2 100644 --- a/Koha/BackgroundJob/BatchUpdateItem.pm +++ b/Koha/BackgroundJob/BatchUpdateItem.pm @@ -27,7 +27,6 @@ use MARC::Field; use C4::Biblio; use C4::Items; -use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); use Koha::SearchEngine::Indexer; use Koha::Items; @@ -85,9 +84,7 @@ regex_mod allows to modify existing subfield's values using a regular expression sub process { my ( $self, $args ) = @_; - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + if ( $self->status eq 'cancelled' ) { return; } @@ -95,7 +92,7 @@ sub process { # Then we will start from scratch and so double process the same records my $job_progress = 0; - $job->started_on(dt_from_string)->progress($job_progress) + $self->started_on(dt_from_string)->progress($job_progress) ->status('started')->store; my @record_ids = @{ $args->{record_ids} }; @@ -120,7 +117,7 @@ sub process { $exclude_from_local_holds_priority, callback => sub { my ($progress) = @_; - $job->progress($progress)->store; + $self->progress($progress)->store; }, } ); @@ -133,13 +130,13 @@ sub process { if ( $_ =~ /Rollback failed/ ); # Rollback failed }; - $job->discard_changes; - my $job_data = decode_json encode_utf8 $job->data; + $self->discard_changes; + my $job_data = decode_json encode_utf8 $self->data; $job_data->{report} = $report; - $job->ended_on(dt_from_string)->data( encode_json $job_data); - $job->status('finished') if $job->status ne 'cancelled'; - $job->store; + $self->ended_on(dt_from_string)->data( encode_json $job_data); + $self->status('finished') if $self->status ne 'cancelled'; + $self->store; } =head3 enqueue @@ -173,9 +170,7 @@ Sent the infos to generate the table containing the details of the modified item sub additional_report { my ( $self, $args ) = @_; - my $job = Koha::BackgroundJobs->find( $args->{job_id} ); - - my $itemnumbers = $job->report->{modified_itemnumbers}; + my $itemnumbers = $self->report->{modified_itemnumbers}; if ( scalar(@$itemnumbers) > C4::Context->preference('MaxItemsToDisplayForBatchMod') ) { return { too_many_items_display => 1 }; } else { -- 2.39.5