Fixed stopper issue (found with an audit) due to copies of DBObjectSearch not cloned (or not cloned well)

There is still one place where this should be fixed, but it reveals another bug so we've decided to leave it as is for the moment (see comment in DBObjectSearch::AddCondition_PointingTo)

git-svn-id: http://svn.code.sf.net/p/itop/code/trunk@2497 a333f486-631f-4898-b8df-5754b55c2be0

application/displayblock.class.inc.php

@@ -52,7 +52,7 @@ class DisplayBlock
 	
 	public function __construct(DBObjectSearch $oFilter, $sStyle = 'list', $bAsynchronous = false, $aParams = array(), $oSet = null)
 	{
-		$this->m_oFilter = clone $oFilter;
+		$this->m_oFilter = $oFilter->DeepClone();
 		$this->m_aConditions = array();
 		$this->m_sStyle = $sStyle;
 		$this->m_bAsynchronous = $bAsynchronous;
@@ -414,7 +414,7 @@ class DisplayBlock
 				foreach($aGroupBy as $iRow => $iCount)
 				{
 					// Build the search for this subset
-					$oSubsetSearch = clone $this->m_oFilter;
+					$oSubsetSearch = $this->m_oFilter->DeepClone();
 					$oCondition = new BinaryExpression($oGroupByExp, '=', new ScalarExpression($aValues[$iRow]));
 					$oSubsetSearch->AddConditionExpression($oCondition);
 					$sFilter = urlencode($oSubsetSearch->serialize());
@@ -492,7 +492,7 @@ class DisplayBlock
 
 					$sHtml .= "<table>\n";
 					// Construct a new (parametric) query that will return the content of this block
-					$oBlockFilter = clone $this->m_oFilter;
+					$oBlockFilter = $this->m_oFilter->DeepClone();
 					$aExpressions = array();
 					$index = 0;
 					foreach($aGroupByFields as $aField)
@@ -726,7 +726,7 @@ class DisplayBlock
 				$oAttDef = MetaModel::GetAttributeDef($sClass, $sStateAttrCode);
 				foreach($aStates as $sStateValue)
 				{
-					$oFilter = clone($this->m_oFilter);
+					$oFilter = $this->m_oFilter->DeepClone();
 					$oFilter->AddCondition($sStateAttrCode, $sStateValue, '=');
 					$oSet = new DBObjectSet($oFilter);
 					$aCounts[$sStateValue] = $oSet->Count();
@@ -911,7 +911,7 @@ EOF
 				foreach($aGroupBy as $iRow => $iCount)
 				{
 					// Build the search for this subset
-					$oSubsetSearch = clone $this->m_oFilter;
+					$oSubsetSearch = $this->m_oFilter->DeepClone();
 					$oCondition = new BinaryExpression($oGroupByExp, '=', new ScalarExpression($aValues[$iRow]));
 					$oSubsetSearch->AddConditionExpression($oCondition);
 					$aURLs[$idx] = $oSubsetSearch->serialize();

core/dbobjectsearch.class.php

@@ -71,6 +71,14 @@ class DBObjectSearch
 		$this->m_aModifierProperties = array();
 	}
 
+	/**
+	 * Perform a deep clone (as opposed to "clone" which does copy a reference to the underlying objects
+	 **/	 	
+	public function DeepClone()
+	{
+		return unserialize(serialize($this));
+	}
+
 	public function AllowAllData() {$this->m_bAllowAllData = true;}
 	public function IsAllDataAllowed() {return $this->m_bAllowAllData;}
 	public function IsDataFiltered() {return $this->m_bDataFiltered; }
@@ -658,6 +666,10 @@ class DBObjectSearch
 
 	public function AddCondition_PointingTo(DBObjectSearch $oFilter, $sExtKeyAttCode, $iOperatorCode = TREE_OPERATOR_EQUALS)
 	{
+		// Note: though it seems to be a good practice to clone the given source filter
+		//       (as it was done and fixed an issue in MergeWith())
+		//       this was not implemented here because it was causing a regression (login as admin, select an org, click on any badge) 
+		// buggy: $oFilter = $oFilter->DeepClone();
 		$aAliasTranslation = array();
 		$res = $this->AddCondition_PointingTo_InNameSpace($oFilter, $sExtKeyAttCode, $this->m_aClasses, $aAliasTranslation, $iOperatorCode);
 		$this->TransferConditionExpression($oFilter, $aAliasTranslation);
@@ -689,6 +701,7 @@ class DBObjectSearch
 
 	public function AddCondition_ReferencedBy(DBObjectSearch $oFilter, $sForeignExtKeyAttCode)
 	{
+		$oFilter = $oFilter->DeepClone();
 		$aAliasTranslation = array();
 		$res = $this->AddCondition_ReferencedBy_InNameSpace($oFilter, $sForeignExtKeyAttCode, $this->m_aClasses, $aAliasTranslation);
 		$this->TransferConditionExpression($oFilter, $aAliasTranslation);
@@ -721,22 +734,13 @@ class DBObjectSearch
 			$oFilter->AddToNamespace($aClassAliases, $aAliasTranslation);
 
 			// #@# The condition expression found in that filter should not be used - could be another kind of structure like a join spec tree !!!!
-			//$oNewFilter = clone $oFilter;
+			//$oNewFilter = $oFilter->DeepClone();
 			//$oNewFilter->ResetCondition();
 
 			$oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode]= $oFilter;
 		}
 	}
 
-	public function AddCondition_LinkedTo(DBObjectSearch $oLinkFilter, $sExtKeyAttCodeToMe, $sExtKeyAttCodeTarget, DBObjectSearch $oFilterTarget)
-	{
-		$oLinkFilterFinal = clone $oLinkFilter;
-		// todo : new function prototype
-		$oLinkFilterFinal->AddCondition_PointingTo($sExtKeyAttCodeToMe);
-
-		$this->AddCondition_ReferencedBy($oLinkFilterFinal, $sExtKeyAttCodeToMe);
-	}
-
 	public function AddCondition_RelatedTo(DBObjectSearch $oFilter, $sRelCode, $iMaxDepth)
 	{
 		MyHelpers::CheckValueInArray('relation code', $sRelCode, MetaModel::EnumRelations());
@@ -745,6 +749,7 @@ class DBObjectSearch
 
 	public function MergeWith($oFilter)
 	{
+		$oFilter = $oFilter->DeepClone();
 		$aAliasTranslation = array();
 		$res = $this->MergeWith_InNamespace($oFilter, $this->m_aClasses, $aAliasTranslation);
 		$this->TransferConditionExpression($oFilter, $aAliasTranslation);
@@ -1166,7 +1171,7 @@ class DBObjectSearch
 		if ($bOQLCacheEnabled && array_key_exists($sQuery, self::$m_aOQLQueries))
 		{
 			// hit!
-			$oClone = clone self::$m_aOQLQueries[$sQuery];
+			$oClone = self::$m_aOQLQueries[$sQuery]->DeepClone();
 			if (!is_null($aParams))
 			{
 				$oClone->m_aParams = $aParams;
@@ -1316,7 +1321,7 @@ class DBObjectSearch
 
 		if ($bOQLCacheEnabled)
 		{
-			self::$m_aOQLQueries[$sQuery] = clone $oResultFilter;
+			self::$m_aOQLQueries[$sQuery] = $oResultFilter->DeepClone();
 		}
 
 		return $oResultFilter;

core/dbobjectset.class.php

@@ -42,7 +42,7 @@ class DBObjectSet
 
 	public function __construct(DBObjectSearch $oFilter, $aOrderBy = array(), $aArgs = array(), $aExtendedDataSpec = null, $iLimitCount = 0, $iLimitStart = 0)
 	{
-		$this->m_oFilter = clone $oFilter;
+		$this->m_oFilter = $oFilter->DeepClone();
 		$this->m_aAddedIds = array();
 		$this->m_aOrderBy = $aOrderBy;
 		$this->m_aArgs = $aArgs;
@@ -271,7 +271,7 @@ class DBObjectSet
 	public function GetFilter()
 	{
 		// Make sure that we carry on the parameters of the set with the filter
-		$oFilter = clone $this->m_oFilter;
+		$oFilter = $this->m_oFilter->DeepClone();
 		$oFilter->SetInternalParams(array_merge($oFilter->GetInternalParams(), $this->m_aArgs));
 		
 		if (count($this->m_aAddedIds) == 0)

core/metamodel.class.php

@@ -2305,7 +2305,7 @@ abstract class MetaModel
 					$oKPI->ComputeStats('Query APC (store)', $sOqlQuery);
 				}
 
-				self::$m_aQueryStructCache[$sOqlId] = clone $oSelect;
+				self::$m_aQueryStructCache[$sOqlId] = $oSelect->DeepClone();
 			}
 		}
 

core/sqlquery.class.inc.php

@@ -72,6 +72,14 @@ class SQLQuery
 		$this->m_oSelectedIdField = $oSelectedIdField;
 	}
 
+	/**
+	 * Perform a deep clone (as opposed to "clone" which does copy a reference to the underlying objects
+	 **/	 	
+	public function DeepClone()
+	{
+		return unserialize(serialize($this));
+	}
+
 	public function GetTableAlias()
 	{
 		return $this->m_sTableAlias;

pages/audit.php

@@ -94,7 +94,7 @@ function GetRuleResultFilter($iRuleId, $oDefinitionFilter, $oAppContext)
 	if ($oRule->Get('valid_flag') == 'false')
 	{
 		// The query returns directly the invalid elements
-		$oFilter = $oRuleFilter; 
+		$oFilter = $oRuleFilter;
 		$oFilter->MergeWith($oDefinitionFilter);
 	}
 	else
@@ -106,7 +106,7 @@ function GetRuleResultFilter($iRuleId, $oDefinitionFilter, $oAppContext)
 		{
 			$aValidIds[] = $aRow['id'];
 		}
-		$oFilter = clone $oDefinitionFilter;
+		$oFilter = $oDefinitionFilter->DeepClone();
 		if (count($aValidIds) > 0)
 		{
 			$aInDefSet = array();