composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-05-17 06:18:44 +02:00
Code Issues Packages Projects Releases Wiki Activity

♻️ N°4325 refactor CMDBSource mysqli attributes to a separate wrapper class (#237)

Browse Source 
In 2.7.5 with N°3513 we added a second mysqli attribute in CMDBSource, so that we can test transactions (see TransactionsTest).

But this wasn't documented, and was really causing confusion !

This refactor wraps both attributes in a dedicated object so that the logic is clearer.
This commit is contained in:
Pierre Goiffon
2021-09-24 11:45:39 +02:00
committed by GitHub
parent 86538cf88e
commit 4cf4c0e4c3
7 changed files with 121 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
core/DbConnectionWrapper.php Normal file
View File

@@ -0,0 +1,67 @@
<?php
/*
* @copyright Copyright (C) 2010-2021 Combodo SARL
* @license http://opensource.org/licenses/AGPL-3.0
*/
namespace Combodo\iTop\Core;
use mysqli;
/**
* mysqli object is really hard to mock as it contains lots of attributes & methods ! Thought we need to mock it to test transactions !
*
* To solve this, a new attribute exists and is only used in specific use cases, so there are just few things to mock.
*
* This object adds more readability than previous model with 2 attributes in {@see CMDBSource}.
*
* @used-by \CMDBSource
*
* @since 3.0.0 N°4325 Object creation
* This wrapper handles the 2 {@mysqli myqsli} attributes that were previously in {@see CMDBSource}
* To allow testing we added a second mysqli object (N°3513 in 2.7.5) and code became a bit confusing :/
* With this wrapper everything is in the same place, and we can express the intention more clearly !
*/
class DbConnectionWrapper
{
/** @var mysqli */
protected static $oDbCnxStandard;
/**
* Can contain a genuine mysqli object, or a mock that would emulate {@see mysqli::query()}
*
* @var mysqli
* @used-by \Combodo\iTop\Test\UnitTest\Core\TransactionsTest
*/
protected static $oDbCnxMockableForQuery;
/**
* @param bool $bIsForQuery set to true if using {@see mysqli::query()}
*
* @return \mysqli|null
*/
public static function GetDbConnection(bool $bIsForQuery = false): ?mysqli
{
if ($bIsForQuery) {
return static::$oDbCnxMockableForQuery;
}
return static::$oDbCnxStandard;
}
public static function SetDbConnection(mysqli $oMysqli): void
{
static::$oDbCnxStandard = $oMysqli;
static::SetDbConnectionMockForQuery($oMysqli);
}
/**
* Use this to register a mock that will handle {@see mysqli::query()}
*
* @param \mysqli $oMysqli
*/
public static function SetDbConnectionMockForQuery(mysqli $oMysqli): void
{
static::$oDbCnxMockableForQuery = $oMysqli;
}
}

118
core/cmdbsource.class.inc.php
View File

@@ -24,13 +24,15 @@
* @license http://opensource.org/licenses/AGPL-3.0
*/
use Combodo\iTop\Core\DbConnectionWrapper;
require_once('MyHelpers.class.inc.php');
require_once(APPROOT.'core/kpi.class.inc.php');
/**
* CMDBSource
* database access wrapper
* database access wrapper
*
* @package iTopORM
*/
@@ -66,11 +68,6 @@ class CMDBSource
*/
protected static $m_sDBTlsCA;
/** @var mysqli $m_oMysqli */
protected static $m_oMysqli;
/** @var mysqli or mock used for test purpose, only used in query() method */
protected static $oMySQLiForQuery;
/**
* @var int number of level for nested transactions : 0 if no transaction was ever opened, +1 for each 'START TRANSACTION' sent
* @since 2.7.0 N°679
@@ -135,8 +132,8 @@ class CMDBSource
self::$m_bDBTlsEnabled = empty($bTlsEnabled) ? false : $bTlsEnabled;
self::$m_sDBTlsCA = empty($sTlsCA) ? null : $sTlsCA;
self::$m_oMysqli = self::GetMysqliInstance($sServer, $sUser, $sPwd, $sSource, $bTlsEnabled, $sTlsCA, true);
self::SetMySQLiForQuery(self::$m_oMysqli);
$oMysqli = self::GetMysqliInstance($sServer, $sUser, $sPwd, $sSource, $bTlsEnabled, $sTlsCA, true);
DbConnectionWrapper::SetDbConnection($oMysqli);
}
/**
@@ -345,7 +342,8 @@ class CMDBSource
{
// In case we don't have rights to enumerate the databases
// Let's try to connect directly
return @((bool)self::$m_oMysqli->query("USE `$sSource`"));
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
return @((bool)DbConnectionWrapper::GetDbConnection(true)->query("USE `$sSource`"));
}
}
@@ -361,7 +359,7 @@ class CMDBSource
*/
public static function GetServerInfo()
{
return mysqli_get_server_info ( self::$m_oMysqli );
return mysqli_get_server_info(DbConnectionWrapper::GetDbConnection());
}
/**
@@ -392,9 +390,9 @@ class CMDBSource
*/
public static function SelectDB($sSource)
{
if (!((bool)self::$m_oMysqli->query("USE `$sSource`")))
{
throw new MySQLException('Could not select DB', array('db_name'=>$sSource));
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
if (!((bool)DbConnectionWrapper::GetDbConnection(true)->query("USE `$sSource`"))) {
throw new MySQLException('Could not select DB', array('db_name' => $sSource));
}
self::$m_sDBName = $sSource;
}
@@ -448,48 +446,24 @@ class CMDBSource
*/
public static function GetMysqli()
{
return self::$m_oMysqli;
}
/**
* @return
*/
private static function GetMySQLiForQuery()
{
return self::$oMySQLiForQuery;
}
/**
* Used for test purpose (mysqli mock)
* @param $oMySQLi
*/
private static function SetMySQLiForQuery($oMySQLi)
{
self::$oMySQLiForQuery = $oMySQLi;
return DbConnectionWrapper::GetDbConnection(false);
}
public static function GetErrNo()
{
if (self::$m_oMysqli->errno != 0)
{
return self::$m_oMysqli->errno;
}
else
{
return self::$m_oMysqli->connect_errno;
if (DbConnectionWrapper::GetDbConnection()->errno != 0) {
return DbConnectionWrapper::GetDbConnection()->errno;
} else {
return DbConnectionWrapper::GetDbConnection()->connect_errno;
}
}
public static function GetError()
{
if (self::$m_oMysqli->error != '')
{
return self::$m_oMysqli->error;
}
else
{
return self::$m_oMysqli->connect_error;
if (DbConnectionWrapper::GetDbConnection()->error != '') {
return DbConnectionWrapper::GetDbConnection()->error;
} else {
return DbConnectionWrapper::GetDbConnection()->connect_error;
}
}
@@ -529,7 +503,8 @@ class CMDBSource
// Quote if not a number or a numeric string
if ($bAlways || is_string($value))
{
$value = $cQuoteStyle . self::$m_oMysqli->real_escape_string($value) . $cQuoteStyle;
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$value = $cQuoteStyle.DbConnectionWrapper::GetDbConnection()->real_escape_string($value).$cQuoteStyle;
}
return $value;
}
@@ -612,26 +587,25 @@ class CMDBSource
$oKPI = new ExecutionKPI();
try
{
$oResult = self::GetMySQLiForQuery()->query($sSql);
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$oResult = DbConnectionWrapper::GetDbConnection(true)->query($sSql);
}
catch (mysqli_sql_exception $e)
{
self::LogDeadLock($e);
self::LogDeadLock($e, true);
throw new MySQLException('Failed to issue SQL query', array('query' => $sSql, $e));
}
$oKPI->ComputeStats('Query exec (mySQL)', $sSql);
if ($oResult === false)
{
if ($oResult === false) {
$aContext = array('query' => $sSql);
$iMySqlErrorNo = self::$m_oMysqli->errno;
$iMySqlErrorNo = DbConnectionWrapper::GetDbConnection(true)->errno;
$aMySqlHasGoneAwayErrorCodes = MySQLHasGoneAwayException::getErrorCodes();
if (in_array($iMySqlErrorNo, $aMySqlHasGoneAwayErrorCodes))
{
if (in_array($iMySqlErrorNo, $aMySqlHasGoneAwayErrorCodes)) {
throw new MySQLHasGoneAwayException(self::GetError(), $aContext);
}
$e = new MySQLException('Failed to issue SQL query', $aContext);
self::LogDeadLock($e);
self::LogDeadLock($e, true);
throw $e;
}
@@ -640,23 +614,24 @@ class CMDBSource
/**
* @param \Exception $e
* @param bool $bForQuery to get the proper DB connection
*
* @since 2.7.1
* @since 3.0.0 N°4325 add new optional parameter to use the correct DB connection
*/
private static function LogDeadLock(Exception $e)
private static function LogDeadLock(Exception $e, $bForQuery = false)
{
// checks MySQL error code
$iMySqlErrorNo = self::$m_oMysqli->errno;
if (!in_array($iMySqlErrorNo, array(self::MYSQL_ERRNO_WAIT_TIMEOUT, self::MYSQL_ERRNO_DEADLOCK)))
{
$iMySqlErrorNo = DbConnectionWrapper::GetDbConnection($bForQuery)->errno;
if (!in_array($iMySqlErrorNo, array(self::MYSQL_ERRNO_WAIT_TIMEOUT, self::MYSQL_ERRNO_DEADLOCK))) {
return;
}
// Get error info
$sUser = UserRights::GetUser();
$oError = self::$m_oMysqli->query('SHOW ENGINE INNODB STATUS');
if ($oError !== false)
{
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$oError = DbConnectionWrapper::GetDbConnection(true)->query('SHOW ENGINE INNODB STATUS');
if ($oError !== false) {
$aData = $oError->fetch_all(MYSQLI_ASSOC);
$sInnodbStatus = $aData[0];
}
@@ -829,7 +804,7 @@ class CMDBSource
public static function GetInsertId()
{
$iRes = self::$m_oMysqli->insert_id;
$iRes = DbConnectionWrapper::GetDbConnection()->insert_id;
if (is_null($iRes))
{
return 0;
@@ -871,7 +846,8 @@ class CMDBSource
$oKPI = new ExecutionKPI();
try
{
$oResult = self::GetMySQLiForQuery()->query($sSql);
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$oResult = DbConnectionWrapper::GetDbConnection(true)->query($sSql);
}
catch(mysqli_sql_exception $e)
{
@@ -912,7 +888,8 @@ class CMDBSource
$oKPI = new ExecutionKPI();
try
{
$oResult = self::GetMySQLiForQuery()->query($sSql);
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$oResult = DbConnectionWrapper::GetDbConnection(true)->query($sSql);
}
catch(mysqli_sql_exception $e)
{
@@ -962,7 +939,8 @@ class CMDBSource
$aData = array();
try
{
$oResult = self::$m_oMysqli->query($sSql);
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$oResult = DbConnectionWrapper::GetDbConnection(true)->query($sSql);
}
catch(mysqli_sql_exception $e)
{
@@ -994,7 +972,8 @@ class CMDBSource
{
try
{
$oResult = self::GetMySQLiForQuery()->query($sSql);
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$oResult = DbConnectionWrapper::GetDbConnection(true)->query($sSql);
}
catch(mysqli_sql_exception $e)
{
@@ -1019,7 +998,7 @@ class CMDBSource
public static function AffectedRows()
{
return self::$m_oMysqli->affected_rows;
return DbConnectionWrapper::GetDbConnection()->affected_rows;
}
public static function FetchArray($oResult)
@@ -1482,7 +1461,8 @@ class CMDBSource
$sSql = "SELECT * FROM `$sTable`";
try
{
$oResult = self::GetMySQLiForQuery()->query($sSql);
/** @noinspection NullPointerExceptionInspection this shouldn't be called with un-init DB */
$oResult = DbConnectionWrapper::GetDbConnection(true)->query($sSql);
}
catch(mysqli_sql_exception $e)
{

4
lib/composer/ClassLoader.php
View File

@@ -420,7 +420,7 @@ class ClassLoader
* Loads the given class or interface.
*
* @param string $class The name of the class
* @return true|null True if loaded, null otherwise
* @return bool|null True if loaded, null otherwise
*/
public function loadClass($class)
{
@@ -429,8 +429,6 @@ class ClassLoader
return true;
}
return null;
}
/**

1
lib/composer/autoload_classmap.php
View File

@@ -299,6 +299,7 @@ return array(
'Combodo\\iTop\\Controller\\Base\\Layout\\ActivityPanelController' => $baseDir . '/sources/Controller/Base/Layout/ActivityPanelController.php',
'Combodo\\iTop\\Controller\\PreferencesController' => $baseDir . '/sources/Controller/PreferencesController.php',
'Combodo\\iTop\\Core\\CMDBChange\\CMDBChangeOrigin' => $baseDir . '/sources/Core/CMDBChange/CMDBChangeOrigin.php',
'Combodo\\iTop\\Core\\DbConnectionWrapper' => $baseDir . '/core/DbConnectionWrapper.php',
'Combodo\\iTop\\Core\\MetaModel\\FriendlyNameType' => $baseDir . '/sources/Core/MetaModel/FriendlyNameType.php',
'Combodo\\iTop\\DesignDocument' => $baseDir . '/core/designdocument.class.inc.php',
'Combodo\\iTop\\DesignElement' => $baseDir . '/core/designdocument.class.inc.php',

1
lib/composer/autoload_static.php
View File

@@ -529,6 +529,7 @@ class ComposerStaticInit0018331147de7601e7552f7da8e3bb8b
'Combodo\\iTop\\Controller\\Base\\Layout\\ActivityPanelController' => __DIR__ . '/../..' . '/sources/Controller/Base/Layout/ActivityPanelController.php',
'Combodo\\iTop\\Controller\\PreferencesController' => __DIR__ . '/../..' . '/sources/Controller/PreferencesController.php',
'Combodo\\iTop\\Core\\CMDBChange\\CMDBChangeOrigin' => __DIR__ . '/../..' . '/sources/Core/CMDBChange/CMDBChangeOrigin.php',
'Combodo\\iTop\\Core\\DbConnectionWrapper' => __DIR__ . '/../..' . '/core/DbConnectionWrapper.php',
'Combodo\\iTop\\Core\\MetaModel\\FriendlyNameType' => __DIR__ . '/../..' . '/sources/Core/MetaModel/FriendlyNameType.php',
'Combodo\\iTop\\DesignDocument' => __DIR__ . '/../..' . '/core/designdocument.class.inc.php',
'Combodo\\iTop\\DesignElement' => __DIR__ . '/../..' . '/core/designdocument.class.inc.php',

16
lib/composer/platform_check.php
View File

@@ -8,22 +8,6 @@ if (!(PHP_VERSION_ID >= 70103)) {
$issues[] = 'Your Composer dependencies require a PHP version ">= 7.1.3". You are running ' . PHP_VERSION . '.';
}
$missingExtensions = array();
extension_loaded('ctype') || $missingExtensions[] = 'ctype';
extension_loaded('dom') || $missingExtensions[] = 'dom';
extension_loaded('gd') || $missingExtensions[] = 'gd';
extension_loaded('iconv') || $missingExtensions[] = 'iconv';
extension_loaded('json') || $missingExtensions[] = 'json';
extension_loaded('libxml') || $missingExtensions[] = 'libxml';
extension_loaded('mysqli') || $missingExtensions[] = 'mysqli';
extension_loaded('soap') || $missingExtensions[] = 'soap';
extension_loaded('tokenizer') || $missingExtensions[] = 'tokenizer';
extension_loaded('xml') || $missingExtensions[] = 'xml';
if ($missingExtensions) {
$issues[] = 'Your Composer dependencies require the following PHP extensions to be installed: ' . implode(', ', $missingExtensions) . '.';
}
if ($issues) {
if (!headers_sent()) {
header('HTTP/1.1 500 Internal Server Error');

3
test/core/CMDBSource/TransactionsTest.php
View File

@@ -7,6 +7,7 @@
namespace Combodo\iTop\Test\UnitTest\Core;
use CMDBSource;
use Combodo\iTop\Core\DbConnectionWrapper;
use Combodo\iTop\Test\UnitTest\ItopTestCase;
use Exception;
use MetaModel;
@@ -48,7 +49,7 @@ class TransactionsTest extends ItopTestCase
}
));
$this->InvokeNonPublicStaticMethod('CMDBSource', 'SetMySQLiForQuery', [$oMockMysqli]);
DbConnectionWrapper::SetDbConnectionMockForQuery($oMockMysqli);
}
/**