https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=44035
Bug ID: 44035
Summary: Negotiate request and response body formats in the
REST API, replacing the XML/JSON hooks
Initiative type: ---
Sponsorship ---
status:
Product: Koha
Version: Main
Hardware: All
OS: All
Status: NEW
Severity: normal
Priority: P5 - low
Component: REST API
Assignee: [email protected]
Reporter: [email protected]
QA Contact: [email protected]
CC: [email protected]
Target Milestone: ---
Background
Bug 37762 added two hooks to Koha::REST::V1 so ISO18626 routes could use XML:
- around_action rewrites any request body sent as application/xml into JSON,
replacing $c->req->body.
- after_dispatch rewrites any response with an application/xml content type
from JSON back into XML.
Both run for every route, whatever the route declares. The ISO18626 controller
relies on the rewrite and calls decode_json( $c->req->body ).
For plugins that implement a native XML protocol, such as an NCIP server, the
controller never sees the message the client sent. Element attributes and
ordering are lost, and the response hook dies if the plugin returns XML it
built itself. Bug 43460 addressed this with an x-koha-raw-payload spec
extension to opt a route out, plus a series of follow-ups (the conversion
running twice because around_action also fires for the auth bridge, an
unreachable 400, a 500 on XML responses).
I think we over-reached in 37762, and 43460 is a patch over the underlying
design. The routes already say what they want via consumes, produces and their
body schema. The hooks ignore all of that and guess from one header.
What we actually want
Controllers should not make format choices, or even know which format was used.
They should be able to ask for the request as a validated Perl structure and
render a Perl structure back, and the REST layer should handle the format from
Content-Type, Accept, consumes and produces.
Mojolicious::Plugin::OpenAPI already does the checking half. It rejects a
request whose Content-Type is not in consumes and a request whose Accept cannot
be satisfied from produces, and it records the negotiated response type. What
it cannot do is read or write anything but JSON, and it documents
openapi.parse_request_body as the helper to override for other formats.
Proposal
Add a Koha::REST::Plugin::Payload plugin that wraps two OpenAPI plugin helpers
with a small table of codecs keyed by media type (JSON and XML to start with):
- openapi.parse_request_body decodes the body with the codec for its
Content-Type, and that structure is what gets validated against the body
schema. The request is never modified: $c->req->body stays the bytes the client
sent. Form bodies, bodies declared as a string, and bodies that cannot be
decoded fall through to the default parser, so validation still rejects them.
- openapi.build_response_body encodes a structure rendered with render( openapi
=> ... ) using the codec for the type negotiated from Accept and produces.
- A new $c->payload helper returns the request body as a structure whatever
format arrived. It is the structure that was validated, coerced to the types
the schema declares.
A controller then looks the same regardless of format:
my $c = shift->openapi->valid_input or return;
my $data = $c->payload;
...
return $c->render( status => 200, openapi => $result );
Adding a format later is a new codec entry and does not touch controllers.
Spec-driven behaviour replaces the opt-out flag:
- A body declared as an object is decoded and validated. The controller gets a
structure.
- A body declared as a string is left alone and the controller gets the raw
bytes. This covers NCIP-style protocols where attributes and ordering matter.
This removes both hooks, x-koha-raw-payload, and the $last guard and
double-conversion problems that came with them. Conversion now happens during
input validation, after the authentication bridge, so a request rejected there
is never parsed.
Design notes and open questions
- $c->payload is deliberately not built on $c->validation->output. Bugs 33556,
40417 and 40424 moved us off it because the plugin author said it is an
internal structure that is not stable. Controllers keep using normal Mojo
accessors, with $c->payload as the format-agnostic one for routes that accept
more than one format. Existing $c->req->json callers are unaffected and need no
migration.
- I looked at adding $c->req->xml for symmetry with $c->req->json. That would
need adding a method to Mojo::Message::Request, either by monkey-patching it
globally or by installing a subclass in an after_build_tx hook. Mojolicious
does not emit after_build_tx for embedded apps, and the REST API is embedded in
the staff and OPAC apps through Koha::App::Plugin::RESTV1, so the hook would
silently do nothing there.
- Third-party plugins: anything that currently reads the JSON-rewritten
$c->req->body on an XML route would need to switch to $c->payload. That
deserves a release-notes entry. The ISO18626 controller is the only in-tree
case and is updated as part of this.
- x-koha-raw-payload was added by bug 43460. If it has not been in a release we
can drop it cleanly. If it has, we should decide between removing it and
keeping it as a documented no-op for a cycle.
- In core, the old after_dispatch conversion appears to have been effectively
unused. ISO18626 renders its own XML with a content type of "application/xml;
charset=UTF-8", which the hook's exact match on "application/xml" never
matched. I have not audited third-party plugins for it.
- With codecs applied by negotiated type, error bodies on an XML-producing
route will come out as XML too. This needs to_xml to behave sensibly for
multi-key hashes, which currently pick an arbitrary root (see the "Move
parse_xml and to_xml to Koha::Util::XML" bug this depends on). I would like
opinions on whether errors should follow the negotiated format or always be
JSON.
- XML is schema-guided. A lone element versus a list of one is ambiguous in
XML, so the XML codec uses the spec definitions to decide which properties are
arrays. JSON never needs this. The codec interface leaves room for passing the
body schema rather than the global definitions.
- XML element attributes are still not represented in the decoded structure.
Protocols where they matter should declare the body as a string.
--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
