Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix loose typing of constructor for transaction level, and remove public property. #22535

Merged
merged 13 commits into from
Sep 6, 2024
30 changes: 28 additions & 2 deletions core/Db.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Exception;
use Piwik\Db\Adapter;
use Piwik\Db\Schema;
use Piwik\Db\TransactionalDatabaseInterface;

/**
* Contains SQL related helper functions for Piwik's MySQL database.
Expand All @@ -32,7 +33,7 @@
*
* @api
*/
class Db
class Db implements TransactionalDatabaseInterface
{
public const SQL_MODE = 'NO_AUTO_VALUE_ON_ZERO';

Expand All @@ -42,7 +43,8 @@ class Db
private static $logQueries = true;

// this is used for indicate TransactionLevel Cache
public $supportsUncommitted;
private $supportsTransactionLevelForNonLockingReads;

/**
* Returns the database connection and creates it if it hasn't been already.
*
Expand Down Expand Up @@ -875,4 +877,28 @@ public static function isOptimizeInnoDBSupported($version = null)
{
return Db\Schema::getInstance()->isOptimizeInnoDBSupported();
}

public function setSupportsTransactionLevelForNonLockingReads(bool $supports = null): void
{
$this->supportsTransactionLevelForNonLockingReads = $supports;
}

public function getSupportsTransactionLevelForNonLockingReads(): ?bool
{
return $this->supportsTransactionLevelForNonLockingReads;
}

public function getCurrentTransactionIsolationLevelForSession(): string
{
try {
return self::fetchOne('SELECT @@TX_ISOLATION');
} catch (Exception $e) {
return self::fetchOne('SELECT @@transaction_isolation');
}
}

public function setTransactionIsolationLevel(string $level): void
{
self::query("SET SESSION TRANSACTION ISOLATION LEVEL $level");
}
}
27 changes: 25 additions & 2 deletions core/Db/Adapter/Mysqli.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ class Mysqli extends Zend_Db_Adapter_Mysqli implements AdapterInterface
* @param array|Zend_Config $config database configuration
*/

// this is used for indicate TransactionLevel Cache
public $supportsUncommitted;
private $supportsTransactionLevelForNonLockingReads;

public function __construct($config)
{
Expand Down Expand Up @@ -255,4 +254,28 @@ public function getClientVersion()

return $major . '.' . $minor . '.' . $revision;
}

public function setSupportsTransactionLevelForNonLockingReads(bool $supports = null): void
{
$this->supportsTransactionLevelForNonLockingReads = $supports;
}

public function getSupportsTransactionLevelForNonLockingReads(): ?bool
{
return $this->supportsTransactionLevelForNonLockingReads;
}

public function getCurrentTransactionIsolationLevelForSession(): string
{
try {
return $this->fetchOne('SELECT @@TX_ISOLATION');
} catch (Exception $e) {
return $this->fetchOne('SELECT @@transaction_isolation');
}
}

public function setTransactionIsolationLevel(string $level): void
{
$this->query("SET SESSION TRANSACTION ISOLATION LEVEL $level");
}
}
27 changes: 25 additions & 2 deletions core/Db/Adapter/Pdo/Mysql.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ class Mysql extends Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface
* @param array|Zend_Config $config database configuration
*/

// this is used for indicate TransactionLevel Cache
public $supportsUncommitted;
private $supportsTransactionLevelForNonLockingReads;

public function __construct($config)
{
Expand Down Expand Up @@ -352,4 +351,28 @@ protected function _dsn() // phpcs:ignore PSR2.Methods.MethodDeclaration.Undersc

return parent::_dsn();
}

public function setSupportsTransactionLevelForNonLockingReads(bool $supports = null): void
{
$this->supportsTransactionLevelForNonLockingReads = $supports;
}

public function getSupportsTransactionLevelForNonLockingReads(): ?bool
{
return $this->supportsTransactionLevelForNonLockingReads;
}

public function getCurrentTransactionIsolationLevelForSession(): string
{
try {
return $this->fetchOne('SELECT @@TX_ISOLATION');
} catch (Exception $e) {
return $this->fetchOne('SELECT @@transaction_isolation');
}
}

public function setTransactionIsolationLevel(string $level): void
{
$this->query("SET SESSION TRANSACTION ISOLATION LEVEL $level");
}
}
2 changes: 1 addition & 1 deletion core/Db/AdapterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

/**
*/
interface AdapterInterface
interface AdapterInterface extends TransactionalDatabaseInterface
{
/**
* Reset the configuration variables in this adapter.
Expand Down
35 changes: 14 additions & 21 deletions core/Db/TransactionLevel.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,13 @@ class TransactionLevel
private $statusBackup;

/**
* @var \Piwik\Tracker\Db|\Piwik\Db\AdapterInterface|\Piwik\Db $db
* @var TransactionalDatabaseInterface $transactionalDatabase
*/
private $db;
private $transactionalDatabase;

/**
* @param \Piwik\Tracker\Db|\Piwik\Db\AdapterInterface|\Piwik\Db $db
*/
public function __construct($db)
public function __construct(TransactionalDatabaseInterface $transactionalDatabase)
{
$this->db = $db;
$this->transactionalDatabase = $transactionalDatabase;
}

public function canLikelySetTransactionLevel()
Expand All @@ -48,38 +45,34 @@ public function setUncommitted()

public function setTransactionLevelForNonLockingReads(): bool
{
if ($this->db->supportsUncommitted === false) {
if ($this->transactionalDatabase->getSupportsTransactionLevelForNonLockingReads() === false) {
// we know "Uncommitted" transaction level is not supported, we don't need to do anything as it won't work to set the status
return false;
}

try {
$backup = $this->db->fetchOne('SELECT @@TX_ISOLATION');
$backup = $this->transactionalDatabase->getCurrentTransactionIsolationLevelForSession();
} catch (\Exception $e) {
try {
$backup = $this->db->fetchOne('SELECT @@transaction_isolation');
} catch (\Exception $e) {
$this->db->supportsUncommitted = false;
return false;
}
$this->transactionalDatabase->setSupportsTransactionLevelForNonLockingReads(false);
return false;
}

try {
$this->db->query('SET SESSION TRANSACTION ISOLATION LEVEL ' . Schema::getInstance()->getSupportedReadIsolationTransactionLevel());
$this->transactionalDatabase->setTransactionIsolationLevel(Schema::getInstance()->getSupportedReadIsolationTransactionLevel());

$this->statusBackup = $backup;

if ($this->db->supportsUncommitted === null) {
if ($this->transactionalDatabase->getSupportsTransactionLevelForNonLockingReads() === null) {
// the first time we need to check if the transaction level actually works by
// trying to set something w/ the new transaction isolation level
Option::set(self::TEST_OPTION_NAME, '1');
}

$this->db->supportsUncommitted = true;
$this->transactionalDatabase->setSupportsTransactionLevelForNonLockingReads(true);
} catch (\Exception $e) {
// setting the transaction level status did not work
// catch eg 1665 Cannot execute statement: impossible to write to binary log since BINLOG_FORMAT = STATEMENT and at least one table uses a storage engine limited to row-based logging. InnoDB is limited to row-logging when transaction isolation level is READ COMMITTED or READ UNCOMMITTED
$this->db->supportsUncommitted = false;
$this->transactionalDatabase->setSupportsTransactionLevelForNonLockingReads(false);
$this->restorePreviousStatus();
return false;
}
Expand All @@ -95,9 +88,9 @@ public function restorePreviousStatus()

$value = str_replace('-', ' ', $value);
if (in_array($value, array('REPEATABLE READ', 'READ COMMITTED', 'SERIALIZABLE'))) {
$this->db->query('SET SESSION TRANSACTION ISOLATION LEVEL ' . $value);
$this->transactionalDatabase->setTransactionIsolationLevel($value);
} elseif ($value !== 'READ UNCOMMITTED') {
$this->db->query('SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ');
$this->transactionalDatabase->setTransactionIsolationLevel('REPEATABLE READ');
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions core/Db/TransactionalDatabaseInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Piwik\Db;

interface TransactionalDatabaseInterface
{
public function getCurrentTransactionIsolationLevelForSession(): string;
public function setTransactionIsolationLevel(string $level): void;
public function setSupportsTransactionLevelForNonLockingReads(bool $supports = null): void;
public function getSupportsTransactionLevelForNonLockingReads(): ?bool;
}
30 changes: 27 additions & 3 deletions core/Tracker/Db.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Piwik\Common;
use Piwik\Config;
use Piwik\Db\Adapter;
use Piwik\Db\TransactionalDatabaseInterface;
use Piwik\Piwik;
use Piwik\Timer;
use Piwik\Tracker\Db\DbException;
Expand All @@ -23,16 +24,15 @@
* We wrote this simple class
*
*/
abstract class Db
abstract class Db implements TransactionalDatabaseInterface
{
protected static $profiling = false;

protected $queriesProfiling = array();

protected $connection = null;

// this is used for indicate TransactionLevel Cache
public $supportsUncommitted = null;
protected $supportsTransactionLevelForNonLockingReads;

/**
* Enables the SQL profiling.
Expand Down Expand Up @@ -301,4 +301,28 @@ public static function connectPiwikTrackerDb()
}
return $db;
}

public function getCurrentTransactionIsolationLevelForSession(): string
{
try {
return self::fetchOne('SELECT @@TX_ISOLATION');
} catch (Exception $e) {
return self::fetchOne('SELECT @@transaction_isolation');
}
}

public function setTransactionIsolationLevel(string $level): void
{
self::query("SET SESSION TRANSACTION ISOLATION LEVEL $level");
}

public function setSupportsTransactionLevelForNonLockingReads(bool $supports = null): void
{
$this->supportsTransactionLevelForNonLockingReads = $supports;
}

public function getSupportsTransactionLevelForNonLockingReads(): ?bool
{
return $this->supportsTransactionLevelForNonLockingReads;
}
}
4 changes: 2 additions & 2 deletions tests/PHPUnit/Integration/DataAccess/LogAggregatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,13 @@ public function testGenerateQuerySwitchesSupportsUncommittedToTrueWhenSupports()

$db = Db::get();

$db->supportsUncommitted = null;
$db->setSupportsTransactionLevelForNonLockingReads(null);

$this->logAggregator->generateQuery('test, test2', 'log_visit', '1=1', false, '5');

$this->setSqlRequirePrimaryKeySetting(0);

$this->assertTrue($db->supportsUncommitted);
$this->assertTrue($db->getSupportsTransactionLevelForNonLockingReads());
}

public function testGetSegmentTmpTableName()
Expand Down
Loading