diff --git a/core/dbobjectsearch.class.php b/core/dbobjectsearch.class.php index 63d31de09..0b17f05c1 100644 --- a/core/dbobjectsearch.class.php +++ b/core/dbobjectsearch.class.php @@ -1145,17 +1145,24 @@ class DBObjectSearch extends DBSearch $oDbObject = $this; } - + /** + * This callback add joins based on the classes found inside ExternalFieldExpression::$aFields + * to do so, it is applied on each Expression of $this->m_oSearchCondition. + * + * @param $oExpression + */ $callback = function ($oExpression) use ($oDbObject) { if (!$oExpression instanceof ExternalFieldExpression) { return; } + + /** @var FieldExpression[] $aFieldExpressionsPointingTo */ $aFields = $oExpression->GetFields(); - $aExpressionNewConditions = array(); $aRealiasingMap = array(); + $aExpressionNewConditions = array(); foreach ($aFields as $aFieldExpressionPointingTo) { $oFilter = new DBObjectSearch($aFieldExpressionPointingTo['sClass']); @@ -1170,7 +1177,7 @@ class DBObjectSearch extends DBSearch /** - * the iteration beleow is weird because wee need to + * the iteration below is weird because wee need to * - iterate in the reverse order * - the iteration access the "index+1" so wee start at "length-1" (which is "count()-2") * - whe stop at the index 1, because the index 0 is merged into the $oDbObject @@ -1211,9 +1218,11 @@ class DBObjectSearch extends DBSearch }; //end of the callback definition + //Add the joins $this->m_oSearchCondition->Browse($callback); - $this->m_oSearchCondition->Translate(array()); + //replace the ExternalFieldExpression by a FieldExpression (based on last ExternalFieldExpression::$aFields) + $this->m_oSearchCondition->Translate(array(), false, false); return $oDbObject; } diff --git a/core/oql/oqlquery.class.inc.php b/core/oql/oqlquery.class.inc.php index af5f49d73..e5c929ac7 100644 --- a/core/oql/oqlquery.class.inc.php +++ b/core/oql/oqlquery.class.inc.php @@ -221,7 +221,7 @@ class ExternalFieldOqlExpression extends ExternalFieldExpression implements Chec if (is_null($sParentAlias)) { - $oFieldOqlExpression->RefreshAlias($oModelReflection, $aAliases, $sSourceQuery); + $oFieldOqlExpression->RefreshAlias($oModelReflection, $aAliases, $sSourceQuery, FieldOqlExpression::ALLOW_EXTERNAL_FIELDS); } else { @@ -267,7 +267,9 @@ class ExternalFieldOqlExpression extends ExternalFieldExpression implements Chec class FieldOqlExpression extends FieldExpression implements CheckableExpression { - protected $m_oParent; + const ALLOW_EXTERNAL_FIELDS = true; + const DISALLOW_EXTERNAL_FIELDS = false; + protected $m_oParent; protected $m_oName; public function __construct($oName, $oParent = null) @@ -316,16 +318,19 @@ class FieldOqlExpression extends FieldExpression implements CheckableExpression } } - /** - * Used by Check => throws exception if error - * - * @param \ModelReflection $oModelReflection - * @param $aAliases - * @param $sSourceQuery - * - * @throws \OqlNormalizeException - */ - public function RefreshAlias(ModelReflection $oModelReflection, $aAliases, $sSourceQuery) + /** + * Used by self::Check and ExternalFieldOqlExpression::Check => throws exception if error + * + * this method has a side effect : if not previously set $this->m_sParent if computed then set. + * + * @param \ModelReflection $oModelReflection + * @param $aAliases + * @param $sSourceQuery + * @param bool $bAllowExternalFields should external fields be authorised? used only by @see ExternalFieldOqlExpression + * + * @throws OqlNormalizeException + */ + public function RefreshAlias(ModelReflection $oModelReflection, $aAliases, $sSourceQuery, $bAllowExternalFields = self::DISALLOW_EXTERNAL_FIELDS) { $sClassAlias = $this->GetParent(); $sFltCode = $this->GetName(); @@ -343,14 +348,29 @@ class FieldOqlExpression extends FieldExpression implements CheckableExpression } if (!array_key_exists($sFltCode, $aFieldClasses)) { - throw new OqlNormalizeException('Unknown filter code', $sSourceQuery, $this->GetNameDetails(), array_keys($aFieldClasses)); + if (count($aAliases) > 1) + { + throw new OqlNormalizeException('Ambiguous filter code', $sSourceQuery, $this->GetNameDetails(), array_keys($aFieldClasses)); + } + + $sClassAlias = reset($aAliases); + if (self::DISALLOW_EXTERNAL_FIELDS == $bAllowExternalFields || false == $oModelReflection->IsValidAttCode($sClassAlias, $sFltCode)) + { + throw new OqlNormalizeException('Unknown filter code', $sSourceQuery, $this->GetNameDetails(), array_keys($aFieldClasses)); + } + $this->SetParent($sClassAlias); + } - if (count($aFieldClasses[$sFltCode]) > 1) + elseif (count($aFieldClasses[$sFltCode]) > 1) { throw new OqlNormalizeException('Ambiguous filter code', $sSourceQuery, $this->GetNameDetails()); } - $sClassAlias = $aFieldClasses[$sFltCode][0]; - $this->SetParent($sClassAlias); + else + { + $sClassAlias = $aFieldClasses[$sFltCode][0]; + $this->SetParent($sClassAlias); + } + } } } diff --git a/test/core/oql/OqlInterpreterTest.php b/test/core/oql/OqlInterpreterTest.php index 8402d2722..3c6acf1f3 100644 --- a/test/core/oql/OqlInterpreterTest.php +++ b/test/core/oql/OqlInterpreterTest.php @@ -8,8 +8,13 @@ namespace Combodo\iTop\Test\UnitTest\Core\Oql; +use DBObjectSearch; +use DBObjectSet; +use ExpressionCache; use OqlInterpreter; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; +use OqlNormalizeException; +use OQLParserException; class OqlInterpreterTest extends ItopDataTestCase { @@ -24,9 +29,13 @@ class OqlInterpreterTest extends ItopDataTestCase require_once(APPROOT."core/oql/oqlinterpreter.class.inc.php"); require_once(APPROOT.'core/dbobject.class.php'); require_once(APPROOT."core/dbobjectsearch.class.php"); + require_once(APPROOT.'core/dbobjectset.class.php'); require_once(APPROOT."core/modelreflection.class.inc.php"); - + require_once(APPROOT."core/expressioncache.class.inc.php"); + $sCacheFileName = ExpressionCache::GetCacheFileName(); + unlink($sCacheFileName); + ExpressionCache::Warmup(); } @@ -36,7 +45,7 @@ class OqlInterpreterTest extends ItopDataTestCase { $sQuery = "SELECT Contact WHERE org_id->deliverymodel_id->name = 'Standard support' AND 1 AND 1 AND 1"; - $oDbObjectSearch = \DBObjectSearch::FromOQL($sQuery); + $oDbObjectSearch = DBObjectSearch::FromOQL($sQuery); $oDbObjectSearchExpandedClone1 = $oDbObjectSearch->ShorthandExpansion(true); $oDbObjectSearchExpandedClone2 = $oDbObjectSearch->ShorthandExpansion(true); $oDbObjectSearchExpandedBase1 = $oDbObjectSearch->ShorthandExpansion(); @@ -57,23 +66,41 @@ class OqlInterpreterTest extends ItopDataTestCase * @dataProvider ShorthandExpansionProvider * @param $sQuery */ - public function testShorthandExpansion($sQuery) + public function testShorthandExpansion($sQuery, $sExpectedException, $sExpectedOqlEquals, $aExpectedSqlContains) { - $oDbObjectSearch = \DBObjectSearch::FromOQL($sQuery); - $oDbObjectSearchExpanded = $oDbObjectSearch->ShorthandExpansion(); + if (!empty($sExpectedException)) { + $this->expectException($sExpectedException); + } - $this->assertSame($oDbObjectSearch, $oDbObjectSearchExpanded); + $oDbObjectSearch = DBObjectSearch::FromOQL($sQuery); - $this->debug($oDbObjectSearch->ToOQL()); + $sSql = $oDbObjectSearch->MakeSelectQuery(); + $sOql = $oDbObjectSearch->ToOQL(); + $oSet = new DBObjectSet($oDbObjectSearch); + $iCount = $oSet->Count(); + + $this->assertInternalType('numeric', $iCount); + $this->assertEquals($sExpectedOqlEquals, $sOql); + foreach ($aExpectedSqlContains as $sExpectedSqlContain) + { + $this->assertContains($sExpectedSqlContain, $sSql); + } + + $this->debug($sSql); + $this->debug($sOql); } public function ShorthandExpansionProvider() { return array( - array("SELECT Contact WHERE org_id->deliverymodel_id->name = 'tato'"), - array('SELECT Contact WHERE cis_list->name = "Cluster1"'), - array('SELECT Contact WHERE cis_list->name like "%m%"'), + array("SELECT Contact WHERE org_id->deliverymodel_id->name = 'Standard support'", false, 'SELECT `Contact` FROM Contact AS `Contact` WHERE (Contact.org_id->deliverymodel_id->name = \'Standard support\')', array( + 'JOIN (`organization`', + 'JOIN `deliverymodel`', + )), + array('SELECT Contact WHERE org_id->deliverymodel_id->contacts_list->role_id->name != ""', OqlNormalizeException::class, '', array()), + array('SELECT Contact WHERE cis_list->name = "Cluster1"', OqlNormalizeException::class, '', array()), + array('SELECT Contact WHERE cis_list->name LIKE "%m%"', OqlNormalizeException::class, '', array()), ); } @@ -87,14 +114,20 @@ class OqlInterpreterTest extends ItopDataTestCase $oOql = new OqlInterpreter($sQuery); $oTrash = $oOql->Parse(); // Not expecting a given format, otherwise use ParseExpression/ParseObjectQuery/ParseValueSetQuery - $this->debug($oTrash); + $this->assertInstanceOf(\OqlObjectQuery::class, $oTrash); + + + $this->debug($sQuery); } public function ParseProvider() { return array( array("SELECT Contact WHERE org_id->deliverymodel_id->name = 'Standard support'"), - array("SELECT Contact WHERE cis_list->name = 'toto'"), + array("SELECT Contact WHERE org_id->foo->bar LIKE 'Standard support'"), + array("SELECT Contact WHERE cis_list->name = 3"), + array("SELECT Contact WHERE cis_list->name < 3"), + array("SELECT Contact WHERE cis_list->name >- 3"), ); }