Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions.
Summary: Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full...
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Olli-Antti Kivilahti
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-12 09:52 UTC by Olli-Antti Kivilahti
Modified: 2023-09-29 14:16 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions. (12.05 KB, patch)
2023-04-12 10:20 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions. (12.06 KB, patch)
2023-04-12 12:01 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions. (13.18 KB, patch)
2023-09-27 17:39 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions. (13.24 KB, patch)
2023-09-28 00:55 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2023-04-12 09:52:26 UTC
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.
Comment 1 Olli-Antti Kivilahti 2023-04-12 10:20:01 UTC
Created attachment 149520 [details] [review]
Bug 33503 - Plugin OpenAPI2.0 specification schema fragments are not resolved. Merge full schema definitions.

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.
Comment 2 Olli-Antti Kivilahti 2023-04-12 12:01:29 UTC
Created attachment 149533 [details] [review]
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.
Comment 3 Olli-Antti Kivilahti 2023-04-18 10:13:58 UTC
From the plugin, one can implement this hook with this code fragment:

sub api_spec {
    my ( $plugin, $args ) = @_;
    return JSON::Validator::Schema::OpenAPIv2->new;

    my $schema2 = JSON::Validator::Schema::OpenAPIv2->new;
    $schema2->resolve($plugin->mbf_dir() . "/openapi.yaml");
    return $schema2->bundle->data; #Bundle merges the external/internal/local references in the plugin schema path.
}
Comment 4 Victor Grousset/tuxayo 2023-09-27 05:45:55 UTC
Test plan (asked in chat):
1. prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t

Turns out it fails ^^"

```
t/db_dependent/Koha/REST/Plugin/PluginRoutes.t .. 2/4     # Looks like you planned 16 tests but ran 17.
t/db_dependent/Koha/REST/Plugin/PluginRoutes.t .. 3/4 
#   Failed test 'Permissions and access to plugin routes tests'
#   at t/db_dependent/Koha/REST/Plugin/PluginRoutes.t line 206.
t/db_dependent/Koha/REST/Plugin/PluginRoutes.t .. 4/4 # Looks like you failed 1 test of 4.
t/db_dependent/Koha/REST/Plugin/PluginRoutes.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/4 subtests 
```
Comment 5 Olli-Antti Kivilahti 2023-09-27 17:39:08 UTC
Created attachment 156304 [details] [review]
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.
Comment 6 Victor Grousset/tuxayo 2023-09-28 00:55:32 UTC
Created attachment 156312 [details] [review]
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.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 7 Victor Grousset/tuxayo 2023-09-28 01:04:50 UTC
So it was just a number-of-tests problem?! :o
Sorry, I could have fixed it. I was confused by «Failed test 'Permissions and access to plugin routes tests'»

Anyway, test pass so signed-off.

I gave a go at the alternative:
«Install this plugin, and observe the routes are not loaded into the REST API»
How?
Comment 8 Olli-Antti Kivilahti 2023-09-28 01:39:34 UTC
(In reply to Victor Grousset/tuxayo from comment #7)
> So it was just a number-of-tests problem?! :o
> Sorry, I could have fixed it. I was confused by «Failed test 'Permissions
> and access to plugin routes tests'»
> 
> Anyway, test pass so signed-off.
> 
> I gave a go at the alternative:
> «Install this plugin, and observe the routes are not loaded into the REST
> API»
> How?

Make a GET request to /api/v1/ and you can see the schema definition.
The plugin's openapi-directory has the path definitions and you can see that the path definitions are not loaded in the aforementioned schema definition from /api/v1/
This is standard Koha-plugin development workflow and it is a bit difficult to test this if there is no previous experience with developing plugins with complex rest api schemas.
You have to restart plack for the route definitions to take place.

I appreciate the sign-off.
Also the plugin loaded into the test case explains more briefly what is the exact case that is being tested.
Most plugins just have the /paths defined in the OpenAPI schema and not very many different paths.
Comment 9 Victor Grousset/tuxayo 2023-09-28 15:04:40 UTC
> Install this plugin, and observe the routes are not loaded into the REST API
> [...]
> Make a GET request to /api/v1/ and you can see the schema definition.

Indeed, /api/v1/ content didn't chance with and without the patch + plugin (after restart_all)
Comment 10 Jonathan Druart 2023-09-29 14:16:08 UTC
1. QA failures
 WARN   Koha/REST/Plugin/PluginRoutes.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 28, now: 52)

 WARN   t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 30, now: 31)

 FAIL   t/lib/plugins/Koha/Plugin/BundledApiSpec.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 2)

 OK     t/lib/plugins/Koha/Plugin/BundledApiSpec/openapi.yaml


Processing additional checks

        * Commit title does not start with 'Bug XXXXX: ' - 5b055c172d3

2. bundledass?..

3. You are talking about a regression, did you bisect/investigate from where it could come?