Bug 20582 - Turn Koha into a Mojolicious application
Summary: Turn Koha into a Mojolicious application
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Target Milestone: ---
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 20630 20635
  Show dependency treegraph
 
Reported: 2018-04-13 16:31 UTC by Julian Maurice
Modified: 2019-01-30 14:56 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20582: Turn Koha into a Mojolicious application (?) (11.44 KB, patch)
2018-04-13 16:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Turn Koha into a Mojolicious application (8.69 KB, patch)
2018-04-19 06:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Add routes for some svc/ scripts (7.51 KB, patch)
2018-04-19 06:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Enable SetEnv middleware in bin/koha.psgi (1.25 KB, patch)
2018-04-19 06:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Flush memory caches before every request (814 bytes, patch)
2018-04-19 06:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Fix memory cache flush (1.62 KB, patch)
2018-09-30 09:14 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Handle multipart content-type (1.99 KB, patch)
2018-10-31 09:50 UTC, Julian Maurice
Details | Diff | Splinter Review
Fix PDF generation (631 bytes, patch)
2018-11-02 02:54 UTC, Jerome Charaoui
Details | Diff | Splinter Review
Bug 20582: Handle multipart content-type (1.00 KB, patch)
2018-11-17 11:22 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Turn Koha into a Mojolicious application (8.75 KB, patch)
2019-01-30 14:45 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20582: Add routes for some svc/ scripts (7.57 KB, patch)
2019-01-30 14:46 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20582: Enable SetEnv middleware in bin/koha.psgi (1.30 KB, patch)
2019-01-30 14:46 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20582: Flush memory caches before every request (869 bytes, patch)
2019-01-30 14:46 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20582: Fix memory cache flush (1.67 KB, patch)
2019-01-30 14:46 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20582: Handle multipart content-type (1.06 KB, patch)
2019-01-30 14:46 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2018-04-13 16:31:55 UTC

    
Comment 1 Julian Maurice 2018-04-13 16:32:45 UTC
Created attachment 74173 [details] [review]
Bug 20582: Turn Koha into a Mojolicious application (?)

This patch is a proof-of-concept of Koha as a Mojolicious application

This has several benefits:
- Run `morbo script/koha` and you're set : fully functional Koha without
  apache2/nginx (URL rewrites and static files are handled by the app)
- apache2/nginx configuration is much simpler (an example of nginx
  configuration is included in the patch)
- starman and plack middlewares can still be used for debug or gzip
  compression for instance (see script/koha.psgi)
- All that is needed to run koha is in the source code and not hidden in
  an obscure directory (I'm looking at you, debian/templates/plack.psgi)
- Using Test::Mojo we can test the whole application, as we do with the
  REST API (which is a Mojolicious application too)
- It's another step in the direction of dropping CGI support
- It also opens a way for converting CGI scripts into Mojolicious
  controllers and actions (even if that's not possible at the moment
  because of the authentication code)
- It requires zero changes to existing files, so it should apply nicely
  forever (no rebase needed \o/)

A few downsides too:
- It is not complete : some things from debian/templates/plack.psgi are
  missing, like the L1 cache flush or Koha::Middleware::SetEnv. But that
  shouldn't be too hard to add.
- Already mentioned above, but the actual state of get_template_and_user
  and checkauth completely forbids the use of Mojolicious controllers.
  We really need to rewrite C4::Auth
- Not sure if it's a downside or not, but as OPAC and Intranet are on
  the same host:port, if you authenticate on OPAC you will be
  authenticated on Intranet too.
  This can probably be changed if it's really a problem.
- ... see FIXME in the code

Other remarks:
- It uses the same mechanism as Plack::App::CGIBin to deal with CGI
  scripts, so it should be equivalent in terms of performance
- I did not test the install and upgrade process (yet)

How to test ?
- Run `morbo script/koha`, then go to:
  - http://localhost:3000/  -  A basic page that redirects you to OPAC
  - Then navigate in the OPAC, searching for bugs
  - http://localhost:3000/intranet  -  The same page that redirects you
                                       to intranet
  - Then navigate in the Intranet, still searching for bugs
  - http://localhost:3000/api/v1/...  -  The REST API
- Read the code (and the comments), it's not very long
Comment 2 Katrin Fischer 2018-04-13 17:13:55 UTC
Hi Julian, a bit over my head here, but sounds like something that should go on the mailing list or the next dev meeting agenda too.
Comment 3 Julian Maurice 2018-04-16 07:48:30 UTC
(In reply to Katrin Fischer from comment #2)
> Hi Julian, a bit over my head here, but sounds like something that should go
> on the mailing list or the next dev meeting agenda too.

I wanted to but didn't have the time. I will try to write something later today
Comment 4 Julian Maurice 2018-04-16 17:51:07 UTC
(In reply to Julian Maurice from comment #3)
> I wanted to but didn't have the time. I will try to write something later
> today
http://lists.koha-community.org/pipermail/koha-devel/2018-April/044465.html

https://framavox.org/d/tTGf0Bzd/bug-20582-turn-koha-into-a-mojolicious-application
Comment 5 Kyle M Hall 2018-04-18 13:01:37 UTC
I love this! Moving Koha into a Mojolicious would make Koha a first class plack enabled application! I think Koha would benefit greatly in the long term from using a good framework like Mojolicious.
Comment 6 Julian Maurice 2018-04-19 06:49:37 UTC
Created attachment 74517 [details] [review]
Bug 20582: Turn Koha into a Mojolicious application

This patch is a proof-of-concept of Koha as a Mojolicious application

This has several benefits:
- Run `morbo bin/koha` and you're set : fully functional Koha without
  apache2/nginx (URL rewrites and static files are handled by the app)
- apache2/nginx configuration is much simpler (an example of nginx
  configuration is included in the patch)
- starman and plack middlewares can still be used for debug or gzip
  compression for instance (see bin/koha.psgi)
- All that is needed to run koha is in the source code and not hidden in
  an obscure directory (I'm looking at you, debian/templates/plack.psgi)
- Using Test::Mojo we can test the whole application, as we do with the
  REST API (which is a Mojolicious application too)
- It's another step in the direction of dropping CGI support
- It also opens a way for converting CGI scripts into Mojolicious
  controllers and actions (even if that's not possible at the moment
  because of the authentication code)
- It requires zero changes to existing files, so it should apply nicely
  forever (no rebase needed \o/)

A few downsides too:
- It is not complete : some things from debian/templates/plack.psgi are
  missing, like the L1 cache flush or Koha::Middleware::SetEnv. But that
  shouldn't be too hard to add.
- Already mentioned above, but the actual state of get_template_and_user
  and checkauth completely forbids the use of Mojolicious controllers.
  We really need to rewrite C4::Auth
- Not sure if it's a downside or not, but as OPAC and Intranet are on
  the same host:port, if you authenticate on OPAC you will be
  authenticated on Intranet too.
  This can probably be changed if it's really a problem.
- ... see FIXME in the code

Other remarks:
- It uses the same mechanism as Plack::App::CGIBin to deal with CGI
  scripts, so it should be equivalent in terms of performance

How to test ?
- Run `morbo bin/koha`, then go to:
  - http://localhost:3000/  -  Redirects to OPAC main page
  - Then navigate in the OPAC, searching for bugs
  - http://localhost:3000/intranet  -  Redirects to intranet main page
  - Then navigate in the Intranet, still searching for bugs
  - http://localhost:3000/api/v1/...  -  The REST API
  - Test the web installer
- Read the code (and the comments), it's not very long
Comment 7 Julian Maurice 2018-04-19 06:49:45 UTC
Created attachment 74518 [details] [review]
Bug 20582: Add routes for some svc/ scripts

svc/club/enroll, svc/club/cancel_enrollment and svc/report are available
in different versions: one for the OPAC, one for the intranet
This patch creates new routes that override the CGI scripts and that
work for both OPAC and intranet
Comment 8 Julian Maurice 2018-04-19 06:49:50 UTC
Created attachment 74519 [details] [review]
Bug 20582: Enable SetEnv middleware in bin/koha.psgi

and make sure the psgi env is passed to CGI scripts
Comment 9 Julian Maurice 2018-04-19 06:49:56 UTC
Created attachment 74520 [details] [review]
Bug 20582: Flush memory caches before every request
Comment 10 Julian Maurice 2018-04-19 06:54:15 UTC
I did a bit of cleanup in the initial patch and added some missing parts in additional patches.
I think it's now ready for testing.
Comment 11 David Cook 2018-04-19 23:42:26 UTC
After reviewing the patches, this seems awesome! It seems like a great way to move Koha into the 21st century and allow for incremental improvements.

I see it doesn't need PERL5LIB but what about KOHA_CONF?
Comment 12 Julian Maurice 2018-04-20 07:11:15 UTC
(In reply to David Cook from comment #11)
> After reviewing the patches, this seems awesome! It seems like a great way
> to move Koha into the 21st century and allow for incremental improvements.
> 
> I see it doesn't need PERL5LIB but what about KOHA_CONF?

That's a good question... I've done nothing to pass this environment variable to the CGI environment. I'm wondering what value it actually uses... probably the /etc/koha/koha-conf.xml (because it defaults to that), and that would be a bug. I will look at it.
Anyway, it is still required.
Comment 13 Julian Maurice 2018-04-20 07:32:13 UTC
Actually, C4::Context get compiled before the execution of a CGI script, so it gets the same environment as the Mojolicious app (so if you run `env KOHA_CONF=foo morbo bin/koha` C4::Context.pm will load foo)
I think it's the same with debian/templates/plack.psgi
Comment 14 Martin Renvoize 2018-05-17 15:30:04 UTC
Comment on attachment 74517 [details] [review]
Bug 20582: Turn Koha into a Mojolicious application

Review of attachment 74517 [details] [review]:
-----------------------------------------------------------------

::: Koha/App/Koha.pm
@@ +1,3 @@
> +package Koha::App::Koha;
> +
> +use Modern::Perl;

This really isn't required as you're using Mojo::Base below which turns on pretty much all the useful stuff from Modern::Perl and is more stable in it's actions...

use strict;
use warnings;
use utf8;
use feature ':5.10';
use IO::Handle ();
require Mojolicious;
push @ISA, 'Mojolicious';
sub has { Mojo::Base::attr(__PACKAGE__, @_) }
Comment 15 David Cook 2018-06-22 00:14:04 UTC
Any progress on this one in the Koha community?
Comment 16 Julian Maurice 2018-06-22 07:41:02 UTC
(In reply to Martin Renvoize from comment #14)
> This really isn't required as you're using Mojo::Base below which turns on
> pretty much all the useful stuff from Modern::Perl and is more stable in
> it's actions...

If it doesn't do any harm I'd say we should keep using Modern::Perl for consistency, but it's not a strong opinion.


(In reply to David Cook from comment #15)
> Any progress on this one in the Koha community?

The bug status is Needs Signoff, which means it's waiting for testers ;)
Comment 17 Julian Maurice 2018-09-30 09:14:40 UTC
Created attachment 79648 [details] [review]
Bug 20582: Fix memory cache flush

Cache flush should be in around_action hook instead of before_dispatch
Otherwise it is flushed before every request (even static files
requests)
Also, make sure the Cache::Memory cache is really flushed (not only L1
cache)
Comment 18 Jerome Charaoui 2018-10-30 22:48:27 UTC
Has anyone tested this with file uploads? For some reason I'm unable to upload any files. For example, when I try to stage a MARC upload, the application responds with an HTTP 200 OK with the following content: {"fileid":null,"errors":null,"status":"failed"}
Comment 19 David Cook 2018-10-31 05:00:47 UTC
Comment on attachment 74517 [details] [review]
Bug 20582: Turn Koha into a Mojolicious application

Review of attachment 74517 [details] [review]:
-----------------------------------------------------------------

::: Koha/App/Plugin/CGIBinKoha.pm
@@ +44,5 @@
> +        $c->rendered;
> +    })->name('cgi');
> +}
> +
> +sub _psgi_env {

Could we use https://mojolicious.org/perldoc/Mojo/Message/Request#env to just get a PSGI environment hash from Mojolicious? 

I haven't delved into these patches, but aren't we basically just using Mojolicious as a proxy to CGI::Emulate::PSGI, which looks like it comes straight out of Plack::App::WrapCGI anyway?
Comment 20 David Cook 2018-10-31 05:02:32 UTC
Jerome and I were chatting in IRC and it seems like $c->req->body might not be properly handling multipart/form-data data.

But could we just take the psgi.input straight from Mojolicious and plug it into CGI::Emulate::PSGI's app callback? I don't see why we couldn't do that in theory.
Comment 21 Julian Maurice 2018-10-31 09:24:28 UTC
(In reply to David Cook from comment #20)
> Jerome and I were chatting in IRC and it seems like $c->req->body might not
> be properly handling multipart/form-data data.
> 
> But could we just take the psgi.input straight from Mojolicious and plug it
> into CGI::Emulate::PSGI's app callback? I don't see why we couldn't do that
> in theory.

I don't think we can, psgi.input is defined only when running in a PSGI environment (app run by starman for instance) and $c->req->env is an empty hashref when the app is run by morbo or hypnotoad.
Comment 22 Julian Maurice 2018-10-31 09:50:43 UTC
Created attachment 81687 [details] [review]
Bug 20582: Handle multipart content-type

Mojo::Message::Request's body attribute does not contain anything when
the content-type is multipart/*
We have to rebuild the multipart body for CGI scripts

Test plan:
1. Go to Tools » Upload and try to upload a file
Comment 23 David Cook 2018-10-31 23:06:55 UTC
(In reply to Julian Maurice from comment #21)
> I don't think we can, psgi.input is defined only when running in a PSGI
> environment (app run by starman for instance) and $c->req->env is an empty
> hashref when the app is run by morbo or hypnotoad.

Ahhh, I see. I forgot about morbo and hypnotoad. Neither CGI nor PSGI, they're totally bespoke Mojolicious web servers it seems? I guess that's why they would be more optimized for Mojolicious...
Comment 24 David Cook 2018-10-31 23:09:01 UTC
(In reply to Julian Maurice from comment #22)
> Created attachment 81687 [details] [review] [review]
> Bug 20582: Handle multipart content-type
> 
> Mojo::Message::Request's body attribute does not contain anything when
> the content-type is multipart/*
> We have to rebuild the multipart body for CGI scripts
> 
> Test plan:
> 1. Go to Tools » Upload and try to upload a file

Seems such a shame that Mojolicious doesn't just provide the actual original body of the message... 

I was just idly curious about this bug so I'll leave testing to Jerome.
Comment 25 Jerome Charaoui 2018-11-02 02:54:56 UTC
Created attachment 81857 [details] [review]
Fix PDF generation

Here's a patch to fix generating PDFs from the Acquisitions module.
Comment 26 Jerome Charaoui 2018-11-02 02:58:33 UTC
(In reply to Julian Maurice from comment #22)
> Created attachment 81687 [details] [review] [review]
> Bug 20582: Handle multipart content-type
> 
> Mojo::Message::Request's body attribute does not contain anything when
> the content-type is multipart/*
> We have to rebuild the multipart body for CGI scripts
> 
> Test plan:
> 1. Go to Tools » Upload and try to upload a file

Thank you for being so quick to provide a patch!

I can confirm that it fixes upload functions within Koha.
Comment 27 Julian Maurice 2018-11-03 12:24:53 UTC
(In reply to Jerome Charaoui from comment #26)
> Thank you for being so quick to provide a patch!
> 
> I can confirm that it fixes upload functions within Koha.

Thank you for testing it ;)

(In reply to Jerome Charaoui from comment #25)
> Created attachment 81857 [details] [review] [review]
> Fix PDF generation
> 
> Here's a patch to fix generating PDFs from the Acquisitions module.

How did you generate this patch ? It's modifying a path that doesn't exist (b/site/profile/files/koha/lib/Koha/App/Koha.pm), I bet it won't apply nicely with `git bz apply`.
Also the fix in itself references a path that only exist on 'standard' install (intranet/cgi-bin/acqui) so it won't work on a 'dev' install. Can you provide steps to reproduce the bug ?
Comment 28 Jerome Charaoui 2018-11-03 23:57:28 UTC
(In reply to Julian Maurice from comment #27)
> (In reply to Jerome Charaoui from comment #26)
> > Thank you for being so quick to provide a patch!
> > 
> > I can confirm that it fixes upload functions within Koha.
> 
> Thank you for testing it ;)
> 
> (In reply to Jerome Charaoui from comment #25)
> > Created attachment 81857 [details] [review] [review] [review]
> > Fix PDF generation
> > 
> > Here's a patch to fix generating PDFs from the Acquisitions module.
> 
> How did you generate this patch ? It's modifying a path that doesn't exist
> (b/site/profile/files/koha/lib/Koha/App/Koha.pm), I bet it won't apply
> nicely with `git bz apply`.
> Also the fix in itself references a path that only exist on 'standard'
> install (intranet/cgi-bin/acqui) so it won't work on a 'dev' install. Can
> you provide steps to reproduce the bug ?

Oops, sorry for fumbling that patch, my bad.

The following error message is generated when exporting a acquisitions basketgroup in PDF format (https://imgur.com/a/UmXpppW) :

[debug] GET "/cgi-bin/koha/acqui/basketgroup.pl"
[debug] Routing to a callback
[error] Undefined subroutine &CGI::Compile::ROOT::usr_share_koha_intranet_cgi_2dbin_acqui_basketgroup_2epl::printpdf called at /usr/share/koha/intranet/cgi-bin/acqui/basketgroup.pl line 213.
Comment 29 Jerome Charaoui 2018-11-05 19:12:37 UTC
I'd like to share some results of our first Koha/Mojolicious lab that happened earlier today. Functionality-wise everything was up to par. One thing I noticed is that the 10 hypnotoad workers quickly grew memory hungry. Here is the smemstat output:

  PID       Swap       USS       PSS       RSS User       Command          
  8719    28.3 M   501.0 M   502.7 M   525.6 M root       /usr/share/koha/bin/koha
  8718    71.6 M   484.9 M   486.6 M   509.4 M root       /usr/share/koha/bin/koha                      
  8726    28.3 M   483.7 M   485.4 M   508.3 M root       /usr/share/koha/bin/koha
  8724    28.3 M   479.7 M   481.4 M   504.4 M root       /usr/share/koha/bin/koha
  8721    28.3 M   454.2 M   456.0 M   478.9 M root       /usr/share/koha/bin/koha    
  8720    28.6 M   452.0 M   453.7 M   476.6 M root       /usr/share/koha/bin/koha
  8723    28.3 M   427.4 M   429.1 M   451.9 M root       /usr/share/koha/bin/koha               
 14932    28.3 M   306.5 M   308.2 M   330.9 M root       /usr/share/koha/bin/koha
 23653    28.4 M   247.6 M   249.4 M   272.3 M root       /usr/share/koha/bin/koha
 23103    28.3 M   240.3 M   242.1 M   264.9 M root       /usr/share/koha/bin/koha

We can see all the workers using up between 240 to 500M of unshared memory. When hypnotoad starts fresh, each worker uses up about 125M.

This is the current hypnotoad config:

{
  basepath => '/usr/share/koha',
  hypnotoad => {
    listen  => ['http://127.0.0.1:3000'],
    workers => 10,
    pid_file => '/run/koha/koha-app.pid'
  }
};

In order to deal with this I modified the config by adding "accepts => 200" so that workers get recycled more quickly. If you have any other ideas as to how we could deal with memory usage, please let us know!
Comment 30 David Cook 2018-11-05 23:26:13 UTC
(In reply to Jerome Charaoui from comment #29)
> In order to deal with this I modified the config by adding "accepts => 200"
> so that workers get recycled more quickly. If you have any other ideas as to
> how we could deal with memory usage, please let us know!

I think we'd have to do more profiling, but I have a couple of thoughts.

1) I think memory usage will always go up, because when the worker first starts, it's only pre-loading a small selection of modules. As more work gets done, I think more modules get loaded into memory, and they stay there for the life of the worker. But how much memory is used by loading modules... that's something I don't know. I have looked a bit at memory use based on modules being loaded and noticed big differences. 

2) I'm guessing maybe we have memory leaks. My first guess would DBIx::Class objects hanging around longer than they should. Some of that might be scoping and some of that might be caching in DBIx::Class. Mind you that's just a guess. This is something that could maybe be investigated and changed. 

But yeah... both just guesses. I'd say it would need application level profiling to really know. 

But considering how a multi-threaded Tomcat process can eat up GBs of memory, a multi-process hyponotoad eating up GBs of memory collectively doesn't really shock me either.

--

Oh one thought... if we pre-loaded more modules into the master hypnotoad process, in theory there would be less in the unshared memory, since they'd be using the shared memory for the modules. 

I once encountered a legacy mod_perl app which wasn't preloading enough modules, and it caused performance problems when new workers were forked because they had to reload lots of modules. I preloaded everything important into the master, and performance went way up. Memory wasn't really a big deal for the app as we had lots to spare, but I assume copy-on-write was used so that the preloaded modules were in shared memory. 

Just my 2 cents.
Comment 31 Jerome Charaoui 2018-11-08 16:00:05 UTC
Thanks David for your input, much appreciated.

I'm currently testing with Mojolicious::Plugin::SizeLimit. I'm hoping this will avoid frequest restarts while also keeping memory usage under control. I'll report back when I have some results.
Comment 32 Julian Maurice 2018-11-16 17:13:58 UTC
Comment on attachment 81857 [details] [review]
Fix PDF generation

Obsoleting this patch. It doesn't apply and the bug it tries to fix is in master (it is not related to this bug).

I opened bug 21853
Comment 33 Julian Maurice 2018-11-17 11:22:10 UTC
Created attachment 82423 [details] [review]
Bug 20582: Handle multipart content-type

Mojo::Message::Request's body attribute does not contain anything when
the content-type is multipart/*
We have to rebuild the multipart body for CGI scripts

Test plan:
1. Go to Tools » Upload and try to upload a file
Comment 34 Julian Maurice 2018-11-17 11:24:44 UTC
(In reply to David Cook from comment #24)
> Seems such a shame that Mojolicious doesn't just provide the actual original
> body of the message... 

Actually it does. I rewrote the last patch in a much simpler way.
Comment 35 David Cook 2018-11-19 06:03:29 UTC
(In reply to Julian Maurice from comment #34)
> (In reply to David Cook from comment #24)
> > Seems such a shame that Mojolicious doesn't just provide the actual original
> > body of the message... 
> 
> Actually it does. I rewrote the last patch in a much simpler way.

I think build_body reassembles the body from its component parts after taking apart the body, which seems efficient, but if it works, it works.
Comment 36 Josef Moravec 2019-01-30 14:45:58 UTC
Created attachment 84526 [details] [review]
Bug 20582: Turn Koha into a Mojolicious application

This patch is a proof-of-concept of Koha as a Mojolicious application

This has several benefits:
- Run `morbo bin/koha` and you're set : fully functional Koha without
  apache2/nginx (URL rewrites and static files are handled by the app)
- apache2/nginx configuration is much simpler (an example of nginx
  configuration is included in the patch)
- starman and plack middlewares can still be used for debug or gzip
  compression for instance (see bin/koha.psgi)
- All that is needed to run koha is in the source code and not hidden in
  an obscure directory (I'm looking at you, debian/templates/plack.psgi)
- Using Test::Mojo we can test the whole application, as we do with the
  REST API (which is a Mojolicious application too)
- It's another step in the direction of dropping CGI support
- It also opens a way for converting CGI scripts into Mojolicious
  controllers and actions (even if that's not possible at the moment
  because of the authentication code)
- It requires zero changes to existing files, so it should apply nicely
  forever (no rebase needed \o/)

A few downsides too:
- It is not complete : some things from debian/templates/plack.psgi are
  missing, like the L1 cache flush or Koha::Middleware::SetEnv. But that
  shouldn't be too hard to add.
- Already mentioned above, but the actual state of get_template_and_user
  and checkauth completely forbids the use of Mojolicious controllers.
  We really need to rewrite C4::Auth
- Not sure if it's a downside or not, but as OPAC and Intranet are on
  the same host:port, if you authenticate on OPAC you will be
  authenticated on Intranet too.
  This can probably be changed if it's really a problem.
- ... see FIXME in the code

Other remarks:
- It uses the same mechanism as Plack::App::CGIBin to deal with CGI
  scripts, so it should be equivalent in terms of performance

How to test ?
- Run `morbo bin/koha`, then go to:
  - http://localhost:3000/  -  Redirects to OPAC main page
  - Then navigate in the OPAC, searching for bugs
  - http://localhost:3000/intranet  -  Redirects to intranet main page
  - Then navigate in the Intranet, still searching for bugs
  - http://localhost:3000/api/v1/...  -  The REST API
  - Test the web installer
- Read the code (and the comments), it's not very long

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 37 Josef Moravec 2019-01-30 14:46:06 UTC
Created attachment 84527 [details] [review]
Bug 20582: Add routes for some svc/ scripts

svc/club/enroll, svc/club/cancel_enrollment and svc/report are available
in different versions: one for the OPAC, one for the intranet
This patch creates new routes that override the CGI scripts and that
work for both OPAC and intranet

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 38 Josef Moravec 2019-01-30 14:46:13 UTC
Created attachment 84528 [details] [review]
Bug 20582: Enable SetEnv middleware in bin/koha.psgi

and make sure the psgi env is passed to CGI scripts

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 39 Josef Moravec 2019-01-30 14:46:21 UTC
Created attachment 84529 [details] [review]
Bug 20582: Flush memory caches before every request

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 40 Josef Moravec 2019-01-30 14:46:32 UTC
Created attachment 84530 [details] [review]
Bug 20582: Fix memory cache flush

Cache flush should be in around_action hook instead of before_dispatch
Otherwise it is flushed before every request (even static files
requests)
Also, make sure the Cache::Memory cache is really flushed (not only L1
cache)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 41 Josef Moravec 2019-01-30 14:46:46 UTC
Created attachment 84531 [details] [review]
Bug 20582: Handle multipart content-type

Mojo::Message::Request's body attribute does not contain anything when
the content-type is multipart/*
We have to rebuild the multipart body for CGI scripts

Test plan:
1. Go to Tools » Upload and try to upload a file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 42 Josef Moravec 2019-01-30 14:56:48 UTC
Signed off as it works as advertised and it would be great to see it moving ;)

Just one note: we need to add Mojolicious::Plugin::TemplateToolkit to dependencies and make a debian package (adding Mirko to CC)