From 0210bd96ea6569d5ae42ae902fb7d880dded0636 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 11 Oct 2021 16:26:12 +0200 Subject: [PATCH] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. Signed-off-by: Nick Clemens --- Koha/BackgroundJob/BatchUpdateItem.pm | 36 +++--- Koha/Items.pm | 156 +++++++++++++------------- 2 files changed, 96 insertions(+), 96 deletions(-) diff --git a/Koha/BackgroundJob/BatchUpdateItem.pm b/Koha/BackgroundJob/BatchUpdateItem.pm index 1dbc922682..8336b16e23 100644 --- a/Koha/BackgroundJob/BatchUpdateItem.pm +++ b/Koha/BackgroundJob/BatchUpdateItem.pm @@ -111,27 +111,22 @@ sub process { }; try { - my $schema = Koha::Database->new->schema; - $schema->txn_do( - sub { - my ($results) = - Koha::Items->search( { itemnumber => \@record_ids } ) - ->batch_update( - { - regex_mod => $regex_mod, - new_values => $new_values, - exclude_from_local_holds_priority => - $exclude_from_local_holds_priority, - callback => sub { - my ($progress) = @_; - $job->progress($progress)->store; - }, - } - ); - $report->{modified_itemnumbers} = $results->{modified_itemnumbers}; - $report->{modified_fields} = $results->{modified_fields}; + my ($results) = + Koha::Items->search( { itemnumber => \@record_ids } ) + ->batch_update( + { + regex_mod => $regex_mod, + new_values => $new_values, + exclude_from_local_holds_priority => + $exclude_from_local_holds_priority, + callback => sub { + my ($progress) = @_; + $job->progress($progress)->store; + }, } - ); + ); + $report->{modified_itemnumbers} = $results->{modified_itemnumbers}; + $report->{modified_fields} = $results->{modified_fields}; } catch { warn $_; @@ -139,6 +134,7 @@ sub process { if ( $_ =~ /Rollback failed/ ); # Rollback failed }; + $job->discard_changes; my $job_data = decode_json encode_utf8 $job->data; $job_data->{report} = $report; diff --git a/Koha/Items.pm b/Koha/Items.pm index 8d5a01cdc7..7349bc4f38 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -20,6 +20,7 @@ package Koha::Items; use Modern::Perl; use Array::Utils qw( array_minus ); use List::MoreUtils qw( uniq ); +use Try::Tiny; use C4::Context; use C4::Biblio qw( GetMarcStructure GetMarcFromKohaField ); @@ -242,98 +243,101 @@ sub batch_update { my (@modified_itemnumbers, $modified_fields); my $i; + my $schema = Koha::Database->new->schema; while ( my $item = $self->next ) { - my $modified_holds_priority = 0; - if ( defined $exclude_from_local_holds_priority ) { - if(!defined $item->exclude_from_local_holds_priority || $item->exclude_from_local_holds_priority != $exclude_from_local_holds_priority) { - $item->exclude_from_local_holds_priority($exclude_from_local_holds_priority)->store; - $modified_holds_priority = 1; + try {$schema->txn_do(sub { + my $modified_holds_priority = 0; + if ( defined $exclude_from_local_holds_priority ) { + if(!defined $item->exclude_from_local_holds_priority || $item->exclude_from_local_holds_priority != $exclude_from_local_holds_priority) { + $item->exclude_from_local_holds_priority($exclude_from_local_holds_priority)->store; + $modified_holds_priority = 1; + } } - } - - my $modified = 0; - my $new_values = {%$new_values}; # Don't modify the original - - my $old_values = $item->unblessed; - if ( $item->more_subfields_xml ) { - $old_values = { - %$old_values, - %{$item->additional_attributes->to_hashref}, - }; - } - for my $attr ( keys %$regex_mod ) { - my $old_value = $old_values->{$attr}; + my $modified = 0; + my $new_values = {%$new_values}; # Don't modify the original - next unless $old_value; + my $old_values = $item->unblessed; + if ( $item->more_subfields_xml ) { + $old_values = { + %$old_values, + %{$item->additional_attributes->to_hashref}, + }; + } - my $value = apply_regex( - { - %{ $regex_mod->{$attr} }, - value => $old_value, - } - ); + for my $attr ( keys %$regex_mod ) { + my $old_value = $old_values->{$attr}; - $new_values->{$attr} = $value; - } + next unless $old_value; - for my $attribute ( keys %$new_values ) { - next if $attribute eq 'more_subfields_xml'; # Already counted before + my $value = apply_regex( + { + %{ $regex_mod->{$attr} }, + value => $old_value, + } + ); - my $old = $old_values->{$attribute}; - my $new = $new_values->{$attribute}; - $modified++ - if ( defined $old xor defined $new ) - || ( defined $old && defined $new && $new ne $old ); - } + $new_values->{$attr} = $value; + } - { # Dealing with more_subfields_xml - - my $frameworkcode = $item->biblio->frameworkcode; - my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 }); - my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); - - my @more_subfield_tags = map { - ( - ref($_) - && %$_ - && !$_->{kohafield} # Get subfields that are not mapped - ) - ? $_->{tagsubfield} - : () - } values %{ $tagslib->{$itemtag} }; - - my $more_subfields_xml = Koha::Item::Attributes->new( - { - map { - exists $new_values->{$_} ? ( $_ => $new_values->{$_} ) - : exists $old_values->{$_} - ? ( $_ => $old_values->{$_} ) - : () - } @more_subfield_tags - } - )->to_marcxml($frameworkcode); + for my $attribute ( keys %$new_values ) { + next if $attribute eq 'more_subfields_xml'; # Already counted before - $new_values->{more_subfields_xml} = $more_subfields_xml; + my $old = $old_values->{$attribute}; + my $new = $new_values->{$attribute}; + $modified++ + if ( defined $old xor defined $new ) + || ( defined $old && defined $new && $new ne $old ); + } - delete $new_values->{$_} for @more_subfield_tags; # Clean the hash + { # Dealing with more_subfields_xml + + my $frameworkcode = $item->biblio->frameworkcode; + my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 }); + my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); + + my @more_subfield_tags = map { + ( + ref($_) + && %$_ + && !$_->{kohafield} # Get subfields that are not mapped + ) + ? $_->{tagsubfield} + : () + } values %{ $tagslib->{$itemtag} }; + + my $more_subfields_xml = Koha::Item::Attributes->new( + { + map { + exists $new_values->{$_} ? ( $_ => $new_values->{$_} ) + : exists $old_values->{$_} + ? ( $_ => $old_values->{$_} ) + : () + } @more_subfield_tags + } + )->to_marcxml($frameworkcode); + + $new_values->{more_subfields_xml} = $more_subfields_xml; + + delete $new_values->{$_} for @more_subfield_tags; # Clean the hash - } + } - if ( $modified ) { - my $itemlost_pre = $item->itemlost; - $item->set($new_values)->store({skip_record_index => 1}); + if ( $modified ) { + my $itemlost_pre = $item->itemlost; + $item->set($new_values)->store({skip_record_index => 1}); - LostItem( - $item->itemnumber, 'batchmod', undef, - { skip_record_index => 1 } - ) if $item->itemlost - and not $itemlost_pre; + LostItem( + $item->itemnumber, 'batchmod', undef, + { skip_record_index => 1 } + ) if $item->itemlost + and not $itemlost_pre; - push @modified_itemnumbers, $item->itemnumber if $modified || $modified_holds_priority; - $modified_fields += $modified + $modified_holds_priority; - } + push @modified_itemnumbers, $item->itemnumber if $modified || $modified_holds_priority; + $modified_fields += $modified + $modified_holds_priority; + } + })}; if ( $callback ) { $callback->(++$i); -- 2.20.1