From a74443a3425eab08924fb62783cfab546400bba2 Mon Sep 17 00:00:00 2001 From: Andrew Nicols <andrew@nicols.co.uk> Date: Tue, 10 Oct 2023 23:25:58 +0800 Subject: [PATCH 1/2] MDL-79638 phpunit: Adjust version checks for mysql/mariadb hack This hack was introduced to work around a bug in MySQL 5.6.14 and MariaDB at the time. https://bugs.mysql.com/bug.php?id=69882 It was addressed a few months later in 5.6.16, and 5.7.4. MariaDB merged version 5.6.16 of MySQL's InnoDB engine in MariaDB 10.0.11 and got the patch from there. Moodle has required MySQL 5.7, and MariaDB 10.2.29 since Moodle 3.11 and it is therefore safe to remove these hacks for these versions. --- lib/testing/classes/util.php | 38 ++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/testing/classes/util.php b/lib/testing/classes/util.php index 737c7532472..0c241e6d2b9 100644 --- a/lib/testing/classes/util.php +++ b/lib/testing/classes/util.php @@ -643,7 +643,7 @@ abstract class testing_util { $borkedmysql = false; if ($DB->get_dbfamily() === 'mysql') { $version = $DB->get_server_info(); - if (version_compare($version['version'], '5.6.0') == 1 and version_compare($version['version'], '5.6.16') == -1) { + if (version_compare($version['version'], '5.7.4', '<')) { // Everything that comes from Oracle is evil! // // See http://dev.mysql.com/doc/refman/5.6/en/alter-table.html @@ -652,31 +652,27 @@ abstract class testing_util { // From 5.6.16 release notes: // InnoDB: The ALTER TABLE INPLACE algorithm would fail to decrease the auto-increment value. // (Bug #17250787, Bug #69882) - $borkedmysql = true; - - } else if (version_compare($version['version'], '10.0.0') == 1) { - // And MariaDB is no better! - // Let's hope they pick the patch sometime later... + // This also impacts MySQL < 5.7.4. $borkedmysql = true; } - } - if ($borkedmysql) { - $mysqlsequences = array(); - $prefix = $DB->get_prefix(); - $rs = $DB->get_recordset_sql("SHOW TABLE STATUS LIKE ?", array($prefix.'%')); - foreach ($rs as $info) { - $table = strtolower($info->name); - if (strpos($table, $prefix) !== 0) { - // Incorrect table match caused by _ char. - continue; - } - if (!is_null($info->auto_increment)) { - $table = preg_replace('/^'.preg_quote($prefix, '/').'/', '', $table); - $mysqlsequences[$table] = $info->auto_increment; + if ($borkedmysql) { + $mysqlsequences = array(); + $prefix = $DB->get_prefix(); + $rs = $DB->get_recordset_sql("SHOW TABLE STATUS LIKE ?", array($prefix.'%')); + foreach ($rs as $info) { + $table = strtolower($info->name); + if (strpos($table, $prefix) !== 0) { + // Incorrect table match caused by _ char. + continue; + } + if (!is_null($info->auto_increment)) { + $table = preg_replace('/^'.preg_quote($prefix, '/').'/', '', $table); + $mysqlsequences[$table] = $info->auto_increment; + } } + $rs->close(); } - $rs->close(); } foreach ($data as $table => $records) { From e7263580b913ea2b7bd2d59751733bd5964ff692 Mon Sep 17 00:00:00 2001 From: Andrew Nicols <andrew@nicols.co.uk> Date: Tue, 10 Oct 2023 23:56:19 +0800 Subject: [PATCH 2/2] MDL-79638 phpunit: Add unit test for increment resets --- lib/testing/tests/util_test.php | 85 +++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 lib/testing/tests/util_test.php diff --git a/lib/testing/tests/util_test.php b/lib/testing/tests/util_test.php new file mode 100644 index 00000000000..68f8b0f8a54 --- /dev/null +++ b/lib/testing/tests/util_test.php @@ -0,0 +1,85 @@ +<?php +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see <http://www.gnu.org/licenses/>. + +namespace core\testing; + +/** + * Testing util tests. + * + * @package core + * @category phpunit + * @copyright 2023 Andrew Lyons <andrew@nicols.co.uk> + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class util_test extends \advanced_testcase { + /** + * Note: This test is required for the other two parts because the first time + * a table is written to it may not have had the initial value reset. + * + * @coversNothing + */ + public function test_increment_reset_part_one(): void { + global $DB; + + switch ($DB->get_dbfamily()) { + case 'mssql': + $this->markTestSkipped('MSSQL does not support sequences'); + return; + case 'mysql': + $version = $DB->get_server_info(); + if (version_compare($version['version'], '5.7.4', '<')) { + return; + } + } + + $this->resetAfterTest(); + $DB->insert_record('config_plugins', [ + 'plugin' => 'example', + 'name' => 'test_increment', + 'value' => 0, + ]); + } + + /** + * @coversNothing + * @depends test_increment_reset_part_one + */ + public function test_increment_reset_part_two(): int { + global $DB; + + $this->resetAfterTest(); + return $DB->insert_record('config_plugins', [ + 'plugin' => 'example', + 'name' => 'test_increment', + 'value' => 0, + ]); + } + + /** + * @depends test_increment_reset_part_two + */ + public function test_increment_reset_part_three(int $previousid): void { + global $DB; + + $this->resetAfterTest(); + $id = $DB->insert_record('config_plugins', [ + 'plugin' => 'example', + 'name' => 'test_increment', + 'value' => 0, + ]); + $this->assertEquals($previousid, $id); + } +}