Skip to content
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

[4.0] Upgrade from j3 to J4 sql updates not run #30913

Closed
brianteeman opened this issue Oct 4, 2020 · 86 comments
Closed

[4.0] Upgrade from j3 to J4 sql updates not run #30913

brianteeman opened this issue Oct 4, 2020 · 86 comments

Comments

@brianteeman
Copy link
Contributor

brianteeman commented Oct 4, 2020

Steps to reproduce the issue

I followed the upgrade procedure as documented here https://www.joomla.org/announcements/release-news/5822-joomla-4-0-0-beta4-and-joomla-3-10-alpha2.html

  • Go to the Joomla Update Component Options and switch the Update Channel to “Testing” and the Minimum Stability option to “Alpha”.
  • Update to 3.10 alpha2 using the Joomla Update Component.
  • After you have updated to 3.10 alpha 2 please switch the Update Channel to “Custom URL” and set the Custom URL to https://update.joomla.org/core/test/310to4_list.xml
  • Now you get the update to the latest 4.0 beta release.

Expected result

Site updated with perhaps a few errors to resolve

Actual result

500 error
Table 'brian_db1.ayle_workflow_associations' doesn't exist Table 'brian_db1.ayle_workflow_associations' doesn't exist

Checked the database and NONE of the workflows tables are present

Further checks show that none of the administrator\components\com_admin\sql\updates\mysql\ sql scripts appear to have run

Database Version | 5.5.5-10.5.5-MariaDB
PHP Version | 7.3.23

-- | --

@brianteeman brianteeman changed the title [4.0] Upgrade from j3 sql workflow error [4.0] Upgrade from j3 to J4 sql updates not run Oct 4, 2020
@richard67 richard67 self-assigned this Oct 4, 2020
@richard67
Copy link
Member

@brianteeman I can't reproduce that here, starting with current staging (and the version patched from "3.9.22-rc" to "3.9.22-dev", because there seems to be another issue with version comparison when it contains "-rc", so the update to 3.10-alpha2 wasn't found, but that's another issue).

So I need more info:

  • What was your starting point? Staging branch on a git clone, or a 3.9.21 installation?
  • Server OS (I tested with Linux, don't have a server on Windows here)
  • DB type and version: MySQL 8? Or pre 8? Or MariaDB?

@brianteeman
Copy link
Contributor Author

@richard67 I think I have worked out the cause. Give me 10 minutes to confirm

@richard67
Copy link
Member

I give you as many minutes as you need ;-)

@brianteeman
Copy link
Contributor Author

OK so I managed to get further this time and I can see what is happening and what we need to fix.

The problem is with isSite and isAdmin which have been REMOVED in favour of isClient('site') and isClient('admin')

If you have a system plugin that uses those then it kills your site BEFORE the final stages of the update.

Normally you would do the update and then the site logs you out and you have to log back in and then enter your details again to complete the process.

The problem is that if you have a system plugin using either of those methods then the upgrade process dies at that point and there is no way to continue and you have a completely bricked web site

The isSite and isAdmin are VERY widely used and if we dont do something about this then its bye bye joomla.

You couldnt replicate it because you were testing with just core extensions.

It is a known issue https://docs.joomla.org/Potential_backward_compatibility_issues_in_Joomla_4#CMSApplication

It has been reported many many times https://github.com/joomla/joomla-cms/search?q=isSite&type=issues but I dont think anyone has reported before that it breaks upgrades and kills the sites. Just that a specific extension or template needed updating.

@richard67 richard67 removed their assignment Oct 4, 2020
@richard67
Copy link
Member

Unassigned me because it's not a database issue. But I agree it has to be solved.

@brianteeman
Copy link
Contributor Author

Can it please be marked as a release blocker

@wilsonge
Copy link
Contributor

wilsonge commented Oct 4, 2020

The problem is that these functions specifically will not function as expected with the new console and api applications because they were largely used in the context of

if ($app->isAdmin()) { // stuff } else { // site stuff }

Or vice versa. I saw that pattern used lots (and even used it myself once upon a time)

@brianteeman
Copy link
Contributor Author

We've ignored these reports since the change was made. Always blaming the developer for using a deprecated function and telling people just to a search and replace.

We can say its been deprecated for a few years as much as we want (I've said it myself) but when your site is dead you're not interested in assigning blame - you just want your site to work.

When it is in a system plugin then it kills the update and leaves the site in an unusable state that can only be recovered by starting again or manually running each sql script in the com_admin/ directory to even get to a state where the search and replace will be useful. That is assuming that you even know that you have to run those 72 sql scripts manually and know how to do it.

I suggested it before "why cant the old code be aliased to the new code" if the removal of the code really cant be reverted.

Finally - we can't even always blame the developer. Until recently the user profile plugin used this code and users were actively encourage to fork that plugin and use it as an example. I doubt that anyone who did that realised that the code needed to be changed. I know I didnt!

@Fedik
Copy link
Member

Fedik commented Oct 4, 2020

@wilsonge suggestion:
Create isSite()/isAdmin() only for SiteApplication and AdministratorApplication (and mark them depreciated)

In this way an old extensions will continue to work, and new extensions (which use Api/Cli Application) must use new method isClient() from CMSApplication

@zero-24
Copy link
Contributor

zero-24 commented Oct 4, 2020

@brianteeman just a question not to blame anyone but what did the pre upgrade checker said for that extension? Has the Developer declared that extension to be 4.x compatible or not?

@zero-24
Copy link
Contributor

zero-24 commented Oct 4, 2020

@wilsonge suggestion:
Create isSite()/isAdmin() only for SiteApplication and AdministratorApplication (and mark them depreciated)

In this way an old extensions will continue to work, and new extensions (which use Api/Cli Application) must use new method isClient() from CMSApplication

Hmm they are deprecated for a longer time already. I'm not sure whether another new method to deprecated them helps here.

The problem is that when that plugin is triggerd by an cli app and it uses isAdmin or isSite that would fail the cli app right as this would try to call an not defined method. Or i'm missing something?

@brianteeman
Copy link
Contributor Author

No the pre-update check didnt say anything.

Just to remind you this is NOT about the offending code spitting out an error message or not working. That is easy to resolve. The problem is that this problem kills the update process leaving you with a partially updated site that is completely unusable and can only be recovered by completing deleting and reinstalling a backup.

@zero-24
Copy link
Contributor

zero-24 commented Oct 4, 2020

No the pre-update check didnt say anything.

Just to remind you this is NOT about the offending code spitting out an error message or not working. That is easy to resolve. The problem is that this problem kills the update process leaving you with a partially updated site that is completely unusable and can only be recovered by completing deleting and reinstalling a backup.

Hmm can you send me the plugin in question? It should atleast say something.

I get the thing with the broken update. But that is how joomla works right now. We update the CMS with the same application that also runs other parts so all system events are triggered too. In an ideal world we would have an dedicated update app that would trigger dedicated plugin events only used in the updater.

I'm not sure how we could solve that as all other breaking changes introduced in 4.x could result into similiar issues when used in plugins.

Even an 'disable broken extensions' thing like Wordpress did would not help here i guess as it first need to fail in order to act and at this point the update is already broken.

And I'm not sure whether it is wise to make an conditional to all system plugin events that they should not run in the update process.

So i'm a bit out of ideas how to solve that other than in the extension itself.

@brianteeman
Copy link
Contributor Author

sorry it did says something - the "no information available" message

This is not a case of a broken extension - we can live with that. This is a completely destroyed and unrecoverable web site

It is easy to test and see what happens. Just create a system plugin with code like this and do the update

	function onAfterRoute()
	{

		$app = JFactory::getApplication();

		if ( $app->isAdmin() )
		{
			return;
		}
	}

So i'm a bit out of ideas how to solve that other than in the extension itself.

Revert the removal of the code

@HLeithner
Copy link
Member

if the extension is not ready for j4 you should not update your site.

@zero-24
Copy link
Contributor

zero-24 commented Oct 4, 2020

sorry it did says something - the "no information available" message

Yes and that means that extension should be carefully checked upfront the Upgrade right?

This is not a case of a broken extension - we can live with that. This is a completely destroyed and unrecoverable web site

It is easy to test and see what happens. Just create a system plugin with code like this and do the update

	function onAfterRoute()
	{

		$app = JFactory::getApplication();

		if ( $app->isAdmin() )
		{
			return;
		}
	}

I know i just wanted the plugin to check why it did not show up in the pre Upgrade checker.

Btw you can break your site by calling any removed or broken code at that point even when you Upgrade from 3.9.21 to 3.9.22 (well there we have no sql stuff so you might be save here).

So i'm a bit out of ideas how to solve that other than in the extension itself.

Revert the removal of the code

I'm not sure whether that is a good solution too as mention by george. By that argument we have to revert all breaking changes out of 4.x as that might break the site on upgrade.

We might have to add just another more and bigger warning when system plugins are not compatible or no info is there?

Or we might disable extensions not compatible? That could break the site to but 'maybe' just the frontend and hopefully the backend still works?

We could also add just another warning and confirmation once you are upgrading when incompatible extensions are detected?

@brianteeman
Copy link
Contributor Author

if the extension is not ready for j4 you should not update your site.

Of course not. That was NOT the case here. This was one of the million extensions that provided no update information

@zero-24
Copy link
Contributor

zero-24 commented Oct 4, 2020

if the extension is not ready for j4 you should not update your site.

Of course not. That was NOT the case here. This was one of the million extensions that provided no update information

Any suggestion how to handle that? (other than reverting all breaking changes out of 4.x) Maybe one of my suggestions above can help with that? So special extensions like system plugins get a dedicated notice once found incompatibel or no info?

@Fedik
Copy link
Member

Fedik commented Oct 4, 2020

The problem is that when that plugin is triggerd by an cli app and it uses isAdmin or isSite that would fail the cli app right as this would try to call an not defined method.

That makes sense :)

Another idea, more fancy, but should keep us on safe side.
After compatibility check and before actually the upgrade:
The script store a map of enabled core/non-core extensions => turn off all non core extensions => make upgrade => restore disabled extensions.

With this we can be sure that 3d extension will not crash the upgrade (even if it say "it is compatible")

@HLeithner
Copy link
Member

if the extension is not ready for j4 you should not update your site.

Of course not. That was NOT the case here. This was one of the million extensions that provided no update information

ok then this screen is missleading, if we hvae no information to a component then this component has to be marked as incompatible.

I tested one of my own components and ask my self which version the pre-update checker shows:
image

whats version 1.3.2 and why is it compatible?

after switching to https://update.joomla.org/core/test/310to4_list.xml it shows a big red block saying no update information available....

image

I would suggest to make a more explicit warning for major versions with known b/c breaks. If any extension is not marked as compatible don't let the user update without explicit confirmation.

@HLeithner
Copy link
Member

The problem is that when that plugin is triggerd by an cli app and it uses isAdmin or isSite that would fail the cli app right as this would try to call an not defined method.

That makes sense :)

Another idea, more fancy, but should keep us on safe side.
After compatibility check and before actually the upgrade:
The script store a map of enabled core/non-core extensions => turn off all non core extensions => make upgrade => restore disabled extensions.

With this we can be sure that 3d extension will not crash the upgrade (even if it say "it is compatible")

sadly that will not work, because there are badly written plugins that thinks that the __construct function is a good place to execute code and joomla can't disabled this...

@Fedik
Copy link
Member

Fedik commented Oct 4, 2020

sadly that will not work, because there are badly written plugins that thinks that the __construct function...

hm, why? if extension is "disabled" it should not be even booted

Or can go even further, and turn off all extensions, leave only bare minimum, and then re-enable

@brianteeman
Copy link
Contributor Author

sorry it did says something - the "no information available" message

Yes and that means that extension should be carefully checked upfront the Upgrade right?

If it does mean that then it needs to say it - which it doesn't

This is the output of my test site with 50% extensions providing no information including several very well known ones

updatereport.pdf

This is just being plain stubborn to prove a point that joomla extensions are not made by real developers. We know for a fact that the isSite/isAdmin change on a system plugin (at least) will break the update. We have no choice but to fix it. We can not bury our head in the sand and blame others. We've had reports of this since the beginning.

@brianteeman
Copy link
Contributor Author

The problem is that when that plugin is triggerd by an cli app and it uses isAdmin or isSite that would fail the cli app right as this would try to call an not defined method.

That makes sense :)

That's a problem but it is not fatal. (ignoring the fact that a very small % will even use a cli app). If you get a failure with a message then you can react to it. This is a fatal error and would be the nail in the coffin for many joomla sites

@brianteeman
Copy link
Contributor Author

@alikon because you are thinking as a developer and not as a user/site owner AND because even if you were a developer you would not have had any deprecation notices in your logs about this.

Thanks for calling me stupid. Been called much much worse.

@brianteeman
Copy link
Contributor Author

@alikon look at the example from @nikosdion He has isSite in his code but he wrote it so that its not a problem. His other extension also had isSite in the code but it was a problem.

If both of those extensions did not yet have a j4 compatible flag are you really expecting a site owner to go through both those components and understand which occurrences they have to change and which ones they can leave alone.

I wouldn't have a clue

You are also assuming that everyone is using extensions from mass market developers. Very many people have code written for them so none of that code will have the J4 compatible flag and every line of that will need to be checked.

@brianteeman
Copy link
Contributor Author

@alikon Finally even you were submitting code like this to core #23335 despite it being flagged as deprecated #22683

@alikon
Copy link
Contributor

alikon commented Oct 5, 2020

#23335 was from the past Dec 26, 2018 some thing like 2 y ago isn't it?
but nevermind

i'm sorry but i cannot follow you, all your points are specious

this discussion is going nowhere imho

currently we have :

what do you want more ??

for a magic wand we are still working 😠

@brianteeman
Copy link
Contributor Author

It is going nowhere because you are not thinking like a user. My point of mentioning that PR was that it was created AFTER the deprecation. Even the author of the PR that removed the code in J4 uses it extensively in the very latest release of their software.

IF a user would see those docs AND
IF the deprecation was in the logs AND
IF the pre-update check said NOT compatible THEN
I agree with you

Clearly developers haven't seen those docs so we cant expexct users to see AND understand them so we fail at step 1
The deprecation isnt in the deprecation logs see #30926 so we fail at step 2

But fine keep sticking your head in the sand and ignoring the problem. If two very experienced users failed to update with the component then its a real problem. If you want to be responsible for the end of joomla fine but I dont.

@ceciogit
Copy link

ceciogit commented Oct 5, 2020

hi @nikosdion , far from me associate akeeba bkp to feces. are you serious? akeeba is an extension i love and use from years.
finally this is the prove my english sucks. lol. moreover because i had to be crazy answering to you telling your extension is sh*tty.. gosh..

i was refering to akeeba for the "backup b4 update" AND other sh*tty extensions that block the update. akeeba was not mean being part of feces plugins.

i am really sorry for misunderstanding.

@nikosdion
Copy link
Contributor

@ceciogit Thank you for your reply. I understand that it was just a misunderstanding because of the language barrier. Apologies for my reaction. I misunderstood you. Big hug.

@alikon Both you and @brianteeman are partly right and partly wrong. From a developer's perspective I understand why isSite and isAdmin had to be removed: we now have at least for different applications (site, admin, CLI and API) in Joomla, these two methods don't make sense in the CLI and API applications.

However, Brian is right in that the deprecation in SiteApplication and AdministratorApplication did NOT come with a deprecation notice in the log which is what Joomla recommends developers consult to detect deprecated features in their code! Moreover, using isSite/isAdmin is so ingrained to Joomla developers that it's hard to kick the habit. The proof is that the person who committed the obsolescence of these methods still uses them in his own production code even today. Therefore we need a programmatic solution to a. not break sites and b. gently inform developers they should start replacing these obsolete calls.

There is an elegant code solution. Create a Trait which implements isSite/isAdmin as shims to isClient and uses trigger_error to issue an E_USER_WARNING and logs the use to the deprecated log. The message in both latter cases should be "Using isSite() is deprecated and will remove. Use isClient('site') instead." You get the idea. Use this trait ONLY in SiteApplication and AdministratorApplication and mark it as @deprecated 5.0. This solution will prevent the bulk of migrations fail and gives adequate feedback to developers to fix their code. This should have been done in 3.8 but, hey, we can't travel back in time.

Yes, these system plugins will still break the CLI and API applications. Big effin' deal. The target audience of these applications is vert strongly correlated with the target audience of the deprecation notice. If you want to be overly pedantic create another Trait for these two applications which throws a RuntimeException when either method is called. This will provide even stronger feedback to the developers who will test their code with the new, non-HTML, non-web applications of Joomla 4.

The bottom line is that you do not need to shoot your feet by disabling system plugins on update (that's an insane non-solution which cause deeper and bigger problems). You don't need to rage-quit. You don't need to bicker forever over spilt milk. There are always elegant, obvious (or not-so-obvious), programmatic solutions.

@Fedik
Copy link
Member

Fedik commented Oct 6, 2020

Use this trait ONLY in SiteApplication and AdministratorApplication and mark it as deprecated

That what was suggested at beginning #30913 (comment)

do not need to shoot your feet by disabling system plugins on update

Why a plugins need to be enable at time when executed "update script" and "update SQL"? There nothing to do for them.
Joomla update have isolated script for "unpack" the files, but not for further critical steps. That why it so easy to explode.

However I think we already come to conclusion, that @HLeithner idea "to block the update if there an extension that cannot confirm compatibility" will be a good solution.

@richard67
Copy link
Member

However I think we already come to conclusion, that @HLeithner idea "to block the update if there an extension that cannot confirm compatibility" will be a good solution.

Hmm, correct me if I'm wrong, but as far as I remember the 4.0 compatibility information comes from the update server's XML file. If that is right, only extensions using update servers can confirm compatibility. Extensions which have to be installed or updated "manually" with zip upload will always be listed as with unknown compatibility status. Shall these block updating the core?

@Fedik
Copy link
Member

Fedik commented Oct 6, 2020

If that is right, only extensions using update servers can confirm compatibility.

hm, If it true, then we have one more issue that need to fix somehow :)
Because "self made" extensions will not have "update server".

Every extension XML have version="3.xx" thing:
<extension type="component" version="3.xx" method="upgrade">
Maybe we can use it somehow for local extensions, to allow something like version="3.10,4.0".
But I am not sure here.

@nikosdion
Copy link
Contributor

@Fedik Fair points and they do have reasonable answers 😄

Why a plugins need to be enable at time when executed "update script" and "update SQL"? There nothing to do for them.

Top reason off the top of my head: you are using a MySQL cluster which necessitates installing a custom database driver, only possible with a system plugin. Disabling the plugin would update the (meant to be read-only) slave but not the (meant to be write-only) master.

The other reason is that system plugins are loaded very early in the application boot, before you can run custom code. So in the failure mode that we are discussing here you'd have to disable these plugins in the database. However, this can only be done BEFORE you start the extraction. This means that an extraction problem or a post-install script failure would end up with your site's system plugins being permanently disabled. Let's say that magically this wasn't an issue. Where you'd store the IDs of the system plugins previously enabled? It can't be the session because there's no guarantee it will still exist at the end of the update (especially when you're moving to a new major version). You'd have to store them in the database and modify the post-installation script to use that information. This makes recovery from a failure possible but VERY HARD for the target audience. You could try to solve it by having the application always look for that information in the DB and enable system plugin the next time it boots up, before it looks for system plugins. However, this now opens an exploitable security hole. I can think of at least three fun ways I can hack your site with file-only access and you'd be none the wiser – my evil plugin would never show up enabled and you'd see no modified files.

Another solution which kinda helps is having the post-installation script NOT implement the application interface. This is problematic on many fronts, especially maintainability. This is EXACTLY what I did when I was upgrading my dev site. The worst part is that it doesn't solve the post-update problem which is also EXACTLY the problem I ran into. Let me tell you why this is bad. Joomla 4 implements AJAX login on the backend. Logging into my restored site failed consistently. I could not figure out why until I edited configuration.php to set error reporting to maximum AND enabled logging of PHP errors to a file in my php.ini. Then and only then could I see the exception in my PHP error log which pointed the finger to a 3PD plugin. There was sod all to help me on the browser. Just that the login failed. As a user my experience was that J4 was broken. As a developer I was left screaming "who the fuck thought this is a good idea?!".

So, why go through all that pain when a Trait would do? IMHO the simplest solution is always the best. Don't add a thick layer of obscurity which relies on institutional knowledge which will disappear when those of us participating in this discussion stop contributing for any reason. If the past 15 years taught us anything is that every time we implemented a convoluted solution it resulted in a clusterfsck two to four years down the line when institutional knowledge was lost due to volunteer turnover and some well-meaning volunteer committed something which made the whole thing blow up.

Regarding extensions without an update server, they should never be update blockers. They should simply be marked as unknown status which is the current state. Anything else will be perceived by users as a bug which makes updates impossible. They'll try to McGuyver the update with catastrophic results.

PS: Maybe y'all should try upgrading REAL WORLD sites to Joomla 4 to see what REAL WORLD users will experience. Y'all are trapped in testing clean installs. Yeah, sure, clean installs migrate (relatively) easily. Big deal. The devil is in the details and you need to take into account as many details as possible. Your users don't care if the problem comes from Joomla core or a 3PD extensions. They perceive Joomla 4 to be broken.

Dammit, people! I tried to tell you in person in January and almost nobody would listen. Users don't give a crap about whether you're using MVC, if you have middleware and shit like that. THEY USE WORDPRESS FOR CRYING OUT LOUD! Its code is a hot mess. They still use it because they don't have to think hard about updates the vast majority of time. When Joomla offers a subpar user experience they WILL go to a different CMS, code quality be damned. They just want to get shit done instead of battling with the software. It feels like I am screaming into the void again...

@Fedik
Copy link
Member

Fedik commented Oct 6, 2020

Okay, that makes sense. With some part I agree and some not, but this not for this topic.

Regarding extensions without an update server, they should never be update blockers.

While we talking @brianteeman made a concept #30930 :)

@brianteeman
Copy link
Contributor Author

brianteeman commented Oct 6, 2020

My concept would still be much much better with the code @nikosdion proposes which I have been trying to say in my non-developer language for almost a year #24642 (comment)

@alikon
Copy link
Contributor

alikon commented Oct 6, 2020

added some missing deprecations Log #30938

@GeraintEdwards
Copy link
Contributor

GeraintEdwards commented Oct 7, 2020

The trait suggested by @nikosdion would seem to be a good idea and complimentary to #30930 - the downside is that it only catches a couple of the possible problems that may arise. I just ran a test upgrade and found some code using JFactory::getAcl() - there will be other offending calls from plugins so we are unlikely to catch them all if users try to upgrade a site with a plugin that isn't compatible.

I hesitate to wade in to this one with a different (or complimentary) approach - but the problem Brian encountered was specifically during com_joomlaupdate and task=update.finalise to avoid bricking a site during the upgrade so we only really need to deal with this situation. An upgraded site with a broken extension is more straightforward to resolve so the goal should be to get to successfully upgraded site even if an extension is still broken??

Using try/catch in the triggerEvent method of EventAware seems to get around this (from my limited testing)

`
try
{
$result = $dispatcher->dispatch($eventName, $event);
}
catch ( \Throwable $throwable)
{
$this->getLogger()->error(sprintf('Dispatcher not able to trigger event %s ', $eventName));

		return [];
	}

`
But leaving this code modification in after the upgrade is completed is almost certainly not a good idea. Doing something specific to the context of "update.finalise" would be really ugly code. Not sure of how we could make the code 'specific' to the context since the onAfterInitialise is called before the component code.

@zero-24
Copy link
Contributor

zero-24 commented Oct 7, 2020

I would personaly would not like to introduce specific code for the update process. When there is a plugin that breaks than it should break.

As mention also in the other issue i'm working on a proposal / RFC for the pre Upgrade checker that issues a warning / error once there a system plugins with unknown compatibliy enabled and requests another dedicated SU confirmation that this site should possible be bricked. It would be an additional check to the pre upgrade checker that also gets improved with the strings that brian proposed in the other issue. So we have a much clearer message to our users.

But please give me a bit more time to come up with that PR.

@zero-24
Copy link
Contributor

zero-24 commented Oct 7, 2020

And all that without breaking legimitiate use cases like the system plugin that loads the database handler. Or trying to catch an issue or just partly patch it and break other parts of the application.

@brianteeman
Copy link
Contributor Author

An upgraded site with a broken extension is more straightforward to resolve so the goal should be to get to successfully upgraded site even if an extension is still broken??

Yes

@zero-24
Copy link
Contributor

zero-24 commented Oct 7, 2020

Yes i agree on that too. :-)

@richard67
Copy link
Member

The main thing is we have to make sure that the update SQL scripts are run, because a later db fix will only fix structure (if we could come to that point that the fix would be possible), but many problems with broken backend after broken update come not only from structure but also from data, e.g. menu items missing in backend in the admin menu (menu type "main").

Or we make sure that the db fixer can be used and that it can detect a broken update by the schema version in database and offers to finalize the broken update.

@zero-24
Copy link
Contributor

zero-24 commented Oct 7, 2020

The main thing is we have to make sure that the update SQL scripts are run, because a later db fix will only fix structure (if we could come to that point that the fix would be possible), but many problems with broken backend after broken update come not only from structure but also from data, e.g. menu items missing in backend in the admin menu (menu type "main").

Or we make sure that the db fixer can be used and that it can detect a broken update by the schema version in database and offers to finalize the broken update.

Yes that is the reason we want that only confirmed compatible extensions are enabled to run at that point of the upgrade so any issue they might produce does not brick the updgrade.

The impact of such error with a large upgrade like 3.10 to 4.x is much higher than on smaler updates for sure. But IIRC the finalise step is also not only about the database stuff.

In the end we have to make sure that all steps for the upgrade run correctly and that any plugin that could break that upgrade is checked upfront.

@richard67
Copy link
Member

Yes, but incompatible plugins are not the only problem. If you just break the database connection at the wrong point of time during the update, when running the first few SQL update scripts, then you end with a brick, too. Not sure if this can be easily fixed.

@nikosdion
Copy link
Contributor

@richard67 You can. I've been doing that in Akeeba Backup for 6+ years. If the DB query fails you check if the connection still pings. If not, reconnect and run the query again.

The biggest problem Joomla has during updates is that its schema update mechanism is NOT idempotent. You have a bunch of SQL files with an arbitrary number of queries which must succeed all at once or the whole update is hosed to oblivion. This does happen in real world usage.

At one point it was suggested that we use Doctrine exactly for this reason. The problem is that Doctrine is a rather thick, very opinionated abstraction layer which doesn't let us apply the kind of fine grained optimisations we need per database server type.

The solution I came up with several years ago was my own, mostly idempotent, schema installer and updater. An XML file wraps blocks of SQL queries in conditions. The idea is that before running the SQL query/-ies the condition is true, afterwards it is false.

As soon as I started using it in my software the number of SQL issues on update dropped to zero and I was able to add an automatic schema fix feature regardless of the state the schema was in. There was a radical drop in support requests with regards to database issues.

I did try to get that, or something similar (and improved -- I now know the limitations of my design) to Joomla 4 but... let's say politics won and leave it at that.

@zero-24
Copy link
Contributor

zero-24 commented Oct 10, 2020

Please find here a RFC regarding the issue discussed here: #31042

@richard67
Copy link
Member

@richard67 You can.

If the "you" means "one", i.e. anyone: Ok.

But if it means me personally: Not sure if I can. I have limited time and currently also limited energy to work on that. At least I can't do it alone. I might help with the one or other thing though.

@zero-24
Copy link
Contributor

zero-24 commented Oct 24, 2020

Closing as there is now a PR from @GeraintEdwards at: #31200. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests