Attachment 65109 Details for Bug 18137 – Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible

[patch] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible

Bug-18137-Make-cities-MojoliciousPluginOpenAPI-com.patch (text/plain), 10.47 KB, created by Tomás Cohen Arazi (tcohen) on 2017-07-19 16:34:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2017-07-19 16:34:39 UTC

Size: 10.47 KB

patch

obsolete

>From d2c03085c48018995e2144eb9f69fbd2865151eb Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@jns.fi>
>Date: Mon, 20 Feb 2017 19:58:28 +0200
>Subject: [PATCH] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI
> compatible
>
>Also:
>- adding some missing and new response definitions into Swagger spec.
>
>To test:
>1. prove t/db_dependent/api/v1/cities.t
>
>Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
>
>Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/REST/V1/Cities.pm           | 71 ++++++++++++++++++++++------------------
> api/v1/swagger/paths/cities.json | 64 +++++++++++++++++++++++++++++-------
> t/db_dependent/api/v1/cities.t   |  2 +-
> 3 files changed, 94 insertions(+), 43 deletions(-)
>
>diff --git a/Koha/REST/V1/Cities.pm b/Koha/REST/V1/Cities.pm
>index 5d32ea9..0feb36a 100644
>--- a/Koha/REST/V1/Cities.pm
>+++ b/Koha/REST/V1/Cities.pm
>@@ -25,11 +25,11 @@ use Koha::Cities;
> use Try::Tiny;
> 
> sub list {
>-    my ( $c, $args, $cb ) = @_;
>+    my $c = shift->openapi->valid_input or return;
> 
>     my $cities;
>     my $filter;
>-    $args //= {};
>+    my $args = $c->req->params->to_hash;
> 
>     for my $filter_param ( keys %$args ) {
>         $filter->{$filter_param} = { LIKE => $args->{$filter_param} . "%" };
>@@ -37,96 +37,105 @@ sub list {
> 
>     return try {
>         $cities = Koha::Cities->search($filter);
>-        return $c->$cb( $cities, 200 );
>+        return $c->render( status => 200, openapi => $cities );
>     }
>     catch {
>         if ( $_->isa('DBIx::Class::Exception') ) {
>-            return $c->$cb( { error => $_->{msg} }, 500 );
>+            return $c->render( status  => 500,
>+                               openapi => { error => $_->msg } );
>         }
>         else {
>-            return $c->$cb(
>-                { error => "Something went wrong, check the logs." }, 500 );
>+            return $c->render( status => 500,
>+                openapi => { error => "Something went wrong, check the logs."} );
>         }
>     };
>+
> }
> 
> sub get {
>-    my ( $c, $args, $cb ) = @_;
>+    my $c = shift->openapi->valid_input or return;
> 
>-    my $city = Koha::Cities->find( $args->{cityid} );
>+    my $city = Koha::Cities->find( $c->validation->param('cityid') );
>     unless ($city) {
>-        return $c->$cb( { error => "City not found" }, 404 );
>+        return $c->render( status  => 404,
>+                           openapi => { error => "City not found" } );
>     }
> 
>-    return $c->$cb( $city, 200 );
>+    return $c->render( status => 200, openapi => $city );
> }
> 
> sub add {
>-    my ( $c, $args, $cb ) = @_;
>+    my $c = shift->openapi->valid_input or return;
> 
>-    my $city = Koha::City->new( $args->{body} );
>+    my $city = Koha::City->new( $c->validation->param('body') );
> 
>     return try {
>         $city->store;
>-        return $c->$cb( $city, 200 );
>+        return $c->render( status => 200, openapi => $city );
>     }
>     catch {
>         if ( $_->isa('DBIx::Class::Exception') ) {
>-            return $c->$cb( { error => $_->msg }, 500 );
>+            return $c->render( status  => 500,
>+                               openapi => { error => $_->message } );
>         }
>         else {
>-            return $c->$cb(
>-                { error => "Something went wrong, check the logs." }, 500 );
>+            return $c->render( status => 500,
>+                openapi => { error => "Something went wrong, check the logs."} );
>         }
>     };
> }
> 
> sub update {
>-    my ( $c, $args, $cb ) = @_;
>+    my $c = shift->openapi->valid_input or return;
> 
>     my $city;
> 
>     return try {
>-        $city = Koha::Cities->find( $args->{cityid} );
>-        $city->set( $args->{body} );
>+        $city = Koha::Cities->find( $c->validation->param('cityid') );
>+        my $params = $c->req->json;
>+        $city->set( $params );
>         $city->store();
>-        return $c->$cb( $city, 200 );
>+        return $c->render( status => 200, openapi => $city );
>     }
>     catch {
>         if ( not defined $city ) {
>-            return $c->$cb( { error => "Object not found" }, 404 );
>+            return $c->render( status  => 404,
>+                               openapi => { error => "Object not found" } );
>         }
>         elsif ( $_->isa('Koha::Exceptions::Object') ) {
>-            return $c->$cb( { error => $_->message }, 500 );
>+            return $c->render( status  => 500,
>+                               openapi => { error => $_->message } );
>         }
>         else {
>-            return $c->$cb(
>-                { error => "Something went wrong, check the logs." }, 500 );
>+            return $c->render( status => 500,
>+                openapi => { error => "Something went wrong, check the logs."} );
>         }
>     };
> 
> }
> 
> sub delete {
>-    my ( $c, $args, $cb ) = @_;
>+    my $c = shift->openapi->valid_input or return;
> 
>     my $city;
> 
>     return try {
>-        $city = Koha::Cities->find( $args->{cityid} );
>+        $city = Koha::Cities->find( $c->validation->param('cityid') );
>         $city->delete;
>-        return $c->$cb( "", 200 );
>+        return $c->render( status => 200, openapi => "" );
>     }
>     catch {
>         if ( not defined $city ) {
>-            return $c->$cb( { error => "Object not found" }, 404 );
>+            return $c->render( status  => 404,
>+                               openapi => { error => "Object not found" } );
>         }
>         elsif ( $_->isa('DBIx::Class::Exception') ) {
>-            return $c->$cb( { error => $_->msg }, 500 );
>+            return $c->render( status  => 500,
>+                               openapi => { error => $_->msg } );
>         }
>         else {
>-            return $c->$cb(
>-                { error => "Something went wrong, check the logs." }, 500 );
>+            return $c->render( status => 500,
>+                openapi => { error => "Something went wrong, check the logs."} );
>         }
>     };
> 
>diff --git a/api/v1/swagger/paths/cities.json b/api/v1/swagger/paths/cities.json
>index 650d224..8b739d6 100644
>--- a/api/v1/swagger/paths/cities.json
>+++ b/api/v1/swagger/paths/cities.json
>@@ -1,7 +1,7 @@
> {
>   "/cities": {
>     "get": {
>-      "x-mojo-controller": "Koha::REST::V1::Cities",
>+      "x-mojo-to": "Cities#list",
>       "operationId": "list",
>       "tags": ["cities"],
>       "produces": [
>@@ -53,11 +53,17 @@
>           "schema": {
>             "$ref": "../definitions.json#/error"
>           }
>+        },
>+        "503": {
>+          "description": "Under maintenance",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>         }
>       }
>     },
>     "post": {
>-      "x-mojo-controller": "Koha::REST::V1::Cities",
>+      "x-mojo-to": "Cities#add",
>       "operationId": "add",
>       "tags": ["cities"],
>       "parameters": [{
>@@ -79,6 +85,12 @@
>             "$ref": "../definitions.json#/city"
>           }
>         },
>+        "401": {
>+          "description": "Authentication required",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>         "403": {
>           "description": "Access forbidden",
>           "schema": {
>@@ -90,6 +102,12 @@
>           "schema": {
>             "$ref": "../definitions.json#/error"
>           }
>+        },
>+        "503": {
>+          "description": "Under maintenance",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>         }
>       },
>       "x-koha-authorization": {
>@@ -101,7 +119,7 @@
>   },
>   "/cities/{cityid}": {
>     "get": {
>-      "x-mojo-controller": "Koha::REST::V1::Cities",
>+      "x-mojo-to": "Cities#get",
>       "operationId": "get",
>       "tags": ["cities"],
>       "parameters": [{
>@@ -117,12 +135,6 @@
>             "$ref": "../definitions.json#/city"
>           }
>         },
>-        "403": {
>-          "description": "Access forbidden",
>-          "schema": {
>-            "$ref": "../definitions.json#/error"
>-          }
>-        },
>         "404": {
>           "description": "City not found",
>           "schema": {
>@@ -134,11 +146,17 @@
>           "schema": {
>             "$ref": "../definitions.json#/error"
>           }
>+        },
>+        "503": {
>+          "description": "Under maintenance",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>         }
>       }
>     },
>     "put": {
>-      "x-mojo-controller": "Koha::REST::V1::Cities",
>+      "x-mojo-to": "Cities#update",
>       "operationId": "update",
>       "tags": ["cities"],
>       "parameters": [{
>@@ -162,6 +180,12 @@
>             "$ref": "../definitions.json#/city"
>           }
>         },
>+        "401": {
>+          "description": "Authentication required",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>         "403": {
>           "description": "Access forbidden",
>           "schema": {
>@@ -179,6 +203,12 @@
>           "schema": {
>             "$ref": "../definitions.json#/error"
>           }
>+        },
>+        "503": {
>+          "description": "Under maintenance",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>         }
>       },
>       "x-koha-authorization": {
>@@ -188,7 +218,7 @@
>       }
>     },
>     "delete": {
>-      "x-mojo-controller": "Koha::REST::V1::Cities",
>+      "x-mojo-to": "Cities#delete",
>       "operationId": "delete",
>       "tags": ["cities"],
>       "parameters": [{
>@@ -204,6 +234,12 @@
>             "type": "string"
>           }
>         },
>+        "401": {
>+          "description": "Authentication required",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>         "403": {
>           "description": "Access forbidden",
>           "schema": {
>@@ -221,6 +257,12 @@
>           "schema": {
>             "$ref": "../definitions.json#/error"
>           }
>+        },
>+        "503": {
>+          "description": "Under maintenance",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>         }
>       },
>       "x-koha-authorization": {
>diff --git a/t/db_dependent/api/v1/cities.t b/t/db_dependent/api/v1/cities.t
>index e6e13c6..ac8fe43 100644
>--- a/t/db_dependent/api/v1/cities.t
>+++ b/t/db_dependent/api/v1/cities.t
>@@ -332,7 +332,7 @@ subtest 'delete() tests' => sub {
>     $tx->req->cookies(
>         { name => 'CGISESSID', value => $authorized_session_id } );
>     $tx->req->env( { REMOTE_ADDR => $remote_address } );
>-    $t->request_ok($tx)->status_is(200)->content_is('');
>+    $t->request_ok($tx)->status_is(200)->content_is('""');
> 
>     $tx = $t->ua->build_tx( DELETE => "/api/v1/cities/$city_id" );
>     $tx->req->cookies(
>-- 
>2.7.4

Actions: View | Diff | Splinter Review

Attachments on bug 18137: 60490 | 60491 | 60492 | 60493 | 60494 | 60495 | 60533 | 60837 | 60838 | 60839 | 60840 | 60841 | 60842 | 60843 | 60866 | 60867 | 61163 | 61166 | 62780 | 62781 | 62782 | 62783 | 62784 | 62785 | 62786 | 62787 | 62788 | 62789 | 62816 | 63025 | 63026 | 63027 | 63028 | 63029 | 63030 | 63031 | 63032 | 63033 | 63034 | 63177 | 63178 | 63179 | 63180 | 63181 | 63182 | 63183 | 63184 | 63185 | 63186 | 63187 | 63188 | 63189 | 63190 | 63191 | 63192 | 63193 | 63194 | 63195 | 63196 | 63283 | 65106 | 65107 | 65108 | 65109 | 65110 | 65111 | 65112 | 65113 | 65114 | 65115 | 65404 | 65713 | 65714 | 66802 | 66803 | 66804 | 66805 | 66806 | 66807 | 66808 | 66809 | 66810 | 66811 | 66812 | 66813 | 66814 | 66815 | 66816 | 66817 | 66818 | 66819