diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index f52c6a69b..8b6cd837a 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -671,6 +671,11 @@ class CMDBSource */ private static function DBQuery($sSql) { + $sShortSQL = str_replace("\n", " ", substr($sSql, 0, 120)); + if (substr_compare($sShortSQL, "SELECT", 0, strlen("SELECT")) !== 0) { + IssueLog::Trace("$sShortSQL", 'cmdbsource'); + } + $oKPI = new ExecutionKPI(); try { @@ -754,10 +759,15 @@ class CMDBSource */ private static function StartTransaction() { + $aStackTrace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT , 3); + $sCaller = 'From '.$aStackTrace[1]['file'].'('.$aStackTrace[1]['line'].'): '.$aStackTrace[2]['class'].'->'.$aStackTrace[2]['function'].'()'; $bHasExistingTransactions = self::IsInsideTransaction(); if (!$bHasExistingTransactions) { + IssueLog::Trace("START TRANSACTION $sCaller", 'cmdbsource'); self::DBQuery('START TRANSACTION'); + } else { + IssueLog::Trace("Ignore nested (".self::$m_iTransactionLevel.") START TRANSACTION $sCaller", 'cmdbsource'); } self::AddTransactionLevel(); @@ -775,9 +785,12 @@ class CMDBSource */ private static function Commit() { + $aStackTrace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT , 3); + $sCaller = 'From '.$aStackTrace[1]['file'].'('.$aStackTrace[1]['line'].'): '.$aStackTrace[2]['class'].'->'.$aStackTrace[2]['function'].'()'; if (!self::IsInsideTransaction()) { // should not happen ! + IssueLog::Error("No Transaction COMMIT $sCaller", 'cmdbsource'); throw new MySQLNoTransactionException('Trying to commit transaction whereas none have been started !', null); } @@ -785,8 +798,10 @@ class CMDBSource if (self::IsInsideTransaction()) { + IssueLog::Trace("Ignore nested (".self::$m_iTransactionLevel.") COMMIT $sCaller", 'cmdbsource'); return; } + IssueLog::Trace("COMMIT $sCaller", 'cmdbsource'); self::DBQuery('COMMIT'); } @@ -805,17 +820,22 @@ class CMDBSource */ private static function Rollback() { + $aStackTrace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT , 3); + $sCaller = 'From '.$aStackTrace[1]['file'].'('.$aStackTrace[1]['line'].'): '.$aStackTrace[2]['class'].'->'.$aStackTrace[2]['function'].'()'; if (!self::IsInsideTransaction()) { // should not happen ! + IssueLog::Error("No Transaction ROLLBACK $sCaller", 'cmdbsource'); throw new MySQLNoTransactionException('Trying to commit transaction whereas none have been started !', null); } self::RemoveLastTransactionLevel(); if (self::IsInsideTransaction()) { + IssueLog::Trace("Ignore nested (".self::$m_iTransactionLevel.") ROLLBACK $sCaller", 'cmdbsource'); return; } + IssueLog::Trace("ROLLBACK $sCaller", 'cmdbsource'); self::DBQuery('ROLLBACK'); } @@ -865,6 +885,17 @@ class CMDBSource self::$m_iTransactionLevel = 0; } + public static function IsDeadlockException(Exception $e) + { + while ($e instanceof Exception) { + if (($e instanceof MySQLException) && ($e->getCode() == 1213)) { + return true; + } + $e = $e->getPrevious(); + } + return false; + } + /** * * @deprecated 2.7.0 N°1627 use ItopCounter instead diff --git a/core/coreexception.class.inc.php b/core/coreexception.class.inc.php index cf6469807..a85417786 100644 --- a/core/coreexception.class.inc.php +++ b/core/coreexception.class.inc.php @@ -149,14 +149,14 @@ class CoreCannotSaveObjectException extends CoreException * * @param array $aContextData containing at least those keys : issues, id, class */ - public function __construct($aContextData) + public function __construct($aContextData, $oPrevious = null) { $this->aIssues = $aContextData['issues']; $this->iObjectId = $aContextData['id']; $this->sObjectClass = $aContextData['class']; $sIssues = implode(', ', $this->aIssues); - parent::__construct($sIssues, $aContextData); + parent::__construct($sIssues, $aContextData, '', $oPrevious); } /** diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 46efb0a4d..6a4d23193 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -2739,50 +2739,72 @@ abstract class DBObject implements iDisplay } } + $iTransactionRetry = 1; $bIsTransactionEnabled = MetaModel::GetConfig()->Get('db_core_transactions_enabled'); - try + if ($bIsTransactionEnabled) { - if ($bIsTransactionEnabled) - { - CMDBSource::Query('START TRANSACTION'); - } - - // First query built upon on the root class, because the ID must be created first - $this->m_iKey = $this->DBInsertSingleTable($sRootClass); - - // Then do the leaf class, if different from the root class - if ($sClass != $sRootClass) - { - $this->DBInsertSingleTable($sClass); - } - - // Then do the other classes - foreach (MetaModel::EnumParentClasses($sClass) as $sParentClass) - { - if ($sParentClass == $sRootClass) - { - continue; - } - $this->DBInsertSingleTable($sParentClass); - } - - $this->OnObjectKeyReady(); - - $this->DBWriteLinks(); - $this->WriteExternalAttributes(); - - if ($bIsTransactionEnabled) - { - CMDBSource::Query('COMMIT'); - } + // TODO Deep clone this object before the transaction (to use it in case of rollback) + // $iTransactionRetryCount = MetaModel::GetConfig()->Get('db_core_transactions_retry_count'); + $iTransactionRetryCount = 1; + $iTransactionRetryDelay = MetaModel::GetConfig()->Get('db_core_transactions_retry_delay_ms'); + $iTransactionRetry = $iTransactionRetryCount; } - catch (Exception $e) - { - if ($bIsTransactionEnabled) - { - CMDBSource::Query('ROLLBACK'); + while ($iTransactionRetry > 0) { + try { + $iTransactionRetry--; + if ($bIsTransactionEnabled) { + CMDBSource::Query('START TRANSACTION'); + } + + // First query built upon on the root class, because the ID must be created first + $this->m_iKey = $this->DBInsertSingleTable($sRootClass); + + // Then do the leaf class, if different from the root class + if ($sClass != $sRootClass) { + $this->DBInsertSingleTable($sClass); + } + + // Then do the other classes + foreach (MetaModel::EnumParentClasses($sClass) as $sParentClass) { + if ($sParentClass == $sRootClass) { + continue; + } + $this->DBInsertSingleTable($sParentClass); + } + + $this->OnObjectKeyReady(); + + $this->DBWriteLinks(); + $this->WriteExternalAttributes(); + + if ($bIsTransactionEnabled) { + CMDBSource::Query('COMMIT'); + } + break; + } + catch (Exception $e) { + IssueLog::Error($e->getMessage()); + if ($bIsTransactionEnabled) + { + CMDBSource::Query('ROLLBACK'); + if (!CMDBSource::IsInsideTransaction() && CMDBSource::IsDeadlockException($e)) + { + // Deadlock found when trying to get lock; try restarting transaction (only in main transaction) + if ($iTransactionRetry > 0) + { + // wait and retry + IssueLog::Error("Insert TRANSACTION Retrying..."); + usleep(random_int(1, 5) * 1000 * $iTransactionRetryDelay * ($iTransactionRetryCount - $iTransactionRetry)); + continue; + } + else + { + IssueLog::Error("Insert Deadlock TRANSACTION prevention failed."); + } + } + } + throw $e; } - throw $e; } $this->m_bIsInDB = true; @@ -3156,9 +3178,11 @@ abstract class DBObject implements iDisplay $bIsTransactionEnabled = MetaModel::GetConfig()->Get('db_core_transactions_enabled'); if ($bIsTransactionEnabled) { - $iIsTransactionRetryCount = MetaModel::GetConfig()->Get('db_core_transactions_retry_count'); + // TODO Deep clone this object before the transaction (to use it in case of rollback) + // $iTransactionRetryCount = MetaModel::GetConfig()->Get('db_core_transactions_retry_count'); + $iTransactionRetryCount = 1; $iIsTransactionRetryDelay = MetaModel::GetConfig()->Get('db_core_transactions_retry_delay_ms'); - $iTransactionRetry = $iIsTransactionRetryCount; + $iTransactionRetry = $iTransactionRetryCount; } while ($iTransactionRetry > 0) { @@ -3246,18 +3270,18 @@ abstract class DBObject implements iDisplay } catch (MySQLException $e) { + IssueLog::Error($e->getMessage()); if ($bIsTransactionEnabled) { CMDBSource::Query('ROLLBACK'); - if ($e->getCode() == 1213) + if (!CMDBSource::IsInsideTransaction() && CMDBSource::IsDeadlockException($e)) { - // Deadlock found when trying to get lock; try restarting transaction - IssueLog::Error($e->getMessage()); + // Deadlock found when trying to get lock; try restarting transaction (only in main transaction) if ($iTransactionRetry > 0) { // wait and retry IssueLog::Error("Update TRANSACTION Retrying..."); - usleep(random_int(1, 5) * 1000 * $iIsTransactionRetryDelay * ($iIsTransactionRetryCount - $iTransactionRetry)); + usleep(random_int(1, 5) * 1000 * $iIsTransactionRetryDelay * ($iTransactionRetryCount - $iTransactionRetry)); continue; } else @@ -3271,10 +3295,11 @@ abstract class DBObject implements iDisplay 'id' => $this->GetKey(), 'class' => get_class($this), 'issues' => $aErrors - )); + ), $e); } catch (CoreCannotSaveObjectException $e) { + IssueLog::Error($e->getMessage()); if ($bIsTransactionEnabled) { CMDBSource::Query('ROLLBACK'); @@ -3283,6 +3308,7 @@ abstract class DBObject implements iDisplay } catch (Exception $e) { + IssueLog::Error($e->getMessage()); if ($bIsTransactionEnabled) { CMDBSource::Query('ROLLBACK'); @@ -3494,9 +3520,11 @@ abstract class DBObject implements iDisplay $bIsTransactionEnabled = MetaModel::GetConfig()->Get('db_core_transactions_enabled'); if ($bIsTransactionEnabled) { - $iIsTransactionRetryCount = MetaModel::GetConfig()->Get('db_core_transactions_retry_count'); - $iIsTransactionRetryDelay = MetaModel::GetConfig()->Get('db_core_transactions_retry_delay_ms'); - $iTransactionRetry = $iIsTransactionRetryCount; + // TODO Deep clone this object before the transaction (to use it in case of rollback) + // $iTransactionRetryCount = MetaModel::GetConfig()->Get('db_core_transactions_retry_count'); + $iTransactionRetryCount = 1; + $iTransactionRetryDelay = MetaModel::GetConfig()->Get('db_core_transactions_retry_delay_ms'); + $iTransactionRetry = $iTransactionRetryCount; } while ($iTransactionRetry > 0) { @@ -3519,18 +3547,18 @@ abstract class DBObject implements iDisplay } catch (MySQLException $e) { + IssueLog::Error($e->getMessage()); if ($bIsTransactionEnabled) { CMDBSource::Query('ROLLBACK'); - if ($e->getCode() == 1213) + if (!CMDBSource::IsInsideTransaction() && CMDBSource::IsDeadlockException($e)) { // Deadlock found when trying to get lock; try restarting transaction - IssueLog::Error($e->getMessage()); if ($iTransactionRetry > 0) { // wait and retry IssueLog::Error("Delete TRANSACTION Retrying..."); - usleep(random_int(1, 5) * 1000 * $iIsTransactionRetryDelay * ($iIsTransactionRetryCount - $iTransactionRetry)); + usleep(random_int(1, 5) * 1000 * $iTransactionRetryDelay * ($iTransactionRetryCount - $iTransactionRetry)); continue; } else diff --git a/core/log.class.inc.php b/core/log.class.inc.php index 38a4a5e40..c8754dc1e 100644 --- a/core/log.class.inc.php +++ b/core/log.class.inc.php @@ -632,6 +632,8 @@ abstract class LogAPI } } + $sUser = UserRights::GetUserId(); + $sMessage = str_pad($sUser, 5)." | $sMessage"; static::$m_oFileLog->$sLevel($sMessage, $sChannel, $aContext); } diff --git a/datamodels/2.x/itop-portal-base/en.dict.itop-portal-base.php b/datamodels/2.x/itop-portal-base/en.dict.itop-portal-base.php index aa767f8b2..ae1243d5b 100644 --- a/datamodels/2.x/itop-portal-base/en.dict.itop-portal-base.php +++ b/datamodels/2.x/itop-portal-base/en.dict.itop-portal-base.php @@ -63,6 +63,8 @@ Dict::Add('EN US', 'English', 'English', array( 'Portal:File:DisplayInfo+' => '%1$s (%2$s) Open / Download', 'Portal:Calendar-FirstDayOfWeek' => 'en-us', //work with moment.js locales 'Portal:Form:Close:Warning' => 'Do you want to leave this form ? Data entered may be lost', + 'Portal:Error:ObjectCannotBeCreated' => 'Error: object cannot be created. Check associated objects and attachments before submitting again this form.', + 'Portal:Error:ObjectCannotBeUpdated' => 'Error: object cannot be updated. Check associated objects and attachments before submitting again this form.', )); // UserProfile brick diff --git a/datamodels/2.x/itop-portal-base/fr.dict.itop-portal-base.php b/datamodels/2.x/itop-portal-base/fr.dict.itop-portal-base.php index ccab6bcdf..db2104434 100644 --- a/datamodels/2.x/itop-portal-base/fr.dict.itop-portal-base.php +++ b/datamodels/2.x/itop-portal-base/fr.dict.itop-portal-base.php @@ -62,6 +62,8 @@ Dict::Add('FR FR', 'French', 'Français', array( 'Portal:File:DisplayInfo+' => '%1$s (%2$s) Ouvrir / Télécharger', 'Portal:Calendar-FirstDayOfWeek' => 'fr', //work with moment.js locales 'Portal:Form:Close:Warning' => 'Voulez-vous quitter ce formulaire ? Les données saisies seront perdues', + 'Portal:Error:ObjectCannotBeCreated' => 'Erreur: L\'objet n\'a pas été créé. Vérifiez les objets liés et les attachements avant de soumettre à nouveau le formulaire.', + 'Portal:Error:ObjectCannotBeUpdated' => 'Erreur: L\'objet n\'a pas été modifié. Vérifiez les objets liés et les attachements avant de soumettre à nouveau le formulaire.', )); // UserProfile brick diff --git a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php index d93f3debd..0944c0bbd 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php +++ b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php @@ -25,14 +25,12 @@ use AttributeDateTime; use AttributeTagSet; use CMDBChangeOpAttachmentAdded; use CMDBChangeOpAttachmentRemoved; -use CMDBSource; use Combodo\iTop\Form\Field\Field; use Combodo\iTop\Form\Field\FileUploadField; use Combodo\iTop\Form\Field\LabelField; use Combodo\iTop\Form\Form; use Combodo\iTop\Form\FormManager; use Combodo\iTop\Portal\Helper\ApplicationHelper; -use CoreCannotSaveObjectException; use DBObject; use DBObjectSearch; use DBObjectSet; @@ -1108,30 +1106,28 @@ class ObjectFormManager extends FormManager return $aData; } - // The try catch is essentially to start a MySQL transaction in order to ensure that all or none objects are persisted when creating an object with links - try - { - $sObjectClass = get_class($this->oObject); + $sObjectClass = get_class($this->oObject); - // Starting transaction - CMDBSource::Query('START TRANSACTION'); + try { // Forcing allowed writing on the object if necessary. This is used in some particular cases. $bAllowWrite = ($sObjectClass === 'Person' && $this->oObject->GetKey() == UserRights::GetContactId()); - if ($bAllowWrite) - { + if ($bAllowWrite) { $this->oObject->AllowWrite(true); } // Writing object to DB - $bActivateTriggers = (!$this->oObject->IsNew() && $this->oObject->IsModified()); + $bIsNew = $this->oObject->IsNew(); $bWasModified = $this->oObject->IsModified(); + $bActivateTriggers = (!$bIsNew && $bWasModified); try { $this->oObject->DBWrite(); } - catch (CoreCannotSaveObjectException $e) - { - throw new Exception($e->getHtmlMessage()); + catch (Exception $e) { + if ($bIsNew) { + throw new Exception(Dict::S('Portal:Error:ObjectCannotBeCreated')); + } + throw new Exception(Dict::S('Portal:Error:ObjectCannotBeUpdated')); } // Finalizing images link to object, otherwise it will be cleaned by the GC InlineImage::FinalizeInlineImages($this->oObject); @@ -1142,9 +1138,6 @@ class ObjectFormManager extends FormManager $this->FinalizeAttachments($aArgs['attachmentIds']); } - // Ending transaction with a commit as everything was fine - CMDBSource::Query('COMMIT'); - // Checking if we have to apply a stimulus if (isset($aArgs['applyStimulus'])) { @@ -1192,14 +1185,11 @@ class ObjectFormManager extends FormManager } catch (Exception $e) { - // End transaction with a rollback as something failed - CMDBSource::Query('ROLLBACK'); $aData['valid'] = false; $aData['messages']['error'] += array('_main' => array($e->getMessage())); - IssueLog::Error(__METHOD__.' at line '.__LINE__.' : Rollback during submit ('.$e->getMessage().')'); + IssueLog::Error(__METHOD__.' at line '.__LINE__.' : '.$e->getMessage()); } - return $aData; }