Encapsulated plugins: "extra_functions" approach?

[nickcollie]

See discussion at: https://www.zen-cart.com/showthread.php?228235-Encapsulated-Plugins&p=1382654

I thought that I could simply reproduce the admin structure in the plugin package. So, I could put extra_functions in /zc_plugins/package/version1/admin/functions/extra_functions and they would get loaded. This does not happen.

Here's the relevant code from application_bootstrap.php

This does not work for functions

$fs = FileSystem::getInstance();
$fs->setInstalledPlugins($installedPlugins);
$fs->loadFilesFromPluginsDirectory($installedPlugins, 'admin/includes/extra_configures', '~^[^\._].*\.php$~i');
$fs->loadFilesFromPluginsDirectory($installedPlugins, 'admin/includes/extra_datafiles', '~^[^\._].*\.php$~i');

If I do this then it works for functions

$fs = FileSystem::getInstance();
$fs->setInstalledPlugins($installedPlugins);
$fs->loadFilesFromPluginsDirectory($installedPlugins, 'admin/includes/extra_configures', '~^[^\._].*\.php$~i');
$fs->loadFilesFromPluginsDirectory($installedPlugins, 'admin/includes/extra_datafiles', '~^[^\._].*\.php$~i');
$fs->loadFilesFromPluginsDirectory($installedPlugins, 'admin/includes/functions/extra_functions', '~^[^\._].*\.php$~i');

Version I think this applies to all versions using encapsulated plugins

To Reproduce Create a function in an appropriate plugin extra_functions directory. It is not loaded

Expected behavior The file is loaded.

Additional context There seems to be a bit of discussion as to whether this is desired behaviour or not. I strongly believe it is desired. We can use plugin specific functions before encapsulated pplugins. It would be strange if we could not after that (very nice) improvement. But who knows?

[drbyte]

Seeking an opinion: The core ZC dir structure uses: /admin/includes/functions/extra_functions/ But there's no need for the extra subdir in a plugin, so should zc_plugins stuff expect to still use ... functions/extra_functions? or just be /admin/includes/extra_functions/ Or even /admin/includes/functions ?

I'd prefer just "functions", not "functions/extra_functions" or "extra_functions". But I know people get patterns in their head and quickly get confused if the pattern changes.

So, seeking feedback to help inform the decision...

[torvista]

Legacy plugins all seem to use /extra_functions, but I assume that is/was just an easy way to get them auto-included on all pages? How should function files be included? In the docs there is no mention of the admin /functions directory: https://docs.zen-cart.com/dev/plugins/encapsulated_plugins/directory_structure/

[scottcwilson]

Prefer to maintain the existing hierarchy. It's redundant but things become more complex if the plugin hierarchy doesn't mirror the core.

[nickcollie]

I have mixed feelings. The options are:

(1) functions (2) functions/extra_functions (3) extra_functions

I think (3) has nothing going for it.

I think (2) preserves structure and that was my first thought .

Thinking about the question Chris asked, I came around to (1). The reason is that the dream must be to move to a situation in which all plugins are encapsulated. At which point the existing functions/extra_functions directory becomes redundant or at least discouraged. If one is discouraging the original extra_functions directory then it seems strange to be encouraging its use in the plugin structure.

Is there any reason why we cannot just enable both (1) and (2) ?

[lat9]

Doesn't the extra_functions approach for a plugin depend on the scope to be applied to the associated functions?

I can see merit in having the capability for a plugin to have both a functions/plugin_functions.php (loaded when needed) and a functions/extra_functions/plugin_extra_functions.php that are unconditionally loaded as they're used on multiple pages.

[nickcollie]

Why not load them all, all of the time? This is admin, so the saving in loading conditionally, if there is one, really doesn't matter. And it is simpler.

[mc12345678]

Why not load them all, all of the time? This is admin, so the saving in loading conditionally, if there is one, really doesn't matter. And it is simpler.

An issue to attempt to avoid would be two plugins using the same function name (in either an includes/functions or includes/functions/extra_functions file where the function is expected to be applicable only to the plugin that is using it...

E.g., two or more plugins name a plugin: getData with whatever number of arguments. Whichever gets loaded first wins with a subsequent load causing an error because the function already exists.

While I don't believe access remains to some of the way back when ZC documentation, at one point there was discussion that a path forwards was for functions to exist as a part of a class. In this way the "function" could be overridden, could actually be declared/defined the same in multiple/other plugins, but would be referenced/accessed by use of the associated class' objectName. One of the other issues I recall being described as to why it hadn't happened yet and the impact such a transition would cause to legacy software.

For the time being, the existing method of plugin installation is still possible outside of the zc_install process and already there is a lot to be done to a plugin to make it compatible with the zc_install process either by instruction revision or program modification. Seems like this could just be an additional step to be taken which may further the ability for plugins to build off of plugins.

As I stated in the related post at least in intent, it seems to me that transitioning to a process/build that uses the more object oriented process for providing plugin functions would be a direction to head. If not with the implementation of this new plugin structure then when better to do so?

That said, it appears that for (when fully implemented) the catalog side part the issue is already addressed in the following file: https://github.com/zencart/zencart/blob/0bf8a8f076843612e87e25966f9f855f901dc2cd/includes/modules/extra_functions.php as included by: https://github.com/zencart/zencart/blob/0bf8a8f076843612e87e25966f9f855f901dc2cd/includes/init_includes/init_general_funcs.php#L38

[lat9]

Why not load them all, all of the time? This is admin, so the saving in loading conditionally, if there is one, really doesn't matter. And it is simpler.

In addition to what @mc12345678 indicated above, loading all those extra functions all the time also consumes unneeded memory ... some of those extra-functions files are quite large. That takes away the memory resources needed for large reports or other tools.

[nickcollie]

Regarding naming conflicts with functions - that problem exists whether one chooses loading from includes/functions or includes/functions/extra_functions or any other location, or even if one enables both. So, not really relevant.

Regarding classes/functions - probably everyone on this thread is experienced enough to understand the relative advantages of both. And some of may even be getting a bit bored of those advantages being reiterated - who knows? But Zen Cart is unlikely to disallow functions in the near future. So, not really relevant.

Memory resources - gosh I must have been spoilt by the servers I work on. If loading a single file of functions causes a problem then I understand your point. Anyway, I would suggest that the 'large reports' need to be optimised. On larger/busier installations for instance I have always rewritten files such orders.php, mail.php and the like to be more resource effective and done the same with custom reporting that clients have requested. There are much better places to gain resources than a single file load. Also those solutions tend to be much more robust moving forward.

Just my thoughts.

The bottom line of this thread is that plugin authors need to know where to put functions for encapsulated plugins. (or they need to be told that they can't use them)

[scottcwilson]

See #5505 - you shouldn't use the extra_* folders in an encapsulated plugin.

[mc12345678]

See #5505 - you shouldn't use the extra_* folders in an encapsulated plugin.

Wait, no extra_ folders? Not extra_configures, not extra_datafiles? What vision then is there for encapsulated plugins? And when is the display logs included and encapsulated* plugin getting revised to adhere to this statement? Or is it going to be returned to being a plugin that must otherwise be installed?

What about if a plugin provides code intentionally impacting every page load? don't use any of the extra* folders? Really? So a non-encapsulated plugin is instead the way to go using the extra* folders instead?

What's the difference and how does any of that apply to the above issue? The primary problem expressed was the attempt to use non-loaded code to externally "frame" operation or execute code not yet declared. As stated at me, such a problem potentially exists in any code load attempt. Further, the examples provided of inaccessible code suggest that the loaded code is attempting to hold all code for all versions even though the installation process is expected to properly place necessary/associated code negating the need to "externally" determine something like the current version.