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>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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(
+            return $c->render( status => 500,
-                { error => "Something went wrong, check the logs." }, 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(
+            return $c->render( status => 500,
-                { error => "Something went wrong, check the logs." }, 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 = Koha::Cities->find( $c->validation->param('cityid') );
-        $city->set( $args->{body} );
+        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(
+            return $c->render( status => 500,
-                { error => "Something went wrong, check the logs." }, 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(
+            return $c->render( status => 500,
-                { error => "Something went wrong, check the logs." }, 500 );
+                openapi => { error => "Something went wrong, check the logs."} );
         }
     };
 

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": {

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(