[BUG 4.6.11] Installing/upgrading plugins is actually quite broken

[teraßyte]

First here a list of the plugins attached to this topic with a list of what they do in each upgrade step:

• 1.0.0 => Adds 1 setting (testing_upgrade_setting). The install script creates 1 table (aaaa_install) and updates the setting value to 3.
• 1.0.1 => The upgrade script updates the setting value to 4.
• 1.0.2 => The upgrade script creates 1 table (aaaa_upgrade).
• 2.0.0 => The upgrade scripts for versions 1.0.1 and 1.02 are removed (empty XML entries). The upgrade script for version 2.0.0 creates 1 table (aaaa_upgrade) and updates the setting value to 5.
• 2.0.1 => The upgrade scripts for versions 1.0.1 and 1.02 are restored (no empty XML entries). The upgrade script for version 2.0.0 only updates the setting value to 5 and the one for version 2.0.1 does nothing.

======

Now, let's start with a list of tests to be done with these plugins:

===

TEST #1 (all OK):

1) Install version 1.0.0:

• The table aaaa_install is created.
• The setting testing_upgrade_setting is updated to value 3.

2) Upgrade the plugin to version 1.0.1:

• The setting testing_upgrade_setting is updated to value 4.

3) Upgrade the plugin to version 1.0.2:

• The table aaaa_upgrade is created.

 

All working as expected in this test.

===

TEST #2 (install BUG):

1) Now uninstall the plugin and install version 1.0.1 directly:

• The table aaaa_install is created.
• The setting testing_upgrade_setting is WRONGLY updated to value 4.
  • The code to update the value to 4 is present ONLY in the ips_plugins_setup_upg_10001 class which shouldn't run on install.

This problem is caused by the code below in applications/core/modules/admin/applications/plugins.php (lines 851-870):

if ( $class AND $xml->getAttribute('long') )
{
	if ( $xml->getAttribute('long') == 10000 AND $data['isNew'] )
	{
		/* Installing, so use install file which is bundled with <version long="10000"> */
		$key   = 'plugin_' . $plugin->id . '_setup_install_class';
		\IPS\Data\Store::i()->$key = $class;

		$data['storeKeys'][]    = $key;
		$data['setUpClasses']['install'] = 'install';
	}
	else if ( $data['currentVersionId'] < $xml->getAttribute('long') )
	{
		$key = 'plugin_' . $plugin->id . '_setup_' . $xml->getAttribute('long') . '_class';
		\IPS\Data\Store::i()->$key = $class;

		$data['storeKeys'][]    = $key;
		$data['setUpClasses'][ $xml->getAttribute('long') ] = $xml->getAttribute('long');
	}
}

The elseif code in this case should be executed only if it's not a new installation:

else if ( !$data['isNew'] AND $data['currentVersionId'] < $xml->getAttribute('long') )

At least I don't expect "upgrade scripts" to run when I'm doing a fresh install since any changes would be already included in the "install script".

 

A similar issue happens also if you install 1.0.2 directly as a new plugin. But in that case also the table aaaa_upgrade included in the 1.0.2 upgrade step is created in addition to the setting being updated.

===

TEST #3 (all OK):

1) Continuing from the second test, upgrade the plugin from version 1.0.1 to version 1.0.2 directly:

• The table aaaa_upgrade is created.

 

All working as expected in this test.

===

TEST #4 (upgrade BUG):

1) Uninstall the plugin and install version 1.0.0 again:

• The table aaaa_install is created.
• The setting testing_upgrade_setting is updated to value 3.

2) Upgrade the plugin directly to version 2.0.0 (skipping version 1.0.1 and 1.0.2):

• The table aaaa_upgrade is NOT created.
• The setting testing_upgrade_setting is NOT updated to value 5.

 

The upgrade in this case doesn't work because the 1.0.1 and 1.0.2 upgrade files were deleted before exporting the plugin. The upgrade code is completely skipping running the upgrade scripts if even one is missing. The problem here is that if we can't skip including empty upgrade scripts (I usually delete them) we need to have some kind of warning when downloading/exporting that it's gonna cause issues.

===

TEST #5 (install BUG which actually makes it work):

1) Uninstall the plugin and install version 2.0.0 directly:

• The table aaaa_install is created.
• The setting testing_upgrade_setting is updated to value 3.

 

This is actually the expected result for a new install but the upgrade scripts in this case aren't running simply because 2 of them are missing and thus the code is skipping them all.

===

TEST #6 (install BUG same as TEST #2):

1) Uninstall the plugin and install version 2.0.1 directly:

• The table aaaa_install is created but also the table aaaa_upgrade is WRONGLY created.
• The setting testing_upgrade_setting is WRONGLY updated to value 5 instead of remaning set to 3 from the install script.

 

This is again an install bug which executes all the upgrade scripts on install. But unlike version 2.0.0 (with its missing upgrade scripts) version 2.0.1 has them all restored which makes the code run them.

 

======

Sorry for the really long post but the whole thing was driving me crazy so I wanted to be sure to cover every scenario. To sum up this bug report there's 2 problems:

1. Installing a plugin from scratch should run ONLY the install script.
2. Either fix the code not running any upgrade script if some are deleted/missing before exporting (empty XML entries), or add some kind of IN_DEV warning when downloading/exporing the plugins to make us aware that some required(?) files are missing.

Testing Upgrades 1.0.0.xml Testing Upgrades 1.0.1.xml Testing Upgrades 1.0.2.xml Testing Upgrades 2.0.0.xml Testing Upgrades 2.0.1.xml

Edited April 1, 2022 by teraßyte
fixed codebox

[teraßyte]

Btw, I ended up investigating this because I'm updating my Bump Up Topics plugin and the new version wasn't running the upgrade script.

[Maxxius]

finally you are updating topic bump mod 😄 great to see you back!

[teraßyte]

1 minute ago, Maxxius said:

finally you are updating topic bump mod 😄 great to see you back!

New 4.0.0 version is already out. Next version (coming soon) will have a debump option. 😉

[Daniel F]

Thanks, I have logged this to our bug tracker.

[teraßyte]

9 months later... this still isn't fixed even with the fix already provided in the first post... 🙄

[teraßyte]

Just hit a "table already exists" error installing a plugin locally because this bug isn't fixed. 🙄

 

Adding a check in the upgrade file to not execute the create table query is an option, but that's just a workaround and not the proper fix.