Bug 24577

Summary: API schema merging breaks with older versions of JSON module
Product: Koha Reporter: Andrew Isherwood <bugzilla>
Component: REST APIAssignee: Bugs List <koha-bugs>
Status: RESOLVED INVALID QA Contact:
Severity: normal    
Priority: P5 - low CC: martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:

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.