From 444d9e36c6c81dd8b10f186e3cf7478804c68eaa Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 19 Mar 2015 12:50:15 +0000 Subject: [PATCH] OQL enhancement: allow JOIN on a objclass/objkey pair of attributes (requires benchmarking) SVN:trunk[3506] --- core/attributedef.class.inc.php | 85 +++++++++++++++---- core/cmdbchangeop.class.inc.php | 2 +- core/metamodel.class.php | 23 ++++- core/oql/oqlquery.class.inc.php | 26 ++++-- .../datamodel.itop-attachments.xml | 6 +- .../datamodel.itop-attachments.xml | 6 +- setup/compiler.class.inc.php | 26 ++++-- setup/itopdesignformat.class.inc.php | 35 +++++++- test/testlist.inc.php | 5 ++ 9 files changed, 171 insertions(+), 43 deletions(-) diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index cfa71d118..354d8464d 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -1,5 +1,5 @@ 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; } @@ -1197,6 +1197,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 * diff --git a/core/cmdbchangeop.class.inc.php b/core/cmdbchangeop.class.inc.php index 2d72dbdf8..61359e836 100644 --- a/core/cmdbchangeop.class.inc.php +++ b/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 diff --git a/core/metamodel.class.php b/core/metamodel.class.php index 919b08ddc..f17b554fb 100644 --- a/core/metamodel.class.php +++ b/core/metamodel.class.php @@ -1,5 +1,5 @@ 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); diff --git a/core/oql/oqlquery.class.inc.php b/core/oql/oqlquery.class.inc.php index 168d60388..946c75724 100644 --- a/core/oql/oqlquery.class.inc.php +++ b/core/oql/oqlquery.class.inc.php @@ -1,5 +1,5 @@ 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]; diff --git a/datamodels/1.x/itop-attachments/datamodel.itop-attachments.xml b/datamodels/1.x/itop-attachments/datamodel.itop-attachments.xml index 5330a8170..810ff5b13 100644 --- a/datamodels/1.x/itop-attachments/datamodel.itop-attachments.xml +++ b/datamodels/1.x/itop-attachments/datamodel.itop-attachments.xml @@ -1,5 +1,5 @@ - + DBObject @@ -64,10 +64,10 @@ false - + item_id - true + item_class item_org_id diff --git a/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml b/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml index b282b2cf8..fb0b98d20 100755 --- a/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml +++ b/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml @@ -1,5 +1,5 @@ - + DBObject @@ -69,10 +69,10 @@ false - + item_id - true + item_class item_org_id diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index 0d0549284..cc8326353 100644 --- a/setup/compiler.class.inc.php +++ b/setup/compiler.class.inc.php @@ -1,5 +1,5 @@ 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; diff --git a/setup/itopdesignformat.class.inc.php b/setup/itopdesignformat.class.inc.php index cf725f49b..3410a56c5 100644 --- a/setup/itopdesignformat.class.inc.php +++ b/setup/itopdesignformat.class.inc.php @@ -1,5 +1,5 @@ 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, ), @@ -362,6 +368,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 diff --git a/test/testlist.inc.php b/test/testlist.inc.php index 25fd5a71a..29e62d238 100644 --- a/test/testlist.inc.php +++ b/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;