Description
Lari Taskula
2017-02-17 11:16:22 UTC
Created attachment 60490 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI as Perl dependency This patch also upgrades Mojolicious version requirement to 6.40 as it is the minimum requirement set by Mojolicious::Plugin::OpenAPI. To test: 1. sudo cpanm Mojolicious::Plugin::OpenAPI@1.10 Created attachment 60491 [details] [review] Bug 18137: Add useful Koha::Exceptions Created attachment 60492 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Created attachment 60493 [details] [review] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Created attachment 60494 [details] [review] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Created attachment 60495 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Mojolicious::Plugin::OpenAPI is at the time of this post at version 1.09 in CPAN, however I am having problems with the CPAN version, as follows: Could not load document from /home/ubuntu/kohaclone/api/v1/swagger/../parameters.json: Can't open file "/home/ubuntu/kohaclone/api/v1/swagger/../parameters.json": No such file or directory at /usr/local/share/perl/5.18.2/JSON/Validator.pm line 140. Fortunately, it looks like Mojolicious::Plugin::OpenAPI version 1.10 (currently at unreleased status in GitHub) fixes this issue. So if you are testing this before 1.10 is released, I would suggest using the source provided directly in GitHub. https://github.com/jhthorsen/mojolicious-plugin-openapi Created attachment 60533 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. (In reply to Lari Taskula from comment #7) > Mojolicious::Plugin::OpenAPI is at the time of this post at version 1.09 in > CPAN, however I am having problems with the CPAN version, as follows: > > Could not load document from > /home/ubuntu/kohaclone/api/v1/swagger/../parameters.json: Can't open file > "/home/ubuntu/kohaclone/api/v1/swagger/../parameters.json": No such file or > directory at /usr/local/share/perl/5.18.2/JSON/Validator.pm line 140. > > Fortunately, it looks like Mojolicious::Plugin::OpenAPI version 1.10 > (currently at unreleased status in GitHub) fixes this issue. So if you are > testing this before 1.10 is released, I would suggest using the source > provided directly in GitHub. > https://github.com/jhthorsen/mojolicious-plugin-openapi It appears v 1.10 was released right after I posted this. So cpanm Mojolicious::Plugin::OpenAPI@1.10 should be fine. I would like to point out that I am experiencing an issue with JSON::Validator, where $refs are sometimes left unresolved in our spec and it causes some error messages and crashes. This issue is being tracked at https://github.com/jhthorsen/json-validator/pull/52. Created attachment 60837 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI as Perl dependency This patch also upgrades Mojolicious version requirement to 6.40 as it is the minimum requirement set by Mojolicious::Plugin::OpenAPI. To test: 1. sudo cpanm Mojolicious::Plugin::OpenAPI@1.10 Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 60838 [details] [review] Bug 18137: Add useful Koha::Exceptions Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 60839 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 60840 [details] [review] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 60841 [details] [review] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 60842 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 60843 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> (In reply to Lari Taskula from comment #10) > I would like to point out that I am experiencing an issue with > JSON::Validator, where $refs are sometimes left unresolved in our spec and > it causes some error messages and crashes. This issue is being tracked at > https://github.com/jhthorsen/json-validator/pull/52. The developer of JSON::Validator has been working hard on it and it now seems to have been fixed. The fix will appear in version 0.96 of JSON::Validator. That should probably be added as dependency once it is released. Just tested, looks good for me, just a minor issue found by qa test tool: FAIL Koha/REST/V1/Auth.pm FAIL pod in file Koha/REST/V1/Auth.pm *** WARNING: =head3 without preceding higher level Created attachment 60866 [details] [review] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Created attachment 60867 [details] [review] Bug 18137: (QA-follow-up) Fix pod fail I like it, and I'm testing it. My only concern is dependencies. I tried to install Mojolicious 6.40 along with Mojolicious::Plugin::OpenAPI 1.10 and it wouldn't install. 1.10 needs newer Mojo (7.something). Pleaes provide feedback about the dependencies so I can test it. I submitted Mojolicious::Plugin::OpenAPI to Debian a few days ago (1.10). 1.13 is out by now. And both versions depend on versions of libjson-validator-perl not yet in Debian unstable, and the tests in the latest version are broken. Can't build at the moment, but it seemed possible to get everything working in Jessie (including Mojolicious 7.x). I'll give an update when I know more. (In reply to Mirko Tietgen from comment #24) > I submitted Mojolicious::Plugin::OpenAPI to Debian a few days ago (1.10). > 1.13 is out by now. And both versions depend on versions of > libjson-validator-perl not yet in Debian unstable, and the tests in the > latest version are broken. Thanks for looking at this Mirko! I had a look at JSON::Validator version 0.96. There indeed seemed to be some issues with the tests in the latest version. I noticed there is a commit to json-validator in GitHub as well as some IRC discussion in #swagger @irc.perl.org suggesting that tests are broken on an older version of Test::More. Using the latest version of Test::More passes the tests. I tested an older version of Test::More (1.001014) and tests are indeed failing: $ sudo cpanm Test::More@1.001014 --> Working on Test::More Fetching http://backpan.perl.org/authors/id/E/EX/EXODIST/Test-Simple-1.001014.tar.gz ... OK Configuring Test-Simple-1.001014 ... OK Building and testing Test-Simple-1.001014 ... OK Successfully installed Test-Simple-1.001014 (downgraded from 1.302078) 1 distribution installed $ prove t .. .. Test Summary Report ------------------- t/acceptance.t (Wstat: 0 Tests: 265 Failed: 0) TODO passed: 45-47, 49-52, 56-57 t/jv-formats.t (Wstat: 256 Tests: 28 Failed: 1) Failed test: 27 Non-zero exit status: 1 t/jv-object.t (Wstat: 256 Tests: 19 Failed: 1) Failed test: 18 Non-zero exit status: 1 t/openapi-formats.t (Wstat: 512 Tests: 24 Failed: 2) Failed tests: 12, 22 Non-zero exit status: 2 t/openapi-response.t (Wstat: 0 Tests: 7 Failed: 0) TODO passed: 1 t/openapi.t (Wstat: 0 Tests: 8 Failed: 0) TODO passed: 1 Files=44, Tests=558, 11 wallclock secs ( 0.24 usr 0.06 sys + 9.82 cusr 1.04 csys = 11.16 CPU) Result: FAIL Whereas the new version passes: $ sudo cpanm Test::More --> Working on Test::More Fetching http://www.cpan.org/authors/id/E/EX/EXODIST/Test-Simple-1.302078.tar.gz ... OK Configuring Test-Simple-1.302078 ... OK Building and testing Test-Simple-1.302078 ... OK Successfully installed Test-Simple-1.302078 (upgraded from 1.001014) 1 distribution installed $ prove t .. .. All tests successful. Test Summary Report ------------------- t/acceptance.t (Wstat: 0 Tests: 265 Failed: 0) TODO passed: 45-47, 49-52, 56-57 t/jv-object.t (Wstat: 0 Tests: 19 Failed: 0) TODO passed: 9 t/openapi-formats.t (Wstat: 0 Tests: 24 Failed: 0) TODO passed: 7, 10-11 t/openapi-response.t (Wstat: 0 Tests: 7 Failed: 0) TODO passed: 1 t/openapi.t (Wstat: 0 Tests: 8 Failed: 0) TODO passed: 1 Files=44, Tests=558, 12 wallclock secs ( 0.23 usr 0.08 sys + 10.66 cusr 1.12 csys = 12.09 CPU) Result: PASS Could this be related to the issue you were having with the tests? Anyway, as a side note to comment #18, Mojolicious::Plugin::OpenAPI 1.13 has a requirement for JSON::Validator 0.95, which is not enough for us. We need JSON::Validator 0.96 or newer. It will be a requirement in currently unreleased Mojolicious::Plugin::OpenAPI version 1.14. Should we wait for that or explicitly define requirement for JSON::Validator 0.96 in PerlDependencies.pm? What comes to Mojolicious - I've attempted to run our REST API on Mojolicious 7.26 and so far it seems ok. Tests are also passing for me. Created attachment 61163 [details] [review] Bug 18137: (follow-up) operationId must be unique operationId has the following documentation: "Unique string used to identify the operation. The id MUST be unique among all operations described in the API." This patch modifies operationIds to be unique accross our API operations. Created attachment 61166 [details] [review] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 (In reply to Lari Taskula from comment #25) > Anyway, as a side note to comment #18, Mojolicious::Plugin::OpenAPI 1.13 has > a requirement for JSON::Validator 0.95, which is not enough for us. We need > JSON::Validator 0.96 or newer. It will be a requirement in currently > unreleased Mojolicious::Plugin::OpenAPI version 1.14. Should we wait for > that or explicitly define requirement for JSON::Validator 0.96 in > PerlDependencies.pm? We can define it now and maybe have to adjust in case it changes. Mojolicious::Plugin::OpenAPI 1.14 is not released yet, I can't work further on the Debian package until it is. Created attachment 62780 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI as Perl dependency This patch also upgrades Mojolicious version requirement to 6.40 as it is the minimum requirement set by Mojolicious::Plugin::OpenAPI. To test: 1. sudo cpanm Mojolicious::Plugin::OpenAPI@1.10 Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 62781 [details] [review] Bug 18137: Add useful Koha::Exceptions Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 62782 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 62783 [details] [review] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 62784 [details] [review] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 62785 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 62786 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 62787 [details] [review] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Created attachment 62788 [details] [review] Bug 18137: (QA-follow-up) Fix pod fail Created attachment 62789 [details] [review] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 Just rebasing these on master and squashing some forgotten "use"s into Koha::REST::V1::Auth. (In reply to Mirko Tietgen from comment #28) > Mojolicious::Plugin::OpenAPI 1.14 is not released yet, I can't work further > on the Debian package until it is. @ Lari: JSON::Validator 0.97 has been released. Mojolicious::Plugin::OpenAPI 1.14 has not been released. If I understand correctly, Mojolicious::Plugin::OpenAPI 1.14 is not a requirement for us, just JSON::Validator 0.96?.Then please specify 0.96 as a dependency and I will check if I can build JSON::Validator 0.97 and Mojolicious::Plugin::OpenAPI 1.13 now after the update. (In reply to Mirko Tietgen from comment #40) > (In reply to Mirko Tietgen from comment #28) > > > Mojolicious::Plugin::OpenAPI 1.14 is not released yet, I can't work further > > on the Debian package until it is. > > @ Lari: > > JSON::Validator 0.97 has been released. > Mojolicious::Plugin::OpenAPI 1.14 has not been released. > > If I understand correctly, Mojolicious::Plugin::OpenAPI 1.14 is not a > requirement for us, just JSON::Validator 0.96?.Then please specify 0.96 as a > dependency and I will check if I can build JSON::Validator 0.97 and > Mojolicious::Plugin::OpenAPI 1.13 now after the update. (In reply to Mirko Tietgen from comment #40) > (In reply to Mirko Tietgen from comment #28) > > > Mojolicious::Plugin::OpenAPI 1.14 is not released yet, I can't work further > > on the Debian package until it is. > > @ Lari: > > JSON::Validator 0.97 has been released. > Mojolicious::Plugin::OpenAPI 1.14 has not been released. > > If I understand correctly, Mojolicious::Plugin::OpenAPI 1.14 is not a > requirement for us, just JSON::Validator 0.96?.Then please specify 0.96 as a > dependency and I will check if I can build JSON::Validator 0.97 and > Mojolicious::Plugin::OpenAPI 1.13 now after the update. Thanks for checking! I'll do that! Also, Tomas experienced issues in installing Mojolicious 6.40 together with M::P::OpenAPI 1.10 - I couldn't reproduce this issue, but perhaps it is a safe bet to upgrade Mojolicious at the same time. We have been successfully running Koha REST API on Mojolicious versions between 7.26-7.29, and Koha's REST tests are also passing on the latest release 7.31. This upgrade will fail test t/db_dependent/api/v1/swagger/definitions.t. I will try to patch that in another Bug. Created attachment 62816 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> (In reply to Lari Taskula from comment #41) > This upgrade will fail test t/db_dependent/api/v1/swagger/definitions.t. I > will try to patch that in another Bug. Bug 18508 Created attachment 63025 [details] [review] [SIGNED-OFF] Bug 18137: Add useful Koha::Exceptions Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63026 [details] [review] [SIGNED-OFF] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63027 [details] [review] [SIGNED-OFF] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63028 [details] [review] [SIGNED-OFF] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63029 [details] [review] [SIGNED-OFF] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63030 [details] [review] [SIGNED-OFF] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63031 [details] [review] [SIGNED-OFF] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63032 [details] [review] [SIGNED-OFF] Bug 18137: (QA-follow-up) Fix pod fail Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63033 [details] [review] [SIGNED-OFF] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63034 [details] [review] [SIGNED-OFF] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Ok, I tested this successfully using: $ sudo cpanm Mojolicious@7.31 $ sudo cpanm JSON::Validator@0.96 $ sudo cpanm Mojolicious::Plugin::OpenAPI@1.13 Tests pass: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/api/v1/ t/db_dependent/api/v1/auth.t ..... Use of uninitialized value $lasttime in numeric lt (<) at /home/vagrant/kohaclone/C4/Auth.pm line 1673. t/db_dependent/api/v1/auth.t ..... ok t/db_dependent/api/v1/cities.t ... ok t/db_dependent/api/v1/holds.t .... ok t/db_dependent/api/v1/patrons.t .. ok All tests successful. Files=4, Tests=31, 10 wallclock secs ( 0.05 usr 0.00 sys + 8.52 cusr 1.26 csys = 9.83 CPU) Result: PASS I also tested the API using Firefox's HttpRequester, with excellent results. I'm signing this patchset, but will wait for Mirko to tell about the deps, because otherwise we could end up breaking the stable release due to unpackaged dependencies. Created attachment 63177 [details] [review] Bug 18137: Add useful Koha::Exceptions Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63178 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63179 [details] [review] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63180 [details] [review] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63181 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63182 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63183 [details] [review] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63184 [details] [review] Bug 18137: (QA-follow-up) Fix pod fail Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63185 [details] [review] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63186 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I messed up with the packages/obsoletes, re-submitting. Created attachment 63187 [details] [review] Bug 18137: Add useful Koha::Exceptions Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63188 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63189 [details] [review] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63190 [details] [review] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63191 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63192 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63193 [details] [review] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63194 [details] [review] Bug 18137: (QA-follow-up) Fix pod fail Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63195 [details] [review] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63196 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63283 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (In reply to Lari Taskula from comment #76) > Created attachment 63283 [details] [review] [review] > Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as > dependencies > > Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> > > Signed-off-by: Josef Moravec <josef.moravec@gmail.com> > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Rebased on master. This patch now maintains requirement as defined by Bug 17190. I have uploaded libmojolicious-perl_7.21+dfsg-1~kohadev1 to the unstable repository. I have uploaded preliminary versions of libjson-validator-perl_0.97-1~kohadev1_all.deb libmojolicious-plugin-openapi-perl_1.15-1~kohadev1_all.deb to the unstable repository According to Tomás, the newer versions break dev setups, so we are back to libmojolicious-perl 6.15 and libjson-validator-perl 0.67 for the time being. Created attachment 65106 [details] [review] Bug 18137: Add useful Koha::Exceptions Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65107 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65108 [details] [review] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65109 [details] [review] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65110 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65111 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65112 [details] [review] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65113 [details] [review] Bug 18137: (QA-follow-up) Fix pod fail Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65114 [details] [review] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65115 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> The QA script reports POD coverage issues. Created attachment 65404 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Edit (tcohen): I've changed the version numbers to match those Mirko has already successfully packaged and are known to work for this patchset. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I get a warning and a failure from tests: t/db_dependent/api/v1/auth.t ................. Use of uninitialized value $lasttime in numeric lt (<) at /home/vagrant/kohaclone/C4/Auth.pm line 1679. t/db_dependent/api/v1/swagger/definitions.t .. Can't locate object method "cache_dir" via package "Swagger2::SchemaValidator" at /usr/share/perl5/Swagger2.pm line 41. (In reply to Jonathan Druart from comment #94) > I get a warning and a failure from tests: > > t/db_dependent/api/v1/auth.t ................. Use of uninitialized value > $lasttime in numeric lt (<) at /home/vagrant/kohaclone/C4/Auth.pm line 1679. > > t/db_dependent/api/v1/swagger/definitions.t .. Can't locate object method > "cache_dir" via package "Swagger2::SchemaValidator" at > /usr/share/perl5/Swagger2.pm line 41. See Bug 18508 I missed definitions.t on testing. And confirm this is fixed by Lari's patch. Note: I'm all for removing definitions.t BTW, just wanted to note it. Created attachment 65713 [details] [review] Bug 18137: (QA followup) Make sure the session exists and is expired on expiration tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 65714 [details] [review] Bug 18137: (QA followup) Make sure the session exists and is expired on expiration tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Comment on attachment 65107 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI Review of attachment 65107 [details] [review]: ----------------------------------------------------------------- ::: Koha/REST/V1/Auth.pm @@ +57,5 @@ > + json => { error => 'Something went wrong, check the logs.' } > + ); > + } > + if ($_->isa('Koha::Exceptions::UnderMaintenance')) { > + return $c->render(status => 503, json => { error => $_->error }); Should these calls to render not all be 'openapi => { error => $_->error }` as aposed to `json => { error => $_->error }`.. else none of these responses are getting validated. (In reply to Martin Renvoize from comment #99) > Comment on attachment 65107 [details] [review] [review] > Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI > > Review of attachment 65107 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: Koha/REST/V1/Auth.pm > @@ +57,5 @@ > > + json => { error => 'Something went wrong, check the logs.' } > > + ); > > + } > > + if ($_->isa('Koha::Exceptions::UnderMaintenance')) { > > + return $c->render(status => 503, json => { error => $_->error }); > > Should these calls to render not all be 'openapi => { error => $_->error }` > as aposed to `json => { error => $_->error }`.. else none of these responses > are getting validated. That's because the auth check is done using ->under, and as the check itself is calling ->render, it is breaking the dispatch chain (not breaking like in 'stop working' but like short-circuiting to another way of returning the error. Clarifying to avoid possible idiomatic issues). I think we should leave it as-is and deal with this problem on a separate bug, or leave it. Because it could involve a big re-engineering to make the check let the chain continue while propagating the exception. I can't even imagine how to do it without adding irrelevant checks and repetitive code to controller classes. Created attachment 66802 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66803 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66804 [details] [review] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66805 [details] [review] Bug 18137: (QA-follow-up) Fix pod fail Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66806 [details] [review] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66807 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Edit (tcohen): I've changed the version numbers to match those Mirko has already successfully packaged and are known to work for this patchset. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66808 [details] [review] Bug 18137: (QA followup) Make sure the session exists and is expired on expiration tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Created attachment 66809 [details] [review] Bug 18137: Add useful Koha::Exceptions Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66810 [details] [review] Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI This patch migrates from Swagger2 to Mojolicious::Plugin::OpenAPI as Swagger2 is no longer actively maintained. This migration involves some minor changes to our Swagger specification documents and to controllers. Each operation is migrated in following patches separately. Please see Mojolicious::Plugin::OpenAPI and its tutorial for more documentation. The patch also refactors some API authentication -related code by taking advantage of Koha::Exceptions. Authentication is now handled via Mojolicious's "under->to" functionality. The actual authentication & authorization checks are moved to Koha::REST::V1::Auth. Added a HTTP 503 response for when database update is required, instead of returning an authentication failure as before. To test: 1. prove t/db_dependent/api/v1/auth.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66811 [details] [review] Bug 18137: Make /patrons Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. - fixing failing test due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66812 [details] [review] Bug 18137: Make /cities Mojolicious::Plugin::OpenAPI compatible Also: - adding some missing and new response definitions into Swagger spec. To test: 1. prove t/db_dependent/api/v1/cities.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66813 [details] [review] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible Also - adding some missing and new response definitions into Swagger spec. - fixing failing tests due to Bug 17932's change of boolean values To test: 1. prove t/db_dependent/api/v1/holds.t Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66814 [details] [review] Bug 18137: Remove x-mojo-around-action Mojolicious::Plugin::OpenAPI does not support x-mojo-around action. This patch removes it from our specification document. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66815 [details] [review] Bug 18137: (follow-up) Fix handling DBIx::Class::Exception messages - DBIx::Class::Exception should use ->{msg} Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66816 [details] [review] Bug 18137: (QA-follow-up) Fix pod fail Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66817 [details] [review] Bug 18137: (follow-up) Sort definitions.json Before this file grows, we should sort it alphabetically. To test: 1. prove t/db_dependent/api/v1 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66818 [details] [review] Bug 18137: List Mojolicious::Plugin::OpenAPI and JSON::Validator as dependencies Edit (tcohen): I've changed the version numbers to match those Mirko has already successfully packaged and are known to work for this patchset. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 66819 [details] [review] Bug 18137: (QA followup) Make sure the session exists and is expired on expiration tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Rebased against master, which changed Koha::REST::V1::Hold a bit. Pushed to master for 17.11, thanks to everybody involved! Enhancement not pushed to 17.05.x |