Bug 24577 - API schema merging breaks with older versions of JSON module
Summary: API schema merging breaks with older versions of JSON module
Status: RESOLVED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-04 11:30 UTC by Andrew Isherwood
Modified: 2023-12-31 14:35 UTC (History)
1 user (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2020-02-04 11:30:26 UTC
I discovered this while deploying a plugin that defines an API route. If the installation is using a version of the JSON module < 2.90, schema merging doesn't work correctly, these are the sort of errors that were being generated during validation of the merged schema:

{
    'path' => 
 '/paths/~1contrib~1ill_availability_unpaywall~1ill_availability_search_unpaywall/get/parameters/0',
    'message' => 'oneOf failed: oneOf failed: Properties not allowed: type. oneOf failed: (Not in enum list: header.. Expected boolean - got JSON::XS::Boolean.) (Not in enum list: formData.. Expected boolean - got JSON::XS::Boolean.) Expected boolean - got JSON::XS::Boolean. (Expected boolean - got JSON::XS::Boolean.. Not in enum list: path.) Properties not allowed: name, required, type, description, in.'
}

Pretty nonsensical and a nightmare to try and get to the bottom of. I was seeing this using JSON 2.61.

I believe the change that was implemented in 2.90 that we're dependent on, is the top one listed here: https://metacpan.org/changes/release/MAKAMAKA/JSON-2.90

Koha is currently specifying 2.07 as the required version. I'm wondering if it is worth moving this to 2.90.
Comment 1 Martin Renvoize 2020-02-04 12:40:55 UTC
So.. we already require Mojolicious.. so I'd be tempted to switch to Mojo::JSON.. I believe it's the fastest pure perl implementation of JSON to date and it will automatically use Cpanel::JSON::XS when found to go faster still.

It's also not introducing a new dependency, but rather using one we're already importing.

Thoughts?
Comment 2 Andrew Isherwood 2020-02-04 13:45:36 UTC
Sounds like a possibility. Though I think we'd have to audit all 172 uses of the JSON module and ensure that a) Mojo::JSON fulfils all requirements (JSON does a lot) and b) has an straightforward migration path
Comment 3 Katrin Fischer 2023-12-31 14:26:27 UTC
Is this one still valid?
Comment 4 Martin Renvoize 2023-12-31 14:35:56 UTC
Pretty sure this is invalid now.