From f64a9b7f5945ed6f8ee4a65ff449261a327de6a6 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 29 Nov 2022 16:01:04 -0300 Subject: [PATCH] Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields https://bugs.koha-community.org/show_bug.cgi?id=32371 --- Koha/BackgroundJob.pm | 65 ++++--------------- misc/background_jobs_worker.pl | 2 +- t/db_dependent/Koha/BackgroundJob.t | 20 +++--- t/db_dependent/Koha/BackgroundJobs.t | 2 +- .../Koha/BackgroundJobs/BatchUpdateBiblio.t | 13 +++- t/lib/Koha/BackgroundJob/BatchTest.pm | 4 +- 6 files changed, 38 insertions(+), 68 deletions(-) diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index 727bc3b2f7b..61a0cbcdf33 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -17,7 +17,6 @@ package Koha::BackgroundJob; use Modern::Perl; use Encode qw(); -use JSON; use Carp qw( croak ); use Net::Stomp; use Try::Tiny qw( catch try ); @@ -28,7 +27,7 @@ use Koha::Exceptions; use Koha::Plugins; use Koha::Exceptions::BackgroundJob; -use base qw( Koha::Object ); +use base qw( Koha::Object Koha::Object::JSONFields ); =head1 NAME @@ -101,21 +100,18 @@ 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 = $self->json; my $borrowernumber = (C4::Context->userenv) ? C4::Context->userenv->{number} : undef; $job_context->{interface} = C4::Context->interface; - my $json_context = $json->encode($job_context); - my $json_args = $json->encode($job_args); + + $self->set_encoded_json_field( { data => $job_context, field => 'context' } ); + $self->set_encoded_json_field( { data => $job_args, field => 'data' } ); $self->set( - { - status => 'new', + { status => 'new', type => $job_type, queue => $job_queue, size => $job_size, - data => $json_args, - context => $json_context, enqueued_on => dt_from_string, borrowernumber => $borrowernumber, } @@ -131,7 +127,7 @@ sub enqueue { }; return unless $conn; - $json_args = $json->encode($job_args); + my $json_args = $self->_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. @@ -169,7 +165,7 @@ sub process { $args ||= {}; if ( $self->context ) { - my $context = $self->json->decode($self->context); + my $context = $self->decode_json_field({ field => 'context' }); C4::Context->_new_userenv(-1); C4::Context->interface( $context->{interface} ); C4::Context->set_userenv( @@ -249,28 +245,9 @@ sub finish { my ( $self, $data ) = @_; $self->status('finished') unless $self->status eq 'cancelled' or $self->status eq 'failed'; + $self->set_encoded_json_field({ data => $data, field => 'data' }); - return $self->set( - { - ended_on => \'NOW()', - data => $self->json->encode($data), - } - )->store; -} - -=head3 json - - my $JSON_object = $self->json; - -Returns a JSON object with utf8 disabled. Encoding to UTF-8 should be -done later. - -=cut - -sub json { - my ( $self ) = @_; - $self->{_json} //= JSON->new->utf8(0); # TODO Should we allow_nonref ? - return $self->{_json}; + return $self->set({ ended_on => \'NOW()' })->store; } =head3 decoded_data @@ -284,21 +261,7 @@ Returns the decoded JSON contents from $self->data. sub decoded_data { my ($self) = @_; - return $self->data ? $self->json->decode( $self->data ) : undef; -} - -=head3 set_encoded_data - - $self->set_encoded_data( $data ); - -Serializes I<$data> as a JSON string and sets the I attribute with it. - -=cut - -sub set_encoded_data { - my ( $self, $data ) = @_; - - return $self->data( $data ? $self->json->encode($data) : undef ); + return $self->decode_json_field({ field => 'data' }); } =head3 job_type @@ -319,7 +282,7 @@ sub messages { my ( $self ) = @_; my @messages; - my $data_dump = $self->json->decode($self->data); + my $data_dump = $self->decode_json_field({ field => 'data' }); if ( exists $data_dump->{messages} ) { @messages = @{ $data_dump->{messages} }; } @@ -336,7 +299,7 @@ Report of the job. sub report { my ( $self ) = @_; - my $data_dump = $self->json->decode($self->data); + my $data_dump = $self->decode_json_field({ field => 'data' }); return $data_dump->{report} || {}; } @@ -489,9 +452,9 @@ sub to_api { my $json = $self->SUPER::to_api( $params ); - $json->{context} = $self->json->decode($self->context) + $json->{context} = $self->decode_json_field({ field => 'context' }) if defined $self->context; - $json->{data} = $self->decoded_data; + $json->{data} = $self->decode_json_field({ field => 'data' }); return $json; } diff --git a/misc/background_jobs_worker.pl b/misc/background_jobs_worker.pl index eb36f34f964..ee5e7293bd2 100755 --- a/misc/background_jobs_worker.pl +++ b/misc/background_jobs_worker.pl @@ -102,7 +102,7 @@ while (1) { } else { my $jobs = Koha::BackgroundJobs->search({ status => 'new', queue => \@queues }); while ( my $job = $jobs->next ) { - my $args = $job->json->decode($job->data); + my $args = $job->decoded_data; process_job( $job, { job_id => $job->id, %$args } ); } sleep 10; diff --git a/t/db_dependent/Koha/BackgroundJob.t b/t/db_dependent/Koha/BackgroundJob.t index 80674fbde58..2af7b16d543 100755 --- a/t/db_dependent/Koha/BackgroundJob.t +++ b/t/db_dependent/Koha/BackgroundJob.t @@ -108,7 +108,7 @@ subtest 'enqueue() tests' => sub { is( $job->size, 3, 'Three steps' ); is( $job->status, 'new', 'Initial status set correctly' ); is( $job->borrowernumber, $patron->id, 'Borrowernumber set from userenv' ); - is_deeply( $job->json->decode( $job->context ), $job_context, 'Context set from userenv + interface' ); + is_deeply( $job->decode_json_field({ field => 'context' }), $job_context, 'Context set from userenv + interface' ); $schema->storage->txn_rollback; }; @@ -150,14 +150,14 @@ subtest 'start(), step() and finish() tests' => sub { is( $job->status, 'cancelled', "'finish' leaves 'cancelled' untouched" ); isnt( $job->ended_on, undef, 'ended_on set' ); - is_deeply( $job->json->decode( $job->data ), $data ); + is_deeply( $job->decode_json_field({ field => 'data' }), $data ); $job->status('started')->store; $job->finish( $data ); is( $job->status, 'finished' ); isnt( $job->ended_on, undef, 'ended_on set' ); - is_deeply( $job->json->decode( $job->data ), $data ); + is_deeply( $job->decode_json_field({ field => 'data' }), $data ); throws_ok { $job->start; } @@ -224,7 +224,7 @@ subtest 'process tests' => sub { is_deeply( C4::Context->interface, 'intranet', "Interface set from job context on process" ); # Manually add a job (->new->store) without context - my $json = $job->json; # sorry, quickly borrowing your json object + my $json = $job->_json; # sorry, quickly borrowing your json object my $data = $json->encode({ a => 'a', b => 'b' }); my $incomplete_job = t::lib::Koha::BackgroundJob::BatchTest->new( { status => 'new', @@ -242,24 +242,24 @@ subtest 'process tests' => sub { $schema->storage->txn_rollback; }; -subtest 'decoded_data() and set_encoded_data() tests' => sub { +subtest 'decoded_data() tests' => sub { plan tests => 8; $schema->storage->txn_begin; - my $job = Koha::BackgroundJob::BatchUpdateItem->new->set_encoded_data( undef ); + my $job = Koha::BackgroundJob::BatchUpdateItem->new->set_encoded_json_field({ data => undef, field => 'data' }); is( $job->decoded_data, undef, 'undef is undef' ); my $data = { some => 'data' }; - $job->set_encoded_data( $data ); + $job->set_encoded_json_field({ data => $data, field => 'data' }); - is_deeply( $job->json->decode($job->data), $data, 'decode what we sent' ); + is_deeply( $job->_json->decode($job->data), $data, 'decode what we sent' ); is_deeply( $job->decoded_data, $data, 'check with decoded_data' ); # Let's get some Unicode stuff into the game $data = { favorite_Chinese => [ '葑', '癱' ], latin_dancing => [ '¢', '¥', 'á', 'û' ] }; - $job->set_encoded_data( $data )->store; + $job->set_encoded_json_field({ data => $data, field => 'data' })->store; $job->discard_changes; # refresh is_deeply( $job->decoded_data, $data, 'Deep compare with Unicode data' ); @@ -274,7 +274,7 @@ subtest 'decoded_data() and set_encoded_data() tests' => sub { push @{$utf8_data->{$k}}, Encode::encode('UTF-8', $c); } } - $job->set_encoded_data( $utf8_data )->store; + $job->set_encoded_json_field({ data => $utf8_data, field => 'data' })->store; $job->discard_changes; # refresh is_deeply( $job->decoded_data, $utf8_data, 'Deep compare with utf8_data' ); # Need more evidence? diff --git a/t/db_dependent/Koha/BackgroundJobs.t b/t/db_dependent/Koha/BackgroundJobs.t index c8089d4488b..40292c6dd1a 100755 --- a/t/db_dependent/Koha/BackgroundJobs.t +++ b/t/db_dependent/Koha/BackgroundJobs.t @@ -62,7 +62,7 @@ my $job_id = t::lib::Koha::BackgroundJob::BatchTest->new->enqueue( # Enqueuing a new job my $new_job = Koha::BackgroundJobs->find($job_id); ok( $new_job, 'New job correctly enqueued' ); -is_deeply( $new_job->json->decode( $new_job->data ), +is_deeply( $new_job->_json->decode( $new_job->data ), $data, 'data retrieved and json encoded correctly' ); is( t::lib::Dates::compare( $new_job->enqueued_on, dt_from_string ), 0, 'enqueued_on correctly filled with now()' ); diff --git a/t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t b/t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t index aa9b0e49d56..13ac116e6f0 100755 --- a/t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t +++ b/t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t @@ -51,13 +51,20 @@ subtest "Exceptions must be stringified" => sub { type => 'batch_biblio_record_modification', } ); - my $data = $job->json->encode({ record_ids => [ $biblio->biblionumber ] }); - $job->data( $data )->store; + + $job->set_encoded_json_field( + { data => { record_ids => [ $biblio->biblionumber ] }, + field => 'data' + } + )->store; + $job = Koha::BackgroundJobs->find( $job->id ); $job->process( { job_id => $job->id, record_ids => [ $biblio->biblionumber ] } ); - $data = $job->json->decode( $job->get_from_storage->data ); + $job = $job->get_from_storage; + + my $data = $job->decode_json_field({ field => 'data' }); is_deeply( $data->{messages}->[0], { diff --git a/t/lib/Koha/BackgroundJob/BatchTest.pm b/t/lib/Koha/BackgroundJob/BatchTest.pm index 1ae84ef4e97..2d7c919459a 100644 --- a/t/lib/Koha/BackgroundJob/BatchTest.pm +++ b/t/lib/Koha/BackgroundJob/BatchTest.pm @@ -56,12 +56,12 @@ sub process { $self->progress( ++$job_progress )->store; } - my $job_data = $self->json->decode($self->data); + my $job_data = $self->_json->decode($self->data); $job_data->{messages} = \@messages; $job_data->{report} = $report; $self->ended_on(dt_from_string) - ->data( $self->json->encode($job_data) ); + ->set_encoded_json_field({ data => $job_data, field => 'data' }); $self->status('finished') if $self->status ne 'cancelled'; $self->store; } -- 2.34.1