From b4c7f30bbd7bb21aff0af8f7312a267235bcc8a1 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 --- 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 61332553a1a..9227a6af8a4 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 f8cfe00b021..5fdf83bc07c 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 c401b1e1aee..afff055e360 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.34.1