From d03d15125337a4e647e0ea8696f9e01a10e728ff Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 12 Sep 2022 13:02:25 -0300 Subject: [PATCH] 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 Signed-off-by: Martin Renvoize --- Koha/BackgroundJob.pm | 26 ++++++- Koha/REST/V1/BackgroundJobs.pm | 39 +++++++--- .../swagger/definitions/background_job.yaml | 63 --------------- api/v1/swagger/definitions/job.yaml | 54 +++++++++++++ .../paths/{background_jobs.yaml => jobs.yaml} | 77 ++++--------------- api/v1/swagger/swagger.yaml | 27 ++++--- .../api/v1/{background_jobs.t => jobs.t} | 42 +++++----- t/lib/TestBuilder.pm | 3 + 8 files changed, 162 insertions(+), 169 deletions(-) delete mode 100644 api/v1/swagger/definitions/background_job.yaml create mode 100644 api/v1/swagger/definitions/job.yaml rename api/v1/swagger/paths/{background_jobs.yaml => jobs.yaml} (50%) rename t/db_dependent/api/v1/{background_jobs.t => jobs.t} (68%) diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index 2a2e9a4185..5b14a78ef2 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -469,6 +469,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 @@ -478,8 +499,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', }; } diff --git a/Koha/REST/V1/BackgroundJobs.pm b/Koha/REST/V1/BackgroundJobs.pm index 251d5657f3..cb1543d0fe 100644 --- a/Koha/REST/V1/BackgroundJobs.pm +++ b/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 { diff --git a/api/v1/swagger/definitions/background_job.yaml b/api/v1/swagger/definitions/background_job.yaml deleted file mode 100644 index 09cbbe652c..0000000000 --- a/api/v1/swagger/definitions/background_job.yaml +++ /dev/null @@ -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 diff --git a/api/v1/swagger/definitions/job.yaml b/api/v1/swagger/definitions/job.yaml new file mode 100644 index 0000000000..966e3a3ef8 --- /dev/null +++ b/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 diff --git a/api/v1/swagger/paths/background_jobs.yaml b/api/v1/swagger/paths/jobs.yaml similarity index 50% rename from api/v1/swagger/paths/background_jobs.yaml rename to api/v1/swagger/paths/jobs.yaml index ecc80a21f2..c7f0f98904 100644 --- a/api/v1/swagger/paths/background_jobs.yaml +++ b/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": diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 3d4be6e63d..5bdbab2fa8 100644 --- a/api/v1/swagger/swagger.yaml +++ b/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 diff --git a/t/db_dependent/api/v1/background_jobs.t b/t/db_dependent/api/v1/jobs.t similarity index 68% rename from t/db_dependent/api/v1/background_jobs.t rename to t/db_dependent/api/v1/jobs.t index 11181b13a8..4d993c68e0 100755 --- a/t/db_dependent/api/v1/background_jobs.t +++ b/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' ); } diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm index 59ba592da0..d6e88e7b8a 100644 --- a/t/lib/TestBuilder.pm +++ b/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, -- 2.20.1