From ad9df97626f0cba772a1f6066feb059f08f13938 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Thu, 22 Jun 2023 08:42:49 +1200 Subject: [PATCH 1/2] FIX LastPage method returns true if TotalPages equals 0 --- src/ORM/PaginatedList.php | 2 +- tests/php/ORM/PaginatedListTest.php | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/ORM/PaginatedList.php b/src/ORM/PaginatedList.php index f7289e9a8..ac9d95ab2 100644 --- a/src/ORM/PaginatedList.php +++ b/src/ORM/PaginatedList.php @@ -416,7 +416,7 @@ class PaginatedList extends ListDecorator */ public function LastPage() { - return $this->CurrentPage() == $this->TotalPages(); + return $this->CurrentPage() >= $this->TotalPages(); } /** diff --git a/tests/php/ORM/PaginatedListTest.php b/tests/php/ORM/PaginatedListTest.php index fb63403d6..e00603d53 100644 --- a/tests/php/ORM/PaginatedListTest.php +++ b/tests/php/ORM/PaginatedListTest.php @@ -303,9 +303,20 @@ class PaginatedListTest extends SapphireTest $list = new PaginatedList(new ArrayList()); $list->setTotalItems(50); + $this->assertFalse($list->LastPage()); + $list->setCurrentPage(4); $this->assertFalse($list->LastPage()); $list->setCurrentPage(5); $this->assertTrue($list->LastPage()); + $list->setCurrentPage(6); + $this->assertTrue($list->LastPage()); + + $emptyList = new PaginatedList(new ArrayList()); + $emptyList->setTotalItems(0); + + $this->assertTrue($emptyList->LastPage()); + $emptyList->setCurrentPage(1); + $this->assertTrue($emptyList->LastPage()); } public function testNotLastPage() From 8c3ba810524f7bbcba92c36deb4194f84a0d075f Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Fri, 7 Jul 2023 15:56:31 +1200 Subject: [PATCH 2/2] FIX PHP 8.1 support in MySQLiConnector::query errors (#10570) * FIX PHP 8.1 support in MySQLiConnector::query errors The default error reporting mode in PHP 8.1 has changed from using errors reported on the connection handle to throwing mysqli_sql_exception. query() makes no allowance for this, and functions up the call stack expect to catch Silverstripe\ORM\Connect\DatabaseException instead - resulting in the MySQLi exception going all the way up to halt the system. We can use a try, catch, and finally to retain backwards compatibility, no matter which setting (e.g. PHP version default) someone has enabled. * Move MySQLConnector test skip call into setUp() As review feedback; marking the test as skipped in a private function obfuscated where the call was happening and made it harder to skimread the tests. Moving this into a setUp function makes it obvious the check is run before each test case, and skipped if necessary. --- src/ORM/Connect/MySQLiConnector.php | 17 ++-- tests/php/ORM/MySQLiConnectorTest.php | 113 ++++++++++++++------------ 2 files changed, 75 insertions(+), 55 deletions(-) diff --git a/src/ORM/Connect/MySQLiConnector.php b/src/ORM/Connect/MySQLiConnector.php index 20a45d97c..ca6cad20c 100644 --- a/src/ORM/Connect/MySQLiConnector.php +++ b/src/ORM/Connect/MySQLiConnector.php @@ -181,12 +181,19 @@ class MySQLiConnector extends DBConnector { $this->beforeQuery($sql); - // Benchmark query - $handle = $this->dbConn->query($sql ?? '', MYSQLI_STORE_RESULT); + $error = null; + $handle = null; - if (!$handle || $this->dbConn->error) { - $this->databaseError($this->getLastError(), $errorLevel, $sql); - return null; + try { + // Benchmark query + $handle = $this->dbConn->query($sql ?? '', MYSQLI_STORE_RESULT); + } catch (mysqli_sql_exception $e) { + $error = $e->getMessage(); + } finally { + if (!$handle || $this->dbConn->error) { + $this->databaseError($error ?? $this->getLastError(), $errorLevel, $sql); + return null; + } } // Some non-select queries return true on success diff --git a/tests/php/ORM/MySQLiConnectorTest.php b/tests/php/ORM/MySQLiConnectorTest.php index a037d58ed..eb0d3a138 100644 --- a/tests/php/ORM/MySQLiConnectorTest.php +++ b/tests/php/ORM/MySQLiConnectorTest.php @@ -2,10 +2,12 @@ namespace SilverStripe\ORM\Tests; +use mysqli_driver; use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\TestOnly; -use SilverStripe\ORM\Tests\MySQLiConnectorTest\MySQLiConnector; +use SilverStripe\ORM\Connect\DatabaseException; use SilverStripe\ORM\DB; +use SilverStripe\ORM\Tests\MySQLiConnectorTest\MySQLiConnector; use SilverStripe\Tests\ORM\Utf8\Utf8TestHelper; /** @@ -13,21 +15,47 @@ use SilverStripe\Tests\ORM\Utf8\Utf8TestHelper; */ class MySQLiConnectorTest extends SapphireTest implements TestOnly { + /** @var array project database settings configuration */ + private $config = []; + + private function getConnector(?string $charset = null, ?string $collation = null, bool $selectDb = false) + { + $config = $this->config; + + if ($charset) { + $config['charset'] = $charset; + } + if ($collation) { + $config['collation'] = $collation; + } + + $config['database'] = 'information_schema'; + + $connector = new MySQLiConnector(); + $connector->connect($config, $selectDb); + + return $connector; + } + + public function setUp(): void + { + parent::setUp(); + + $config = DB::getConfig(); + + if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') { + $this->markTestSkipped("The test only relevant for MySQL - but $config[type] is in use"); + } + + $this->config = $config; + } + /** * @dataProvider charsetProvider */ public function testConnectionCharsetControl($charset, $defaultCollation) { - $config = DB::getConfig(); - $config['charset'] = $charset; - $config['database'] = 'information_schema'; - - if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') { - return $this->markTestSkipped('The test only relevant for MySQL'); - } - - $connector = new MySQLiConnector(); - $connector->connect($config); + $connector = $this->getConnector($charset); $connection = $connector->getMysqliConnection(); $cset = $connection->get_charset(); @@ -47,17 +75,7 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly */ public function testConnectionCollationControl($charset, $defaultCollation, $customCollation) { - $config = DB::getConfig(); - $config['charset'] = $charset; - $config['collation'] = $customCollation; - $config['database'] = 'information_schema'; - - if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') { - return $this->markTestSkipped('The test only relevant for MySQL'); - } - - $connector = new MySQLiConnector(); - $connector->connect($config); + $connector = $this->getConnector($charset, $customCollation); $connection = $connector->getMysqliConnection(); $cset = $connection->get_charset(); @@ -101,20 +119,7 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly public function testUtf8mb4GeneralCollation() { - $charset = 'utf8mb4'; - $collation = 'utf8mb4_general_ci'; - - $config = DB::getConfig(); - $config['charset'] = $charset; - $config['collation'] = $collation; - $config['database'] = 'information_schema'; - - if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') { - return $this->markTestSkipped('The test only relevant for MySQL'); - } - - $connector = new MySQLiConnector(); - $connector->connect($config, true); + $connector = $this->getConnector('utf8mb4', 'utf8mb4_general_ci', true); $connection = $connector->getMysqliConnection(); $result = $connection->query( @@ -127,20 +132,7 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly public function testUtf8mb4UnicodeCollation() { - $charset = 'utf8mb4'; - $collation = 'utf8mb4_unicode_ci'; - - $config = DB::getConfig(); - $config['charset'] = $charset; - $config['collation'] = $collation; - $config['database'] = 'information_schema'; - - if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') { - return $this->markTestSkipped('The test only relevant for MySQL'); - } - - $connector = new MySQLiConnector(); - $connector->connect($config, true); + $connector = $this->getConnector('utf8mb4', 'utf8mb4_unicode_ci', true); $connection = $connector->getMysqliConnection(); $result = $connection->query( @@ -151,4 +143,25 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly $this->assertEquals('rßt', $result[0][0]); $this->assertEquals('rst', $result[1][0]); } + + public function testQueryThrowsDatabaseErrorOnMySQLiError() + { + $connector = $this->getConnector(); + $driver = new mysqli_driver(); + // The default with PHP >= 8.0 + $driver->report_mode = MYSQLI_REPORT_OFF; + $this->expectException(DatabaseException::class); + $connector = $this->getConnector(null, null, true); + $connector->query('force an error with invalid SQL'); + } + + public function testQueryThrowsDatabaseErrorOnMySQLiException() + { + $driver = new mysqli_driver(); + // The default since PHP 8.1 - https://www.php.net/manual/en/mysqli-driver.report-mode.php + $driver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT; + $this->expectException(DatabaseException::class); + $connector = $this->getConnector(null, null, true); + $connector->query('force an error with invalid SQL'); + } }