Attachment 145215 Details for Bug 32370 – Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields

[patch] Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields

Bug-32370-follow-up-Make-KohaBackgroundJob-use-Koh.patch (text/plain), 11.91 KB, created by Nick Clemens (kidclamp) on 2023-01-11 16:41:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2023-01-11 16:41:19 UTC

Size: 11.91 KB

patch

obsolete

>From 6c7ac977c3a84260017486b3349b763fe4c249eb Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> 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 727bc3b2f7..61a0cbcdf3 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<data> 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 eb36f34f96..ee5e7293bd 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 4f1f4d9bbe..37f5b9c6da 100755
>--- a/t/db_dependent/Koha/BackgroundJob.t
>+++ b/t/db_dependent/Koha/BackgroundJob.t
>@@ -109,7 +109,7 @@ subtest 'enqueue() tests' => sub {
>     is( $job->status,         'new',       'Initial status set correctly' );
>     is( $job->borrowernumber, $patron->id, 'Borrowernumber set from userenv' );
>     is( $job->queue,          'long_tasks', 'BatchUpdateItem should use the long_tasks queue' );
>-    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;
> };
>@@ -151,14 +151,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; }
>@@ -225,7 +225,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',
>@@ -243,24 +243,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' );
>@@ -275,7 +275,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 c8089d4488..40292c6dd1 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 aa9b0e49d5..13ac116e6f 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 1ae84ef4e9..2d7c919459 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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 32370: 144329 | 144330 | 144331 | 144332 | 144333 | 144334 | 144347 | 144376 | 144377 | 144378 | 144445 | 145213 | 145214 | 145215