|
Description
Nick Clemens (kidclamp)
2022-10-14 14:35:16 UTC
Created attachment 144555 [details] [review] Bug 31795: Add REST endpoint to add an authority To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Make a POST request to /api/v1/authorities with one of the following content type header - application/json - application/marcxml+xml - application/marc-in-json - application/marc 5. Check that the authority is created 6. Sign off I had a go at testing using Postman, but I'm not familiar enough with APIs etc., to know how to work out what needs to be in the body for the format. If you could provide an example of what to put in the body for one of the formats, I'll have another go. (In reply to David Nind from comment #2) > I had a go at testing using Postman, but I'm not familiar enough with APIs > etc., to know how to work out what needs to be in the body for the format. > > If you could provide an example of what to put in the body for one of the > formats, I'll have another go. Hi David, I got this from KTD, but an example of marcxml would be <?xml version="1.0" encoding="UTF-8"?> <record xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd" xmlns="http://www.loc.gov/MARC21/slim"> <leader>00390nz a2200133o 4500</leader> <controlfield tag="001">1707</controlfield> <controlfield tag="003">csc</controlfield> <controlfield tag="005">20140507162102.0</controlfield> <controlfield tag="008">140507|| aca||aabn | a|a d</controlfield> <datafield tag="040" ind1=" " ind2=" "> <subfield code="a">csc</subfield> <subfield code="c">csc</subfield> </datafield> <datafield tag="150" ind1=" " ind2=" "> <subfield code="a">C++ (Computer program language)</subfield> </datafield> <datafield tag="667" ind1=" " ind2=" "> <subfield code="a">Machine generated authority record.</subfield> </datafield> <datafield tag="670" ind1=" " ind2=" "> <subfield code="a">Work cat.: (csc)0: Josuttis, Nicolai M. 5582, The C++ standard library :, c2012.</subfield> </datafield> <datafield tag="942" ind1=" " ind2=" "> <subfield code="a">TOPIC_TERM</subfield> </datafield> </record> Maybe you can change some data so that Koha won't think it's a duplicate Cheers! Created attachment 144746 [details] [review] Bug 31795: Add REST endpoint to add an authority To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Make a POST request to /api/v1/authorities with one of the following content type header - application/json - application/marcxml+xml - application/marc-in-json - application/marc 4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>" 5. Check that the authority is created 6. Sign off Created attachment 144747 [details] [review] Bug 31795: (follow-up) fix header name Created attachment 144749 [details] [review] Bug 31795: Add REST endpoint to add an authority To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Make a POST request to /api/v1/authorities with one of the following content type header - application/json - application/marcxml+xml - application/marc-in-json - application/marc 4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>" 5. Check that the authority is created 6. Sign off Signed-off-by: David Nind <david@davidnind.com> Created attachment 144750 [details] [review] Bug 31795: (follow-up) fix header name Signed-off-by: David Nind <david@davidnind.com> (In reply to Agustín Moyano from comment #3) > (In reply to David Nind from comment #2) >> ... > Hi David, I got this from KTD, but an example of marcxml would be > ... Thanks Agustín! I removed the 001 tag and changed the name in the 150 tag. In Postman I created a new HTTP request and added these details: 1. Changed to a POST with the URL as http://127.0.0.1:8081/api/v1/authorities 2. Body: changed to 'raw', pasted in the amended XML for the MARC authority record, and changed the type to XML 3. Added Headers: . x-authority-type: used TOPIC_TERM as the value . Content-Type: used application/marcxml+xml as the value 4. For authorization, add the Basic Auth type with koha as the username and password The new authority record was then created! David Applying: Bug 31795: Add REST endpoint to add an authority error: sha1 information is lacking or useless (api/v1/swagger/swagger.yaml). error: could not build fake ancestor Created attachment 149380 [details] [review] Bug 31795: Add REST endpoint to add an authority To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Make a POST request to /api/v1/authorities with one of the following content type header - application/json - application/marcxml+xml - application/marc-in-json - application/marc 4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>" 5. Check that the authority is created 6. Sign off Signed-off-by: David Nind <david@davidnind.com> Created attachment 149407 [details] [review] Bug 31795: (QA follow-up) Use x-koha-override header Despite its title, this patch does a couple more things. 1. The tests are rewritten to cover more things, and also to avoid deleting all authorities inside the transaction. It is really not required. 2. It makes the endpoint rely on the already generically implemented x-koha-override header, which is intended for the same use case as x-confirm-not-duplicate is for. 3. It changes the return codes to match the coding guidelines [1] 4. Only checks for duplicates if no override passed. To test: 1. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => SUCCESS: Tests pass! 2. Apply this follow-up 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D [1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.1_POST Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 149409 [details] [review] Bug 31795: Fix required permissions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> The tests don't pass after applying the patches:
prove t/db_dependent/api/v1/authorities.t
t/db_dependent/api/v1/authorities.t .. 1/3
# Failed test '409 Conflict'
# at t/db_dependent/api/v1/authorities.t line 204.
# got: '201'
# expected: '409'
# Failed test 'Location header is only set when the new resource is created'
# at t/db_dependent/api/v1/authorities.t line 204.
# Failed test 'similar match for JSON Pointer "/error"'
# at t/db_dependent/api/v1/authorities.t line 204.
# undef
# doesn't match '(?^u:Duplicate record (\d*))'
# Failed test 'exact match for JSON Pointer "/error_code"'
# at t/db_dependent/api/v1/authorities.t line 204.
# got: undef
# expected: 'duplicate'
# Looks like you failed 4 tests of 19.
t/db_dependent/api/v1/authorities.t .. 3/3
# Failed test 'post() tests'
# at t/db_dependent/api/v1/authorities.t line 227.
# Looks like you failed 1 test of 3.
t/db_dependent/api/v1/authorities.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests
Test Summary Report
-------------------
t/db_dependent/api/v1/authorities.t (Wstat: 256 Tests: 3 Failed: 1)
Failed test: 3
Non-zero exit status: 1
Files=1, Tests=3, 4 wallclock secs ( 0.02 usr 0.00 sys + 3.67 cusr 0.14 csys = 3.83 CPU)
Result: FAIL
(In reply to David Nind from comment #13) > The tests don't pass after applying the patches: > > prove t/db_dependent/api/v1/authorities.t > t/db_dependent/api/v1/authorities.t .. 1/3 > # Failed test '409 Conflict' > # at t/db_dependent/api/v1/authorities.t line 204. > # got: '201' > # expected: '409' This is odd. It fails on Zebra, and passes on ES. It means the FindDuplicateAuthority method is not behaving the same on each. I'll mock the call to the method and file a new bug for the method specifically. Created attachment 149451 [details] [review] Bug 31795: (follow-up) Mock call to FindDuplicateAuthority When running against Zebra, this tests fail because it requires the record to be indexed timely. In theory, a 5 seconds delay should be enough for the indexer to pick up. But that's not the point of the test, and we should better just mock the query in this case. To test: 1. Have ktd launch with zebra: $ ktd up -d 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Launch ktd with ES: $ ktd down $ ktd --es7 up -d 6. Repeat 2 => SUCCESS: Tests pass! 7. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 149459 [details] [review] Bug 31795: Add REST endpoint to add an authority To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Make a POST request to /api/v1/authorities with one of the following content type header - application/json - application/marcxml+xml - application/marc-in-json - application/marc 4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>" 5. Check that the authority is created 6. Sign off Signed-off-by: David Nind <david@davidnind.com> Created attachment 149460 [details] [review] Bug 31795: (QA follow-up) Use x-koha-override header Despite its title, this patch does a couple more things. 1. The tests are rewritten to cover more things, and also to avoid deleting all authorities inside the transaction. It is really not required. 2. It makes the endpoint rely on the already generically implemented x-koha-override header, which is intended for the same use case as x-confirm-not-duplicate is for. 3. It changes the return codes to match the coding guidelines [1] 4. Only checks for duplicates if no override passed. To test: 1. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => SUCCESS: Tests pass! 2. Apply this follow-up 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D [1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.1_POST Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Created attachment 149461 [details] [review] Bug 31795: Fix required permissions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Created attachment 149462 [details] [review] Bug 31795: (follow-up) Mock call to FindDuplicateAuthority When running against Zebra, this tests fail because it requires the record to be indexed timely. In theory, a 5 seconds delay should be enough for the indexer to pick up. But that's not the point of the test, and we should better just mock the query in this case. To test: 1. Have ktd launch with zebra: $ ktd up -d 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Launch ktd with ES: $ ktd down $ ktd --es7 up -d 6. Repeat 2 => SUCCESS: Tests pass! 7. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150370 [details] [review] Bug 31795: Add REST endpoint to add an authority To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Make a POST request to /api/v1/authorities with one of the following content type header - application/json - application/marcxml+xml - application/marc-in-json - application/marc 4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>" 5. Check that the authority is created 6. Sign off Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 150371 [details] [review] Bug 31795: (QA follow-up) Use x-koha-override header Despite its title, this patch does a couple more things. 1. The tests are rewritten to cover more things, and also to avoid deleting all authorities inside the transaction. It is really not required. 2. It makes the endpoint rely on the already generically implemented x-koha-override header, which is intended for the same use case as x-confirm-not-duplicate is for. 3. It changes the return codes to match the coding guidelines [1] 4. Only checks for duplicates if no override passed. To test: 1. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => SUCCESS: Tests pass! 2. Apply this follow-up 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D [1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.1_POST Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 150372 [details] [review] Bug 31795: Fix required permissions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 150373 [details] [review] Bug 31795: (follow-up) Mock call to FindDuplicateAuthority When running against Zebra, this tests fail because it requires the record to be indexed timely. In theory, a 5 seconds delay should be enough for the indexer to pick up. But that's not the point of the test, and we should better just mock the query in this case. To test: 1. Have ktd launch with zebra: $ ktd up -d 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Launch ktd with ES: $ ktd down $ ktd --es7 up -d 6. Repeat 2 => SUCCESS: Tests pass! 7. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 23.05. Nice work everyone, thanks! Enhancement - not backporting to 22.11.x Good work everyone! |