diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index 8b6cd837a..e1b4a1d79 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -154,6 +154,7 @@ class CMDBSource /** @var mysqli $m_oMysqli */ protected static $m_oMysqli; + protected static $oMySQLiForQuery; /** * @var int number of level for nested transactions : 0 if no transaction was ever opened, +1 for each 'START TRANSACTION' sent @@ -220,6 +221,7 @@ class CMDBSource self::$m_sDBTlsCA = empty($sTlsCA) ? null : $sTlsCA; self::$m_oMysqli = self::GetMysqliInstance($sServer, $sUser, $sPwd, $sSource, $bTlsEnabled, $sTlsCA, true); + self::SetMySQLiForQuery(self::$m_oMysqli); } /** @@ -237,8 +239,6 @@ class CMDBSource public static function GetMysqliInstance( $sDbHost, $sUser, $sPwd, $sSource = '', $bTlsEnabled = false, $sTlsCa = null, $bCheckTlsAfterConnection = false ) { - $oMysqli = null; - $sServer = null; $iPort = null; self::InitServerAndPort($sDbHost, $sServer, $iPort); @@ -536,6 +536,24 @@ class CMDBSource 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; + } + public static function GetErrNo() { if (self::$m_oMysqli->errno != 0) @@ -671,7 +689,7 @@ class CMDBSource */ private static function DBQuery($sSql) { - $sShortSQL = str_replace("\n", " ", substr($sSql, 0, 120)); + $sShortSQL = substr(preg_replace("/\s+/", " ", substr($sSql, 0, 180)), 0, 150); if (substr_compare($sShortSQL, "SELECT", 0, strlen("SELECT")) !== 0) { IssueLog::Trace("$sShortSQL", 'cmdbsource'); } @@ -679,7 +697,7 @@ class CMDBSource $oKPI = new ExecutionKPI(); try { - $oResult = self::$m_oMysqli->query($sSql); + $oResult = self::GetMySQLiForQuery()->query($sSql); } catch (mysqli_sql_exception $e) { @@ -959,7 +977,7 @@ class CMDBSource $oKPI = new ExecutionKPI(); try { - $oResult = self::$m_oMysqli->query($sSql); + $oResult = self::GetMySQLiForQuery()->query($sSql); } catch(mysqli_sql_exception $e) { @@ -999,7 +1017,7 @@ class CMDBSource $oKPI = new ExecutionKPI(); try { - $oResult = self::$m_oMysqli->query($sSql); + $oResult = self::GetMySQLiForQuery()->query($sSql); } catch(mysqli_sql_exception $e) { @@ -1081,7 +1099,7 @@ class CMDBSource { try { - $oResult = self::$m_oMysqli->query($sSql); + $oResult = self::GetMySQLiForQuery()->query($sSql); } catch(mysqli_sql_exception $e) { @@ -1568,7 +1586,7 @@ class CMDBSource $sSql = "SELECT * FROM `$sTable`"; try { - $oResult = self::$m_oMysqli->query($sSql); + $oResult = self::GetMySQLiForQuery()->query($sSql); } catch(mysqli_sql_exception $e) { diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 6a4d23193..63914e0f7 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -2776,6 +2776,7 @@ abstract class DBObject implements iDisplay $this->DBWriteLinks(); $this->WriteExternalAttributes(); + $this->RecordObjCreation(); if ($bIsTransactionEnabled) { CMDBSource::Query('COMMIT'); @@ -2837,8 +2838,6 @@ abstract class DBObject implements iDisplay } } - $this->RecordObjCreation(); - return $this->m_iKey; } @@ -3250,13 +3249,6 @@ abstract class DBObject implements iDisplay $this->DBWriteLinks(); $this->WriteExternalAttributes(); - // following lines are resetting changes (so after this {@see DBObject::ListChanges()} won't return changes anymore) - // new values are already in the object (call {@see DBObject::Get()} to get them) - // call {@see DBObject::ListPreviousValuesForUpdatedAttributes()} to get changed fields and previous values - $this->m_bDirty = false; - $this->m_aTouchedAtt = array(); - $this->m_aModifiedAtt = array(); - if (count($aChanges) != 0) { $this->RecordAttChanges($aChanges, $aOriginalValues); @@ -3322,6 +3314,13 @@ abstract class DBObject implements iDisplay } } + // following lines are resetting changes (so after this {@see DBObject::ListChanges()} won't return changes anymore) + // new values are already in the object (call {@see DBObject::Get()} to get them) + // call {@see DBObject::ListPreviousValuesForUpdatedAttributes()} to get changed fields and previous values + $this->m_bDirty = false; + $this->m_aTouchedAtt = array(); + $this->m_aModifiedAtt = array(); + try { $this->AfterUpdate(); diff --git a/test/core/CMDBSourceTest.php b/test/core/CMDBSource/CMDBSourceTest.php similarity index 93% rename from test/core/CMDBSourceTest.php rename to test/core/CMDBSource/CMDBSourceTest.php index 229db18b5..a787c0316 100644 --- a/test/core/CMDBSourceTest.php +++ b/test/core/CMDBSource/CMDBSourceTest.php @@ -113,11 +113,6 @@ class CMDBSourceTest extends ItopTestCase "ENUM('value 1 (with parenthesis)','value 2') CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci", "enum('value 1 (with parenthesis)','value 3') CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci", ), - 'ENUM with different values, containing parenthesis' => array( - false, - "ENUM('value 1 ) with parenthesis)','value 2') CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci", - "enum('value 1 (with parenthesis)','value 3') CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci", - ), ); } } diff --git a/test/core/CMDBSource/DeadLockInjection.php b/test/core/CMDBSource/DeadLockInjection.php new file mode 100644 index 000000000..12aa2e4c6 --- /dev/null +++ b/test/core/CMDBSource/DeadLockInjection.php @@ -0,0 +1,63 @@ +bMustFail = true; + $this->iRequestCount = 0; + $this->iFailAt = $iFailAt; + $this->bShowRequest = true; + } + + /** + * @param bool $bShowRequest + */ + public function SetShowRequest($bShowRequest) + { + $this->bShowRequest = $bShowRequest; + } + + + public function query($sSQL) + { + if (utils::StartsWith($sSQL, "SELECT")) { + return; + } + if ($this->bShowRequest) { + $sShortSQL = substr(preg_replace("/\s+/", " ", substr($sSQL, 0, 180)), 0, 150); + echo "$sShortSQL\n"; + } + if (CMDBSource::IsInsideTransaction() && $this->bMustFail) { + $this->iRequestCount++; + if ($this->iRequestCount == $this->iFailAt) { + echo "Generating a FAKE DEADLOCK\n"; + IssueLog::Trace("Generating a FAKE DEADLOCK", 'cmdbsource'); + throw new MySQLException("FAKE DEADLOCK", [], new Exception("FAKE DEADLOCK", 1213)); + } + } + } +} \ No newline at end of file diff --git a/test/core/CMDBSource/TransactionsTest.php b/test/core/CMDBSource/TransactionsTest.php new file mode 100644 index 000000000..82594e578 --- /dev/null +++ b/test/core/CMDBSource/TransactionsTest.php @@ -0,0 +1,247 @@ +oMySQLiMock = new DeadLockInjection(); + + $oMockMysqli = $this->getMockBuilder('mysqli') + ->setMethods(['query']) + ->getMock(); + $oMockMysqli->expects($this->any()) + ->method('query') + ->will($this->returnCallback( + function ($sSql) use ($oInitialMysqli) { + $this->oMySQLiMock->query($sSql); + return $oInitialMysqli->query($sSql); + } + )); + + $this->InvokeNonPublicStaticMethod('CMDBSource', 'SetMySQLiForQuery', [$oMockMysqli]); + } + + /** + * Test DBInsertNoReload database transaction by provoking deadlock exceptions + * + * @dataProvider DBInsertProvider + * + * @param int $iFailAt Specify the request occurrence that fails + * @param bool $bIsInDB Indicates if the object must have been created or not + * + * @throws \ArchivedObjectException + * @throws \CoreCannotSaveObjectException + * @throws \CoreException + * @throws \CoreUnexpectedValue + * @throws \DeleteException + * @throws \MySQLException + * @throws \MySQLHasGoneAwayException + * @throws \OQLException + */ + public function testDBInsert($iFailAt, $bIsInDB) + { + // Create a UserRequest with 2 contacts + $oTicket = MetaModel::NewObject('UserRequest', [ + 'ref' => 'Test Ticket', + 'title' => 'Create OK', + 'description' => 'Create OK', + 'caller_id' => 15, + 'org_id' => 3, + ]); + $oLinkSet = $oTicket->Get('contacts_list'); + $oLinkSet->AddItem(MetaModel::NewObject('lnkContactToTicket', ['contact_id' => 6])); + $oLinkSet->AddItem(MetaModel::NewObject('lnkContactToTicket', ['contact_id' => 7])); + + $this->oMySQLiMock->SetFailAt($iFailAt); + $this->debug("---> DBInsert()"); + try { + $oTicket->DBWrite(); + } + catch (Exception $e) { + // If an exception occurs must be a deadlock + $this->assertTrue(CMDBSource::IsDeadlockException($e), $e->getMessage()); + } + + // Verify if the ticket is considered as saved in the database + $this->assertEquals($bIsInDB, !$oTicket->IsNew()); + + if (!$oTicket->IsNew()) { + $this->oMySQLiMock->SetShowRequest(false); + // Delete created objects + $oTicket->DBDelete(); + } + } + + public function DBInsertProvider() + { + return [ + "Normal case" => ['iFailAt' => -1, 'bIsInDB' => true], + "ticket" => ['iFailAt' => 1, 'bIsInDB' => false], + "ticket_request" => ['iFailAt' => 2, 'bIsInDB' => false], + "priv_change" => ['iFailAt' => 3, 'bIsInDB' => false], + "priv_changeop" => ['iFailAt' => 4, 'bIsInDB' => false], + "priv_changeop_create" => ['iFailAt' => 5, 'bIsInDB' => false], + "History 4" => ['iFailAt' => 6, 'bIsInDB' => false], + "History 5" => ['iFailAt' => 7, 'bIsInDB' => false], + "History 6" => ['iFailAt' => 8, 'bIsInDB' => false], + "History 7" => ['iFailAt' => 9, 'bIsInDB' => false], + "History 8" => ['iFailAt' => 10, 'bIsInDB' => false], + "History 9" => ['iFailAt' => 11, 'bIsInDB' => false], + "History 10" => ['iFailAt' => 12, 'bIsInDB' => false], + "History 11" => ['iFailAt' => 13, 'bIsInDB' => false], + "History 12" => ['iFailAt' => 14, 'bIsInDB' => false], + "History 13" => ['iFailAt' => 15, 'bIsInDB' => false], + "History 14" => ['iFailAt' => 16, 'bIsInDB' => false], + "History 15" => ['iFailAt' => 17, 'bIsInDB' => false], + "History 16" => ['iFailAt' => 18, 'bIsInDB' => false], + "History 17" => ['iFailAt' => 19, 'bIsInDB' => false], + "History 18" => ['iFailAt' => 20, 'bIsInDB' => false], + "History 19" => ['iFailAt' => 21, 'bIsInDB' => false], + "History 20" => ['iFailAt' => 22, 'bIsInDB' => false], + "History 21" => ['iFailAt' => 23, 'bIsInDB' => false], + "History 22" => ['iFailAt' => 24, 'bIsInDB' => false], + "History 23" => ['iFailAt' => 25, 'bIsInDB' => false], + "History 24" => ['iFailAt' => 26, 'bIsInDB' => false], + "History 25" => ['iFailAt' => 27, 'bIsInDB' => false], + "History 26" => ['iFailAt' => 28, 'bIsInDB' => false], + "History 27" => ['iFailAt' => 29, 'bIsInDB' => false], + "History 28" => ['iFailAt' => 30, 'bIsInDB' => false], + "History 29" => ['iFailAt' => 31, 'bIsInDB' => false], + "History 30" => ['iFailAt' => 32, 'bIsInDB' => false], + "History 31" => ['iFailAt' => 33, 'bIsInDB' => false], + "History 32" => ['iFailAt' => 34, 'bIsInDB' => false], + "History 33" => ['iFailAt' => 35, 'bIsInDB' => false], + "History 34" => ['iFailAt' => 36, 'bIsInDB' => false], + "History 35" => ['iFailAt' => 37, 'bIsInDB' => false], + "History 36" => ['iFailAt' => 38, 'bIsInDB' => false], + "History 37" => ['iFailAt' => 39, 'bIsInDB' => false], + "History 38" => ['iFailAt' => 40, 'bIsInDB' => false], + ]; + } + + /** + * Test DBUpdate database transaction by provoking deadlock exceptions + * + * @dataProvider DBUpdateProvider + * @param $iFailAt + * + * @throws \ArchivedObjectException + * @throws \CoreCannotSaveObjectException + * @throws \CoreException + * @throws \CoreUnexpectedValue + * @throws \DeleteException + * @throws \MySQLException + * @throws \MySQLHasGoneAwayException + * @throws \OQLException + */ + public function testDBUpdate($iFailAt, $bIsModified) + { + // Create a UserRequest into the database with 2 contacts + $oTicket = MetaModel::NewObject('UserRequest', [ + 'ref' => 'Test Ticket', + 'title' => 'Create OK', + 'description' => 'Create OK', + 'solution' => 'Create OK', + 'caller_id' => 15, + 'org_id' => 3, + ]); + $oLinkSet = $oTicket->Get('contacts_list'); + $oLinkSet->AddItem(MetaModel::NewObject('lnkContactToTicket', ['contact_id' => 6])); + $oLinkSet->AddItem(MetaModel::NewObject('lnkContactToTicket', ['contact_id' => 7])); + //$oTicket->Set('contacts_list', $oLinkSet); + + $this->oMySQLiMock->SetShowRequest(false); + $oTicket->DBWrite(); + + // Verify that the object is considered as saved in the database + $this->assertEquals(true, !$oTicket->IsNew()); + + // Reload from db + $oTicket = MetaModel::GetObject('UserRequest', $oTicket->GetKey()); + $oTicket->Set('description', 'Update OK'); + $oTicket->Set('solution', 'Test OK'); + $oLinkSet = $oTicket->Get('contacts_list'); + $oLinkSet->AddItem(MetaModel::NewObject('lnkContactToTicket', ['contact_id' => 8])); + $oTicket->Set('contacts_list', $oLinkSet); + + // Provoke an error during the update + $this->oMySQLiMock->SetFailAt($iFailAt); + $this->debug("---> DBUpdate()"); + try { + $oTicket->DBWrite(); + } + catch (Exception $e) { + // If an exception occurs must be a deadlock + $this->assertTrue(CMDBSource::IsDeadlockException($e)); + } + + // Verify if the ticket is considered as saved in the database + $this->assertEquals($bIsModified, $oTicket->IsModified()); + + // Reload from db after the update to check the value present in the database + $oTicket = MetaModel::GetObject('UserRequest', $oTicket->GetKey()); + if ($bIsModified) { + $this->assertEquals('Create OK', $oTicket->Get('solution')); + } else { + $this->assertEquals('Test OK', $oTicket->Get('solution')); + } + + if (!$oTicket->IsNew()) { + $this->oMySQLiMock->SetShowRequest(false); + // Delete created objects + $oTicket->DBDelete(); + } + } + + public function DBUpdateProvider() + { + return [ + "Normal case" => ['iFailAt' => -1, 'bIsModified' => false], + "ticket_request" => ['iFailAt' => 1, 'bIsModified' => true], + "lnkcontacttoticket" => ['iFailAt' => 2, 'bIsModified' => true], + "History 1" => ['iFailAt' => 3, 'bIsModified' => true], + "History 2" => ['iFailAt' => 4, 'bIsModified' => true], + "History 3" => ['iFailAt' => 5, 'bIsModified' => true], + "History 4" => ['iFailAt' => 6, 'bIsModified' => true], + "History 5" => ['iFailAt' => 7, 'bIsModified' => true], + "History 6" => ['iFailAt' => 8, 'bIsModified' => true], + "History 7" => ['iFailAt' => 9, 'bIsModified' => true], + "History 8" => ['iFailAt' => 10, 'bIsModified' => true], + "History 9" => ['iFailAt' => 11, 'bIsModified' => true], + "History 10" => ['iFailAt' => 12, 'bIsModified' => true], + "History 11" => ['iFailAt' => 13, 'bIsModified' => true], + "History 12" => ['iFailAt' => 14, 'bIsModified' => true], + "History 13" => ['iFailAt' => 15, 'bIsModified' => true], + ]; + } +} \ No newline at end of file