CORE-POS / IS4C

Cooperative Operational Retail Environment
http://www.core-pos.com
GNU General Public License v2.0
63 stars 44 forks source link

Not finding classes during Available Plugins list creation #1095

Open flathat opened 2 years ago

flathat commented 2 years ago

I'm having intermittent trouble with the creation of the Available Plugins list in InstallPluginsPage.php. In one of two sites (a dev and a production) that are pretty close to identical, on the dev site InstallPluginsPage produces these errors and on the production it doesn't. When errors are reported the Available Plugins list fails. $LIB/PrintHandler/PrintHandler.php exists.

/* These are in OrderTablePage.php but sometimes fail at least in the context of
creating the Available Plugins list */
if (!class_exists('COREPOS\\pos\\lib\\PrintHandlers\\ESCPOSPrintHandler')) {
    include(__DIR__ . '/../../../../pos/is4c-nf/lib/PrintHandlers/ESCPOSPrintHandler.php');
}
if (!class_exists('COREPOS\\pos\\lib\\PrintHandlers\\ESCNetRawHandler')) {
    include(__DIR__ . '/../../../../pos/is4c-nf/lib/PrintHandlers/ESCNetRawHandler.php');
}

These errors result from OrderTabletPage.

Sep 23 02:51:25 us02.server.plus fannie[48360]: (warning) Class 'COREPOS\pos\lib\PrintHandlers\PrintHandler' not found Line 17, /home/muyujiev/public_html/lvc/IS4C/pos/is4c-nf/lib/PrintHandlers/ESCPOSPrintHandler.php
Sep 23 02:51:25 us02.server.plus fannie[48360]: (error) Class 'COREPOS\pos\lib\PrintHandlers\PrintHandler' not found Line 17, File /home/muyujiev/public_html/lvc/IS4C/pos/is4c-nf/lib/PrintHandlers/ESCPOSPrintHandler.php

Adding this to OrderTabletPage before the other *PrintHandler include()s prevents the errors:

if (!class_exists('COREPOS\\pos\\lib\\PrintHandlers\\PrintHandler')) {
    include(__DIR__ . '/../../../../pos/is4c-nf/lib/PrintHandlers/PrintHandler.php');
}

On a similar if perhaps not actually related issue, on the production site an initial run of InstallPluginsPage usually produces these errors (and the list fails) but after running InstallStoresPage another run of InstallPluginsPage usually succeeds and doesn't report errors. This has happened a number of times on the production site especially in the context of installing a new plugin or restoring one that was set aside to get around this problem but I haven't seen it yet on the dev site.

[2021-09-22 22:44:24] fannie.WARNING: Uncaught exception: (Error) Class 'FannieTaskArgs' not found Line 31, /var/www/leverett/fannie/modules/plugins2.0/CoopCred/tasks/CoopCredOneLaneSyncTask.php [] []
[2021-09-22 22:58:38] fannie.WARNING: Uncaught exception: (Error) Class 'CoreWarehouseModel' not found Line 32, /var/www/leverett/fannie/modules/plugins2.0/CoreWarehouse/models/MemberSummaryModel.php []  [] 

I'm looking for a strategy, or something to do after a failure, or ideally before failure, to get classes to load and load in the required order.

gohanman commented 2 years ago

I'm pretty sure you're looking at two different issues here. With OrderTabletPage, if composer is being used it does work because while FannieAPI doesn't know how to find COREPOS\pos\lib\PrintHandlers\PrintHandler, composer's autoloader does. But your correction with the extra include is correct because composer should still be optional.

The second issue isn't completely unrelated, but it's different enough (and complicated enough) that I'm going to make it a separate post.

gohanman commented 2 years ago

Now, about FannieTaskArgs etc. The reason that it works at all is possibly a bit of a bug. This is going to cover the details of class loading in some depth.

Here's how FannieAPI locates a class, basically:

  1. If it has a namespace, it can just map the namespace directly to a path and check if the proper file is there.
  2. If it's not namespaced, it looks through classlib2.0 for a matching filename.
  3. If the class still hasn't been found, it looks through plugins2.0. In this case if it finds a matching file, it checks whether that plugin is enabled and only loads the class if it is enabled.

That "basically" is where things go awry a bit. Because walking the filesystem looking for things is mildly expensive in terms of execution time, FannieAPI caches classes that it finds. If it's asked to load a class that it already knows about, it just includes the file.

The upshot is if you ask for a class that's part of a plugin and that plugin is inactive, it won't load. But if you ask for that same class again, it'll work because in the process of handling the first ask the class wound up in the cache of known classes. But more importantly to the issue, anything else that was examined in the course of finding that first requested class is also now in the cache and load-able. Why is it intermittent? The file operations aren't idempotent. When the OS is asked for a directory's contents, the entries can be returned in any order. That's going to add variability to both when class lookups occur and which parts of plugins2.0 wind up getting cached by the lookups.

This is arguably wrong behavior. FannieAPI really isn't enforcing the plugin distinction in a proper way. I'm not 100% sure what the best way to handle it is.

The way it's handled on the lane is the whole tree gets walked on startup and then stuck into $_SESSION. I'm not sure that's a great solution. I don't think there are still issues with side-by-side Office installs and $_SESSION, but that specific kind of bug can be really annoying if it crops up and the page you're looking at is unexpectedly loading files from an entirely different copy of Office, and side-by-sides do happen more often on the backend. It's also require plugin files to do some manual includes if the startup mapping is honoring the enabled/disabled plugin distinction and only making enabled classes available.

The caching in FannieAPI could track the plugin status within the cache so the directory structure is still explored as-needed, but it'd have the same drawback where plugin code has to do some extra manual includes.

Possibly a good option would be to just not use FannieAPI::listModules() to enumerate plugins. The filename of the plugin class already needs to match the name of the plugin directory. There's no real need to check the other files and include them to discover what kind of classes they contain. This wouldn't address the correctness of the current behavior at all but would address the immediate issue.