1627 - Ticket ref sometimes duplicate

🐛 fix ref. generation when inside a transaction (by opening a new connection).
note: the portal make uses of such a transaction.

core/counter.class.inc.php

@@ -24,6 +24,7 @@
  */
 final class ItopCounter
 {
+
 	/**
 	 * Key based counter.
 	 * The counter is protected against concurrency script.
@@ -35,13 +36,9 @@ final class ItopCounter
 	 *  * `0` when no $oNewObjectValueProvider is given (or null)
 	 *  * `$oNewObjectValueProvider() + 1` otherwise
 	 *
-	 * @throws \ArchivedObjectException
-	 * @throws \CoreCannotSaveObjectException
 	 * @throws \CoreException
-	 * @throws \CoreOqlMultipleResultsForbiddenException
-	 * @throws \CoreUnexpectedValue
 	 * @throws \MySQLException
-	 * @throws \OQLException
+	 * @throws \Exception
 	 */
 	public static function Inc($sCounterName, $oNewObjectValueProvider = null)
 	{
@@ -50,35 +47,92 @@ final class ItopCounter
 		$oiTopMutex = new iTopMutex($sMutexKeyName);
 		$oiTopMutex->Lock();
 
-		$oFilter = DBObjectSearch::FromOQL('SELECT KeyValueStore WHERE key_name=:key_name AND namespace=:namespace', array(
+		$bIsInsideTransaction = CMDBSource::IsInsideTransaction();
-			'key_name'  => $sCounterName,
+		if ($bIsInsideTransaction)
-			'namespace' => $sSelfClassName,
-		));
-		$oCounter = $oFilter->GetFirstResult();
-		if (is_null($oCounter))
 		{
-			if (null != $oNewObjectValueProvider)
+			// # Transaction isolation hack:
+			// When inside a transaction, we need to open a new connection for the counter.
+			// So it is visible immediately to the connections outside of the transaction.
+			// Either way, the lock is not long enought, and there would be duplicate ref.
+			//
+			// SELECT ... FOR UPDATE would have also worked but with the cost of extra long lock (until the commit),
+			// we did not wanted this! As opening a short connection is less prone to starving than a long running one.
+			// Plus it would trigger way more deadlocks!
+			$hDBLink = self::InitMySQLSession();
+		}
+		else
+		{
+			$hDBLink = CMDBSource::GetMysqli();
+		}
+
+		try
+		{
+			$oFilter = DBObjectSearch::FromOQL('SELECT KeyValueStore WHERE key_name=:key_name AND namespace=:namespace', array(
+				'key_name'  => $sCounterName,
+				'namespace' => $sSelfClassName,
+			));
+			$oAttDef = MetaModel::GetAttributeDef('KeyValueStore', 'value');
+			$aAttToLoad = array('KeyValueStore' => array('value' => $oAttDef));
+			$sSql = $oFilter->MakeSelectQuery(array(), array(), $aAttToLoad);
+			$hResult = mysqli_query($hDBLink, $sSql);
+			$aCounter = mysqli_fetch_array($hResult, MYSQLI_NUM);
+			mysqli_free_result($hResult);
+
+			//Rebuild the filter, as the MakeSelectQuery polluted the orignal and it cannot be reused
+			$oFilter = DBObjectSearch::FromOQL('SELECT KeyValueStore WHERE key_name=:key_name AND namespace=:namespace', array(
+				'key_name'  => $sCounterName,
+				'namespace' => $sSelfClassName,
+			));
+
+			if (is_null($aCounter))
 			{
-				$iComputedValue = $oNewObjectValueProvider();
+				if (null != $oNewObjectValueProvider)
+				{
+					$iComputedValue = $oNewObjectValueProvider();
+				}
+				else
+				{
+					$iComputedValue = 0;
+				}
+
+				$iCurrentValue = $iComputedValue + 1;
+
+				$aQueryParams = array(
+					'key_name'  => $sCounterName,
+					'value'     => "$iCurrentValue",
+					'namespace' => $sSelfClassName,
+				);
+
+				$sSql = $oFilter->MakeInsertQuery($aQueryParams);
 			}
 			else
 			{
-				$iComputedValue = 0;
+				$iCurrentValue = (int) $aCounter[1];
+				$iCurrentValue++;
+				$aQueryParams = array(
+					'value'     => "$iCurrentValue",
+				);
+
+				$sSql = $oFilter->MakeUpdateQuery($aQueryParams);
 			}
-			$oCounter = MetaModel::NewObject('KeyValueStore', array(
+
-				'key_name'  => $sCounterName,
+			$hResult = mysqli_query($hDBLink, $sSql);
-				'value'     => $iComputedValue,
+			mysqli_free_result($hResult);
-				'namespace' => $sSelfClassName,
+
-			));
+		}
+		catch(Exception $e)
+		{
+			IssueLog::Error($e->getMessage());
+			throw $e;
+		}
+		finally
+		{
+			if ($bIsInsideTransaction)
+			{
+				mysqli_close($hDBLink);
+			}
+			$oiTopMutex->Unlock();
 		}
-
-		$iCurrentValue = (int) $oCounter->Get('value');
-		$iCurrentValue++;
-
-		$oCounter->Set('value', $iCurrentValue);
-		$oCounter->DBWrite();
-
-		$oiTopMutex->Unlock();
 
 		return $iCurrentValue;
 	}
@@ -114,6 +168,32 @@ final class ItopCounter
 
 		return self::Inc($sRootClass, $oNewObjectCallback);
 	}
+
+	/**
+	 * @return \mysqli
+	 * @throws \ConfigException
+	 * @throws \CoreException
+	 * @throws \MySQLException
+	 */
+	private static function InitMySQLSession()
+	{
+		$oConfig = utils::GetConfig();
+		$sDBHost = $oConfig->Get('db_host');
+		$sDBUser = $oConfig->Get('db_user');
+		$sDBPwd  = $oConfig->Get('db_pwd');
+		$sDBName = $oConfig->Get('db_name');
+		$bDBTlsEnabled = $oConfig->Get('db_tls.enabled');
+		$sDBTlsCA = $oConfig->Get('db_tls.ca');
+
+		$hDBLink = CMDBSource::GetMysqliInstance($sDBHost, $sDBUser, $sDBPwd, $sDBName, $bDBTlsEnabled, $sDBTlsCA, false);
+
+		if (!$hDBLink)
+		{
+			throw new Exception("Could not connect to the DB server (host=$sDBHost, user=$sDBUser): ".mysqli_connect_error().' (mysql errno: '.mysqli_connect_errno().')');
+		}
+
+		return $hDBLink;
+	}
 }
 
 

core/dbobjectsearch.class.php

@@ -1798,6 +1798,24 @@ class DBObjectSearch extends DBSearch
 		return $sRet;
 	}
 
+	/**
+	 * Generate an INSERT statement.
+	 * Note : unlike `RenderUpdate` and `RenderSelect`, it is limited to one and only one table.
+	 *
+	 * @param array $aValues is an array of $sAttCode => $value
+	 * @param array $aArgs
+	 *
+	 * @return string
+	 * @throws \CoreException
+	 */
+	public function MakeInsertQuery($aValues, $aArgs = array())
+	{
+		$oSQLObjectQueryBuilder = new SQLObjectQueryBuilder($this);
+		$oSQLQuery = $oSQLObjectQueryBuilder->MakeSQLObjectUpdateQuery($aValues);
+		$aScalarArgs = MetaModel::PrepareQueryArguments($aArgs, $this->GetInternalParams());
+		$sRet = $oSQLQuery->RenderInsert($aScalarArgs);
+		return $sRet;
+	}
 
 	/**
 	 * Get an SQLObjectQuery from the search. This SQLObjectQuery can be rendered as a select, select group by, update or delete

core/sqlobjectquery.class.inc.php

@@ -318,7 +318,40 @@ class SQLObjectQuery extends SQLQuery
 		return "UPDATE $sFrom SET $sValues WHERE $sWhere";
 	}
 
-	// Interface, build the SQL query
+
+	/**
+	 * Generate an INSERT statement.
+	 * Note : unlike `RenderUpdate` and `RenderSelect`, it is limited to one and only one table.
+	 *
+	 *
+	 * @param array $aArgs
+	 * @return string
+	 * @throws CoreException
+	 */
+	public function RenderInsert($aArgs = array())
+	{
+		$this->PrepareRendering();
+		$aJoinInfo = reset($this->__aFrom);
+
+		if ($aJoinInfo['jointype'] != 'first' || count($this->__aFrom) > 1)
+		{
+			throw new CoreException('Cannot render insert');
+		}
+
+		$sFrom   = "`{$aJoinInfo['tablename']}`";
+
+		$sColList = '`'.implode('`,`', array_keys($this->m_aValues)).'`';
+
+		$aSetValues = array();
+		foreach ($this->__aSetValues as $sFieldSpec => $value)
+		{
+			$aSetValues[] = CMDBSource::Quote($value);
+		}
+		$sValues = implode(',', $aSetValues);
+
+		return "INSERT INTO $sFrom ($sColList) VALUES  ($sValues)";
+	}
+
 
 	/**
 	 * @param array $aOrderBy