Skip to content

[5.3] Fix core update information retrieval after changing the update channel or stability options #44954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 1, 2025

Conversation

SniperSister
Copy link
Contributor

@SniperSister SniperSister commented Feb 22, 2025

Summary of Changes

In the pre-TUF era, each and every update channel and event some of the stability options in the config of com_joomlaupdate had it's own update XML. After switching that the XML, the existing update information in the database had to be purged before new information could be fetched. That was done by calling the "applyUpdateSite" method in the DisplayController of com_joomlaupdate because that controller task would be shown after the config has been changed.

That approach has two flaws:

  1. it only works in a "user does stuff in backend" mode as the logic resides in the display controller; config changes via the API will not trigger the purge
  2. TUF has one repo URL for the channels and stability options - so the current code fails to recognize these changes and shows the wrong updates.

This PR now adds a new plugin that listens for config save events of com_joomlaupdate, applies the new config and always purges the now outdated information. That fixes both issues.

Testing Instructions

  • set the EXTRA_VERSION constant in libraries/src/Version.php to "alpha-4" - that's necessary for the test because otherwise the default update channel will not show any updates
  • browse to "options" of the core update component and set the update channel to "default"
  • hit save & close
  • Update for 5.3.0-beta will be offered
  • change the channel to "custom" and use https://artifacts.joomla.org/drone/joomla/joomla-cms/5.3-dev/44954/downloads/82486/pr_list.xml as custom url
  • hit save & close
  • switch back to default
  • hit save & close

Actual result BEFORE applying this Pull Request

Update information is not fetched correctly, showing available updates for the "previous" channel

Expected result AFTER applying this Pull Request

Correct information is shown

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

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-5.3-dev labels Feb 22, 2025
@SniperSister
Copy link
Contributor Author

Fixed, thanks @brianteeman

@brianteeman
Copy link
Contributor

why is this a plugin and not part of the component.?
as a plugin it could be disabled but for what benefit?

And if it is to be a plugin then it needs to be added to the list of core extensions libraries\src\Extension\ExtensionHelper.php

@SniperSister
Copy link
Contributor Author

why is this a plugin and not part of the component.?

Because the action, that is relevant for the purge of the updates, does not happen in com_joomlaupdate but in com_config; and com_config triggers plugin events.

And if it is to be a plugin then it needs to be added to the list of core extensions libraries\src\Extension\ExtensionHelper.php

Good catch, done!

@webgras
Copy link
Contributor

webgras commented Feb 22, 2025

For testing: do I need to be on the latest version (5.2.4) or i.e. on 5.2.3?

@SniperSister
Copy link
Contributor Author

For testing: do I need to be on the latest version (5.2.4) or i.e. on 5.2.3?

The provided test instruction don't rely on the actual retrieval of update information but on the execution of "proof statement". So, for these instructions the version does not matter.

@webgras
Copy link
Contributor

webgras commented Feb 22, 2025

I have tested this item ✅ successfully on 257002e

For testing you need to be on a current version, otherwise you will not see the "Check for Updates" button and you cannot test.
But on the latest version, I tested successfully and saw the "purge triggered" message


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

@webgras
Copy link
Contributor

webgras commented Feb 22, 2025

I tested on localhost with MariaDB


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

@komalm
Copy link

komalm commented Feb 22, 2025

I have tested this item 🔴 unsuccessfully on 257002e

Steps Followed for Testing:

  • Applied the patch using Joomla! Patch Tester.
  • Edited UpdateModel.php and added die("purge triggered"); to verify the purge execution.
  • Navigated to System > Update > Joomla! Update.
  • Changed the Minimum Stability option in the Update settings.
  • Clicked Save & Close, expecting the purge process to trigger.

Observed Behavior:

  • No confirmation of the purge trigger.
  • The expected output (purge triggered) was not displayed.

Expected Behavior (Not Achieved):

@SniperSister
Copy link
Contributor Author

@komalm have you installed and activated the new plugin before performing the test? applying the patch alone will not be sufficient.

@richard67
Copy link
Member

@SniperSister The testing instructions tell to toggle the minimum stability. But they don't tell to check that the purge is also triggered by changing the update channel without and with the PR, i.e. that this still works with the PR. I think the instructions should also cover that case.

@richard67
Copy link
Member

richard67 commented Feb 23, 2025

@SniperSister There is something wrong when changing the update channel with this PR applied. I've done a real test with the version number patched to 5.3.0-alpha4. When I use the custom URL of this PR with minimum stability = stable, then the update to this PR is found. That's right because the PR builds are tagged as stable in the custom update XML. Then I switch back to the default channel where nothing should be found. But it still shows the update from the custom URL, and the URL of the update site in database is still the custom URL.

Hint: To patch the version to alpha4 I've edited files administrator/manifests/files/joomla.xml and libraries/src/Version.php and then used the database fixer to apply the patched version.

@richard67
Copy link
Member

I have tested this item 🔴 unsuccessfully on 4eb1439

Details see my previous comment. Maybe we need a similar fix to #44951 regarding the kind of task?


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

@richard67
Copy link
Member

Or maybe the plugin is listening to the wrong event? When I add some error log for debugging, it shows that the purge is triggered, but the values of $updateURL and $updateType are those for the old value and not the new value of the update site.

@SniperSister
Copy link
Contributor Author

@richard67 stupid question: plugin was installed and enabled during the test?

@richard67
Copy link
Member

richard67 commented Feb 23, 2025

@richard67 stupid question: plugin was installed and enabled during the test?

Sure. I have updated a 5.3-dev installation to the custom update URL of this PR to see if your update SQL works. Then I have checked that the plugin is installed and enabled. Then I have patched the version to 5.3.0-alpha4 like I described in the comment before my test result. Then I have tested.

Instead of a die, I did use error_log for producing debug output to my error log at the place where you said the die should be added.

I've still had the custom URL of the PR entered, minimum stability was stable, and I only changed the update channel from "Custom URL" to "Default" and vice versa and saved after each change.

My debug log has shown that the purge always gets triggered one time, but the values of $updateURL and $updateType are those for the channel before the channel change and save, i.e when I changed from "Custom URL" to "Default", the update type in the log was "Collection" and the URL was the one of the PR, and when I changed from "Default" to "Custom URL" the update type was TUF and the URL was the default one.

@richard67
Copy link
Member

richard67 commented Feb 23, 2025

@SniperSister This is the debug code which I've used in the applyUpdateSite method of the UpdateModel, just at the end after the $db->execute();:

        // DEBUG
        error_log('DEBUG: Purge triggered');
        error_log('DEBUG: $updateURL="' . $updateURL . '"');
        error_log('DEBUG: $updateType="' . $updateType . '"');

The 2 variables are determined from the params gotten with the statement at the top of the method in line :

$params = ComponentHelper::getParams('com_joomlaupdate');

switch ($params->get('updatesource', 'default')) {
...

Maybe this still gets the old values from before save?

@SniperSister
Copy link
Contributor Author

@richard67 I was able to reproduce your finding and have update the PR and the test instructions accordingly

@webgras a re-test would be much appreciated!

@richard67
Copy link
Member

I have tested this item ✅ successfully on 9498976

In addition to the testing instructions, I've also tested that it works when the update channel is changed with the CLI, and that the update SQL scripts are working. The latter was tested with MySQL and PostgreSQL.


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

@tecpromotion
Copy link
Contributor

I have tested this item ✅ successfully on 9498976


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

@tecpromotion
Copy link
Contributor

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 1, 2025
@rdeutz rdeutz merged commit 2c859aa into joomla:5.3-dev Mar 1, 2025
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 1, 2025
@richard67 richard67 added this to the Joomla! 5.3.0 milestone Mar 1, 2025
@tecpromotion tecpromotion mentioned this pull request Mar 2, 2025
4 tasks
Kostelano added a commit to JPathRu/localisation that referenced this pull request Apr 14, 2025
joomla/joomla-cms#41496 - (upmerge с 5.2х)
joomla/joomla-cms#42530 +
joomla/joomla-cms#43994 - (upmerge с 5.2х)
joomla/joomla-cms#44009 - (upmerge с 5.2х)
joomla/joomla-cms#44010 - (upmerge с 5.2х)
joomla/joomla-cms#44161 +
joomla/joomla-cms#44187 - (upmerge с 5.2х)
joomla/joomla-cms#44207 - (upmerge с 5.2х)
joomla/joomla-cms#44271 +
joomla/joomla-cms#44273 +
joomla/joomla-cms#44288 - (только для en-GB)
joomla/joomla-cms#44348 - (upmerge с 5.2х)
joomla/joomla-cms#44366 +
joomla/joomla-cms#44367 - (upmerge с 5.2х)
joomla/joomla-cms#44434 - (upmerge с 5.2х)
joomla/joomla-cms#44448 - (upmerge с 5.2х)
joomla/joomla-cms#44462 +
joomla/joomla-cms#44487 - (upmerge с 5.2х)
joomla/joomla-cms#44587 +
joomla/joomla-cms#44600 +
joomla/joomla-cms#44604 +
joomla/joomla-cms#44621 - (upmerge с 5.2х)
joomla/joomla-cms#44623 +
joomla/joomla-cms#44632 +
joomla/joomla-cms#44640 - (позже был REVERT joomla/joomla-cms#44845)
joomla/joomla-cms#44714 - (upmerge с 5.2х)
joomla/joomla-cms#44756 +
joomla/joomla-cms#44768 - (upmerge с 5.2х)
joomla/joomla-cms#44792 - (только для en-GB)
joomla/joomla-cms#44813 +
joomla/joomla-cms#44822 - (upmerge с 5.2х)
joomla/joomla-cms#44839 +
joomla/joomla-cms#44871 +
joomla/joomla-cms#44954 +
joomla/joomla-cms#45034 - (upmerge с 5.2х)
joomla/joomla-cms#45058 - (только для en-GB)
joomla/joomla-cms#45064 +
joomla/joomla-cms#45078 - (только для en-GB)
joomla/joomla-cms#45130 - (upmerge с 5.2х)
joomla/joomla-cms#45240 - (upmerge с 5.2х)
joomla/joomla-cms#45246 - (только для др. пакетов)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators PR-5.3-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants