Skip to content

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Jul 14, 2024

Summary of Changes

This is an effort to remove deprecated code from the system. The Installer class depends on the Adapter class, which in turn has been deprecated for a long time already. This PR removes the code of the Adapter class from the Installer class and replaces it with some sane methods to add adapters when required.

This PR does some cleanup and it adds the following methods:

  • loadAdapters(): This iterates over the set folder and adds all files it finds as available installer adapters. To keep it light, the array only contains the name as key and either the class name as value or an object. The object is instantiated just in time when needed.
  • getAdapters(): This method keeps the behavior of the $custom parameter, but otherwise still just returns the list of available adapters
  • getAdapter(): This new method replaces loadAdapter() and either returns an object which was previously made or instantiates it from the class stored in the $adapters array.
  • loadAdapter(): This one gets deprecated in favour of getAdapter(). Just to keep the naming consistent.
  • setAdapter(): This new method allows to hand in a name for the adapter and either a class name or an instantiated object for the respective adapter type. This would allow you to add custom adapter types and also to overwrite existing adapter types.

I reused some method names from the Adapter class. This PR is a b/c break and thus targeted towards 6.0.

Testing Instructions

  • Install an extension via the backend extension installer.
  • Install an extension via the CLI. (php cli/joomla.php extension:install)

There should be no difference between before and after applying the change and the extension should be installed like normal.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org: CMS Adapter removed Manual#301

  • No documentation changes for manual.joomla.org needed

@Hackwar Hackwar added the b/c break This item changes the behavior in an incompatible why. HEADS UP label Jul 14, 2024
@Hackwar Hackwar requested a review from rdeutz as a code owner July 14, 2024 21:43
@muhme
Copy link
Contributor

muhme commented May 30, 2025

I have tested this item 🔴 unsuccessfully on ce7745c

Tested on 6.0-dev branch with local installation on macOS, PHP 8.4.5 with module zitat-service.de
Before applying the PR

  • Installed extension from web, configured and checked its working, uninstalled the extension, checked is gone
  • Installed extension with php cli/joomla.php extension:install --url and uninstalled
  • Installed extension with php cli/joomla.php extension:install --path and uninstalled

Applied the patch by gh pr checkout 43792 && brew services restart httpd

  • Set Logging 'Log Almost Everything' and 'Log Deprecated API'
  • ✅ Installed extension from web, configured and checked its working, uninstalled the extension, checked is gone
  • ✅ Installed extension with upload package file, configured and checked its working, uninstalled the extension, checked is gone
  • ✅ Installed extension from folder, configured and checked its working, uninstalled the extension, checked is gone
  • ❌ Installed extension from URL, configured and checked its working, uninstalled the extension, checked is gone
    • Error message
      php cli/joomla.php extension:install --path ~/Downloads/mod_zitat_service_de_2.0.3.zip
      In ExtensionInstallCommand.php line 73:
      
      Too few arguments to function Joomla\CMS\Console\ExtensionInstallCommand::__construct(), 0 passed in /Users/hlu/Desktop/no_backup/joomla-cms/60/libraries/src/Service/Provider/Console.php on line 160 and exactly 1 expected
      ```
      
  • php cli/joomla.php extension:install --url https://github.com/muhme/quote_joomla/releases/download/2.0.3/mod_zitat_service_de_2.0.3.zip

@Hackwar
Copy link
Member Author

Hackwar commented May 31, 2025

I fixed the issue. Could you please test again?

@muhme
Copy link
Contributor

muhme commented Jun 1, 2025

I have tested this item ✅ successfully on c4bba80


Re-Tested with fresh 6.0-dev branch plus gh pr checkout 43792 in local installation on macOS, PHP 8.4.5 with module zitat-service.de

  • Set Logging 'Log Almost Everything' and 'Log Deprecated API', both 'Behaviour - Backward Compatibility' plugins are disabled
  • ✅ Installed extension from web, configured the Site-module and checked its working, uninstalled the extension, checked is gone
  • ✅ Installed extension with upload package file, configured the Site-module and checked its working, uninstalled the extension, checked is gone
  • ✅ Installed extension from folder, configured the Site-module and checked its working, uninstalled the extension, checked is gone
  • ✅ Installed extension from URL, configured the Site-module and checked its working, uninstalled the extension, checked is gone
  • ✅ Installed extension with php cli/joomla.php extension:install --path ~/Downloads/mod_zitat_service_de_2.0.3.zip
    • [OK] Extension installed successfully.
    • Configured the Site-module and checked its working, uninstalled the extension, checked is gone
  • ✅ Installed extension with php cli/joomla.php extension:install --url https://github.com/muhme/quote_joomla/releases/download/2.0.3/mod_zitat_service_de_2.0.3.zip
    • [OK] Extension installed successfully.
    • Configured the Site-module and checked its working
  • ✅ Checked failed System Test phpmin-system-mysql, only one already known error 'PHP Warning: Undefined array key "status"', fix comes with 45547
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43792.

@Bodge-IT
Copy link
Contributor

Bodge-IT commented Jun 6, 2025

I have tested this item ✅ successfully on 870546b

Tested and works both ways.


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

@richard67
Copy link
Member

I've restored @muhme 's test result in the issue tracker because the commit which has invalidated the human tests count was just a clean branch update.

@Bodge-IT Please, when doing branch updates which are clean, i.e. not had conflicts to be resolved, then restore any previous human tests in the issue tracker by using the "Alter test" button.

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 10, 2025
@Bodge-IT Bodge-IT merged commit bb33139 into joomla:6.0-dev Jun 11, 2025
4 of 5 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 11, 2025
@Bodge-IT
Copy link
Contributor

Thanks for the persistence on this @Hackwar and to Heiko for testing so thoroughly

@Bodge-IT Bodge-IT added this to the Joomla! 6.0.0 milestone Jun 11, 2025
@Hackwar
Copy link
Member Author

Hackwar commented Jun 11, 2025

Thank you!!!

@Hackwar Hackwar deleted the 6.0-installer-adapter branch June 11, 2025 08:09
*
* @throws \InvalidArgumentException
* @since 3.4
* @deprecated __DEPLOY_VERSION__ will be removed in 7.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've just noticed that the deprecation message here is wrong. If we deprecate this in 6.0, it can not be removed before 8.0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method comes from the adapter class and has been deprecated there since 1.5. It is now copied over here and directly deprecated. That should be fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then the deprecation comment should be adapted. Not sure though how a "since 3.4" fit to "deprecated 1.5".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the `since 3.4 is wrong and comes from incorrect modifications at that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b/c break This item changes the behavior in an incompatible why. HEADS UP Documentation Required Feature PR-6.0-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants