In Bug 36736, a call to get_enabled_plugins was added to background jobs to ensure plugins wouldn't break the database transaction. We need a fix for this problem in general (Database transactions breaking due to plugin hooks being called), not just for background jobs.
We had discussed that at the hackfest and it has caused great trouble for some plugin users. We should also consider adding a warning to documentation, even the advent calendar.
Created attachment 174150 [details] [review] Bug 38384: Trigger plugin modules loading as soon as possible in Koha
I think this may solve the issue. I don't have a specific test plan to move this forward though.
Is this ready for testing, Kyle? And thanks!
The problem is that this fix creates a circular dependency that just breaks. We actually need a way to move the logic within 'get_enabled_plugins' into the BEGIN block of C4::Context w/o depending on Koha::Plugins at all.
To test (preliminary): 1) Get the koha-plugin-fancyplugin from KohaAdvent. 2) Install it 3) Use test_transaction.pl and observe that due to the reinitialization, the transaction breaks and the biblionumber is returned(!) but after that we can't find the corresponding biblio. Then: 4) Find a way to implement comment #5 5) Apply your patch 6) See that the biblio is found by Koha::Biblios->find
Created attachment 174264 [details] koha-plugin-fancyplugin from KohaAdvent
Created attachment 174265 [details] Test case (script) to reproduce the breaking database transaction
Using Koha::Cache::Memory::Lite in C4::Context's BEGIN should be find, though.
Created attachment 174280 [details] [review] Bug 38384: Inline plugin loading into C4::Context's BEGIN block
Not too sure on that latest patch. I essentially only translated the call to Koha::Plugins->get_enabled_plugins to an inlined version without dependencies on other local modules. This is however very ugly, I am just curious what others think. Thoughts?
Created attachment 174282 [details] [review] Bug 38384: Prevent initialization failure by checking for existence of plugin_data first
I don't think we should be using XML::Simple here, even for a simple use case. Worth exploring other fast alternatives though... Btw, you should be able to use Koha::Database instead of DBI directly there, since it's already lightweight (and uses the same modules, so it's same-same but with less duplicated code).
(In reply to David Cook from comment #13) > Btw, you should be able to use Koha::Database instead of DBI directly there, > since it's already lightweight (and uses the same modules, so it's same-same > but with less duplicated code). For instance, look at bug 37657 where C4::Context->dbh is replaced by Koha::Database->dbh for big speed gain.
We have "TableExists" in C4::Installer - it doesn't need a different syntax per engine, could we do the same here? Select all form the table and check if any warning
(In reply to Nick Clemens (kidclamp) from comment #15) > We have "TableExists" in C4::Installer - it doesn't need a different syntax > per engine, could we do the same here? Select all form the table and check > if any warning Did you comment on the right bug here, Nick?
@David I think Nick means the "TableExists" here: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174282&action=diff&headers=0#a/C4/Context.pm_sec3