From d504fb209fd608380e15dd98483c9a3d83fce61b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Espi=C3=A9?= Date: Tue, 10 Oct 2017 10:00:05 +0000 Subject: [PATCH] =?UTF-8?q?N=C2=B0454=20-=20Check=20data=20validity=20duri?= =?UTF-8?q?ng=20CSV=20import=20*=20Added=20additional=20checks=20for=20ext?= =?UTF-8?q?ernal=20keys=20(including=20hierarchical=20ones)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SVN:trunk[4999] --- core/attributedef.class.inc.php | 52 +++++++++++++++++++++------------ core/bulkchange.class.inc.php | 28 +++++++++--------- core/dbobject.class.php | 13 +++++---- 3 files changed, 55 insertions(+), 38 deletions(-) diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index d0a1fa0f9..54000de4e 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -1076,7 +1076,7 @@ class AttributeLinkedSet extends AttributeDefinition return ''; 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($oHostObject)); } } @@ -2990,7 +2990,6 @@ class AttributeCaseLog extends AttributeLongText */ public function GetAsPlainText($value, $oHostObj = null) { - $value = $oObj->Get($sAttCode); if ($value instanceOf ormCaseLog) { @@ -4604,6 +4603,11 @@ class AttributeExternalKey extends AttributeDBFieldVoid return $oSet; } + public function GetAllowedValuesAsFilter($aArgs = array(), $sContains = '', $iAdditionalValue = null) + { + return DBObjectSearch::FromOQL($this->GetValuesDef()->GetFilterExpression()); + } + public function GetDeletionPropagationOption() { return $this->Get("on_target_delete"); @@ -4804,18 +4808,12 @@ class AttributeHierarchicalKey extends AttributeExternalKey public function GetAllowedValues($aArgs = array(), $sContains = '') { - if (array_key_exists('this', $aArgs)) + $oFilter = $this->GetHierachicalFilter($aArgs, $sContains); + if ($oFilter) { - // Hierarchical keys have one more constraint: the "parent value" cannot be - // "under" themselves - $iRootId = $aArgs['this']->GetKey(); - if ($iRootId > 0) // ignore objects that do no exist in the database... - { - $oValSetDef = $this->GetValuesDef(); - $sClass = $this->m_sTargetClass; - $oFilter = DBObjectSearch::FromOQL("SELECT $sClass AS node JOIN $sClass AS root ON node.".$this->GetCode()." NOT BELOW root.id WHERE root.id = $iRootId"); - $oValSetDef->AddCondition($oFilter); - } + $oValSetDef = $this->GetValuesDef(); + $oValSetDef->AddCondition($oFilter); + return $oValSetDef->GetValues($aArgs, $sContains); } else { @@ -4826,6 +4824,27 @@ class AttributeHierarchicalKey extends AttributeExternalKey public function GetAllowedValuesAsObjectSet($aArgs = array(), $sContains = '', $iAdditionalValue = null) { $oValSetDef = $this->GetValuesDef(); + $oFilter = $this->GetHierachicalFilter($aArgs, $sContains, $iAdditionalValue); + if ($oFilter) + { + $oValSetDef->AddCondition($oFilter); + } + $oSet = $oValSetDef->ToObjectSet($aArgs, $sContains, $iAdditionalValue); + return $oSet; + } + + public function GetAllowedValuesAsFilter($aArgs = array(), $sContains = '', $iAdditionalValue = null) + { + $oFilter = $this->GetHierachicalFilter($aArgs, $sContains, $iAdditionalValue); + if ($oFilter) + { + return $oFilter; + } + return parent::GetAllowedValuesAsFilter($aArgs, $sContains, $iAdditionalValue); + } + + private function GetHierachicalFilter($aArgs = array(), $sContains = '', $iAdditionalValue = null) + { if (array_key_exists('this', $aArgs)) { // Hierarchical keys have one more constraint: the "parent value" cannot be @@ -4833,14 +4852,11 @@ class AttributeHierarchicalKey extends AttributeExternalKey $iRootId = $aArgs['this']->GetKey(); if ($iRootId > 0) // ignore objects that do no exist in the database... { - $aValuesSetDef = $this->GetValuesDef(); $sClass = $this->m_sTargetClass; - $oFilter = DBObjectSearch::FromOQL("SELECT $sClass AS node JOIN $sClass AS root ON node.".$this->GetCode()." NOT BELOW root.id WHERE root.id = $iRootId"); - $oValSetDef->AddCondition($oFilter); + return DBObjectSearch::FromOQL("SELECT $sClass AS node JOIN $sClass AS root ON node.".$this->GetCode()." NOT BELOW root.id WHERE root.id = $iRootId"); } } - $oSet = $oValSetDef->ToObjectSet($aArgs, $sContains, $iAdditionalValue); - return $oSet; + return false; } /** diff --git a/core/bulkchange.class.inc.php b/core/bulkchange.class.inc.php index 6e9b918c2..8b568b8e7 100644 --- a/core/bulkchange.class.inc.php +++ b/core/bulkchange.class.inc.php @@ -320,7 +320,7 @@ class BulkChange // Returns true if the CSV data specifies that the external key must be left undefined protected function IsNullExternalKeySpec($aRowData, $sAttCode) { - $oExtKey = MetaModel::GetAttributeDef($this->m_sClass, $sAttCode); + //$oExtKey = MetaModel::GetAttributeDef($this->m_sClass, $sAttCode); foreach ($this->m_aExtKeys[$sAttCode] as $sForeignAttCode => $iCol) { // The foreign attribute is one of our reconciliation key @@ -366,7 +366,9 @@ class BulkChange } else { - $oReconFilter = new DBObjectSearch($oExtKey->GetTargetClass()); + // Check for additional rules + $oReconFilter = $oExtKey->GetAllowedValuesAsFilter(array('this' => $oTargetObj)); + $aCacheKeys = array(); foreach ($aKeyConfig as $sForeignAttCode => $iCol) { @@ -385,7 +387,6 @@ class BulkChange $aResults[$iCol] = new CellStatus_Void($aRowData[$iCol]); } $sCacheKey = implode('_|_', $aCacheKeys); // Unique key for this query... - $iCount = 0; $iForeignKey = null; $sOQL = ''; // TODO: check if *too long* keys can lead to collisions... and skip the cache in such a case... @@ -405,7 +406,7 @@ class BulkChange else { // Cache miss, let's initialize it - $oExtObjects = new CMDBObjectSet($oReconFilter); + $oExtObjects = new CMDBObjectSet($oReconFilter, array(), array('this' => $oTargetObj)); $iCount = $oExtObjects->Count(); if ($iCount == 1) { @@ -533,13 +534,13 @@ class BulkChange { $sCurValue = $oTargetObj->GetAsHTML($sAttCode, $this->m_bLocalizedValues); $sOrigValue = $oTargetObj->GetOriginalAsHTML($sAttCode, $this->m_bLocalizedValues); - $sInput = htmlentities($aRowData[$iCol], ENT_QUOTES, 'UTF-8'); + //$sInput = htmlentities($aRowData[$iCol], ENT_QUOTES, 'UTF-8'); } else { $sCurValue = $oTargetObj->GetAsCSV($sAttCode, $this->m_sReportCsvSep, $this->m_sReportCsvDelimiter, $this->m_bLocalizedValues); $sOrigValue = $oTargetObj->GetOriginalAsCSV($sAttCode, $this->m_sReportCsvSep, $this->m_sReportCsvDelimiter, $this->m_bLocalizedValues); - $sInput = $aRowData[$iCol]; + //$sInput = $aRowData[$iCol]; } if (isset($aErrors[$sAttCode])) { @@ -610,10 +611,6 @@ class BulkChange throw new BulkChangeException('Invalid attribute code', array('class' => get_class($oTargetObj), 'attcode' => $sAttCode)); } $oTargetObj->Set($sAttCode, $value); - if (!array_key_exists($sAttCode, $this->m_aAttList)) - { - // #@# will be out of the reporting... (counted anyway) - } } // Reporting on fields @@ -655,7 +652,7 @@ class BulkChange if (count($aErrors) > 0) { - $sErrors = implode(', ', $aErrors); + //$sErrors = implode(', ', $aErrors); $aResult[$iRow]["__STATUS__"] = new RowStatus_Issue(Dict::S('UI:CSVReport-Row-Issue-Attribute')); return $oTargetObj; } @@ -684,7 +681,7 @@ class BulkChange if ($oChange) { $newID = $oTargetObj->DBInsertTrackedNoReload($oChange); - $aResult[$iRow]["__STATUS__"] = new RowStatus_NewObj($this->m_sClass, $newID); + $aResult[$iRow]["__STATUS__"] = new RowStatus_NewObj(); $aResult[$iRow]["finalclass"] = get_class($oTargetObj); $aResult[$iRow]["id"] = new CellStatus_Void($newID); } @@ -708,7 +705,7 @@ class BulkChange if (count($aErrors) > 0) { - $sErrors = implode(', ', $aErrors); + //$sErrors = implode(', ', $aErrors); $aResult[$iRow]["__STATUS__"] = new RowStatus_Issue(Dict::S('UI:CSVReport-Row-Issue-Attribute')); return; } @@ -749,7 +746,7 @@ class BulkChange if (count($aErrors) > 0) { - $sErrors = implode(', ', $aErrors); + //$sErrors = implode(', ', $aErrors); $aResult[$iRow]["__STATUS__"] = new RowStatus_Issue(Dict::S('UI:CSVReport-Row-Issue-Attribute')); return; } @@ -1176,6 +1173,9 @@ EOF /** * Display the details of an import + * @param iTopWebPage $oPage + * @param $iChange + * @throws Exception */ static function DisplayImportHistoryDetails(iTopWebPage $oPage, $iChange) { diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 28bc4dc1c..466a74cd1 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -1207,13 +1207,14 @@ abstract class DBObject implements iDisplay { return "Target object not found ($sTargetClass::$toCheck)"; } - } - if ($oAtt->IsHierarchicalKey()) - { - // This check cannot be deactivated since otherwise the user may break things by a CSV import of a bulk modify - if ($toCheck == $this->GetKey()) + // Check allowed values + $aValues = $oAtt->GetAllowedValues($this->ToArgsForQuery()); + if (count($aValues) > 0) { - return "An object can not be its own parent in a hierarchy (".$oAtt->Getlabel()." = $toCheck)"; + if (!array_key_exists($toCheck, $aValues)) + { + return "Value not allowed [$toCheck]"; + } } } }