Bug 30982: API tweaks

This patch makes the following changes to the 'background_jobs' API:

* We now call them 'jobs'
* Removed deprecated query parameter definitions
* Added only_current query parameter
* Controller gets adapted to use $rs->filter_by_current when
  only_current is passed

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Koha/BackgroundJob.pm

@@ -472,6 +472,27 @@ sub plugin_types_to_classes {
     return $self->{_plugin_mapping};
 }
 
+=head3 to_api
+
+    my $json = $job->to_api;
+
+Overloaded method that returns a JSON representation of the Koha::BackgroundJob object,
+suitable for API output.
+
+=cut
+
+sub to_api {
+    my ( $self, $params ) = @_;
+
+    my $json = $self->SUPER::to_api( $params );
+
+    $json->{context} = $self->json->decode($self->context)
+      if defined $self->context;
+    $json->{data} = $self->decoded_data;
+
+    return $json;
+}
+
 =head3 to_api_mapping
 
 This method returns the mapping for representing a Koha::BackgroundJob object
@@ -481,8 +502,11 @@ on the API.
 
 sub to_api_mapping {
     return {
-        id             => 'background_job_id',
+        id             => 'job_id',
         borrowernumber => 'patron_id',
+        ended_on       => 'ended_date',
+        enqueued_on    => 'enqueued_date',
+        started_on     => 'started_date',
     };
 }
 

Koha/REST/V1/BackgroundJobs.pm

@@ -29,49 +29,66 @@ use Try::Tiny;
 
 =head3 list
 
+Controller function that handles listing Koha::BackgrounJob objects
+
 =cut
 
 sub list {
     my $c = shift->openapi->valid_input or return;
 
     return try {
-        my $background_jobs_set = Koha::BackgroundJobs->new;
-        my $background_jobs     = $c->objects->search($background_jobs_set);
-        return $c->render( status => 200, openapi => $background_jobs );
-    }
-    catch {
+
+        my $only_current = delete $c->validation->output->{only_current};
+
+        my $bj_rs = Koha::BackgroundJobs->new;
+
+        if ($only_current) {
+            $bj_rs = $bj_rs->filter_by_current;
+        }
+
+        return $c->render(
+            status  => 200,
+            openapi => $c->objects->search($bj_rs)
+        );
+    } catch {
         $c->unhandled_exception($_);
     };
 }
 
+=head3 get
+
+Controller function that handles retrieving a single Koha::BackgroundJob object
+
+=cut
+
 sub get {
     my $c = shift->openapi->valid_input or return;
 
     return try {
 
-        my $background_job_id = $c->validation->param('background_job_id');
-        my $patron            = $c->stash('koha.user');
+        my $job_id = $c->validation->param('job_id');
+        my $patron = $c->stash('koha.user');
 
         my $can_manage_background_jobs =
           $patron->has_permission( { parameters => 'manage_background_jobs' } );
 
-        my $background_job = Koha::BackgroundJobs->find($background_job_id);
+        my $job = Koha::BackgroundJobs->find($job_id);
 
         return $c->render(
             status  => 404,
             openapi => { error => "Object not found" }
-        ) unless $background_job;
+        ) unless $job;
 
         return $c->render(
             status  => 403,
             openapi => { error => "Cannot see background job info" }
           )
           if !$can_manage_background_jobs
-          && $background_job->borrowernumber != $patron->borrowernumber;
+          && $job->borrowernumber != $patron->borrowernumber;
 
         return $c->render(
             status  => 200,
-            openapi => $background_job->to_api
+            openapi => $job->to_api
         );
     }
     catch {

api/v1/swagger/definitions/background_job.yaml

@@ -1,63 +0,0 @@
----
-type: object
-properties:
-  background_job_id:
-    type: integer
-    description: internally assigned background_job identifier
-    readOnly: true
-  status:
-    description: background_job status
-    type:
-      - string
-      - "null"
-  progress:
-    description: background_job progress
-    type:
-      - string
-      - "null"
-  size:
-    description: background_job size
-    type:
-      - string
-      - "null"
-  patron_id:
-    description: background_job enqueuer
-    type:
-      - string
-      - "null"
-  type:
-    description: background_job type
-    type:
-      - string
-      - "null"
-  queue:
-    description: background_job queue
-    type:
-      - string
-      - "null"
-  data:
-    description: background_job data
-    type:
-      - string
-      - "null"
-  context:
-    description: background_job context
-    type:
-      - string
-      - "null"
-  enqueued_on:
-    description: background_job enqueue date
-    type:
-      - string
-      - "null"
-  started_on:
-    description: background_job start date
-    type:
-      - string
-      - "null"
-  ended_on:
-    description: background_job end date
-    type:
-      - string
-      - "null"
-additionalProperties: false

api/v1/swagger/definitions/job.yaml

@@ -0,0 +1,54 @@
+---
+type: object
+properties:
+  job_id:
+    type: integer
+    description: internally assigned job identifier
+    readOnly: true
+  status:
+    description: job status
+    type: string
+  progress:
+    description: job progress
+    type:
+      - string
+      - "null"
+  size:
+    description: job size
+    type:
+      - string
+      - "null"
+  patron_id:
+    description: job enqueuer
+    type:
+      - string
+      - "null"
+  type:
+    description: job type
+    type: string
+  queue:
+    description: job queue
+    type: string
+  data:
+    description: job data
+    type: object
+  context:
+    description: job context
+    type: object
+  enqueued_date:
+    description: job enqueue date
+    type: string
+    format: date-time
+  started_date:
+    description: job start date
+    type:
+      - string
+      - "null"
+    format: date-time
+  ended_date:
+    description: job end date
+    type:
+      - string
+      - "null"
+    format: date-time
+additionalProperties: false

api/v1/swagger/paths/jobs.yaml

@@ -1,64 +1,19 @@
 ---
-/background_jobs:
+/jobs:
   get:
     x-mojo-to: BackgroundJobs#list
-    operationId: listBackgroundJobs
+    operationId: listJobs
     tags:
-      - background_jobs
-    summary: List background jobs
+      - jobs
+    summary: List jobs
     produces:
       - application/json
     parameters:
-      - name: status
+      - name: only_current
         in: query
-        description: Case insensative search on job status
         required: false
-        type: string
-      - name: progress
-        in: query
-        description: Case insensative search on job progress
-        required: false
-        type: string
-      - name: size
-        in: query
-        description: Case insensative search on job size
-        required: false
-        type: string
-      - name: patron_id
-        in: query
-        description: Case insensative search on job enqueuer id
-        required: false
-        type: string
-      - name: type
-        in: query
-        description: Case insensative search on job type
-        required: false
-        type: string
-      - name: queue
-        in: query
-        description: Case insensative search on job queue
-        required: false
-        type: string
-      - name: context
-        in: query
-        description: Case insensative search on context
-        required: false
-        type: string
-      - name: enqueued_on
-        in: query
-        description: Case insensative search on job enqueue date
-        required: false
-        type: string
-      - name: started_on
-        in: query
-        description: Case insensative search on job start date
-        required: false
-        type: string
-      - name: ended_on
-        in: query
-        description: Case insensative search on job end date
-        required: false
-        type: string
+        type: boolean
+        description: If should filter out not current jobs
       - $ref: "../swagger.yaml#/parameters/match"
       - $ref: "../swagger.yaml#/parameters/order_by"
       - $ref: "../swagger.yaml#/parameters/page"
@@ -73,7 +28,7 @@
         schema:
           type: array
           items:
-            $ref: "../swagger.yaml#/definitions/background_job"
+            $ref: "../swagger.yaml#/definitions/job"
       "403":
         description: Access forbidden
         schema:
@@ -92,28 +47,28 @@
     x-koha-authorization:
       permissions:
         catalogue: "1"
-"/background_jobs/{background_job_id}":
+"/jobs/{job_id}":
   get:
     x-mojo-to: BackgroundJobs#get
-    operationId: getBackgroundJob
+    operationId: getJob
     tags:
-      - background_job
-    summary: Get background job
+      - jobs
+    summary: Get a job
     parameters:
-      - $ref: "../swagger.yaml#/parameters/background_job_id_pp"
+      - $ref: "../swagger.yaml#/parameters/job_id_pp"
     produces:
       - application/json
     responses:
       "200":
-        description: A background job
+        description: A job
         schema:
-          $ref: "../swagger.yaml#/definitions/background_job"
+          $ref: "../swagger.yaml#/definitions/job"
       "403":
         description: Access forbidden
         schema:
           $ref: "../swagger.yaml#/definitions/error"
       "404":
-        description: Background job not found
+        description: Job not found
         schema:
           $ref: "../swagger.yaml#/definitions/error"
       "500":

api/v1/swagger/swagger.yaml

@@ -8,8 +8,6 @@ definitions:
     $ref: ./definitions/advancededitormacro.yaml
   allows_renewal:
     $ref: ./definitions/allows_renewal.yaml
-  background_job:
-    $ref: ./definitions/background_job.yaml
   basket:
     $ref: ./definitions/basket.yaml
   bundle_link:
@@ -48,6 +46,8 @@ definitions:
     $ref: ./definitions/item.yaml
   item_group:
     $ref: ./definitions/item_group.yaml
+  job:
+    $ref: ./definitions/job.yaml
   library:
     $ref: ./definitions/library.yaml
   order:
@@ -105,10 +105,6 @@ paths:
     $ref: "./paths/article_requests.yaml#/~1article_requests~1{article_request_id}"
   /auth/otp/token_delivery:
     $ref: paths/auth.yaml#/~1auth~1otp~1token_delivery
-  /background_jobs:
-    $ref: ./paths/background_jobs.yaml#/~1background_jobs
-  "/background_jobs/{background_job_id}":
-    $ref: "./paths/background_jobs.yaml#/~1background_jobs~1{background_job_id}"
   "/biblios/{biblio_id}":
     $ref: "./paths/biblios.yaml#/~1biblios~1{biblio_id}"
   "/biblios/{biblio_id}/checkouts":
@@ -185,6 +181,10 @@ paths:
     $ref: ./paths/items.yaml#/~1items~1{item_id}~1bundled_items~1{bundled_item_id}
   "/items/{item_id}/pickup_locations":
     $ref: "./paths/items.yaml#/~1items~1{item_id}~1pickup_locations"
+  /jobs:
+    $ref: ./paths/jobs.yaml#/~1jobs
+  "/jobs/{job_id}":
+    $ref: "./paths/jobs.yaml#/~1jobs~1{job_id}"
   /libraries:
     $ref: ./paths/libraries.yaml#/~1libraries
   "/libraries/{library_id}":
@@ -260,12 +260,6 @@ parameters:
     name: advancededitormacro_id
     required: true
     type: integer
-  background_job_id_pp:
-    description: Job internal identifier
-    in: path
-    name: background_job_id
-    required: true
-    type: integer
   biblio_id_pp:
     description: Record internal identifier
     in: path
@@ -338,6 +332,12 @@ parameters:
     name: item_id
     required: true
     type: integer
+  job_id_pp:
+    description: Job internal identifier
+    in: path
+    name: job_id
+    required: true
+    type: integer
   library_id_pp:
     description: Internal library identifier
     in: path
@@ -604,6 +604,9 @@ tags:
   - description: "Manage items\n"
     name: items
     x-displayName: Items
+  - description: "Manage jobs\n"
+    name: jobs
+    x-displayName: Jobs
   - description: "Manage libraries\n"
     name: libraries
     x-displayName: Libraries

t/db_dependent/api/v1/jobs.t

@@ -65,11 +65,11 @@ my $patron = $builder->build_object(
 $patron->set_password( { password => $password, skip_validation => 1 } );
 my $patron_userid = $patron->userid;
 
-$t->get_ok("//$librarian_userid:$password@/api/v1/background_jobs")
+$t->get_ok("//$librarian_userid:$password@/api/v1/jobs")
   ->status_is(200)
   ->json_is( [] );
 
-my $background_job = $builder->build_object(
+my $job = $builder->build_object(
     {
         class => 'Koha::BackgroundJobs',
         value => {
@@ -86,39 +86,39 @@ my $background_job = $builder->build_object(
 );
 
 {
-    $t->get_ok("//$superlibrarian_userid:$password@/api/v1/background_jobs")
-      ->status_is(200)->json_is( [ $background_job->to_api ] );
+    $t->get_ok("//$superlibrarian_userid:$password@/api/v1/jobs")
+      ->status_is(200)->json_is( [ $job->to_api ] );
 
-    $t->get_ok("//$librarian_userid:$password@/api/v1/background_jobs")
+    $t->get_ok("//$librarian_userid:$password@/api/v1/jobs")
       ->status_is(200)->json_is( [] );
 
-    $t->get_ok("//$patron_userid:$password@/api/v1/background_jobs")
+    $t->get_ok("//$patron_userid:$password@/api/v1/jobs")
       ->status_is(403);
 
-    $background_job->borrowernumber( $librarian->borrowernumber )->store;
+    $job->borrowernumber( $librarian->borrowernumber )->store;
 
-    $t->get_ok("//$librarian_userid:$password@/api/v1/background_jobs")
-      ->status_is(200)->json_is( [ $background_job->to_api ] );
+    $t->get_ok("//$librarian_userid:$password@/api/v1/jobs")
+      ->status_is(200)->json_is( [ $job->to_api ] );
 }
 
 {
-    $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/background_jobs/"
-          . $background_job->id )->status_is(200)
-      ->json_is( $background_job->to_api );
+    $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/jobs/"
+          . $job->id )->status_is(200)
+      ->json_is( $job->to_api );
 
-    $t->get_ok( "//$librarian_userid:$password@/api/v1/background_jobs/"
-          . $background_job->id )->status_is(200)
-      ->json_is( $background_job->to_api );
+    $t->get_ok( "//$librarian_userid:$password@/api/v1/jobs/"
+          . $job->id )->status_is(200)
+      ->json_is( $job->to_api );
 
-    $background_job->borrowernumber( $superlibrarian->borrowernumber )->store;
-    $t->get_ok( "//$librarian_userid:$password@/api/v1/background_jobs/"
-          . $background_job->id )->status_is(403);
+    $job->borrowernumber( $superlibrarian->borrowernumber )->store;
+    $t->get_ok( "//$librarian_userid:$password@/api/v1/jobs/"
+          . $job->id )->status_is(403);
 }
 
 {
-    $background_job->delete;
-    $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/background_jobs/"
-          . $background_job->id )->status_is(404)
+    $job->delete;
+    $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/jobs/"
+          . $job->id )->status_is(404)
       ->json_is( '/error' => 'Object not found' );
 }
 

t/lib/TestBuilder.pm

@@ -558,6 +558,9 @@ sub _gen_blob {
 sub _gen_default_values {
     my ($self) = @_;
     return {
+        BackgroundJob => {
+            context => '{}'
+        },
         Borrower => {
             login_attempts => 0,
             gonenoaddress  => undef,