From d95805c6aacc28490c17b42d22277e966f1292c1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Joonas=20Kylm=C3=A4l=C3=A4?= Date: Tue, 6 Sep 2022 22:32:11 +0300 Subject: [PATCH] Bug 31351: Koha::BackgroundJob: Let database connection object handle utf8 transcoding MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Our database connections have been set up so that they will automatically convert perl encoded strings to utf8 encoded strings when sending data to database tables and decode the utf8 encoded strings from the tables back to internal perl strings. As we can see from a call in Koha/BackgroundJob.pm we are encoding the perl internal string to utf8 and then decoding it back to perl internal string: my $data_dump = decode_json encode_utf8 $self->data; We can skip this unnecessary encode<->decode step (as the database object has done the decoding for us) by simply calling the JSON->new->decode() method which doesn't perform any string decoding. Furthermore, the original code was buggy and didn't always remember to encode the unencoded strings, in Koha::BackgroundJob::process we can see my $context = decode_json($self->context); is missing the encode step. Now after this change encoding before decoding is not necessary as we are using the methods from the JSON module that do not perform any transcoding. Note to those concerned whether the old data in the database is compatible with this new code: Luckily our database connection object seems to be smart and didn't utf8 encode the utf8 returned data from the old encode_json() calls (probably checks the utf8 flag for the string (Encode::is_utf8($str))). To test whether this fixes the original bug reported of not being able to schedule background jobs with koha user having non-ASCII letters in their surname: 1) Change your staff users surname/lastname to "ääää" 2) Log out and back in. 3) Go to a biblio record detail page and click "Select all" in the items table 4) Click Delete selected items and proceed with the deletion 5) Notice the batch item deletion job has failed status 6) Apply patch and repeat but this time the deletion job should finish. Signed-off-by: Katrin Fischer Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi --- Koha/BackgroundJob.pm | 22 +++++++++---------- Koha/BackgroundJob/BatchCancelHold.pm | 7 +++--- Koha/BackgroundJob/BatchDeleteAuthority.pm | 7 +++--- Koha/BackgroundJob/BatchDeleteBiblio.pm | 7 +++--- Koha/BackgroundJob/BatchDeleteItem.pm | 7 +++--- Koha/BackgroundJob/BatchUpdateAuthority.pm | 7 +++--- Koha/BackgroundJob/BatchUpdateBiblio.pm | 7 +++--- .../BatchUpdateBiblioHoldsQueue.pm | 7 +++--- Koha/BackgroundJob/BatchUpdateItem.pm | 8 +++---- 9 files changed, 43 insertions(+), 36 deletions(-) diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index dcfba14659..69a39f9402 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -16,8 +16,7 @@ package Koha::BackgroundJob; # along with Koha; if not, see . use Modern::Perl; -use JSON qw( decode_json encode_json ); -use Encode qw( encode_utf8 ); +use JSON; use Carp qw( croak ); use Net::Stomp; use Try::Tiny qw( catch try ); @@ -101,11 +100,12 @@ sub enqueue { my $job_args = $params->{job_args}; my $job_context = $params->{job_context} // C4::Context->userenv; my $job_queue = $params->{job_queue} // 'default'; + my $json = JSON->new; my $borrowernumber = (C4::Context->userenv) ? C4::Context->userenv->{number} : undef; $job_context->{interface} = C4::Context->interface; - my $json_context = encode_json $job_context; - my $json_args = encode_json $job_args; + my $json_context = $json->encode($job_context); + my $json_args = $json->encode($job_args); $self->set( { @@ -130,7 +130,7 @@ sub enqueue { }; return unless $conn; - $json_args = encode_json $job_args; + $json_args = $json->encode($job_args); try { # This namespace is wrong, it must be a vhost instead. # But to do so it needs to be created on the server => much more work when a new Koha instance is created. @@ -167,7 +167,7 @@ sub process { $args ||= {}; if ( $self->context ) { - my $context = decode_json($self->context); + my $context = JSON->new->decode($self->context); C4::Context->_new_userenv(-1); C4::Context->interface( $context->{interface} ); C4::Context->set_userenv( @@ -251,7 +251,7 @@ sub finish { return $self->set( { ended_on => \'NOW()', - data => encode_json($data), + data => JSON->new->encode($data), } )->store; } @@ -267,7 +267,7 @@ Returns the decoded JSON contents from $self->data. sub decoded_data { my ($self) = @_; - return $self->data ? decode_json( $self->data ) : undef; + return $self->data ? JSON->new->decode( $self->data ) : undef; } =head3 set_encoded_data @@ -281,7 +281,7 @@ Serializes I<$data> as a JSON string and sets the I attribute with it. sub set_encoded_data { my ( $self, $data ) = @_; - return $self->data( $data ? encode_json($data) : undef ); + return $self->data( $data ? JSON->new->encode($data) : undef ); } =head3 job_type @@ -302,7 +302,7 @@ sub messages { my ( $self ) = @_; my @messages; - my $data_dump = decode_json encode_utf8 $self->data; + my $data_dump = JSON->new->decode($self->data); if ( exists $data_dump->{messages} ) { @messages = @{ $data_dump->{messages} }; } @@ -319,7 +319,7 @@ Report of the job. sub report { my ( $self ) = @_; - my $data_dump = decode_json encode_utf8 $self->data; + my $data_dump = JSON->new->decode($self->data); return $data_dump->{report} || {}; } diff --git a/Koha/BackgroundJob/BatchCancelHold.pm b/Koha/BackgroundJob/BatchCancelHold.pm index 51648aecec..9fdcb5515a 100644 --- a/Koha/BackgroundJob/BatchCancelHold.pm +++ b/Koha/BackgroundJob/BatchCancelHold.pm @@ -16,7 +16,7 @@ package Koha::BackgroundJob::BatchCancelHold; # along with Koha; if not, see . use Modern::Perl; -use JSON qw( encode_json decode_json ); +use JSON; use Koha::DateUtils qw( dt_from_string ); use Koha::Holds; @@ -109,11 +109,12 @@ sub process { $self->progress( ++$job_progress )->store; } - my $job_data = decode_json $self->data; + my $json = JSON->new; + my $job_data = $json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; - $self->ended_on(dt_from_string)->data( encode_json $job_data); + $self->ended_on(dt_from_string)->data($json->encode($job_data)); $self->status('finished') if $self->status ne 'cancelled'; $self->store; diff --git a/Koha/BackgroundJob/BatchDeleteAuthority.pm b/Koha/BackgroundJob/BatchDeleteAuthority.pm index 64d10f43ab..5e407f80af 100644 --- a/Koha/BackgroundJob/BatchDeleteAuthority.pm +++ b/Koha/BackgroundJob/BatchDeleteAuthority.pm @@ -1,7 +1,7 @@ package Koha::BackgroundJob::BatchDeleteAuthority; use Modern::Perl; -use JSON qw( encode_json decode_json ); +use JSON; use C4::AuthoritiesMarc; @@ -84,12 +84,13 @@ sub process { $indexer->index_records( \@deleted_authids, "recordDelete", "authorityserver" ); } - my $job_data = decode_json $self->data; + my $json = JSON->new; + my $job_data = $json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; $self->ended_on(dt_from_string) - ->data(encode_json $job_data); + ->data($json->encode($job_data)); $self->status('finished') if $self->status ne 'cancelled'; $self->store; } diff --git a/Koha/BackgroundJob/BatchDeleteBiblio.pm b/Koha/BackgroundJob/BatchDeleteBiblio.pm index b5984ce44f..6186500da0 100644 --- a/Koha/BackgroundJob/BatchDeleteBiblio.pm +++ b/Koha/BackgroundJob/BatchDeleteBiblio.pm @@ -1,7 +1,7 @@ package Koha::BackgroundJob::BatchDeleteBiblio; use Modern::Perl; -use JSON qw( encode_json decode_json ); +use JSON; use C4::Biblio; @@ -167,12 +167,13 @@ sub process { ) if C4::Context->preference('RealTimeHoldsQueue'); } - my $job_data = decode_json $self->data; + my $json = JSON->new; + my $job_data = $json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; $self->ended_on(dt_from_string) - ->data(encode_json $job_data); + ->data($json->encode($job_data)); $self->status('finished') if $self->status ne 'cancelled'; $self->store; } diff --git a/Koha/BackgroundJob/BatchDeleteItem.pm b/Koha/BackgroundJob/BatchDeleteItem.pm index 57371289a9..1f717b3d80 100644 --- a/Koha/BackgroundJob/BatchDeleteItem.pm +++ b/Koha/BackgroundJob/BatchDeleteItem.pm @@ -22,7 +22,7 @@ Koha::BackgroundJob::BatchDeleteItem - Background job derived class to process i =cut use Modern::Perl; -use JSON qw( encode_json decode_json ); +use JSON; use List::MoreUtils qw( uniq ); use Try::Tiny; @@ -199,11 +199,12 @@ sub process { $report->{not_deleted_itemnumbers} = \@not_deleted_itemnumbers; $report->{deleted_biblionumbers} = \@deleted_biblionumbers; - my $job_data = decode_json $self->data; + my $json = JSON->new; + my $job_data = $json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; - $self->ended_on(dt_from_string)->data( encode_json $job_data); + $self->ended_on(dt_from_string)->data( $json->encode($job_data)); $self->status('finished') if $self->status ne 'cancelled'; $self->store; } diff --git a/Koha/BackgroundJob/BatchUpdateAuthority.pm b/Koha/BackgroundJob/BatchUpdateAuthority.pm index ca23f7cf86..33afd61e12 100644 --- a/Koha/BackgroundJob/BatchUpdateAuthority.pm +++ b/Koha/BackgroundJob/BatchUpdateAuthority.pm @@ -16,7 +16,7 @@ package Koha::BackgroundJob::BatchUpdateAuthority; # along with Koha; if not, see . use Modern::Perl; -use JSON qw( decode_json encode_json ); +use JSON; use C4::MarcModificationTemplates qw( ModifyRecordWithTemplate ); use C4::AuthoritiesMarc qw( ModAuthority ); @@ -106,12 +106,13 @@ sub process { my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::AUTHORITIES_INDEX }); $indexer->index_records( \@record_ids, "specialUpdate", "authorityserver" ); - my $job_data = decode_json $self->data; + my $json = JSON->new; + my $job_data = $json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; $self->ended_on(dt_from_string) - ->data(encode_json $job_data); + ->data($json->encode($job_data)); $self->status('finished') if $self->status ne 'cancelled'; $self->store; diff --git a/Koha/BackgroundJob/BatchUpdateBiblio.pm b/Koha/BackgroundJob/BatchUpdateBiblio.pm index 63d1277ff4..20cb296303 100644 --- a/Koha/BackgroundJob/BatchUpdateBiblio.pm +++ b/Koha/BackgroundJob/BatchUpdateBiblio.pm @@ -16,7 +16,7 @@ package Koha::BackgroundJob::BatchUpdateBiblio; # along with Koha; if not, see . use Modern::Perl; -use JSON qw( decode_json encode_json ); +use JSON; use Koha::Biblios; use Koha::DateUtils qw( dt_from_string ); @@ -118,12 +118,13 @@ sub process { my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); $indexer->index_records( \@record_ids, "specialUpdate", "biblioserver" ); - my $job_data = decode_json $self->data; + my $json = JSON->new; + my $job_data = $json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; $self->ended_on(dt_from_string) - ->data(encode_json $job_data); + ->data($json->encode($job_data)); $self->status('finished') if $self->status ne 'cancelled'; $self->store; } diff --git a/Koha/BackgroundJob/BatchUpdateBiblioHoldsQueue.pm b/Koha/BackgroundJob/BatchUpdateBiblioHoldsQueue.pm index c85de205ff..607ae22174 100644 --- a/Koha/BackgroundJob/BatchUpdateBiblioHoldsQueue.pm +++ b/Koha/BackgroundJob/BatchUpdateBiblioHoldsQueue.pm @@ -17,7 +17,7 @@ package Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue; use Modern::Perl; -use JSON qw( encode_json decode_json ); +use JSON; use Try::Tiny; use Koha::Biblios; @@ -120,14 +120,15 @@ sub process { $self->progress( $self->progress + 1 )->store; } - my $job_data = decode_json $self->data; + my $json = JSON->new; + my $job_data = $json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; $self->set( { ended_on => \'NOW()', - data => encode_json $job_data, + data => $json->encode($job_data), status => 'finished', } )->store; diff --git a/Koha/BackgroundJob/BatchUpdateItem.pm b/Koha/BackgroundJob/BatchUpdateItem.pm index d12c08497c..8c1e8aefbd 100644 --- a/Koha/BackgroundJob/BatchUpdateItem.pm +++ b/Koha/BackgroundJob/BatchUpdateItem.pm @@ -16,8 +16,7 @@ package Koha::BackgroundJob::BatchUpdateItem; # along with Koha; if not, see . use Modern::Perl; -use JSON qw( encode_json decode_json ); -use Encode qw( encode_utf8 ); +use JSON; use List::MoreUtils qw( uniq ); use Try::Tiny; @@ -130,11 +129,12 @@ sub process { if ( $_ =~ /Rollback failed/ ); # Rollback failed }; + my $json = JSON->new; $self->discard_changes; - my $job_data = decode_json encode_utf8 $self->data; + my $job_data = $json->decode($self->data); $job_data->{report} = $report; - $self->ended_on(dt_from_string)->data( encode_json $job_data); + $self->ended_on(dt_from_string)->data($json->encode($job_data)); $self->status('finished') if $self->status ne 'cancelled'; $self->store; } -- 2.39.5