https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43460 --- Comment #36 from David Cook <dcook@prosentient.com.au> --- (In reply to Pedro Amorim (ammopt) from comment #35)
(In reply to David Cook from comment #34)
I haven't reviewed the code but... wouldn't it have made more sense to make this XML to JSON conversion opt-in at the controller level?
Requiring x-koha-raw-payload at the request end... makes no sense to me. We're making everything bend around the ILL? Shouldn't ILL be the exception to the rule and not the other way around?
Hi David, despite the x- prefix, x-koha-raw-payload isn't an HTTP header. It's an OpenAPI vendor extension the route author sets in the route's spec, the same way x-koha-authorization is. e.g. in a plugin's openapi.json: "post": { "x-mojo-to": "MyPlugin::Controller#ncip", "consumes": ["application/xml"], "produces": ["application/xml"], "x-koha-raw-payload": true, ... }
Ah thanks for the clarification, Pedro!
On opt-in vs opt-out: before 37762 Koha core didn't accept generic application/xml payloads (MARCXML has its own content type and isn't affected).
Koha core might not have accepted application/xml but I know that there are Koha plugins that do. This looks like a breaking change that could affect an unknown number of systems.
The intent was for them to be first-class in the REST API, validated against the same OpenAPI spec as JSON, so controllers don't need to care which format came in. For that to hold for every route without extra setup, the conversion is the default.
Sorry, what's "them" in this context?
We're not making everything bend around ILL, we're setting a standard which ISO18626 ILL just happens to be the first consumer of. Routes that want to handle the raw XML themselves (like the NCIP plugin) are the exception, hence the opt-out, at the cost of those plugins needing the flag.
I suppose that you can call it setting a standard, but it reverses an existing feature and adds new dev requirements. It breaks backwards compatibility. Which is fine in a major release, although I wish we had a Bugzilla field to flag compatibility breakages so we could put it in release notes automatically... I suppose the initial conversion of XML to JSON was added in 26.05.00 (in bug 37762), so it does make sense to backport it to 26.05.x... -- But the more I think about it... yeah this still feels wrong. Sending "Content-Type: application/xml" and getting back "application/json" automatically? That's weird. If you had "Accept: application/json" that's another story... that could be a compelling case for an automatic conversion. But oh well. The genie is out of the bottle now... -- You are receiving this mail because: You are watching all bug changes.