From 185b431ebfc869dde06ab87efcd562fc786388ae Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 16 Mar 2023 10:50:33 -0300 Subject: [PATCH] Bug 33227: Remove invalid spec and adjust the code accordingly The spec contains a non-standard definition for the body param. Removing it from the spec makes us need to handle the 'body' manually in the case of JSON data. This patch basically does that. It also changes the uses of $c->validation, which are discouraged by the Mojolicious::Plugin::OpenAPI dev/maintainer. I do it to highlight what we must do in other places and the fact that there's no behavior change. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t \ xt/api.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Biblios.pm | 70 +++++++++++++++++++++++---------------- api/v1/swagger/paths/biblios.yaml | 16 --------- t/db_dependent/api/v1/biblios.t | 2 +- 3 files changed, 42 insertions(+), 46 deletions(-) diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index 61332553a1..9227a6af8a 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -491,21 +491,26 @@ sub add { my $c = shift->openapi->valid_input or return; try { - my $body = $c->validation->param('Body'); + my $headers = $c->req->headers; - my $flavour = $c->validation->param('x-marc-schema'); - $flavour = C4::Context->preference('marcflavour') unless $flavour; + my $flavour = $headers->header('x-marc-schema'); + $flavour //= C4::Context->preference('marcflavour'); my $record; - my $frameworkcode = $c->validation->param('x-framework-id'); - 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 { + my $frameworkcode = $headers->header('x-framework-id'); + 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 { return $c->render( status => 406, openapi => [ @@ -519,12 +524,12 @@ sub add { my ( $duplicatebiblionumber, $duplicatetitle ); ( $duplicatebiblionumber, $duplicatetitle ) = FindDuplicate($record); - my $confirm_not_duplicate = $c->validation->param('x-confirm-not-duplicate'); + my $confirm_not_duplicate = $headers->header('x-confirm-not-duplicate'); return $c->render( status => 400, openapi => { - error => "Duplicate biblio $duplicatebiblionumber" + error => "Duplicate biblio $duplicatebiblionumber", } ) unless !$duplicatebiblionumber || $confirm_not_duplicate; @@ -550,10 +555,10 @@ Controller function that handles modifying an biblio object sub update { my $c = shift->openapi->valid_input or return; - my $biblionumber = $c->validation->param('biblio_id'); - my $biblio = Koha::Biblios->find( $biblionumber ); + my $biblio_id = $c->param('biblio_id'); + my $biblio = Koha::Biblios->find($biblio_id); - if ( not defined $biblio ) { + if ( ! defined $biblio ) { return $c->render( status => 404, openapi => { error => "Object not found" } @@ -561,20 +566,27 @@ sub update { } try { - my $body = $c->validation->param('Body'); + my $headers = $c->req->headers; + + my $flavour = $headers->header('x-marc-schema'); + $flavour //= C4::Context->preference('marcflavour'); - my $flavour = $c->validation->param('x-marc-schema'); - $flavour = C4::Context->preference('marcflavour') unless $flavour; + my $frameworkcode = $headers->header('x-framework-id') || $biblio->frameworkcode; + + my $content_type = $headers->content_type; my $record; - my $frameworkcode = $c->validation->param('x-framework-id') || $biblio->frameworkcode; - 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 { + + 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 { return $c->render( status => 406, openapi => [ @@ -586,11 +598,11 @@ sub update { ); } - ModBiblio( $record, $biblionumber, $frameworkcode ); + ModBiblio( $record, $biblio_id, $frameworkcode ); $c->render( status => 200, - openapi => { id => $biblionumber } + openapi => { id => $biblio_id } ); } catch { diff --git a/api/v1/swagger/paths/biblios.yaml b/api/v1/swagger/paths/biblios.yaml index f8cfe00b02..5fdf83bc07 100644 --- a/api/v1/swagger/paths/biblios.yaml +++ b/api/v1/swagger/paths/biblios.yaml @@ -7,14 +7,6 @@ - biblios summary: Add biblio parameters: - - name: Body - in: body - description: A JSON object or the Marc string describing a biblio - required: true - schema: - type: - - string - - object - $ref: "../swagger.yaml#/parameters/framework_id_header" - $ref: "../swagger.yaml#/parameters/marc_schema_header" - $ref: "../swagger.yaml#/parameters/confirm_not_duplicate_header" @@ -214,14 +206,6 @@ summary: Update biblio parameters: - $ref: "../swagger.yaml#/parameters/biblio_id_pp" - - name: Body - in: body - description: A JSON object or the Marc string describing a biblio - required: true - schema: - type: - - string - - object - $ref: "../swagger.yaml#/parameters/framework_id_header" - $ref: "../swagger.yaml#/parameters/marc_schema_header" - $ref: "../swagger.yaml#/parameters/confirm_not_duplicate_header" diff --git a/t/db_dependent/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t index c401b1e1ae..afff055e36 100755 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -1125,7 +1125,7 @@ subtest 'post() tests' => sub { } ); - $t->post_ok("//$userid:$password@/api/v1/biblios" => {'Content-Type' => 'application/marcxml+xml', 'x-framework-id' => $frameworkcode, "x-march-schema" => 'INVALID'}) + $t->post_ok("//$userid:$password@/api/v1/biblios" => {'Content-Type' => 'application/marcxml+xml', 'x-framework-id' => $frameworkcode, "x-marc-schema" => 'INVALID'}) ->status_is(400, 'Invalid header x-marc-schema'); $t->post_ok("//$userid:$password@/api/v1/biblios" => {'Content-Type' => 'application/marcxml+xml', 'x-framework-id' => $frameworkcode} => $marcxml) -- 2.11.0