[#46054] - [6.1] Improvements to extension updater
- Pending
- Medium
- Build: 6.1-dev
- # 46054
- Diff
- BrainforgeUK:6.1-dev
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
Added event handler response checks for:
- onInstallerBeforePackageDownload
- onInstallerBeforeUpdateSiteDownload
Added events:
- AfterPackageDownloadEvent
- AfterPackageDownloadFailedEvent
Testing Instructions
-
Install these extensions - no need to enable them.
plg_test_test1v1.zip
plg_test_test2v1.zip -
Go to check for updates.
v2.0.0 of these will be available.
plg_test_test1
plg_test_test2 -
Update plg_test_test1
Works fine, updated to v2.0.0 -
Update plg_test_test2
- Before this pull request .. 403 response error message.
- After this pull request ... get subscription message from the update server.
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
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:
-
No documentation changes for manual.joomla.org needed
https://manual.joomla.org/docs/building-extensions/install-update/update-server/ will need to be updated along the following lines:
- Update servers can now return JSON containing a message and other information.
- The event listeners provide additional flexibility for developers who wish to make use of them.
Here is an example of a generic installer plugin which provides similar functionality for update servers which don't provide a JSON response. It also attempts to detect an update site which returns a HTML document and not a ZIP!
plg_installer_generic.zip
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_installer Libraries |
| Labels |
Added:
PR-6.1-dev
|
||
| Title |
|
||||||
| Title |
|
||||||
| Labels |
Added:
Feature
|
||
@BrainforgeUK Three more suggestions, then I think PHPCS should be ok.
in case you missed it: #46045 (comment)
@BrainforgeUK Code style is ok now. But PHPStan reports errors. You can see them when checking the changed files on GitHub https://github.com/joomla/joomla-cms/pull/46054/files .
@BrainforgeUK It is best practice to create a pr in a branch of your fork. This way it is easier for you to create multiple independent PR to the same branch of Joomla. Just a note for the future
Re - in case you missed it: #46045 (comment)
I agree requiring an extension developer to write installer plugin(s) in order to handle special events returned by their update server is limiting. Hence I have now provided the capability for the update server to return the package in JSON data together with messages and any other useful information.
The Joomla! code changes I propose provide basic handling for this JSON data. If they wish an extension developer can still capture after events and process them in any way they wish. Handling of existing update servers is unchanged. Above I have provided plg_installer_generic.zip which illustrates how these events can be handled.
I called it generic with the view that it may form the basis of something which could be shipped with Joomla! core and enabled or replaced with something developer specific as need arises. Although if, in the unlikely event that, all update servers respond with JSON data such a generic event handler would be redundant!
If the JSON handling part of this proposal is rejected then please leave the events in as I have a use case for them.
Re: https://github.com/joomla/joomla-cms/pull/46054#issuecomment-3271070728
What should I be doing about the PHPstan error - eventawareinterface ?
Re: https://github.com/joomla/joomla-cms/pull/46054#issuecomment-3271070728
What should I be doing about the PHPstan error - eventawareinterface ?
@BrainforgeUK I don’t know the right solution.
@Fedik Could you advise?
Can ignore that, $app->getDispatcher() will stay. The deprecation is triggered for EventAwareInterface wich also have this method. I think PHPstan got confused.
Aside that, the PR have many other code issues. But before I start complaining we need to agree if this PR is needed. Maybe in some other form as @Ruud68 commented.
I have responded to @Ruud68 comments above.
https://github.com/joomla/joomla-cms/pull/46054#issuecomment-3271808514
Re - in case you missed it: #46045 (comment)
I agree requiring an extension developer to write installer plugin(s) in order to handle special events returned by their update server is limiting. Hence I have now provided the capability for the update server to return the package in JSON data together with messages and any other useful information.
The Joomla! code changes I propose provide basic handling for this JSON data. If they wish an extension developer can still capture after events and process them in any way they wish. Handling of existing update servers is unchanged. Above I have provided plg_installer_generic.zip which illustrates how these events can be handled.
I called it generic with the view that it may form the basis of something which could be shipped with Joomla! core and enabled or replaced with something developer specific as need arises. Although if, in the unlikely event that, all update servers respond with JSON data such a generic event handler would be redundant!
If the JSON handling part of this proposal is rejected then please leave the events in as I have a use case for them.
Thanks for your detailed response. My question was based on your first PR but I see now, walking through the code, that you have altered it, I think this is indeed an improvement and adds value not only to extension developers, but also to 'customers' giving them more detailed information on why a download didn't succeed.
Aside that, the PR have many other code issues. But before I start complaining we need to agree if this PR is needed. Maybe in some other form as @Ruud68 commented. (@Fedik )
One of my extensions is for selling subscriptions and providing downloads using download keys acting as a update server.
I think this is, from an end-user perspective, a valuable functionality. But that said: so I thought where the introduced change logs, but there are not many update servers providing these. When I look at all the extensions I have installed from different suppliers, only my own provide the change logs: others provide that information in different ways or not at all.
but even with that taken into consideration I would say: go!
Thanks Ruud68 for your comments. I am obviously 'small fry'! This pull request came out of an overdue upgrade to my update server and thoughts on where a project in the pipeline might go ...
@BrainforgeUK Could you apply my change suggestions to fix PHPCS? It can be easily done by going to the changed files of your PR on GitHub here https://github.com/joomla/joomla-cms/pull/46054/files and for each suggestion using the button to add it to a batch, and then apply that batch with the button at the top.