[Bug 28201] New: Add API routes to update and create biblio record
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 Bug ID: 28201 Summary: Add API routes to update and create biblio record Change sponsored?: --- Product: Koha Version: master Hardware: All URL: https://wiki.koha-community.org/wiki/Biblios_endpoint_ RFC OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: joonas.kylmala@helsinki.fi CC: tomascohen@gmail.com Blocks: 17371 We need PUT and POST API routes to modify and create biblio records. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17371 [Bug 17371] Add routes for biblios [OMNIBUS] -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=7613 CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #1 from Fridolin Somers <fridolin.somers@biblibre.com> --- Maybe this is close to svc/import_bib from Bug 7613 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #2 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 128360 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128360&action=edit Bug 28201: Add API route to create a biblio Example usage: POST /api/v1/biblios Accept: application/json { "marcxml": "<record>...</record>", "framework_id": "FA" } It can return data in the same formats as GET /api/v1/biblios/:biblio_id depending on the value of the Accept request header: - application/json - application/marcxml+xml - application/marc-in-json - application/marc - text/plain Test plan: 1. Try requesting this endpoint with your favorite API tool (I recommend https://github.com/frigus02/RESTer) 2. Run `prove t/db_dependent/api/v1/biblios/post.t` -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |julian.maurice@biblibre.com Summary|Add API routes to update |Add API routes to create |and create biblio record |biblio record --- Comment #3 from Julian Maurice <julian.maurice@biblibre.com> --- Removing the "update" part of this bug. It should be done in another bug IMO. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I like where this is going, but I think this needs some more thinking. In my opinion: - The request body should include the raw record - Content/Type should tell the controller how it should handle the raw record in terms of serialization format (i.e. application/marcxml+xml, application/marc-in-json, etc) - The framework should be passed as a header. I propose x-koha-cataloguing-fw to ease your work. - We still need the 'biblio_metadata.schema' attribute, put it in some header as well: x-koha-metadata-schema. Let me know if you will have time to do it. This is a nice goal for 22.05 :-D -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Tomás Cohen Arazi from comment #4)
I like where this is going, but I think this needs some more thinking.
In my opinion: - The request body should include the raw record - Content/Type should tell the controller how it should handle the raw record in terms of serialization format (i.e. application/marcxml+xml, application/marc-in-json, etc)
That's what I wanted to do at first, but I couldn't figure out how to pass non-json data to the API. If I remember correctly the body was always empty when trying to use it in the controller. So I opted for the easy way. Do we have examples of Koha API routes that accept data other than JSON ? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Julian Maurice from comment #5)
(In reply to Tomás Cohen Arazi from comment #4)
I like where this is going, but I think this needs some more thinking.
In my opinion: - The request body should include the raw record - Content/Type should tell the controller how it should handle the raw record in terms of serialization format (i.e. application/marcxml+xml, application/marc-in-json, etc)
That's what I wanted to do at first, but I couldn't figure out how to pass non-json data to the API. If I remember correctly the body was always empty when trying to use it in the controller. So I opted for the easy way. Do we have examples of Koha API routes that accept data other than JSON ?
Try "schema": { "type": "string" } for the body parameter. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #6)
(In reply to Julian Maurice from comment #5)
(In reply to Tomás Cohen Arazi from comment #4)
I like where this is going, but I think this needs some more thinking.
In my opinion: - The request body should include the raw record - Content/Type should tell the controller how it should handle the raw record in terms of serialization format (i.e. application/marcxml+xml, application/marc-in-json, etc)
That's what I wanted to do at first, but I couldn't figure out how to pass non-json data to the API. If I remember correctly the body was always empty when trying to use it in the controller. So I opted for the easy way. Do we have examples of Koha API routes that accept data other than JSON ?
Try
"schema": { "type": "string" }
for the body parameter.
Ohhhh that's cool. Previously, I had no luck getting the validator to accept anything but JSON, but if that works that would be awesome... So for examples... api/v1/swagger/paths/biblios.json Koha/REST/V1/Biblios.pm -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- Looking at https://docs.mojolicious.org/Mojolicious/Plugin/DefaultHelpers#respond_to and it looks to me like Koha/REST/V1/Biblios.pm might actually be a bit inefficient. It looks like it runs every output option (e.g. "as_xml_record", "to_mij", "as_usmarc", "as_formatted") regardless of what content type is actually served to the user... might be worthwhile wrapping those method calls in anonymous functions... But that's neither here nor there 😅 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #8)
Looking at https://docs.mojolicious.org/Mojolicious/Plugin/DefaultHelpers#respond_to and it looks to me like Koha/REST/V1/Biblios.pm might actually be a bit inefficient. It looks like it runs every output option (e.g. "as_xml_record", "to_mij", "as_usmarc", "as_formatted") regardless of what content type is actually served to the user... might be worthwhile wrapping those method calls in anonymous functions...
But that's neither here nor there 😅
I don't think it is working as you suggest. Not I read the docs the same way you do. Let's run it on a debugger. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #9)
(In reply to David Cook from comment #8)
Looking at https://docs.mojolicious.org/Mojolicious/Plugin/DefaultHelpers#respond_to and it looks to me like Koha/REST/V1/Biblios.pm might actually be a bit inefficient. It looks like it runs every output option (e.g. "as_xml_record", "to_mij", "as_usmarc", "as_formatted") regardless of what content type is actually served to the user... might be worthwhile wrapping those method calls in anonymous functions...
But that's neither here nor there 😅
I don't think it is working as you suggest. Not I read the docs the same way you do. Let's run it on a debugger.
There is a little error in what I said. If application/json is sent as an Accept header, then only a JSON response will be generated. However, looking at the code, if application/json is not sent in the Accept header, then we're creating a hash composed of marcxml, mij, marc, txt, and any keys with their corresponding values. That's just straight up Perl code. That said, it is possible I'm wrong about wrapping the method calls in anonymous functions though. I don't think the method should be called until the anonymous function is run, but it could be worth checking with a debugger. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- I'll open a separate report for that so I stop distracting from the real purpose of this report. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29953 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128360|0 |1 is obsolete| | --- Comment #12 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 135058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135058&action=edit Bug 28201: Add API route to create a biblio Example usage: POST /api/v1/biblios Accept: application/json { "marcxml": "<record>...</record>", "framework_id": "FA" } It can return data in the same formats as GET /api/v1/biblios/:biblio_id depending on the value of the Accept request header: - application/json - application/marcxml+xml - application/marc-in-json - application/marc - text/plain Test plan: 1. Try requesting this endpoint with your favorite API tool (I recommend https://github.com/frigus02/RESTer) 2. Run `prove t/db_dependent/api/v1/biblios/post.t` -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #13 from Julian Maurice <julian.maurice@biblibre.com> --- Patch rebased on master + added the use of anonymous subroutines for respond_to. There is no need to test with a debugger IMO. It's pretty obvious that we pass a hash to respond_to, and this hash has to be built before calling respond_to (unless some kind of dark magic is involved here :)) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion --- Comment #14 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Tomás Cohen Arazi from comment #4)
I like where this is going, but I think this needs some more thinking.
In my opinion: - The request body should include the raw record - Content/Type should tell the controller how it should handle the raw record in terms of serialization format (i.e. application/marcxml+xml, application/marc-in-json, etc) - The framework should be passed as a header. I propose x-koha-cataloguing-fw to ease your work. - We still need the 'biblio_metadata.schema' attribute, put it in some header as well: x-koha-metadata-schema.
The more I think about this, the more I disagree with it. The frameworkcode is a property of a biblio record in the same way as the MARC record. I don't see why it shouldn't be passed with the body. Especially as the response to GET /biblio/:id already returns the frameworkcode in the response body. Actually, I think we should go in the opposite direction and pass the MARC record inside a "metadata" property. Something like this: POST /api/v1/biblios { "framework_id": "", "metadata": [ { "format": "marcxml", "schema": "UNIMARC", "metadata": "<record>...</record>" } ] } It's more in line with the database structure. And we may want to set other columns of biblio or biblioitems (for those that are not mapped to a MARC subfield) What do you think ? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Julian Maurice from comment #14)
(In reply to Tomás Cohen Arazi from comment #4)
I like where this is going, but I think this needs some more thinking.
In my opinion: - The request body should include the raw record - Content/Type should tell the controller how it should handle the raw record in terms of serialization format (i.e. application/marcxml+xml, application/marc-in-json, etc) - The framework should be passed as a header. I propose x-koha-cataloguing-fw to ease your work. - We still need the 'biblio_metadata.schema' attribute, put it in some header as well: x-koha-metadata-schema.
The more I think about this, the more I disagree with it. The frameworkcode is a property of a biblio record in the same way as the MARC record. I don't see why it shouldn't be passed with the body. Especially as the response to GET /biblio/:id already returns the frameworkcode in the response body. Actually, I think we should go in the opposite direction and pass the MARC record inside a "metadata" property. Something like this:
POST /api/v1/biblios
{ "framework_id": "", "metadata": [ { "format": "marcxml", "schema": "UNIMARC", "metadata": "<record>...</record>" } ] }
It's more in line with the database structure. And we may want to set other columns of biblio or biblioitems (for those that are not mapped to a MARC subfield) What do you think ?
I think you got it right here. I'd say the 'metadata' attribute should always be Base64 encoded, to support binary formats as well as XML. I've seen that in other APIs. This should be better documented on the spec. And maybe the update should be PUT /biblios/:biblio_id/metadata -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- What's missing here to move it out of discussion? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28201 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |RESOLVED Resolution|--- |DUPLICATE --- Comment #17 from Julian Maurice <julian.maurice@biblibre.com> --- This was replaced by bug 31800 *** This bug has been marked as a duplicate of bug 31800 *** -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org