? Success

User tests: Successful: Unsuccessful:

avatar mbabker
mbabker
14 Dec 2014

This is a start at refactoring some of the internals of the JInstallerAdapter objects. Right now, there is a lot of duplicate code in each of the adapters, behaviors are rather inconsistent, and all in all the code is an ugly mess.

This PR starts to clean some of that up. First, a new JInstallerAdapter class is introduced which all adapters are now extending from. For B/C purposes, it continues to extend JAdapterInstance however this inheritance is marked as deprecated for 4.0.

Next, several generic methods are introduced into the new abstract class to try and cut some duplicated code out of the system.

The install routines of the component and module adapters have been refactored to demonstrate some of this approach. The uninstall routine is also cleaned up some to use this code.

Note that further refactoring is possible and encouraged across all of the adapters. One of my goals is to implement the changes introduced at https://github.com/joomla-projects/joomla-cms/tree/feature-JInstaller/libraries/cms/installer in a B/C manner. The work in that branch standardized the install routine across all adapters (including supported features like install scripts) and had started work on standardizing the uninstall routines.

Testing Instructions

Simply apply the patch and install and uninstall extensions; mainly components and modules. There are no B/C breaking changes to the public API in JInstaller and changes in the public API extension adapters should be B/C should any developer have extended these classes.

Votes

# of Users Experiencing Issue
0/1
Average Importance Score
3.00

avatar mbabker mbabker - open - 14 Dec 2014
avatar jissues-bot jissues-bot - change - 14 Dec 2014
Labels Added: ?
avatar brianteeman brianteeman - change - 15 Dec 2014
Category Libraries
avatar waader
waader - comment - 15 Dec 2014

I get the following errors

Notice: Undefined variable: element in C:\Program Files (x86)\Ampps\www\joomla34\libraries\cms\installer\adapter\component.php on line 1026

Fatal error: Cannot access protected property JInstaller::$extension_message in C:\Program Files (x86)\Ampps\www\joomla34\libraries\cms\installer\adapter.php on line 374

when I do this:

  • install current staging branch
  • install acymailing (starter edition)
  • install com_patchtester
  • install your patch via com_patchtester
  • uninstall acymailing in Extension manager -> Manage
avatar mbabker
mbabker - comment - 15 Dec 2014

Thanks @waader fixes are pushed up.

avatar waader
waader - comment - 15 Dec 2014

Now I get a 404 error Component not found
when I follow this procedure:

  • install current staging branch
  • install com_patchtester
  • install your patch via com_patchtester
  • install acymailing (starter edition)
avatar mbabker
mbabker - comment - 15 Dec 2014

Thanks again, error fixed.

avatar waader
waader - comment - 15 Dec 2014

I found some warnings in the error.log

[Mon Dec 15 19:51:58.250757 2014] [:error] [pid 2344:tid 1356] [client 127.0.0.1:9041] PHP Warning: constant() [function.constant]: Couldn't find constant JPATH_ in C:\Program Files (x86)\Ampps\www\joomla34\libraries\cms\installer\adapter\module.php on line 256, referer: http://localhost/joomla34/administrator/index.php?option=com_installer&view=manage
[Mon Dec 15 19:51:58.310757 2014] [:error] [pid 2344:tid 1356] [client 127.0.0.1:9041] PHP Warning: Invalid argument supplied for foreach() in C:\Program Files (x86)\Ampps\www\joomla34\libraries\cms\installer\adapter\module.php on line 936, referer: http://localhost/joomla34/administrator/index.php?option=com_installer&view=manage

when I do this:

  • install current staging branch
  • install com_patchtester
  • install your patch via com_patchtester
  • install acymailing (starter edition)
  • uninstall acymailing with all plugins and the module and component alltogether
avatar mbabker
mbabker - comment - 15 Dec 2014

I've fixed the invalid argument error. "Couldn't find constant" message is an AcyMailing bug (I can replicate it on a new install of their com_acymailing_starter_v4.8.1_2014-12-09_22-24-27 package with staging). The module's XML manifest is still written to Joomla 1.5 standards and their installer doesn't install the additional extensions through the Joomla API; rather while they are copying the files over they are doing a str_replace on some items to make the manifest compatible with the 1.6+ standard. Their top level tag is missing the client="site" attribute, which if I'm not mistaken is basically required from 1.6 forward. So, when the uninstall routine is trying to load the module language files, it tries to get the client attribute to know where to search for the language file; having no attribute results in a null string which creates the incorrect JPATH_ constant.

avatar waader
waader - comment - 15 Dec 2014

Thanks! I have posted the problem in the acymailing forum and will continue testing.

avatar nikosdion
nikosdion - comment - 16 Dec 2014

Almost there. I got this warning:
Warning: Missing argument 1 for JInstallerAdapterModule::checkExistingExtension(), called in MY_SITE_ROOT/libraries/cms/installer/adapter/module.php on line 471 and defined in MY_SITE_ROOT/libraries/cms/installer/adapter/module.php on line 55
installing Akeeba Subscriptions 4.1.2

I also get JLIB_INSTALLER_ERROR_NO_FILE for each installed plugin, but only with Akeeba Subs and ARS (probably an error in my XML manifests?).

avatar mbabker
mbabker - comment - 16 Dec 2014

Fixed the missing argument message. I'll have to dig in a little more later on for the other error.

avatar nikosdion
nikosdion - comment - 17 Dec 2014

@test Successful

The other error was my bad. Sorry. I can confirm that your PR as of commit 6540750 works fine over here.

avatar waader
waader - comment - 17 Dec 2014

@test success!

I installed/uninstalled a lot of components. Quite often I saw messages like "Error uninstalling ." or other things. But that´s unrelated to this patch.

avatar waader waader - test_item - 17 Dec 2014 - Tested successfully
avatar wilsonge wilsonge - close - 22 Dec 2014
avatar wilsonge
wilsonge - comment - 22 Dec 2014

Merged with 5822406 Thanks Michael!

avatar wilsonge wilsonge - close - 22 Dec 2014
avatar wilsonge wilsonge - change - 22 Dec 2014
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2014-12-22 17:18:46
avatar wilsonge wilsonge - reference | 5822406 - 22 Dec 14
avatar wilsonge wilsonge - change - 22 Dec 2014
Milestone Added:
avatar mbabker mbabker - head_ref_deleted - 24 Dec 2014

Add a Comment

Login with GitHub to post a comment