Fixed two bugs revealed with specific constraints (query expression like 'SELECT b FROM a JOIN b', AND the organization context is set)

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

core/dbobjectsearch.class.php

@@ -339,7 +339,7 @@ class DBObjectSearch
 		{
 			foreach($aPointingTo as $iOperatorCode => $aFilter)
 			{
-				foreach($aFilter as $sAlias => $oExtFilter)
+				foreach($aFilter as $oExtFilter)
 				{
 					$oExtFilter->RenameParam($sOldName, $sNewName);
 				}
@@ -586,7 +586,7 @@ class DBObjectSearch
 		{
 			foreach($aPointingTo as $iOperatorCode => $aFilter)
 			{
-				foreach($aFilter as $sAlias => $oFilter)
+				foreach($aFilter as $oFilter)
 				{
 					$oFilter->AddToNameSpace($aClassAliases, $aAliasTranslation);
 				}
@@ -602,6 +602,48 @@ class DBObjectSearch
 		}
 	}
 
+
+	// Browse the tree nodes recursively
+	//
+	protected function GetNode($sAlias)
+	{
+		if ($this->GetFirstJoinedClassAlias() == $sAlias)
+		{
+			return $this;
+		}
+		else
+		{
+			foreach($this->m_aPointingTo as $sExtKeyAttCode=>$aPointingTo)
+			{
+				foreach($aPointingTo as $iOperatorCode => $aFilter)
+				{
+					foreach($aFilter as $oFilter)
+					{
+						$ret = $oFilter->GetNode($sAlias);
+						if (is_object($ret))
+						{
+							return $ret;
+						}
+					}
+				}
+			}
+			foreach($this->m_aReferencedBy as $sForeignClass=>$aReferences)
+			{
+				foreach($aReferences as $sForeignExtKeyAttCode=>$oForeignFilter)
+				{
+					$ret = $oForeignFilter->GetNode($sAlias);
+					if (is_object($ret))
+					{
+						return $ret;
+					}
+				}
+			}
+		}
+		// Not found
+		return null;
+	}
+
+
 	public function AddCondition_PointingTo(DBObjectSearch $oFilter, $sExtKeyAttCode, $iOperatorCode = TREE_OPERATOR_EQUALS)
 	{
 		$aAliasTranslation = array();
@@ -626,29 +668,11 @@ class DBObjectSearch
 			throw new CoreException("The specified tree operator $iOperatorCode is not applicable to the key '{$this->GetClass()}::$sExtKeyAttCode', which is not a HierarchicalKey");
 		}
 
-		$bSamePointingTo = false;
-		if (array_key_exists($sExtKeyAttCode, $this->m_aPointingTo))
-		{
-			if (array_key_exists($iOperatorCode, $this->m_aPointingTo[$sExtKeyAttCode]))
-			{
-				if (array_key_exists($oFilter->GetClassAlias(), $this->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode]))
-				{
-					$bSamePointingTo = true;
-				}
-			}
-		}
+		// Find the node on which the new tree must be attached (most of the time it is "this")
+		$oReceivingFilter = $this->GetNode($this->GetClassAlias());
 
-		if ($bSamePointingTo)
-		{
-			// Same ext key, alias and same operator, merge the filters together
-			$oFilter->AddToNamespace($aClassAliases, $aAliasTranslation, true /* Don't translate the main alias */);
-			$this->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode][$oFilter->GetClassAlias()] = $oFilter;
-		}
-		else
-		{
-			$oFilter->AddToNamespace($aClassAliases, $aAliasTranslation);
-			$this->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode][$oFilter->GetClassAlias()] = $oFilter;
-		}
+		$oFilter->AddToNamespace($aClassAliases, $aAliasTranslation);
+		$oReceivingFilter->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode][] = $oFilter;
 	}
 
 	public function AddCondition_ReferencedBy(DBObjectSearch $oFilter, $sForeignExtKeyAttCode)
@@ -672,9 +696,13 @@ class DBObjectSearch
 		{
 			throw new CoreException("The specified filter (objects referencing an object of class {$this->GetClass()}) is not compatible with the key '{$sForeignClass}::$sForeignExtKeyAttCode', which is pointing to {$oAttExtKey->GetTargetClass()}");
 		}
+
+		// Find the node on which the new tree must be attached (most of the time it is "this")
+		$oReceivingFilter = $this->GetNode($this->GetClassAlias());
+
 		if (array_key_exists($sForeignClass, $this->m_aReferencedBy) && array_key_exists($sForeignExtKeyAttCode, $this->m_aReferencedBy[$sForeignClass]))
 		{
-			$this->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode]->MergeWith_InNamespace($oFilter, $aClassAliases, $aAliasTranslation);
+			$oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode]->MergeWith_InNamespace($oFilter, $aClassAliases, $aAliasTranslation);
 		}
 		else
 		{
@@ -684,7 +712,7 @@ class DBObjectSearch
 			//$oNewFilter = clone $oFilter;
 			//$oNewFilter->ResetCondition();
 
-			$this->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode]= $oFilter;
+			$oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode]= $oFilter;
 		}
 	}
 
@@ -728,7 +756,7 @@ class DBObjectSearch
 		{
 			foreach($aPointingTo as $iOperatorCode => $aFilter)
 			{
-				foreach($aFilter as $sAlias => $oExtFilter)
+				foreach($aFilter as $oExtFilter)
 				{
 					$this->AddCondition_PointingTo_InNamespace($oExtFilter, $sExtKeyAttCode, $aClassAliases, $aAliasTranslation, $iOperatorCode);
 				}
@@ -951,7 +979,7 @@ class DBObjectSearch
 		{
 			foreach($aPointingTo as $iOperatorCode => $aFilter)
 			{
-				foreach($aFilter as $sAlias => $oFilter)
+				foreach($aFilter as $oFilter)
 				{
 					switch($iOperatorCode)
 					{

core/metamodel.class.php

@@ -2096,36 +2096,37 @@ abstract class MetaModel
 		// Check the order by specification, and prefix with the class alias
 		// and make sure that the ordering columns are going to be selected
 		//
+		$sClass = $oFilter->GetClass();
+		$sClassAlias = $oFilter->GetClassAlias();
 		$aOrderSpec = array();
 		foreach ($aOrderBy as $sFieldAlias => $bAscending)
 		{
 			if ($sFieldAlias != 'id')
 			{
-				MyHelpers::CheckValueInArray('field name in ORDER BY spec', $sFieldAlias, self::GetAttributesList($oFilter->GetFirstJoinedClass()));
+				MyHelpers::CheckValueInArray('field name in ORDER BY spec', $sFieldAlias, self::GetAttributesList($sClass));
 			}
 			if (!is_bool($bAscending))
 			{
 				throw new CoreException("Wrong direction in ORDER BY spec, found '$bAscending' and expecting a boolean value");
 			}
-			$sFirstClassAlias = $oFilter->GetClassAlias();
 			
-			if (self::IsValidAttCode($oFilter->GetClass(), $sFieldAlias))
+			if (self::IsValidAttCode($sClass, $sFieldAlias))
 			{
-				$oAttDef = self::GetAttributeDef($oFilter->GetClass(), $sFieldAlias);
-				foreach($oAttDef->GetOrderBySQLExpressions($sFirstClassAlias) as $sSQLExpression)
+				$oAttDef = self::GetAttributeDef($sClass, $sFieldAlias);
+				foreach($oAttDef->GetOrderBySQLExpressions($sClassAlias) as $sSQLExpression)
 				{
 					$aOrderSpec[$sSQLExpression] = $bAscending;
 				}
 			}
 			else
 			{
-				$aOrderSpec['`'.$sFirstClassAlias.$sFieldAlias.'`'] = $bAscending;
+				$aOrderSpec['`'.$sClassAlias.$sFieldAlias.'`'] = $bAscending;
 			}
 
 			// Make sure that the columns used for sorting are present in the loaded columns
-			if (!is_null($aAttToLoad) && !isset($aAttToLoad[$sFirstClassAlias][$sFieldAlias]))
+			if (!is_null($aAttToLoad) && !isset($aAttToLoad[$sClassAlias][$sFieldAlias]))
 			{
-				$aAttToLoad[$sFirstClassAlias][$sFieldAlias] = MetaModel::GetAttributeDef($oFilter->GetFirstJoinedClass(), $sFieldAlias);
+				$aAttToLoad[$sClassAlias][$sFieldAlias] = MetaModel::GetAttributeDef($sClass, $sFieldAlias);
 			}			
 		}
 
@@ -2135,7 +2136,7 @@ abstract class MetaModel
 		try
 		{
 			$sRes = $oSelect->RenderSelect($aOrderSpec, $aScalarArgs, $iLimitCount, $iLimitStart, $bGetCount);
-			if ($oFilter->GetClassAlias() == 'itop')
+			if ($sClassAlias == 'itop')
 			{
 				echo $sRes."<br/>\n";
 			}
@@ -2827,7 +2828,7 @@ abstract class MetaModel
 		{
 			foreach($aPointingTo as $iOperatorCode => $aFilter)
 			{
-				foreach($aFilter as $sAlias => $oExtFilter)
+				foreach($aFilter as $oExtFilter)
 				{
 					if (!MetaModel::IsValidAttCode($sTableClass, $sKeyAttCode)) continue; // Not defined in the class, skip it
 					// The aliases should not conflict because normalization occured while building the filter

test/testlist.inc.php

@@ -1100,7 +1100,7 @@ class TestItopEfficiency extends TestBizModel
 		return 'Measure time to perform the queries';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function DoBenchmark($sOqlQuery)
 	{
@@ -1225,7 +1225,7 @@ class TestQueries extends TestBizModel
 		return 'Try as many queries as possible';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function DoBenchmark($sOqlQuery)
 	{
@@ -1322,6 +1322,106 @@ class TestQueries extends TestBizModel
 }
 
 ///////////////////////////////////////////////////////////////////////////
+// Check programmaticaly built queries
+///////////////////////////////////////////////////////////////////////////
+
+class TestQueriesByAPI extends TestBizModel
+{
+	static public function GetName()
+	{
+		return 'Itop - queries build programmaticaly';
+	}
+
+	static public function GetDescription()
+	{
+		return 'Validate the DBObjectSearch API, through a set of complex (though realistic cases)';
+	}
+
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
+
+	protected function DoExecute()
+	{
+		// Note: relying on eval() - after upgrading to PHP 5.3 we can move to closure (aka anonymous functions)
+		$aQueries = array(
+			'Basic (validate the test)' => array(
+				'search' => '
+$oSearch = DBObjectSearch::FromOQL("SELECT P FROM Organization AS O JOIN Person AS P ON P.org_id = O.id WHERE org_id = 2");
+				',
+				'oql' => 'SELECT P FROM Organization AS O JOIN Person AS P ON P.org_id = O.id WHERE P.org_id = 2'
+			),
+			'Double constraint' => array(
+				'search' => '
+$oSearch = DBObjectSearch::FromOQL("SELECT Contact AS c");
+$sClass = $oSearch->GetClass();
+$sFilterCode = "org_id";
+
+$oAttDef = MetaModel::GetAttributeDef($sClass, $sFilterCode);
+
+if ($oAttDef->IsExternalKey())
+{
+	$sHierarchicalKeyCode = MetaModel::IsHierarchicalClass($oAttDef->GetTargetClass());
+	
+	if ($sHierarchicalKeyCode !== false)
+	{
+		$oFilter = new DBObjectSearch($oAttDef->GetTargetClass(), "ORGA");
+		$oFilter->AddCondition("id", 2);
+		$oHKFilter = new DBObjectSearch($oAttDef->GetTargetClass(), "ORGA");
+		$oHKFilter->AddCondition_PointingTo(clone $oFilter, $sHierarchicalKeyCode, TREE_OPERATOR_BELOW);
+
+		$oSearch->AddCondition_PointingTo(clone $oHKFilter, $sFilterCode);
+
+		$oFilter = new DBObjectSearch($oAttDef->GetTargetClass(), "ORGA");
+		$oFilter->AddCondition("id", 2);
+		$oHKFilter = new DBObjectSearch($oAttDef->GetTargetClass(), "ORGA");
+		$oHKFilter->AddCondition_PointingTo(clone $oFilter, $sHierarchicalKeyCode, TREE_OPERATOR_BELOW);
+
+		$oSearch->AddCondition_PointingTo(clone $oHKFilter, $sFilterCode);
+	}
+}
+				',
+				'oql' => 'SELECT Contact AS C JOIN Organization ???'
+			),
+			'Simplified issue' => array(
+				'search' => '
+$oSearch = DBObjectSearch::FromOQL("SELECT P FROM Organization AS O JOIN Person AS P ON P.org_id = O.id WHERE O.id = 2");
+$oOrgSearch = new DBObjectSearch("Organization", "O2");
+$oOrgSearch->AddCondition("id", 2);
+$oSearch->AddCondition_PointingTo($oOrgSearch, "org_id");
+				',
+				'oql' => 'SELECT P FROM Organization AS O JOIN Person AS P ON P.org_id = O.id JOIN Organization AS O2 ON P.org_id = O2.id WHERE O.id = 2 AND O2.id = 2'
+			),
+		);
+		foreach ($aQueries as $sQueryDesc => $aQuerySpec)
+		{
+			echo "<h2>Query $sQueryDesc</h2>\n";
+			echo "<p>Using code: ".highlight_string("<?php\n".trim($aQuerySpec['search'])."\n?".'>', true)."</p>\n";
+			echo "<p>Expected OQL: ".$aQuerySpec['oql']."</p>\n";
+
+			if (isset($oSearch))
+			{
+				unset($oSearch);
+			}
+			eval($aQuerySpec['search']);
+			$sResOQL = $oSearch->ToOQL();
+			echo "<p>Resulting OQL: ".$sResOQL."</p>\n";
+
+			echo "<pre>";
+			print_r($oSearch);
+			echo "</pre>";
+
+			$sSQL = MetaModel::MakeSelectQuery($oSearch);
+			$res = CMDBSource::Query($sSQL);
+			foreach (CMDBSource::ExplainQuery($sSQL) as $aRow)
+			{
+			}
+		}
+
+//		throw new UnitTestException("Expecting result '{$aWebService['expected result']}', but got '$res'");
+
+	}
+}
+
+///////////////////////////////////////////////////////////////////////////
 // Test bulk load API
 ///////////////////////////////////////////////////////////////////////////
 
@@ -1337,7 +1437,7 @@ class TestItopBulkLoad extends TestBizModel
 		return 'Execute a bulk change at the Core API level';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	
 	protected function DoExecute()
@@ -1906,7 +2006,7 @@ class TestDataExchange extends TestBizModel
 		return 'Test REST services: synchro_import and synchro_exec';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function DoExecScenario($aSingleScenario)
 	{
@@ -2910,7 +3010,7 @@ abstract class TestSoapDirect extends TestBizModel
 	static public function GetName() {return 'Test web services locally';}
 	static public function GetDescription() {return 'Invoke the service directly (troubleshooting)';}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected $m_aTestSpecs;
 
@@ -3010,7 +3110,7 @@ class TestTriggerAndEmail extends TestBizModel
 	static public function GetName() {return 'Test trigger and email';}
 	static public function GetDescription() {return 'Create a trigger and an email, then activates the trigger';}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function CreateEmailSpec($oTrigger, $sStatus, $sTo, $sCC, $sTesterEmail)
 	{
@@ -3154,7 +3254,7 @@ class TestDBProperties extends TestBizModel
 		return 'Write and read a dummy property';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function DoExecute()
 	{
@@ -3177,7 +3277,7 @@ class TestCreateObjects extends TestBizModel
 		return 'Create weird objects (reproduce a bug?)';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function DoExecute()
 	{
@@ -3238,7 +3338,7 @@ class TestSetLinkset extends TestBizModel
 		return 'Create a user account, setting its profile by the mean of a string (prerequisite to CSV import of linksets)';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function DoExecute()
 	{
@@ -3272,7 +3372,7 @@ class TestEmailAsynchronous extends TestBizModel
 		return 'Queues a request to send an email';
 	}
 
-	static public function GetConfigFile() {return '/config-itop.php';}
+	static public function GetConfigFile() {return 'conf/production/config-itop.php';}
 
 	protected function DoExecute()
 	{