N°454 - Check data validity during CSV import

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

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

core/attributedef.class.inc.php

@@ -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;
 	}
 
 	/**

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

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