mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
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.
This commit is contained in:
parent
ab4802caaf
commit
8c3ba81052
@ -181,12 +181,19 @@ class MySQLiConnector extends DBConnector
|
|||||||
{
|
{
|
||||||
$this->beforeQuery($sql);
|
$this->beforeQuery($sql);
|
||||||
|
|
||||||
// Benchmark query
|
$error = null;
|
||||||
$handle = $this->dbConn->query($sql ?? '', MYSQLI_STORE_RESULT);
|
$handle = null;
|
||||||
|
|
||||||
if (!$handle || $this->dbConn->error) {
|
try {
|
||||||
$this->databaseError($this->getLastError(), $errorLevel, $sql);
|
// Benchmark query
|
||||||
return null;
|
$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
|
// Some non-select queries return true on success
|
||||||
|
@ -2,10 +2,12 @@
|
|||||||
|
|
||||||
namespace SilverStripe\ORM\Tests;
|
namespace SilverStripe\ORM\Tests;
|
||||||
|
|
||||||
|
use mysqli_driver;
|
||||||
use SilverStripe\Dev\SapphireTest;
|
use SilverStripe\Dev\SapphireTest;
|
||||||
use SilverStripe\Dev\TestOnly;
|
use SilverStripe\Dev\TestOnly;
|
||||||
use SilverStripe\ORM\Tests\MySQLiConnectorTest\MySQLiConnector;
|
use SilverStripe\ORM\Connect\DatabaseException;
|
||||||
use SilverStripe\ORM\DB;
|
use SilverStripe\ORM\DB;
|
||||||
|
use SilverStripe\ORM\Tests\MySQLiConnectorTest\MySQLiConnector;
|
||||||
use SilverStripe\Tests\ORM\Utf8\Utf8TestHelper;
|
use SilverStripe\Tests\ORM\Utf8\Utf8TestHelper;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -13,21 +15,47 @@ use SilverStripe\Tests\ORM\Utf8\Utf8TestHelper;
|
|||||||
*/
|
*/
|
||||||
class MySQLiConnectorTest extends SapphireTest implements TestOnly
|
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
|
* @dataProvider charsetProvider
|
||||||
*/
|
*/
|
||||||
public function testConnectionCharsetControl($charset, $defaultCollation)
|
public function testConnectionCharsetControl($charset, $defaultCollation)
|
||||||
{
|
{
|
||||||
$config = DB::getConfig();
|
$connector = $this->getConnector($charset);
|
||||||
$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);
|
|
||||||
$connection = $connector->getMysqliConnection();
|
$connection = $connector->getMysqliConnection();
|
||||||
|
|
||||||
$cset = $connection->get_charset();
|
$cset = $connection->get_charset();
|
||||||
@ -47,17 +75,7 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly
|
|||||||
*/
|
*/
|
||||||
public function testConnectionCollationControl($charset, $defaultCollation, $customCollation)
|
public function testConnectionCollationControl($charset, $defaultCollation, $customCollation)
|
||||||
{
|
{
|
||||||
$config = DB::getConfig();
|
$connector = $this->getConnector($charset, $customCollation);
|
||||||
$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);
|
|
||||||
$connection = $connector->getMysqliConnection();
|
$connection = $connector->getMysqliConnection();
|
||||||
|
|
||||||
$cset = $connection->get_charset();
|
$cset = $connection->get_charset();
|
||||||
@ -101,20 +119,7 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly
|
|||||||
|
|
||||||
public function testUtf8mb4GeneralCollation()
|
public function testUtf8mb4GeneralCollation()
|
||||||
{
|
{
|
||||||
$charset = 'utf8mb4';
|
$connector = $this->getConnector('utf8mb4', 'utf8mb4_general_ci', true);
|
||||||
$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);
|
|
||||||
$connection = $connector->getMysqliConnection();
|
$connection = $connector->getMysqliConnection();
|
||||||
|
|
||||||
$result = $connection->query(
|
$result = $connection->query(
|
||||||
@ -127,20 +132,7 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly
|
|||||||
|
|
||||||
public function testUtf8mb4UnicodeCollation()
|
public function testUtf8mb4UnicodeCollation()
|
||||||
{
|
{
|
||||||
$charset = 'utf8mb4';
|
$connector = $this->getConnector('utf8mb4', 'utf8mb4_unicode_ci', true);
|
||||||
$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);
|
|
||||||
$connection = $connector->getMysqliConnection();
|
$connection = $connector->getMysqliConnection();
|
||||||
|
|
||||||
$result = $connection->query(
|
$result = $connection->query(
|
||||||
@ -151,4 +143,25 @@ class MySQLiConnectorTest extends SapphireTest implements TestOnly
|
|||||||
$this->assertEquals('rßt', $result[0][0]);
|
$this->assertEquals('rßt', $result[0][0]);
|
||||||
$this->assertEquals('rst', $result[1][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');
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user