Bug 20582 - Turn Koha into a Mojolicious application
Summary: Turn Koha into a Mojolicious application
Status: Passed QA
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)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords: rel_20_11_target, RM_priority
Depends on:
Blocks: 20630 20635
  Show dependency treegraph
 
Reported: 2018-04-13 16:31 UTC by Julian Maurice
Modified: 2020-09-14 23:21 UTC (History)
18 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


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
Bug 20582: Remove usage of Mojo plugin TemplateToolkit (1.35 KB, patch)
2019-06-26 06:07 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Turn Koha into a Mojolicious application (16.50 KB, patch)
2020-02-19 17:51 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Suppress warning in CGIBinKoha.pm (963 bytes, patch)
2020-04-24 08:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Turn Koha into a Mojolicious application (16.56 KB, patch)
2020-04-26 03:35 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 20582: Suppress warning in CGIBinKoha.pm (1021 bytes, patch)
2020-04-26 03:35 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 20582: Fix PSGI file when behind a reverse proxy (1.32 KB, patch)
2020-08-28 13:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Add very simple configuration file for Apache (1019 bytes, patch)
2020-08-28 13:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Fix a cache issue in Koha::App::{Opac,Intranet} (1.98 KB, patch)
2020-08-28 13:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20582: Turn Koha into a Mojolicious application (16.69 KB, patch)
2020-09-11 10:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20582: Suppress warning in CGIBinKoha.pm (1.12 KB, patch)
2020-09-11 10:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20582: Fix PSGI file when behind a reverse proxy (1.44 KB, patch)
2020-09-11 10:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20582: Add very simple configuration file for Apache (1.06 KB, patch)
2020-09-11 10:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20582: Fix a cache issue in Koha::App::{Opac,Intranet} (2.05 KB, patch)
2020-09-11 10:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20582: (QA follow-up) Add POD to satisfy coding guidelines (2.74 KB, patch)
2020-09-11 10:50 UTC, Martin Renvoize
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)
Comment 43 Katrin Fischer 2019-04-25 12:11:48 UTC
I am moving this to "Milestone Future" and "In Discussion" - to be put back into the QA queue for 19.11 (see discussion during yesterday's Dev meeting)
Comment 44 Martin Renvoize 2019-06-25 22:29:32 UTC
We're in the 19.11 cycle now and nearing my 'adventurous pushing' window where I hope to move things like this along.  It would be great to see a bit more testing and get some collective QA eyes on it over the next couple of weeks with an aim to having a clear way forward for pushing it and end users (well, system administrators) tentatively enabling this deployment method.
Comment 45 David Cook 2019-06-26 05:23:03 UTC
I'm time poor at the moment but interested in this report and especially #20630.

Will keep it on my "to test" list and try to get to it.
Comment 46 Julian Maurice 2019-06-26 05:59:41 UTC
(In reply to Josef Moravec from comment #42)
> 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)

It is not really needed. I added this module as a 'will be useful in the future' stuff, but it can be removed.

(In reply to Martin Renvoize from comment #44)
> We're in the 19.11 cycle now and nearing my 'adventurous pushing' window
> where I hope to move things like this along.  It would be great to see a bit
> more testing and get some collective QA eyes on it over the next couple of
> weeks with an aim to having a clear way forward for pushing it and end users
> (well, system administrators) tentatively enabling this deployment method.

I'm glad you have an "adventurous pushing" window :)
I'll try to be as reactive as possible to answer questions and to fix bugs.
Comment 47 Julian Maurice 2019-06-26 06:07:45 UTC
Created attachment 91009 [details] [review]
Bug 20582: Remove usage of Mojo plugin TemplateToolkit

It is not used anywhere yet (it will be useful if we start rewriting CGI
scripts as Mojo controllers)
Comment 48 Joonas Kylmälä 2020-02-19 12:32:55 UTC
These patches introduce new files without the GPL license header which is mandatory as per our coding guidelines: <https://wiki.koha-community.org/wiki/Coding_Guidelines>. The patches should be re-done so that there is the license header. Setting this as 'Failed QA' until this is resolved.
Comment 49 Julian Maurice 2020-02-19 17:51:36 UTC
Created attachment 99267 [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:
- Development setup is easier. No need for apache or nginx. Just run
  `morbo bin/intranet` or `morbo bin/opac` and go to
  http://localhost:3000 (URL rewrites and static files are handled by
  the app)
- apache2/nginx configuration is simpler too (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 app.psgi)
- Using Test::Mojo we can test the whole application, as we do with the
  REST API (which is a Mojolicious application too)
- It 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 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/intranet`, then go to http://localhost:3000/ and try to
  find bugs. Check the REST API at http://localhost:3000/api/v1
- Run `morbo bin/opac`, then go to http://localhost:3000/ and try to
  find bugs. Check the REST API at http://localhost:3000/api/v1
- Run `starman -l :5000 -l :5001` and verify that intranet
  (http://localhost:5000) and opac (http://localhost:5001) work normally
- Read the code (and the comments), it's not very long
Comment 50 Julian Maurice 2020-02-19 17:59:32 UTC
I squashed all patches, added the missing license headers, and made small changes to emphasize the fact that opac and intranet are two different apps. This makes the patch smaller and removes one of the FIXMEs.
I also had to fix OPAC login when running app.psgi with starman (see comment in Koha::App::Plugin::CGIBinKoha::_psgi_env)

Back to needs signoff.
Comment 51 Victor Grousset/tuxayo 2020-04-20 23:44:53 UTC
Does anyone manages to import a staged record?

The progress bar stays to 0%

Even with plack stopped (in case it blocks something), the behviour is the same.


## Logs (plack stopped)

### confirm staging

[Mon Apr 20 23:39:54 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl"
[Mon Apr 20 23:39:54 2020] [debug] Routing to a callback
Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234.
Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
Use of uninitialized value in concatenation (.) or string at /usr/share/perl5/MARC/File/XML.pm line 397, <__ANONIO__> chunk 2.
[Mon Apr 20 23:39:54 2020] [debug] 200 OK (0.098054s, 10.198/s)
seek() on closed filehandle _GEN_4 at /usr/share/perl5/CGI/Emulate/PSGI.pm line 34.
[Mon Apr 20 23:39:54 2020] [error] Can't seek stdout handle: Bad file descriptor at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 56.

[Mon Apr 20 23:39:54 2020] [debug] Template "exception.development.html.ep" not found
[Mon Apr 20 23:39:54 2020] [debug] Template "exception.html.ep" not found
[Mon Apr 20 23:39:54 2020] [debug] Rendering template "mojo/debug.html.ep"
[Mon Apr 20 23:39:54 2020] [debug] Rendering template "mojo/menubar.html.ep"
[Mon Apr 20 23:39:54 2020] [debug] Your secret passphrase needs to be changed
[Mon Apr 20 23:39:54 2020] [debug] 500 Internal Server Error (0.258607s, 3.867/s)
[Mon Apr 20 23:39:55 2020] [debug] GET "/cgi-bin/koha/tools/background-job-progress.pl"
[Mon Apr 20 23:39:55 2020] [debug] Routing to a callback
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Mon Apr 20 23:39:55 2020] [debug] 200 OK (0.028605s, 34.959/s)
[Mon Apr 20 23:39:55 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl"
[Mon Apr 20 23:39:55 2020] [debug] Routing to a callback
Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234.
Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Mon Apr 20 23:39:55 2020] [debug] 200 OK (0.099609s, 10.039/s)

### confirm import

[Mon Apr 20 23:40:53 2020] [debug] POST "/cgi-bin/koha/tools/manage-marc-import.pl"
[Mon Apr 20 23:40:53 2020] [debug] Routing to a callback
Subroutine redo_matching redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 140.
Subroutine create_labelbatch_from_importbatch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 184.
Subroutine import_batches_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 204.
Subroutine commit_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 232.
Subroutine revert_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 271.
Subroutine put_in_background redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 309.
Subroutine progress_callback redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 344.
Subroutine add_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 354.
Subroutine add_saved_job_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 360.
Subroutine import_records_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 368.
Subroutine batch_info redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 390.
Subroutine add_matcher_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 422.
Subroutine add_page_numbers redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 435.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1588.
[Mon Apr 20 23:40:53 2020] [debug] 200 OK (0.094207s, 10.615/s)
Comment 52 Victor Grousset/tuxayo 2020-04-24 00:18:24 UTC
I retried with the patches applied before the container was started (in case that made a difference)

I don't think it's related, but the end changed a bit and now I get:
> rollback ineffective with AutoCommit enabled

Can someone else confirm? Maybe my setup is wrong or I missed an implict step.

### confirm staging

[Fri Apr 24 00:03:10 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl"
[Fri Apr 24 00:03:10 2020] [debug] Routing to a callback
Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234.
Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
Use of uninitialized value in concatenation (.) or string at /usr/share/perl5/MARC/File/XML.pm line 397, <__ANONIO__> chunk 2.
[Fri Apr 24 00:03:10 2020] [debug] 200 OK (0.105929s, 9.440/s)
seek() on closed filehandle _GEN_5 at /usr/share/perl5/CGI/Emulate/PSGI.pm line 34.
[Fri Apr 24 00:03:10 2020] [error] Can't seek stdout handle: Bad file descriptor at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 56.

[Fri Apr 24 00:03:10 2020] [debug] Template "exception.development.html.ep" not found
[Fri Apr 24 00:03:10 2020] [debug] Template "exception.html.ep" not found
[Fri Apr 24 00:03:10 2020] [debug] Rendering template "mojo/debug.html.ep"
[Fri Apr 24 00:03:10 2020] [debug] Rendering template "mojo/menubar.html.ep"
[Fri Apr 24 00:03:10 2020] [debug] Your secret passphrase needs to be changed
[Fri Apr 24 00:03:10 2020] [debug] 500 Internal Server Error (0.269868s, 3.706/s)
[Fri Apr 24 00:03:11 2020] [debug] GET "/cgi-bin/koha/tools/background-job-progress.pl"
[Fri Apr 24 00:03:11 2020] [debug] Routing to a callback
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:11 2020] [debug] 200 OK (0.029011s, 34.470/s)
[Fri Apr 24 00:03:11 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl"
[Fri Apr 24 00:03:11 2020] [debug] Routing to a callback
Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234.
Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:11 2020] [debug] 200 OK (0.085216s, 11.735/s)


### manage-marc-import

[Fri Apr 24 00:03:20 2020] [debug] GET "/cgi-bin/koha/tools/manage-marc-import.pl"
[Fri Apr 24 00:03:20 2020] [debug] Routing to a callback
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:21 2020] [debug] 200 OK (0.219592s, 4.554/s)
[Fri Apr 24 00:03:22 2020] [debug] POST "/cgi-bin/koha/tools/batch_records_ajax.pl"
[Fri Apr 24 00:03:22 2020] [debug] Routing to a callback
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:22 2020] [debug] 200 OK (0.03023s, 33.080/s)


### confirm import

[Fri Apr 24 00:03:23 2020] [debug] POST "/cgi-bin/koha/tools/manage-marc-import.pl"
[Fri Apr 24 00:03:23 2020] [debug] Routing to a callback
Subroutine redo_matching redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 140.
Subroutine create_labelbatch_from_importbatch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 184.
Subroutine import_batches_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 204.
Subroutine commit_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 232.
Subroutine revert_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 271.
Subroutine put_in_background redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 309.
Subroutine progress_callback redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 344.
Subroutine add_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 354.
Subroutine add_saved_job_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 360.
Subroutine import_records_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 368.
Subroutine batch_info redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 390.
Subroutine add_matcher_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 422.
Subroutine add_page_numbers redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 435.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
rollback ineffective with AutoCommit enabled at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1651.
[Fri Apr 24 00:03:24 2020] [debug] 200 OK (0.116588s, 8.577/s)
Comment 53 Julian Maurice 2020-04-24 08:22:01 UTC
I think that's the same problem than with starman: background jobs don't work in this environment.
See bug 17240 and bug 15218
Comment 54 Julian Maurice 2020-04-24 08:26:40 UTC
Created attachment 103633 [details] [review]
Bug 20582: Suppress warning in CGIBinKoha.pm
Comment 55 Victor Grousset/tuxayo 2020-04-24 14:07:31 UTC
> I think that's the same problem than with starman: background jobs don't work in this environment.

What do you mean by "in this environment" ?

Note: I can import on the same setup when using Koha via the apache2&CGI way.
Comment 56 Julian Maurice 2020-04-24 14:16:16 UTC
(In reply to Victor Grousset/tuxayo from comment #55)
> > I think that's the same problem than with starman: background jobs don't work in this environment.
> 
> What do you mean by "in this environment" ?

I mean starman/morbo/hypnotoad/...
Background jobs work only in CGI mode.
Comment 57 Victor Grousset/tuxayo 2020-04-24 15:36:56 UTC
So it's not an issue for the current patch?

IIUC: That would be an issue only is morbo/Mojolicious was the only way the new patch allows.
Comment 58 Victor Grousset/tuxayo 2020-04-24 15:39:25 UTC
Does anyone has an idea bout what specific stuff to test for this patch?

@Julian: since it doesn't replace the old mode, we don't need to test all Koha right? ^^"

So no worries about this moving forward a week before the feature freeze?
Comment 59 Julian Maurice 2020-04-24 16:13:32 UTC
(In reply to Victor Grousset/tuxayo from comment #58)
> Does anyone has an idea bout what specific stuff to test for this patch?
> 
> @Julian: since it doesn't replace the old mode, we don't need to test all
> Koha right? ^^"
> 
> So no worries about this moving forward a week before the feature freeze?

In my opinion it could be pushed as an experimental feature. It is completely optional and doesn't break anything when using Plack or CGI mode.
But I'm sure the QA team has other priorities at this point of the release cycle, so don't expect it in 20.05 ;)
Comment 60 Victor Grousset/tuxayo 2020-04-26 03:35:33 UTC
Created attachment 103720 [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:
- Development setup is easier. No need for apache or nginx. Just run
  `morbo bin/intranet` or `morbo bin/opac` and go to
  http://localhost:3000 (URL rewrites and static files are handled by
  the app)
- apache2/nginx configuration is simpler too (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 app.psgi)
- Using Test::Mojo we can test the whole application, as we do with the
  REST API (which is a Mojolicious application too)
- It 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 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/intranet`, then go to http://localhost:3000/ and try to
  find bugs. Check the REST API at http://localhost:3000/api/v1
- Run `morbo bin/opac`, then go to http://localhost:3000/ and try to
  find bugs. Check the REST API at http://localhost:3000/api/v1
- Run `starman -l :5000 -l :5001` and verify that intranet
  (http://localhost:5000) and opac (http://localhost:5001) work normally
- Read the code (and the comments), it's not very long

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 61 Victor Grousset/tuxayo 2020-04-26 03:35:38 UTC
Created attachment 103721 [details] [review]
Bug 20582: Suppress warning in CGIBinKoha.pm

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 62 Victor Grousset/tuxayo 2020-04-26 03:36:12 UTC
- did some browsing 
- a check out
- a check in
- created a patron
- deleted a patron
- searched a patron
- changed a syspref
- searched the catalog
- deleted an item
- deleted a record
- created a record from Z39.50
- created an item
- created a record from SRU
- OPAC
  - logged in
  - searched a record
  - placed a hold
  - canceled it
  - added items to a public list
- classic ways of accessing the intranet and OPAC with koha-testing-docker still work
- went to /api/v1 when running `morbo bin/opac` and `morbo bin/intranet`
- ran out of relevant ideas
Comment 63 David Cook 2020-06-26 02:01:03 UTC
It would be great to see this move forward now that we're at the start of a new release cycle.
Comment 64 David Cook 2020-06-26 03:40:44 UTC
I've been thinking that a standalone Mojolicious-based OPAC would be great, but that would involve a huge amount of work (even if there are < 100 OPAC .pl scripts), so maybe it's more feasible to slowly move from CGI scripts to Mojolicious routes here...
Comment 65 Jonathan Druart 2020-08-07 07:44:39 UTC
Julian, what's missing to have this ready for push?
Can you provide a config for apache?

If I understand correctly, we are not modifying any existing Koha core files (there are very few changes on bug 20630 that should be safe) and so we could have this pushed to be tested by developers, when production sites would continue to use the standard plack architecture. It that correct?
Comment 66 Julian Maurice 2020-08-07 08:32:29 UTC
(In reply to Jonathan Druart from comment #65)
> Julian, what's missing to have this ready for push?
> Can you provide a config for apache?
> 
> If I understand correctly, we are not modifying any existing Koha core files
> (there are very few changes on bug 20630 that should be safe) and so we
> could have this pushed to be tested by developers, when production sites
> would continue to use the standard plack architecture. It that correct?

This patch can be pushed safely. It will not change anything for existing and new installations.
I cannot provide an apache config right now (I'm on vacation), but it should not be difficult to write one that just does ProxyPass and ProxyPassReverse, like in the provided nginx config file. I will try to write one next week.
Comment 67 Julian Maurice 2020-08-28 13:43:16 UTC
Created attachment 109282 [details] [review]
Bug 20582: Fix PSGI file when behind a reverse proxy

ReverseProxy middleware can modify SERVER_PORT which is used to
determines which app (opac or intranet) to run, so the choice should be
made before ReverseProxy middleware
Comment 68 Julian Maurice 2020-08-28 13:43:21 UTC
Created attachment 109283 [details] [review]
Bug 20582: Add very simple configuration file for Apache
Comment 69 Julian Maurice 2020-08-28 13:43:26 UTC
Created attachment 109284 [details] [review]
Bug 20582: Fix a cache issue in Koha::App::{Opac,Intranet}

This was a workaround I wrote when Koha::Caches->flush_L1_caches was not
flushing all caches correctly. Now it's not needed and it causes
problems, so it must be removed.
Comment 70 Martin Renvoize 2020-09-11 10:49:28 UTC
Created attachment 109895 [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:
- Development setup is easier. No need for apache or nginx. Just run
  `morbo bin/intranet` or `morbo bin/opac` and go to
  http://localhost:3000 (URL rewrites and static files are handled by
  the app)
- apache2/nginx configuration is simpler too (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 app.psgi)
- Using Test::Mojo we can test the whole application, as we do with the
  REST API (which is a Mojolicious application too)
- It 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 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/intranet`, then go to http://localhost:3000/ and try to
  find bugs. Check the REST API at http://localhost:3000/api/v1
- Run `morbo bin/opac`, then go to http://localhost:3000/ and try to
  find bugs. Check the REST API at http://localhost:3000/api/v1
- Run `starman -l :5000 -l :5001` and verify that intranet
  (http://localhost:5000) and opac (http://localhost:5001) work normally
- Read the code (and the comments), it's not very long

Signed-off-by: Jerome Charaoui <jcharaoui@cmaisonneuve.qc.ca>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 71 Martin Renvoize 2020-09-11 10:49:39 UTC
Created attachment 109896 [details] [review]
Bug 20582: Suppress warning in CGIBinKoha.pm

Signed-off-by: Jerome Charaoui <jcharaoui@cmaisonneuve.qc.ca>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 72 Martin Renvoize 2020-09-11 10:49:48 UTC
Created attachment 109897 [details] [review]
Bug 20582: Fix PSGI file when behind a reverse proxy

ReverseProxy middleware can modify SERVER_PORT which is used to
determines which app (opac or intranet) to run, so the choice should be
made before ReverseProxy middleware

Signed-off-by: Jerome Charaoui <jcharaoui@cmaisonneuve.qc.ca>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 73 Martin Renvoize 2020-09-11 10:49:55 UTC
Created attachment 109898 [details] [review]
Bug 20582: Add very simple configuration file for Apache

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 74 Martin Renvoize 2020-09-11 10:50:01 UTC
Created attachment 109899 [details] [review]
Bug 20582: Fix a cache issue in Koha::App::{Opac,Intranet}

This was a workaround I wrote when Koha::Caches->flush_L1_caches was not
flushing all caches correctly. Now it's not needed and it causes
problems, so it must be removed.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 75 Martin Renvoize 2020-09-11 10:50:10 UTC
Created attachment 109900 [details] [review]
Bug 20582: (QA follow-up) Add POD to satisfy coding guidelines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 76 Martin Renvoize 2020-09-11 10:55:57 UTC
This works well and is very self contained so I'm passing QA on it.

It's designed for developers only at this point, for further testing and I would suggest we encourage devs to start running with this environment as a stepping stone to developing with Mojolicious in mind.

Next steps:
1) We need to think about authentication; I would suggest writing a fresh Auth mechanism designed with Mojolicious best practices in mind which can be used initially with and intercept of the existing Koha cookies.
2) Once we have the above, we can start developing new controllers as we need them in a Mojo focused way and setup the router to pass such routes directly to the Mojo based controller.
3) Pick some simple controllers to migrate

Question
1) Does anyone still run under CGI only.. i.e. not via plack?  If we wish to continue supporting this, we will need to allow setting the apache/niginx configs to run the Mojo app in CGI emulation (which is known to be slow.. but then CGI in Koha is now known to be slow).  I eventually see hypnotoad as replacing both Apache/CGI and Plack running environments, perhaps with Nginx/Apache for static resources/Proxying.
Comment 77 Kyle M Hall 2020-09-11 12:10:11 UTC
> Question
> 1) Does anyone still run under CGI only.. i.e. not via plack?  If we wish to
> continue supporting this, we will need to allow setting the apache/niginx
> configs to run the Mojo app in CGI emulation (which is known to be slow..
> but then CGI in Koha is now known to be slow).  I eventually see hypnotoad
> as replacing both Apache/CGI and Plack running environments, perhaps with
> Nginx/Apache for static resources/Proxying.

I'm really looking forward to what we can do with this!

We have just a couple partners in CGI only mode for Shib, but that's all.
Comment 78 Katrin Fischer 2020-09-12 10:03:36 UTC
> I'm really looking forward to what we can do with this!
> 
> We have just a couple partners in CGI only mode for Shib, but that's all.

We had one for the same reason, but with 18.11 we are able to use Shib with Plack. It appears to just work and was such a performance boost.
Comment 79 David Cook 2020-09-13 23:32:36 UTC
Totally agree with you, Martin!

We still have a number of instances that are CGI-only, but they're all being migrated to Plack over time. Nothing new on CGI.

The community still needs to get rid of CGI dependence (even when running Plack) for some scripts, but hopefully my discovery on bug 26400 might help a little bit with that. Plus the message queue in bug 22417 for background tasks.
Comment 80 Martin Renvoize 2020-09-14 10:17:44 UTC
I think if/when we can get the Mojo side authentication routines nailed we can likely more easily add in native shibboleth handling at the same time which would save us the "nasty" headers work around too :)
Comment 81 David Cook 2020-09-14 23:21:32 UTC
(In reply to Martin Renvoize from comment #80)
> I think if/when we can get the Mojo side authentication routines nailed we
> can likely more easily add in native shibboleth handling at the same time
> which would save us the "nasty" headers work around too :)

I'm not familiar with the Shibboleth work, but I'm already sold on the idea.