User tests: Successful: Unsuccessful:
If you have the plugin System - Page Cache enabled and you use Joomla CLI entry point with an absolute path (as it happens when it's triggered by a CRON job) a fatal error would happen.
This is a side effect caused by enabling the plugin System - Page Cache. Once enabled, it's always loaded, even in contexts where cache should never be allowed (ie backend, CLI etc etc).
In the constructor it tries to parse the current URL; usually this won't cause any harm, beside a warning:
php joomla.php list
PHP Notice: Undefined index: HTTP_HOST in /var/www/html/joomla/libraries/src/Uri/Uri.php on line 103
However, everything breaks if an absolute path is supplied:
php /var/www/html/joomla/cli/joomla.php list
PHP Notice: Undefined index: HTTP_HOST in /var/www/html/joomla/libraries/src/Uri/Uri.php on line 103
RuntimeException {#539
#message: "Could not parse the requested URI http:///var/www/html/joomla/cli/joomla.php"
#code: 0
#file: "/var/www/html/joomla/libraries/vendor/joomla/uri/src/AbstractUri.php"
#line: 373
trace: {
/var/www/html/joomla/libraries/vendor/joomla/uri/src/AbstractUri.php:373 { …}
/var/www/html/joomla/libraries/src/Uri/Uri.php:305 {
Joomla\CMS\Uri\Uri->parse($uri)^
› {
› \treturn parent::parse($uri);
› }
}
/var/www/html/joomla/libraries/vendor/joomla/uri/src/AbstractUri.php:111 { …}
/var/www/html/joomla/libraries/src/Uri/Uri.php:121 { …}
/var/www/html/joomla/plugins/system/cache/cache.php:71 { …}
/var/www/html/joomla/libraries/src/Extension/ExtensionManagerTrait.php:242 { …}
/var/www/html/joomla/libraries/src/Extension/ExtensionManagerTrait.php:160 { …}
/var/www/html/joomla/libraries/src/Extension/ExtensionManagerTrait.php:94 { …}
/var/www/html/joomla/libraries/src/Plugin/PluginHelper.php:235 { …}
/var/www/html/joomla/libraries/src/Plugin/PluginHelper.php:193 { …}
/var/www/html/joomla/libraries/src/Application/ConsoleApplication.php:228 { …}
/var/www/html/joomla/cli/joomla.php:76 { …}
}
}
This happens because the absolute path is "converted" into a URL http:////var/www/html/joomla/cli/joomla.php
(note the 3 slashes) and then it fails to be parsed.
The proper solution is to let this plugin run only when it makes sense, in the public section of the website.
1 - Enable the System - Page Cache plugin
2 - Open a terminal, navigate to the cli folder and type php joomla.php list
3 - Only a notice should happen
4 - Now use the full path php /path/to/cli/joomla.php list
A fatal error should happen
Everything should work as expected
None
Status | New | ⇒ | Pending |
Category | ⇒ | Front End Plugins |
Yes, this is the right approach to solve the issue. This PR solves #35722 @alikon @ceford @PhilETaylor #35625 @Kubik-Rubik
This PR solves #35722 ...
@anibalsanchez You mean it replaces @alikon 's PR?
I have tested this item
Works as described. Without the patch I get PHP Warning: Undefined array key "HTTP_HOST" in /Users/ceford/Sites/j4test/libraries/src/Uri/Uri.php on line 103 and a stack trace. With the patch it works without error.
@richard67 Yes, I mean that it replaces it.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
RTC
@tampe125 As your pull request has 2 good tests, I've set the RTC status, which means ready to commit. But it would be nice if you could check @PhilETaylor 's suggestion about changing the code comments. If you change that, it will not need to be tested again because it's just code comments. Thanks in advance.
Ok, I'll update code comments.
Regarding @alikon comment, let me know if cache should be enabled on API or not.
If my understanding is correct, this plugin should handle the cache for HTML rendering (correct me if I'm wrong), so even if caching API would be "technically" right, that's out of the scope of this plugin.
Labels |
Added:
?
|
One additional note on the issue: the problem's source is the execution of a "Web Only" plugin in the Console context.
The proposed conditional to avoid the execution is going to solve the issue. However, the same problem will be found in all the "Web Only" plugins, and more of these conditionals will be required.
It could be a better solution in the long term to create an interface "WebApplicationPlugin" and avoid executing these plugins in the dispatcher.
But then people will complain that they need to update their extensions. Can't win.
@brianteeman All of these developments are part of the new official support of the Console Apps. So, they are needed to complete the implementation.
In the past, you could hack your way around these problems. For instance, to create a native command based on Joomla. But, they were error-prone, and you always got notices and warnings.
You missed the tongue in cheek comment - never mind
@anibalsanchez I strongly disagree with your proposal. The EventDispatcher is finally and correctly disentangled from the application object. In fact, it is possible to instantiate it outside the scope of an application and dispatch any kind of custom events. What you proposed is a major step backwards as it would make EventDispatcher aware of and entangled with the application object.
Moreover, what you propose does not address what happens if another web application is added to Joomla or what happens if a developer implements a custom application e.g. by extending WebApplication or CliApplication (deprecated) / ConsoleApplication. Extending WebApplication to create custom entry point files which fire Joomla's system plugins is something very common with payment processor integrations. There are several payment processors which do not allow query string parameters in the callback URLs, making the use of a component or com_ajax impossible without some serious (and precarious!) .htaccess / web.config / NginX configuration magic.
Ultimately, it's the plugin developer's responsibility to consider the context of their plugin and decide which Joomla applications it should run in. A plugin which must only fire in the frontend can use Factory::getApplication()->isClient('site')
to do that. A plugin which can run in any HTML application and only for HTML output can check whether the application supports getDocument() and check the object type of the document. A plugin which can only run in a CLI context can check if the application name is cli
— even custom CLI applications are meant to do that.
Also, to set the history straight, using an EventDispatcher that's disentangled from the Application object has ABSOLUTELY NOTHING AT ALL to do with the ConsoleApplication or the ApiApplication. I can tell you that as the developer who integrated the Joomla Framework Event package with Joomla 4 back in August 2015, long before either of these applications were created. Using immutable events has all to do with performance and preventing problematic interactions between different plugins handling the same event depending on their order (for example: plugins changing the type of a parameter passed by reference between array and object, breaking plugins further down the event pipeline). Eventually the “classic” plugins will go away and it will no longer be possible to pass parameters by reference in an event being handled by plugins. This will solve all those issues of yesteryear since you can only add to the Event's results while making for better performance because of the superior handling of events in the Events package.
@brianteeman I edited my original comment to only include the reasoning of my disagreement with Anibal. I removed my reply to your reply so you can now delete your reply to my comment as well.
@nikosdion Thanks for the analysis. I agree that the is better if the EventDispatcher knows nothing about the clients.
The problem is that all system plugins are auto-subscribed to all events regardless of the application context and they have to check the context and eject the execution. It would be better to look for a way to discriminate the events at the source.
@anibalsanchez You really have no idea how Joomla 4 works. Fair enough, let me elucidate.
Let's start backwards, with Joomla 1.0 to 3.10 inclusive. The plugin system in those versions was a clumsy implementation of the Observable/Observer pattern. As a result, every plugin being loaded would immediately have all its public methods registered as observers. This could, indeed, cause problems. You needed to duplicate the code which checks the conditions for whether your Observer should fire in each and every public method — unless you implemented clever hacks like using a flag in the plugin object which is initialised in the constructor, a pattern I have used extensively.
In Joomla 4 the legacy plugins do get the same treatment BUT THIS IS NOT THE CANONICAL WAY TO WRITE PLUGINS! As I explained before, Joomla 4 uses the Joomla Framework Event package to handle immutable Events instead of the clumsy implementation of the Observable/Observer pattern of yesteryear.
Joomla 4 already has theJoomla\Event\SubscriberInterface
. A plugin class which implements this interface WILL NOT HAVE all of its public methods magically become Observers! Instead, the plugins loader expects that it implements the getSubscribedEvents
method of the interface which returns a dictionary (array with string keys). The key of each array element is the event name and the value is the (public) method name in the plugin which handles the event. This allows the developer to decide which events to handle based on the execution context.
Furthermore, the public methods in the plugin no longer receive a number of parameters and return whatever random data type. Instead, they receive a Joomla\Event\Event
object and return void
. The result of handling the event is communicated by setting the result
argument in the event. Each event handler can see the stack of results preceding it being called (the result
named argument is an array) and can choose to do things like not execute if there is a certain value already returned, replace the result stack or append to the result stack. This solves issues like on after user login event handlers firing when another handler before them has already marked a failed execution. It also allows for things like a plugin which can conditionally clear the on content prepare results.
Anyway, the things you should take from this is that a PROPERLY IMPLEMENTED, NATIVE, JOOMLA 4 plugin can decide for itself which event handlers to register. It's not FORCED to register everything anymore! Therefore your suggestion is completely useless and violates the architecture of Joomla 4.
Further to that, what you propose is simply a terrible idea that will NOT work correctly because we have more applications than the ones already built into Joomla itself. Anyone can extend from WebApplication, CMSApplication, ConsoleApplication or even CliApplication to create their own custom application. I already told you that this does happen with payment processor callback plugins. Having interfaces which only address the core applications omits all custom applications. This means that the system behaviour will be vastly different in those custom applications. I will give you two examples where this can lead to catastrophic failure:
Things can get even more hairy than that. This was just the low hanging fruit based on two things I was providing support for just this morning.
The other major architectural problem is that Joomla 4 is designed to allow legacy Joomla 3 extensions to work on Joomla 4 (with minimal modifications) to drive the adoption of Joomla 4. To clarify, the goal is that a Joomla 3 extension can be slightly modified to work on BOTH Joomla 3.10 AND Joomla 4.x with the same package. Developers need to focus on the few things that break between Joomla 3 and 4 instead of reimplementing their entire application from scratch with Joomla 4 core MVC right away. What you propose would completely break this expectation. You would need separate packages for Joomla 3 and 4 since there's not going to be a Joomla 3.11 and you can't arbitrarily add a bunch of interfaces that do sod all in Joomla 3.10, nor can you break Joomla 3.10 by having the interfaces do something.
Even worse, you enter a situation where a plugin can EITHER work on Joomla 3/4.0 OR Joomla 4.whatever. This makes upgrading Joomla 3 site and updating Joomla 4 sites impossible. You can't install the new version of the plugins on an older Joomla version and you can't update to the newer Joomla version before installing new versions of the plugins. Essentially, you are asking people to trash their sites and build them afresh. And no, disabling all plugins before an update/upgrade or uninstalling all extensions is not a solution; if you have ever maintained a real world site you know why. So, this immediately becomes a no–go. You could have interfaces to DISABLE execution in certain core applications but why do that when there's the SubscriberInterface which allows you to do that and have even more fine grained control?! It all sounds like a pointless exercise because you are missing a fundamental architecture point of Joomla 4, the migration from Observers to Events in plugins.
The correct solution, as I said before, is documenting and communicating this. I would also say that Joomla itself should move its core plugins from the legacy implementation to the modern Event system. I don't know why it's not done yet. The Events package was integrated in August 2016 and the SubscriberInterface was added a few months later, in February 2017.
Thanks for the clarification @nikosdion I didn't know there is now a better way to subscribe to events.
@anibalsanchez Considering that you are definitely not a newbie developer I am worried that Joomla may have not made it very clear that the plugin system has changed into an event subscription system. If you know who would be best to contact about developer documentation please do and let them know I can help with an example plugin if need be. I have no idea who to contact about this :/
I read the comments by @nikosdion and was wondering how to turn them into useful documentation. By coincide, this morning I was working on the Joomla 4 Tutorials project page where there is a description of Creating a Plugin for Joomla - the history shows it was started by WilsonGE and contributed to by Sandra97 plus others. Is that clear enough? It is here: https://docs.joomla.org/J4.x:Creating_a_Plugin_for_Joomla
I would offer to do revision but my level of understanding of the architecture is probably not good enough!
@nikosdion @ceford In my case, when I read the code and the documentation, in theory, it sounds great, but at the end of the day, you find the same errors as before. For instance, the previous ideas to solve this bug are:
$_SERVER['HTTP_HOST']
#35722if (!$this->app->isClient('site')) { return; }
What is missing is a simple way to know how to do it right. The available information (and our previous experiences) led us to do it as before. For instance, in this case, it looks like that the modern way to do it right would be something like:
public static function getSubscribedEvents(): array
{
if ($this->app->isClient('site')) {
return [
'onAfterRoute' => 'onAfterRoute',
'onAfterRender' => 'onAfterRender',
'onAfterRespond' => 'onAfterRespond',
];
}
return [];
}
@ceford This is kinda right but not quite. The recommended method is a mélange of the Joomla 3 plugin–is–a–class way and the Joomla 4 plugin–is–a–bootable–extension way. I think it was written like that because the installer can still not install a fully native Joomla 4 plugin without doing some workarounds.
For the curious, here's what a fully native Joomla 4 plugin looks like and here is the workaround to the Joomla installer bug — yeah, the workaround is an empty legacy plugin class which is never loaded. The reason to use this method is, of course, that you have class autoloading for the plugin which makes a lot of sense in complex plugins, like the one I linked to. The alternative was to use traits upon traits to make the code more readable, something I've done in Joomla 4's plugins/system/webauthn.
But I digress. I think the current documentation page is at the very least a good start because it tells people to use the SubscriberInterface. Once a developer gets that right everything else is a matter of very simple refactoring down the line.
@anibalsanchez I can tell you my opinion on every method and I then I will tell you what I told Davide when he asked me.
Setting HTTP_HOST in CLI: Yes... but no. The reason for that is that Joomla\CMS\Uri\Uri
borks hard in CLI as it has a hard dependency on that server variable. Joomla should have a Global Configuration (read: configuration.php
) option for the canonical URL to the site to use in CLI only. Currently there's only $live_site
which applies to the web applications too. However, it's wrong to set $live_site because it doesn't take into account non-www to www redirections (or vice versa) you may have set at the server level and has a hardcoded protocol (http:// or https://) which makes things complicated. It can definitely lead to redirection loops. We see 1–2 of these every month and we always know ”ah, someone set $live_site without realising what it does”. Also, this would fix several issues — notably sending emails from CLI that need to reference a URL on the site — but not the bug in the page cache plugin.
Checking the application: Yes, it's the correct approach in the context of the code of this plugin as it stands right now. That's basically what I told Davide when he asked me how it should be fixed. I told him that to go for the minimally viable solution, checking if we are in the correct application context, to minimise the chance of this fix being rejected.
Using SubscriberInterface. Sure, that's the best approach — as long as we simplify the if–block to return early when the application type doesn't match. Early returning if–blocks are much easier to read. However, the code you typed is not enough! You need to change the signatures of onAfterRoute
, onAfterRender
and onAfterRespond
methods to take a single argument of the \Joomla\Event\Event
type and change the way they return results (the Event methods are always void). This is a refactoring, not a fix, and cannot possibly be included in a patch release.
Here's an example of production code which demonstrates how it works: https://github.com/akeeba/release-system/blob/cbcfb6d8ddd000b8a07e8ba7dec66f1a12ea4f5e/plugins/content/arsdlid/src/Extension/Arsdlid.php#L77-L96 Compare that code with any other onContentPrepare event handler to understand the difference.
Instead of public function onContentPrepare($context, $article, $params, $limitstart)
you have
public function onContentPrepare(Event $event)
{
[$context, $article, $params, $limitstart] = $event->getArguments();
The rest of the body goes on as per usual.
Since that code is for an event that does not have a return value it doesn't show how you return a value. If you were handling an event where you need to return $yourResult
you would need to do this:
$result = $event->getArgument('result', []);
$event->setArgument('result', array_merge(is_array($result) ? $result : [$result], [$yourResult]));
// No other return here; the method is void
As it becomes immediately obvious, changing the method signatures is not something that you can approve without a second thought. Someone needs to go and do a lot of testing to make sure that when these events fire we don't get any unexpected artefacts. To the best of my knowledge only editor-xtd
plugins do not support Events because they are not real plugins, they are being instantiated by the Editor helper and their onDisplay method accessed directly, without going through the EventDispatcher. I would assume this particular plugin is safe from this kind of legacy access but I can't be sure without a lot more digging.
So, the only acceptable solution for a patch release is what Davide implemented in this PR. It's the best compromise between being practical and pedantic.
In theory, today, Joomla 4.1 Alpha 2 is being released. I run a few tests with SubscriberInterface
and it works perfectly fine. The improvement to using the SubscriberInterface
to discriminate the events per execution context could be an excellent addition. In the end, it is a way to make sense of the new Joomla 4 benefits.
@ceford About the Joomla 4 plugin documentation: I think that the page has the bare minimum information. By experience, I know that a plugin must be located in a specific folder, with a specific file name, with a particular class name, with a set of methods, and now I know that on J4, it can use the new SubscriberInterface
to work based on the new execution pipeline. That's only the part of developing a plugin ... there is a lot more info needed to create a plugin. In my opinion, documentation for developers must address at least all the points that we have discussed here. I know that writing documentation is complex, and I always try to think in terms of structure and content. A single page with a few tips is not enough to consider a topic documented. That's the worst part of the current documentation, single random pages without organization or structure.
About documentation examples with a respectable organization, a couple of links:
@anibalsanchez I would encourage you to make a PR against the 4.1-dev branch with the One True Joomla 4 Way. Add me as a code reviewer so I can take a look.
For Joomla 4.0 this PR here is correct and perfect, addressing a major issue: you cannot set up CRON jobs on a real world commercial host if the Page Cache plugin is enabled.
Since this is not an immediately obvious conclusion let me explain. CRON jobs in commercial hosting (be it cPanel, Plesk, a custom hosting control panel or directly editing the crontab
) require a single command line. Users typically write something to the tune of /usr/local/bin/php74 /home/myuser/public_html/cli/joomla.php some:command --parameter=1
. With the Page Cache plugin enabled this fails immediately. One would have to write it as cd /home/myuser/public_html/cli; /usr/local/bin/php74 ./joomla.php some:command --parameter=1
. Sounds simple, right? Well, depending on your hosting control panel this might not work as the hosting control panel may not escape and include the command line in double quotes which would end up with your CRON job only running the cd
to change the working directory without actually running the command! You'd then have to create a .sh
script and now what used to be a simple “create this simple command line” instruction becomes a very involved process which requires some basic knowledge of Linux. That's really the most important reason to fix it in 4.0. We can't tell people ”yo, for the next 5 or so months you can EITHER use CRON jobs OR the Page Cache plugin — unless you know your way around Linux”. It'd be a form of gatekeeping.
I have tested this item
Agreed. This PR solves the issue.
I am working on converting an example I have to the pure way. Right now I am stuck on the public static function getSubscribedEvents(): array
in the plugin file. I think that needs to be in the provider but I don't know how - not the function but a way tell the container to listen for these events. At the moment my Command command:action is not defined
.
@anibalsanchez - I agree, organisation of the documentation is poor and any attempt to improve it may make it worse. Searching turns up stuff going back to J1.5 so you get a haystack when looking for a needle. I will take a look at the ones you mentioned. Thanks.
Having used the cli package myself for a few a few months I created a tutorial just a week ago. I have now figured out how to use the pure Joomla 4 plugin architecture with a cli command. It seems the example provided by @nikosdion does not work with the cli interface because some of the code needs an AbstractCommand parent rather than a CMSPlugin parent. I just know in my bones that the two-stage solution I have come up with is probably not the best, or even dangerously wrong. If anyone has time to take a look it is here: https://github.com/ceford/j4xdemos-plg-onoffbydate
The content of a tutorial on the Joomla 4 Tutorials page features this stuff and I don't want to lead anyone astray.
@ceford Oof, we are getting way off topic here. There are many ways to skin a cat but there's definitely an easier way than yours. Not necessarily better, but far more straightforward for a task as simple as registering a simple CLI command which doesn't depend on any other extension.
First, make sure your plugin class (the one extending CMSPlugin) gets an instance to the application:
protected $app;
(yes, no code required, it's populated by the CMSPlugin constructor automatically, all you need to do is declare the property protected or public).
Make sure your plugin implements the SubscriberInterface.
Register an event handler for the application.before_execute
event:
public static function getSubscribedEvents(): array
{
return [
Joomla\Application\ApplicationEvents\ApplicationEvents::BEFORE_EXECUTE => 'registerCLICommands',
];
}
In the registerCliCommands method you can instantiate your command and push it to the Console application:
public function registerCLICommands(ApplicationEvent $event)
{
$commandObject = new \Joomla\Plugin\System\Onoffbydate\Console\OnoffbydateCommand();
$this->app->addCommand($commandObject);
}
Finally, edit your command to add its name:
protected static $defaultName = 'onoffbydate:action';
No need to mess with services. Everything about the command is included in a single command class and command registration is included in a single plugin class. You can scale this very easily by having an array of command classes' FQNs (fully qualified names), e.g.
$commands = [
\Joomla\Plugin\System\Onoffbydate\Console\OnoffbydateCommand::class,
\Joomla\Plugin\System\Onoffbydate\Console\AnotherCommand::class,
];
foreach ($commands as $fqn)
{
$commandObject = new $fqn();
$this->app->addCommand($commandObject);
}
We use this pattern in our (for a fee) software to have a single console
plugin register all the commands we include under the component's namespace. It's very easy to maintain.
If you want to hit me up with more questions feel free to go to www.dionysopoulos.me (my blog) and use Contact Me to get in touch.
@nikosdion Thank you. Your suggestions are now incorporated into the plugin and it works. The associated tutorial is here: https://docs.joomla.org/J4_CLI_example_-_Onoffbydate#Check
@anibalsanchez I looked at the documents you cited and remembered I have seen them before. My immediate thought is they are not Wikis! But those sorts of layouts are what I would like to see.
I have tested this item
Patch all good. Sorry about diverting the thread into documentation.
@ceford You're welcome! I am glad I could help
BTW, the reason I have not filed a successful test is that Davide is a coworker and asked me about how to best address this issue. I believe that me filing a successful test on code implementing a solution I verbally described would be a case of “the priest blessing his beard” as we say in my part of the world.
I have tested this item
@Kubik-Rubik you suggest a change and at the same time submit a successful test? What shall we do now? Set RTC? Or wait until your suggested change gets some reply?
@richard67 Well, the check is unnecessary here, but it does not interfere with the primary purpose of this pull request. :-)
@Kubik-Rubik So if I set RTC it might get merged with that unnecessary check and it will be forgotten, and if the check is removed we have to remove RTC and test again to be sure it was redundant. That is not very effective. Either a PR is good as it is or it is not. If I have successfully tested and I know there is still something wrong in code I think I should not give a good test.
Knowing that the PR possibly added redundant code I don’t feel comfortable to give RTC now. Shall someone else do that.
@richard67 Can't you make the change directly as someone with commit rights to the repo?
@richard67 Can't you make the change directly as someone with commit rights to the repo?
@nikosdion I prefer to leave that to someone else.
I’ve pinged other maintainers so they can decide.
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-11-05 23:32:01 |
Closed_By | ⇒ | HLeithner |
Thanks for the PR and thanks for the explanation @nikosdion sadly the documentation (didn't found a pull request with documentation to the new Event/Subscriber system). Adding this to the documentation would be great. Here in this PR it get lost...
Harald, I think it would be most beneficial to write a tutorial for a modern plugin. It’s on my long list of things I’d like to do — once I have converted my last two extensions to fully native Joomla 4 code. I am nearly done with one, the other one is much smaller. I believe that by the time 4.1 will be released I will be able to write a short tutorial, explaining the what and the why.
@HLeithner I've submitted PR #35986 for review to organize and help in the plugin improvement and documentation.
@anibalsanchez