Bug 29325: 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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 29325: (QA follow-up) Remove unexisting parameters of BatchRevertRecords

There is no interval and callback as in BatchCommitRecords.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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 <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 29325: Fix import from staff client

same test as before, but via the staff client

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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 <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm
index 8295c93..cb0b643 100644 (file)
--- a/C4/ImportBatch.pm
+++ b/C4/ImportBatch.pm
@@ -542,6 +542,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;
@@ -581,11 +583,17 @@ sub BatchCommitRecords {
 
     my $rec_num = 0;
     my @biblio_ids;
+    $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++;
@@ -710,6 +718,7 @@ sub BatchCommitRecords {
             SetImportRecordStatus($rowref->{'import_record_id'}, 'ignored');
         }
     }
+    $schema->txn_commit; # Commit final records that may not have hit callback threshold
     $sth->finish();
 
     if ( @biblio_ids ) {

diff --git a/misc/commit_file.pl b/misc/commit_file.pl
index 1e1dd3f..da504d2 100755 (executable)
--- a/misc/commit_file.pl
+++ b/misc/commit_file.pl
@@ -1,7 +1,13 @@
 #!/usr/bin/perl
 
-use strict;
-use warnings;
+use Modern::Perl;
+
+BEGIN {
+    # find Koha's Perl modules
+    # test carefully before changing this
+    use FindBin ();
+    eval { require "$FindBin::Bin/kohalib.pl" };
+}
 
 use Koha::Script;
 use C4::Context;
@@ -39,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;
@@ -53,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";
 }
@@ -105,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_;
@@ -124,9 +127,8 @@ _SUMMARY_
 
 
 sub print_progress_and_commit {
-    my $recs = shift;
+    my ( $recs, $schema ) = @_;
     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 e84178f..f74db44 100755 (executable)
--- a/tools/manage-marc-import.pl
+++ b/tools/manage-marc-import.pl
@@ -235,21 +235,17 @@ sub commit_batch {
     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,
@@ -281,7 +277,7 @@ sub revert_batch {
             my $callback = sub { };
             if ($runinbackground) {
                 $job = put_in_background($import_batch_id);
-                $callback = progress_callback( $job, $dbh );
+                $callback = progress_callback( $job );
             }
             (
                 $num_deleted,       $num_errors, $num_reverted,
@@ -347,11 +343,9 @@ sub put_in_background {
 
 sub progress_callback {
     my $job = shift;
-    my $dbh = shift;
     return sub {
         my $progress = shift;
         $job->progress($progress);
-        $dbh->commit();
     }
 }