? ? Pending

User tests: Successful: Unsuccessful:

avatar wilsonge
wilsonge
29 Mar 2020

Partial Pull Request for Issue #24587 .
Fixes #27007

This ones a bit long - i started by adding the interface and then realised how broken CliApplication was when actually implementing/testing it

Summary of Changes

  • Adds getLanguage to the CMSApplicationInterface
  • Adds these methods to the two cli applications which previously didn't have them
  • Adds missing getInput method to the CliApplication which was in the interface but missing
  • Fixes typo in triggerEvent function inCliApplication
  • Adds property messages used for enqueueMessage in the CliApplication but wasn't defined
  • Fix CliApplication calling wrong parent method
  • Various fixes to finder indexer plugin to make it work

Testing Instructions

Check CLI application - see commands below If you want you can access any web page too to ensure the interface in the web applications doesn't error too.

Commands to run:
php cli/joomla.php (you can also run as many of the commands from here as you'd like to prove this)
php cli/finder_indexer.php

Expected result

Working console apps when new method added to interface

Actual result

Finder Indexer Broken

Documentation Changes Required

Yup interface documentation

avatar wilsonge wilsonge - open - 29 Mar 2020
avatar wilsonge wilsonge - change - 29 Mar 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 29 Mar 2020
Category CLI Libraries
avatar wilsonge wilsonge - change - 29 Mar 2020
The description was changed
avatar wilsonge wilsonge - edited - 29 Mar 2020
avatar richard67
richard67 - comment - 29 Mar 2020

I have tested this item successfully on 6b6000b

All commands of cli/joomla.php still work.
cli/finder_indexer.php still works.
Com_finder in general still works.
Backend and frontend still work.
I hope I haven't forgotten anything.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506.

avatar richard67 richard67 - test_item - 29 Mar 2020 - Tested successfully
avatar wilsonge wilsonge - change - 29 Mar 2020
The description was changed
avatar wilsonge wilsonge - edited - 29 Mar 2020
avatar wilsonge
wilsonge - comment - 29 Mar 2020

Worth noting before this patch the com_finder cli was broken :) so that one is fixed with this patch

avatar richard67
richard67 - comment - 29 Mar 2020

Worth noting before this patch the com_finder cli was broken :) so that one is fixed with this patch

Any issue exists which we can close?

avatar richard67
richard67 - comment - 29 Mar 2020

P.S.: Can confirm it fixes broken cli/finder_indexer.php.

avatar richard67
richard67 - comment - 29 Mar 2020

@wilsonge I've found issue #27007 ... does that fit?

avatar wilsonge
wilsonge - comment - 29 Mar 2020

That's the one - didn't know there was an issue :) but added bonus!

avatar wilsonge wilsonge - change - 29 Mar 2020
The description was changed
avatar wilsonge wilsonge - edited - 29 Mar 2020
avatar richard67
richard67 - comment - 29 Mar 2020

@skurvish Please test if this PR here solves your issue #27007 , and if so, mark your test result at the issue tracker https://issues.joomla.org/tracker/joomla-cms/28506 by using the "Test this" button, selecting the appropriate result (Success I hope) and the submitting.

avatar skurvish
skurvish - comment - 29 Mar 2020

Will do, busy on a few other things right now but given the isolation I should find time over the next few days.

avatar skurvish
skurvish - comment - 29 Mar 2020

I am not able to test this as the commits for the PR do not seem to be in the 4.0-dev branch.

avatar richard67
richard67 - comment - 29 Mar 2020

@skurvish It is a pull request (PR) and so not merged yet into the 4.0-dev branch. It needs test before. You can find here a full install download package which includes the patch of this PR: https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/28506/downloads/30756/Joomla_4.0.0-beta1-dev+pr.28506-Development-Full_Package.zip. Or you have a 4.0-dev installation and apply the patch to that, e.g. using the Patchtester component.

avatar skurvish
skurvish - comment - 29 Mar 2020

I can confirm that both the joomla and finder cli apps work. I would like to test my own cli if I can before I agree all is well. Quick question, will this be ported to J3.9 or will this be a J4 only implementation?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506.

avatar richard67
richard67 - comment - 29 Mar 2020

@skurvish Could you mark your test result at the issue tracker https://issues.joomla.org/tracker/joomla-cms/28506 by using the "Test this" button, selecting the appropriate result (Success I hope) and then submitting? Thanks in advance.

As I can see this is a fix for J4. No idea if it can and will be backported to J3.

avatar wilsonge
wilsonge - comment - 29 Mar 2020

This doesn’t need to get backported to 3.9 - these changes are only required due to work in the 4.0 branch. I will be documenting what changes you need to make to have things work on 3.9 and 4.0 for cli scripts though :)

avatar skurvish
skurvish - comment - 29 Mar 2020

That would be awesome @wilsonge.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506.

avatar wilsonge
wilsonge - comment - 29 Mar 2020

There are 4 main things:

  1. if you are interfacing with core extensions you'll need to load the namespacemapper in your cli script https://github.com/joomla/joomla-cms/pull/28506/files#diff-64483624db166a09dbe03732dc7f7893R147 - this ones a bit trickier - I haven't experimented yet - but instead of the trait you may just need to include these 3 lines inside the doExecute method and inside a if (version_compare(JVERSION, '4.0', 'ge'))

  2. You'll need to add a getName function to your CLI script https://github.com/joomla/joomla-cms/pull/28506/files#diff-64483624db166a09dbe03732dc7f7893R339 that will return a unique string to identify your application (this will work fine in all Joomla versions so you can do this immediately from 3.9 without issues)

  3. You'll need to boot your application slightly differently by using the Dependency Injection Container rather than using JApplicationCli::getInstance like we do here https://github.com/joomla/joomla-cms/pull/28506/files#diff-64483624db166a09dbe03732dc7f7893L473-L491 (you should be able to do an if/else with a JVersion condition here)

  4. The usual stuff around the use of deprecated code in 3 that we have removed in 4 (obviously this applies just as much to CLI scripts as everything else)

avatar skurvish
skurvish - comment - 29 Mar 2020

Great, thanks. I am out tomorrow but can see if I can integrate and test these changes Tuesday/Wednesday and report back. If I run into any issues where should I report them, here somewhere else?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506.

avatar wilsonge
wilsonge - comment - 29 Mar 2020

Yeah just post here. I'm going to merge this now as I need this interface change to unblock several other things. If it becomes complex I may get you to open a new issue if needs arise. But here will do as a starting point :) Thank you in advance for any feedback will be really valuable for me to help document migration paths!

avatar wilsonge wilsonge - change - 29 Mar 2020
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-03-29 23:23:38
Closed_By wilsonge
Labels Added: ? ?
avatar wilsonge wilsonge - close - 29 Mar 2020
avatar wilsonge wilsonge - merge - 29 Mar 2020
avatar skurvish
skurvish - comment - 2 Apr 2020

I was not able to get the cli app to work without including the joomla configuration in the container. In order to accomplish this I needed to include_once( configuration.php), then add as the second param in the return statement: new Registry(new \JConfig).

My app does rely on the the config file and to use Factory::getConfig() in my app required this. I suppose I could have done the same thing in my app but this made more sense to inject it into the ApplicationCli container.

avatar skurvish
skurvish - comment - 2 Apr 2020

@wilsonge I am also trying to get the ApplicationWeb interface to work. When I create the container then run the getContainer, the app object returned does not have a dispatcher attached to it (i.e. it is null in the object). Did you look at the web interface and might it require similar changes you made to the cli interface?

avatar skurvish
skurvish - comment - 4 Apr 2020

@wilsonge Hey George, I solved my problems. Turns out in J4 I need to use the CMSApplication class instead of WebApplication. I must also set the dispatcher and session in order to load plugins.

avatar wilsonge
wilsonge - comment - 8 Apr 2020

I was not able to get the cli app to work without including the joomla configuration in the container. In order to accomplish this I needed to include_once( configuration.php), then add as the second param in the return statement: new Registry(new \JConfig).

If you boot your app with the container like here https://github.com/joomla/joomla-cms/blob/4.0-dev/cli/finder_indexer.php#L490-L514 that shouldn't be an issue.

I am also trying to get the ApplicationWeb interface to work. When I create the container then run the getContainer, the app object returned does not have a dispatcher attached to it (i.e. it is null in the object). Did you look at the web interface and might it require similar changes you made to the cli interface?

You'll need to set the dispatcher into the app yourself. See how we boot up the web application classes ourselves in the container https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Service/Provider/Application.php#L58-L69

There's nothing missing as far as I know. However don't have any custom web apps to hand to test for you either to give a better idea. Can you give some sample code if the above doesn't help

avatar skurvish
skurvish - comment - 9 Apr 2020

I attempted to use the container setup exactly like the finder app but the configuration was not available in my app and numerous errors occurred. Inserting the config as stated solved the issue.

Your examples for the web app show it using the SiteApplication and AdministratorApplication classes. I attempted to use the WebApplication class which is what I used in J3.x but that didn't work at all. I changed to using the CMSApplication class and that at least instantiated OK but doing the $container->get(DispatcherInterface::class) returned null so I created a dispatcher using new \Joomla\Event\Dispatcher and set it using the setDispatcher method.

I am not sure any of this is right but it is working. I would be happy to keep working on it to try new things to sort it out properly if you would like. I guess I would rather get it correct and working than just working and perhaps not in the preffered/correct manner.

avatar skurvish
skurvish - comment - 18 Apr 2020

@wilsonge did you want me to try anything else?

avatar infograf768 infograf768 - change - 20 Apr 2020
Title
Add getLanguage to application interface and cleanup console/cli apps
[4.0] Add getLanguage to application interface and cleanup console/cli apps
avatar infograf768 infograf768 - edited - 20 Apr 2020

Add a Comment

Login with GitHub to post a comment