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