Bugzilla – Attachment 156304 Details for
Bug 33503
Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions.
Bug-33503---Plugin-OpenAPI20-specification-schema-.patch (text/plain), 13.18 KB, created by
Olli-Antti Kivilahti
on 2023-09-27 17:39:08 UTC
(
hide
)
Description:
Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions.
Filename:
MIME Type:
Creator:
Olli-Antti Kivilahti
Created:
2023-09-27 17:39:08 UTC
Size:
13.18 KB
patch
obsolete
>From 9cfd1f6f210f0ff0741848df1e9acba90944b6fa Mon Sep 17 00:00:00 2001 >From: Olli-Antti Kivilahti <olli-antti.kivilahti@hypernova.fi> >Date: Wed, 12 Apr 2023 12:52:41 +0300 >Subject: [PATCH] Bug 33503 - Plugin OpenAPI2.0 specification schema fragments > are not resolved. Merge full schema definitions. > >Koha::REST::Plugin::PluginRoutes.pm simply injects a plugin's OpenAPI2.0 route >definitions into the /paths -object. >Possibly after Bug 30194, dereferencing OpenAPI2.0 schema fragments/components >has started to fail. >This plugin: > https://github.com/Hypernova-Oy/koha-plugin-self-service/releases/tag/v1.0.11 >used to get it's REST API paths injected and dereferenced/bundled correctly, >for Koha versions 22.05 and before, but in the new Koha version the same schema >definitions no longer work. > >This patch adds a new plugin hook, > api_spec >which injects a full OpenAPI2.0 compatible schema into the full Koha schema, >avoiding to overwrite any existing definitions/parameters/info/paths/etc. >This way we maintain backwards compatibility with existing plugins, which are >not broken, and maintain updated plugins' ability to use complex schemas. >We need to inject full schema definitions, because of the way how the >new OpenAPI validator does dereferencing, >by creating /parameters and /definitions -objects inside the plugin's OpenAPI >schema object. The api_route()-plugin hook then proceeds to strip away only the >paths/routes and loses the internal data structure references to the schema >fragments/components. > >If the api_spec is defined in the plugin, the api_routes-hook is never called. > >TEST PLAN: > >a) Run the modified test. >or >b) Install this plugin, and observe the routes are not loaded into the REST API. > https://github.com/Hypernova-Oy/koha-plugin-self-service/releases/tag/v1.0.11 >b1) Deploying this patch doesnt fix the plugin, as the whole subsystem to load > OpenAPI2 spec has changed, and the plugins updated with workarounds. > Plugins can be updated to support api_spec once it has been pushed. >--- > Koha/REST/Plugin/PluginRoutes.pm | 77 ++++++++++++++++++- > .../Koha/REST/Plugin/PluginRoutes.t | 3 +- > t/lib/plugins/Koha/Plugin/BundledApiSpec.pm | 46 +++++++++++ > .../Koha/Plugin/BundledApiSpec/openapi.yaml | 69 +++++++++++++++++ > .../BundledApiSpec/openapi/definitions.json | 5 ++ > .../openapi/definitions/finger.json | 12 +++ > .../BundledApiSpec/openapi/parameters.json | 18 +++++ > 7 files changed, 226 insertions(+), 4 deletions(-) > create mode 100644 t/lib/plugins/Koha/Plugin/BundledApiSpec.pm > create mode 100644 t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi.yaml > create mode 100644 t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions.json > create mode 100644 t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions/finger.json > create mode 100644 t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/parameters.json > >diff --git a/Koha/REST/Plugin/PluginRoutes.pm b/Koha/REST/Plugin/PluginRoutes.pm >index c275f2578c..fac7d48be1 100644 >--- a/Koha/REST/Plugin/PluginRoutes.pm >+++ b/Koha/REST/Plugin/PluginRoutes.pm >@@ -102,11 +102,13 @@ sub inject_routes { > sub merge_spec { > my ( $spec, $plugin ) = @_; > >- if($plugin->can('api_routes')) { >+ # Prefer the new api_spec() to inject a full OpenAPI2 spec instead of just the route definitions, if both are defined, to prevent duplicate routes. >+ # This way we can use the same plugin release for the new and the old versions of Koha. >+ if($plugin->can('api_routes') && not($plugin->can('api_spec'))) { # Deprecated > my $plugin_spec = $plugin->api_routes; > > foreach my $route ( keys %{ $plugin_spec } ) { >- my $THE_route = '/contrib/' . $plugin->api_namespace . $route; >+ my $THE_route = get_normalized_plugin_path($plugin->api_namespace, $route); > if ( exists $spec->{ $THE_route } ) { > # Route exists, overwriting is forbidden > Koha::Exceptions::Plugin::ForbiddenAction->throw( >@@ -118,12 +120,57 @@ sub merge_spec { > } > } > >+ if($plugin->can('api_spec')) { >+ my $plugin_namespace = $plugin->api_namespace; >+ my $plugin_spec = $plugin->api_spec; >+ my @errors; >+ if ($plugin_spec->{definitions}) { >+ while (my ($k, $v) = each %{$plugin_spec->{definitions}}) { >+ if ($spec->{definitions}->{$k}) { >+ push(@errors, "Duplicate OpenAPI definition '$k'. Plugin needs to rename one of it's definitions."); >+ } else { >+ $spec->{definitions}->{$k} = $v; >+ } >+ } >+ } >+ if ($plugin_spec->{parameters}) { >+ while (my ($k, $v) = each %{$plugin_spec->{parameters}}) { >+ if ($spec->{parameters}->{$k}) { >+ push(@errors, "Duplicate OpenAPI parameter '$k'. Plugin needs to rename one of it's parameters."); >+ } else { >+ $spec->{parameters}->{$k} = $v; >+ } >+ } >+ } >+ if ($plugin_spec->{tags}) { >+ if ($spec->{tags}) { >+ push(@{$spec->{tags}}, @{$plugin_spec->{tags}}); >+ } else { >+ $spec->{tags} = $plugin_spec->{tags}; >+ } >+ } >+ if ($plugin_spec->{paths}) { >+ while (my ($k, $v) = each %{$plugin_spec->{paths}}) { >+ if ($spec->{paths}->{$k}) { >+ push(@errors, "Duplicate OpenAPI parameter '$k'. Plugin needs to rename one of it's paths."); >+ } else { >+ $spec->{paths}->{get_normalized_plugin_path($plugin_namespace, $k)} = $v; >+ } >+ } >+ } >+ if (@errors) { >+ Koha::Exceptions::Plugin::ForbiddenAction->throw( >+ "Plugin '".$plugin->{class}."' had the following errors while merging OpenAPI specs:\n".join(', ', @errors) >+ ); >+ } >+ } >+ > if($plugin->can('static_routes')) { > my $plugin_spec = $plugin->static_routes; > > foreach my $route ( keys %{ $plugin_spec } ) { > >- my $THE_route = '/contrib/' . $plugin->api_namespace . '/static'.$route; >+ my $THE_route = get_normalized_plugin_static_path($plugin->api_namespace, $route); > if ( exists $spec->{ $THE_route } ) { > # Route exists, overwriting is forbidden > Koha::Exceptions::Plugin::ForbiddenAction->throw( >@@ -153,4 +200,28 @@ sub spec_ok { > return !$schema->is_invalid; > } > >+=head3 get_normalized_plugin_path >+ >+public static subroutine >+Normalizes a plugin's route path to /contrib/<plugin-namespace>/<plugin-route-path> >+ >+=cut >+ >+sub get_normalized_plugin_path { >+ my ($plugin_api_namespace, $plugin_path_name) = @_; >+ return '/contrib/' . $plugin_api_namespace . $plugin_path_name; >+} >+ >+=head3 get_normalized_plugin_static_path >+ >+public static subroutine >+Normalizes a plugin's static route path to /contrib/<plugin-namespace>/static/<plugin-route-path> >+ >+=cut >+ >+sub get_normalized_plugin_static_path { >+ my ($plugin_api_namespace, $plugin_path_name) = @_; >+ return '/contrib/' . $plugin_api_namespace . '/static' . $plugin_path_name; >+} >+ > 1; >diff --git a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t >index 66746508ed..98e7138a32 100755 >--- a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t >+++ b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t >@@ -129,7 +129,7 @@ subtest 'Disabled plugins tests' => sub { > > subtest 'Permissions and access to plugin routes tests' => sub { > >- plan tests => 16; >+ plan tests => 17; > > $schema->storage->txn_begin; > >@@ -164,6 +164,7 @@ subtest 'Permissions and access to plugin routes tests' => sub { > my $routes = get_defined_routes($t); > ok( exists $routes->{'/contrib/testplugin/patrons/bother'}, 'Route exists' ); > ok( exists $routes->{'/contrib/testplugin/public/patrons/bother'}, 'Route exists' ); >+ ok( exists $routes->{'/contrib/bundledass/patrons/<:patron_id>/finger'}, 'Bundled Route exists' ); > > C4::Context->set_preference( 'RESTPublicAnonymousRequests', 0 ); > >diff --git a/t/lib/plugins/Koha/Plugin/BundledApiSpec.pm b/t/lib/plugins/Koha/Plugin/BundledApiSpec.pm >new file mode 100644 >index 0000000000..368d8609e8 >--- /dev/null >+++ b/t/lib/plugins/Koha/Plugin/BundledApiSpec.pm >@@ -0,0 +1,46 @@ >+package Koha::Plugin::BundledApiSpec; >+ >+use Modern::Perl; >+ >+use base qw(Koha::Plugins::Base); >+ >+our $VERSION = 0.01; >+our $metadata = { >+ name => 'Bundled OpenAPI Spec Plugin', >+ author => 'Olli-Antti Kivilahti', >+ description => 'Test plugin for testing the bundling of OpenAPI schema fragments', >+ date_authored => '2023-', >+ date_updated => '2023-04-06', >+ minimum_version => '21.11', >+ maximum_version => undef, >+ version => $VERSION, >+ my_example_tag => 'bundled_api_spec', >+}; >+ >+sub new { >+ my ( $class, $args ) = @_; >+ $args->{'metadata'} = $metadata; >+ my $self = $class->SUPER::new($args); >+ return $self; >+} >+ >+sub api_namespace { >+ return "bundledass"; >+} >+ >+# One should never define these both, but only the other. >+#sub api_route { >+# my ( $self, $args ) = @_; >+# return JSON::Validator->new->schema($self->mbf_dir() . "/openapi.yaml")->schema->{data}; >+#} >+ >+# One should never define these both, but only the other. Automated test suite should disable the other for testing. >+sub api_spec { >+ my ( $self, $args ) = @_; >+ >+ my $schema2 = JSON::Validator::Schema::OpenAPIv2->new; >+ $schema2->resolve($self->mbf_dir() . "/openapi.yaml"); >+ return $schema2->bundle->data; >+} >+ >+1; >diff --git a/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi.yaml b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi.yaml >new file mode 100644 >index 0000000000..1cf2132118 >--- /dev/null >+++ b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi.yaml >@@ -0,0 +1,69 @@ >+--- >+ >+swagger: "2.0" >+info: >+ title: API Title >+ version: "1.0" >+ >+paths: >+ /patrons/{patron_id}/finger: >+ delete: >+ x-mojo-to: Koha::Plugin::BundledApiSpec#finger_delete >+ operationId: finger_delete >+ tags: >+ - plugin >+ produces: >+ - application/json >+ parameters: >+ - $ref: openapi/parameters.json#/patron_idPathParam >+ x-koha-authorization: >+ permissions: >+ borrowers: finger >+ responses: >+ "200": >+ description: Deleted ok >+ schema: >+ type: object >+ required: >+ - deleted_count >+ properties: >+ deleted_count: >+ description: How many Block-objects have been deleted in the DB >+ type: integer >+ get: >+ x-mojo-to: Koha::Plugin::BundledApiSpec#finger_get >+ operationId: finger_get >+ tags: >+ - plugin >+ produces: >+ - application/json >+ parameters: >+ - $ref: openapi/parameters.json#/patron_idPathParam >+ x-koha-authorization: >+ permissions: >+ borrowers: finger >+ responses: >+ "200": >+ description: List of fingers >+ schema: >+ type: array >+ items: >+ $ref: openapi/definitions.json#/finger >+ post: >+ x-mojo-to: Koha::Plugin::BundledApiSpec#finger_post >+ operationId: finger_post >+ tags: >+ - plugin >+ produces: >+ - application/json >+ parameters: >+ - $ref: openapi/parameters.json#/patron_idPathParam >+ - $ref: openapi/parameters.json#/finger >+ x-koha-authorization: >+ permissions: >+ borrowers: finger >+ responses: >+ "200": >+ description: The created self-service block >+ schema: >+ $ref: openapi/definitions.json#/finger >diff --git a/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions.json b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions.json >new file mode 100644 >index 0000000000..2ee34eac20 >--- /dev/null >+++ b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions.json >@@ -0,0 +1,5 @@ >+{ >+ "finger": { >+ "$ref": "./definitions/finger.json" >+ } >+} >\ No newline at end of file >diff --git a/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions/finger.json b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions/finger.json >new file mode 100644 >index 0000000000..9adc5fb6fe >--- /dev/null >+++ b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/definitions/finger.json >@@ -0,0 +1,12 @@ >+{ >+ "type": "object", >+ "properties": { >+ "patron_id": { >+ "type": "integer", >+ "description": "The user that has the finger" >+ }, >+ "type": { >+ "type": "string" >+ } >+ } >+} >\ No newline at end of file >diff --git a/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/parameters.json b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/parameters.json >new file mode 100644 >index 0000000000..9beb538348 >--- /dev/null >+++ b/t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi/parameters.json >@@ -0,0 +1,18 @@ >+{ >+ "patron_idPathParam": { >+ "name": "patron_id", >+ "in": "path", >+ "description": "Internal Patron identifier", >+ "required": true, >+ "type": "integer" >+ }, >+ "finger": { >+ "name": "finger", >+ "description": "Finger", >+ "in": "body", >+ "required": true, >+ "schema": { >+ "$ref": "./definitions.json#/finger" >+ } >+ } >+} >\ No newline at end of file >-- >2.34.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 33503
:
149520
|
149533
|
156304
|
156312