N°454 - Check data validity during CSV import

* Added additional checks for external keys (including hierarchical ones)

SVN:trunk[4999]
This commit is contained in:
Eric Espié
2017-10-10 10:00:05 +00:00
parent ee53c3a71e
commit d504fb209f
3 changed files with 55 additions and 38 deletions

View File

@@ -1076,7 +1076,7 @@ class AttributeLinkedSet extends AttributeDefinition
return '<ul><li>'.implode("</li><li>", $aNames).'</li></ul>';
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;
}
/**

View File

@@ -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)
{

View File

@@ -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]";
}
}
}
}