From e6512fab69173f7eb1d3207351dfd901a7ca269e Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 26 Oct 2021 13:54:15 -0300 Subject: [PATCH] Bug 29325: [21.11.x] Fix commit_file.pl This patch fixes the broken commit_file.pl script. It doesn't deal with commiting the import from the UI. To test: 1. Pick a file for staging: $ kshell k$ misc/stage_file.pl --file TestDataImportKoha.mrc => SUCCESS: All good 2. Commit! k$ misc/commit_file.pl --batch-number 1 => FAIL: You see DBIx::Class::Storage::DBI::_exec_txn_begin(): DBI Exception: DBD::mysql::db begin_work failed: Already in a transaction at /kohadevbox/koha/C4/Biblio.pm line 303 3. Apply this patch 4. Repeat 2 => SUCCESS: Commit succeeds 5. Sign off :-D Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Bug 29325: (QA follow-up) Remove unexisting parameters of BatchRevertRecords There is no interval and callback as in BatchCommitRecords. Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Bug 29325: Call progress callback one last time to confirm comppletion Previously after finishing the loop we were still in a transaction that never completed - we should report progress when done one final time to commit the last records To test: 1 - Stage a file with > 100 records 2 - Commit file 3 - Confirm batch is imported and no records left as staged Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Bug 29325: Fix import from staff client same test as before, but via the staff client Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Bug 29325: Handle the transaction in BatchCommitRecords Requiring the callback to commit was breaking reversion, and likely elsewhere Let's simplify and say that the routine iteself will handle the txn and commit TO test: 1 - Stage a file 2 - Import a file 3 - Revert a file 4 - Test staff client and command line Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Signed-off-by: Marcel de Rooy Bug 29325: (QA follow-up) Tidy up tools/manage-marc-import.pl: sub commit_batch does no longer need $schema tools/manage-marc-import.pl: sub revert_batch: calling BatchRevertRecords which has no interval and callback misc/commit_file.pl: sub print_progress_and_commit does no longer commit, renamed misc/commit_file.pl: sub print_progress does no longer have a schema parameter misc/commit_file.pl: sub revert_batch reported deleted items as added Signed-off-by: Marcel de Rooy --- C4/ImportBatch.pm | 11 ++++++++++- misc/commit_file.pl | 18 +++++++----------- tools/manage-marc-import.pl | 33 ++++++++++++--------------------- 3 files changed, 29 insertions(+), 33 deletions(-) diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index 1bc4ab46da..920e970e89 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -569,6 +569,8 @@ sub BatchCommitRecords { my $batch_id = shift; my $framework = shift; + my $schema = Koha::Database->schema; + # optional callback to monitor status # of job my $progress_interval = 0; @@ -607,11 +609,17 @@ sub BatchCommitRecords { my $logged_in_patron = Koha::Patrons->find( $userenv->{number} ); my $rec_num = 0; + $schema->txn_begin; # We commit in a transaction while (my $rowref = $sth->fetchrow_hashref) { $record_type = $rowref->{'record_type'}; + $rec_num++; + if ($progress_interval and (0 == ($rec_num % $progress_interval))) { - &$progress_callback($rec_num); + # report progress and commit + $schema->txn_commit; + &$progress_callback( $rec_num ); + $schema->txn_begin; } if ($rowref->{'status'} eq 'error' or $rowref->{'status'} eq 'imported') { $num_ignored++; @@ -726,6 +734,7 @@ sub BatchCommitRecords { SetImportRecordStatus($rowref->{'import_record_id'}, 'ignored'); } } + $schema->txn_commit; # Commit final records that may not have hit callback threshold $sth->finish(); SetImportBatchStatus($batch_id, 'imported'); return ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored); diff --git a/misc/commit_file.pl b/misc/commit_file.pl index 543d7b06c7..d18dda68c4 100755 --- a/misc/commit_file.pl +++ b/misc/commit_file.pl @@ -1,7 +1,7 @@ #!/usr/bin/perl -use strict; -use warnings; +use Modern::Perl; + BEGIN { # find Koha's Perl modules # test carefully before changing this @@ -45,8 +45,6 @@ if ($list_batches) { # in future, probably should tie to a real user account C4::Context->set_userenv(0, 'batch', 0, 'batch', 'batch', 'batch', 'batch'); -my $dbh = C4::Context->dbh; -$dbh->{AutoCommit} = 0; if ($batch_number =~ /^\d+$/ and $batch_number > 0) { my $batch = GetImportBatch($batch_number); die "$0: import batch $batch_number does not exist in database\n" unless defined $batch; @@ -59,7 +57,6 @@ if ($batch_number =~ /^\d+$/ and $batch_number > 0) { unless $batch->{'import_status'} eq "staged" or $batch->{'import_status'} eq "reverted"; process_batch($batch_number); } - $dbh->commit(); } else { die "$0: please specify a numeric batch ID\n"; } @@ -86,7 +83,7 @@ sub process_batch { print "... importing MARC records -- please wait\n"; my ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored) = - BatchCommitRecords($import_batch_id, $framework, 100, \&print_progress_and_commit); + BatchCommitRecords($import_batch_id, $framework, 100, \&print_progress); print "... finished importing MARC records\n"; print <<_SUMMARY_; @@ -111,7 +108,7 @@ sub revert_batch { print "... reverting batch -- please wait\n"; my ($num_deleted, $num_errors, $num_reverted, $num_items_deleted, $num_ignored) = - BatchRevertRecords($import_batch_id, 100, \&print_progress_and_commit); + BatchRevertRecords( $import_batch_id ); print "... finished reverting batch\n"; print <<_SUMMARY_; @@ -123,16 +120,15 @@ Number of records deleted: $num_deleted Number of errors: $num_errors Number of records reverted: $num_reverted Number of records ignored: $num_ignored -Number of items added: $num_items_deleted +Number of items deleted: $num_items_deleted _SUMMARY_ } -sub print_progress_and_commit { - my $recs = shift; +sub print_progress { + my ( $recs ) = @_; print "... processed $recs records\n"; - $dbh->commit(); } sub print_usage { diff --git a/tools/manage-marc-import.pl b/tools/manage-marc-import.pl index 87aae2afd9..778ef47b9e 100755 --- a/tools/manage-marc-import.pl +++ b/tools/manage-marc-import.pl @@ -232,22 +232,17 @@ sub commit_batch { my $job = undef; my ( $num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored ); - my $schema = Koha::Database->new->schema; - $schema->storage->txn_do( - sub { - my $callback = sub { }; - if ($runinbackground) { - $job = put_in_background($import_batch_id); - $callback = progress_callback( $job, $dbh ); - } - ( - $num_added, $num_updated, $num_items_added, - $num_items_replaced, $num_items_errored, $num_ignored - ) - = BatchCommitRecords( $import_batch_id, $framework, 50, - $callback ); - } - ); + my $callback = sub { }; + if ($runinbackground) { + $job = put_in_background($import_batch_id); + $callback = progress_callback( $job ); + } + ( + $num_added, $num_updated, $num_items_added, + $num_items_replaced, $num_items_errored, $num_ignored + ) + = BatchCommitRecords( $import_batch_id, $framework, 50, + $callback ); my $results = { did_commit => 1, @@ -276,15 +271,13 @@ sub revert_batch { my $schema = Koha::Database->new->schema; $schema->txn_do( sub { - my $callback = sub { }; if ($runinbackground) { $job = put_in_background($import_batch_id); - $callback = progress_callback( $job, $dbh ); } ( $num_deleted, $num_errors, $num_reverted, $num_items_deleted, $num_ignored - ) = BatchRevertRecords( $import_batch_id, 50, $callback ); + ) = BatchRevertRecords( $import_batch_id ); } ); @@ -340,11 +333,9 @@ sub put_in_background { sub progress_callback { my $job = shift; - my $dbh = shift; return sub { my $progress = shift; $job->progress($progress); - $dbh->commit(); } } -- 2.30.2