From 9913a6c00d0abe2fd648da9246873dc7e1852a08 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 10 Apr 2023 14:14:12 -0300 Subject: [PATCH] Bug 31795: (QA follow-up) Use x-koha-override header Despite its title, this patch does a couple more things. 1. The tests are rewritten to cover more things, and also to avoid deleting all authorities inside the transaction. It is really not required. 2. It makes the endpoint rely on the already generically implemented x-koha-override header, which is intended for the same use case as x-confirm-not-duplicate is for. 3. It changes the return codes to match the coding guidelines [1] 4. Only checks for duplicates if no override passed. To test: 1. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => SUCCESS: Tests pass! 2. Apply this follow-up 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D [1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.1_POST Signed-off-by: Tomas Cohen Arazi Signed-off-by: David Nind --- Koha/REST/V1/Authorities.pm | 67 +++++++++++++-------------- api/v1/swagger/paths/authorities.yaml | 45 ++++++++++++------ t/db_dependent/api/v1/authorities.t | 50 +++++++++++++------- 3 files changed, 96 insertions(+), 66 deletions(-) diff --git a/Koha/REST/V1/Authorities.pm b/Koha/REST/V1/Authorities.pm index eac54c3df5..8a6c24b2c1 100644 --- a/Koha/REST/V1/Authorities.pm +++ b/Koha/REST/V1/Authorities.pm @@ -138,7 +138,8 @@ sub add { my $c = shift->openapi->valid_input or return; try { - my $body = $c->validation->param('Body'); + my $headers = $c->req->headers; + my $overrides = $c->stash('koha.overrides'); my $flavour = C4::Context->preference('marcflavour') eq 'UNIMARC' @@ -146,49 +147,45 @@ sub add { : 'MARC21'; my $record; - my $authtypecode; - if ( $c->req->headers->content_type =~ m/application\/json/ ) { - $record = MARC::Record->new_from_xml( $body->{marcxml}, 'UTF-8', $flavour ); - $authtypecode = $body->{authtypecode}; + my $authtypecode = $headers->header('x-authority-type'); + my $content_type = $headers->content_type; + + if ( $content_type =~ m/application\/marcxml\+xml/ ) { + $record = MARC::Record->new_from_xml( $c->req->body, 'UTF-8', $flavour ); + } elsif ( $content_type =~ m/application\/marc-in-json/ ) { + $record = MARC::Record->new_from_mij_structure( $c->req->json ); + } elsif ( $content_type =~ m/application\/marc/ ) { + $record = MARC::Record->new_from_usmarc( $c->req->body ); } else { - $authtypecode = $c->validation->param('x-authority-type'); - if ( $c->req->headers->content_type =~ m/application\/marcxml\+xml/ ) { - $record = MARC::Record->new_from_xml( $body, 'UTF-8', $flavour ); - } elsif ( $c->req->headers->content_type =~ m/application\/marc-in-json/ ) { - $record = MARC::Record->new_from_mij_structure( $body ); - } elsif ( $c->req->headers->content_type =~ m/application\/marc/ ) { - $record = MARC::Record->new_from_usmarc( $body ); - } else { - return $c->render( - status => 406, - openapi => [ - "application/json", - "application/marcxml+xml", - "application/marc-in-json", - "application/marc" - ] - ); - } + return $c->render( + status => 406, + openapi => [ + "application/marcxml+xml", + "application/marc-in-json", + "application/marc" + ] + ); } - my ($duplicateauthid,$duplicateauthvalue); - ($duplicateauthid,$duplicateauthvalue) = FindDuplicateAuthority($record,$authtypecode); - - my $confirm_not_duplicate = $c->validation->param('x-confirm-not-duplicate'); + unless ( $overrides->{any} || $overrides->{duplicate} ) { + my ( $duplicateauthid, $duplicateauthvalue ) = FindDuplicateAuthority( $record, $authtypecode ); - return $c->render( - status => 400, - openapi => { - error => "Duplicate authority $duplicateauthid" - } - ) unless !$duplicateauthid || $confirm_not_duplicate; + return $c->render( + status => 409, + openapi => { + error => "Duplicate record ($duplicateauthid)", + error_code => 'duplicate', + } + ) unless !$duplicateauthid; + } my $authid = AddAuthority( $record, undef, $authtypecode ); + $c->res->headers->location($c->req->url->to_string . '/' . $authid); $c->render( - status => 200, - openapi => { id => $authid } + status => 201, + openapi => q{}, ); } catch { diff --git a/api/v1/swagger/paths/authorities.yaml b/api/v1/swagger/paths/authorities.yaml index a6575ce809..e733cf764e 100644 --- a/api/v1/swagger/paths/authorities.yaml +++ b/api/v1/swagger/paths/authorities.yaml @@ -6,24 +6,36 @@ tags: - authorities summary: Add authority + description: | + Add an authority record to Koha. An optional `x-authority-type` + may be passed to specify the cataloguing framework to be used (instead + of the default). + + The request body is expected to contain a MARC record in the format specified in + the `Content-type` header you pass. Possible values for this header and the corresponding + record formats expected are listed below: + + * application/marcxml+xml: MARCXML + * application/marc-in-json: MARC-in-JSON + * application/marc: Raw USMARC binary data parameters: - - name: Body - in: body - description: A JSON object or the Marc string describing an authority - required: true - schema: - type: - - string - - object - $ref: "../swagger.yaml#/parameters/authority_type_header" - - $ref: "../swagger.yaml#/parameters/confirm_not_duplicate_header" - produces: - - application/json + - name: x-koha-override + in: header + required: false + description: Overrides list sent as a request header + type: array + items: + type: string + enum: + - any + - duplicate + collectionFormat: csv responses: - "200": + "201": description: An authority "400": - description: Bad request + description: Bad request. schema: $ref: "../swagger.yaml#/definitions/error" "401": @@ -41,6 +53,13 @@ description: Accepted content-types items: type: string + "409": + description: | + Conflict creating the resource. Possible `error_code` attribute values: + + * `duplicate` + schema: + $ref: "../swagger.yaml#/definitions/error" "500": description: | Internal server error. Possible `error_code` attribute values: diff --git a/t/db_dependent/api/v1/authorities.t b/t/db_dependent/api/v1/authorities.t index 938fbfd214..1ead19fc8d 100755 --- a/t/db_dependent/api/v1/authorities.t +++ b/t/db_dependent/api/v1/authorities.t @@ -155,12 +155,10 @@ subtest 'delete() tests' => sub { subtest 'post() tests' => sub { - plan tests => 14; + plan tests => 19; $schema->storage->txn_begin; - Koha::Authorities->delete; - my $patron = $builder->build_object( { class => 'Koha::Patrons', @@ -202,21 +200,37 @@ subtest 'post() tests' => sub { } ); - $t->post_ok("//$userid:$password@/api/v1/authorities" => json => $json) - ->status_is(200) - ->json_has('/id'); - - $t->post_ok("//$userid:$password@/api/v1/authorities" => {'Content-Type' => 'application/marcxml+xml', 'x-authority-type' => 'CORPO_NAME'} => $marcxml) - ->status_is(200) - ->json_has('/id'); - - $t->post_ok("//$userid:$password@/api/v1/authorities" => {'Content-Type' => 'application/marc-in-json', 'x-authority-type' => 'CORPO_NAME'} => $mij) - ->status_is(200) - ->json_has('/id'); - - $t->post_ok("//$userid:$password@/api/v1/authorities" => {'Content-Type' => 'application/marc', 'x-authority-type' => 'CORPO_NAME'} => $marc) - ->status_is(200) - ->json_has('/id'); + # x-koha-override passed to make sure it goes through + $t->post_ok("//$userid:$password@/api/v1/authorities" => {'Content-Type' => 'application/marcxml+xml', 'x-authority-type' => 'CORPO_NAME', 'x-koha-override' => 'any' } => $marcxml) + ->status_is(201) + ->json_is(q{}) + ->header_like( + Location => qr|^\/api\/v1\/authorities/\d*|, + 'SWAGGER3.4.1' + ); + + # x-koha-override not passed to force block because duplicate + $t->post_ok("//$userid:$password@/api/v1/authorities" => {'Content-Type' => 'application/marc-in-json', 'x-authority-type' => 'CORPO_NAME' } => $mij) + ->status_is(409) + ->header_exists_not( 'Location', 'Location header is only set when the new resource is created' ) + ->json_like( '/error' => qr/Duplicate record (\d*)/ ) + ->json_is( '/error_code' => q{duplicate} ); + + $t->post_ok("//$userid:$password@/api/v1/authorities" => {'Content-Type' => 'application/marc-in-json', 'x-authority-type' => 'CORPO_NAME', 'x-koha-override' => 'duplicate' } => $mij) + ->status_is(201) + ->json_is(q{}) + ->header_like( + Location => qr|^\/api\/v1\/authorities/\d*|, + 'SWAGGER3.4.1' + ); + + $t->post_ok("//$userid:$password@/api/v1/authorities" => {'Content-Type' => 'application/marc', 'x-authority-type' => 'CORPO_NAME', 'x-koha-override' => 'duplicate' } => $marc) + ->status_is(201) + ->json_is(q{}) + ->header_like( + Location => qr|^\/api\/v1\/authorities/\d*|, + 'SWAGGER3.4.1' + ); $schema->storage->txn_rollback; }; -- 2.30.2