Bug 21101

Created attachment 77180 [details] [review]
Bug 21101: Rename engine var in C4::XSLT

'engine' is both a method, used outisde module, and a module shared variable.

I rename the variable 'xslth' to avoid confusion and use 'engine()' method inside the module.

Created attachment 77181 [details] [review]
Bug 21101: Allow to disable XSLT cache

Koha::XSLT_Handler has a cache.
When Plack is used, the XSLT display have a cache that can last several minutes.
This is good for performance but bad for XSLT files edition.

I propose a syspref to enable/disable this cache, enabled by default.

Test plan :
1) Enable Plack
2) Check syspref XSLTCache is enabled
3) Define an XSLT file path in some XSLT display, ie OPACXSLTResultsDisplay
4) Perform a search and look at results
5) Edit XSLT file to add some text, for example next to title
6) Perform a search and look at results => The change is not there, you can try several refresh
7) Undo action 5)
2) Set syspref XSLTCache disabled
8) Perform a search and look at results => you see original display
9) Repeat action 5)
10) Perform a search and look at results => you see the change, you can try several refreshs

Do we really need a pref here? Why? Are these xslt files edited that regularly? If you edit a script, you need to restart Plack. If you edit a XSLT file, just do the same?

(In reply to Marcel de Rooy from comment #3)
> Do we really need a pref here?

This is the justification I thought of after asking myself the same question: If the XSLT preferences are there to empower the librarian to make their own changes to XSLT, they need a way to test without having server access. Not a very common occurrence, but valid.

Most of our customers have access to XSLT editing (through a webdave space), but they don't have any SSH access.
=> restarting plack & updating XSLT don't require the same kind of permission. With this dev, a librarian can :
 * turn the pref ON
 * play with XSLT and test
 * once it's OK, turn pref OFF and get caching & performance back.

The library is happy and our support team is happy too, one less thing to do :D

(In reply to Paul Poulain from comment #5)
> Most of our customers have access to XSLT editing (through a webdave space),
> but they don't have any SSH access.
> => restarting plack & updating XSLT don't require the same kind of
> permission. With this dev, a librarian can :
>  * turn the pref ON
>  * play with XSLT and test
>  * once it's OK, turn pref OFF and get caching & performance back.
> 
> The library is happy and our support team is happy too, one less thing to do
> :D

We need a couple 'flush' buttons in about.pl, what do you think?

- flush memcached
- flush template-toolkit cache
- flush XSLT cache

(In reply to Tomás Cohen Arazi from comment #6)
> We need a couple 'flush' buttons in about.pl, what do you think?
> 
> - flush memcached
> - flush template-toolkit cache
> - flush XSLT cache

Hmm. This might be better than toggling preferences.
Where: About or Administration?
The need to flush the XSLT cache separately might be very limited (although supported with $xslt->refresh). If we gracefully(!) restart Plack, we achieve the same and more?

Marcel: in the about page :-D

(In reply to Marcel de Rooy from comment #7)
> (In reply to Tomás Cohen Arazi from comment #6)
> > We need a couple 'flush' buttons in about.pl, what do you think?
> > 
> > - flush memcached
> > - flush template-toolkit cache
> > - flush XSLT cache
> 
> Hmm. This might be better than toggling preferences.
> Where: About or Administration?
> The need to flush the XSLT cache separately might be very limited (although
> supported with $xslt->refresh). If we gracefully(!) restart Plack, we
> achieve the same and more?

We need at least one for memcached and one for plack.
Note that starman has a "reload" option that can be implemented by sending to the starman master pid : /bin/kill -HUP <pid>

But I'm still in favor of a syspref because editing XSLT must be done in a lot of steps meaning :
 - Modify something
 - Save
 - Refreshing the HTML page

Avoid a step with cache flush is I think the best.