From d6092daa54a079f560cc33f4726fd26afa1866ac Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 8 Mar 2022 12:56:48 -0300 Subject: [PATCH] Bug 30194: Adapt Koha to the latest JSON::Validator version This patch makes V1.pm and PluginRoutes.pm work with the JSON::Validator breaking changes introduced after v4. JSON::Validator got stricter, and it seems there's some bug in how it resolves references, so I propose we bundle the YAML spec into a single file using a gulp task. This will save load time too, so has its advantages too. To test: 1. Edit /etc/apt/sources.list.d/koha.list 2. Duplicate the existing line below, replacing 'dev' for 'exp' 3. Upgrade the packages $ apt update ; apt dist-upgrade -y 4. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t \ t/db_dependent/api/v1 => SUCCESS: Tests pass! 5. Restart plack and check the logs: $ koha-plack --restart kohadev ; tail -f /var/log/koha/kohadev/*.log => SUCCESS: Nothing seems broken, things work 6. Point your browser to http://localhost:8081/api/v1/ => SUCCESS: The API is loaded 7. Sign off :-D Signed-off-by: Martin Renvoize Signed-off-by: Mason James Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart Signed-off-by: Fridolin Somers --- Koha/REST/Plugin/PluginRoutes.pm | 33 +++++-------- Koha/REST/V1.pm | 49 +++++++------------ .../Koha/REST/Plugin/PluginRoutes.t | 16 +++--- t/db_dependent/Koha/REST/V1.t | 2 - 4 files changed, 39 insertions(+), 61 deletions(-) diff --git a/Koha/REST/Plugin/PluginRoutes.pm b/Koha/REST/Plugin/PluginRoutes.pm index cf7eea882b..e97a29c764 100644 --- a/Koha/REST/Plugin/PluginRoutes.pm +++ b/Koha/REST/Plugin/PluginRoutes.pm @@ -24,6 +24,7 @@ use Koha::Plugins; use Koha::Logger; use Clone qw( clone ); +use JSON::Validator::Schema::OpenAPIv2; use Try::Tiny qw( catch try ); =head1 NAME @@ -41,8 +42,8 @@ Koha::REST::Plugin::PluginRoutes sub register { my ( $self, $app, $config ) = @_; - my $spec = $config->{spec}; - my $validator = $config->{validator}; + my $spec = $config->{spec}; + my $validate = $config->{validate}; my @plugins; @@ -58,7 +59,7 @@ sub register { ); foreach my $plugin ( @plugins ) { - $spec = $self->inject_routes( $spec, $plugin, $validator ); + $spec = $self->inject_routes( $spec, $plugin, $validate ); } } @@ -71,14 +72,14 @@ sub register { =cut sub inject_routes { - my ( $self, $spec, $plugin, $validator ) = @_; + my ( $self, $spec, $plugin, $validate ) = @_; - return merge_spec( $spec, $plugin ) unless $validator; + return merge_spec( $spec, $plugin ) unless $validate; return try { my $backup_spec = merge_spec( clone($spec), $plugin ); - if ( $self->spec_ok( $backup_spec, $validator ) ) { + if ( $self->spec_ok( $backup_spec, $validate ) ) { $spec = merge_spec( $spec, $plugin ); } else { @@ -145,23 +146,15 @@ sub merge_spec { =cut sub spec_ok { - my ( $self, $spec, $validator ) = @_; + my ( $self, $spec ) = @_; - my $schema = $self->{'swagger-v2-schema'}; + my $schema = JSON::Validator::Schema::OpenAPIv2->new( $spec ); - return try { - $validator->load_and_validate_schema( - $spec, - { - allow_invalid_ref => 1, - schema => ( $schema ) ? $schema : undef, - } - ); - return 1; - } - catch { - return 0; + if ($schema->is_invalid) { + warn $schema->errors->[0]; } + + return !$schema->is_invalid; } 1; diff --git a/Koha/REST/V1.pm b/Koha/REST/V1.pm index 1c22a769d7..62d326e0fd 100644 --- a/Koha/REST/V1.pm +++ b/Koha/REST/V1.pm @@ -22,7 +22,8 @@ use Mojo::Base 'Mojolicious'; use C4::Context; use Koha::Logger; -use JSON::Validator::OpenAPI::Mojolicious; +use JSON::Validator::Schema::OpenAPIv2; + use Try::Tiny qw( catch try ); =head1 NAME @@ -68,25 +69,23 @@ sub startup { $self->secrets([$secret_passphrase]); } - my $validator = JSON::Validator::OpenAPI::Mojolicious->new; + my $spec_file = $self->home->rel_file("api/v1/swagger/swagger.yaml"); push @{$self->routes->namespaces}, 'Koha::Plugin'; # Try to load and merge all schemas first and validate the result just once. - my $spec; - my $swagger_schema = $self->home->rel_file("api/swagger-v2-schema.json"); try { - $spec = $validator->bundle( - { - replace => 1, - schema => $self->home->rel_file("api/v1/swagger/swagger.yaml") - } - ); + + my $schema = JSON::Validator::Schema::OpenAPIv2->new; + + $schema->resolve( $spec_file ); + + my $spec = $schema->bundle->data; $self->plugin( 'Koha::REST::Plugin::PluginRoutes' => { - spec => $spec, - validator => undef + spec => $spec, + validate => 0, } ) unless C4::Context->needs_install; # load only if Koha is installed @@ -94,11 +93,6 @@ sub startup { OpenAPI => { spec => $spec, route => $self->routes->under('/api/v1')->to('Auth#under'), - schema => ( $swagger_schema ) ? $swagger_schema : undef, - allow_invalid_ref => - 1, # required by our spec because $ref directly under - # Paths-, Parameters-, Definitions- & Info-object - # is not allowed by the OpenAPI specification. } ); } @@ -111,19 +105,16 @@ sub startup { $logger->error("Warning: Could not load REST API spec bundle: " . $_); try { - $validator->load_and_validate_schema( - $self->home->rel_file("api/v1/swagger/swagger.yaml"), - { - allow_invalid_ref => 1, - schema => ( $swagger_schema ) ? $swagger_schema : undef, - } - ); - $spec = $validator->schema->data; + my $schema = JSON::Validator::Schema::OpenAPIv2->new; + $schema->resolve( $spec_file ); + + my $spec = $schema->bundle->data; + $self->plugin( 'Koha::REST::Plugin::PluginRoutes' => { - spec => $spec, - validator => $validator + spec => $spec, + validate => 1 } ) unless C4::Context->needs_install; # load only if Koha is installed @@ -131,10 +122,6 @@ sub startup { OpenAPI => { spec => $spec, route => $self->routes->under('/api/v1')->to('Auth#under'), - allow_invalid_ref => - 1, # required by our spec because $ref directly under - # Paths-, Parameters-, Definitions- & Info-object - # is not allowed by the OpenAPI specification. } ); } diff --git a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t index 9c6db7b89b..1bd91c9015 100755 --- a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t +++ b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t @@ -25,8 +25,6 @@ use Test::MockModule; use File::Basename; use t::lib::Mocks; -use JSON::Validator::OpenAPI::Mojolicious; - # Dummy app for testing the plugin use Mojolicious::Lite; @@ -72,8 +70,9 @@ subtest 'Bad plugins tests' => sub { my $t; warning_like { $t = Test::Mojo->new('Koha::REST::V1'); } - [qr{Could not load REST API spec bundle: Invalid JSON specification}, - qr{The resulting spec is invalid. Skipping Bad API Route Plugin},], + [qr{Could not load REST API spec bundle: /paths/~0001contrib~0001badass}, + qr{bother_wrong/put/parameters/0: /oneOf/1 Properties not allowed:}, + qr{Plugin Koha::Plugin::BadAPIRoute route injection failed: The resulting spec is invalid. Skipping Bad API Route Plugin},], 'Bad plugins raise warning'; my $routes = get_defined_routes($t); @@ -153,8 +152,9 @@ subtest 'Permissions and access to plugin routes tests' => sub { my $t; warning_like { $t = Test::Mojo->new('Koha::REST::V1'); } - [qr{Could not load REST API spec bundle: Invalid JSON specification}, - qr{The resulting spec is invalid. Skipping Bad API Route Plugin},], + [qr{Could not load REST API spec bundle: /paths/~0001contrib~0001badass}, + qr{bother_wrong/put/parameters/0: /oneOf/1 Properties not allowed:}, + qr{Plugin Koha::Plugin::BadAPIRoute route injection failed: The resulting spec is invalid. Skipping Bad API Route Plugin},], 'Bad plugins raise warning'; my $routes = get_defined_routes($t); @@ -262,8 +262,8 @@ sub traverse_routes { my $path = $route->pattern->unparsed || '/'; # Methods - my $via = $route->via; - my $verb = !$via ? '*' : uc join ',', @$via; + my $methods = $route->methods // []; + my $verb = !$methods ? '*' : uc join ',', @$methods; $routes->{$path}->{$verb} = 1; $depth++; diff --git a/t/db_dependent/Koha/REST/V1.t b/t/db_dependent/Koha/REST/V1.t index faddea9da3..fc333a0814 100755 --- a/t/db_dependent/Koha/REST/V1.t +++ b/t/db_dependent/Koha/REST/V1.t @@ -21,8 +21,6 @@ use Test::More tests => 1; use Test::Mojo; use Test::Warn; -use JSON::Validator::OpenAPI::Mojolicious; - subtest 'Type definition tests' => sub { plan tests => 4; -- 2.39.5