From e50b57a7b4ac050b918364af9867df7b39d0a33b 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 --- 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 eac54c3df5b..8a6c24b2c1f 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 a6575ce8095..e733cf764ee 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 938fbfd214b..1ead19fc8d4 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.34.1