Bugzilla – Attachment 140310 Details for
Bug 31351
Worker dies on reindex job when operator last name/first name/branch name contains non-ASCII chars
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31351: Koha::BackgroundJob: Let database connection object handle utf8 transcoding
Bug-31351-KohaBackgroundJob-Let-database-connectio.patch (text/plain), 13.89 KB, created by
Marcel de Rooy
on 2022-09-07 14:34:15 UTC
(
hide
)
Description:
Bug 31351: Koha::BackgroundJob: Let database connection object handle utf8 transcoding
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2022-09-07 14:34:15 UTC
Size:
13.89 KB
patch
obsolete
>From 2bd0e86be403b5f89a0c95fee8b4c5afde190a82 Mon Sep 17 00:00:00 2001 >From: =?utf-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@iki.fi> >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=utf-8 >Content-Transfer-Encoding: 8bit >Content-Type: text/plain; charset=utf-8 > >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 <katrin.fischer.83@web.de> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > 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 <http://www.gnu.org/licenses>. > > 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<data> 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 <http://www.gnu.org/licenses>. > > 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 <http://www.gnu.org/licenses>. > > 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 <http://www.gnu.org/licenses>. > > 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 <http://www.gnu.org/licenses>. > > 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.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 31351
:
139058
|
139438
|
140222
|
140236
|
140262
| 140310 |
140311
|
140312
|
140313
|
140314
|
140315