Bug 28201 - Add API routes to create biblio record
Summary: Add API routes to create biblio record
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact:
URL: https://wiki.koha-community.org/wiki/...
Keywords:
Depends on:
Blocks: 17371
  Show dependency treegraph
 
Reported: 2021-04-23 07:57 UTC by Joonas Kylmälä
Modified: 2022-06-18 21:49 UTC (History)
4 users (show)

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


Attachments
Bug 28201: Add API route to create a biblio (8.07 KB, patch)
2021-12-08 11:08 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 28201: Add API route to create a biblio (7.63 KB, patch)
2022-05-17 11:21 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2021-04-23 07:57:06 UTC
We need PUT and POST API routes to modify and create biblio records.
Comment 1 Fridolin Somers 2021-10-01 23:01:59 UTC
Maybe this is close to svc/import_bib from Bug 7613
Comment 2 Julian Maurice 2021-12-08 11:08:53 UTC
Created attachment 128360 [details] [review]
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`
Comment 3 Julian Maurice 2021-12-08 11:09:58 UTC
Removing the "update" part of this bug. It should be done in another bug IMO.
Comment 4 Tomás Cohen Arazi 2022-01-07 18:41:36 UTC
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
Comment 5 Julian Maurice 2022-01-25 08:40:32 UTC
(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 ?
Comment 6 Tomás Cohen Arazi 2022-01-25 12:32:10 UTC
(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.
Comment 7 David Cook 2022-01-26 23:53:14 UTC
(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
Comment 8 David Cook 2022-01-26 23:59:33 UTC
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 😅
Comment 9 Tomás Cohen Arazi 2022-01-27 01:46:42 UTC
(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.
Comment 10 David Cook 2022-01-27 02:24:07 UTC
(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.
Comment 11 David Cook 2022-01-27 02:24:31 UTC
I'll open a separate report for that so I stop distracting from the real purpose of this report.
Comment 12 Julian Maurice 2022-05-17 11:21:40 UTC
Created attachment 135058 [details] [review]
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`
Comment 13 Julian Maurice 2022-05-17 11:29:34 UTC
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 :))
Comment 14 Julian Maurice 2022-05-17 11:59:16 UTC
(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 ?
Comment 15 Tomás Cohen Arazi 2022-05-17 12:26:23 UTC
(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
Comment 16 Katrin Fischer 2022-06-18 21:49:56 UTC
What's missing here to move it out of discussion?