OQL enhancement: allow JOIN on a objclass/objkey pair of attributes (requires benchmarking)

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

core/attributedef.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2013 Combodo SARL
+// Copyright (C) 2010-2015 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -496,23 +496,23 @@ abstract class AttributeDefinition
 	 * @param $bLocalize bool Whether or not to localize the value
 	 */
 	public function GetForTemplate($value, $sVerb, $oHostObject = null, $bLocalize = true)
-	{
-		if ($this->IsScalar())
-		{
-			switch ($sVerb)
-			{
-				case '':
-				return $value;
-				
-				case 'html':
-				return $this->GetAsHtml($value, $oHostObject, $bLocalize);
-				
-				case 'label':
+	{
+		if ($this->IsScalar())
+		{
+			switch ($sVerb)
+			{
+				case '':
+				return $value;
+				
+				case 'html':
+				return $this->GetAsHtml($value, $oHostObject, $bLocalize);
+				
+				case 'label':
 				return $this->GetEditValue($value);
 				
 				default:
-				throw new Exception("Unknown verb '$sVerb' for attribute ".$this->GetCode().' in class '.get_class($oHostObj));
-			}
+				throw new Exception("Unknown verb '$sVerb' for attribute ".$this->GetCode().' in class '.get_class($oHostObj));
+			}
 		}
 		return null;
 	}
@@ -1198,6 +1198,61 @@ class AttributeInteger extends AttributeDBField
 }
 
 /**
+ * An external key for which the class is defined as the value of another attribute 
+ *
+ * @package     iTopORM
+ */
+class AttributeObjectKey extends AttributeDBFieldVoid
+{
+	static public function ListExpectedParams()
+	{
+		return array_merge(parent::ListExpectedParams(), array('class_attcode', 'is_null_allowed'));
+	}
+
+	public function GetEditClass() {return "String";}
+	protected function GetSQLCol() {return "INT(11)";}
+
+	public function GetDefaultValue() {return 0;}
+	public function IsNullAllowed()
+	{
+		return $this->Get("is_null_allowed");
+	}
+
+
+	public function GetBasicFilterOperators()
+	{
+		return parent::GetBasicFilterOperators();
+	}
+	public function GetBasicFilterLooseOperator()
+	{
+		return parent::GetBasicFilterLooseOperator();
+	}
+
+	public function GetBasicFilterSQLExpr($sOpCode, $value)
+	{
+		return parent::GetBasicFilterSQLExpr($sOpCode, $value);
+	} 
+
+	public function GetNullValue()
+	{
+		return 0;
+	} 
+
+	public function IsNull($proposedValue)
+	{
+		return ($proposedValue == 0);
+	} 
+
+	public function MakeRealValue($proposedValue, $oHostObj)
+	{
+		if (is_null($proposedValue)) return 0;
+		if ($proposedValue === '') return 0;
+		if (MetaModel::IsValidObject($proposedValue)) return $proposedValue->GetKey();
+		return (int)$proposedValue;
+	}
+}
+
+/**
  * Display an integer between 0 and 100 as a percentage / horizontal bar graph 
  *
  * @package     iTopORM

core/cmdbchangeop.class.inc.php

@@ -55,7 +55,7 @@ class CMDBChangeOp extends DBObject
 		MetaModel::Init_AddAttribute(new AttributeExternalField("date", array("allowed_values"=>null, "extkey_attcode"=>"change", "target_attcode"=>"date")));
 		MetaModel::Init_AddAttribute(new AttributeExternalField("userinfo", array("allowed_values"=>null, "extkey_attcode"=>"change", "target_attcode"=>"userinfo")));
 		MetaModel::Init_AddAttribute(new AttributeString("objclass", array("allowed_values"=>null, "sql"=>"objclass", "default_value"=>"", "is_null_allowed"=>false, "depends_on"=>array())));
-		MetaModel::Init_AddAttribute(new AttributeInteger("objkey", array("allowed_values"=>null, "sql"=>"objkey", "default_value"=>0, "is_null_allowed"=>false, "depends_on"=>array())));
+		MetaModel::Init_AddAttribute(new AttributeObjectKey("objkey", array("allowed_values"=>null, "class_attcode"=>"objclass", "sql"=>"objkey", "is_null_allowed"=>false, "depends_on"=>array())));
 
 		MetaModel::Init_SetZListItems('details', array('change', 'date', 'userinfo')); // Attributes to be displayed for the complete details
 		MetaModel::Init_SetZListItems('list', array('change', 'date', 'userinfo')); // Attributes to be displayed for the complete details

core/metamodel.class.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2014 Combodo SARL
+// Copyright (C) 2010-2015 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -2856,6 +2856,17 @@ abstract class MetaModel
 				$sForeignClassAlias = $oForeignFilter->GetFirstJoinedClassAlias();
 				$oBuild->m_oQBExpressions->PushJoinField(new FieldExpression($sForeignKeyAttCode, $sForeignClassAlias));
 
+				if ($oForeignKeyAttDef instanceof AttributeObjectKey)
+				{
+					$sClassAttCode = $oForeignKeyAttDef->Get('class_attcode');
+
+					// Add the condition: `$sForeignClassAlias`.$sClassAttCode IN (subclasses of $sClass')
+					$oClassListExpr = ListExpression::FromScalars(self::EnumChildClasses($sClass, ENUM_CHILD_CLASSES_ALL));
+					$oClassExpr = new FieldExpression($sClassAttCode, $sForeignClassAlias);
+					$oClassRestriction = new BinaryExpression($oClassExpr, 'IN', $oClassListExpr);
+					$oBuild->m_oQBExpressions->AddCondition($oClassRestriction);
+				}
+
 				$oSelectForeign = self::MakeQuery($oBuild, $oForeignFilter, $aAttToLoad);
 
 				$oJoinExpr = $oBuild->m_oQBExpressions->PopJoinField();
@@ -3108,6 +3119,16 @@ abstract class MetaModel
 							$sLocalKeyField = current($aCols); // get the first column for an external key
 			
 							self::DbgTrace("External key $sKeyAttCode, Join on $sLocalKeyField = $sExternalKeyField");
+							if ($oKeyAttDef instanceof AttributeObjectKey)
+							{
+								$sClassAttCode = $oKeyAttDef->Get('class_attcode');
+
+								// Add the condition: `$sTargetAlias`.$sClassAttCode IN (subclasses of $sKeyClass')
+								$oClassListExpr = ListExpression::FromScalars(self::EnumChildClasses($sKeyClass, ENUM_CHILD_CLASSES_ALL));
+								$oClassExpr = new FieldExpression($sClassAttCode, $sTargetAlias);
+								$oClassRestriction = new BinaryExpression($oClassExpr, 'IN', $oClassListExpr);
+								$oBuild->m_oQBExpressions->AddCondition($oClassRestriction);
+							}
 							if ($oKeyAttDef->IsNullAllowed())
 							{
 								$oSelectBase->AddLeftJoin($oSelectExtKey, $sLocalKeyField, $sExternalKeyField, $sExternalKeyTable);

core/oql/oqlquery.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2013 Combodo SARL
+// Copyright (C) 2010-2015 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -388,17 +388,22 @@ class OqlObjectQuery extends OqlQuery
 					throw new OqlNormalizeException('Unknown class in join condition (right expression)', $sSourceQuery, $oRightField->GetParentDetails(), array_keys($aAliases));
 				}
 				$aExtKeys = $oModelReflection->ListAttributes($aAliases[$sFromClass], 'AttributeExternalKey');
-				if (!array_key_exists($sExtKeyAttCode, $aExtKeys))
+				$aObjKeys = $oModelReflection->ListAttributes($aAliases[$sFromClass], 'AttributeObjectKey');
+				$aAllKeys = array_merge($aExtKeys, $aObjKeys);
+				if (!array_key_exists($sExtKeyAttCode, $aAllKeys))
 				{
-					throw new OqlNormalizeException('Unknown external key in join condition (left expression)', $sSourceQuery, $oLeftField->GetNameDetails(), array_keys($aExtKeys));
+					throw new OqlNormalizeException('Unknown key in join condition (left expression)', $sSourceQuery, $oLeftField->GetNameDetails(), array_keys($aAllKeys));
 				}
 
 				if ($sFromClass == $sJoinClassAlias)
 				{
-					$sTargetClass = $oModelReflection->GetAttributeProperty($aAliases[$sFromClass], $sExtKeyAttCode, 'targetclass');
-					if(!$oModelReflection->IsSameFamilyBranch($aAliases[$sToClass], $sTargetClass))
+					if (array_key_exists($sExtKeyAttCode, $aExtKeys)) // Skip that check for object keys
 					{
-						throw new OqlNormalizeException("The joined class ($aAliases[$sFromClass]) is not compatible with the external key, which is pointing to $sTargetClass", $sSourceQuery, $oLeftField->GetNameDetails());
+						$sTargetClass = $oModelReflection->GetAttributeProperty($aAliases[$sFromClass], $sExtKeyAttCode, 'targetclass');
+						if(!$oModelReflection->IsSameFamilyBranch($aAliases[$sToClass], $sTargetClass))
+						{
+							throw new OqlNormalizeException("The joined class ($aAliases[$sFromClass]) is not compatible with the external key, which is pointing to $sTargetClass", $sSourceQuery, $oLeftField->GetNameDetails());
+						}
 					}
 				}
 				else
@@ -434,10 +439,13 @@ class OqlObjectQuery extends OqlQuery
 						$iOperatorCode = TREE_OPERATOR_NOT_ABOVE_STRICT;
 						break;
 					}
-					$sTargetClass = $oModelReflection->GetAttributeProperty($aAliases[$sFromClass], $sExtKeyAttCode, 'targetclass');
-					if(!$oModelReflection->IsSameFamilyBranch($aAliases[$sToClass], $sTargetClass))
+					if (array_key_exists($sExtKeyAttCode, $aExtKeys)) // Skip that check for object keys
 					{
-						throw new OqlNormalizeException("The joined class ($aAliases[$sToClass]) is not compatible with the external key, which is pointing to $sTargetClass", $sSourceQuery, $oLeftField->GetNameDetails());
+						$sTargetClass = $oModelReflection->GetAttributeProperty($aAliases[$sFromClass], $sExtKeyAttCode, 'targetclass');
+						if(!$oModelReflection->IsSameFamilyBranch($aAliases[$sToClass], $sTargetClass))
+						{
+							throw new OqlNormalizeException("The joined class ($aAliases[$sToClass]) is not compatible with the external key, which is pointing to $sTargetClass", $sSourceQuery, $oLeftField->GetNameDetails());
+						}
 					}
 					$aAttList = $oModelReflection->ListAttributes($aAliases[$sFromClass]);
 					$sAttType = $aAttList[$sExtKeyAttCode];

datamodels/1.x/itop-attachments/datamodel.itop-attachments.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.0">
+<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2">
   <classes>
     <class id="Attachment" _delta="define">
       <parent>DBObject</parent>
@@ -64,10 +64,10 @@
           <default_value/>
           <is_null_allowed>false</is_null_allowed>
         </field>
-        <field id="item_id" xsi:type="AttributeString">
+        <field id="item_id" xsi:type="AttributeObjectKey">
           <sql>item_id</sql>
-          <default_value/>
           <is_null_allowed>true</is_null_allowed>
+          <class_attcode>item_class</class_attcode>
         </field>
         <field id="item_org_id" xsi:type="AttributeInteger">
           <sql>item_org_id</sql>

datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.1">
+<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2">
   <classes>
     <class id="Attachment" _delta="define">
       <parent>DBObject</parent>
@@ -69,10 +69,10 @@
           <default_value/>
           <is_null_allowed>false</is_null_allowed>
         </field>
-        <field id="item_id" xsi:type="AttributeString">
+        <field id="item_id" xsi:type="AttributeObjectKey">
           <sql>item_id</sql>
-          <default_value/>
           <is_null_allowed>true</is_null_allowed>
+          <class_attcode>item_class</class_attcode>
         </field>
         <field id="item_org_id" xsi:type="AttributeInteger">
           <sql>item_org_id</sql>

setup/compiler.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2011-2013 Combodo SARL
+// Copyright (C) 2011-2015 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -841,9 +841,9 @@ EOF;
 		
 				if ($sAttType == 'AttributeLinkedSetIndirect')
 				{
-					$aParameters['linked_class'] = $this->GetMandatoryPropString($oField, 'linked_class', '');
-					$aParameters['ext_key_to_me'] = $this->GetMandatoryPropString($oField, 'ext_key_to_me', '');
-					$aParameters['ext_key_to_remote'] = $this->GetMandatoryPropString($oField, 'ext_key_to_remote', '');
+					$aParameters['linked_class'] = $this->GetMandatoryPropString($oField, 'linked_class');
+					$aParameters['ext_key_to_me'] = $this->GetMandatoryPropString($oField, 'ext_key_to_me');
+					$aParameters['ext_key_to_remote'] = $this->GetMandatoryPropString($oField, 'ext_key_to_remote');
 					$aParameters['allowed_values'] = 'null';
 					$aParameters['count_min'] = $this->GetPropNumber($oField, 'count_min', 0);
 					$aParameters['count_max'] = $this->GetPropNumber($oField, 'count_max', 0);
@@ -852,8 +852,8 @@ EOF;
 				}
 				elseif ($sAttType == 'AttributeLinkedSet')
 				{
-					$aParameters['linked_class'] = $this->GetMandatoryPropString($oField, 'linked_class', '');
-					$aParameters['ext_key_to_me'] = $this->GetMandatoryPropString($oField, 'ext_key_to_me', '');
+					$aParameters['linked_class'] = $this->GetMandatoryPropString($oField, 'linked_class');
+					$aParameters['ext_key_to_me'] = $this->GetMandatoryPropString($oField, 'ext_key_to_me');
 					$aParameters['count_min'] = $this->GetPropNumber($oField, 'count_min', 0);
 					$aParameters['count_max'] = $this->GetPropNumber($oField, 'count_max', 0);
 					$sEditMode = $oField->GetChildText('edit_mode');
@@ -885,7 +885,7 @@ EOF;
 					{
 						$aParameters['allowed_values'] = 'null'; // or "new ValueSetObjects('SELECT xxxx')"
 					}
-					$aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql', '');
+					$aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql');
 					$aParameters['is_null_allowed'] = $this->GetPropBoolean($oField, 'is_null_allowed', false);
 					$aParameters['on_target_delete'] = $oField->GetChildText('on_target_delete');
 					$aParameters['depends_on'] = $sDependencies;
@@ -894,6 +894,14 @@ EOF;
 					$aParameters['allow_target_creation'] = $this->GetPropBoolean($oField, 'allow_target_creation');
 					$aParameters['display_style'] = $this->GetPropString($oField, 'display_style', 'select');
 				}
+				elseif ($sAttType == 'AttributeObjectKey')
+				{
+					$aParameters['class_attcode'] = $this->GetMandatoryPropString($oField, 'class_attcode');
+					$aParameters['allowed_values'] = 'null';
+					$aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql');
+					$aParameters['is_null_allowed'] = $this->GetPropBoolean($oField, 'is_null_allowed', false);
+					$aParameters['depends_on'] = $sDependencies;
+				}
 				elseif ($sAttType == 'AttributeHierarchicalKey')
 				{
 					if ($sOql = $oField->GetChildText('filter'))
@@ -905,7 +913,7 @@ EOF;
 					{
 						$aParameters['allowed_values'] = 'null'; // or "new ValueSetObjects('SELECT xxxx')"
 					}
-					$aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql', '');
+					$aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql');
 					$aParameters['is_null_allowed'] = $this->GetPropBoolean($oField, 'is_null_allowed', false);
 					$aParameters['on_target_delete'] = $oField->GetChildText('on_target_delete');
 					$aParameters['depends_on'] = $sDependencies;
@@ -1019,7 +1027,7 @@ EOF;
 				else
 				{
 					$aParameters['allowed_values'] = 'null'; // or "new ValueSetEnum('SELECT xxxx')"
-					$aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql', '');
+					$aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql');
 					$aParameters['default_value'] = $this->GetPropString($oField, 'default_value', '');
 					$aParameters['is_null_allowed'] = $this->GetPropBoolean($oField, 'is_null_allowed', false);
 					$aParameters['depends_on'] = $sDependencies;

setup/itopdesignformat.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2014 Combodo SARL
+// Copyright (C) 2014-2015 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -35,7 +35,7 @@
  * }
  */
  
-define('ITOP_DESIGN_LATEST_VERSION', '1.1');
+define('ITOP_DESIGN_LATEST_VERSION', '1.2');
  
 class iTopDesignFormat
 {
@@ -49,6 +49,12 @@ class iTopDesignFormat
 		'1.1' => array(
 			'previous' => '1.0',
 			'go_to_previous' => 'From11To10',
+			'next' => '1.2',
+			'go_to_next' => 'From11To12',
+		),
+		'1.2' => array(
+			'previous' => '1.1',
+			'go_to_previous' => 'From12To11',
 			'next' => null,
 			'go_to_next' => null,
 		),
@@ -363,6 +369,31 @@ class iTopDesignFormat
 	}
 
 	/**
+	 * Upgrade the format from version 1.1 to 1.2
+	 * @return void (Errors are logged)
+	 */
+	protected function From11To12($oFactory)
+	{
+		// Do nothing
+	}
+
+	/**
+	 * Downgrade the format from version 1.2 to 1.1
+	 * @return void (Errors are logged)
+	 */
+	protected function From12To11($oFactory)
+	{
+		$oXPath = new DOMXPath($this->oDocument);
+		// Transform ObjectKey attributes into Integer
+		$oNodeList = $oXPath->query("/itop_design/classes//class/fields/field[@xsi:type='AttributeObjectKey']");
+		foreach ($oNodeList as $oNode)
+		{
+			$oNode->setAttribute('xsi:type', 'AttributeInteger');
+			// The property class_attcode is left there (doing no harm)
+		}
+	}
+
+	/**
 	 * Delete a node from the DOM and make sure to also remove the immediately following line break (DOMText), if any.
 	 * This prevents generating empty lines in the middle of the XML
 	 * @param DOMNode $oNode

test/testlist.inc.php

@@ -367,6 +367,11 @@ class TestOQLNormalization extends TestBizModel
 			'SELECT p, l FROM Person AS p JOIN Location AS l ON p.location_id = l.id' => true,
 			'SELECT foo FROM Person AS p JOIN Location AS l ON p.location_id = l.id' => false,
 			'SELECT p, foo FROM Person AS p JOIN Location AS l ON p.location_id = l.id' => false,
+
+			// Joins based on AttributeObjectKey
+			//
+			'SELECT Attachment AS a JOIN UserRequest AS r ON a.item_id = r.id' => true,
+			'SELECT UserRequest AS r JOIN Attachment AS a ON a.item_id = r.id' => true,
 		);
 
 		$iErrors = 0;