Attachment 148269 Details for Bug 33227 – Bug 33227: Remove invalid spec and adjust the code accordingly

[patch] Bug 33227: Remove invalid spec and adjust the code accordingly

Bug-33227-Remove-invalid-spec-and-adjust-the-code-.patch (text/plain), 8.05 KB, created by Tomás Cohen Arazi (tcohen) on 2023-03-16 15:13:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2023-03-16 15:13:34 UTC

Size: 8.05 KB

patch

obsolete

>From b4c7f30bbd7bb21aff0af8f7312a267235bcc8a1 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 33227: 148213 | 148214 | 148268 | 148269 | 148407 | 148408 | 148410 | 148411 | 168707