At the moment, Koha plugins run via Plack::App::CGIBin, which buffers responses and otherwise prevents plugins from fully leveraging the power of Plack and Mojolicious.
Created attachment 139308 [details] [review] Bug 31380: Build core functionality for Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool 4) Note that it looks perfect 5) The following URL will generate a 500 error, but technically the Koha Plugin could easily be crafted to support both CGI and Mojolicious::Controller objects: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool
Created attachment 139309 [details] Koha Plugin with Mojolicious controller
The Koha::Auth class contains a function which should use Bug 31389 instead, so that will be my next thing to do. Currently, the Koha::Template class generates the following warnings: [2022/08/18 00:55:50] [WARN] no query in gettemplate at /kohadevbox/koha/C4/Templates.pm line 216. [2022/08/18 00:55:50] [WARN] no query in themelanguage at /kohadevbox/koha/C4/Templates.pm line 276. [2022/08/18 00:55:50] [WARN] no query in themelanguage at /kohadevbox/koha/C4/Templates.pm line 276. Bug 31390 should take care of those warnings though.
Created attachment 139312 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
Created attachment 139313 [details] Koha Plugin with Mojolicious controller
I think that this is ready for sign off. Let me know if you have any additional feedback, and I can keep working on it. Otherwise, I'd consider this done!
Created attachment 139314 [details] [review] Bug 31380: Build core functionality for Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool 4) Note that it looks perfect 5) The following URL will generate a 500 error, but technically the Koha Plugin could easily be crafted to support both CGI and Mojolicious::Controller objects: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool
Created attachment 139315 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
Created attachment 139316 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both
Created attachment 139317 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
Created attachment 139318 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both
Created attachment 139319 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
Created attachment 139320 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both
Created attachment 139321 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
Created attachment 139322 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both
Created attachment 139323 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
Created attachment 139324 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both
Created attachment 139325 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
I'm pretty happy with these patches. The Koha::Mojo::Staff class will evolve over time, but I think that it's more than sufficient for running Koha Plugins now.
Created attachment 139327 [details] [review] Bug 31380: Add unit tests
Created attachment 139328 [details] [review] Bug 31380: Add unit tests
Created attachment 139394 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 139395 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 139396 [details] [review] Bug 31380: Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 139818 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 139819 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 139820 [details] [review] Bug 31380: Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Ha, meant to sign-off on all three but Kyle already did, so. Just tested it out and I love this. Great work!
(In reply to Paul Derscheid from comment #28) > Ha, meant to sign-off on all three but Kyle already did, so. > Just tested it out and I love this. Great work! An additional signoff line only provides more evidence of testing. So never hesitate to add one :)
David, just glancing over but this code looks very good. Just thinking that Koha::Auth may be quite ambitiously named for now. How long will we have C4::Auth and Koha::Auth side by side? Do we actually need the 'staff' script or was it jusy easy while testing?
(In reply to Marcel de Rooy from comment #30) > David, just glancing over but this code looks very good. Thanks, Marcel :) > Just thinking that Koha::Auth may be quite ambitiously named for now. > How long will we have C4::Auth and Koha::Auth side by side? Probably for a while, but Koha::Auth can be a target for any new auth work or for refactoring work. In Bug 31389, I've created Koha::Auth::Permissions, which contains a function to be used in C4::Auth. I suppose we could always use Koha::Mojo::Auth or some other Koha::Auth::* module instead of Koha::Auth. > Do we actually need the 'staff' script or was it jusy easy while testing? The "staff" script is for non-Plack enabled Koha instances. "api/v1/app.pl" works similarly. Although, in plack.psgi, the API gets loaded by loading that file whereas I just load the Koha::Mojo::Staff module as I think that's cleaner.
Does anyone need anything more from me on this one?
Created attachment 140673 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140674 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140675 [details] [review] Bug 31380: Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This one took me a little while to wrap my head around.. probably just too many bugs on the mind. Anywho, now I have it looks great and all works as expected.. I think we'll need to carefully document it somewhere to showcase it. Tests are passing and the changes are covered nicely. Passing QA
We certainly need docs.
I like where this is headed to. I think we need some more tests here, particularly for the 'staff_authorize' and 'prepare_template', unless I'm missing something. I noticed at first glance a subtle issue with __koha__user__ vs. __koha_user__ that is not caught by the provided tests. Otherwise this is great! It would be great (also) if you could perltidy some of this code. I usually select the code I want to tidy, and just ctrl+k ctrl+f on VScode. Please do it inline. Thanks!
(In reply to Tomás Cohen Arazi from comment #38) > I think we need some more tests here, particularly for the 'staff_authorize' > and 'prepare_template', unless I'm missing something. I noticed at first > glance a subtle issue with __koha__user__ vs. __koha_user__ that is not > caught by the provided tests. Yep, can certainly do that, when I have some time. > It would be great (also) if you could perltidy some of this code. I usually > select the code I want to tidy, and just ctrl+k ctrl+f on VScode. Please do > it inline. Thanks! What's the issue? I think it passed the QA tools, so that should've pointed out any perltidy issues? Because I'm on Windows, I need to do all my editing within the Docker container, so no VScode for me. But it looks like KTD has a ":Tidy" command in Vim, so I could use that. But this is fresh code that I thought I was writing in accordance with the Koha Coding Guidelines? Do we need to update those or am I just out of date?
(In reply to Tomás Cohen Arazi from comment #38) > I think we need some more tests here, particularly for the 'staff_authorize' > and 'prepare_template', unless I'm missing something. I noticed at first > glance a subtle issue with __koha__user__ vs. __koha_user__ that is not > caught by the provided tests. Good catch with __koha__user__. I hadn't used that return value anywhere yet. Following my unit tests on Bug 30962 and looking at t/Koha/REST/Plugin/Query.t, I think I now know how to write the tests you're talking about. I'll give it a go. > It would be great (also) if you could perltidy some of this code. I usually > select the code I want to tidy, and just ctrl+k ctrl+f on VScode. Please do > it inline. Thanks! I think there's a problem with the perltidy vim macro in KTD but otherwise I'm open to it...
Created attachment 144968 [details] Koha Plugin with Mojolicious controller Updating attached plugin to use updated staff interface templates and to use some code updates
Created attachment 144969 [details] [review] Bug 31380: Allow Koha Plugins to run using Mojolicious controllers This patch adds a new mount point in plack.psgi, which makes Koha able to run Mojolicious controllers. For example: http://localhost:8081/cgi-bin/koha/staff/plugins/run 0a) Apply patch 0b) cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi 0c) koha-plack --restart kohadev 1) Upload Koha Plugin koha-plugin-mojolicious.kpz 2) koha-plack --restart kohadev 3) Go to http://localhost:8081/cgi-bin/koha/plugins/plugins-home.pl 4) Next to "Mojolicious Test", click "Actions", then click "Run tool" 5) Note that the web page looks perfect 6) Note that the URL is a bit different to normal: http://localhost:8081/cgi-bin/koha/staff/plugins/run?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool instead of: http://localhost:8081/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3AProsentient%3A%3AMojolicious&method=tool Note: The test Koha plugin will only work with a Mojolicious controller, but it could be easily made to work with both 7) Run the above test using an unauthenticated user, use a staff user without plugin permissions, try with a staff user with plugin permissions, and try with a superlibrarian 8) Run tests prove t/db_dependent/Koha/Auth.t prove t/db_dependent/Koha/Mojo/Plugins/Core.t prove t/db_dependent/Koha/Template.t
Created attachment 144970 [details] [review] Bug 31380: Add plugin hook for choosing Mojolicious or CGI script controller This change checks for a "use_mojolicious" method in the plugin in order to determine whether it should use a Mojolicious controller or a CGI script controller. Note that the check is for the existence of the method, so it is backwards compatible with all existing Koha Plugins.
I've added a lot more tests, streamlined the code more, and resolved an issue with the authorization process. I could probably add a couple more end-to-end tests but I've run out of time today and I'm away the next 5 days. Moving this back to "Needs Signoff" to get eyes on it in the meantime...