Bug 22522: Add support for current Mojolicious and related packages

This patch allows tests to succeed with the following versions:
JSON::Validator 3.18
Mojolicious 8.32
Mojolicious::Plugin::OpenAPI 2.21

Also Mojolicious::Plugin::OpenAPI version 1.17 and later 1.x versions now work.

Calling valid_input in under() would cause ' Use of uninitialized value $_[2] ' in more recent OpenAPI plugins, so that was changed too. As far as I can see this does not affect authorization.

Signed-off-by: Mason James <mason@kohaaloha.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Koha/REST/V1/Auth.pm

@@ -55,14 +55,14 @@ This subroutine is called before every request to API.
 =cut
 
 sub under {
-    my $c = shift->openapi->valid_input or return;;
+    my ( $c ) = @_;
 
     my $status = 0;
 
     try {
 
         # /api/v1/{namespace}
-        my $namespace = $c->req->url->to_abs->path->[2];
+        my $namespace = $c->req->url->to_abs->path->[2] // '';
 
         if ( $namespace eq 'public'
             and !C4::Context->preference('RESTPublicAPI') )
@@ -136,7 +136,10 @@ sub authenticate_api_request {
 
     my $user;
 
-    my $spec = $c->match->endpoint->pattern->defaults->{'openapi.op_spec'};
+    # The following supports retrieval of spec with Mojolicious::Plugin::OpenAPI@1.17 and later (first one)
+    # and older versions (second one).
+    # TODO: remove the latter 'openapi.op_spec' if minimum version is bumped to at least 1.17.
+    my $spec = $c->openapi->spec || $c->match->endpoint->pattern->defaults->{'openapi.op_spec'};
 
     $c->stash_embed({ spec => $spec });
 
@@ -205,7 +208,6 @@ sub authenticate_api_request {
         if ($status eq "ok") {
             my $session = get_session($sessionID);
             $user = Koha::Patrons->find($session->param('number'));
-            # $c->stash('koha.user' => $user);
         }
         elsif ($status eq "maintenance") {
             Koha::Exceptions::UnderMaintenance->throw(

t/db_dependent/Koha/REST/Plugin/Objects.t

@@ -69,14 +69,15 @@ my $builder = t::lib::TestBuilder->new;
 
 subtest 'objects.search helper' => sub {
 
-    plan tests => 34;
+    plan tests => 38;
 
     $schema->storage->txn_begin;
 
-    # Remove existing cities to have more control on the search restuls
+    # Remove existing cities to have more control on the search results
     Koha::Cities->delete;
 
-    # Create two sample patrons that match the query
+    # Create three sample cities that match the query. This makes sure we
+    # always have a "next" link regardless of Mojolicious::Plugin::OpenAPI version.
     $builder->build_object({
         class => 'Koha::Cities',
         value => {
@@ -89,10 +90,16 @@ subtest 'objects.search helper' => sub {
             city_name => 'Manuela'
         }
     });
+    $builder->build_object({
+        class => 'Koha::Cities',
+        value => {
+            city_name => 'Manuelab'
+        }
+    });
 
     $t->get_ok('/cities?name=manuel&_per_page=1&_page=1')
         ->status_is(200)
-        ->header_like( 'Link' => qr/<http:\/\/.*\?.*&_page=2.*>; rel="next",/ )
+        ->header_like( 'Link' => qr/<http:\/\/.*[\?&]_page=2.*>; rel="next",/ )
         ->json_has('/0')
         ->json_hasnt('/1')
         ->json_is('/0/name' => 'Manuel');
@@ -105,16 +112,18 @@ subtest 'objects.search helper' => sub {
     });
 
     # _match=starts_with
-    $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=starts_with')
+    $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=starts_with')
         ->status_is(200)
         ->json_has('/0')
         ->json_has('/1')
-        ->json_hasnt('/2')
+        ->json_has('/2')
+        ->json_hasnt('/3')
         ->json_is('/0/name' => 'Manuel')
-        ->json_is('/1/name' => 'Manuela');
+        ->json_is('/1/name' => 'Manuela')
+        ->json_is('/2/name' => 'Manuelab');
 
     # _match=ends_with
-    $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=ends_with')
+    $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=ends_with')
         ->status_is(200)
         ->json_has('/0')
         ->json_has('/1')
@@ -123,22 +132,24 @@ subtest 'objects.search helper' => sub {
         ->json_is('/1/name' => 'Emanuel');
 
     # _match=exact
-    $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=exact')
+    $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=exact')
         ->status_is(200)
         ->json_has('/0')
         ->json_hasnt('/1')
         ->json_is('/0/name' => 'Manuel');
 
     # _match=contains
-    $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=contains')
+    $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=contains')
         ->status_is(200)
         ->json_has('/0')
         ->json_has('/1')
         ->json_has('/2')
-        ->json_hasnt('/3')
+        ->json_has('/3')
+        ->json_hasnt('/4')
         ->json_is('/0/name' => 'Manuel')
         ->json_is('/1/name' => 'Manuela')
-        ->json_is('/2/name' => 'Emanuel');
+        ->json_is('/2/name' => 'Manuelab')
+        ->json_is('/3/name' => 'Emanuel');
 
     $schema->storage->txn_rollback;
 };

t/db_dependent/Koha/REST/Plugin/PluginRoutes.t

@@ -50,6 +50,8 @@ subtest 'Bad plugins tests' => sub {
     t::lib::Mocks::mock_config( 'enable_plugins', 1 );
     t::lib::Mocks::mock_preference( 'UseKohaPlugins', 1 );
 
+    # remove any existing plugins that might interfere
+    Koha::Plugins::Methods->search->delete;
     my $plugins = Koha::Plugins->new;
     $plugins->InstallPlugins;
 
@@ -66,8 +68,10 @@ subtest 'Bad plugins tests' => sub {
         'Bad plugins raise warning';
 
     my $routes = get_defined_routes($t);
-    ok( !exists $routes->{'/contrib/badass/patrons/(:patron_id)/bother_wrong'}, 'Route doesn\'t exist' );
-    ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'}, 'Route exists' );
+    # Support placeholders () and <>  (latter style used starting with Mojolicious::Plugin::OpenAPI@1.28)
+    # TODO: remove () if minimum version is bumped to at least 1.28.
+    ok( !exists $routes->{'/contrib/badass/patrons/(:patron_id)/bother_wrong'} && !exists $routes->{'/contrib/badass/patrons/<:patron_id>/bother_wrong'}, 'Route doesn\'t exist' );
+    ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id>)/bother'} || exists $routes->{'/contrib/testplugin/patrons/<:patron_id>/bother'}, 'Route exists' );
 
     $schema->storage->txn_rollback;
 };
@@ -98,7 +102,9 @@ subtest 'Disabled plugins tests' => sub {
     my $t = Test::Mojo->new('Koha::REST::V1');
 
     my $routes = get_defined_routes($t);
-    ok( !exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'},
+    # Support placeholders () and <>  (latter style used starting with Mojolicious::Plugin::OpenAPI@1.28)
+    # TODO: remove () if minimum version is bumped to at least 1.28.
+    ok( !exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'} && !exists $routes->{'/contrib/testplugin/patrons/<:patron_id>/bother'},
         'Plugin disabled, route not defined' );
 
     $good_plugin->enable;
@@ -106,7 +112,9 @@ subtest 'Disabled plugins tests' => sub {
     $t      = Test::Mojo->new('Koha::REST::V1');
     $routes = get_defined_routes($t);
 
-    ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'},
+    # Support placeholders () and <>  (latter style used starting with Mojolicious::Plugin::OpenAPI@1.28)
+    # TODO: remove () if minimum version is bumped to at least 1.28.
+    ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'} || exists $routes->{'/contrib/testplugin/patrons/<:patron_id>/bother'},
         'Plugin enabled, route defined' );
 
     $schema->storage->txn_rollback;