[#27991] - Add Package preparePreUpdate handler
- Closed
- 13 Mar 2020
- Medium
- Build: staging
- # 27991
- Diff
- Ruud68:Package-preparePreUpdate
User tests: Successful: Unsuccessful:
Pull Request for improved / added functionality
Summary of Changes
the Joomla Updater has a preparePreUpdate method that can handle updates to components, modules and plugins. This method can be used to for example dynamically add a download key to the download URL for the updated component, module, plugin.
This change extends the functionality to also be able to handle updating of packages as this is currently missing. Recommendation here is that the package holds a component with the same name as the package (so pkg_myextension holds com_myextension). The prepareUpdate function in the component will then be called to handle the preparePreUpdate actions needed for the package.
Testing Instructions
Update a package where a download ID needs to be added to the download URL
Expected result
The download ID is added via the page component helper function prepareUpdate and the download (and update) is successful
Actual result
The package update doesn't call the helper function prepareUpdate and the download and update is unsuccessful as the required download id is missing.
Documentation Changes Required
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_installer |
As this would be classed as a new feature it would need to be rebased on joomla 4 as we are not accepting new features on staging
@mbabker good point I will see if I can 'hook' it up in something package related other then a component
@alikon, the download key is an example of a use case, but not exclusively what this change is about: the j4download manager is not suited for other things then downloadkeys. Just as the component, also the package needs a preupdate handler in J3 and in j4.
@brianteeman, I understand but I find it hard to make these decisions on a non existing plan for J4. With the risk of having no (foreseeable) J4 and an outdated J3. Not everybody I know off is eagerly awaiting to move to J4.
This is NOT a b/c change and as such could definitely go into J3 and into J4
I didnt make the policy but the policy is clear.
@alikon, thanks for pointing that out, I had seen that PR some (very long) time ago, had to move on since then doing what this PR is hoping to prevent :(
@brianteeman, I understand, but I also know that a policy is to facilitate and give clearity based on direction and planning at the time it is drafted. Just like you cannot merge an old PR a policy also needs to rebase :)
yeah that pr originally comes from a GSOC 2017 work......
but keep pushing never surrender
Hmm i tend to agree with @mbabker here. To me a package is a logical collection of components, plugins and modules. When you have more than one of them i think the cleanest thing would be an installer plugin as the usual package itself does not have a helper class (or any php code) and this suggestion here would only apply to packages with components but would fail on a package only existing with modules.
What does others think?
Hi, so I have been looking at other ways to somehow trigger a method in preparePreUpdate. Although a package can have a [package].script.php file in the manifest, that [package].script.php is only executed on actual install / update: so that is only available AFTER downloading the package.
The only things installed (and therefore available before downloading a package) are components, module, plugin in the package. Not the [package].script.php file itself. So that is a dead-end.
So the only way i currently see this working is either via a component in the package (as i proposed), or via a installer plugin
@HLeithner @zero-24 Can you make a decision if this new feature can go in j3 please
fyi: this is not breaking B/C. it just adds functionality. Also when this needs to go into J4 it will automatically be back-ported into J3.10 (if I understand it correctly), so adding it into J3.9 will automatically include in into J4 as not all J3.9 versions have been synced into J4.
also when this needs to go into J4 it will automatically be back-ported into J3.10 (if I understand it correctly)
No that is not correct
@Ruud68 is there a reason why you don't update the components/plugin/modules in the package separately? And yes it will not go into 3.x series, I think for j4 it could make sense for consistent but should be it's own package file and not a random file of an installed extension which maybe is part or is not a part of the package. Also it's possible that one package has multiple components or only plugins...
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-03-13 11:06:23 |
| Closed_By | ⇒ | HLeithner | |
| Labels |
Added:
?
|
||
Yes there is a reason: the reason them also being in a package rather then distributed stand-alone: they have e.g. dependencies on each other so updating one part of the package can and will lead to issues. That is what a package is for IMO.
It cannot be part of the package file as nothing of the package file is installed (and nothing can be called). You can do this by introducing an additional installer plugin. This PR gives another option WITHOUT creating yet another 'bloated' plugin that has only one use case. It is the responsibility of the method called in the component to check if it is the components package that is updated.
This is the same for plugins that add download keys: all plugins respond to every update of every extension / module / plugin > it is up to the plugin to add the correct download key to the correct update. No difference.
If you develop extensions and distribute packages then this PR is adding value, I know... I'm an extension developer and I have developed this together with an other developer who ran into the same 'issue' :)
The alternative is to create my own update logic (like some extension developers are doing: now why would they be doing that?) I thought more people would benefit from this PR then only me when I create my own update logic.
Just my 2cts...
Anyway the PR is closed already, so what is the point. What is the correct way IMO to handle PR's (and issues) is to only have the person who opened the PR / issue be able to close it. Asking a question and in the same time closing the PR is useless and to be honest frustrating to feel: it feels like talking to a closed door.
again, just my two cents. a small change in behavior that will bring this project a lot further :)
An issue and an PR can always be reopened so closing this shouldn't be a problem, maybe it looks a bit rude but better to close a PR that will not be merged (and can be discussed further) then leave every PR open for ever..
Also the solution in J4 should solve your problem or is this wrong?
The other thing is that we try to reduce magically called functions, so plugin or service or a defined function in the xml would be a better solution.
Thanks for the explanation.
If this is what you are referring to: #25553 well as said earlier in this thread, at that time (2017) that might have been a 'killer' feature, but since then the world has moved on.
My PR is NOT only about adding a download key, it is more.
With this PR I am for example able to validate the download key and give useful information to the user about why the upgrade is failing (just as an example) opposed to the current generic error.
And this is one of the reason I see a problem. if you have 10 paid extensions all try to validate the key on update and need 3 seconds we have a good chance to run in a timeout.
As said you cannot stop that as it is already happening with the current available code. Only packages are not following that route. components, modules, plugins work just fine.
I get a lot of support messages from people asking me what is wrong with the update mechanism: either their key expired, they didn't put one in or they did but had to do a rebuild (erasing all the keys), or the key is expired. Or they put in a new key, but because of caching the old one was still used. So they needed to clear the update cache and click 'find new updates' which then removes the keys from the table, but also has them run into site timing out getting the updates.... (I myself once ran into a corrupt database due to a 504 time out when getting the updates) This is not good user experience.
This is feedback I get as developer and although I understand why it is not working from a technical perspective, I do not understand why it is not working from a user perspective.
J4 is not going to improve here so there is an opportunity.
templates and libraries have the same problem then. So if you propose a good solution for j4 and can implement it we have something we can talk about.
Correct these probably have the same issue. I'm not on the J4 bandwagon but will look if i can 'rally' some Joomla developers in my network to brainstorm on this. Not feeling to positive about this though as most I know have left or are leaving for 'the greener pastures'.
You can close this PR :))) will implement for now by extending a system plugin in those packages (workaround).
It shouldn’t be assumed a package has a component or that a component is required to add functionality like this to a package (there is a reason packages are able to have their own update scripts, any kind of add-on functionality should not mandate that a feature can only work if a package is structured a specific way).