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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm
index 6133255..9227a6a 100644 (file)
--- 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 f8cfe00..5fdf83b 100644 (file)
--- 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 c401b1e..afff055 100755 (executable)
--- 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)