Let the 2.0.0 update SQL run again after an interrupted update (B5)

sql/updates/2.0.0.sql

@@ -1,10 +1,18 @@
 -- Update DB schema to version 2.0.0
 
+-- If an update stops halfway (e.g. a timeout converting a large table), the
+-- schema version stays at the previous one, and the next attempt runs this
+-- file again from the start: every statement has to work on tables which
+-- already have its change. Statements which can't be written that way in
+-- MySQL (ADD COLUMN, ADD INDEX) are marked "CAN FAIL" for Joomla's installer
+-- (Joomla 4.2+), which then goes on if they fail. Checked by
+-- tests/Unit/UpdateSqlTest.php and tests/Integration/UpdateSqlTest.php.
+
 -- number of requests rejected because of a block, and the time of the
 -- latest one (issue #219); existing blocks start with no data
-ALTER TABLE `#__bfstop_bannedip` ADD COLUMN attempts int unsigned NOT NULL DEFAULT 0;
+ALTER TABLE `#__bfstop_bannedip` ADD COLUMN attempts int unsigned NOT NULL DEFAULT 0 /** CAN FAIL **/;
 
-ALTER TABLE `#__bfstop_bannedip` ADD COLUMN last_attempt datetime NULL DEFAULT NULL;
+ALTER TABLE `#__bfstop_bannedip` ADD COLUMN last_attempt datetime NULL DEFAULT NULL /** CAN FAIL **/;
 
 
 -- use utf8mb4 like Joomla's own tables; with utf8 (3-byte), a failed login
@@ -18,7 +26,7 @@ ALTER TABLE `#__bfstop_unblock` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4
 ALTER TABLE `#__bfstop_unblock_token` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
 
 -- the user an unblock link was sent to, to limit the links per user
-ALTER TABLE `#__bfstop_unblock_token` ADD COLUMN username varchar(150) NULL DEFAULT NULL;
+ALTER TABLE `#__bfstop_unblock_token` ADD COLUMN username varchar(150) NULL DEFAULT NULL /** CAN FAIL **/;
 
 ALTER TABLE `#__bfstop_allowlist` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
 
@@ -60,10 +68,10 @@ INSERT INTO `#__bfstop_username_stats` (username, attempts, first_attempt, last_
 	ON DUPLICATE KEY UPDATE attempts=attempts;
 
 -- speeds up the per-username (account-level) throttle query
-ALTER TABLE `#__bfstop_failedlogin` ADD INDEX username_logtime (username, logtime);
+ALTER TABLE `#__bfstop_failedlogin` ADD INDEX username_logtime (username, logtime) /** CAN FAIL **/;
 
 -- every request looks up the visitor's address in the block and allow list;
 -- addresses and networks are stored in canonical form (the plugin brings
 -- existing entries into it once, IpHelper::canonicalEntry())
-ALTER TABLE `#__bfstop_bannedip` ADD INDEX ipaddress (ipaddress);
-ALTER TABLE `#__bfstop_allowlist` ADD INDEX ipaddress (ipaddress);
+ALTER TABLE `#__bfstop_bannedip` ADD INDEX ipaddress (ipaddress) /** CAN FAIL **/;
+ALTER TABLE `#__bfstop_allowlist` ADD INDEX ipaddress (ipaddress) /** CAN FAIL **/;

tests/Integration/UpdateSqlTest.php

@@ -0,0 +1,64 @@
+<?php
+/*
+ * @package BFStop Plugin (bfstop) for Joomla!
+ * @author Bernhard Froehler
+ * @copyright (C) Bernhard Froehler
+ * @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
+**/
+namespace Codeling\Bfstop\Tests\Integration;
+
+use Joomla\CMS\Installer\Installer;
+
+/**
+ * Runs the update SQL of 2.0.0 through Joomla's installer on tables which
+ * already have all of its changes - as an attempt to update again after one
+ * which stopped halfway does (see tests/Unit/UpdateSqlTest.php): it has to
+ * get through, and record the new schema version.
+ */
+class UpdateSqlTest extends IntegrationTestCase
+{
+	protected function emptyTables()
+	{
+		// not needed here
+	}
+
+	private function schemaVersion($extensionId)
+	{
+		return $this->queryValue('SELECT version_id FROM #__schemas WHERE extension_id='.(int) $extensionId);
+	}
+
+	public function testUpdateCanRunAgainOnTablesItAlreadyChanged()
+	{
+		if ($this->db->getServerType() !== 'mysql')
+		{
+			$this->markTestSkipped('PostgreSQL is supported from 2.0.0 on, there is no update SQL for it yet');
+		}
+		$extensionId = (int) $this->queryValue("SELECT extension_id FROM #__extensions WHERE type='plugin' AND element='bfstop'");
+		$original = $this->schemaVersion($extensionId);
+		$this->assertNotNull($original);
+		try
+		{
+			$this->db->setQuery("UPDATE #__schemas SET version_id='1.4.3' WHERE extension_id=".$extensionId);
+			$this->db->execute();
+
+			$root = dirname(__DIR__, 2);
+			$manifest = simplexml_load_file($root.'/bfstop.xml');
+			$installer = new Installer();
+			$installer->setDatabase($this->db);
+			$installer->setPath('extension_root', $root);
+			$result = $installer->parseSchemaUpdates($manifest->update->schemas, $extensionId);
+
+			$this->assertNotFalse($result, 'an update SQL statement failed, see the "Update" log of the Joomla site');
+			$versions = array_map(function ($file) { return basename($file, '.sql'); },
+				glob($root.'/sql/updates/*.sql'));
+			usort($versions, 'version_compare');
+			$this->assertSame(end($versions), $this->schemaVersion($extensionId));
+		}
+		finally
+		{
+			$this->db->setQuery('UPDATE #__schemas SET version_id='.$this->db->quote($original).
+				' WHERE extension_id='.$extensionId);
+			$this->db->execute();
+		}
+	}
+}

tests/Unit/UpdateSqlTest.php

@@ -0,0 +1,86 @@
+<?php
+/*
+ * @package BFStop Plugin (bfstop) for Joomla!
+ * @author Bernhard Froehler
+ * @copyright (C) Bernhard Froehler
+ * @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
+**/
+namespace Codeling\Bfstop\Tests\Unit;
+
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * An update which stopped halfway leaves the schema version where it was, so
+ * the next attempt runs the whole update file again: a statement which fails
+ * on a table it already changed would stop every later attempt as well. The
+ * MySQL statements which can't be written to work a second time have to be
+ * marked "CAN FAIL" (see sql/updates/2.0.0.sql); the update files of earlier
+ * versions predate that.
+ */
+class UpdateSqlTest extends TestCase
+{
+	private const Marker = '/** CAN FAIL **/';
+
+	// ALTER TABLE changes which fail if they were made before
+	private const NotRepeatable = '/\b(ADD\s+(COLUMN|INDEX|KEY|UNIQUE|CONSTRAINT|PRIMARY|FULLTEXT)|DROP\s+(COLUMN|INDEX|KEY|PRIMARY|FOREIGN)|RENAME\s+(COLUMN|INDEX|KEY|TO))\b/i';
+
+	public static function updateFiles(): array
+	{
+		$files = array();
+		foreach (glob(dirname(__DIR__, 2).'/sql/updates/*.sql') as $file)
+		{
+			if (version_compare(basename($file, '.sql'), '2.0.0', '>='))
+			{
+				$files[basename($file)] = array($file);
+			}
+		}
+		return $files;
+	}
+
+	private static function statements($file)
+	{
+		$sql = preg_replace('/^\s*--.*$/m', '', file_get_contents($file));
+		return array_filter(array_map('trim', explode(';', $sql)), 'strlen');
+	}
+
+	#[DataProvider('updateFiles')]
+	public function testStatementsWhichCanOnlyRunOnceAreMarkedCanFail($file)
+	{
+		foreach (self::statements($file) as $statement)
+		{
+			if (preg_match(self::NotRepeatable, $statement))
+			{
+				$this->assertStringEndsWith(self::Marker, $statement,
+					basename($file).": fails if run again, mark it CAN FAIL: $statement");
+			}
+		}
+	}
+
+	#[DataProvider('updateFiles')]
+	public function testTablesAreCreatedOnlyIfTheyDontExist($file)
+	{
+		foreach (self::statements($file) as $statement)
+		{
+			if (preg_match('/^CREATE\s+TABLE\b/i', $statement))
+			{
+				$this->assertMatchesRegularExpression('/^CREATE\s+TABLE\s+IF\s+NOT\s+EXISTS\b/i', $statement,
+					basename($file));
+			}
+		}
+	}
+
+	public function testTheMarkerIsWhereJoomlaLooksForIt()
+	{
+		// Installer::parseSchemaUpdates() only recognises it right before the
+		// semicolon which ends the statement
+		$marked = 0;
+		foreach (self::updateFiles() as list($file))
+		{
+			$sql = preg_replace('/^\s*--.*$/m', '', file_get_contents($file));
+			$marked += substr_count($sql, self::Marker);
+			$this->assertSame(substr_count($sql, self::Marker), substr_count($sql, ' '.self::Marker.';'), basename($file));
+		}
+		$this->assertGreaterThan(0, $marked);
+	}
+}